-
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
feat(levm): add support for EIP-7840 and add EVMConfig
to LEVM.
#1860
Conversation
|
Benchmark Results ComparisonPR ResultsBenchmark Results: Factorial
Benchmark Results: Factorial - Recursive
Benchmark Results: Fibonacci
Benchmark Results: ManyHashes
Benchmark Results: BubbleSort
Benchmark Results: ERC20 - Transfer
Benchmark Results: ERC20 - Mint
Benchmark Results: ERC20 - Approval
Main ResultsBenchmark Results: Factorial
Benchmark Results: Factorial - Recursive
Benchmark Results: Fibonacci
Benchmark Results: ManyHashes
Benchmark Results: BubbleSort
Benchmark Results: ERC20 - Transfer
Benchmark Results: ERC20 - Mint
Benchmark Results: ERC20 - Approval
|
|
2d94528
to
8c701a3
Compare
|
ChainConfig
to LEVM.EVMConfig
to LEVM.
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. Left some comments about the documentation
Co-authored-by: Ivan Litteri <[email protected]>
Co-authored-by: Ivan Litteri <[email protected]>
Co-authored-by: Ivan Litteri <[email protected]>
Co-authored-by: Ivan Litteri <[email protected]>
Co-authored-by: Ivan Litteri <[email protected]>
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.
👍
|
pub const TARGET_BLOB_GAS_PER_BLOCK: u64 = 393216; // TARGET_BLOB_NUMBER_PER_BLOCK * GAS_PER_BLOB | ||
pub const TARGET_BLOB_GAS_PER_BLOCK_PECTRA: u64 = 786432; // TARGET_BLOB_NUMBER_PER_BLOCK * GAS_PER_BLOB | ||
|
||
pub const MIN_BASE_FEE_PER_BLOB_GAS: U256 = U256::one(); | ||
|
||
// WARNING: Do _not_ use the BLOB_BASE_FEE_UPDATE_FRACTION_* family of | ||
// constants as is. Use the `get_blob_base_fee_update_fraction_value` | ||
// function instead | ||
pub const BLOB_BASE_FEE_UPDATE_FRACTION: U256 = U256([3338477, 0, 0, 0]); | ||
pub const BLOB_BASE_FEE_UPDATE_FRACTION_PRAGUE: U256 = U256([5007716, 0, 0, 0]); // Defined in [EIP-7691](https://eips.ethereum.org/EIPS/eip-7691) | ||
pub const BLOB_BASE_FEE_UPDATE_FRACTION: u64 = 3338477; | ||
pub const BLOB_BASE_FEE_UPDATE_FRACTION_PRAGUE: u64 = 5007716; // Defined in [EIP-7691](https://eips.ethereum.org/EIPS/eip-7691) | ||
|
||
// WARNING: Do _not_ use the MAX_BLOB_COUNT_* family of constants as | ||
// is. Use the `max_blobs_per_block` function instead | ||
pub const MAX_BLOB_COUNT: usize = 6; | ||
pub const MAX_BLOB_COUNT_ELECTRA: usize = 9; | ||
pub const MAX_BLOB_COUNT: u64 = 6; | ||
pub const MAX_BLOB_COUNT_ELECTRA: u64 = 9; |
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.
Why these changes from U256 to u64?
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.
Those functions are now only used int the EVMConfig
struct. That struct has a ForkBlobSchedule
inside it, which uses u64 for all it fields (Note: ForkBlobSchedule
is used by the L1).
So, it the constants were left as U256 they were needed to be cast down; which returned a Result
, whose Err
case needed to be handled. This made it difficult to create a config since it could "technically" fail, even though it was casting down a constant.
I decided to change them to u64 in order to make it easier to use ForkBlobSchedule
. Furthermore, casting a u64 to U256 doesn't return a Result; so there was no Err
to handle.
pub const fn max_blobs_per_block(fork: Fork) -> usize { | ||
match fork { | ||
Fork::Prague => MAX_BLOB_COUNT_ELECTRA, | ||
Fork::PragueEof => MAX_BLOB_COUNT_ELECTRA, | ||
_ => MAX_BLOB_COUNT, | ||
} | ||
} | ||
|
||
/// According to EIP-7691 | ||
/// (https://eips.ethereum.org/EIPS/eip-7691#specification): | ||
/// | ||
/// "These changes imply that get_base_fee_per_blob_gas and | ||
/// calc_excess_blob_gas functions defined in EIP-4844 use the new | ||
/// values for the first block of the fork (and for all subsequent | ||
/// blocks)." | ||
|
||
pub const fn get_blob_base_fee_update_fraction_value(fork: Fork) -> U256 { | ||
match fork { | ||
Fork::Prague => BLOB_BASE_FEE_UPDATE_FRACTION_PRAGUE, | ||
Fork::PragueEof => BLOB_BASE_FEE_UPDATE_FRACTION_PRAGUE, | ||
_ => BLOB_BASE_FEE_UPDATE_FRACTION, | ||
} | ||
} | ||
|
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 don't fully understand why we removed these functions
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.
They were moved to the EVMConfig
struct here: https://github.com/lambdaclass/ethrex/pull/1860/files#diff-e20d30a6efb08393eee586d44f3bc4a8df6cbd53ec8ea5bb4d5def88c9487de1R111-R133
} | ||
} | ||
|
||
/// According to [EIP-7691](https://eips.ethereum.org/EIPS/eip-7691#specification): |
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.
Great work documenting the corresponding EIPs 👍
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.
Thanks 😊
|
ab0a37e
to
04ab00c
Compare
Motivation
PR #1810 introduced support for EIP-7840 in L1. That EIP introduced the notion that certain blobhash related constants could be modified by Genesis file (for more information see commit: 8610b4c).
Description
This introduces a new struct called
EVMConfig
to LEVM. It only holds the currentFork
and the desiredForkBlobSchedule
.In most cases, the default
ForkBlobSchedule
will want to be used (for that purpose, theEVMConfig::canonical_values(fork: Fork) -> ForkBlobSchedule
functions exists). This is unless you want to use a customForkBlobSchedule
like described in EIP-7840.Closes #1813