diff --git a/src/llmq/quorums.cpp b/src/llmq/quorums.cpp index e1714d9e0..b410a9251 100644 --- a/src/llmq/quorums.cpp +++ b/src/llmq/quorums.cpp @@ -166,8 +166,6 @@ void CQuorumManager::UpdatedBlockTip(const CBlockIndex* pindexNew, const CBlockI return; } - LOCK(cs_main); - for (auto& p : Params().GetConsensus().llmqs) { EnsureQuorumConnections(p.first, pindexNew); } @@ -175,18 +173,16 @@ void CQuorumManager::UpdatedBlockTip(const CBlockIndex* pindexNew, const CBlockI void CQuorumManager::EnsureQuorumConnections(Consensus::LLMQType llmqType, const CBlockIndex* pindexNew) { - AssertLockHeld(cs_main); - const auto& params = Params().GetConsensus().llmqs.at(llmqType); auto myProTxHash = activeMasternodeInfo.proTxHash; - auto lastQuorums = ScanQuorums(llmqType, (size_t)params.keepOldConnections); + auto lastQuorums = ScanQuorums(llmqType, pindexNew, (size_t)params.keepOldConnections); auto connmanQuorumsToDelete = g_connman->GetMasternodeQuorums(llmqType); // don't remove connections for the currently in-progress DKG round int curDkgHeight = pindexNew->nHeight - (pindexNew->nHeight % params.dkgInterval); - auto curDkgBlock = chainActive[curDkgHeight]->GetBlockHash(); + auto curDkgBlock = pindexNew->GetAncestor(curDkgHeight)->GetBlockHash(); connmanQuorumsToDelete.erase(curDkgBlock); for (auto& quorum : lastQuorums) { @@ -222,19 +218,14 @@ void CQuorumManager::EnsureQuorumConnections(Consensus::LLMQType llmqType, const } } -bool CQuorumManager::BuildQuorumFromCommitment(const CFinalCommitment& qc, std::shared_ptr& quorum) const +bool CQuorumManager::BuildQuorumFromCommitment(const CFinalCommitment& qc, const CBlockIndex* pindexQuorum, std::shared_ptr& quorum) const { - AssertLockHeld(cs_main); + assert(pindexQuorum); + assert(qc.quorumHash == pindexQuorum->GetBlockHash()); - if (!mapBlockIndex.count(qc.quorumHash)) { - LogPrintf("CQuorumManager::%s -- block %s not found", __func__, qc.quorumHash.ToString()); - return false; - } - auto& params = Params().GetConsensus().llmqs.at((Consensus::LLMQType)qc.llmqType); - auto quorumIndex = mapBlockIndex[qc.quorumHash]; auto members = CLLMQUtils::GetAllQuorumMembers((Consensus::LLMQType)qc.llmqType, qc.quorumHash); - quorum->Init(qc.quorumHash, quorumIndex->nHeight, members, qc.validMembers, qc.quorumPublicKey); + quorum->Init(qc.quorumHash, pindexQuorum->nHeight, members, qc.validMembers, qc.quorumPublicKey); bool hasValidVvec = false; if (quorum->ReadContributions(evoDb)) { @@ -301,11 +292,15 @@ bool CQuorumManager::HasQuorum(Consensus::LLMQType llmqType, const uint256& quor std::vector CQuorumManager::ScanQuorums(Consensus::LLMQType llmqType, size_t maxCount) { - LOCK(cs_main); - return ScanQuorums(llmqType, chainActive.Tip()->GetBlockHash(), maxCount); + const CBlockIndex* pindex; + { + LOCK(cs_main); + pindex = chainActive.Tip(); + } + return ScanQuorums(llmqType, pindex, maxCount); } -std::vector CQuorumManager::ScanQuorums(Consensus::LLMQType llmqType, const uint256& startBlock, size_t maxCount) +std::vector CQuorumManager::ScanQuorums(Consensus::LLMQType llmqType, const CBlockIndex* pindexStart, size_t maxCount) { std::vector result; result.reserve(maxCount); @@ -318,19 +313,13 @@ std::vector CQuorumManager::ScanQuorums(Consensus::LLMQType llmqTyp return result; } - LOCK(cs_main); - if (!mapBlockIndex.count(startBlock)) { - return result; - } - - CBlockIndex* pindex = mapBlockIndex[startBlock]; - pindex = chainActive[pindex->nHeight - (pindex->nHeight % params.dkgInterval)]; + auto pindex = pindexStart->GetAncestor(pindexStart->nHeight - (pindexStart->nHeight % params.dkgInterval)); while (pindex != nullptr && pindex->nHeight >= params.dkgInterval && result.size() < maxCount && deterministicMNManager->IsDIP3Enforced(pindex->nHeight)) { - auto quorum = GetQuorum(llmqType, pindex->GetBlockHash()); + auto quorum = GetQuorum(llmqType, pindex); if (quorum) { result.emplace_back(quorum); } @@ -348,7 +337,25 @@ std::vector CQuorumManager::ScanQuorums(Consensus::LLMQType llmqTyp CQuorumCPtr CQuorumManager::GetQuorum(Consensus::LLMQType llmqType, const uint256& quorumHash) { - AssertLockHeld(cs_main); + CBlockIndex* pindexQuorum; + { + LOCK(cs_main); + auto quorumIt = mapBlockIndex.find(quorumHash); + + if (quorumIt == mapBlockIndex.end()) { + LogPrintf("CQuorumManager::%s -- block %s not found", __func__, quorumHash.ToString()); + return nullptr; + } + pindexQuorum = quorumIt->second; + } + return GetQuorum(llmqType, pindexQuorum); +} + +CQuorumCPtr CQuorumManager::GetQuorum(Consensus::LLMQType llmqType, const CBlockIndex* pindexQuorum) +{ + assert(pindexQuorum); + + auto quorumHash = pindexQuorum->GetBlockHash(); // we must check this before we look into the cache. Reorgs might have happened which would mean we might have // cached quorums which are not in the active chain anymore @@ -371,7 +378,8 @@ CQuorumCPtr CQuorumManager::GetQuorum(Consensus::LLMQType llmqType, const uint25 auto& params = Params().GetConsensus().llmqs.at(llmqType); auto quorum = std::make_shared(params, blsWorker); - if (!BuildQuorumFromCommitment(qc, quorum)) { + + if (!BuildQuorumFromCommitment(qc, pindexQuorum, quorum)) { return nullptr; } diff --git a/src/llmq/quorums.h b/src/llmq/quorums.h index 86bfe4f27..5d88f3147 100644 --- a/src/llmq/quorums.h +++ b/src/llmq/quorums.h @@ -91,20 +91,24 @@ public: void UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload); -public: bool HasQuorum(Consensus::LLMQType llmqType, const uint256& quorumHash); - // all these methods will lock cs_main - CQuorumCPtr GetQuorum(Consensus::LLMQType llmqType,const uint256& quorumHash); + // all these methods will lock cs_main for a short period of time + CQuorumCPtr GetQuorum(Consensus::LLMQType llmqType, const uint256& quorumHash); CQuorumCPtr GetNewestQuorum(Consensus::LLMQType llmqType); std::vector ScanQuorums(Consensus::LLMQType llmqType, size_t maxCount); - std::vector ScanQuorums(Consensus::LLMQType llmqType, const uint256& startBlock, size_t maxCount); + + // this one is cs_main-free + std::vector ScanQuorums(Consensus::LLMQType llmqType, const CBlockIndex* pindexStart, size_t maxCount); private: + // all private methods here are cs_main-free void EnsureQuorumConnections(Consensus::LLMQType llmqType, const CBlockIndex *pindexNew); - bool BuildQuorumFromCommitment(const CFinalCommitment& qc, std::shared_ptr& quorum) const; + bool BuildQuorumFromCommitment(const CFinalCommitment& qc, const CBlockIndex* pindexQuorum, std::shared_ptr& quorum) const; bool BuildQuorumContributions(const CFinalCommitment& fqc, std::shared_ptr& quorum) const; + + CQuorumCPtr GetQuorum(Consensus::LLMQType llmqType, const CBlockIndex* pindex); }; extern CQuorumManager* quorumManager; diff --git a/src/llmq/quorums_signing.cpp b/src/llmq/quorums_signing.cpp index 3812ecd95..3fc01a9a9 100644 --- a/src/llmq/quorums_signing.cpp +++ b/src/llmq/quorums_signing.cpp @@ -259,19 +259,15 @@ bool CSigningManager::PreVerifyRecoveredSig(NodeId nodeId, const CRecoveredSig& return false; } - CQuorumCPtr quorum; - { - LOCK(cs_main); + CQuorumCPtr quorum = quorumManager->GetQuorum(llmqType, recoveredSig.quorumHash); - quorum = quorumManager->GetQuorum(llmqType, recoveredSig.quorumHash); - if (!quorum) { - LogPrintf("CSigningManager::%s -- quorum %s not found, node=%d\n", __func__, - recoveredSig.quorumHash.ToString(), nodeId); - return false; - } - if (!CLLMQUtils::IsQuorumActive(llmqType, quorum->quorumHash)) { - return false; - } + if (!quorum) { + LogPrintf("CSigningManager::%s -- quorum %s not found, node=%d\n", __func__, + recoveredSig.quorumHash.ToString(), nodeId); + return false; + } + if (!CLLMQUtils::IsQuorumActive(llmqType, quorum->quorumHash)) { + return false; } if (!recoveredSig.sig.IsValid()) { @@ -316,37 +312,34 @@ void CSigningManager::CollectPendingRecoveredSigsToVerify( } } - { - LOCK(cs_main); - for (auto& p : retSigShares) { - NodeId nodeId = p.first; - auto& v = p.second; + for (auto& p : retSigShares) { + NodeId nodeId = p.first; + auto& v = p.second; - for (auto it = v.begin(); it != v.end();) { - auto& recSig = *it; + for (auto it = v.begin(); it != v.end();) { + auto& recSig = *it; - Consensus::LLMQType llmqType = (Consensus::LLMQType) recSig.llmqType; - auto quorumKey = std::make_pair((Consensus::LLMQType)recSig.llmqType, recSig.quorumHash); - if (!retQuorums.count(quorumKey)) { - CQuorumCPtr quorum = quorumManager->GetQuorum(llmqType, recSig.quorumHash); - if (!quorum) { - LogPrintf("CSigningManager::%s -- quorum %s not found, node=%d\n", __func__, - recSig.quorumHash.ToString(), nodeId); - it = v.erase(it); - continue; - } - if (!CLLMQUtils::IsQuorumActive(llmqType, quorum->quorumHash)) { - LogPrintf("CSigningManager::%s -- quorum %s not active anymore, node=%d\n", __func__, - recSig.quorumHash.ToString(), nodeId); - it = v.erase(it); - continue; - } - - retQuorums.emplace(quorumKey, quorum); + Consensus::LLMQType llmqType = (Consensus::LLMQType) recSig.llmqType; + auto quorumKey = std::make_pair((Consensus::LLMQType)recSig.llmqType, recSig.quorumHash); + if (!retQuorums.count(quorumKey)) { + CQuorumCPtr quorum = quorumManager->GetQuorum(llmqType, recSig.quorumHash); + if (!quorum) { + LogPrintf("CSigningManager::%s -- quorum %s not found, node=%d\n", __func__, + recSig.quorumHash.ToString(), nodeId); + it = v.erase(it); + continue; + } + if (!CLLMQUtils::IsQuorumActive(llmqType, quorum->quorumHash)) { + LogPrintf("CSigningManager::%s -- quorum %s not active anymore, node=%d\n", __func__, + recSig.quorumHash.ToString(), nodeId); + it = v.erase(it); + continue; } - ++it; + retQuorums.emplace(quorumKey, quorum); } + + ++it; } } } @@ -561,17 +554,17 @@ CQuorumCPtr CSigningManager::SelectQuorumForSigning(Consensus::LLMQType llmqType auto& llmqParams = Params().GetConsensus().llmqs.at(llmqType); size_t poolSize = (size_t)llmqParams.signingActiveQuorumCount; - uint256 startBlock; + CBlockIndex* pindexStart; { LOCK(cs_main); int startBlockHeight = signHeight - SIGN_HEIGHT_OFFSET; if (startBlockHeight > chainActive.Height()) { return nullptr; } - startBlock = chainActive[startBlockHeight]->GetBlockHash(); + pindexStart = chainActive[startBlockHeight]; } - auto quorums = quorumManager->ScanQuorums(llmqType, startBlock, poolSize); + auto quorums = quorumManager->ScanQuorums(llmqType, pindexStart, poolSize); if (quorums.empty()) { return nullptr; } diff --git a/src/llmq/quorums_signing_shares.cpp b/src/llmq/quorums_signing_shares.cpp index 9215e7a78..f07e220bc 100644 --- a/src/llmq/quorums_signing_shares.cpp +++ b/src/llmq/quorums_signing_shares.cpp @@ -347,31 +347,26 @@ bool CSigSharesManager::PreVerifyBatchedSigShares(NodeId nodeId, const CBatchedS return false; } - CQuorumCPtr quorum; - { - LOCK(cs_main); - - quorum = quorumManager->GetQuorum(llmqType, batchedSigShares.quorumHash); - if (!quorum) { - // TODO should we ban here? - LogPrintf("CSigSharesManager::%s -- quorum %s not found, node=%d\n", __func__, - batchedSigShares.quorumHash.ToString(), nodeId); - return false; - } - if (!CLLMQUtils::IsQuorumActive(llmqType, quorum->quorumHash)) { - // quorum is too old - return false; - } - if (!quorum->IsMember(activeMasternodeInfo.proTxHash)) { - // we're not a member so we can't verify it (we actually shouldn't have received it) - return false; - } - if (quorum->quorumVvec == nullptr) { - // TODO we should allow to ask other nodes for the quorum vvec if we missed it in the DKG - LogPrintf("CSigSharesManager::%s -- we don't have the quorum vvec for %s, no verification possible. node=%d\n", __func__, - batchedSigShares.quorumHash.ToString(), nodeId); - return false; - } + CQuorumCPtr quorum = quorumManager->GetQuorum(llmqType, batchedSigShares.quorumHash); + if (!quorum) { + // TODO should we ban here? + LogPrintf("CSigSharesManager::%s -- quorum %s not found, node=%d\n", __func__, + batchedSigShares.quorumHash.ToString(), nodeId); + return false; + } + if (!CLLMQUtils::IsQuorumActive(llmqType, quorum->quorumHash)) { + // quorum is too old + return false; + } + if (!quorum->IsMember(activeMasternodeInfo.proTxHash)) { + // we're not a member so we can't verify it (we actually shouldn't have received it) + return false; + } + if (quorum->quorumVvec == nullptr) { + // TODO we should allow to ask other nodes for the quorum vvec if we missed it in the DKG + LogPrintf("CSigSharesManager::%s -- we don't have the quorum vvec for %s, no verification possible. node=%d\n", __func__, + batchedSigShares.quorumHash.ToString(), nodeId); + return false; } std::set dupMembers; @@ -994,17 +989,15 @@ void CSigSharesManager::Cleanup() } } - { - // Find quorums which became inactive - LOCK(cs_main); - for (auto it = quorumsToCheck.begin(); it != quorumsToCheck.end(); ) { - if (CLLMQUtils::IsQuorumActive(it->first, it->second)) { - it = quorumsToCheck.erase(it); - } else { - ++it; - } + // Find quorums which became inactive + for (auto it = quorumsToCheck.begin(); it != quorumsToCheck.end(); ) { + if (CLLMQUtils::IsQuorumActive(it->first, it->second)) { + it = quorumsToCheck.erase(it); + } else { + ++it; } } + { // Now delete sessions which are for inactive quorums LOCK(cs); diff --git a/src/llmq/quorums_utils.cpp b/src/llmq/quorums_utils.cpp index e35acc4f6..525606a5f 100644 --- a/src/llmq/quorums_utils.cpp +++ b/src/llmq/quorums_utils.cpp @@ -99,8 +99,6 @@ std::set CLLMQUtils::CalcDeterministicWatchConnections(Consensus::LLMQTy bool CLLMQUtils::IsQuorumActive(Consensus::LLMQType llmqType, const uint256& quorumHash) { - AssertLockHeld(cs_main); - auto& params = Params().GetConsensus().llmqs.at(llmqType); // sig shares and recovered sigs are only accepted from recent/active quorums diff --git a/src/rpc/rpcquorums.cpp b/src/rpc/rpcquorums.cpp index db63ea241..905a90d9c 100644 --- a/src/rpc/rpcquorums.cpp +++ b/src/rpc/rpcquorums.cpp @@ -38,7 +38,7 @@ UniValue quorum_list(const JSONRPCRequest& request) for (auto& p : Params().GetConsensus().llmqs) { UniValue v(UniValue::VARR); - auto quorums = llmq::quorumManager->ScanQuorums(p.first, chainActive.Tip()->GetBlockHash(), count); + auto quorums = llmq::quorumManager->ScanQuorums(p.first, chainActive.Tip(), count); for (auto& q : quorums) { v.push_back(q->quorumHash.ToString()); }