Skip to content

[BUG] Rate limiter checks all windows even after a hit - excess Redis writes on limited clients #685

Description

@Priyanshubhartistm

Describe the bug

Both isRateLimited() in src/handlers/event-message-handler.ts and src/adapters/web-socket-adapter.ts iterate every configured rate limit window and call rateLimiter.hit() for each one — even after an earlier window in the same loop has already reported the client as rate-limited. Each hit() call executes a Lua script against Redis (a ZADD/HSET write via SlidingWindowRateLimiter or EWMARateLimiter), so a burst of requests from a client that is already being rate-limited generates N x Redis writes per message/event instead of bailing out on the first exceeded window.

// event-message-handler.ts - isRateLimited()
let limited = false
for (const { rate, period, kinds } of rateLimits) {
  if (Array.isArray(kinds) && !kinds.some(isEventKindOrRangeMatch(event))) {
    continue
  }

  const isRateLimited = await hit({ period, rate, kinds })

  if (isRateLimited) {
    logger('rate limited %s: %d events / %d ms exceeded', event.pubkey, rate, period)
    limited = true // should return true here, but continues writing to all other buckets
  }
}

return limited

The identical pattern exists in WebSocketAdapter.isRateLimited() for message-level rate limiting.

To Reproduce

  1. Configure two or more rateLimits windows under limits.event.rateLimits (or limits.message.rateLimits) in .nostr/settings.yaml.
  2. Send enough events/messages from a single client to exceed the first configured window.
  3. Observe (e.g. via Redis MONITOR) that hit() - and therefore a Redis write - is still invoked for every remaining configured window on each subsequent message, even though the client is already known to be rate-limited.

Expected behavior

Once any window reports the client as rate-limited, the loop should return true immediately instead of continuing to hit the remaining windows:

for (const { rate, period, kinds } of rateLimits) {
  if (Array.isArray(kinds) && !kinds.some(isEventKindOrRangeMatch(event))) {
    continue
  }

  const isRateLimited = await hit({ period, rate, kinds })

  if (isRateLimited) {
    logger('rate limited %s: %d events / %d ms exceeded', event.pubkey, rate, period)
    return true
  }
}

return false

Screenshots

N/A

System (please complete the following information):

  • OS: Linux
  • Platform: Docker
  • Version: 3.0.0

Logs

N/A - behavior is only observable via Redis write volume (e.g. MONITOR or command count metrics), not application logs.

Additional context

Affects both event rate limiting (EventMessageHandler.isRateLimited()) and message rate limiting (WebSocketAdapter.isRateLimited()) - same root cause in both places. Under sustained abuse from a single already-limited client, this multiplies Redis load by the number of configured rate limit windows for no behavioral benefit, since the method already returns true/rejects the message regardless.

Metadata

Metadata

Labels

bugSomething isn't working

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions