Skip to content

chore(specs, tests): restore spilled state gas to reservoir on revert/halt#2378

Merged
spencer-tb merged 1 commit intoethereum:eips/amsterdam/eip-8037from
spencer-tb:eips/amsterdam/eip-8037
Mar 6, 2026
Merged

chore(specs, tests): restore spilled state gas to reservoir on revert/halt#2378
spencer-tb merged 1 commit intoethereum:eips/amsterdam/eip-8037from
spencer-tb:eips/amsterdam/eip-8037

Conversation

@spencer-tb
Copy link
Copy Markdown
Contributor

@spencer-tb spencer-tb commented Mar 2, 2026

🗒️ Description

When a child frame's state gas spills from the reservoir into gas_left and then reverts or halts, the spilled state gas was lost. Since state changes are rolled back, all state gas (reservoir + spill) should be restored to the parent's reservoir, not just the original reservoir.

🔗 Related Issues or PRs

N/A.

✅ Checklist

  • All: Ran fast tox checks to avoid unnecessary CI fails, see also Code Standards and Enabling Pre-commit Checks:
    uvx tox -e static
  • All: PR title adheres to the repo standard - it will be used as the squash commit message and should start type(scope):.
  • All: Considered updating the online docs in the ./docs/ directory.
  • All: Set appropriate labels for the changes (only maintainers can apply labels).

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

…/halt

When a child frame's state gas spills from the reservoir into gas_left
and then reverts or halts, the spilled state gas was lost. Since state
changes are rolled back, all state gas (reservoir + spill) should be
restored to the parent's reservoir, not just the original reservoir.
@spencer-tb spencer-tb added A-spec-specs Area: Specification—The Ethereum specification itself (eg. `src/ethereum/*`) C-chore Category: chore A-tests Area: Consensus tests. labels Mar 2, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 2, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (eips/amsterdam/eip-8037@066b8be). Learn more about missing BASE report.

Additional details and impacted files
@@                    Coverage Diff                     @@
##             eips/amsterdam/eip-8037    #2378   +/-   ##
==========================================================
  Coverage                           ?   82.59%           
==========================================================
  Files                              ?      642           
  Lines                              ?    39495           
  Branches                           ?     4062           
==========================================================
  Hits                               ?    32620           
  Misses                             ?     6154           
  Partials                           ?      721           
Flag Coverage Δ
unittests 82.59% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread src/ethereum/forks/amsterdam/vm/__init__.py
@jwasinger
Copy link
Copy Markdown
Contributor

Isn't the state gas spent by a frame not refunded in the case of revert? From the EIP:

State gas charged for account creation (CREATE, CALL to new account, and EOA delegation) is consumed even if the frame reverts — state changes are rolled back but gas is not refunded.

I'm guessing that this also applies to SSTORE as well:

Refunds use refund_counter rather than direct gas accounting decrements, so that reverted frames do not benefit from the refund.

@spencer-tb spencer-tb force-pushed the eips/amsterdam/eip-8037 branch from 43b63ca to eb27582 Compare March 6, 2026 17:24
@spencer-tb
Copy link
Copy Markdown
Contributor Author

Thx @jwasinger. Comments below!

These two EIP quotes are about different items compared to what the PR changes. What this PR changes:

  • When a child frame's state gas spills from the reservoir into gas_left and then reverts/halts, the spilled portion was previously lost. The update restores child_evm.state_gas_used + child_evm.state_gas_left (the full forwarded amount) instead of only the original reservoir.
  • This now matches the updated EIP: "On child revert or exceptional halt, all state gas consumed by the child, both from the reservoir and any that spilled into gas_left, is restored to the parent's reservoir."

@spencer-tb spencer-tb force-pushed the eips/amsterdam/eip-8037 branch from eb27582 to 7ce782a Compare March 6, 2026 17:43
@spencer-tb spencer-tb merged commit 2dd44d2 into ethereum:eips/amsterdam/eip-8037 Mar 6, 2026
25 of 27 checks passed
felix314159 pushed a commit to felix314159/execution-specs that referenced this pull request Apr 14, 2026
spencer-tb added a commit to spencer-tb/execution-specs that referenced this pull request Apr 21, 2026
spencer-tb added a commit to spencer-tb/execution-specs that referenced this pull request Apr 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-spec-specs Area: Specification—The Ethereum specification itself (eg. `src/ethereum/*`) A-tests Area: Consensus tests. C-chore Category: chore

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants