From 16b1710d97464f134a526634a412a4b1b6cc8639 Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Mon, 18 Aug 2025 21:36:01 +0200 Subject: [PATCH 1/2] 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. Github-Pull: #33212 Rebased-From: 01b95ac6f496e24e525b2fc9d69ee8b543da65ff --- src/index/base.cpp | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/index/base.cpp b/src/index/base.cpp index 1169a1c86b7..e08f9083992 100644 --- a/src/index/base.cpp +++ b/src/index/base.cpp @@ -253,18 +253,13 @@ bool BaseIndex::Rewind(const CBlockIndex* current_tip, const CBlockIndex* new_ti return false; } - // 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; } From fcac8022d839572f5d8781096eec14ca7ea2e0dd Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Mon, 18 Aug 2025 21:21:48 +0200 Subject: [PATCH 2/2] test: index with an unclean restart after a reorg This test fails without the previous commit. Github-Pull: #33212 Rebased-From: a602f6fb7bf5f9e57299f4d6e246c82379fad8d2 --- test/functional/feature_coinstatsindex.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/functional/feature_coinstatsindex.py b/test/functional/feature_coinstatsindex.py index 764f027c857..dd1484f8b3e 100755 --- a/test/functional/feature_coinstatsindex.py +++ b/test/functional/feature_coinstatsindex.py @@ -322,6 +322,21 @@ class CoinStatsIndexTest(BitcoinTestFramework): res1 = index_node.gettxoutsetinfo(hash_type='muhash', hash_or_height=None, use_index=True) assert_equal(res["muhash"], res1["muhash"]) + self.log.info("Test index with an unclean restart after a reorg") + self.restart_node(1, extra_args=self.extra_args[1]) + committed_height = index_node.getblockcount() + self.generate(index_node, 2, sync_fun=self.no_op) + self.sync_index_node() + block2 = index_node.getbestblockhash() + index_node.invalidateblock(block2) + self.generatetoaddress(index_node, 1, getnewdestination()[2], sync_fun=self.no_op) + self.sync_index_node() + index_node.kill_process() + self.start_node(1, extra_args=self.extra_args[1]) + self.sync_index_node() + # Because of the unclean shutdown above, indexes reset to the point we last committed them to disk. + assert_equal(index_node.getindexinfo()['coinstatsindex']['best_block_height'], committed_height) + if __name__ == '__main__': CoinStatsIndexTest(__file__).main()