Merge bitcoin/bitcoin#32694: index: move disk read lookups to base class

029ba1a21d index: remove CBlockIndex access from CustomAppend() (furszy)
91b7ab6c69 refactor: index, simplify CopyHeightIndexToHashIndex to process single block (furszy)
6f1392cc42 indexes, refactor: Remove remaining CBlockIndex* uses in index Rewind methods (Ryan Ofsky)
0a248708dc indexes, refactor: Stop requiring CBlockIndex type to call IsBIP30Unspendable (Ryan Ofsky)
331a25cb16 test: indexes, avoid creating threads when sync runs synchronously (furszy)

Pull request description:

  Combining common refactors from #24230 and #26966, aiming to move both efforts forward while reducing their size and review burden.

  Broadly, #24230 focuses on enabling indexes to run in a separate process, and #26966 aims to parallelize the indexes initial synchronization process. A shared prerequisite for both is ensuring that only the base index class interacts with the node’s chain internals - child index classes should instead operate solely through chain events.

  This PR moves disk read lookups from child index classes to the base index class. It also includes a few documentation improvements and a test-only code cleanup.

ACKs for top commit:
  maflcko:
    review ACK 029ba1a21d 👡
  achow101:
    ACK 029ba1a21d
  TheCharlatan:
    Re-ACK 029ba1a21d
  davidgumberg:
    ACK 029ba1a21d
  mzumsande:
    Code Review ACK 029ba1a21d

Tree-SHA512: f073af407fc86f228cb47a32c7bcf2241551cc89ff32059317eb81d5b86fd5fda35f228d2567e0aedbc9fd6826291f5fee05619db35ba44108421ae04d11e6fb

src/index/base.cpp

@@ -14,6 +14,7 @@
 #include <node/database_args.h>
 #include <node/interface_ui.h>
 #include <tinyformat.h>
+#include <undo.h>
 #include <util/string.h>
 #include <util/thread.h>
 #include <util/translation.h>
@@ -143,9 +144,44 @@ static const CBlockIndex* NextSyncBlock(const CBlockIndex* pindex_prev, CChain&
         return pindex;
     }
 
+    // Since block is not in the chain, return the next block in the chain AFTER the last common ancestor.
+    // Caller will be responsible for rewinding back to the common ancestor.
     return chain.Next(chain.FindFork(pindex_prev));
 }
 
