fix(comlink): prevent deep pagination from overloading primary DB

[2dpetkov]

Changelist

Why:

Over the weekend of 2026-07-10/11, a bot repeatedly deep-paginated /v4/fills and /v4/fundingPayments (page=97-111&limit=1000, i.e. OFFSET ~96,000-110,000).
Root-cause: ~12 repeated block-processing/Kafka-lag pages over ~54 hours to these OFFSET/LIMIT scans landing on the RDS primary instead of a read replica, driving DB load to 1.2-2.4x vCPU capacity and starving ender's write path.
A 30-day Datadog APM traffic audit (query-string page/limit facets on express.request spans) confirmed this wasn't a one-off:
live traffic on fundingPayments still sits at offset 91,000-149,000, the exact range that triggered the incident, plus separate extreme scraper tiers on fills reaching offsets in the millions.

How:

Three independent, layered mitigations:

1. Fixed the actual bug. fills, fundingPayments, trades, and trade-history controllers built a partial Postgres options object ({ orderBy: [...] }) whenever they added custom pagination ordering, which — because of how the JS default parameter (options: Options = DEFAULT_POSTGRES_OPTIONS) works — silently discarded the read-replica default and fell back to the primary.
   Confirmed via orders-controller.ts and historical-pnl-controller.ts, which already do this correctly ({ ...DEFAULT_POSTGRES_OPTIONS, orderBy: [...] }) — this is a consistency fix, not a new pattern.
   Also includes a lower-priority companion fix on transfers-controller.ts's /between endpoint (same missing spread, but no page param so no OFFSET-depth exposure).
2. Weighted the existing rate limiter by OFFSET depth. getPointCost now charges 1 + floor(offset / API_LIMIT_V4) points instead of a flat 1, reusing the existing RateLimit-* headers and 429 response — deep pagination drains a client's budget fast while normal shallow pagination is unaffected.
   Implemented centrally (getPointCost, shared by every rateLimiterMiddleware call site) rather than per-endpoint, since the same unbounded-.offset() pattern exists in 6+ other stores (order-table, pnl-ticks-table, transfer-table, etc.) that haven't been targeted yet but are equally exposed.
3. Added a hard ceiling as a backstop. New MAX_PAGINATION_OFFSET config (default 25,000), enforced via a cross-field validator on page/limit, returning the existing 400 { errors } shape with guidance to narrow the time-range filter instead of paging deeper.
   Value validated against real 30-day traffic: sits ~66,000 below the incident's own offset range with margin to spare, while only affecting an already-unusual slice of traffic (limit=500, pages 40-120 on fills).

Deliberately out of scope (called out for reviewers, not silently dropped): the COUNT(*) run on every paginated query (blocked by an existing test asserting accurate totalResults on page 2 — would be a client-facing contract change), and true cursor/keyset pagination (the real long-term fix for OFFSET cost, but a bigger API design change).

What:

• fix(comlink): route paginated fills/funding/trades/history reads to the read replica — fills-controller.ts, funding-payments-controller.ts, trade-history-controller.ts, trades-controller.ts, transfers-controller.ts, and their controller tests.
• feat(comlink): weight rate-limit cost by pagination offset depth — lib/rate-limit.ts and its test.
• feat(comlink): cap max pagination offset to bound query cost — config.ts, lib/validation/schemas.ts, lib/validation/helpers.ts (test harness route), and lib/validation/schemas.test.ts.

Test Plan

Full comlink Jest suite: 557/557 passing (NODE_ENV=test pnpm test, run against the dockerized postgres-test/redis containers).

New coverage added: read-replica regression guards on all four fixed controllers (asserts the findAll options argument is actually spread from the live DEFAULT_POSTGRES_OPTIONS object, independent of whatever USE_READ_REPLICA evaluates to in the test env); getPointCost unit tests covering the weighting formula, internal-IP bypass, and adversarial/garbage page values; offset-ceiling boundary tests (MAX_PAGINATION_OFFSET exactly vs. one page past it, and the default-limit case).

Caught and fixed one real bug during this verification pass: the offset-ceiling validator compared Number.isInteger() against the raw (string) query value instead of coercing first, so it never actually rejected anything — confirmed by two new tests failing 400-expected/200-actual, fixed, rebuilt, and reconfirmed.

Not yet done: a live E2E check against a running comlink process (only the infra containers — postgres/postgres-test/redis/kafka — were up during this pass, not the service itself).
Worth a manual curl smoke test against deep pagination (expect 400 past the ceiling, watch RateLimit-Remaining drop faster with deeper page) before merge if you want the extra confidence.

Author/Reviewer Checklist

