Preserve Digest state across snapshot bootstrap and restart

[a-shannon]

With stateType="digest" and UTXO snapshot bootstrap enabled, snapshot reconstruction previously installed a UTXO state and persisted AVL-root versions. The Digest loader interprets versions as block IDs, so a restart before the first full block could fall back to genesis while history still reported the snapshot as applied. Incoming detached chunks were also dropped because the receive branch required a local UTXO reader.

This addresses the Digest configuration concern in #2496. The correction installs the configured state representation, persists the snapshot header ID, root and reconstructed context, and preserves a usable Digest rollback anchor across restart. Requested chunks can be received in both modes; Digest does not gain snapshot-serving capabilities. A failure after entering state-store reconstruction or preparation stops the holder in both Digest and UTXO modes, rather than continuing with potentially mutated state.

Two follow-ups address cafebedouin's review:

• Snapshot reconstruction now consumes one strict pass over the manifest's chunk slots. Each stored chunk must be present, parse and verify its expected subtree ID. Missing, unparseable or misplaced chunks produce StateWriteFailure; this is a storage read-back correction, not a bypass of normal network intake checks.
• Ordinary pruning also advances the retained-block floor, so that floor alone does not prove a snapshot was installed. A Digest node with headers but no full block can now restart after enabling snapshot bootstrap when the persisted genesis version, root and raw context are all intact. Every other candidate still undergoes strict snapshot recovery. Missing or corrupt metadata never triggers genesis recreation. First-block header replay is omitted only for an exact snapshot parent height whose state was successfully installed or verified on recovery.

Fixed review increments and integration

The current i2464 target at ff4d9a1806f4f411e848680a7c4e4b0623725309 already includes the ordinary-header synchronization behavior from #2545. That PR is no longer an integration prerequisite. This branch incorporates the updated target, keeps its current startup tests, and removes the duplicate synchronization implementation. The current nine-file diff contains only the snapshot follow-up and its direct tests.

Previously reviewed increments remain available:

1. Historical prerequisite #2545 at 036c2c258f7aceef00ead6d92cfa238f1e4722bf.
2. Original checkpoint and receive increment. The two-file receive change remains separately reviewable.
3. cafebedouin's two-file completeness increment, retaining the original tests-then-fix commits.
4. Pruned Digest startup and UTXO failure coverage, the subsequent six-file increment.

#2496 has now merged into v6.0.6; this follow-up remains open against i2464 and was not part of that merge. Its release/target placement now needs a maintainer decision after validation. Independent review confirmed that the target alignment retains the snapshot patch and all upstream holder changes.

The deferred #2424 and combined wallet review retain earlier adapted checkpoint/receive changes. They do not yet contain these two new increments; their refresh belongs to their own integration step.

Validation at 02ae9072e19b55e61a177200f0005fa5cf286146

• 60/60 focused and affected tests passed locally on the updated target at a469d6097: 57 cases across eight snapshot, Digest-history, bootstrap and network suites, plus 3 prepared-UTXO holder cases. This includes 7 bootstrap/lifecycle and 25 direct snapshot-state cases. Java 17, Scala 2.12, Sigma 6.0.6. The subsequent fixture-only change creates its state directory before opening native LevelDB; the affected pruning case passes its focused rerun (1/1), while the remaining local evidence is unchanged.
• Before the completeness fix, all three missing/parse/wrong-ID regressions fail while both controls pass. With the fix, all five pass.
• The new pruning regression fails before the startup correction. It uses production header processing to advance the floor, closes/reopens real stores, compares the original configuration with utxoBootstrap=true, then applies the first full block after the corrected restart.
• Lifecycle coverage includes real store mutation followed by reported failure in both state modes, shutdown, no state publication and no applied marker. Persisted-genesis classification has separate version, root, missing-field and raw-context negatives. Removing the root or raw-context guard makes its corresponding regression fail; the version guard is also enforced independently by the Digest constructor.
• Independent source and fixture review found no blocker. An existing older-peer synchronizer case timed out in the earlier broader run, then passed its isolated retry (1/1). No synchronizer fix or timeout diagnosis is claimed.
• Current CI run, head 02ae9072e: completed with six successful jobs and two failed jobs. A request to rerun only node and integration has been sent to the maintainers; no rerun has started at this update.
• The preceding Linux node job exited with code 2 at the missing storage parent. The corrected fixture, including ordinary pruning and both post-write-failure cases, executes successfully in the new node job. That job nevertheless fails: 895 passed, 1 failed, 2 ignored. The failure is the existing ErgoNodeViewHolderSpec case txScriptFailure carries failing transaction id; its isolated local rerun passes all four configurations, so the CI cause remains unestablished. Current integration has 21/22 passed, with DeepRollBackSpec timing out during convergence. The preceding integration run separately timed out in ForkResolutionSpec (21/22 passed). No fix is claimed for these other failures. Repository permissions prevent us from rerunning the failed jobs.

