Bound and evict stale buckets in the in-memory rate limiter

[Lakes41]

Difficulty: Advanced
Type: performance

Background

lib/rate-limit.ts implements a token-bucket rate limiter for the integration gateway routes, keyed by `ip:${ip}` and `wallet:${address}`. Buckets are stored in a module-level Map<string, Bucket> (const buckets = new Map<string, Bucket>()), created lazily on first request via getBucket() and never removed.

Problem

There is no eviction of stale entries. Every unique IP or wallet address that ever calls a rate-limited route (/api/integration/membership, /api/integration/verify) creates a permanent entry in buckets for the lifetime of the Node process. On a long-running instance exposed to the public internet, this is an unbounded-growth memory leak — an attacker could also deliberately spray requests from many spoofed X-Forwarded-For values to inflate memory usage, since the IP key is derived directly from a client-controllable header with no allowlist of trusted proxies.

Expected Outcome

The rate limiter's memory footprint stays bounded regardless of how many distinct IPs/wallets have made requests over the process lifetime, without changing the observable rate-limiting behavior (RateLimitResult semantics, retryAfter, remaining) for any individual key.

Suggested Implementation

1. Add a lastAccess timestamp to the Bucket interface (or reuse last) and implement a periodic or on-access sweep that deletes buckets whose tokens are fully refilled (i.e., inactive for at least WINDOW_MS) and haven't been touched recently — e.g., prune entries idle longer than N × WINDOW_MS.
2. Choose an eviction strategy appropriate for a single Node process without adding new runtime dependencies — e.g., an LRU cap (MAX_BUCKETS) combined with lazy sweep-on-write, or a setInterval sweep guarded to only run in a real server runtime (not during tests/edge builds).
3. Ensure the sweep doesn't introduce race conditions with concurrent take() calls (Node's request handling is single-threaded per event-loop tick, but be careful with any async boundaries introduced).
4. Add a way to reset/inspect bucket count for tests (mirroring resetLiveApiResilienceState() in lib/api/live.ts for the circuit breaker).
5. Update the file's doc comment (currently describing the "SINGLE-INSTANCE CAVEAT") to describe the new eviction behavior.
6. Extend test/rate-limit.test.ts with tests asserting the bucket map does not grow unboundedly after many distinct keys go idle.

Acceptance Criteria

• After a large number of distinct IP/wallet keys are rate-limited and then go idle, the internal store size is bounded (verified via a test-only introspection export).
• Existing rate-limit behavior (limited, retryAfter, remaining for active keys) is unchanged — all current test/rate-limit.test.ts assertions still pass.
• No new runtime dependency is introduced.
• Sweep/eviction logic is covered by new unit tests, including a test that an evicted-then-returning key gets a fresh, correctly-initialized bucket (not incorrectly rate-limited or given a free pass).
• npm run typecheck, npm run lint, npm test pass.

Likely Affected Files/Directories

• lib/rate-limit.ts
• test/rate-limit.test.ts
• docs/deployment.md (if it references rate-limiter behavior)

---

[TheWeirdDee]

Hi Maintainer, I’d like to take on this issue.

I’ve reviewed the rate limiter implementation and understand the core problem: the module-level bucket map grows indefinitely as new IPs and wallet addresses hit the integration routes, creating a potential memory leak on long-running instances.

I have experience with TypeScript/Node.js, API infrastructure, and performance-focused backend work. I’d implement bounded bucket eviction without changing the existing RateLimitResult behavior or introducing another runtime dependency.

My approach would be to:

• Add idle-access tracking to buckets and implement lazy/periodic eviction for inactive entries.
• Use a bounded strategy such as an LRU-style cap combined with sweeping, while ensuring active buckets are never incorrectly evicted.
• Keep the implementation safe for Node’s event-loop concurrency model.
• Add test-only bucket inspection/reset functionality similar to the existing resilience-state helpers.
• Add regression tests covering large numbers of idle keys, active-key preservation, eviction, and re-entry after eviction.
• Review the handling of X-Forwarded-For so the eviction work does not accidentally reinforce an unsafe client-controlled IP identity assumption.
• Update the rate limiter documentation to clearly describe its single-instance behavior and eviction policy.

I’ll validate that existing rate-limit semantics remain unchanged, particularly limited, retryAfter, and remaining, and run the required typecheck, lint, and test suites.

No blockers on my end. I’m ready to implement this as a focused performance fix without unrelated refactoring.

[grantfox-oss]

🦊 GrantFox — @TheWeirdDee has been assigned to this issue as part of the Third Campaign campaign!

Next steps:

1. Open a Pull Request referencing this issue (e.g., Closes #299)
2. Your PR will be reviewed by the Adamantine-guild maintainers

Good luck! Track your progress on GrantFox.

[grantfox-oss]

🎉 This issue has been marked as completed on GrantFox as part of the Third Campaign campaign!

@TheWeirdDee's PR #398 was approved and merged by @Lakes41.

🏆 @TheWeirdDee: You earned 35 FoxPoints for this contribution! Your current tier: Explorer (691 total points). Track your full progress on GrantFox.

👏 Great work, @TheWeirdDee! Keep contributing to Adamantine-guild.