verifyChain never anchors the recomputed tail to chainHead, so tail corruption verifies clean

[andreij6]

Found and verified with Claude Opus 5. Every line number cited below was re-checked against the published tip; the proofs are runnable from the repo root and modify nothing.

Three defects in the archive subsystem. The first is the one that needs a careful read, because it is a second, independent defect in a function that already has a filed defect. Issue #3 item 2 reports that verifyChain skips one link per page and still returns ok = true — that is a missing inbound seed. What follows is a missing outbound anchor: the loop never compares the recomputed final hash to the stored, certified chainHead. The two gaps do not overlap and do not share a fix. A single full-range call — exactly what tests/test_archive_chain.sh and the launch checklist run — has an inbound gap of precisely zero and is still blind to any corruption of the newest event. Conversely, the one-line fix suggested for #3 item 2 (seed prev from the preceding event) leaves the tail unanchored. A maintainer who applies that fix will reasonably conclude the function is now sound; it will not be. The other two findings are a leaked archive canister that no automated pass can reach after an empty-archive failover, and a per-page full sort of the caller's entire event index.

Full write-ups are in REVIEW-FINDINGS.md under the matching ## N. headings (50, 25, 26).

---

1. Finding 50 — verifyChain never anchors the recomputed tail to chainHead, so corruption of the newest event verifies clean

Where: src/backend/ArchiveCanister.mo:231-260 (verifyChain); the verification loop at :241-258; the sole link comparison at :246; the success return at :259. chainHead is declared at :55, advanced on append at :192, published into certified data at :198, and exposed by getCertifiedHead at :212-224. Append-time chain enforcement is at :175-182. Callers that trust the result: tests/test_archive_chain.sh:98, :108, :121; tests/test_archive_replay.sh:184; tests/test_archive_failover.sh:165; docs/pre-mainnet-checklist.md:187.

What is wrong

The whole verification body, unedited:

    let capped = Nat.min(limit, 10_000);
    var s = start;
    var prev : ?Types.UserEvent = null;
    var checked = 0;
    while (s < nextExpected and checked < capped) {
      switch (loadAt(globalIndex, s - base)) {
        case (?e) {
          switch (prev) {
            case (?p) {
              if (e.prevHash != ?EventChain.hash(p)) {
                return { checked; ok = false; brokenAt = ?e.seq; nextSeq = null };
              };
            };
            case null {};
          };
          prev := ?e;
        };
        case null { return { checked; ok = false; brokenAt = ?s; nextSeq = null } };
      };
      checked += 1;
      s += 1;
    };
    { checked; ok = true; brokenAt = null; nextSeq = if (s < nextExpected) { ?s } else { null } };

chainHead does not appear. The only integrity test in the function is e.prevHash != ?EventChain.hash(p) at :246 — event N's stored prevHash field against the recomputed hash of event N−1. The last event's own recomputed hash is assigned to prev at :252 for a successor that never arrives, the loop exits, and :259 returns ok = true unconditionally. Over a range of checked events the function performs exactly checked − 1 comparisons; the checked-th link — the one binding the tape's tail to the certified head — is never performed at any range, any page size, any call.

Two consequences follow directly. Corrupting the newest stored event is invisible: it has no successor, so its prevHash field is never consulted and its own hash is never checked against anything. And any consistent rewrite of a tail suffix — mutate events N−k…N and re-link each one's prevHash to its rewritten predecessor — is likewise invisible, because every comparison the function makes is internal to the rewritten suffix. chainHead, the one value the rewrite cannot forge because the subnet certified it at :198, is the only thing that would catch either case, and the function never looks at it.

The doc comment above the function (:227-228) states the opposite: "Checks BOTH directions of integrity: each event's recomputed hash equals its successor's prevHash." Read literally, that sentence describes one direction, and it is the direction that leaves the head unverified.

Why it matters

Append-time enforcement at :175-182 means corrupt data cannot enter through appendBatch, so this is a detection gap rather than an insertion path — the same qualification issue #3 item 2 makes. Within that scope it is the more consequential of the two gaps, for three reasons.

It is not bounded by page size. The #3.2 gap is pages − 1 links and vanishes at pages = 1; this gap is exactly one link and is present on every call, including the single full-range call that every test and the launch checklist actually make.