Historical evidence: head 63c9b79d had 35 receive/synchronizer cases plus 44 checkpoint/lifecycle cases and 8/8 CI checks. Those results belong to that earlier head.

The fixtures use fake PoW and small states. The network fixture registers its manifest and requests directly; full wire-manifest negotiation, TCP framing, process interruption and production-network qualification are outside this evidence. The per-subtree identity checks do not rederive a full materialized-tree digest. Imported AVL payload remains on disk in Digest mode; cleanup is outside this PR, and the mainnet-sized clean(0) undo batch has not been measured. Inconsistent stores remain available for recovery. #2464 remains the broader qualification issue. Consensus validation and NiPoPoW proof handling are unchanged.

[cafebedouin]

Flagging a gap in the same function this PR touches (createPersistentProver), surfaced by the AI-assisted node audit — it composes with your StateWriteFailure change rather than colliding with it.

The gap. downloadedChunksIterator() reads each chunk as

historyStorage.get(chunkId).flatMap(bs => SubtreeSerializer.parseBytesTry(bs).toOption)

so a subtree that is missing on read-back (get → None) or fails to deserialize (parseBytesTry → Failure → toOption → None) is silently dropped by the flatMap. createPersistentProver then hands that short iterator straight to VersionedLDBAVLStorage.recreate(manifest, downloadedChunksIterator(), …) and records the manifest digest — over a tree missing subtrees the manifest declares. Nothing checks that the number of restored subtrees equals manifest.subtreesIds.size, so an incomplete restore currently returns Success.

Witness. A local spec that registers one unparseable chunk (Array[Byte](1,2,3)) among otherwise-valid subtrees still gets a successful prover back from createPersistentProver; the all-chunks control passes too. The failure is silent, not merely logged.

Suggested addition — a completeness pre-check at the top of the Some(manifest) branch, before the transfer, orthogonal to the write-phase recovery you're adding:

val restored = downloadedChunksIterator().size
if (restored != manifest.subtreesIds.size) {
  val m = s"Incomplete UTXO snapshot restore: $restored of ${manifest.subtreesIds.size} subtrees; failing closed"
  log.error(m)
  Failure(new IllegalStateException(m))
} else {
  // existing reconstructStateContextBeforeEpoch → recreate → StateWriteFailure recovery
}

Two caveats:

• This is the minimal fail-closed version. It counts subtrees; it doesn't re-derive the tree digest from the reconstructed nodes and compare to the manifest, which would also catch a chunk that parses to the wrong content. If you'd rather go straight to the stronger check, that supersedes this.
• The count consumes the iterator once for .size and recreate re-iterates — a second DB scan. A production version should thread a single pass (count while iterating, or fold the check into recreate).

