fix(indexer): link limit fills to their own swap by per-pair ordinal (#316) #774

Merged

Brouie merged 1 commit from qa/316-fill-swap-linkage into main

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

Brouie commented

2026-06-05 06:50:16 +00:00

(Migrated from gitlab.com)

Follow-up to #287 — the fill->swap linkage I flagged as still using MIN(id).

process_limit_order_fill resolved the parent swap via swap_id_for_tx_pair = SELECT id ... ORDER BY id ASC LIMIT 1 — always the FIRST swap on the pair in the tx. With multiple swaps on the same pair in one tx (router revisit / batch), fills from the 2nd swap mis-linked to the 1st.

Fills carry no swap_index on-chain, so I derive it from the deterministic parser walk: maker fills are emitted before their swap action in the same execute (the pair adds book_fill_events, then the swap attribute), so a fill belongs to the upcoming swap on its pair. parse_limit_order_fills now tracks a per-pair swap counter that mirrors parse_swaps' one-swap-per-event detection exactly, and tags each fill with that ordinal. process_limit_order_fill links via swap_id_for_tx_pair_index(tx, pair, swap_index) — the unique (tx_hash, pair_id, swap_index) key from #287 — instead of MIN(id).

No schema change (link is the existing swap_event_id FK).
Single-swap txs unchanged (swap_index 0).
Historical multi-swap rows aren't backfilled (no stored fill ordinal); single-swap history was already correct, so the only un-fixable case is historical multi-swap-same-pair, which would need a re-index.

Tests: parse_limit_order_fills_assigns_swap_index_per_pair_swap (parser unit: two swaps one pair -> fills carry 0/1/1) + limit_fill_swap_linkage.rs (integration: each swap_index resolves to its own swap_events row, absent ordinal -> None).

Follow-up to #287 — the fill->swap linkage I flagged as still using MIN(id). `process_limit_order_fill` resolved the parent swap via `swap_id_for_tx_pair` = `SELECT id ... ORDER BY id ASC LIMIT 1` — always the FIRST swap on the pair in the tx. With multiple swaps on the same pair in one tx (router revisit / batch), fills from the 2nd swap mis-linked to the 1st. Fills carry no swap_index on-chain, so I derive it from the deterministic parser walk: maker fills are emitted before their `swap` action in the same execute (the pair adds `book_fill_events`, then the swap attribute), so a fill belongs to the upcoming swap on its pair. `parse_limit_order_fills` now tracks a per-pair swap counter that mirrors `parse_swaps`' one-swap-per-event detection exactly, and tags each fill with that ordinal. `process_limit_order_fill` links via `swap_id_for_tx_pair_index(tx, pair, swap_index)` — the unique `(tx_hash, pair_id, swap_index)` key from #287 — instead of MIN(id). - No schema change (link is the existing `swap_event_id` FK). - Single-swap txs unchanged (swap_index 0). - Historical multi-swap rows aren't backfilled (no stored fill ordinal); single-swap history was already correct, so the only un-fixable case is historical multi-swap-same-pair, which would need a re-index. Tests: `parse_limit_order_fills_assigns_swap_index_per_pair_swap` (parser unit: two swaps one pair -> fills carry 0/1/1) + `limit_fill_swap_linkage.rs` (integration: each swap_index resolves to its own swap_events row, absent ordinal -> None).

Brouie commented

2026-06-05 06:50:35 +00:00

(Migrated from gitlab.com)

mentioned in issue #316

PlasticDigits commented

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

(Migrated from gitlab.com)

Security review

Commit reviewed: f1caac4f50aef2ea2d075ccd775679959645e534
Scope: Indexer parser + DB query change linking limit_order_fills.swap_event_id via per-pair swap_index ordinal (swap_id_for_tx_pair_index), plus docs and integration tests.

Outcome: FINDINGS: 0 medium+

Summary: Reviewed the MR diff for injection, authn/authz, secret leakage, SSRF/XSS, deserialization, and supply-chain risk. This change is an internal indexer data-linkage correctness fix:

swap_id_for_tx_pair_index uses sqlx parameterized binds ($1–$3); no new SQL injection surface.
swap_index is derived deterministically inside parse_limit_order_fills from wasm event walk order (mirroring parse_swaps); it is not sourced from HTTP/API input.
Swaps are persisted before fills in process_block_txs, so the FK lookup runs after swap rows exist.
swap_event_id remains optional; a miss yields None rather than an incorrect forced link.
No HTTP route, auth, rate-limit, logging, or dependency changes.

No plausible medium+ attack path identified on this diff. Inline threads: none.

## Security review **Commit reviewed:** `f1caac4f50aef2ea2d075ccd775679959645e534` **Scope:** Indexer parser + DB query change linking `limit_order_fills.swap_event_id` via per-pair `swap_index` ordinal (`swap_id_for_tx_pair_index`), plus docs and integration tests. **Outcome:** `FINDINGS: 0` medium+ **Summary:** Reviewed the MR diff for injection, authn/authz, secret leakage, SSRF/XSS, deserialization, and supply-chain risk. This change is an internal indexer data-linkage correctness fix: - `swap_id_for_tx_pair_index` uses sqlx parameterized binds (`$1`–`$3`); no new SQL injection surface. - `swap_index` is derived deterministically inside `parse_limit_order_fills` from wasm event walk order (mirroring `parse_swaps`); it is not sourced from HTTP/API input. - Swaps are persisted before fills in `process_block_txs`, so the FK lookup runs after swap rows exist. - `swap_event_id` remains optional; a miss yields `None` rather than an incorrect forced link. - No HTTP route, auth, rate-limit, logging, or dependency changes. No plausible medium+ attack path identified on this diff. Inline threads: none.

PlasticDigits commented

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

(Migrated from gitlab.com)

mentioned in commit 8c591a47a4

mentioned in commit 8c591a47a458eaf0a23bf8f63f18254203243747