code/cl8y-dex-terraclassic

Fork 0

/traders/leaderboard sorts on unindexed columns (seq scan + sort per request) #280

New issue

Closed

opened 2026-06-03 07:12:05 +00:00 by Brouie · 9 comments

Brouie commented

2026-06-03 07:12:05 +00:00

(Migrated from gitlab.com)

Severity: Medium
Reachability: Unauthenticated HTTP — /api/v1/traders/leaderboard.
Affected: get_leaderboard (indexer/src/db/queries/traders.rs) + missing indexes on traders.
Root cause: the leaderboard sorts on volume_24h / volume_7d / volume_30d / total_trades, and none of those columns have an index.

Summary

get_leaderboard builds SELECT * FROM traders ORDER BY {order_col} DESC LIMIT $1, where order_col is one of volume_24h, volume_7d, volume_30d, total_trades, plus the PnL/fees columns. The PnL/fees columns and total_volume_usd are indexed — but volume_24h, volume_7d, volume_30d, and total_trades are not. The default UI sort is almost certainly 24h volume, i.e. the unindexed path.

So the common case is a full sequential scan of traders plus a top-N sort, on every public request, and it gets worse as the trader table grows. (order_col is whitelisted through a match, so there's no injection — purely the index gap.)

Current codebase

traders.rs get_leaderboard: format!("SELECT * FROM traders ORDER BY {} DESC LIMIT $1", order_col).
migrations: indexes exist on total_volume_usd, tier_id, and the four PnL columns; none on volume_24h / volume_7d / volume_30d / total_trades.

Recommended direction

Add btree indexes: traders(volume_24h DESC), (volume_7d DESC), (volume_30d DESC), (total_trades DESC).
Consider a short-TTL cache on the leaderboard response since it doesn't need to be real-time.

Acceptance criteria

Each sortable leaderboard column is index-backed (EXPLAIN shows an index scan + limit, no full sort).
Leaderboard latency stays flat as the trader table grows.

**Severity:** Medium **Reachability:** Unauthenticated HTTP — `/api/v1/traders/leaderboard`. **Affected:** `get_leaderboard` (`indexer/src/db/queries/traders.rs`) + missing indexes on `traders`. **Root cause:** the leaderboard sorts on `volume_24h` / `volume_7d` / `volume_30d` / `total_trades`, and none of those columns have an index. ## Summary `get_leaderboard` builds `SELECT * FROM traders ORDER BY {order_col} DESC LIMIT $1`, where `order_col` is one of `volume_24h`, `volume_7d`, `volume_30d`, `total_trades`, plus the PnL/fees columns. The PnL/fees columns and `total_volume_usd` are indexed — but `volume_24h`, `volume_7d`, `volume_30d`, and `total_trades` are not. The default UI sort is almost certainly 24h volume, i.e. the unindexed path. So the common case is a full sequential scan of `traders` plus a top-N sort, on every public request, and it gets worse as the trader table grows. (`order_col` is whitelisted through a match, so there's no injection — purely the index gap.) ## Current codebase - `traders.rs` `get_leaderboard`: `format!("SELECT * FROM traders ORDER BY {} DESC LIMIT $1", order_col)`. - migrations: indexes exist on `total_volume_usd`, `tier_id`, and the four PnL columns; none on `volume_24h` / `volume_7d` / `volume_30d` / `total_trades`. ## Recommended direction 1. Add btree indexes: `traders(volume_24h DESC)`, `(volume_7d DESC)`, `(volume_30d DESC)`, `(total_trades DESC)`. 2. Consider a short-TTL cache on the leaderboard response since it doesn't need to be real-time. ## Acceptance criteria - [ ] Each sortable leaderboard column is index-backed (`EXPLAIN` shows an index scan + limit, no full sort). - [ ] Leaderboard latency stays flat as the trader table grows.

PlasticDigits commented

2026-06-03 10:47:38 +00:00

(Migrated from gitlab.com)

Approved both btree and ttl cache, can be 1 minute

Brouie commented

2026-06-04 03:20:55 +00:00

(Migrated from gitlab.com)

Fixed — both parts you approved.

Indexes (migration 20260604120000_traders_leaderboard_sort_indexes.sql): btree on traders(volume_24h DESC), (volume_7d DESC), (volume_30d DESC), (total_trades DESC), matching the existing idx_traders_volume format. (The PnL/fees + total_volume sort columns were already indexed.)

Cache: 60s TTL on the /traders/leaderboard response keyed by (sort_by, limit), modeled on the existing route_solver module-level cache (OnceLock<Mutex> + retain/evict).

Proven live on the QA Postgres (forced enable_seqscan=off since the localnet traders table is tiny, to make the planner reveal whether an index path exists):

BEFORE:  Limit -> Sort (Sort Key: volume_24h DESC) -> Seq Scan on traders   (cost 1e10)
AFTER:   Limit -> Index Scan using idx_traders_volume_24h on traders        (cost 12.16, no Sort node)

So the full top-N sort is gone and the query rides the index. Per the AC, EXPLAIN shows index scan + limit (no full sort); the 60s cache bounds latency under a request burst.

cargo check clean. Branch qa/280-leaderboard-index-cache, MR fork→main (no closing keyword). @PlasticDigits

Fixed — both parts you approved. **Indexes** (migration `20260604120000_traders_leaderboard_sort_indexes.sql`): btree on `traders(volume_24h DESC)`, `(volume_7d DESC)`, `(volume_30d DESC)`, `(total_trades DESC)`, matching the existing `idx_traders_volume` format. (The PnL/fees + total_volume sort columns were already indexed.) **Cache**: 60s TTL on the `/traders/leaderboard` response keyed by `(sort_by, limit)`, modeled on the existing route_solver module-level cache (OnceLock<Mutex<HashMap>> + retain/evict). Proven live on the QA Postgres (forced `enable_seqscan=off` since the localnet `traders` table is tiny, to make the planner reveal whether an index path exists): ``` BEFORE: Limit -> Sort (Sort Key: volume_24h DESC) -> Seq Scan on traders (cost 1e10) AFTER: Limit -> Index Scan using idx_traders_volume_24h on traders (cost 12.16, no Sort node) ``` So the full top-N sort is gone and the query rides the index. Per the AC, EXPLAIN shows index scan + limit (no full sort); the 60s cache bounds latency under a request burst. cargo check clean. Branch `qa/280-leaderboard-index-cache`, MR fork→main (no closing keyword). @PlasticDigits

Brouie commented

2026-06-04 03:20:57 +00:00

(Migrated from gitlab.com)

mentioned in merge request !741

PlasticDigits commented

2026-06-04 08:02:47 +00:00

(Migrated from gitlab.com)

mentioned in commit 4a95548d53

mentioned in commit 4a95548d53adce4af34fb00d0e2883564e2aba3f

Brouie commented

2026-06-05 01:21:10 +00:00

(Migrated from gitlab.com)

Verified both halves — the four sort indexes and the 60s cache.

AC1 (each sortable column index-backed, EXPLAIN shows index scan + limit, no full sort): the migration created btree on traders(volume_24h/7d/30d DESC) and (total_trades DESC), live in the DB. EXPLAIN on the leaderboard query (with enable_seqscan=off, since the seeded traders table is one row so the planner won't reach for an index on its own) shows Limit -> Index Scan using idx_traders_volume_24h with no Sort node, same for volume_7d / volume_30d / total_trades; worst_trade DESC rides the ASC index backward. The per-request top-N sort is gone on every sortable column.

AC2 (latency flat as the table grows): the index removes the sort and the 60s cache (keyed by sort+limit) bounds it under a burst — repeat calls serve byte-identical from cache. Invalid sort key is rejected at the whitelist (400) before the DB.

@PlasticDigits good to close.

Verified both halves — the four sort indexes and the 60s cache. AC1 (each sortable column index-backed, EXPLAIN shows index scan + limit, no full sort): the migration created btree on `traders(volume_24h/7d/30d DESC)` and `(total_trades DESC)`, live in the DB. EXPLAIN on the leaderboard query (with `enable_seqscan=off`, since the seeded `traders` table is one row so the planner won't reach for an index on its own) shows `Limit -> Index Scan using idx_traders_volume_24h` with no Sort node, same for volume_7d / volume_30d / total_trades; worst_trade DESC rides the ASC index backward. The per-request top-N sort is gone on every sortable column. AC2 (latency flat as the table grows): the index removes the sort and the 60s cache (keyed by sort+limit) bounds it under a burst — repeat calls serve byte-identical from cache. Invalid sort key is rejected at the whitelist (400) before the DB. @PlasticDigits good to close.

PlasticDigits commented

2026-06-05 04:01:57 +00:00

(Migrated from gitlab.com)

Verification (Cloud Agent) — GitLab #280

Verified fix on main (9f0babe) — migration 20260604120000_traders_leaderboard_sort_indexes.sql and 60s leaderboard cache in indexer/src/api/traders.rs are present. No repo changes required.

Acceptance criteria

Item	Result	How verified
AC1 — Each sortable leaderboard column is index-backed; `EXPLAIN` shows index scan + limit, no full sort	PASS	Postgres `dex_indexer` + `dex_indexer_test` with `SET enable_seqscan = off`. All nine `ORDER BY … DESC LIMIT 50` plans are `Limit → Index Scan` (or `Index Scan Backward` for `worst_trade_pnl` on `idx_traders_worst_trade`); no `Sort` node. New indexes: `idx_traders_volume_24h`, `_7d`, `_30d`, `_total_trades`.
AC2 — Leaderboard latency stays flat as trader table grows	PASS (design + smoke)	Index removes top-N sort; 60s TTL cache keyed `sort_by\|limit` (`LEADERBOARD_CACHE_TTL = 60s`, mirrors route_solver pattern). Two rapid `GET /api/v1/traders/leaderboard?sort=volume_24h&limit=5` responses byte-identical. Full load test not run (table has 0 rows on `dex_indexer`).

Additional checks

Check	Result	Command / output
Migration applied on live DB	PASS	`_sqlx_migrations` contains `20260604120000`; `\di idx_traders_volume_24h` exists
Invalid sort rejected before DB	PASS	`curl …?sort=hacked_column` → 400 with whitelist message
All documented sort columns	PASS	All nine `sort=` values → 200 on running indexer (`:3001`)
Automated tests	PASS	`cargo test -p cl8y-dex-indexer --test api_traders leaderboard` (4/4); `cargo test -p cl8y-dex-indexer --test security leaderboard_all_documented_sort_columns_accepted`; `cargo check`; `cargo test --lib` (98/98)
SQL injection guard	PASS	`order_col` whitelisted in `get_leaderboard` (`traders.rs`); API `VALID_SORTS` rejects unknown sorts

Infrastructure used

Docker Postgres (make-style compose postgres on :5432)
Indexer started briefly with minimal env for HTTP smoke (FACTORY_ADDRESS dummy, POLL_INTERVAL_MS=600000)

Closing as verified on main — no MR from this pass.

## Verification (Cloud Agent) — GitLab #280 Verified fix on `main` (`9f0babe`) — migration `20260604120000_traders_leaderboard_sort_indexes.sql` and 60s leaderboard cache in `indexer/src/api/traders.rs` are present. No repo changes required. ### Acceptance criteria | Item | Result | How verified | |------|--------|--------------| | **AC1** — Each sortable leaderboard column is index-backed; `EXPLAIN` shows index scan + limit, no full sort | **PASS** | Postgres `dex_indexer` + `dex_indexer_test` with `SET enable_seqscan = off`. All nine `ORDER BY … DESC LIMIT 50` plans are `Limit → Index Scan` (or `Index Scan Backward` for `worst_trade_pnl` on `idx_traders_worst_trade`); no `Sort` node. New indexes: `idx_traders_volume_24h`, `_7d`, `_30d`, `_total_trades`. | | **AC2** — Leaderboard latency stays flat as trader table grows | **PASS** (design + smoke) | Index removes top-N sort; 60s TTL cache keyed `sort_by\|limit` (`LEADERBOARD_CACHE_TTL = 60s`, mirrors route_solver pattern). Two rapid `GET /api/v1/traders/leaderboard?sort=volume_24h&limit=5` responses byte-identical. Full load test not run (table has 0 rows on `dex_indexer`). | ### Additional checks | Check | Result | Command / output | |-------|--------|------------------| | Migration applied on live DB | **PASS** | `_sqlx_migrations` contains `20260604120000`; `\di idx_traders_volume_24h` exists | | Invalid sort rejected before DB | **PASS** | `curl …?sort=hacked_column` → **400** with whitelist message | | All documented sort columns | **PASS** | All nine `sort=` values → **200** on running indexer (`:3001`) | | Automated tests | **PASS** | `cargo test -p cl8y-dex-indexer --test api_traders leaderboard` (4/4); `cargo test -p cl8y-dex-indexer --test security leaderboard_all_documented_sort_columns_accepted`; `cargo check`; `cargo test --lib` (98/98) | | SQL injection guard | **PASS** | `order_col` whitelisted in `get_leaderboard` (`traders.rs`); API `VALID_SORTS` rejects unknown sorts | ### Infrastructure used - Docker Postgres (`make`-style compose postgres on `:5432`) - Indexer started briefly with minimal env for HTTP smoke (`FACTORY_ADDRESS` dummy, `POLL_INTERVAL_MS=600000`) Closing as verified on `main` — no MR from this pass.

PlasticDigits (Migrated from gitlab.com) closed this issue

2026-06-05 04:02:10 +00:00

PlasticDigits commented

2026-08-26 03:08:07 +00:00

(Migrated from gitlab.com)

mentioned in issue #657