From 029ba1a21d570f7db6c4366ec9a30a381b56d6fb Mon Sep 17 00:00:00 2001 From: furszy Date: Sat, 18 Feb 2023 10:39:24 -0300 Subject: [PATCH] index: remove CBlockIndex access from CustomAppend() Moved CBlockUndo disk read lookups from child index classes to the base index class. The goal is for child index classes to synchronize only through events, without directly accessing the chain database. This change will enable future parallel synchronization mechanisms, reduce database access (when batched), and contribute toward the goal of running indexes in a separate process (with no chain database access). Besides that, this commit also documents how NextSyncBlock() behaves. It is not immediately clear this function could return the first block after the fork point during a reorg. --- src/index/base.cpp | 59 ++++++++++++++++++++++------------ src/index/base.h | 2 ++ src/index/blockfilterindex.cpp | 22 +++++-------- src/index/blockfilterindex.h | 2 ++ src/index/coinstatsindex.cpp | 11 ++----- src/interfaces/chain.h | 2 ++ 6 files changed, 55 insertions(+), 43 deletions(-) diff --git a/src/index/base.cpp b/src/index/base.cpp index 0e76bafeca..a5c0cc29c9 100644 --- a/src/index/base.cpp +++ b/src/index/base.cpp @@ -144,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 = █ + } + + 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(); @@ -192,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 = █ - } - 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) { @@ -337,17 +359,14 @@ void BaseIndex::BlockConnected(ChainstateRole role, const std::shared_ptrGetBlockHash().ToString()); - return; } } diff --git a/src/index/base.h b/src/index/base.h index 841bfcb00e..4131b06cad 100644 --- a/src/index/base.h +++ b/src/index/base.h @@ -93,6 +93,8 @@ private: /// 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 diff --git a/src/index/blockfilterindex.cpp b/src/index/blockfilterindex.cpp index de55170daa..de43e8e271 100644 --- a/src/index/blockfilterindex.cpp +++ b/src/index/blockfilterindex.cpp @@ -13,7 +13,6 @@ #include #include #include -#include /* 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 chain, Blo m_filter_fileseq = std::make_unique(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& block) { if (!m_db->Read(DB_FILTER_POS, m_next_filter_pos)) { @@ -250,19 +256,7 @@ std::optional 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 diff --git a/src/index/blockfilterindex.h b/src/index/blockfilterindex.h index 13543b961c..983cf32206 100644 --- a/src/index/blockfilterindex.h +++ b/src/index/blockfilterindex.h @@ -52,6 +52,8 @@ private: std::optional ReadFilterHeader(int height, const uint256& expected_block_hash); protected: + interfaces::Chain::NotifyOptions CustomOptions() override; + bool CustomInit(const std::optional& block) override; bool CustomCommit(CDBBatch& batch) override; diff --git a/src/index/coinstatsindex.cpp b/src/index/coinstatsindex.cpp index 28e7d66e8f..ef2af8a567 100644 --- a/src/index/coinstatsindex.cpp +++ b/src/index/coinstatsindex.cpp @@ -114,19 +114,11 @@ CoinStatsIndex::CoinStatsIndex(std::unique_ptr 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 read_out; if (!m_db->Read(DBHeightKey(block.height - 1), read_out)) { return false; @@ -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]}; @@ -383,6 +375,7 @@ bool CoinStatsIndex::CustomCommit(CDBBatch& batch) 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; diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index f98eeeed8b..f6e831624d 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -330,6 +330,8 @@ public: //! 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.