code/cl8y-dex-terraclassic

Fork 0

Indexer rate limiter runs at 1/RPS of intended rate: per_second(rps) sets the replenish period, not requests-per-second #355

New issue

Closed

opened 2026-06-10 02:21:19 +00:00 by Brouie · 10 comments

Brouie commented

2026-06-10 02:21:19 +00:00

(Migrated from gitlab.com)

Summary

apply_rate_limit_layer (indexer/src/api/mod.rs:118-145) builds the governor with .per_second(rps).burst_size(rps*2). In tower_governor 0.8.0, per_second(n) means "replenish ONE token every n seconds" — it is the period, not the rate. Every configured limit therefore serves a burst of 2xRPS and then ONE request per RPS seconds:

lcd-heavy default "10 RPS" (RATE_LIMIT_LCD_HEAVY_RPS, config.rs:194-202) = burst 20, then 1 req / 10 s, keyed per client IP, one bucket shared across ALL lcd-heavy routes (order-book-head, limit-book + shallow + insert-hints, route/solve + /best, cg/cmc orderbook).
general "60 RPS" = burst 120, then 1 req / 60 s.

Live proof (LocalTerra, `83dc192`; RATE_LIMIT_RPS=0 so only the lcd-heavy layer was active)

30 rapid GETs to /api/v1/pairs/{addr}/limit-book?side=bid: exactly 20x 200 then 10x 429 (burst = 2x10); response headers x-ratelimit-limit: 20.
order-book-head ~10 s later: 1x 200 then 29x 429 — same bucket across lcd-heavy routes, barely refilled.
Probing 1 req/s after draining: one 200 at ~t+4s, the next ~10 s later — replenish measured at 1 token / 10 s. A drained bucket needs ~200 s to recover.
Contrast: 90 rapid GETs to /api/v1/pairs (general layer off at 0): 90x 200.

Impact

Production-facing: RUN_MODE=prod force-resets zeros back to 60/10 (config.rs:184-202), so any client behind one IP/NAT gets ~20 book/route-solve requests and then starvation at 1 per 10 s. The dapp's own trade-pair prefetch fires 5 parallel calls per pair click and 5 more per hovered option, plus book polling — it drains its own bucket within seconds of normal use. This produced live 429s and a degraded trade page during the #337 browser pass, and plausibly contributes to the symptoms reported in #350/#346.
QA/docs gotcha worth a line: RATE_LIMIT_RPS=0 disables only the general layer; lcd-heavy stays on via its own env default. Both must be 0 to fully disable (dev mode only).

Fix direction

Use the per-second semantics intentionally — e.g. construct the quota at Quota::per_second(rps) equivalents or .per_millisecond(1000 / rps) — keep burst_size, and add a test asserting sustained throughput ~= RPS over a few seconds. The existing 429 tests only assert that bursts get limited, which passes under both semantics, so they never caught the inversion.

Severity: High for production readiness (self-inflicted API starvation under normal dapp usage). Found working the SW/TR rows of #337.

cc @PlasticDigits

### Summary `apply_rate_limit_layer` (indexer/src/api/mod.rs:118-145) builds the governor with `.per_second(rps).burst_size(rps*2)`. In tower_governor 0.8.0, `per_second(n)` means "replenish ONE token every n seconds" — it is the period, not the rate. Every configured limit therefore serves a burst of 2xRPS and then ONE request per RPS seconds: - lcd-heavy default "10 RPS" (RATE_LIMIT_LCD_HEAVY_RPS, config.rs:194-202) = burst 20, then 1 req / 10 s, keyed per client IP, one bucket shared across ALL lcd-heavy routes (order-book-head, limit-book + shallow + insert-hints, route/solve + /best, cg/cmc orderbook). - general "60 RPS" = burst 120, then 1 req / 60 s. ### Live proof (LocalTerra, 83dc192; RATE_LIMIT_RPS=0 so only the lcd-heavy layer was active) - 30 rapid GETs to /api/v1/pairs/{addr}/limit-book?side=bid: exactly 20x 200 then 10x 429 (burst = 2x10); response headers `x-ratelimit-limit: 20`. - order-book-head ~10 s later: 1x 200 then 29x 429 — same bucket across lcd-heavy routes, barely refilled. - Probing 1 req/s after draining: one 200 at ~t+4s, the next ~10 s later — replenish measured at 1 token / 10 s. A drained bucket needs ~200 s to recover. - Contrast: 90 rapid GETs to /api/v1/pairs (general layer off at 0): 90x 200. ### Impact - Production-facing: RUN_MODE=prod force-resets zeros back to 60/10 (config.rs:184-202), so any client behind one IP/NAT gets ~20 book/route-solve requests and then starvation at 1 per 10 s. The dapp's own trade-pair prefetch fires 5 parallel calls per pair click and 5 more per hovered option, plus book polling — it drains its own bucket within seconds of normal use. This produced live 429s and a degraded trade page during the #337 browser pass, and plausibly contributes to the symptoms reported in #350/#346. - QA/docs gotcha worth a line: RATE_LIMIT_RPS=0 disables only the general layer; lcd-heavy stays on via its own env default. Both must be 0 to fully disable (dev mode only). ### Fix direction Use the per-second semantics intentionally — e.g. construct the quota at `Quota::per_second(rps)` equivalents or `.per_millisecond(1000 / rps)` — keep burst_size, and add a test asserting sustained throughput ~= RPS over a few seconds. The existing 429 tests only assert that bursts get limited, which passes under both semantics, so they never caught the inversion. Severity: High for production readiness (self-inflicted API starvation under normal dapp usage). Found working the SW/TR rows of #337. cc @PlasticDigits

