pruner: use teeWriter and HEAD as prune target, fix genesis root validation#310
pruner: use teeWriter and HEAD as prune target, fix genesis root validation#310
Conversation
…dation - Add teeWriter to persist trie nodes to disk during GenerateTrie, ensuring pruning works correctly even after unclean shutdowns. - Use HEAD directly as the pruning target instead of HEAD-127, eliminating unnecessary height rollback on L2 chains where reorgs don't occur. - Resolve genesis root via ReadDiskStateRoot in extractGenesis so that zkTrie roots (overridden via GenesisStateRoot) are correctly mapped to the actual MPT disk root. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdded an unexported Changes
Sequence Diagram(s)sequenceDiagram
participant Pruner
participant Snapshot as snapshot.GenerateTrie
participant Tee as teeWriter
participant DB as KeyValueDB
participant Bloom as stateBloom
Pruner->>Snapshot: GenerateTrie(snaptree, root, db, writer)
Snapshot->>Tee: Put/Delete(node)
Tee->>DB: Put/Delete(node)
Tee->>Bloom: Record(node)
Tee-->>Snapshot: ack
Snapshot-->>Pruner: trie generation complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
core/state/pruner/pruner.go (1)
267-304:⚠️ Potential issue | 🟠 MajorPreserve
middleStateRootsfor explicit non-HEAD targets.This path now always calls
prune(..., nil, ...), butprune()only force-deletes intermediate snapshot roots when that map is populated.RecoverPruningstill buildsmiddleRootsfor the same reason. If a caller prunes to an older user-specifiedroot, the intermediate diff-layer roots can survive bloom false-positives and leave dangling state behind.Suggested fix
func (p *Pruner) Prune(root common.Hash) error { + var middleRoots map[common.Hash]struct{} + if root == (common.Hash{}) { // Use HEAD as the pruning target. On L2 chains reorgs are // essentially non-existent, so the HEAD-127 safety margin @@ root = p.headHeader.Root log.Info("Selecting HEAD as the pruning target", "root", root, "height", p.headHeader.Number.Uint64()) } else { log.Info("Selecting user-specified state as the pruning target", "root", root) + + found := false + layers := p.snaptree.Snapshots(p.headHeader.Root, 128, true) + middleRoots = make(map[common.Hash]struct{}) + for _, layer := range layers { + if layer.Root() == root { + found = true + break + } + middleRoots[layer.Root()] = struct{}{} + } + if !found { + return errors.New("non-existent target state") + } } @@ - return prune(p.snaptree, root, p.db, p.stateBloom, filterName, nil, start) + return prune(p.snaptree, root, p.db, p.stateBloom, filterName, middleRoots, start) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/state/pruner/pruner.go` around lines 267 - 304, The current path always calls prune(..., nil, ...) which prevents prune() from force-deleting intermediate snapshot roots; when pruning to an explicit non-HEAD root you must preserve and pass the middleStateRoots map so intermediate diff-layer roots are removed. Modify the code that selects the target root (the branch where root != (common.Hash{})) to build or reuse the same middleStateRoots map constructed by RecoverPruning (or compute it the same way) and pass that map into prune(...) instead of nil (keep using prune(p.snaptree, root, p.db, p.stateBloom, filterName, middleStateRoots, start)). Ensure the symbol middleStateRoots is populated before the call and used in prune so intermediate roots are cleaned up.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@core/state/pruner/pruner.go`:
- Around line 382-389: The code currently treats any error from
rawdb.ReadDiskStateRoot as “mapping absent” and silently falls back to
genesis.Root(); update the logic around rawdb.ReadDiskStateRoot(genesisRoot) so
only a not-found/missing mapping is ignored—propagate or return any real DB/read
errors instead. Specifically, modify the genesisRoot resolution block that calls
rawdb.ReadDiskStateRoot and assigns genesisRoot (and the subsequent
trie.NewSecure call) to check the error kind (or replace ReadDiskStateRoot with
a helper that returns (mptRoot, ok, err)); if ok==false keep genesisRoot, but if
err!=nil return/log the error up the call stack instead of swallowing it. Ensure
callers of this function handle the returned error appropriately.
---
Outside diff comments:
In `@core/state/pruner/pruner.go`:
- Around line 267-304: The current path always calls prune(..., nil, ...) which
prevents prune() from force-deleting intermediate snapshot roots; when pruning
to an explicit non-HEAD root you must preserve and pass the middleStateRoots map
so intermediate diff-layer roots are removed. Modify the code that selects the
target root (the branch where root != (common.Hash{})) to build or reuse the
same middleStateRoots map constructed by RecoverPruning (or compute it the same
way) and pass that map into prune(...) instead of nil (keep using
prune(p.snaptree, root, p.db, p.stateBloom, filterName, middleStateRoots,
start)). Ensure the symbol middleStateRoots is populated before the call and
used in prune so intermediate roots are cleaned up.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 83e20ed3-68f5-4843-85d0-723903ce8d75
📒 Files selected for processing (1)
core/state/pruner/pruner.go
Keep local-test.zip untracked and out of repository history from this point. Made-with: Cursor
Treat missing or invalid disk-state-root mapping for genesis as an explicit error during pruning instead of silently falling back. Made-with: Cursor
* ci: support multi-platform Docker image build (amd64 + arm64) (#298) * ci: support multi-platform Docker image build (amd64 + arm64) Use docker/build-push-action with QEMU and buildx to build multi-arch images. Mac arm64 users can now pull and run the image natively. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * ci: add workflow_dispatch for manual Docker image build Allow manually triggering the Docker build from GitHub Actions UI with a tag name input, useful for re-building existing tags. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * ci: fix incorrect COMMIT and VERSION on manual dispatch Use git rev-parse HEAD for COMMIT and stripped version for VERSION build-arg, so they are correct in both tag-push and workflow_dispatch. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: fletcher.fan <fletcher.fan@bitget.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> * Fix RLP decoding for MorphTx (#299) * implement version-aware RLP decoding for MorphTx * fix morph tx * pruner: fall back to disk snapshot root when journal is missing (#300) * pruner: fall back to disk snapshot root when journal is missing When geth is killed uncleanly (SIGKILL before BlockChain.Stop writes the snapshot journal), prune-state fails with: WARN Loaded snapshot journal diskroot=XXX diffs=missing ERROR head doesn't match snapshot: have XXX, want YYY NewPruner now reads the persisted disk snapshot root via rawdb.ReadSnapshotRoot and retries snapshot initialisation with that root when the normal head-based init fails. Prune() then uses the disk root as the pruning target directly, bypassing the requirement for 128 in-memory diff layers that cannot exist when the journal was not written. Normal flow (clean shutdown, journal present) is unchanged. Made-with: Cursor * pruner: fix Cap panic on disk-layer-only tree and add generation wait log Two follow-up fixes to the journal-missing fallback (56ae344): 1. Skip snaptree.Cap(root, 0) when root is already the disk layer. Cap requires a diffLayer as its target; calling it on a disk-layer-only tree (which is exactly what the fallback produces) returns "snapshot is disk layer" and aborts after all the heavy bloom-filter and DB-sweep work is done. Guard with DiskRoot() != root. 2. Add log lines around the fallback snapshot.New() call to make it visible when snapshot generation must be resumed (async=false blocks until generation finishes, which can take hours for large state). * pruner: rename diskRoot to snapDiskRoot to avoid confusion with diskStateRoot --------- Co-authored-by: corey <corey.zhang@bitget.com> * Revert "pruner: fall back to disk snapshot root when journal is missing (#300)" (#309) This reverts commit b3c5552. Co-authored-by: corey <corey.zhang@bitget.com> * tracers: fix Morph fee-token tracing paths (#308) * tracers: fix Morph fee-token tracing paths Keep Morph fee-token system calls bracketed consistently, forward system-call hooks through mux tracers, and make traceCall precredit alt-fee balances so tracing matches execution more closely. Constraint: Preserve user-visible tracer output while keeping prestate and traceCall behavior correct for Morph fee-token transactions Confidence: medium Scope-risk: moderate Not-tested: Full eth/tracers/internal/tracetest suite still has pre-existing fixture and VM failures on this branch * tracers: fix prestateTracer account discovery when DisableStorage is set * tracers: harden Morph fee-token trace edge cases Prevent flatCallTracer from touching hidden system-call frames and keep traceCall's synthetic fee-token precredits out of prestate views so debug RPCs stay stable and prestate output matches chain state. Constraint: Preserve Morph alt-fee trace execution without leaking synthetic prestate or hidden system-call frames Confidence: high Scope-risk: moderate * core: require balanced system-call trace hooks Only bracket fee-token helper calls when both start and end hooks are present so partial tracer wiring cannot leak system-call depth across a trace. Constraint: Preserve existing V2-over-legacy hook selection while restoring balanced start/end semantics Confidence: high Scope-risk: narrow Not-tested: Full core package outside TestStartSystemCallTrace * fix: handle nil parameter in morph_diskRoot RPC to prevent panic (#311) When morph_diskRoot is called without parameters, blockNrOrHash is nil, causing a nil pointer dereference crash. Default to latest block when no parameter is provided, consistent with other eth RPC methods. * pruner: use teeWriter and HEAD as prune target, fix genesis root validation (#310) * pruner: use teeWriter and HEAD as prune target, fix genesis root validation - Add teeWriter to persist trie nodes to disk during GenerateTrie, ensuring pruning works correctly even after unclean shutdowns. - Use HEAD directly as the pruning target instead of HEAD-127, eliminating unnecessary height rollback on L2 chains where reorgs don't occur. - Resolve genesis root via ReadDiskStateRoot in extractGenesis so that zkTrie roots (overridden via GenesisStateRoot) are correctly mapped to the actual MPT disk root. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * handle error * remove accidentally committed local test zip Keep local-test.zip untracked and out of repository history from this point. Made-with: Cursor * pruner: enforce genesis disk-root mapping Treat missing or invalid disk-state-root mapping for genesis as an explicit error during pruning instead of silently falling back. Made-with: Cursor --------- Co-authored-by: corey <corey.zhang@bitget.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: fletcher.fan <fletcher.fan@bitget.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Segue <huoda.china@163.com> Co-authored-by: corey <coreyx1992@gmail.com> Co-authored-by: corey <corey.zhang@bitget.com> Co-authored-by: panos <pan107104@outlook.com>
Problems
1. Genesis root validation failure during pruning
During
prune-state,extractGenesiscallstrie.NewSecurewith the genesis block's root. On Morph, the genesis root stored in the block header may be a zkTrie root (overridden viaGenesisStateRootfor block hash compatibility) rather than the actual MPT disk root, causingtrie.NewSecureto fail and aborting the entire pruning process.2. Block height regression after pruning
The pruner previously targeted HEAD-127 as the prune target, meaning after pruning the node would restart at a state 127 blocks behind the actual chain head. On L2 chains like Morph where reorgs are essentially non-existent, this safety margin is unnecessary and causes undesirable height rollback.
Solution
extractGenesis, resolve the genesis root viarawdb.ReadDiskStateRootbefore opening the trie, so the actual MPT disk root is used even when the header contains a zkTrie root.GenerateTrie, ensuring trie data is fully persisted to disk regardless of how the node was previously shut down.teeWriter, this guarantees zero height drop after pruning.🤖 Generated with Claude Code
Summary by CodeRabbit