#35307 blockstorage: keep snapshot base in normal blockfile range
https://github.com/bitcoin/bitcoin/pull/35307 · · +85/-26 in 5 files, 1 commits · labels: Block storage
Goal
- Prevent node crashes and false database corruption errors during assumeutxo background validation
- Ensures nodes do not fail startup or require reindexing if the snapshot base block arrived out-of-band
Adjusts block storage accounting so that the assumeutxo snapshot base block remains in the normal blockfile range rather than being classified as an assumed block. It also updates `FlushChainstateBlockFile` to respect snapshot chainstates, halts `VerifyDB` disconnect walks before the snapshot base, and holds unlinked blocks in `m_blocks_unlinked` until historical parents arrive.
Problem: If a node receives and writes the snapshot base block before `loadtxoutset()` is invoked (such as via `submitblock`), the base block is placed in the normal blockfile cursor. Subsequent background validation or node restarts will either trip an assertion on a missing blockfile cursor in `WriteBlockUndo` or fail startup in `VerifyDB` with a false-positive database corruption error requiring reindexing.
Category: Validation (#15 of 48)
P3 · bug fix
- P3 because it prevents an irrecoverable abort or false database corruption error in assumeutxo validation
- The crash only triggers if the base block was submitted out-of-band before loading the snapshot
Fixes an irrecoverable node abort and startup verification crash in assumeutxo background validation. As shuv-amp noted, when the base block is present before snapshot activation, the node hits `Assertion 'm_blockfile_cursors[type]' failed` or halts on startup with `Corrupted block database detected`. However, reaching this state requires out-of-band block submission before loading the snapshot, making it an edge-case stability fix.
Membership: Touches block storage management (`src/node/blockstorage.*`) and chainstate verification logic (`src/validation.cpp`).
Factors: security/stability 2, bug 2, performance 0, user value 1, leverage 0
Reviewability: Ready
- Ready for review: rebased, passes CI, and includes functional test coverage
The patch is rebased, passes CI, and includes functional test coverage for the failure scenario.
Author status: active, rebased and provided detailed startup failure logs
Resolved concerns:
- mzumsande questioned whether someone would load a snapshot after having already downloaded the base block; shuv-amp clarified that out-of-band submissions (like `submitblock`) create this state and result in an irrecoverable crash without this fix.
Agreement: Neutral
- Neutral feedback focused on the realism of the failure scenario
- Questioned why someone would load a snapshot when already at that block height (mzumsande)
- Author explained that out-of-band block submissions create an accepted state that crashes without a fix
mzumsande questioned the motivation for the scenario; author explained the irrecoverable crash path with no further response.
One reviewer questioned the plausibility of the use case, and the author addressed the query by explaining that accepted block storage states should not crash or corrupt the node. No further objections have been raised.
- mzumsande asked why someone would load a snapshot when already at that block height
- shuv-amp clarified that `submitblock` before `loadtxoutset()` creates an accepted blockstorage state that triggers an assertion abort or startup failure
Review verdicts (DrahtBot): 0
Files
70 lines under test/bench/ci.
- test/functional/feature_assumeutxo.py +44/-11
- src/node/blockstorage.cpp +18/-6
- src/validation.cpp +13/-2
- test/functional/wallet_assumeutxo.py +9/-6
- src/node/blockstorage.h +1/-1
Card
This PR fixes a bug in assumeutxo where having the snapshot base block stored on disk prior to calling `loadtxoutset()` causes the node to either abort via an assertion in `WriteBlockUndo()` or fail startup with a false database corruption error in `VerifyDB()`. It reclassifies the snapshot base block to the normal blockfile range, guards snapshot flush behavior, and prevents `VerifyDB()` from disconnecting past the snapshot base. While the triggering condition requires out-of-band block submission like `submitblock`, the resulting failure is unrecoverable without reindexing. The PR is clean and ready for review.