#36702 validation: don't treat snapshot block as linked on startup
| Reviewability | Reviews | Agreement | Size | ||||
|---|---|---|---|---|---|---|---|
| #36702 validation: don't treat snapshot block as linked on startup | Ready | 0 | Strong | 8 + 7 tests | |||
|
|
|
|
| |||
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):
- reviewer: code review 2026-10-31
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
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