Pending-match finaliser: unguarded Nat fee subtraction in finalisePendingMatch + stale 'pending-match path' comment

Summary

While auditing src/backend/main.mo @ 1fff1d7, I found two adjacent issues in the pending-match finalisation path (finalisePendingMatch):

1. Unguarded Nat fee subtraction at lines 1533 and 1539 — the only settlement path in the codebase where a fee is subtracted without a guard. If quoteFeeFor(...) >= debit, the heartbeat traps.
2. A stale design comment around line 1985–1990 contradicts the actual wiring of the taker ProtectionCtx, which will mislead whoever un-seals the path.

Both are latent today (the path is currently unreachable — verified below), so this is a correctness/robustness report per SECURITY.md, not an incident.

1. Unguarded fee subtraction in finalisePendingMatch

// src/backend/main.mo:1531-1540 (@ 1fff1d7)
let makerCredit : Nat = if (pm.takerDebitToken == Types.QUOTE_TOKEN) {
  let fee = quoteFeeFor(pm.makerPrincipal, pm.takerDebitAmount, #makerCredit);
  creditTreasury(fee);
  pm.takerDebitAmount - fee            // ← traps if fee >= takerDebitAmount
} else { pm.takerDebitAmount };
...
let takerCredit : Nat = if (pm.makerDebitToken == Types.QUOTE_TOKEN) {
  let fee = quoteFeeFor(pm.takerPrincipal, pm.makerDebitAmount, #takerCredit);
  creditTreasury(fee);
  pm.makerDebitAmount - fee            // ← same
} else { pm.makerDebitAmount };

Motoko Nat subtraction traps on underflow. This is inconsistent with the function's own fail-closed discipline: every other fallible operation two lines above (subReserved desync) is explicitly handled with "never launder it into balances" logic and a loud log. A trap here is worse than a desync: finalisePendingMatch runs from finaliseExpiredPending() (heartbeat, line 7547), and a Motoko trap rolls back the entire heartbeat update — so one bad pending match would permanently block order expiry, oracle-stall release, cross-swap release, and every other finaliser duty, every cycle, until a manual fix.

Reachability (verified, current head):

• The only createPendingMatch call site is the taker ctx onPendingFill (line 10284), which fires only when getMakerWindow(makerOrderId) != 0.
• The only writer to orderSettlementWindows (line 1983) is AMM quote placement, gated on pool.protectionWindowSec > 0 (default 8s per AMM.mo:71).
• But the taker ctx that reads those windows also sets isNonTakeable = Principal.equal(makerOwner, ammPrincipal()) — so AMM quotes, the only windowed orders, are skipped before the window check. Net: no pending match can be created on the current wiring. (I confirmed the "DORMANT" comment is accurate by full call-graph trace.)
• Even after un-sealing, current fee constants make the trap arithmetically impossible: max TAKER_TENTH_BPS = 100 (0.1%), so fee = gross/1000 < gross for every gross >= 1; matchDebits guarantees debit >= 1 (price, quantity > 0).

Why it still matters: the code comment on the path says it "stays a TODO until this path is un-sealed" — and the precondition is already half-built (windows are live on every AMM quote; the live taker ctx reads them). The guard costs two lines now versus a hard-to-diagnose heartbeat wedge later, and it restores the fail-closed consistency the rest of the function documents.

Suggested fix (matches the sibling desync handling):

let fee = quoteFeeFor(pm.makerPrincipal, pm.takerDebitAmount, #makerCredit);
if (fee >= pm.takerDebitAmount) {
  // log loudly, drop the match fail-closed like the desync branch
} else {
  creditTreasury(fee);
  Accounts.addBalance(accounts, pm.makerPrincipal, pm.takerDebitToken, pm.takerDebitAmount - fee);
}

(or SafeMath.subOrZero + log, though an explicit branch preserves the "refund what actually released" discipline better.)

2. Stale comment: "Takers still hit this quote via the pending-match path"

src/backend/main.mo ~1985–1990 (AMM quote placement) says:

Takers still hit this quote via the pending-match path (market orders / swaps).

But the taker ProtectionCtx (line 10252) excludes AMM makers via isNonTakeable, so takers never reach AMM quotes through matching — limit takers rest, market takers walk past, and AMM quotes are only filled by ammSweepResting (line 2435 ctx, getMakerWindow = 0, immediate settlement). Whoever implements the un-seal by trusting that comment (e.g. by only relaxing isNonTakeable for market orders) would expect pending matches to appear and get none — or worse, assume the finaliser is exercised when it isn't. Worth re-writing to match the current wiring (sweep-only fills).

Testing

Static review + call-graph trace only (no dfx toolchain locally); happy to build a failing-test case for issue 1 if you point me at the test harness conventions — tests/ has Motoko suites I couldn't run locally yet.

Start in src/backend/main.mo at finalisePendingMatch and trace its call from finaliseExpiredPending, then read the surrounding fee and desync handling. Add coverage for fee underflow behavior and update the stale AMM quote comment around lines 1985–1990 to match the current sweep-only wiring. Check the Motoko suites under tests/ and follow their conventions; done means the guard and comment are covered without changing unrelated finaliser behavior.