feat(spec-specs, tests): update EIP-8037 to use source based refunds - #2999
Conversation
spencer-tb
added
A-spec-specs
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## forks/amsterdam #2999 +/- ##
================================================
Coverage 93.20% 93.21%
================================================
Files 620 620
Lines 38759 38787 +28
Branches 3341 3341
================================================
+ Hits 36127 36154 +27
Misses 1773 1773
- Partials 859 860 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
8bd822f to
2d4541e
Compare
kclowes
left a comment
kclowes
left a comment
There was a problem hiding this comment.
Those cats 😆
Nice work! I left a couple small comments, but looks good overall!
The comments here and here are wrong, but the tests still pass correctly afaict.
Missing test via Claude - feel free to take or leave:
Combined own-frame + child-propagated spill, then revert: No test covers the case where the parent itself spills state gas AND a successful child propagates its own state_gas_from_gas_left to the parent, and then the parent reverts. The test_top_level_failure_propagated_state_gas covers child-propagated spill only (parent charges no state gas of its own). Adding parent-level charges that also spill would exercise whether the accumulated state_gas_from_gas_left = own_spill + child_spill is correctly returned to gas_left on revert rather than reservoir.
|
Thanks @kclowes! 🙏 Used your rename suggestion, fixed the comments, and added the test scenario by folding it together with the existing |
The header `gas_used` is `max(block regular gas, block state gas)`. After the source-based refund rework (ethereum#2999) and the EIP-8038 state-access repricing, the hand-rolled expectation in `test_sstore_restoration_ancestor_revert` drifted on both terms. The spec produces 157111 (157108 for `DELEGATECALL`) but the test wanted 163911, an excess of 6800. That 6800 is two compensating errors that no longer cancel under the EIP-8038 prices: - The caller's outer `SSTORE(slot0, CALL(probe))` writes the probe's `0` return (the probe OOGs) into a fresh slot, a cold no-op charging `COLD_STORAGE_ACCESS` (3000). `regular_cost` assumed a cold set (`COLD_STORAGE_WRITE`, 13000), over-counting by 10000. - `probe_burned` subtracted `COLD_ACCOUNT_ACCESS + 2 * WARM_ACCESS`. The probe burns its full forwarded budget (`probe_gas`) on the OOG, and the CALL's cold-access surcharge already sits in the caller's regular cost, so the subtraction under-counted by 3200. Net: `+10000 - 3200 = +6800`. The fix: - Annotate the caller's outer SSTORE with no-op metadata (`key_warm=False`, `original_value=current_value=new_value=0`) so `regular_cost` prices it as the cold no-op it is. - Use the full `probe_gas` in place of `probe_burned`. - Drop the `expected_state` SSTORE-set term, since no set persists. Inner's set and clear cancel, middle reverts, the probe OOGs, and the outer SSTORE is a no-op, so block state gas is zero and `gas_used` is the regular total. Verified by reconstructing all three call opcodes from fork-derived components against the t8n output (`CALL` and `CALLCODE` 157111, `DELEGATECALL` 157108). All 154 tests in `test_state_gas_sstore.py` pass.
2 participants
🗒️ Description
Update the EIP-8037 reservoir model so state gas refunds are source based LIFO.
🔗 Related Issues or PRs
Aligns with the second point in ethereum/EIPs#11807.
✅ Checklist
just statictype(scope):.Cute Animal Picture