🤖 Kimi Code Review

This PR updates EIP-8037 (Amsterdam/BAL) gas accounting logic and test fixtures. The changes are structurally sound and fix consensus-critical gas limit validation bugs.

Correctness & Consensus

crates/blockchain/payload.rs:845-854
The change from max(block_regular, block_state) to block_regular_gas_used for remaining_gas is correct per EIP-8037. Per-tx validation should only check regular gas against the block limit; state gas validation occurs at block finalization.

crates/vm/backends/levm/mod.rs:129-141 and 427-439
The introduction of TX_MAX_GAS_LIMIT_AMSTERDAM cap (EIP-7825) and separation of regular vs. state gas checks fixes a critical bug where valid transactions could be rejected due to state gas accounting during per-tx validation. The logic now correctly:

1. Caps the checked gas limit at TX_MAX_GAS_LIMIT_AMSTERDAM
2. Validates only against block_regular_gas_used (not the max)
3. Preserves pre-Amsterdam cumulative gas behavior

crates/vm/levm/src/opcode_handlers/system.rs:471-476 and 496-501
Moving the is_static check before stack pops and gas charging for Amsterdam prevents incorrect gas consumption in static contexts. This is consensus-critical: previously, gas could be consumed before the static context exception was raised.

crates/vm/levm/src/opcode_handlers/system.rs:693-696
Charging state gas after initcode size validation prevents burning state gas on oversized CREATE operations. This is correct per EIP-8037's "no gas consumption on exceptional halt" semantics for initcode size violations.

Potential Issues

Error Message Compatibility
crates/vm/backends/levm/mod.rs:72 and 991 - The error message no longer contains "Block gas used overflow". The removed comment indicated this substring was required for the EELS exception mapper. Verify that:

1. The EELS mapper regexes were updated to match the new format
2. The fixture update to v5.6.1 includes corresponding expectation changes

If the mapper wasn't updated, EELS tests will fail despite valid logic.

Minor Suggestions

crates/vm/backends/levm/mod.rs:963-991
Consider adding a comment referencing EIP-8037 block-end validation where running_block_gas_after is checked, similar to the comment in payload.rs, for consistency:

// EIP-8037: Final block validation ensures max(regular, state) <= gas_limit

Security Assessment

• Static Context Hardening: The early is_static return prevents potential DoS vectors where attackers could waste gas in static calls (Amsterdam only).
• State Gas Accounting: The separation of state gas from regular gas limits prevents potential block stuffing attacks where state gas could be used to bypass block gas limits.

Summary

The PR correctly implements EIP-8037's gas accounting separation. The fixture update to v5.6.1 is appropriate given the behavioral changes. Ensure the EELS exception mapper compatibility is handled (either via regex updates or confirmed fixture expectations) before merging.

Approve with minor verification requested on error message format compatibility.

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

Lines of code report

Total lines added: 16
Total lines removed: 0
Total lines changed: 16

+-----------------------------------------------------+-------+------+
| File                                                | Lines | Diff |
+-----------------------------------------------------+-------+------+
| ethrex/crates/blockchain/payload.rs                 | 800   | +8   |
+-----------------------------------------------------+-------+------+
| ethrex/crates/common/constants.rs                   | 52    | +1   |
+-----------------------------------------------------+-------+------+
| ethrex/crates/vm/backends/levm/mod.rs               | 2035  | +4   |
+-----------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/opcode_handlers/system.rs | 949   | +3   |
+-----------------------------------------------------+-------+------+

🤖 Codex Code Review

Findings

