Cross-token swap settles the sell leg, then returns #err and skips the fill-capture hooks

[andreij6]

Found and verified with Claude Opus 5. Every line number cited below was re-checked against the published tip; the proofs are runnable from the repo root and modify nothing.

Three defects on the settlement path, ordered by how much value they can move. The first is the one to look at: executeSwapCross's all-or-nothing branch settles the sell leg synchronously and can then return #err from two points below that settlement, with no rollback and with every fill-capture hook sitting underneath those returns — the caller is told the swap failed while holding the proceeds, and the settled trades never reach emitFillEvents, so they are absent from the hash-chained archive permanently. The second is arguably worse per-occurrence but narrower in blast radius: openPosition's poolPositions upsert writes size = 0 on every call, including an increase, and bookPoolSide feeds that stored zero into MarginPools.applyFill, whose same-direction branch then sets newEntry = fillPrice and throws the VWAP away. The third is a missing range check on maxSlippage in swap()/quoteSwap() that turns an out-of-range value into a Nat-underflow trap instead of the structured #err every sibling endpoint returns.

---

1. executeSwapCross settles the sell leg, then returns #err and skips the fill-capture hooks

Where: src/backend/main.mo:11120-11152 (executeSwapCross, defined at :11105); hooks at :11138-11145; refreshRolling24h at :11317 and its emitFillEvents call at :11322.

