blockstorage: keep snapshot base in normal blockfile range #35307

shuv-amp commented at 3:57 PM on May 17, 2026: contributor

A node that has the assumeutxo snapshot base block on disk before loadtxoutset() runs ends up in a state it cannot recover from. Depending on whether it is restarted, it either aborts or refuses to start.

Aborting, when background validation reaches the base block:

node/blockstorage.cpp:985 bool node::BlockManager::WriteBlockUndo(const CBlockUndo &, BlockValidationState &, CBlockIndex &): Assertion `m_blockfile_cursors[type]' failed.

Refusing to start, if the node is restarted after the snapshot is loaded:

[ReadBlockUndo] OpenUndoFile failed for FlatFilePos(nFile=-1, nPos=0) while reading block undo
[DisconnectBlock] failure reading undo data
[VerifyDB] Verification error: irrecoverable inconsistency in block data at 299
Corrupted block database detected.
Please restart with -reindex or -reindex-chainstate to recover.

The block database is not corrupt, so the reindex that message asks for is wasted work.

Reaching this needs the base block stored before the snapshot is loaded, which submitblock does: it is accepted and written while BlockManager::m_snapshot_height is still unset, so it goes to the normal blockfile cursor. loadtxoutset() then sets m_snapshot_height to that same height, and BlockfileTypeForHeight() starts reporting ASSUMED for it.

From there the two failures follow:

WriteBlockUndo() looks up the cursor for the block's height and does *Assert(m_blockfile_cursors[type]). The block is now ASSUMED, but nothing above the snapshot height has been written, so that cursor does not exist and the assertion fires when the background chainstate connects the base. FindNextBlockPos() creates the cursor lazily in the same situation; WriteBlockUndo() asserts instead.
VerifyDB() skips blocks without BLOCK_HAVE_DATA on a snapshot chainstate (validation.cpp:4674), which is why the base is normally left alone. Here the base does have data, and no undo data because the snapshot chainstate never connected it, so the skip does not apply and DisconnectBlock() is attempted on it.

The base block is connected by the background chainstate, not the snapshot one, so it belongs to the normal blockfile range. BlockfileTypeForHeight() now classifies only blocks above the snapshot height as ASSUMED. A snapshot chainstate sitting at the base height must not flush the normal cursor, so FlushChainstateBlockFile() takes the chainstate into account.

VerifyDB() stops before disconnecting the snapshot base, where the snapshot database does not have the ancestor UTXO data the disconnect needs.

When a snapshot chainstate is loaded from disk, the base's hardcoded m_chain_tx_count does not mean the background chainstate has processed its parents, so the base is kept in m_blocks_unlinked until they arrive, and a historical chainstate does not add its target to the block index candidates before then.

The functional test submits the base block, loads the snapshot, restarts, then feeds the missing historical blocks and checks that background validation completes. Both failures above are what it hits without this change.

Tested:

build/test/functional/test_runner.py feature_assumeutxo.py wallet_assumeutxo.py feature_pruning.py feature_reindex.py --timeout-factor=4
build/bin/test_bitcoin

DrahtBot added the label Block storage on May 17, 2026

DrahtBot commented at 3:57 PM on May 17, 2026: contributor

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

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35307.

Reviews

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

#35731 (Indexes: Harden the flush-error notification invariant by arejula27)
#35714 (validation: stop writes after flush failure by l0rinc)
#30342 (kernel, logging: Pass Logger instances to kernel objects by ryanofsky)
#29700 (kernel, refactor: return error status on all fatal errors by ryanofsky)

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.

shuv-amp force-pushed on May 17, 2026

shuv-amp renamed this:
~~blockstorage: keep assumeutxo base block in normal blockfile range~~
blockstorage: handle undo for assumeutxo base block already on disk
on May 17, 2026

shuv-amp marked this as ready for review on May 17, 2026

mzumsande commented at 1:45 PM on May 18, 2026: contributor

why would someone load a snapshot when they already downloaded the block, which means that their tip must be no more than 1024 blocks behind that block.

shuv-amp commented at 3:47 PM on May 18, 2026: contributor

I don't think the normal P2P download path is a strong motivation here. In that case, as you say, the node would already be close to the snapshot height.

The case covered here is that this blockstorage state is accepted through local block submission/import before snapshot activation. For example, submitblock can store the snapshot base block while m_snapshot_height is still unset, so it is accounted under the normal blockfile cursor. After loadtxoutset(), the same height maps to ASSUMED; if no assumed blockfile cursor exists yet, background validation can later connect that block and hit the cursor assertion in WriteBlockUndo().

So I’d frame this as making an accepted blockstorage state not abort, rather than as an important/common assumeutxo path. If maintainers prefer not to support that state, rejecting it during snapshot activation would be another possible direction.

shuv-amp commented at 7:28 PM on May 19, 2026: contributor

Taking this back to draft for now.

shuv-amp marked this as a draft on May 19, 2026

shuv-amp force-pushed on May 21, 2026

shuv-amp renamed this:
~~blockstorage: handle undo for assumeutxo base block already on disk~~
blockstorage: keep snapshot base in normal blockfile range
on May 21, 2026

DrahtBot added the label CI failed on May 21, 2026

shuv-amp marked this as ready for review on May 21, 2026

DrahtBot removed the label CI failed on May 22, 2026

DrahtBot added the label Needs rebase on Jun 8, 2026

shuv-amp force-pushed on Jun 9, 2026

DrahtBot removed the label Needs rebase on Jun 9, 2026

sedited added this to a project on Jul 5, 2026

sedited changed the project status on Jul 5, 2026

sedited requested review from ryanofsky on Aug 10, 2026

blockstorage: keep snapshot base in normal blockfile range

The background chainstate connects the assumeutxo snapshot base block. If
that block is accepted before loadtxoutset() sets m_snapshot_height, it is
written using the normal blockfile cursor.

After snapshot activation, classify only blocks above the base height as
ASSUMED. This keeps the already-stored base block and its undo data on the
normal cursor, while snapshot-chainstate flushes at the base height still
avoid flushing normal block data.

On restart, keep an already-stored base block in m_blocks_unlinked until
its parents are processed, and avoid adding the historical target to
setBlockIndexCandidates before then. Stop VerifyDB before disconnecting the
snapshot base, where the snapshot database lacks the ancestor UTXO data.

Add functional coverage for the base block already being on disk before
snapshot activation, including a restart before background validation
completes. Adjust the wallet assumeutxo prune-height test so the backup at
the snapshot base remains available.

d07900cf8c

shuv-amp force-pushed on Aug 19, 2026

shuv-amp commented at 10:11 PM on August 19, 2026: contributor

Following up with what this actually does on master, in May I only described the assertion.

If the node is restarted after the snapshot is loaded, it doesn't come back up:

[ReadBlockUndo] OpenUndoFile failed for FlatFilePos(nFile=-1, nPos=0) while reading block undo
[DisconnectBlock] failure reading undo data
[VerifyDB] Verification error: irrecoverable inconsistency in block data at 299
Corrupted block database detected.
Please restart with -reindex or -reindex-chainstate to recover.

The database isn't corrupt, with this change the same sequence restarts and background validation completes. On a snapshot chainstate VerifyDB() stops its walk at the first block with no data, which is normally the base, so it never gets that far. Here the base does have data, and no undo data because the snapshot chainstate never connected it, so the walk goes into it and the disconnect fails. The WriteBlockUndo() assertion I mentioned in May is the other path, when the node keeps running rather than restarting.

I've kept this as fixing the state rather than rejecting it at activation, since rejecting wouldn't help a node that has already reached it.

Rebased.