feat(l1): support bal-devnet-7 (bal@v7.1.1)#6653
Conversation
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.
**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).
|
Lines of code reportTotal lines added: Detailed view |
Benchmark Results ComparisonBenchmark Results: MstoreBench
Detailed ResultsBenchmark Results: BubbleSort
Benchmark Results: ERC20Approval
Benchmark Results: ERC20Mint
Benchmark Results: ERC20Transfer
Benchmark Results: Factorial
Benchmark Results: FactorialRecursive
Benchmark Results: Fibonacci
Benchmark Results: FibonacciRecursive
Benchmark Results: ManyHashes
Benchmark Results: MstoreBench
Benchmark Results: Push
Benchmark Results: SstoreBench_no_opt
|
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.
🤖 Kimi Code ReviewThis 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 ReviewGas Accounting Constants (EIP-8037) —
System Call Reservoir — CREATE Collision Handling —
Top-Level Failure Policy —
State Refund Channel — AUTH_BASE Refund — Code Quality & SafetyError Handling & Observability — Memory Safety — Arithmetic Safety — Testing & FixturesFixture Update — BAL Decoder Test — Removed Obsolete Tests — Minor Observations
ConclusionThe PR correctly implements the bal-devnet-7 specification Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt |
🤖 Claude Code ReviewNow I have a thorough picture of the PR. Let me write the review. PR #6653 —
|
Greptile SummaryThis PR aligns ethrex's EIP-8037 state-gas implementation with the bal-devnet-7 specification (EELS PRs #2815–#2848), bumps Amsterdam fixtures from
Confidence Score: 3/5The 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
|
| 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]
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
| 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; | ||
| } |
There was a problem hiding this 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.
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.
🤖 Codex Code ReviewFindings
I couldn’t run Automated review by OpenAI Codex · gpt-5.4 · custom prompt |
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.
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.
Motivation
Bring ethrex up to bal-devnet-7 (BAL fixtures
bal@v7.1.1). Stackedon 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.0tobal@v7.1.1,and bumps the pinned hive version past the ethrex
--http.apifix.Main changes:
NEW_ACCOUNT;intrinsic_state_gas_chargedpreserved across the failure path.EELS.
state_gas_usedsubtractsstate_refund.set_delegationrefund via dedicatedstate_refundchannel.set_delegationrefundsAUTH_BASEon existing delegation(EELS PR feat(l2): remove public inputs from verify() function in OnChainProposer #2836).
set_delegationrefundsAUTH_BASEon delegation clear(EELS PR perf(core): make disklayer in snapshots use the database #2848, v7.1.1).
revertdoesn't unmark the account as existing (fix(levm): revert doesn't unmark the account as existing #6592).(fix(levm): account erroneously considered as existing after zero-value transfer #6591).
gas_usedmismatch.bal@v7.1.1.recognised state-gas constant multiples.
bal@v5.7.0(zkevm@v0.3.3 bundle, pre-bal-7).
docs/known_issues.mdand hiveKNOWN_EXCLUDED_TESTS.--http.apiflagfeature-detect fix (
c4d839b3, hive fix(levm): mulmod should not overflow #1485). Without this, hivestarts ethrex with the default HTTP namespace allowlist
(
eth,net,web3) and tests touchingadmin_*/debug_*/txpool_*fail.
Local test run
./run_test.shagainsttests-bal@v7.1.1: 2,145 / 2,145 pass.cargo test -p ethrex-test --tests: 453 / 453 pass.Checklist
STORE_SCHEMA_VERSION(crates/storage/lib.rs) if the PRincludes breaking changes to the
Storerequiring a re-sync.