What is wrong. In the noPartialFill branch, leg 1 goes through executeMarketOrderProtected, which for zero-window makers mutates balances, fills orders, and records trades synchronously. Two early returns sit after that settlement:

        if (noPartialFill) {
          let protectionCtx = buildProtectionCtx(timestamp);
          let sellResult = MatchingEngine.executeMarketOrderProtected(
            orderStore, accounts, sellMarket, sellToken, caller, #sell, amount, maxSlippage, false, timestamp, protectionCtx,
          );
          if (sellResult.totalFilled == 0) { return #err("No liquidity on " # sellMarket) };
          let icpusdObtained = Fixed.mul(sellResult.totalFilled, sellResult.avgPrice, false);
          let availIcpusd  = Accounts.getBalance(accounts, caller, Types.QUOTE_TOKEN);
          let icpusdToSpend = Nat.min(icpusdObtained, availIcpusd);
          let buyQuantity  = switch (OrderBook.findBestMatch(orderStore, buyMarket, #buy)) {
            case null { return #err("No liquidity on " # buyMarket) };
            // Leave room for the taker fee: the swapper owes tradeCost+takerFee but
            // only holds `icpusdToSpend` (its sell proceeds). Size so cost+fee fits.
            case (?ask) { if (ask.price > 0) { Fixed.div(Fixed.mulDiv(icpusdToSpend, 10_000, 10_000 + TAKER_FEE_BPS, false), ask.price, false) } else { return #err("Invalid price") } };
          };

The #err at :11130 and the #err at :11133 both return past the entire hook block:

          updateStatsAfterTrades(sellMarket, sellResult.trades);
          updateStatsAfterTrades(buyMarket,  buyResult.trades);
          refreshRolling24h(sellMarket, sellResult.trades, timestamp);
          refreshRolling24h(buyMarket,  buyResult.trades,  timestamp);
          let allAffected = List.empty<Principal>();
          for (u in sellResult.affectedUsers.vals()) { List.add(allAffected, u) };
          for (u in buyResult.affectedUsers.vals())  { List.add(allAffected, u) };
          adjustAffectedUsers(Iter.toArray(List.values(allAffected)), timestamp);

There is no compensating unwind of the settled sell leg between :11124 and those returns. Separately, both engine calls hard-code the noPartialFill argument to false (:11123 and :11136), so the engine's own all-or-nothing pre-check never runs on the cross path; the caller's requested semantics survive only as the branch selector.

Why it matters. A cross swap requested as all-or-nothing against a buy market with no resting asks — an AMM ladder pulled during an oracle stall, quotes expired and swept — sells the user's base token into ICPUSD, commits those fills, and then answers #err("No liquidity on <buyMarket>"). Three consequences follow. The user is told the operation failed while irreversibly holding ICPUSD, and any client that retries on #err double-sells. refreshRolling24h is the single fill-capture choke point — emitFillEvents has exactly one caller, at :11322 — so those settled fills are permanently absent from the hash-chained archive tape and per-user fill history, and 24 h volume under-counts them. Affected users' orders are never adjusted.

Proof. From the repo root:

$ node proofs/h11-cross-swap-sell-leg-orphaned.mjs
executeSwapCross #marketOrder/noPartialFill block: main.mo:11105-11153
  sell leg SETTLES (engine mutates Accounts) at line : 11122
  early "return #err" AFTER it at lines             : 11130, 11133
  compensation/rollback between them               : NONE
  fill-capture hooks live at lines                 : updateStatsAfterTrades=11138, refreshRolling24h=11140, adjustAffectedUsers=11145
  noPartialFill arg passed to engine (sell/buy)    : false / false
  public swap traps on #err                        : false

OBSERVED : sell leg settles at :11122; 2 early #err at 11130,11133 return with NO rollback, and every fill-capture hook (11138-11145) sits BELOW those returns, so on that path the user's already-sold tokens are gone, no stats/rolling/adjust run, and the engine's noPartialFill pre-check never ran (both calls hard-code false).
CORRECT  : either no state is mutated before the last failure point, or each early #err first unwinds the settled sell leg AND runs the same stats/rolling/adjust hooks for sellResult.trades.
BUG REPRODUCED: settled sell leg abandoned by early #err with no rollback and hooks unreachable.

How to confirm. Read :11120-11152 top to bottom and note that the only statements between the sell-leg engine call and the two return #err lines are pure reads. Then confirm emitFillEvents (:7063) has a single call site, inside refreshRolling24h at :11322, so anything that skips the hook block is invisible to the archive. swap() propagates the #err as a normal reply (:10974), so there is no trap-and-rollback saving the state.

Relationship to issue #6. Item 5 of issue #6 cites this same function and an overlapping line range, but for a different defect: on the success path the synchronous noPartialFill cross swap never credits bumpPartyVolume. This report is about the error path — settled value left uncaptured by updateStatsAfterTrades / refreshRolling24h / adjustAffectedUsers and, in the archive's case, unrecoverable. The two do not overlap in either trigger or effect; fixing one does not fix the other.

Correct behaviour, one line: either nothing settles before the last possible failure point, or an early failure returns #ok with fullyFilled = false after running the same hooks over sellResult.trades.

---

2. openPosition's poolPositions upsert writes size = 0 on every call, destroying the VWAP entry price

Where: src/backend/main.mo:9426-9434 (inside openPosition, :9327); consumed at :4337-4342 in bookPoolSide; the arithmetic is src/backend/lib/MarginPools.mo:77-85; the partial healer is reconcilePoolPositions at :4490-4492.

What is wrong. The upsert stores a flat zero for size with no branch on prior, while carefully preserving entryPrice, realizedPnl, and openedAt:

    // Upsert the position record (provisional entry; size is derived at read).
    let k = posKey(poolId, marketId);
    let prior = Map.get(poolPositions, Text.compare, k);
    Map.add(poolPositions, Text.compare, k, {
      poolId; marketId; baseToken;
      size        = 0;
      entryPrice  = switch (prior) { case (?p) { if (p.entryPrice > 0) { p.entryPrice } else { refPrice } }; case null { refPrice } };
      realizedPnl = switch (prior) { case (?p) { p.realizedPnl }; case null { 0 } };
      openedAt    = switch (prior) { case (?p) { p.openedAt }; case null { now } };
    });

Map.add replaces the key, so an increase of an existing position lands with size = 0. Settlement then reads the stored field, not a derived one:

        let (size0, entry0, realized0, openedAt0) = switch (Map.get(poolPositions, Text.compare, k)) {
          case (?p) { (p.size, p.entryPrice, p.realizedPnl, p.openedAt) };
          case null { (0, 0, 0, now) };
        };
        let r = MarginPools.applyFill(size0, entry0, signedFill, price);

With size0 == 0, applyFill takes the same-direction branch and the VWAP collapses to the fill price:

    let sameDir = size == 0 or ((size > 0) == (fillSize > 0));
    if (sameDir) {
      // Open or increase in the same direction → VWAP the entry.
      let absS = Int.abs(size);
      let absF = Int.abs(fillSize);
      // (|S|·entry + |F|·fill) / (|S|+|F|): the qty scale cancels → a price.
      let newEntry = if (absS + absF == 0) { fillPrice }
                     else { (absS * entry + absF * fillPrice) / (absS + absF) };
      { size = newSize; entryPrice = newEntry; realizedDelta = 0 }

|S| = 0 makes newEntry exactly fillPrice. reconcilePoolPositions restores size from the derived net and nothing else:

            let derived = poolNetSize(poolId, pos.baseToken);
            if (derived == 0) { List.add(drops, k) }
            else if (derived != pos.size) { List.add(shrinks, { pos with size = derived }) };

So the size heals; the entry price does not.

Why it matters. Every increase of an already-filled margin-pool position silently rebases its cost basis to the latest fill. Unrealized PnL in getMyPositions is wrong from that moment onward, and because entryPrice is a stored truth with no derivation path, the corruption is durable rather than transient — the reconcile pass will never notice it. Realized PnL books against the wrong basis when the position is eventually closed, and the recorded size is understated between the upsert and the next reconcile, which is the window in which risk and liquidation reads happen.

Proof. The Motoko driver calls the real MarginPools.applyFill from src/backend/lib/, with the two inputs being the state openPosition actually leaves behind versus the state it should:

// Part 1 of proof N1 (driven by n1-openposition-zeroes-stored-size.sh).
// Pure divergence: the state openPosition writes (size = 0, entryPrice kept)
// vs the real state (size = prior size) fed to the SAME settlement function.
import Debug "mo:core/Debug";
import MP "../src/backend/lib/MarginPools";

// 1 BTC long @ $50,000 already on the books; add 1 BTC @ $60,000.
let ONE_BTC = 100_000_000;
let P50 = 5_000_000_000_000;
let P60 = 6_000_000_000_000;

let real  = MP.applyFill(ONE_BTC, P50, ONE_BTC, P60); // size never zeroed
let wiped = MP.applyFill(0,       P50, ONE_BTC, P60); // exactly what main.mo:9430 leaves behind

Debug.print("CORRECT size=" # debug_show (real.size) # " entry=" # debug_show (real.entryPrice));
Debug.print("OBSERVED size=" # debug_show (wiped.size) # " entry=" # debug_show (wiped.entryPrice));
if (wiped.entryPrice == P60 and wiped.entryPrice != real.entryPrice and wiped.size != real.size) {
  Debug.print("DIVERGES");
} else { Debug.print("SAME") };

The shell wrapper first re-asserts the source wiring — that :9430 still writes size = 0, that :9431 still preserves the prior entry, that :4339/:4342 still feed the stored p.size into applyFill — and reports NO BUG if any of those stop holding.

$ bash proofs/n1-openposition-zeroes-stored-size.sh
SOURCE: main.mo:9430 writes size=0 (entryPrice kept); main.mo:4339->4342 feeds that stored size into applyFill
CORRECT size=+200_000_000 entry=5_500_000_000_000
OBSERVED size=+100_000_000 entry=6_000_000_000_000
Add 1 BTC @ $60,000 to an existing 1 BTC long @ $50,000:
  CORRECT  : size 2 BTC, entryPrice $55,000 (running VWAP)
  OBSERVED : size 1 BTC, entryPrice $60,000 (VWAP discarded, replaced by latest fill)
BUG REPRODUCED: openPosition zeroes pos.size, so bookPoolSide's applyFill sees size=0, discards the stored VWAP entry price and halves the recorded size — permanently corrupting entryPrice/realizedPnl/openedAt.

The second proof runs the same divergence on the larger 5-BTC-plus-3-BTC scenario:

$ bash proofs/h2-vwap-entry-discard.sh
H2 — MarginPools.applyFill entry-price VWAP
  stored size preserved (5e8): entry=10_375_000_000  size=+800_000_000
  stored size zeroed  (main.mo writes size = 0): entry=11_000_000_000  size=+300_000_000
  CORRECT entryPrice  = 10_375_000_000 (103.75 VWAP)
  OBSERVED entryPrice = 11_000_000_000 (110.00 = the fill price)
PURE-DIVERGENCE CONFIRMED: zeroed size discards the weighted entry (and size +800_000_000 -> +300_000_000)
  main.mo openPosition upsert (source text): 9430:      size        = 0;
  main.mo bookPoolSide feeds that stored size into applyFill:
    4342:        let r = MarginPools.applyFill(size0, entry0, signedFill, price);
BUG REPRODUCED: openPosition zeroes pos.size, so bookPoolSide's applyFill sees size=0, discards the stored VWAP entry price and halves the recorded size — permanently corrupting entryPrice/realizedPnl/openedAt.

How to confirm. Follow the single value: :9430 writes size = 0; :4339 reads p.size back out of the same map; :4342 passes it as applyFill's first argument; MarginPools.mo:83-84 divides by |S| + |F| where |S| is now 0. Then read reconcilePoolPositions at :4490-4492 and confirm the only field it rewrites is size.

Related context. Issue #6 item 3 (the clampToInitialMargin de-lever trap) touches the same margin-pool lifecycle but shares no code with this — it is a different function and a different failure. No existing issue covers the upsert.

Correct behaviour, one line: the upsert should carry the prior size forward, or not run at all when a record already exists, since only a brand-new record needs the provisional entry.

---

3. swap() and quoteSwap() never range-check maxSlippage

Where: src/backend/main.mo:11076-11077 (executeSwapDirect, :11048) and :10921 (quoteSwap, :10886); the unguarded public entry point is swap() at :10974.

What is wrong. The slippage cap is computed by subtracting the caller's value from Fixed.SCALE (Fixed.mo:24, 100_000_000):

        let cap = switch (side) {
          case (#buy)  { Fixed.mul(refPrice, Fixed.SCALE + maxSlippage, true) };
          case (#sell) { Fixed.mul(refPrice, Fixed.SCALE - maxSlippage, false) };
        };

swap() validates amount, token distinctness, balance, minimum notional, and initial margin (:10983-11007), but never the slippage. quoteSwap substitutes a default only for the zero case and passes anything else straight through:

    let slip = if (maxSlippage == 0) { 5_000_000 } else { maxSlippage };   // default 5%, like the UI

and then performs the identical subtraction at :10921. Nat subtraction traps in Motoko, so any sell-direction leg with maxSlippage > 1e8 aborts the message. Every sibling endpoint guards this with the same literal check: openPosition at :9380, previewOpenPosition at :9537, closePosition at :9627, and placeMarketOrder at :10791, all in the form

        if (maxSlippage < 1_000_000 or maxSlippage > 25_000_000) { return #err("Slippage must be 1%–25%") };

executeSwapCross forwards the same unvalidated value into the engine (:11123, :11136), so base-to-base swaps are exposed through that path as well.

Why it matters. This is the mildest of the three: the message traps and rolls back, so no state is corrupted. The consequence is interface, not integrity — an integrator with a units bug (basis points scaled by 1e8, so 200 % arrives as 200_000_000) gets an opaque canister trap instead of the structured #err("Slippage must be 1%–25%") that every neighbouring endpoint returns, and the same trap fires in the read-only quoteSwap query the UI uses for previews, so the preview path fails without a diagnosable reason.

Proof. No runnable proof for this one; it is a source-level absence. The verification is the grep below plus the two line reads.

How to confirm.

$ grep -n "maxSlippage < 1_000_000" src/backend/main.mo
9380:        if (maxSlippage < 1_000_000 or maxSlippage > 25_000_000) { return #err("Slippage must be 1%–25%") };
9537:        if (maxSlippage < 1_000_000 or maxSlippage > 25_000_000) { return refuse("Slippage must be 1%–25%", borrowTok, 0) };
9627:        if (maxSlippage < 1_000_000 or maxSlippage > 25_000_000) { return #err("Slippage must be 1%–25%") };
10791:    if (maxSlippage < 1_000_000 or maxSlippage > 25_000_000) { return #err("Slippage must be between 1% and 25%") };

Four hits, none of them in swap() (:10974-11046) or quoteSwap() (:10886). Then read :11077 and :10921 and note that the subtrahend is caller-controlled and unbounded above. Note that exactly 1e8 does not trap and buy legs use SCALE + maxSlippage, so the trap is specific to sell-direction legs above 1e8.

Relationship to issue #5. Issue #5 covers unvalidated input that either persists to state or drives cycle spend; neither applies here, since the message traps and rolls back. It is referenced only because the remedy belongs to the same "validate at the boundary" theme — the check already exists four times in this file and is simply missing from these two entry points.

Correct behaviour, one line: the same 1 %–25 % range check the sibling endpoints already apply, at the top of swap() and applied to quoteSwap's slip.

---

Method note

These findings were located and verified with Claude Opus 5. Every line number cited above was re-read against the working tree at the time of writing rather than carried over from the analysis pass; where the analysis had drifted, the citation here reflects the source. The two shell proofs and the one Node proof are runnable from the repository root, read the real src/backend/ sources, and modify nothing — the Motoko drivers are interpreted with moc -r against the pinned toolchain and the shell wrappers are read-only over main.mo. Both shell proofs are self-invalidating: they print NO BUG and exit non-zero if the source wiring they assert stops holding, so they can be left in CI as regression guards.

---

Getting the proof scripts. The proofs/ paths referenced above are not in this repository — they ship separately, so nothing is added to your tree. All of them are here:

https://gist.github.com/andreij6/ed9f244e47a71a786405bc7959550d4b

To run them, clone the gist into a proofs/ directory at the root of a public-multidex checkout:

git clone https://gist.github.com/ed9f244e47a71a786405bc7959550d4b.git proofs
bash proofs/run_all_proofs.sh

Verified end to end from a clean checkout. The scripts are read-only: they compile and read the product source, modify nothing, and contain no fixes. Each prints the observed value alongside the correct one, exits 0 with BUG REPRODUCED: while the defect is present, and flips to exit 1 once it is fixed. The gist README maps every proof to its issue and also includes the proofs for candidates that were investigated and not filed.

[dominicwilliams]

Three items, three different outcomes.

Item 1 — executeSwapCross settles the sell leg, then returns #err with no rollback. Confirmed, not fixed, and it is the most consequential open item from your wave. Both parts are real and each is independently serious:

• The caller is told the swap failed while holding the proceeds, so a client that retries on #err double-sells. That is a correctness failure at the API contract level, not merely an internal one.
• Because refreshRolling24h is the sole call site of emitFillEvents, the settled fills are permanently absent from the hash-chained archive — so the tape that is supposed to be the durable record silently omits exactly the trades that took this path.

It sits at the top of our outstanding list and has not been fixed.

Item 2 — the openPosition VWAP wipe. Fixed, but not by us reading your report. It was fixed independently after a live incident on 2026-08-02, before your report was triaged — the position-accounting race that froze Realized and rewrote Entry. Your filing and the incident describe the same defect. Credit for finding it stands regardless of which arrived first; I mention the sequence only because you are entitled to know your report did not cause the fix.

Item 3 — swap()/quoteSwap() never range-check maxSlippage. Confirmed, not fixed. An out-of-range value traps instead of returning the structured #err that four sibling endpoints already return. The inconsistency is the point: the validation exists in the codebase, on the position-opening path, and simply was not applied here. It is open.

The code is in #30, the first push to this mirror since the July snapshot; your line numbers against 0241cba still resolve.