swap_events ON CONFLICT (tx_hash, pair_id) collapses two same-pair swaps in one tx #287

Closed
opened 2026-06-03 07:19:52 +00:00 by Brouie · 20 comments
Brouie commented 2026-06-03 07:19:52 +00:00 (Migrated from gitlab.com)

Severity: Low
Reachability: Any tx that swaps the same pair twice (a route that reuses a pair, e.g. A→B→A).
Affected: swap persistence (indexer/src/db/queries/swap_events.rs).
Root cause: the upsert conflict key is (tx_hash, pair_id), so two distinct swaps on the same pair in one tx collapse to one row.

Summary

INSERT INTO swap_events ... ON CONFLICT (tx_hash, pair_id) DO NOTHING. If a single tx produces two genuine swaps on the same pair — a multi-hop route that traverses a pair twice, or a triangular path that reuses one — the second insert is silently dropped, undercounting that pair's volume and trade count.

Off-chain analytics accuracy only; no fund or state impact. Filing it Low so the conflict key gets an intra-tx discriminator.

Current codebase

  • swap_events.rs: ON CONFLICT (tx_hash, pair_id) DO NOTHING; the row carries no event/message index.
  • Add the per-tx event (or message/log) index to the row and the conflict key, so two same-pair swaps in one tx are distinct rows. The unique index that guards lifecycle dedup may want the same treatment.

Acceptance criteria

  • A tx with two real swaps on the same pair stores two rows.
  • Genuine duplicate-delivery of the same swap is still deduped.
**Severity:** Low **Reachability:** Any tx that swaps the same pair twice (a route that reuses a pair, e.g. A→B→A). **Affected:** swap persistence (`indexer/src/db/queries/swap_events.rs`). **Root cause:** the upsert conflict key is `(tx_hash, pair_id)`, so two distinct swaps on the same pair in one tx collapse to one row. ## Summary `INSERT INTO swap_events ... ON CONFLICT (tx_hash, pair_id) DO NOTHING`. If a single tx produces two genuine swaps on the same pair — a multi-hop route that traverses a pair twice, or a triangular path that reuses one — the second insert is silently dropped, undercounting that pair's volume and trade count. Off-chain analytics accuracy only; no fund or state impact. Filing it Low so the conflict key gets an intra-tx discriminator. ## Current codebase - `swap_events.rs`: `ON CONFLICT (tx_hash, pair_id) DO NOTHING`; the row carries no event/message index. ## Recommended direction - Add the per-tx event (or message/log) index to the row and the conflict key, so two same-pair swaps in one tx are distinct rows. The unique index that guards lifecycle dedup may want the same treatment. ## Acceptance criteria - [ ] A tx with two real swaps on the same pair stores two rows. - [ ] Genuine duplicate-delivery of the same swap is still deduped.
PlasticDigits commented 2026-06-03 10:57:23 +00:00 (Migrated from gitlab.com)

Approved, also the lifecycle dedup. We also need to assign a unique id to each swap, seperate from txid - so if a tx has one or more swaps, it will have multiple uuids for each swap but same txhash

Approved, also the lifecycle dedup. We also need to assign a unique id to each swap, seperate from txid - so if a tx has one or more swaps, it will have multiple uuids for each swap but same txhash
Brouie commented 2026-06-04 05:27:02 +00:00 (Migrated from gitlab.com)

mentioned in issue #285

mentioned in issue #285
Brouie commented 2026-06-04 05:27:05 +00:00 (Migrated from gitlab.com)

mentioned in merge request !744

mentioned in merge request !744
Brouie commented 2026-06-04 06:30:06 +00:00 (Migrated from gitlab.com)

Implementation plan (your per-tx swap ordinal direction). Medium, indexer-only.

  • Add deterministic swap_index: i32 to ParsedSwap (parser.rs) from the parser's swap-push order, fold it into insert_swap's conflict key + tighten trade_exists to per-(tx_hash,pair_id,swap_index). The limit lifecycle dedup is already keyed by order_id (correct) — no change there.
  • Migration: ADD swap_index INTEGER NOT NULL DEFAULT 0, backfill row_number()-1 OVER (PARTITION BY tx_hash,pair_id ORDER BY id) (historical is already single-row from the prior dedup), drop the old unique index → new (tx_hash,pair_id,swap_index). Precedent: 20260326120000_swap_events_unique_limit_lifecycle.sql. Optional swap_uid MUST be deterministic uuid_v5(tx|pair|index) — a random/DB-default uuid violates "reproducible across reindex".
  • Ordinal source: there is NO on-chain msg_index in the LCD structs (Event/TxLog carry only type+attributes), so derive it from the parser's deterministic event walk (cosmos tx event order is fixed). Rebase on #285 (e951e61, same parser scoping) — count only swaps that pass the post-#285 _contract_address emitter check.

