fix: replace never-resetting byte counter with sliding window in rate limiter

[berkelmali]

Description:

The totalBytes counter in ClientMsgRateLimiter never resets. In long-running game sessions, legitimate clients accumulate bytes over the entire game duration and eventually get false-kicked when they exceed the 2MB threshold. This replaces the monotonically increasing counter with a 60-second sliding window. Old byte events are evicted as they age out, so only recent throughput is measured. A running totalBytes counter is maintained for O(1) per-message checks.

Please complete the following:

• I have added screenshots for all UI updates
• I process any text displayed to the user through translateText() and I've added it to the en.json file
• I have added relevant tests to the test directory
• I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced

Please put your Discord username so you can be contacted if a bug or regression is found:

barfires

[coderabbitai]

Walkthrough

ClientMsgRateLimiter now enforces byte-rate limits using a 60-second rolling window instead of a static accumulator. Per-client byte events are tracked with timestamps, entries outside the window are evicted, and the total bytes cap is enforced against only the active window.

Changes

Rolling-Window Byte Limiting

Layer / File(s)	Summary
Constants and Data Shape src/server/ClientMsgRateLimiter.ts	BYTE_WINDOW_MS constant added for 60-second window. ClientBucket type gains byteEvents array to store timestamped byte consumption events.
Rolling-Window Byte Accounting src/server/ClientMsgRateLimiter.ts	check() method calculates current time and cutoff threshold, removes expired events outside the window, appends the new byte event, and checks the total against the 2MB cap.
Bucket Initialization src/server/ClientMsgRateLimiter.ts	getOrCreate() initializes the byteEvents array for newly created client buckets.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🪟 Bytes flow through time's glass window,
Old events fade when sixty seconds grow old,
Two megabytes ask: "Will you let them through?"
A rolling frontier keeps rate limits pure.
Time-bound and fair, the limiter stands tall. 📊

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: replacing a never-resetting byte counter with a sliding window mechanism in the rate limiter.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description check	✅ Passed	The pull request description clearly explains the problem (never-resetting byte counter causing false kicks) and the solution (60-second sliding window), directly matching the changeset in ClientMsgRateLimiter.ts.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/server/ClientMsgRateLimiter.ts`:
- Around line 30-33: The loop that evicts old events uses
bucket.byteEvents.shift() which reindexes the array and causes CPU spikes under
high traffic; update the data structure in ClientMsgRateLimiter to avoid
shifting by implementing a head index or ring buffer for bucket.byteEvents (and
the similar occurrences around the code that manipulate bucket.byteEvents at the
other noted sites), removing uses of Array.shift() and instead incrementing a
head pointer and decrementing bucket.totalBytes when evicting, and ensure any
checks (length/emptiness and iteration) account for the head pointer so logic in
the functions/methods that reference bucket.byteEvents continues to work
correctly.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 767180cc-9072-4d6b-a4e0-7858d47d8c51

📥 Commits

Reviewing files that changed from the base of the PR and between 4889cb8 and c5c8050.

📒 Files selected for processing (1)

• src/server/ClientMsgRateLimiter.ts

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Array.shift() in the hot path can cause CPU spikes under spam traffic.

This loop reindexes the whole array on every eviction, so worst-case cost grows fast with many tiny messages. In a rate-limiter path, that can be used as a DoS vector.

Suggested local refactor (head index queue, no shift)

 interface ClientBucket {
   perSecond: RateLimiter;
   perMinute: RateLimiter;
   byteEvents: Array<{ at: number; bytes: number }>;
+  byteEventsHead: number;
   totalBytes: number;
 }

 // in check()
- while (bucket.byteEvents.length > 0 && bucket.byteEvents[0].at < cutoff) {
-   const evicted = bucket.byteEvents.shift()!;
+ while (
+   bucket.byteEventsHead < bucket.byteEvents.length &&
+   bucket.byteEvents[bucket.byteEventsHead].at < cutoff
+ ) {
+   const evicted = bucket.byteEvents[bucket.byteEventsHead++]!;
    bucket.totalBytes -= evicted.bytes;
  }

+ // periodic compact to keep memory bounded
+ if (bucket.byteEventsHead > 1024 && bucket.byteEventsHead * 2 > bucket.byteEvents.length) {
+   bucket.byteEvents = bucket.byteEvents.slice(bucket.byteEventsHead);
+   bucket.byteEventsHead = 0;
+ }

  bucket.byteEvents.push({ at: now, bytes });

 // in getOrCreate()
   const bucket = {
     perSecond: new RateLimiter({ tokensPerInterval: INTENTS_PER_SECOND, interval: "second" }),
     perMinute: new RateLimiter({ tokensPerInterval: INTENTS_PER_MINUTE, interval: "minute" }),
     byteEvents: [],
+    byteEventsHead: 0,
     totalBytes: 0,
   };

Also applies to: 35-35, 74-74

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/server/ClientMsgRateLimiter.ts` around lines 30 - 33, The loop that
evicts old events uses bucket.byteEvents.shift() which reindexes the array and
causes CPU spikes under high traffic; update the data structure in
ClientMsgRateLimiter to avoid shifting by implementing a head index or ring
buffer for bucket.byteEvents (and the similar occurrences around the code that
manipulate bucket.byteEvents at the other noted sites), removing uses of
Array.shift() and instead incrementing a head pointer and decrementing
bucket.totalBytes when evicting, and ensure any checks (length/emptiness and
iteration) account for the head pointer so logic in the functions/methods that
reference bucket.byteEvents continues to work correctly.