🤖 Kimi Code Review

The PR implements a critical fix for EIP-8037 state gas charging order to comply with ethereum/EIPs#11421. The changes ensure that regular gas is consumed before state gas, preventing state gas from being consumed when regular gas OOG occurs (which would incorrectly inflate the parent's reservoir on frame failure).

Security & Correctness

crates/vm/levm/src/opcode_handlers/system.rs:92-107
The CALL handler logic correctly accounts for state gas spill in child gas computation:

• Line 98-100: Calculates spill correctly as STATE_GAS_NEW_ACCOUNT - from_reservoir where from_reservoir is capped by min()
• Line 101-103: Validates sufficient gas exists to cover the spill before proceeding
• Line 115-117: Charges state gas after regular gas consumption

crates/vm/levm/src/opcode_handlers/stack_memory_storage_flow.rs:287-306
The SSTORE handler properly reorders state gas charging:

• Lines 287-293: Determines state gas need before any charging
• Lines 295-300: Charges regular gas first via increase_consumed_gas()
• Lines 302-304: Charges state gas only after regular gas succeeds

This ordering is consensus-critical—if state gas were charged first and the subsequent regular gas charge failed, the consumed state gas would leak from the reservoir incorrectly.

Suggestions

1. Documentation consistency: In system.rs:92, the comment references EIPs#11421. Consider using the full reference ethereum/EIPs#11421 for clarity, matching the style used elsewhere.

2. Comment typo: In system.rs:96, "inflame" should likely be "inflate" (though this appears to be existing text).

Verification

The fixture bump from bal@v5.4.0 to bal@v5.5.1 is appropriate as the new fixtures likely contain test cases for this specific gas ordering edge case.

Verdict: Correct and safe implementation of the EIP clarification. The reordering prevents an edge case where failed calls could incorrectly consume state gas while preserving the state gas reservoir semantics for successful operations.

Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt

🤖 Codex Code Review

1. Consensus regression risk: the Amsterdam state-gas ordering changes in /home/runner/work/ethrex/ethrex/crates/vm/levm/src/opcode_handlers/system.rs#L92 and /home/runner/work/ethrex/ethrex/crates/vm/levm/src/opcode_handlers/stack_memory_storage_flow.rs#L287 are consensus-critical, but the PR adds no regression coverage for the exact behavior it is fixing. Please add cases that prove:
   • CALL to an empty account with value on Amsterdam fails on regular-gas OOG without consuming state gas or changing the parent reservoir.
   • SSTORE 0 -> nonzero on Amsterdam fails on regular-gas OOG without consuming state gas.
   • The same operations still consume state gas once the regular gas charge succeeds.

I did not find an obvious functional bug in the code change itself. The fixture URL bumps and the reordered charging logic look internally consistent with the surrounding EIP-8037 handling; the main gap is missing regression tests for a subtle gas-accounting rule.

Automated review by OpenAI Codex · gpt-5.4 · custom prompt

🤖 Kimi Code Review

Review Summary

The PR updates EIP-8037 (Amsterdam) state gas charging to follow the corrected ordering per ethereum/EIPs#11421: regular gas must be charged before state gas. This prevents state gas from being consumed (and spilling into the parent's reservoir) when regular gas OOG occurs.

EVM Opcode Changes

crates/vm/levm/src/opcode_handlers/stack_memory_storage_flow.rs (lines 287-307)

The SSTORE handler correctly moves state gas charging after regular gas consumption. The needs_state_gas flag preserves the conditional logic while delaying the charge.

crates/vm/levm/src/opcode_handlers/system.rs (lines 92-128)

The CALL handler changes are more subtle and correctly handle the child gas limit calculation:

1. Lines 98-103: Preemptively calculates potential spill from the reservoir and subtracts it from gas_left (child gas allowance). This ensures the child doesn't receive gas that will be needed for the state gas spill.
2. Lines 108-110: Charges regular gas first.
3. Lines 115-117: Charges state gas after, ensuring regular gas OOG prevents state gas consumption.

Minor Style Note: The gas_left shadowing (line 98) is valid Rust but slightly confusing. Consider using a distinct name like effective_gas_left for clarity.

Safety Checks: Proper use of checked_sub (line 102) prevents underflow when accounting for spill. The min operation (line 99) ensures spill calculation is safe.

