Skip to content

snapshot: translate zkRoot→mptRoot in runtime Snapshot/Snapshots lookups#306

Closed
curryxbo wants to merge 1 commit intomainfrom
fix/snapshot-zk-runtime-lookup
Closed

snapshot: translate zkRoot→mptRoot in runtime Snapshot/Snapshots lookups#306
curryxbo wants to merge 1 commit intomainfrom
fix/snapshot-zk-runtime-lookup

Conversation

@curryxbo
Copy link
Copy Markdown
Contributor

@curryxbo curryxbo commented Mar 30, 2026

Summary

  • For ZK-era blocks, Tree.Snapshot(blockRoot) and Tree.Snapshots(root, ...) perform a direct t.layers[root] lookup using the block header's zkStateRoot (Poseidon hash), but snapshot layers are keyed by the locally-computed mptStateRoot (Keccak256) — so the lookup always returns nil.
  • This causes statedb.New() to set sdb.snap = nil, which means StateDB.Commit() never calls snaps.Update()snapshot diff layers never accumulate during block import on MPT nodes processing ZK-era blocks.
  • blockchain.skipBlock() has the same issue: it probes the snapshot tree with the header root and gets nil, leading to incorrect block-skip decisions.

Fix

Add rawdb.ReadDiskStateRoot(diskdb, root) translation at the top of both Snapshot() and Snapshots(). When a zkRoot→mptRoot mapping exists, the lookup is redirected to the correct layer. When no mapping exists (standard blocks or already an mptRoot), the call is a no-op — the root stays unchanged.

The translation is consistent with all other paths that already normalize roots: OpenTrie(), loadSnapshot(), generateSnapshot(), setHeadBeyondRoot(), etc.

Impact

After this fix, MPT nodes processing ZK-era blocks will:

  • Correctly find snapshot layers during block import
  • Accumulate diff layers in the snapshot tree
  • Write proper snapshot journals on clean shutdown (no more diffs=missing)
  • No longer require repair on every restart

Test plan

  • Run MPT node on testnet for 128+ blocks, verify snapshot journal has diff layers on shutdown
  • Verify Loaded snapshot journal diskroot=... diffs=... shows actual diff count on restart
  • Run prune-state after node has accumulated diff layers — should work without DiskRoot fallback

Fixes #305

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Enhanced snapshot state root translation to consistently apply disk mapping lookups across all retrieval methods, improving reliability of state access operations.
  • Documentation

    • Updated documentation clarifying state root format specifications, including header roots versus snapshot-layer roots and their mapping translation behavior.

…ots()

For ZK-era blocks, block headers carry a zkStateRoot (Poseidon hash)
while snapshot layers are keyed by the locally-computed mptStateRoot
(Keccak256). Runtime consumers (statedb.New, blockchain.skipBlock) call
Snapshot(header.Root) which performs a direct map lookup — always
returning nil for ZK-era blocks.

This causes:
- sdb.snap = nil → StateDB.Commit never calls snaps.Update
- Snapshot diff layers never accumulate during block import
- Journal always has diffs=missing, every restart triggers repair

Add zkRoot→mptRoot normalization (via ReadDiskStateRoot) at the top of
both Snapshot() and Snapshots(). When the root has no mapping (standard
blocks or already an mptRoot), the translation is a no-op. When a
mapping exists, the lookup is redirected to the correct mptRoot layer.

Fixes #305

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@curryxbo curryxbo requested a review from a team as a code owner March 30, 2026 08:43
@curryxbo curryxbo requested review from tomatoishealthy and removed request for a team March 30, 2026 08:43
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 30, 2026

📝 Walkthrough

Walkthrough

This PR fixes snapshot tree lookup failures for ZK-era blocks by adding root translation. The changes normalize zkStateRoot (header value) to mptStateRoot (snapshot layer key) in two lookup functions before map access, ensuring snapshot diff layers are properly created during block import.

Changes

Cohort / File(s) Summary
Snapshot root translation
core/state/snapshot/snapshot.go
Updated Snapshot() and Snapshots() to translate input block root through rawdb.ReadDiskStateRoot() before layer lookup. Expanded documentation explaining ZK-era root format differences (zkStateRoot vs. mptStateRoot).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

bug

Suggested reviewers

  • panos-xyz
  • twcctop

Poem

🐰 A snapshot seeks its missing layer,
But zkRoot hides beneath the snare—
A little translation, swift and clean,
Now diffs accumulate serene! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding zkRoot→mptRoot translation in the Tree.Snapshot and Tree.Snapshots runtime lookup methods.
Linked Issues check ✅ Passed The code changes implement the proposed fix from issue #305: adding rawdb.ReadDiskStateRoot translation to Tree.Snapshot() and Tree.Snapshots() before map lookups.
Out of Scope Changes check ✅ Passed All changes are directly scoped to the linked issue #305 requirements: translation logic in Snapshot() and Snapshots() methods with supporting documentation.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/snapshot-zk-runtime-lookup

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.11.4)

Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions
The command is terminated due to an error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
core/state/snapshot/snapshot.go (1)

303-305: Optional: Consider debug logging for translation events.

Other translation sites in the codebase (e.g., rollup/tracing/tracing.go:147-154) log when root translation occurs. This could aid debugging snapshot issues without impacting performance significantly at Debug level.

💡 Optional logging for observability
 	if mptRoot, err := rawdb.ReadDiskStateRoot(t.diskdb, blockRoot); err == nil {
+		log.Debug("Snapshot lookup translated zkRoot to mptRoot", "zkRoot", blockRoot, "mptRoot", mptRoot)
 		blockRoot = mptRoot
 	}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@core/state/snapshot/snapshot.go` around lines 303 - 305, Add a Debug-level
log when a state root is translated by rawdb.ReadDiskStateRoot so translation
events are observable; inside the if that calls
rawdb.ReadDiskStateRoot(t.diskdb, blockRoot) and assigns mptRoot to blockRoot,
emit a debug via the existing logger (or t.logger) indicating the original
blockRoot and the translated mptRoot and the fact translation occurred (use the
function/method context where variables mptRoot, blockRoot and ReadDiskStateRoot
are used) so it only logs at Debug level and does not change behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@core/state/snapshot/snapshot.go`:
- Around line 303-305: Add a Debug-level log when a state root is translated by
rawdb.ReadDiskStateRoot so translation events are observable; inside the if that
calls rawdb.ReadDiskStateRoot(t.diskdb, blockRoot) and assigns mptRoot to
blockRoot, emit a debug via the existing logger (or t.logger) indicating the
original blockRoot and the translated mptRoot and the fact translation occurred
(use the function/method context where variables mptRoot, blockRoot and
ReadDiskStateRoot are used) so it only logs at Debug level and does not change
behavior.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0efecaaa-7d0b-4327-846d-017b4d62e06b

📥 Commits

Reviewing files that changed from the base of the PR and between b3c5552 and c86611b.

📒 Files selected for processing (1)
  • core/state/snapshot/snapshot.go

@curryxbo curryxbo closed this Mar 31, 2026
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.

snapshot: normalize zkRoot→mptRoot in Tree.Snapshot() and Tree.Snapshots() for ZK-era blocks

1 participant