chore(l1): sync EIP-8037 with EIPs PR 11573#6546
Conversation
- pin CPSB at 1174 (drop dead dynamic-formula consts and update doc) - rename STATE_BYTES_PER_AUTH_ONLY -> STATE_BYTES_PER_AUTH_BASE; drop STATE_BYTES_PER_AUTH_TOTAL (compute as NEW_ACCOUNT + AUTH_BASE inline) - add SYSTEM_MAX_SSTORES_PER_CALL=16, SYS_CALL_STATE_GAS_RESERVOIR=601_088, SYS_CALL_GAS_LIMIT_AMSTERDAM=30_601_088; place the extra in state_gas_reservoir for system calls on Amsterdam+ and bypass TX_MAX_GAS_LIMIT for them - replace dynamic-formula CPSB unit tests with a fixed-1174 assertion
Lines of code reportTotal lines added: Detailed view |
Address review on PR #6546. Two issues: 1. prepare_execution read self.tx.gas_limit() to compute the reservoir. For system calls the tx is built from EIP1559Transaction::default(), so tx.gas_limit() = 0 and the reservoir was always 0 — the extra 601k we added to env.gas_limit just sat in gas_remaining as unreserved execution gas instead of being separated into state_gas_reservoir. Read env.gas_limit on the system-call branch (and keep tx.gas_limit() on the user-tx branch so eth_call/simulate paths, where env.gas_limit defaults to the block max, are unaffected). 2. SYSTEM_MAX_SSTORES_PER_CALL lived in gas_cost.rs while the 601_088 reservoir literal lived in constants.rs, decoupled. Move SYSTEM_MAX_SSTORES_PER_CALL to constants.rs and derive SYS_CALL_STATE_GAS_RESERVOIR symbolically so the two stay in lockstep.
Benchmark Results ComparisonNo significant difference was registered for any benchmark run. Detailed ResultsBenchmark Results: BubbleSort
Benchmark Results: ERC20Approval
Benchmark Results: ERC20Mint
Benchmark Results: ERC20Transfer
Benchmark Results: Factorial
Benchmark Results: FactorialRecursive
Benchmark Results: Fibonacci
Benchmark Results: FibonacciRecursive
Benchmark Results: ManyHashes
Benchmark Results: MstoreBench
Benchmark Results: Push
Benchmark Results: SstoreBench_no_opt
|
🤖 Kimi Code ReviewOverall Assessment: The PR correctly implements EIP-8037 (multidimensional gas) updates per the simplified EIP-11573 specification. The logic for system call gas reservoir splitting is sound, and safety checks (saturating arithmetic) are properly used. One import issue needs verification. Issues Found1. Missing Import Check (Potential Compilation Error)
Correctness & Safety Observations2. System Call Gas Logic (Correct)
3. Constant Calculation Verification
4. EIP-7702 Auth Byte Accounting
5. Gas Limit Source Selection
Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt |
🤖 Claude Code ReviewNow I have enough context to write a thorough review. PR Review:
|
Greptile SummaryThis PR syncs the EIP-8037 implementation with the upstream spec rewrite in ethereum/EIPs#11573: CPSB is now a fixed constant (1174), the auth-bytes naming is updated to the additive Confidence Score: 4/5Safe to merge; changes are numerically faithful to the spec and all altered paths are guarded by fork checks. No logic or security issues found. Two P2 style observations: the CPSB literal is duplicated in SYS_CALL_STATE_GAS_RESERVOIR rather than derived from the canonical function, and cost_per_state_byte silently discards its argument. Neither affects correctness today. crates/vm/levm/src/constants.rs — inlined CPSB literal; crates/vm/levm/src/gas_cost.rs — unused parameter
|
| Filename | Overview |
|---|---|
| crates/vm/levm/src/constants.rs | Adds three new constants for Amsterdam system-call gas budgeting; arithmetic is correct (32117416 = 601,088; 30M + 601,088 = 30,601,088) but inlines the CPSB literal rather than deriving it from the canonical function. |
| crates/vm/levm/src/gas_cost.rs | Removes dead dynamic-formula constants and the renamed STATE_BYTES_PER_AUTH_TOTAL/ONLY pair; cost_per_state_byte now unconditionally returns 1174 but silently accepts a _block_gas_limit arg. |
| crates/vm/levm/src/state_diff.rs | Renames constants in bytes(), computes auth_total_bytes additively; result is numerically identical (112 + 23 = 135) to the old STATE_BYTES_PER_AUTH_TOTAL. |
| crates/vm/levm/src/utils.rs | System-call reservoir split logic is correctly gated on is_system_call; pre-consumes exactly SYS_CALL_STATE_GAS_RESERVOIR leaving 30M execution budget; ordinary-tx path unchanged. |
| crates/vm/backends/levm/mod.rs | Correctly selects SYS_CALL_GAS_LIMIT_AMSTERDAM vs SYS_CALL_GAS_LIMIT based on fork; the + TX_BASE_COST padding for intrinsic gas consumption is preserved. |
| test/tests/levm/eip8037_tests.rs | Old #[ignore]-tagged dynamic-formula tests removed; replaced with a single parametric test_cpsb_is_fixed_at_1174 covering the full expected spread of block-gas-limit values. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[generic_system_contract_levm] -->|fork >= Amsterdam| B["sys_call_gas_limit = SYS_CALL_GAS_LIMIT_AMSTERDAM = 30_601_088"]
A -->|fork < Amsterdam| C["sys_call_gas_limit = SYS_CALL_GAS_LIMIT = 30_000_000"]
B --> D["env.gas_limit = sys_call_gas_limit + TX_BASE_COST"]
C --> D
D --> E[prepare_execution]
E --> F{fork >= Amsterdam?}
F -->|No| G[No reservoir split]
F -->|Yes| H{is_system_call?}
H -->|Yes| I["execution_gas = env.gas_limit - total_gas\nreservoir = execution_gas - SYS_CALL_GAS_LIMIT = 601_088\ngas_remaining for execution = 30_000_000"]
H -->|No - user tx| J["regular_gas_budget = TX_MAX_GAS_LIMIT_AMSTERDAM - regular_gas\ngas_left = min(regular_gas_budget, execution_gas)\nreservoir = execution_gas - gas_left"]
I --> K["state_gas_reservoir = reservoir\ngas_remaining reduced by reservoir"]
J --> K
Prompt To Fix All With AI
This is a comment left during a code review.
Path: crates/vm/levm/src/constants.rs
Line: 34-35
Comment:
**Hardcoded CPSB value creates maintenance coupling**
`SYS_CALL_STATE_GAS_RESERVOIR` inlines `1174` directly rather than deriving it through `cost_per_state_byte`. This means a future change to the CPSB value in `gas_cost::cost_per_state_byte` will silently leave `SYS_CALL_STATE_GAS_RESERVOIR` (and therefore `SYS_CALL_GAS_LIMIT_AMSTERDAM`) stale. The comment already flags this risk, but it may be worth at minimum a `debug_assert_eq!(SYS_CALL_STATE_GAS_RESERVOIR, gas_cost::STATE_BYTES_PER_STORAGE_SET * cost_per_state_byte(0) * SYSTEM_MAX_SSTORES_PER_CALL)` in a test to make the divergence loud.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: crates/vm/levm/src/gas_cost.rs
Line: 176-178
Comment:
**Silently-ignored parameter may mislead callers**
The `_block_gas_limit` argument is now permanently unused but still accepted. Any caller that constructs its own `block_gas_limit` value and passes it expecting it to influence the cost will silently get `1174` regardless. Consider making the contract explicit by changing the signature to `cost_per_state_byte() -> u64` (a breaking change at call sites, but honest) or at least adding an inline note to make it clear the call sites don't need to supply a meaningful value.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix(l1): read env.gas_limit for system-c..." | Re-trigger Greptile
🤖 Codex Code ReviewFindings
Otherwise, the fixed-CPSB change and the I couldn’t run the Rust tests in this sandbox because Automated review by OpenAI Codex · gpt-5.4 · custom prompt |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| pub const SYS_CALL_STATE_GAS_RESERVOIR: u64 = | ||
| crate::gas_cost::STATE_BYTES_PER_STORAGE_SET * 1174 * SYSTEM_MAX_SSTORES_PER_CALL; |
There was a problem hiding this comment.
SYS_CALL_STATE_GAS_RESERVOIR inlines 1174 directly rather than deriving it through cost_per_state_byte. This means a future change to the CPSB value in gas_cost::cost_per_state_byte will silently leave SYS_CALL_STATE_GAS_RESERVOIR (and therefore SYS_CALL_GAS_LIMIT_AMSTERDAM) stale. The comment already flags this risk, but it may be worth at minimum a debug_assert_eq!(SYS_CALL_STATE_GAS_RESERVOIR, gas_cost::STATE_BYTES_PER_STORAGE_SET * cost_per_state_byte(0) * SYSTEM_MAX_SSTORES_PER_CALL) in a test to make the divergence loud.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/vm/levm/src/constants.rs
Line: 34-35
Comment:
**Hardcoded CPSB value creates maintenance coupling**
`SYS_CALL_STATE_GAS_RESERVOIR` inlines `1174` directly rather than deriving it through `cost_per_state_byte`. This means a future change to the CPSB value in `gas_cost::cost_per_state_byte` will silently leave `SYS_CALL_STATE_GAS_RESERVOIR` (and therefore `SYS_CALL_GAS_LIMIT_AMSTERDAM`) stale. The comment already flags this risk, but it may be worth at minimum a `debug_assert_eq!(SYS_CALL_STATE_GAS_RESERVOIR, gas_cost::STATE_BYTES_PER_STORAGE_SET * cost_per_state_byte(0) * SYSTEM_MAX_SSTORES_PER_CALL)` in a test to make the divergence loud.
How can I resolve this? If you propose a fix, please make it concise.| pub fn cost_per_state_byte(_block_gas_limit: u64) -> u64 { | ||
| 1174 | ||
| } |
There was a problem hiding this comment.
The _block_gas_limit argument is now permanently unused but still accepted. Any caller that constructs its own block_gas_limit value and passes it expecting it to influence the cost will silently get 1174 regardless. Consider making the contract explicit by changing the signature to cost_per_state_byte() -> u64 (a breaking change at call sites, but honest) or at least adding an inline note to make it clear the call sites don't need to supply a meaningful value.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/vm/levm/src/gas_cost.rs
Line: 176-178
Comment:
**Silently-ignored parameter may mislead callers**
The `_block_gas_limit` argument is now permanently unused but still accepted. Any caller that constructs its own `block_gas_limit` value and passes it expecting it to influence the cost will silently get `1174` regardless. Consider making the contract explicit by changing the signature to `cost_per_state_byte() -> u64` (a breaking change at call sites, but honest) or at least adding an inline note to make it clear the call sites don't need to supply a meaningful value.
How can I resolve this? If you propose a fix, please make it concise.
| /// the block gas limit; PR 11573 collapses that to a single fixed constant, so | ||
| /// `block_gas_limit` is no longer an input. The argument is retained to keep call | ||
| /// sites stable; if a future EIP re-derives CPSB, change the body. | ||
| pub fn cost_per_state_byte(_block_gas_limit: u64) -> u64 { |
There was a problem hiding this comment.
Do we still need this function if the value will be left fixed?
ElFantasma
left a comment
ElFantasma
left a comment
There was a problem hiding this comment.
Just a couple of nit suggestions
| // existing 30M execution margin under the higher state-creation costs. | ||
| // | ||
| // 30_000_000 + 32 * 1174 * 16 = 30_601_088 | ||
| pub const SYS_CALL_GAS_LIMIT_AMSTERDAM: u64 = SYS_CALL_GAS_LIMIT + SYS_CALL_STATE_GAS_RESERVOIR; |
There was a problem hiding this comment.
SYS_CALL_GAS_LIMIT_AMSTERDAM = 30_000_000 + 32 * 1174 * 16 = 30_601_088 checks out against the spec. SYSTEM_MAX_SSTORES_PER_CALL = 16 is correctly anchored to MAX_WITHDRAWAL_REQUESTS_PER_BLOCK (EIP-7002, the loosest bound). The hardcoded 1174 here (rather than calling cost_per_state_byte()) is justified by the doc — cost_per_state_byte isn't const. Worth a const_assert! (or similar) somewhere that pins cost_per_state_byte(0) == 1174 so the coupling can't drift silently if a future PR parameterizes CPSB.
| // paths `env.gas_limit` may default to the block max while the wrapping | ||
| // `tx` has `gas_limit = 0`. So we read whichever source carries the budget | ||
| // for the path we're on. | ||
| let gas_limit = if self.env.is_system_call { |
There was a problem hiding this comment.
Doc/impl mismatch: the surrounding comment says "For ordinary user txs the two agree (set together at the env-builder), but for eth_call/simulation paths env.gas_limit may default to the block max while the wrapping tx has gas_limit = 0. So we read whichever source carries the budget for the path we're on" — but the branch below only switches on is_system_call, not on is_simulation/eth_call. So for an Amsterdam+ eth_call (non-system-call) we hit the tx.gas_limit() arm, which is 0, making execution_gas = 0 and the reservoir/gas_left logic effectively no-op. In practice that's probably fine because eth_call doesn't depend on the GAS opcode return value the way real txs do, but the source comment is misleading as written.
Two possible fixes — pick whichever matches your intent:
- Tighten the comment to system-call vs user-tx only and drop the eth_call digression, since the code doesn't actually treat eth_call specially.
- Add an eth_call/simulation-aware branch (e.g., key on
self.env.is_simulationif that flag exists) that also readsenv.gas_limitinstead oftx.gas_limit(), if you do want simulations to get the same reservoir handling as system calls.
4 participants
Summary
ethereum/EIPs#11573 ("Update EIP-8037: fixed CPSB + frame accounting") rewrote large parts of the spec we already implement on this branch. The bulk of the rewrite (frame-end state-gas accounting, deferred SELFDESTRUCT, revert/halt cancellation) was already in place; this PR closes the remaining drift between our code and the new EIP text.
Gaps closed
Fixed CPSB doc + dead-const cleanup — PR 11573 collapses the dynamic, gas-limit-dependent
cost_per_state_byteformula (with quantization) into a single permanent constant of 1174. We were already returning 1174, but the function carried a "TEMPORARY for bal-devnet-4" doc and the formula constants (BLOCKS_PER_YEAR,TARGET_STATE_GROWTH_PER_YEAR,CPSB_SIGNIFICANT_BITS,CPSB_OFFSET) were dead code. Removed the consts, rewrote the doc, replaced the obsolete dynamic-formula unit tests intest/tests/levm/eip8037_tests.rswith a singletest_cpsb_is_fixed_at_1174that asserts 1174 across a spread of inputs.Auth-bytes constant rename — Spec now defines
STATE_BYTES_PER_AUTH_BASE = 23and computes the per-auth state cost additively as(STATE_BYTES_PER_NEW_ACCOUNT + STATE_BYTES_PER_AUTH_BASE) x CPSB. We had precomputedSTATE_BYTES_PER_AUTH_TOTAL = 135andSTATE_BYTES_PER_AUTH_ONLY = 23. Renamed_AUTH_ONLYto_AUTH_BASEand dropped_AUTH_TOTAL; consumers incrates/vm/levm/src/state_diff.rs:38(bytes()),crates/vm/levm/src/utils.rs:570(intrinsic gas ineip7702_set_access_code), andcrates/vm/levm/src/utils.rs:690(intrinsic_gas_dimensions) now compute the additive form inline. Bytes accounting is unchanged (135 = 112 + 23).System-call gas-limit bump — PR 11573 introduces
SYSTEM_CALL_GAS_LIMIT = 30_000_000 + STATE_BYTES_PER_STORAGE_SET x CPSB x SYSTEM_MAX_SSTORES_PER_CALL(= 30,601,088 withSYSTEM_MAX_SSTORES_PER_CALL = 16, matchingMAX_WITHDRAWAL_REQUESTS_PER_BLOCKfrom EIP-7002). The extra 601,088 is placed instate_gas_reservoir;gas_leftkeeps the legacy 30M execution budget. System calls remain not subject to the EIP-7825TX_MAX_GAS_LIMITcap and don't contribute toblock_regular_gas_used/block_state_gas_used. Wired this incrates/vm/levm/src/constants.rs(new constantsSYS_CALL_GAS_LIMIT_AMSTERDAM,SYS_CALL_STATE_GAS_RESERVOIR,SYSTEM_MAX_SSTORES_PER_CALL),crates/vm/backends/levm/mod.rs:2533(system-call env uses the bumped budget on Amsterdam+), andcrates/vm/levm/src/utils.rs:451(reservoir setup inprepare_executionspecial-casesis_system_calland bypassesTX_MAX_GAS_LIMIT_AMSTERDAM).Test parity — Replaced the now-obsolete dynamic-CPSB unit tests with one that pins CPSB = 1174 across a spread of
block_gas_limitinputs (1, 5M, 30M, 96M, 120M, 500M, u64::MAX).Verification
cargo fmt --all,cargo check --workspace --tests,cargo clippy -p ethrex-levm -p ethrex-vm -p ethrex-blockchain --testsall clean.levm::eip8037_*unit tests: 28/28 pass.blockchain::builder_validator_parity_tests: 14/14 pass.blockchain::mempool_tests: 16/16 pass.make -C tooling/ef_tests/blockchain testagainstbal@v5.7.0Amsterdam fixtures: identical pass/fail set vs. baseline (131 pass / 55 fail ineip8037_state_creation_gas_cost_increase, all 55 pre-existing fixture-vs-spec drift). Thebal@v5.7.0set predates PR 11573 so it does not exercise the new system-call gas-limit; that gap will be covered when EEST publishes a fixtures tag matching PR 11573.Test plan