fix(evm): GetCommittedState must not return post-mutation value for prestate-absent keys - #4869
Open
envestcc wants to merge 1 commit into
Open
Conversation
…restate-absent keys
`contract.GetState` propagates `trie.ErrNotExist` for a storage key that
is absent in the pre-tx trie without recording the key. If the same key
is later written (via `SetState`) and re-read within the same tx, the
follow-up `GetState` succeeds and populates `committed[key]` with the
just-written value. `GetCommittedState` then returns that post-mutation
value as if it were the tx-start prestate, which is wrong.
EIP-2200's SSTORE dynamic gas uses `committed` to decide whether a write
is a dirty in-place update (SLOAD_GAS, 100) or a full slot reset
(SSTORE_RESET, 2900). When the cache is polluted, a second write to a
previously-absent slot is charged 2800 extra gas per SSTORE. In the ioID
device registration flow — which mints an ERC-721 and then transfers it
to an ERC-6551 token-bound account, hitting `_owners[tokenId]` twice
inside a single tx — this pushes execution over the gas limit and
reverts (mainnet tx 0xfe792b0c... at block 48,900,885 and subsequent
device registrations, all reverting since 2026-04-28).
Fix: track prestate-absent keys in a `missing` map alongside `committed`.
GetCommittedState short-circuits missing keys to `trie.ErrNotExist`
without touching the live trie; GetState records missing observations
and does not overwrite prestate tracking with a live-trie value once a
key has been observed missing; SetState skips the priming GetState call
for keys already classified as missing.
Because this is a state-transition semantics change, the new behavior is
gated on a new feature flag `CorrectPrestateForAbsentKeys` wired to
`g.IsToBeEnabled(height)` (staged, no scheduled fork height yet). Prior
to the fork height the historical (buggy) code path is preserved byte
for byte, so catch-up from mainnet state pre-fork replays identically.
Post-fork, the fix takes effect.
Plumbing:
- `FeatureCtx.CorrectPrestateForAbsentKeys` in `action/protocol/context.go`
- `CorrectPrestateForAbsentKeysOption` on `StateDBAdapter`
- `newContract` / `newContractAdapter` take an explicit `trackAbsent`
bool, propagated through `Snapshot`
- `evm.go` `prepareStateDBAdapter` toggles the option from the
feature ctx.
Test:
`TestGetCommittedStateAbsentKey` now covers both fork sides:
- post-fork (trackAbsent=true): repro pattern must return the true
prestate ErrNotExist for GetCommittedState.
- pre-fork (trackAbsent=false): the buggy behavior is preserved
exactly — GetCommittedState returns the post-mutation value.
This guards both the fix and the pre-fork determinism required for
mainnet catch-up.
envestcc
force-pushed
the
fix/committed-state-post-mutation-pollution
branch
from
July 8, 2026 11:52
9ae1b33 to
6c57f1e
Compare
|
envestcc
added a commit
that referenced
this pull request
Aug 17, 2026
Brings rc_2.5.0's IIP-59 work up to the current state of #4953 (head 1c9a36b), squashing the increment since the previously integrated head 82d342d. Main content is the PR's refactoring pass: recompute voter rewards from frozen buckets, separate voter reward responsibilities, simplify the drain pagination / settlement state plumbing, collapse the epoch drain cursor, persist candidate reward opt-in, and cap voter reward chunks at 256. Notes: - The PR already forward-ported the rc-only work from #4968 (exported Unpack/ABI/Topic0 surface plus unpack_test.go) and #4969 (event rename), so the 16 conflicts were all "both sides changed the same hunk" and were resolved to the PR side. Verified afterwards that the distributedlog exported surface and unpack_test.go survive. - Three commits already cherry-picked onto rc_2.5.0 (-stop-at-height, state write queue dump, fund-before-sentinel write order) merged without conflict, being byte-identical. - The PR's refactor dropped the exported NewCandidateStateManager in favour of NewCandidateStateManagerWithContext. #4854's handlers_blspop_test.go still called the old constructor; updated its three call sites to match the surrounding idiom. - go.mod moves from the v0.6.11 pseudo-version to the tagged iotex-proto v0.6.12, which contains blsPop. - Carries master's #4972 (actpool keccak bundle hash) because the PR merged master in. Feature gates from #4854 (EnforceBLSPoP) and #4869 (CorrectPrestateForAbsentKeys) verified intact. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
envestcc
added a commit
that referenced
this pull request
Aug 17, 2026
Doc-comment list reindentation and declaration column alignment, as produced by the repo's own `make fmt` (which `make test` depends on). No semantic change -- `git diff -w` against the parent is empty. Keeps `make test` from leaving a dirty tree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
envestcc
added a commit
that referenced
this pull request
Aug 17, 2026
Next fork after Yap, continuing the island naming sequence at Z. Moves the four features that were sitting on the ToBeEnabledBlockHeight placeholder onto ZanzibarBlockHeight: - FixInContractTransferLogTopic - NoVoterRewardDistribution (IIP-59, gated as !IsZanzibar) - EnforceBLSPoP (#4854) - CorrectPrestateForAbsentKeys (#4869) ToBeEnabledBlockHeight stays as the placeholder for the next round of WIP features, per the convention documented on the field. Also moved: the staking owner-index backfill, which fires on the single block equal to the activation height rather than on a range, and the IIP-59 genesis validation (ordering against Greenland and Xingu, and the epochsPerRewardEra >= 2 check), both of which keyed off the placeholder. Mainnet default is MaxUint64 -- unscheduled. Testnet is scheduled in iotex-bootstrap's genesis_testnet.yaml. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Summary
contract.GetStatepropagatestrie.ErrNotExistfor a storage key that is absent in the pre-tx trie without recording the observation. If the same key is later written (viaSetState) and re-read within the same tx, the follow-upGetStatesucceeds and populatescommitted[key]with the just-written value.GetCommittedStatethen returns that post-mutation value as if it were the tx-start prestate.EIP-2200's SSTORE dynamic gas uses
committedto decide whether a write is a dirty in-place update (SLOAD_GAS, 100) or a full slot reset (SSTORE_RESET, 2900). With the polluted cache, the second write to a previously-absent slot is charged 2800 extra gas per SSTORE. In an ERC-721 mint immediately followed by a transfer within the same tx — for example an ERC-6551 token-bound-account setup that touches_owners[tokenId]twice — the accumulated overcharge is large enough to OOG-revert a transaction that would otherwise succeed under the intended gas semantics.Fix
Track prestate-absent keys in a new
missingmap alongsidecommitted:GetCommittedStateshort-circuits missing keys totrie.ErrNotExistwithout touching the live trie.GetStaterecordsErrNotExistobservations inmissing, and does not overwritecommittedwith a live-trie value once a key has been observed missing.SetStateskips the primingGetStatecall for keys already inmissing(they cannot resolve to a real prestate later in the tx).Snapshotpropagates the sharedmissingmap,Commitresets it alongsidecommitted, andnewContractinitializes it.Hardfork gate
Because this changes state-transition semantics for the class of transactions that hit the pattern above, the new behavior is gated on a new feature flag
CorrectPrestateForAbsentKeys, currently wired tog.IsToBeEnabled(height)— staged, no scheduled fork height yet. Pre-fork execution keeps the historical (buggy) code path byte-for-byte so mainnet catch-up from state prior to the fork height replays identically. Post-fork execution gets the fix.Plumbing follows the existing pattern used by
AsyncContractTrieOption,FixSnapshotOrderOption, etc.:FeatureCtx.CorrectPrestateForAbsentKeysinaction/protocol/context.goCorrectPrestateForAbsentKeysOptiononStateDBAdapternewContract/newContractAdaptertake an explicittrackAbsentbool, propagated throughSnapshotevm.goprepareStateDBAdaptertoggles the option from the feature ctxRegression test
TestGetCommittedStateAbsentKeycovers both fork sides:trackAbsent=true): the repro pattern must returntrie.ErrNotExistforGetCommittedStateon a prestate-absent key.trackAbsent=false): the buggy behavior is preserved —GetCommittedStatereturns the post-mutation value. This guards mainnet catch-up determinism.Both async and sync modes are exercised (4 subtests total).
Verification against real mainnet state
Applied the post-fork behavior to a debug build of v2.4.2, restarted a full node's mainnet catch-up against a stopped-at-target-1 snapshot, let it execute the block containing failing tx
0xfe792b0c…7ec54a:pc=7929 depth=62900(SSTORE_RESET)100(dirty in-place)pc=5788 depth=4)ExecuteContract EVM done statusCodeAt the target block's tip, the fixed node's post-execution state digest diverges from the mined block header (which was produced by the still-buggy production build) — this is the expected outcome and confirms the fix is state-affecting, which is precisely why it needs the fork gate.
Test plan
go test ./action/protocol/execution/evm/...(all tests pass)