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

inner representation of ChainId shouldn't be String #196

Open
tdelabro opened this issue Mar 19, 2024 · 0 comments
Open

inner representation of ChainId shouldn't be String #196

tdelabro opened this issue Mar 19, 2024 · 0 comments

Comments

@tdelabro
Copy link
Contributor

tdelabro commented Mar 19, 2024

https://github.com/starkware-libs/starknet-api/blob/main/src/core.rs#L20-L21

here ChainId is represented as a String

But in the specs the ChainId is used to compute the tx hash of all the transactions. This means it should fit in a Felt meaning it should be at most a ShortString of max size 31 characters.

I recommend we store it as a Felt for its internal representation, as it will simplify its usage for tx hash computation, and implement Display on it in a way that returns the short string representation.

With the current inner repr as String and no new method to make sure the max length is respected, anybody can write code that will break the protocol.

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

No branches or pull requests

1 participant