It is the link with the highest value. The head is the only hash the IC certificate vouches for. Every other link in the chain is self-referential — recomputed from bytes the canister itself serves. Anchoring the tail to chainHead is what converts verifyChain from "this tape is internally consistent" into "this tape is the tape the subnet attested to". Without it the function only ever proves the weaker statement, which is the statement a tamperer can satisfy by construction.

It is named as the permanent public audit path for sealed seasons. main.mo:14162-14166 documents the season boundary: the chain is sealed and never deleted, and the canisters live on "publicly queryable via getEventsRange/verifyChain forever". On a sealed archive there is no further append to advance the head and no successor batch to expose the discrepancy, so a tail corruption is permanently reported as clean by the endpoint the season record points auditors at.

The mitigation, stated honestly. The browser verifier does this correctly. verifyChainClient (src/frontend/src/ledger.js:194-278) compares the recomputed head against each archive's certified head, both in the lastN path (:238-240) and per archive in the full-history path (:259-261), and validates the IC certificate over certified_data at :267-274. So a user who clicks "Verify in my browser" is not exposed to this defect. The exposure is to everything that trusts verifyChain alone: the three shell tests above, operator spot-checks, and the season-record audit path. (Note that issue #4 item 3 reports the certificate leg of that browser verifier as separately non-functional; the head-anchoring comparisons at :238 and :259 are unaffected by that and do run.)

tests/test_archive_chain.sh does not close the gap either, despite appearing to. §3 (:98-104) asserts ok = true and checked = SEALED_LAST + 1 and then separately reads getCertifiedHead, but only checks its headSeq field — never that the head hash is derivable from the tape. §4 (:107, :112-116) checks that the successor archive's first prevHash equals the predecessor's published head, which again compares the successor to the published value, not the predecessor's stored tape to it. A corrupted final event on the sealed archive passes §3 and §4 both. (Minor, same lines: the §3 success message prints "$WANT/$WANT links" for WANT events, which is one more link than the function verifies even when it is working as intended.)

How to confirm

Two checks, in increasing effort.

Without building anything, against any local archive that holds events. Read headSeq from getCertifiedHead, then ask verifyChain to verify exactly that one event:

icp canister call <ARCHIVE_ID> getCertifiedHead '()' --query
icp canister call <ARCHIVE_ID> verifyChain '(<headSeq> : nat, 1 : nat)' --query

The second call returns { checked = 1; ok = true; brokenAt = null; nextSeq = null }. It entered the loop once with prev = null, took the case null {} branch at :250, compared nothing at all, and reported success. That single result is the defect in isolation: ok = true is returned by a call that performed zero hash comparisons and had a certified head sitting in scope the whole time.

With a scratch build, to see the failure mode itself. In a copy of ArchiveCanister.mo, make appendBatch store a perturbed copy of one chosen event while leaving the head computed from the original — that is exactly the state "the region bytes no longer match the certified head". Pick the target seq first from a clean run's getCertifiedHead().headSeq, so the perturbed event is the newest stored one and has no successor. In the else branch at :183-193, store { e with ts = e.ts + 1 } for that seq only, and leave :192 computing chainHead := ?EventChain.hash(e) from the unperturbed e. (ts participates in the canonical form — lib/EventChain.mo:56, the fourth field after "v1", prevHash and seq — so the stored event's hash genuinely differs.) Then, on that archive:

• verifyChain(0, 10_000) returns { checked = N; ok = true; brokenAt = null; nextSeq = null } — a clean full-range audit.
• getCertifiedHead() returns the head covering the original event, which nothing on the tape now hashes to.
• tests/test_archive_chain.sh §3 and §4 pass.
• The browser verifier throws "recomputed head ≠ archive head — the tape does not match its own hash chain" (ledger.js:239).

Choose any earlier seq as the target instead and verifyChain catches it immediately at the following event, which is the useful control: the function's detection works everywhere except the one position where the certified head is the only available witness.

Relationship to issue #3 item 2, and to issue #4

This is the point of this report. Issue #3 item 2 cites the identical line range (ArchiveCanister.mo:231-259) and issue #4 item 3 closes with a note confirming that reading. Both concern the inbound gap: var prev : ?Types.UserEvent = null at :239 is re-initialised on every call, so the first event of each page has its inbound link skipped, leaving pages − 1 links unchecked across a paged audit.

This report is the outbound gap: the value in prev when the loop exits is never compared to chainHead.

They are independent in both directions, and it is worth being concrete about it:

