OhShii Labs review, 3/8 · 6 findings (#6.1–#6.6): liquidation evasion, a de-lever trap, and release priority that is purchasable

[rvnt9999]

OhShii Labs review, 3/8: liquidation evasion, a de-lever trap, and release priority that is purchasable

Third of eight. Theme: exchange mechanics where a stated guarantee — "loss stops at the pool wall", "you must always be able to de-lever", "release priority is earned, not bought" — has a seam. None requires a controller.

The first item is the one we would fix first in this group.

---

1. One base unit of ICPUSD dust makes a short margin pool permanently un-liquidatable

Where: src/backend/lib/Liquidator.mo:123-136 (pickCollateral), :444-453 (seizeOnce cross-token repay), :521-527 (the driver loop), src/backend/lib/BorrowEngine.mo:278

pickCollateral prefers ICPUSD unconditionally for any non-quote debt, selecting on balance > 0 alone with no value floor:

      for (v in vals.vals()) {
        if (v.token == Types.QUOTE_TOKEN and v.balance > 0) {
          icpusd := ?v;
        } else if (v.token != debtToken and v.balance > 0) { ... };
      };
      switch (icpusd) { case (?v) { ?v }; case null { bestBase } };

seizeOnce rolls its seize back and returns #err when the derived repay rounds to zero, and the driver aborts the entire liquidation rather than trying the next collateral:

        case (#err(e)) {
          // seizeOnce rolled back its own work. Stop here; if we'd
          // already made progress, finalise with what we got.
          if (not madeProgress) { stopErr := ?e };
          break L;
        };

Why it matters. partialSeizeQty clamps seizeQty to coll.balance (:187), so a 1-base-unit ICPUSD holding gives seizeQty = 1; Fixed.div(1, dPrice, false) = 1·10⁸/dPrice = 0 for every token priced above $1; toRepay = 0; writeOffLoan rejects zero; the seize rolls back; break L fires with madeProgress = false. The pool's real collateral is never examined. This repeats on every 30 s sweep (main.mo:3346) and every post-fill hook, indefinitely.

The comment directly above pickCollateral states the behaviour the code fails to deliver: "Fall back to the highest-$ OTHER base asset → two-leg route … This is what lets a short (owes BTC, holds only SOL) be liquidated, rather than sitting un-recoverable."

Worked example (BTC $60,000, SOL $200):

1. createMarginPool → cross pool P; fundMarginPool(P, $60,000).
2. openPosition(P, "BTC-ICPUSD", #sell, 2.125 BTC) — post-borrow health (60,000 + 0.9·127,500)/127,500 = 1.372 ≥ 1.25, and the sale is risk-reducing so it fills. Pool holds $187,500 ICPUSD, owes 2.125 BTC.
3. openPosition(P, "SOL-ICPUSD", #buy, 937.5 SOL) — the release clamp permits the whole quote balance. Pool: $187,500 SOL, quote 0, health exactly 1.25.
4. fundMarginPool(P, 1) — one base unit, $0.00000001. (main.mo:9272 rejects only amount == 0.)
5. BTC rallies 40% → health 0.89, deeply liquidatable.
6. Every sweep picks the 1-unit ICPUSD row, computes toRepay = 0, rolls back, and breaks. The $187,500 of SOL is never seized.

Impact. Complete defeat of the liquidation engine for any short/base-debt pool, for 10⁻⁸ ICPUSD. The attacker holds a free perpetual option — wait for reversion with no penalty and no forced close, or walk away leaving an unrecoverable loan. Because absorbBadDebt never runs, uncoveredBadDebtUsd and getMarginRiskSummary never show the loss, so LPs underwrite a deficit their own risk panel cannot display, and the vault's VAULT_BORROW_FRACTION_CAP capacity is consumed by zombie loans.

Suggested direction (any one works; we would do 2+3):

1. In pickCollateral, skip a candidate whose seize cannot repay — reject when Fixed.div(Fixed.mul(coll.balance, coll.refPrice, false), dPrice, false) == 0 — and prefer highest contribUsd over unconditional ICPUSD.
2. In the driver, on #err mark that token tried and continue; break L only once every collateral has been tried.
3. In seizeOnce, treat toRepay == 0 as #skip, never #err.

Adjacent: the direct (same-token) branch does not roll back its seize on repay failure (:404-406 returns #err after the subtractBalance/addBalance at :389-392, unlike the cross branch at :449-451). Bounded to ≤1 base unit today, but it contradicts the function's own contract comment at :371.

---

2. The post-fill liquidation hook bypasses the stale-mark guard the batch path enforces

Where: main.mo:7964; contrast :3400-3403 and :3486-3491

Batch path:

      // F1: never liquidate on a stale mark; resume once the oracle refreshes.
      if (userMarksFreshAt(user, now)) {
        ignore tryLiquidate(user, now);
      };

Post-fill path:

          // Phase 2B: after a fill changes this user's balance,
          // their margin health may have crossed below 1.15. Fire
          // the liquidator. If they're healthy it returns immediately.
          ignore tryLiquidate(user, timestamp);      // no freshness check

Why it matters. tryLiquidate (:8058) performs no freshness check of its own — it reads marginPriceLookup, which by design returns a frozen refPrice of any age (:5828-5836). The F1 invariant is enforced only at the two batch call sites. The rationale the code itself gives at :3487-3489 — "forcing a liquidation at a stale price could wrongly seize a healthy account or let an underwater one dodge (loss socialized to LPs)" — applies identically here. Note a >2.5% pended jump deliberately does not advance refPriceUpdatedNs, so the breaker itself manufactures the stale window.

Worked example. getAmmPool(...).refPriceUpdatedNs and getPriceFeedStats() are public, so the window is observable. Frozen mark $200, true market $240. A victim pool long 1,000 SOL with $170,000 debt reads 0.85 × 200,000/170,000 = 1.00 < 1.15 — liquidatable on paper — while the true health is 1.20. Batch sweeps skip it; any fill that places the victim into affected routes through adjustAffectedUsers → tryLiquidate and absorbs their SOL at the frozen mid: SOL worth $240,000 surrendered to clear $170,000 plus a 5% penalty. The surplus is capturable by pre-positioning in stakeInsurance or vault LP.

Suggested direction. One line — wrap the call in userMarksFreshAt. Better: move the guard inside tryLiquidate at :8068 so no future call site can miss it. This is the same decision validated in two places with only one correct.

---

3. A short cannot be closed once health falls into the 1.15–1.25 band

Where: main.mo:8366-8403 (clampToInitialMargin), applied at :2574; entry closePosition at :9602-9637

    let haircut : Int = Fixed.mul(oracle, baseLtv, false);
    let deltaPerUnit : Int = switch (side) {
      case (#buy)  { haircut - (qp : Int) };
      case (#sell) { (qp : Int) - haircut };
    };
    if (deltaPerUnit >= 0) { return #full };
    ...
    let headroom : Int = (h.collateralUsd : Int) - (Fixed.mul(Types.INITIAL_HEALTH_RATIO, h.debtUsd, true) : Int);
    if (headroom <= 0) { return #none( ... ) };

Why it matters. The gate scores a fill purely on the instantaneous LTV-weighted collateral delta and ignores the debt repayment the fill triggers. Closing a short means buying the borrowed base: the pool spends quote (LTV 1.0) for base (LTV 0.9/0.85/0.80), so deltaPerUnit = 0.9·mark − qp < 0 for any qp ≥ 0.9·mark — every realistic close price. deleveragePool (:4460) retires the loan after the fill, which the gate never sees.

Two other places assert the opposite invariant: gateInitialMargin's comment, "any trade that holds or improves health (you must always be able to de-lever)" (:8293-8294), and deriskPoolIfUnderInitial's, "Risk-REDUCING orders … are KEPT — closing is always allowed" (:4285-4287). Unlike gateInitialMargin, the clamp has no and newHealth < health.healthRatio escape clause, so it is a hard floor.

Worked example. Pool short 2 BTC, mark $60,000, debt $120,000, collateral $150,000, health 1.25. BTC ticks up 1% → health 1.238. closePosition(..., maxSlippage = 2%) stages #buy at $61,812; at release haircut = 54,540, deltaPerUnit = −7,272, headroom = 150,000 − 1.25·121,200 = −1,500 ≤ 0 → order killed. Same at any limit above 0.9·mark. The user cannot close; at health < 1.15 the liquidator takes a 5% penalty ≈ $6,060 that a permitted close would have avoided.

Longs are only partially affected — a #sell at qp ≥ ltv·mark passes — so a long can close while maxSlippage ≤ 1 − ltv, but a long choosing the permitted 25% slippage is killed the same way.

Suggested direction. Model the post-fill deleverage: for a fill whose sign is opposite to poolNetSize, credit the debt reduction. Minimum viable — return #full when the fill strictly reduces |poolNetSize(poolId, baseToken)| and does not increase debtUsd; or port gateInitialMargin's escape clause into the clamp. tests/test_partial_close.sh and tests/test_margin_pools.sh exercise closes on healthy pools only.

---

4. A free, indefinitely renewable L4 quote shield: mmQuoteStamp is set at staging and never cleared on cancel

Where: main.mo:8612-8615 (write), :8715-8724 (cancel), :390-393 (deferredCommitted), :932-945 (shield predicates)

        if (levelOf(caller) == 4) {
          Map.add(mmQuoteStamp, Text.compare, Principal.toText(caller) # "#" # marketId, timestamp);
          Map.add(mmOwnerStamp, Text.compare, Principal.toText(caller), timestamp);
        };

The cancel path clears four side maps but not the two stamps:

        ignore subReserved(d.owner, d.reservedTok, d.reservedAmt);
        removeDeferredExec(orderId);
        ignore Map.delete(deferredFok, Nat.compare, orderId);
        ignore Map.delete(deferredPostOnly, Nat.compare, orderId);
        ignore Map.delete(deferredExpiry, Nat.compare, orderId);

And post-only stages bypass the 3-second anti-free-look lock:

  func deferredCommitted(id : Nat, ts : Int, now : Int) : Bool {
    if (Option.get(Map.get(deferredPostOnly, Nat.compare, id), false)) { return false };
    now - ts < DEFERRED_COMMIT_NS;
  };

grep -n "mmQuoteStamp\|mmOwnerStamp" returns writes only at :8613-8614 and Map.clear at :14039-14040 (resetExchange). There is no other deletion.

Why it matters. The shield's justification (docs/access-prioritization-design.md:202-213) is that the MM "has seen — had the chance to react to — the price that fills them", because "their tier-priority requote/cancel in that same pass runs first". That premise requires a live staged intent. A staged-then-immediately-cancelled post-only order satisfies the stamp with no intent at all, and the reservation is refunded, so it costs nothing.

The result is displayed depth users cannot take: isMMShieldedFresh makes every resting order of that owner non-takeable by staged user takers, while ammSweepResting's ctx sets isNonTakeable = func(_, _) { false } (:2155), so the AMM still fills those quotes when they are mispriced in the maker's favour — a one-way option. isMMShieldedStale (:939-945) is owner-level and market-agnostic with a 30 s TTL, so one stamp covers the maker's entire book on all markets; a two-call loop every ~29 s holds it indefinitely, and getReleaseInfo(M) lets it be re-armed immediately after each refPriceUpdatedNs advance. Meanwhile the quotes still count toward the uptime sample that earned L4 (sampleUptime, :6050-6083, reads getUserOpenOrders with no takeability condition).

docs/market-maker-program.md:301-302 lists exactly this as an unshipped guardrail: "verify staged-then-cancelled orders don't stamp freshness (free shield refresh) — charge or exclude them."

Suggested direction. Delete both stamps in cancelOwnSpotOrder's staged branch and in cancelAllUserOrders; better, move the stamp from placement to release so an intent that never lands never shields; scope isMMShieldedStale to the market its stamp names; and publish shield-denial counts per market as the doc requires.

---

5. Release priority and shed immunity are purchasable with self-dealt volume — rank 2 costs about fifteen dollars

Where: main.mo:2843-2850 (priority sort), :753-775, :800-847 (thresholds), :6906-6911 (archiveOwnerOf), :2865-2868

Priority is a pure function of the earned level; the level is a pure function of traded volume. Self-trade prevention compares beneficial owners, and archiveOwnerOf resolves margin pools only — two sibling wallets are two beneficiaries by construction. tests/test_price_collar.sh:9-10 says so in prose: "Self-trade prevention does not see this: the two sides are genuinely different beneficiaries."

The docs' only stated anti-wash argument is fee net-negativity (docs/progressive-incentives-design.md:60-62). That defends profit; it does not defend the level.

Cost, worked. A rests a bid inside the AMM spread, B stages a marketable sell at the same price; both print at the mark, so value transfer is zero and the only cost is fees. weightedWinOf = 2·maker + taker, so A's W = 2V. effLevelThreshold(2) = $2,000,000 × max(1%, exVol/$100M); on a young venue the scale sits at the SCALE_MIN_BPS = 100 floor, so the L3 bar is $20,000 of W → V = $10,000 of self-dealt notional. At MAKER_TENTH_BPS[0] = 50 and TAKER_TENTH_BPS[0] = 100 that is 15 bps = $15, and capital needed is only the churn unit ($1,000 recycled 10×). The scale does not escape: at V = 10⁴ the right side of 2V ≥ 2×10⁶ × max(0.01, V/10⁸) is still the 1% floor.

Rank 2 buys front-of-queue in every release pass ahead of every honest L0–L2 order regardless of arrival time (tests/test_release_priority.sh:54-61 pins this for a legitimately earned L4), shed immunity at both floors, and taker fee 10 → 7 bps. At the L1 bar the same purchase is $100 of maker volume, ≈$0.15 in fees, for rank 1.

Compounding factor. Scorecard volume is credited on exactly one settlement path. grep -n "bumpPartyVolume\|exVolCur +=" returns one credit site with one caller, processDeferred (:2752). Four paths that record real trades never reach it: ammSweepResting (:2180-2187), processDeferredExpiry's users-only fallback (:2813-2820), releaseCrossSwap's two legs (:2975, :2996), and the synchronous noPartialFill cross-swap (:11135-11138). ammSweepResting is the primary way the AMM fills resting user orders — the behaviour the README describes as the protocol's quotes yielding to resting user orders. So an honest maker filled by the sweep earns zero maker credit toward the ladder built to reward market making, and exVolCur is under-counted, holding levelScaleBps() at the floor far longer than real volume warrants — which is what makes the purchase above cost $15 instead of $1,500.

Suggested direction. Exclude related-party volume (the wash-detection fold docs/play-anti-sybil-design.md:114-124 schedules for Phase 3); cap the share of a key's window volume attributable to one counterparty; raise SCALE_MIN_BPS or floor the L3/L4 bars absolutely; factor the volume credit into a single creditTradeVolume(trades) helper invoked wherever updateStatsAfterTrades is, so the two cannot drift again. No test asserts that a two-principal round trip does not move myLevel; tests/test_access_levels.sh:113-117 asserts the opposite.

---

6. STAGED_CAP_PER_OWNER is keyed on the raw principal, but one account controls 65 of them

Where: main.mo:2243-2246, :814-816, :3544, :9410/:9631, :6115-6121

    // Per-owner staged-queue cap: bounds deferred-queue occupancy per principal
    // so no single (funded, registered) caller can crowd the release pass.
    if (not isInternalPrincipal(owner) and stagedCountOf(owner) >= STAGED_CAP_PER_OWNER) { return null };

stagedCountOf keys on Principal.toText(owner), but openPosition/closePosition stage under the pool principal and MAX_POOLS_PER_OWNER = 64. One account therefore gets 65 independent 32-slot budgets = 2,080 staged entries, while tierRankOf resolves every pool through scorecardKeyOf to the owner's level — so all 65 sort at the same rank with FIFO only within the tier. One owner can occupy up to 2,080 front-of-queue slots, which is what the cap's own comment says it prevents.

Second-order: recomputeShedFloor keys on the global Map.size(deferredExecs) and SHED_SOFT_STAGED = 2_000 is below one account's ceiling. inspect then refuses every caller below the floor pre-consensus — and the attacker buys themselves out for $0.15 via item 5, so the floor they raise excludes everyone but themselves.

Suggested direction. Key stagedCountByOwner on scorecardKeyOf(owner) — the same key the level and fee ladders already use; the mismatch between the cap key and the tenancy boundary is the bug. Additionally cap per-owner occupancy of a single release pass and round-robin across keys, and drive _shedFloor from distinct-owner depth. Note tests/test_tier_shed.sh only exercises the setTestShedFloor dev pin, which requireDevHook makes unreachable on #play, so real depth is untested.

---

— Ravenith, OhShii Labs

[rvnt9999]

Precision on our SAFE note: the link resolves for replaceMyOrder/cancelAll, but the public cancelMyOrder doesn't consult it; @andreij6 #14 is right on that endpoint.

[dominicwilliams]

The highest-scoring report of the eight. Five of six findings are fixed; one is not, and I want to be explicit about which.

#6.1 — one dust payment makes a short pool permanently un-liquidatable. Confirmed, and verification escalated it three ways, two of which change the severity:

1. It is not "exactly one base unit". Any dust below the debt token's whole-unit price triggers it — the real window is ~$0.00168, double your estimate, because the derived repay floors twice (once converting to debt units, once applying the penalty multiplier).
2. pickCollateral's same-token first pass has the identical unconditional-balance defect. That is a second, independent route to the same trap; your report treated it as a bounded ≤1-unit leak.
3. It is self-inflicting — no dust payment and no attacker are required. The cross-token seize refunds a remainder (retainedColl rounds up), and that refund is itself sub-threshold dust. Measured on the pre-fix code: a pool holding $10,000 ICPUSD + $200,000 SOL has its ICPUSD seized first, creating a 160,000-unit residue below the 168,000 threshold; the next iteration picks that residue, floors to zero, and breaks — leaving all 1000 SOL untouched and returning #insolvent. absorbBadDebt then socialises a loss to the insurance fund against a fully-collateralised position.

So the trigger is not "someone sends dust" but "any partial close whose seize is balance-capped". Fixed.

#6.2 — the post-fill liquidation hook has no freshness guard. Confirmed: both batch call sites had one, adjustAffectedUsers did not, and tryLiquidate performed none of its own while marginPriceLookup returns a frozen refPrice of any age — with the breaker manufacturing the stale window, since a pended jump deliberately withholds refPriceUpdatedNs. Fixed by putting the guard inside tryLiquidate, per your own recommendation, so no future call site can miss it.

#6.3 — a short cannot be closed in the 1.15–1.25 health band. Confirmed, and the algebra is worse than described: headroom is independent of trade size and is ≤ 0 exactly when health ≤ 1.25, so #partial is unreachable rather than merely rare. Fixed.

#6.6 — STAGED_CAP_PER_OWNER keys on the raw principal. Confirmed: pools stage under pool principals, giving one account 65 × 32 = 2,080 slots — above SHED_SOFT_STAGED, so one account could raise the shed floor that excludes everyone else. Fixed.

#6.4 — mmQuoteStamp/mmOwnerStamp cleared nowhere except resetExchange. Confirmed: a staged-then-cancelled post-only order bought a free, renewable, market-agnostic L4 shield. Fixed.

#6.5 — volume credit reaches the scorecard on exactly one settlement path. Confirmed, NOT fixed. All four bypassing paths are real: AMM-sweep fills, both cross-swap legs, and the expiry fallback. Honest makers filled by the AMM sweep earn nothing, and the under-counted exchange volume pins the level scale at its floor — which is, as you identified, why buying rank 2 costs ~$15. Routing all four through one creditTradeVolume helper touches the settlement path in four places and deserves its own change with its own fee-conservation tests, rather than riding along inside a security pass. It is open.

One later correction to our fix, from a third reviewer. #15 found that #6.1's remedy fixed the dust trap and the missing rollback but left the classifier reading only the resulting health, so budget exhaustion could still be reported as insolvency. That is now fixed too, via a named classifySeizingPass with an invariant asserted across every liquidation shape in the test file: #insolvent may only be returned when nothing seizable is left.

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