Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: src-5 implement #233

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

docs: src-5 implement #233

wants to merge 9 commits into from

Conversation

Jemiiah
Copy link
Contributor

@Jemiiah Jemiiah commented Jul 8, 2024

**Issue(s) : #109

@Jemiiah Jemiiah changed the title feat: src-5 implement docs: src-5 implement Jul 8, 2024
Copy link
Contributor

@julio4 julio4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR is empty, please double check that you actually committed your changes

@Jemiiah
Copy link
Contributor Author

Jemiiah commented Jul 10, 2024

my apologies

@Jemiiah
Copy link
Contributor Author

Jemiiah commented Jul 10, 2024

hello @julio could you please review i made another commit

listings/src-5 interface.md Outdated Show resolved Hide resolved
Comment on lines 10 to 11
* Detect if a contract implements SRC-5.
* Detect if a contract implements any given interface.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SRC-5 is an interface, so you can just say

  • Detect if a contract implements any given interface. (including SRC-5)


### SRC-5 offers a standardized method to:

* Identify interfaces.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Identify has the same meaning of detection. Maybe you meant provide a way to generate an unique identifier for each interface.

Comment on lines 18 to 22

trait IMyContract {
fn foo(some: u256) -> felt252;
}
With Cairo 2.0, generic traits can represent a set of interfaces:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

put the code content:

  • in code block with
    ```rust
    ...
    ```
  • include codes from a listing (to ensure that all codes in the book is always compilable)

The function selector in Starknet is the starknet_keccak hash of the function name. The extended function selector, for SRC-5, is the starknet_keccak hash of the function signature:


fn_name(param1_type,param2_type,...)->output_type
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will not be in a listing, so use `fn_name(param1_type,param2_type,...)->output_type`

Comment on lines 103 to 191
``` use openzeppelin::account::interface::ISRC5_ID;
use openzeppelin::introspection::src5::SRC5Component;
use starknet::contract::ContractAddress;
use starknet::syscalls::call_contract_syscall;
use starknet::alloc::arrays::ArrayTrait;
use starknet::core::keccak::starknet_keccak;

// Define ISRC5 interface
trait ISRC5 {
fn supports_interface(interface_id: felt252) -> bool;
}

// Define example interfaces
trait IExample1 {
fn example_function1(param1: felt252) -> felt252;
}

trait IExample2 {
fn example_function2(param1: felt252, param2: felt252) -> felt252;
}

// Storage structure
struct Storage {
public_key: felt252,
src5: SRC5Component::Storage,
}

// Event definitions
enum Event {
AccountCreated: felt252,
SRC5Event: SRC5Component::Event,
}

// Contract implementation
@contract
namespace ExampleContract {
struct State {
storage: Storage,
}

// Constructor
#[constructor]
fn constructor(ref state: State, public_key: felt252) {
state.storage.public_key = public_key;
emit(Event::AccountCreated(public_key));
state.storage.src5.register_interface(ISRC5_ID);
}

// Implementation of ISRC5
#[external]
fn supports_interface(ref state: State, interface_id: felt252) -> bool {
state.storage.src5.supports_interface(interface_id)
}

// Implementation of IExample1
#[external]
fn example_function1(param1: felt252) -> felt252 {
// Your implementation here
return starknet_keccak(param1.to_bytes());
}

// Implementation of IExample2
#[external]
fn example_function2(param1: felt252, param2: felt252) -> felt252 {
// Your implementation here
return param1 + param2;
}

// Additional functions or internal logic can be added here
}

// ISRC5 trait implementation
impl ISRC5 for State {
fn supports_interface(ref self, interface_id: felt252) -> bool {
self.storage.src5.supports_interface(interface_id)
}
}

// Interface registration during contract deployment
fn main() {
let state = ExampleContract::State {
storage: Storage {
public_key: felt252::default(),
src5: SRC5Component::Storage::default(),
},
};
ExampleContract::constructor(state, 12345.into());
}
```
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be in a listing



fn_name(param1_type,param2_type,...)->output_type
For example, for a function with zero parameters and no return value:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Be sure to double check whitespaces and typos.

