Skip to content

[simplex]: retract bids one at a time, and on cancel, expiry and close; stop bidding on completed orders - #1422

Merged
seunlanlege merged 4 commits into
fix/simplex-chain-is-the-budgetfrom
fix/simplex-per-bid-retraction
Oct 21, 2026
Merged

seunlanlege merged 4 commits into
fix/simplex-chain-is-the-budgetfrom
fix/simplex-per-bid-retraction

Conversation

@seunlanlege

@seunlanlege seunlanlege commented Oct 16, 2026 •

Copy link
Copy Markdown
Member

Stacked on #1421 (stack #1417). Review only this PR's commits.

This PR fixes the per-bid retraction findings from the 2026-10-15 simplex audit: c22, c23, c33, c34, c42, and the round-2 expiry low. With holds gone (#1421), what retraction protects is the BRIDGE deposit and fee on each Hyperbridge bid, plus taking a withdrawn price out of discovery.

Why:

  • Retraction ran per incoming order, never per bid. A pass marked every row on the commitment retracted, including a bid stored while retractBid was awaiting. That bid was never retracted on chain, so its deposit stayed locked forever.
  • The multi-bid loop kept placing bids on an order that had already filled.
  • A cancelled, expired or closed limit order left its bids live and executable at the old price.
  • stop() disconnected Hyperbridge before draining queued retractions.

Fix (src/core/filler.ts, BidStore, limit-orders.ts, simplex.ts, boot.ts):

  • Per-bid retraction:

    • enqueueRetraction(commitment, { bids?, rows? }) snapshots the live rows when the pass starts, retracts them, and marks only those row ids retracted.
    • markRetracted and markDead take an optional row-id filter.
    • Per-row outcomes: success, BidNotFound and legacy rows close; failed and pooled rows stay open and are flagged dead by row. The sweep retracts only due rows.
  • No bids on a completed order: completedOrders, a set of the last 20,000 completed commitments (oldest dropped), stops the bid loop. Every complete fill on a configured chain enters it, a rival's included, not only orders we bid on, so the cap is set well above what a bid loop can see go by. A bid stored after completion is retracted at once.

  • Retract when a limit order stops being usable: IntentFiller.retractBidsForLimitOrder(id) retracts the live bids that limit order priced, across commitments, and never throws. It runs:

    • after a successful cancel;
    • on the new expired and rejected events (retire and post are POSTABLE-guarded and emit only when the write applies);
    • on filled, including a close through reconcileTallies;
    • after settlement's syncToChain for orders that are no longer open.

    While a limit order is live, its bids are left to the chain (debitOrder).

  • Attribution: a retracted bid that still executes is recorded, because retraction can't stop a placer that already fetched the bid until its validUntil. The escrow the fill released decides which bid it was, since a bid releases its signed take or less, so only a bid whose take covers the release can have been it. In order:

    1. live bids whose take covers the release, best rate first;
    2. then retracted bids in the window whose take covers the release, best rate first (a live bid the escrow rules out does not hide one);
    3. then the payout fallbacks (the payout that matches the delivery, the closest one above it, the order the bids went out in), among the live bids if there are any.

    A retracted bid is in the window when it was stored no more than bidValiditySeconds + the 30 s discovery pad (BID_DISCOVERY_PAD_SECONDS) + the 120 s bid permit skew (BID_PERMIT_SKEW_SECONDS) before the fill. The window is counted back from the timestamp of the block the fill landed in (getBlock, read only when there is a retracted candidate), not from when settlement runs. The arrival time is the fallback when the fill carries no block or the read fails.

  • Shutdown: stop() drains evaluations, chain queues, settlement and rebalances first, then retractions (bounded at 30 s), and disconnects last. Shutdown stops the limit-order lifecycle first.

Limits:

  • Retraction narrows exposure but can't revoke a bid already fetched. bidValiditySeconds is the hard bound.
  • A limit order with more than 500 bid rows leaves its oldest live bids to the 1 h sweep.
  • Shutdown waits for in-flight settlement and tally work.
  • Custom BidStore implementations must honour the new rows filter.

Validation

Run in sdk/packages/simplex at the PR head:

  • CI's test job commands: simplex lint exit 0 (warnings only); test:data 139, test:unit 266 and test:filler 74 (5 skipped) passed.
  • Core, data, orderbook and services suites (vitest --maxConcurrency=1 src/tests/core src/tests/data src/tests/orderbook src/tests/services): 800 passed, 4 skipped.
  • Full offline suite (pnpm --filter="@hyperbridge/simplex" test): 1599 passed, 29 skipped, 10 failed. The failures are exactly the six live/credential files (rebalancers/binance, cctp, usdt0, and strategies/fx, stable, vault .testnet).
  • Typecheck: tsc shows the 14 errors already on main.
  • What the tests cover:
    • the mid-retraction race, a real interleaving;
    • per-bid scoping, legacy rows, and mixed outcomes;
    • no bids after completion;
    • cancel, expiry, reject and reconcile-close retraction through Simplex with a real service and filler;
    • status-based sibling retraction;
    • a retracted bid that still executes is recorded;
    • a live bid beats a retracted, better-rate one;
    • a stale retracted bid isn't attributed;
    • the stop drain order and its timeout;
    • row filters on both store backends.

Not covered: no live Hyperbridge run.

@Wizdave97 Wizdave97 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: changes needed before merge. I reviewed only this PR's commit on top of #1421.

The per-row snapshot fixes the stranded-deposit race. What I checked:

  • A row stored during a pass is either retracted by its own scoped pass or picked up by the TTL sweep.
  • Pooled and failed rows are marked dead one row at a time and retried.
  • filled is only emitted when the dust-floor close actually applies.
  • stop() adds no new wait that could hang forever.

Must fix

  1. Bids priced by a cancelled or expired limit order keep going out, and nothing retracts them before the 1 h sweep (filler.ts:1264-1275, :1332-1346).
    • executeOrder uses the plans cached at evaluation (getBidPlans). The loop only checks hasCompleted(order.id). It never checks the status of plan.limitOrderId, and neither fx.submitBid nor prepareBidUserOp does either.
    • Scenario: the operator cancels limit order A while a queued task holds a plan priced from A, or partway through a multi-plan loop (about 8 s per bid).
    • retractBidsForLimitOrder(A) takes its snapshot at that moment, so any bid placed afterwards is stored as live. Those bids sit in discovery at the withdrawn price for bidValiditySeconds plus the pad, and their deposit stays locked until the 1 h sweep.
    • The cancel doesn't change the on-chain cap, which is still size, so these bids remain executable.
    • That is the exposure this PR sets out to close. There is a guard for swap-order completion, but not for limit-order closure.
    • Suggestion: before each bid, skip a plan whose limit order isn't open or resizing. After store, check again, and if it has closed, call enqueueRetraction(commitment, { bids: [result.bid] }). Alternatively, keep a closedLimitOrders set fed by retractBidsForLimitOrder.

Should fix

  1. Completion is recorded behind the settlement queue (filler.ts:261-286).

    • markCompleted runs inside handleOrderFilledOnChain and handleRivalCompletion. Both are queued on settlementQueue, which runs one task at a time, behind reconcileTallies and other fills' syncToChain → resize, which make orderbook HTTP calls and chain reads.
    • Meanwhile the bid loop keeps placing bids that can only revert with Filled(), each paying the BRIDGE fee. They are retracted later, so this costs fees, not deposits.
    • The "recorded before anything is read" comment at :1472 assumes this happens promptly.
    • Suggestion: call markCompleted directly in the two monitor.on(...) listeners, before settlementQueue.add.
  2. Preferring a live bid in fill attribution ignores the escrow evidence (filler.ts:1590-1596).

    • The code uses the live bids whenever any exist, and otherwise falls back to bids[0].
    • Example: live bid RB with take 40, and retracted bid RA with take 100. A placer who had already fetched RA executes it, releasing 100. RB can't have been the one executed (the amount released is at most its take), yet the fill is recorded against B, capped at RB's payout. RB is then also excluded from its own later fill.
    • The docstring says retracted bids are used "only when no live bid could have been it", but the code means "when no live bid exists". remaining follows the chain, so this only skews fill history and profit.
    • Suggestion: filter live bids by take >= released first, then retracted ones, before the payout and bids[0] fallbacks.
  3. The window for attributing a retracted bid starts from settlement time and has only a 60 s margin (filler.ts:60, :1566).

    • A backlog on the settlement queue (see 2), and slow blocks stretching the block-based validUntil, both eat into that margin.
    • #1411 sets BID_PERMIT_SKEW_SECONDS = 120 for exactly this tail where a bid can still execute. A retracted bid that executes between +60 and +120 s is therefore not attributed.
    • Suggestion: start the window from when the event arrived or from the block timestamp, and reuse #1411's constant.

Notes:

  • completedOrders counts every rival completion on every configured chain, not only orders we bid on. Eviction during a loop would still need 1000 newer completions, so this is fine.
  • The BidStore rows requirement is a silent hazard: a custom store that ignores the filter still type-checks and brings back the stranded-deposit bug. Call it out as a breaking change in the release notes.

Merge: the conflict with #1411 in ContractInteractionService.ts is only the BID_DISCOVERY_PAD_SECONDS export. Take #1411's side, which also adds BID_PERMIT_SKEW_SECONDS.

CI: swaps fails on onchain-limit-refusal because of #1421 (e2e/budget.mjs:422 still reads reserved). Every other scenario passed, so this goes green once #1421 is fixed.

seunlanlege added a commit that referenced this pull request Oct 19, 2026
- executeOrder skips a cached bid plan whose limit order is no longer
  open/resizing, and re-checks after the bid row is stored: a bid that went
  out while its order closed is retracted at once (just that bid).
- Completion is recorded in the two EventMonitor listeners, ahead of the
  settlement queue, for complete fills only, so the bid loop stops while
  settlement is queued behind reconcile or orderbook work.
- Fill attribution follows the escrow evidence: live bids whose take covers
  the release, then retracted bids in the window, before the payout and
  first-bid fallbacks.
- A retracted bid's window is counted back from when the fill event arrived,
  not from settlement; the margin is RETRACTED_BID_EXECUTION_MARGIN_SECONDS (120).
- Note: the BidStore `rows` filter is stated as a breaking change for custom stores.
- Merge: retire()'s POSTABLE guard matches #1410's identical change; the
  rebase test fixtures follow the fill's block number from #1421.
@seunlanlege
seunlanlege force-pushed the fix/simplex-per-bid-retraction branch from b464047 to 15ed6a2 Compare October 19, 2026 12:28
seunlanlege added a commit that referenced this pull request Oct 19, 2026
- executeOrder skips a cached bid plan whose limit order is no longer
  open/resizing, and re-checks after the bid row is stored: a bid that went
  out while its order closed is retracted at once (just that bid).
- Completion is recorded in the two EventMonitor listeners, ahead of the
  settlement queue, for complete fills only, so the bid loop stops while
  settlement is queued behind reconcile or orderbook work.
- Fill attribution follows the escrow evidence: live bids whose take covers
  the release, then retracted bids in the window, before the payout and
  first-bid fallbacks.
- A retracted bid's window is counted back from when the fill event arrived,
  not from settlement; the margin is RETRACTED_BID_EXECUTION_MARGIN_SECONDS (120).
- Note: the BidStore `rows` filter is stated as a breaking change for custom stores.
- Merge: retire()'s POSTABLE guard matches #1410's identical change; the
  rebase test fixtures follow the fill's block number from #1421.
@seunlanlege
seunlanlege force-pushed the fix/simplex-per-bid-retraction branch from 15ed6a2 to bbbe1c6 Compare October 19, 2026 13:26
seunlanlege added a commit that referenced this pull request Oct 19, 2026
seunlanlege added a commit that referenced this pull request Oct 19, 2026
- executeOrder skips a cached bid plan whose limit order is no longer
  open/resizing, and re-checks after the bid row is stored: a bid that went
  out while its order closed is retracted at once (just that bid).
- Completion is recorded in the two EventMonitor listeners, ahead of the
  settlement queue, for complete fills only, so the bid loop stops while
  settlement is queued behind reconcile or orderbook work.
- Fill attribution follows the escrow evidence: live bids whose take covers
  the release, then retracted bids in the window, before the payout and
  first-bid fallbacks.
- A retracted bid's window is counted back from when the fill event arrived,
  not from settlement; the margin is RETRACTED_BID_EXECUTION_MARGIN_SECONDS (120).
- Note: the BidStore `rows` filter is stated as a breaking change for custom stores.
- Merge: retire()'s POSTABLE guard matches #1410's identical change; the
  rebase test fixtures follow the fill's block number from #1421.
@seunlanlege
seunlanlege force-pushed the fix/simplex-per-bid-retraction branch from bbbe1c6 to 1641aa4 Compare October 19, 2026 15:08
seunlanlege added a commit that referenced this pull request Oct 19, 2026

@Wizdave97 Wizdave97 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved. All four round-1 findings are fixed:

  • Plans priced from a closed limit order are skipped, and a bid stored after the order closed is retracted.
  • Completion is recorded in the event listener, not behind the settlement queue.
  • Fill attribution checks the escrow evidence first.
  • The retracted-bid window counts back from the fill block, with a 120 s margin.

Non-blocking follow-ups:

  • Attribution edge case. A live bid with no take is passed over in favour of a retracted bid whose take covers the escrow (filler.ts:117-124). This only happens for legacy rows and only skews fill history.
  • PR description. It still says completedOrders holds 1000, that "live bids are preferred", and "+60 s".
  • After #1411 lands. Point RETRACTED_BID_EXECUTION_MARGIN_SECONDS at BID_PERMIT_SKEW_SECONDS.
  • CI. The failed swaps run isn't caused by this PR.
    • multi-leg-same-input failed because CacheService's 60 s expiry ran out partway through the bid loop while Gargantua inclusion was slow ("No cached gas estimate found").
    • That code predates this stack, and the fix is tracked separately.

seunlanlege added a commit that referenced this pull request Oct 20, 2026
- executeOrder skips a cached bid plan whose limit order is no longer
  open/resizing, and re-checks after the bid row is stored: a bid that went
  out while its order closed is retracted at once (just that bid).
- Completion is recorded in the two EventMonitor listeners, ahead of the
  settlement queue, for complete fills only, so the bid loop stops while
  settlement is queued behind reconcile or orderbook work.
- Fill attribution follows the escrow evidence: live bids whose take covers
  the release, then retracted bids in the window, before the payout and
  first-bid fallbacks.
- A retracted bid's window is counted back from when the fill event arrived,
  not from settlement; the margin is RETRACTED_BID_EXECUTION_MARGIN_SECONDS (120).
- Note: the BidStore `rows` filter is stated as a breaking change for custom stores.
- Merge: retire()'s POSTABLE guard matches #1410's identical change; the
  rebase test fixtures follow the fill's block number from #1421.
@seunlanlege
seunlanlege force-pushed the fix/simplex-per-bid-retraction branch from 1641aa4 to fab2020 Compare October 20, 2026 11:46
seunlanlege added a commit that referenced this pull request Oct 20, 2026
- executeOrder skips a cached bid plan whose limit order is no longer
  open/resizing, and re-checks after the bid row is stored: a bid that went
  out while its order closed is retracted at once (just that bid).
- Completion is recorded in the two EventMonitor listeners, ahead of the
  settlement queue, for complete fills only, so the bid loop stops while
  settlement is queued behind reconcile or orderbook work.
- Fill attribution follows the escrow evidence: live bids whose take covers
  the release, then retracted bids in the window, before the payout and
  first-bid fallbacks.
- A retracted bid's window is counted back from when the fill event arrived,
  not from settlement; the margin is RETRACTED_BID_EXECUTION_MARGIN_SECONDS (120).
- Note: the BidStore `rows` filter is stated as a breaking change for custom stores.
- Merge: retire()'s POSTABLE guard matches #1410's identical change; the
  rebase test fixtures follow the fill's block number from #1421.
@seunlanlege
seunlanlege force-pushed the fix/simplex-per-bid-retraction branch from fab2020 to 3743845 Compare October 20, 2026 12:30
@seunlanlege
seunlanlege merged commit 5a8f171 into main Oct 21, 2026
5 of 6 checks passed
@seunlanlege
seunlanlege deleted the fix/simplex-per-bid-retraction branch October 21, 2026 09:52
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.

2 participants