You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
OhShii Labs review, round 2 — 4/6 · 5 findings (#26.1–#26.5): the archive chain — one bad segment fails every read, and the segments nobody can fix are the ones nobody observes #26
Round 2, part 4 of 6. The archive sidecar chain is the venue's permanent ledger and the backing
for the History surface, the OQL deep-history path and the proof-of-reserves replay. Five items,
all in the read/observability/lifecycle half rather than the write half.
Delineation, stated up front because three issues are adjacent here.
Menese DeFi Team evaluation, Part 1/2: bounding work and state under load #2 (Menese) item 3 reports that archives are spawned without wasm_memory_limit / wasm_memory_threshold and that ArchiveCanister.mo has no lowmemory() hook. Different
defect from anything below; their memory wall is an additional trigger for item 2 here, not
the same finding.
allArchivePrincipals() is built from exactly three sources: _archivesSealed (:6276), archive0 (:6277), _archiveNext (:6278). performWorldWipe(true) empties all three in one
message.
tickArchiveFuel is the only automatic funder and iterates exactly that list at :6298;
both of its fundArchive calls (:6325 watermark, :6339 blind-fund) are inside the loop. fundArchive has exactly three callers — :6325, :6339, and adminFundArchive (:6348),
which funds archive0 only and returns #err("no archive sidecar spawned yet") when it is null.
Repo-wide, deposit_cycles in first-party source appears at :6284 (archives), :6439/:6453
(Bridge) and :6490 (arb) — the last three take effectiveBridge/effectiveArb, never a
caller-supplied principal. There is no endpoint that deposits cycles into an arbitrary
canister id, and List.add(_archivesSealed, …) occurs only at :7092 and :7402, both inside
the ship path — there is no re-attach.
The prior season's archives therefore hold their data, are unreachable from the funding loop, and
drain until the IC deletes them. #18 Finding 25's correction is what makes this reachable: the forSeason branch detaches without deleting, so the canisters genuinely survive the reset —
they are simply no longer anyone's responsibility.
Fix direction. Keep a separate append-only registry of sealed principals that survives the
detach and that tickArchiveFuel iterates, or add an adminFundCanister(principal, amount)
gated to controllers so a detached segment can be rescued at all.
2. One unreachable segment fails every History and OQL read, for every caller
Where:main.mo:14575 (chain loop), :14581 and :14595 (the two federated awaits), :14550 (no auth check); contrast :7459-7472
Both federated awaits sit bare inside label chain for (cid in List.values(visit)). There is no try/catch, no partial result and no skip for a segment already observed frozen. One rejected
hop fails the entire composite query.
The same file already does this correctly: adminUpgradeArchives wraps each per-archive hop
in try { … } catch (_) { … } at :7459-7472. This is a sibling fix applied to one of two
places.
Triggers, none of which requires a fault injected by an attacker:
adminUpgradeArchives (:7450) walks allArchivePrincipals() upgrading each segment in
place; any archiveExecute landing inside a segment's upgrade window hits a reject;
a blackholed segment whose status can no longer be read (item 3 below).
archiveExecute (:14550) has no auth check at all, so an anonymous caller reaches the loop
— which means the blast radius is every user of the History page, not only the caller who
triggered it.
Fix direction. Wrap each hop and return what was gathered: catch (_) { List.add(degraded, cid); continue chain }, plus a degraded : [Principal] or complete : Bool field on the result so the UI can say "segment X unavailable" instead of
failing the page. Mirror :7459-7472. Additionally skip any cid whose _archiveObs entry
reports frozen = true.
3. Blackholed segments are never re-observed, so the chain table shows a stale ok forever
_archiveObs is written only inside the try, after ic00.canister_status (:6300)
succeeds — and canister_status requires controllership. Seal-time blackholing
(update_settings { controllers = ?[] } at :7413-7426, gated by _blackholeAtSeal, default false at :6745) permanently removes that controllership. From then on the status call
rejects, control falls to the catch at :6326, and _archiveObs for that principal is never
refreshed again. getArchiveChain's mk (:6378-6390) serves whatever is in _archiveObs, and
the frontend renders it.
So the segments that are immutable by design and therefore unfixable are exactly the segments
whose health is never observed again, and the dashboard reports the last-known ok indefinitely.
The fix already exists 100 lines away and is not applied.tickBridgeFuel (:6422-6445)
reads the callee's own cyclesBalance() query at :6425-6426precisely because canister_status would need controllership — the comment at :6411-6413 says so. ArchiveCanister.stats() (ArchiveCanister.mo:346-370) is a public query returning cycles,
and the frontend already calls it directly. The affordance exists, is already consumed elsewhere,
and is the one thing that restores visibility on the segments that cannot be status-read.
Fix direction. Fall back to stats()/cyclesBalance() when canister_status rejects, and
mark the observation as degraded rather than leaving the previous one in place. Also log the
silent skip at :6337 — today a blackholed segment that fails the blind-fund headroom test is
skipped with no event at all.
4. An archive spawn is unrecorded across its own await, and the epoch-race cleanup swallows the principal
Two related lifecycle leaks at the same three spawn sites.
(a) Unrecorded spawn.archive0 := ?fresh / _archiveNext := ?fresh happen only in the continuation of await Archive.Archive(...). The canister exists the moment the call returns;
the record of it exists only if the continuation runs. An upgrade landing in that window leaks a
3T-cycle canister that main controls but can no longer fund (allArchivePrincipals never learns
of it), list, or delete.
(b) Swallowed cleanup. When a reset races a spawn, the epoch guard destroys the orphan with try { await ic00.stop_canister(...); await ic00.delete_canister(...) } catch (_) {} at :7293-7297 and :7377-7380. A bare swallow: if either call fails, the principal — which exists
only in that continuation — is lost with no trace on-chain and no log line.
Note this is not#2 item 3: their finding is about the settings the child is created with
(memory limit, threshold, lowmemory()); ours is about the spawn not being recorded. Applying
memory settings at creation still leaks the canister; persisting the principal before the await
still ships a 3 GiB/threshold-0 archive. Both fixes are wanted.
Fix direction. Write the intended principal to a _pendingSpawn field before the await —
under enhanced orthogonal persistence a plain var is already stable, so this is a one-line
change — and have a heartbeat subtask reconcile it. Replace the bare catch (_) {} with a log
line carrying the principal.
5. adminReplayStep has no single-flight guard and advances its cursor only after the await
adminReplayStep is the executable proof-of-reserves audit: it folds the archived tape forward
and compares the result against live balances. _replayCursor is read before the await at :7543 and advanced only in the continuation at :7576, and there is no in-flight guard.
Two overlapping controller calls — a double-click on an ops page, or a retry after a slow reply —
both read the same cursor, both fold the same page, and the tape is double-counted. The output is
a false reserve alarm on a healthy venue, which is the worst possible failure mode for an
integrity check: it burns operator trust in the one mechanism meant to detect real divergence.
Fix direction. The pattern is already in the file: guard with a flag set before the first
await and cleared in a finally, exactly as tickShipEvents does at :7245-7246 and :7440
(which we verified is correct on every axis — it is the repository's only finally and it earns
it). Advance the cursor optimistically before the await and roll it back on the error arm, or
carry the expected cursor into the response so a stale fold is detectable.
Round 2, part 4 of 6. The archive sidecar chain is the venue's permanent ledger and the backing
for the History surface, the OQL deep-history path and the proof-of-reserves replay. Five items,
all in the read/observability/lifecycle half rather than the write half.
Delineation, stated up front because three issues are adjacent here.
getEventsForPrincipals' full gather-and-sortbefore the page cap. We had that independently and are not re-filing it; credit is his.
verifyChain never anchors the recomputed tail to chainHead, so tail corruption verifies clean #18 Finding 25 also corrects our OhShii Labs review, 7/8 · 7 findings (#10.1–#10.7): deploy pipeline, controller-key hygiene, and posture/documentation drift #10 item 1, which we accept — and item 1 below builds on that
correction rather than disputing it.
wasm_memory_limit/wasm_memory_thresholdand thatArchiveCanister.mohas nolowmemory()hook. Differentdefect from anything below; their memory wall is an additional trigger for item 2 here, not
the same finding.
verifyChainpage boundaries corrects a clearance of ours; accepted,not re-filed, and nothing below touches
verifyChain.1. The season detach drops sealed archives out of every funding path, so the "permanent" ledger drains until the IC deletes it
Where:
main.mo:14161,:14167-14169(the detach),:6274-6280(allArchivePrincipals),:6297-6344(tickArchiveFuel),:6282-6290(fundArchive),:6348-6353(adminFundArchive)allArchivePrincipals()is built from exactly three sources:_archivesSealed(:6276),archive0(:6277),_archiveNext(:6278).performWorldWipe(true)empties all three in onemessage.
tickArchiveFuelis the only automatic funder and iterates exactly that list at:6298;both of its
fundArchivecalls (:6325watermark,:6339blind-fund) are inside the loop.fundArchivehas exactly three callers —:6325,:6339, andadminFundArchive(:6348),which funds
archive0only and returns#err("no archive sidecar spawned yet")when it is null.Repo-wide,
deposit_cyclesin first-party source appears at:6284(archives),:6439/:6453(Bridge) and
:6490(arb) — the last three takeeffectiveBridge/effectiveArb, never acaller-supplied principal. There is no endpoint that deposits cycles into an arbitrary
canister id, and
List.add(_archivesSealed, …)occurs only at:7092and:7402, both insidethe ship path — there is no re-attach.
The prior season's archives therefore hold their data, are unreachable from the funding loop, and
drain until the IC deletes them.
#18Finding 25's correction is what makes this reachable: theforSeasonbranch detaches without deleting, so the canisters genuinely survive the reset —they are simply no longer anyone's responsibility.
Fix direction. Keep a separate append-only registry of sealed principals that survives the
detach and that
tickArchiveFueliterates, or add anadminFundCanister(principal, amount)gated to controllers so a detached segment can be rescued at all.
2. One unreachable segment fails every History and OQL read, for every caller
Where:
main.mo:14575(chain loop),:14581and:14595(the two federated awaits),:14550(no auth check); contrast:7459-7472Both federated awaits sit bare inside
label chain for (cid in List.values(visit)). There is notry/catch, no partial result and no skip for a segment already observed frozen. One rejectedhop fails the entire composite query.
The same file already does this correctly:
adminUpgradeArchiveswraps each per-archive hopin
try { … } catch (_) { … }at:7459-7472. This is a sibling fix applied to one of twoplaces.
Triggers, none of which requires a fault injected by an attacker:
adminUpgradeArchives(:7450) walksallArchivePrincipals()upgrading each segment inplace; any
archiveExecutelanding inside a segment's upgrade window hits a reject;archiveExecute(:14550) has no auth check at all, so an anonymous caller reaches the loop— which means the blast radius is every user of the History page, not only the caller who
triggered it.
Fix direction. Wrap each hop and return what was gathered:
catch (_) { List.add(degraded, cid); continue chain }, plus adegraded : [Principal]orcomplete : Boolfield on the result so the UI can say "segment X unavailable" instead offailing the page. Mirror
:7459-7472. Additionally skip any cid whose_archiveObsentryreports
frozen = true.3. Blackholed segments are never re-observed, so the chain table shows a stale
okforeverWhere:
main.mo:6297-6344,:6300,:6306-6309,:6326-6341,:6377-6390,:7413-7426,:6745;ArchiveCanister.mo:346-370_archiveObsis written only inside thetry, afteric00.canister_status(:6300)succeeds — and
canister_statusrequires controllership. Seal-time blackholing(
update_settings { controllers = ?[] }at:7413-7426, gated by_blackholeAtSeal, defaultfalseat:6745) permanently removes that controllership. From then on the status callrejects, control falls to the catch at
:6326, and_archiveObsfor that principal is neverrefreshed again.
getArchiveChain'smk(:6378-6390) serves whatever is in_archiveObs, andthe frontend renders it.
So the segments that are immutable by design and therefore unfixable are exactly the segments
whose health is never observed again, and the dashboard reports the last-known
okindefinitely.The fix already exists 100 lines away and is not applied.
tickBridgeFuel(:6422-6445)reads the callee's own
cyclesBalance()query at:6425-6426precisely becausecanister_statuswould need controllership — the comment at:6411-6413says so.ArchiveCanister.stats()(ArchiveCanister.mo:346-370) is a public query returningcycles,and the frontend already calls it directly. The affordance exists, is already consumed elsewhere,
and is the one thing that restores visibility on the segments that cannot be status-read.
Fix direction. Fall back to
stats()/cyclesBalance()whencanister_statusrejects, andmark the observation as degraded rather than leaving the previous one in place. Also log the
silent skip at
:6337— today a blackholed segment that fails the blind-fund headroom test isskipped with no event at all.
4. An archive spawn is unrecorded across its own await, and the epoch-race cleanup swallows the principal
Where:
main.mo:7281-7301,:7293-7297,:7373-7390,:7377-7380,:7134-7144Two related lifecycle leaks at the same three spawn sites.
(a) Unrecorded spawn.
archive0 := ?fresh/_archiveNext := ?freshhappen only in thecontinuation of
await Archive.Archive(...). The canister exists the moment the call returns;the record of it exists only if the continuation runs. An upgrade landing in that window leaks a
3T-cycle canister that main controls but can no longer fund (
allArchivePrincipalsnever learnsof it), list, or delete.
(b) Swallowed cleanup. When a reset races a spawn, the epoch guard destroys the orphan with
try { await ic00.stop_canister(...); await ic00.delete_canister(...) } catch (_) {}at:7293-7297and:7377-7380. A bare swallow: if either call fails, the principal — which existsonly in that continuation — is lost with no trace on-chain and no log line.
Note this is not #2 item 3: their finding is about the settings the child is created with
(memory limit, threshold,
lowmemory()); ours is about the spawn not being recorded. Applyingmemory settings at creation still leaks the canister; persisting the principal before the await
still ships a 3 GiB/threshold-0 archive. Both fixes are wanted.
Fix direction. Write the intended principal to a
_pendingSpawnfield before the await —under enhanced orthogonal persistence a plain
varis already stable, so this is a one-linechange — and have a heartbeat subtask reconcile it. Replace the bare
catch (_) {}with a logline carrying the principal.
5.
adminReplayStephas no single-flight guard and advances its cursor only after the awaitWhere:
main.mo:7517-7520,:7529,:7543,:7545-7578,:7576adminReplayStepis the executable proof-of-reserves audit: it folds the archived tape forwardand compares the result against live balances.
_replayCursoris read before the await at:7543and advanced only in the continuation at:7576, and there is no in-flight guard.Two overlapping controller calls — a double-click on an ops page, or a retry after a slow reply —
both read the same cursor, both fold the same page, and the tape is double-counted. The output is
a false reserve alarm on a healthy venue, which is the worst possible failure mode for an
integrity check: it burns operator trust in the one mechanism meant to detect real divergence.
Fix direction. The pattern is already in the file: guard with a flag set before the first
await and cleared in a
finally, exactly astickShipEventsdoes at:7245-7246and:7440(which we verified is correct on every axis — it is the repository's only
finallyand it earnsit). Advance the cursor optimistically before the await and roll it back on the error arm, or
carry the expected cursor into the response so a stale fold is detectable.
— Ravenith, OhShii Labs