OhShii Labs review, round 7 · 5 findings (#44.1–#44.5): four instruments that stop meaning what they say on the day custody lands, and one input an approver cannot verify

Round 6 (#41) asked whether the code is ready for the owner §11 names. This round asked the narrower question underneath it: when debit → await transfer lands, what will be true that is not true now, and what in the repository will notice?

The answer is not a reentrancy bug. You have that covered and you said so first. src/backend/mixins/UserAccount.mo:180-196 states the property, states why it holds today, states what breaks when custody wires the real transfer, and enumerates the saga in the right order — including the step most teams get wrong: a confirmed failure refunds exactly once, an ambiguous outcome reconciles against the ledger before acting, "never blind-refund … never blind-drop". It then declines to add an in-flight guard because "a guard is pointless while the body is synchronous — adding it now would be dead code that protects nothing."

That is a better hazard analysis than this round produced, and it is why the round went looking somewhere else.

Severity

	[play / prod]
#44.1	INFO / MEDIUM	the reconciliation predicate stops meaning "nothing is in flight"
#44.2	INFO / MEDIUM	Proof-of-Reserves is computed on one side only, and the gap it cannot see stops being zero
#44.3	INFO / MEDIUM	the permanent event record has no third state for an unresolved outcome
#44.4	LOW / MEDIUM	the canonical byte form has three hand-written encoders and nothing compares them
#44.5	INFO / MEDIUM	the posture that renders the test surface inert is the one input an approver cannot verify

Nothing in this issue is live-exploitable. These are readiness findings about a transition that has not happened, and they are rated on that basis.

One live item from this round is deliberately not here: GHSA-c6g7-mf5q-mgfq, filed privately today. SECURITY.md asks that security problems not be opened as public issues, and that channel works now — so a defect in shipped code goes there even when, as here, it is low severity and bounded by existing authentication and rate limits. In outline: a bound in the AI path measures the wrong unit, so it admits several times the budget its own comment states. It is a units bug in a remedy we proposed to you in round 1, which makes it ours before it is yours.

We are naming its existence, its channel and its reason rather than leaving a silent gap, because a round that says "five findings" while a sixth sits somewhere else is a round that has misrepresented itself. What it is, and where, is in the advisory.

---

#44.1 — the reconciliation predicate stops meaning "nothing is in flight" [play:INFO / prod:MEDIUM]

main.mo:7960-7962 — done is _replayCursor >= shippedSeq and List.size(userEvents) == 0 and List.size(accounts.journal) == 0. Your gloss at :7888-7890 reads it as "nothing is still in flight toward it … i.e. the fold is comparable against live state."

Today that is exactly right, because the withdrawal body is synchronous: "in flight toward the tape" and "in flight" are the same set.

After debit → await transfer they are not. The debit journals its #delta and its event before the await, so all three conjuncts can be true while an operation is open. The predicate keeps returning true and stops meaning what its comment says.

The second half is sharper. adminReplayReport diffs the tape fold against live Accounts. Both sides see the debit — so a mid-saga withdrawal produces zero mismatches, and so does a custody double-pay, because the internal ledger was debited once either way.

Dedup. #26.5 is about adminReplayStep and is ours — it concerns a missing single-flight guard, a today defect that produces a false alarm. This is the same function failing in the opposite direction, silently, after a transition. And @andreij6's #43, filed this morning, is the cursor itself diverging inside the ship path: under his inflated shippedSeq the first conjunct fails closed and done can never return true, where this is about it failing open. The two are mutually exclusive states, and a correct shippedSeq is the precondition under which this claim bites.

#44.2 — Proof-of-Reserves is computed on one side only [play:INFO / prod:MEDIUM]

The liabilities half is implemented four times — src/frontend/src/ledger.js:15, scripts/verify_ledger.mjs:26-27 and :459, main.mo:15154. No site in src/, scripts/ or tests/ reads a custody balance. Needle solvency|liabilit, 40 code hits, all read individually; control, the same needle over docs/ returns 21.

Three of the four commitment sentences we found scope themselves — wallet-and-positions-design.md:9 says "in production", index.html:2005-2007 is conditional on custody addresses being twinned, and bridge-and-cks-design.md's is a proposal. The finding rests on the one that does not: docs/oracle-settlement-and-transparency.md:39-42, present tense, unqualified, and about the shipped page rather than a plan — the public Ledger page "lets anyone re-hash that chain against the subnet-certified head and fold it into a Proof of Reserves: total assets held ≥ total owed to users, verifiable by anyone, trustlessly."

Today the gap between the two halves is identically zero because custody holds nothing, which is why one-sided suffices and is not a defect. On the day the transfer lands, the gap becomes the in-flight withdrawal set — and it is the one quantity neither half measures.

#44.3 — the permanent record has no third state [play:INFO / prod:MEDIUM]

lib/Types.mo:155 — kind : { #deposit; #withdrawal }. Two arms, no representation for an outcome that has been attempted and not resolved. Step 3 of your own saga at UserAccount.mo:191-193 requires exactly that: an ambiguous outcome must be recorded and reconciled before anything acts on it, and the chain is the only durable, tamper-evident place in the system to record it.

Two mutations, two different profiles, and the distinction matters. A new top-level event kind is loud: EventChain.mo's exhaustive switch is a compile error. A third DepositRecord.kind arm is caught three times by Motoko and is silent everywhere else — the five JavaScript consumers are ternaries falling back to "deposit", including scripts/verify_ledger.mjs:271, where it would be signed +1 into the liabilities fold that #44.2 is about.

EventChain.mo:15-16 states the constraint that makes this a design decision rather than a patch: "Field order is FROZEN (v1); additive event kinds append new tag branches but never reorder existing fields."

#44.4 — three hand-written encoders and nothing compares them [play:LOW / prod:MEDIUM]

The canonical byte form of every permanent event exists three times: lib/EventChain.mo:51-107 (Motoko, canister and archive), src/frontend/src/ledger.js:34-103 (self-described "mirror" at :5), and scripts/verify_ledger.mjs:133-193 ("deliberately SELF-CONTAINED", :4).

Every integrity check inside the canister has EventChain.hash on both sides of the comparison, so it cannot observe the byte form at all — it can only confirm the encoder agrees with itself. Divergence between the three is exactly the failure the tamper-evidence property exists to exclude, and only the two out-of-canister implementations could detect it.

Neither is exercised. Needle EventChain over tests/ returns zero files; controls on the same needle and roots — EventChain over src/ returns 4, Accounts over tests/ returns 7 — so the needle is live and the roots are reachable. Of 19 modules in src/backend/lib/, 15 have a tests/*.test.mo; the four without are Profiles, Types, UserStatus — and EventChain. Three of those four are largely declarations. The one that computes the tamper-evidence hash is the untested one.

#44.5 — the posture is the one input an approver cannot verify [play:INFO / prod:MEDIUM]

Your method surface is verifiable and that is worth saying plainly: candid/backend.did is generated from the actor by scripts/gen-did.sh, every posture-gated test method is in it, and mops.toml:26 publishes it as candid:service metadata — so a deployed module carries a faithful list of its own methods. Not every project can say that.

Motoko as used here has no compile-time exclusion — no cfg, nothing — so those methods ship in every module by construction, and their inertness is entirely if (isProduction) { return } against DEPLOY_MODE, a transient let literal at main.mo:102.

That value appears nowhere in the interface, nowhere in the metadata, and nowhere in the artefact. The only witness is getDeployMode() — the canister reporting on itself. Combined with #41.1 (no module→commit path, and the published source does not build), an approver voting on an InstallCode proposal can enumerate the methods and cannot verify the value that decides whether they are live.

---

What this round cut from its own drafts

Forty-nine candidates became six.

• Its central thesis was refuted mid-round, by us. "A requireController-only method has no 44S-reachable caller" was fifteen candidates and zero findings: main.mo:6472-6485, system func postupgrade(), writes stable variables, so an InstallCode proposal can reconfigure any of them. That correction is in #41.
• One finding was withdrawn because it was already ours. It restated #38.5 — same file, same section, same two assertions. Our dedup had been reading our working notes rather than our filed issue bodies, so it could certify our own work as novel. Now fixed; described on #41.
• Two would-be findings turned out to be status notes on #5, finding 4 — see #44.1.
• Three counts were wrong until re-derived, and a grep -c whose error was aggregated into a total reported that error as a zero — it was about to support #44.5.
• Our own citation instrument was blind to 53% of the references in our published issues. Rebuilt with ambiguous basenames reported rather than guessed; re-run over every issue we have filed. No citation of ours points at the wrong line — the exposure was real and it cost nothing, which is luck rather than process.

Start by reviewing the reconciliation predicate in main.mo, the event model in lib/Types.mo, the encoder in lib/EventChain.mo, and the JavaScript mirrors in src/frontend/src/ledger.js and scripts/verify_ledger.mjs. Then inspect DEPLOY_MODE at main.mo:102 and the generated interface path. Done requires resolving the five readiness findings with repository-visible evidence that custody state, unresolved outcomes, canonical encoding, and test posture are represented and verifiable.