From 6004e067693edc1118ae5748d67eedf5ed914dd0 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Fri, 5 Jul 2024 03:45:14 +0700 Subject: [PATCH] feat: return enum in RecoveredSig verifying code, apply for RPC submitchainlock It helps to detect case of Tip is way too far behind: the signature can be still valid but core can't verify it yet but later it may become a valid signature. --- src/evo/cbtx.cpp | 2 +- src/llmq/chainlocks.cpp | 9 ++++++--- src/llmq/chainlocks.h | 3 ++- src/llmq/quorums.cpp | 7 ++++--- src/llmq/quorums.h | 13 ++++++++++--- src/rpc/quorums.cpp | 38 ++++++++++++++++++++++++++------------ 6 files changed, 49 insertions(+), 23 deletions(-) diff --git a/src/evo/cbtx.cpp b/src/evo/cbtx.cpp index 6f4c171b21..29a9999965 100644 --- a/src/evo/cbtx.cpp +++ b/src/evo/cbtx.cpp @@ -374,7 +374,7 @@ bool CheckCbTxBestChainlock(const CBlock& block, const CBlockIndex* pindex, return true; } uint256 curBlockCoinbaseCLBlockHash = pindex->GetAncestor(curBlockCoinbaseCLHeight)->GetBlockHash(); - if (!chainlock_handler.VerifyChainLock(llmq::CChainLockSig(curBlockCoinbaseCLHeight, curBlockCoinbaseCLBlockHash, opt_cbTx->bestCLSignature))) { + if (chainlock_handler.VerifyChainLock(llmq::CChainLockSig(curBlockCoinbaseCLHeight, curBlockCoinbaseCLBlockHash, opt_cbTx->bestCLSignature)) != llmq::VerifyRecSigStatus::Valid) { return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-invalid-clsig"); } } else if (opt_cbTx->bestCLHeightDiff != 0) { diff --git a/src/llmq/chainlocks.cpp b/src/llmq/chainlocks.cpp index eaea589290..39130b5a07 100644 --- a/src/llmq/chainlocks.cpp +++ b/src/llmq/chainlocks.cpp @@ -19,6 +19,7 @@ #include #include #include +#include #include #include @@ -130,8 +131,8 @@ PeerMsgRet CChainLocksHandler::ProcessNewChainLock(const NodeId from, const llmq } } - if (!VerifyChainLock(clsig)) { - LogPrint(BCLog::CHAINLOCKS, "CChainLocksHandler::%s -- invalid CLSIG (%s), peer=%d\n", __func__, clsig.ToString(), from); + if (const auto ret = VerifyChainLock(clsig); ret != VerifyRecSigStatus::Valid) { + LogPrint(BCLog::CHAINLOCKS, "CChainLocksHandler::%s -- invalid CLSIG (%s), status=%d peer=%d\n", __func__, clsig.ToString(), ToUnderlying(ret), from); if (from != -1) { return tl::unexpected{10}; } @@ -551,10 +552,12 @@ bool CChainLocksHandler::HasChainLock(int nHeight, const uint256& blockHash) con return InternalHasChainLock(nHeight, blockHash); } -bool CChainLocksHandler::VerifyChainLock(const CChainLockSig& clsig) const + +VerifyRecSigStatus CChainLocksHandler::VerifyChainLock(const CChainLockSig& clsig) const { const auto llmqType = Params().GetConsensus().llmqTypeChainLocks; const uint256 nRequestId = ::SerializeHash(std::make_pair(llmq::CLSIG_REQUESTID_PREFIX, clsig.getHeight())); + return llmq::VerifyRecoveredSig(llmqType, m_chainstate.m_chain, qman, clsig.getHeight(), nRequestId, clsig.getBlockHash(), clsig.getSig()); } diff --git a/src/llmq/chainlocks.h b/src/llmq/chainlocks.h index a21cc342e7..f229f5b608 100644 --- a/src/llmq/chainlocks.h +++ b/src/llmq/chainlocks.h @@ -9,6 +9,7 @@ #include #include +#include #include #include #include @@ -114,7 +115,7 @@ public: bool HasChainLock(int nHeight, const uint256& blockHash) const EXCLUSIVE_LOCKS_REQUIRED(!cs); bool HasConflictingChainLock(int nHeight, const uint256& blockHash) const EXCLUSIVE_LOCKS_REQUIRED(!cs); - bool VerifyChainLock(const CChainLockSig& clsig) const; + VerifyRecSigStatus VerifyChainLock(const CChainLockSig& clsig) const; bool IsTxSafeForMining(const uint256& txid) const EXCLUSIVE_LOCKS_REQUIRED(!cs); diff --git a/src/llmq/quorums.cpp b/src/llmq/quorums.cpp index d2607f74fe..3f729aa1e4 100644 --- a/src/llmq/quorums.cpp +++ b/src/llmq/quorums.cpp @@ -1169,7 +1169,7 @@ CQuorumCPtr SelectQuorumForSigning(const Consensus::LLMQParams& llmq_params, con } } -bool VerifyRecoveredSig(Consensus::LLMQType llmqType, const CChain& active_chain, const CQuorumManager& qman, +VerifyRecSigStatus VerifyRecoveredSig(Consensus::LLMQType llmqType, const CChain& active_chain, const CQuorumManager& qman, int signedAtHeight, const uint256& id, const uint256& msgHash, const CBLSSignature& sig, const int signOffset) { @@ -1177,10 +1177,11 @@ bool VerifyRecoveredSig(Consensus::LLMQType llmqType, const CChain& active_chain assert(llmq_params_opt.has_value()); auto quorum = SelectQuorumForSigning(llmq_params_opt.value(), active_chain, qman, id, signedAtHeight, signOffset); if (!quorum) { - return false; + return VerifyRecSigStatus::NoQuorum; } uint256 signHash = BuildSignHash(llmqType, quorum->qc->quorumHash, id, msgHash); - return sig.VerifyInsecure(quorum->qc->quorumPublicKey, signHash); + const bool ret = sig.VerifyInsecure(quorum->qc->quorumPublicKey, signHash); + return ret ? VerifyRecSigStatus::Valid : VerifyRecSigStatus::Invalid; } } // namespace llmq diff --git a/src/llmq/quorums.h b/src/llmq/quorums.h index dcf0bcf906..91b5eceb55 100644 --- a/src/llmq/quorums.h +++ b/src/llmq/quorums.h @@ -36,6 +36,13 @@ using CDeterministicMNCPtr = std::shared_ptr; namespace llmq { +enum class VerifyRecSigStatus +{ + NoQuorum, + Invalid, + Valid, +}; + class CDKGSessionManager; class CQuorumBlockProcessor; @@ -298,9 +305,9 @@ CQuorumCPtr SelectQuorumForSigning(const Consensus::LLMQParams& llmq_params, con 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, const CChain& active_chain, const CQuorumManager& qman, - int signedAtHeight, const uint256& id, const uint256& msgHash, const CBLSSignature& sig, - int signOffset = SIGN_HEIGHT_OFFSET); +VerifyRecSigStatus VerifyRecoveredSig(Consensus::LLMQType llmqType, const CChain& active_chain, const CQuorumManager& qman, + int signedAtHeight, const uint256& id, const uint256& msgHash, const CBLSSignature& sig, + int signOffset = SIGN_HEIGHT_OFFSET); } // namespace llmq template struct SaltedHasherImpl; diff --git a/src/rpc/quorums.cpp b/src/rpc/quorums.cpp index a45cfa808f..7b523541ba 100644 --- a/src/rpc/quorums.cpp +++ b/src/rpc/quorums.cpp @@ -500,6 +500,18 @@ static RPCHelpMan quorum_sign() }; } +static bool VerifyRecoveredSigLatestQuorums(const Consensus::LLMQParams& llmq_params, const CChain& active_chain, const llmq::CQuorumManager& qman, + int signHeight, const uint256& id, const uint256& msgHash, const CBLSSignature& sig) +{ + // First check against the current active set, if it fails check against the last active set + for (int signOffset : {0, llmq_params.dkgInterval}) { + if (llmq::VerifyRecoveredSig(llmq_params.type, active_chain, qman, signHeight, id, msgHash, sig, signOffset) == llmq::VerifyRecSigStatus::Valid) { + return true; + } + } + return false; +} + static RPCHelpMan quorum_verify() { return RPCHelpMan{"quorum verify", @@ -545,10 +557,7 @@ static RPCHelpMan quorum_verify() if (!request.params[5].isNull()) { signHeight = ParseInt32V(request.params[5], "signHeight"); } - // First check against the current active set, if it fails check against the last active set - int signOffset{llmq_params_opt->dkgInterval}; - return llmq::VerifyRecoveredSig(llmqType, chainman.ActiveChain(), *llmq_ctx.qman, signHeight, id, msgHash, sig, 0) || - llmq::VerifyRecoveredSig(llmqType, chainman.ActiveChain(), *llmq_ctx.qman, signHeight, id, msgHash, sig, signOffset); + return VerifyRecoveredSigLatestQuorums(*llmq_params_opt, chainman.ActiveChain(), *llmq_ctx.qman, signHeight, id, msgHash, sig); } uint256 quorumHash(ParseHashV(request.params[4], "quorumHash")); @@ -958,7 +967,7 @@ static RPCHelpMan verifychainlock() } const LLMQContext& llmq_ctx = EnsureLLMQContext(node); - return llmq_ctx.clhandler->VerifyChainLock(llmq::CChainLockSig(nBlockHeight, nBlockHash, sig)); + return llmq_ctx.clhandler->VerifyChainLock(llmq::CChainLockSig(nBlockHeight, nBlockHash, sig)) == llmq::VerifyRecSigStatus::Valid; }, }; } @@ -1030,12 +1039,9 @@ static RPCHelpMan verifyislock() const LLMQContext& llmq_ctx = EnsureLLMQContext(node); auto llmqType = Params().GetConsensus().llmqTypeDIP0024InstantSend; - // First check against the current active set, if it fails check against the last active set const auto llmq_params_opt = Params().GetLLMQ(llmqType); CHECK_NONFATAL(llmq_params_opt.has_value()); - const int signOffset{llmq_params_opt->dkgInterval}; - return llmq::VerifyRecoveredSig(llmqType, chainman.ActiveChain(), *llmq_ctx.qman, signHeight, id, txid, sig, 0) || - llmq::VerifyRecoveredSig(llmqType, chainman.ActiveChain(), *llmq_ctx.qman, signHeight, id, txid, sig, signOffset); + return VerifyRecoveredSigLatestQuorums(*llmq_params_opt, chainman.ActiveChain(), *llmq_ctx.qman, signHeight, id, txid, sig); }, }; } @@ -1060,7 +1066,8 @@ static RPCHelpMan submitchainlock() if (nBlockHeight <= 0) { throw JSONRPCError(RPC_INVALID_PARAMETER, "invalid block height"); } - const LLMQContext& llmq_ctx = EnsureLLMQContext(EnsureAnyNodeContext(request.context)); + const NodeContext& node = EnsureAnyNodeContext(request.context); + const LLMQContext& llmq_ctx = EnsureLLMQContext(node); const int32_t bestCLHeight = llmq_ctx.clhandler->GetBestChainLock().getHeight(); if (nBlockHeight <= bestCLHeight) return bestCLHeight; @@ -1070,8 +1077,15 @@ static RPCHelpMan submitchainlock() } - auto clsig = llmq::CChainLockSig(nBlockHeight, nBlockHash, sig); - if (!llmq_ctx.clhandler->VerifyChainLock(clsig)) { + const auto clsig{llmq::CChainLockSig(nBlockHeight, nBlockHash, sig)}; + const llmq::VerifyRecSigStatus ret{llmq_ctx.clhandler->VerifyChainLock(clsig)}; + if (ret == llmq::VerifyRecSigStatus::NoQuorum) { + LOCK(cs_main); + const ChainstateManager& chainman = EnsureChainman(node); + const CBlockIndex* pIndex{chainman.ActiveChain().Tip()}; + throw JSONRPCError(RPC_MISC_ERROR, strprintf("No quorum found. Current tip height: %d hash: %s\n", pIndex->nHeight, pIndex->GetBlockHash().ToString())); + } + if (ret != llmq::VerifyRecSigStatus::Valid) { throw JSONRPCError(RPC_INVALID_PARAMETER, "invalid signature"); }