• On a single full-range call (verifyChain(0, 10_000) over an archive of ≤10,000 events — the call test_archive_chain.sh:98, test_archive_replay.sh:184 and test_archive_failover.sh:165 all make), pages = 1, so the #3.2 gap is exactly zero links and the tail is still unanchored. The already-filed defect contributes nothing on the code path the repository actually exercises; this one contributes on every path.
• The fix proposed for #3.2 — seed prev from loadAt(globalIndex, start - base - 1) when start > chainStartSeq — closes every boundary link and changes nothing about the return at :259. After that fix, verifyChain(headSeq, 1) still returns ok = true having verified the single inbound link and still never touching chainHead.
• Conversely, adding the outbound anchor does not close the per-page skip: a mid-range page neither starts from a seeded predecessor nor ends at the head.

A complete fix is both: seed prev on entry, and before returning ok = true, when the range reached nextExpected, compare EventChain.hash(prev) against chainHead. Fixing either alone leaves verifyChain returning ok = true over a tape that does not match its own certified head.

Correct behaviour, one line: a call that reaches the end of the tape must not return ok = true unless the last recomputed hash equals the certified chainHead.

---

2. Finding 25 — L1 archive failover abandons an empty wedged canister that no fuel, upgrade, or reset pass can reach

Where: src/backend/main.mo:7087-7103 (sealActiveArchiveAtAcked); the tracked branch at :7092, the untracked branch at :7095-7098, the shared pointer clear at :7099. Call sites: :7149 (L1 emergency roll) and :7178 (L2 shed). Enumeration: allArchivePrincipals at :6274-6280, backed by _archivesSealed (:6641), archive0 and _archiveNext. Consumers of that enumeration: tickArchiveFuel at :6297-6298 and the reset delete loop at :14173-14186. Endowment ARCHIVE_INITIAL_CYCLES at :6803, gated by archiveSpawnCycles at :6819-6825. Roll cooldown EMERGENCY_ROLL_COOLDOWN_NS at :6860, armed at :7256-7257.

What is wrong

