fix(specs,tests): check SSTORE access cost before the implicit read - #3111
Closed
nerolation wants to merge 1 commit into
Closed
nerolation wants to merge 1 commit into
nerolation wants to merge 1 commit into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## forks/amsterdam #3111 +/- ##
================================================
Coverage 93.30% 93.30%
================================================
Files 624 624
Lines 36986 36987 +1
Branches 3383 3383
================================================
+ Hits 34511 34512 +1
Misses 1693 1693
Partials 782 782
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:
|
15 tasks
Contributor
Author
|
Close in favor of #3064 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🗒️ Description
EIP-7928 pre-state validation for
SSTORErequires the slot's access cost to be covered, in addition to the EIP-2200 stipend sentry, before the implicit read of the current storage value. Since EIP-8038 raisedCOLD_STORAGE_ACCESS(3000) aboveCALL_STIPEND(2300), the sentry alone no longer implies the access cost is covered: a coldSSTOREin a frame with 2301–2999 gas passed the sentry, performed the implicit read, and recorded the slot in the block access list before failing incharge_gas. Storage reads survive frame rollback by design, so the unaffordable slot ended up in the BAL and its hash.This PR checks the cold/warm access cost before the implicit read in the Amsterdam
SSTORE, so such a slot is neither read nor recorded. Gas usage, refunds, and post-state are unchanged: the frame runs out of gas either way. The BAL is the only observable difference.Test changes in
test_bal_sstore_and_oog:oog_at_eip2200_stipend_plus_1now expects no BAL entry, previously expected a storage read.oog_at_cold_access_cost_minus_1case: no BAL entry.oog_at_cold_access_costcase: storage read in BAL, implicitSLOADdone, OOG on the write cost.🔗 Related Issues or PRs
EIP-7928 wording update here.
✅ Checklist
All: PR title has the form
<type>(<area>):, where<type>and<area>come from an appropriateC-<type>andA-<area>label. The title should match the target squash commit message.All: Considered updating the online docs in the [./docs/](/ethereum/execution-specs/blob/HEAD/docs/) directory.
All: Set appropriate labels for the changes, only maintainers can apply labels.