validation: fix false corruption errors after pruning or AssumeUTXO #36452

pull l0rinc wants to merge 3 commits into bitcoin:master from l0rinc:l0rinc/verify-pruned-undo changing 3 files +31 −5
  1. l0rinc commented at 12:37 PM on October 7, 2026: contributor

    Problem: getblockfrompeer restores a pruned block's body without recreating its undo record. If startup verification reaches that block at level >2, it attempts to disconnect it and reports database corruption, preventing the node from restarting.

    Fix: Stop deep verification when a pruned node reaches missing undo data, using the existing result for unavailable history. The same allowance covers missing undo on AssumeUTXO snapshot chainstates, matching the existing treatment of unavailable snapshot history. Level 4 still reconnects the newer blocks that were disconnected. verifychain returns false to indicate incomplete verification. Lower verification levels and corruption checks for undo records marked as present retain their existing behavior.

    This came up during pruned txindex review. The pending pruneassumevalid change also needs verification to handle missing undo correctly, so this fix is being split out to land first.

  2. DrahtBot added the label Validation on Oct 7, 2026
  3. DrahtBot commented at 12:37 PM on October 7, 2026: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    <!--006a51241073e994b41acfe9ec718e94-->

    External sites

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

    See the guideline and AI policy for information on the review process. A summary of reviews will appear here.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #36598 <sub><img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36598.svg"></sub> ([Demo] validation: remove AssumeUTXO by l0rinc)
    • #35307 <sub><img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35307.svg"></sub> (blockstorage: keep snapshot base in normal blockfile range by shuv-amp)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

    <!--5faf32d7da4f0f540f40219e4f7537a3-->

  4. andrewtoth commented at 6:12 PM on October 7, 2026: contributor

    IIUC this bug can't realistically be triggered with default checkblocks=6 config. This would require a checkblocks value of > 288 (or 0 to go all the way back) and the user would have to call getblockfrompeer for the block at the exact prune boundary. If a single fully-pruned block sits between the last non-pruned block and the fetched block with no undo, then this bug will not trigger.

  5. l0rinc force-pushed on Oct 8, 2026
  6. l0rinc commented at 6:41 AM on October 8, 2026: contributor

    @andrewtoth, that’s right for ordinary pruning, but this can also happen with default checks under AssumeUTXO before background validation reaches the requested block, so I’ve added a small test for that and expanded the verifychain test coverage following Ralph AI’s suggestions - hope it's clearer now.

    This change is also a prerequisite for the pruneassumevalid change I’m currently working on, which won’t retain the 288-block reorg buffer in the assumevalid region during IBD since we don’t expect reorgs there after headers-first sync.

  7. l0rinc renamed this:
    validation: fix false corruption errors after refetching pruned blocks
    validation: fix false corruption errors after pruning or AssumeUTXO
    on Oct 15, 2026
  8. test: characterize verification without undo
    Restore the highest pruned block so verification reaches its missing undo before an older missing block body.
    Record the corruption error from deep verifychain checks and confirm that level 2 can still verify the available bodies.
    Record the failed deep startup check and confirm that restarting with shallow checks preserves the chain tip and restored block.
    
    Record the same corruption error from default `verifychain` checks on an AssumeUTXO snapshot tip before background validation reaches it.
    5fcd51dbe9
  9. refactor: extract verification data allowance
    Name the existing pruning-or-snapshot allowance so the following undo-data check can reuse it.
    Rename the skipped-data flag to cover unavailable undo as well as block bodies.
    b4bec61ae6
  10. validation: stop verification at pruned undo
    Refetching a pruned block with `getblockfrompeer` restores its body but not its undo record.
    Stop startup verification and `verifychain` when their disconnect checks need missing pruned undo, rather than reporting database corruption.
    The existing allowance for unavailable snapshot history also covers missing undo on AssumeUTXO snapshot chainstates.
    Check available block bodies before stopping for missing undo, preserving the checks at lower verification levels.
    e262a1fbf7
  11. l0rinc force-pushed on Oct 15, 2026
  12. cyb3ralbert commented at 8:41 AM on October 28, 2026: contributor

    Question first: when the snapshot base block's body is present but has no undo data (the state the new feature_assumeutxo.py case creates; it arrives the same way via submitblock or as an unsolicited block message from a peer), a restart of that node with default -checkblocks/-checklevel on e262a1fbf7 now passes VerifyDB (Block verification stopping at height 299 (no undo data).) and then aborts at startup: validation.cpp:3144: CBlockIndex* Chainstate::FindMostWorkChain(): Assertion `pindexTest->HaveNumChainTxs() || pindexTest->nHeight == 0' failed. Is the AssumeUTXO part of this PR meant to make startup verification stop erroring on such a node, with the restart itself left to #35307? A line in the description would make that explicit.

    The abort is not introduced here: the merge-base with -checklevel=1 (so VerifyDB does not fail first) hits the same assertion on the same line, and so does a snapshot tip above the base (base + 6 blocks, base body delivered first) on both trees; without the base body the restart works on both. With #35307 merged the same restarts succeed, and applying only its PopulateBlockIndexCandidates hunk on top of this head is enough.

    Related: #35307 adds if (is_snapshot_cs && pindex == chainstate.SnapshotBase()) break; in the same nCheckLevel >= 3 block, and this PR removes is_snapshot_cs. The two conflict in validation.cpp, and keeping #35307's hunk as is does not compile ('is_snapshot_cs' was not declared in this scope). Its break does not set skipped_missing_data, so the order of the two checks decides what verifychain() returns at the snapshot base: with this PR's check first the new feature_assumeutxo.py assertion passes, with #35307's first it fails (not(True == False)). Which value is intended once both land?

    Tested on Linux aarch64 (Debian 13), GCC 14.2.0, Release, e262a1fbf7 (merges cleanly with master de96f3ae6d). feature_assumeutxo.py and rpc_getblockfrompeer.py from this PR fail on the merge-base cce32b1687 (Block verification stopping at height 299 / 752 (no undo data). not found) and pass on the head. On a pruned regtest node with the highest pruned block (249) refetched via getblockfrompeer, a restart with -checkblocks=0 fails at -checklevel=3 and 4 on the merge-base (irrecoverable inconsistency in block data at 249) and starts on the head (checked last 551 blocks); level 2 and default checks start on both. verifychain at levels 3 and 4 returns false both before and after. A flipped last byte of the newest rev*.dat is still rejected at levels 2 and 3 on both trees (found bad undo data at 800). The neighbouring prune, reindex and assumeutxo functional tests (10, incl. feature_pruning.py and wallet_assumeutxo.py) pass on the head. With #35307 merged (conflict resolved by keeping both checks), feature_assumeutxo.py, rpc_getblockfrompeer.py and wallet_assumeutxo.py pass.

    Reading the code: the new check sits after the level 0-2 reads; it tests the BLOCK_HAVE_UNDO bit directly, while the level-2 check reads the undo position via GetUndoPos(). PruneOneBlockFile clears the bit and zeroes the position together. The code comment notes that a block refetched after pruning has its body but not its undo data. Nit: SKIPPED_MISSING_BLOCKS now also covers a missing undo record; the local flag was renamed, the enum value was not.


github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-10-30 09:40 UTC