Skip to content

Commit 0225574

Browse files
fix(llmq): drop and score unrequested quorum commitments
QFCOMMITMENT is only ever sent in reply to a GETDATA (commitments are announced via AddMineableCommitment -> PeerRelayInv), so one we have no in-flight request for was never asked for. Most of the checks in ProcessMessage reject without scoring the peer -- deliberately, since we may just be lagging behind or on another chain -- so an unsolicited sender could repeat the block lookups and mineable-commitment probes indefinitely at no cost. Authorize on PeerConsumeGetDataResponse before any of that work, replacing the m_to_erase round-trip that resolved to the weaker ReceivedResponse. CQuorumBlockProcessor holds no PeerManagerInternal reference (net_processing already includes this header, so taking one would make the dependency circular), so the check is passed in as a predicate from the call site. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent b8a1813 commit 0225574

3 files changed

Lines changed: 35 additions & 4 deletions

File tree

‎src/llmq/blockprocessor.cpp‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,8 @@ CQuorumBlockProcessor::~CQuorumBlockProcessor()
6363
}
6464

6565
MessageProcessingResult CQuorumBlockProcessor::ProcessMessage(const CNode& peer, std::string_view msg_type,
66-
CDataStream& vRecv)
66+
CDataStream& vRecv,
67+
const ConsumeRequestFn& consume_request)
6768
{
6869
if (msg_type != NetMsgType::QFCOMMITMENT) {
6970
return {};
@@ -72,8 +73,20 @@ MessageProcessingResult CQuorumBlockProcessor::ProcessMessage(const CNode& peer,
7273
CFinalCommitment qc;
7374
vRecv >> qc;
7475

76+
// A QFCOMMITMENT is only ever sent in reply to a GETDATA (see ProcessGetData), so one we have no
77+
// in-flight request for was never asked for. Drop it up front: most of the checks below reject
78+
// without scoring the peer -- deliberately, since we may just be lagging behind -- so an
79+
// unsolicited peer could otherwise repeat the block lookups and map probes indefinitely. A bare
80+
// announcement deliberately does not qualify: it would let the peer authorise its own payload by
81+
// sending INV first.
82+
if (!consume_request(CInv{MSG_QUORUM_FINAL_COMMITMENT, ::SerializeHash(qc)})) {
83+
LogPrint(BCLog::LLMQ, "CQuorumBlockProcessor::%s -- unrequested commitment from peer=%d\n", __func__,
84+
peer.GetId());
85+
return MisbehavingError{UNREQUESTED_OBJECT_MISBEHAVIOR_SCORE, "unrequested quorum commitment"};
86+
}
87+
88+
// Note: no m_to_erase, the request was already consumed by the solicitation check above.
7589
MessageProcessingResult ret;
76-
ret.m_to_erase = CInv{MSG_QUORUM_FINAL_COMMITMENT, ::SerializeHash(qc)};
7790

7891
if (qc.IsNull()) {
7992
LogPrint(BCLog::LLMQ, "CQuorumBlockProcessor::%s -- null commitment from peer=%d\n", __func__, peer.GetId());

‎src/llmq/blockprocessor.h‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
#include <gsl/pointers.h>
2020

21+
#include <functional>
2122
#include <optional>
2223

2324
class BlockValidationState;
@@ -62,7 +63,18 @@ class CQuorumBlockProcessor
6263
CQuorumSnapshotManager& qsnapman, int8_t bls_threads);
6364
~CQuorumBlockProcessor();
6465

65-
[[nodiscard]] MessageProcessingResult ProcessMessage(const CNode& peer, std::string_view msg_type, CDataStream& vRecv)
66+
//! Predicate answering "did we ask this peer for the inv?", consuming the pending request as a
67+
//! side effect. Passed in rather than reached through PeerManagerInternal because net_processing
68+
//! already depends on this header; see ProcessMessage for how it is used.
69+
//!
70+
//! Must be invoked without ::cs_main held -- the implementation takes it. Thread-safety
71+
//! analysis cannot check this through the type-erased std::function, so keep any call site
72+
//! outside ProcessMessage's own LOCK(::cs_main) block.
73+
using ConsumeRequestFn = std::function<bool(const CInv&)>;
74+
75+
[[nodiscard]] MessageProcessingResult ProcessMessage(const CNode& peer, std::string_view msg_type,
76+
CDataStream& vRecv,
77+
const ConsumeRequestFn& consume_request)
6678
EXCLUSIVE_LOCKS_REQUIRED(!minableCommitmentsCs);
6779

6880
bool ProcessBlock(const CBlock& block, gsl::not_null<const CBlockIndex*> pindex, BlockValidationState& state,

‎src/net_processing.cpp‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5542,7 +5542,13 @@ void PeerManagerImpl::ProcessMessage(
55425542
PostProcessMessage(m_cj_walletman->processMessage(pfrom, m_chainman.ActiveChainstate(), m_connman, m_mempool, msg_type, vRecv), pfrom.GetId());
55435543
}
55445544
PostProcessMessage(CMNAuth::ProcessMessage(pfrom, peer->m_their_services, m_connman, m_mn_metaman, m_nodeman, m_mn_sync, m_dmnman->GetListAtChainTip(), msg_type, vRecv), pfrom.GetId());
5545-
PostProcessMessage(m_llmq_ctx->quorum_block_processor->ProcessMessage(pfrom, msg_type, vRecv), pfrom.GetId());
5545+
PostProcessMessage(m_llmq_ctx->quorum_block_processor->ProcessMessage(
5546+
pfrom, msg_type, vRecv,
5547+
[this, &pfrom](const CInv& inv) EXCLUSIVE_LOCKS_REQUIRED(!::cs_main) {
5548+
return WITH_LOCK(::cs_main,
5549+
return PeerConsumeGetDataResponse(pfrom.GetId(), inv));
5550+
}),
5551+
pfrom.GetId());
55465552
PostProcessMessage(ProcessPlatformBanMessage(pfrom.GetId(), msg_type, vRecv), pfrom.GetId());
55475553

55485554
if (msg_type == NetMsgType::CLSIG) {

0 commit comments

Comments
 (0)