-
Notifications
You must be signed in to change notification settings - Fork 37
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
Implement EIP-4788 for Cancun #40
Conversation
mpt_read_leaf: | ||
global mpt_read_leaf: |
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.
The global
label changes on mpt/insert.asm
and mpt/read.asm
are unrelated to this PR, but I got extremely confused when debugging through the Kernel logs when seeing mpt_read_leaf_not_found
, while it actually was, because all these labels are below their global negative counterpart.
Hence I'd think this is better debugging wise if they were also made global, to prevent any confusion.
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.
(partial review)
@@ -41,10 +41,13 @@ global hash_initial_tries: | |||
// stack: trie_data_full_len | |||
%mstore_global_metadata(@GLOBAL_METADATA_TRIE_DATA_SIZE) | |||
|
|||
// If txn_idx == 0, update the beacon_root. |
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.
DQ: where do we check if we are beyond the Cancun HF block?
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.
We don't, neither for Shanghai at the moment, i.e. chains must use the correct prover according to the HF they are currently on.
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.
LGTM
|
||
const HISTORY_BUFFER_LENGTH: (&str, u64) = ("HISTORY_BUFFER_LENGTH", 8191); | ||
|
||
pub const BEACON_ROOTS_CONTRACT_CODE: [u8; 106] = hex!("60618060095f395ff33373fffffffffffffffffffffffffffffffffffffffe14604d57602036146024575f5ffd5b5f35801560495762001fff810690815414603c575f5ffd5b62001fff01545f5260205ff35b5f5ffd5b62001fff42064281555f359062001fff015500"); |
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 is the initcode, not the actual contract bytecode.
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.
Ah shoot right, the bytecode is
0x3373fffffffffffffffffffffffffffffffffffffffe14604d57602036146024575f5ffd5b5f35801560495762001fff810690815414603c575f5ffd5b62001fff01545f5260205ff35b5f5ffd5b62001fff42064281555f359062001fff015500
Will fix it. Thanks @wborgeaud!
Implements EIP-4788 (Beacon block root in the EVM).
Because all integration tests had to be changed to support the same beacon roots contract update, I created a
testing_utils
module to just export a bunch of utility functions instead of duplicating them across test files.closes #33