Skip to content

refactor(spec-specs): credit CREATE refund before child incorporation - #3099

Closed
chfast wants to merge 1 commit into
ethereum:forks/amsterdamfrom
chfast:chore/amsterdam-create-refund-symmetry
Closed

chfast wants to merge 1 commit into
ethereum:forks/amsterdamfrom
chfast:chore/amsterdam-create-refund-symmetry

Conversation

@chfast

@chfast chfast commented Jul 3, 2026

Copy link
Copy Markdown
Member

🗒️ Description

On the CREATE success path, generic_create credited the NEW_ACCOUNT state-gas refund (target-already-alive case) after incorporate_child_on_success, which folds the child's spill into the parent. The refund's LIFO gas_left/reservoir split was therefore computed against the combined parent+child spill, unlike the error path (whose incorporate_child_on_error does not fold the child's spill).

Credit before incorporating the child so the refund reverses only the parent's own NEW_ACCOUNT spill, matching the error path. The split is tx-unobservable: the total refund is preserved, and over-cap frames carry gas_left near TX_MAX_GAS_LIMIT so the routing never gates a downstream charge. Verified by filling the eip8037 suite: 1567 fixtures byte-for-byte identical before and after.

🔗 Related Issues or PRs

N/A.

✅ Checklist

  • All: Ran fast static checks to avoid unnecessary CI fails, see also Code Standards and Verifying Changes:
    just static
  • All: PR title have the form <type>(<area>):, where <type> and <area> come from an approrpriate C-<type>, respectively A-<area>, label. The title should match the a target squash commit message.
  • All: Considered updating the online docs in the ./docs/ directory.
  • All: Set appropriate labels for the changes (only maintainers can apply labels).
  • Tests: For PRs implementing a missed test case, update the post-mortem document to add an entry the list.
  • Ported Tests: Add the following docstring to manually enhanced tests from ./tests/ported_static/:
    @manually-enhanced: Do not overwrite. Post-state expectations corrected
    manually (see PR #2784).
    

On the CREATE success path, generic_create credited the NEW_ACCOUNT
state-gas refund (target-already-alive case) after
incorporate_child_on_success, which folds the child's spill into the
parent. The refund's LIFO gas_left/reservoir split was therefore
computed against the combined parent+child spill, unlike the error path
(whose incorporate_child_on_error does not fold the child's spill).

Credit before incorporating the child so the refund reverses only the
parent's own NEW_ACCOUNT spill, matching the error path. The split is
tx-unobservable: the total refund is preserved, and over-cap frames
carry gas_left near TX_MAX_GAS_LIMIT so the routing never gates a
downstream charge. Verified by filling the eip8037 suite: 1567 fixtures
byte-for-byte identical before and after.

Claude-Session: https://claude.ai/code/session_01Nw3qUNd4aNzVypuzNhbQyg
@codecov

codecov Bot commented Jul 3, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.24%. Comparing base (7603ff6) to head (9a8e8d8).

Additional details and impacted files
@@               Coverage Diff                @@
##           forks/amsterdam    #3099   +/-   ##
================================================
  Coverage            93.24%   93.24%           
================================================
  Files                  624      624           
  Lines                36986    36986           
  Branches              3383     3383           
================================================
  Hits                 34489    34489           
  Misses                1704     1704           
  Partials               793      793           
Flag Coverage Δ
unittests 93.24% <100.00%> (ø)

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

☔ View full report in Codecov by Harness.
📢 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.

@spencer-tb spencer-tb added A-spec-specs Area: Specification—The Ethereum specification itself (eg. `src/ethereum/*`) C-refactor Category: refactor labels Jul 6, 2026
@spencer-tb spencer-tb added this to the Glamsterdam Devnet 7 Finalize milestone Jul 6, 2026
if target_alive:
# Target already existed: no new account, refund the state gas.
# Credit before incorporating the child so the refund reverses
# the parent's own spill only (as the error path does).

@gurukamath gurukamath Jul 13, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If the parent's StateGasCosts.NEW_ACCOUNT charge is covered by the state_gas_reservoir but the child's initcode spills state charges into gas_left and then successfully deploys onto an already-alive address, the two orderings route the refund differently:

  1. The old code credits the child's spill back into gas_left while refunding StateGasCosts.NEW_ACCOUNT
  2. The new code credits everything in StateGasCosts.NEW_ACCOUNT to the reservoir.

Since the GAS opcode excludes the reservoir, this is observable behaviour. Probably needs a test case.

@gurukamath

Copy link
Copy Markdown
Contributor

Also, should we do the refund first in generic_call as well?

@gurukamath gurukamath self-assigned this Jul 14, 2026
chfast added a commit to chfast/execution-specs that referenced this pull request Jul 14, 2026
The NEW_ACCOUNT refund on a successful CREATE onto an alive target is
credited LIFO against the incorporated child spill, landing in the
parent's gas_left where the GAS opcode (which excludes the reservoir)
observes it. Complements test_create_onto_alive_refunds_to_gas_left,
which covers the parent-spill case that is insensitive to the credit
vs incorporation order. The factory stores the gas measured across
the CREATE, pinning the refund routing: reordering the credit before
child incorporation (PR ethereum#3099) shifts the stored value by exactly
NEW_ACCOUNT (183,600).
@chfast

chfast commented Jul 14, 2026

Copy link
Copy Markdown
Member Author

Added test, let's review that one first: #3163.

@chfast
chfast marked this pull request as draft July 14, 2026 08:36
chfast added a commit to chfast/execution-specs that referenced this pull request Jul 14, 2026
The NEW_ACCOUNT refund on a successful CREATE onto an alive target is
credited LIFO against the incorporated child spill, landing in the
parent's gas_left where the GAS opcode (which excludes the reservoir)
observes it. Complements test_create_onto_alive_refunds_to_gas_left,
which covers the parent-spill case that is insensitive to the credit
vs incorporation order. The factory stores the gas measured across
the CREATE, pinning the refund routing: reordering the credit before
child incorporation (PR ethereum#3099) shifts the stored value by exactly
NEW_ACCOUNT (183,600). Parametrized over CREATE and CREATE2.
gurukamath pushed a commit that referenced this pull request Jul 14, 2026
The NEW_ACCOUNT refund on a successful CREATE onto an alive target is
credited LIFO against the incorporated child spill, landing in the
parent's gas_left where the GAS opcode (which excludes the reservoir)
observes it. Complements test_create_onto_alive_refunds_to_gas_left,
which covers the parent-spill case that is insensitive to the credit
vs incorporation order. The factory stores the gas measured across
the CREATE, pinning the refund routing: reordering the credit before
child incorporation (PR #3099) shifts the stored value by exactly
NEW_ACCOUNT (183,600). Parametrized over CREATE and CREATE2.
@gurukamath

Copy link
Copy Markdown
Contributor

Added test, let's review that one first: #3163.

#3163 merged

@chfast chfast closed this Jul 14, 2026
@chfast
chfast deleted the chore/amsterdam-create-refund-symmetry branch July 14, 2026 13:24
@spencer-tb

Copy link
Copy Markdown
Contributor

Accidental close?

@chfast

chfast commented Jul 14, 2026

Copy link
Copy Markdown
Member Author

No. Not equivalent as proven by the test added in #3163.

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/*`) C-refactor Category: refactor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants