swap_events ON CONFLICT (tx_hash, pair_id) collapses two same-pair swaps in one tx #287
Labels
No labels
agent:fix_bugfix
agent:fix_conflicts
agent:fix_security
agent:gap_analysis
agent:implement
agent:implement
agent:implement
agent:open_issues
agent:ready
agent:research
agent:security_audit
agent:verify
architecture
backend
blocker:hybrid
blocker:launch
blocker:limit-orders
blocker:v2
block:log_only
block:security
bug
ci
contracts
correctness
deploy
dev
devops
docs
documentation
duplicate
e2e
enhancement
epic
feature
frontend
functional-completion
gas
good first issue
governance
help wanted
high-risk
hooks
hybrid
indexer
infra
infrastructure
integrators
invalid
launch-blocker
limit-orders
localnet
localterra
low priority
missing-implementation
needs-design
ops
performance
priority
high
priority
medium
product
qa
QA
question
ready
ready
research
scripts
security
security-hardening
smartcontracts
tech-debt
testing
ux
UX
v2
verification
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
code/cl8y-dex-terraclassic#287
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
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
Acceptance criteria
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
mentioned in issue #285
mentioned in merge request !744
Implementation plan (your per-tx swap ordinal direction). Medium, indexer-only.
swap_index: i32toParsedSwap(parser.rs) from the parser's swap-push order, fold it intoinsert_swap's conflict key + tightentrade_existsto per-(tx_hash,pair_id,swap_index). The limit lifecycle dedup is already keyed by order_id (correct) — no change there.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. Optionalswap_uidMUST be deterministic uuid_v5(tx|pair|index) — a random/DB-default uuid violates "reproducible across reindex".e951e61, same parser scoping) — count only swaps that pass the post-#285_contract_addressemitter check.Hidden bug this exposes — must be decided, not silently left:
limit_order_fills::swap_id_for_tx_pairresolves a fill's parent swap viaMIN(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 (precedentswap_events_hybrid_columns.rs): two same-pair swaps in one tx → two rows; duplicate-delivery of one swap → still deduped. @PlasticDigitsmentioned in merge request !752
Shipped your per-tx swap-ordinal direction — MR !752.
The collapse:
insert_swapdidON 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:
ParsedSwapgets a deterministicswap_index(0-based, per(tx, pair), from parser walk order); the unique key widens to(tx_hash, pair_id, swap_index)(migration backfills viarow_number(), drops the old 2-col index, adds the 3-col one);insert_swap's conflict key andtrade_existsboth scope to it. So every swap is stored with its own row/id — your "multiple ids, same txhash". Testparse_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_pairstill resolvesMIN(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. @PlasticDigitsmentioned in commit
39a681f82fThat is a serious issue
mentioned in issue #316
marked as related to #316
mentioned in merge request !774
mentioned in commit
d3f0a6c33ementioned in merge request !780
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
cargo test parse_swaps_assigns_per_pair_swap_index --lib→ ok.cargo test --test limit_fill_swap_linkage -j 1 -- --test-threads=1→ two distinctswap_eventsids forswap_index0 and 1. DB:COUNT(*)=2fortx_hash='TX316_MULTI_SWAP_SAME_PAIR'.idx_swap_events_tx_hash_pair_id_swap_index. Manual SQL: secondINSERT … ON CONFLICT (tx_hash, pair_id, swap_index) DO NOTHING→ 0 rows; count stays 1. Parsertrade_exists+insert_swapuse 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.
mentioned in commit
1c4ad9212dmentioned in merge request !787
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
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 distinctswap_eventsids forswap_index0 and 1. DB:SELECT swap_index, COUNT(*) … WHERE tx_hash='TX316_MULTI_SWAP_SAME_PAIR'→ rows forswap_index0 and 1.idx_swap_events_tx_hash_pair_id_swap_indexon(tx_hash, pair_id, swap_index). Manual SQL ondex_indexer_test: secondINSERT … ON CONFLICT (tx_hash, pair_id, swap_index) DO NOTHINGfor same key → count stays 1 (DEDUP PASS). Code:insert_swap+trade_exists(tx_hash, pair_id, swap_index)in parser.Related (comment thread)
swap_index(#316)cargo test parse_limit_order_fills_assigns_swap_index_per_pair_swap --lib→ ok.limit_fill_swap_linkageresolves fills by ordinal, notMIN(id).No repo changes in this verification pass. Closing as complete.
mentioned in merge request !799
mentioned in issue #331
mentioned in issue #337