Skip to content

fix(dex): persist purchase orders before the swap and guard in-flight duplicates#4248

Merged
TaprootFreak merged 6 commits into
developfrom
fix/dex-purchase-inflight-guard
Jul 17, 2026
Merged

fix(dex): persist purchase orders before the swap and guard in-flight duplicates#4248
TaprootFreak merged 6 commits into
developfrom
fix/dex-purchase-inflight-guard

Conversation

@TaprootFreak

@TaprootFreak TaprootFreak commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator

What

Closes the broadcast-before-persist double-swap on the DEX liquidity-purchase path (part 2 of #4192; mirrors the payout fix from #4181).

Why

PurchaseStrategy.purchaseLiquidity() created the LiquidityOrder in memory, dispatched the swap (bookLiquiditySwapdexService.swap) and only then saved the order for the first time. A crash in that window left no row at all; the upstream caller (e.g. a buy-crypto batch still in Created) re-issued the purchase and swapped a second time. Nothing in the DB prevented a duplicate purchase order either — the (context, correlationId) index is non-unique.

How

  • Persist before the swap: the order row is written first and is the in-flight marker (txId IS NULL, isComplete = false).
  • Partial unique index (migration): UNIQUE (context, "correlationId") WHERE "isComplete" = false AND "type" = 'Purchase' — a concurrent duplicate purchase for the same correlation now fails loudly at INSERT, before anything reaches the chain. Scoped deliberately: a Reservation may legitimately coexist in-flight with the same correlationId, and completed/cancelled rows (isComplete = true) keep historical duplicates out of the index. Production data was checked: exactly one in-flight row existed (no duplicates), so the index creation is safe.
  • Broadcast boundary: EvmClient.doSwap wraps exactly the wallet.sendTransaction call (plus an empty-hash guard) in TxBroadcastError. The gas limit is estimated explicitly before the boundary (mirroring the existing sendNativeCoin/sendToken pattern) — estimation reverts, including the routine Uniswap slippage revert (Too little received), stay plain pre-broadcast errors. Because a direct provider.estimateGas revert surfaces the reason at e.reason (a send-time estimation historically nested it at e.error.reason), DexEvmService.swap now checks the whole reason chain, so the existing PriceSlippageException mapping and the retry self-healing keep working. All PurchaseStrategy chains are EVM-based and share this boundary.
  • Error handling in purchaseLiquidity: a TxBroadcastError (or any error with a txId already assigned) keeps the row in-flight — the unique index then blocks blind re-purchases until an operator investigates. A plain pre-broadcast error cancels the row (removing it from the partial index) and rethrows, preserving today's retry-on-next-cron self-healing exactly.
  • txId is persisted immediately after dispatch; target-amount estimation runs afterwards on purpose — if it fails, finalizePurchaseOrders still picks up the already-broadcast tx.
  • Reader determinism: cancelled purchase attempts now remain persisted, so the single-row readers (fetchLiquidityTransactionResult, checkOrderReady, checkOrderCompletion) read the newest row per (context, correlationId) — a cancelled attempt can no longer shadow the live retry row.
  • Escalation: stale in-flight purchase rows (older than 15 minutes, isComplete = false AND isReady = false — a ready order is progressing normally toward batch completion and is not stranded) trigger ONE monitoring mail with two labeled lists — rows without a txId (verify on-chain absence before cancelling) and rows that dispatched but never readied (reconcile manually; retries are blocked by the in-flight guard). No auto-cancel; the query skips eager relations.

Tests

  • order persisted before the swap dispatch (call-order assertion)
  • plain pre-broadcast error → order cancelled + saved, error rethrown
  • TxBroadcastError → order stays in-flight (no cancel), error rethrown
  • txId persisted right after a successful dispatch; estimation failure does not lose it
  • escalation selects all stale in-flight purchase rows (type/isComplete/age), labels both classes and mails once
  • swap boundary (evm-client.spec.ts): send failure / empty hash → TxBroadcastError; an estimateGas revert (the slippage shape) stays a plain error

… duplicates

The liquidity purchase dispatched the swap before the order was ever saved: a
crash in that window left no row at all, so the upstream caller re-issued the
purchase and swapped a second time - and nothing prevented duplicate purchase
orders, the (context, correlationId) index is non-unique.

The order row is now persisted first as the in-flight marker, and a partial
unique index (isComplete = false AND type = 'Purchase') rejects a concurrent
duplicate purchase at INSERT, before anything reaches the chain (scoped so
reservations and historical rows stay out). EvmClient.doSwap gets an exact
broadcast boundary: only send failures wrap as TxBroadcastError and keep the
row in-flight; provable pre-broadcast errors cancel the row and preserve the
existing retry semantics. The tx id is persisted immediately after dispatch,
and stranded in-flight purchases alert after 15 minutes (no auto-cancel).

Part 2 of #4192
…readers deterministic

Review follow-ups: doSwap now estimates the gas limit explicitly before the
broadcast boundary - estimation reverts (including the routine slippage
revert) stay plain pre-broadcast errors, preserving the slippage handling and
the retry self-healing. The single-row correlation readers return the newest
row so a persisted cancelled attempt cannot shadow the live retry row. The
stranded-purchase alert now covers rows with a dispatched-but-never-completed
txId as well (two labeled lists, no eager loads), the entity comment no
longer misstates TypeORM's partial-index capability, and the doSwap boundary
has dedicated tests.
…ders from stranded alerts

Review follow-up: moving the gas estimation before the broadcast boundary
changed the ethers error shape - a direct estimateGas revert surfaces the
reason at e.reason, while the previous send-time estimation nested it at
e.error.reason. The slippage check now walks the whole reason chain so
PriceSlippageException is still raised and the retry self-heals. The
stranded-purchase alert additionally requires isReady = false, so healthy
but slow in-flight buys (ready, awaiting batch completion) no longer produce
false operator alarms. Adds a dex-evm.service slippage-mapping spec and uses
the realistic ethers error shape in the doSwap test.
@TaprootFreak

Copy link
Copy Markdown
Collaborator Author

Review protocol: 3 full review passes (conformity + logic lens each) until zero findings. Pass 1 surfaced a BLOCKER (the implicit gas estimation lived inside the broadcast boundary, so a slippage revert became a fail-closed TxBroadcastError and stuck the order in-flight) plus two MAJORs (persisted cancel rows shadowed the live retry row in the single-row readers; the stranded alert missed dispatched-but-uncompleted rows). Pass 2 caught that moving the estimation before the boundary changed the ethers error shape and broke the downstream slippage check (verified against the vendored ethers source) and that the alert now over-flagged healthy slow buys. Fixed by walking the full revert-reason chain and filtering the alert to not-yet-ready orders, with a dedicated dex-evm.service slippage-mapping spec. Pass 3: clean on both lenses.

@TaprootFreak
TaprootFreak marked this pull request as ready for review July 16, 2026 22:17
@TaprootFreak
TaprootFreak merged commit f938975 into develop Jul 17, 2026
8 checks passed
@TaprootFreak
TaprootFreak deleted the fix/dex-purchase-inflight-guard branch July 17, 2026 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant