Skip to content

feat(l1): support bal-devnet-7 (bal@v7.1.1)#6653

Merged
edg-l merged 28 commits into
mainfrom
bal-devnet-7-pr
May 18, 2026
Merged

feat(l1): support bal-devnet-7 (bal@v7.1.1)#6653
edg-l merged 28 commits into
mainfrom
bal-devnet-7-pr

Conversation

@edg-l

@edg-l edg-l commented May 13, 2026

Copy link
Copy Markdown
Contributor

Motivation

Bring ethrex up to bal-devnet-7 (BAL fixtures bal@v7.1.1). Stacked
on top of #bal-devnet-6-pr (now in main).

Description

Aligns EIP-8037 state-gas accounting with bal-devnet-7 spec progression
(EELS PRs #2815 / #2816 / #2823 / #2827 / #2828 / #2836 / #2845 / #2848),
bumps Amsterdam fixtures from snobal-devnet-6@v1.1.0 to bal@v7.1.1,
and bumps the pinned hive version past the ethrex --http.api fix.

Main changes:

  • EIP-8037 state-gas alignment with bal-devnet-7:
    • System-call state-gas reservoir.
    • Halt refunds spilled state gas (Policy A).
    • Tx-level CREATE failure refunds intrinsic NEW_ACCOUNT;
      intrinsic_state_gas_charged preserved across the failure path.
    • Tx-CREATE collision refund with regular-gas burn; billing matches
      EELS.
    • Cross-frame revert leaks inline credits.
    • Cross-frame revert reservoir formula fix.
    • Block-level state_gas_used subtracts state_refund.
    • Remove same-tx SELFDESTRUCT state-gas refund (EELS PR refactor(l1): move hash from Block to BlockHeader #2845, v7.1.0).
  • EIP-7702:
  • levm fixes pulled from main:
  • Tooling / tests:
    • Per-tx gas-dimension dump on block gas_used mismatch.
    • Bump Amsterdam fixtures to bal@v7.1.1.
    • Annotate BAL balance-mismatch errors with gas-equivalent diff and
      recognised state-gas constant multiples.
    • Unskip 74 bal-devnet-6 Amsterdam fixtures now passing.
    • Skip 21 stale EIP-8025 fixtures pinned at bal@v5.7.0
      (zkevm@v0.3.3 bundle, pre-bal-7).
    • Drop stale bal-devnet-6 known-issues entries from
      docs/known_issues.md and hive KNOWN_EXCLUDED_TESTS.
  • CI:
    • Bump pinned hive version past the ethrex --http.api flag
      feature-detect fix (c4d839b3, hive fix(levm): mulmod should not overflow #1485). Without this, hive
      starts ethrex with the default HTTP namespace allowlist
      (eth,net,web3) and tests touching admin_*/debug_*/txpool_*
      fail.

Local test run

./run_test.sh against tests-bal@v7.1.1: 2,145 / 2,145 pass.
cargo test -p ethrex-test --tests: 453 / 453 pass.

Checklist

  • Updated STORE_SCHEMA_VERSION (crates/storage/lib.rs) if the PR
    includes breaking changes to the Store requiring a re-sync.

edg-l and others added 24 commits May 13, 2026 16:54
Port execution-specs#2815: on exceptional halt, preserve all state-gas
counters and let the parent's incorporate-on-error refund the full child
charge to the reservoir, symmetric with REVERT. Replaces the prior halt-time
spill reclassification (Policy B) at every boundary.

Deletes the Policy B plumbing (regular_gas_reclassified, the top-message
entry-reservoir snapshot, two CallFrame snapshots) and unifies handle_return_*
HALT/REVERT arms. Rewrites two Policy-B-specific tests to assert Policy A
math.
When `validate_gas_used` fails, log per-tx breakdown (status, gas_used,
regular vs state, gas_spent, gas_refunded) plus block-level sums. Lets
divergent tx + dimension be located without re-running with extra tracing.

Adds `TxGasBreakdown` and `TxStatus` to `BlockExecutionResult`, populated by
the three levm execute paths (sequential `execute_block`, sequential
`execute_block_pipeline` body, parallel `execute_block_parallel`). L2
producer/committer paths leave the field empty; the diagnostic falls back to
a one-liner there.

Cost is one Vec<TxGasBreakdown> push per tx (~80 bytes). No formatting or
log emission unless the mismatch fires.
- bump COST_PER_STATE_BYTE 1174 → 1530 and byte counts
  (NEW_ACCOUNT 112→120, STORAGE_SET 32→64, AUTH_TOTAL 135→143)
- split same-tx selfdestruct refund: for the tx-created top-level
  target, route the NEW_ACCOUNT portion through the state_refund
  channel (block-accounted only, no reservoir credit, no execution
  clamp); non-account portion keeps the existing reservoir+absorbed
  clamped path
Pin every Amsterdam fixture site to `tests-bal@v7.0.0` (execution-specs
`devnets/bal/7`), the bal-devnet-7 fixture release published 2026-05-11.
Mirrored on execution-spec-tests as `bal@v7.0.0`.

Sites updated:
- tooling/ef_tests/blockchain/.fixtures_url_amsterdam
- tooling/ef_tests/state/.fixtures_url_amsterdam
- .github/config/hive/amsterdam.yaml (eels_commit e87390b6 → bcc2a876)
- Makefile (AMSTERDAM_FIXTURES_BRANCH: devnets/snobal/6 → devnets/bal/7)
- tooling/ef_tests/blockchain/Makefile (stale comment ref)

The bal-7 release contains parameter recalibration (CPSB 1174→1530,
NEW_ACCOUNT 112→120, STORAGE_SET 32→64) — already implemented on this
branch — plus the bug-fix set from execution-specs#2823 (tx-CREATE
halt/revert intrinsic refund, isolated 0→x→0 SSTORE refund,
CREATE-collision regular gas) and the SYSTEM_CALL_GAS_LIMIT raise,
which are the remaining bal-7 work items.
Per execution-specs commit 7b3e8016 ("eip-8037 system transaction gas
state reservoir"): system transactions now receive an extra state-gas
reservoir of `STATE_BYTES_PER_STORAGE_SET × cost_per_state_byte ×
SYSTEM_MAX_SSTORES_PER_CALL` on top of `SYS_CALL_GAS_LIMIT (30M)`. With
the bal-devnet-7 constants (STORAGE_SET=64, CPSB=1530, SSTORES=16) the
extra is 1,566,720 gas — enough to absorb 16 cold SSTORE state-gas
charges without spilling into the 30M regular budget.

The reservoir is allocated ADDITIVELY (not pre-consumed from
gas_remaining): system contracts (EIP-2935, EIP-4788, EIP-7002,
EIP-7251) get the full 30M regular budget plus the state-gas reservoir,
so state-gas growth alone cannot OOG them.

Wired by overriding `state_gas_reservoir` / `state_gas_reservoir_initial`
in `add_intrinsic_gas` after the normal derivation when `is_system_call`.
Uses the pre-computed `vm.state_gas_storage_set` field.
…UNT (bal-devnet-7)

EELS PR #2823 (commit eb80b438a) extends Policy A's top-level failure
refund so that a CREATE transaction that reverts or halts also returns
the intrinsic `STATE_BYTES_PER_NEW_ACCOUNT × cost_per_state_byte`
charge — paid via `add_intrinsic_gas` at tx start — back to the
reservoir. Mirrors the long-standing inner-CREATE failure rule.

Spec (fork.py::process_transaction):
  if tx_output.error is not None:
      tx_output.state_gas_left += tx_output.state_gas_used    # Policy A
      tx_output.state_gas_used = Uint(0)
      if isinstance(tx.to, Bytes0):
          new_account_refund = STATE_BYTES_PER_NEW_ACCOUNT * COST_PER_STATE_BYTE
          tx_output.state_gas_left += new_account_refund      # to reservoir
          tx_output.state_refund   += new_account_refund      # block-level

ethrex impl in `finalize_execution`, after the Policy A
execution-portion refund, when `is_create()`:
- `state_gas_reservoir += new_account_refund` (sender gets gas back)
- `state_gas_refund_absorbed += new_account_refund` (net block-level
  state_gas_used drops to 0 via the existing `net_state_gas_used`
  subtraction)
- `intrinsic_state_gas_charged -= new_account_refund` so
  `refund_sender`'s `regular_gas = raw_consumed - intrinsic_state - ...`
  formula doesn't double-deduct the now-refunded amount; the gas that
  was actually burned remains in the regular dimension where it sits.

Updates the existing `test_top_halt_phantom_drain_with_real_spill_under_policy_a`
test to assert the post-#2823 behavior: state_gas_used = 0 for a halted
CREATE tx (was state_new before).
…ate_gas_charged

Initial item-5 impl decremented `intrinsic_state_gas_charged` by
`new_account_refund` on top-level CREATE-tx failure, on the theory that
the refunded gas should not be subtracted from `raw_consumed` in the
regular-gas formula. That was wrong.

Per the EELS test_failed_create_tx_refunds_intrinsic_new_account
expectation:
  expected_gas_used = intrinsic_regular + regular_consumed
                    = gas_limit - create_state_gas
For an INVALID halt: expected = gas_limit - state_new.

`refund_sender`'s `regular_gas = raw_consumed - intrinsic_state - ...`
produces exactly that when `intrinsic_state_gas_charged` is left intact.
Clearing it overcounts regular by state_new (drops gas_used by state_new
in the wrong direction). Conceptually: the intrinsic burn is still in
`raw_consumed`, the refund returns it via the reservoir to the user,
and the regular-gas subtraction keeps it out of the regular dimension —
neither dimension counts it, which matches the spec.

Reservoir credit and `state_gas_refund_absorbed` bump (the two genuine
behavior changes from #2823) remain.
…-devnet-7)

Two fixes for the tx-level CREATE-collision path required by EELS
bal@v7.0.0:

1. Burn the forwarded gas in the regular dimension. EIP-684 + EELS
   `process_message_call` mark a CREATE-tx collision as
   `regular_gas_used = message.gas`. ethrex's collision branch in
   `handle_create_transaction` was returning with
   `current_call_frame.gas_remaining` still holding the
   post-intrinsic leftover, which leaked back to the sender as unused
   gas and never reached the regular dimension via
   `refund_sender`'s `raw_consumed - … + reclassified` formula. Now
   we zero `gas_remaining` so `raw_consumed = gas_limit` downstream.

2. Refund the intrinsic `STATE_BYTES_PER_NEW_ACCOUNT × CPSB` on
   collision. Previously the `!is_collision()` gate in
   `finalize_execution` excluded collision from BOTH Policy A and the
   item-5 NEW_ACCOUNT refund. Per EELS PR #2823 (commit eb80b438a)
   `process_transaction`, a tx-CREATE collision is just another
   failure variant that gets the intrinsic NEW_ACCOUNT refund:
       if tx_output.error is not None and isinstance(tx.to, Bytes0):
           tx_output.state_refund += create_state_gas
   Restructure the failure branch so Policy A still skips on
   collision (no execution state-gas to recover), but the
   `is_create()` intrinsic refund fires on every non-success.

After both: `regular_gas = gas_limit - intrinsic_state`,
`state_gas = 0`, header `gas_used = gas_limit - state_new` ✓
(matches EELS `test_create_tx_collision_refunds_intrinsic_new_account`).

Closes the 7-fixture CREATE-collision cluster in the bal-devnet-7
ef-test failures.
…t-7)

EELS PR #2823 (commit eb80b438a) simplifies `incorporate_child_on_error`
to:
    evm.state_gas_left += child_evm.state_gas_used + child_evm.state_gas_left

Removes the pre-#2823 subtraction of `child_evm.state_gas_refund` (the
"unwind on revert" of inline credits the child applied) on the rationale
that any inline credits the child applied are keyed to charges (its own
SSTORE or CREATE pre-charge) that are themselves rolled back, so the
matching `state_gas_left + state_gas_used` sum already reflects the
correct amount to return to the parent.

In ethrex's clamp-and-spill model the analogous tracking is the
`state_gas_refund_absorbed_snapshot` restore and the
`credit_against_drain_delta` subtraction in handle_return_call /
handle_return_create's Revert arms. Drop both.

Behavior change: an SSTORE 0→x→0 credit applied in a frame that later
reverts now leaks up to the parent's reservoir, matching the bal-7
test inversions:
- test_sstore_restoration_sub_frame_revert
- test_sstore_restoration_ancestor_revert
- test_subcall_set_clear_revert_pays_no_state_gas
and the broader stSStoreTest port group whose sub-contract calls
exercise this revert path.

Leaves `state_gas_used = state_gas_used_snapshot` (gross charges undone
on revert — same as before, matches EELS `state_gas_used` not flowing
to parent on error) and `state_gas_refund_pending = pending_snapshot`
(pending credits discarded with the child, per #2823 docstring).
…hannel (bal-devnet-7)

EIPs#11611 / EELS PR #2816 spec the EIP-7702 existing-authority refund
as a dedicated `MessageCallOutput.state_refund` channel: a separate
monotonic accumulator subtracted from `tx_state_gas` at the end of
`process_transaction`. `state_gas_used` and intrinsic state-gas are
explicitly kept immutable after validation so failure-path accounting
(Policy A's `execution_portion` math, regular-gas derivation, etc.)
stays consistent.

ethrex's prior implementation simulated the channel inline by
pre-decrementing both `state_gas_used` and `intrinsic_state_gas_charged`
in `set_delegation`. That worked on success (math nets out at refund_sender)
but corrupted every failure path because Policy A and the block-gas
formula re-derive from the pre-decremented counters — auth refunds
were double-applied, lost, or mis-routed depending on the failure mode.

This commit reifies the spec channel:
- New `VM.state_refund: u64` field (mirrors EELS `state_refund`)
- `set_delegation` (utils.rs:345-367): bumps `state_gas_reservoir` and
  `state_refund`; no longer touches `state_gas_used` or
  `intrinsic_state_gas_charged`
- `refund_sender` (default_hook.rs:254-264): subtracts `vm.state_refund`
  from the state dimension at the end, identically on success and failure

Fixes the 5 state_gas_set_code / intrinsic_gas_cost failures:
- auth_sender_billing_after_failure
- existing_account_auth_header_gas_used_reflects_refund
- existing_auth_refund_survives_top_level_revert
- auth_state_gas_in_header_after_failure
- prague/eip7702_set_code_tx/gas/intrinsic_gas_cost (duplicate-auth case)

`state_refund` lives on the VM, never snapshotted, so it naturally
survives revert/halt/OOG without any call-frame backup machinery.
EELS fork.py:1199-1205 computes per-tx state-gas as
  tx_state_gas = intrinsic_state_gas + state_gas_used - state_refund
and adds it to block_state_gas_used. Our `net_state_gas_used` was
subtracting only state_gas_refund_absorbed/pending and missing the
state_refund channel (EIP-7702 set_delegation refunds + CREATE-tx-failure
intrinsic NEW_ACCOUNT refunds). Result: block-level state dimension
inflated by the refund amount on those paths.

Fixes the 6 amsterdam/eip8037_state_creation_gas_cost_increase/state_gas_set_code
failures whose delta was exactly +183_600 (= NEW_ACCOUNT × CPSB).
Pre-bal-7 behavior charged the sender the full gas_limit on a tx-CREATE
address collision. EELS bal-devnet-7 (interpreter.py:120-145 + fork.py
failure block) preserves state_gas_reservoir across the collision and
adds new_account_refund to it, so:
  gas_spent = max(gas_limit - reservoir, calldata_floor)
  block_state_gas = 0
  block_regular_gas = effective_regular

The collision branch in default_hook also now subtracts both refund
channels (state_gas_refund_absorbed/pending + state_refund) from
state_gas so the block state dimension lands at 0 rather than
migrating into regular_gas via report.gas_used - report.state_gas_used.

Fixes 7 tests: 1 amsterdam (create_tx_collision_refunds_intrinsic_new_account)
and 6 paris/ported_static collision fixtures run under Amsterdam fork.
f4e47e1 over-corrected: dropping the state_gas_refund_absorbed
snapshot restore let reverted children's inline credits leak into the
parent's block-level deduction. EELS-equivalent per
incorporate_child_on_error is

  parent.state_gas_left += child.state_gas_used + child.state_gas_left

where EELS credit_state_gas_refund decrements the child's
state_gas_used (applied = min(amount, state_gas_used)). The child's
state_gas_used is therefore already net of any inline credits the
child applied.

Mapping to ethrex's gross-used + absorbed-deduction model:

  used_delta     = state_gas_used     - snapshot
  absorbed_delta = state_gas_refund_absorbed - snapshot
  child_net_used = used_delta - absorbed_delta
  new_reservoir  = state_gas_reservoir + child_net_used

state_gas_refund_absorbed is also restored to snapshot so the child's
absorbed credit dies with the child.

Fixes 22 SSTORE-restoration + ported_static/stSStoreTest failures.
The zkevm@v0.3.3 fixture bundle (the only bundle that ships
executionWitness, used by test-stateless-zkevm) is filled against an
older bal spec and disagrees with bal@v7.0.0 gas accounting:
storage_set/new_account/cpsb constants pre-recalibration plus pre-
EELS-#2815/#2816/#2823/#2827/#2828 refund-channel semantics.

Skips the 21 remaining gas mismatches in the eip8025_optional_proofs
filter (witness_codes_*, witness_state_*, validation_state_*),
analogous to the bal@v5.6.1 block already at the top of the list.
Re-enable once the zkevm bundle is regenerated against bal-7.
…n bal@v7.0.0)

Originally added in 530dd47 against snobal-devnet-6 fixtures and
bal-devnet-6 spec semantics. After the bal@v7.0.0 fixture bump
(94273e4) plus the EIP-8037/EIP-7702 alignment commits, these
Amsterdam-fork tests pass and the skips are obsolete.
**Motivation**

When an account creation reverts, we incorrectly mark it as existing for
the purposes of EIP7702, leading to charging gas incorrectly.

**Description**

Currently we track accounts that exist in the trie for purposes of
charging EIP7702 gas (the price is different if the authority already
exists in the trie). This is saved in the `exists` field of the account,
which is currently not saved in the callframe backup.

This means when an account creation is reverted, we don't unmark it as
existing. This causes an incorrect amount of gas to be charged.

The PR contains a regression test, that should fail on the first commit
and pass on the second.
…ation

Mirrors EELS PR #2836 (merged 2026-05-12, devnets/bal/7):
ethereum/execution-specs#2836

When the authority's pre-state code slot already holds a 7702 delegation
indicator (overwrite or clear), refill the 23-byte AUTH_BASE portion of
intrinsic state gas via both the reservoir and the state_refund channel.
Keys off the pre-state code, not the auth target, so it applies whether
auth.address is a new delegation target or zero (clear).

Breaks ef-tests until upstream re-cuts fixtures past tests-bal@v7.0.0;
no skips added per the bal-devnet-7 alignment policy.
Mirrors EELS PR #2845 (tests-bal@v7.1.0, devnets/bal/7): all SELFDESTRUCT
state-gas refunds are removed from EIP-8037. Drops the
apply_same_tx_selfdestruct_state_refund pass and its call site in the
default hook; shared clamp-and-spill bookkeeping fields stay.
Updates the Amsterdam fixture URL (now hosted on execution-specs releases)
and the hive eels_commit to 73083f054c (tip of tests-bal@v7.1.0). Local
ef-tests blockchain suite: 8721/0/0 passing.
…t diff

When BAL validation fails on a balance change, the error now prints the
wei diff plus a heuristic that maps it to a multiple of well-known
EIP-8037 state-gas constants (NEW_ACCOUNT, STORAGE_SET, AUTH_BASE,
AUTH_TOTAL) at common test gas_prices. Avoids manual division and
recognition of stride constants when triaging spec/fixture drift.

Also adds a regression test that decodes the exact sender BalanceChange
bytes from the tests-bal@v7.1.0 fixture for
`test_call_value_to_self_destructed_same_tx_account`, so a future
encoding/serde change can't silently shift the parsed `post_balance`.
These entries described the snobal-devnet-6 fixture-vs-impl mismatch:
74 blockchain ef-tests under SKIPPED_BASE and the matching 32 hive
consume-engine test functions under KNOWN_EXCLUDED_TESTS. Both sets
share the same root cause and the same fix: bumping fixtures to bal@v7
and aligning the impl with bal-devnet-7 semantics.

The blockchain skip list was removed in 8e6f3c6 ("unskip 74
bal-devnet-6 Amsterdam fixtures"); the corresponding docs and hive
exclusions were left behind. After the v7.1.0 fixture bump and a clean
2,145/2,145 hive `bal` run, neither set of tests fails anymore.

Keep the stateless / EIP-8025 zkevm@v0.3.3 entry in known_issues.md
(still valid; zkevm bundle is pre-bal-7).
Keep the three flaky hive-framework `Invalid Missing Ancestor` patterns
in check-hive-results.sh (unrelated to bal-devnet anything).
ethrex ab15f37 hardened the HTTP JSON-RPC defaults, gating
`admin_*`/`debug_*`/`txpool_*` behind a new `--http.api` allowlist.
Hive c4d839b3 (#1485) feature-detects the flag in `clients/ethrex/
ethrex.sh` and opts those namespaces back in. The pinned version
`1c27560a` predates that fix, so hive runs against current main start
ethrex without the namespace allowlist and any test touching the
gated namespaces gets `MethodNotFound`.

Bump to current hive HEAD (3b6bde03), which includes the fix plus a
Cargo.lock dependency-drift safeguard (#1492).
Mirrors EELS PR #2848 (tests-bal@v7.1.1, devnets/bal/7):
ethereum/execution-specs#2848

PR #2836 already refunded the 23-byte AUTH_BASE portion when the
authority's pre-state code slot held a delegation indicator. #2848
broadens the condition: when the auth is a clear (`auth.address ==
0x00`), no new indicator bytes are written regardless of pre-state
code, so the AUTH_BASE refill applies even against an authority with
no prior code.

Equivalent to EELS:
    if authority_account.code_hash != EMPTY_CODE_HASH
       or auth.address == NULL_ADDRESS:
        refund = STATE_BYTES_PER_AUTH_BASE * COST_PER_STATE_BYTE
Picks up EELS PR #2848 (refund AUTH_BASE on delegation clear),
implemented in the previous commit. Eels_commit bumped to bcb8dc5f8
(tip of tests-bal@v7.1.1).
@github-actions

github-actions Bot commented May 13, 2026

Copy link
Copy Markdown

⚠️ Known Issues — intentionally skipped tests

Source: docs/known_issues.md

Known Issues

Tests intentionally excluded from CI. Source of truth for the Known
Issues
section the L1 workflow appends to each ef-tests job summary
and posts as a sticky PR comment.

EF Tests — Stateless coverage narrowed to EIP-8025 optional-proofs

make -C tooling/ef_tests/blockchain test calls test-stateless-zkevm
instead of test-stateless. The zkevm@v0.3.3 fixtures are filled against
bal@v5.6.1, out of sync with current bal spec; the broad target trips ~549
fixtures. Re-broaden once the zkevm bundle is regenerated.

Why and resolution path

PR #6527 broadened
test-stateless to extract the entire for_amsterdam/ tree from the
zkevm bundle and run all of it under --features stateless; combined with
this branch's bal-devnet-7 semantics that scope produces ~549
GasUsedMismatch / ReceiptsRootMismatch /
BlockAccessListHashMismatch failures.

test-stateless-zkevm filters cargo to the eip8025_optional_proofs
suite, which still validates the stateless harness without the bal-version
mismatch.

Re-broaden by switching test: back to test-stateless in
tooling/ef_tests/blockchain/Makefile once the zkevm bundle is regenerated
against the current bal spec.

@github-actions github-actions Bot added the L1 Ethereum client label May 13, 2026
@github-actions

github-actions Bot commented May 13, 2026

Copy link
Copy Markdown

Lines of code report

Total lines added: 261
Total lines removed: 129
Total lines changed: 390

Detailed view
+-----------------------------------------------------+-------+------+
| File                                                | Lines | Diff |
+-----------------------------------------------------+-------+------+
| ethrex/crates/blockchain/blockchain.rs              | 2546  | +26  |
+-----------------------------------------------------+-------+------+
| ethrex/crates/common/types/block_access_list.rs     | 1152  | +45  |
+-----------------------------------------------------+-------+------+
| ethrex/crates/l2/sequencer/block_producer.rs        | 324   | +1   |
+-----------------------------------------------------+-------+------+
| ethrex/crates/l2/sequencer/l1_committer.rs          | 1371  | +1   |
+-----------------------------------------------------+-------+------+
| ethrex/crates/vm/backends/levm/mod.rs               | 2448  | +59  |
+-----------------------------------------------------+-------+------+
| ethrex/crates/vm/backends/mod.rs                    | 335   | +96  |
+-----------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/call_frame.rs             | 382   | -8   |
+-----------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/constants.rs              | 64    | +1   |
+-----------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/execution_handlers.rs     | 163   | +1   |
+-----------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/gas_cost.rs               | 876   | +1   |
+-----------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/hooks/default_hook.rs     | 494   | -43  |
+-----------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/opcode_handlers/system.rs | 985   | -78  |
+-----------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/utils.rs                  | 631   | +26  |
+-----------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/vm.rs                     | 740   | +4   |
+-----------------------------------------------------+-------+------+

@edg-l edg-l moved this to In Progress in ethrex_l1 May 13, 2026
@github-actions

github-actions Bot commented May 13, 2026

Copy link
Copy Markdown

Benchmark Results Comparison

Benchmark Results: MstoreBench

Command Mean [s] Min [s] Max [s] Relative
main_revm_MstoreBench 302.9 ± 89.5 267.8 556.7 1.31 ± 0.39
main_levm_MstoreBench 234.2 ± 7.1 231.2 254.3 1.01 ± 0.03
pr_revm_MstoreBench 270.3 ± 1.3 268.0 272.1 1.17 ± 0.01
Detailed Results

Benchmark Results: BubbleSort

Command Mean [s] Min [s] Max [s] Relative
main_revm_BubbleSort 3.282 ± 0.059 3.222 3.396 1.12 ± 0.02
main_levm_BubbleSort 2.921 ± 0.026 2.901 2.968 1.00
pr_revm_BubbleSort 3.302 ± 0.060 3.237 3.443 1.13 ± 0.02
pr_levm_BubbleSort 2.949 ± 0.073 2.890 3.093 1.01 ± 0.03

Benchmark Results: ERC20Approval

Command Mean [s] Min [s] Max [s] Relative
main_revm_ERC20Approval 1.069 ± 0.027 1.042 1.120 1.01 ± 0.03
main_levm_ERC20Approval 1.108 ± 0.011 1.094 1.125 1.05 ± 0.01
pr_revm_ERC20Approval 1.054 ± 0.009 1.043 1.072 1.00
pr_levm_ERC20Approval 1.107 ± 0.010 1.096 1.124 1.05 ± 0.01

Benchmark Results: ERC20Mint

Command Mean [ms] Min [ms] Max [ms] Relative
main_revm_ERC20Mint 143.3 ± 1.1 142.1 145.8 1.00 ± 0.01
main_levm_ERC20Mint 167.3 ± 1.2 165.9 169.5 1.17 ± 0.01
pr_revm_ERC20Mint 142.7 ± 1.1 141.5 144.7 1.00
pr_levm_ERC20Mint 170.0 ± 4.7 166.4 181.7 1.19 ± 0.03

Benchmark Results: ERC20Transfer

Command Mean [ms] Min [ms] Max [ms] Relative
main_revm_ERC20Transfer 251.8 ± 3.4 248.5 259.4 1.01 ± 0.02
main_levm_ERC20Transfer 277.1 ± 1.8 275.0 280.9 1.11 ± 0.01
pr_revm_ERC20Transfer 249.6 ± 2.9 247.6 257.2 1.00
pr_levm_ERC20Transfer 276.9 ± 1.2 274.9 279.1 1.11 ± 0.01

Benchmark Results: Factorial

Command Mean [ms] Min [ms] Max [ms] Relative
main_revm_Factorial 223.2 ± 1.9 221.9 228.4 1.00 ± 0.01
main_levm_Factorial 247.3 ± 2.5 244.7 252.4 1.11 ± 0.01
pr_revm_Factorial 223.1 ± 0.5 222.3 224.2 1.00
pr_levm_Factorial 247.9 ± 5.2 245.6 262.6 1.11 ± 0.02

Benchmark Results: FactorialRecursive

Command Mean [s] Min [s] Max [s] Relative
main_revm_FactorialRecursive 1.631 ± 0.049 1.532 1.684 1.00
main_levm_FactorialRecursive 9.902 ± 0.115 9.667 10.078 6.07 ± 0.20
pr_revm_FactorialRecursive 1.640 ± 0.027 1.590 1.690 1.01 ± 0.03
pr_levm_FactorialRecursive 9.950 ± 0.108 9.791 10.119 6.10 ± 0.20

Benchmark Results: Fibonacci

Command Mean [ms] Min [ms] Max [ms] Relative
main_revm_Fibonacci 211.3 ± 3.3 206.3 219.4 1.00
main_levm_Fibonacci 233.5 ± 29.2 217.5 308.1 1.10 ± 0.14
pr_revm_Fibonacci 212.0 ± 0.9 210.5 213.2 1.00 ± 0.02
pr_levm_Fibonacci 220.2 ± 3.8 217.4 230.4 1.04 ± 0.02

Benchmark Results: FibonacciRecursive

Command Mean [ms] Min [ms] Max [ms] Relative
main_revm_FibonacciRecursive 863.8 ± 13.1 845.2 882.8 1.24 ± 0.03
main_levm_FibonacciRecursive 709.4 ± 34.8 681.4 793.4 1.02 ± 0.05
pr_revm_FibonacciRecursive 864.7 ± 10.0 846.5 881.8 1.24 ± 0.02
pr_levm_FibonacciRecursive 696.4 ± 10.5 683.7 718.2 1.00

Benchmark Results: ManyHashes

Command Mean [ms] Min [ms] Max [ms] Relative
main_revm_ManyHashes 9.2 ± 0.1 9.0 9.4 1.00 ± 0.02
main_levm_ManyHashes 10.6 ± 0.1 10.5 10.7 1.16 ± 0.02
pr_revm_ManyHashes 9.1 ± 0.1 9.0 9.4 1.00
pr_levm_ManyHashes 10.6 ± 0.1 10.5 10.7 1.16 ± 0.02

Benchmark Results: MstoreBench

Command Mean [ms] Min [ms] Max [ms] Relative
main_revm_MstoreBench 302.9 ± 89.5 267.8 556.7 1.31 ± 0.39
main_levm_MstoreBench 234.2 ± 7.1 231.2 254.3 1.01 ± 0.03
pr_revm_MstoreBench 270.3 ± 1.3 268.0 272.1 1.17 ± 0.01
pr_levm_MstoreBench 231.3 ± 1.4 229.4 234.2 1.00

Benchmark Results: Push

Command Mean [ms] Min [ms] Max [ms] Relative
main_revm_Push 315.1 ± 2.3 313.0 319.9 1.10 ± 0.01
main_levm_Push 287.9 ± 2.9 285.3 294.5 1.00 ± 0.01
pr_revm_Push 314.5 ± 1.0 313.1 315.7 1.10 ± 0.01
pr_levm_Push 286.7 ± 1.8 284.7 290.8 1.00

Benchmark Results: SstoreBench_no_opt

Command Mean [ms] Min [ms] Max [ms] Relative
main_revm_SstoreBench_no_opt 179.3 ± 1.7 177.6 183.7 1.56 ± 0.02
main_levm_SstoreBench_no_opt 116.0 ± 3.6 114.0 125.9 1.01 ± 0.03
pr_revm_SstoreBench_no_opt 180.0 ± 4.3 177.3 192.1 1.56 ± 0.04
pr_levm_SstoreBench_no_opt 115.2 ± 1.0 113.8 116.4 1.00

Two hand-rolled tests asserting specific `gas_used` values for
top-level CREATE-tx halt scenarios. Both encoded "Policy A" semantics
hard-coded to v7.0.0 numbers; the second one drifted after EELS
PR #2823's intrinsic NEW_ACCOUNT refund and again after the v7.1.x
spec series. ef-tests at tests-bal@v7.1.1 (2,145/2,145 green) cover
the same scenarios with fewer footguns, and the bespoke "Policy A"
shorthand wasn't carrying its weight.
@edg-l edg-l force-pushed the bal-devnet-7-pr branch from 1e11c8a to 1a25700 Compare May 13, 2026 16:57
@edg-l edg-l marked this pull request as ready for review May 14, 2026 10:23
@edg-l edg-l requested review from a team, ManuelBilbao, avilagaston9 and ilitteri as code owners May 14, 2026 10:23
@ethrex-project-sync ethrex-project-sync Bot moved this from In Progress to In Review in ethrex_l1 May 14, 2026
@github-actions

Copy link
Copy Markdown

🤖 Kimi Code Review

This PR updates the Amsterdam fork implementation from bal-devnet-6 to bal-devnet-7 (tests-bal@v7.1.1), incorporating EIP-8037 gas accounting changes, EIP-7702 refinements, and improved observability. The changes align the implementation with the latest execution-specs (EELS) commits.

Critical Consensus Code Review

Gas Accounting Constants (EIP-8037)crates/vm/levm/src/gas_cost.rs:165-174
The constant updates correctly reflect the bal-devnet-7 recalibration:

  • STATE_BYTES_PER_NEW_ACCOUNT: 112 → 120
  • STATE_BYTES_PER_STORAGE_SET: 32 → 64
  • STATE_BYTES_PER_AUTH_BASE: 23 (new)
  • cost_per_state_byte: 1174 → 1530

System Call Reservoircrates/vm/levm/src/utils.rs:480-490
The implementation correctly grants system transactions (EIP-2935, EIP-4788) an extra state-gas reservoir of STATE_BYTES_PER_STORAGE_SET * cpsb * 16 on top of the 30M regular budget. This prevents SSTORE-heavy system contracts from OOG-ing on state-gas growth alone.

CREATE Collision Handlingcrates/vm/levm/src/execution_handlers.rs:131-139 & crates/vm/levm/src/hooks/default_hook.rs:146-183
The collision logic now correctly:

  1. Sets gas_remaining = 0 to ensure full forwarded gas is consumed
  2. Preserves the reservoir (does not burn it)
  3. Applies new_account_refund to both reservoir and state_refund per EELS PR feat(l1): backward cap compatibility #2823

Top-Level Failure Policycrates/vm/levm/src/vm.rs:943-1017
The simplified "Policy A" implementation correctly handles REVERT, ExceptionalHalt, and OOG uniformly:

  • Refills reservoir with execution portion of state-gas
  • Applies CREATE-tx intrinsic refund for all failure modes (including collision)
  • Removes the complex reclassification logic from PR Bring nodes to memory on cache miss #2689 that was causing divergence

State Refund Channelcrates/vm/levm/src/vm.rs:498-505
The new state_refund field correctly implements EELS PR #2816, separating EIP-7702 existing-authority refunds from per-frame state_gas_refund_absorbed/pending. This survives revert/halt/OOG as required.

AUTH_BASE Refundcrates/vm/levm/src/utils.rs:374-393
Correctly implements PR #2836/#2848: refunds STATE_BYTES_PER_AUTH_BASE * cpsb when no new delegation indicator bytes are written (overwrite case or clear-to-empty case).

Code Quality & Safety

Error Handling & Observabilitycrates/blockchain/blockchain.rs:380-387, crates/vm/backends/mod.rs:360-395
The addition of log_gas_used_mismatch with per-transaction gas breakdowns (regular vs state vs refunded) significantly improves debugging for consensus failures. The structured logging includes transaction indices, hashes, and dimensional splits.

Memory Safetycrates/vm/levm/src/call_frame.rs:310-321
Clean removal of obsolete snapshot fields (state_gas_spill_snapshot, regular_gas_reclassified_snapshot) reduces CallFrame memory footprint and eliminates stale state risks.

Arithmetic Safetycrates/vm/levm/src/hooks/default_hook.rs:254-280
Consistent use of saturating_sub and checked_add prevents overflow/underflow in gas accounting. The subtraction order in refund_sender correctly implements the EELS formula: state_gas = intrinsic + execution - execution_refund - state_refund.

Testing & Fixtures

Fixture Updatetooling/ef_tests/blockchain/.fixtures_url_amsterdam, .github/config/hive/amsterdam.yaml
Updated to tests-bal@v7.1.1 (execution-specs devnets/bal/7). The removal of 74 skipped tests from SKIPPED_BASE and 32 excluded hive tests indicates the implementation now passes the previously failing Amsterdam fixtures.

BAL Decoder Testcrates/common/types/block_access_list.rs:1596-1677
The decode_v7_1_0_sender_balance_change test provides a valuable sanity check for RLP decoding correctness against known fixture bytes.

Removed Obsolete Teststest/tests/levm/eip8037_top_level_failure_tests.rs
Appropriate removal of ethrex-specific divergence guards now that the implementation aligns with EELS Policy A.

Minor Observations

  1. Debug Helpercrates/vm/backends/levm/mod.rs:2854-2898
    The describe_balance_diff function is an excellent debugging aid that categorizes balance mismatches as multiples of well-known state-gas constants.

  2. System Call Reservoir Initializationcrates/vm/levm/src/utils.rs:488-489
    While the code sets state_gas_reservoir = sys_reservoir (replacing any previous value), this is correct because system calls do not carry EIP-7702 auth tuples, so the previous value would be 0. The comment clarifies this is additive relative to the regular gas limit, not the reservoir.

Conclusion

The PR correctly implements the bal-devnet-7 specification


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

@github-actions

Copy link
Copy Markdown

🤖 Claude Code Review

Now I have a thorough picture of the PR. Let me write the review.


PR #6653feat(l1): support bal-devnet-7 (bal@v7.1.1) — Code Review

Overall Assessment

This is a large, well-scoped spec-bump PR. The core algorithmic change — replacing the "PR #2689 reclassification" model with Policy A (EELS PR #2815) — dramatically simplifies the EIP-8037 state-gas logic, and the diff justifies that simplification with 2,145/2,145 tests passing. The code is well-commented with EELS cross-references throughout. A handful of issues are worth addressing before merge.


Medium Concerns

1. System-call reservoir: assignment vs. comment says "additive"

crates/vm/levm/src/utils.rs, end of the Amsterdam fork block in prepare_execution:

// This is additive, not pre-consumed from gas_remaining.
if self.env.is_system_call {
    let sys_reservoir = self
        .state_gas_storage_set
        .saturating_mul(SYSTEM_MAX_SSTORES_PER_CALL);
    self.state_gas_reservoir = sys_reservoir;       // ← assignment, not +=
    self.state_gas_reservoir_initial = sys_reservoir;
}

The surrounding comment says "ON TOP of the 30M regular budget" and "additive, not pre-consumed from gas_remaining", yet the code uses = (overwrite) not +=.

For system calls, the general-path reservoir is gas_limit - intrinsic_state_gas_charged = SYS_CALL_GAS_LIMIT - 0 = 30_000_000. This block then replaces that 30M with ~1.5M (16 SSTOREs). If the intent is truly "additive" to the 30M budget, the code should be += — but that would give a 31.5M reservoir, which doesn't match "extra reservoir for system calls that are SSTORE-heavy."

The likely intent is correct (a small dedicated state-gas buffer, not an addition to 30M), but the comment is misleading. "Additive" here means the state-gas reservoir is in addition to — not deducted from — the regular gas budget, not that it adds to the current reservoir value. Please clarify the comment to avoid future confusion, e.g.:

This replaces the default reservoir (which inherits the full 30M regular budget) with a small dedicated state-gas budget. "Additive" means the reservoir is independent of gas_remaining, not that this value is added to the existing reservoir.

2. state_gas_spill_outstanding snapshots now silently discarded

crates/vm/levm/src/opcode_handlers/system.rs, both handle_return_call and the CREATE return handler:

let _ = state_gas_spill_outstanding_snapshot;

In the old code these snapshots were restored on ExceptionalHalt. Under Policy A they're deliberately not restored (let _ = ... is intentional). However, state_gas_spill_outstanding is still a live field on VM and is still read inside credit_state_gas_refund to track how much spill can be credited. If the snapshot is not restored across halts, the field accumulates lifetime spill from halted subtrees. Please confirm this is either: (a) acceptable under Policy A because state_gas_spill_outstanding is no longer used in any formula after this PR, or (b) that the non-restoration is intentional and tested. If (a), consider removing the field entirely to avoid misleading state.


Minor / Low-Risk

3. is_multiple_of requires Rust ≥ 1.86

crates/vm/backends/levm/mod.rs, describe_balance_diff:

if !mag_u128.is_multiple_of(gp) {
    continue;
}

u128::is_multiple_of was stabilized in Rust 1.86.0 (2025-03-13). If the project's MSRV predates that, this will fail to compile on stable. The equivalent mag_u128 % gp == 0 works on all stable versions. Please check the MSRV or replace with the modulo check.

4. Misleading sign convention in describe_balance_diff

crates/vm/backends/levm/mod.rs:

let (sign, mag) = if expected >= actual {
    ("+", expected - actual)
} else {
    ("-", actual - expected)
};

"+" means expected > actual (BAL expects more than exec produced). When this appears in the mismatch error message alongside "BAL={expected}, exec={actual}", reading "+X wei" as "we are over by X" is backwards from usual convention. A brief comment clarifying the sign convention (e.g., "positive = exec under-produced relative to BAL") would prevent misreadings during incident triage.

5. Unused-variable style in destructure arms

crates/vm/levm/src/opcode_handlers/system.rs, in both revert arms:

let _ = state_gas_credit_against_drain_snapshot;
let _ = state_gas_reservoir_snapshot;
let _ = state_gas_spill_outstanding_snapshot;

The idiomatic Rust way to suppress unused-variable warnings is to prefix the binding in the pattern with _:

_state_gas_credit_against_drain_snapshot,
_state_gas_reservoir_snapshot,
_state_gas_spill_outstanding_snapshot,

The current style (let _ = after the destructure) is valid but unusual and will draw questions from readers. If the intent is to keep the names visible to document that they exist but are now unused, a brief comment in the destructure would serve better.

6. Very long inline byte-layout comments in the BAL decode test

crates/common/types/block_access_list.rs, decode_v7_1_0_sender_balance_change:

The ~20-line comment block inside the test (lines 188–208 in the diff) explaining hex byte layout interleaved with corrections ("Wait, let me recount the inner:") reads as a working scratchpad. It's fine to leave a compact byte-layout table, but the self-corrections should be trimmed before merge so the test remains clean reference material.


Things That Look Correct

  • The Policy A simplification in handle_return_call / CREATE return handler is significantly cleaner than the old reclassification logic. The unified child_net_used = used_delta - absorbed_delta formula is easy to verify against EELS.
  • state_refund field lifetime (survives revert/halt/OOG because it's on VM, not in call-frame backup) is handled correctly and matches the described EELS MessageCallOutput.state_refund semantics.
  • The CREATE collision path correctly zeroes gas_remaining before returning the collision result (execution_handlers.rs) so raw_consumed = gas_limit downstream in refund_sender. The sequencing with finalize_execution's state_gas_refund_absorbed bump (adding new_account_refund) and the hook's exec_refund computation is consistent.
  • Removal of apply_same_tx_selfdestruct_state_refund per EELS PR refactor(l1): move hash from Block to BlockHeader #2845 is clean; the old function touched live accounting fields and its removal simplifies finalization.
  • TxGasBreakdown / log_gas_used_mismatch are purely diagnostic and carry no consensus risk.
  • The 21 newly-added EIP-8025 skips are correctly scoped to the zkevm@v0.3.3 bundle with a clear resolution path.
  • SYSTEM_MAX_SSTORES_PER_CALL = 16 and the updated STATE_BYTES_PER_* / cost_per_state_byte constants match the referenced EELS PRs.

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

@greptile-apps

greptile-apps Bot commented May 14, 2026

Copy link
Copy Markdown

Greptile Summary

This PR aligns ethrex's EIP-8037 state-gas implementation with the bal-devnet-7 specification (EELS PRs #2815#2848), bumps Amsterdam fixtures from snobal-devnet-6@v1.1.0 to bal@v7.1.1, and upgrades the pinned hive version. The core change replaces the previous halt-reclassification/spill formula with a unified Policy A reservoir refill, and adds a new state_refund channel that mirrors EELS MessageCallOutput.state_refund for block-level accounting.

  • State-gas constant recalibration: NEW_ACCOUNT 112→120 bytes, STORAGE_SET 32→64 bytes, AUTH_TOTAL 135→143 bytes, cost_per_state_byte 1174→1530; adds STATE_BYTES_PER_AUTH_BASE = 23 and the system-call extra state-gas reservoir (SYSTEM_MAX_SSTORES_PER_CALL = 16).
  • Failure path rewrite: removes regular_gas_reclassified / state_gas_reservoir_at_top_message_entry fields and the SELFDESTRUCT same-tx refund; collision handling now refunds unused gas to the sender instead of burning the full gas limit; cross-frame revert adopts the child_net_used = used_delta − absorbed_delta formula.
  • Tooling / CI: per-tx gas-dimension breakdown logged on block_gas_used mismatch; 74 formerly-skipped Amsterdam fixtures now pass; 21 stale EIP-8025 fixtures added to the skip list; hive bumped past the --http.api detection fix.

Confidence Score: 3/5

The accounting changes are extensive and spec-aligned, but the system-call reservoir initialization overwrites state_gas_reservoir without undoing the earlier gas_remaining decrement, leaving system contracts with ~16.7M regular gas instead of ~30M.

The system-call gas budget discrepancy is a concrete defect introduced in this PR: gas_remaining is reduced by ~13.2M during the regular reservoir path and never restored before the sys_reservoir override, so EIP-2935 and EIP-4788 system contracts start execution with roughly half the regular gas the spec intends. Current system contracts are simple enough that they won't OOG at 16.7M, explaining why all 2145 tests pass — but any future or more complex system contract could hit this limit. The rest of the PR is well-commented, aligns with the cited EELS PRs, and is backed by extensive passing ef-tests.

crates/vm/levm/src/utils.rs — system-call reservoir override block at the end of add_intrinsic_gas

Important Files Changed

Filename Overview
crates/vm/levm/src/utils.rs Adds set_delegation refund channels and system-call extra reservoir; the sys-reservoir override doesn't undo the prior gas_remaining pre-consumption, leaving system calls with ~16.7M instead of ~30M regular gas.
crates/vm/levm/src/vm.rs Major rework of EIP-8037 top-level failure path: replaces halt-reclassification/spill logic with unified Policy A reservoir refill; removes regular_gas_reclassified and state_gas_reservoir_at_top_message_entry; adds state_refund and state_gas_auth_base.
crates/vm/levm/src/hooks/default_hook.rs Rewrites collision handling (sender now gets a refund for unused gas), removes same-tx SELFDESTRUCT state-gas refund, and drops the reclassification term from refund_sender.
crates/vm/levm/src/opcode_handlers/system.rs Cross-frame revert logic simplified to new EELS PR #2823 formula; removes the spill/reclassify split between REVERT and ExceptionalHalt.
crates/vm/levm/src/gas_cost.rs Updates EIP-8037 state-byte constants for bal-devnet-7 and bumps cost_per_state_byte from 1174 to 1530; adds STATE_BYTES_PER_AUTH_BASE = 23.
crates/vm/backends/mod.rs Adds TxGasBreakdown/TxStatus structs and log_gas_used_mismatch for per-tx gas-dimension diagnosis.
crates/vm/backends/levm/mod.rs Threads TxGasBreakdown through all three execution paths and adds describe_balance_diff for diagnostic error messages.
crates/blockchain/blockchain.rs Wraps validate_gas_used calls to emit per-tx gas breakdown on mismatch before returning the error.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[TX enters finalize_execution] --> B{Amsterdam fork?}
    B -- No --> Z[Report: gas_used unchanged]
    B -- Yes --> C{tx success?}
    C -- Yes --> Z
    C -- No --> D{is_collision?}
    D -- No: Policy A --> E[execution_portion = state_gas_used - intrinsic - absorbed - pending]
    E --> F[state_gas_refund_absorbed += execution_portion\nstate_gas_reservoir += execution_portion]
    D -- Yes --> G[skip execution refund]
    F --> H{is_create tx?}
    G --> H
    H -- Yes --> I[new_account_refund added to reservoir + refund_absorbed]
    H -- No --> J[refund_sender]
    I --> J
    J --> K{is_collision? hook}
    K -- Yes --> L[regular_gas = gas_limit - state_gas_reservoir\ngas_spent = effective_regular\nreturn remainder to sender]
    K -- No --> M[state_gas = state_gas_used - exec_refund - state_refund\nregular_gas = raw - intrinsic - reservoir_initial - spill]
Loading
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
crates/vm/levm/src/utils.rs:490-496
**System-call `gas_remaining` pre-consumed but not restored before `sys_reservoir` override**

`add_intrinsic_gas` runs the regular reservoir path first: for a system call with `gas_limit = SYS_CALL_GAS_LIMIT + TX_BASE_COST = 30,021,000`, the computed `reservoir ≈ 13,243,784`, so `gas_remaining` is decremented by that amount (down to ~16.7M). The block immediately after then overwrites `state_gas_reservoir` with `sys_reservoir ≈ 1,566,720`, but never restores the `gas_remaining` deduction — leaving the system call with ~16.7M regular gas instead of the intended ~30M. The fix is to guard the regular-path reservoir computation with `!self.env.is_system_call`, or to undo the `gas_remaining` decrement before setting `sys_reservoir`.

Reviews (1): Last reviewed commit: "chore(test): drop eip8037_top_level_fail..." | Re-trigger Greptile

Comment on lines +490 to +496
if self.env.is_system_call {
let sys_reservoir = self
.state_gas_storage_set
.saturating_mul(SYSTEM_MAX_SSTORES_PER_CALL);
self.state_gas_reservoir = sys_reservoir;
self.state_gas_reservoir_initial = sys_reservoir;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 System-call gas_remaining pre-consumed but not restored before sys_reservoir override

add_intrinsic_gas runs the regular reservoir path first: for a system call with gas_limit = SYS_CALL_GAS_LIMIT + TX_BASE_COST = 30,021,000, the computed reservoir ≈ 13,243,784, so gas_remaining is decremented by that amount (down to ~16.7M). The block immediately after then overwrites state_gas_reservoir with sys_reservoir ≈ 1,566,720, but never restores the gas_remaining deduction — leaving the system call with ~16.7M regular gas instead of the intended ~30M. The fix is to guard the regular-path reservoir computation with !self.env.is_system_call, or to undo the gas_remaining decrement before setting sys_reservoir.

Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/vm/levm/src/utils.rs
Line: 490-496

Comment:
**System-call `gas_remaining` pre-consumed but not restored before `sys_reservoir` override**

`add_intrinsic_gas` runs the regular reservoir path first: for a system call with `gas_limit = SYS_CALL_GAS_LIMIT + TX_BASE_COST = 30,021,000`, the computed `reservoir ≈ 13,243,784`, so `gas_remaining` is decremented by that amount (down to ~16.7M). The block immediately after then overwrites `state_gas_reservoir` with `sys_reservoir ≈ 1,566,720`, but never restores the `gas_remaining` deduction — leaving the system call with ~16.7M regular gas instead of the intended ~30M. The fix is to guard the regular-path reservoir computation with `!self.env.is_system_call`, or to undo the `gas_remaining` decrement before setting `sys_reservoir`.

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

@github-actions

Copy link
Copy Markdown

🤖 Codex Code Review

Findings

  1. The new Amsterdam CREATE-collision refund semantics only exist in default_hook.rs, but VMType::L2 bypasses that path entirely in l2_hook.rs. finalize_non_privileged_execution still derives actual_gas_used from the pre-refund ctx_result.gas_used in l2_hook.rs and then feeds that into refund_sender in l2_hook.rs. If L2 runs Amsterdam semantics, a colliding CREATE will still charge the full pre-refund gas instead of returning the preserved reservoir plus the STATE_BYTES_PER_NEW_ACCOUNT refund this PR adds on L1.

  2. The new gas-mismatch diagnostics add unconditional hot-path hashing. tx.hash() is now called for every executed tx in levm/mod.rs, levm/mod.rs, and levm/mod.rs, while Transaction::hash() still falls back to canonical encoding plus Keccak on first use in transaction.rs. Since those hashes are only used on rare gas_used mismatches, this is avoidable block-import/sync overhead.

I couldn’t run cargo test here because the sandbox blocks Cargo/rustup writes under ~/.cargo and ~/.rustup, and the workspace depends on git-sourced crates.


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

Comment thread crates/vm/levm/src/utils.rs
Comment thread crates/vm/levm/src/utils.rs
Comment thread crates/vm/levm/src/opcode_handlers/system.rs Outdated
Address review comments on bal-devnet-7 PR.

- utils.rs: system calls no longer pre-consume `gas_remaining` for the
  regular-tx reservoir before the system-reservoir override. Gate the
  regular-path reservoir block on `!is_system_call` so system contracts
  get the full SYS_CALL_GAS_LIMIT regular budget plus the dedicated
  state-gas reservoir, matching EELS
  `process_unchecked_system_transaction`.
- call_frame.rs / system.rs: drop `state_gas_reservoir_snapshot` and
  `state_gas_credit_against_drain_snapshot` (Policy A no longer restores
  them on revert). Keep `state_gas_spill_outstanding_snapshot` since
  `credit_state_gas_refund` still reads it from the active frame.
- Refresh surrounding comments to match Policy A wording and remove
  stale PR #2689 / bal-devnet-6 references.
@edg-l edg-l requested a review from a team as a code owner May 15, 2026 11:29
The diagnostic helper is reachable only via the L1 BAL-validation path
(execute_block_parallel → validate_tx_execution). Under the L2 build
profile that path is not visible to per-crate dead-code analysis, so
`cargo clippy --features l2,l2-sql -- -D warnings` errors out.
@edg-l edg-l requested a review from ElFantasma May 15, 2026 13:01
@edg-l edg-l added this pull request to the merge queue May 18, 2026
Merged via the queue into main with commit fee57b1 May 18, 2026
76 of 77 checks passed
@github-project-automation github-project-automation Bot moved this from In Review to Done in ethrex_l1 May 18, 2026
@edg-l edg-l deleted the bal-devnet-7-pr branch May 18, 2026 10:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

L1 Ethereum client

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants