Skip to content

Commit

Permalink
feat: return enum in RecoveredSig verifying code, apply for RPC submi…
Browse files Browse the repository at this point in the history
…tchainlock

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.
  • Loading branch information
knst committed Jul 8, 2024
1 parent 130b6d1 commit 6004e06
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 23 deletions.
2 changes: 1 addition & 1 deletion src/evo/cbtx.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
9 changes: 6 additions & 3 deletions src/llmq/chainlocks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include <txmempool.h>
#include <util/thread.h>
#include <util/time.h>
#include <util/underlying.h>
#include <validation.h>
#include <validationinterface.h>

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

Expand Down
3 changes: 2 additions & 1 deletion src/llmq/chainlocks.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

#include <crypto/common.h>
#include <llmq/signing.h>
#include <llmq/quorums.h>
#include <net.h>
#include <net_types.h>
#include <primitives/block.h>
Expand Down Expand Up @@ -114,7 +115,7 @@ class CChainLocksHandler : public CRecoveredSigsListener

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

Expand Down
7 changes: 4 additions & 3 deletions src/llmq/quorums.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1169,18 +1169,19 @@ 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)
{
const auto& llmq_params_opt = Params().GetLLMQ(llmqType);
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
13 changes: 10 additions & 3 deletions src/llmq/quorums.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,13 @@ using CDeterministicMNCPtr = std::shared_ptr<const CDeterministicMN>;

namespace llmq
{
enum class VerifyRecSigStatus
{
NoQuorum,
Invalid,
Valid,
};

class CDKGSessionManager;
class CQuorumBlockProcessor;

Expand Down Expand Up @@ -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<typename T> struct SaltedHasherImpl;
Expand Down
38 changes: 26 additions & 12 deletions src/rpc/quorums.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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"));
Expand Down Expand Up @@ -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;
},
};
}
Expand Down Expand Up @@ -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);
},
};
}
Expand All @@ -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;

Expand All @@ -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");
}

Expand Down

0 comments on commit 6004e06

Please sign in to comment.