refactor: (mostly) introduce two new cache structures to minimize cs_…

…main contention during llmq signing

src/llmq/ehf_signals.cpp

@@ -75,7 +75,7 @@ void CEHFSignalsHandler::trySignEHFSignal(int bit, const CBlockIndex* const pind
         return;
     }
 
-    const auto quorum = llmq::SelectQuorumForSigning(llmq_params_opt.value(), m_chainman.ActiveChain(), qman, requestId);
+    const auto quorum = qman.SelectQuorumForSigning(llmq_params_opt.value(), m_chainman.ActiveChain(), requestId);
     if (!quorum) {
         LogPrintf("CEHFSignalsHandler::trySignEHFSignal no quorum for id=%s\n", requestId.ToString());
         return;

src/llmq/instantsend.cpp

@@ -905,7 +905,7 @@ std::unordered_set<uint256, StaticSaltedHasher> CInstantSendManager::ProcessPend
             nSignHeight = blockIndex->nHeight + dkgInterval - 1;
         }
 
-        auto quorum = llmq::SelectQuorumForSigning(llmq_params, m_chainstate.m_chain, qman, id, nSignHeight, signOffset);
+        auto quorum = qman.SelectQuorumForSigning(llmq_params, m_chainstate.m_chain, id, nSignHeight, signOffset);
         if (!quorum) {
             // should not happen, but if one fails to select, all others will also fail to select
             return {};

src/llmq/quorums.cpp

@@ -303,6 +303,8 @@ void CQuorumManager::TriggerQuorumDataRecoveryThreads(const CBlockIndex* pIndex)
 
 void CQuorumManager::UpdatedBlockTip(const CBlockIndex* pindexNew, bool fInitialDownload) const
 {
+    cachedChainTip = pindexNew;
+
     if (!m_mn_sync.IsBlockchainSynced()) return;
 
     for (const auto& params : Params().GetConsensus().llmqs) {
@@ -519,8 +521,7 @@ bool CQuorumManager::RequestQuorumData(CNode* pfrom, Consensus::LLMQType llmqTyp
 
 std::vector<CQuorumCPtr> CQuorumManager::ScanQuorums(Consensus::LLMQType llmqType, size_t nCountRequested) const
 {
-    const CBlockIndex* pindex = WITH_LOCK(cs_main, return m_chainstate.m_chain.Tip());
-    return ScanQuorums(llmqType, pindex, nCountRequested);
+    return ScanQuorums(llmqType, cachedChainTip, nCountRequested);
 }
 
 std::vector<CQuorumCPtr> CQuorumManager::ScanQuorums(Consensus::LLMQType llmqType, const CBlockIndex* pindexStart, size_t nCountRequested) const
@@ -633,7 +634,20 @@ std::vector<CQuorumCPtr> CQuorumManager::ScanQuorums(Consensus::LLMQType llmqTyp
 
 CQuorumCPtr CQuorumManager::GetQuorum(Consensus::LLMQType llmqType, const uint256& quorumHash) const
 {
-    const CBlockIndex* pQuorumBaseBlockIndex = WITH_LOCK(cs_main, return m_chainstate.m_blockman.LookupBlockIndex(quorumHash));
+    const CBlockIndex* pQuorumBaseBlockIndex = [&]() {
+        // Lock contention may still be high here; consider using a shared lock
+        // We cannot hold cs_quorumBaseBlockIndexCache the whole time as that creates lock-order inversion with cs_main;
+        // 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));
+            if (pindex) {
+                LOCK(cs_quorumBaseBlockIndexCache);
+                quorumBaseBlockIndexCache.insert(quorumHash, pindex);
+            }
+        }
+        return pindex;
+    }();
     if (!pQuorumBaseBlockIndex) {
         LogPrint(BCLog::LLMQ, "CQuorumManager::%s -- block %s not found\n", __func__, quorumHash.ToString());
         return nullptr;
@@ -1187,8 +1201,8 @@ void CQuorumManager::MigrateOldQuorumDB(CEvoDB& evoDb) const
     LogPrint(BCLog::LLMQ, "CQuorumManager::%s -- done\n", __func__);
 }
 
-CQuorumCPtr SelectQuorumForSigning(const Consensus::LLMQParams& llmq_params, const CChain& active_chain, const CQuorumManager& qman,
-                                   const uint256& selectionHash, int signHeight, int signOffset)
+CQuorumCPtr CQuorumManager::SelectQuorumForSigning(const Consensus::LLMQParams& llmq_params, const CChain& active_chain,
+                                   const uint256& selectionHash, int signHeight, int signOffset) const
 {
     size_t poolSize = llmq_params.signingActiveQuorumCount;
 
@@ -1206,7 +1220,7 @@ CQuorumCPtr SelectQuorumForSigning(const Consensus::LLMQParams& llmq_params, con
     }
 
     if (IsQuorumRotationEnabled(llmq_params, pindexStart)) {
-        auto quorums = qman.ScanQuorums(llmq_params.type, pindexStart, poolSize);
+        auto quorums = ScanQuorums(llmq_params.type, pindexStart, poolSize);
         if (quorums.empty()) {
             return nullptr;
         }
@@ -1230,7 +1244,7 @@ CQuorumCPtr SelectQuorumForSigning(const Consensus::LLMQParams& llmq_params, con
         }
         return *itQuorum;
     } else {
-        auto quorums = qman.ScanQuorums(llmq_params.type, pindexStart, poolSize);
+        auto quorums = ScanQuorums(llmq_params.type, pindexStart, poolSize);
         if (quorums.empty()) {
             return nullptr;
         }
@@ -1255,7 +1269,7 @@ VerifyRecSigStatus VerifyRecoveredSig(Consensus::LLMQType llmqType, const CChain
 {
     const auto& llmq_params_opt = Params().GetLLMQ(llmqType);
     assert(llmq_params_opt.has_value());
-    auto quorum = SelectQuorumForSigning(llmq_params_opt.value(), active_chain, qman, id, signedAtHeight, signOffset);
+    auto quorum = qman.SelectQuorumForSigning(llmq_params_opt.value(), active_chain, id, signedAtHeight, signOffset);
     if (!quorum) {
         return VerifyRecSigStatus::NoQuorum;
     }

src/llmq/quorums.h

@@ -223,6 +223,11 @@ class CQuorum
     bool ReadContributions(const CDBWrapper& db);
 };
 
+// when selecting a quorum for signing and verification, we use CQuorumManager::SelectQuorum with this offset as
+// starting height for scanning. This is because otherwise the resulting signatures would not be verifiable by nodes
+// which are not 100% at the chain tip.
+static constexpr int SIGN_HEIGHT_OFFSET{8};
+
 /**
  * The quorum manager maintains quorums which were mined on chain. When a quorum is requested from the manager,
  * it will lookup the commitment (through CQuorumBlockProcessor) and build a CQuorum object from it.
@@ -252,6 +257,12 @@ class CQuorumManager
     mutable Mutex cs_cleanup;
     mutable std::map<Consensus::LLMQType, unordered_lru_cache<uint256, uint256, StaticSaltedHasher>> cleanupQuorumsCache GUARDED_BY(cs_cleanup);
 
+    mutable std::atomic<const CBlockIndex*> cachedChainTip{nullptr};
+
+    mutable Mutex cs_quorumBaseBlockIndexCache;
+    // On mainnet, we have around 62 quorums active at any point; let's cache a little more than double that to be safe.
+    mutable unordered_lru_cache<uint256 /*quorum_hash*/, const CBlockIndex* /*pindex*/, StaticSaltedHasher, 128 /*max_size*/> quorumBaseBlockIndexCache;
+
     mutable ctpl::thread_pool workerPool;
     mutable CThreadInterrupt quorumThreadInterrupt;
 
@@ -282,6 +293,9 @@ class CQuorumManager
     // this one is cs_main-free
     std::vector<CQuorumCPtr> ScanQuorums(Consensus::LLMQType llmqType, const CBlockIndex* pindexStart, size_t nCountRequested) const;
 
+    CQuorumCPtr SelectQuorumForSigning(const Consensus::LLMQParams& llmq_params, const CChain& active_chain,
+                                       const uint256& selectionHash, int signHeight = -1 /*chain tip*/, int signOffset = SIGN_HEIGHT_OFFSET) const;
+
 private:
     // all private methods here are cs_main-free
     void CheckQuorumConnections(const Consensus::LLMQParams& llmqParams, const CBlockIndex *pindexNew) const;
@@ -302,14 +316,6 @@ class CQuorumManager
     void MigrateOldQuorumDB(CEvoDB& evoDb) const;
 };
 
-// when selecting a quorum for signing and verification, we use CQuorumManager::SelectQuorum with this offset as
-// starting height for scanning. This is because otherwise the resulting signatures would not be verifiable by nodes
-// which are not 100% at the chain tip.
-static constexpr int SIGN_HEIGHT_OFFSET{8};
-
-CQuorumCPtr SelectQuorumForSigning(const Consensus::LLMQParams& llmq_params, const CChain& active_chain, const CQuorumManager& qman,
-                                   const uint256& selectionHash, int signHeight = -1 /*chain tip*/, int signOffset = SIGN_HEIGHT_OFFSET);
-
 // Verifies a recovered sig that was signed while the chain tip was at signedAtTip
 VerifyRecSigStatus VerifyRecoveredSig(Consensus::LLMQType llmqType, const CChain& active_chain, const CQuorumManager& qman,
                                       int signedAtHeight, const uint256& id, const uint256& msgHash, const CBLSSignature& sig,

src/llmq/signing.cpp

@@ -704,7 +704,7 @@ bool CSigningManager::AsyncSignIfMember(Consensus::LLMQType llmqType, CSigShares
             // TODO fix this by re-signing when the next block arrives, but only when that block results in a change of the quorum list and no recovered signature has been created in the mean time
             const auto &llmq_params_opt = Params().GetLLMQ(llmqType);
             assert(llmq_params_opt.has_value());
-            return SelectQuorumForSigning(llmq_params_opt.value(), m_chainstate.m_chain, qman, id);
+            return qman.SelectQuorumForSigning(llmq_params_opt.value(), m_chainstate.m_chain, id);
         } else {
             return qman.GetQuorum(llmqType, quorumHash);
         }

src/rpc/quorums.cpp

@@ -453,7 +453,7 @@ static UniValue quorum_sign_helper(const JSONRPCRequest& request, Consensus::LLM
     } else {
         const auto pQuorum = [&]() {
             if (quorumHash.IsNull()) {
-                return llmq::SelectQuorumForSigning(llmq_params_opt.value(), chainman.ActiveChain(), *llmq_ctx.qman, id);
+                return llmq_ctx.qman->SelectQuorumForSigning(llmq_params_opt.value(), chainman.ActiveChain(), id);
             } else {
                 return llmq_ctx.qman->GetQuorum(llmqType, quorumHash);
             }
@@ -724,7 +724,7 @@ static RPCHelpMan quorum_selectquorum()
 
     UniValue ret(UniValue::VOBJ);
 
-    const auto quorum = llmq::SelectQuorumForSigning(llmq_params_opt.value(), chainman.ActiveChain(), *llmq_ctx.qman, id);
+    const auto quorum = llmq_ctx.qman->SelectQuorumForSigning(llmq_params_opt.value(), chainman.ActiveChain(), id);
     if (!quorum) {
         throw JSONRPCError(RPC_MISC_ERROR, "no quorums active");
     }