From 38cdcf48e567079bdebb2988bb633c79f90af352 Mon Sep 17 00:00:00 2001
From: spypsy <spypsy@outlook.com>
Date: Wed, 25 Mar 2026 14:04:30 +0000
Subject: [PATCH] fix(p2p): check peer rate limit before global to prevent
 quota starvation

Fixes https://linear.app/aztec-labs/issue/A-758

Made-with: Cursor
---
 .../reqresp/rate-limiter/rate_limiter.test.ts | 21 ++++++++++++++++++
 .../reqresp/rate-limiter/rate_limiter.ts      | 22 +++++++++++--------
 2 files changed, 34 insertions(+), 9 deletions(-)

diff --git a/yarn-project/p2p/src/services/reqresp/rate-limiter/rate_limiter.test.ts b/yarn-project/p2p/src/services/reqresp/rate-limiter/rate_limiter.test.ts
index 80a6a6e423c9..6a11fbe9a017 100644
--- a/yarn-project/p2p/src/services/reqresp/rate-limiter/rate_limiter.test.ts
+++ b/yarn-project/p2p/src/services/reqresp/rate-limiter/rate_limiter.test.ts
@@ -156,6 +156,27 @@ describe('rate limiter', () => {
     multiProtocolRateLimiter.stop();
   });
 
+  it('Should not consume global quota when a peer is denied', () => {
+    // peer1 has a limit of 5/s; peer2 is a different peer
+    const spammingPeer = makePeer('spammer');
+    const legitimatePeer = makePeer('legit');
+
+    // Exhaust spammingPeer's per-peer quota (5 requests)
+    for (let i = 0; i < 5; i++) {
+      expect(rateLimiter.allow(ReqRespSubProtocol.TX, spammingPeer)).toBe(RateLimitStatus.Allowed);
+    }
+
+    // All further spammer requests are denied at the peer level
+    // With the bug, each of these would also consume a global token
+    for (let i = 0; i < 9; i++) {
+      expect(rateLimiter.allow(ReqRespSubProtocol.TX, spammingPeer)).toBe(RateLimitStatus.DeniedPeer);
+    }
+
+    // The legitimate peer should still be allowed: the global quota (10/s) must not
+    // have been consumed by the spammer's peer-denied requests
+    expect(rateLimiter.allow(ReqRespSubProtocol.TX, legitimatePeer)).toBe(RateLimitStatus.Allowed);
+  });
+
   it('Should allow requests if no rate limiter is configured', () => {
     const rateLimiter = new RequestResponseRateLimiter(peerScoring, {} as ReqRespSubProtocolRateLimits);
     expect(rateLimiter.allow(ReqRespSubProtocol.TX, makePeer('peer1'))).toBe(RateLimitStatus.Allowed);
diff --git a/yarn-project/p2p/src/services/reqresp/rate-limiter/rate_limiter.ts b/yarn-project/p2p/src/services/reqresp/rate-limiter/rate_limiter.ts
index 782021be1daa..79e6e915827b 100644
--- a/yarn-project/p2p/src/services/reqresp/rate-limiter/rate_limiter.ts
+++ b/yarn-project/p2p/src/services/reqresp/rate-limiter/rate_limiter.ts
@@ -97,9 +97,10 @@ export function prettyPrintRateLimitStatus(status: RateLimitStatus) {
  * 2. Individual rate limits for each peer.
  *
  * How it works:
- * - When a request comes in, it first checks against the global rate limit.
- * - If the global limit allows, it then checks against the specific peer's rate limit.
- * - The request is only allowed if both the global and peer-specific limits allow it.
+ * - When a request comes in, it first checks against the peer's individual rate limit.
+ * - If the peer limit allows, it then checks against the global rate limit.
+ * - The request is only allowed if both the peer-specific and global limits allow it.
+ * - Checking peer limit first ensures a rate-limited peer cannot exhaust the global quota.
  * - It automatically creates and manages rate limiters for new peers as they make requests.
  * - It periodically cleans up rate limiters for inactive peers to conserve memory.
  *
@@ -119,10 +120,6 @@ export class SubProtocolRateLimiter {
   }
 
   allow(peerId: PeerId): RateLimitStatus {
-    if (!this.globalLimiter.allow()) {
-      return RateLimitStatus.DeniedGlobal;
-    }
-
     const peerIdStr = peerId.toString();
     let peerLimiter: PeerRateLimiter | undefined = this.peerLimiters.get(peerIdStr);
     if (!peerLimiter) {
@@ -135,10 +132,17 @@ export class SubProtocolRateLimiter {
     } else {
       peerLimiter.lastAccess = Date.now();
     }
-    const peerLimitAllowed = peerLimiter.limiter.allow();
-    if (!peerLimitAllowed) {
+
+    // Check peer limit first: a rate-limited peer must not consume global quota,
+    // otherwise one spamming peer can starve all others by exhausting the global bucket.
+    if (!peerLimiter.limiter.allow()) {
       return RateLimitStatus.DeniedPeer;
     }
+
+    if (!this.globalLimiter.allow()) {
+      return RateLimitStatus.DeniedGlobal;
+    }
+
     return RateLimitStatus.Allowed;
   }