From dcdc0fa4da2c58c28137dd9b0136a765fc014c0d Mon Sep 17 00:00:00 2001 From: pasta Date: Sun, 2 Aug 2026 20:14:05 -0500 Subject: [PATCH 1/2] perf(llmq): check quorum activity before materializing the quorum on QSIGREC VerifyAndProcessRecoveredSig called GetQuorum before IsQuorumActive. GetQuorum takes the peer-supplied quorum hash and rebuilds arbitrary historical mined commitments (DMN list replay plus member selection) on a cache miss, whereas IsQuorumActive is bounded to the keepOldConnections most recent quorums at the tip. An unsolicited QSIGREC naming an inactive quorum hash therefore forced the expensive path before the cheap gate could reject it. Swap the order so the cheap gate runs first. Once IsQuorumActive passes, the hash is one of those recent quorums, so the subsequent GetQuorum is usually a cache hit and at worst rebuilds a recent quorum rather than an arbitrary historical one. A null quorum there is no longer peer-controlled, so it is logged without a misbehaviour score. --- src/llmq/signing.cpp | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/src/llmq/signing.cpp b/src/llmq/signing.cpp index efdcd13cd747..344e1dd38117 100644 --- a/src/llmq/signing.cpp +++ b/src/llmq/signing.cpp @@ -366,14 +366,15 @@ bool CSigningManager::GetRecoveredSigForGetData(const uint256& hash, CRecoveredS void CSigningManager::VerifyAndProcessRecoveredSig(NodeId from, std::shared_ptr recoveredSig) { auto llmq_type = recoveredSig->getLlmqType(); - auto quorum = qman.GetQuorum(llmq_type, recoveredSig->getQuorumHash()); - - if (!quorum) { - LogPrint(BCLog::LLMQ, "CSigningManager::%s -- quorum %s not found\n", __func__, - recoveredSig->getQuorumHash().ToString()); - return; - } - if (!IsQuorumActive(llmq_type, qman, quorum->qc->quorumHash)) { + const uint256& quorum_hash = recoveredSig->getQuorumHash(); + + // Cheap gate first. IsQuorumActive is bounded to the keepOldConnections most recent + // quorums at the tip, and that set is shared and cached across callers. GetQuorum, by + // contrast, takes the peer-supplied hash and can rebuild an arbitrary historical mined + // commitment (DMN list replay + member selection) on a cache miss — do not let an + // unsolicited QSIGREC force that work for inactive hashes. + // Caller (NetSigning) has already rejected unknown llmq types. + if (!IsQuorumActive(llmq_type, qman, quorum_hash)) { return; } @@ -383,6 +384,19 @@ void CSigningManager::VerifyAndProcessRecoveredSig(NodeId from, std::shared_ptr< return; } + // Once IsQuorumActive has passed, quorum_hash is one of the recent quorums ScanQuorums + // covers, so this is usually served from cache. ScanQuorums and GetQuorum keep separate + // LRUs, so a rebuild is still possible here, but only of a recent quorum — never of the + // arbitrary historical one a peer could otherwise name. + auto quorum = qman.GetQuorum(llmq_type, quorum_hash); + if (!quorum) { + // Reported active by ScanQuorums but no longer materializable (e.g. reorg). + // Not peer-controlled once the hash is restricted to the active set, so no score. + LogPrint(BCLog::LLMQ, "CSigningManager::%s -- quorum %s not found\n", __func__, + quorum_hash.ToString()); + return; + } + LogPrint(BCLog::LLMQ, "CSigningManager::%s -- signHash=%s, id=%s, msgHash=%s, node=%d\n", __func__, recoveredSig->buildSignHash().ToString(), recoveredSig->getId().ToString(), recoveredSig->getMsgHash().ToString(), from); From 5619e24256b3cb50520b4b53b4999cc1a6f318f5 Mon Sep 17 00:00:00 2001 From: pasta Date: Mon, 3 Aug 2026 21:14:01 -0500 Subject: [PATCH 2/2] refactor: trim comment density in VerifyAndProcessRecoveredSig Condense the reordering rationale to one line per block, keeping only the non-obvious part (why the gate order matters, why a null quorum here isn't peer-controlled). --- src/llmq/signing.cpp | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/src/llmq/signing.cpp b/src/llmq/signing.cpp index 344e1dd38117..b19ebb8bd3a7 100644 --- a/src/llmq/signing.cpp +++ b/src/llmq/signing.cpp @@ -368,12 +368,8 @@ void CSigningManager::VerifyAndProcessRecoveredSig(NodeId from, std::shared_ptr< auto llmq_type = recoveredSig->getLlmqType(); const uint256& quorum_hash = recoveredSig->getQuorumHash(); - // Cheap gate first. IsQuorumActive is bounded to the keepOldConnections most recent - // quorums at the tip, and that set is shared and cached across callers. GetQuorum, by - // contrast, takes the peer-supplied hash and can rebuild an arbitrary historical mined - // commitment (DMN list replay + member selection) on a cache miss — do not let an - // unsolicited QSIGREC force that work for inactive hashes. - // Caller (NetSigning) has already rejected unknown llmq types. + // Cheap gate first: GetQuorum can rebuild an arbitrary historical quorum on a cache miss, + // so don't let an unsolicited QSIGREC force that work for an inactive hash. if (!IsQuorumActive(llmq_type, qman, quorum_hash)) { return; } @@ -384,14 +380,9 @@ void CSigningManager::VerifyAndProcessRecoveredSig(NodeId from, std::shared_ptr< return; } - // Once IsQuorumActive has passed, quorum_hash is one of the recent quorums ScanQuorums - // covers, so this is usually served from cache. ScanQuorums and GetQuorum keep separate - // LRUs, so a rebuild is still possible here, but only of a recent quorum — never of the - // arbitrary historical one a peer could otherwise name. auto quorum = qman.GetQuorum(llmq_type, quorum_hash); if (!quorum) { - // Reported active by ScanQuorums but no longer materializable (e.g. reorg). - // Not peer-controlled once the hash is restricted to the active set, so no score. + // Active per ScanQuorums but no longer materializable (e.g. reorg); not peer-controlled, so no score. LogPrint(BCLog::LLMQ, "CSigningManager::%s -- quorum %s not found\n", __func__, quorum_hash.ToString()); return;