llmq|rpc|test: Fix "quorumHash" parameter of "quorum sign" (#3914)

* llmq: Add an optional quorum hash to CSigningManager::AsyncSignIfMember

Allows to select the quorum to sign by its hash.

* rpc: Fix quorum selection of "quorum sign"

* test: Test the optional "quorumHash" parameter of "quorum sign"

* llmq: Move quorum checks up to avoid calling WriteVoteForId if they fail
This commit is contained in:
dustinface 2021-01-11 18:17:41 +01:00 committed by GitHub
parent f3e8401da5
commit 0f62d55af6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 35 additions and 29 deletions

View File

@ -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());
}

View File

@ -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);

View File

@ -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);

View File

@ -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())) {

View File

@ -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