• state-breaking — not applicable (comlink is the indexer's read API; no protocol/app state touched)
• indexer-postgres-breaking — not applicable (no schema changes, only query options/validation)
• proposal-breaking — not applicable
• feature label — not applicable (single self-contained fix)
• backport — your call depending on which release branches serve this traffic

Suggest adding the bug label (root-cause fix for the 2026-07-10/11 weekend paging incident).

---

Update: hardened after code review

A multi-angle review (correctness, security, test coverage, consistency) surfaced two real bugs in this PR's own new code, both now fixed:

• Validation bypass: a duplicate page query key (?page=1&page=999999999) parses as an array in Express; Number([...]) on that is NaN, and both the offset ceiling and the rate-limit weighter treated "can't parse it" as "some other validator will catch it" — but express-validator's isInt only checks an array's first element, so nothing actually did.
  The request went through with a NaN offset, which Knex silently drops, returning wrong (unpaginated) data instead of a 400.
  Fixed by explicitly rejecting non-scalar page values in the validator.
• Rate-limiter bypass: an oversized-but-valid page/limit could produce a pointCost large enough to serialize to exponential notation, which Redis's INCRBY rejects — tripping an existing fail-open branch in the rate-limit middleware that silently lets the request through with zero rate limiting applied.
  Before this PR, pointCost was always 0 or 1, so this path was unreachable; fixed by clamping cost to the rate limiter's own point capacity.

Also: the offset-ceiling error message no longer echoes the computed offset or the exact MAX_PAGINATION_OFFSET value back to the client (kept the config name itself in the message as a debugging clue, dropped the numbers, so the ceiling can't be read straight off a 400 response).

Test suite is now 565/565 passing, including new coverage for both bugs above, backfilled read-replica regression tests for transfers-controller.ts and the parentSubaccount route variants (previously untested), and one end-to-end test replaying a deep-pagination request through the real middleware chain against /v4/fundingPayments.

Surfaced but out of scope for this PR (flagged for follow-up, not fixed here): isIndexerIp trusts cf-connecting-ip/x-forwarded-for unconditionally with no code in this repo confirming comlink's origin is locked to Cloudflare-only traffic — if it isn't, this (pre-existing, unrelated to this PR) gap bypasses rate limiting entirely, not just the new pagination-depth weighting. Worth a separate ticket to whoever owns the infra layer.

Summary by CodeRabbit

• New Features

  • Added a configurable maximum pagination offset, defaulting to 25,000.
  • Added validation to reject requests that exceed the allowed pagination depth.
  • Deep pagination now consumes more rate-limit capacity, while internal requests remain unaffected.

• Bug Fixes

  • Improved consistency and reliability of paginated data retrieval across fills, trades, funding payments, trade history, and transfers endpoints.

• Tests

  • Added regression coverage for pagination query handling, offset validation, and pagination-aware rate limiting.

[coderabbitai]

📝 Walkthrough

Walkthrough

Pagination validation now caps computed offsets, rate limits scale with pagination depth, and paginated Comlink database reads preserve default Postgres options. Tests cover validation boundaries, rate-limit behavior, deep-page rejection, and runtime query-option propagation.

Changes

Pagination controls and database reads

Layer / File(s)	Summary
Pagination offset validation indexer/services/comlink/src/config.ts, indexer/services/comlink/src/lib/validation/*, indexer/services/comlink/__tests__/lib/validation/*	Adds MAX_PAGINATION_OFFSET, validates page and limit combinations, rejects excessive offsets and repeated page parameters, and tests boundary behavior.
Pagination-aware rate limiting indexer/services/comlink/src/lib/rate-limit.ts, indexer/services/comlink/__tests__/lib/rate-limit.test.ts	Calculates point costs from pagination depth, handles invalid and internal requests, clamps excessive costs, and tests middleware behavior.
Paginated Postgres query options indexer/services/comlink/src/controllers/api/v4/*-controller.ts	Merges DEFAULT_POSTGRES_OPTIONS into paginated fills, funding payments, trade history, trades, and transfers queries.
Query option regression guards indexer/services/comlink/__tests__/controllers/api/v4/*-controller.test.ts	Verifies paginated controller queries receive runtime-spread default Postgres options and rejects an excessively deep funding-payments request.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

• dydxprotocol/v4-chain#1420: Modifies overlapping pagination controller paths.
• dydxprotocol/v4-chain#2790: Modifies the fills parent-subaccount pagination query path.
• dydxprotocol/v4-chain#2900: Modifies the funding-payments pagination flow.

Suggested reviewers: shrenujb, anmolagrawal345, affanv14, christopher-li, tqin7

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
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.
Title check	✅ Passed	The title clearly matches the main change: hardening Comlink against deep pagination overloading the primary database.
Description check	✅ Passed	The description includes Changelist, Test Plan, and checklist sections and covers the required changes and testing.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.}

[mergify]

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

• Queue this pull request