Revert "pruner: fall back to disk snapshot root when journal is missing (#300)"

[curryxbo]

Reverts #300.

The fallback logic introduced in #300 (using the snapshot disk root when the journal is missing) is overly broad and masks the real cause of snapshot/journal mismatches. Reverting it to keep the pruner behavior explicit and predictable.

A follow-up PR will replace this with a cleaner approach using a teeWriter that writes the target trie directly to disk during pruning, ensuring zero height drop after restart without relying on snapshot state.

Summary by CodeRabbit

• Refactor
  • Simplified snapshot pruning logic by removing fallback mechanisms and direct error handling. The pruning process now consistently derives targets from the snapshot tree, with explicit error reporting when snapshot requirements are unmet.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR removes the snapDiskRoot fallback field and related logic from the Pruner struct. Instead of falling back to the disk-layer root when snapshot initialization fails, errors are now returned directly. The pruning target selection is simplified to always derive from the active snapshot tree without alternatives.

Changes

Cohort / File(s)	Summary
Pruner Snapshot Fallback Removal core/state/pruner/pruner.go	Removed snapDiskRoot struct field, eliminated fallback logic in NewPruner that re-initialized snapshots from disk-layer root on failure, removed conditional in prune that skipped snaptree.Cap when target equaled disk root, and simplified Prune to always derive pruning target from snapshot tree without fallback alternatives.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• pruner: fall back to disk snapshot root when journal is missing #300: Directly modifies the same Pruner codepaths including snapDiskRoot fallback logic, NewPruner initialization, and Cap/journal handling in the same functions.

Suggested labels

bug

Suggested reviewers

• twcctop
• panos-xyz

Poem

🐰 A snapDiskRoot once held sway,
A fallback for a rainy day,
But simpler paths lead us home,
No more roots to make us roam! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately and specifically describes the main change: reverting the fallback logic for disk snapshot root when journal is missing, which aligns with the file changes removing snapDiskRoot and related logic.

_{✏️ 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 revert/pr-300-only

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (2)

core/state/pruner/pruner.go (2)

251-264: Increased strictness: pruning requires exactly 128 diff layers.

The reverted fallback to snapDiskRoot is removed. Now if fewer than 128 diff layers exist, the pruner returns an error rather than falling back to the disk root.

This means:

• Recently pruned chains must wait for 128 new blocks before pruning again
• Chains with fewer than 128 total blocks cannot be pruned

The error message at line 260 is helpful, but operators should be aware of this behavioral change. Consider documenting this requirement in the CLI help text or release notes.

📝 Optional: Add more context to the error message

 		if len(layers) != 128 {
 			// Reject if the accumulated diff layers are less than 128. It
 			// means in most of normal cases, there is no associated state
 			// with bottom-most diff layer.
-			return fmt.Errorf("snapshot not old enough yet: need %d more blocks", 128-len(layers))
+			return fmt.Errorf("snapshot not old enough yet: have %d diff layers, need 128 (wait for %d more blocks)", len(layers), 128-len(layers))
 		}

🤖 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 251 - 264, The pruner now fails
when fewer than 128 diff layers exist (root, layers computed via
p.snaptree.Snapshots(p.headHeader.Root,...)) and returns fmt.Errorf(...) instead
of falling back to snapDiskRoot; update the error reporting and user docs:
change the fmt.Errorf call to include the actual len(layers) and an explicit
statement that pruning requires exactly 128 diff layers (e.g. "pruning requires
128 diff layers; have %d; need %d more blocks"), and also add a corresponding
note to the CLI help text or release notes explaining the 128-block requirement
so operators are aware of the new behavior.

---

191-193: Consider adding early validation to reject disk-layer roots explicitly.

Lines 191-193 now unconditionally call Cap(), which will fail with "snapshot [%#x] is disk layer" if the user specifies a disk-layer root (see snapshot.go:378).

For auto-selected roots this is safe since Snapshots(..., 128, true) excludes disk layers. However, when users explicitly specify a root via the CLI, there's no validation before prune() is called, so an error will only surface deep in Cap().

While this explicit failure may be intentional (avoiding silent masking), adding an early check in Prune() to reject disk-layer roots with a clearer error message would improve user experience.

🤖 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 191 - 193, Add an explicit early
validation in Prune() before calling snaptree.Cap: detect if the provided root
is a disk-layer snapshot (use the existing predicate in the snapshot/snaptree
package—e.g., snaptree.IsDiskLayer(root) or the equivalent method on the
snapshot type) and return a clear error like "specified root is a disk-layer
snapshot; pruning requires a non-disk root" instead of letting snaptree.Cap fail
later; place this check immediately before the snaptree.Cap(root, 0) call in
Prune().

🤖 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/pruner/pruner.go`:
- Around line 251-264: The pruner now fails when fewer than 128 diff layers
exist (root, layers computed via p.snaptree.Snapshots(p.headHeader.Root,...))
and returns fmt.Errorf(...) instead of falling back to snapDiskRoot; update the
error reporting and user docs: change the fmt.Errorf call to include the actual
len(layers) and an explicit statement that pruning requires exactly 128 diff
layers (e.g. "pruning requires 128 diff layers; have %d; need %d more blocks"),
and also add a corresponding note to the CLI help text or release notes
explaining the 128-block requirement so operators are aware of the new behavior.
- Around line 191-193: Add an explicit early validation in Prune() before
calling snaptree.Cap: detect if the provided root is a disk-layer snapshot (use
the existing predicate in the snapshot/snaptree package—e.g.,
snaptree.IsDiskLayer(root) or the equivalent method on the snapshot type) and
return a clear error like "specified root is a disk-layer snapshot; pruning
requires a non-disk root" instead of letting snaptree.Cap fail later; place this
check immediately before the snaptree.Cap(root, 0) call in Prune().

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 55796b60-e7ab-4e28-8d8c-011cde21f5b1

📥 Commits

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

📒 Files selected for processing (1)

• core/state/pruner/pruner.go

[curryxbo]

This PR is intentionally just a revert of #300. We're aware it removes the fallback fix — a proper replacement will come in a follow-up PR. The goal here is to cleanly revert first, then address the fix separately.

[panos-xyz]

This PR is intentionally just a revert of #300. We're aware it removes the fallback fix — a proper replacement will come in a follow-up PR. The goal here is to cleanly revert first, then address the fix separately.

LGTM