index: don't commit state in BaseIndex::Rewind
What the developer wrote
Authored by Martin Zumsande
index: don't commit state in BaseIndex::Rewind
The committed state of an index should never
be ahead of the flushed chainstate. Otherwise, in the case
of an unclean shutdown, the blocks necessary to revert
from the prematurely committed state would not be
available, which would corrupt the coinstatsindex in particular.
Instead, the index state will be committed with the next
ChainStateFlushed notification.
What changed, and why it matters
This change fixes a crash-and-restart bug in Bitcoin Core's index tracking. Previously, during a blockchain reorganization (a 'reorg'), an index could save its new position to disk before the main chain data was fully flushed. If the computer then shut down uncleanly, the index could point to blocks that had already been pruned away, leaving the index corrupted on restart. The fix removes that premature save so the index only commits its position when the main chain state is safely flushed.
Apply the patch. Nodes running indexes (especially coinstatsindex) should upgrade to avoid potential index corruption after reorgs and unclean shutdowns. If an affected node fails to start after an unclean shutdown, reindexing may be required.
Security signals we found
Data-integrity / corruption bug in index persistence during reorg
Premature commit of index state relative to chainstate flush
Potential unclean-shutdown-induced index corruption, specifically coinstatsindex
Fix removes unsafe Commit() call and reverts failure-handling rollback logic
Evidence from the diff
BaseIndex::Rewind() previously called Commit() immediately after SetBestBlockIndex(new_tip) during a reorg rewind. That could persist an index best-block locator ahead of the flushed chainstate. On unclean shutdown, if the chainstate rolled back or pruning removed blocks needed to revert from that prematurely committed index state, the coinstatsindex (and potentially other indexes) could become inconsistent. The patch removes the in-Rewind Commit() and its failure-handling rollback, deferring index state persistence until the next ChainStateFlushed notification, ensuring the committed index state never exceeds the flushed chainstate.
Changed components
src/index/base.cppBaseIndex::Rewindcoinstatsindexblock index persistenceInspect captured patch +2 / −7
diff --git a/src/index/base.cpp b/src/index/base.cpp
index fdd0e0d8..e984aaf5 100644
--- a/src/index/base.cpp
+++ b/src/index/base.cpp
@@ -301,18 +301,13 @@ bool BaseIndex::Rewind(const CBlockIndex* current_tip, const CBlockIndex* new_ti
}
}
- // In the case of a reorg, ensure persisted block locator is not stale.
+ // Don't commit here - the committed index state must never be ahead of the
+ // flushed chainstate, otherwise unclean restarts would lead to index corruption.
// Pruning has a minimum of 288 blocks-to-keep and getting the index
// out of sync may be possible but a users fault.
// In case we reorg beyond the pruned depth, ReadBlock would
// throw and lead to a graceful shutdown
SetBestBlockIndex(new_tip);
- if (!Commit()) {
- // If commit fails, revert the best block index to avoid corruption.
- SetBestBlockIndex(current_tip);
- return false;
- }
-
return true;
}
Why this scored 64/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.