The seal routine has two branches, and only one of them records the canister:

        let activeCount : Nat = if (shippedSeq > _activeFirstSeq) { shippedSeq - _activeFirstSeq } else { 0 };
        if (activeCount > 0) {
          List.add(_archivesSealed, { canisterId = Principal.fromActor(a); firstSeq = _activeFirstSeq; lastSeq = shippedSeq - 1 : Nat });
          logEvent("error", "system", "ARCHIVE FAILOVER (" # why # "): sealed wedged archive " # Principal.toText(Principal.fromActor(a))
            # " at its acked seq " # Nat.toText(shippedSeq - 1 : Nat) # " (kept controlled for ops recovery)", null);
        } else {
          logEvent("error", "system", "ARCHIVE FAILOVER (" # why # "): abandoned empty wedged archive "
            # Principal.toText(Principal.fromActor(a)) # " (no acked events)", null);
        };
        archive0 := null;

When the wedged archive has acked zero events, the else branch logs and nothing else. archive0 := null at :7099 runs on both paths, so the pointer is cleared either way — but the List.add(_archivesSealed, …) at :7092 is inside the non-empty branch only. The abandoned canister's principal now exists in exactly one place: the text of a log line.

That matters because allArchivePrincipals is the single enumeration the automated passes use, and it is built entirely from the three live pointers:

  func allArchivePrincipals() : [Principal] {
    let out = List.empty<Principal>();
    for (s in List.values(_archivesSealed)) { List.add(out, s.canisterId) };
    switch (archive0)     { case (?a) { List.add(out, Principal.fromActor(a)) }; case null {} };
    switch (_archiveNext) { case (?a) { List.add(out, Principal.fromActor(a)) }; case null {} };
    Iter.toArray(List.values(out));
  };

An archive that is in none of the three is in none of the passes. tickArchiveFuel iterates it (:6298) and so never tops the orphan up. The reset delete loop iterates the array it captured from the same function (:14173, deleting at :14180-14181) and so never deletes it — the orphan survives every non-production reset. adminUpgradeArchives iterates it too (:7450-7455) and so never upgrades it.

The trigger is not limited to the L1 roll. Both emergencyRollArchive (:7149) and shedOldestEvents (:7178) call this function, so any failover against an archive that has acked nothing takes the untracked branch.

Why it matters

Each orphan carries the endowment attached at spawn: ARCHIVE_INITIAL_CYCLES = 3_000_000_000_000 on a funded subnet (:6803), or whatever the engine attached under the #cloudEngine path (archiveSpawnCycles returns ?0 there, :6820, letting the engine's create path decide). Those cycles are stranded — the fuel pass cannot find the canister to top it up, and the reset pass cannot find it to reclaim it.

Two things bound the damage, and both should be stated plainly:

• It requires an archive that acked exactly zero events. A freshly rolled-in archive that takes even one batch before wedging goes down the activeCount > 0 path and is sealed and tracked correctly. The window is "spawned, installed, rolled in, and every appendBatch since has failed" — a persistently bad archive wasm after an upgrade, or a child born frozen.
• The drain is self-limiting. archiveSpawnCycles returns null — blocking the spawn with a top-up log rather than minting an underfunded child — as soon as the parent's balance falls below ARCHIVE_INITIAL_CYCLES + ARCHIVE_FEEDER_MIN_HEADROOM (:6822-6824). So on a funded subnet the loop stops itself well before the DEX is drained; the cost is the stranded endowments plus a spawn-blocked archive chain, not an unbounded burn.

Within those bounds the repeat rate is real: EMERGENCY_ROLL_COOLDOWN_NS = 60_000_000_000 (:6860) is armed at :7257 only when the roll condition fires, so the roll–spawn–abandon sequence can repeat once a minute for as long as fresh archives keep failing their first batch.

Recovery is possible but manual: main remains a controller of the orphan and the error log at :7096-7097 carries its principal, so an operator who reads the logs can stop and delete it by hand. The accurate claim is that no automated pass can reach it, and that the record it depends on for recovery is a log line rather than state.

How to confirm

By reading, in three steps. Confirm List.add(_archivesSealed, …) at :7092 is inside the activeCount > 0 branch and has no counterpart in the else at :7095-7098, while archive0 := null at :7099 is outside both. Confirm allArchivePrincipals (:6274-6280) reads only _archivesSealed, archive0 and _archiveNext. Then grep the consumers — tickArchiveFuel at :6298, the reset capture at :14173 — and confirm no other enumeration of archive principals exists anywhere in main.mo.

Empirically, on a local replica: spawn an archive chain, force a first-append failure on the freshly rolled-in archive (a deliberately broken archive wasm is sufficient — the child installs and its appendBatch traps), let _shipFailStreak reach the roll threshold, and observe the abandoned empty wedged archive <principal> log line. Then call getArchives() and confirm that principal does not appear, and run resetExchange on a non-production build and confirm via canister_status from the controller identity that the canister still exists after the delete loop has run.

Relationship to filed issues

Issue #9 item 4 is the closest overlap and quotes the adjacent machinery — the L2 shed dispatch at :7252, shedOldestEvents at :7170-7243, and the L1 roll condition four lines below at :7256. It discusses sealing the wedged archive at its acked prefix and the history gap that follows. It says nothing about the empty branch: that when the acked prefix is empty the canister is never added to _archivesSealed and therefore drops out of allArchivePrincipals. Gating the shed on _shipFailStreak as that item recommends does not touch this — it changes when the seal fires, not what the empty branch records. Worth noting that the comment at :7250-7251 shows the ordering was chosen partly to avoid "spawning one only to abandon it", so the abandonment case was in view; what is missing is the record of the canister once it is abandoned.

Issue #2 item 3 is adjacent in subject: spawned archives inherit the 3 GiB default memory limit with wasm_memory_threshold = 0 and no lowmemory hook, which is one of the ways an archive reaches the state where its appends start failing. Different defect, plausible upstream cause.

Issue #10 cites the same delete loop twice — item 1 lists resetExchange → performWorldWipe, which "stops and deletes every archive canister" (main.mo:14171-14186), as blast radius from a compromised controller key, and item 6 repeats it in the posture-drift discussion. That loop is also the only automated reclaim path for a leaked archive, and this finding is the reason it is incomplete: it deletes every archive the enumeration knows about, and an abandoned-empty archive is not in the enumeration.

Correct behaviour, one line: an archive whose pointer is cleared must remain enumerable — either recorded in a stable list that allArchivePrincipals includes, or stopped and deleted before the pointer is dropped.

---

3. Finding 26 — getEventsForPrincipals copies and re-sorts the caller's entire index on every 200-row page

Where: src/backend/ArchiveCanister.mo:301-324 (getEventsForPrincipals); the gather at :306-312, the array copy at :313, the full sort at :314, the page slice at :316-322. Callers: getMyArchivedEvents (src/backend/main.mo:6775-6794, federating at :6793) and the OQL archive federation (main.mo:14575-14586, cap at :14500).

What is wrong

Every call gathers all index entries for the requested principals, copies them into an array, and sorts the whole array — before looking at offset or limit:

    let entries = List.empty<(Nat64, Nat)>();
    for (p in principals.vals()) {
      switch (Map.get(userIndex, Text.compare, Principal.toText(p))) {
        case (?l) { for (e in List.values(l)) { List.add(entries, e) } };
        case null {};
      };
    };
    let arr = Iter.toArray(List.values(entries));
    let sorted = Array.sort(arr, func(a : (Nat64, Nat), b : (Nat64, Nat)) : Order.Order { Nat64.compare(b.0, a.0) });
    let total = sorted.size();
    let capped = if (limit > 200) { 200 } else { limit };

The sort is redundant on its own terms. The function's own comment at :299-300 states the invariant that makes it so: "Region offset is monotonic with global seq, so sorting the gathered index entries by offset descending = newest-first." If offset is monotonic with seq, each per-principal list is already in ascending order by construction, and newest-first for a single principal is a reverse walk. For several principals it is a k-way merge from the list tails. Either way the work is proportional to the page, not to the history.

To be precise about the cost, because it is easy to overstate: what is copied and sorted is the index entries — (Nat64, Nat) tuples of region offset and length, not events. Only the page's rows are decoded from the Region, one load per row inside the while at :319-322, capped at 200. So the per-page cost is O(E) allocation plus O(E log E) comparator invocations over E = the caller's total event count in this archive, plus at most 200 Region reads. Paging K pages costs K · O(E log E), since nothing is cached between calls.

Why it matters

This is a cost and latency defect, not a correctness or safety one — the endpoint is a query, it is owner-gated, and the results it returns are correct. Worth saying plainly so it is triaged accordingly.

The cost lands on the canister and scales with the heaviest users. An account with a large history in a single archive re-sorts that entire index on every page of 200 rows it reads back, so deep history becomes slowest exactly for the accounts that have the most of it. Growth is bounded per call by one archive's contents rather than by lifetime history, since archives seal and the frontend pages each archive separately — but an archive's cap is large, so the bound is not a small number.

The federation path multiplies it within a single request. getMyArchivedEvents (main.mo:6775-6794) is one call per page, so a client walk is K calls and K sorts. The OQL path at main.mo:14575-14585 loops off += 200 internally against the same endpoint until ARCHIVE_OQL_CAP = 1000 is met (:14500, commented as "≤5 pages of 200"), which is up to five full sorts of the same array inside one query, per archive visited.

How to confirm

By reading :306-322: the sort at :314 precedes any use of offset or limit, and capped is not computed until :316. Then confirm the ordering invariant that makes it unnecessary — store appends to the per-user list with a strictly increasing dataEnd, and appendBatch only stores in strict seq order (:169-174), which is what the comment at :299-300 asserts.

Empirically: seed one principal with a large event count in a local archive, then time getEventsForPrincipals([p], 0, 200) against getEventsForPrincipals([p], 0, 1). The two cost essentially the same, and both scale with the seeded count rather than with limit — a page of one row does the same work as a page of two hundred. Timing the same call at increasing seeded sizes shows the E log E term directly.

Relationship to issue #8 item 2

Issue #8 item 2 cites this same function at adjacent lines — it quotes the owner gate and its rationale comment (ArchiveCanister.mo:287-305) to argue that the gate is ineffective, because getEventsRange and getDepositWithdrawals (:326-341, :376-389) republish user and counterparty on every row and let anyone rebuild the per-principal index offline.

That is a privacy defect and this is not, and I am not re-litigating it. This finding concerns :313-314 — the body below the gate — and holds regardless of how the gate question is resolved. If the gate is kept, the sort still runs per page for every owner-federated call. If the gate is dropped as ineffective, the sort runs per page for more callers. The two findings share a function and nothing else.

Correct behaviour, one line: pagination should walk the already-ordered per-principal lists from the tail, so a page costs O(offset + limit) rather than O(E log E).

---

Provenance. Found and verified with Claude Opus 5. Every line citation above was re-checked against the working tree before filing rather than carried over from the analysis pass; where the analysis had drifted, the citation here reflects the source. The confirmation recipes read the checked-in sources and, apart from the explicitly-labelled scratch build in finding 1, modify nothing.