From 613c46fe9e39f55b0f0daa18fee20b4120db2539 Mon Sep 17 00:00:00 2001 From: James O'Beirne Date: Wed, 27 Mar 2019 17:07:32 -0400 Subject: [PATCH 1/4] refactoring: move block metadata structures into BlockManager Separate out the management of chain-agnostic block metadata from any given CChainState instance. This allows us to avoid duplicating data like `mapBlockIndex` unnecessarily for multiple chainstates. This also adds a CChainState constructor that accepts and sets m_blockman. Ultimately this reference will point to a BlockMan instance that is shared across CChainStates. This commit can be decomposed into smaller commits if necessary. --- src/validation.cpp | 145 ++++++++++++++++++++++++--------------------- src/validation.h | 128 ++++++++++++++++++++++++--------------- 2 files changed, 158 insertions(+), 115 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index 262b6856a..75d5bb9fe 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -77,7 +77,11 @@ bool CBlockIndexWorkComparator::operator()(const CBlockIndex *pa, const CBlockIn return false; } -static CChainState g_chainstate; +namespace { +BlockManager g_blockman; +} // anon namespace + +static CChainState g_chainstate(g_blockman); CChainState& ChainstateActive() { return g_chainstate; } @@ -95,7 +99,7 @@ CChain& ChainActive() { return g_chainstate.m_chain; } */ RecursiveMutex cs_main; -BlockMap& mapBlockIndex = ::ChainstateActive().mapBlockIndex; +BlockMap& mapBlockIndex = g_blockman.m_block_index; CBlockIndex *pindexBestHeader = nullptr; Mutex g_best_block_mutex; std::condition_variable g_best_block_cv; @@ -127,11 +131,6 @@ CScript COINBASE_FLAGS; namespace { CBlockIndex *&pindexBestInvalid = ::ChainstateActive().pindexBestInvalid; - /** All pairs A->B, where A (or one of its ancestors) misses transactions, but B has transactions. - * Pruned nodes may have entries where B is missing data. - */ - std::multimap& mapBlocksUnlinked = ::ChainstateActive().mapBlocksUnlinked; - CCriticalSection cs_LastBlockFile; std::vector vinfoBlockFile; int nLastBlockFile = 0; @@ -1160,7 +1159,7 @@ void static InvalidChainFound(CBlockIndex* pindexNew) EXCLUSIVE_LOCKS_REQUIRED(c void CChainState::InvalidBlockFound(CBlockIndex *pindex, const CValidationState &state) { if (state.GetReason() != ValidationInvalidReason::BLOCK_MUTATED) { pindex->nStatus |= BLOCK_FAILED_VALID; - m_failed_blocks.insert(pindex); + m_blockman.m_failed_blocks.insert(pindex); setDirtyBlockIndex.insert(pindex); setBlockIndexCandidates.erase(pindex); InvalidChainFound(pindex); @@ -1695,8 +1694,8 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl // relative to a piece of software is an objective fact these defaults can be easily reviewed. // This setting doesn't force the selection of any particular chain but makes validating some faster by // effectively caching the result of part of the verification. - BlockMap::const_iterator it = mapBlockIndex.find(hashAssumeValid); - if (it != mapBlockIndex.end()) { + BlockMap::const_iterator it = m_blockman.m_block_index.find(hashAssumeValid); + if (it != m_blockman.m_block_index.end()) { if (it->second->GetAncestor(pindex->nHeight) == pindex && pindexBestHeader->GetAncestor(pindex->nHeight) == pindex && pindexBestHeader->nChainWork >= nMinimumChainWork) { @@ -2366,10 +2365,11 @@ CBlockIndex* CChainState::FindMostWorkChain() { if (fFailedChain) { pindexFailed->nStatus |= BLOCK_FAILED_CHILD; } else if (fMissingData) { - // If we're missing data, then add back to mapBlocksUnlinked, + // If we're missing data, then add back to m_blocks_unlinked, // so that if the block arrives in the future we can try adding // to setBlockIndexCandidates again. - mapBlocksUnlinked.insert(std::make_pair(pindexFailed->pprev, pindexFailed)); + m_blockman.m_blocks_unlinked.insert( + std::make_pair(pindexFailed->pprev, pindexFailed)); } setBlockIndexCandidates.erase(pindexFailed); pindexFailed = pindexFailed->pprev; @@ -2720,12 +2720,12 @@ bool CChainState::InvalidateBlock(CValidationState& state, const CChainParams& c to_mark_failed->nStatus |= BLOCK_FAILED_VALID; setDirtyBlockIndex.insert(to_mark_failed); setBlockIndexCandidates.erase(to_mark_failed); - m_failed_blocks.insert(to_mark_failed); + m_blockman.m_failed_blocks.insert(to_mark_failed); // The resulting new best tip may not be in setBlockIndexCandidates anymore, so // add it again. - BlockMap::iterator it = mapBlockIndex.begin(); - while (it != mapBlockIndex.end()) { + BlockMap::iterator it = m_blockman.m_block_index.begin(); + while (it != m_blockman.m_block_index.end()) { if (it->second->IsValid(BLOCK_VALID_TRANSACTIONS) && it->second->HaveTxsDownloaded() && !setBlockIndexCandidates.value_comp()(it->second, m_chain.Tip())) { setBlockIndexCandidates.insert(it->second); } @@ -2752,8 +2752,8 @@ void CChainState::ResetBlockFailureFlags(CBlockIndex *pindex) { int nHeight = pindex->nHeight; // Remove the invalidity flag from this block and all its descendants. - BlockMap::iterator it = mapBlockIndex.begin(); - while (it != mapBlockIndex.end()) { + BlockMap::iterator it = m_blockman.m_block_index.begin(); + while (it != m_blockman.m_block_index.end()) { if (!it->second->IsValid() && it->second->GetAncestor(nHeight) == pindex) { it->second->nStatus &= ~BLOCK_FAILED_MASK; setDirtyBlockIndex.insert(it->second); @@ -2764,7 +2764,7 @@ void CChainState::ResetBlockFailureFlags(CBlockIndex *pindex) { // Reset invalid block marker if it was pointing to one of those. pindexBestInvalid = nullptr; } - m_failed_blocks.erase(it->second); + m_blockman.m_failed_blocks.erase(it->second); } it++; } @@ -2774,7 +2774,7 @@ void CChainState::ResetBlockFailureFlags(CBlockIndex *pindex) { if (pindex->nStatus & BLOCK_FAILED_MASK) { pindex->nStatus &= ~BLOCK_FAILED_MASK; setDirtyBlockIndex.insert(pindex); - m_failed_blocks.erase(pindex); + m_blockman.m_failed_blocks.erase(pindex); } pindex = pindex->pprev; } @@ -2784,14 +2784,14 @@ void ResetBlockFailureFlags(CBlockIndex *pindex) { return ::ChainstateActive().ResetBlockFailureFlags(pindex); } -CBlockIndex* CChainState::AddToBlockIndex(const CBlockHeader& block) +CBlockIndex* BlockManager::AddToBlockIndex(const CBlockHeader& block) { AssertLockHeld(cs_main); // Check for duplicate uint256 hash = block.GetHash(); - BlockMap::iterator it = mapBlockIndex.find(hash); - if (it != mapBlockIndex.end()) + BlockMap::iterator it = m_block_index.find(hash); + if (it != m_block_index.end()) return it->second; // Construct new block index object @@ -2800,10 +2800,10 @@ CBlockIndex* CChainState::AddToBlockIndex(const CBlockHeader& block) // to avoid miners withholding blocks but broadcasting headers, to get a // competitive advantage. pindexNew->nSequenceId = 0; - BlockMap::iterator mi = mapBlockIndex.insert(std::make_pair(hash, pindexNew)).first; + BlockMap::iterator mi = m_block_index.insert(std::make_pair(hash, pindexNew)).first; pindexNew->phashBlock = &((*mi).first); - BlockMap::iterator miPrev = mapBlockIndex.find(block.hashPrevBlock); - if (miPrev != mapBlockIndex.end()) + BlockMap::iterator miPrev = m_block_index.find(block.hashPrevBlock); + if (miPrev != m_block_index.end()) { pindexNew->pprev = (*miPrev).second; pindexNew->nHeight = pindexNew->pprev->nHeight + 1; @@ -2852,17 +2852,17 @@ void CChainState::ReceivedBlockTransactions(const CBlock& block, CBlockIndex* pi if (m_chain.Tip() == nullptr || !setBlockIndexCandidates.value_comp()(pindex, m_chain.Tip())) { setBlockIndexCandidates.insert(pindex); } - std::pair::iterator, std::multimap::iterator> range = mapBlocksUnlinked.equal_range(pindex); + std::pair::iterator, std::multimap::iterator> range = m_blockman.m_blocks_unlinked.equal_range(pindex); while (range.first != range.second) { std::multimap::iterator it = range.first; queue.push_back(it->second); range.first++; - mapBlocksUnlinked.erase(it); + m_blockman.m_blocks_unlinked.erase(it); } } } else { if (pindexNew->pprev && pindexNew->pprev->IsValid(BLOCK_VALID_TREE)) { - mapBlocksUnlinked.insert(std::make_pair(pindexNew->pprev, pindexNew)); + m_blockman.m_blocks_unlinked.insert(std::make_pair(pindexNew->pprev, pindexNew)); } } } @@ -3230,15 +3230,15 @@ static bool ContextualCheckBlock(const CBlock& block, CValidationState& state, c return true; } -bool CChainState::AcceptBlockHeader(const CBlockHeader& block, CValidationState& state, const CChainParams& chainparams, CBlockIndex** ppindex) +bool BlockManager::AcceptBlockHeader(const CBlockHeader& block, CValidationState& state, const CChainParams& chainparams, CBlockIndex** ppindex) { AssertLockHeld(cs_main); // Check for duplicate uint256 hash = block.GetHash(); - BlockMap::iterator miSelf = mapBlockIndex.find(hash); + BlockMap::iterator miSelf = m_block_index.find(hash); CBlockIndex *pindex = nullptr; if (hash != chainparams.GetConsensus().hashGenesisBlock) { - if (miSelf != mapBlockIndex.end()) { + if (miSelf != m_block_index.end()) { // Block header is already known. pindex = miSelf->second; if (ppindex) @@ -3253,8 +3253,8 @@ bool CChainState::AcceptBlockHeader(const CBlockHeader& block, CValidationState& // Get prev block index CBlockIndex* pindexPrev = nullptr; - BlockMap::iterator mi = mapBlockIndex.find(block.hashPrevBlock); - if (mi == mapBlockIndex.end()) + BlockMap::iterator mi = m_block_index.find(block.hashPrevBlock); + if (mi == m_block_index.end()) return state.Invalid(ValidationInvalidReason::BLOCK_MISSING_PREV, error("%s: prev block not found", __func__), 0, "prev-blk-not-found"); pindexPrev = (*mi).second; if (pindexPrev->nStatus & BLOCK_FAILED_MASK) @@ -3306,8 +3306,6 @@ bool CChainState::AcceptBlockHeader(const CBlockHeader& block, CValidationState& if (ppindex) *ppindex = pindex; - CheckBlockIndex(chainparams.GetConsensus()); - return true; } @@ -3319,7 +3317,10 @@ bool ProcessNewBlockHeaders(const std::vector& headers, CValidatio LOCK(cs_main); for (const CBlockHeader& header : headers) { CBlockIndex *pindex = nullptr; // Use a temp pindex instead of ppindex to avoid a const_cast - if (!::ChainstateActive().AcceptBlockHeader(header, state, chainparams, &pindex)) { + bool accepted = g_blockman.AcceptBlockHeader(header, state, chainparams, &pindex); + ::ChainstateActive().CheckBlockIndex(chainparams.GetConsensus()); + + if (!accepted) { if (first_invalid) *first_invalid = header; return false; } @@ -3362,7 +3363,10 @@ bool CChainState::AcceptBlock(const std::shared_ptr& pblock, CVali CBlockIndex *pindexDummy = nullptr; CBlockIndex *&pindex = ppindex ? *ppindex : pindexDummy; - if (!AcceptBlockHeader(block, state, chainparams, &pindex)) + bool accepted_header = m_blockman.AcceptBlockHeader(block, state, chainparams, &pindex); + CheckBlockIndex(chainparams.GetConsensus()); + + if (!accepted_header) return false; // Try to process all requested blocks that we don't have, but only @@ -3513,7 +3517,7 @@ void PruneOneBlockFile(const int fileNumber) { LOCK(cs_LastBlockFile); - for (const auto& entry : mapBlockIndex) { + for (const auto& entry : g_blockman.m_block_index) { CBlockIndex* pindex = entry.second; if (pindex->nFile == fileNumber) { pindex->nStatus &= ~BLOCK_HAVE_DATA; @@ -3523,16 +3527,16 @@ void PruneOneBlockFile(const int fileNumber) pindex->nUndoPos = 0; setDirtyBlockIndex.insert(pindex); - // Prune from mapBlocksUnlinked -- any block we prune would have + // Prune from m_blocks_unlinked -- any block we prune would have // to be downloaded again in order to consider its chain, at which // point it would be considered as a candidate for - // mapBlocksUnlinked or setBlockIndexCandidates. - std::pair::iterator, std::multimap::iterator> range = mapBlocksUnlinked.equal_range(pindex->pprev); + // m_blocks_unlinked or setBlockIndexCandidates. + auto range = g_blockman.m_blocks_unlinked.equal_range(pindex->pprev); while (range.first != range.second) { std::multimap::iterator _it = range.first; range.first++; if (_it->second == pindex) { - mapBlocksUnlinked.erase(_it); + g_blockman.m_blocks_unlinked.erase(_it); } } } @@ -3681,7 +3685,7 @@ fs::path GetBlockPosFilename(const FlatFilePos &pos) return BlockFileSeq().FileName(pos); } -CBlockIndex * CChainState::InsertBlockIndex(const uint256& hash) +CBlockIndex * BlockManager::InsertBlockIndex(const uint256& hash) { AssertLockHeld(cs_main); @@ -3689,19 +3693,19 @@ CBlockIndex * CChainState::InsertBlockIndex(const uint256& hash) return nullptr; // Return existing - BlockMap::iterator mi = mapBlockIndex.find(hash); - if (mi != mapBlockIndex.end()) + BlockMap::iterator mi = m_block_index.find(hash); + if (mi != m_block_index.end()) return (*mi).second; // Create new CBlockIndex* pindexNew = new CBlockIndex(); - mi = mapBlockIndex.insert(std::make_pair(hash, pindexNew)).first; + mi = m_block_index.insert(std::make_pair(hash, pindexNew)).first; pindexNew->phashBlock = &((*mi).first); return pindexNew; } -bool CChainState::LoadBlockIndex(const Consensus::Params& consensus_params, CBlockTreeDB& blocktree) +bool BlockManager::LoadBlockIndex(const Consensus::Params& consensus_params, CBlockTreeDB& blocktree) { if (!blocktree.LoadBlockIndexGuts(consensus_params, [this](const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { return this->InsertBlockIndex(hash); })) return false; @@ -3729,7 +3733,7 @@ bool CChainState::LoadBlockIndex(const Consensus::Params& consensus_params, CBlo pindex->nChainTx = pindex->pprev->nChainTx + pindex->nTx; } else { pindex->nChainTx = 0; - mapBlocksUnlinked.insert(std::make_pair(pindex->pprev, pindex)); + m_blocks_unlinked.insert(std::make_pair(pindex->pprev, pindex)); } } else { pindex->nChainTx = pindex->nTx; @@ -3740,7 +3744,7 @@ bool CChainState::LoadBlockIndex(const Consensus::Params& consensus_params, CBlo setDirtyBlockIndex.insert(pindex); } if (pindex->IsValid(BLOCK_VALID_TRANSACTIONS) && (pindex->HaveTxsDownloaded() || pindex->pprev == nullptr)) - setBlockIndexCandidates.insert(pindex); + ::ChainstateActive().setBlockIndexCandidates.insert(pindex); if (pindex->nStatus & BLOCK_FAILED_MASK && (!pindexBestInvalid || pindex->nChainWork > pindexBestInvalid->nChainWork)) pindexBestInvalid = pindex; if (pindex->pprev) @@ -3752,9 +3756,20 @@ bool CChainState::LoadBlockIndex(const Consensus::Params& consensus_params, CBlo return true; } +void BlockManager::Unload() { + m_failed_blocks.clear(); + m_blocks_unlinked.clear(); + + for (const BlockMap::value_type& entry : m_block_index) { + delete entry.second; + } + + m_block_index.clear(); +} + bool static LoadBlockIndexDB(const CChainParams& chainparams) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { - if (!::ChainstateActive().LoadBlockIndex(chainparams.GetConsensus(), *pblocktree)) + if (!g_blockman.LoadBlockIndex(chainparams.GetConsensus(), *pblocktree)) return false; // Load block file info @@ -4051,10 +4066,10 @@ void CChainState::EraseBlockData(CBlockIndex* index) setDirtyBlockIndex.insert(index); // Update indexes setBlockIndexCandidates.erase(index); - std::pair::iterator, std::multimap::iterator> ret = mapBlocksUnlinked.equal_range(index->pprev); + auto ret = m_blockman.m_blocks_unlinked.equal_range(index->pprev); while (ret.first != ret.second) { if (ret.first->second == index) { - mapBlocksUnlinked.erase(ret.first++); + m_blockman.m_blocks_unlinked.erase(ret.first++); } else { ++ret.first; } @@ -4180,7 +4195,6 @@ bool RewindBlockIndex(const CChainParams& params) { void CChainState::UnloadBlockIndex() { nBlockSequenceId = 1; - m_failed_blocks.clear(); setBlockIndexCandidates.clear(); } @@ -4191,10 +4205,10 @@ void UnloadBlockIndex() { LOCK(cs_main); ::ChainActive().SetTip(nullptr); + g_blockman.Unload(); pindexBestInvalid = nullptr; pindexBestHeader = nullptr; mempool.clear(); - mapBlocksUnlinked.clear(); vinfoBlockFile.clear(); nLastBlockFile = 0; setDirtyBlockIndex.clear(); @@ -4203,11 +4217,6 @@ void UnloadBlockIndex() for (int b = 0; b < VERSIONBITS_NUM_BITS; b++) { warningcache[b].clear(); } - - for (const BlockMap::value_type& entry : mapBlockIndex) { - delete entry.second; - } - mapBlockIndex.clear(); fHavePruned = false; ::ChainstateActive().UnloadBlockIndex(); @@ -4251,7 +4260,7 @@ bool CChainState::LoadGenesisBlock(const CChainParams& chainparams) FlatFilePos blockPos = SaveBlockToDisk(block, 0, chainparams, nullptr); if (blockPos.IsNull()) return error("%s: writing genesis block to disk failed", __func__); - CBlockIndex *pindex = AddToBlockIndex(block); + CBlockIndex *pindex = m_blockman.AddToBlockIndex(block); ReceivedBlockTransactions(block, pindex, blockPos, chainparams.GetConsensus()); } catch (const std::runtime_error& e) { return error("%s: failed to write genesis block: %s", __func__, e.what()); @@ -4482,13 +4491,13 @@ void CChainState::CheckBlockIndex(const Consensus::Params& consensusParams) } // If some parent is missing, then it could be that this block was in // setBlockIndexCandidates but had to be removed because of the missing data. - // In this case it must be in mapBlocksUnlinked -- see test below. + // In this case it must be in m_blocks_unlinked -- see test below. } } else { // If this block sorts worse than the current tip or some ancestor's block has never been seen, it cannot be in setBlockIndexCandidates. assert(setBlockIndexCandidates.count(pindex) == 0); } - // Check whether this block is in mapBlocksUnlinked. - std::pair::iterator,std::multimap::iterator> rangeUnlinked = mapBlocksUnlinked.equal_range(pindex->pprev); + // Check whether this block is in m_blocks_unlinked. + std::pair::iterator,std::multimap::iterator> rangeUnlinked = m_blockman.m_blocks_unlinked.equal_range(pindex->pprev); bool foundInUnlinked = false; while (rangeUnlinked.first != rangeUnlinked.second) { assert(rangeUnlinked.first->first == pindex->pprev); @@ -4499,22 +4508,22 @@ void CChainState::CheckBlockIndex(const Consensus::Params& consensusParams) rangeUnlinked.first++; } if (pindex->pprev && (pindex->nStatus & BLOCK_HAVE_DATA) && pindexFirstNeverProcessed != nullptr && pindexFirstInvalid == nullptr) { - // If this block has block data available, some parent was never received, and has no invalid parents, it must be in mapBlocksUnlinked. + // If this block has block data available, some parent was never received, and has no invalid parents, it must be in m_blocks_unlinked. assert(foundInUnlinked); } - if (!(pindex->nStatus & BLOCK_HAVE_DATA)) assert(!foundInUnlinked); // Can't be in mapBlocksUnlinked if we don't HAVE_DATA - if (pindexFirstMissing == nullptr) assert(!foundInUnlinked); // We aren't missing data for any parent -- cannot be in mapBlocksUnlinked. + if (!(pindex->nStatus & BLOCK_HAVE_DATA)) assert(!foundInUnlinked); // Can't be in m_blocks_unlinked if we don't HAVE_DATA + if (pindexFirstMissing == nullptr) assert(!foundInUnlinked); // We aren't missing data for any parent -- cannot be in m_blocks_unlinked. if (pindex->pprev && (pindex->nStatus & BLOCK_HAVE_DATA) && pindexFirstNeverProcessed == nullptr && pindexFirstMissing != nullptr) { // We HAVE_DATA for this block, have received data for all parents at some point, but we're currently missing data for some parent. assert(fHavePruned); // We must have pruned. - // This block may have entered mapBlocksUnlinked if: + // This block may have entered m_blocks_unlinked if: // - it has a descendant that at some point had more work than the // tip, and // - we tried switching to that descendant but were missing // data for some intermediate block between m_chain and the // tip. // So if this block is itself better than m_chain.Tip() and it wasn't in - // setBlockIndexCandidates, then it must be in mapBlocksUnlinked. + // setBlockIndexCandidates, then it must be in m_blocks_unlinked. if (!CBlockIndexWorkComparator()(pindex, m_chain.Tip()) && setBlockIndexCandidates.count(pindex) == 0) { if (pindexFirstInvalid == nullptr) { assert(foundInUnlinked); diff --git a/src/validation.h b/src/validation.h index 9573d6204..09afbf7b1 100644 --- a/src/validation.h +++ b/src/validation.h @@ -439,27 +439,80 @@ struct CBlockIndexWorkComparator }; /** - * CChainState stores and provides an API to update our local knowledge of the - * current best chain and header tree. + * Maintains a tree of blocks (stored in `m_block_index`) which is consulted + * to determine where the most-work tip is. * - * It generally provides access to the current block tree, as well as functions - * to provide new data, which it will appropriately validate and incorporate in - * its state as necessary. + * This data is used mostly in `CChainState` - information about, e.g., + * candidate tips is not maintained here. + */ +class BlockManager { +public: + BlockMap m_block_index GUARDED_BY(cs_main); + + /** In order to efficiently track invalidity of headers, we keep the set of + * blocks which we tried to connect and found to be invalid here (ie which + * were set to BLOCK_FAILED_VALID since the last restart). We can then + * walk this set and check if a new header is a descendant of something in + * this set, preventing us from having to walk m_block_index when we try + * to connect a bad block and fail. + * + * While this is more complicated than marking everything which descends + * from an invalid block as invalid at the time we discover it to be + * invalid, doing so would require walking all of m_block_index to find all + * descendants. Since this case should be very rare, keeping track of all + * BLOCK_FAILED_VALID blocks in a set should be just fine and work just as + * well. + * + * Because we already walk m_block_index in height-order at startup, we go + * ahead and mark descendants of invalid blocks as FAILED_CHILD at that time, + * instead of putting things in this set. + */ + std::set m_failed_blocks; + + /** + * All pairs A->B, where A (or one of its ancestors) misses transactions, but B has transactions. + * Pruned nodes may have entries where B is missing data. + */ + std::multimap m_blocks_unlinked; + + bool LoadBlockIndex( + const Consensus::Params& consensus_params, + CBlockTreeDB& blocktree) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + + /** Clear all data members. */ + void Unload() EXCLUSIVE_LOCKS_REQUIRED(cs_main); + + CBlockIndex* AddToBlockIndex(const CBlockHeader& block) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + /** Create a new block index entry for a given block hash */ + CBlockIndex* InsertBlockIndex(const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + + /** + * If a block header hasn't already been seen, call CheckBlockHeader on it, ensure + * that it doesn't descend from an invalid block, and then add it to mapBlockIndex. + */ + bool AcceptBlockHeader( + const CBlockHeader& block, + CValidationState& state, + const CChainParams& chainparams, + CBlockIndex** ppindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main); +}; + +/** + * CChainState stores and provides an API to update our local knowledge of the + * current best chain. * * Eventually, the API here is targeted at being exposed externally as a * consumable libconsensus library, so any functions added must only call * other class member functions, pure functions in other parts of the consensus * library, callbacks via the validation interface, or read/write-to-disk * functions (eventually this will also be via callbacks). + * + * Anything that is contingent on the current tip of the chain is stored here, + * whereas block information and metadata independent of the current tip is + * kept in `BlockMetadataManager`. */ class CChainState { private: - /** - * The set of all CBlockIndex entries with BLOCK_VALID_TRANSACTIONS (for itself and all ancestors) and - * as good as our current tip or better. Entries may be failed, though, and pruning nodes may be - * missing the data for the block. - */ - std::set setBlockIndexCandidates; /** * Every received block is assigned a unique and increasing identifier, so we @@ -473,26 +526,6 @@ private: /** chainwork for the last block that preciousblock has been applied to. */ arith_uint256 nLastPreciousChainwork = 0; - /** In order to efficiently track invalidity of headers, we keep the set of - * blocks which we tried to connect and found to be invalid here (ie which - * were set to BLOCK_FAILED_VALID since the last restart). We can then - * walk this set and check if a new header is a descendant of something in - * this set, preventing us from having to walk mapBlockIndex when we try - * to connect a bad block and fail. - * - * While this is more complicated than marking everything which descends - * from an invalid block as invalid at the time we discover it to be - * invalid, doing so would require walking all of mapBlockIndex to find all - * descendants. Since this case should be very rare, keeping track of all - * BLOCK_FAILED_VALID blocks in a set should be just fine and work just as - * well. - * - * Because we already walk mapBlockIndex in height-order at startup, we go - * ahead and mark descendants of invalid blocks as FAILED_CHILD at that time, - * instead of putting things in this set. - */ - std::set m_failed_blocks; - /** * the ChainState CriticalSection * A lock that must be held when modifying this ChainState - held in ActivateBestChain() @@ -507,15 +540,24 @@ private: */ mutable std::atomic m_cached_finished_ibd{false}; + //! Reference to a BlockManager instance which itself is shared across all + //! CChainState instances. Keeping a local reference allows us to test more + //! easily as opposed to referencing a global. + BlockManager& m_blockman; + public: + CChainState(BlockManager& blockman) : m_blockman(blockman) { } + //! The current chain of blockheaders we consult and build on. //! @see CChain, CBlockIndex. CChain m_chain; - BlockMap mapBlockIndex GUARDED_BY(cs_main); - std::multimap mapBlocksUnlinked; CBlockIndex *pindexBestInvalid = nullptr; - - bool LoadBlockIndex(const Consensus::Params& consensus_params, CBlockTreeDB& blocktree) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + /** + * The set of all CBlockIndex entries with BLOCK_VALID_TRANSACTIONS (for itself and all ancestors) and + * as good as our current tip or better. Entries may be failed, though, and pruning nodes may be + * missing the data for the block. + */ + std::set setBlockIndexCandidates; /** * Update the on-disk chain state. @@ -541,11 +583,6 @@ public: bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams, std::shared_ptr pblock) LOCKS_EXCLUDED(cs_main); - /** - * If a block header hasn't already been seen, call CheckBlockHeader on it, ensure - * that it doesn't descend from an invalid block, and then add it to mapBlockIndex. - */ - bool AcceptBlockHeader(const CBlockHeader& block, CValidationState& state, const CChainParams& chainparams, CBlockIndex** ppindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main); bool AcceptBlock(const std::shared_ptr& pblock, CValidationState& state, const CChainParams& chainparams, CBlockIndex** ppindex, bool fRequested, const FlatFilePos* dbp, bool* fNewBlock) EXCLUSIVE_LOCKS_REQUIRED(cs_main); // Block (dis)connection on a given view: @@ -572,13 +609,6 @@ public: /** Check whether we are doing an initial block download (synchronizing from disk or network) */ bool IsInitialBlockDownload() const; -private: - bool ActivateBestChainStep(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexMostWork, const std::shared_ptr& pblock, bool& fInvalidFound, ConnectTrace& connectTrace) EXCLUSIVE_LOCKS_REQUIRED(cs_main, ::mempool.cs); - bool ConnectTip(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const std::shared_ptr& pblock, ConnectTrace& connectTrace, DisconnectedBlockTransactions& disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, ::mempool.cs); - - CBlockIndex* AddToBlockIndex(const CBlockHeader& block) EXCLUSIVE_LOCKS_REQUIRED(cs_main); - /** Create a new block index entry for a given block hash */ - CBlockIndex* InsertBlockIndex(const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main); /** * Make various assertions about the state of the block index. * @@ -586,6 +616,10 @@ private: */ void CheckBlockIndex(const Consensus::Params& consensusParams); +private: + bool ActivateBestChainStep(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexMostWork, const std::shared_ptr& pblock, bool& fInvalidFound, ConnectTrace& connectTrace) EXCLUSIVE_LOCKS_REQUIRED(cs_main, ::mempool.cs); + bool ConnectTip(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexNew, const std::shared_ptr& pblock, ConnectTrace& connectTrace, DisconnectedBlockTransactions& disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, ::mempool.cs); + void InvalidBlockFound(CBlockIndex *pindex, const CValidationState &state) EXCLUSIVE_LOCKS_REQUIRED(cs_main); CBlockIndex* FindMostWorkChain() EXCLUSIVE_LOCKS_REQUIRED(cs_main); void ReceivedBlockTransactions(const CBlock& block, CBlockIndex* pindexNew, const FlatFilePos& pos, const Consensus::Params& consensusParams) EXCLUSIVE_LOCKS_REQUIRED(cs_main); From 4ed55dfcd7894fd5ba6395f244a17ab1f8e786d4 Mon Sep 17 00:00:00 2001 From: James O'Beirne Date: Mon, 24 Jun 2019 15:22:33 -0400 Subject: [PATCH 2/4] refactoring: add block_index_candidates arg to LoadBlockIndex Prevents BlockManager from having to reference ChainstateActive() within one of its methods which improves encapsulation and makes testing easier. --- src/validation.cpp | 13 +++++++++---- src/validation.h | 12 +++++++++++- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index 75d5bb9fe..3e71928d1 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3705,7 +3705,10 @@ CBlockIndex * BlockManager::InsertBlockIndex(const uint256& hash) return pindexNew; } -bool BlockManager::LoadBlockIndex(const Consensus::Params& consensus_params, CBlockTreeDB& blocktree) +bool BlockManager::LoadBlockIndex( + const Consensus::Params& consensus_params, + CBlockTreeDB& blocktree, + std::set& block_index_candidates) { if (!blocktree.LoadBlockIndexGuts(consensus_params, [this](const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { return this->InsertBlockIndex(hash); })) return false; @@ -3743,8 +3746,9 @@ bool BlockManager::LoadBlockIndex(const Consensus::Params& consensus_params, CBl pindex->nStatus |= BLOCK_FAILED_CHILD; setDirtyBlockIndex.insert(pindex); } - if (pindex->IsValid(BLOCK_VALID_TRANSACTIONS) && (pindex->HaveTxsDownloaded() || pindex->pprev == nullptr)) - ::ChainstateActive().setBlockIndexCandidates.insert(pindex); + if (pindex->IsValid(BLOCK_VALID_TRANSACTIONS) && (pindex->HaveTxsDownloaded() || pindex->pprev == nullptr)) { + block_index_candidates.insert(pindex); + } if (pindex->nStatus & BLOCK_FAILED_MASK && (!pindexBestInvalid || pindex->nChainWork > pindexBestInvalid->nChainWork)) pindexBestInvalid = pindex; if (pindex->pprev) @@ -3769,7 +3773,8 @@ void BlockManager::Unload() { bool static LoadBlockIndexDB(const CChainParams& chainparams) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { - if (!g_blockman.LoadBlockIndex(chainparams.GetConsensus(), *pblocktree)) + if (!g_blockman.LoadBlockIndex( + chainparams.GetConsensus(), *pblocktree, ::ChainstateActive().setBlockIndexCandidates)) return false; // Load block file info diff --git a/src/validation.h b/src/validation.h index 09afbf7b1..ab18f662a 100644 --- a/src/validation.h +++ b/src/validation.h @@ -475,9 +475,19 @@ public: */ std::multimap m_blocks_unlinked; + /** + * Load the blocktree off disk and into memory. Populate certain metadata + * per index entry (nStatus, nChainWork, nTimeMax, etc.) as well as peripheral + * collections like setDirtyBlockIndex. + * + * @param[out] block_index_candidates Fill this set with any valid blocks for + * which we've downloaded all transactions. + */ bool LoadBlockIndex( const Consensus::Params& consensus_params, - CBlockTreeDB& blocktree) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + CBlockTreeDB& blocktree, + std::set& block_index_candidates) + EXCLUSIVE_LOCKS_REQUIRED(cs_main); /** Clear all data members. */ void Unload() EXCLUSIVE_LOCKS_REQUIRED(cs_main); From 55d525ab9004631d30dcc60a1ec5d9cd6c6afe56 Mon Sep 17 00:00:00 2001 From: James O'Beirne Date: Mon, 24 Jun 2019 13:57:01 -0400 Subject: [PATCH 3/4] refactoring: make pindexBestInvalid internal to validation.cpp There's no need to have this member live on CChainState since it's only used in validation.cpp. --- src/validation.cpp | 2 +- src/validation.h | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index 3e71928d1..0a84a0eec 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -129,7 +129,7 @@ CScript COINBASE_FLAGS; // Internal stuff namespace { - CBlockIndex *&pindexBestInvalid = ::ChainstateActive().pindexBestInvalid; + CBlockIndex* pindexBestInvalid = nullptr; CCriticalSection cs_LastBlockFile; std::vector vinfoBlockFile; diff --git a/src/validation.h b/src/validation.h index ab18f662a..4e971901b 100644 --- a/src/validation.h +++ b/src/validation.h @@ -561,7 +561,6 @@ public: //! The current chain of blockheaders we consult and build on. //! @see CChain, CBlockIndex. CChain m_chain; - CBlockIndex *pindexBestInvalid = nullptr; /** * The set of all CBlockIndex entries with BLOCK_VALID_TRANSACTIONS (for itself and all ancestors) and * as good as our current tip or better. Entries may be failed, though, and pruning nodes may be From 682a1d0f2004d808b87b3106d0dfae547005e638 Mon Sep 17 00:00:00 2001 From: James O'Beirne Date: Wed, 10 Apr 2019 14:34:46 -0400 Subject: [PATCH 4/4] refactoring: remove mapBlockIndex global in lieu of ::BlockIndex(). --- src/init.cpp | 11 ++++--- src/rpc/blockchain.cpp | 4 +-- src/txdb.cpp | 2 +- src/validation.cpp | 55 +++++++++++++++++++------------- src/validation.h | 13 +++----- src/wallet/test/wallet_tests.cpp | 2 +- 6 files changed, 48 insertions(+), 39 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 8a83ded47..3ad94ed1c 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -492,7 +492,7 @@ void SetupServerArgs() "and level 4 tries to reconnect the blocks, " "each level includes the checks of the previous levels " "(0-4, default: %u)", DEFAULT_CHECKLEVEL), true, OptionsCategory::DEBUG_TEST); - gArgs.AddArg("-checkblockindex", strprintf("Do a full consistency check for mapBlockIndex, setBlockIndexCandidates, ::ChainActive() and mapBlocksUnlinked occasionally. (default: %u, regtest: %u)", defaultChainParams->DefaultConsistencyChecks(), regtestChainParams->DefaultConsistencyChecks()), true, OptionsCategory::DEBUG_TEST); + gArgs.AddArg("-checkblockindex", strprintf("Do a full consistency check for the block tree, setBlockIndexCandidates, ::ChainActive() and mapBlocksUnlinked occasionally. (default: %u, regtest: %u)", defaultChainParams->DefaultConsistencyChecks(), regtestChainParams->DefaultConsistencyChecks()), true, OptionsCategory::DEBUG_TEST); gArgs.AddArg("-checkmempool=", strprintf("Run checks every transactions (default: %u, regtest: %u)", defaultChainParams->DefaultConsistencyChecks(), regtestChainParams->DefaultConsistencyChecks()), true, OptionsCategory::DEBUG_TEST); gArgs.AddArg("-checkpoints", strprintf("Disable expensive verification for known chain history (default: %u)", DEFAULT_CHECKPOINTS_ENABLED), true, OptionsCategory::DEBUG_TEST); gArgs.AddArg("-deprecatedrpc=", "Allows deprecated RPC method(s) to be used", true, OptionsCategory::DEBUG_TEST); @@ -1517,7 +1517,8 @@ bool AppInitMain(InitInterfaces& interfaces) // If the loaded chain has a wrong genesis, bail out immediately // (we're likely using a testnet datadir, or the other way around). - if (!mapBlockIndex.empty() && !LookupBlockIndex(chainparams.GetConsensus().hashGenesisBlock)) { + if (!::BlockIndex().empty() && + !LookupBlockIndex(chainparams.GetConsensus().hashGenesisBlock)) { return InitError(_("Incorrect or no genesis block found. Wrong datadir for network?")); } @@ -1538,7 +1539,7 @@ bool AppInitMain(InitInterfaces& interfaces) } // At this point we're either in reindex or we've loaded a useful - // block tree into mapBlockIndex! + // block tree into BlockIndex()! pcoinsdbview.reset(new CCoinsViewDB(nCoinDBCache, false, fReset || fReindexChainState)); pcoinscatcher.reset(new CCoinsViewErrorCatcher(pcoinsdbview.get())); @@ -1577,7 +1578,7 @@ bool AppInitMain(InitInterfaces& interfaces) if (!fReset) { // Note that RewindBlockIndex MUST run even if we're about to -reindex-chainstate. // It both disconnects blocks based on ::ChainActive(), and drops block data in - // mapBlockIndex based on lack of available witness data. + // BlockIndex() based on lack of available witness data. uiInterface.InitMessage(_("Rewinding blocks...")); if (!RewindBlockIndex(chainparams)) { strLoadError = _("Unable to rewind the database to a pre-fork state. You will need to redownload the blockchain"); @@ -1749,7 +1750,7 @@ bool AppInitMain(InitInterfaces& interfaces) //// debug print { LOCK(cs_main); - LogPrintf("mapBlockIndex.size() = %u\n", mapBlockIndex.size()); + LogPrintf("block tree size = %u\n", ::BlockIndex().size()); chain_active_height = ::ChainActive().Height(); } LogPrintf("nBestHeight = %d\n", chain_active_height); diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 743efc1e6..b1027dffd 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1456,7 +1456,7 @@ static UniValue getchaintips(const JSONRPCRequest& request) /* * Idea: the set of chain tips is ::ChainActive().tip, plus orphan blocks which do not have another orphan building off of them. * Algorithm: - * - Make one pass through mapBlockIndex, picking out the orphan blocks, and also storing a set of the orphan block's pprev pointers. + * - Make one pass through g_blockman.m_block_index, picking out the orphan blocks, and also storing a set of the orphan block's pprev pointers. * - Iterate through the orphan blocks. If the block isn't pointed to by another orphan, it is a chain tip. * - add ::ChainActive().Tip() */ @@ -1464,7 +1464,7 @@ static UniValue getchaintips(const JSONRPCRequest& request) std::set setOrphans; std::set setPrevs; - for (const std::pair& item : mapBlockIndex) + for (const std::pair& item : ::BlockIndex()) { if (!::ChainActive().Contains(item.second)) { setOrphans.insert(item.second); diff --git a/src/txdb.cpp b/src/txdb.cpp index 73fe2a8ee..90b92969b 100644 --- a/src/txdb.cpp +++ b/src/txdb.cpp @@ -250,7 +250,7 @@ bool CBlockTreeDB::LoadBlockIndexGuts(const Consensus::Params& consensusParams, pcursor->Seek(std::make_pair(DB_BLOCK_INDEX, uint256())); - // Load mapBlockIndex + // Load m_block_index while (pcursor->Valid()) { boost::this_thread::interruption_point(); if (ShutdownRequested()) return false; diff --git a/src/validation.cpp b/src/validation.cpp index 0a84a0eec..0bc6167ba 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -99,7 +99,6 @@ CChain& ChainActive() { return g_chainstate.m_chain; } */ RecursiveMutex cs_main; -BlockMap& mapBlockIndex = g_blockman.m_block_index; CBlockIndex *pindexBestHeader = nullptr; Mutex g_best_block_mutex; std::condition_variable g_best_block_cv; @@ -147,6 +146,13 @@ namespace { std::set setDirtyFileInfo; } // anon namespace +CBlockIndex* LookupBlockIndex(const uint256& hash) +{ + AssertLockHeld(cs_main); + BlockMap::const_iterator it = g_blockman.m_block_index.find(hash); + return it == g_blockman.m_block_index.end() ? nullptr : it->second; +} + CBlockIndex* FindForkInGlobalIndex(const CChain& chain, const CBlockLocator& locator) { AssertLockHeld(cs_main); @@ -1046,6 +1052,11 @@ bool CChainState::IsInitialBlockDownload() const static CBlockIndex *pindexBestForkTip = nullptr, *pindexBestForkBase = nullptr; +BlockMap& BlockIndex() +{ + return g_blockman.m_block_index; +} + static void AlertNotify(const std::string& strMessage) { uiInterface.NotifyAlertChanged(); @@ -3117,7 +3128,7 @@ static bool ContextualCheckBlockHeader(const CBlockHeader& block, CValidationSta if (fCheckpointsEnabled) { // Don't accept any forks from the main chain prior to last checkpoint. // GetLastCheckpoint finds the last checkpoint in MapCheckpoints that's in our - // MapBlockIndex. + // g_blockman.m_block_index. CBlockIndex* pcheckpoint = GetLastCheckpoint(params.Checkpoints()); if (pcheckpoint && nHeight < pcheckpoint->nHeight) return state.Invalid(ValidationInvalidReason::BLOCK_CHECKPOINT, error("%s: forked chain older than last checkpoint (height %d)", __func__, nHeight), REJECT_CHECKPOINT, "bad-fork-prior-to-checkpoint"); @@ -3715,8 +3726,8 @@ bool BlockManager::LoadBlockIndex( // Calculate nChainWork std::vector > vSortedByHeight; - vSortedByHeight.reserve(mapBlockIndex.size()); - for (const std::pair& item : mapBlockIndex) + vSortedByHeight.reserve(m_block_index.size()); + for (const std::pair& item : m_block_index) { CBlockIndex* pindex = item.second; vSortedByHeight.push_back(std::make_pair(pindex->nHeight, pindex)); @@ -3797,7 +3808,7 @@ bool static LoadBlockIndexDB(const CChainParams& chainparams) EXCLUSIVE_LOCKS_RE // Check presence of blk files LogPrintf("Checking all blk files are present...\n"); std::set setBlkDataFiles; - for (const std::pair& item : mapBlockIndex) + for (const std::pair& item : g_blockman.m_block_index) { CBlockIndex* pindex = item.second; if (pindex->nStatus & BLOCK_HAVE_DATA) { @@ -3996,16 +4007,16 @@ bool CChainState::ReplayBlocks(const CChainParams& params, CCoinsView* view) const CBlockIndex* pindexNew; // New tip during the interrupted flush. const CBlockIndex* pindexFork = nullptr; // Latest block common to both the old and the new tip. - if (mapBlockIndex.count(hashHeads[0]) == 0) { + if (m_blockman.m_block_index.count(hashHeads[0]) == 0) { return error("ReplayBlocks(): reorganization to unknown block requested"); } - pindexNew = mapBlockIndex[hashHeads[0]]; + pindexNew = m_blockman.m_block_index[hashHeads[0]]; if (!hashHeads[1].IsNull()) { // The old tip is allowed to be 0, indicating it's the first flush. - if (mapBlockIndex.count(hashHeads[1]) == 0) { + if (m_blockman.m_block_index.count(hashHeads[1]) == 0) { return error("ReplayBlocks(): reorganization from unknown block requested"); } - pindexOld = mapBlockIndex[hashHeads[1]]; + pindexOld = m_blockman.m_block_index[hashHeads[1]]; pindexFork = LastCommonAncestor(pindexOld, pindexNew); assert(pindexFork != nullptr); } @@ -4094,7 +4105,7 @@ bool CChainState::RewindBlockIndex(const CChainParams& params) // blocks will be dealt with below (releasing cs_main in between). { LOCK(cs_main); - for (const auto& entry : mapBlockIndex) { + for (const auto& entry : m_blockman.m_block_index) { if (IsWitnessEnabled(entry.second->pprev, params.GetConsensus()) && !(entry.second->nStatus & BLOCK_OPT_WITNESS) && !m_chain.Contains(entry.second)) { EraseBlockData(entry.second); } @@ -4234,7 +4245,7 @@ bool LoadBlockIndex(const CChainParams& chainparams) if (!fReindex) { bool ret = LoadBlockIndexDB(chainparams); if (!ret) return false; - needs_init = mapBlockIndex.empty(); + needs_init = g_blockman.m_block_index.empty(); } if (needs_init) { @@ -4254,10 +4265,10 @@ bool CChainState::LoadGenesisBlock(const CChainParams& chainparams) LOCK(cs_main); // Check whether we're already initialized by checking for genesis in - // mapBlockIndex. Note that we can't use m_chain here, since it is + // m_blockman.m_block_index. Note that we can't use m_chain here, since it is // set based on the coins db, not the block index db, which is the only // thing loaded at this point. - if (mapBlockIndex.count(chainparams.GenesisBlock().GetHash())) + if (m_blockman.m_block_index.count(chainparams.GenesisBlock().GetHash())) return true; try { @@ -4410,20 +4421,20 @@ void CChainState::CheckBlockIndex(const Consensus::Params& consensusParams) LOCK(cs_main); // During a reindex, we read the genesis block and call CheckBlockIndex before ActivateBestChain, - // so we have the genesis block in mapBlockIndex but no active chain. (A few of the tests when - // iterating the block tree require that m_chain has been initialized.) + // so we have the genesis block in m_blockman.m_block_index but no active chain. (A few of the + // tests when iterating the block tree require that m_chain has been initialized.) if (m_chain.Height() < 0) { - assert(mapBlockIndex.size() <= 1); + assert(m_blockman.m_block_index.size() <= 1); return; } // Build forward-pointing map of the entire block tree. std::multimap forward; - for (const std::pair& entry : mapBlockIndex) { + for (const std::pair& entry : m_blockman.m_block_index) { forward.insert(std::make_pair(entry.second->pprev, entry.second)); } - assert(forward.size() == mapBlockIndex.size()); + assert(forward.size() == m_blockman.m_block_index.size()); std::pair::iterator,std::multimap::iterator> rangeGenesis = forward.equal_range(nullptr); CBlockIndex *pindex = rangeGenesis.first->second; @@ -4477,7 +4488,7 @@ void CChainState::CheckBlockIndex(const Consensus::Params& consensusParams) assert(pindex->nHeight == nHeight); // nHeight must be consistent. assert(pindex->pprev == nullptr || pindex->nChainWork >= pindex->pprev->nChainWork); // For every block except the genesis block, the chainwork must be larger than the parent's. assert(nHeight < 2 || (pindex->pskip && (pindex->pskip->nHeight < nHeight))); // The pskip pointer must point back for all but the first 2 blocks. - assert(pindexFirstNotTreeValid == nullptr); // All mapBlockIndex entries must at least be TREE valid + assert(pindexFirstNotTreeValid == nullptr); // All m_blockman.m_block_index entries must at least be TREE valid if ((pindex->nStatus & BLOCK_VALID_MASK) >= BLOCK_VALID_TREE) assert(pindexFirstNotTreeValid == nullptr); // TREE valid implies all parents are TREE valid if ((pindex->nStatus & BLOCK_VALID_MASK) >= BLOCK_VALID_CHAIN) assert(pindexFirstNotChainValid == nullptr); // CHAIN valid implies all parents are CHAIN valid if ((pindex->nStatus & BLOCK_VALID_MASK) >= BLOCK_VALID_SCRIPTS) assert(pindexFirstNotScriptsValid == nullptr); // SCRIPTS valid implies all parents are SCRIPTS valid @@ -4772,10 +4783,10 @@ public: CMainCleanup() {} ~CMainCleanup() { // block headers - BlockMap::iterator it1 = mapBlockIndex.begin(); - for (; it1 != mapBlockIndex.end(); it1++) + BlockMap::iterator it1 = g_blockman.m_block_index.begin(); + for (; it1 != g_blockman.m_block_index.end(); it1++) delete (*it1).second; - mapBlockIndex.clear(); + g_blockman.m_block_index.clear(); } }; static CMainCleanup instance_of_cmaincleanup; diff --git a/src/validation.h b/src/validation.h index 4e971901b..a1b8029e0 100644 --- a/src/validation.h +++ b/src/validation.h @@ -144,7 +144,6 @@ extern CCriticalSection cs_main; extern CBlockPolicyEstimator feeEstimator; extern CTxMemPool mempool; typedef std::unordered_map BlockMap; -extern BlockMap& mapBlockIndex GUARDED_BY(cs_main); extern Mutex g_best_block_mutex; extern std::condition_variable g_best_block_cv; extern uint256 g_best_block; @@ -406,12 +405,7 @@ public: /** Replay blocks that aren't fully applied to the database. */ bool ReplayBlocks(const CChainParams& params, CCoinsView* view); -inline CBlockIndex* LookupBlockIndex(const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main) -{ - AssertLockHeld(cs_main); - BlockMap::const_iterator it = mapBlockIndex.find(hash); - return it == mapBlockIndex.end() ? nullptr : it->second; -} +CBlockIndex* LookupBlockIndex(const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main); /** Find the last common block between the parameter chain and a locator. */ CBlockIndex* FindForkInGlobalIndex(const CChain& chain, const CBlockLocator& locator) EXCLUSIVE_LOCKS_REQUIRED(cs_main); @@ -498,7 +492,7 @@ public: /** * If a block header hasn't already been seen, call CheckBlockHeader on it, ensure - * that it doesn't descend from an invalid block, and then add it to mapBlockIndex. + * that it doesn't descend from an invalid block, and then add it to m_block_index. */ bool AcceptBlockHeader( const CBlockHeader& block, @@ -658,6 +652,9 @@ CChainState& ChainstateActive(); /** @returns the most-work chain. */ CChain& ChainActive(); +/** @returns the global block index map. */ +BlockMap& BlockIndex(); + /** Global variable that points to the coins database (protected by cs_main) */ extern std::unique_ptr pcoinsdbview; diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index ef95d0544..c7a39af81 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -272,7 +272,7 @@ static int64_t AddTx(CWallet& wallet, uint32_t lockTime, int64_t mockTime, int64 if (blockTime > 0) { auto locked_chain = wallet.chain().lock(); LockAssertion lock(::cs_main); - auto inserted = mapBlockIndex.emplace(GetRandHash(), new CBlockIndex); + auto inserted = ::BlockIndex().emplace(GetRandHash(), new CBlockIndex); assert(inserted.second); const uint256& hash = inserted.first->first; block = inserted.first->second;