Fixture Updates

The fixture URL updates from bal@v5.4.0 to bal@v5.5.1 are consistent across:

• .github/workflows/daily_hive_report.yaml (line 125)
• .github/workflows/pr-main_l1.yaml (line 241)
• Makefile (line 151)
• Documentation and

Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt

🤖 Claude Code Review

Now I have enough context for a thorough review.

PR Review: fix(l1): bump BAL fixtures to v5.5.1 and fix state gas charging order

Summary

This PR fixes a consensus-critical ordering bug in EIP-8037 state gas charging and bumps the Amsterdam fixture suite from v5.4.0 to v5.5.1. The fix aligns with ethereum/EIPs#11421: regular gas must be charged before state gas, otherwise an OOG on regular gas would leave consumed state gas behind, incorrectly inflating the parent's reservoir via incorporate_child_on_error.

Correctness

SSTORE (stack_memory_storage_flow.rs:287-306) — correct

The refactor is clean:

• needs_state_gas is computed before any charging (lines 290-294)
• Regular gas is charged via increase_consumed_gas (line 296-303)
• State gas is charged only if regular gas succeeded (lines 305-307)

If increase_consumed_gas returns OutOfGas, the ? short-circuits before increase_state_gas is ever called. This is exactly the desired semantics.

CALL (system.rs:96-129) — correct, with a note

The "spill peek" approach at lines 99-103 is sound: the computation mirrors the internals of increase_state_gas to pre-deduct the expected spill from gas_left before passing it to gas_cost::call() for child gas allocation. Because neither gas_cost::call() nor increase_consumed_gas modify state_gas_reservoir, the peek value is still valid when increase_state_gas is called at line 128.

However, lines 99-100 duplicate the arithmetic that increase_state_gas already performs internally:

// vm.rs:553-555 inside increase_state_gas:
let from_reservoir = self.state_gas_reservoir.min(gas);
let spill = gas - from_reservoir;

This coupling means that if increase_state_gas's logic ever changes, the peek in system.rs could silently drift. Consider extracting a peek_state_gas_spill(gas) helper on VM that both the CALL handler and increase_state_gas can share, or at minimum add a comment noting this intentional duplication.

