Fix two potential issues in the way pending islocks are processed (dashpay#3678)

* Always check for previous quorum set in llmq IS

* Refactor SelectQuorumForSigning and related code

Should have no changes in behaviour

* Do not use SIGN_HEIGHT_OFFSET when checking pending IS locks, use actual chain tip

This commit actually changes the behaviour

Author: UdjinM6

src/llmq/quorums_signing.cpp

@@ -764,18 +764,12 @@ bool CSigningManager::AsyncSignIfMember(Consensus::LLMQType llmqType, const uint
         }
     }
 
-    int tipHeight;
-    {
-        LOCK(cs_main);
-        tipHeight = chainActive.Height();
-    }
-
     // This might end up giving different results on different members
     // This might happen when we are on the brink of confirming a new quorum
     // This gives a slight risk of not getting enough shares to recover a signature
     // But at least it shouldn't be possible to get conflicting recovered signatures
     // 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
-    CQuorumCPtr quorum = SelectQuorumForSigning(llmqType, tipHeight, id);
+    CQuorumCPtr quorum = SelectQuorumForSigning(llmqType, id);
     if (!quorum) {
         LogPrint(BCLog::LLMQ, "CSigningManager::%s -- failed to select quorum. id=%s, msgHash=%s\n", __func__, id.ToString(), msgHash.ToString());
         return false;
@@ -835,27 +829,25 @@ bool CSigningManager::GetVoteForId(Consensus::LLMQType llmqType, const uint256&
     return db.GetVoteForId(llmqType, id, msgHashRet);
 }
 
-std::vector<CQuorumCPtr> CSigningManager::GetActiveQuorumSet(Consensus::LLMQType llmqType, int signHeight)
+CQuorumCPtr CSigningManager::SelectQuorumForSigning(Consensus::LLMQType llmqType, const uint256& selectionHash, int signHeight, int signOffset)
 {
     auto& llmqParams = Params().GetConsensus().llmqs.at(llmqType);
     size_t poolSize = (size_t)llmqParams.signingActiveQuorumCount;
 
     CBlockIndex* pindexStart;
     {
         LOCK(cs_main);
-        int startBlockHeight = signHeight - SIGN_HEIGHT_OFFSET;
+        if (signHeight == -1) {
+            signHeight = chainActive.Height();
+        }
+        int startBlockHeight = signHeight - signOffset;
         if (startBlockHeight > chainActive.Height()) {
             return {};
         }
         pindexStart = chainActive[startBlockHeight];
     }
 
-    return quorumManager->ScanQuorums(llmqType, pindexStart, poolSize);
-}
-
-CQuorumCPtr CSigningManager::SelectQuorumForSigning(Consensus::LLMQType llmqType, int signHeight, const uint256& selectionHash)
-{
-    auto quorums = GetActiveQuorumSet(llmqType, signHeight);
+    auto quorums =  quorumManager->ScanQuorums(llmqType, pindexStart, poolSize);
     if (quorums.empty()) {
         return nullptr;
     }
@@ -875,7 +867,7 @@ CQuorumCPtr CSigningManager::SelectQuorumForSigning(Consensus::LLMQType llmqType
 
 bool CSigningManager::VerifyRecoveredSig(Consensus::LLMQType llmqType, int signedAtHeight, const uint256& id, const uint256& msgHash, const CBLSSignature& sig)
 {
-    auto quorum = SelectQuorumForSigning(llmqType, signedAtHeight, id);
+    auto quorum = SelectQuorumForSigning(llmqType, id, signedAtHeight);
     if (!quorum) {
         return false;
     }

src/llmq/quorums_signing.h

@@ -165,7 +165,7 @@ class CSigningManager
     bool GetVoteForId(Consensus::LLMQType llmqType, const uint256& id, uint256& msgHashRet);
 
     std::vector<CQuorumCPtr> GetActiveQuorumSet(Consensus::LLMQType llmqType, int signHeight);
-    CQuorumCPtr SelectQuorumForSigning(Consensus::LLMQType llmqType, int signHeight, const uint256& selectionHash);
+    CQuorumCPtr SelectQuorumForSigning(Consensus::LLMQType llmqType, 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
     bool VerifyRecoveredSig(Consensus::LLMQType llmqType, int signedAtHeight, const uint256& id, const uint256& msgHash, const CBLSSignature& sig);
 };
@@ -174,4 +174,4 @@ extern std::unique_ptr<CSigningManager> quorumSigningManager;
 
 } // namespace llmq
 
-#endif // PIVX_LLMQ_QUORUMS_SIGNING_H
+#endif // PIVX_LLMQ_QUORUMS_SIGNING_H

src/rpc/rpcquorums.cpp

@@ -362,15 +362,9 @@ UniValue quorumselectquorum(const JSONRPCRequest& request)
 
     uint256 id = ParseHashV(request.params[1], "id");
 
-    int tipHeight;
-    {
-        LOCK(cs_main);
-        tipHeight = chainActive.Height();
-    }
-
     UniValue ret(UniValue::VOBJ);
 
-    auto quorum = llmq::quorumSigningManager->SelectQuorumForSigning(llmqType, tipHeight, id);
+    auto quorum = llmq::quorumSigningManager->SelectQuorumForSigning(llmqType, id);
     if (!quorum) {
         throw JSONRPCError(RPC_MISC_ERROR, "no quorums active");
     }