Fix: limit_order_fill → swap_events linkage uses MIN(id) (GitLab #287 followup) #316

Closed
opened 2026-06-05 04:08:29 +00:00 by PlasticDigits · 17 comments
PlasticDigits commented 2026-06-05 04:08:29 +00:00 (Migrated from gitlab.com)

Current codebase

GitLab #287 added swap_index to swap_events — multiple swaps per (tx_hash, pair_id) now stored with unique (tx_hash, pair_id, swap_index) (migration 20260605000000_swap_events_per_tx_pair_swap_index.sql). Parser assigns swap_index per pair in wasm walk order (parser.rs).

Limit fill linkage still broken: process_limit_order_fill calls:

let swap_event_id = limit_order_fills::swap_id_for_tx_pair(pool, tx_hash, pair.id).await?;

swap_id_for_tx_pair (indexer/src/db/queries/limit_order_fills.rs):

SELECT id FROM swap_events WHERE tx_hash = $1 AND pair_id = $2 ORDER BY id ASC LIMIT 1

This always links fills to the first swap row (MIN internal id), not the swap that produced the fill when a tx has multiple swaps on the same pair (router revisit, batch).

limit_order_fill wasm events do not currently emit swap_index.

Follow-up: GitLab #287.

Why this is needed

Incorrect swap_event_id breaks:

  • Per-fill analytics tied to swap attrs (book_input, expired_parks_*)
  • Integrator volume attribution (headline volume vs fill detail)
  • QA invariants linking limit fills to hybrid swap rows

Constraints / guardrails

  • Prefer attributing fills to correct swap by event ordering within same tx/pair if on-chain swap_index attr not available.
  • If contract change needed, add swap_index (or swap_event_index) to limit_order_fill_event attrs — coordinate wasm + indexer + backward compat.
  • Backfill migration for historical rows: best-effort (single-swap txs unaffected).
  • Do not break fill_exists dedup key (tx_hash, pair_id, order_id).

Relevant files

Area Path
Linkage query indexer/src/db/queries/limit_order_fills.rs
Parser indexer/src/indexer/parser.rs (process_limit_order_fill, parse_swaps)
Swap queries indexer/src/db/queries/swap_events.rs
Pair contract events smartcontracts/contracts/pair/src/orderbook.rs (limit_order_fill_event)
Migration indexer/migrations/20260605000000_swap_events_per_tx_pair_swap_index.sql
Docs docs/integrators-hybrid-volume.md
  1. Short term (indexer): When processing fills, track per-(tx,pair) swap counter shared with swap parser pass — link fill to swap with matching ordinal in same tx processing batch.
  2. Long term (contract): Emit swap_index on limit_order_fill events during hybrid execute for durable LCD replay.
  3. Replace swap_id_for_tx_pair with swap_id_for_tx_pair_index(tx, pair, swap_index).
  4. Add integration test: one tx, two swaps same pair, fills attach to second swap.

Acceptance criteria

  • Multi-swap same-pair tx: each fill's swap_event_id points to correct swap_events row.
  • Single-swap txs: behavior unchanged (swap_index 0).
  • Test coverage in indexer integration tests.
  • Integrator doc notes fill ↔ swap linkage semantics.

Test plan (all paths)

Path Expected
1 swap, N fills All link to swap_index 0
2 swaps same pair, fills on 2nd only swap_event_id = second row
Pool-only swap + no fills N/A
Replay indexer block Idempotent linkage

Attack / abuse / hack vectors

Vector Note
Mis-linked volume double count Correct FK prevents wrong join
Forged fill events (#285) Emitter scoping separate

Verification criteria

  • Indexer integration test with synthetic multi-swap tx fixture.
  • SQL spot check: JOIN swap_events ON swap_event_id = swap_events.id matches swap_index ordering.
  • cargo test indexer suite green.
## Current codebase GitLab **#287** added `swap_index` to `swap_events` — multiple swaps per `(tx_hash, pair_id)` now stored with unique `(tx_hash, pair_id, swap_index)` (migration `20260605000000_swap_events_per_tx_pair_swap_index.sql`). Parser assigns `swap_index` per pair in wasm walk order (`parser.rs`). **Limit fill linkage still broken:** `process_limit_order_fill` calls: ```rust let swap_event_id = limit_order_fills::swap_id_for_tx_pair(pool, tx_hash, pair.id).await?; ``` `swap_id_for_tx_pair` (`indexer/src/db/queries/limit_order_fills.rs`): ```sql SELECT id FROM swap_events WHERE tx_hash = $1 AND pair_id = $2 ORDER BY id ASC LIMIT 1 ``` This always links fills to the **first** swap row (MIN internal id), not the swap that produced the fill when a tx has multiple swaps on the same pair (router revisit, batch). `limit_order_fill` wasm events do **not** currently emit `swap_index`. Follow-up: GitLab **#287**. ## Why this is needed Incorrect `swap_event_id` breaks: - Per-fill analytics tied to swap attrs (`book_input`, `expired_parks_*`) - Integrator volume attribution (headline volume vs fill detail) - QA invariants linking limit fills to hybrid swap rows ## Constraints / guardrails - Prefer attributing fills to correct swap by **event ordering** within same tx/pair if on-chain `swap_index` attr not available. - If contract change needed, add `swap_index` (or `swap_event_index`) to `limit_order_fill_event` attrs — coordinate wasm + indexer + backward compat. - Backfill migration for historical rows: best-effort (single-swap txs unaffected). - Do not break `fill_exists` dedup key `(tx_hash, pair_id, order_id)`. ## Relevant files | Area | Path | |------|------| | Linkage query | `indexer/src/db/queries/limit_order_fills.rs` | | Parser | `indexer/src/indexer/parser.rs` (`process_limit_order_fill`, `parse_swaps`) | | Swap queries | `indexer/src/db/queries/swap_events.rs` | | Pair contract events | `smartcontracts/contracts/pair/src/orderbook.rs` (`limit_order_fill_event`) | | Migration | `indexer/migrations/20260605000000_swap_events_per_tx_pair_swap_index.sql` | | Docs | `docs/integrators-hybrid-volume.md` | ## Recommended direction 1. **Short term (indexer):** When processing fills, track per-(tx,pair) swap counter shared with swap parser pass — link fill to swap with matching ordinal in same tx processing batch. 2. **Long term (contract):** Emit `swap_index` on `limit_order_fill` events during hybrid execute for durable LCD replay. 3. Replace `swap_id_for_tx_pair` with `swap_id_for_tx_pair_index(tx, pair, swap_index)`. 4. Add integration test: one tx, two swaps same pair, fills attach to second swap. ## Acceptance criteria - [ ] Multi-swap same-pair tx: each fill's `swap_event_id` points to correct `swap_events` row. - [ ] Single-swap txs: behavior unchanged (swap_index 0). - [ ] Test coverage in indexer integration tests. - [ ] Integrator doc notes fill ↔ swap linkage semantics. ## Test plan (all paths) | Path | Expected | |------|----------| | 1 swap, N fills | All link to swap_index 0 | | 2 swaps same pair, fills on 2nd only | swap_event_id = second row | | Pool-only swap + no fills | N/A | | Replay indexer block | Idempotent linkage | ## Attack / abuse / hack vectors | Vector | Note | |--------|------| | Mis-linked volume double count | Correct FK prevents wrong join | | Forged fill events (#285) | Emitter scoping separate | ## Verification criteria - Indexer integration test with synthetic multi-swap tx fixture. - SQL spot check: `JOIN swap_events ON swap_event_id = swap_events.id` matches `swap_index` ordering. - `cargo test` indexer suite green.
PlasticDigits commented 2026-06-05 04:08:30 +00:00 (Migrated from gitlab.com)

marked as related to #287

marked as related to #287
Brouie commented 2026-06-05 06:50:19 +00:00 (Migrated from gitlab.com)

mentioned in merge request !774

mentioned in merge request !774
Brouie commented 2026-06-05 06:50:35 +00:00 (Migrated from gitlab.com)

Took this — it's the MIN(id) linkage I flagged on #287. MR !774.

process_limit_order_fill was resolving the parent swap with swap_id_for_tx_pair = SELECT id ... ORDER BY id ASC LIMIT 1, so every fill linked to the FIRST swap on the pair. Two swaps on one pair in a tx (router revisit / batch) -> the 2nd swap's fills mis-linked to the 1st.

Fills don't carry 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 keeps a per-pair swap counter that mirrors parse_swaps' one-swap-per-event detection exactly (so the ordinals line up with the persisted swap_events.swap_index), and tags each fill. process_limit_order_fill then links via swap_id_for_tx_pair_index(tx, pair, swap_index) on the unique (tx_hash, pair_id, swap_index) key from #287, not MIN(id).

No schema change (the link is the existing swap_event_id FK). Single-swap txs unchanged (index 0). Historical multi-swap-same-pair rows can't be backfilled (fills don't store an ordinal) — single-swap history was already correct, so only historical multi-swap would need a re-index; flagging that rather than shipping a migration that can't actually fix it.

Tests: parse_limit_order_fills_assigns_swap_index_per_pair_swap (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). Verified green.

Separate heads-up while I was in here: tests/limit_order_parked_lifecycle.rs::park_event_then_claim_updates_db_and_api_filters is RED on main (fails with my changes stashed too). Its wasm_park_tx/wasm_claim_tx fixtures still use the unreserved contract_address key, but #285 (e951e61) scoped lifecycle events to the reserved _contract_address only — so the park event never matches and the order stays active. Test-only fixture staleness from the #285 merge; I'll put up a small fix for it. @PlasticDigits

Took this — it's the MIN(id) linkage I flagged on #287. MR !774. process_limit_order_fill was resolving the parent swap with swap_id_for_tx_pair = SELECT id ... ORDER BY id ASC LIMIT 1, so every fill linked to the FIRST swap on the pair. Two swaps on one pair in a tx (router revisit / batch) -> the 2nd swap's fills mis-linked to the 1st. Fills don't carry 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 keeps a per-pair swap counter that mirrors parse_swaps' one-swap-per-event detection exactly (so the ordinals line up with the persisted swap_events.swap_index), and tags each fill. process_limit_order_fill then links via swap_id_for_tx_pair_index(tx, pair, swap_index) on the unique (tx_hash, pair_id, swap_index) key from #287, not MIN(id). No schema change (the link is the existing swap_event_id FK). Single-swap txs unchanged (index 0). Historical multi-swap-same-pair rows can't be backfilled (fills don't store an ordinal) — single-swap history was already correct, so only historical multi-swap would need a re-index; flagging that rather than shipping a migration that can't actually fix it. Tests: parse_limit_order_fills_assigns_swap_index_per_pair_swap (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). Verified green. Separate heads-up while I was in here: tests/limit_order_parked_lifecycle.rs::park_event_then_claim_updates_db_and_api_filters is RED on main (fails with my changes stashed too). Its wasm_park_tx/wasm_claim_tx fixtures still use the unreserved contract_address key, but #285 (e951e61) scoped lifecycle events to the reserved _contract_address only — so the park event never matches and the order stays active. Test-only fixture staleness from the #285 merge; I'll put up a small fix for it. @PlasticDigits
Brouie commented 2026-06-05 06:52:09 +00:00 (Migrated from gitlab.com)

mentioned in merge request !775

mentioned in merge request !775
Brouie commented 2026-06-05 06:52:23 +00:00 (Migrated from gitlab.com)

mentioned in issue #285

mentioned in issue #285
PlasticDigits commented 2026-06-05 07:03:49 +00:00 (Migrated from gitlab.com)

mentioned in commit 8c591a47a4

mentioned in commit 8c591a47a458eaf0a23bf8f63f18254203243747
PlasticDigits commented 2026-06-05 09:47:03 +00:00 (Migrated from gitlab.com)

mentioned in merge request !780

mentioned in merge request !780
PlasticDigits commented 2026-06-05 09:47:22 +00:00 (Migrated from gitlab.com)

mentioned in issue #287

mentioned in issue #287
PlasticDigits commented 2026-06-05 09:55:26 +00:00 (Migrated from gitlab.com)

mentioned in merge request !783

mentioned in merge request !783
ghost1 commented 2026-06-05 12:41:22 +00:00 (Migrated from gitlab.com)

mentioned in commit a121e8a7fd

mentioned in commit a121e8a7fd0ea97a86c448749d020ab8f819b00f
PlasticDigits commented 2026-06-05 12:41:53 +00:00 (Migrated from gitlab.com)

mentioned in merge request !804

mentioned in merge request !804
PlasticDigits commented 2026-06-05 12:41:59 +00:00 (Migrated from gitlab.com)

Implementation verification — #316

Status

The indexer fix was already merged to main in f1caac4 (MR qa/316-fill-swap-linkage). This pass adds the remaining integrator documentation and re-verifies all acceptance criteria.

MR: !804 — docs: fill↔swap linkage by swap_index for integrators (#316)

What changed (code — already on main)

  • swap_id_for_tx_pair (ORDER BY id ASC LIMIT 1) replaced by swap_id_for_tx_pair_index(tx, pair, swap_index) keyed on (tx_hash, pair_id, swap_index).
  • parse_limit_order_fills assigns per-pair swap_index from parser walk order (fills precede their swap action).
  • process_limit_order_fill links via the fill's swap_index.

What changed (this branch — docs only)

  • docs/integrators-hybrid-volume.md — Fill ↔ swap linkage section
  • skills/AGENTS_INTEGRATOR_HYBRID_VOLUME.md — cross-link + test commands

Acceptance checklist

Criterion Command / step Result
Multi-swap same-pair tx: each fill's swap_event_id → correct swap_events row cd indexer && cargo test --test limit_fill_swap_linkage -- --test-threads=1 PASS
Single-swap txs unchanged (swap_index 0) cd indexer && cargo test parse_limit_order_fills_assigns_swap_index --lib PASS
Indexer integration test coverage indexer/tests/limit_fill_swap_linkage.rs PASS
Integrator doc notes fill ↔ swap linkage docs/integrators-hybrid-volume.md § Fill ↔ swap linkage (+ indexer-invariants.md limit fill rows row) PASS

Verification commands (third parties)

cd indexer
cargo test parse_limit_order_fills_assigns_swap_index --lib
cargo test --test limit_fill_swap_linkage -- --test-threads=1

Follow-ups (not blockers)

  • Contract (long term): emit swap_index on limit_order_fill wasm events for durable LCD replay.
  • Backfill: historical multi-swap mis-links are best-effort only; single-swap history was already correct.
## Implementation verification — [#316](https://gitlab.com/PlasticDigits/cl8y-dex-terraclassic/-/work_items/316) ### Status The **indexer fix** was already merged to `main` in `f1caac4` (MR `qa/316-fill-swap-linkage`). This pass adds the remaining **integrator documentation** and re-verifies all acceptance criteria. **MR:** [!804 — docs: fill↔swap linkage by swap_index for integrators (#316)](https://gitlab.com/PlasticDigits/cl8y-dex-terraclassic/-/merge_requests/86) ### What changed (code — already on `main`) - `swap_id_for_tx_pair` (`ORDER BY id ASC LIMIT 1`) replaced by `swap_id_for_tx_pair_index(tx, pair, swap_index)` keyed on `(tx_hash, pair_id, swap_index)`. - `parse_limit_order_fills` assigns per-pair `swap_index` from parser walk order (fills precede their `swap` action). - `process_limit_order_fill` links via the fill's `swap_index`. ### What changed (this branch — docs only) - `docs/integrators-hybrid-volume.md` — **Fill ↔ swap linkage** section - `skills/AGENTS_INTEGRATOR_HYBRID_VOLUME.md` — cross-link + test commands ### Acceptance checklist | Criterion | Command / step | Result | |-----------|----------------|--------| | Multi-swap same-pair tx: each fill's `swap_event_id` → correct `swap_events` row | `cd indexer && cargo test --test limit_fill_swap_linkage -- --test-threads=1` | **PASS** | | Single-swap txs unchanged (`swap_index` 0) | `cd indexer && cargo test parse_limit_order_fills_assigns_swap_index --lib` | **PASS** | | Indexer integration test coverage | `indexer/tests/limit_fill_swap_linkage.rs` | **PASS** | | Integrator doc notes fill ↔ swap linkage | `docs/integrators-hybrid-volume.md` § Fill ↔ swap linkage (+ `indexer-invariants.md` limit fill rows row) | **PASS** | ### Verification commands (third parties) ```bash cd indexer cargo test parse_limit_order_fills_assigns_swap_index --lib cargo test --test limit_fill_swap_linkage -- --test-threads=1 ``` ### Follow-ups (not blockers) - **Contract (long term):** emit `swap_index` on `limit_order_fill` wasm events for durable LCD replay. - **Backfill:** historical multi-swap mis-links are best-effort only; single-swap history was already correct.
PlasticDigits commented 2026-06-05 12:43:06 +00:00 (Migrated from gitlab.com)

mentioned in commit 22fcfdf310

mentioned in commit 22fcfdf31060c18ad1759af7614f685779ddab35
PlasticDigits commented 2026-06-05 13:38:30 +00:00 (Migrated from gitlab.com)

Verification — #316

Independent QA pass on branch cursor/gitlab-issue-verification-b68c (includes merged indexer fix MR !774 f1caac4 and docs MR !804).

Acceptance criteria

Criterion How verified Result
Multi-swap same-pair tx: each fill's swap_event_id → correct swap_events row cd indexer && cargo test --test limit_fill_swap_linkage -- --test-threads=1 — fill_links_to_its_own_swap_not_the_first_on_the_pair resolves swap_index 0 → first row, swap_index 1 → second row (not MIN(id)) PASS
Single-swap txs unchanged (swap_index 0) cd indexer && cargo test parse_limit_order_fills_assigns_swap_index --lib + parse_limit_order_fills_twenty_makers_merged_before_transfer asserts all fills carry swap_index == 0 PASS
Indexer integration test coverage indexer/tests/limit_fill_swap_linkage.rs present and green PASS
Integrator doc notes fill ↔ swap linkage docs/integrators-hybrid-volume.md § Fill ↔ swap linkage (swap_event_id); skills/AGENTS_INTEGRATOR_HYBRID_VOLUME.md; docs/indexer-invariants.md limit fill rows row PASS

Verification criteria (issue body)

Check Command / evidence Result
Synthetic multi-swap tx fixture limit_fill_swap_linkage.rs PASS
SQL linkage by swap_index (not ORDER BY id) swap_id_for_tx_pair_index query on (tx_hash, pair_id, swap_index) — integration test PASS
Indexer test suite green cd indexer && cargo test --lib → 118 passed; integration test above PASS

Implementation summary (already on main)

  • swap_id_for_tx_pair (ORDER BY id ASC LIMIT 1) replaced by swap_id_for_tx_pair_index(tx, pair, swap_index).
  • parse_limit_order_fills assigns per-pair swap_index from parser walk order (fills precede their swap action).
  • process_limit_order_fill links via the fill's swap_index.

No additional repo changes required from this verification pass.

## Verification — [#316](https://gitlab.com/PlasticDigits/cl8y-dex-terraclassic/-/work_items/316) Independent QA pass on branch `cursor/gitlab-issue-verification-b68c` (includes merged indexer fix MR !774 `f1caac4` and docs MR !804). ### Acceptance criteria | Criterion | How verified | Result | |-----------|--------------|--------| | Multi-swap same-pair tx: each fill's `swap_event_id` → correct `swap_events` row | `cd indexer && cargo test --test limit_fill_swap_linkage -- --test-threads=1` — `fill_links_to_its_own_swap_not_the_first_on_the_pair` resolves `swap_index` 0 → first row, `swap_index` 1 → second row (not MIN(id)) | **PASS** | | Single-swap txs unchanged (`swap_index` 0) | `cd indexer && cargo test parse_limit_order_fills_assigns_swap_index --lib` + `parse_limit_order_fills_twenty_makers_merged_before_transfer` asserts all fills carry `swap_index == 0` | **PASS** | | Indexer integration test coverage | `indexer/tests/limit_fill_swap_linkage.rs` present and green | **PASS** | | Integrator doc notes fill ↔ swap linkage | `docs/integrators-hybrid-volume.md` § **Fill ↔ swap linkage (`swap_event_id`)**; `skills/AGENTS_INTEGRATOR_HYBRID_VOLUME.md`; `docs/indexer-invariants.md` limit fill rows row | **PASS** | ### Verification criteria (issue body) | Check | Command / evidence | Result | |-------|-------------------|--------| | Synthetic multi-swap tx fixture | `limit_fill_swap_linkage.rs` | **PASS** | | SQL linkage by `swap_index` (not `ORDER BY id`) | `swap_id_for_tx_pair_index` query on `(tx_hash, pair_id, swap_index)` — integration test | **PASS** | | Indexer test suite green | `cd indexer && cargo test --lib` → 118 passed; integration test above | **PASS** | ### Implementation summary (already on `main`) - `swap_id_for_tx_pair` (`ORDER BY id ASC LIMIT 1`) replaced by `swap_id_for_tx_pair_index(tx, pair, swap_index)`. - `parse_limit_order_fills` assigns per-pair `swap_index` from parser walk order (fills precede their `swap` action). - `process_limit_order_fill` links via the fill's `swap_index`. No additional repo changes required from this verification pass.
PlasticDigits (Migrated from gitlab.com) closed this issue 2026-06-05 13:38:35 +00:00
PlasticDigits commented 2026-06-05 13:44:30 +00:00 (Migrated from gitlab.com)

mentioned in issue #331

mentioned in issue #331
PlasticDigits commented 2026-06-05 13:44:30 +00:00 (Migrated from gitlab.com)

marked as related to #331

marked as related to #331
PlasticDigits commented 2026-06-05 14:07:09 +00:00 (Migrated from gitlab.com)

mentioned in merge request !817

mentioned in merge request !817
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
code/cl8y-dex-terraclassic#316
No description provided.