1. Consensus-critical: the new Amsterdam pre-check is now regular-gas-only, but the sequential execution paths no longer have a compensating post-tx max(regular,state) <= gas_limit check. In both [crates/vm/backends/levm/mod.rs:129](/home/runner/work/ethrex/ethrex/crates/vm/backends/levm/mod.rs#L129) and [crates/vm/backends/levm/mod.rs:427](/home/runner/work/ethrex/ethrex/crates/vm/backends/levm/mod.rs#L427), the PR changed admission to block_regular_gas_used + min(tx.gas_limit(), TX_MAX_GAS_LIMIT_AMSTERDAM), but after execution the loops only recompute block_gas_used at [crates/vm/backends/levm/mod.rs:170](/home/runner/work/ethrex/ethrex/crates/vm/backends/levm/mod.rs#L170) and [crates/vm/backends/levm/mod.rs:483](/home/runner/work/ethrex/ethrex/crates/vm/backends/levm/mod.rs#L483) without rejecting overflow. A last Amsterdam tx can therefore push block_state_gas_used over header.gas_limit and still be accepted. The parallel path added the needed post-check at [crates/vm/backends/levm/mod.rs:991](/home/runner/work/ethrex/ethrex/crates/vm/backends/levm/mod.rs#L991); the sequential paths need the same guard.

2. Block production now uses a different admission rule from block validation, which will skip valid Amsterdam transactions and underfill blocks. After this PR, remaining_gas is updated from regular gas only at [crates/blockchain/payload.rs:848](/home/runner/work/ethrex/ethrex/crates/blockchain/payload.rs#L848), but mempool selection still rejects txs with remaining_gas < tx.gas_limit() at [crates/blockchain/payload.rs:609](/home/runner/work/ethrex/ethrex/crates/blockchain/payload.rs#L609). That is stricter than the new validator rule (min(tx.gas_limit(), TX_MAX_GAS_LIMIT_AMSTERDAM) against regular gas), so valid Amsterdam txs with large total gas limits but acceptable regular-gas usage will be skipped by the builder. This is not a safety bug like Point 1, but it is a correctness/liveness regression for payload packing.

Aside from those two issues, the CREATE/CREATE2 static-context and initcode-ordering changes look reasonable, and I did not see a Rust-specific safety or error-handling regression in the touched code.

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

🤖 Claude Code Review

Here is the review:

PR Review: fix(l1): bump BAL fixtures to v5.6.1 and fix EIP-8037 state gas ordering

Summary

This PR makes three independent changes: a fixture version bump (v5.5.1 → v5.6.1), EIP-8037/EIP-7825 gas accounting corrections, and CREATE/CREATE2 ordering fixes. The logic is well-motivated and references specific spec issues. Overall the changes are correct; a few points worth noting below.

Correctness

is_static ordering in OpCreateHandler/OpCreate2Handler (system.rs:473, 498)

Moving the Amsterdam+ is_static check to before the stack pops is correct and matches the spec reference (#2608). The pre-Amsterdam guard in generic_create() (line 701) is unchanged in its effective position relative to subcall gas reservation — this is fine since increase_state_gas (the new Amsterdam-only call at line 694) is never reached for pre-Amsterdam, so the pre-Amsterdam execution path is behaviorally identical to before the PR.

STATE_GAS_NEW_ACCOUNT placement in generic_create() (system.rs:694–695)

Charging state gas after the MAX_INIT_CODE_SIZE check is the correct fix. Previously, an oversized CREATE would error out after burning state gas for an account never actually created.

Per-tx gas check (levm/mod.rs:134–142)

Switching from max(regular, state) to regular only for the per-tx cap is correct per EIP-8037: the spec says the block-end constraint is max(sum_regular, sum_state) ≤ gas_limit, not that each individual transaction's reservation must account for state gas.

TX_MAX_GAS_LIMIT_AMSTERDAM cap in execute_block_parallel (levm/mod.rs:979–983)

The parallel path is Amsterdam-only (entered only when header_bal is Some), so applying the cap unconditionally here without a fork branch is correct.

remaining_gas in payload.rs:848–853

Subtracting only block_regular_gas_used from gas_limit for Amsterdam is consistent with the updated per-tx check semantics. The block builder may technically allow a transaction that pushes the final max(regular, state) slightly over the limit — but block-end validation enforces the true constraint, so this is an acceptable ordering.

Concerns

Error message regression risk (levm/mod.rs:71–74)

The removed comment explicitly stated:

"Message must contain 'Gas allowance exceeded' and 'Block gas used overflow' as literal substrings for the EELS exception mapper... Can be simplified once we update the mapper regexes."

The "Block gas used overflow" substring is now gone from the check_gas_limit error. The PR description claims all 1103 consume-engine tests pass, which is encouraging, but it would be worth explicitly confirming (in the PR or a commit message) that the EELS exception mapper regex was updated before removing this constraint — otherwise this could silently break non-Amsterdam EELS test runs where the new format is unexpected.

Pre-Amsterdam tx gas cap not applied in L1 sequential paths

In execute_block and execute_block_pipeline, the pre-Amsterdam branch calls:

check_gas_limit(cumulative_gas_used, tx.gas_limit(), block.header.gas_limit)?;

without the .min(TX_MAX_GAS_LIMIT_AMSTERDAM) cap. The l2_hook.rs (line 585) explicitly enforces EIP-7825 for fork < Amsterdam on L2, suggesting the cap should apply there too. If this PR is Amsterdam-only in intent, a comment clarifying why the cap is omitted on the pre-Amsterdam L1 path would help avoid future confusion.

Semantic gap: generic_create charges state gas for failed deploys beyond initcode size

The new state-gas placement is correct for the MAX_INIT_CODE_SIZE validation, but the increase_state_gas(STATE_GAS_NEW_ACCOUNT) at line 694 is followed by additional failure modes (insufficient balance, depth limit, nonce overflow) in generic_create() that push 0 to the stack without creating an account. State gas is charged for those too. If this matches the spec, it's fine — but a comment would clarify intent (e.g. "state gas is charged even on soft failures per EIP-8037 §X").

Minor

• The block_regular_gas_used accumulation in the parallel post-execution loop (line 986) correctly uses pre-execution value for the check_gas_limit pre-tx check and then accumulates — this mirrors the sequential loop ordering correctly.
• The fixture URL updates are mechanical and consistent across all 6 locations.
• Removing the stale EELS mapper comment is correct cleanup if the mapper has been updated.

Verdict

The core fixes are correct and well-reasoned. The main actionable items are: (1) confirm or document the EELS mapper update given the error message format change, and (2) clarify whether EIP-7825 should apply to the pre-Amsterdam L1 path. No blocking issues.

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

Re: bot review concerns

Error message regression: The EELS exception mapper was updated in a companion PR (ethereum/execution-specs, branch ethrex/update-exception-mapper-gas-overflow). Both GAS_USED_OVERFLOW and GAS_ALLOWANCE_EXCEEDED now map to "Gas allowance exceeded".

Pre-Amsterdam TX_MAX_GAS_LIMIT: EIP-7825 cap is enforced at tx validation time (blockchain.rs:2439), not block execution. Pre-Amsterdam forks either predate the cap entirely or already reject oversized txs before reaching this path.

State gas on soft-fail CREATE: Matches spec. In EELS generic_create, charge_state_gas (line 96) precedes the soft-failure checks (balance/depth/nonce, lines 116-124). State gas is intentionally non-refundable on soft failure.

Benchmark Results Comparison

No significant difference was registered for any benchmark run.

🤖 Kimi Code Review

Here is my review of the PR implementing EIP-8037 gas accounting changes for the Amsterdam fork.

Critical Issues

1. EELS Exception Mapper Compatibility (Potential Breaking Change)

File: crates/vm/backends/levm/mod.rs
Lines: 72, 994

The error messages no longer contain "Block gas used overflow", which was explicitly required by the EELS exception mapper (see deleted comment at line 71-73). Unless the mapper regexes were externally updated, this will cause test failures:

// Before (required by mapper):
"Gas allowance exceeded: Block gas used overflow: ..."

// After (current):
"Gas allowance exceeded: ..."

Recommendation: Either restore the substring or confirm the mapper at execution-specs/ethrex.py has been updated to match the new format.

2. Inconsistent Gas Cap Constants

Files: crates/blockchain/payload.rs (line 611), crates/vm/backends/levm/mod.rs (line 132)

The code uses two different constants for the same 30M gas limit:

• POST_OSAKA_GAS_LIMIT_CAP in payload building
• TX_MAX_GAS_LIMIT_AMSTERDAM in VM execution

If these differ in value, this is a consensus-critical bug. If they're identical, consolidate them to prevent future drift.

Logic & Safety

3. CREATE Opcode Static Check Ordering

File: crates/vm/levm/src/opcode_handlers/system.rs
Lines: 471-474, 496-499, 691-694

Moving the is_static check before stack pops for Amsterdam (lines 471-474) is correct and consistent with SSTORE/CALL/SELFDESTRUCT behavior. The pre-Amsterdam path retains the check inside generic_create (lines 691-694), preserving backward compatibility.

4. State Gas Charging Location

File: crates/vm/levm/src/opcode_handlers/system.rs
Lines: 691-694

Moving increase_state_gas to after the initcode size validation (lines 685-689) prevents burning state gas on oversized CREATE operations. This is a valid optimization for Amsterdam, but ensure this doesn't affect gas reporting in failure receipts.

5. Gas Accounting Logic

File: crates/blockchain/payload.rs
Lines: 849-856, 611-618

The separation of regular gas (per-tx check) from state gas (block-end validation) correctly implements EIP-8037:

• Per-tx: `min

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

🤖 Codex Code Review

Findings

1. High: the payload builder can now produce Amsterdam blocks whose gas_used exceeds gas_limit. The new admission check only considers regular gas in crates/blockchain/payload.rs, and remaining_gas is likewise updated from block_regular_gas_used only in crates/blockchain/payload.rs. That would be fine if there were a compensating guard on max(block_regular_gas_used, block_state_gas_used) <= gas_limit, but finalization just writes context.gas_used() into the header in crates/blockchain/payload.rs. A sequence of txs with low regular gas but high state gas can now be included and yield an invalid payload, which is a consensus-critical block production bug. The builder needs a post-tx rollback/reject path, or a state-aware admission check, before this is safe.

I did not find another clear correctness or security issue in the touched LEVM validation paths from the diff alone. I did not run tests.

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

🤖 Claude Code Review

Here is the review:

PR Review: fix(l1): bump BAL fixtures to v5.6.1 and fix EIP-8037 state gas ordering

Summary

This PR makes three targeted correctness fixes to the Amsterdam/EIP-8037 implementation and bumps fixture URLs. The logic changes are well-motivated by the referenced upstream execution-specs PRs. Overall the code is correct, but there are two issues worth addressing.

Issues

1. Constant naming mismatch — POST_OSAKA_GAS_LIMIT_CAP vs TX_MAX_GAS_LIMIT_AMSTERDAM

payload.rs:612 uses POST_OSAKA_GAS_LIMIT_CAP for the Amsterdam per-tx gas cap:

head_tx.tx.gas_limit().min(POST_OSAKA_GAS_LIMIT_CAP)

levm/mod.rs:137,435,981 correctly uses TX_MAX_GAS_LIMIT_AMSTERDAM for the same semantic cap.

Both happen to equal 16_777_216 (1 << 24), but they are separately defined constants — one in ethrex_common::constants, one in ethrex_levm::constants. If either is updated independently, payload.rs will silently diverge from the VM.

POST_OSAKA_GAS_LIMIT_CAP is also semantically named for Osaka, not Amsterdam, making the intent unclear at the call site. The Amsterdam-specific cap in payload.rs should use TX_MAX_GAS_LIMIT_AMSTERDAM (or the two constants should be unified with a single source of truth) to keep the semantics aligned.

2. Error message substrings removed without confirming EELS mapper update

levm/mod.rs previously had this comment:

/// NOTE: Message must contain "Gas allowance exceeded" and "Block gas used overflow"
/// as literal substrings for the EELS exception mapper (see execution-specs ethrex.py).
/// Can be simplified once we update the mapper regexes.

This PR removes the comment and changes both error messages to drop "Block gas used overflow". The PR description says tests pass, but it does not mention that the EELS mapper regex was updated. If the ethrex.py mapper in ethereum/execution-specs still pattern-matches on "Block gas used overflow", running tests against any EELS version that includes the old mapper will fail to classify these errors and silently accept invalid blocks.

Please confirm that the EELS mapper was updated (or link to the upstream PR that does so), or alternatively keep the substring until the mapper change lands.

Correctness — Changes Look Right

EIP-8037 is_static ordering (system.rs):
Moving the is_static guard to the top of OpCreateHandler::eval and OpCreate2Handler::eval for Amsterdam+ (before stack pops and gas charging) is correct per execution-specs#2608. Pre-Amsterdam behavior in generic_create() is preserved with an explicit fork < Amsterdam guard at system.rs:700.

State gas after initcode size check (generic_create, line 691–695):
increase_state_gas(STATE_GAS_NEW_ACCOUNT) now runs after the MAX_INIT_CODE_SIZE guard. Oversized CREATE therefore fails with OutOfGas without consuming state gas — correct; an account was never created.

Per-tx gas check against regular gas only (levm/mod.rs):
All three execution paths (execute_block, execute_block_pipeline, execute_block_parallel) now use block_regular_gas_used + min(tx.gas_limit, TX_MAX_GAS_LIMIT_AMSTERDAM) for the per-tx guard and validate max(regular, state) ≤ gas_limit only at block end. This is the correct reading of EIP-8037 and execution-specs#2583.

remaining_gas in payload.rs (lines 849–859):
Tracking gas_limit − block_regular_gas_used instead of gas_limit − max(regular, state) for the per-tx reservation is consistent with the EIP-8037 per-tx semantics: using the max was overly conservative and could skip transactions that actually fit within the block's regular gas budget.

execute_block_parallel retrospective check (line 973–984):
The post-execution gas check for the parallel path now applies the same min(tx_gas_limit, TX_MAX_GAS_LIMIT_AMSTERDAM) cap. Since actual tx_regular_gas ≤ min(tx_gas_limit, TX_MAX_GAS_LIMIT_AMSTERDAM) by the reservoir mechanism, the check is conservative in the right direction.

Minor

The fixture URL bump is applied consistently across all six locations (Makefile, two CI workflows, docs, and two .fixtures_url_* files). No issues.

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

Greptile Summary

This PR applies two EIP-8037 correctness fixes and bumps BAL fixtures from v5.5.1 to v5.6.1. The is_static guard in OpCreateHandler/OpCreate2Handler is now checked before stack pops for Amsterdam+ (aligning CREATE/CREATE2 with SSTORE, CALL, and SELFDESTRUCT), and increase_state_gas(STATE_GAS_NEW_ACCOUNT) is moved inside generic_create() to after the initcode size check so a rejected oversized CREATE no longer inflates the block state gas reservoir. The execution-layer changes in levm/mod.rs correctly restrict per-tx fit checks to regular gas only and apply the TX_MAX_GAS_LIMIT_AMSTERDAM cap per EIP-7825, applied symmetrically across all three execute_block paths.

Confidence Score: 5/5

Safe to merge — all remaining findings are P2 style suggestions with no correctness impact

No P0 or P1 issues found. The one P2 finding is a naming inconsistency between POST_OSAKA_GAS_LIMIT_CAP and TX_MAX_GAS_LIMIT_AMSTERDAM that has no runtime impact since both constants equal 16,777,216. The logic fixes are correct and verified by 1103 passing BAL consume-engine tests per the PR description.

No files require special attention; optional cleanup at crates/blockchain/payload.rs:612 to use TX_MAX_GAS_LIMIT_AMSTERDAM for naming consistency with the rest of this PR

Sequence Diagram

sequenceDiagram
    participant EVM as EVM dispatch
    participant H as OpCreate(2)Handler
    participant GC as generic_create()

    EVM->>H: eval(vm)

    Note over H: Amsterdam+: is_static guard BEFORE stack pops (NEW)
    H-->>EVM: OpcodeNotAllowedInStaticContext (if static)

    H->>H: stack.pop() [value, offset, len (+salt)]
    H->>H: increase_consumed_gas(create_cost)
    H->>GC: generic_create(value, offset, len, salt)

    Note over GC: Check code_size <= MAX_INIT_CODE_SIZE
    GC-->>H: OutOfGas if oversized — no state gas charged (FIXED)

    Note over GC: Amsterdam+: increase_state_gas(STATE_GAS_NEW_ACCOUNT) AFTER size check (MOVED)
    GC->>GC: Pre-Amsterdam is_static guard (fork < Amsterdam)
    GC->>GC: Reserve subcall gas, load initcode, deploy
    GC-->>H: OpcodeResult

This is a comment left during a code review.
Path: crates/blockchain/payload.rs
Line: 611-615

Comment:
**Inconsistent constant for Amsterdam gas cap**

The inline comment says "cap at `TX_MAX_GAS_LIMIT`" but the code uses `POST_OSAKA_GAS_LIMIT_CAP`. Every other Amsterdam-gated site in this PR (`levm/mod.rs` lines 137 and 435) uses `TX_MAX_GAS_LIMIT_AMSTERDAM`. Both constants equal 16,777,216 (`1 << 24`), so there is no runtime impact, but using the Osaka-named constant in an Amsterdam-gated branch forces readers to look up the definition to confirm equivalence.

```suggestion
            let tx_gas_reservation = if context.is_amsterdam {
                head_tx.tx.gas_limit().min(TX_MAX_GAS_LIMIT_AMSTERDAM)
```

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "fix(l1): bump BAL fixtures to v5.6.1 and..." | Re-trigger Greptile}