perf(room-state): carry a signature digest in the member_info summary, not the signature

[sanity]

Problem

The room summary is re-sent to every interested peer on every state change, and on the live Freenet network interest_sync_summaries is 49.8% of all outbound bytes — the single largest consumer, ahead of contract updates themselves. The measured mean summary message is 29.1 KB, against a chat message capped at 1,000 bytes.

MemberInfoV1::Summary carried a raw ed25519 Signature per member. Measured: 134.08 CBOR bytes per entry, of which ~124 is the signature — about 92%. At the Official room's ~470 records that term alone is ~63 KB.

(Those two figures come from different measurements and must not be multiplied together. The 29.1 KB is a fleet-wide mean across all rooms, most of which are far smaller than the Official room, which is why a single large room's member_info term exceeds it. There is no per-room summary measurement on record, so this PR makes no claim about what any one room's total summary weighed.)

Correction: the earlier "66 bytes per signature" figure was wrong

Issue #571, this PR's original body, and the first review round all used 66 bytes for the signature and ~78 bytes/entry. That is the CBOR byte string encoding — what River's own SignatureBytes newtype produces via serialize_bytes. It is not what ed25519::Signature does: its Serialize calls serialize_tuple(64), ciborium maps a tuple to a CBOR array, and a uniformly random byte costs 2 bytes there whenever it is >= 24. The real figure is ~124.

This is also the whole explanation for the impossible arithmetic flagged in review (470 x 66 = 31 KB of signatures exceeding a stated 29 KB total): 66 was simply the wrong number.

Demonstrated rather than argued — swapping the test's baseline to SignatureBytes reproduces the old claim exactly:

before (u32, SignatureBytes): 36193 B = 77.01 B/entry   <- the "~78" that was quoted
before (u32, Signature):      63019 B = 134.08 B/entry  <- what the summary actually used

So the win is ~1.7x larger than this PR originally claimed, not smaller. The deferred DirectMessagesSummary follow-up genuinely is ~66 bytes/entry, because it does use SignatureBytes — two different encodings of the same 64 bytes, and they must not be reasoned about with one number.

