From a2d860f797d8ef0ff40bb6109b874c3aebe0f54e Mon Sep 17 00:00:00 2001 From: Claude Code Date: Wed, 3 Dec 2025 09:31:12 -0600 Subject: [PATCH] Merge bitcoin/bitcoin#25193: indexes: Read the locator's top block during init, allow interaction with reindex-chainstate --- src/index/base.cpp | 20 ++++++++++-- src/init.cpp | 21 ++++-------- src/node/blockstorage.cpp | 2 ++ src/node/blockstorage.h | 1 + src/test/util/setup_common.cpp | 1 + test/functional/feature_coinstatsindex.py | 39 ++++++++++++++++++----- test/functional/p2p_blockfilters.py | 7 ---- 7 files changed, 59 insertions(+), 32 deletions(-) diff --git a/src/index/base.cpp b/src/index/base.cpp index d585cac6dd53..b9267168174b 100644 --- a/src/index/base.cpp +++ b/src/index/base.cpp @@ -18,6 +18,8 @@ using node::PruneLockInfo; using node::ReadBlockFromDisk; using node::fPruneMode; +using node::g_indexes_ready_to_sync; + constexpr uint8_t DB_BEST_BLOCK{'B'}; constexpr auto SYNC_LOG_INTERVAL{30s}; @@ -69,14 +71,22 @@ bool BaseIndex::Init() if (locator.IsNull()) { SetBestBlockIndex(nullptr); } else { - SetBestBlockIndex(m_chainstate->FindForkInGlobalIndex(locator)); + // Setting the best block to the locator's top block. If it is not part of the + // best chain, we will rewind to the fork point during index sync + const CBlockIndex* locator_index{m_chainstate->m_blockman.LookupBlockIndex(locator.vHave.at(0))}; + if (!locator_index) { + return InitError(strprintf(Untranslated("%s: best block of the index not found. Please rebuild the index."), GetName())); + } + SetBestBlockIndex(locator_index); } // Note: this will latch to true immediately if the user starts up with an empty // datadir and an index enabled. If this is the case, indexation will happen solely // via `BlockConnected` signals until, possibly, the next restart. m_synced = m_best_block_index.load() == active_chain.Tip(); - if (!m_synced) { + + // Skip pruning check if indexes are not ready to sync (because reindex-chainstate has wiped the chain). + if (!m_synced && g_indexes_ready_to_sync) { bool prune_violation = false; if (!m_best_block_index) { // index is not built yet @@ -131,6 +141,12 @@ static const CBlockIndex* NextSyncBlock(const CBlockIndex* pindex_prev, CChain& void BaseIndex::ThreadSync() { + // Wait for a possible reindex-chainstate to finish until continuing + // with the index sync + while (!g_indexes_ready_to_sync) { + if (!m_interrupt.sleep_for(std::chrono::milliseconds(500))) return; + } + const CBlockIndex* pindex = m_best_block_index.load(); if (!m_synced) { auto& consensus_params = Params().GetConsensus(); diff --git a/src/init.cpp b/src/init.cpp index 5e63df2639be..8b02ff439811 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -149,6 +149,7 @@ using node::DEFAULT_PRINTPRIORITY; using node::DEFAULT_SPENTINDEX; using node::DEFAULT_STOPAFTERBLOCKIMPORT; using node::DEFAULT_TIMESTAMPINDEX; +using node::g_indexes_ready_to_sync; using node::LoadChainstate; using node::NodeContext; using node::ThreadImport; @@ -572,7 +573,7 @@ void SetupServerArgs(ArgsManager& argsman) argsman.AddArg("-pid=", strprintf("Specify pid file. Relative paths will be prefixed by a net-specific datadir location. (default: %s)", BITCOIN_PID_FILENAME), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-prune=", strprintf("Reduce storage requirements by enabling pruning (deleting) of old blocks. This allows the pruneblockchain RPC to be called to delete specific blocks, and enables automatic pruning of old blocks if a target size in MiB is provided. This mode is incompatible with -txindex, -rescan and -disablegovernance=false. " "Warning: Reverting this setting requires re-downloading the entire blockchain. " - "(default: 0 = disable pruning blocks, 1 = allow manual pruning via RPC, >%u = automatically prune block files to stay under the specified target size in MiB)", MIN_DISK_SPACE_FOR_BLOCK_FILES / 1024 / 1024), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); + "(default: 0 = disable pruning blocks, 1 = allow manual pruning via RPC, >=%u = automatically prune block files to stay under the specified target size in MiB)", MIN_DISK_SPACE_FOR_BLOCK_FILES / 1024 / 1024), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-settings=", strprintf("Specify path to dynamic settings data file. Can be disabled with -nosettings. File is written at runtime and not meant to be edited by users (use %s instead for custom settings). Relative paths will be prefixed by datadir location. (default: %s)", BITCOIN_CONF_FILENAME, BITCOIN_SETTINGS_FILENAME), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-syncmempool", strprintf("Sync mempool from other nodes on start (default: %u)", DEFAULT_SYNC_MEMPOOL), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); #if HAVE_SYSTEM @@ -588,7 +589,7 @@ void SetupServerArgs(ArgsManager& argsman) argsman.AddArg("-addressindex", strprintf("Maintain a full address index, used to query for the balance, txids and unspent outputs for addresses (default: %u)", DEFAULT_ADDRESSINDEX), ArgsManager::ALLOW_ANY, OptionsCategory::INDEXING); argsman.AddArg("-reindex", "Rebuild chain state and block index from the blk*.dat files on disk. This will also rebuild active optional indexes.", ArgsManager::ALLOW_ANY, OptionsCategory::INDEXING); - argsman.AddArg("-reindex-chainstate", "Rebuild chain state from the currently indexed blocks. When in pruning mode or if blocks on disk might be corrupted, use full -reindex instead. Deactivate all optional indexes before running this.", ArgsManager::ALLOW_ANY, OptionsCategory::INDEXING); + argsman.AddArg("-reindex-chainstate", "Rebuild chain state from the currently indexed blocks. When in pruning mode or if blocks on disk might be corrupted, use full -reindex instead.", ArgsManager::ALLOW_ANY, OptionsCategory::INDEXING); argsman.AddArg("-spentindex", strprintf("Maintain a full spent index, used to query the spending txid and input index for an outpoint (default: %u)", DEFAULT_SPENTINDEX), ArgsManager::ALLOW_ANY, OptionsCategory::INDEXING); argsman.AddArg("-timestampindex", strprintf("Maintain a timestamp index for block hashes, used to query blocks hashes by a range of timestamps (default: %u)", DEFAULT_TIMESTAMPINDEX), ArgsManager::ALLOW_ANY, OptionsCategory::INDEXING); argsman.AddArg("-txindex", strprintf("Maintain a full transaction index, used by the getrawtransaction rpc call (default: %u)", DEFAULT_TXINDEX), ArgsManager::ALLOW_ANY, OptionsCategory::INDEXING); @@ -1386,19 +1387,6 @@ bool AppInitParameterInteraction(const ArgsManager& args) nMaxTipAge = args.GetIntArg("-maxtipage", DEFAULT_MAX_TIP_AGE); - if (args.GetBoolArg("-reindex-chainstate", false)) { - // indexes that must be deactivated to prevent index corruption, see #24630 - if (args.GetBoolArg("-coinstatsindex", DEFAULT_COINSTATSINDEX)) { - return InitError(_("-reindex-chainstate option is not compatible with -coinstatsindex. Please temporarily disable coinstatsindex while using -reindex-chainstate, or replace -reindex-chainstate with -reindex to fully rebuild all indexes.")); - } - if (g_enabled_filter_types.count(BlockFilterType::BASIC_FILTER)) { - return InitError(_("-reindex-chainstate option is not compatible with -blockfilterindex. Please temporarily disable blockfilterindex while using -reindex-chainstate, or replace -reindex-chainstate with -reindex to fully rebuild all indexes.")); - } - if (args.GetBoolArg("-txindex", DEFAULT_TXINDEX)) { - return InitError(_("-reindex-chainstate option is not compatible with -txindex. Please temporarily disable txindex while using -reindex-chainstate, or replace -reindex-chainstate with -reindex to fully rebuild all indexes.")); - } - } - try { const bool fRecoveryEnabled{llmq::QuorumDataRecoveryEnabled()}; const bool fQuorumVvecRequestsEnabled{llmq::GetEnabledQuorumVvecSyncEntries().size() > 0}; @@ -2235,6 +2223,9 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) } // ********************************************************* Step 8: start indexers + + // If reindex-chainstate was specified, delay syncing indexes until ThreadImport has reindexed the chain + if (!fReindexChainState) g_indexes_ready_to_sync = true; if (args.GetBoolArg("-txindex", DEFAULT_TXINDEX)) { g_txindex = std::make_unique(cache_sizes.tx_index, false, fReindex); if (!g_txindex->Start(chainman.ActiveChainstate())) { diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index 76fd09843ac7..227123826bc6 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -26,6 +26,7 @@ namespace node { std::atomic_bool fImporting(false); std::atomic_bool fReindex(false); +std::atomic_bool g_indexes_ready_to_sync{false}; bool fPruneMode = false; uint64_t nPruneTarget = 0; @@ -898,5 +899,6 @@ void ThreadImport(ChainstateManager& chainman, std::vector vImportFile } // End scope of CImportingNow chainman.ActiveChainstate().LoadMempool(args); + g_indexes_ready_to_sync = true; } } // namespace node diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h index b627c26162fd..ae0edaae2b4d 100644 --- a/src/node/blockstorage.h +++ b/src/node/blockstorage.h @@ -50,6 +50,7 @@ static constexpr size_t BLOCK_SERIALIZATION_HEADER_SIZE = CMessageHeader::MESSAG extern std::atomic_bool fImporting; extern std::atomic_bool fReindex; +extern std::atomic_bool g_indexes_ready_to_sync; /** Pruning-related variables and constants */ /** True if we're running in -prune mode. */ extern bool fPruneMode; diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 5d657d1c4d15..f6209597af0d 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -229,6 +229,7 @@ BasicTestingSetup::BasicTestingSetup(const std::string& chainName, const std::ve noui_connect(); noui_connected = true; } + node::g_indexes_ready_to_sync = true; bls::bls_legacy_scheme.store(true); } diff --git a/test/functional/feature_coinstatsindex.py b/test/functional/feature_coinstatsindex.py index 48f4d12bf32d..a0b63c490ba7 100755 --- a/test/functional/feature_coinstatsindex.py +++ b/test/functional/feature_coinstatsindex.py @@ -54,6 +54,7 @@ def run_test(self): self._test_use_index_option() self._test_reorg_index() self._test_index_rejects_hash_serialized() + self._test_init_index_after_reorg() def block_subsidy(self, prev_bits): # Subsidy calculations valid till block 4500 @@ -75,6 +76,9 @@ def block_sanity_check(self, block_info, prev_height): block_info['new_outputs_ex_coinbase'] + block_info['coinbase'] + block_info['unspendable'] ) + def sync_index_node(self): + self.wait_until(lambda: self.nodes[1].getindexinfo()['coinstatsindex']['synced'] is True) + def _test_coin_stats_index(self): node = self.nodes[0] index_node = self.nodes[1] @@ -241,18 +245,16 @@ def _test_coin_stats_index(self): self.log.info("Test that the index works with -reindex") self.restart_node(1, extra_args=["-coinstatsindex", "-reindex"]) + self.sync_index_node() res11 = index_node.gettxoutsetinfo('muhash') assert_equal(res11, res10) - self.log.info("Test that -reindex-chainstate is disallowed with coinstatsindex") + self.log.info("Test that the index works with -reindex-chainstate") - self.stop_node(1) - self.nodes[1].assert_start_raises_init_error( - expected_msg='Error: -reindex-chainstate option is not compatible with -coinstatsindex. ' - 'Please temporarily disable coinstatsindex while using -reindex-chainstate, or replace -reindex-chainstate with -reindex to fully rebuild all indexes.', - extra_args=['-coinstatsindex', '-reindex-chainstate'], - ) - self.restart_node(1, extra_args=["-coinstatsindex"]) + self.restart_node(1, extra_args=["-coinstatsindex", "-reindex-chainstate"]) + self.sync_index_node() + res12 = index_node.gettxoutsetinfo('muhash') + assert_equal(res12, res10) self.log.info("Test obtaining info for a non-existent block hash") assert_raises_rpc_error(-5, "Block not found", index_node.gettxoutsetinfo, hash_type="none", hash_or_height="ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", use_index=True) @@ -274,6 +276,7 @@ def _test_reorg_index(self): index_node = self.nodes[1] reorg_blocks = self.generatetoaddress(index_node, 2, getnewdestination()[2]) reorg_block = reorg_blocks[1] + self.sync_index_node() res_invalid = index_node.gettxoutsetinfo('muhash') index_node.invalidateblock(reorg_blocks[0]) assert_equal(index_node.gettxoutsetinfo('muhash')['height'], 110) @@ -313,6 +316,26 @@ def _test_index_rejects_hash_serialized(self): for use_index in {True, False, None}: assert_raises_rpc_error(-8, msg, self.nodes[1].gettxoutsetinfo, hash_type='hash_serialized_2', hash_or_height=111, use_index=use_index) + def _test_init_index_after_reorg(self): + self.log.info("Test a reorg while the index is deactivated") + index_node = self.nodes[1] + block = self.nodes[0].getbestblockhash() + self.generate(index_node, 2, sync_fun=self.no_op) + self.sync_index_node() + + # Restart without index + self.restart_node(1, extra_args=[]) + self.connect_nodes(0, 1) + index_node.invalidateblock(block) + self.generatetoaddress(index_node, 5, getnewdestination()[2]) + res = index_node.gettxoutsetinfo(hash_type='muhash', hash_or_height=None, use_index=False) + + # Restart with index that still has its best block on the old chain + self.restart_node(1, extra_args=self.extra_args[1]) + self.sync_index_node() + res1 = index_node.gettxoutsetinfo(hash_type='muhash', hash_or_height=None, use_index=True) + assert_equal(res["muhash"], res1["muhash"]) + if __name__ == '__main__': CoinStatsIndexTest().main() diff --git a/test/functional/p2p_blockfilters.py b/test/functional/p2p_blockfilters.py index 9df7f26ecca7..5aa8e6bba172 100755 --- a/test/functional/p2p_blockfilters.py +++ b/test/functional/p2p_blockfilters.py @@ -293,13 +293,6 @@ def run_test(self): msg = "Error: Unknown -blockfilterindex value abc." self.nodes[0].assert_start_raises_init_error(expected_msg=msg) - self.log.info("Test -blockfilterindex with -reindex-chainstate raises an error") - self.nodes[0].assert_start_raises_init_error( - expected_msg='Error: -reindex-chainstate option is not compatible with -blockfilterindex. ' - 'Please temporarily disable blockfilterindex while using -reindex-chainstate, or replace -reindex-chainstate with -reindex to fully rebuild all indexes.', - extra_args=['-blockfilterindex', '-reindex-chainstate'], - ) - self.test_special_transactions_in_filters() def create_simple_assetlock(self, base_tx, base_tx_value, amount_locked_1, amount_locked_2=0):