-
Notifications
You must be signed in to change notification settings - Fork 20
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
stronger types for: address, u256 and bytes #63
Conversation
d471727
to
168c3f9
Compare
aced6d8
to
1b22cc9
Compare
@@ -258,7 +258,9 @@ pub fn rollup_read_advance_state_request( | |||
app_contract: Default::default(), | |||
block_number: 0, | |||
block_timestamp: 0, | |||
prev_randao: Default::default(), | |||
prev_randao: { cmt_u256_t { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prev_randao: { cmt_u256_t { | |
prev_randao: { cmt_abi_u256_t { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we use EVM instead of ABI? Or even EVMABI... I don't know how this will progress and if different ABIs will show up. Not a big deal. We can keep ABI and change it later when something else finally appears.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This library was going to be called evm_...
originally, so evm_abi_...
in this case. But you know how naming things go...
We can keep ABI and change it later when something else finally appears.
ok
|
||
/** EVM u256 in big endian format */ | ||
typedef struct cmt_abi_u256 { | ||
uint8_t data[CMT_WORD_LENGTH]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Chan we change CMT_WORD_LENGTH to CMT_ABI_U256_LENGTH here? And elsewhere, can we change it to CMT_HASH_LENGTH or CMT_KECCAC_LENGTH wherever that is the thing with the size?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Chan we change CMT_WORD_LENGTH to CMT_ABI_U256_LENGTH here?
sure
And elsewhere, can we change it to CMT_HASH_LENGTH or CMT_KECCAC_LENGTH wherever that is the thing with the size?
There is a CMT_KECCAK_LENGTH
already in keccak.h
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a lot neater.
This allows users to build and link without a release installation by preserving the header files relative path and `-I<cmt-root-dir>/include` when building.
1b22cc9
to
ce96c95
Compare
a26da7d
to
6a4629e
Compare
96c4eed
to
8188e84
Compare
No description provided.