CLLMQUtils::IsQuorumActive() shouldn't require cs_main to be held

PIVX-Project · Apr 7, 2024 · bff6e6e · bff6e6e
1 parent fdba615
commit bff6e6e
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 73 deletions.
diff --git a/src/llmq/quorums_signing.cpp b/src/llmq/quorums_signing.cpp
@@ -257,19 +257,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 (!llmq::utils::IsQuorumActive(llmqType, quorum->pindexQuorum->GetBlockHash())) {
-            return false;
-        }
+    if (!quorum) {
+        LogPrintf("CSigningManager::%s -- quorum %s not found, node=%d\n", __func__,
+            recoveredSig.quorumHash.ToString(), nodeId);
+        return false;
+    }
+    if (!llmq::utils::IsQuorumActive(llmqType, quorum->pindexQuorum->GetBlockHash())) {
+        return false;
     }
 
     if (!recoveredSig.sig.IsValid()) {
@@ -312,37 +308,34 @@ void CSigningManager::CollectPendingRecoveredSigsToVerify(
         }
     }
 
-    {
-        LOCK(cs_main);
-        for (auto& p : retSigShares) {
-            NodeId nodeId = p.first;
-            auto& v = p.second;
-
-            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 (!llmq::utils::IsQuorumActive(llmqType, quorum->pindexQuorum->GetBlockHash())) {
-                        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);
+    for (auto& p : retSigShares) {
+        NodeId nodeId = p.first;
+        auto& v = p.second;
+
+        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 (!llmq::utils::IsQuorumActive(llmqType, quorum->pindexQuorum->GetBlockHash())) {
+                    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;
         }
     }
 }

diff --git a/src/llmq/quorums_signing_shares.cpp b/src/llmq/quorums_signing_shares.cpp
@@ -347,31 +347,27 @@ bool CSigSharesManager::PreVerifyBatchedSigShares(NodeId nodeId, const CBatchedS
         return false;
     }
 
-    CQuorumCPtr quorum;
-    {
-        LOCK(cs_main);
+    CQuorumCPtr quorum = quorumManager->GetQuorum(llmqType, batchedSigShares.quorumHash);
 
-        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 (!llmq::utils::IsQuorumActive(llmqType, quorum->pindexQuorum->GetBlockHash())) {
-            // quorum is too old
-            return false;
-        }
-        if (!quorum->IsMember(activeMasternodeManager->GetProTx())) {
-            // 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;
-        }
+    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 (!llmq::utils::IsQuorumActive(llmqType, quorum->pindexQuorum->GetBlockHash())) {
+        // quorum is too old
+        return false;
+    }
+    if (!quorum->IsMember(activeMasternodeManager->GetProTx())) {
+        // 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<uint16_t> dupMembers;
@@ -993,17 +989,15 @@ void CSigSharesManager::Cleanup()
         }
     }
 
-    {
-        // Find quorums which became inactive
-        LOCK(cs_main);
-        for (auto it = quorumsToCheck.begin(); it != quorumsToCheck.end();) {
-            if (llmq::utils::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 (llmq::utils::IsQuorumActive(it->first, it->second)) {
+            it = quorumsToCheck.erase(it);
+        } else {
+            ++it;
         }
     }
+
     {
         // Now delete sessions which are for inactive quorums
         LOCK(cs);