Hidden bug this exposes — must be decided, not silently left: limit_order_fills::swap_id_for_tx_pair resolves a fill's parent swap via MIN(id) WHERE tx_hash+pair_id. Once a tx carries two same-pair swaps, that mis-attributes fills to the first swap. Needs a join on swap_index (or accept the ambiguity explicitly). Test (precedent swap_events_hybrid_columns.rs): two same-pair swaps in one tx → two rows; duplicate-delivery of one swap → still deduped. @PlasticDigits

Implementation plan (your per-tx swap ordinal direction). Medium, indexer-only. - Add deterministic `swap_index: i32` to `ParsedSwap` (parser.rs) from the parser's swap-push order, fold it into `insert_swap`'s conflict key + tighten `trade_exists` to per-(tx_hash,pair_id,swap_index). The limit lifecycle dedup is already keyed by order_id (correct) — no change there. - Migration: ADD swap_index INTEGER NOT NULL DEFAULT 0, backfill `row_number()-1 OVER (PARTITION BY tx_hash,pair_id ORDER BY id)` (historical is already single-row from the prior dedup), drop the old unique index → new `(tx_hash,pair_id,swap_index)`. Precedent: `20260326120000_swap_events_unique_limit_lifecycle.sql`. Optional `swap_uid` MUST be deterministic uuid_v5(tx|pair|index) — a random/DB-default uuid violates "reproducible across reindex". - Ordinal source: there is NO on-chain msg_index in the LCD structs (Event/TxLog carry only type+attributes), so derive it from the parser's deterministic event walk (cosmos tx event order is fixed). Rebase on #285 (e951e61, same parser scoping) — count only swaps that pass the post-#285 `_contract_address` emitter check. **Hidden bug this exposes — must be decided, not silently left:** `limit_order_fills::swap_id_for_tx_pair` resolves a fill's parent swap via `MIN(id) WHERE tx_hash+pair_id`. Once a tx carries two same-pair swaps, that mis-attributes fills to the first swap. Needs a join on swap_index (or accept the ambiguity explicitly). Test (precedent `swap_events_hybrid_columns.rs`): two same-pair swaps in one tx → two rows; duplicate-delivery of one swap → still deduped. @PlasticDigits
Brouie commented 2026-06-05 01:47:51 +00:00 (Migrated from gitlab.com)

mentioned in merge request !752

mentioned in merge request !752
Brouie commented 2026-06-05 01:47:59 +00:00 (Migrated from gitlab.com)

Shipped your per-tx swap-ordinal direction — MR !752.

The collapse: insert_swap did ON CONFLICT (tx_hash, pair_id) DO NOTHING, so a tx with two swaps on the same pair (a route revisiting a pair, batched swaps) kept the first and dropped the rest.

Fix: ParsedSwap gets a deterministic swap_index (0-based, per (tx, pair), from parser walk order); the unique key widens to (tx_hash, pair_id, swap_index) (migration backfills via row_number(), drops the old 2-col index, adds the 3-col one); insert_swap's conflict key and trade_exists both scope to it. So every swap is stored with its own row/id — your "multiple ids, same txhash". Test parse_swaps_assigns_per_pair_swap_index (two same-pair swaps -> 0,1; different pair restarts at 0); full indexer suite green.

One honest scope call: the fill->swap linkage swap_id_for_tx_pair still resolves MIN(id), so in the rare multi-swap-same-pair tx the fills point at the first swap. That's strictly better than today (the swaps are no longer lost), but ordinal-correct fill linkage needs the parser to carry the swap ordinal onto each fill (there's no LCD msg_index to lean on) — I'd rather do that as a focused follow-up than bolt a half-association in here. Migration is additive and only bites once the new binary is deployed. @PlasticDigits

