diff --git a/src/llmq/quorums_signing.cpp b/src/llmq/quorums_signing.cpp index 9918161be2..4a9852db25 100644 --- a/src/llmq/quorums_signing.cpp +++ b/src/llmq/quorums_signing.cpp @@ -930,9 +930,9 @@ CQuorumCPtr CSigningManager::SelectQuorumForSigning(Consensus::LLMQType llmqType return quorums[scores.front().second]; } -bool CSigningManager::VerifyRecoveredSig(Consensus::LLMQType llmqType, int signedAtHeight, const uint256& id, const uint256& msgHash, const CBLSSignature& sig) +bool CSigningManager::VerifyRecoveredSig(Consensus::LLMQType llmqType, int signedAtHeight, const uint256& id, const uint256& msgHash, const CBLSSignature& sig, const int signOffset) { - auto quorum = SelectQuorumForSigning(llmqType, id, signedAtHeight); + auto quorum = SelectQuorumForSigning(llmqType, id, signedAtHeight, signOffset); if (!quorum) { return false; } diff --git a/src/llmq/quorums_signing.h b/src/llmq/quorums_signing.h index ca5bfda58e..c135b86aaa 100644 --- a/src/llmq/quorums_signing.h +++ b/src/llmq/quorums_signing.h @@ -186,7 +186,7 @@ public: static 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 - static bool VerifyRecoveredSig(Consensus::LLMQType llmqType, int signedAtHeight, const uint256& id, const uint256& msgHash, const CBLSSignature& sig); + static bool VerifyRecoveredSig(Consensus::LLMQType llmqType, int signedAtHeight, const uint256& id, const uint256& msgHash, const CBLSSignature& sig, int signOffset = SIGN_HEIGHT_OFFSET); }; extern CSigningManager* quorumSigningManager; diff --git a/src/rpc/rpcquorums.cpp b/src/rpc/rpcquorums.cpp index 087d05cfbb..8caaa28a81 100644 --- a/src/rpc/rpcquorums.cpp +++ b/src/rpc/rpcquorums.cpp @@ -432,30 +432,26 @@ UniValue quorum_sigs_cmd(const JSONRPCRequest& request) throw JSONRPCError(RPC_INVALID_PARAMETER, "invalid signature format"); } - llmq::CQuorumCPtr quorum{nullptr}; if (request.params[5].isNull() || (request.params[5].get_str().empty() && !request.params[6].isNull())) { int signHeight{-1}; if (!request.params[6].isNull()) { signHeight = ParseInt32V(request.params[6], "signHeight"); } - // First check against the current active set - quorum = llmq::quorumSigningManager->SelectQuorumForSigning(llmqType, id, signHeight, 0); - if (!quorum) { - // Then check against the previous active set in case it changed recently - int signOffset = Params().GetConsensus().llmqs.at(llmqType).dkgInterval; - quorum = llmq::quorumSigningManager->SelectQuorumForSigning(llmqType, id, signHeight, signOffset); - } + // First check against the current active set, if it fails check against the last active set + int signOffset{Params().GetConsensus().llmqs.at(llmqType).dkgInterval}; + return llmq::quorumSigningManager->VerifyRecoveredSig(llmqType, signHeight, id, msgHash, sig, 0) || + llmq::quorumSigningManager->VerifyRecoveredSig(llmqType, signHeight, id, msgHash, sig, signOffset); } else { uint256 quorumHash = ParseHashV(request.params[5], "quorumHash"); - quorum = llmq::quorumManager->GetQuorum(llmqType, quorumHash); - } + llmq::CQuorumCPtr quorum = llmq::quorumManager->GetQuorum(llmqType, quorumHash); - if (!quorum) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "quorum not found"); - } + if (!quorum) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "quorum not found"); + } - uint256 signHash = llmq::CLLMQUtils::BuildSignHash(llmqType, quorum->qc.quorumHash, id, msgHash); - return sig.VerifyInsecure(quorum->qc.quorumPublicKey, signHash); + uint256 signHash = llmq::CLLMQUtils::BuildSignHash(llmqType, quorum->qc.quorumHash, id, msgHash); + return sig.VerifyInsecure(quorum->qc.quorumPublicKey, signHash); + } } else if (cmd == "hasrecsig") { return llmq::quorumSigningManager->HasRecoveredSig(llmqType, id, msgHash); } else if (cmd == "getrecsig") { @@ -710,21 +706,12 @@ UniValue verifyislock(const JSONRPCRequest& request) signHeight = pindexMined->nHeight; } - // First check against the current active set auto llmqType = Params().GetConsensus().llmqTypeInstantSend; - auto quorum = llmq::quorumSigningManager->SelectQuorumForSigning(llmqType, id, signHeight, 0); - if (!quorum) { - // Then check against the previous active set in case it changed recently - int signOffset = Params().GetConsensus().llmqs.at(llmqType).dkgInterval; - quorum = llmq::quorumSigningManager->SelectQuorumForSigning(llmqType, id, signHeight, signOffset); - if (!quorum) { - // None of the above - throw JSONRPCError(RPC_INVALID_PARAMETER, "quorum not found"); - } - } - uint256 signHash = llmq::CLLMQUtils::BuildSignHash(llmqType, quorum->qc.quorumHash, id, txid); - return sig.VerifyInsecure(quorum->qc.quorumPublicKey, signHash); + // First check against the current active set, if it fails check against the last active set + int signOffset{Params().GetConsensus().llmqs.at(llmqType).dkgInterval}; + return llmq::quorumSigningManager->VerifyRecoveredSig(llmqType, signHeight, id, txid, sig, 0) || + llmq::quorumSigningManager->VerifyRecoveredSig(llmqType, signHeight, id, txid, sig, signOffset); } static const CRPCCommand commands[] = diff --git a/test/functional/feature_llmq_signing.py b/test/functional/feature_llmq_signing.py index 917abac01d..9106d96887 100755 --- a/test/functional/feature_llmq_signing.py +++ b/test/functional/feature_llmq_signing.py @@ -119,18 +119,39 @@ class LLMQSigningTest(DashTestFramework): assert(node.quorum("verify", 100, id, msgHash, recsig["sig"])) assert(node.quorum("verify", 100, id, msgHash, recsig["sig"], "", height)) assert(not node.quorum("verify", 100, id, msgHashConflict, recsig["sig"])) - assert_raises_rpc_error(-8, "quorum not found", node.quorum, "verify", 100, id, msgHash, recsig["sig"], "", height_bad) + assert not node.quorum("verify", 100, id, msgHash, recsig["sig"], "", height_bad) # Use specifc quorum assert(node.quorum("verify", 100, id, msgHash, recsig["sig"], recsig["quorumHash"])) assert(not node.quorum("verify", 100, id, msgHashConflict, recsig["sig"], recsig["quorumHash"])) assert_raises_rpc_error(-8, "quorum not found", node.quorum, "verify", 100, id, msgHash, recsig["sig"], hash_bad) - recsig_time = self.mocktime - # Mine one more quorum, so that we have 2 active ones, nothing should change self.mine_quorum() assert_sigs_nochange(True, False, True, 3) + # Create a recovered sig for the oldest quorum i.e. the active quorum which will be moved + # out of the active set when a new quorum appears + request_id = 2 + oldest_quorum_hash = node.quorum("list")["llmq_test"][-1] + # Search for a request id which selects the last active quorum + while True: + selected_hash = node.quorum('selectquorum', 100, uint256_to_string(request_id))["quorumHash"] + if selected_hash == oldest_quorum_hash: + break + else: + request_id += 1 + # Produce the recovered signature + id = uint256_to_string(request_id) + for mn in self.mninfo: + mn.node.quorum("sign", 100, id, msgHash) + # And mine a quorum to move the quorum which signed out of the active set + self.mine_quorum() + # Verify the recovered sig. This triggers the "signHeight + dkgInterval" verification + recsig = node.quorum("getrecsig", 100, id, msgHash) + assert node.quorum("verify", 100, id, msgHash, recsig["sig"], "", node.getblockcount()) + + recsig_time = self.mocktime + # Mine 2 more quorums, so that the one used for the the recovered sig should become inactive, nothing should change self.mine_quorum() self.mine_quorum() @@ -152,7 +173,7 @@ class LLMQSigningTest(DashTestFramework): wait_for_sigs(True, False, True, 15) if self.options.spork21: - id = "0000000000000000000000000000000000000000000000000000000000000002" + id = uint256_to_string(request_id + 1) # Isolate the node that is responsible for the recovery of a signature and assert that recovery fails q = self.nodes[0].quorum('selectquorum', 100, id) diff --git a/test/functional/rpc_verifyislock.py b/test/functional/rpc_verifyislock.py index bdbc9b152c..b7e34b5d30 100755 --- a/test/functional/rpc_verifyislock.py +++ b/test/functional/rpc_verifyislock.py @@ -5,7 +5,7 @@ from test_framework.messages import CTransaction, FromHex, hash256, ser_compact_size, ser_string from test_framework.test_framework import DashTestFramework -from test_framework.util import assert_raises_rpc_error, wait_until +from test_framework.util import bytes_to_hex_str, satoshi_round, wait_until ''' rpc_verifyislock.py @@ -20,6 +20,14 @@ class RPCVerifyISLockTest(DashTestFramework): self.set_dash_test_params(6, 5, [["-whitelist=127.0.0.1"], [], [], [], [], []], fast_dip3_enforcement=True) self.set_dash_llmq_test_params(5, 3) + def get_request_id(self, tx_hex): + tx = FromHex(CTransaction(), tx_hex) + + request_id_buf = ser_string(b"islock") + ser_compact_size(len(tx.vin)) + for txin in tx.vin: + request_id_buf += txin.prevout.serialize() + return hash256(request_id_buf)[::-1].hex() + def run_test(self): node = self.nodes[0] @@ -31,26 +39,55 @@ class RPCVerifyISLockTest(DashTestFramework): txid = node.sendtoaddress(node.getnewaddress(), 1) self.wait_for_instantlock(txid, node) - tx = FromHex(CTransaction(), node.getrawtransaction(txid)) - - request_id_buf = ser_string(b"islock") + ser_compact_size(len(tx.vin)) - for txin in tx.vin: - request_id_buf += txin.prevout.serialize() - request_id = hash256(request_id_buf)[::-1].hex() - + request_id = self.get_request_id(self.nodes[0].getrawtransaction(txid)) wait_until(lambda: node.quorum("hasrecsig", 100, request_id, txid)) rec_sig = node.quorum("getrecsig", 100, request_id, txid)['sig'] assert(node.verifyislock(request_id, txid, rec_sig)) # Not mined, should use maxHeight - assert_raises_rpc_error(-8, "quorum not found", node.verifyislock, request_id, txid, rec_sig, 1) + assert not node.verifyislock(request_id, txid, rec_sig, 1) node.generate(1) assert(txid not in node.getrawmempool()) # Mined but at higher height, should use maxHeight - assert_raises_rpc_error(-8, "quorum not found", node.verifyislock, request_id, txid, rec_sig, 1) + assert not node.verifyislock(request_id, txid, rec_sig, 1) # Mined, should ignore higher maxHeight assert(node.verifyislock(request_id, txid, rec_sig, node.getblockcount() + 100)) + # Mine one more quorum to have a full active set + self.mine_quorum() + # Create an ISLOCK for the oldest quorum i.e. the active quorum which will be moved + # out of the active set when a new quorum appears + selected_hash = None + request_id = None + oldest_quorum_hash = node.quorum("list")["llmq_test"][-1] + utxos = node.listunspent() + fee = 0.001 + amount = 1 + # Try all available utxo's until we have one resulting in a request id which selects the + # last active quorum + for utxo in utxos: + in_amount = float(utxo['amount']) + if in_amount < amount + fee: + continue + outputs = dict() + outputs[node.getnewaddress()] = satoshi_round(amount) + change = in_amount - amount - fee + if change > 0: + outputs[node.getnewaddress()] = satoshi_round(change) + rawtx = node.createrawtransaction([utxo], outputs) + rawtx = node.signrawtransactionwithwallet(rawtx)["hex"] + request_id = self.get_request_id(rawtx) + selected_hash = node.quorum('selectquorum', 100, request_id)["quorumHash"] + if selected_hash == oldest_quorum_hash: + break + assert selected_hash == oldest_quorum_hash + # Create the ISLOCK, then mine a quorum to move the signing quorum out of the active set + islock = self.create_islock(rawtx) + self.mine_quorum() + # Send the tx and verify the ISLOCK. This triggers the "signHeight + dkgInterval" verification + rawtx_txid = node.sendrawtransaction(rawtx) + assert node.verifyislock(request_id, rawtx_txid, bytes_to_hex_str(islock.sig), node.getblockcount()) + if __name__ == '__main__': RPCVerifyISLockTest().main()