Skip to content

fix(evm): GetCommittedState must not return post-mutation value for prestate-absent keys#4869

Open
envestcc wants to merge 1 commit into
iotexproject:masterfrom
envestcc:fix/committed-state-post-mutation-pollution
Open

fix(evm): GetCommittedState must not return post-mutation value for prestate-absent keys#4869
envestcc wants to merge 1 commit into
iotexproject:masterfrom
envestcc:fix/committed-state-post-mutation-pollution

Conversation

@envestcc

@envestcc envestcc commented Jul 6, 2026

Copy link
Copy Markdown
Member

Summary

contract.GetState propagates trie.ErrNotExist for a storage key that is absent in the pre-tx trie without recording the observation. 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.

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). 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 missing map alongside committed:

  • GetCommittedState short-circuits missing keys to trie.ErrNotExist without touching the live trie.
  • GetState records ErrNotExist observations in missing, and does not overwrite committed with a live-trie value once a key has been observed missing.
  • SetState skips the priming GetState call for keys already in missing (they cannot resolve to a real prestate later in the tx).
  • Snapshot propagates the shared missing map, Commit resets it alongside committed, and newContract initializes 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 to g.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.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

Regression test

TestGetCommittedStateAbsentKey covers both fork sides:

  • post-fork (trackAbsent=true): the repro pattern must return trie.ErrNotExist for GetCommittedState on a prestate-absent key.
  • pre-fork (trackAbsent=false): the buggy behavior is preserved — GetCommittedState returns 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:

pre-fork (mainnet reality) post-fork (with this PR)
SSTORE at pc=7929 depth=6 cost 2900 (SSTORE_RESET) cost 100 (dirty in-place)
out-of-gas events during tx ≥1 (pc=5788 depth=4) 0
ExecuteContract EVM done statusCode 106 (revert) 1 (success)
gas consumed 826,574 (reverted) 768,498

At 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)
  • Fork-gated regression test asserts both pre- and post-fork behavior
  • Reviewer input on the target hardfork height once release planning picks one

@envestcc envestcc requested a review from a team as a code owner July 6, 2026 08:56
…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 envestcc force-pushed the fix/committed-state-post-mutation-pollution branch from 9ae1b33 to 6c57f1e Compare July 8, 2026 11:52
@sonarqubecloud

sonarqubecloud Bot commented Jul 8, 2026

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant