From bec86ae32683ac56b4e6ba9c9b7d21cfbdf4ac03 Mon Sep 17 00:00:00 2001 From: Carl Dong Date: Fri, 8 Jan 2021 18:56:48 -0500 Subject: blockstorage: Make m_block_index own CBlockIndex's Instead of having CBlockIndex's live on the heap, which requires manual memory management, have them be owned by m_block_index. This means that they will live and die with BlockManager. A change to BlockManager::LookupBlockIndex: - Previously, it was a const member function returning a non-const CBlockIndex* - Now, there's are const and non-const versions of BlockManager::LookupBlockIndex returning a CBlockIndex with the same const-ness as the member function: (e.g. const CBlockIndex* LookupBlockIndex(...) const) See next commit for some weirdness that this eliminates. The range based for-loops are modernize (using auto + destructuring) in a future commit. --- src/node/blockstorage.cpp | 46 +++++++++++++++++++++++----------------- src/node/blockstorage.h | 11 +++++++--- src/rpc/blockchain.cpp | 8 +++---- src/validation.cpp | 36 +++++++++++++++---------------- src/wallet/test/wallet_tests.cpp | 4 ++-- 5 files changed, 58 insertions(+), 47 deletions(-) (limited to 'src') diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index 8a99130fd0..a13c1a9956 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -32,11 +32,18 @@ static FILE* OpenUndoFile(const FlatFilePos& pos, bool fReadOnly = false); static FlatFileSeq BlockFileSeq(); static FlatFileSeq UndoFileSeq(); -CBlockIndex* BlockManager::LookupBlockIndex(const uint256& hash) const +CBlockIndex* BlockManager::LookupBlockIndex(const uint256& hash) +{ + AssertLockHeld(cs_main); + BlockMap::iterator it = m_block_index.find(hash); + return it == m_block_index.end() ? nullptr : &it->second; +} + +const CBlockIndex* BlockManager::LookupBlockIndex(const uint256& hash) const { AssertLockHeld(cs_main); BlockMap::const_iterator it = m_block_index.find(hash); - return it == m_block_index.end() ? nullptr : it->second; + return it == m_block_index.end() ? nullptr : &it->second; } CBlockIndex* BlockManager::AddToBlockIndex(const CBlockHeader& block) @@ -47,20 +54,22 @@ CBlockIndex* BlockManager::AddToBlockIndex(const CBlockHeader& block) uint256 hash = block.GetHash(); BlockMap::iterator it = m_block_index.find(hash); if (it != m_block_index.end()) { - return it->second; + return &it->second; } // Construct new block index object - CBlockIndex* pindexNew = new CBlockIndex(block); + CBlockIndex new_index{block}; // We assign the sequence id to blocks only when the full data is available, // to avoid miners withholding blocks but broadcasting headers, to get a // competitive advantage. - pindexNew->nSequenceId = 0; - BlockMap::iterator mi = m_block_index.insert(std::make_pair(hash, pindexNew)).first; + new_index.nSequenceId = 0; + BlockMap::iterator mi = m_block_index.insert(std::make_pair(hash, std::move(new_index))).first; + + CBlockIndex* pindexNew = &(*mi).second; pindexNew->phashBlock = &((*mi).first); BlockMap::iterator miPrev = m_block_index.find(block.hashPrevBlock); if (miPrev != m_block_index.end()) { - pindexNew->pprev = (*miPrev).second; + pindexNew->pprev = &(*miPrev).second; pindexNew->nHeight = pindexNew->pprev->nHeight + 1; pindexNew->BuildSkip(); } @@ -80,8 +89,8 @@ void BlockManager::PruneOneBlockFile(const int fileNumber) AssertLockHeld(cs_main); LOCK(cs_LastBlockFile); - for (const auto& entry : m_block_index) { - CBlockIndex* pindex = entry.second; + for (auto& entry : m_block_index) { + CBlockIndex* pindex = &entry.second; if (pindex->nFile == fileNumber) { pindex->nStatus &= ~BLOCK_HAVE_DATA; pindex->nStatus &= ~BLOCK_HAVE_UNDO; @@ -202,12 +211,13 @@ CBlockIndex* BlockManager::InsertBlockIndex(const uint256& hash) // Return existing BlockMap::iterator mi = m_block_index.find(hash); if (mi != m_block_index.end()) { - return (*mi).second; + return &(*mi).second; } // Create new - CBlockIndex* pindexNew = new CBlockIndex(); - mi = m_block_index.insert(std::make_pair(hash, pindexNew)).first; + CBlockIndex new_index{}; + mi = m_block_index.insert(std::make_pair(hash, std::move(new_index))).first; + CBlockIndex* pindexNew = &(*mi).second; pindexNew->phashBlock = &((*mi).first); return pindexNew; @@ -224,8 +234,8 @@ bool BlockManager::LoadBlockIndex( // Calculate nChainWork std::vector> vSortedByHeight; vSortedByHeight.reserve(m_block_index.size()); - for (const std::pair& item : m_block_index) { - CBlockIndex* pindex = item.second; + for (std::pair& item : m_block_index) { + CBlockIndex* pindex = &item.second; vSortedByHeight.push_back(std::make_pair(pindex->nHeight, pindex)); } sort(vSortedByHeight.begin(), vSortedByHeight.end()); @@ -327,10 +337,6 @@ void BlockManager::Unload() { m_blocks_unlinked.clear(); - for (const BlockMap::value_type& entry : m_block_index) { - delete entry.second; - } - m_block_index.clear(); m_blockfile_info.clear(); @@ -386,8 +392,8 @@ bool BlockManager::LoadBlockIndexDB(ChainstateManager& chainman) // Check presence of blk files LogPrintf("Checking all blk files are present...\n"); std::set setBlkDataFiles; - for (const std::pair& item : m_block_index) { - CBlockIndex* pindex = item.second; + for (const std::pair& item : m_block_index) { + const CBlockIndex* pindex = &item.second; if (pindex->nStatus & BLOCK_HAVE_DATA) { setBlkDataFiles.insert(pindex->nFile); } diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h index 42e46797d2..be1ed83064 100644 --- a/src/node/blockstorage.h +++ b/src/node/blockstorage.h @@ -5,6 +5,7 @@ #ifndef BITCOIN_NODE_BLOCKSTORAGE_H #define BITCOIN_NODE_BLOCKSTORAGE_H +#include #include #include // For CMessageHeader::MessageStartChars #include @@ -20,7 +21,6 @@ class ArgsManager; class BlockValidationState; class CBlock; class CBlockFileInfo; -class CBlockIndex; class CBlockUndo; class CChain; class CChainParams; @@ -52,7 +52,11 @@ extern bool fPruneMode; /** Number of MiB of block files that we're trying to stay below. */ extern uint64_t nPruneTarget; -typedef std::unordered_map BlockMap; +// Because validation code takes pointers to the map's CBlockIndex objects, if +// we ever switch to another associative container, we need to either use a +// container that has stable addressing (true of all std associative +// containers), or make the key a `std::unique_ptr` +typedef std::unordered_map BlockMap; struct CBlockIndexWorkComparator { bool operator()(const CBlockIndex* pa, const CBlockIndex* pb) const; @@ -144,7 +148,8 @@ public: //! Mark one block file as pruned (modify associated database entries) void PruneOneBlockFile(const int fileNumber) EXCLUSIVE_LOCKS_REQUIRED(cs_main); - CBlockIndex* LookupBlockIndex(const uint256& hash) const EXCLUSIVE_LOCKS_REQUIRED(cs_main); + CBlockIndex* LookupBlockIndex(const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + const CBlockIndex* LookupBlockIndex(const uint256& hash) const EXCLUSIVE_LOCKS_REQUIRED(cs_main); /** Get block file info entry for one block file */ CBlockFileInfo* GetBlockFileInfo(size_t n); diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 9817c80cbd..2c69fdd457 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1753,10 +1753,10 @@ static RPCHelpMan getchaintips() std::set setOrphans; std::set setPrevs; - for (const std::pair& item : chainman.BlockIndex()) { - if (!active_chain.Contains(item.second)) { - setOrphans.insert(item.second); - setPrevs.insert(item.second->pprev); + for (const std::pair& item : chainman.BlockIndex()) { + if (!active_chain.Contains(&item.second)) { + setOrphans.insert(&item.second); + setPrevs.insert(item.second.pprev); } } diff --git a/src/validation.cpp b/src/validation.cpp index 035b5783c3..fdb1518b3c 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1978,7 +1978,7 @@ bool CChainState::ConnectBlock(const CBlock& block, BlockValidationState& state, // effectively caching the result of part of the verification. 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 && + if (it->second.GetAncestor(pindex->nHeight) == pindex && pindexBestHeader->GetAncestor(pindex->nHeight) == pindex && pindexBestHeader->nChainWork >= nMinimumChainWork) { // This block is a member of the assumed verified chain and an ancestor of the best header. @@ -3035,8 +3035,8 @@ bool CChainState::InvalidateBlock(BlockValidationState& state, CBlockIndex* pind { LOCK(cs_main); - for (const auto& entry : m_blockman.m_block_index) { - CBlockIndex *candidate = entry.second; + for (auto& entry : m_blockman.m_block_index) { + CBlockIndex* candidate = &entry.second; // We don't need to put anything in our active chain into the // multimap, because those candidates will be found and considered // as we disconnect. @@ -3135,8 +3135,8 @@ bool CChainState::InvalidateBlock(BlockValidationState& state, CBlockIndex* pind // to setBlockIndexCandidates. 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); + if (it->second.IsValid(BLOCK_VALID_TRANSACTIONS) && it->second.HaveTxsDownloaded() && !setBlockIndexCandidates.value_comp()(&it->second, m_chain.Tip())) { + setBlockIndexCandidates.insert(&it->second); } it++; } @@ -3159,17 +3159,17 @@ void CChainState::ResetBlockFailureFlags(CBlockIndex *pindex) { // Remove the invalidity flag from this block and all its descendants. 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; - m_blockman.m_dirty_blockindex.insert(it->second); - if (it->second->IsValid(BLOCK_VALID_TRANSACTIONS) && it->second->HaveTxsDownloaded() && setBlockIndexCandidates.value_comp()(m_chain.Tip(), it->second)) { - setBlockIndexCandidates.insert(it->second); + if (!it->second.IsValid() && it->second.GetAncestor(nHeight) == pindex) { + it->second.nStatus &= ~BLOCK_FAILED_MASK; + m_blockman.m_dirty_blockindex.insert(&it->second); + if (it->second.IsValid(BLOCK_VALID_TRANSACTIONS) && it->second.HaveTxsDownloaded() && setBlockIndexCandidates.value_comp()(m_chain.Tip(), &it->second)) { + setBlockIndexCandidates.insert(&it->second); } - if (it->second == m_chainman.m_best_invalid) { + if (&it->second == m_chainman.m_best_invalid) { // Reset invalid block marker if it was pointing to one of those. m_chainman.m_best_invalid = nullptr; } - m_chainman.m_failed_blocks.erase(it->second); + m_chainman.m_failed_blocks.erase(&it->second); } it++; } @@ -3500,7 +3500,7 @@ bool ChainstateManager::AcceptBlockHeader(const CBlockHeader& block, BlockValida if (hash != chainparams.GetConsensus().hashGenesisBlock) { if (miSelf != m_blockman.m_block_index.end()) { // Block header is already known. - CBlockIndex* pindex = miSelf->second; + CBlockIndex* pindex = &(miSelf->second); if (ppindex) *ppindex = pindex; if (pindex->nStatus & BLOCK_FAILED_MASK) { @@ -3522,7 +3522,7 @@ bool ChainstateManager::AcceptBlockHeader(const CBlockHeader& block, BlockValida LogPrint(BCLog::VALIDATION, "%s: %s prev block not found\n", __func__, hash.ToString()); return state.Invalid(BlockValidationResult::BLOCK_MISSING_PREV, "prev-blk-not-found"); } - pindexPrev = (*mi).second; + pindexPrev = &((*mi).second); if (pindexPrev->nStatus & BLOCK_FAILED_MASK) { LogPrint(BCLog::VALIDATION, "%s: %s prev block invalid\n", __func__, hash.ToString()); return state.Invalid(BlockValidationResult::BLOCK_INVALID_PREV, "bad-prevblk"); @@ -3988,13 +3988,13 @@ bool CChainState::ReplayBlocks() if (m_blockman.m_block_index.count(hashHeads[0]) == 0) { return error("ReplayBlocks(): reorganization to unknown block requested"); } - pindexNew = m_blockman.m_block_index[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 (m_blockman.m_block_index.count(hashHeads[1]) == 0) { return error("ReplayBlocks(): reorganization from unknown block requested"); } - pindexOld = m_blockman.m_block_index[hashHeads[1]]; + pindexOld = &(m_blockman.m_block_index[hashHeads[1]]); pindexFork = LastCommonAncestor(pindexOld, pindexNew); assert(pindexFork != nullptr); } @@ -4261,8 +4261,8 @@ void CChainState::CheckBlockIndex() // Build forward-pointing map of the entire block tree. std::multimap forward; - for (const std::pair& entry : m_blockman.m_block_index) { - forward.insert(std::make_pair(entry.second->pprev, entry.second)); + for (std::pair& entry : m_blockman.m_block_index) { + forward.insert(std::make_pair(entry.second.pprev, &entry.second)); } assert(forward.size() == m_blockman.m_block_index.size()); diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 7693c9c0e8..c59f7e6f05 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -367,10 +367,10 @@ static int64_t AddTx(ChainstateManager& chainman, CWallet& wallet, uint32_t lock CBlockIndex* block = nullptr; if (blockTime > 0) { LOCK(cs_main); - auto inserted = chainman.BlockIndex().emplace(GetRandHash(), new CBlockIndex); + auto inserted = chainman.BlockIndex().emplace(std::piecewise_construct, std::make_tuple(GetRandHash()), std::make_tuple()); assert(inserted.second); const uint256& hash = inserted.first->first; - block = inserted.first->second; + block = &inserted.first->second; block->nTime = blockTime; block->phashBlock = &hash; state = TxStateConfirmed{hash, block->nHeight, /*position_in_block=*/0}; -- cgit v1.2.3 From 531dce034718523967808a89c18ba69a1e3e5a1f Mon Sep 17 00:00:00 2001 From: Carl Dong Date: Mon, 12 Jul 2021 15:09:26 -0400 Subject: tests: Remove now-unnecessary manual Unload's These manual calls to Unload() are no longer necessary because CBlockIndex's no longer live in the heap as of the previous commit. --- src/test/validation_chainstate_tests.cpp | 3 --- src/test/validation_chainstatemanager_tests.cpp | 2 -- 2 files changed, 5 deletions(-) (limited to 'src') diff --git a/src/test/validation_chainstate_tests.cpp b/src/test/validation_chainstate_tests.cpp index 1beef5cf04..b0d7389d39 100644 --- a/src/test/validation_chainstate_tests.cpp +++ b/src/test/validation_chainstate_tests.cpp @@ -72,9 +72,6 @@ BOOST_AUTO_TEST_CASE(validation_chainstate_resize_caches) // The view cache should be empty since we had to destruct to downsize. BOOST_CHECK(!c1.CoinsTip().HaveCoinInCache(outpoint)); } - - // Avoid triggering the address sanitizer. - WITH_LOCK(::cs_main, manager.Unload()); } //! Test UpdateTip behavior for both active and background chainstates. diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index 26392e690d..5d0ec593e3 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -99,8 +99,6 @@ BOOST_AUTO_TEST_CASE(chainstatemanager) // Let scheduler events finish running to avoid accessing memory that is going to be unloaded SyncWithValidationInterfaceQueue(); - - WITH_LOCK(::cs_main, manager.Unload()); } //! Test rebalancing the caches associated with each chainstate. -- cgit v1.2.3 From dd79dad17545424d145e846026518d70da594380 Mon Sep 17 00:00:00 2001 From: Carl Dong Date: Thu, 13 Jan 2022 12:37:06 -0500 Subject: refactor: Rewrite InsertBlockIndex with try_emplace Credit to ajtowns for this suggestion, thanks! --- src/node/blockstorage.cpp | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) (limited to 'src') diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index a13c1a9956..0619af5426 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -208,19 +208,13 @@ CBlockIndex* BlockManager::InsertBlockIndex(const uint256& hash) return nullptr; } - // Return existing - BlockMap::iterator mi = m_block_index.find(hash); - if (mi != m_block_index.end()) { - return &(*mi).second; + // Return existing or create new + auto [mi, inserted] = m_block_index.try_emplace(hash); + CBlockIndex* pindex = &(*mi).second; + if (inserted) { + pindex->phashBlock = &((*mi).first); } - - // Create new - CBlockIndex new_index{}; - mi = m_block_index.insert(std::make_pair(hash, std::move(new_index))).first; - CBlockIndex* pindexNew = &(*mi).second; - pindexNew->phashBlock = &((*mi).first); - - return pindexNew; + return pindex; } bool BlockManager::LoadBlockIndex( -- cgit v1.2.3 From c2a1655799c5d5dab9b14bd2a6b2d2296efd6964 Mon Sep 17 00:00:00 2001 From: Carl Dong Date: Thu, 13 Jan 2022 12:38:23 -0500 Subject: style-only: Use using instead of typedef for BlockMap --- src/node/blockstorage.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h index be1ed83064..12224f7a5d 100644 --- a/src/node/blockstorage.h +++ b/src/node/blockstorage.h @@ -56,7 +56,7 @@ extern uint64_t nPruneTarget; // we ever switch to another associative container, we need to either use a // container that has stable addressing (true of all std associative // containers), or make the key a `std::unique_ptr` -typedef std::unordered_map BlockMap; +using BlockMap = std::unordered_map; struct CBlockIndexWorkComparator { bool operator()(const CBlockIndex* pa, const CBlockIndex* pb) const; -- cgit v1.2.3 From c05cf7aa1e1c15089753897a10c14762027d4b99 Mon Sep 17 00:00:00 2001 From: Carl Dong Date: Thu, 13 Jan 2022 12:44:19 -0500 Subject: style: Modernize range-based loops over m_block_index --- src/node/blockstorage.cpp | 8 ++++---- src/rpc/blockchain.cpp | 8 ++++---- src/validation.cpp | 30 +++++++++++++----------------- 3 files changed, 21 insertions(+), 25 deletions(-) (limited to 'src') diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index 0619af5426..476f0d8148 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -228,8 +228,8 @@ bool BlockManager::LoadBlockIndex( // Calculate nChainWork std::vector> vSortedByHeight; vSortedByHeight.reserve(m_block_index.size()); - for (std::pair& item : m_block_index) { - CBlockIndex* pindex = &item.second; + for (auto& [_, block_index] : m_block_index) { + CBlockIndex* pindex = &block_index; vSortedByHeight.push_back(std::make_pair(pindex->nHeight, pindex)); } sort(vSortedByHeight.begin(), vSortedByHeight.end()); @@ -386,8 +386,8 @@ bool BlockManager::LoadBlockIndexDB(ChainstateManager& chainman) // Check presence of blk files LogPrintf("Checking all blk files are present...\n"); std::set setBlkDataFiles; - for (const std::pair& item : m_block_index) { - const CBlockIndex* pindex = &item.second; + for (const auto& [_, block_index] : m_block_index) { + const CBlockIndex* pindex = &block_index; if (pindex->nStatus & BLOCK_HAVE_DATA) { setBlkDataFiles.insert(pindex->nFile); } diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 2c69fdd457..86dfbbae35 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1753,10 +1753,10 @@ static RPCHelpMan getchaintips() std::set setOrphans; std::set setPrevs; - for (const std::pair& item : chainman.BlockIndex()) { - if (!active_chain.Contains(&item.second)) { - setOrphans.insert(&item.second); - setPrevs.insert(item.second.pprev); + for (const auto& [_, block_index] : chainman.BlockIndex()) { + if (!active_chain.Contains(&block_index)) { + setOrphans.insert(&block_index); + setPrevs.insert(block_index.pprev); } } diff --git a/src/validation.cpp b/src/validation.cpp index fdb1518b3c..b659bb6538 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3133,12 +3133,10 @@ bool CChainState::InvalidateBlock(BlockValidationState& state, CBlockIndex* pind // it up here, this should be an essentially unobservable error. // Loop back over all block index entries and add any missing entries // to setBlockIndexCandidates. - 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); + for (auto& [_, block_index] : m_blockman.m_block_index) { + if (block_index.IsValid(BLOCK_VALID_TRANSACTIONS) && block_index.HaveTxsDownloaded() && !setBlockIndexCandidates.value_comp()(&block_index, m_chain.Tip())) { + setBlockIndexCandidates.insert(&block_index); } - it++; } InvalidChainFound(to_mark_failed); @@ -3157,21 +3155,19 @@ void CChainState::ResetBlockFailureFlags(CBlockIndex *pindex) { int nHeight = pindex->nHeight; // Remove the invalidity flag from this block and all its descendants. - 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; - m_blockman.m_dirty_blockindex.insert(&it->second); - if (it->second.IsValid(BLOCK_VALID_TRANSACTIONS) && it->second.HaveTxsDownloaded() && setBlockIndexCandidates.value_comp()(m_chain.Tip(), &it->second)) { - setBlockIndexCandidates.insert(&it->second); + for (auto& [_, block_index] : m_blockman.m_block_index) { + if (!block_index.IsValid() && block_index.GetAncestor(nHeight) == pindex) { + block_index.nStatus &= ~BLOCK_FAILED_MASK; + m_blockman.m_dirty_blockindex.insert(&block_index); + if (block_index.IsValid(BLOCK_VALID_TRANSACTIONS) && block_index.HaveTxsDownloaded() && setBlockIndexCandidates.value_comp()(m_chain.Tip(), &block_index)) { + setBlockIndexCandidates.insert(&block_index); } - if (&it->second == m_chainman.m_best_invalid) { + if (&block_index == m_chainman.m_best_invalid) { // Reset invalid block marker if it was pointing to one of those. m_chainman.m_best_invalid = nullptr; } - m_chainman.m_failed_blocks.erase(&it->second); + m_chainman.m_failed_blocks.erase(&block_index); } - it++; } // Remove the invalidity flag from all ancestors too. @@ -4261,8 +4257,8 @@ void CChainState::CheckBlockIndex() // Build forward-pointing map of the entire block tree. std::multimap forward; - for (std::pair& entry : m_blockman.m_block_index) { - forward.insert(std::make_pair(entry.second.pprev, &entry.second)); + for (auto& [_, block_index] : m_blockman.m_block_index) { + forward.emplace(block_index.pprev, &block_index); } assert(forward.size() == m_blockman.m_block_index.size()); -- cgit v1.2.3 From 6c23c415613d8b847e6f6a2f872be893da9f4384 Mon Sep 17 00:00:00 2001 From: Carl Dong Date: Wed, 19 Jan 2022 13:55:40 -0500 Subject: refactor: Rewrite AddToBlockIndex with try_emplace --- src/node/blockstorage.cpp | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) (limited to 'src') diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index 476f0d8148..7392830261 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -50,22 +50,17 @@ CBlockIndex* BlockManager::AddToBlockIndex(const CBlockHeader& block) { AssertLockHeld(cs_main); - // Check for duplicate - uint256 hash = block.GetHash(); - BlockMap::iterator it = m_block_index.find(hash); - if (it != m_block_index.end()) { - return &it->second; + auto [mi, inserted] = m_block_index.try_emplace(block.GetHash(), block); + if (!inserted) { + return &mi->second; } + CBlockIndex* pindexNew = &(*mi).second; - // Construct new block index object - CBlockIndex new_index{block}; // We assign the sequence id to blocks only when the full data is available, // to avoid miners withholding blocks but broadcasting headers, to get a // competitive advantage. - new_index.nSequenceId = 0; - BlockMap::iterator mi = m_block_index.insert(std::make_pair(hash, std::move(new_index))).first; + pindexNew->nSequenceId = 0; - CBlockIndex* pindexNew = &(*mi).second; pindexNew->phashBlock = &((*mi).first); BlockMap::iterator miPrev = m_block_index.find(block.hashPrevBlock); if (miPrev != m_block_index.end()) { -- cgit v1.2.3