The signature is never verified in the summary path. It exists only as the equal-version tiebreak discriminator (#411 round 4 B) and is compared for ordering and equality. A digest does that identically.

Approach

type Summary = BTreeMap<MemberId, (u32, SigDigest)> — exactly what member_info_rank returns. The summary value now is the rank, so delta() compares against it directly rather than re-deriving a rank from a stored signature.

SigDigest is a 128-bit BLAKE3 digest of the signature, serialized as a CBOR byte string. Measured on 470 entries with realistic MemberIds, through the real summarize(): 134.08 → 28.01 CBOR bytes per entry, a 4.8x reduction, so the Official room's member_info term goes ~63 KB → ~13 KB.

Both figures are measured in the test, not derived: member_info_summary_stays_small_per_entry rebuilds the pre-change shape from the same records and prints both. Given that a wrong per-entry figure survived an issue, a PR body, and a review round, a size claim that is the entire justification for a change should not rest on arithmetic.

Why 128 bits and not 64. A collision here is not cosmetic and does not self-correct. Two same-version records whose discriminators tie are indistinguishable to anti-entropy: summarize advertises an identical pair on both peers, delta filters on strict > so neither peer offers its record to the other, apply_delta replaces only on strict > so each keeps whatever arrived first, and full-state merge routes through summarize → delta → apply_delta so it does not rescue either. The two halves of the network then disagree permanently and silently on that member's deputies, i.e. on ban authority — precisely the bug #411 round 4 B added the discriminator to fix, and something the old raw-signature compare made impossible rather than merely unlikely.

A member self-signs their own record and has unlimited grinding entropy for it (preferred_nickname is free-form, deputies entries are never validated for membership), so the attacker controls both sides of the comparison. At 64 bits that is a ~2^32 birthday search — hours on commodity hardware. 128 bits puts it at ~2^64. This follows the precedent already in this repo: direct_messages::PurgeToken derives a 16-byte BLAKE3 value from a signature under a strictly weaker threat model and still chose 128 bits.

Why BLAKE3 and not freenet_scaffold::util::fast_hash. fast_hash is a base-31 polynomial — fine for the accidental collisions MessageId/BanId care about, trivially collidable by construction, which would price the attack above at roughly nothing regardless of width.

Why a hand-written Serialize. This is the same trap as the 66-vs-124 one above, in the other direction. A [u8; 16] through the serde derive emits a 16-element CBOR array (32 bytes), not a byte string (17) — measured, that puts the entry at 42.53 B/entry, over the 32-byte bound. The hand-written Serialize calling serialize_bytes is what makes 28.01 achievable, and it is the reason a bare [u8; 16] was not used.

Measured alternatives, all at 470 entries with realistic MemberIds:

shape	B/entry
(u32, Signature) — before	134.08
(u32, u64) — 64-bit digest	20.01
(u32, SigDigest) — shipped	28.01
(u32, u64, u64) — two halves	29.01
(u32, [u8; 16]) via serde derive	42.53

Consistent tie direction. canonical() used max_by_key, which returns the last maximum, while dedup_to_canonical() and apply_delta keep the first. Widening the digest makes a tie between distinct records infeasible to mint, but leaving two selectors of "the canonical record" disagreeing is a latent trap: a state holding duplicates would answer deputies_of one way on a freshly-GET'd full state and the other way after the next apply_delta ran dedup. All three now keep the first.

Cost traded the other way: contract CPU

This buys bytes with WASM CPU, which since freenet-core 0.2.105 is itself an eviction axis, so it is worth stating rather than leaving implicit.

Before this change member_info_rank returned (version, signature.to_bytes()) — a 64-byte copy, no hashing. Now every rank costs one BLAKE3 hash of 64 bytes:

• dedup_to_canonical runs on every apply_delta (and again in post_apply_cleanup) and computes one digest per stored member_info record — ~470 hashes per apply in the Official room, where before it copied 470 signatures.
• is_ban_authorized calls deputies_of two-plus-ancestor-depth times per ban validated; each goes through canonical, which scans the record vector as before but now hashes the matching record — normally exactly one.

Measured natively, 470 records, best-of-5 over 200 reps:

	optimized	unoptimized
summarize() before (64-byte copy)	60.01 us	314.46 us
summarize() after (blake3)	83.21 us	442.75 us
delta	+23.20 us (1.4x)	+128.29 us (1.4x)
470 x signature.to_bytes()	1.49 us	29.50 us
470 x blake3(64 bytes)	64.39 us	160.91 us
per-signature blake3	0.137 us	0.342 us

What is NOT measured, stated plainly: the cost inside the contract WASM under Cranelift OptLevel::None. These are native timings, and native profiling of this codebase's contracts is known to mislead badly (roughly 30x, which is why .claude/rules says to profile contracts under wasmtime -O opt-level=0 rather than natively). So no absolute per-UPDATE figure is claimed here. The durable number is the 1.4x on summarize(), which holds across both native profiles and is a property of the work being done rather than of the compiler.

Two mitigations are in this PR: canonical computes each candidate's rank once rather than once per comparison, and dedup_to_canonical stores (rank, info) in its map instead of recomputing the incumbent's digest on every collision. A further HashSet pre-pass was raised in review and deliberately not taken — out of scope for this PR.

Behaviour change to be aware of

The equal-version tiebreak now orders by digest rather than raw signature bytes, so a different record can win a tie. Safe because every peer applies the same deterministic rule, and this re-keys the contract regardless.

This re-keys the room contract, so it needs the standard River migration. Left as draft until that is sequenced. The bundle, as one commit per repo precedent:

• rebuild room_contract.wasm (both committed copies)
• register the outgoing hash in common/legacy_room_contracts.toml
• bump the len() == 30 count and the fingerprint pin in common/src/migration.rs
• bump the riverctl version
• bump the moderation key pin per the two-signal-staleness procedure
• before migrating, verify the Official room's state carries no equal-version member_info duplicate. A surviving duplicate would flip ban authority at re-PUT, because the winner is chosen by a different rule after this change. post_apply_cleanup dedups, so this is unlikely — a one-line riverctl check converts unlikely into verified, and it is cheap next to the cost of getting it wrong on the live room.

Three of the four green CI ticks are blind to this diff. Flagging so the green row is not read as clearance:

• check-room-contract-migration compares the committed ui/public/contracts/room_contract.wasm, which this PR deliberately does not rebuild, so it reports "no migration entry needed" for a change that alters contract behaviour.
• check-cli-wasm.yml's sync step only cmps the two committed WASM copies (ui/public/contracts/ vs cli/contracts/) against each other, never against source — so it passes while both are stale, even though its own trigger list claims common/src/** coverage.
• The same workflow's riverctl-bump guard greps the diff for contracts/room-contract/src/, contracts/room-contract/Cargo.toml, or a committed .wasm. common/src/ is not in that list, so it does not fire on this diff at all.

The concrete drift while the rebuild is deferred: the committed WASM runs the old tiebreak, river-core source runs the new one. The eventual WASM-rebuild commit will be caught correctly by the first check, since it compares against main's old hash — but CI will not tell you the bundle is missing before then.

Scope note on the other sub-summaries

There are nine type Summary declarations under common/src/room_state/. Three are fixed-size scalars (configuration.rs, version.rs, upgrade.rs) and do not scale with content. Of the six that do, four already carry compact hashes or small tuples (member.rs, ban.rs, message.rs, secret.rs). Two carried raw 64-byte signatures: this one, and DirectMessagesSummary.message_signatures: BTreeSet<SignatureBytes> (66 bytes per DM held), which the same fix applies to. That one is deferred so this change can be reviewed on its own, and because member_info is the measured dominant term.

Testing

• sig_digest_golden_vector (new) — ONE fixed signature, ONE fixed expected digest, ONE fixed expected CBOR encoding. This pins the four things every peer must agree on: that the hash is BLAKE3 over signature.to_bytes(), that the digest is the first 16 bytes, that they are kept in natural order, and that the value encodes as a 17-byte byte string. Expected bytes were produced with the b3sum CLI outside the crate, not by running the code under test.

  This exists because every other check on the digest compares randomly-keyed signatures, which detects a byte-order change only intermittently. Measured: with the digest bytes reversed, the existing oracles passed 11 of 30 runs — a 37% miss rate. The golden vector fails 100% of the time. (Verified by toggling: reversed order, last-16-instead-of-first-16, and derived-Serialize each fail it.)

• canonical_and_dedup_break_rank_ties_identically (new) — builds two records that share a signature but differ in deputies via with_signature, which is what a digest collision would look like to these two pure orderings, and asserts both keep the same one. Verified to fail when canonical is reverted to max_by_key. Records built the normal way cannot test this: ed25519 signing is deterministic over the whole MemberInfo, so two records that tie on rank are byte-identical and the assertion would hold either way.

• member_info_summary_stays_small_per_entry — rewritten to be realistic, and to measure its own baseline. It now calls the real summarize() on built state instead of hand-constructing the summary type; derives MemberIds from real VerifyingKeys (deterministically seeded, so the measurement is reproducible and cannot flake); and rebuilds the pre-change (u32, Signature) shape from the same records so the before/after figures are both measured in one place. The old MemberId(FastHash(i)) for small i encodes in 1-3 bytes against a real key's ~9 and understated the entry by ~30%.

  It carries a second assertion pinning the baseline above 100 B/entry, whose whole job is to catch the encoding confusion above: verified to fail (at 77.01 B/entry) when the baseline is measured against SignatureBytes instead of Signature. Verified to fail on the forward direction too, when SigDigest uses the derived Serialize (42.53 B/entry against the 32 B bound).

• deputy_ban_test's winner oracles and room_data's key-search fixture recompute the digest from BLAKE3 independently rather than calling into river-core, so a change to the digest function fails the assertion instead of silently agreeing.

• apply_deputy_change_uses_canonical_base_and_version's fixture had to flip its push order. It relied on canonical's old last-wins tie-break to make a version-only selector return the wrong record; with canonical now keeping the first, clean-first would have let that regression pass by accident. Loser-first restores the detection and additionally catches the first-match .find() regression the test's own header claims to target. Verified: regressing canonical to .find() fails it.

• outranks_keeps_the_incumbent_on_a_tie (new) — the tie direction was asserted in prose at five separate > call sites and pinned by nothing: relaxing any one to >= left all 409 tests green, because reaching the tie branch behaviourally needs a real digest collision (verify checks every stored record's signature). All five now route through a single outranks() with one test. Verified: > → >= in outranks now fails 4 tests including this one.

• canonical_selects_by_digest_not_just_version (new) — canonical's digest arm had no river-core coverage; a version-only-keep-first mutation passed all 409 and was caught only by a river-ui test. Deterministic by construction: two with_signature records at the same version with fixed signatures [1; 64] and [2; 64], whose digests (29c04cc4… and fe969aba…, from b3sum) are known to differ, loser placed first. Verified: the version-only mutation now fails exactly this test (209 passed, 1 failed).

• .claude/rules/contract-summary-determinism.md gains a "summary values are a wire-format commitment too" section covering hash choice, width against the threat model, truncation/order, and encoding — plus the golden-vector and realistic-size-guard requirements.

Results

suite	result
cargo test -p river-core --tests	411 passed, 0 failed
cargo test -p river-core --lib	210 passed, 0 failed
cargo test -p river-core --lib --features ecies-randomized,migration,mentions	252 passed, 0 failed
cargo test -p riverctl --lib	314 passed, 0 failed
cargo test -p river-ui --bins	814 passed, 0 failed
cargo test -p river-ui --bins --features example-data,no-sync	819 passed, 0 failed

cargo fmt --check clean.

Flake check. The previous head carried a ~45% flake in equal_version_member_info_diff_detected_by_anti_entropy, so a single green run proves nothing here. The four affected groups — deputy_ban_test equal_version_member_info, river-core --lib member_info, summary_determinism_test, and river-ui apply_deputy_change — were each run 40 consecutive times: 1,440 test executions, 0 failures, no FAILED lines, every exit code 0.

Clippy. cargo clippy --workspace --all-targets -- -D warnings does not pass on this repo and did not before this change — the clippy workflow is checked in as clippy.yml.disabled. It stops at 7 errors in river-core (ban.rs ×5, dm_body.rs, message.rs — none touched by this PR); allowing those three lints (doc_lazy_continuation, assertions_on_constants, repeat_once) lets it run the whole workspace, where it finds 54 more across 17 files in ui/ and cli/. All 61 sites were cross-referenced against the lines this diff touches: none land on a changed line, and nothing new comes from this change. Left for a separate PR rather than fixed here, since unrelated lint cleanup riding along in a perf(...) PR is the scope creep AGENTS.md warns about.

Closes #571

[AI-assisted - Claude]

[sanity]

Review: #572 (head b1db4ed1)

Verdict: Needs Changes. One blocking finding. The change is well-built otherwise: consumer coverage is complete, the summary/delta equivalence holds, determinism is preserved, and the size claim is real and independently re-derived (78 → 21 bytes/entry, ~37 KB → ~10 KB at 470 records).

Lenses run: skeptical (adversarial bug-hunt) and code-first (read-code-before-description), independently, blind to each other. Coverage gap, stated honestly: the testing lens was stopped part-way to relieve build contention and the wire-compat/CRDT-semantics lens did not return, so this is 2 of 4. Both surviving lenses reached the blocking finding independently and cited the same precedent, which is why it is reported with confidence; the coverage gap means test-strength findings below may be incomplete.

---

BLOCKING — the 64-bit digest is inside attacker reach, and the repo already decided this question the other way

common/src/room_state/member_info.rs:144 truncates blake3 to 8 bytes. The doc comment at :136-137 states the threat model correctly and then draws the wrong conclusion:

A member self-signs their own record, so they could otherwise mint colliding pairs at will. blake3 removes that.

At 64 bits it does not remove it. It prices it at a ~2^32 birthday search, which is hours on commodity hardware. The attacker has unlimited grinding entropy: preferred_nickname is free-form and deputies entries are never validated for membership.

Why a collision is unrecoverable rather than merely annoying. Both lenses traced this independently and agree:

• apply_delta (member_info.rs:356-363) replaces only on strict >, so each peer keeps whichever record arrived first.
• summarize (:240-253) emits an identical (V, d) on both peers, so anti-entropy sees agreement.
• delta (:273-280) filters on strict >, so neither peer ever offers its record to the other.
• Full-state merge does not rescue it: freenet-scaffold implements merge as summarize → delta → apply_delta, and contracts/room-contract/src/lib.rs:139-145 routes UpdateData::State through exactly that path.

common/src/room_state/member.rs:378-393 resolves ban authority through deputies_of → canonical, so the two halves of the network permanently disagree on who may ban whom, silently. That is #411 round 4 B reinstated by construction, i.e. the precise bug this discriminator was added to fix. Under the old raw-signature compare it was impossible (different records ⟹ different signatures ⟹ different ranks). This converts an impossibility into a priced attack.

The precedent, which is the strongest argument here. common/src/room_state/direct_messages.rs:162-167 faced the same choice and picked 16 bytes:

128 bits gives a ~2^64 birthday bound - adequate against worst-case attacker-chosen signature grinding (an attacker who can sign as themselves cannot influence which token any other member's purge list contains, and the recipient is the sole signer of their own purge list).

That reasoning describes a weaker threat model than this one, and chose twice the width. Here the attacker controls both sides of the comparison.

Fix: widen to [u8; 16]. Cost is ~9 CBOR bytes per entry (21 → ~30), still ~2.6x better than the 78 being replaced, and still inside the new test's 32-byte bound. Worth taking now specifically because this PR re-keys the contract: the width cannot be revised cheaply afterward.

Should fix

Tie-direction disagreement between two selectors. canonical() uses max_by_key (:39-44), which returns the last maximum; dedup_to_canonical() (:72-87) and apply_delta keep the first. A peer that GETs a full state containing both records answers deputies_of one way, then flips after the next unrelated apply_delta runs dedup. Widening makes this unreachable, but two selectors disagreeing on ties is a latent trap worth closing on its own.

The byte-order pin is only ~50% effective. :139-143 calls from_le_bytes load-bearing, but the oracles at common/tests/deputy_ban_test.rs:1579 and :1661 compare randomly-keyed signatures, so a switch to from_be_bytes would agree about half the time and fail intermittently. By this project's own standard that is a broken detector. A golden vector — one fixed 64-byte signature to one fixed expected digest — makes it deterministic. No golden vector exists anywhere today.

The size guard measures unrealistically cheap keys. common/tests/summary_determinism_test.rs:344-360 builds MemberId(FastHash(i)) for i in 0..470, which CBOR-encodes in 1-3 bytes against a real MemberId's ~9, so it measures ~13-16 bytes/entry where production is ~21. The assertion still holds, but the reported figure understates production by ~30%. It also never calls summarize() — it hand-builds the type, so it cannot catch a regression in how summarize populates it, only a type-shape change that would be a compile error anyway.

Doc claims that no longer survive their own arithmetic. :122-125 says the signature was "~84% of the summary's bytes and made the whole room summary ~29 KB", but 470 × 66 = 31 KB of signature bytes alone, exceeding the stated total. Reconcile against the actual measured record count (records exist only for members who set a nickname, which is likely the explanation). Also :137 "blake3 removes that" per the blocking finding, and :95 "Total, deterministic ordering" is now a total preorder, whose ties are exactly the dangerous case.

Nine stale comments describing the old rule, five inside the rewritten file: member_info.rs:230, :236, :931, :1286, :1291; ui/src/room_data.rs:877, :6339, :6416; ui/src/components/members/member_info_modal/nickname_field.rs:193. Also .claude/rules/contract-summary-determinism.md:59 still describes the value type as Signature. AGENTS.md asks for stale docs fixed in the same PR.

Bytes traded for contract CPU, unmentioned in a perf(...) PR. is_ban_authorized calls deputies_of at member.rs:378, :384, :393, each an O(|member_info|) hash sweep through canonical, inside the per-ban loop at :279 — roughly O(bans × chain_depth × member_info_count) blake3 hashes per validation, under Cranelift OptLevel::None. Since 0.2.105 per-contract WASM CPU is an eviction axis, so this deserves a sentence. Separately dedup_to_canonical (:77-78) recomputes the incumbent's digest on every collision; storing (rank, info) in the map fixes that for free.

The body's test claim is stale. It says "All 406 river-core tests pass", but commit b1db4ed1 then documents that equal_version_member_info_diff_detected_by_anti_entropy was a ~45% flake (89 failures in 200 runs) and that river-ui did not compile — the green run won the coin flip. That fix is good work, honestly written up in the commit message; the body just needs to stop carrying the original claim. 406 also matches none of the counts in that commit.

Consider

sig_digest's doc and the body cite "#571" as though it were the PR that changed behavior; #571 is the issue, this is #572. The body's "three of the five sub-summaries" undercounts: there are nine type Summary declarations under common/src/room_state/, including SecretsSummary (secret.rs:251) alongside the acknowledged DirectMessagesSummary.

Checked and clean — recorded so coverage is visible

• expect() on the digest slice (:146-150): unreachable. [..8] of a fixed 32-byte array; try_into is infallible.
• Summary/delta agreement: compared line-by-line against the pre-change form. Equivalent outside a digest collision; the "summary value IS the rank" refactor removes a derivation step rather than adding one.
• No missed call sites. Verified by grepping all uses of the changed types rather than reading the diff — member_info_rank, sig_digest, type Summary (all nine), summarize/StateSummary across ui/, cli/, contracts/, delegates/, plus (u32, Signature) prose repo-wide. All five rank consumers route through the single member_info_rank; notably the PR does not take the tempting shortcut of keeping the raw-signature compare on non-wire paths, which would have silently reinstated #411 round 4 B.
• Determinism: still BTreeMap; the only HashMap (dedup scratch, :72) is drained to a Vec and sorted by member_id. The #4857 HashMap-ordering class is not reintroduced.
• No vacuous tests: reverting the type breaks compilation rather than passing silently, and the oracles are genuine independent recomputations. A constant sig_digest makes equal_version_member_info_resolves_deterministically_across_apply_order fail on its first assertion.
• No client/contract skew: nothing persists a summary; the UI's summary is consumed locally to build a delta, and riverctl sends summary: None.
• No new WASM dependency: blake3 already ships in both river-core and river-ui and already runs inside the room contract.

Note on the green checks

All 7 checks pass at b1db4ed1, but check-room-contract-migration is vacuous for this diff: it compares the committed ui/public/contracts/room_contract.wasm, which this PR does not touch, so it reports "no migration entry needed" for a change that alters contract behavior. The body already acknowledges the re-key and holds the PR as draft; flagging only so the green tick is not read as migration clearance. The eventual WASM-rebuild commit will be caught correctly, since it compares against main's old hash.

[AI-assisted - Claude]

[sanity]

Correction to my review above, on the byte arithmetic and therefore on the recommended fix. I verified the serializers rather than continuing to reason from the figure everyone (including me) had been repeating.

ed25519::Signature::serialize calls serialize_tuple(64) and pushes 64 individual u8 elements (ed25519-2.2.3/src/serde.rs:7-18); ciborium's serialize_tuple delegates to serialize_seq, i.e. a CBOR array (ciborium-0.2.2/src/ser/mod.rs:236-238). So a Signature encodes as an array of 64 unsigned ints, ~124 bytes — not the 66 bytes of a serialize_bytes encoding. 66 is what River's own SignatureBytes newtype produces (direct_messages.rs:217-221), which is a different type.

Corrected per-entry figures (MemberId ≈ 9-byte CBOR int for a real hash):

	bytes/entry
before	~135
this PR (u64)	~20
with a 128-bit digest as (u32, u64, u64)	~29

Two consequences.

The recommended fix changes shape. My review said [u8; 16]. That is wrong: Rust arrays serde-serialize as tuples, so [u8; 16] becomes a CBOR array of 16 small ints, ~31 bytes for the digest alone and ~43 for the entry — which would break the 32-byte bound at summary_determinism_test.rs:356 that I claimed it would fit under. Use (u32, u64, u64) instead, carrying the two 8-byte halves of the digest. ~29 bytes/entry, comfortably inside the bound, and it keeps essentially all of the bandwidth win.

Finding C is resolved, and in the PR's favour. The arithmetic was impossible (470 × 66 = 31 KB of signatures exceeding a stated 29 KB total) because 66 was the wrong per-signature figure. The real reduction is larger than the PR claims. The body and the member_info.rs doc comments should carry the corrected numbers rather than the understated ones.

Note the deferred DirectMessagesSummary follow-up genuinely is ~66 bytes/entry, because it uses SignatureBytes. Two different encodings; they must not be reasoned about with a single figure.

Credit where due: this came from the wire-format lens, which traced the serializers instead of accepting the number the other two lenses and I had all repeated. Worth recording that the figure survived three independent reviews before anyone checked it.

The blocking finding itself is unaffected — 64 bits is still inside a ~2^32 birthday grind, the divergence is still unhealable, and the 128-bit precedent in direct_messages.rs still stands. Only the remedy's encoding changes.

[AI-assisted - Claude]

[sanity]

Re-review: #572 at head e8092b0e (the two post-review fix commits)

Verdict: no blocking findings. One medium (disclosure), four low, four info. Adversarial lens with every claim re-verified by execution, not reading.

Checked and clean — the load-bearing set

• Round-trip: the SigDigest serde pair is a byte-for-byte copy of the shipped PurgeToken pattern. Probe tests drove the REAL decode path (full ChatRoomStateV1Summary through ciborium and back, byte-identical, then delta() off the decoded summary — exactly what get_state_delta does, and grep confirms it is the only real decode path; nothing persists a summary). Wrong lengths (0/8/15/17/32) all rejected. One benign asymmetry: the deserializer also accepts the 16-element array form; the serializer never emits it, so no divergence is possible.
• Golden vector reproduced externally: b3sum over 0x00..0x3f matches EXPECTED exactly. All three mutations run for real: bytes reversed FAILS, last-16 selection FAILS, derived-impl FAILS on both the vector and the size guard (42.53 B/entry, over the bound).
• All five rank consumers first-wins on strict >, checked one by one, including the hand-rolled canonical loop (no off-by-one). The tie branch is unreachable without a genuine collision — verify checks every record's signature, so the "latent trap, not reachable bug" framing is accurate.
• The in-test baseline is not a drifted reconstruction: git show of the base commit confirms the old type and the old summarize inserted exactly what the test rebuilds, same real serde impl.
• Size guard deterministic (3 identical runs; seeded keys, no timestamps, fixed-width encoding): 134.08 → 28.01 B/entry, 4.8x.
• The 2^64 doc claim is conservative — it omits that each birthday candidate costs an ed25519 signing op, and the retired 64-bit figure ("hours on commodity hardware") also checks out arithmetically.

Medium — three of four green CI ticks are blind to this change; the body names only one

The body discloses check-room-contract-migration's vacuity. Also blind: "Check CLI WASM Sync" only cmps the two committed WASM copies against each other (never against source — while its own trigger list claims common/src/** coverage), and the riverctl bump guard doesn't fire on common/src/ changes. Concrete drift while the rebuild is deferred: committed WASM runs the old tiebreak, river-core source runs the new one. Extend the existing disclosure paragraph to name check-cli-wasm.yml.

Low

1. apply_delta's tie direction is asserted in prose, pinned by nothing — > → >= there leaves all 409 tests green (hard to reach behaviorally since that branch verifies signatures). Extract a shared outranks() and test it once, or source-pin the three sites.
2. canonical's digest arm has no river-core test — a version-only-keep-first mutation passes all 409; only a UI test catches it. Cheap deterministic test: two with_signature records, same version, fixed signatures, greater digest must win. This PR rewrote canonical, so now is the moment.
3. The loser-first refixture trades detection of last-wins for detection of first-match/version-only — acceptable, since canonical_and_dedup_break_rank_ties_identically was confirmed to catch last-wins at workspace level.
4. "The absolute cost is small" is the one unmeasured claim in a measure-don't-derive PR. Measured: summarize() 470 records ~18 → ~97 µs; ≈7 ms per UPDATE under the Cranelift no-opt penalty across the three passes. Fine — but put the numbers in. (The HashSet pre-pass optimization is noted and deliberately NOT requested — scope.)

Info

The "32 bytes" derive figure is an expectation for random content, not fixed (worth one word); the "37% miss" figure for the reversed-digest oracle reproduced at 1/12 rather than 11/30 — conclusion unchanged (intermittent = broken), figure softened; one stale comment at room_synchronizer.rs:186 ("signature map" → digest map); and a pre-migration check for the bundle: an equal-version duplicate surviving in the Official room's state would flip ban authority under the new tiebreak at re-PUT — post_apply_cleanup makes this unlikely, and a one-line riverctl duplicate check before migrating converts unlikely into verified.

Scope

Source-only review. The deferred bundle (WASM rebuild + legacy_room_contracts.toml + riverctl bump + moderation-key pin) still stands before leaving draft and needs its own pass — and per the medium finding, CI will not catch its absence for you.

[AI-assisted - Claude]

[sanity]

PRE-MIGRATION GATE — must clear before the re-key PUT, not before merge

The migration bundle has landed (fe07a725), so this PR now re-keys the room contract on publish. One check has to happen between merge and the re-key PUT, and it cannot currently be run:

Verify the Official room's state carries no equal-version member_info duplicate.

Why it gates. This PR changes the equal-version tiebreak from raw signature bytes to a 128-bit digest. If two records for the same member at the same version survive in the live state, the old rule and the new rule can select different winners — and deputies_of resolves ban authority through that winner (member.rs:378-393). The re-PUT would silently move who may ban whom. post_apply_cleanup dedups, so this is unlikely; the check converts unlikely into verified.

Status: NOT satisfied. What I have is indicative, not gating:

Official room (owner 4uNUKFzZQCn...), riverctl local store, written 2026-07-29 20:58
  477 member_info records
  477 distinct member_ids
    0 duplicate member_ids (any version)
    0 EQUAL-VERSION duplicates

Reassuring, but the snapshot is stale — it reports 477 records while a live riverctl debug room-state reports member_count: 118. It is a client-side cache, and no read-only riverctl command refreshes it (verified: room list, member list, debug room-state, debug contract-get all leave its mtime untouched).

It cannot be run live today. member list is canonicalized through MemberInfoV1::canonical, so duplicates are invisible by construction; debug room-state is aggregates only; debug contract-get prints a summary rather than raw state. Nothing surfaces per-record version. Filed as #577 with two suggested fixes.

Before the re-key PUT, one of these must hold

• riverctl: no read-only command exposes per-record member_info version, so the equal-version-duplicate check cannot be run live #577 lands and the check runs clean against live state, or
• @sanity explicitly waives it, having seen the stale-snapshot numbers above

Deliberately not a merge blocker: merging changes no live room state. Publishing does.

Also note — three of four green ticks are blind to the source change

Detailed in the PR body. check-room-contract-migration is now meaningful (the bundle rebuilt the WASM, so it compares against main's old hash), but check-cli-wasm.yml only cmps the two committed WASM copies against each other, never against source, and its riverctl-bump guard's path list omits common/src/.

[AI-assisted - Claude]

[sanity]

Pre-migration gate: CLEARED with live data (see the gate comment above).

Ran the live equal-version-duplicate check tonight using the #577 tool (draft PR #578), against the live Official room via a node websocket (raw records, no canonicalization, read-only run):

• total member_info records: 127
• distinct member_ids: 127
• duplicate member_ids (any version): 0
• equal-version duplicates: 0 — PASS

Cross-check: debug room-state on the same node reported member_count 126 (+1 owner record = 127, consistent). This also confirms the earlier 477-record local-store snapshot was stale, as suspected.

Conclusion: the #572 tiebreak change (raw signature bytes → SigDigest ordering) cannot flip any winner in the live Official room. The re-key publish is clear on this risk; remaining publish requirement is only the two-contract sequencing (riverctl + UI together).

[AI-assisted - Claude]