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

# PLAN v3 review β€” canonical BAO ROOT finalization (radio `258724bd`, Hemera `23f3bbcf`)

**Verdict: APPROVE**

The v2 blocker is closed with citations I independently traced to the supplied bytes; all three v2 optional notes were adopted and I verified each numerically. The design is correct against the pinned primaries, the affected interval and boundary matrix are exact, and every pinned error variant/event sequence matches source. Remaining findings are receipt/recording items and pre-existing defects β€” none makes the implementation wrong or hides the break. Source-only; I executed nothing.

## 1. B1 (memory-store producer closure) β€” resolved, verified

Traced end-to-end in the now-supplied `store/mem.rs`:

- `import_bytes` β†’ `PreOrderMemOutboard::create(&data, IROH_BLOCK_SIZE)`; `ImportEntry.outboard`.
- `Actor::finish_import` keys by `import_data.outboard.root()` (the `Outboard` impl returns the `root` field) β†’ `get_or_create_entry(hash)`, then `CompleteStorage::new(data, outboard.data)`.
- `import_byte_stream` and non-wasm `import_path` both terminate in `import_bytes(res.into(), scope, format, tx)`. The wasm `import_path` errors out. No independent derivation.
- `CompleteStorage::create(data)` β†’ `PreOrderMemOutboard::create` β†’ `outboard.root()`; `readonly_mem::ReadonlyMemStore::new` is its only non-test caller.
- `PreOrderMemOutboard::create` β†’ `io::outboard::outboard` β†’ private `outboard.rs::hash_block` (reduction #2). So both memory stores inherit `hash_group`.
- `import_bao` / `BaoFileStorage::Partial` retain the **caller-supplied** request hash (`ImportBaoRequest{hash,size}` β†’ `get_or_create_entry`) and complete via `CompleteStorage::new`, not `create` β€” no whole-body key derivation. Research's statement is accurate.

`PostOrderMemOutboard::create` also routes through the same `outboard()` and is cross-checked by the existing `post_order_outboard_all_nodes_match_pre_order` root assertion; it carries no reduction of its own, so its omission from step 2 is harmless.

The planned tests are decisive at an affected length: `D`=8192 at log4 β†’ `chunks=2, cpb=16, blocks=1`, so `ReadonlyMemStore::new([D])` exercises `CompleteStorage::create` and `MemStore::add_bytes(D)` exercises `import_bytes`/`finish_import`. `list().hashes()` is race-free: `finish_import` inserts into `state.data` *before* sending `Done(tt)`, and `with_tag`'s `CreateTag` is a later round-trip to the same actor. `export_bao(key, ranges(0,2)).bao_to_vec()` is exactly `LE64(8192)||D` (8200 B): `pre_order_chunks_filtered` with `blocks==1` emits one `Leaf{0, min(8192,65536)=8192, is_root:true}` after `leaf_range 0..16` overlaps `0..2`; `into_byte_stream` maps `Size`β†’8 LE bytes, `Leaf`β†’data, `Done`β†’end. `get_bytes` returns `D`. `mem.rs::smoke` (65536 B β€” itself inside the affected interval) survives: every value is derived from the new key, and `blocks==1` means `import_bao` sees no `Parent`.

## 2. N1–N3 β€” adopted, verified

- **N1.** Matrix now includes **16385/log4** = 5 chunks, `blocks=ceil(5/16)=1`, final leaf 1 byte (five leaves + partial-last in one group βœ“) and **28672/log2** = 7 chunks, `blocks=2` as 4+3 (odd promotion inside a **non-root** group βœ“). 24576/log2 = 6 chunks β†’ 4+2, both non-root βœ“. 65537/log4 = 17 chunks β†’ 16+1 covers the one-chunk non-root block.
- **N2.** `meta.rs` excerpt supplied: `raw_outboard_size(size) = BaoTree::new(size, IROH_BLOCK_SIZE).outboard_size()`, and `tree.rs::outboard_size` returns 0 for `blocks<=1`. So `raw_outboard_size(8192)==0` and `is_inlined_all(8192)` reduces to `8192 <= max_data_inlined`: **true** at 32768 (tiny β†’ `Hash::new(&cmd.data)`), **false** at 4096 β†’ `import_byte_stream_mid`, where `get_import_source` sees `is_inlined_data(8192)==false`, so `size > max_data_inlined` writes the temp file and yields `ImportSource::TempFile` β†’ `compute_outboard` β†’ `init_outboard`. The two routes are genuinely separated, exactly as claimed.
- **N3.** `validate_ranges_impl`'s `blocks==1` branch yields `Ok(ChunkNum(0)..tree.chunks())` = `0..ChunkNum(2)` for 8192 B on match, and on mismatch yields nothing and returns `Ok(())`. Plan's pin is literally right, including the noted granularity divergence from the leaf branch (which yields `0..16` at log4) being out of scope.

## 3. Independent canon and boundary re-derivation

`specs/tree.md` Β§Root Finalization and `rs/src/tree.rs::{fixed_chunk_root, merge_range}` agree: single chunk β†’ `hash_leaf(d,0,true)` (CHUNK|ROOT); multi-chunk β†’ `hash_node(l,r,true)` (PARENT|ROOT); interiors non-ROOT; leaf counter is the **absolute** chunk index; `split = largest power of two < count`. CDC `root_hash` is correctly excluded, with `tree.4k_zeros` forbidden for the right reason (`root_hash_uniform_4096_splits_at_2048`).

All five reductions are behaviourally identical today, including `sync::hash_subtree` (its `len<=CHUNK_LEN` shortcut equals the others' empty/single-chunk returns because `is_single_chunk = len<=CHUNK_SIZE && is_root`); consolidation is exactly behaviour-preserving modulo the one flag. `is_root && level.len()==2` identifies the final iteration precisely, since `next.len()=ceil(len/2)==1 ⟺ len==2` under `while len>1`; I hand-checked bottom-up-with-promotion against `merge_range` for n=3,5,7 and group composition for 4+2/4+3/16+1. For `blocks>1` the root node `npot(blocks)-1` always has an existing right child, so the ROOT `Parent` is always emitted and never inherited down to a leaf β€” no internal group can be ROOT-finalized, and multi-group roots are unchanged. Affected iff `4096 < N <= 4096Β·2^g`: log4 4097..65536, log2 4097..16384 βœ“. Published anchors hold pre- and post-fix (`Hash::new(b"")=ea57b2e6…`, `Hash::new(b"hello")=626fa46e…` via `tree_hash` β†’ `blocks<=1` β†’ `chunk_cv(Β·,0,true)`).

Error/event pins all check out: combined `decode::DecodeError::LeafMismatch{start_chunk:0}`; `SliceDecodeError::LeafMismatch{start_chunk:0}` (the `blocks<=1` branch); `sync::EncodeError::LeafHashMismatch(ChunkNum(0))`; FSM `io::error::DecodeError::LeafHashMismatch(ChunkNum(0))`; mixed = `Size(8192)`, `Error(LeafHashMismatch(ChunkNum(0)))`, `Ok(())` with no `Parent`/`Leaf`/`Done` (the `Leaf` arm returns before any `send`), canonical = `Size(8192)`, `Leaf{offset:0,data:D}`, `Done`, `Ok(())`. `BlobTicket::{to,from}_bytes` pass the hash through unchanged βœ“. Privacy is fine: a private `hash_group` in `io/mod.rs` is visible to `io::{outboard,decode,slice,sync}` as descendants; keeping `let _ = block_bytes;` in the public wrapper avoids an `unused_variables` error under `-Dwarnings` while preserving the documented parameter name. A recording backend is implementable within `HashBackend`'s bounds (`AsRef<[u8]> + Clone + Eq + Debug`); `hash_group` touches only `chunk_hash`/`parent_hash`. PR32's 131072 B fixture (`9ccd2809…`/`108022aa…`) is unaffected and its `half_root`+single-ROOT shape is already post-fix.

## 4. Decision on the deliberate behavioural identity change

**Accept, as scoped.** It is mandated by pinned primary authority and restores the property `specs/tree.md` states explicitly: a file's identity must never equal an internal chaining value. Today an 8192-byte log4 root *is* `parent_cv(chunk_cv(d0,0,false), chunk_cv(d1,1,false), false)` β€” an internal CV of any larger file sharing that prefix at the same absolute counters. Conditions, all already in the plan and required in the receipt: source-level `semver-checks` green certifies **nothing** about content identity and must be stated as such; no alias/dual-acceptance (it would re-admit the collision this removes); old references become unresolvable rather than silently wrong (`readonly_mem::export_bao` builds its outboard from the *requested* hash, so a corrected provider returns `LeafHashMismatch` instead of serving an old-keyed body; old tickets still parse but name an unservable root); recovery needs the complete body plus rebuilt references, as partial affected-length records have an empty outboard and no retained root provenance; no deployment inventory exists; rollback is asymmetric. Version/tag/release consequences belong to the release train as a recorded compatibility note, not to this patch.

## 5. Optional notes (non-blocking)

1. **Persist the scope evidence out of `/tmp`.** The scan script/index (`f33b503a…`) and the v1/v2 packs exist only under `/tmp`, which is volatile; per `cyber/CLAUDE.md` ("memory is cache, repos are truth"), the artifact backing the "no additional production reduction" boundary should land in the task dir or the new `audit/` receipt. Add it to step 6's freeze list.
2. **State the policy for additional test updates.** Step 5 says "replace only the named grouped-defect-preservation test," but unsupplied modules may pin affected-length literals β€” `store/mod.rs` declares `mod test;` (no `cfg(test)`), and `store/fs.rs::tests::test_data` is referenced by `util.rs`. Cross-checks survive (all derivations move together); only hardcoded literals break. One sentence distinguishing "mechanical literal update, recorded" from "new defect, return to planning" prevents either silent edits or a stall.
3. **Name the HashSeq/collection cascade.** `collection.rs::to_blobs` builds `links_bytes = 32Β·(1+children)`, so collections with **128..2047 children** change their HashSeq root even when every child is small; `meta_bytes` above 4096 B shifts the meta hash too. Covered by "references must be rebuilt," but it is a crisp, checkable consequence worth one line in the receipt.
4. **Argument order when consolidating.** `sync::hash_subtree(backend, start_chunk, data, is_root)` reverses `data`/`start_chunk` relative to `hash_block`; its two call sites (`validate_ranges_impl` `blocks==1`, `validate_node_rec` leaf) must be swapped carefully.
5. **Write new files in the `cargo make format` style** (`imports_granularity=Crate`, `group_imports=StdExternalCrate`) so they don't add to the recorded `gate-format-expanded` drift; `slice_traversal.rs` and `export_pairs.rs` already follow it.
6. `store/fs.rs` (SHA-pinned, excluded) and `fs/bao_file.rs` remain line-level-only evidence. Properly bounded β€” worth repeating that boundary in the receipt rather than letting "no additional reduction" read as repository-wide.

## 6. Existing defects to record (not caused by, not in scope of, this patch)

1. **`mem.rs::import_bao` panics on any `Parent`**: `pair[..64].copy_from_slice(parent.pair.0.as_bytes())` copies 32 bytes into a 64-byte slice. Latent only because every current test uses single-group data (`smoke` = 65536 B = 1 block). Same old-width class as PR32; `print_outboard` (128/64 strides) shares it. Now citable from the pack.
2. `Hash::EMPTY = [0u8;32]` is not the real empty root (`ea57b2e6…`): `observe_with_opts` short-circuits on it, `import_bao_reader` accepts a zero-size body only for it, and `MemStore` keeps a phantom `empty_hash` handle while a genuine `add_bytes(b"")` indexes under the canonical root. `hash.rs::test_empty_hash` pins only determinism.
3. Two distinct public `DecodeError` types (`io::error` vs `io::decode`) with the same name; the plan distinguishes them per path correctly.
4. `slice::decode_slice`'s `blocks<=1` branch ignores `ranges` and `declared_size`, folding trailing bytes into the leaf; `decode::decode` checks size only after the walk.
5. `sync::valid_ranges` granularity inconsistency (per-file vs per-block) inside a touched function.
6. `sync::encode_ranges_validated` panics (`load(*node)?.unwrap()`, `stack.pop().unwrap()`) where `mixed.rs` returns `EncodeError` for the same conditions.
7. `BaoTree::blocks()` is `.max(1)`, so the `blocks == 0` branches in `tree.rs` and `sync::validate_ranges_impl` are dead.
8. `radio-integration-tests` links registry `cyber-hemera 0.1.2` alongside path `0.3.1` via `cyber-bao` β€” two `Hash` types in one crate; the planned `-p radio-integration-tests` gate inherits it.
9. Carried baseline reds from both 2026-10-09 receipts: cyber-bao Clippy (first `io/mixed.rs:115 clone_on_copy`), workspace `Endpoint`/`PublicKey` local-vs-registry mismatches and `iroh/bench` imports, format drift, `v0.1.0` lock unresolvable under `--locked`, unavailable nextest/deny/`nightly-2025-10-09`. Repeat on the exact parent for parity; never suppress.

## 7. Limits

Source-only: no build, probe, gate, metadata query or measurement; the 8192-byte literals are relied on from the archived receipts, not re-measured. Not an executed ring-1 gate, not rung/product acceptance, not readiness, not a cryptographic audit of Hemera. The two 2026-10-09 reviews approved traversal and parent-width scopes only; the appended v2 REQUEST CHANGES is historical evidence, not approval. This patch is a prerequisite for β€” and evidences nothing about β€” bounded/`no_std` verification, lazy checked geometry, resumable bounded hashing, pooled admission, or any sealed `(particle, root, length)` association; it does not close Kadek foundation row 4, and it establishes no equivalence between a canonical BAO root and a plain Hemera particle. Keeping that claim out of `Hash::new`'s docs is correct and must be preserved. Bounded proof, the empty-root sentinel and particle/root association remain open.

Homonyms

cyb/honeycrisp/audit/2026-10-09-nebu-package-identity/plan-review-approved.txt
cyb/honeycrisp/audit/2026-10-10-unimem-block-access/plan-review-approved.txt

Graph