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

Update to SNIP12 version 1 #2

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

Update to SNIP12 version 1 #2

wants to merge 16 commits into from

Conversation

gaetbout
Copy link
Contributor

No description provided.

Copy link

@Leonard-Pat Leonard-Pat left a comment

Choose a reason for hiding this comment

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

Very nice I think it would be useful to have:
Struct with enum
Struct within struct
ikd if thats too much work :D

}

function getTypedDataHash(myStruct: StructWithString, chainId: string, owner: bigint): string {
console.log(JSON.stringify(getTypedData(myStruct, chainId)));

Choose a reason for hiding this comment

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

should this be here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it is to be able to copy paste it as a CONST in the .cairo file, tbh I should prob print the escaped version 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually you were right.
Although the idea of printing the escaped version is handy, it is not straigtforward...
Removed them all :D

@gaetbout
Copy link
Contributor Author

struct within struct is u256 and SNIP-12 doesn't support enum, but still maybe there should be an example indeed 🤔

@gaetbout
Copy link
Contributor Author

Can someone double check about version: shortString.encodeShortString("1").
I think I made a mistake and it shouldn't be encoded. Can someone confirm?

use off_chain_signature::interfaces::{IOffChainMessageHash, IStructHash, v1::StarknetDomain};

const STRUCT_WITH_U256_TYPE_HASH: felt252 =
selector!("\"StructWithString\"(\"some_felt252\":\"felt\",\"some_string\":\"string\")");
Copy link

Choose a reason for hiding this comment

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

i would user human readable names to encourage builders to use readable data to sign, For instance "Some String"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated all V1 files, as this was not encouraged in V0 I left them as is.
Should I update V0 to use CamelCase naming in the interface for the ts files?

interface SimpleStruct {
  some_felt252: string;
  some_u128: string;
}
// Will become
interface SimpleStruct {
  someFelt252: string;
  someU128: string;
}

?

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