Shipped your per-tx swap-ordinal direction — MR !752. The collapse: `insert_swap` did `ON CONFLICT (tx_hash, pair_id) DO NOTHING`, so a tx with two swaps on the same pair (a route revisiting a pair, batched swaps) kept the first and dropped the rest. Fix: `ParsedSwap` gets a deterministic `swap_index` (0-based, per `(tx, pair)`, from parser walk order); the unique key widens to `(tx_hash, pair_id, swap_index)` (migration backfills via `row_number()`, drops the old 2-col index, adds the 3-col one); `insert_swap`'s conflict key and `trade_exists` both scope to it. So every swap is stored with its own row/id — your "multiple ids, same txhash". Test `parse_swaps_assigns_per_pair_swap_index` (two same-pair swaps -> 0,1; different pair restarts at 0); full indexer suite green. One honest scope call: the fill->swap linkage `swap_id_for_tx_pair` still resolves `MIN(id)`, so in the rare multi-swap-same-pair tx the fills point at the first swap. That's strictly better than today (the swaps are no longer lost), but ordinal-correct fill linkage needs the parser to carry the swap ordinal onto each fill (there's no LCD msg_index to lean on) — I'd rather do that as a focused follow-up than bolt a half-association in here. Migration is additive and only bites once the new binary is deployed. @PlasticDigits
PlasticDigits commented 2026-06-05 03:19:10 +00:00 (Migrated from gitlab.com)

mentioned in commit 39a681f82f

mentioned in commit 39a681f82fa21a804c2609a64aa46ab452ecf8a4
PlasticDigits commented 2026-06-05 03:58:35 +00:00 (Migrated from gitlab.com)

That is a serious issue

That is a serious issue
PlasticDigits commented 2026-06-05 04:08:30 +00:00 (Migrated from gitlab.com)

mentioned in issue #316

mentioned in issue #316
PlasticDigits commented 2026-06-05 04:08:30 +00:00 (Migrated from gitlab.com)

marked as related to #316

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

mentioned in merge request !774

mentioned in merge request !774
ghost1 commented 2026-06-05 09:46:40 +00:00 (Migrated from gitlab.com)

mentioned in commit d3f0a6c33e

mentioned in commit d3f0a6c33e21b765408685698b5a8b6ed0fbeaca
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:21 +00:00 (Migrated from gitlab.com)

Verification complete (agent:verify)

Issue: https://gitlab.com/PlasticDigits/cl8y-dex-terraclassic/-/work_items/287

Implementation: Already on main (MR !752 — swap_index, unique (tx_hash, pair_id, swap_index), parser ordinals; fill linkage fixed in #316).

Doc drift MR: !780 — aligns invariants/runbook/seed SQL with the new dedup key.

Acceptance criteria

Criterion Result Evidence
A tx with two real swaps on the same pair stores two rows PASS cargo test parse_swaps_assigns_per_pair_swap_index --lib → ok. cargo test --test limit_fill_swap_linkage -j 1 -- --test-threads=1 → two distinct swap_events ids for swap_index 0 and 1. DB: COUNT(*)=2 for tx_hash='TX316_MULTI_SWAP_SAME_PAIR'.
Genuine duplicate-delivery of the same swap is still deduped PASS Unique index idx_swap_events_tx_hash_pair_id_swap_index. Manual SQL: second INSERT … ON CONFLICT (tx_hash, pair_id, swap_index) DO NOTHING → 0 rows; count stays 1. Parser trade_exists + insert_swap use same key.

Related (comment thread): Limit fill → swap linkage by swap_index — PASS (parse_limit_order_fills_assigns_swap_index_per_pair_swap, limit_fill_swap_linkage).

Issue left open pending doc MR !780 merge.

## Verification complete (agent:verify) Issue: https://gitlab.com/PlasticDigits/cl8y-dex-terraclassic/-/work_items/287 **Implementation:** Already on `main` (MR !752 — `swap_index`, unique `(tx_hash, pair_id, swap_index)`, parser ordinals; fill linkage fixed in #316). **Doc drift MR:** !780 — aligns invariants/runbook/seed SQL with the new dedup key. ### Acceptance criteria | Criterion | Result | Evidence | |-----------|--------|----------| | A tx with two real swaps on the same pair stores two rows | **PASS** | `cargo test parse_swaps_assigns_per_pair_swap_index --lib` → ok. `cargo test --test limit_fill_swap_linkage -j 1 -- --test-threads=1` → two distinct `swap_events` ids for `swap_index` 0 and 1. DB: `COUNT(*)=2` for `tx_hash='TX316_MULTI_SWAP_SAME_PAIR'`. | | Genuine duplicate-delivery of the same swap is still deduped | **PASS** | Unique index `idx_swap_events_tx_hash_pair_id_swap_index`. Manual SQL: second `INSERT … ON CONFLICT (tx_hash, pair_id, swap_index) DO NOTHING` → 0 rows; count stays 1. Parser `trade_exists` + `insert_swap` use same key. | **Related (comment thread):** Limit fill → swap linkage by `swap_index` — **PASS** (`parse_limit_order_fills_assigns_swap_index_per_pair_swap`, `limit_fill_swap_linkage`). Issue left **open** pending doc MR !780 merge.
PlasticDigits commented 2026-06-05 10:00:28 +00:00 (Migrated from gitlab.com)

mentioned in commit 1c4ad9212d

mentioned in commit 1c4ad9212d5a99662730d2be7832e33924ef01b3
PlasticDigits commented 2026-06-05 10:05:46 +00:00 (Migrated from gitlab.com)

mentioned in merge request !787

mentioned in merge request !787
PlasticDigits commented 2026-06-05 11:02:40 +00:00 (Migrated from gitlab.com)

Verification complete (agent:verify)

Issue: https://gitlab.com/PlasticDigits/cl8y-dex-terraclassic/-/work_items/287

Implementation: On main (MR !752 — swap_index, unique (tx_hash, pair_id, swap_index), parser ordinals; fill linkage via #316). Doc drift MR !780 is merged.

Acceptance criteria

Criterion Result How verified
A tx with two real swaps on the same pair stores two rows PASS cargo test parse_swaps_assigns_per_pair_swap_index --lib → ok (ordinals 0, 1 on same pair; different pair restarts at 0). cargo test --test limit_fill_swap_linkage -j 1 -- --test-threads=1 → two distinct swap_events ids for swap_index 0 and 1. DB: SELECT swap_index, COUNT(*) … WHERE tx_hash='TX316_MULTI_SWAP_SAME_PAIR' → rows for swap_index 0 and 1.
Genuine duplicate-delivery of the same swap is still deduped PASS Schema: unique index idx_swap_events_tx_hash_pair_id_swap_index on (tx_hash, pair_id, swap_index). Manual SQL on dex_indexer_test: second INSERT … ON CONFLICT (tx_hash, pair_id, swap_index) DO NOTHING for same key → count stays 1 (DEDUP PASS). Code: insert_swap + trade_exists(tx_hash, pair_id, swap_index) in parser.
Item Result Evidence
Limit fill → swap linkage by swap_index (#316) PASS cargo test parse_limit_order_fills_assigns_swap_index_per_pair_swap --lib → ok. limit_fill_swap_linkage resolves fills by ordinal, not MIN(id).

No repo changes in this verification pass. Closing as complete.

## Verification complete (agent:verify) Issue: https://gitlab.com/PlasticDigits/cl8y-dex-terraclassic/-/work_items/287 **Implementation:** On `main` (MR !752 — `swap_index`, unique `(tx_hash, pair_id, swap_index)`, parser ordinals; fill linkage via #316). Doc drift MR !780 is **merged**. ### Acceptance criteria | Criterion | Result | How verified | |-----------|--------|--------------| | A tx with two real swaps on the same pair stores two rows | **PASS** | `cargo test parse_swaps_assigns_per_pair_swap_index --lib` → ok (ordinals 0, 1 on same pair; different pair restarts at 0). `cargo test --test limit_fill_swap_linkage -j 1 -- --test-threads=1` → two distinct `swap_events` ids for `swap_index` 0 and 1. DB: `SELECT swap_index, COUNT(*) … WHERE tx_hash='TX316_MULTI_SWAP_SAME_PAIR'` → rows for `swap_index` 0 and 1. | | Genuine duplicate-delivery of the same swap is still deduped | **PASS** | Schema: unique index `idx_swap_events_tx_hash_pair_id_swap_index` on `(tx_hash, pair_id, swap_index)`. Manual SQL on `dex_indexer_test`: second `INSERT … ON CONFLICT (tx_hash, pair_id, swap_index) DO NOTHING` for same key → count stays **1** (`DEDUP PASS`). Code: `insert_swap` + `trade_exists(tx_hash, pair_id, swap_index)` in parser. | ### Related (comment thread) | Item | Result | Evidence | |------|--------|----------| | Limit fill → swap linkage by `swap_index` (#316) | **PASS** | `cargo test parse_limit_order_fills_assigns_swap_index_per_pair_swap --lib` → ok. `limit_fill_swap_linkage` resolves fills by ordinal, not `MIN(id)`. | No repo changes in this verification pass. Closing as complete.
PlasticDigits (Migrated from gitlab.com) closed this issue 2026-06-05 11:02:43 +00:00
PlasticDigits commented 2026-06-05 11:23:19 +00:00 (Migrated from gitlab.com)

mentioned in merge request !799

mentioned in merge request !799
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-07 12:14:16 +00:00 (Migrated from gitlab.com)

mentioned in issue #337

mentioned in issue #337
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#287
No description provided.