Brouie commented

2026-06-10 02:22:59 +00:00

(Migrated from gitlab.com)

mentioned in issue #350

Brouie commented

2026-06-10 02:31:17 +00:00

(Migrated from gitlab.com)

mentioned in issue #337

PlasticDigits commented

2026-06-10 03:33:09 +00:00

(Migrated from gitlab.com)

mentioned in commit bdfeeef671

mentioned in commit bdfeeef671cdfee58dbb21b59825995005b1b7b5

PlasticDigits commented

2026-06-10 03:33:17 +00:00

(Migrated from gitlab.com)

mentioned in merge request !852

PlasticDigits commented

2026-06-10 03:33:17 +00:00

(Migrated from gitlab.com)

Implementation

MR opened on local1/355-impl-rate-limiter-rps.

Rate limiter now uses period(1s / RPS) instead of per_second(RPS). Added sustained-throughput test and documented that both env vars must be 0 to disable in dev.

Verification checklist

Rebuild and redeploy indexer container
/trade pair switch + book polling — no 429s under normal use
cargo test --test security rate_limit — all 4 tests pass
Prod defaults still 60 global / 10 LCD-heavy RPS

## Implementation MR opened on `local1/355-impl-rate-limiter-rps`. Rate limiter now uses `period(1s / RPS)` instead of `per_second(RPS)`. Added sustained-throughput test and documented that both env vars must be 0 to disable in dev. ### Verification checklist - [ ] Rebuild and redeploy indexer container - [ ] `/trade` pair switch + book polling — no 429s under normal use - [ ] `cargo test --test security rate_limit` — all 4 tests pass - [ ] Prod defaults still 60 global / 10 LCD-heavy RPS

PlasticDigits commented

2026-06-10 05:31:28 +00:00

(Migrated from gitlab.com)

mentioned in commit 0ee39be2a8

mentioned in commit 0ee39be2a83a5cfc76596aa4de3ce160f1650a70

PlasticDigits (Migrated from gitlab.com) closed this issue

2026-06-10 05:31:29 +00:00

Brouie commented

2026-06-10 06:29:46 +00:00

(Migrated from gitlab.com)

Verified the merged fix (!852) live on e198dcb. Rebuilt the indexer and ran it with RATE_LIMIT_LCD_HEAVY_RPS=10: after draining the burst and waiting ~2s, 10/10 requests in a ~1s window returned 200, where the pre-fix per_second-as-period bug would have allowed ~0-1 (1 token / 10s). The limiter now replenishes at true RPS. Confirmed lcd-heavy is a separate layer from the general RATE_LIMIT_RPS, as noted.

Verified the merged fix (!852) live on e198dcb. Rebuilt the indexer and ran it with RATE_LIMIT_LCD_HEAVY_RPS=10: after draining the burst and waiting ~2s, 10/10 requests in a ~1s window returned 200, where the pre-fix per_second-as-period bug would have allowed ~0-1 (1 token / 10s). The limiter now replenishes at true RPS. Confirmed lcd-heavy is a separate layer from the general RATE_LIMIT_RPS, as noted.

PlasticDigits commented

2026-06-12 05:18:39 +00:00

(Migrated from gitlab.com)

mentioned in merge request !873