Note on gas_left and eip7702_gas_consumed: In the needs_state_gas = true branch (line 101), gas_left is computed as gas_remaining - spill without subtracting eip7702_gas_consumed. This matches the pre-PR behavior exactly (the old code read gas_remaining after the state gas charge, which also didn't factor in eip7702). This appears intentional per EELS semantics, but a brief comment explaining why eip7702 is excluded here would help future readers.

CREATE/CREATE2 (system.rs:470-481, 504-506) — no change needed, already correct

Regular gas is charged first (increase_consumed_gas at lines 471-476 / 495-501), then state gas (increase_state_gas at lines 481 / 506). The comment "BEFORE generic_create() reserves child gas" refers to timing relative to generic_create(), not relative to the regular gas charge — this is correct and doesn't need the same fix as CALL/SSTORE.

SELFDESTRUCT — no change, already correct (as documented in the PR description).

Security

The bug being fixed is consensus-critical: in the old ordering, a failed CALL or SSTORE (regular gas OOG) would allow previously spent state gas to re-enter the parent's reservoir, making more state gas available than legitimately earned. This could be exploited to perform state-writing operations at below-expected cost by carefully crafting calls that trigger OOG after state gas consumption. The fix closes this window.

Minor Issues

• Comment inconsistency: system.rs:93 uses EIPs#11421 while the PR description and stack_memory_storage_flow.rs:288 use the full form ethereum/EIPs#11421. GitHub will not auto-link EIPs#11421 as a cross-repo reference. Use the full form consistently.

• Fixture URL encoding inconsistency: tooling/ef_tests/blockchain/.fixtures_url_amsterdam and state/.fixtures_url_amsterdam use bal%40v5.5.1 (URL-encoded @), while CI YAML files use the literal bal@v5.5.1. Both forms are functionally correct in their respective contexts (URL-encoded for raw URL files, literal in YAML strings), so this is fine.

Verdict

The fix is correct and safe. The reordering precisely targets the identified bug, preserves the semantics for success cases, and the fixture bump is consistent across all relevant locations. The one structural concern (duplicated spill arithmetic) is worth a follow-up but is not a blocker.

Automated review by Claude (Anthropic) · sonnet · custom prompt

🤖 Codex Code Review

1. system.rs recomputes gas_left from vm.current_call_frame.gas_remaining and drops the eip7702_gas_consumed subtraction done at system.rs. Today that is probably masked because needs_state_gas should only happen for an empty account, while an EIP-7702 delegated target is not empty, but the code is now relying on that invariant implicitly. Using the already-adjusted gas_left.checked_sub(spill) would keep the arithmetic locally correct and avoid a future gas-allocation drift if that assumption changes.

2. No regression tests were added for the reordered Amsterdam state-gas charging in system.rs and stack_memory_storage_flow.rs. This is consensus-sensitive OOG behavior. I would want explicit coverage for:

   • CALL to an empty account with value where regular gas fails before state gas.
   • CALL where state gas spills from the reservoir into gas_remaining.
   • SSTORE 0 -> nonzero where the regular charge fails first.
   • A nested-call revert case proving the parent reservoir is unchanged.

Beyond that, the change looks directionally right: charging regular gas before Amsterdam state gas matches the stated EELS ordering intent, and I did not see another obvious correctness or security regression in the diff.

I could not run cargo check in this environment because rustup tried to write under /home/runner/.rustup/tmp and hit a read-only filesystem error.

Automated review by OpenAI Codex · gpt-5.4 · custom prompt

🤖 Claude Code Review

Here is my review of PR #6389:

PR Review: fix(l1): bump BAL fixtures to v5.5.1 and fix state gas charging order

Summary

This PR addresses two related ordering bugs in EIP-8037 (Amsterdam+) state gas handling, per EIPs#11421. The spec mandates that regular gas must be charged before state gas — the previous code did the opposite, allowing spilled state gas to incorrectly inflate the parent's reservoir when regular gas OOG occurred. The fixture bump is mechanical.

stack_memory_storage_flow.rs — SSTORE fix

The change is clean and correct. The boolean flag approach is idiomatic and readable.

Before: increase_state_gas(STATE_GAS_STORAGE_SET) was called before increase_consumed_gas, meaning a subsequent regular-gas OOG would unwind the call frame without having consumed the state gas, causing it to flow back to the parent reservoir via incorporate_child_on_error.

After: Regular gas is charged first; state gas follows only if that succeeds. The condition logic is identical to before, just lifted into a needs_state_gas bool. No correctness concerns here.

system.rs — CALL fix

This is more complex because the state gas spill affects the child gas limit computation (the 63/64 rule needs accurate gas_left). The solution peeks at the reservoir to pre-compute the expected spill without mutating anything:

// system.rs:99-103
let from_reservoir = vm.state_gas_reservoir.min(STATE_GAS_NEW_ACCOUNT);
let spill = STATE_GAS_NEW_ACCOUNT - from_reservoir;
(vm.current_call_frame.gas_remaining as u64)
    .checked_sub(spill)
    .ok_or(ExceptionalHalt::OutOfGas)?

This produces the same gas_left value that the old code produced (old code read gas_remaining after the state gas charge had already deducted spill), so child gas computation is preserved exactly.

One subtle observation: The needs_state_gas branch at line 101 uses vm.current_call_frame.gas_remaining as u64 directly rather than the previously-computed gas_left (which already subtracted eip7702_gas_consumed at line 88–90). This means eip7702_gas_consumed is not reflected in gas_left when needs_state_gas is true. However:

• This is pre-existing behavior — the old code also used raw gas_remaining as u64 in that branch.
• In practice, the two conditions are mutually exclusive: EIP-7702 delegation sets account code (making address_is_empty false), so needs_state_gas would be false when eip7702_gas_consumed > 0.

This is worth a comment clarifying the mutual exclusivity, but it's not a regression introduced by this PR.

The early OOG check (line 102–103, checked_sub(spill)) fires before regular gas is charged. Per the spec, regular gas should be charged first. In this edge case (reservoir so small that spill alone exceeds gas_remaining), the outcome is OOG regardless of ordering, so the semantic result is correct — though strictly, the spec ordering is inverted for this specific failure path.

Non-code changes

• CI workflows, Makefile, docs, and .fixtures_url_amsterdam files consistently updated from bal@v5.4.0 → bal@v5.5.1. All five locations updated, no stragglers.

Overall

The bug fix is correct and well-motivated. The increase_state_gas implementation in vm.rs:547 is already written to be atomic (no partial mutation on OOG), so the new ordering interacts cleanly with it. The comments in the changed code clearly reference EIPs#11421, which is good for auditability.

No blocking issues. The two minor observations above (pre-existing eip7702 blind spot and early-OOG ordering for the spill-overflow edge case) are worth awareness but not defects introduced by this PR.

Automated review by Claude (Anthropic) · sonnet · custom prompt

Greptile Summary

This PR bumps the Amsterdam EIP test fixtures from bal@v5.4.0 to bal@v5.5.1 and fixes the EIP-8037 state gas charging order for SSTORE and CALL opcodes to match the EELS reference implementation (per EIPs PR 11421).

Key changes:

• stack_memory_storage_flow.rs (SSTORE): Introduces a needs_state_gas boolean to defer STATE_GAS_STORAGE_SET until after the regular gas charge succeeds. Previously, if regular gas OOG occurred after state gas was already drawn from the reservoir, the parent's incorporate_child_on_error handler would incorrectly inflate its reservoir by the child's state_gas_used.
• system.rs (CALL): Fixes the same ordering issue for new-account creation state gas. Because the pre-call gas_left used to compute the 63/64 child gas limit depended on the post-spill value of gas_remaining, the fix pre-computes the expected spill by peeking at state_gas_reservoir (without modifying it), adjusts gas_left accordingly, charges regular gas first, and only then calls increase_state_gas. This preserves the identical child gas limit the old code produced while respecting the required charge ordering.
• Fixture bumps: All CI workflow references, the Makefile, the two .fixtures_url_amsterdam files, and the docs are consistently updated to the new fixture version.
• The implementation is well-aligned with the codebase's EIP-8037 model (global state_gas_reservoir, state_gas_used_snapshot for child-revert restoration). The pre-computation of spill in CALL is safe because state_gas_reservoir is not mutated between the peek and the actual increase_state_gas call.

Confidence Score: 5/5

• This PR is safe to merge; it correctly fixes a spec-compliance bug and performs a straightforward fixture version bump.
• The logic change is focused and well-reasoned. The pre-computation in the CALL handler correctly mirrors what increase_state_gas would do, state_gas_reservoir is not mutated between the peek and the actual charge, the child gas limit is unchanged, the revert/snapshot accounting is unaffected, and the SSTORE change is a simple reorder with no impact on the storage mutation path. All fixture URL changes are consistent across the repository.
• No files require special attention.

Sequence Diagram

sequenceDiagram
    participant Parent as Parent Frame
    participant RegGas as Regular Gas
    participant StateGas as State Gas (EIP-8037)
    participant Child as Child Frame

    Note over Parent,Child: SSTORE (new slot: 0→nonzero)
    Parent->>RegGas: increase_consumed_gas(sstore_cost) [OOG exits here]
    Parent->>StateGas: increase_state_gas(STATE_GAS_STORAGE_SET) [OOG exits here]
    Parent->>Parent: mutate storage slot

    Note over Parent,Child: CALL (empty account + value)
    Parent->>Parent: peek reservoir → compute expected spill
    Parent->>Parent: gas_left = gas_remaining − spill (for 63/64 child limit)
    Parent->>RegGas: increase_consumed_gas(call_cost + eip7702) [OOG exits here]
    Parent->>StateGas: increase_state_gas(STATE_GAS_NEW_ACCOUNT) [OOG exits here]
    Parent->>Child: generic_call(gas_limit) — state_gas_used_snapshot recorded
    alt Child reverts
        Child-->>Parent: incorporate_child_on_error: reservoir += child_state_gas_used
    else Child succeeds
        Child-->>Parent: merge state
    end

_{Last reviewed commit: "fix(l1): bump BAL fi..."}

Lines of code report

Total lines added: 10
Total lines removed: 0
Total lines changed: 10

+-----------------------------------------------------+-------+------+
| File                                                | Lines | Diff |
+-----------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/opcode_handlers/system.rs | 946   | +10  |
+-----------------------------------------------------+-------+------+

Benchmark Results Comparison

Benchmark Results: Fibonacci