fix: stop hitting additional rate limit windows once a client is already limited (#686)

isRateLimited() in EventMessageHandler and WebSocketAdapter iterated every
configured rate limit window and called rateLimiter.hit() even after an
earlier window had already tripped. Each hit() call executes a Lua script
against Redis, so a client already being rate-limited caused N redundant
Redis writes per message instead of one. Both methods now return as soon
as the first exceeded window is found.

Signed-off-by: Priyanshubhartistm <bhartipriyanshustm@gmail.com>
Co-authored-by: Ricardo Cabral <me@ricardocabral.io>

Author: Priyanshubhartistm

.changeset/rate-limiter-continues-after-hit.md

@@ -0,0 +1,10 @@
+---
+"nostream": patch
+---
+
+fix: stop checking additional rate limit windows once a client is already rate-limited
+
+`isRateLimited()` in `EventMessageHandler` and `WebSocketAdapter` looped through every
+configured rate limit window even after one had already tripped, calling `rateLimiter.hit()`
+(a Redis write) for each remaining window. Both now return as soon as the first exceeded
+window is found, avoiding redundant Redis writes for clients that are already being limited.

src/adapters/web-socket-adapter.ts

@@ -236,18 +236,17 @@ export class WebSocketAdapter extends EventEmitter implements IWebSocketAdapter
 
     const hit = (period: number, rate: number) => rateLimiter.hit(`${client}:message:${period}`, 1, { period, rate })
 
-    let limited = false
     for (const { rate, period } of rateLimits) {
       const isRateLimited = await hit(period, rate)
 
       if (isRateLimited) {
         logger('rate limited %s: %d messages / %d ms exceeded', client, rate, period)
 
-        limited = true
+        return true
       }
     }
 
-    return limited
+    return false
   }
 
   private onClientPong() {

src/handlers/event-message-handler.ts

@@ -351,7 +351,6 @@ export class EventMessageHandler implements IMessageHandler {
       return rateLimiter.hit(key, 1, { period, rate })
     }
 
-    let limited = false
     for (const { rate, period, kinds } of rateLimits) {
       // skip if event kind does not apply
       if (Array.isArray(kinds) && !kinds.some(isEventKindOrRangeMatch(event))) {
@@ -363,11 +362,11 @@ export class EventMessageHandler implements IMessageHandler {
       if (isRateLimited) {
         logger('rate limited %s: %d events / %d ms exceeded', event.pubkey, rate, period)
 
-        limited = true
+        return true
       }
     }
 
-    return limited
+    return false
   }
 
   protected async isUserAdmitted(event: Event): Promise<string | undefined> {

test/unit/adapters/web-socket-adapter.spec.ts

@@ -423,6 +423,34 @@ describe('WebSocketAdapter', () => {
       expect(client.send).to.have.been.called
     })
 
+    it('stops hitting subsequent rate limit windows once one is exceeded', async () => {
+      client.readyState = WebSocket.OPEN
+
+      const hitStub = sandbox.stub().resolves(true)
+
+      settingsFactory.returns({
+        network: { remoteIpHeader: '' },
+        limits: {
+          message: {
+            rateLimits: [
+              { period: 60000, rate: 1 },
+              { period: 180, rate: 3 },
+            ],
+            ipWhitelist: [],
+          },
+        },
+      })
+
+      slidingWindowRateLimiter.returns({ hit: hitStub })
+
+      const messageCall = client.on.getCalls().find((call: any) => call.args[0] === 'message')
+      const onMessage = messageCall.args[1]
+
+      await onMessage(Buffer.from(JSON.stringify(['EVENT', {}])))
+
+      expect(hitStub).to.have.been.calledOnce
+    })
+
     it('does not rate limit when no rateLimits are configured', async () => {
       client.readyState = WebSocket.OPEN
 

test/unit/handlers/event-message-handler.spec.ts

@@ -1022,6 +1022,26 @@ describe('EventMessageHandler', () => {
       )
       expect(actualResult).to.be.true
     })
+
+    it('stops hitting subsequent rate limit windows once one is exceeded', async () => {
+      eventLimits.rateLimits = [
+        {
+          period: 60000,
+          rate: 1,
+        },
+        {
+          period: 180,
+          rate: 3,
+        },
+      ]
+
+      rateLimiterHitStub.onFirstCall().resolves(true)
+
+      const actualResult = await (handler as any).isRateLimited(event)
+
+      expect(rateLimiterHitStub).to.have.been.calledOnce
+      expect(actualResult).to.be.true
+    })
   })
 
   describe('isUserAdmitted', () => {