Skip to content

feat(chat): sliding window message frequency and flood limiter - #138

Open
Rodrigoue9 wants to merge 8 commits into
Bitcoindefi:mainfrom
Rodrigoue9:feat/chat-channel-rate-limiter
Open

feat(chat): sliding window message frequency and flood limiter#138
Rodrigoue9 wants to merge 8 commits into
Bitcoindefi:mainfrom
Rodrigoue9:feat/chat-channel-rate-limiter

Conversation

@Rodrigoue9

Copy link
Copy Markdown

Chat Flood Limiter

  • Sliding window message frequency guard for game chat channels.
  • Prevents spam and buffer saturation in multiplayer rooms.

Ready for review! 🚀

* Bitcoindefi/OpenAO - Chat Flood Rate Limiter
*/
export class ChatRateLimiter {
private userMessageTimestamps = new Map<string, number[]>();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance: ChatRateLimiter map grows unbounded, never evicts users

userMessageTimestamps retains an entry for every userId that ever sent a message and is never cleaned up, even after a user's timestamps all age out of the window. On a long-running multiplayer server this map grows without bound (the exact buffer saturation the limiter is meant to prevent). Add eviction: after filtering, delete the key when the array is empty, and/or periodically sweep stale entries.

Persist the filtered array so stale timestamps don't accumulate; consider a periodic sweep that deletes keys whose arrays are empty to bound memory.:

const timestamps = (this.userMessageTimestamps.get(userId) || []).filter(t => now - t < this.windowMs);

if (timestamps.length >= this.maxMessagesPerWindow) {
  this.userMessageTimestamps.set(userId, timestamps);
  return { allowed: false, remaining: 0 };
}

timestamps.push(now);
this.userMessageTimestamps.set(userId, timestamps);
return { allowed: true, remaining: this.maxMessagesPerWindow - timestamps.length };
  • Apply fix

Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎

@gitar-bot

gitar-bot Bot commented Aug 25, 2026

Copy link
Copy Markdown
Code Review ⚠️ Changes requested 0 resolved / 1 findings

Adds a sliding window message frequency and flood limiter for game chat channels, but the userMessageTimestamps map grows unbounded and never evicts inactive users, causing a memory leak.

⚠️ Performance: ChatRateLimiter map grows unbounded, never evicts users

📄 api/src/lib/chatRateLimiter.ts:5 📄 api/src/lib/chatRateLimiter.ts:14-22

userMessageTimestamps retains an entry for every userId that ever sent a message and is never cleaned up, even after a user's timestamps all age out of the window. On a long-running multiplayer server this map grows without bound (the exact buffer saturation the limiter is meant to prevent). Add eviction: after filtering, delete the key when the array is empty, and/or periodically sweep stale entries.

Persist the filtered array so stale timestamps don't accumulate; consider a periodic sweep that deletes keys whose arrays are empty to bound memory.
const timestamps = (this.userMessageTimestamps.get(userId) || []).filter(t => now - t < this.windowMs);

if (timestamps.length >= this.maxMessagesPerWindow) {
  this.userMessageTimestamps.set(userId, timestamps);
  return { allowed: false, remaining: 0 };
}

timestamps.push(now);
this.userMessageTimestamps.set(userId, timestamps);
return { allowed: true, remaining: this.maxMessagesPerWindow - timestamps.length };
🤖 Prompt for agents
Code Review: Adds a sliding window message frequency and flood limiter for game chat channels, but the userMessageTimestamps map grows unbounded and never evicts inactive users, causing a memory leak.

1. ⚠️ Performance: ChatRateLimiter map grows unbounded, never evicts users
   Files: api/src/lib/chatRateLimiter.ts:5, api/src/lib/chatRateLimiter.ts:14-22

   `userMessageTimestamps` retains an entry for every userId that ever sent a message and is never cleaned up, even after a user's timestamps all age out of the window. On a long-running multiplayer server this map grows without bound (the exact buffer saturation the limiter is meant to prevent). Add eviction: after filtering, delete the key when the array is empty, and/or periodically sweep stale entries.

   Fix (Persist the filtered array so stale timestamps don't accumulate; consider a periodic sweep that deletes keys whose arrays are empty to bound memory.):
   const timestamps = (this.userMessageTimestamps.get(userId) || []).filter(t => now - t < this.windowMs);
   
   if (timestamps.length >= this.maxMessagesPerWindow) {
     this.userMessageTimestamps.set(userId, timestamps);
     return { allowed: false, remaining: 0 };
   }
   
   timestamps.push(now);
   this.userMessageTimestamps.set(userId, timestamps);
   return { allowed: true, remaining: this.maxMessagesPerWindow - timestamps.length };

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Important

Your trial ends in 7 days — upgrade now to keep code review, CI analysis, auto-apply, custom automations, and more.

Was this helpful? React with 👍 / 👎 | Gitar

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant