OhShii Labs review, 6/8 · 4 findings (#9.1–#9.4): the oracle input path and the archive shed

[rvnt9999]

OhShii Labs review, 6/8: the oracle input path and the archive shed

Sixth of eight. Theme: the mark drives collateral valuation and liquidations, and the archive is the ledger of record. Both have a seam.

The Menese team covered two oracle items in #3 (items 3 and 4). Item 1 below is the same defect they isolated as their item 3 — we reached it independently and reproduce their framing, which is sharper than ours. Their item 4 (the parser dropping scientific notation and first-matching keys) we did not find and consider a good catch; it compounds directly with item 1.

---

1. The 2.5% breaker is a per-update rate limit with no absolute anchor, and the cadence is attacker-controlled

Where: main.mo:12939 (PRICE_JUMP_BPS), :12972-13045 (acceptOrPendPrice), :12901 (PRICE_MIN_SOURCES = 2), :13212-13219, src/backend/lib/PriceFeed.mo:199-213

    let jumpBps = Float.abs(Fixed.toFloat(newPrice) - Fixed.toFloat(oldPrice)) / Fixed.toFloat(oldPrice) * 10000.0;
    if (jumpBps <= PRICE_JUMP_BPS) {
      ignore Map.delete(pendingPriceJumps, Text.compare, asset);
      return true;
    };

The breaker compares the new price only to the previous accepted mark, never to an independent anchor, so a sequence of ≤2.5% moves is unbounded in aggregate. Three properties compose:

1. Cadence is user-armed. Every staged order arms a GEPTOR ~1 s out (:2297), taking accept opportunities from ~1/45 s to ~1/1.5 s — a ~30× increase any user can trigger.
2. The source floor is 2, and below 3 samples the robust median degenerates. (This is the Menese Menese DeFi Team evaluation, Part 2/2: value, integrity, and the oracle input path #3 item 3 finding.) trimOutliers returns the untrimmed set for n < 3, and the sample stddev of two agreeing sources is ≈0, so the stddevBps ≤ 50.0 gate passes trivially. Two surviving sources set the mark outright.
3. The only independent anchor is advisory. applyFreshAggregate raises _divergenceAlarms above 300 bps but never vetoes (:13212-13219), and per docs/deployment-modes.md the XRC anchor is deliberately unwired on #play — so on the live posture there is no anchor at all.

Worked walk. SOL at $200 with two attacker-influenced surviving sources: publish $204.9 (2.45%, accepted, no pend), arm the next GEPTOR with a dust staged order, publish $209.9, repeat. After ~60 accepts (~90 s) the mark is 200 × 1.0245⁶⁰ ≈ $855 — a 4.3× move with zero breaker events. Every short pool on SOL is then deeply liquidatable at a fabricated mark, and getMarginHeatmap published the exact notional resting in each 1% band beforehand, so the walk distance and the harvest are both known in advance.

The breaker's comment claims it defends "against a single source briefly returning a wildly off price" — which it does. It is presented elsewhere (docs/oracle-xrc-fallback-design.md §3.4) as bounding manipulation generally, which it does not.

Suggested direction. Enforce an absolute band against the XRC anchor — reject, not merely alarm, beyond XRC_DIVERGENCE_ALARM_BPS — and wire XRC on #play as soon as it is reachable; add a cumulative drift breaker (freeze and require confirmation when the mark has moved more than X% within a rolling window, independent of per-tick size); raise PRICE_MIN_SOURCES to 3 so trimOutliers is always active, or rule that sourceCount < 3 may only hold the mark, never move it; and enforce minimum wall-clock spacing between accepted moves so cadence is not user-controllable.

Related, unfixed elsewhere: the XRC fallback bypasses the source-quality floor that the earlier M3 fix installed on the primary path. tickXrcAnchors records xrcSources at :13174 and, by repo-wide grep, nothing ever reads it. So on the exact path where the primary was rejected as low-quality, the substitute is accepted with no quality floor whatsoever. Adding and a.xrcSources >= XRC_MIN_SOURCES to xrcAnchorFresh (:13186-13191) closes it.

---

2. geptorFetchAndSweep has no single-flight guard: concurrent fetches apply a stale price stamped fresh

Where: main.mo:3017-3056, :13201-13248 (applyFreshAggregate), :13254-13310 (refreshMultiSourcePrice)

  func processGeptorDue(now : Int) : async () {
    for ((mid, fireAt) in Map.entries(_geptorDeadline)) { if (now >= fireAt) { List.add(due, mid) } };
    for (mid in List.values(due)) {
      ignore Map.delete(_geptorDeadline, Text.compare, mid);
      ignore geptorFetchAndSweep(mid);
    };
  };

The deadline is deleted before the fetch fires and there is no per-market in-flight flag. GEPTOR_DELAY_NS = 1 s, but refreshMultiSourcePrice fans out to 8 HTTPS outcalls and returns only when the slowest completes. Whenever a fetch exceeds 1 s — the normal case — multiple chains for the same market are in flight and complete in arbitrary order.

applyFreshAggregate takes now from the caller's continuation time and writes AMM.withRefPrice(p, newPx, now). It never compares against p.refPriceUpdatedNs and never inspects the aggregate's sample times. So a chain whose readings were sampled 10 s ago can overwrite one sampled 2 s ago, and stamp the result as brand new.

That inverts the design premise stated at :3028-3031: "fetch a fresh oracle price so refPriceUpdatedNs advances past any just-rested crossing orders … they were placed before this fetch, so they can't be sniping a stale price." Every freshness gate keyed on refPriceUpdatedNs — vaultPricesStale, extMarketSwap's age check (:4931), ammShouldRequote — is then satisfied by a false timestamp, and processDeferredSwaps settles staged value against that mark.

Second-order: in-flight chains ≈ ceil(fetch_latency / 1 s) per market, each holding 8 outcalls, so a degraded oracle increases fan-out precisely when the system is stressed — positive feedback with no damper.

Note _priceRefreshInFlight exists but protects only tickPriceRefresh (:13398), and even there it is cleared outside finally (:13431), so a trap in the synchronous continuation — which is not an Error and is not caught — latches it true until the next upgrade. The shipper 6,000 lines away gets this right (tickShipEvents:7440 uses finally, which we verified also covers early return).

Suggested direction. A per-market in-flight guard released in finally, mirroring tickShipEvents; and — the load-bearing half — make applyFreshAggregate monotonic: carry the aggregate's earliest reading timestamp and refuse to apply an aggregate whose sample window predates p.refPriceUpdatedNs. Stamp refPriceUpdatedNs from the sample time, not the continuation time. Also add finally to tickPriceRefresh.

---

3. Every staged order arms an 8-outcall fetch, with no freshness precondition and no global cap

Where: main.mo:2296-2299 (arm), :3034-3056, :11936 (outcallCycles), :3403/:3490

The periodic tick runs at 30 s and only when the mark is ≥45 s old. The GEPTOR path has neither bound: one placeLimitOrder arms a fetch ~1 s out, and each fans out to all 8 PRICE_SOURCES at OUTCALL_PRICE_CYCLES = 3 B each — roughly a 30× amplification over the scheduled cadence, driven by attacker-chosen ingress, with the canister paying and the caller paying nothing.

The escalation that makes this more than a cost issue: when Cycles.balance() falls below the outcall cost, outcallCycles returns 0, every outcall fails, sourceCount = 0 < minSources(), and refPrice freezes. After MARGIN_MAX_REFPRICE_AGE_NS = 5 min, userMarksFreshAt returns false and runLiquidationBatch skips both its planning and execution phases for everyone (:3403, :3490). The fail-closed staleness design — correct in isolation — converts the drain into a liquidation moratorium whose timing the attacker chooses, protecting their own underwater pools.

Suggested direction. Skip the fetch when now - pool.refPriceUpdatedNs is under a few hundred ms and requote off the cached aggregate; add a single-tuple global counter bounding GEPTOR fetches per window across all markets and callers, checked before the fan-out; add a MIN_CYCLES_RESERVE floor at the top of geptorFetchAndSweep that degrades to requote-only; and emit an observable event on saturation.

---

4. The archive's L2 shed fires on queue depth alone, so a healthy-but-backlogged shipper destroys 50,000 events of "permanent" history

Where: main.mo:7252, :7168-7243 (shedOldestEvents), constants at :7161-7162

      // ── L2 FIRST: hard heap floor. If shipping is broken and the queue hit
      // the cap, seal the wedged archive at its acked prefix + drop the oldest
      // events (recording a gap). ...
      if (List.size(userEvents) >= shipHardCap()) { shedOldestEvents() };          // depth only
      // ── L1: roll away from a persistently-failing archive ...
      if (_shipFailStreak >= shipRollThreshold() and Time.now() >= _emergencyRollAfterNs) {

The comment says "if shipping is broken and the queue hit the cap". The code tests only the second half. _shipFailStreak exists, is maintained correctly (:7320, 7323, 7331, 7438), and is consulted by the L1 roll on the very next line — but it is not conjoined to the L2 shed.

Drain is hard-capped at SHIP_BATCH_MAX = 2_000 per HB_SHIP_NS = 10 s = 200 events/s, and neither the batch size nor the tick interval adapts as the queue approaches the cap. So a perfectly healthy archive plus ~21 minutes at 400 events/s reaches the 250k cap, and shedOldestEvents irreversibly drops 50,000 events, calls recordGap (:7107-7117), and emits a full balance re-baseline. There is no undo — those events were never shipped.

Each settled trade emits #fill for both sides plus #orderClosed, so 400/s is not an exotic rate. The same effect occurs passively during any stop or upgrade window longer than ~21 minutes at normal volume, since heartbeats do not run while the canister is stopped.

SECURITY.md names "the archive's append-only guarantees" as in scope; this produces a permanent, queryable gap with no external cause, from a mechanism designed as the response to a broken shipper.

Suggested direction. Gate the shed on _shipFailStreak >= shipRollThreshold() and depth, as the comment intends; and add back-pressure before it — raise SHIP_BATCH_MAX or shorten the tick once the queue passes ~50% of the cap — so a healthy-but-behind shipper catches up instead of amputating.

Related, in the same subsystem: the DEX tops up the Bridge and the arb from a self-reported balance (:6422-6445, :6473-6496). amount = BRIDGE_CYCLES_HIGH - bal where bal is whatever the callee returned; the comment at :6412 is explicit that canister_status — the authoritative source used for archives at :6300 — is deliberately not used because the DEX is not a controller. There is no plausibility check, no cooldown and no lifetime cap, so a callee that always answers 0 extracts 2 T every 5 minutes from each loop until the DEX is pinned at its freeze margin. deposit_cycles is irreversible. A plausibility check (a canister at rest cannot burn 1 T in 5 min) plus a rolling-window cap would bound it.

---

— Ravenith, OhShii Labs

[rvnt9999]

See #17 - our proposed remedy here is superseded; the defect stands, the fix is MAD + gate-on-surviving-cluster.

[dominicwilliams]

Three of the four findings are confirmed and fixed. One correction and one withdrawal to record.

#9.1 — the 2.5% breaker has no absolute anchor, and XRC is unwired on every #play deploy. Confirmed, and your framing was broader than the docs': play_start.sh runs setXrcCanister "(null)" on every #play bring-up, not only on the cloud-engine path the documentation described. So there was no anchor and no alarm, and xrcSources was written and never read. Fixed.

One citation correction, immaterial to the finding: you attribute the framing to oracle-xrc-fallback-design.md §3.4. It is §3.3, and it is more candid than your quote implies — it names the ratcheting risk explicitly, and §3.5 records that auto-halt was deliberately rejected for v1. The doc was more honest than the code; that is the actual failure.

#9.2 — geptorFetchAndSweep has no single-flight guard, and applyFreshAggregate stamps continuation time. Confirmed, and this was the most dangerous of the four because of the compounding: a stale sample could overwrite a fresher one and be stamped brand new, defeating every gate keyed on refPriceUpdatedNs at once. Fixed. Your observation that tickShipEvents holds the only finally in 15k lines was accurate and is now less true.

#9.4 — the archive's L2 shed fires on queue depth alone. Confirmed, including the part that makes it severe: its own comment says "if shipping is broken and the queue hit the cap", while _shipFailStreak is maintained and consulted by the L1 roll on the very next line. ~21 minutes of backlog — or any stop/upgrade window that long — permanently destroyed 50,000 events, with no adversary required. Fixed.

#9.1's proposed remedy was withdrawn — by you, and correctly. Raising PRICE_MIN_SOURCES to 3 moves the failure from "the trim never runs" to "the trim runs and mathematically cannot reject", because the band was computed over the whole set including the outlier. You withdrew it in the #17 thread after andreij6 demonstrated the masking. The raise had already landed here, which is exactly why that catch was worth having. The real fix is now a MAD-based band (50% breakdown point, so no single reading can inflate it at any magnitude) with a Croux-Rousseeuw finite-sample correction and a 50bps width floor — see #17.

Not addressed: the cycle-drain liquidation moratorium finding in this report has no verdict in our triage write-up and no code change. That is a gap in our triage, not a judgement on the finding.

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