interface_id ^= function_id
print('IAccount ID:', hex(interface_id))

compute_interface_id()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Jemiiah
Copy link
Contributor Author

Jemiiah commented Jul 15, 2024

@julio4 made all necessary corrections

Copy link
Contributor

@julio4 julio4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might have been a bit confused with the separation between listings and src:

  • listings contains only scarb projects, with the actual contracts sources
  • src contains all the .md files, and use mdbook include directive to add the source to each examples from listings

We do that way so we can compile, tests, lint, exports all examples and only add documentation "on top of it".

So in your case you should keep all the markdown documentation inside src, and only add a listing as a scarb project. Take a look at other examples if you're not sure how to do it.

@Jemiiah
Copy link
Contributor Author

Jemiiah commented Jul 19, 2024

would implement the corrections today and make the push

Copy link
Contributor

@julio4 julio4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Lots of small changes requested, and also I think it's best to simplify the example. But overall looking great, thank you!
Be sure to review and resolve all change requests, it makes reviewing faster!

listings/src5_interface/src/lib.cairo Outdated Show resolved Hide resolved
Comment on lines +3 to +4
#[cfg(test)]
mod tests;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can add test module directly inside main module behind a test anchor

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You moved test functions so you can remove tests module definition here

Comment on lines 5 to 7



Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whitespace

@@ -0,0 +1,113 @@
## SRC-5: A comprhensive Overview

SRC-5 is a standard for smart contract interface introspection in Starknet, inspired by the Ethereum ERC-165 standard. It provides a method for contracts to publish and detect the interfaces they implement, ensuring standardized interaction. It provides a method for contracts to publish and detect the interfaces they implement, ensuring standardized interaction. You can find more information and details refer to the [SRC-5 specification](https://github.com/starknet-io/SNIPs/blob/main/SNIPS/snip-5.md#simple-summary) and the [Ethereum ERC-165 standard](https://eips.ethereum.org/EIPS/eip-165).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove sentence duplications, add some breaklines, fix multiple whitespaces, move link for ERC-165 directly when you mention it first.

@@ -0,0 +1,113 @@
## SRC-5: A comprhensive Overview
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comprhensive typo

src/src5 implementation.md Show resolved Hide resolved

#### Detecting Any Given Interface
* Confirm if the contract implements SRC-5.
* If confirmed, call supports_interface(interface_id) to check for specific interfaces.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use ``

* If confirmed, call supports_interface(interface_id) to check for specific interfaces.
* If not, manually inspect the contract methods.

Below shows a contract implementing the ```ISRC5``` trait :
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add which interface it exposes as well
"Below shows a contract implementing SRC5 to expose the xxx interface:"

Comment on lines 56 to 73
from starkware.starknet.public.abi import starknet_keccak

signatures = [
'supports_interface(felt252)->E((),())',
'is_valid_signature(felt252,Array<felt252>)->E((),())',
'__execute__(Array<(ContractAddress,felt252,Array<felt252>)>)->Array<(@Array<felt252>)>',
'__validate__(Array<(ContractAddress,felt252,Array<felt252>)>)->felt252',
'__validate_declare__(felt252)->felt252'
]

def compute_interface_id():
interface_id = 0x0
for sig in signatures:
function_id = starknet_keccak(sig.encode())
interface_id ^= function_id
print('IAccount ID:', hex(interface_id))

compute_interface_id()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, it might be a good idea to move this code snippet inside the listing directory in a python file

listings/src5_interface/src/src5_interface.cairo Outdated Show resolved Hide resolved
@Jemiiah
Copy link
Contributor Author

Jemiiah commented Jul 24, 2024

@julio4 please review ser

Copy link
Contributor

@julio4 julio4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please review and resolve all change requests before requesting and run the verification script as well

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrong format, please try to run the code before pushing it

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not valid cairo syntax, please refer to official documentation, or our counter example, and don't use AI generated content.

Comment on lines +3 to +4
#[cfg(test)]
mod tests;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You moved test functions so you can remove tests module definition here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants