soft3/radio/audit/2026-10-10-bao-root-policy/code-review-approved.txt

# CODE review — canonical BAO ROOT finalization (candidate v4 `9ce51f8d…`, parent `258724bd`, Hemera `23f3bbcf`)

**Verdict: APPROVE**

PLAN v3's approval is not inherited; I re-derived the semantics from the supplied bytes. The implementation is correct against pinned Hemera authority, ROOT is applied exactly once and only on a terminal complete-file node, all traced producers/verifiers/stores/tickets agree, and the scope boundaries (public signature, counters, odd promotion, allocation, geometry, `Hash::EMPTY`, MemStore width) are respected. Findings below are receipt/traceability items and pre-existing defects; none is a new regression.

## What I verified in the diff

**Single shared reducer.** `cyber-bao/src/io/mod.rs::hash_group<B: HashBackend>` is the only reduction left. The four private clones are gone (`outboard.rs::hash_block`, `decode.rs::hash_block_for_verify`, `slice.rs::hash_block_for_verify`, `sync.rs::hash_subtree`) and their call sites now use `super::hash_group`. The sync argument-order hazard was handled correctly: `hash_subtree(&backend, 0, &tmp, true)` → `hash_group(&backend, &tmp, 0, true)` and `hash_subtree(backend, start_chunk, &buf, is_root)` → `hash_group(backend, &buf, start_chunk, is_root)`. Public `io::hash_block` keeps `block_bytes` plus `let _ = block_bytes;` and delegates, so `fsm.rs`, `mixed.rs`, `sync::encode_ranges_validated` and `iroh-blobs/src/util.rs::outboard_with_progress::hash_subtree` inherit the fix with no edit — consistent with semver 202/202 pass on both crates.

**ROOT applied exactly once, only at the file root.** `is_root && level.len() == 2` is evaluated on the *current* level before `next` is built; `while level.len() > 1` means `len == 2` is reached exactly once and is always the final iteration (`ceil(2/2) == 1`). No earlier parent and no promoted odd node can receive ROOT. Reaching `hash_group(.., is_root = true)` with more than one chunk requires `BaoChunk::Leaf{is_root: true}`, which `tree.rs` emits only via the `blocks == 1` early return in `pre_order_chunks`/`post_order_chunks`: for `blocks >= 2` the root node `blocks.next_power_of_two() - 1` always has a populated right child (`blocks > npot/2`), so `right_exists` is true and `is_root` is never inherited down to a leaf. Equivalently `hash_group(is_root=true)` ⟺ the group *is* the whole file. Empty/single-chunk and multi-group roots are therefore unchanged, and the delta is confined to `4096 < N <= 4096·2^g` (log4: 4097..65536).

**Behaviour-preserving otherwise.** Chunk counters remain absolute (`counter = start_chunk + i`), `is_single_chunk = data.len() <= CHUNK_SIZE && is_root` matches the old per-site logic including the old `sync` `len <= CHUNK_LEN` shortcut, odd promotion still `level[i].clone()` (generic `B::Hash: Clone`, no Poseidon hardwiring), and the Vec/SmallVec allocation strategy is untouched. The only differing flag is the terminal parent's.

**Generic-backend evidence is decisive.** `root_policy_tests::Recording` uses a deliberately non-`Copy` `type Hash = Vec<u8>` with length-prefixed, injective encodings, and `unreachable!()` on `hash_size`/`zero_hash`/`hash_from_bytes` (so the reducer is proven not to serialize). `empty_and_single_leaf_preserve_absolute_counter_and_root` pins counter 37 for len 0/1/4096 × root false/true; `finalization_and_odd_promotion_are_independent_of_group_position` pins full expected trees for 2/3/5 leaves at counter 37 with `parents == [false; n-1]` and last `== root`. This is exactly the guard that a non-root group cannot be ROOT-finalized.

**Producer/verifier/store/ticket agreement.** `tests/root_policy.rs` carries the required evidence: the independent 8192-byte literal `2abf65dd…0190` (and old CV `11890a7e…7389`) reaching `hash_block`, `outboard`, `PreOrderMemOutboard`, `PostOrderMemOutboard`, `encode`, `extract_slice_ranges`; acceptance through `decode`, `decode_slice`, `encode_ranges_validated` (`== wire[8..]`) and `valid_ranges` with **exact** cardinality 1 and `ChunkNum(0)..ChunkNum(2)`; old-CV rejection with the exact pinned variants per path and empty encoder output; mixed = `Size(8192), Leaf{0,D}, Done, Ok(())` vs `Size(8192), Error(LeafHashMismatch(ChunkNum(0)))` with no trailing `Done` (slice patterns, so extra items would fail); FSM leaf-then-`Done` vs `LeafHashMismatch`. `grouping_preserves_fixed_tree_at_boundaries` compares `outboard(..).root` with `fixed_chunk_root` across 14 lengths × logs 0/1/2/4 — a whole-tree equivalence check that also covers the required 16385/log4, 24576/log2 and 28672/log2 cases. `non_root_partial_and_odd_groups_verify_with_absolute_counters` supplies the unchanged right-only multi-group proofs (24576/log2 4+2, 28672/log2 4+3, 65537/log4 16+1 — the last pinning that a single-chunk non-root group is not root-finalized). `published_empty_and_hello_tree_vectors_are_unchanged` uses the published tree vectors and correctly avoids CDC `tree.4k_zeros`. Native side: `hash/root_policy.rs` pins the published anchors, `Hash::new == fixed_chunk_root == literal`, and that both canonical- and old-rooted `BlobTicket`s round-trip byte-identically without normalisation; `export_pairs.rs` exercises `ReadonlyMemStore` (→`CompleteStorage::create`), `MemStore::add_bytes` (→`import_bytes`/`finish_import`, `list().hashes()`), and two separate `FsStore`s at `is_inlined_all(8192)` true/false, each asserting the key equals `Hash::new`, `fixed_chunk_root` and the literal, plus exact `LE64(8192)||D` export and `get_bytes`. Test counts are internally consistent (cyber-bao 7 default / 8 all-features; iroh-blobs 5 `root_policy` = 2 + 3, 14 `export_pairs` = 11 + 3), and PR31/PR32 coverage is intact (`9ccd2809…`, `108022aa…`).