+bool BaseIndex::ProcessBlock(const CBlockIndex* pindex, const CBlock* block_data)
+{
+    interfaces::BlockInfo block_info = kernel::MakeBlockInfo(pindex, block_data);
+
+    CBlock block;
+    if (!block_data) { // disk lookup if block data wasn't provided
+        if (!m_chainstate->m_blockman.ReadBlock(block, *pindex)) {
+            FatalErrorf("%s: Failed to read block %s from disk",
+                        __func__, pindex->GetBlockHash().ToString());
+            return false;
+        }
+        block_info.data = &block;
+    }
+
+    CBlockUndo block_undo;
+    if (CustomOptions().connect_undo_data) {
+        if (pindex->nHeight > 0 && !m_chainstate->m_blockman.ReadBlockUndo(block_undo, *pindex)) {
+            FatalErrorf("%s: Failed to read undo block data %s from disk",
+                        __func__, pindex->GetBlockHash().ToString());
+            return false;
+        }
+        block_info.undo_data = &block_undo;
+    }
+
+    if (!CustomAppend(block_info)) {
+        FatalErrorf("%s: Failed to write block %s to index database",
+                    __func__, pindex->GetBlockHash().ToString());
+        return false;
+    }
+
+    return true;
+}
+
 void BaseIndex::Sync()
 {
     const CBlockIndex* pindex = m_best_block_index.load();
@@ -191,20 +227,7 @@ void BaseIndex::Sync()
             pindex = pindex_next;
 
 
-            CBlock block;
-            interfaces::BlockInfo block_info = kernel::MakeBlockInfo(pindex);
-            if (!m_chainstate->m_blockman.ReadBlock(block, *pindex)) {
-                FatalErrorf("%s: Failed to read block %s from disk",
-                           __func__, pindex->GetBlockHash().ToString());
-                return;
-            } else {
-                block_info.data = &block;
-            }
-            if (!CustomAppend(block_info)) {
-                FatalErrorf("%s: Failed to write block %s to index database",
-                           __func__, pindex->GetBlockHash().ToString());
-                return;
-            }
+            if (!ProcessBlock(pindex)) return; // error logged internally
 
             auto current_time{std::chrono::steady_clock::now()};
             if (last_log_time + SYNC_LOG_INTERVAL < current_time) {
@@ -254,8 +277,28 @@ bool BaseIndex::Rewind(const CBlockIndex* current_tip, const CBlockIndex* new_ti
     assert(current_tip == m_best_block_index);
     assert(current_tip->GetAncestor(new_tip->nHeight) == new_tip);
 
-    if (!CustomRewind({current_tip->GetBlockHash(), current_tip->nHeight}, {new_tip->GetBlockHash(), new_tip->nHeight})) {
-        return false;
+    CBlock block;
+    CBlockUndo block_undo;
+
+    for (const CBlockIndex* iter_tip = current_tip; iter_tip != new_tip; iter_tip = iter_tip->pprev) {
+        interfaces::BlockInfo block_info = kernel::MakeBlockInfo(iter_tip);
+        if (CustomOptions().disconnect_data) {
+            if (!m_chainstate->m_blockman.ReadBlock(block, *iter_tip)) {
+                LogError("%s: Failed to read block %s from disk",
+                             __func__, iter_tip->GetBlockHash().ToString());
+                return false;
+            }
+            block_info.data = &block;
+        }
+        if (CustomOptions().disconnect_undo_data && iter_tip->nHeight > 0) {
+            if (!m_chainstate->m_blockman.ReadBlockUndo(block_undo, *iter_tip)) {
+                return false;
+            }
+            block_info.undo_data = &block_undo;
+        }
+        if (!CustomRemove(block_info)) {
+            return false;
+        }
     }
 
     // In the case of a reorg, ensure persisted block locator is not stale.
@@ -316,17 +359,14 @@ void BaseIndex::BlockConnected(ChainstateRole role, const std::shared_ptr<const
             return;
         }
     }
-    interfaces::BlockInfo block_info = kernel::MakeBlockInfo(pindex, block.get());
-    if (CustomAppend(block_info)) {
+
+    // Dispatch block to child class; errors are logged internally and abort the node.
+    if (ProcessBlock(pindex, block.get())) {
         // Setting the best block index is intentionally the last step of this
         // function, so BlockUntilSyncedToCurrentChain callers waiting for the
         // best block index to be updated can rely on the block being fully
         // processed, and the index object being safe to delete.
         SetBestBlockIndex(pindex);
-    } else {
-        FatalErrorf("%s: Failed to write block %s to index",
-                   __func__, pindex->GetBlockHash().ToString());
-        return;
     }
 }
 

src/index/base.h

@@ -90,9 +90,11 @@ private:
     /// getting corrupted.
     bool Commit();
 
-    /// Loop over disconnected blocks and call CustomRewind.
+    /// Loop over disconnected blocks and call CustomRemove.
     bool Rewind(const CBlockIndex* current_tip, const CBlockIndex* new_tip);
 
+    bool ProcessBlock(const CBlockIndex* pindex, const CBlock* block_data = nullptr);
+
     virtual bool AllowPrune() const = 0;
 
     template <typename... Args>
@@ -107,6 +109,9 @@ protected:
 
     void ChainStateFlushed(ChainstateRole role, const CBlockLocator& locator) override;
 
+    /// Return custom notification options for index.
+    [[nodiscard]] virtual interfaces::Chain::NotifyOptions CustomOptions() { return {}; }
+
     /// Initialize internal state from the database and block index.
     [[nodiscard]] virtual bool CustomInit(const std::optional<interfaces::BlockRef>& block) { return true; }
 
@@ -117,9 +122,8 @@ protected:
     /// commit more index state.
     virtual bool CustomCommit(CDBBatch& batch) { return true; }
 
-    /// Rewind index to an earlier chain tip during a chain reorg. The tip must
-    /// be an ancestor of the current best block.
-    [[nodiscard]] virtual bool CustomRewind(const interfaces::BlockRef& current_tip, const interfaces::BlockRef& new_tip) { return true; }
+    /// Rewind index by one block during a chain reorg.
+    [[nodiscard]] virtual bool CustomRemove(const interfaces::BlockInfo& block) { return true; }
 
     virtual DB& GetDB() const = 0;
 

src/index/blockfilterindex.cpp

@@ -13,7 +13,6 @@
 #include <node/blockstorage.h>
 #include <undo.h>
 #include <util/fs_helpers.h>
-#include <validation.h>
 
 /* The index database stores three items for each block: the disk location of the encoded filter,
  * its dSHA256 hash, and the header. Those belonging to blocks on the active chain are indexed by
@@ -112,6 +111,13 @@ BlockFilterIndex::BlockFilterIndex(std::unique_ptr<interfaces::Chain> chain, Blo
     m_filter_fileseq = std::make_unique<FlatFileSeq>(std::move(path), "fltr", FLTR_FILE_CHUNK_SIZE);
 }
 
+interfaces::Chain::NotifyOptions BlockFilterIndex::CustomOptions()
+{
+    interfaces::Chain::NotifyOptions options;
+    options.connect_undo_data = true;
+    return options;
+}
+
 bool BlockFilterIndex::CustomInit(const std::optional<interfaces::BlockRef>& block)
 {
     if (!m_db->Read(DB_FILTER_POS, m_next_filter_pos)) {
@@ -250,19 +256,7 @@ std::optional<uint256> BlockFilterIndex::ReadFilterHeader(int height, const uint
 
 bool BlockFilterIndex::CustomAppend(const interfaces::BlockInfo& block)
 {
-    CBlockUndo block_undo;
-
-    if (block.height > 0) {
-        // pindex variable gives indexing code access to node internals. It
-        // will be removed in upcoming commit
-        const CBlockIndex* pindex = WITH_LOCK(cs_main, return m_chainstate->m_blockman.LookupBlockIndex(block.hash));
-        if (!m_chainstate->m_blockman.ReadBlockUndo(block_undo, *pindex)) {
-            return false;
-        }
-    }
-
-    BlockFilter filter(m_filter_type, *Assert(block.data), block_undo);
-
+    BlockFilter filter(m_filter_type, *Assert(block.data), *Assert(block.undo_data));
     const uint256& header = filter.ComputeHeader(m_last_header);
     bool res = Write(filter, block.height, header);
     if (res) m_last_header = header; // update last header
@@ -289,42 +283,37 @@ bool BlockFilterIndex::Write(const BlockFilter& filter, uint32_t block_height, c
 }
 
 [[nodiscard]] static bool CopyHeightIndexToHashIndex(CDBIterator& db_it, CDBBatch& batch,
-                                       const std::string& index_name,
-                                       int start_height, int stop_height)
+                                                     const std::string& index_name, int height)
 {
-    DBHeightKey key(start_height);
+    DBHeightKey key(height);
     db_it.Seek(key);
 
-    for (int height = start_height; height <= stop_height; ++height) {
-        if (!db_it.GetKey(key) || key.height != height) {
-            LogError("%s: unexpected key in %s: expected (%c, %d)\n",
-                         __func__, index_name, DB_BLOCK_HEIGHT, height);
-            return false;
-        }
-
-        std::pair<uint256, DBVal> value;
-        if (!db_it.GetValue(value)) {
-            LogError("%s: unable to read value in %s at key (%c, %d)\n",
-                         __func__, index_name, DB_BLOCK_HEIGHT, height);
-            return false;
-        }
-
-        batch.Write(DBHashKey(value.first), std::move(value.second));
-
-        db_it.Next();
+    if (!db_it.GetKey(key) || key.height != height) {
+        LogError("%s: unexpected key in %s: expected (%c, %d)\n",
+                 __func__, index_name, DB_BLOCK_HEIGHT, height);
+        return false;
     }
+
+    std::pair<uint256, DBVal> value;
+    if (!db_it.GetValue(value)) {
+        LogError("%s: unable to read value in %s at key (%c, %d)\n",
+                 __func__, index_name, DB_BLOCK_HEIGHT, height);
+        return false;
+    }
+
+    batch.Write(DBHashKey(value.first), std::move(value.second));
     return true;
 }
 
-bool BlockFilterIndex::CustomRewind(const interfaces::BlockRef& current_tip, const interfaces::BlockRef& new_tip)
+bool BlockFilterIndex::CustomRemove(const interfaces::BlockInfo& block)
 {
     CDBBatch batch(*m_db);
     std::unique_ptr<CDBIterator> db_it(m_db->NewIterator());
 
-    // During a reorg, we need to copy all filters for blocks that are getting disconnected from the
-    // height index to the hash index so we can still find them when the height index entries are
-    // overwritten.
-    if (!CopyHeightIndexToHashIndex(*db_it, batch, m_name, new_tip.height, current_tip.height)) {
+    // During a reorg, we need to copy block filter that is getting disconnected from the
+    // height index to the hash index so we can still find it when the height index entry
+    // is overwritten.
+    if (!CopyHeightIndexToHashIndex(*db_it, batch, m_name, block.height)) {
         return false;
     }
 
@@ -334,8 +323,8 @@ bool BlockFilterIndex::CustomRewind(const interfaces::BlockRef& current_tip, con
     batch.Write(DB_FILTER_POS, m_next_filter_pos);
     if (!m_db->WriteBatch(batch)) return false;
 
-    // Update cached header
-    m_last_header = *Assert(ReadFilterHeader(new_tip.height, new_tip.hash));
+    // Update cached header to the previous block hash
+    m_last_header = *Assert(ReadFilterHeader(block.height - 1, *Assert(block.prev_hash)));
     return true;
 }
 

src/index/blockfilterindex.h

@@ -52,13 +52,15 @@ private:
     std::optional<uint256> ReadFilterHeader(int height, const uint256& expected_block_hash);
 
 protected:
+    interfaces::Chain::NotifyOptions CustomOptions() override;
+
     bool CustomInit(const std::optional<interfaces::BlockRef>& block) override;
 
     bool CustomCommit(CDBBatch& batch) override;
 
     bool CustomAppend(const interfaces::BlockInfo& block) override;
 
-    bool CustomRewind(const interfaces::BlockRef& current_tip, const interfaces::BlockRef& new_tip) override;
+    bool CustomRemove(const interfaces::BlockInfo& block) override;
 
     BaseIndex::DB& GetDB() const LIFETIMEBOUND override { return *m_db; }
 

src/index/coinstatsindex.cpp

@@ -114,19 +114,11 @@ CoinStatsIndex::CoinStatsIndex(std::unique_ptr<interfaces::Chain> chain, size_t
 
 bool CoinStatsIndex::CustomAppend(const interfaces::BlockInfo& block)
 {
-    CBlockUndo block_undo;
     const CAmount block_subsidy{GetBlockSubsidy(block.height, Params().GetConsensus())};
     m_total_subsidy += block_subsidy;
 
     // Ignore genesis block
     if (block.height > 0) {
-        // pindex variable gives indexing code access to node internals. It
-        // will be removed in upcoming commit
-        const CBlockIndex* pindex = WITH_LOCK(cs_main, return m_chainstate->m_blockman.LookupBlockIndex(block.hash));
-        if (!m_chainstate->m_blockman.ReadBlockUndo(block_undo, *pindex)) {
-            return false;
-        }
-
         std::pair<uint256, DBVal> read_out;
         if (!m_db->Read(DBHeightKey(block.height - 1), read_out)) {
             return false;
@@ -150,7 +142,7 @@ bool CoinStatsIndex::CustomAppend(const interfaces::BlockInfo& block)
             const auto& tx{block.data->vtx.at(i)};
 
             // Skip duplicate txid coinbase transactions (BIP30).
-            if (IsBIP30Unspendable(*pindex) && tx->IsCoinBase()) {
+            if (IsBIP30Unspendable(block.hash, block.height) && tx->IsCoinBase()) {
                 m_total_unspendable_amount += block_subsidy;
                 m_total_unspendables_bip30 += block_subsidy;
                 continue;
@@ -183,7 +175,7 @@ bool CoinStatsIndex::CustomAppend(const interfaces::BlockInfo& block)
 
             // The coinbase tx has no undo data since no former output is spent
             if (!tx->IsCoinBase()) {
-                const auto& tx_undo{block_undo.vtxundo.at(i - 1)};
+                const auto& tx_undo{Assert(block.undo_data)->vtxundo.at(i - 1)};
 
                 for (size_t j = 0; j < tx_undo.vprevout.size(); ++j) {
                     Coin coin{tx_undo.vprevout[j]};
@@ -238,67 +230,43 @@ bool CoinStatsIndex::CustomAppend(const interfaces::BlockInfo& block)
 }
 
 [[nodiscard]] static bool CopyHeightIndexToHashIndex(CDBIterator& db_it, CDBBatch& batch,
-                                       const std::string& index_name,
-                                       int start_height, int stop_height)
+                                                     const std::string& index_name, int height)
 {
-    DBHeightKey key{start_height};
+    DBHeightKey key{height};
     db_it.Seek(key);
 
-    for (int height = start_height; height <= stop_height; ++height) {
-        if (!db_it.GetKey(key) || key.height != height) {
-            LogError("%s: unexpected key in %s: expected (%c, %d)\n",
-                         __func__, index_name, DB_BLOCK_HEIGHT, height);
-            return false;
-        }
-
-        std::pair<uint256, DBVal> value;
-        if (!db_it.GetValue(value)) {
-            LogError("%s: unable to read value in %s at key (%c, %d)\n",
-                         __func__, index_name, DB_BLOCK_HEIGHT, height);
-            return false;
-        }
-
-        batch.Write(DBHashKey(value.first), std::move(value.second));
-
-        db_it.Next();
+    if (!db_it.GetKey(key) || key.height != height) {
+        LogError("%s: unexpected key in %s: expected (%c, %d)\n",
+                 __func__, index_name, DB_BLOCK_HEIGHT, height);
+        return false;
     }
+
+    std::pair<uint256, DBVal> value;
+    if (!db_it.GetValue(value)) {
+        LogError("%s: unable to read value in %s at key (%c, %d)\n",
+                 __func__, index_name, DB_BLOCK_HEIGHT, height);
+        return false;
+    }
+
+    batch.Write(DBHashKey(value.first), std::move(value.second));
     return true;
 }
 
-bool CoinStatsIndex::CustomRewind(const interfaces::BlockRef& current_tip, const interfaces::BlockRef& new_tip)
+bool CoinStatsIndex::CustomRemove(const interfaces::BlockInfo& block)
 {
     CDBBatch batch(*m_db);
     std::unique_ptr<CDBIterator> db_it(m_db->NewIterator());
 
-    // During a reorg, we need to copy all hash digests for blocks that are
-    // getting disconnected from the height index to the hash index so we can
-    // still find them when the height index entries are overwritten.
-    if (!CopyHeightIndexToHashIndex(*db_it, batch, m_name, new_tip.height, current_tip.height)) {
+    // During a reorg, copy the block's hash digest from the height index to the hash index,
+    // ensuring it's still accessible after the height index entry is overwritten.
+    if (!CopyHeightIndexToHashIndex(*db_it, batch, m_name, block.height)) {
         return false;
     }
 
     if (!m_db->WriteBatch(batch)) return false;
 
-    {
-        LOCK(cs_main);
-        const CBlockIndex* iter_tip{m_chainstate->m_blockman.LookupBlockIndex(current_tip.hash)};
-        const CBlockIndex* new_tip_index{m_chainstate->m_blockman.LookupBlockIndex(new_tip.hash)};
-
-        do {
-            CBlock block;
-
-            if (!m_chainstate->m_blockman.ReadBlock(block, *iter_tip)) {
-                LogError("%s: Failed to read block %s from disk\n",
-                             __func__, iter_tip->GetBlockHash().ToString());
-                return false;
-            }
-
-            if (!ReverseBlock(block, iter_tip)) {
-                return false; // failure cause logged internally
-            }
-
-            iter_tip = iter_tip->GetAncestor(iter_tip->nHeight - 1);
-        } while (new_tip_index != iter_tip);
+    if (!ReverseBlock(block)) {
+        return false; // failure cause logged internally
     }
 
     return true;
@@ -404,26 +372,30 @@ bool CoinStatsIndex::CustomCommit(CDBBatch& batch)
     return true;
 }
 
-// Reverse a single block as part of a reorg
-bool CoinStatsIndex::ReverseBlock(const CBlock& block, const CBlockIndex* pindex)
+interfaces::Chain::NotifyOptions CoinStatsIndex::CustomOptions()
+{
+    interfaces::Chain::NotifyOptions options;
+    options.connect_undo_data = true;
+    options.disconnect_data = true;
+    options.disconnect_undo_data = true;
+    return options;
+}
+
+// Reverse a single block as part of a reorg
+bool CoinStatsIndex::ReverseBlock(const interfaces::BlockInfo& block)
 {
-    CBlockUndo block_undo;
     std::pair<uint256, DBVal> read_out;
 
-    const CAmount block_subsidy{GetBlockSubsidy(pindex->nHeight, Params().GetConsensus())};
+    const CAmount block_subsidy{GetBlockSubsidy(block.height, Params().GetConsensus())};
     m_total_subsidy -= block_subsidy;
 
     // Ignore genesis block
-    if (pindex->nHeight > 0) {
-        if (!m_chainstate->m_blockman.ReadBlockUndo(block_undo, *pindex)) {
+    if (block.height > 0) {
+        if (!m_db->Read(DBHeightKey(block.height - 1), read_out)) {
             return false;
         }
 
-        if (!m_db->Read(DBHeightKey(pindex->nHeight - 1), read_out)) {
-            return false;
-        }
-
-        uint256 expected_block_hash{pindex->pprev->GetBlockHash()};
+        uint256 expected_block_hash{*block.prev_hash};
         if (read_out.first != expected_block_hash) {
             LogPrintf("WARNING: previous block header belongs to unexpected block %s; expected %s\n",
                       read_out.first.ToString(), expected_block_hash.ToString());
@@ -437,13 +409,15 @@ bool CoinStatsIndex::ReverseBlock(const CBlock& block, const CBlockIndex* pindex
     }
 
     // Remove the new UTXOs that were created from the block
-    for (size_t i = 0; i < block.vtx.size(); ++i) {
-        const auto& tx{block.vtx.at(i)};
+    assert(block.data);
+    assert(block.undo_data);
+    for (size_t i = 0; i < block.data->vtx.size(); ++i) {
+        const auto& tx{block.data->vtx.at(i)};
 
         for (uint32_t j = 0; j < tx->vout.size(); ++j) {
             const CTxOut& out{tx->vout[j]};
             COutPoint outpoint{tx->GetHash(), j};
-            Coin coin{out, pindex->nHeight, tx->IsCoinBase()};
+            Coin coin{out, block.height, tx->IsCoinBase()};
 
             // Skip unspendable coins
             if (coin.out.scriptPubKey.IsUnspendable()) {
@@ -467,7 +441,7 @@ bool CoinStatsIndex::ReverseBlock(const CBlock& block, const CBlockIndex* pindex
 
         // The coinbase tx has no undo data since no former output is spent
         if (!tx->IsCoinBase()) {
-            const auto& tx_undo{block_undo.vtxundo.at(i - 1)};
+            const auto& tx_undo{block.undo_data->vtxundo.at(i - 1)};
 
             for (size_t j = 0; j < tx_undo.vprevout.size(); ++j) {
                 Coin coin{tx_undo.vprevout[j]};

src/index/coinstatsindex.h

@@ -38,18 +38,20 @@ private:
     CAmount m_total_unspendables_scripts{0};
     CAmount m_total_unspendables_unclaimed_rewards{0};
 
-    [[nodiscard]] bool ReverseBlock(const CBlock& block, const CBlockIndex* pindex);
+    [[nodiscard]] bool ReverseBlock(const interfaces::BlockInfo& block);
 
     bool AllowPrune() const override { return true; }
 
 protected:
+    interfaces::Chain::NotifyOptions CustomOptions() override;
+
     bool CustomInit(const std::optional<interfaces::BlockRef>& block) override;
 
     bool CustomCommit(CDBBatch& batch) override;
 
     bool CustomAppend(const interfaces::BlockInfo& block) override;
 
-    bool CustomRewind(const interfaces::BlockRef& current_tip, const interfaces::BlockRef& new_tip) override;
+    bool CustomRemove(const interfaces::BlockInfo& block) override;
 
     BaseIndex::DB& GetDB() const override { return *m_db; }
 

src/interfaces/chain.h

@@ -327,6 +327,17 @@ public:
         virtual void chainStateFlushed(ChainstateRole role, const CBlockLocator& locator) {}
     };
 
+    //! Options specifying which chain notifications are required.
+    struct NotifyOptions
+    {
+        //! Include undo data with block connected notifications.
+        bool connect_undo_data = false;
+        //! Include block data with block disconnected notifications.
+        bool disconnect_data = false;
+        //! Include undo data with block disconnected notifications.
+        bool disconnect_undo_data = false;
+    };
+
     //! Register handler for notifications.
     virtual std::unique_ptr<Handler> handleNotifications(std::shared_ptr<Notifications> notifications) = 0;
 

src/test/blockfilter_index_tests.cpp

@@ -12,7 +12,6 @@
 #include <node/miner.h>
 #include <pow.h>
 #include <test/util/blockfilter.h>
-#include <test/util/index.h>
 #include <test/util/setup_common.h>
 #include <validation.h>
 
@@ -143,10 +142,7 @@ BOOST_FIXTURE_TEST_CASE(blockfilter_index_initial_sync, BuildChainTestingSetup)
     // BlockUntilSyncedToCurrentChain should return false before index is started.
     BOOST_CHECK(!filter_index.BlockUntilSyncedToCurrentChain());
 
-    BOOST_REQUIRE(filter_index.StartBackgroundSync());
-
-    // Allow filter index to catch up with the block index.
-    IndexWaitSynced(filter_index, *Assert(m_node.shutdown_signal));
+    filter_index.Sync();
 
     // Check that filter index has all blocks that were in the chain before it started.
     {

src/test/coinstatsindex_tests.cpp

@@ -6,7 +6,6 @@
 #include <index/coinstatsindex.h>
 #include <interfaces/chain.h>
 #include <kernel/coinstats.h>
-#include <test/util/index.h>
 #include <test/util/setup_common.h>
 #include <test/util/validation.h>
 #include <validation.h>
@@ -33,9 +32,7 @@ BOOST_FIXTURE_TEST_CASE(coinstatsindex_initial_sync, TestChain100Setup)
     // is started.
     BOOST_CHECK(!coin_stats_index.BlockUntilSyncedToCurrentChain());
 
-    BOOST_REQUIRE(coin_stats_index.StartBackgroundSync());
-
-    IndexWaitSynced(coin_stats_index, *Assert(m_node.shutdown_signal));
+    coin_stats_index.Sync();
 
     // Check that CoinStatsIndex works for genesis block.
     const CBlockIndex* genesis_block_index;
@@ -85,8 +82,7 @@ BOOST_FIXTURE_TEST_CASE(coinstatsindex_unclean_shutdown, TestChain100Setup)
     {
         CoinStatsIndex index{interfaces::MakeChain(m_node), 1 << 20};
         BOOST_REQUIRE(index.Init());
-        BOOST_REQUIRE(index.StartBackgroundSync());
-        IndexWaitSynced(index, *Assert(m_node.shutdown_signal));
+        index.Sync();
         std::shared_ptr<const CBlock> new_block;
         CBlockIndex* new_block_index = nullptr;
         {

src/test/txindex_tests.cpp

@@ -6,7 +6,6 @@
 #include <chainparams.h>
 #include <index/txindex.h>
 #include <interfaces/chain.h>
-#include <test/util/index.h>
 #include <test/util/setup_common.h>
 #include <validation.h>
 
@@ -30,10 +29,7 @@ BOOST_FIXTURE_TEST_CASE(txindex_initial_sync, TestChain100Setup)
     // BlockUntilSyncedToCurrentChain should return false before txindex is started.
     BOOST_CHECK(!txindex.BlockUntilSyncedToCurrentChain());
 
-    BOOST_REQUIRE(txindex.StartBackgroundSync());
-
-    // Allow tx index to catch up with the block index.
-    IndexWaitSynced(txindex, *Assert(m_node.shutdown_signal));
+    txindex.Sync();
 
     // Check that txindex excludes genesis block transactions.
     const CBlock& genesis_block = Params().GenesisBlock();

src/test/util/CMakeLists.txt

@@ -6,7 +6,6 @@ add_library(test_util STATIC EXCLUDE_FROM_ALL
   blockfilter.cpp
   coins.cpp
   coverage.cpp
-  index.cpp
   json.cpp
   logging.cpp
   mining.cpp

src/test/util/index.cpp

@@ -1,22 +0,0 @@
-// Copyright (c) 2020-2022 The Bitcoin Core developers
-// Distributed under the MIT software license, see the accompanying
-// file COPYING or http://www.opensource.org/licenses/mit-license.php.
-
-#include <test/util/index.h>
-
-#include <index/base.h>
-#include <util/check.h>
-#include <util/signalinterrupt.h>
-#include <util/time.h>
-
-void IndexWaitSynced(const BaseIndex& index, const util::SignalInterrupt& interrupt)
-{
-    while (!index.BlockUntilSyncedToCurrentChain()) {
-        // Assert shutdown was not requested to abort the test, instead of looping forever, in case
-        // there was an unexpected error in the index that caused it to stop syncing and request a shutdown.
-        Assert(!interrupt);
-
-        UninterruptibleSleep(100ms);
-    }
-    assert(index.GetSummary().synced);
-}

src/test/util/index.h

@@ -1,16 +0,0 @@
-// Copyright (c) 2020-2022 The Bitcoin Core developers
-// Distributed under the MIT software license, see the accompanying
-// file COPYING or http://www.opensource.org/licenses/mit-license.php.
-
-#ifndef BITCOIN_TEST_UTIL_INDEX_H
-#define BITCOIN_TEST_UTIL_INDEX_H
-
-class BaseIndex;
-namespace util {
-class SignalInterrupt;
-} // namespace util
-
-/** Block until the index is synced to the current chain */
-void IndexWaitSynced(const BaseIndex& index, const util::SignalInterrupt& interrupt);
-
-#endif // BITCOIN_TEST_UTIL_INDEX_H

src/validation.cpp

@@ -6328,10 +6328,10 @@ bool IsBIP30Repeat(const CBlockIndex& block_index)
            (block_index.nHeight==91880 && block_index.GetBlockHash() == uint256{"00000000000743f190a18c5577a3c2d2a1f610ae9601ac046a38084ccb7cd721"});
 }
 
-bool IsBIP30Unspendable(const CBlockIndex& block_index)
+bool IsBIP30Unspendable(const uint256& block_hash, int block_height)
 {
-    return (block_index.nHeight==91722 && block_index.GetBlockHash() == uint256{"00000000000271a2dc26e7667f8419f2e15416dc6955e5a6c6cdf3f2574dd08e"}) ||
-           (block_index.nHeight==91812 && block_index.GetBlockHash() == uint256{"00000000000af0aed4792b1acee3d966af36cf5def14935db8de83d6f9306f2f"});
+    return (block_height==91722 && block_hash == uint256{"00000000000271a2dc26e7667f8419f2e15416dc6955e5a6c6cdf3f2574dd08e"}) ||
+           (block_height==91812 && block_hash == uint256{"00000000000af0aed4792b1acee3d966af36cf5def14935db8de83d6f9306f2f"});
 }
 
 static fs::path GetSnapshotCoinsDBPath(Chainstate& cs) EXCLUSIVE_LOCKS_REQUIRED(::cs_main)

src/validation.h

@@ -1332,6 +1332,6 @@ bool DeploymentEnabled(const ChainstateManager& chainman, DEP dep)
 bool IsBIP30Repeat(const CBlockIndex& block_index);
 
 /** Identifies blocks which coinbase output was subsequently overwritten in the UTXO set (see BIP30) */
-bool IsBIP30Unspendable(const CBlockIndex& block_index);
+bool IsBIP30Unspendable(const uint256& block_hash, int block_height);
 
 #endif // BITCOIN_VALIDATION_H