wip: drop cs_main guarding from m_block_index; introduce m_block_inde…

…x_mutex
PastaPastaPasta · Dec 5, 2024 · 53e9c29 · 53e9c29
1 parent 1e55310
commit 53e9c29
Show file tree

Hide file tree

Showing 12 changed files with 138 additions and 109 deletions.
diff --git 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
@@ -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
@@ -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
@@ -26,7 +26,7 @@ UniValue CDKGDebugSessionStatus::ToJson(CDeterministicMNManager& dmnman, const C
 
     std::vector<CDeterministicMNCPtr> 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
@@ -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
@@ -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
@@ -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<uint256, StaticSaltedHasher> 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
@@ -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
@@ -66,6 +66,7 @@ static FlatFileSeq UndoFileSeq();
 std::vector<CBlockIndex*> BlockManager::GetAllBlockIndices()
 {
     AssertLockHeld(cs_main);
+    LOCK(m_block_index_mutex);
     std::vector<CBlockIndex*> rv;
     rv.reserve(m_block_index.size());
     for (auto& [_, block_index] : m_block_index) {
@@ -76,14 +77,14 @@ std::vector<CBlockIndex*> 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<CBlockIndex*> 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<int> 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<int>::iterator it = setBlkDataFiles.begin(); it != setBlkDataFiles.end(); it++) {

diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h
@@ -104,14 +104,14 @@ class BlockManager
      * 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<int>& setFilesToPrune, int nManualPruneHeight, int chain_tip_height);
+    void FindFilesToPruneManual(std::set<int>& 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 @@ class BlockManager
      *
      * @param[out]   setFilesToPrune   The set of file indices that can be unlinked will be returned
      */
-    void FindFilesToPrune(std::set<int>& setFilesToPrune, uint64_t nPruneAfterHeight, int chain_tip_height, int prune_height, bool is_ibd);
+    void FindFilesToPrune(std::set<int>& 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<CBlockFileInfo> m_blockfile_info;
@@ -154,10 +154,11 @@ class BlockManager
     std::unordered_map<std::string, PruneLockInfo> 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<CBlockIndex*> GetAllBlockIndices() EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
+    std::vector<CBlockIndex*> 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 @@ class BlockManager
     std::unique_ptr<CBlockTreeDB> 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 @@ class BlockManager
     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
@@ -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};