From 3679f1ecf5ec25d9a40a23e48797a8c22fa3cb99 Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Mon, 8 Jun 2026 19:25:20 +0200 Subject: [PATCH] index: Don't commit ahead of the flushed chainstate Otherwise, if the node has an unclean restart, indexes with state (coinstatsindex) couldn't reorg to the last flushed tip and would be corrupted. Also updates documentation of Commit() - the locator functionality isn't used, so the previous text was wrong: We must have the best block in our block index after a restart. Co-authored-by: Fabian Jahr --- src/index/base.cpp | 11 +++++++++++ src/index/base.h | 10 +++------- src/test/baseindex_tests.cpp | 7 +++++-- 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/index/base.cpp b/src/index/base.cpp index bcd7c434b2b..68b712bbb12 100644 --- a/src/index/base.cpp +++ b/src/index/base.cpp @@ -279,6 +279,17 @@ void BaseIndex::Commit() // (this could happen if init is interrupted). bool ok = m_best_block_index != nullptr; if (ok) { + // Don't commit if the index best block is not an ancestor of the chainstate's last flushed + // block. Otherwise, after an unclean shutdown, the index could be + // persisted ahead of a chainstate it can no longer roll back to, which + // would corrupt indexes with state (e.g. coinstatsindex). + const CBlockIndex* index_tip = m_best_block_index.load(); + const CBlockIndex* last_flushed = WITH_LOCK(::cs_main, return m_chainstate->GetLastFlushedBlock()); + if (!last_flushed || last_flushed->GetAncestor(index_tip->nHeight) != index_tip) { + LogDebug(BCLog::COINDB, "Skipping commit, index is ahead of flushed chainstate (index height %d, last flush at height %d)", + index_tip->nHeight, last_flushed ? last_flushed->nHeight : -1); + return; + } CDBBatch batch(GetDB()); ok = CustomCommit(batch); if (ok) { diff --git a/src/index/base.h b/src/index/base.h index 00ce800d69c..86ee9029c85 100644 --- a/src/index/base.h +++ b/src/index/base.h @@ -94,13 +94,9 @@ private: CThreadInterrupt m_interrupt; /// Write the current index state (eg. chain block locator and subclass-specific items) to disk. - /// - /// Recommendations for error handling: - /// If called on a successor of the previous committed best block in the index, the index can - /// continue processing without risk of corruption, though the index state will need to catch up - /// from further behind on reboot. If the new state is not a successor of the previous state (due - /// to a chain reorganization), the index must halt until Commit succeeds or else it could end up - /// getting corrupted. + /// Will skip the commit if no block has been indexed yet or if the index's best block is + /// ahead of the chainstate's last flushed block. This avoids persisting state an unclean shutdown + /// could not roll back from. A later call commits when the chainstate has flushed far enough. void Commit(); /// Loop over disconnected blocks and call CustomRemove. diff --git a/src/test/baseindex_tests.cpp b/src/test/baseindex_tests.cpp index 525309bf3a2..16cbab19738 100644 --- a/src/test/baseindex_tests.cpp +++ b/src/test/baseindex_tests.cpp @@ -42,10 +42,13 @@ BOOST_FIXTURE_TEST_CASE(baseindex_no_commit_ahead_of_flush, TestChain100Setup) // Part 1: Sync, then "crash" (stop without flushing). Models a node that // started up, had its index catch up, but never flushed before going down. - sync_index(false, 100, 100); + // The end-of-sync Commit() runs at chain tip (height 100) but + // m_last_flushed_block is null, so it is skipped. + sync_index(false, 100, 0); // Part 2: Restart cleanly. Sync, force a chainstate flush, and drain the // validation queue so the index's ChainStateFlushed callback runs. + // Now m_last_flushed_block == tip == 100 and the index can commit. sync_index(true, 100, 100); // Part 3: Connect a new block on the chain without flushing @@ -53,7 +56,7 @@ BOOST_FIXTURE_TEST_CASE(baseindex_no_commit_ahead_of_flush, TestChain100Setup) // in parallel with Sync(). Here we do it before Sync() to make the race // state deterministic. CreateAndProcessBlock({}, CScript() << OP_TRUE); - sync_index(false, 101, 101); + sync_index(false, 101, 100); } BOOST_AUTO_TEST_SUITE_END()