[sanity]

Fixes #796.

Problem

Once the ban list is full of owner bans, every new deputy ban is evicted as soon as it is added. #702's cap ranked owner bans above all others, so in a full list of owner bans a deputy's ban was always the lowest-ranked entry. In the Freenet Official room the list sat at 200/200 owner bans (the moderator bot and the nickname autoban sweep both sign with the owner key). A deputy ban on a spammer, made in the UI, appeared to succeed and did nothing.

Change

BansV1::enforce_user_ban_cap now evicts by recency, following @sanity's direction:

1. Dead entries go first, oldest first: bans that take no effect (BanResolution::INERT) and later duplicates of an (issuer, target) pair. This classification is unchanged from fix(contract): stop an orphaned ban from forking a room permanently (#423) #702.
2. Live bans are split into owner bans and all other bans. Each pool is guaranteed max_user_bans / 2 slots. Only a pool's bans beyond its guarantee can be evicted, and the oldest of those go first, by (banned_at, id). With an odd cap the spare slot goes to the newer ban, so at cap 1 this is plain recency. The guarantee is a floor, not an allocation: owner bans are often rare, so the other pool can use slots the owner pool leaves free.

Why not one pool by pure recency: banned_at is signed by the issuer and the contract has no clock, so far-future-dated bans always count as newest. Under pure recency any member could flush every ban, owner bans included.

Residual, stated plainly. Any current member, with no moderator rights, can mint live, far-future-dated bans: against throwaway members they invited, or against random ids whose record is held nowhere. Such a flood evicts:

• every older non-owner ban, deputies' included, except in slots owner bans leave unused. This was already true under fix(contract): stop an orphaned ban from forking a room permanently (#423) #702. While the flood stands, a new deputy ban is evicted at once (Ban cap-eviction hardening: transient cross-order divergence + un-ban DoS #413);
• owner bans beyond the newest max_user_bans / 2. This is new, since under fix(contract): stop an orphaned ban from forking a room permanently (#423) #702 owner bans were immune.

Banning the flooder makes all their bans dead, so the next cap pass drops them; bans already evicted stay evicted. A room that wants every owner ban protected sets max_user_bans to at least twice its owner-ban count. In the Freenet Official room that means 400 or more, since it holds 200 owner bans today.

enforced_ban_set_of (DM apply) calls the same function, so the two sites still agree by construction.

Re-key

This changes the room contract and the chat delegate (which links river-core's room state), so the PR re-keys both:

• V35 entries in common/legacy_room_contracts.toml (eb2222c3…) and legacy_delegates.toml (49a3122b…), added from the committed WASMs before the rebuild
• rebuilt room_contract.wasm (ui + cli) and chat_delegate.wasm via scripts/sync-wasm.sh
• re-signed pointer-records.toml
• river-core 0.2.0 → 0.2.1, riverctl 0.2.21 → 0.2.22, plus a CHANGELOG entry

There is no state-format change. Stored states keep deserializing, and a state at or under the cap is untouched.

Heads-up for #793, which also claims V35 and riverctl 0.2.22: whichever PR merges second needs to renumber.

Tests

• New:
  • a_full_list_of_owner_bans_does_not_evict_a_new_deputy_ban: the Full ban list of owner bans silently evicts every new deputy ban #796 regression; fails on main.
  • rare_owner_bans_leave_the_whole_cap_to_other_bans and a_new_owner_ban_evicts_the_oldest_ban_of_the_pool_over_its_share.
  • a_flood_leaves_the_newest_half_cap_of_owner_bans and a_plain_members_random_target_flood_evicts_owner_bans_down_to_the_share: pin the residual.
  • an_odd_caps_spare_slot_goes_to_the_newer_ban and cap_one_keeps_the_newer_ban_whatever_its_pool.
  • a_dead_entry_and_a_pool_eviction_in_one_pass: input-order independence and idempotence.
  • dm_ban_derivation_applies_the_cap_pools: the DM path decides by pools.
• Mutation check: with the share set to 0 (pure recency), five of these fail.
• Rewritten for the new rule: the sybil-flood test now asserts that owner bans survive and that the older deputy ban is evicted (the residual, stated explicitly); cap_evicts_a_duplicate_before_any_live_ban; and three fixtures (cleanup_is_idempotent_over_cap_evicted_ban, dm_ban_derivation_must_apply_the_user_ban_cap, ban_evicted_by_the_ban_cap_keeps_the_targets_messages) whose eviction premise relied on owner-first ranking. Each still tests its original property.
• cargo test -p river-core (all suites), -p riverctl --lib, -p chat-delegate, plus the migration, room-contract-migration and delegate-key tests pass locally. Pointer freshness and both migration checks pass.

[AI-assisted - Claude]

[sanity]

PR review #797, round 1 (HEAD reviewed: 8d39e58)

Tier: Full, because this changes contract merge semantics and ban authorisation and re-keys the room contract and chat delegate.
Reviewers: four independent Claude reviewers (code-first, testing, skeptical, big-picture), each reading blind, plus Codex (codex review --base origin/main). Codex completed and reported no findings.

Consensus

• The algorithm is sound: deterministic, independent of input order, idempotent at the cap, and with no panic path. The candidate set always covers to_remove.
• enforced_ban_set_of (DM apply) still calls the same function, so the two sites agree by construction.
• Re-key bookkeeping is consistent: the V35 hashes equal the previous pointer hashes, the pointer records were re-signed, and versions and changelog were bumped.
• No test was silently dropped. The one inverted assertion (the sybil flood) is explicit.

Findings and resolution (fixed in 73bad66)

1. Residual understated (skeptical, code-first, big-picture). The docs said a flood needs "a member with ban authority". In fact any member can mint live, far-future-dated bans, either against its own invitees or against ids whose record is held nowhere. Such a flood evicts older non-owner bans, as it already could under #702, and also owner bans beyond the newest max_user_bans / 2, which is new. Fixed: this is now stated plainly in ban.rs, room_state.rs and the CHANGELOG, with the mitigation: banning the flooder makes their bans dead, and max_user_bans >= 2 x owner bans protects every owner ban. Ranking mutual bans lower would not close it, because inviter bans give the same reach.

2. Odd caps (testing, skeptical). owner_share = max/2 gave the spare slot to the non-owner pool, which is the one an attacker can fill. At cap 1 the owner got no guarantee. Fixed: both pools are now guaranteed max/2, the spare goes to the newer ban, and cap 1 is plain recency. Tested at cap 3 and cap 1.

3. Test gaps (testing). Added:

   • a dead entry plus pool eviction in one pass, with reversed and rotated input and a second pass, all giving the same result;
   • the DM path deciding by pools (dm_ban_derivation_applies_the_cap_pools, which fails under pure recency);
   • a pin for the plain-member random-target flood.

   Mutation check: setting the share to 0 (pure recency) fails five tests.

4. Stale comments (code-first, testing). The step 0-cap comment in room_state.rs and the doc on cycle_bans_rank_mutual_after_an_earlier_removal still described tier ranking. Fixed.

Not changed

• Surfacing a ban the cap drops in the UI/CLI (skeptical #3). This is a separate UX change, left on #796.
• Coordination with #793: it also claims V35 and riverctl 0.2.22. Whichever PR lands second must chain its "before" hash to the other PR's output and re-sign the pointer records, not just renumber.

Verdict: Needs Changes, re-review required after the fixes. A full re-review on 73bad66 follows.

[AI-assisted - Claude]

[sanity]

PR review #797, rounds 2 and 3 (final HEAD 09790a3)

Tier: Full. Lenses per round: code-first, testing, skeptical and big-picture, each reading blind.
External model:

• Codex (codex review --base origin/main) completed rounds 1 and 2 with no findings.
• In round 3 it failed twice with ERROR: Selected model is at capacity (HTTP 503 from OpenAI). Per policy, the four Claude lenses stood in for it.
• The final diff after round 3 (docs and tests only; logic and contract WASM unchanged) had a separate light re-check.

Round 2 (73bad66): resolved in f32d843

• Pointer record version skipped. The room-contract pointer went 4 → 6, so it was re-signed from main's file: room contract 4 → 5, chat delegate 5 → 6.
• Ban-record classification hardened. The cap step now judges each stored ban record by its own signature against its claimed issuer, and treats a record that fails as dead. Its sort breaks (banned_at, id) ties by record digest, so the result never depends on arrival order. Round 3's skeptical reviewer confirmed that this cannot misclassify a genuine ban, including bans by issuers held only in ban evidence and bans by owner-removed issuers.
• Doc and test gaps: fixed.

Round 3 (f32d843): resolved in 530bc13 / 09790a3

• Doc precision:
  • the odd-cap spare slot goes to the newest ban beyond either pool's share;
  • each pool's guarantee is floor(cap/2), so nothing at cap 1;
  • banning a flooder does not restore bans already pushed out;
  • the input-order claim is reconciled with the CONVERGENCE HONESTY note.
• Tests added:
  • an inert ban goes before older live bans;
  • a ban on an id held nowhere, by an issuer known nowhere, is dead;
  • surplus dead entries go oldest first;
  • several evictions across both pools.
  • One misleadingly named test was renamed.
• Coordination: notes posted on #793 (V36 must chain from this PR's hashes), #792 (clean merge; its tab reload is triggered by this PR's pointer bump) and #758.

Deferred, with reasons

• The CLI debug ban classification has a similar display-only issue. It never affects stored state, because cleanup step 4 checks every record. Follow-up after publish.
• A further pre-existing hardening item was found outside this change's scope. It is being handled separately.

Verdict: Ready to merge once CI is green on 09790a3.

[AI-assisted - Claude]