-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
feat: implement EIP-2935 #7818
feat: implement EIP-2935 #7818
Conversation
6fa24c1
to
921220f
Compare
General direction and implementation is ready for review. I will add tests for this, and merging is also blocked by #7765 |
The test currently fails because the EIP mandates the account is empty (so far), and it is supposed to be exempt from EIP-158, but revm does not handle this right now. That is, because the account has no code, no balance, and no nonce, it is not inserted into the database here: https://github.com/bluealloy/revm/blob/1ca3d39f6a9e9778f8eb0fcb74fe529345a531b4/crates/revm/src/db/states/cache.rs#L140-L151 |
The tests no longer fail since I've added a hack to give the system address a balance of 1 wei, which we are probably going to end up doing for devnet 0 in genesis, since the account would be cleared from state otherwise I've marked this with a |
1de171f
to
bccadfc
Compare
bccadfc
to
60131b9
Compare
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.
On comparison with @lightclient ethereum/go-ethereum@master...lightclient:go-ethereum:prague-devnet-0#diff-7ee5f952a6b64b5c8a5db54348910f6e901dcafce4afa172720c40713fe2e78bR85-R87 EIP-2935 this looks good to me.
All the blockers are merged and the PR is up to date with |
// HACK(onbjerg): This is a temporary workaround to make sure the account does not get cleared | ||
// by state clearing later. This balance will likely be present in the devnet 0 genesis file | ||
// until the EIP itself is fixed. | ||
account.info.balance = U256::from(1); |
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.
Does this create a genesis hash mismatch with other clients? Or any state root conflicts bc we have diff balance vs others in the state?
/// Applies the pre-block call to the EIP-4788 beacon block root contract, using the given block, | ||
/// todo: temporary move over of constants from revm until we've migrated to the latest version | ||
pub const HISTORY_SERVE_WINDOW: usize = 8192; | ||
|
||
/// todo: temporary move over of constants from revm until we've migrated to the latest version | ||
pub const HISTORY_STORAGE_ADDRESS: Address = address!("25a219378dad9b3503c8268c9ca836a52427a4fb"); |
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.
Parameter | Value |
---|---|
FORK_TIMESTAMP | TBD |
HISTORY_STORAGE_ADDRESS | 0x25a219378dad9b3503c8268c9ca836a52427a4fb |
HISTORY_SERVE_WINDOW | 8192 |
BLOCKHASH_OLD_WINDOW | 256 |
from the https://eips.ethereum.org/EIPS/eip-2935, seems OK?
ethereum/EIPs#8577 Per this getting merged we'll need to remove the backfill logic for devnet-1. |
Co-authored-by: Georgios Konstantopoulos <me@gakonst.com>
Implements the pre-block state changes needed to put historical blockhashes into a new system contract as defined by EIP-2935. This also includes special handling for the transition block, where historical hashes are backfilled in.
The changes to the opcode itself can be found in the companion PR.
EIP: https://eips.ethereum.org/EIPS/eip-2935
Companion PR: bluealloy/revm#1354
Ref #7363
Blocked by #7765