diff --git a/src/llmq/quorums_instantsend.cpp b/src/llmq/quorums_instantsend.cpp index 84ca2ee03b..4a65c82f97 100644 --- a/src/llmq/quorums_instantsend.cpp +++ b/src/llmq/quorums_instantsend.cpp @@ -464,7 +464,7 @@ bool CInstantSendManager::ProcessTx(const CTransaction& tx, bool allowReSigning, } LogPrint(BCLog::INSTANTSEND, "CInstantSendManager::%s -- txid=%s: trying to vote on input %s with id %s. allowReSigning=%d\n", __func__, tx.GetHash().ToString(), in.prevout.ToStringShort(), id.ToString(), allowReSigning); - if (quorumSigningManager->AsyncSignIfMember(llmqType, id, tx.GetHash(), allowReSigning)) { + if (quorumSigningManager->AsyncSignIfMember(llmqType, id, tx.GetHash(), {}, allowReSigning)) { LogPrint(BCLog::INSTANTSEND, "CInstantSendManager::%s -- txid=%s: voted on input %s with id %s\n", __func__, tx.GetHash().ToString(), in.prevout.ToStringShort(), id.ToString()); } diff --git a/src/llmq/quorums_signing.cpp b/src/llmq/quorums_signing.cpp index d255f1d084..8bad9ac2e0 100644 --- a/src/llmq/quorums_signing.cpp +++ b/src/llmq/quorums_signing.cpp @@ -777,12 +777,33 @@ void CSigningManager::UnregisterRecoveredSigsListener(CRecoveredSigsListener* l) recoveredSigsListeners.erase(itRem, recoveredSigsListeners.end()); } -bool CSigningManager::AsyncSignIfMember(Consensus::LLMQType llmqType, const uint256& id, const uint256& msgHash, bool allowReSign) +bool CSigningManager::AsyncSignIfMember(Consensus::LLMQType llmqType, const uint256& id, const uint256& msgHash, const uint256& quorumHash, bool allowReSign) { if (!fMasternodeMode || activeMasternodeInfo.proTxHash.IsNull()) { return false; } + CQuorumCPtr quorum; + if (quorumHash.IsNull()) { + // 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 + quorum = SelectQuorumForSigning(llmqType, id); + } else { + quorum = quorumManager->GetQuorum(llmqType, quorumHash); + } + + if (!quorum) { + LogPrint(BCLog::LLMQ, "CSigningManager::%s -- failed to select quorum. id=%s, msgHash=%s\n", __func__, id.ToString(), msgHash.ToString()); + return false; + } + + if (!quorum->IsValidMember(activeMasternodeInfo.proTxHash)) { + return false; + } + { LOCK(cs); @@ -813,21 +834,6 @@ bool CSigningManager::AsyncSignIfMember(Consensus::LLMQType llmqType, const uint } } - // 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, id); - if (!quorum) { - LogPrint(BCLog::LLMQ, "CSigningManager::%s -- failed to select quorum. id=%s, msgHash=%s\n", __func__, id.ToString(), msgHash.ToString()); - return false; - } - - if (!quorum->IsValidMember(activeMasternodeInfo.proTxHash)) { - return false; - } - if (allowReSign) { // make us re-announce all known shares (other nodes might have run into a timeout) quorumSigSharesManager->ForceReAnnouncement(quorum, llmqType, id, msgHash); diff --git a/src/llmq/quorums_signing.h b/src/llmq/quorums_signing.h index 68901f6f35..dce1828a14 100644 --- a/src/llmq/quorums_signing.h +++ b/src/llmq/quorums_signing.h @@ -170,7 +170,7 @@ public: void RegisterRecoveredSigsListener(CRecoveredSigsListener* l); void UnregisterRecoveredSigsListener(CRecoveredSigsListener* l); - bool AsyncSignIfMember(Consensus::LLMQType llmqType, const uint256& id, const uint256& msgHash, bool allowReSign = false); + bool AsyncSignIfMember(Consensus::LLMQType llmqType, const uint256& id, const uint256& msgHash, const uint256& quorumHash = uint256(), bool allowReSign = false); bool HasRecoveredSig(Consensus::LLMQType llmqType, const uint256& id, const uint256& msgHash); bool HasRecoveredSigForId(Consensus::LLMQType llmqType, const uint256& id); bool HasRecoveredSigForSession(const uint256& signHash); diff --git a/src/rpc/rpcquorums.cpp b/src/rpc/rpcquorums.cpp index ff74f9ca74..9498d4f59d 100644 --- a/src/rpc/rpcquorums.cpp +++ b/src/rpc/rpcquorums.cpp @@ -383,17 +383,11 @@ UniValue quorum_sigs_cmd(const JSONRPCRequest& request) uint256 msgHash = ParseHashV(request.params[3], "msgHash"); if (cmd == "sign") { + uint256 quorumHash; if (!request.params[4].isNull()) { - uint256 quorumHash = ParseHashV(request.params[4], "quorumHash"); - auto quorum = llmq::quorumManager->GetQuorum(llmqType, quorumHash); - if (!quorum) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "quorum not found"); - } - llmq::quorumSigSharesManager->AsyncSign(quorum, id, msgHash); - return true; - } else { - return llmq::quorumSigningManager->AsyncSignIfMember(llmqType, id, msgHash); + quorumHash = ParseHashV(request.params[4], "quorumHash"); } + return llmq::quorumSigningManager->AsyncSignIfMember(llmqType, id, msgHash, quorumHash); } else if (cmd == "verify") { CBLSSignature sig; if (!sig.SetHexStr(request.params[4].get_str())) { diff --git a/test/functional/feature_llmq_signing.py b/test/functional/feature_llmq_signing.py index d0636c252b..b3d7468d2e 100755 --- a/test/functional/feature_llmq_signing.py +++ b/test/functional/feature_llmq_signing.py @@ -60,8 +60,14 @@ class LLMQSigningTest(DashTestFramework): self.mninfo[i].node.quorum("sign", 100, id, msgHash) assert_sigs_nochange(False, False, False, 3) - # Sign one more share, should result in recovered sig and conflict for msgHashConflict - self.mninfo[2].node.quorum("sign", 100, id, msgHash) + # Sign one more share and test optional quorumHash parameter. + # Should result in recovered sig and conflict for msgHashConflict + # 1. Providing an invalid quorum hash should fail and cause no changes for sigs + assert(not self.mninfo[2].node.quorum("sign", 100, id, msgHash, msgHash)) + assert_sigs_nochange(False, False, False, 3) + # 2. Providing a valid quorum hash should succeed and finally result in the recovered sig + quorumHash = self.mninfo[2].node.quorum("selectquorum", 100, id)["quorumHash"] + assert(self.mninfo[2].node.quorum("sign", 100, id, msgHash, quorumHash)) wait_for_sigs(True, False, True, 15) # Test `quorum verify` rpc