Happy to open this as a small commit against your branch or a separate PR — whichever fits your sequencing (#2545 → #2546 → into #2496's i2464). Flag either way and I'll match it.

[a-shannon]

@cafebedouin, thanks for flagging this. I checked the read-back and restore paths at 63c9b79dc576461f6266c234612c35d8ed393d73: missing/unparseable chunks are skipped, and recreate consumes the shortened iterator without checking it against the manifest. restorePrunedProver only restores the pruned root, so its success does not establish that all detached subtrees are available. I have not independently run your witness.

Normal network intake already validates requested chunk identities. This gap concerns the later storage read-back; the supplied witness should not be interpreted as malformed peer bytes bypassing that intake check.

Please post a small commit and the failing spec here against the current #2546 head, rather than opening a parallel PR. That keeps this within the existing #2545 -> #2546 review sequence.

The invariant should cover the chunks actually consumed during restoration. A separate count scan followed by another permissive scan is insufficient if read-back changes; equal counts also do not establish the expected chunk identities. Prefer a bounded streaming check against the manifest, with missing or unparseable expected chunks producing failure. Preserve StateWriteFailure for errors after the store-write phase starts, so callers cannot continue serving the previous state after possible mutation.

Please include your complete-snapshot control and separate missing-chunk and unparseable-chunk cases, with the exact tested commit. A parsed wrong-ID substitution is the useful negative for any identity check. A digest taken from the restored pruned root alone should not be described as a full materialization check. We can review the focused increment before integration without changing the unrelated Digest checkpoint behavior.

[cafebedouin]

@a-shannon — done as asked: two commits on top of the current #2546 head 63c9b79dc576461f6266c234612c35d8ed393d73, no parallel PR.

Branch: cafebedouin/ergo:e047-restore-completeness — two-file increment

1. 2ef26c8ec40c9fa2ff83f935b30a0bbdd8757998 — specs only (UtxoSetSnapshotProcessorSpecification). At this commit the three negatives fail: createPersistentProver returns Success.
2. d651d5e3688611c3979f2b9884079119861c520c — the fix (UtxoSetSnapshotProcessor, +24/−1). All five properties pass.

The invariant, per your notes. The count pre-check I first suggested is dropped. createPersistentProver now feeds recreate from a strict manifestChunksIterator(manifest) instead of the permissive downloadedChunksIterator():

• it walks manifest.subtreesIds in order — exactly one historyStorage.get per declared subtree id, lazily, so it is bounded by the manifest and is the same pass recreate consumes (no separate scan that could disagree with a later one);
• each chunk must be present, parse, and verify(expectedId) at its index; the first one that is missing, unparseable, or reads back under a different id throws IllegalStateException naming the index and expected id.

downloadedChunksIterator() is left unchanged to keep the increment small, but after this change it has no production caller (only the existing spec uses it) — removing it, or making it strict too, is your call.

Failure typing. The iterator is consumed inside recreate → store.update, i.e. after the store-write phase has been entered, so the failure surfaces through your existing recoverWith as StateWriteFailure with the IllegalStateException as cause. I kept that deliberately conservative rather than classifying it as a pre-write failure: LDBVersionedStore.update throws before db.write(batch), and the specs assert store.lastVersionID is still None, but in-memory LSN state has been touched by then. If you would rather have a pre-write classification, that is a change in update/recreate, not here.

Specs (each case builds its own processor with its own chunk storage, so nothing stale from another property can mask a result):

case	before fix (2ef26c8ec)	after fix (d651d5e36)
existing register/plan/restore property	pass	pass
complete snapshot is restored (control)	pass	pass
chunk missing on read-back	FAIL (restore = Success)	pass
unparseable stored chunk (Array[Byte](1,2,3))	FAIL (restore = Success)	pass
valid chunk stored under another chunk's id	FAIL (restore = Success)	pass

The wrong-id case keeps the chunk count equal to the manifest and every stored chunk parseable — only identity differs — so it is the negative a count check would have passed.

Regression on d651d5e36, JDK 8: DigestSnapshotBootstrapSpecification, DigestSnapshotNetworkSpecification, DigestSnapshotStateSpecification, ErgoNodeViewSynchronizerSpecification — 55 succeeded, 0 failed. I have not run the full suite or CI on these commits.

Not claimed. Agreed on your intake point: this is a storage read-back gap, not malformed peer bytes bypassing the network-side identity check; the specs write directly through registerDownloadedChunk. The identity check is per-subtree verify(expectedId) against the manifest's declared ids; it is not a re-derivation of the full tree digest from the materialized store, and restorePrunedProver succeeding is not described as one. Digest checkpoint behaviour is untouched.

Cherry-pick, squash or rewrite as suits your sequence.

AI-assisted (Claude Fable 5.1, via Claude Code); tests run locally by me before posting.

[cafebedouin]

@a-shannon — separate from the restore increment above: I read the #2546 increment (036c2c25…63c9b79d, production diff first) looking for things worth settling before merge. One executed finding, two smaller requests. Everything else I checked held up — manifest/snapshot-info receive is reachable in Digest mode, store-write precedes the history marker so a crash between them falls back to a clean re-bootstrap, the clean(0) + empty update leaves a usable rollback anchor, and dropping the first-block header replay is neutral for UTXO (UtxoState ignores header modifiers).

1. isUtxoSnapshotApplied is also true for a pruned node; the new startup path fails closed on it (executed)

isUtxoSnapshotApplied is readMinimalFullBlockHeight() > GenesisHeight. updateBestFullBlock writes the same value as the pruning floor when blocksToKeep >= 0. readStateForStartup now treats Digest && utxoBootstrap && isUtxoSnapshotApplied && bestFullBlockOpt.isEmpty as "a snapshot checkpoint must be on disk", and shuts the node down if readSnapshot cannot verify one.

So an existing pruned Digest store that has a floor but no full block yet, restarted with utxoBootstrap = true, no longer starts:

same store, restart with utxoBootstrap = true	result
#2545 parent 036c2c258f7aceef00ead6d92cfa238f1e4722bf	starts; view returned, floor 20
#2546 head 63c9b79dc576461f6266c234612c35d8ed393d73	actor system terminates: requirement failed: Stored snapshot root does not match canonical history (DigestState.readSnapshot)

Control in both runs: restarting with the original utxoBootstrap = false config starts normally. Fixture: real ErgoNodeViewRef over reopened stores (your Session/closeOwnedStores pattern), Digest, blocksToKeep = 10, votingLength = 20, 45 fake-PoW headers, no full blocks. Caveat: in this fixture header application did not itself advance the floor, so the witness calls history.updateBestFullBlock(lastHeader) directly — the function toDownload calls when the header chain first syncs — to write it. Spec: PrunedDigestFlipWitness (one file on top of 63c9b79d; it reports both outcomes via info rather than asserting the flip, so it runs unchanged at either commit).

It is a narrow, operator-induced window (config change between header sync and the first full block) and reverting the config recovers, so I would not call it severe. But the fail-closed branch is keyed on a predicate that does not mean what the branch assumes, and FullBlockProcessor's new utxoBootstrap && isUtxoSnapshotApplied condition shares it. Suggested change, smallest first:

• in readStateForStartup, take the fail-closed path only when the state store actually carries snapshot evidence (e.g. the snapshot header-id version key that fromSnapshot writes), and otherwise fall through to ErgoState.readOrGenerate as before; or
• persist an explicit snapshot-applied marker in onUtxoSnapshotApplied and key both new consumers on that rather than on the floor.

Either way, the flip case above makes a cheap regression.

2. The post-write shutdown now applies to UTXO mode but is only tested in Digest

case Failure(t: StateWriteFailure) => abortSnapshotStatePreparation(t) is mode-independent, so a UTXO-mode node now also shuts down where it previously logged and continued. I think that is the right behaviour. The lifecycle matrix only runs the postWriteFailure case for StateType.Digest, though; a UTXO variant of that case (the fault-injecting holder is currently built around a DigestState, so it is a small fixture addition rather than one more row) would cover the mode most nodes run, and the description could say the change is not Digest-only.

3. Imported AVL payload in a Digest store — question, not a blocker

You already note that the imported AVL payload stays on disk. For a Digest node that is the full UTXO set, retained indefinitely and never read after fromSnapshot. Is removing it after the anchor is written in scope for a follow-up? If so, an issue reference in the description would help whoever sizes Digest-mode disk later. Related, and within your stated "production qualification is outside this evidence": fromSnapshot's store.clean(0) deletes every undo LSN written by recreate in a single WriteBatch, which at mainnet size is one delete per tree node — worth one observed run before this path is relied on.

AI-assisted (Claude Fable 5.1, via Claude Code); witness run locally on JDK 8 at both commits named above.