From 6cae4a5f8767f1192f0a07797155f9736040a5d9 Mon Sep 17 00:00:00 2001 From: pasta Date: Wed, 20 Nov 2024 21:17:06 -0600 Subject: [PATCH 1/2] refactor: (mostly) introduce two new cache structures to minimize cs_main contention during llmq signing --- src/llmq/ehf_signals.cpp | 2 +- src/llmq/instantsend.cpp | 2 +- src/llmq/quorums.cpp | 30 ++++++++++++++++++++++-------- src/llmq/quorums.h | 22 ++++++++++++++-------- src/llmq/signing.cpp | 2 +- src/rpc/quorums.cpp | 4 ++-- 6 files changed, 41 insertions(+), 21 deletions(-) diff --git a/src/llmq/ehf_signals.cpp b/src/llmq/ehf_signals.cpp index c5388e46dd5e0..0d19e29a2b310 100644 --- a/src/llmq/ehf_signals.cpp +++ b/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; diff --git a/src/llmq/instantsend.cpp b/src/llmq/instantsend.cpp index c50fa939e0f0c..37941ef80d59b 100644 --- a/src/llmq/instantsend.cpp +++ b/src/llmq/instantsend.cpp @@ -905,7 +905,7 @@ std::unordered_set 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 {}; diff --git a/src/llmq/quorums.cpp b/src/llmq/quorums.cpp index 87aef8e79102a..61e008b4e9aa6 100644 --- a/src/llmq/quorums.cpp +++ b/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 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 CQuorumManager::ScanQuorums(Consensus::LLMQType llmqType, const CBlockIndex* pindexStart, size_t nCountRequested) const @@ -633,7 +634,20 @@ std::vector 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; } diff --git a/src/llmq/quorums.h b/src/llmq/quorums.h index fac31afb026bd..4f40c00f9dbaa 100644 --- a/src/llmq/quorums.h +++ b/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> cleanupQuorumsCache GUARDED_BY(cs_cleanup); + mutable std::atomic 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 quorumBaseBlockIndexCache; + mutable ctpl::thread_pool workerPool; mutable CThreadInterrupt quorumThreadInterrupt; @@ -282,6 +293,9 @@ class CQuorumManager // this one is cs_main-free std::vector 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, diff --git a/src/llmq/signing.cpp b/src/llmq/signing.cpp index 68b4949438c07..eb300087a8ff0 100644 --- a/src/llmq/signing.cpp +++ b/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); } diff --git a/src/rpc/quorums.cpp b/src/rpc/quorums.cpp index e4c12e8b218da..f88458f61b599 100644 --- a/src/rpc/quorums.cpp +++ b/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"); } From a8ed736a5953589e233067663eab595d0c1ce8a5 Mon Sep 17 00:00:00 2001 From: pasta Date: Wed, 20 Nov 2024 22:41:26 -0600 Subject: [PATCH 2/2] actually use cachedChainTip --- src/llmq/quorums.cpp | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/llmq/quorums.cpp b/src/llmq/quorums.cpp index 61e008b4e9aa6..95b7f0fb38e05 100644 --- a/src/llmq/quorums.cpp +++ b/src/llmq/quorums.cpp @@ -1206,17 +1206,22 @@ CQuorumCPtr CQuorumManager::SelectQuorumForSigning(const Consensus::LLMQParams& { size_t poolSize = llmq_params.signingActiveQuorumCount; - CBlockIndex* pindexStart; + auto pindexStart = [&]() -> const CBlockIndex* { - LOCK(cs_main); + auto chainTip = cachedChainTip.load(); + if (chainTip == nullptr) return nullptr; + auto cur_height = chainTip->nHeight; if (signHeight == -1) { - signHeight = active_chain.Height(); + signHeight = cur_height; } int startBlockHeight = signHeight - signOffset; - if (startBlockHeight > active_chain.Height() || startBlockHeight < 0) { - return {}; + if (startBlockHeight > cur_height || startBlockHeight < 0) { + return nullptr; } - pindexStart = active_chain[startBlockHeight]; + return chainTip->GetAncestor(startBlockHeight); + }(); + if (pindexStart == nullptr) { + return {}; } if (IsQuorumRotationEnabled(llmq_params, pindexStart)) {