**Scope.** Only the named test was repurposed (`grouped_single_block_root_nonconformance_is_unchanged` → `…_has_canonical_root_finalization`, keeping both literals and `assert_ne!`). Docs now say "canonical fixed-chunk BAO tree root … This is the tree-hash domain; a plain Hemera particle is computed in its own domain" — correct, and it does **not** claim particle equivalence. No alias, fallback, migration, manifest/lock, geometry, `Hash::EMPTY` or MemStore-width change; integrity receipt shows exactly 11 changed files, unchanged Hemera and lock, `git diff --check` 0.

## Behavioural identity break

**Accept as scoped.** It is required by `specs/tree.md` §74–97 and restores the stated invariant that a file root is never an internal CV of a larger file sharing the prefix at the same counters. Conditions, all already recorded: source semver green certifies nothing about wire/key compatibility; no alias or dual acceptance; old keys/tickets/collection references must be reconstructed from complete bodies (partial records cannot); HashSeq bodies of `32·(1+children)` put 128..2047-child collections inside the affected interval; rollback does not migrate canonically-written records. No release/tag/promotion action is taken here.

## Non-blocking findings

1. `io/mod.rs::hash_block` doc line 1 still reads "Compute the root hash for a single block"; it now returns an internal CV unless `is_root`. The added sentences fix the substance — reword the first line.
2. Witness receipt `witness-root-policy-default` reports *1 passed / 5 failed / 0 filtered* = 6 tests, which cannot be reconciled with the 7 default tests in the final file; it ran the v2 overlay (`0381ed3a…`, not in this pack). On the parent I'd expect *2 passed / 5 failed* (`published_…anchors` and `non_root_partial_…` both pass, since multi-group and single-chunk roots are unaffected). Re-run the witness against v4 bytes, or annotate the v2 delta in the receipt, so the failure-first claim is traceable.
3. iroh-blobs was tested only under `--lib root_policy` / `--lib export_pairs`. The unfiltered `--lib` suite is documented as pre-existing hanging/red (2026-10-09 receipt: 27 observed failures, 3 pending, SIGTERM), so skipping is defensible — but the audit should name the residual risk: any affected-length derived hash literal in an unrun, byte-unchanged iroh-blobs test would now fail. From the supplied files I found none (`util.rs` 18433-byte test compares against `Hash::new`; `import.rs` compares outboard bytes only; `mem.rs::smoke` at 65536 is self-consistent; hash/ticket/protocol/collection tests use unaffected lengths or non-derived literals). `store/fs.rs`, `store/test.rs`, `fs/bao_file.rs` were not fully supplied and remain unverified by me.
4. Clippy aborts on pre-existing errors in both crates, so `--all-targets` linting of the two new test files is not certified. The gate index says this; keep it in the audit.
5. `export_pairs/support.rs::root_policy_fixture` hardcodes 4096 instead of `CHUNK_SIZE` and does not call `geometry()`; the `fixed_chunk_root` cross-check mitigates it.
6. `#[cfg(test)] mod root_policy;` sits mid-file in `hash.rs` between `impl Hash` and the trait impls. Cosmetic.

**Pre-existing, out of scope, keep recorded:** `mem.rs::import_bao` copying a 32-byte hash into `pair[..64]` and `print_outboard`'s 128/64 strides; `Hash::EMPTY` ≠ real empty root; duplicate `DecodeError` types; `decode_slice` `blocks<=1` ignoring `ranges`/declared size; `valid_ranges` granularity; `encode_ranges_validated` unwraps; dead `blocks == 0` branches; `radio-integration-tests` linking two Hemera versions; carried fmt/Clippy/`PublicKey`/dependency-policy reds.

## Limits

Source-only; I executed nothing and relied on the supplied receipts for exit codes and counts. Not a cryptographic audit of Hemera, not a bounded-verifier conformance claim, not a product/rung/release gate, and no closure of Kadek foundation row 4. Bounded `no_std` verification, the empty-root sentinel and an authenticated `(particle, root, length)` association remain open.