fix(net): bound QGETDATA request tracking

src/Makefile.test.include

@@ -147,6 +147,7 @@ BITCOIN_TESTS =\
   test/llmq_hash_tests.cpp \
   test/llmq_invalid_type_tests.cpp \
   test/llmq_params_tests.cpp \
+  test/llmq_qgetdata_tests.cpp \
   test/llmq_snapshot_tests.cpp \
   test/llmq_utils_tests.cpp \
   test/logging_tests.cpp \

src/llmq/net_quorum.cpp

@@ -15,6 +15,7 @@
 #include <masternode/sync.h>
 #include <net.h>
 #include <netmessagemaker.h>
+#include <scheduler.h>
 #include <util/helpers.h>
 #include <util/std23.h>
 #include <util/thread.h>
@@ -68,6 +69,11 @@ void NetQuorum::Stop()
     workerPool.stop(true);
 }
 
+void NetQuorum::Schedule(CScheduler& scheduler)
+{
+    scheduler.scheduleEvery([this] { m_qman.CleanupExpiredDataRequests(); }, std::chrono::minutes{1});
+}
+
 void NetQuorum::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRecv)
 {
     if (msg_type == NetMsgType::QGETDATA) {
@@ -104,28 +110,51 @@ void NetQuorum::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataS
             return misbehave;
         };
 
-        const CQuorumDataRequestKey key(pfrom.GetVerifiedProRegTxHash(), false, request.GetQuorumHash(), request.GetLLMQType());
-        const bool request_limit_exceeded = !m_qman.RegisterDataRequest(key, request, /*add_expiry_bias=*/false);
-
+        // Cheap pre-checks before tracking: invalid type / unknown block must not grow
+        // mapQuorumDataRequests — their keyspace is attacker-controlled and unbounded, but
+        // rejecting them costs no storage lookup. Replies are ~request-sized (no amplification).
         if (!Params().GetLLMQ(request.GetLLMQType()).has_value()) {
-            // Unlike the misses below, this one cannot be explained by the peer being ahead of
-            // us: no quorum of an unregistered type can exist on this chain, so there is
-            // nothing to ask about. Answer with the error anyway, then score in full.
-            sendQDATA(CQuorumDataRequest::Errors::QUORUM_TYPE_INVALID, request_limit_exceeded);
+            // Unregistered type cannot exist on this chain — answer, then score in full.
+            sendQDATA(CQuorumDataRequest::Errors::QUORUM_TYPE_INVALID, /*request_limit_exceeded=*/false);
             m_peer_manager->PeerMisbehaving(pfrom.GetId(), 100, "invalid llmqType in QGETDATA");
             return;
         }
 
-        const CBlockIndex* pQuorumBaseBlockIndex = WITH_LOCK(::cs_main, return m_chainman.m_blockman.LookupBlockIndex(request.GetQuorumHash()));
-        if (pQuorumBaseBlockIndex == nullptr) {
-            if (sendQDATA(CQuorumDataRequest::Errors::QUORUM_BLOCK_NOT_FOUND, request_limit_exceeded)) {
-                m_peer_manager->PeerMisbehaving(pfrom.GetId(), 25, "request limit exceeded");
+        const CBlockIndex* pQuorumBaseBlockIndex{nullptr};
+        {
+            LOCK(::cs_main);
+            const auto* pindex = m_chainman.m_blockman.LookupBlockIndex(request.GetQuorumHash());
+            if (pindex != nullptr && m_chainman.ActiveChain().Contains(pindex)) {
+                pQuorumBaseBlockIndex = pindex;
             }
+        }
+        if (pQuorumBaseBlockIndex == nullptr) {
+            // Not misbehavior: the requester may simply be ahead of us or on another fork.
+            sendQDATA(CQuorumDataRequest::Errors::QUORUM_BLOCK_NOT_FOUND, /*request_limit_exceeded=*/false);
+            return;
+        }
+
+        // Active-chain keys are bounded by chain blocks × LLMQ types, so register the request
+        // before the commitment lookup: the lookup misses EvoDB's caches for blocks without a
+        // mined commitment, and rate limiting must gate that cost, not the other way around.
+        const CQuorumDataRequestKey key(pfrom.GetVerifiedProRegTxHash(), false, request.GetQuorumHash(),
+                                        request.GetLLMQType());
+        const auto registration = m_qman.RegisterDataRequest(key, request, /*add_expiry_bias=*/false);
+        if (registration == DataRequestRegistration::RequesterLimitExceeded) {
+            m_peer_manager->PeerMisbehaving(pfrom.GetId(), 25, "too many quorum data requests");
+            return;
+        }
+        if (registration == DataRequestRegistration::CapacityExhausted) {
             return;
         }
+        const bool request_limit_exceeded = registration == DataRequestRegistration::RateLimited;
 
         const auto pQuorum = m_qman.GetQuorum(request.GetLLMQType(), request.GetQuorumHash());
         if (pQuorum == nullptr) {
+            // No mined commitment for this block. Not misbehavior on its own: the commitment is
+            // mined after the quorum base block, so we can know the base block while still lagging
+            // behind the commitment the requester saw. Repeats within the expiry window were
+            // rate-limited above and do score.
             if (sendQDATA(CQuorumDataRequest::Errors::QUORUM_NOT_FOUND, request_limit_exceeded)) {
                 m_peer_manager->PeerMisbehaving(pfrom.GetId(), 25, "request limit exceeded");
             }
@@ -321,7 +350,7 @@ DataRequestStatus NetQuorum::RequestQuorumData(CNode& peer, const CQuorum& quoru
                                     quorum.m_quorum_base_block_index->GetBlockHash(), quorum.qc->llmqType);
     const CQuorumDataRequest request(quorum.qc->llmqType, quorum.m_quorum_base_block_index->GetBlockHash(),
                                      nDataMask, proTxHash);
-    if (!m_qman.RegisterDataRequest(key, request)) {
+    if (m_qman.RegisterDataRequest(key, request) != DataRequestRegistration::Accepted) {
         return m_qman.GetDataRequestStatus(peer.GetVerifiedProRegTxHash(), /*we_requested=*/true,
                                            quorum.m_quorum_base_block_index->GetBlockHash(), quorum.qc->llmqType);
     }

src/llmq/net_quorum.h

@@ -61,6 +61,7 @@ class NetQuorum final : public NetHandler, public CValidationInterface
     void Start() override;
     void Stop() override;
     void Interrupt() override { quorumThreadInterrupt(); }
+    void Schedule(CScheduler& scheduler) override;
 
 protected:
     // CValidationInterface

src/llmq/quorumsman.cpp

@@ -15,6 +15,7 @@
 #include <llmq/params.h>
 #include <llmq/signhash.h>
 #include <llmq/utils.h>
+#include <util/check.h>
 #include <util/helpers.h>
 #include <util/std23.h>
 
@@ -339,6 +340,18 @@ void CQuorumManager::CleanupExpiredDataRequests() const
     auto it = mapQuorumDataRequests.begin();
     while (it != mapQuorumDataRequests.end()) {
         if (it->second.IsExpired(/*add_bias=*/true)) {
+            if (!it->first.m_we_requested) {
+                // On counter desync, skip the decrement: over-counting fails toward stricter
+                // limits, while an underflow would disable the caps entirely.
+                auto count_it = m_inbound_request_counts.find(it->first.proRegTx);
+                if (Assume(count_it != m_inbound_request_counts.end() && count_it->second > 0) &&
+                    --count_it->second == 0) {
+                    m_inbound_request_counts.erase(count_it);
+                }
+                if (Assume(m_inbound_request_count > 0)) {
+                    --m_inbound_request_count;
+                }
+            }
             it = mapQuorumDataRequests.erase(it);
         } else {
             ++it;
@@ -433,19 +446,37 @@ CQuorumCPtr CQuorumManager::GetQuorum(Consensus::LLMQType llmqType,
     return BuildQuorumFromCommitment(llmqType, pQuorumBaseBlockIndex, populate_cache);
 }
 
-bool CQuorumManager::RegisterDataRequest(const CQuorumDataRequestKey& key, const CQuorumDataRequest& request,
-                                          bool add_expiry_bias) const
+DataRequestRegistration CQuorumManager::RegisterDataRequest(const CQuorumDataRequestKey& key,
+                                                            const CQuorumDataRequest& request, bool add_expiry_bias) const
 {
     LOCK(cs_data_requests);
-    auto [old_pair, inserted] = mapQuorumDataRequests.emplace(key, request);
-    if (!inserted) {
-        if (old_pair->second.IsExpired(add_expiry_bias)) {
-            old_pair->second = request;
-            return true;
+    if (auto it = mapQuorumDataRequests.find(key); it != mapQuorumDataRequests.end()) {
+        if (!it->second.IsExpired(add_expiry_bias)) {
+            return DataRequestRegistration::RateLimited;
         }
-        return false;
+        it->second = request;
+        return DataRequestRegistration::Accepted;
     }
-    return true;
+
+    if (!key.m_we_requested) {
+        if (m_inbound_request_count >= MAX_INBOUND_DATA_REQUESTS) {
+            return DataRequestRegistration::CapacityExhausted;
+        }
+        const auto count_it = m_inbound_request_counts.find(key.proRegTx);
+        if (count_it != m_inbound_request_counts.end() && count_it->second >= MAX_INBOUND_DATA_REQUESTS_PER_REQUESTER) {
+            // All unauthenticated qwatch peers share the null identity, so exhaustion of that
+            // budget is not attributable to the connection that happened to arrive last.
+            return key.proRegTx.IsNull() ? DataRequestRegistration::CapacityExhausted
+                                         : DataRequestRegistration::RequesterLimitExceeded;
+        }
+    }
+
+    mapQuorumDataRequests.emplace(key, request);
+    if (!key.m_we_requested) {
+        ++m_inbound_request_counts[key.proRegTx];
+        ++m_inbound_request_count;
+    }
+    return DataRequestRegistration::Accepted;
 }
 
 CQuorumManager::DataResponseValidation CQuorumManager::ValidateDataResponse(

src/llmq/quorumsman.h

@@ -48,6 +48,18 @@ class CDKGSessionManager;
 class CQuorumBlockProcessor;
 class CQuorumSnapshotManager;
 
+//! A peer normally needs data for only a handful of quorums at once. These limits keep
+//! attacker-controlled inbound tracking bounded independently of request expiry or block production.
+static constexpr size_t MAX_INBOUND_DATA_REQUESTS_PER_REQUESTER{64};
+static constexpr size_t MAX_INBOUND_DATA_REQUESTS{4096};
+
+enum class DataRequestRegistration : uint8_t {
+    Accepted,
+    RateLimited,
+    RequesterLimitExceeded,
+    CapacityExhausted,
+};
+
 /**
  * The quorum manager maintains quorums which were mined on chain. When a quorum is requested from the manager,
  * it will lookup the commitment (through CQuorumBlockProcessor) and build a CQuorum object from it.
@@ -70,6 +82,8 @@ class CQuorumManager final
     mutable Mutex cs_data_requests;
     mutable std::unordered_map<CQuorumDataRequestKey, CQuorumDataRequest, StaticSaltedHasher> mapQuorumDataRequests
         GUARDED_BY(cs_data_requests);
+    mutable std::unordered_map<uint256, size_t, StaticSaltedHasher> m_inbound_request_counts GUARDED_BY(cs_data_requests);
+    mutable size_t m_inbound_request_count GUARDED_BY(cs_data_requests){0};
 
     mutable Mutex m_cs_maps;
     mutable std::map<Consensus::LLMQType, Uint256LruHashMap<CQuorumPtr>> mapQuorumsCache
@@ -140,9 +154,10 @@ class CQuorumManager final
     bool IsMasternode() const;
     bool IsWatching() const;
 
-    //! Request tracking for QGETDATA/QDATA — used by NetQuorum and RPC
-    bool RegisterDataRequest(const CQuorumDataRequestKey& key, const CQuorumDataRequest& request,
-                             bool add_expiry_bias = true) const
+    //! Request tracking for QGETDATA/QDATA — used by NetQuorum and RPC. Inbound entries are
+    //! bounded per requester and globally; outbound entries initiated by us do not consume either budget.
+    DataRequestRegistration RegisterDataRequest(const CQuorumDataRequestKey& key, const CQuorumDataRequest& request,
+                                                bool add_expiry_bias = true) const
         EXCLUSIVE_LOCKS_REQUIRED(!cs_data_requests);
     enum class DataResponseValidation : uint8_t { OK, NotRequested, AlreadyReceived, Mismatch };
     DataResponseValidation ValidateDataResponse(const CQuorumDataRequestKey& key,

src/rpc/quorums.cpp

@@ -938,7 +938,7 @@ static RPCHelpMan quorum_getdata()
         if (!quorum->m_quorum_base_block_index) return false;
         const llmq::CQuorumDataRequest request(llmqType, quorum->qc->quorumHash, nDataMask, proTxHash);
         const llmq::CQuorumDataRequestKey key(pNode->GetVerifiedProRegTxHash(), true, quorum->qc->quorumHash, llmqType);
-        if (!llmq_ctx.qman->RegisterDataRequest(key, request)) return false;
+        if (llmq_ctx.qman->RegisterDataRequest(key, request) != llmq::DataRequestRegistration::Accepted) return false;
         connman.PushMessage(pNode, CNetMsgMaker(pNode->GetCommonVersion()).Make(NetMsgType::QGETDATA, request));
         return true;
     });