#36702 validation: don't treat snapshot block as linked on startup

ReviewabilityReviewsAgreementSize
#36702 shuv-amp validation: don't treat snapshot block as linked on startupReady0Strong8 + 7 tests
  • Prevent an assertion crash on startup during AssumeUTXO background sync if restarting with unlinked blocks
  • Avoid forcing a node reindex that wipes the snapshot chainstate to recover from unlinked snapshot blocks
  • Ready for review
  • No review verdicts recorded
  • Verified by reproducing the startup crash and confirming the fix on Windows x64 (ValeraFinebits)
  • 8 lines added or modified outside tests
  • 7 lines added or modified in tests
  • 0 lines removed in total
  • 3 files, 1 commits
Validation#5 of 53
P1 · bug fix
  • P1 because it fixes a startup assertion crash loop that permanently bricks startup without a reindex
  • Prevents losing background AssumeUTXO sync progress when the snapshot block arrives ahead of its parents
Reviewability: ReadyStalePaused
Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets
Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs.
Size: lines added or modified outside tests, then in tests; greener = smaller.

Goal

Prevent an assertion crash on startup during AssumeUTXO background validation if the node restarts while holding the snapshot block without its immediate ancestors. On startup, `LoadBlockIndex()` now adds the snapshot block to `m_blocks_unlinked` if its parent lacks transaction count data, and `PopulateBlockIndexCandidates()` skips registering it as a tip candidate until its ancestors are connected.

Problem: During AssumeUTXO background sync, blocks up to the snapshot block can arrive out of order, placing the snapshot block into memory before its ancestors. If the node restarts in this window, `LoadBlockIndex()` sets the hardcoded chain transaction count for the snapshot block without restoring it to `m_blocks_unlinked`, leading `FindMostWorkChain()` to walk back to an unlinked parent lacking transaction counts and crash on an assertion on every boot. The only manual recovery is `-reindex` or `-reindex-chainstate`, which completely wipes the snapshot chainstate.

Evidence of importance:

  • Author description: 'Assertion failed: (pindexTest->HaveNumChainTxs() || pindexTest->nHeight == 0), function FindMostWorkChain, file validation.cpp, line 3217.'
  • Author notes reproducing it over P2P using a test peer withholding the snapshot block's parent, and reports that cyb3ralbert encountered the same assertion during testing of #36452 (referencing PR #36452 comment).

Scope: Small, targeted change touching `LoadBlockIndex` in `blockstorage.cpp` and `PopulateBlockIndexCandidates` in `validation.cpp`, with an added regression test in `feature_assumeutxo.py`.

Risk of a mistake: medium (validation). A logic error in block index loading or candidate tip selection could stall background AssumeUTXO sync or cause improper chainstate initialization.

Changed since the previous assessment: Now includes the complete diff and patch ID; the logic and scope remain consistent with the prior description.

Reviewability

Ready

Nothing known that would invalidate a review now.

Waiting on (from the thread read):

Agreement

Strong

substantive support, no open objection (ValeraFinebits)

Thread notes: Initial record. ValeraFinebits verified the bug and fix by testing feature_assumeutxo.py on Windows x64. / Initial record. ValeraFinebits verified the reproduction and fix via testing on Windows x64.

Objections, suggestions, and questions: none recorded.

Support:

  • ValeraFinebits (none) (2026-10-31) [tested: feature_assumeutxo.py]: passes with the fix on Windows x64; reverting the C++ changes reproduces the startup assertion in FindMostWorkChain()

Participants:

  • ValeraFinebits (none, support, 1 statements 2026-10-31 to 2026-10-31): tested that the fix resolves the startup assertion in feature_assumeutxo.py

Files

7 lines under test/bench/ci.

  • test/functional/feature_assumeutxo.py +7/-0
  • src/node/blockstorage.cpp +4/-0
  • src/validation.cpp +4/-0

Card

The short text the ranking pass compares across PRs.

Fixes an assertion failure loop on startup for nodes using AssumeUTXO that restart while the snapshot block has been downloaded ahead of its ancestor blocks. In that state, startup assigns the snapshot block a hardcoded transaction count and treats it as a background chain tip candidate, causing FindMostWorkChain to walk back into an unlinked parent and assert fail on every boot. This PR adds the unlinked snapshot block back into m_blocks_unlinked on startup and excludes it from candidate tips until its ancestors arrive. The crash was also independently reported during review testing of #36452 and otherwise requires a full reindex that destroys the snapshot chainstate.

Data

dossier JSON · extract JSON · model ledger, generated 2026-11-01T01:17, confidence high, input hash b5c3e657d50fd3a9
thread read through 2026-10-31 (1 statements), head 80f6ddd6cb
labels: Validation

Categories

Validation

P1 · bug fix

Fixes a startup crash loop in `FindMostWorkChain()` that permanently bricks startup without `-reindex-chainstate`. If a node running AssumeUTXO background validation restarts while holding the snapshot block without its parent, `LoadBlockIndex()` fails to add it to `m_blocks_unlinked`, triggering an assertion failure on every subsequent boot.

Membership: Labeled 'Validation' and modifies `LoadBlockIndex()` in `src/node/blockstorage.cpp` and `PopulateBlockIndexCandidates()` in `src/validation.cpp` to correctly handle AssumeUTXO block index linkage.

Factors: security/stability 2, bug 2, performance 0, user value 1, leverage 0