Fix divereged state root derived from VerifiedMultiProof

[citizen-stig]

Essence of new test:

/// some key/values written before.
let (prover_root, witness) = t.commit();
let nomt::Witness {
    path_proofs,
    operations: nomt::WitnessedOperations { .. },
} = witness;
let mut inner = path_proofs.into_iter().map(|p| p.inner).collect::<Vec<_>>();
inner.sort_by(|a, b| a.terminal.path().cmp(b.terminal.path()));
let multi_proof = MultiProof::from_path_proofs(inner);

/// verify
let verified = verify_multi_proof::<Blake3Hasher>(&multi_proof, prev_root)
    .expect("multiproof must verify against the prior root");

let mut updates: Vec<(KeyPath, Option<ValueHash>)> = accesses
    .iter()
    .filter_map(|(k, op)| match op {
        KeyReadWrite::Write(v) | KeyReadWrite::ReadThenWrite(_, v) => {
            Some((*k, v.as_deref().map(Blake3Hasher::hash_value)))
        }
        KeyReadWrite::Read(_) => None,
    })
    .collect();
updates.sort_by(|a, b| a.0.cmp(&b.0));

let verifier_root = verify_multi_proof_update::<Blake3Hasher>(&verified, updates).unwrap();
assert_eq!(prover_root, verifier_root);

Symptom

verify_multi_proof_update disagreed with the prover's post-update root (session.finish().root()) on multi-proofs where any terminal carries ≥ 2 unique siblings on multi_proof.siblings. Old-root verification (verify_multi_proof) was unaffected — the divergence was entirely in the update step.

Reproduced by four new tests in nomt/tests/compute_root.rs (many_keys_round_trip, many_keys_inserts_only, many_keys_overwrites_only, many_keys_deletes_only), all of which use PRNG-distributed keys over 32 prior leaves. Smaller, structured-key tests passed because they produced terminal_n ≤ 1 for every terminal, which made the bug invisible.

Root cause

nomt/core/src/proof/multi_proof.rs

Lines 640 to 645 in 5fa289a

	self.extend(
	next_terminal.depth - terminal_n + 1,
	next_terminal.unique_siblings.end,
	&proof.siblings,
	true,
	);

(before the fix) passed reverse = true to CommonSiblings::extend when loading a terminal's unique-sibling tail onto the depth-labeled stack:

self.extend(
  next_terminal.depth - terminal_n + 1,
  next_terminal.unique_siblings.end,
  &proof.siblings,
  true,                              // <-- bug
);

The reverse = true branch reversed the slice iteration but still labeled entries with ascending depths via start_depth + i:

  for (i, sibling) in siblings[self.taken_siblings..end].iter().rev().enumerate() {
      self.stack.push((start_depth + i, *sibling))
  }

So for a terminal at depth 8 with terminal_n = 7, the stack ended up as

(2, sibling_at_depth_8), (3, sibling_at_depth_7), …, (8, sibling_at_depth_2)

hash_and_compact_terminal then pops from the top by descending cur_layer (8, 7, …, 2).
Because the labels were anti-correlated with the values, the first hash above the updated sub-trie combined the new leaf with the wrong sibling, and the error propagated to the root.

Why the storage convention forces reverse = false

multi_proof.siblings is stored ascending by depth. Five corroborating sites:

• https://github.com/thrumdev/nomt/blob/5fa289a940bdad55d537b63a6b52b8e75b442004/nomt/src/merkle/seek.rs#L138-#L144 — sibling pushed after position.down(bit), so index 0 is depth 1.

• nomt/nomt/src/merkle/worker.rs

  Line 425 in 5fa289a

  siblings[*depth - 1] = *actual_sibling;

  — siblings[depth - 1] = *actual_sibling.
• https://github.com/thrumdev/nomt/blob/5fa289a940bdad55d537b63a6b52b8e75b442004/core/src/proof/multi_proof.rs#L96-#L101 — prove_unique_path_remainder copies a suffix unchanged.
• https://github.com/thrumdev/nomt/blob/5fa289a940bdad55d537b63a6b52b8e75b442004/core/src/proof/multi_proof.rs#L484-#L488 — verify_range consumes via siblings[..unique_len].iter().rev() (only consistent with ascending storage).

• nomt/core/src/proof/path_proof.rs

  Line 87 in 5fa289a

  let new_root = hash_path::<H>(cur_node, relevant_path, self.siblings.iter().rev().cloned());

  — PathProof::verify consumes via self.siblings.iter().rev().

For hash_and_compact_terminal to pop deepest-first, the stack must be loaded shallowest-first. With ascending storage, that means iterating the slice forward — i.e., reverse = false. The bisection branch at the same call site (line 634) already passed false correctly; only the terminal branch was wrong.

(Aside: the doc comment on PathProof.siblings at core/src/proof/path_proof.rs:62 previously said "descending order by depth" — it contradicted the implementation and the adjacent hash_path comment at line 107. Corrected as part of this PR.)

Why existing tests missed it

Every existing verify_update unit test had terminal_n ≤ 1 for every terminal, where iter().rev() is a no-op for ordering. Traced:

• multi_proof_verify_2_leaves_with_provided_updates: terminal_n = 1 (k0) and 0 (k1).
• multi_proof_verify_4_leaves_with_long_bisections: terminal_n = 0 for all four terminals.
• test_verify_multiproof_siblings_structure: terminal_n = 0 for all four (all sibling info absorbed by bisections).
• test_verify_update_underflow_prefix_paths: negative-path test, never reaches extend.

nomt/tests/witness_check.rs::produced_witness_validity does exercise prover↔verifier round-trip on many keys, but through proof::verify_update (the per-path verifier), which never touches CommonSiblings. The multi-proof update path was effectively untested for the positive case with non-trivial unique-sibling tails.

Fix

Three changes, no API surface change:

1. core/src/proof/multi_proof.rs — removed the reverse: bool parameter from CommonSiblings::extend and the buggy reverse-iteration branch. Both call sites in CommonSiblings::advance (bisection and terminal) now use a single ascending-iteration body. This makes the depth-mapping self-evident: siblings[i] is at depth start_depth + i, matching the storage convention.
2. core/src/proof/path_proof.rs:62 — corrected the stale doc comment from "descending order by depth" to "ascending order by depth".
3. core/src/proof/multi_proof.rs (test module) — added verify_update_terminal_with_multi_unique_siblings, a unit test that constructs a 3-leaf trie producing terminal_n = 7 with a real (non-TERMINATOR) sibling at the deepest position.
   Confirmed it fails under the old reversal and passes after the fix, closing the unit-level coverage gap.

CommonSiblings is module-private and grep confirms no external callers, so removing the parameter is safe.