index: Enable reindex-chainstate with active indexes

This is achieved by letting the index sync thread wait until
reindex-chainstate is finished.

This also disables the pruning check when reindexing the chainstate (which is
incompatible with prune mode) because there would be no chain at this point
in init.

Author: mzumsande

src/index/base.cpp

@@ -23,6 +23,8 @@
 #include <string>
 #include <utility>
 
+using node::g_indexes_ready_to_sync;
+
 constexpr uint8_t DB_BEST_BLOCK{'B'};
 
 constexpr auto SYNC_LOG_INTERVAL{30s};
@@ -105,7 +107,9 @@ bool BaseIndex::Init()
     // 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
@@ -161,6 +165,12 @@ static const CBlockIndex* NextSyncBlock(const CBlockIndex* pindex_prev, CChain&
 void BaseIndex::ThreadSync()
 {
     SetSyscallSandboxPolicy(SyscallSandboxPolicy::TX_INDEX);
+    // 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) {
         std::chrono::steady_clock::time_point last_log_time{0s};

src/init.cpp

@@ -123,6 +123,7 @@ using node::CalculateCacheSizes;
 using node::DEFAULT_PERSIST_MEMPOOL;
 using node::DEFAULT_PRINTPRIORITY;
 using node::fReindex;
+using node::g_indexes_ready_to_sync;
 using node::LoadChainstate;
 using node::MempoolPath;
 using node::NodeContext;
@@ -456,7 +457,7 @@ void SetupServerArgs(ArgsManager& argsman)
             "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);
     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::OPTIONS);
-    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::OPTIONS);
+    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::OPTIONS);
     argsman.AddArg("-settings=<file>", 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);
 #if HAVE_SYSTEM
     argsman.AddArg("-startupnotify=<cmd>", "Execute command on startup.", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
@@ -982,19 +983,6 @@ bool AppInitParameterInteraction(const ArgsManager& args, bool use_syscall_sandb
     if (args.GetIntArg("-rpcserialversion", DEFAULT_RPC_SERIALIZE_VERSION) > 1)
         return InitError(Untranslated("Unknown rpcserialversion requested."));
 
-    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)) {
-            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."));
-        }
-    }
-
 #if defined(USE_SYSCALL_SANDBOX)
     if (args.IsArgSet("-sandbox") && !args.IsArgNegated("-sandbox")) {
         const std::string sandbox_arg{args.GetArg("-sandbox", "")};
@@ -1571,6 +1559,9 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
     RegisterValidationInterface(node.peerman.get());
 
     // ********************************************************* 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)) {
         if (const auto error{WITH_LOCK(cs_main, return CheckLegacyTxindex(*Assert(chainman.m_blockman.m_block_tree_db)))}) {
             return InitError(*error);

src/node/blockstorage.cpp

@@ -27,6 +27,7 @@
 
 namespace node {
 std::atomic_bool fReindex(false);
+std::atomic_bool g_indexes_ready_to_sync{false};
 
 bool CBlockIndexWorkComparator::operator()(const CBlockIndex* pa, const CBlockIndex* pb) const
 {
@@ -940,5 +941,6 @@ void ThreadImport(ChainstateManager& chainman, std::vector<fs::path> vImportFile
         }
     } // End scope of ImportingNow
     chainman.ActiveChainstate().LoadMempool(mempool_path);
+    g_indexes_ready_to_sync = true;
 }
 } // namespace node

src/node/blockstorage.h

@@ -47,6 +47,7 @@ static const unsigned int MAX_BLOCKFILE_SIZE = 0x8000000; // 128 MiB
 static constexpr size_t BLOCK_SERIALIZATION_HEADER_SIZE = CMessageHeader::MESSAGE_START_SIZE + sizeof(unsigned int);
 
 extern std::atomic_bool fReindex;
+extern std::atomic_bool g_indexes_ready_to_sync;
 
 // Because validation code takes pointers to the map's CBlockIndex objects, if
 // we ever switch to another associative container, we need to either use a

src/test/util/setup_common.cpp

@@ -155,6 +155,7 @@ BasicTestingSetup::BasicTestingSetup(const ChainType chainType, const std::vecto
         noui_connect();
         noui_connected = true;
     }
+    node::g_indexes_ready_to_sync = true;
 }
 
 BasicTestingSetup::~BasicTestingSetup()

test/functional/feature_coinstatsindex.py

@@ -231,18 +231,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)
 
     def _test_use_index_option(self):
         self.log.info("Test use_index option for nodes running the index")
@@ -261,6 +259,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)

test/functional/p2p_blockfilters.py

@@ -255,13 +255,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'],
-        )
-
 def compute_last_header(prev_header, hashes):
     """Compute the last filter header from a starting header and a sequence of filter hashes."""
     header = ser_uint256(prev_header)