Skip to content

fix: prevent GetStorageRoot from polluting contract cache and causing incorrect state writeback#4803

Merged
envestcc merged 3 commits intoiotexproject:masterfrom
envestcc:fix/getstorageroot
Mar 17, 2026
Merged

fix: prevent GetStorageRoot from polluting contract cache and causing incorrect state writeback#4803
envestcc merged 3 commits intoiotexproject:masterfrom
envestcc:fix/getstorageroot

Conversation

@envestcc
Copy link
Member

Problem

When validating historical blocks, a "delta state digest doesn't match" error occurs. The root cause is that
the geth upgrade (v1.7.4-0.20260114032628) introduced a GetStorageRoot(address) call in evm.create() for
contract creation collision detection. The previous GetStorageRoot implementation called
getContractWoCreate, which created a contract trie and added the target account to cachedContract — even
for non-contract (EOA) accounts.

This triggered the following chain of events:

  1. GetStorageRoot(addr)getContractWoCreate(addr) → account added to cachedContract
  2. EVM calls Snapshot() → iterates all cached contracts → computes trie.RootHash() for the empty storage
    trie → writes non-zero hash to Account.Root on the original contract object
  3. CommitContracts() writes back all cached contracts via PutState, including this unmodified account
    with a now non-zero Root

In v2.3.3 (old geth), GetStorageRoot did not exist, so the account was never cached and its Root stayed
zero.

Fix

Commit 1: Fix GetStorageRoot (immediate fix)

Modified GetStorageRoot to read the account's Root field directly via accountutil.Recorded without
creating a contract trie or adding to cachedContract. Removed the now-dead getContractWoCreate function.

Commit 2: Skip writing read-only contracts (forward-looking hardening)

Added AlwaysWriteCachedContract feature flag (gated by ToBeEnabled fork height). When disabled (after
fork activation), CommitContracts skips both Commit() and PutState() for contracts that were only read
(not modified by SetState/SetCode). This prevents any future read operation from inadvertently causing
state writes through the cachedContract → Snapshot → CommitContracts chain.

  • Added Dirty() method to Contract interface to check dirtyCode || dirtyState
  • Before fork: old behavior preserved (write all cached contracts)
  • After fork: only dirty contracts are committed and written back

envestcc and others added 2 commits March 12, 2026 08:20
…teback

The geth upgrade introduced a GetStorageRoot call in evm.create() for
contract creation collision checks. The previous implementation used
getContractWoCreate which added the target account to cachedContract.
This caused CommitContracts to write back the account with a recomputed
non-zero empty-trie Root (via Snapshot), diverging from v2.3.3 behavior
where Root remained zero.

Fix: read the account Root directly without creating a contract trie
or adding to the cache.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add AlwaysWriteCachedContract feature flag (gated by ToBeEnabled fork
height, true before fork). When disabled (after fork), CommitContracts
skips both Commit() and PutState() for contracts that were only read
(not modified by SetState/SetCode), preventing read operations from
causing unnecessary writes through the cachedContract → Snapshot →
CommitContracts chain.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@envestcc envestcc requested a review from a team as a code owner March 12, 2026 00:26
@sonarqubecloud
Copy link

@envestcc envestcc merged commit 16a7d00 into iotexproject:master Mar 17, 2026
4 checks passed
@envestcc envestcc deleted the fix/getstorageroot branch March 17, 2026 08:52
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.

2 participants