From 6f1392cc42cde638773f2b697d7d2c58abcdc860 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Sun, 23 Jan 2022 23:20:41 -0500 Subject: [PATCH] indexes, refactor: Remove remaining CBlockIndex* uses in index Rewind methods Move ReadBlock code from CoinStatsIndex::CustomRewind to BaseIndex::Rewind Move ReadUndo code from CoinStatsIndex::ReverseBlock to BaseIndex::Rewind This commit does change behavior slightly. Since the new CustomRemove methods only take a single block at a time instead of a range of disconnected blocks, when they call CopyHeightIndexToHashIndex they will now do an index seek for each removed block instead of only seeking once to the height of the earliest removed block. Seeking instead of scanning is a little worse for performance if there is a >1 block reorg, but probably not noticeable unless the reorg is very deep. --- src/index/base.cpp | 25 ++++++++++++-- src/index/base.h | 10 +++--- src/index/blockfilterindex.cpp | 8 ++--- src/index/blockfilterindex.h | 2 +- src/index/coinstatsindex.cpp | 61 +++++++++++++--------------------- src/index/coinstatsindex.h | 6 ++-- src/interfaces/chain.h | 9 +++++ 7 files changed, 71 insertions(+), 50 deletions(-) diff --git a/src/index/base.cpp b/src/index/base.cpp index 5767116ce53..0e76bafecaa 100644 --- a/src/index/base.cpp +++ b/src/index/base.cpp @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -254,8 +255,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 = █ + } + 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. diff --git a/src/index/base.h b/src/index/base.h index fbd9069a515..841bfcb00e8 100644 --- a/src/index/base.h +++ b/src/index/base.h @@ -90,7 +90,7 @@ 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); virtual bool AllowPrune() const = 0; @@ -107,6 +107,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& block) { return true; } @@ -117,9 +120,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; diff --git a/src/index/blockfilterindex.cpp b/src/index/blockfilterindex.cpp index 5ce85e1f844..460d5d7c41b 100644 --- a/src/index/blockfilterindex.cpp +++ b/src/index/blockfilterindex.cpp @@ -316,7 +316,7 @@ bool BlockFilterIndex::Write(const BlockFilter& filter, uint32_t block_height, c 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 db_it(m_db->NewIterator()); @@ -324,7 +324,7 @@ bool BlockFilterIndex::CustomRewind(const interfaces::BlockRef& current_tip, con // 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)) { + if (!CopyHeightIndexToHashIndex(*db_it, batch, m_name, block.height - 1, block.height)) { return false; } @@ -334,8 +334,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; } diff --git a/src/index/blockfilterindex.h b/src/index/blockfilterindex.h index ccb4845ef5e..13543b961cc 100644 --- a/src/index/blockfilterindex.h +++ b/src/index/blockfilterindex.h @@ -58,7 +58,7 @@ protected: 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; } diff --git a/src/index/coinstatsindex.cpp b/src/index/coinstatsindex.cpp index 9ec034ce3c5..5f82cf93b7e 100644 --- a/src/index/coinstatsindex.cpp +++ b/src/index/coinstatsindex.cpp @@ -265,7 +265,7 @@ bool CoinStatsIndex::CustomAppend(const interfaces::BlockInfo& block) 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 db_it(m_db->NewIterator()); @@ -273,32 +273,14 @@ bool CoinStatsIndex::CustomRewind(const interfaces::BlockRef& current_tip, const // 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)) { + if (!CopyHeightIndexToHashIndex(*db_it, batch, m_name, block.height - 1, 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 +386,29 @@ 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.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 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 +422,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 +454,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]}; diff --git a/src/index/coinstatsindex.h b/src/index/coinstatsindex.h index 885b9e0a860..6e2743688ad 100644 --- a/src/index/coinstatsindex.h +++ b/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& 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; } diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index de99262e982..f98eeeed8bb 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -327,6 +327,15 @@ public: virtual void chainStateFlushed(ChainstateRole role, const CBlockLocator& locator) {} }; + //! Options specifying which chain notifications are required. + struct NotifyOptions + { + //! 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 handleNotifications(std::shared_ptr notifications) = 0;