Skip to content

fix: skip storage entries with missing preimage keys #32051

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 17, 2025

Conversation

PinelliaC
Copy link
Contributor

When GetKey (see https://github.com/ethereum/go-ethereum/blob/v1.15.11/trie/secure_trie.go#L235) is called, a missing preimage can cause the function to return a nil key. This, in turn, makes account.Storage persist an incorrect value.

@PinelliaC PinelliaC requested a review from rjl493456442 as a code owner June 17, 2025 07:55
@rjl493456442 rjl493456442 added this to the 1.15.12 milestone Jun 17, 2025
@rjl493456442 rjl493456442 merged commit 05e1994 into ethereum:master Jun 17, 2025
3 of 4 checks passed
@asdv23
Copy link

asdv23 commented Jun 20, 2025

Great to see this optimized solution merged! To provide full context:
Root cause: Geth's init initialization without --cache.preimages flag caused incomplete preimage caching.
This became critical when:

  1. Dumping state at specific block height for Reth's init-state initialization
  2. Genesis alloc data wrote to stateDB without storing preimages
  3. Contract execution later stored address preimages via SSTORE operations

Critical failure scenario:
When a storage key's preimage was:

  • Stored due to contract execution, BUT
  • Not originally cached from genesis alloc (because the specific storage slot wasn't accessed during genesis initialization)
    → The missing preimage caused the storage key to be misinterpreted as empty value during state dumping
    → Resulting in incorrect stateroot computation in init-state

My initial solution:
Fixed by manually constructing preimages for genesis alloc during dump: mantlenetworkio/op-geth@b609ebe, ensuring complete preimage coverage.

Optimization path:
After discussion with @PinelliaC, we concluded safely ignoring uncached storage-key preimages is valid because:

  • Reth automatically populates genesis storage slots when importing genesis.json
  • Thus omitted preimages during export are harmlessly reinitialized

This PR builds upon that collaborative insight.

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.

3 participants