From 53e9c29b1e613eff91447cc80090312808404819 Mon Sep 17 00:00:00 2001 From: pasta Date: Thu, 5 Dec 2024 14:16:02 -0600 Subject: [PATCH] wip: drop cs_main guarding from m_block_index; introduce m_block_index_mutex --- src/llmq/blockprocessor.cpp | 1 - src/llmq/chainlocks.cpp | 3 +- src/llmq/commitment.cpp | 2 +- src/llmq/debug.cpp | 2 +- src/llmq/dkgsessionhandler.cpp | 2 +- src/llmq/dkgsessionmgr.cpp | 3 +- src/llmq/instantsend.cpp | 9 +-- src/llmq/quorums.cpp | 4 +- src/node/blockstorage.cpp | 44 +++++++---- src/node/blockstorage.h | 25 +++--- src/rpc/quorums.cpp | 15 ++-- src/validation.cpp | 137 +++++++++++++++++++-------------- 12 files changed, 138 insertions(+), 109 deletions(-) diff --git a/src/llmq/blockprocessor.cpp b/src/llmq/blockprocessor.cpp index b11ab32f81..d6b93e90e8 100644 --- a/src/llmq/blockprocessor.cpp +++ b/src/llmq/blockprocessor.cpp @@ -82,7 +82,6 @@ MessageProcessingResult CQuorumBlockProcessor::ProcessMessage(const CNode& peer, // Verify that quorumHash is part of the active chain and that it's the first block in the DKG interval const CBlockIndex* pQuorumBaseBlockIndex; { - LOCK(cs_main); pQuorumBaseBlockIndex = m_chainstate.m_blockman.LookupBlockIndex(qc.quorumHash); if (pQuorumBaseBlockIndex == nullptr) { LogPrint(BCLog::LLMQ, "CQuorumBlockProcessor::%s -- unknown block %s in commitment, peer=%d\n", __func__, diff --git a/src/llmq/chainlocks.cpp b/src/llmq/chainlocks.cpp index 1ef7d22b91..ba318a15bb 100644 --- a/src/llmq/chainlocks.cpp +++ b/src/llmq/chainlocks.cpp @@ -121,7 +121,7 @@ MessageProcessingResult CChainLocksHandler::ProcessNewChainLock(const NodeId fro return {}; } - const CBlockIndex* pindex = WITH_LOCK(cs_main, return m_chainstate.m_blockman.LookupBlockIndex(clsig.getBlockHash())); + const CBlockIndex* pindex = m_chainstate.m_blockman.LookupBlockIndex(clsig.getBlockHash()); { LOCK(cs); @@ -404,7 +404,6 @@ CChainLocksHandler::BlockTxs::mapped_type CChainLocksHandler::GetBlockTxs(const uint32_t blockTime; { - LOCK(cs_main); const auto* pindex = m_chainstate.m_blockman.LookupBlockIndex(blockHash); CBlock block; if (!ReadBlockFromDisk(block, pindex, Params().GetConsensus())) { diff --git a/src/llmq/commitment.cpp b/src/llmq/commitment.cpp index 05607c34a6..5e10f7e7e3 100644 --- a/src/llmq/commitment.cpp +++ b/src/llmq/commitment.cpp @@ -200,7 +200,7 @@ bool CheckLLMQCommitment(CDeterministicMNManager& dmnman, const ChainstateManage return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-qc-height"); } - const CBlockIndex* pQuorumBaseBlockIndex = WITH_LOCK(cs_main, return chainman.m_blockman.LookupBlockIndex(qcTx.commitment.quorumHash)); + const CBlockIndex* pQuorumBaseBlockIndex = chainman.m_blockman.LookupBlockIndex(qcTx.commitment.quorumHash); if (pQuorumBaseBlockIndex == nullptr) { return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-qc-quorum-hash"); } diff --git a/src/llmq/debug.cpp b/src/llmq/debug.cpp index c042fd0c98..26aecd6e38 100644 --- a/src/llmq/debug.cpp +++ b/src/llmq/debug.cpp @@ -26,7 +26,7 @@ UniValue CDKGDebugSessionStatus::ToJson(CDeterministicMNManager& dmnman, const C std::vector dmnMembers; if (detailLevel == 2) { - const CBlockIndex* pindex = WITH_LOCK(cs_main, return chainman.m_blockman.LookupBlockIndex(quorumHash)); + const CBlockIndex* pindex = chainman.m_blockman.LookupBlockIndex(quorumHash); if (pindex != nullptr) { dmnMembers = utils::GetAllQuorumMembers(llmqType, dmnman, pindex); } diff --git a/src/llmq/dkgsessionhandler.cpp b/src/llmq/dkgsessionhandler.cpp index 09e24bf429..e4dedb097e 100644 --- a/src/llmq/dkgsessionhandler.cpp +++ b/src/llmq/dkgsessionhandler.cpp @@ -547,7 +547,7 @@ void CDKGSessionHandler::HandleDKGRound() pendingPrematureCommitments.Clear(); uint256 curQuorumHash = WITH_LOCK(cs_phase_qhash, return quorumHash); - const CBlockIndex* pQuorumBaseBlockIndex = WITH_LOCK(cs_main, return m_chainstate.m_blockman.LookupBlockIndex(curQuorumHash)); + const CBlockIndex* pQuorumBaseBlockIndex = m_chainstate.m_blockman.LookupBlockIndex(curQuorumHash); if (!InitNewQuorum(pQuorumBaseBlockIndex)) { // should actually never happen diff --git a/src/llmq/dkgsessionmgr.cpp b/src/llmq/dkgsessionmgr.cpp index b9e33f30f9..7278520aec 100644 --- a/src/llmq/dkgsessionmgr.cpp +++ b/src/llmq/dkgsessionmgr.cpp @@ -151,7 +151,7 @@ PeerMsgRet CDKGSessionManager::ProcessMessage(CNode& pfrom, PeerManager* peerman // No luck, try to compute if (quorumIndex == -1) { - CBlockIndex* pQuorumBaseBlockIndex = WITH_LOCK(cs_main, return m_chainstate.m_blockman.LookupBlockIndex(quorumHash)); + CBlockIndex* pQuorumBaseBlockIndex = m_chainstate.m_blockman.LookupBlockIndex(quorumHash); if (pQuorumBaseBlockIndex == nullptr) { LogPrintf("CDKGSessionManager -- unknown quorumHash %s\n", quorumHash.ToString()); // NOTE: do not insta-ban for this, we might be lagging behind @@ -404,7 +404,6 @@ void CDKGSessionManager::CleanupOldContributions() const decltype(start) k; pcursor->Seek(start); - LOCK(cs_main); while (pcursor->Valid()) { if (!pcursor->GetKey(k) || std::get<0>(k) != prefix || std::get<1>(k) != params.type) { break; diff --git a/src/llmq/instantsend.cpp b/src/llmq/instantsend.cpp index 598986b55b..8f2d45bf0e 100644 --- a/src/llmq/instantsend.cpp +++ b/src/llmq/instantsend.cpp @@ -589,7 +589,6 @@ bool CInstantSendManager::CheckCanLock(const COutPoint& outpoint, bool printDebu const CBlockIndex* pindexMined; int nTxAge; { - LOCK(cs_main); pindexMined = m_chainstate.m_blockman.LookupBlockIndex(hashBlock); nTxAge = m_chainstate.m_chain.Height() - pindexMined->nHeight + 1; } @@ -752,7 +751,7 @@ PeerMsgRet CInstantSendManager::ProcessMessageInstantSendLock(const CNode& pfrom return tl::unexpected{100}; } - const auto blockIndex = WITH_LOCK(cs_main, return m_chainstate.m_blockman.LookupBlockIndex(islock->cycleHash)); + const auto blockIndex = m_chainstate.m_blockman.LookupBlockIndex(islock->cycleHash); if (blockIndex == nullptr) { // Maybe we don't have the block yet or maybe some peer spams invalid values for cycleHash return tl::unexpected{1}; @@ -893,7 +892,7 @@ std::unordered_set CInstantSendManager::ProcessPend continue; } - const auto blockIndex = WITH_LOCK(cs_main, return m_chainstate.m_blockman.LookupBlockIndex(islock->cycleHash)); + const auto blockIndex = m_chainstate.m_blockman.LookupBlockIndex(islock->cycleHash); if (blockIndex == nullptr) { batchVerifier.badSources.emplace(nodeId); continue; @@ -987,7 +986,7 @@ void CInstantSendManager::ProcessInstantSendLock(NodeId from, const uint256& has const CBlockIndex* pindexMined{nullptr}; // we ignore failure here as we must be able to propagate the lock even if we don't have the TX locally if (tx && !hashBlock.IsNull()) { - pindexMined = WITH_LOCK(cs_main, return m_chainstate.m_blockman.LookupBlockIndex(hashBlock)); + pindexMined = m_chainstate.m_blockman.LookupBlockIndex(hashBlock); // Let's see if the TX that was locked by this islock is already mined in a ChainLocked block. If yes, // we can simply ignore the islock, as the ChainLock implies locking of all TXs in that chain @@ -1388,7 +1387,7 @@ void CInstantSendManager::ResolveBlockConflicts(const uint256& islockHash, const BlockValidationState state; // need non-const pointer - auto pindex2 = WITH_LOCK(::cs_main, return m_chainstate.m_blockman.LookupBlockIndex(pindex->GetBlockHash())); + auto pindex2 = m_chainstate.m_blockman.LookupBlockIndex(pindex->GetBlockHash()); if (!m_chainstate.InvalidateBlock(state, pindex2)) { LogPrintf("CInstantSendManager::%s -- InvalidateBlock failed: %s\n", __func__, state.ToString()); // This should not have happened and we are in a state were it's not safe to continue anymore diff --git a/src/llmq/quorums.cpp b/src/llmq/quorums.cpp index 1b9902a5a8..ad314d7430 100644 --- a/src/llmq/quorums.cpp +++ b/src/llmq/quorums.cpp @@ -639,7 +639,7 @@ CQuorumCPtr CQuorumManager::GetQuorum(Consensus::LLMQType llmqType, const uint25 // We cannot aquire cs_main if we have cs_quorumBaseBlockIndexCache held const CBlockIndex* pindex; if (!WITH_LOCK(cs_quorumBaseBlockIndexCache, return quorumBaseBlockIndexCache.get(quorumHash, pindex))) { - pindex = WITH_LOCK(cs_main, return m_chainstate.m_blockman.LookupBlockIndex(quorumHash)); + pindex = m_chainstate.m_blockman.LookupBlockIndex(quorumHash); if (pindex) { LOCK(cs_quorumBaseBlockIndexCache); quorumBaseBlockIndexCache.insert(quorumHash, pindex); @@ -759,7 +759,7 @@ PeerMsgRet CQuorumManager::ProcessMessage(CNode& pfrom, const std::string& msg_t return sendQDATA(CQuorumDataRequest::Errors::QUORUM_TYPE_INVALID, request_limit_exceeded); } - const CBlockIndex* pQuorumBaseBlockIndex = WITH_LOCK(cs_main, return m_chainstate.m_blockman.LookupBlockIndex(request.GetQuorumHash())); + const CBlockIndex* pQuorumBaseBlockIndex = m_chainstate.m_blockman.LookupBlockIndex(request.GetQuorumHash()); if (pQuorumBaseBlockIndex == nullptr) { return sendQDATA(CQuorumDataRequest::Errors::QUORUM_BLOCK_NOT_FOUND, request_limit_exceeded); } diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index ed1e8005e8..efe858dd35 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -66,6 +66,7 @@ static FlatFileSeq UndoFileSeq(); std::vector BlockManager::GetAllBlockIndices() { AssertLockHeld(cs_main); + LOCK(m_block_index_mutex); std::vector rv; rv.reserve(m_block_index.size()); for (auto& [_, block_index] : m_block_index) { @@ -76,14 +77,14 @@ std::vector BlockManager::GetAllBlockIndices() CBlockIndex* BlockManager::LookupBlockIndex(const uint256& hash) { - AssertLockHeld(cs_main); + LOCK(m_block_index_mutex); 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); + LOCK(m_block_index_mutex); BlockMap::const_iterator it = m_block_index.find(hash); return it == m_block_index.end() ? nullptr : &it->second; } @@ -94,7 +95,7 @@ CBlockIndex* BlockManager::AddToBlockIndex(const CBlockHeader& block, const uint assert(!(nStatus & BLOCK_FAILED_MASK)); // no failed blocks allowed AssertLockHeld(cs_main); - auto [mi, inserted] = m_block_index.try_emplace(hash, block); + auto [mi, inserted] = WITH_LOCK(m_block_index_mutex, return m_block_index.try_emplace(hash, block)); if (!inserted) { return &mi->second; } @@ -106,11 +107,14 @@ CBlockIndex* BlockManager::AddToBlockIndex(const CBlockHeader& block, const uint pindexNew->nSequenceId = 0; pindexNew->phashBlock = &((*mi).first); - 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; - pindexNew->BuildSkip(); + { + LOCK(m_block_index_mutex); + 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; + pindexNew->BuildSkip(); + } } pindexNew->nTimeMax = (pindexNew->pprev ? std::max(pindexNew->pprev->nTimeMax, pindexNew->nTime) : pindexNew->nTime); pindexNew->nChainWork = (pindexNew->pprev ? pindexNew->pprev->nChainWork : 0) + GetBlockProof(*pindexNew); @@ -139,6 +143,7 @@ void BlockManager::PruneOneBlockFile(const int fileNumber) AssertLockHeld(cs_main); LOCK(cs_LastBlockFile); + LOCK(m_block_index_mutex); for (auto& entry : m_block_index) { CBlockIndex* pindex = &entry.second; if (pindex->nFile == fileNumber) { @@ -263,7 +268,7 @@ CBlockIndex* BlockManager::InsertBlockIndex(const uint256& hash) return nullptr; } - const auto [mi, inserted]{m_block_index.try_emplace(hash)}; + const auto [mi, inserted]{WITH_LOCK(m_block_index_mutex, return m_block_index.try_emplace(hash))}; CBlockIndex* pindex = &(*mi).second; if (inserted) { pindex->phashBlock = &((*mi).first); @@ -277,13 +282,15 @@ bool BlockManager::LoadBlockIndex(const Consensus::Params& consensus_params) return false; } - for (auto& [_, block_index] : m_block_index) { - // build m_blockman.m_prev_block_index - if (block_index.pprev) { - m_prev_block_index.emplace(block_index.pprev->GetBlockHash(), &block_index); + { + LOCK(m_block_index_mutex); + for (auto& [_, block_index] : m_block_index) { + // build m_blockman.m_prev_block_index + if (block_index.pprev) { + m_prev_block_index.emplace(block_index.pprev->GetBlockHash(), &block_index); + } } } - // Calculate nChainWork std::vector vSortedByHeight{GetAllBlockIndices()}; std::sort(vSortedByHeight.begin(), vSortedByHeight.end(), @@ -369,9 +376,12 @@ bool BlockManager::LoadBlockIndexDB() // Check presence of blk files LogPrintf("Checking all blk files are present...\n"); std::set setBlkDataFiles; - for (const auto& [_, block_index] : m_block_index) { - if (block_index.nStatus & BLOCK_HAVE_DATA) { - setBlkDataFiles.insert(block_index.nFile); + { + LOCK(m_block_index_mutex); + for (const auto& [_, block_index] : m_block_index) { + if (block_index.nStatus & BLOCK_HAVE_DATA) { + setBlkDataFiles.insert(block_index.nFile); + } } } for (std::set::iterator it = setBlkDataFiles.begin(); it != setBlkDataFiles.end(); it++) { diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h index 5b7468e79c..6f71a775ac 100644 --- a/src/node/blockstorage.h +++ b/src/node/blockstorage.h @@ -104,14 +104,14 @@ private: * collections like m_dirty_blockindex. */ bool LoadBlockIndex(const Consensus::Params& consensus_params) - EXCLUSIVE_LOCKS_REQUIRED(cs_main); + EXCLUSIVE_LOCKS_REQUIRED(cs_main, !m_block_index_mutex); void FlushBlockFile(bool fFinalize = false, bool finalize_undo = false); void FlushUndoFile(int block_file, bool finalize = false); bool FindBlockPos(FlatFilePos& pos, unsigned int nAddSize, unsigned int nHeight, CChain& active_chain, uint64_t nTime, bool fKnown); bool FindUndoPos(BlockValidationState& state, int nFile, FlatFilePos& pos, unsigned int nAddSize); /* Calculate the block/rev files to delete based on height specified by user with RPC command pruneblockchain */ - void FindFilesToPruneManual(std::set& setFilesToPrune, int nManualPruneHeight, int chain_tip_height); + void FindFilesToPruneManual(std::set& setFilesToPrune, int nManualPruneHeight, int chain_tip_height) EXCLUSIVE_LOCKS_REQUIRED(!m_block_index_mutex); /** * Prune block and undo files (blk???.dat and rev???.dat) so that the disk space used is less than a user-defined target. @@ -128,7 +128,7 @@ private: * * @param[out] setFilesToPrune The set of file indices that can be unlinked will be returned */ - void FindFilesToPrune(std::set& setFilesToPrune, uint64_t nPruneAfterHeight, int chain_tip_height, int prune_height, bool is_ibd); + void FindFilesToPrune(std::set& setFilesToPrune, uint64_t nPruneAfterHeight, int chain_tip_height, int prune_height, bool is_ibd) EXCLUSIVE_LOCKS_REQUIRED(!m_block_index_mutex); RecursiveMutex cs_LastBlockFile; std::vector m_blockfile_info; @@ -154,10 +154,11 @@ private: std::unordered_map m_prune_locks GUARDED_BY(::cs_main); public: - BlockMap m_block_index GUARDED_BY(cs_main); + mutable Mutex m_block_index_mutex; + BlockMap m_block_index GUARDED_BY(m_block_index_mutex); PrevBlockMap m_prev_block_index GUARDED_BY(cs_main); - std::vector GetAllBlockIndices() EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + std::vector GetAllBlockIndices() EXCLUSIVE_LOCKS_REQUIRED(::cs_main, !m_block_index_mutex); /** * All pairs A->B, where A (or one of its ancestors) misses transactions, but B has transactions. @@ -168,19 +169,19 @@ public: std::unique_ptr m_block_tree_db GUARDED_BY(::cs_main); bool WriteBlockIndexDB() EXCLUSIVE_LOCKS_REQUIRED(::cs_main); - bool LoadBlockIndexDB() EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + bool LoadBlockIndexDB() EXCLUSIVE_LOCKS_REQUIRED(::cs_main, !m_block_index_mutex); CBlockIndex* AddToBlockIndex(const CBlockHeader& block, const uint256& hash, CBlockIndex*& best_header, enum BlockStatus nStatus = BLOCK_VALID_TREE) - EXCLUSIVE_LOCKS_REQUIRED(cs_main); + EXCLUSIVE_LOCKS_REQUIRED(cs_main, !m_block_index_mutex); /** Create a new block index entry for a given block hash */ - CBlockIndex* InsertBlockIndex(const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + CBlockIndex* InsertBlockIndex(const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main, !m_block_index_mutex); //! Mark one block file as pruned (modify associated database entries) - void PruneOneBlockFile(const int fileNumber) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + void PruneOneBlockFile(const int fileNumber) EXCLUSIVE_LOCKS_REQUIRED(cs_main, !m_block_index_mutex); - CBlockIndex* LookupBlockIndex(const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main); - const CBlockIndex* LookupBlockIndex(const uint256& hash) const EXCLUSIVE_LOCKS_REQUIRED(cs_main); + CBlockIndex* LookupBlockIndex(const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(!m_block_index_mutex); + const CBlockIndex* LookupBlockIndex(const uint256& hash) const EXCLUSIVE_LOCKS_REQUIRED(!m_block_index_mutex); /** Get block file info entry for one block file */ CBlockFileInfo* GetBlockFileInfo(size_t n); @@ -195,7 +196,7 @@ public: uint64_t CalculateCurrentUsage(); //! Returns last CBlockIndex* that is a checkpoint - const CBlockIndex* GetLastCheckpoint(const CCheckpointData& data) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + const CBlockIndex* GetLastCheckpoint(const CCheckpointData& data) EXCLUSIVE_LOCKS_REQUIRED(cs_main, !m_block_index_mutex); //! Find the first block that is not pruned const CBlockIndex* GetFirstStoredBlock(const CBlockIndex& start_block LIFETIMEBOUND) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); diff --git a/src/rpc/quorums.cpp b/src/rpc/quorums.cpp index e4c12e8b21..a56ca03c63 100644 --- a/src/rpc/quorums.cpp +++ b/src/rpc/quorums.cpp @@ -815,7 +815,7 @@ static RPCHelpMan quorum_getdata() } } - const CBlockIndex* pQuorumBaseBlockIndex = WITH_LOCK(cs_main, return chainman.m_blockman.LookupBlockIndex(quorumHash)); + const CBlockIndex* pQuorumBaseBlockIndex = chainman.m_blockman.LookupBlockIndex(quorumHash); return connman.ForNode(nodeId, [&](CNode* pNode) { return llmq_ctx.qman->RequestQuorumData(pNode, llmqType, pQuorumBaseBlockIndex, nDataMask, proTxHash); @@ -968,7 +968,7 @@ static RPCHelpMan verifychainlock() int nBlockHeight; const CBlockIndex* pIndex{nullptr}; if (request.params[2].isNull()) { - pIndex = WITH_LOCK(cs_main, return chainman.m_blockman.LookupBlockIndex(nBlockHash)); + pIndex = chainman.m_blockman.LookupBlockIndex(nBlockHash); if (pIndex == nullptr) { throw JSONRPCError(RPC_INTERNAL_ERROR, "blockHash not found"); } @@ -1029,13 +1029,10 @@ static RPCHelpMan verifyislock() } const CBlockIndex* pindexMined{nullptr}; - { - LOCK(cs_main); - uint256 hash_block; - CTransactionRef tx = GetTransaction(/* block_index */ nullptr, /* mempool */ nullptr, txid, Params().GetConsensus(), hash_block); - if (tx && !hash_block.IsNull()) { - pindexMined = chainman.m_blockman.LookupBlockIndex(hash_block); - } + uint256 hash_block; + CTransactionRef tx = GetTransaction(/* block_index */ nullptr, /* mempool */ nullptr, txid, Params().GetConsensus(), hash_block); + if (tx && !hash_block.IsNull()) { + pindexMined = chainman.m_blockman.LookupBlockIndex(hash_block); } int maxHeight{-1}; diff --git a/src/validation.cpp b/src/validation.cpp index 1074bb065f..2561aec1b8 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1920,6 +1920,7 @@ bool CChainState::ConnectBlock(const CBlock& block, BlockValidationState& state, // 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. + LOCK(m_blockman.m_block_index_mutex); 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 && @@ -3181,6 +3182,7 @@ bool CChainState::InvalidateBlock(BlockValidationState& state, CBlockIndex* pind { LOCK(cs_main); + LOCK(m_blockman.m_block_index_mutex); 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 @@ -3290,12 +3292,15 @@ 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. - for (auto& [_, block_index] : m_blockman.m_block_index) { - if (block_index.IsValid(BLOCK_VALID_TRANSACTIONS) && !(block_index.nStatus & BLOCK_CONFLICT_CHAINLOCK) && block_index.HaveTxsDownloaded() && !setBlockIndexCandidates.value_comp()(&block_index, m_chain.Tip())) { - setBlockIndexCandidates.insert(&block_index); + { + LOCK(m_blockman.m_block_index_mutex); + for (auto& [_, block_index] : m_blockman.m_block_index) { + if (block_index.IsValid(BLOCK_VALID_TRANSACTIONS) && !(block_index.nStatus & BLOCK_CONFLICT_CHAINLOCK) && + block_index.HaveTxsDownloaded() && !setBlockIndexCandidates.value_comp()(&block_index, m_chain.Tip())) { + setBlockIndexCandidates.insert(&block_index); + } } } - InvalidChainFound(to_mark_failed); GetMainSignals().SynchronousUpdatedBlockTip(m_chain.Tip(), nullptr, IsInitialBlockDownload()); GetMainSignals().UpdatedBlockTip(m_chain.Tip(), nullptr, IsInitialBlockDownload()); @@ -3391,14 +3396,17 @@ bool CChainState::MarkConflictingBlock(BlockValidationState& state, CBlockIndex // The resulting new best tip may not be in setBlockIndexCandidates anymore, so // add it again. - 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.nStatus & BLOCK_CONFLICT_CHAINLOCK) && it->second.HaveTxsDownloaded() && !setBlockIndexCandidates.value_comp()(&it->second, m_chain.Tip())) { - setBlockIndexCandidates.insert(&it->second); + { + LOCK(m_blockman.m_block_index_mutex); + 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.nStatus & BLOCK_CONFLICT_CHAINLOCK) && + it->second.HaveTxsDownloaded() && !setBlockIndexCandidates.value_comp()(&it->second, m_chain.Tip())) { + setBlockIndexCandidates.insert(&it->second); + } + it++; } - it++; } - ConflictingChainFound(pindex); GetMainSignals().SynchronousUpdatedBlockTip(m_chain.Tip(), nullptr, IsInitialBlockDownload()); GetMainSignals().UpdatedBlockTip(m_chain.Tip(), nullptr, IsInitialBlockDownload()); @@ -3426,21 +3434,24 @@ void CChainState::ResetBlockFailureFlags(CBlockIndex *pindex) { int nHeight = pindex->nHeight; // Remove the invalidity flag from this block and all its descendants. - 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.nStatus & BLOCK_CONFLICT_CHAINLOCK) && block_index.HaveTxsDownloaded() && setBlockIndexCandidates.value_comp()(m_chain.Tip(), &block_index)) { - setBlockIndexCandidates.insert(&block_index); + { + LOCK(m_blockman.m_block_index_mutex); + 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.nStatus & BLOCK_CONFLICT_CHAINLOCK) && + block_index.HaveTxsDownloaded() && setBlockIndexCandidates.value_comp()(m_chain.Tip(), &block_index)) { + setBlockIndexCandidates.insert(&block_index); + } + 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(&block_index); } - 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(&block_index); } } - // Remove the invalidity flag from all ancestors too. while (pindex != nullptr) { if (pindex->nStatus & BLOCK_FAILED_MASK) { @@ -3748,18 +3759,23 @@ bool ChainstateManager::AcceptBlockHeader(const CBlockHeader& block, BlockValida uint256 hash = block.GetHash(); // TODO : ENABLE BLOCK CACHE IN SPECIFIC CASES - BlockMap::iterator miSelf{m_blockman.m_block_index.find(hash)}; - if (hash != chainparams.GetConsensus().hashGenesisBlock) { + auto existing_block_pindex = [&]() -> CBlockIndex* { + LOCK(m_blockman.m_block_index_mutex); + BlockMap::iterator miSelf{m_blockman.m_block_index.find(hash)}; if (miSelf != m_blockman.m_block_index.end()) { - // Block header is already known. - CBlockIndex* pindex = &(miSelf->second); - if (ppindex) - *ppindex = pindex; - if (pindex->nStatus & BLOCK_FAILED_MASK) { + return &(miSelf->second); + } else { + return nullptr; + } + }(); + if (hash != chainparams.GetConsensus().hashGenesisBlock) { + if (existing_block_pindex) { + if (ppindex) *ppindex = existing_block_pindex; + if (existing_block_pindex->nStatus & BLOCK_FAILED_MASK) { LogPrint(BCLog::VALIDATION, "%s: block %s is marked invalid\n", __func__, hash.ToString()); return state.Invalid(BlockValidationResult::BLOCK_CACHED_INVALID, "duplicate"); } - if (pindex->nStatus & BLOCK_CONFLICT_CHAINLOCK) { + if (existing_block_pindex->nStatus & BLOCK_CONFLICT_CHAINLOCK) { LogPrintf("ERROR: %s: block %s is marked conflicting\n", __func__, hash.ToString()); return state.Invalid(BlockValidationResult::BLOCK_CHAINLOCK, "duplicate"); } @@ -3767,18 +3783,22 @@ bool ChainstateManager::AcceptBlockHeader(const CBlockHeader& block, BlockValida } if (!CheckBlockHeader(block, hash, state, chainparams.GetConsensus())) { - LogPrint(BCLog::VALIDATION, "%s: Consensus::CheckBlockHeader: %s, %s\n", __func__, hash.ToString(), state.ToString()); + LogPrint(BCLog::VALIDATION, "%s: Consensus::CheckBlockHeader: %s, %s\n", __func__, hash.ToString(), + state.ToString()); return false; } // Get prev block index CBlockIndex* pindexPrev = nullptr; - BlockMap::iterator mi{m_blockman.m_block_index.find(block.hashPrevBlock)}; - if (mi == m_blockman.m_block_index.end()) { - LogPrint(BCLog::VALIDATION, "%s: %s prev block not found\n", __func__, hash.ToString()); - return state.Invalid(BlockValidationResult::BLOCK_MISSING_PREV, "prev-blk-not-found"); + { + LOCK(m_blockman.m_block_index_mutex); + BlockMap::iterator mi{m_blockman.m_block_index.find(block.hashPrevBlock)}; + if (mi == m_blockman.m_block_index.end()) { + 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); assert(pindexPrev); if (pindexPrev->nStatus & BLOCK_FAILED_MASK) { @@ -3788,12 +3808,14 @@ bool ChainstateManager::AcceptBlockHeader(const CBlockHeader& block, BlockValida if (pindexPrev->nStatus & BLOCK_CONFLICT_CHAINLOCK) { // it's ok-ish, the other node is probably missing the latest chainlock - LogPrint(BCLog::VALIDATION, "%s: prev block %s conflicts with chainlock\n", __func__, block.hashPrevBlock.ToString()); + LogPrint(BCLog::VALIDATION, "%s: prev block %s conflicts with chainlock\n", __func__, + block.hashPrevBlock.ToString()); return state.Invalid(BlockValidationResult::BLOCK_CHAINLOCK, "bad-prevblk-chainlock"); } if (!ContextualCheckBlockHeader(block, state, m_blockman, chainparams, pindexPrev, GetAdjustedTime())) { - LogPrint(BCLog::VALIDATION, "%s: Consensus::ContextualCheckBlockHeader: %s, %s\n", __func__, hash.ToString(), state.ToString()); + LogPrint(BCLog::VALIDATION, "%s: Consensus::ContextualCheckBlockHeader: %s, %s\n", __func__, + hash.ToString(), state.ToString()); return false; } @@ -3837,7 +3859,7 @@ bool ChainstateManager::AcceptBlockHeader(const CBlockHeader& block, BlockValida } if (llmq::chainLocksHandler->HasConflictingChainLock(pindexPrev->nHeight + 1, hash)) { - if (miSelf == m_blockman.m_block_index.end()) { + if (!existing_block_pindex) { m_blockman.AddToBlockIndex(block, hash, m_best_header, BLOCK_CONFLICT_CHAINLOCK); } LogPrintf("ERROR: %s: header %s conflicts with chainlock\n", __func__, hash.ToString()); @@ -4376,24 +4398,26 @@ bool CChainState::ReplayBlocks() 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 (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]]); - - 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"); + { + LOCK(m_blockman.m_block_index_mutex); + if (m_blockman.m_block_index.count(hashHeads[0]) == 0) { + return error("ReplayBlocks(): reorganization to unknown block requested"); } - pindexOld = &(m_blockman.m_block_index[hashHeads[1]]); - pindexFork = LastCommonAncestor(pindexOld, pindexNew); - assert(pindexFork != nullptr); - const bool fDIP0003Active = pindexOld->nHeight >= m_params.GetConsensus().DIP0003Height; - if (fDIP0003Active && !m_evoDb.VerifyBestBlock(pindexOld->GetBlockHash())) { - return error("ReplayBlocks(DASH): Found EvoDB inconsistency"); + 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]]); + pindexFork = LastCommonAncestor(pindexOld, pindexNew); + assert(pindexFork != nullptr); + const bool fDIP0003Active = pindexOld->nHeight >= m_params.GetConsensus().DIP0003Height; + if (fDIP0003Active && !m_evoDb.VerifyBestBlock(pindexOld->GetBlockHash())) { + return error("ReplayBlocks(DASH): Found EvoDB inconsistency"); + } } } - auto dbTx = m_evoDb.BeginTransaction(); // Rollback along the old branch. @@ -4517,6 +4541,7 @@ bool ChainstateManager::LoadBlockIndex() m_best_header = pindex; } + LOCK(m_blockman.m_block_index_mutex); needs_init = m_blockman.m_block_index.empty(); } @@ -4568,7 +4593,7 @@ bool CChainState::LoadGenesisBlock() // 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 (m_blockman.m_block_index.count(m_params.GenesisBlock().GetHash())) + if (LOCK(m_blockman.m_block_index_mutex); m_blockman.m_block_index.count(m_params.GenesisBlock().GetHash())) return true; try { @@ -4750,6 +4775,7 @@ void CChainState::CheckBlockIndex() LOCK(cs_main); + LOCK(m_blockman.m_block_index_mutex); // During a reindex, we read the genesis block and call CheckBlockIndex before ActivateBestChain, // 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.) @@ -4757,7 +4783,6 @@ void CChainState::CheckBlockIndex() assert(m_blockman.m_block_index.size() <= 1); return; } - // Build forward-pointing map of the entire block tree. std::multimap forward; for (auto& [_, block_index] : m_blockman.m_block_index) {