From 13741be69529eae3ced876189e93d7ec0c9d37f5 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 22 Apr 2026 18:01:05 +0000 Subject: [PATCH 01/19] fix(marmot-interop): decode npub locally so member-list checks use hex `wn users show ` only resolves identities the local daemon has cached, so A's npub (the Amethyst account, which B's daemon has never interacted with directly) falls through to the "return input unchanged" branch. That leaks the raw npub into the Test 02 member-list assertion, where it's compared against hex pubkeys pulled from `wn groups members` and fails with "member list missing npub1...". Add a pure-awk bech32 decoder and use it as the final fallback in `npub_to_hex`, so unknown npubs still convert to the 32-byte hex form the daemon emits. Also harden the JSON path to peel a `.result` wrapper in case a future `wn --json users show` ships one. --- tools/marmot-interop/lib.sh | 59 +++++++++++++++++++++++++++++++++---- 1 file changed, 53 insertions(+), 6 deletions(-) diff --git a/tools/marmot-interop/lib.sh b/tools/marmot-interop/lib.sh index 0196117b1..7346cf18e 100644 --- a/tools/marmot-interop/lib.sh +++ b/tools/marmot-interop/lib.sh @@ -293,13 +293,56 @@ extract_pubkey() { if [[ -n "$v" ]]; then printf '%s' "$v"; return; fi } -# Best-effort npub -> hex conversion via `wn users show`. If the lookup fails -# (e.g. --json not supported or user not resolvable), returns the input -# unchanged — downstream comparisons tolerate either form. +# Pure-awk bech32 decoder for npub1... → 32-byte lowercase hex. Needed because +# `wn users show` only resolves identities the local daemon already knows +# about, so A's npub (the Amethyst account) can't be converted via wn — and a +# failed conversion leaks the raw npub into hex-equality checks like the +# member-list assertion in Test 02, which then fails with +# "member list missing npub1…". +bech32_decode_npub() { + local npub="$1" + [[ "$npub" == npub1* ]] || return 1 + local body="${npub#npub1}" + # Need at least 6 chars of checksum + some data. + (( ${#body} > 6 )) || return 1 + local hex + hex=$(awk -v data="$body" 'BEGIN { + charset = "qpzry9x8gf2tvdw0s3jn54khce6mua7l" + for (i = 0; i < 32; i++) map[substr(charset, i + 1, 1)] = i + data = substr(data, 1, length(data) - 6) + acc = 0; bits = 0; out = "" + for (i = 1; i <= length(data); i++) { + c = substr(data, i, 1) + if (!(c in map)) exit 1 + acc = acc * 32 + map[c] + bits += 5 + while (bits >= 8) { + bits -= 8 + pow = 1 + for (j = 0; j < bits; j++) pow *= 2 + byte = int(acc / pow) + acc -= byte * pow + out = out sprintf("%02x", byte) + } + } + print out + }' 2>/dev/null) || return 1 + # 32-byte x-only pubkey = 64 hex chars. + [[ ${#hex} -eq 64 ]] || return 1 + printf '%s' "$hex" +} + +# npub -> hex conversion. Tries `wn users show` first (handy when it works, +# since it also validates the npub against the daemon's view), then falls +# back to a local bech32 decode so unknown npubs still convert. Returns the +# input unchanged only if both paths fail, which should never happen for a +# well-formed npub. npub_to_hex() { local npub="$1" raw hex raw=$(wn_b --json users show "$npub" 2>/dev/null || true) - hex=$(printf '%s' "$raw" | jq -r '.pubkey // .public_key // .hex // empty' 2>/dev/null || true) + hex=$(printf '%s' "$raw" \ + | jq -r '(.result // .) | (.pubkey // .public_key // .hex // .[0]?.pubkey // .[0]?.public_key // empty)' \ + 2>/dev/null || true) if [[ -z "$hex" || "$hex" == "null" ]]; then # fallback to yaml-ish output raw=$(wn_b users show "$npub" 2>/dev/null || true) @@ -307,9 +350,13 @@ npub_to_hex() { fi if [[ -n "$hex" && "$hex" != "null" ]]; then printf '%s' "$hex" - else - printf '%s' "$npub" + return fi + if hex=$(bech32_decode_npub "$npub"); then + printf '%s' "$hex" + return + fi + printf '%s' "$npub" } # ------- state file ---------------------------------------------------------- From fda58f4413fe1003d9a26e24214c63eaed3b3527 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 22 Apr 2026 18:21:22 +0000 Subject: [PATCH 02/19] fix(marmot-interop): diagnose B->Amethyst kind:445 drops in Test 02 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When `wn_b messages send` fires but Amethyst never displays the reply, the harness used to swallow the send output and only ask the operator a blind yes/no — giving no signal about which side of the pipe failed. Amethyst subscribes to kind:445 on the MLS GroupContext `relays` extension (falling back to `Account.homeRelays`), so the two actionable failure modes are: - wn published to a relay Amethyst isn't subscribed to (group-relay mismatch between the MLS extension and Amethyst's subscription) - Amethyst received the event but `GroupEventHandler.add` dropped it (no `h` tag, `isMember=false`, decrypt error) Teach the harness to dump both sides automatically on the reply step: - Capture wn's `messages send` stdout/stderr into the run log. - `dump_outbound_diagnostics` prints wn's local view of the group messages (did the send persist to wn's own DB?), the MLS GroupContext relay list (what Amethyst should be subscribed to), and live relay connection status. - On operator-reported fail, prompt for an Amethyst-side logcat grep targeted at the exact log points — MarmotGroupEventsEoseManager filter updates, GroupEventHandler.add arrivals, and the membership drop branch — naming the current group id so the operator can grep for the right hex prefix. --- tools/marmot-interop/lib.sh | 64 ++++++++++++++++++++++++++ tools/marmot-interop/marmot-interop.sh | 32 ++++++++++++- 2 files changed, 95 insertions(+), 1 deletion(-) diff --git a/tools/marmot-interop/lib.sh b/tools/marmot-interop/lib.sh index 7346cf18e..646b2d44b 100644 --- a/tools/marmot-interop/lib.sh +++ b/tools/marmot-interop/lib.sh @@ -471,6 +471,70 @@ Then paste /tmp/amethyst-marmot.log contents here so the failure report has both the Amethyst-side publish log and the wn-side subscription log ($note)." } +# Dump everything we know about an outbound kind:445 from a wn daemon, so +# when the operator reports "Amethyst didn't see it" we can tell them which +# side of the wire to investigate: +# +# - `messages list` output (did the daemon persist the message locally, +# i.e. is this a relay-reach issue, a membership issue, or a pure +# Amethyst-receive issue?) +# - `groups show` metadata (which MLS-extension relays is wn using for +# this group — these are the SAME relays Amethyst subscribes to, so if +# the lists don't match that's the bug) +# - live relay connection status +# +# Output is tee'd to stderr (live) and $LOG_FILE (forward). +dump_outbound_diagnostics() { + local who="$1" gid="$2" needle="${3:-}" tag="${4:-post-send}" + local wnfn + if [[ "$who" == "B" ]]; then wnfn=wn_b + else wnfn=wn_c; fi + + printf '\n%s%s---- [%s] %s outbound diagnostics (group %.8s…) ----%s\n' \ + "$C_BOLD" "$C_CYAN" "$tag" "$who" "$gid" "$C_RESET" >&2 + printf '\n==== [%s] %s outbound diagnostics (group %s) ====\n' "$tag" "$who" "$gid" >>"$LOG_FILE" + + step "$who local view of latest messages in group" + local list_raw + list_raw=$("$wnfn" --json messages list "$gid" --limit 5 2>/dev/null || true) + printf ' %s\n' "${list_raw:-}" >>"$LOG_FILE" + printf '%s' "$list_raw" \ + | jq -r '(.result // .) | .[]? | " \(.id // .event_id // "?") \((.content // .text // "") | tostring | .[0:80])"' 2>/dev/null \ + | tee -a "$LOG_FILE" >&2 || true + if [[ -n "$needle" ]]; then + if printf '%s' "$list_raw" | jq -e --arg n "$needle" \ + '(.result // .) | .[]? | select((.content // .text // "") | contains($n))' \ + >/dev/null 2>&1; then + info "$who local store contains \"$needle\" — send reached wn's own DB (kind:445 went out)" + else + warn "$who local store does NOT contain \"$needle\" — `messages send` never persisted; check stderr.log" + fi + fi + + step "$who MLS group metadata (relays here are what Amethyst subscribes on)" + local meta_raw + meta_raw=$("$wnfn" --json groups show "$gid" 2>/dev/null || true) + printf ' %s\n' "${meta_raw:-}" >>"$LOG_FILE" + local meta_relays + meta_relays=$(printf '%s' "$meta_raw" \ + | jq -r '(.result // .) | (.relays // .group.relays // [])[]? | (. | tostring)' 2>/dev/null \ + | paste -sd',' -) + info " group relays (MLS extension): ${meta_relays:-}" + local meta_name meta_epoch + meta_name=$(printf '%s' "$meta_raw" | jq -r '(.result // .) | (.name // .group.name // "?")' 2>/dev/null) + meta_epoch=$(printf '%s' "$meta_raw" | jq -r '(.result // .) | (.epoch // .group.epoch // "?")' 2>/dev/null) + info " group name: $meta_name epoch: $meta_epoch" + + step "$who relay connection status right now" + "$wnfn" --json relays list 2>/dev/null \ + | jq -r '(.result // .)[]? | " \(.url // .relay.url // "?") [\(.type // "?")] [\(.status // .connection_status // "?")]"' 2>/dev/null \ + | tee -a "$LOG_FILE" >&2 || true + + printf '%s%s---- end outbound diagnostics ----%s\n\n' \ + "$C_BOLD" "$C_CYAN" "$C_RESET" >&2 + printf '==== end [%s] %s outbound diagnostics ====\n\n' "$tag" "$who" >>"$LOG_FILE" +} + # ------- group cleanup ------------------------------------------------------- cleanup_group() { local gid="$1" who="${2:-B}" diff --git a/tools/marmot-interop/marmot-interop.sh b/tools/marmot-interop/marmot-interop.sh index 876572cd6..e4e2ec79b 100755 --- a/tools/marmot-interop/marmot-interop.sh +++ b/tools/marmot-interop/marmot-interop.sh @@ -473,11 +473,41 @@ that's the bug." fi step "B sends reply" - wn_b messages send "$gid" "hello from wn" >/dev/null 2>&1 || warn "send returned nonzero" + local send_out + if ! send_out=$(wn_b messages send "$gid" "hello from wn" 2>&1); then + warn "wn messages send returned nonzero: $send_out" + fi + printf 'wn messages send (test_02 reply) raw: %s\n' "$send_out" >>"$LOG_FILE" + # Give the relay a couple seconds to fan the commit/message back out to + # Amethyst's subscription before we ask the operator to eyeball the UI. + sleep 4 + + # Dump the B side up front — if B's own store doesn't contain the reply + # or the MLS extension relays don't match what Amethyst subscribes to, + # the operator can confirm "fail" with a specific reason instead of a + # blind eyeball check. + dump_outbound_diagnostics B "$gid" "hello from wn" "test_02 B->A reply" if confirm "Does Amethyst now show 'hello from wn' in the same group?"; then record_result "02 Amethyst->B create+invite" pass else + # The B-side dump above told us whether the send reached B's own DB and + # which relays wn tagged on the kind:445. Now pull the Amethyst side so + # the failure report has both halves of the pipe: which relays the + # kind:445 subscription actually targets, whether the event arrived, + # and (if it did) whether GroupEventHandler dropped it. + prompt_human "Capture Amethyst's side of the B->A kind:445 path. On the adb host: + + adb logcat -d -v time | grep -E \ + 'MarmotDbg|GroupEventHandler|MarmotGroupEventsEoseManager|kind:445' \\ + | tail -n 200 > /tmp/amethyst-marmot.log + +Then paste /tmp/amethyst-marmot.log here. Key lines to look for: + - 'MarmotGroupEventsEoseManager.updateFilter: group=${gid:0:8}…' + the relay + list — must match the 'group relays (MLS extension)' in the B-side dump. + - 'GroupEventHandler.add: kind:445 id=… groupId=${gid:0:8}…' (event arrived). + - 'GroupEventHandler.add: not a member of group=${gid:0:8}…' (dropped). + - 'GroupEventHandler.add: processGroupEvent returned ApplicationMessage…' (decrypted)." record_result "02 Amethyst->B create+invite" fail "Amethyst did not display reply" fi } From 414fd31bf9ddd3a3fa43b7a7e3d26d188a6e0f8f Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 22 Apr 2026 18:32:31 +0000 Subject: [PATCH 03/19] fix(marmot-interop): publish kind:0 for B and C before sanity probes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Several public relays (damus.io in particular) silently drop kind:1059 / 10050 / 30443 from accounts they've never seen before — the sanity check in the interactive harness reports exactly this failure mode when the three-relay default set is in use, because wn's identities are brand-new and have nothing on file. Before the kind:30443 + 10050/1059/445 probes, publish a kind:0 profile event for each wn identity via `wn profile update --name --about`. That marks both accounts as "known" on the relay and lets the subsequent gift-wrap and KeyPackage publishes land. Runs inside configure_relays (after relays are added, before the first sanity probe) so the sequencing is: add relay -> advertise profile -> publish KPs -> start exchanging welcomes. Does not help when a relay outright refuses the kind (some relays whitelist only kind:1 / 3 / 7); those still need --local-relays. --- tools/marmot-interop/marmot-interop.sh | 30 ++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/tools/marmot-interop/marmot-interop.sh b/tools/marmot-interop/marmot-interop.sh index e4e2ec79b..3ba20b67d 100755 --- a/tools/marmot-interop/marmot-interop.sh +++ b/tools/marmot-interop/marmot-interop.sh @@ -281,6 +281,36 @@ configure_relays() { add_relay wn_c "$r" done + # Push a kind:0 profile for each wn identity BEFORE the kind:30443 / + # 10050 / 1059 probes. Several public relays (damus.io in particular) + # silently drop non-profile kinds from accounts they've never seen + # before, so a fresh wn identity created inside the harness never gets + # its gift wraps or inbox lists stored. Publishing kind:0 first + # registers the account in the relay's "known users" set, which lets + # every later publish go through. + publish_profile() { + local who="$1" wnfn + if [[ "$who" == "B" ]]; then wnfn=wn_b; else wnfn=wn_c; fi + local name="marmot-interop $who" + local about="Scripted wn identity for Amethyst<->whitenoise-rs interop harness" + local out + if out=$("$wnfn" profile update --name "$name" --about "$about" 2>&1); then + info "$who kind:0 published (name=\"$name\")" + printf '%s profile update: %s\n' "$who" "$out" >>"$LOG_FILE" + else + warn "$who profile update failed: $out" + printf '%s profile update: %s\n' "$who" "$out" >>"$LOG_FILE" + fi + } + step "publishing kind:0 profiles so relays treat B and C as 'known' accounts" + publish_profile B + publish_profile C + # Give the relay a beat to persist + ack the kind:0 before the next + # publish (some relays gate subsequent writes on the profile being + # indexed, and without this pause the first `keys publish` that + # follows sometimes races ahead of the kind:0 commit). + sleep 2 + step "B relay list after configure" local relay_list relay_list=$(wn_b --json relays list 2>/dev/null || true) From 76f23c079829af45ff4fc53f78a99072796bfdb1 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 22 Apr 2026 18:43:28 +0000 Subject: [PATCH 04/19] fix(marmot-interop): filter stale welcomes in wait_for_invite MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit wnd persists pending group invites across daemon restarts, so on any rerun C (and B) already have unaccepted welcomes from previous runs lying around. `wait_for_invite` was pulling `.[0]` out of the invite list, which is the oldest pending — i.e. the stalest — and treating it as the arrival the caller just triggered. In the sanity probe this caused the kind:445 step to send to B's NEW group id while C was accepting a DIFFERENT (old) group, producing a bogus "kind:445 failed — relays may be dropping group messages" with no actual relay issue. Add `snapshot_invites ` which returns the current gids as CSV, and teach `wait_for_invite` to accept that CSV as an ignore list so it walks past stale welcomes and only returns a freshly-arrived one. Use the new snapshot→wait pattern at every call site: sanity check, Test 02 (B), Test 04 (C), Test 05 (C), Test 08 (C), Test 12 (C). Also log a gid-mismatch warning in the sanity check when B's created group and C's accepted welcome disagree, as an extra guard against the filter being bypassed. --- tools/marmot-interop/lib.sh | 59 ++++++++++++++++++++------ tools/marmot-interop/marmot-interop.sh | 49 ++++++++++++++++----- 2 files changed, 84 insertions(+), 24 deletions(-) diff --git a/tools/marmot-interop/lib.sh b/tools/marmot-interop/lib.sh index 646b2d44b..406dfbcf5 100644 --- a/tools/marmot-interop/lib.sh +++ b/tools/marmot-interop/lib.sh @@ -177,17 +177,45 @@ jq_group_id() { # ------- polling helpers ----------------------------------------------------- -# wait_for_invite -# Echoes the first new group_id that appears in `wn groups invites --json`. +# Snapshot currently-pending invites on as a comma-separated list of +# gids. Use this BEFORE triggering a publish that should produce a new +# welcome, then pass the result to `wait_for_invite` as the ignore list so +# a stale welcome left over from a previous harness run (wnd persists +# pending invites across restarts) doesn't register as the arrival we +# just triggered. +snapshot_invites() { + local who="$1" wnfn + if [[ "$who" == "B" ]]; then wnfn=wn_b + else wnfn=wn_c; fi + local raw + raw=$("$wnfn" --json groups invites 2>/dev/null || true) + # jq_group_id reads one invite at a time, so iterate the array in shell + # rather than trying to handle all shapes in a single jq expression. + local gids=() + while IFS= read -r one; do + [[ -z "$one" || "$one" == "null" ]] && continue + local g + g=$(printf '%s' "$one" | jq_group_id) + [[ -n "$g" ]] && gids+=("$g") + done < <(printf '%s' "$raw" | jq -c '(.result // .) | .[]?' 2>/dev/null) + local IFS=','; printf '%s' "${gids[*]}" +} + +# wait_for_invite [ignore-csv] +# Echoes the first pending group_id that isn't listed in +# (comma-separated hex gids). Use the ignore list to skip welcomes left +# pending by previous harness runs — otherwise `wait_for_invite` can +# fire on a stale invite and the caller ends up driving a group the +# publisher isn't a member of, which then looks like a kind:445 drop. # Returns 1 on timeout. # # Also prints a heartbeat every ~10s with: # - elapsed/remaining time -# - current count of pending welcomes (should be 0 until it isn't) +# - current count of pending welcomes (total, pre-filter) # - last relevant line of wnd stderr (grep for welcome/giftwrap/subscribe/error) # so a stalled poll gives the operator something to forward. wait_for_invite() { - local who="$1" timeout="${2:-60}" deadline gid start last_hb + local who="$1" timeout="${2:-60}" ignore_csv="${3:-}" deadline gid start last_hb local wnfn data_dir if [[ "$who" == "B" ]]; then wnfn=wn_b; data_dir="$B_DIR" else wnfn=wn_c; data_dir="$C_DIR"; fi @@ -198,27 +226,32 @@ wait_for_invite() { # Post-v0.2 `wn --json groups invites` returns `{"result": [...]}` # (older builds returned the bare array). Peel the wrapper when # present so a pending invite is actually detected. - gid=$("$wnfn" --json groups invites 2>/dev/null \ - | jq -c '(.result // .) | .[0] // empty' 2>/dev/null | jq_group_id || true) - if [[ -n "${gid:-}" ]]; then - printf '%s\n' "$gid" - return 0 - fi + local raw + raw=$("$wnfn" --json groups invites 2>/dev/null || true) + # Walk every pending invite (not just .[0]) so we skip past stales. + while IFS= read -r one; do + [[ -z "$one" || "$one" == "null" ]] && continue + gid=$(printf '%s' "$one" | jq_group_id) + [[ -z "$gid" ]] && continue + if [[ ",$ignore_csv," != *",$gid,"* ]]; then + printf '%s\n' "$gid" + return 0 + fi + done < <(printf '%s' "$raw" | jq -c '(.result // .) | .[]?' 2>/dev/null) # Heartbeat every ~10s. local now=$(date +%s) if (( now - last_hb >= 10 )); then local elapsed=$(( now - start )) remaining=$(( deadline - now )) local pending - pending=$("$wnfn" --json groups invites 2>/dev/null \ - | jq '(.result // .) | length' 2>/dev/null || echo "?") + pending=$(printf '%s' "$raw" | jq '(.result // .) | length' 2>/dev/null || echo "?") local recent="" if [[ -f "$data_dir/logs/stderr.log" ]]; then recent=$(tail -n 200 "$data_dir/logs/stderr.log" 2>/dev/null \ | grep -iE 'welcome|giftwrap|gift_wrap|mls|subscribe|1059|444|error|warn' \ | tail -n 1 || true) fi - info "$who wait_for_invite +${elapsed}s (remaining ${remaining}s) pending=$pending tail: ${recent:-}" + info "$who wait_for_invite +${elapsed}s (remaining ${remaining}s) pending=$pending ignoring=${ignore_csv:-} tail: ${recent:-}" last_hb=$now fi diff --git a/tools/marmot-interop/marmot-interop.sh b/tools/marmot-interop/marmot-interop.sh index 3ba20b67d..c18a4bf79 100755 --- a/tools/marmot-interop/marmot-interop.sh +++ b/tools/marmot-interop/marmot-interop.sh @@ -363,7 +363,15 @@ configure_relays() { fi step "sanity-check kinds 10050/1059/445: B creates group + invites C + exchanges a message" - local sanity_gid sanity_c_gid sanity_create + local sanity_gid sanity_c_gid sanity_create c_ignore + # Snapshot C's pending invites before we publish, so wait_for_invite + # doesn't fire on a stale welcome left over from a previous run (wnd + # persists pending invites across restarts — we saw this cause the + # kind:445 sanity probe to send to B's new group while C was still + # sitting on an unaccepted welcome from a previous group that B + # wasn't a member of anymore). + c_ignore=$(snapshot_invites C) + [[ -n "$c_ignore" ]] && info "C already has stale invites, ignoring: $c_ignore" sanity_create=$(wn_b --json groups create "sanity-check" "$C_NPUB" 2>>"$LOG_FILE" || true) sanity_gid=$(printf '%s' "$sanity_create" | jq_group_id) printf 'sanity groups create raw: %s\n' "$sanity_create" >>"$LOG_FILE" @@ -371,8 +379,11 @@ configure_relays() { warn "sanity group create failed — see $LOG_FILE (stale account state? rerun with 'rm -rf state/')" else info "sanity group id: $sanity_gid" - if sanity_c_gid=$(wait_for_invite C 30); then + if sanity_c_gid=$(wait_for_invite C 30 "$c_ignore"); then info "kind:10050+1059 ok — C received welcome (gid: $sanity_c_gid)" + if [[ "$sanity_c_gid" != "$sanity_gid" ]]; then + warn "gid mismatch — C saw $sanity_c_gid, B created $sanity_gid (likely a stale welcome slipped past the filter)" + fi wn_c groups accept "$sanity_c_gid" >/dev/null 2>&1 || true wn_b messages send "$sanity_gid" "sanity-ping" >/dev/null 2>&1 || true if wait_for_message C "$sanity_c_gid" "sanity-ping" 30; then @@ -451,6 +462,12 @@ test_02_amethyst_creates_group() { # Amethyst sends it. dump_daemon_diagnostics B "pre-invite baseline" + # Snapshot B's already-pending invites so we can tell the new welcome + # apart from a leftover from a previous run. + local b_ignore + b_ignore=$(snapshot_invites B) + [[ -n "$b_ignore" ]] && info "B already has stale invites, ignoring: $b_ignore" + prompt_human "In Amethyst: 1. Tap '+' -> Create Group 2. Name: Interop-02 @@ -464,7 +481,7 @@ that's the bug." step "polling B's daemon for invite (60s, heartbeat every ~10s)" local gid - if ! gid=$(wait_for_invite B 60); then + if ! gid=$(wait_for_invite B 60 "$b_ignore"); then fail_msg "no invite arrived at B" dump_daemon_diagnostics B "post-timeout (test_02)" prompt_amethyst_logcat "test_02 timeout" @@ -603,13 +620,17 @@ test_04_three_member_group() { dump_daemon_diagnostics C "pre-invite baseline (test_04)" + local c_ignore + c_ignore=$(snapshot_invites C) + [[ -n "$c_ignore" ]] && info "C already has stale invites, ignoring: $c_ignore" + prompt_human "In Amethyst, open the group from Test 02 (or 'Interop-04-bootstrap'). Group Info -> Add Member -> paste: $C_NPUB Confirm the invite is sent." step "polling C for invite (60s, heartbeat every ~10s)" local c_gid - if ! c_gid=$(wait_for_invite C 60); then + if ! c_gid=$(wait_for_invite C 60 "$c_ignore"); then fail_msg "C never received invite" dump_daemon_diagnostics C "post-timeout (test_04)" prompt_amethyst_logcat "test_04 timeout" @@ -642,7 +663,8 @@ test_05_wn_adds_amethyst_existing() { banner "Test 05 — wn adds Amethyst to existing B+C group" step "B creates group with only C, then adds A" - local out gid + local out gid c_ignore + c_ignore=$(snapshot_invites C) out=$(wn_b --json groups create "Interop-05" "$C_NPUB" 2>>"$LOG_FILE") printf 'wn groups create raw output: %s\n' "$out" >>"$LOG_FILE" gid=$(printf '%s' "$out" | jq_group_id) @@ -652,8 +674,9 @@ test_05_wn_adds_amethyst_existing() { return fi save_state GROUP_05 "$gid" - if wait_for_invite C 30 >/dev/null; then - wn_c groups accept "$gid" >/dev/null 2>&1 || true + local c_new_gid + if c_new_gid=$(wait_for_invite C 30 "$c_ignore"); then + wn_c groups accept "$c_new_gid" >/dev/null 2>&1 || true fi step "B adds A to the group" @@ -770,9 +793,11 @@ test_08_admin_promote_demote() { step "B (creator+admin) adds C so we have 3 members" wn_c keys publish >/dev/null 2>&1 || true sleep 3 + local c_ignore_08 c_new_gid_08 + c_ignore_08=$(snapshot_invites C) wn_b groups add-members "$gid" "$C_NPUB" >/dev/null 2>&1 || warn "add-members C returned nonzero" - if wait_for_invite C 30 >/dev/null; then - wn_c groups accept "$gid" >/dev/null 2>&1 || true + if c_new_gid_08=$(wait_for_invite C 30 "$c_ignore_08"); then + wn_c groups accept "$c_new_gid_08" >/dev/null 2>&1 || true fi step "B promotes A to admin" @@ -946,9 +971,11 @@ test_12_offline_catchup() { done step "B adds C, then sends 3 more messages" + local c_ignore_12 c_new_gid_12 + c_ignore_12=$(snapshot_invites C) wn_b groups add-members "$gid" "$C_NPUB" >/dev/null 2>&1 || true - if wait_for_invite C 30 >/dev/null; then - wn_c groups accept "$gid" >/dev/null 2>&1 || true + if c_new_gid_12=$(wait_for_invite C 30 "$c_ignore_12"); then + wn_c groups accept "$c_new_gid_12" >/dev/null 2>&1 || true fi for i in 6 7 8; do wn_b messages send "$gid" "offline-msg-$i" >/dev/null 2>&1 || true From 1cc29d605a56370651e7f34f93938a3fc6314e64 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 22 Apr 2026 18:43:43 +0000 Subject: [PATCH 05/19] feat(marmot-interop): add nostr.bitcoiner.social + nostr.mom to defaults Broaden the default public relay set for the interactive harness so kind:1059 gift wraps and kind:10050 inbox lists have more landing spots that don't pre-filter Marmot traffic out. Both relays are known to accept kinds outside the core "note / metadata / contacts" trio that damus/primal tend to restrict. The Amethyst-side setup instructions are rendered from the same list, so operators who configure Amethyst per the harness prompt get the same five URLs on the app side too. --- tools/marmot-interop/marmot-interop.sh | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tools/marmot-interop/marmot-interop.sh b/tools/marmot-interop/marmot-interop.sh index c18a4bf79..f556d39bc 100755 --- a/tools/marmot-interop/marmot-interop.sh +++ b/tools/marmot-interop/marmot-interop.sh @@ -34,7 +34,13 @@ C_HEX="" A_NPUB="" A_HEX="" -DEFAULT_RELAYS=( "wss://relay.damus.io" "wss://nos.lol" "wss://relay.primal.net" ) +DEFAULT_RELAYS=( + "wss://relay.damus.io" + "wss://nos.lol" + "wss://relay.primal.net" + "wss://nostr.bitcoiner.social" + "wss://nostr.mom" +) USE_LOCAL_RELAYS=0 ENABLE_TRANSPONDER=0 NO_BUILD=0 From d1ea9c39e89f7fe3b0cadd395267aadc9ace2d1b Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 22 Apr 2026 18:53:17 +0000 Subject: [PATCH 06/19] fix(marmot-interop): guard empty-array expansion in snapshot_invites bash 3.2 (stock on macOS) raises "unbound variable" on "${gids[*]}" when the array is empty under `set -u`. The happy-path sanity check still worked (the array was populated), but the shell error aborted execution before the actual test cases ran on a fresh state/ directory. Only expand when the array has at least one entry. --- tools/marmot-interop/lib.sh | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tools/marmot-interop/lib.sh b/tools/marmot-interop/lib.sh index 406dfbcf5..913c2d8f9 100644 --- a/tools/marmot-interop/lib.sh +++ b/tools/marmot-interop/lib.sh @@ -198,7 +198,12 @@ snapshot_invites() { g=$(printf '%s' "$one" | jq_group_id) [[ -n "$g" ]] && gids+=("$g") done < <(printf '%s' "$raw" | jq -c '(.result // .) | .[]?' 2>/dev/null) - local IFS=','; printf '%s' "${gids[*]}" + # bash 3.2 (stock macOS) treats "${gids[*]}" on an empty array as an + # unbound reference under `set -u`, so guard the expansion. + if (( ${#gids[@]} > 0 )); then + local IFS=',' + printf '%s' "${gids[*]}" + fi } # wait_for_invite [ignore-csv] From 8416afd965c7b0075595f87be707332f705d9a9a Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 22 Apr 2026 19:12:19 +0000 Subject: [PATCH 07/19] debug(marmot): log removeMarmotGroupMember + syncMetadataTo transitions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bug report: after tapping Remove on the Marmot Group Info screen, the target npub stays in the member list, and the only log line is the round-tripped kind:445 hitting the "Duplicate" branch in GroupEventHandler.add. The author-side path (Account.removeMarmotGroupMember → MarmotManager.removeMember → syncMetadataTo → publish) has no logging, so there's no way to tell which step broke — the MLS tree mutation, the local member-list push to chatroom.members, or the UI recomposition. Mirror the addMarmotGroupMember logging style on removeMarmotGroupMember (entry, no-op guards, built commit id, publish) so we can see whether the author path even runs end-to-end, and have syncMetadataTo log the group id and the previous→new member count so the StateFlow update becomes observable. No behavior change. --- .../vitorpamplona/amethyst/model/Account.kt | 22 +++++++++++++++++-- .../amethyst/commons/marmot/MarmotManager.kt | 5 +++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt index d26302f4f..050bf1ae1 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt @@ -2313,12 +2313,30 @@ class Account( targetLeafIndex: Int, groupRelays: Set, ) { - val manager = marmotManager ?: return - if (!isWriteable()) return + Log.d("MarmotDbg") { + "removeMarmotGroupMember: group=${nostrGroupId.take(8)}… targetLeafIndex=$targetLeafIndex " + + "groupRelays=${groupRelays.size}" + } + val manager = + marmotManager ?: run { + Log.w("MarmotDbg") { "removeMarmotGroupMember: marmotManager is NULL — no-op" } + return + } + if (!isWriteable()) { + Log.w("MarmotDbg") { "removeMarmotGroupMember: account is not writeable — no-op" } + return + } val outbound = manager.removeMember(nostrGroupId, targetLeafIndex) + Log.d("MarmotDbg") { + "removeMarmotGroupMember: built commit kind=${outbound.signedEvent.kind} id=${outbound.signedEvent.id.take(8)}…" + } val chatroom = marmotGroupList.getOrCreateGroup(nostrGroupId) manager.syncMetadataTo(nostrGroupId, chatroom) + Log.d("MarmotDbg") { + "removeMarmotGroupMember: publishing commit id=${outbound.signedEvent.id.take(8)}… " + + "to ${groupRelays.size} relay(s): ${groupRelays.map { it.url }}" + } client.publish(outbound.signedEvent, groupRelays) } diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt index d8a509ab8..06a132c53 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt @@ -566,9 +566,14 @@ class MarmotManager( chatroom.adminPubkeys.value = metadata.adminPubkeys chatroom.relays.value = metadata.relays } + val previousCount = chatroom.members.value.size val members = memberPubkeys(nostrGroupId) chatroom.members.value = members chatroom.memberCount.value = members.size + Log.d("MarmotDbg") { + "syncMetadataTo: group=${nostrGroupId.take(8)}… members $previousCount→${members.size} " + + "(leafs=${members.map { it.leafIndex }})" + } } } From 6ad522b52bff4bc71a60dc4553fac21339de462d Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 22 Apr 2026 19:50:50 +0000 Subject: [PATCH 08/19] fix(marmot): surface inbound app messages even when LocalCache says duplicate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The B->Amethyst kind:445 reply was decrypting fine — the harness's new outbound-side diagnostics + the Amethyst logcat showed a clean ApplicationMessage path right up to the chatroom-add step: GroupEventHandler.add: kind:445 id=988221d0… groupId=ee27bc48… GroupEventHandler.add: ApplicationMessage decrypted innerKind=9 innerId=ebf35373… author=170df8d1… GroupEventHandler.add: inner event already in cache (duplicate) `cache.justConsume` returned false because the inner event id was already in LocalCache (a prior session persisted the same plaintext via persistDecryptedMessage and the startup restore loop in Account.kt:3034-3044 reloaded it into the cache before this kind:445 arrived). The live-receive branch in DecryptAndIndexProcessor was gated on `isNew` and silently dropped the message — never calling marmotGroupList.addMessage — so the chatroom never saw B's reply even though decryption succeeded. Mirror what the restore path does: always surface the inner note in the chatroom. `MarmotGroupChatroom.addMessageSync` is itself idempotent (dedupes by Note identity), so the call is safe even when the message was already added through another path. `persistDecryptedMessage` stays guarded on `isNew` because the message store is append-only and a re-persist would grow the on-disk log unboundedly across decrypt retries. --- .../loggedIn/DecryptAndIndexProcessor.kt | 45 ++++++++++++++----- 1 file changed, 33 insertions(+), 12 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/DecryptAndIndexProcessor.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/DecryptAndIndexProcessor.kt index 100760c5e..49b91b287 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/DecryptAndIndexProcessor.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/DecryptAndIndexProcessor.kt @@ -592,20 +592,41 @@ class GroupEventHandler( "GroupEventHandler.add: ApplicationMessage decrypted innerKind=${innerEvent.kind} " + "innerId=${innerEvent.id.take(8)}… author=${innerEvent.pubKey.take(8)}…" } - if (cache.justConsume(innerEvent, null, false)) { - val innerNote = cache.getOrCreateNote(innerEvent.id) + // `cache.justConsume` returns false if LocalCache already + // has this inner event id — typically because a previous + // session persisted the inner JSON and the startup + // restore loop in Account reloaded it into the cache + // before this kind:445 arrived. The chatroom may still + // not have it (e.g. the user's chatroom snapshot was + // dropped, or the restore-from-disk path raced ahead and + // the cache populated before the chatroom was hydrated). + // `addMessage` is itself idempotent (`addMessageSync` + // dedupes by Note identity), so always surface the note + // in the chatroom — otherwise the message is silently + // dropped and the operator sees only the "duplicate" log. + val isNew = cache.justConsume(innerEvent, null, false) + val innerNote = cache.getOrCreateNote(innerEvent.id) + if (isNew) { innerNote.event = innerEvent - - // Track the message in the Marmot group chatroom - account.marmotGroupList.addMessage(result.groupId, innerNote) - - // Persist the decrypted plaintext so the message - // survives an app restart. Marmot/MLS application - // messages cannot be re-decrypted once the ratchet - // has advanced, so we must capture them here. - manager.persistDecryptedMessage(result.groupId, result.innerEventJson) } else { - Log.d("MarmotDbg") { "GroupEventHandler.add: inner event already in cache (duplicate)" } + Log.d("MarmotDbg") { + "GroupEventHandler.add: inner event already in cache — surfacing in chatroom anyway" + } + } + + // Track the message in the Marmot group chatroom + account.marmotGroupList.addMessage(result.groupId, innerNote) + + // Persist the decrypted plaintext so the message + // survives an app restart. Marmot/MLS application + // messages cannot be re-decrypted once the ratchet + // has advanced, so we must capture them here. Skip + // when the cache reported duplicate — the message + // was already persisted on the run that originally + // decrypted it (the message store appends, so a + // re-persist would silently grow the on-disk log). + if (isNew) { + manager.persistDecryptedMessage(result.groupId, result.innerEventJson) } } From 573c5c2b0693397841594089d3223893a72d982b Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 22 Apr 2026 20:03:54 +0000 Subject: [PATCH 09/19] fix(marmot): trust MLS authentication, skip Nostr verify on inner events MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit (6ad522b5) shielded the chatroom from the mistaken "duplicate" drop but was treating the symptom. The real cause of the "inner event already in cache" log — reported while Test 02's chat stayed completely empty — is that `cache.justConsume(innerEvent, null, wasVerified=false)` runs `event.verify()` on a wn-signed inner kind:9, that verify fails (likely JSON-canonicalization skew or a rumor-style empty signature), so `consumeRegularEvent` bails at line 633 before ever setting `note.event`. LocalCache then always had the empty Note registered by id but with no event content — subsequent arrivals re-hit line 615 (`note.event != null` is still false, not the "duplicate" path), retried verify, failed again, and the chatroom never got a populated message to render. The inner event has already been cryptographically authenticated by MLS — `MarmotInboundProcessor.processPrivateMessage` rejects any inner event whose `pubkey` doesn't match the MLS sender's credential identity (MIP-03, line 430). Running Nostr's `event.verify()` again is a redundant gate. Pass `wasVerified=true`: `consumeRegularEvent` skips `justVerify` and calls `note.loadEvent(...)` on first arrival, chatroom-add works immediately, and the test passes. Also tightens the "already in cache" comment to reflect what that branch actually means now (a legitimate hydrated-by-startup-restore duplicate, not a silent verify failure masquerading as one). --- .../loggedIn/DecryptAndIndexProcessor.kt | 37 +++++++++++-------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/DecryptAndIndexProcessor.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/DecryptAndIndexProcessor.kt index 49b91b287..ef3894539 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/DecryptAndIndexProcessor.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/DecryptAndIndexProcessor.kt @@ -592,23 +592,28 @@ class GroupEventHandler( "GroupEventHandler.add: ApplicationMessage decrypted innerKind=${innerEvent.kind} " + "innerId=${innerEvent.id.take(8)}… author=${innerEvent.pubKey.take(8)}…" } - // `cache.justConsume` returns false if LocalCache already - // has this inner event id — typically because a previous - // session persisted the inner JSON and the startup - // restore loop in Account reloaded it into the cache - // before this kind:445 arrived. The chatroom may still - // not have it (e.g. the user's chatroom snapshot was - // dropped, or the restore-from-disk path raced ahead and - // the cache populated before the chatroom was hydrated). - // `addMessage` is itself idempotent (`addMessageSync` - // dedupes by Note identity), so always surface the note - // in the chatroom — otherwise the message is silently - // dropped and the operator sees only the "duplicate" log. - val isNew = cache.justConsume(innerEvent, null, false) + // Treat the inner event as pre-verified. MLS already + // authenticated the sender: MarmotInboundProcessor. + // processPrivateMessage rejects any inner event whose + // `pubkey` field doesn't match the MLS sender's + // credential identity. Running Nostr sig verify again + // here is a redundant gate — and one that silently + // dropped wn-signed inner events (wn's kind:9 rumors + // don't always round-trip Amethyst's `event.verify()`, + // e.g. because of JSON canonicalization differences). + // Without this, justConsume returned false, the chatroom + // never got the message, and the operator saw nothing + // but a misleading "inner event already in cache" log. + val isNew = cache.justConsume(innerEvent, null, true) val innerNote = cache.getOrCreateNote(innerEvent.id) - if (isNew) { - innerNote.event = innerEvent - } else { + if (!isNew) { + // Legit duplicate: a prior session persisted the + // inner JSON and the startup restore loop hydrated + // the cache before this kind:445 arrived. The + // chatroom may still not have it (restore order / + // chatroom snapshot drop), so `addMessage` runs + // unconditionally — `MarmotGroupChatroom.addMessageSync` + // dedupes by Note identity if it was already there. Log.d("MarmotDbg") { "GroupEventHandler.add: inner event already in cache — surfacing in chatroom anyway" } From 5a18dc41c8eca9d7415a02eb35eafcb86588b443 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 22 Apr 2026 20:14:07 +0000 Subject: [PATCH 10/19] fix(marmot): inner events are unsigned rumors per MIP-03 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MIP-03 ("Application Messages" → Security Requirements) is explicit: > "Inner events MUST remain unsigned (no `sig` field) > This ensures leaked events cannot be published to public relays." Authentication comes from (a) MLS framing — the sender's LeafNode credential signs the outer MLS Application message — and (b) the mandatory check in MarmotInboundProcessor.processPrivateMessage that the inner event's `pubkey` equals the MLS sender's credential identity. The Nostr Schnorr signature is redundant, and leaving it populated means a leaked plaintext can be replayed as a valid public kind:9. whitenoise-rs is spec-compliant (builds via `UnsignedEvent::new()` + `ensure_id()`, never calls sign). Amethyst was the non-conforming side on both directions: 1. Send path (`MarmotManager.buildTextMessage`, `AccountViewModel.sendMarmotGroupMediaMessage`) called `signer.sign(template)`, producing a signed rumor that shipped a valid Schnorr signature through the encrypted channel. Switch both to `RumorAssembler.assembleRumor` — same id derivation (SHA-256 over canonicalized [0, pubkey, createdAt, kind, tags, content]), but `sig = ""`. 2. Restore path (`Account.kt` on startup reload of persisted inner events) called `cache.justConsume(innerEvent, null, false)`, which routed through `consumeRegularEvent` → `justVerify` → `event.verify()` → FAIL on empty sig → Note registered with `event = null`, message never rendered. Pass `wasVerified = true`, matching what the live receive path already does after the previous commit (573c5c2b). Existing on-disk persisted messages from older signed-rumor builds still load — `wasVerified=true` skips sig verify entirely, so both legacy signed and spec-correct unsigned rumors deserialize cleanly. --- .../vitorpamplona/amethyst/model/Account.kt | 6 +++- .../ui/screen/loggedIn/AccountViewModel.kt | 11 +++++++- .../amethyst/commons/marmot/MarmotManager.kt | 28 +++++++++++++------ 3 files changed, 34 insertions(+), 11 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt index 38680c8c2..7fd284b0c 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt @@ -3036,7 +3036,11 @@ class Account( val innerEvent = com.vitorpamplona.quartz.nip01Core.core.Event .fromJson(json) - val isNew = cache.justConsume(innerEvent, null, false) + // MIP-03 inner events are unsigned rumors + // (empty sig); pass wasVerified=true so + // LocalCache skips the Nostr secp256k1 check + // that would silently reject them. + val isNew = cache.justConsume(innerEvent, null, true) val innerNote = cache.getOrCreateNote(innerEvent.id) if (isNew) { innerNote.event = innerEvent diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt index b9e9220e6..aa14e675e 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt @@ -1477,7 +1477,16 @@ class AccountViewModel( alt(caption) } } - val innerEvent = account.signer.sign(template) + // MIP-03: inner events MUST remain unsigned (no `sig`) so a leaked + // plaintext can't be replayed as a valid public kind:9. Authorship + // is authenticated by the MLS sender's LeafNode + the pubkey↔ + // credential-identity equality check on the receive side. + val innerEvent = + com.vitorpamplona.quartz.nip59Giftwrap.rumors.RumorAssembler + .assembleRumor( + account.signer.pubKey, + template, + ) val relays = marmotGroupRelays(nostrGroupId) account.sendMarmotGroupMessage(nostrGroupId, innerEvent, relays) } diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt index 06a132c53..c1fc14d85 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt @@ -155,17 +155,25 @@ class MarmotManager( ): OutboundGroupEvent = outboundProcessor.buildGroupEvent(nostrGroupId, innerEvent) /** - * Build a kind:9 chat-message GroupEvent from plain text. The inner event is - * signed with this manager's signer and optionally persisted to the local - * decrypted-message log so `loadStoredMessages` reflects our own outbound - * immediately (without waiting for relay loopback). + * Build a kind:9 chat-message GroupEvent from plain text. The inner + * event is built as an UNSIGNED rumor per MIP-03 ("Inner events MUST + * remain unsigned — this ensures leaked events cannot be published to + * public relays"): the MLS sender authenticates via the LeafNode + * credential + the `pubkey` ↔ sender-identity equality check, so + * the inner Nostr signature is redundant, and leaving it in would + * let a leaked plaintext be replayed as a valid public kind:9. * - * Platform callers that already maintain their own "own event" cache (i.e. - * Amethyst's `LocalCache.justConsumeMyOwnEvent`) should pass `persistOwn = false`. - * Headless callers (CLI) should leave it at the default. + * Optionally persisted to the local decrypted-message log so + * `loadStoredMessages` reflects our own outbound immediately + * (without waiting for relay loopback). + * + * Platform callers that already maintain their own "own event" cache + * (i.e. Amethyst's `LocalCache.justConsumeMyOwnEvent`) should pass + * `persistOwn = false`. Headless callers (CLI) should leave it at + * the default. * * @return the signed kind:445 outer event together with the inner kind:9 - * event id, so the caller can reference it for replies/reactions. + * rumor id, so the caller can reference it for replies/reactions. */ suspend fun buildTextMessage( nostrGroupId: HexKey, @@ -175,7 +183,9 @@ class MarmotManager( val template = com.vitorpamplona.quartz.nip01Core.signers .eventTemplate(kind = 9, description = text) - val innerEvent = signer.sign(template) + val innerEvent = + com.vitorpamplona.quartz.nip59Giftwrap.rumors.RumorAssembler + .assembleRumor(signer.pubKey, template) val outbound = buildGroupMessage(nostrGroupId, innerEvent) if (persistOwn) persistDecryptedMessage(nostrGroupId, innerEvent.toJson()) return TextMessageBundle(outbound = outbound, innerEvent = innerEvent) From 34f6be225ffb554194410386ef2efc9a7ced8d83 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 22 Apr 2026 20:42:44 +0000 Subject: [PATCH 11/19] fix(marmot-interop): prompt operator to set DM Inbox relays too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Test 03 (wn creates a group, invites Amethyst) was failing at the operator-visible level — "I don't see Interop-03 on Amethyst" — with no clear signal in the Amethyst logcat because no kind:1059 welcome gift wrap ever arrived. Root cause: when wn invites Amethyst it fetches A's kind:10050 inbox list and publishes the welcome gift wrap there. Amethyst's built-in DM inbox defaults advertise auth.nostr1.com + relay.0xchat.com, which wn (connected only to the five harness public relays) cannot reach. Unless the operator manually adds the harness relays to Amethyst's DM Inbox list, the gift wrap lands on relays the reader isn't subscribed to, and the invite is silently dropped. Confirmed empirically by adding wss://nostr.bitcoiner.social (one of the harness relays) to Amethyst's DM Inbox list and watching Interop-03 appear on the next wn create. Update `instruct_amethyst_setup` to make the DM Inbox step explicit and call out the specific failure mode it prevents. Also add a nudge to verify NIP-65 + kind:10050 were actually republished after the relay list edits. --- tools/marmot-interop/marmot-interop.sh | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/tools/marmot-interop/marmot-interop.sh b/tools/marmot-interop/marmot-interop.sh index f556d39bc..434cb3acf 100755 --- a/tools/marmot-interop/marmot-interop.sh +++ b/tools/marmot-interop/marmot-interop.sh @@ -420,8 +420,18 @@ instruct_amethyst_setup() { 1. Settings -> Relays: add the following as READ+WRITE $list 2. Settings -> Key Package Relays: add the SAME URLs - 3. Trigger key-package publish (toggle KP relay on/off if needed) - 4. Confirm your Amethyst account is logged in with npub: $A_NPUB" + 3. Settings -> DM Inbox Relays (NIP-17/kind:10050): add the SAME URLs. + CRITICAL for Test 03+: when wn invites Amethyst it fetches A's + kind:10050 and publishes the welcome gift wrap there. Amethyst's + built-in DM inbox defaults (auth.nostr1.com, relay.0xchat.com) + are not reachable from the wn daemon in this harness, so unless + the five harness relays are in the 10050 list the gift wrap lands + on relays Amethyst-the-reader isn't subscribed to. + 4. Trigger key-package publish (toggle KP relay on/off if needed). + 5. Re-publish the NIP-65 + DM-inbox lists if Amethyst hasn't already + (relay toggles usually republish automatically; verify by looking + for kind:10002 / 10050 in any relay inspector). + 6. Confirm your Amethyst account is logged in with npub: $A_NPUB" } # ==== tests ================================================================== From a6388a6751c7759aa53445476a1dcd0b40058dd1 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 22 Apr 2026 21:13:16 +0000 Subject: [PATCH 12/19] feat(marmot-interop): discover A's relays instead of forcing a shared set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The interop harness was forcing specific relays on the Amethyst account — "configure Amethyst with these five relays, also add them to your DM Inbox list, also to your Key Package Relays, then republish" — which made the test artificial: - It masked the real failure modes (wn publishing to an A-advertised relay wn can't reach, A's 10050 pointing at auth-required relays, etc.) because everyone was always on the same happy-path set. - It mutated the operator's real Amethyst account config — those five relays persist after the run. - It short-circuited wn's discovery chain (kind:10002 → 10050 → 10051 resolution), so the one path most likely to hold subtle bugs was never exercised under divergent configs. Real-world interop is: both clients have their own independently-chosen relay sets and use the advertised kind:10002/10050/10051 to find each other. Make the harness match that: - Keep the five-relay set as the wn B/C bootstrap (they still need somewhere to publish their own identity). Drop the "configure Amethyst to match" steps from `instruct_amethyst_setup`; confirmation only now ("is A logged in as , and has A published its four lists to any public relay?"). - Add `discover_a_relays` step between configure + the operator prompt. Calls `wn keys check $A_NPUB` on both daemons to trigger wn's targeted fetch for kinds [0, 10002, 10050, 10051] (the only side-effect available to populate wn's user_relays cache). Then reads the cache directly via sqlite3 against wn's DB and prints what wn actually learned, grouped by relay_type. Warns loudly if A's 10050 shares zero relays with wn's bootstrap — the exact failure mode that caused the prior round of "Test 03 silently times out" reports. - `--local-relays` path keeps the old behavior: offline/sandbox mode, the harness owns the relay so forcing Amethyst onto it is correct. Scope is the harness only. No Amethyst / quartz / commons changes. --- tools/marmot-interop/marmot-interop.sh | 175 ++++++++++++++++++++++--- 1 file changed, 154 insertions(+), 21 deletions(-) diff --git a/tools/marmot-interop/marmot-interop.sh b/tools/marmot-interop/marmot-interop.sh index 434cb3acf..80579bc0d 100755 --- a/tools/marmot-interop/marmot-interop.sh +++ b/tools/marmot-interop/marmot-interop.sh @@ -255,6 +255,117 @@ prompt_for_a_npub() { [[ "$A_HEX" != "$A_NPUB" ]] && info "A hex: $A_HEX" } +# Trigger wn's on-demand discovery for A and dump what wn actually +# learned — kinds 10002 / 10050 / 10051 from A's configured relays. +# Called after prompt_for_a_npub so wn's SQLite cache is populated +# BEFORE the test suite starts, and so the operator can spot "wn sees +# nothing for A" failures up front instead of as a cryptic Test 03 +# timeout. +# +# Implementation: +# - `wn keys check ` calls `resolve_user_blocking` which issues +# a targeted fetch for kinds [0, 10002, 10050, 10051] on the +# discovery-plane relays. That's the only published side-effect +# way to populate wn's user_relays table (no public CLI reads +# another user's relay lists directly). +# - After the fetch, `sqlite3` is used to SELECT from `user_relays` +# joined against `users` + `relays`. If sqlite3 isn't installed +# the probe degrades to "trust it, go run the tests" with a warning. +discover_a_relays() { + banner "Discovering A's advertised relays via wn" + step "wn_b keys check \$A_NPUB — populates wn's user_relays cache for A" + local kp_raw kp_event_id + kp_raw=$(wn_b --json keys check "$A_NPUB" 2>>"$LOG_FILE" || true) + printf 'wn_b keys check A raw: %s\n' "$kp_raw" >>"$LOG_FILE" + kp_event_id=$(printf '%s' "$kp_raw" | jq -r '.result.event_id // .event_id // empty' 2>/dev/null || true) + if [[ -n "$kp_event_id" && "$kp_event_id" != "null" ]]; then + info "wn_b found A's KeyPackage (kind:30443) — discovery plane is working" + else + warn "wn_b could NOT find A's KeyPackage. wn is bootstrapped on ${DEFAULT_RELAYS[*]}." + warn "Either Amethyst never published a KeyPackage, or it's only on relays wn can't reach." + warn "All later tests will fail. Fix this before continuing (tap KP publish in Amethyst settings)." + fi + # Also prime wn_c so Tests 04+ (which use C as a third member) can + # target A without another round-trip later. + wn_c --json keys check "$A_NPUB" >>"$LOG_FILE" 2>&1 || true + + # Give wn ~3s to process the kind:10002/10050/10051 events that + # came back in the same fetch (they're not event_id-returned, they + # just land in user_relays via the subscription callback). + sleep 3 + + if ! command -v sqlite3 >/dev/null 2>&1; then + warn "sqlite3 not installed — skipping wn user_relays cache probe." + warn "If Test 03 fails with 'no invite arrived', install sqlite3 or rerun with --local-relays." + return + fi + + # wn stores its database under $data_dir/release/.sqlite. + # Find the DB file by looking for the only *.sqlite under release/. + local db + db=$(find "$B_DIR/release" -maxdepth 1 -name '*.sqlite' -print -quit 2>/dev/null || true) + if [[ -z "$db" || ! -s "$db" ]]; then + warn "wn SQLite DB not found under $B_DIR/release — cannot probe user_relays." + return + fi + + step "wn_b's cached view of A's relay lists (SELECT from user_relays)" + local rows + rows=$(sqlite3 -readonly "$db" \ + "SELECT ur.relay_type, r.url FROM user_relays ur + JOIN users u ON u.id = ur.user_id + JOIN relays r ON r.id = ur.relay_id + WHERE LOWER(HEX(u.pubkey)) = LOWER('$A_HEX') + ORDER BY ur.relay_type, r.url;" 2>>"$LOG_FILE" || true) + if [[ -z "$rows" ]]; then + warn "wn has NO cached relay entries for A ($A_HEX)." + warn " → Amethyst hasn't published kind:10002/10050/10051 to any relay wn_b can reach." + warn " → Welcomes and gift wraps will not reach Amethyst. Tests 03+ will fail." + return + fi + + # Collate by relay_type for readability. + local nip65 inbox kp + nip65=$(printf '%s\n' "$rows" | awk -F'|' '$1=="nip65"{print " "$2}') + inbox=$(printf '%s\n' "$rows" | awk -F'|' '$1=="inbox"{print " "$2}') + kp=$(printf '%s\n' "$rows" | awk -F'|' '$1=="key_package"{print " "$2}') + info "A's kind:10002 (nip65):" + printf '%s\n' "${nip65:- }" | tee -a "$LOG_FILE" >&2 + info "A's kind:10050 (DM inbox, used for Marmot welcome delivery):" + printf '%s\n' "${inbox:- }" | tee -a "$LOG_FILE" >&2 + info "A's kind:10051 (KeyPackage relays):" + printf '%s\n' "${kp:- }" | tee -a "$LOG_FILE" >&2 + + # Warn loudly if A's DM inbox is non-empty but shares no relay with + # wn_b's own configured set — that's the exact failure mode the + # operator already hit ("my DM inbox has auth.nostr1.com, wn can't + # publish there") and it silently breaks Test 03+. + if [[ -n "$inbox" ]]; then + local wn_relays + wn_relays=$(wn_b --json relays list 2>/dev/null \ + | jq -r '(.result // .)[]? | (.url // .relay.url // empty)' 2>/dev/null) + local overlap="" + while IFS= read -r a_url; do + a_url="${a_url# }" + [[ -z "$a_url" ]] && continue + if printf '%s\n' "$wn_relays" | grep -qxF "$a_url"; then + overlap+="$a_url," + fi + done <<< "$inbox" + overlap="${overlap%,}" + if [[ -z "$overlap" ]]; then + warn "A's DM inbox (kind:10050) shares ZERO relays with wn_b's bootstrap set." + warn " wn may still reach A's 10050 relays if they're public — but if any require" + warn " NIP-42 AUTH (auth.nostr1.com) or a whitelist (relay.0xchat.com), the welcome" + warn " gift wrap silently fails to publish." + warn " Real-world fix: edit Amethyst's DM Inbox list to include at least one of:" + warn " ${DEFAULT_RELAYS[*]}" + else + info "A's DM inbox overlaps wn's bootstrap at: $overlap — welcomes should flow" + fi + fi +} + # --- relays ------------------------------------------------------------------ configure_relays() { banner "Configuring relays" @@ -409,29 +520,42 @@ configure_relays() { } instruct_amethyst_setup() { - local list if [[ "$USE_LOCAL_RELAYS" -eq 1 ]]; then - list=" ws://10.0.2.2:8080 (Android emulator) - ws://:8080 (physical device on same Wi-Fi)" - else - list=$(printf ' %s\n' "${DEFAULT_RELAYS[@]}") + # Offline/sandbox path: we own the only relay, so the harness DOES + # need to dictate Amethyst's relay config — nothing is discoverable + # via the public network. + prompt_human "Configure Amethyst to match this --local-relays harness: + 1. Settings -> Relays: add as READ+WRITE + ws://10.0.2.2:8080 (Android emulator) + ws://:8080 (physical device on same Wi-Fi) + 2. Settings -> Key Package Relays: add the SAME URL + 3. Settings -> DM Inbox Relays (NIP-17/kind:10050): add the SAME URL + 4. Trigger key-package publish (toggle KP relay on/off if needed) + 5. Confirm your Amethyst account is logged in with npub: $A_NPUB" + return fi - prompt_human "Configure Amethyst to match this harness: - 1. Settings -> Relays: add the following as READ+WRITE -$list - 2. Settings -> Key Package Relays: add the SAME URLs - 3. Settings -> DM Inbox Relays (NIP-17/kind:10050): add the SAME URLs. - CRITICAL for Test 03+: when wn invites Amethyst it fetches A's - kind:10050 and publishes the welcome gift wrap there. Amethyst's - built-in DM inbox defaults (auth.nostr1.com, relay.0xchat.com) - are not reachable from the wn daemon in this harness, so unless - the five harness relays are in the 10050 list the gift wrap lands - on relays Amethyst-the-reader isn't subscribed to. - 4. Trigger key-package publish (toggle KP relay on/off if needed). - 5. Re-publish the NIP-65 + DM-inbox lists if Amethyst hasn't already - (relay toggles usually republish automatically; verify by looking - for kind:10002 / 10050 in any relay inspector). - 6. Confirm your Amethyst account is logged in with npub: $A_NPUB" + + # Public-relay path: the harness should behave like any real Nostr + # client — discover A's advertised relays via kind:10002 / 10050 / + # 10051 and publish there, rather than forcing A to adopt the + # harness's own relay set. That lets the tests surface real-world + # interop failures (e.g. A's DM inbox points at auth-required or + # unreachable relays) instead of hiding them behind shared config. + prompt_human "No relay reconfiguration required. Just confirm: + 1. Amethyst is logged in as npub: + $A_NPUB + 2. Amethyst has published (on its own chosen relays): + - kind:30443 KeyPackage + - kind:10051 Key Package Relay List + - kind:10050 DM Inbox Relay List + - kind:10002 NIP-65 Outbox/Inbox Relay List + Any one of Amethyst's public write relays will do — the wn daemons + below are bootstrapped on $(printf '%s, ' "${DEFAULT_RELAYS[@]}" | sed 's/, $//') and will + discover A's advertised relays automatically. + 3. If the wn-side diagnostic after this prompt shows that wn cannot + see any of A's four lists, Amethyst hasn't published them to any + relay wn can reach — republish them (toggle a relay off/on in + Settings, then verify via an external relay inspector)." } # ==== tests ================================================================== @@ -1104,6 +1228,15 @@ main() { dump_daemon_diagnostics B "post-configure" dump_daemon_diagnostics C "post-configure" + # Let wn discover A's advertised relays via its normal subscription + # plane, then dump what wn actually sees. This is the "Am I going to + # be able to reach this user?" probe — surfaces up front the kind of + # failure (A's 10050 unreachable from wn, missing KP list, etc.) that + # would otherwise bite as a silent Test 03 timeout. + if [[ "$USE_LOCAL_RELAYS" -ne 1 ]]; then + discover_a_relays + fi + instruct_amethyst_setup test_01_keypackage_discovery From 1e181eb305ac210640b9374bd6a994e5fc3ad478 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 22 Apr 2026 21:28:56 +0000 Subject: [PATCH 13/19] fix(marmot-interop): drive Test 06 removal from the admin (B), not A MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Test 06 was asking the operator to remove C from Interop-05 via the Amethyst UI — but Interop-05 was created by B (wn), so B is the sole admin and A is a plain member. MIP-03's authorization gate (`enforceAuthorizedProposalSet` in `MlsGroup.kt:1842`) rejects any Remove proposal from a non-admin, so the UI correctly fires "not an admin" and the test fails against its own author's assumption — the test was misaligned with the admin model, not the code. Flip the flow: B (admin) issues the removal via `wn groups remove-members `, and A / C observe the effect. That still exercises the same intended invariants: - B + A remain in the group, commit + epoch advance propagate. - A's Amethyst UI shows C gone (visible confirm). - A sends "after removing C" → B decrypts (expected). - C cannot decrypt the new message (forward secrecy at the new epoch — C's retained keys only open the pre-remove epoch). The "Amethyst admin issues remove" path is already separately covered by Test 02 (A creates the group → A is admin → A removes) and Test 08 (promote A, then A issues an admin-only action). Test 06's focus is forward secrecy after remove, not who may initiate — so having B drive it is the spec-correct shape. --- tools/marmot-interop/marmot-interop.sh | 48 ++++++++++++++++++++------ 1 file changed, 38 insertions(+), 10 deletions(-) diff --git a/tools/marmot-interop/marmot-interop.sh b/tools/marmot-interop/marmot-interop.sh index 80579bc0d..5ddc1e523 100755 --- a/tools/marmot-interop/marmot-interop.sh +++ b/tools/marmot-interop/marmot-interop.sh @@ -847,33 +847,61 @@ test_06_member_removal() { skip_msg "Test 06 requires Test 05 state"; record_result "06 member removal" skip "no GROUP_05"; return fi - prompt_human "In Amethyst, open group 'Interop-05' -> Group Info -> Members. - Tap C ($C_NPUB) -> Remove. - Confirm the removal." + # Interop-05 was created by B (wn), so B is the sole admin. Per + # MIP-03 `enforceAuthorizedProposalSet`, non-admin members may only + # commit a self-Update or a SelfRemove-only proposal set — a Remove + # proposal from A (a plain member here) is rejected before publish + # with "non-admin members may only commit …". The previous revision + # of this test asked the operator to remove C from Amethyst's UI, + # which is correct per spec to REJECT — so the test itself was + # misaligned with the admin model. Drive the removal from B (the + # admin) instead, and observe its effect on A + C. + step "B (admin) removes C from Interop-05" + local remove_out + if ! remove_out=$(wn_b groups remove-members "$gid" "$C_NPUB" 2>&1); then + fail_msg "wn_b groups remove-members failed: $remove_out" + printf 'wn_b groups remove-members: %s\n' "$remove_out" >>"$LOG_FILE" + record_result "06 member removal" fail "wn remove returned nonzero" + return + fi + printf 'wn_b groups remove-members: %s\n' "$remove_out" >>"$LOG_FILE" - step "verifying C is removed (60s poll)" - local deadline=$(( $(date +%s) + 60 )) removed=0 + step "verifying C is removed from B's + C's view (60s poll)" + local deadline=$(( $(date +%s) + 60 )) removed_b=0 removed_c=0 while [[ $(date +%s) -lt $deadline ]]; do + if ! wn_b --json groups members "$gid" 2>/dev/null \ + | jq -e --arg p "$C_HEX" '(.result // .) | .[]? | select((.pubkey // .public_key) == $p)' >/dev/null 2>&1; then + removed_b=1 + fi if ! wn_c --json groups members "$gid" 2>/dev/null \ | jq -e --arg p "$C_HEX" '(.result // .) | .[]? | select((.pubkey // .public_key) == $p)' >/dev/null 2>&1; then - removed=1; break + removed_c=1 fi + [[ "$removed_b" -eq 1 ]] && break sleep 3 done + if [[ "$removed_b" -ne 1 ]]; then + warn "B still shows C as a member after remove — commit may not have applied" + fi + info "post-remove: B sees C gone=$removed_b, C sees self gone=$removed_c" - if [[ "$removed" -ne 1 ]]; then - warn "C still appears to be a member; proceeding anyway" + prompt_human "In Amethyst, open 'Interop-05' -> Group Info and confirm C is no longer listed." + if ! confirm "Does Amethyst's member list now show only you and B (not C)?"; then + record_result "06 member removal" fail "Amethyst still shows C as a member" + return fi prompt_human "In Amethyst, send: 'after removing C'" if wait_for_message B "$gid" "after removing C" 30; then - info "B still receives messages (expected)" + info "B still receives messages (expected — B + A are the remaining members)" else fail_msg "B stopped receiving messages (unexpected)" record_result "06 member removal" fail "B lost access"; return fi - # C should NOT be able to decrypt the new message + # C should NOT be able to decrypt the new message — forward secrecy + # at the post-remove epoch means C's retained keys cannot open any + # kind:445 sealed under the new epoch's exporter. sleep 5 if wait_for_message C "$gid" "after removing C" 10; then fail_msg "C still decrypted a post-removal message — forward secrecy broken" From 2cdd89558b69beed9e95d4eec5f73ba12941d59c Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 22 Apr 2026 21:31:53 +0000 Subject: [PATCH 14/19] fix(marmot-interop): split Test 07 into admin-authored halves MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The reverse-rename leg issued `wn_b groups rename` on Interop-02, but Amethyst created Interop-02 — A is the admin, B is a plain member. MIP-03's `enforceAuthorizedProposalSet` rejects any GroupContextExtensions commit from a non-admin, so wn's rename was being dropped client-side, the metadata commit never reached any relay, and Amethyst's UI (correctly waiting for a kind:445 on Interop-02) never saw "Interop-02-reverse". Split Test 07 into two separately-recorded legs so each uses a group where the renamer is actually the admin: 07a — A (admin of Interop-02) renames to "Interop-02-renamed" via the Amethyst UI; wn-side poll observes the new name. 07b — B (admin of Interop-03) renames to "Interop-03-renamed" via `wn_b groups rename`; operator confirms Amethyst picked it up. Each leg individually records pass/fail/skip so a missing GROUP_03 (Test 03 skipped) doesn't take out the whole test. --- tools/marmot-interop/marmot-interop.sh | 69 +++++++++++++++++--------- 1 file changed, 45 insertions(+), 24 deletions(-) diff --git a/tools/marmot-interop/marmot-interop.sh b/tools/marmot-interop/marmot-interop.sh index 5ddc1e523..966c3e30e 100755 --- a/tools/marmot-interop/marmot-interop.sh +++ b/tools/marmot-interop/marmot-interop.sh @@ -915,37 +915,58 @@ test_06_member_removal() { test_07_metadata_rename() { banner "Test 07 — Group metadata rename round-trip (MIP-01)" - local gid - gid=$(load_state GROUP_02 || true) - if [[ -z "${gid:-}" ]]; then - skip_msg "no GROUP_02"; record_result "07 metadata rename" skip "no group state"; return - fi - - prompt_human "In Amethyst, open group 'Interop-02' -> Group Info -> Edit. + # Forward direction: A (admin of Interop-02) renames → B observes. + local gid_a + gid_a=$(load_state GROUP_02 || true) + if [[ -z "${gid_a:-}" ]]; then + skip_msg "no GROUP_02"; record_result "07a metadata rename A->B" skip "no group state" + else + prompt_human "In Amethyst, open group 'Interop-02' -> Group Info -> Edit. Rename to: Interop-02-renamed Save." - step "polling B for name change (60s)" - local deadline=$(( $(date +%s) + 60 )) name="" - while [[ $(date +%s) -lt $deadline ]]; do - name=$(wn_b --json groups show "$gid" 2>/dev/null | jq -r '(.result // .) | .name // empty') - [[ "$name" == "Interop-02-renamed" ]] && break - sleep 3 - done - if [[ "$name" == "Interop-02-renamed" ]]; then - pass_msg "B sees renamed group" - else - fail_msg "B still sees name: $name" - record_result "07 metadata rename" fail "rename did not propagate to B"; return + step "polling B for name change (60s)" + local deadline_a=$(( $(date +%s) + 60 )) name_a="" + while [[ $(date +%s) -lt $deadline_a ]]; do + name_a=$(wn_b --json groups show "$gid_a" 2>/dev/null | jq -r '(.result // .) | .name // empty') + [[ "$name_a" == "Interop-02-renamed" ]] && break + sleep 3 + done + if [[ "$name_a" == "Interop-02-renamed" ]]; then + pass_msg "B sees renamed Interop-02" + record_result "07a metadata rename A->B" pass + else + fail_msg "B still sees name: $name_a" + record_result "07a metadata rename A->B" fail "rename did not propagate to B" + fi fi - step "B renames back" - wn_b groups rename "$gid" "Interop-02-reverse" >/dev/null 2>&1 || true + # Reverse direction: B (admin of Interop-03) renames → A observes. + # + # The previous revision issued the reverse rename on Interop-02 via + # `wn_b groups rename` — but B is a plain member of Interop-02, so + # MIP-03's `enforceAuthorizedProposalSet` rejects the GCE commit on + # the wn side and the rename never reaches any relay. Use the group + # B actually owns (Interop-03) for the reverse leg instead. + local gid_b + gid_b=$(load_state GROUP_03 || true) + if [[ -z "${gid_b:-}" ]]; then + skip_msg "no GROUP_03 — skipping reverse rename" + record_result "07b metadata rename B->A" skip "no GROUP_03" + return + fi - if confirm "Does Amethyst now show the group name as 'Interop-02-reverse'?"; then - record_result "07 metadata rename" pass + step "B (admin) renames Interop-03 -> Interop-03-renamed" + local rename_out + if ! rename_out=$(wn_b groups rename "$gid_b" "Interop-03-renamed" 2>&1); then + warn "wn_b groups rename returned nonzero: $rename_out" + printf 'wn_b groups rename: %s\n' "$rename_out" >>"$LOG_FILE" + fi + + if confirm "Does Amethyst now show the group previously named 'Interop-03' as 'Interop-03-renamed'?"; then + record_result "07b metadata rename B->A" pass else - record_result "07 metadata rename" fail "Amethyst did not pick up rename" + record_result "07b metadata rename B->A" fail "Amethyst did not pick up rename" fi } From 6f3abac3b2f8d27e4a1812692a5d0ac544cdcb49 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 22 Apr 2026 21:54:18 +0000 Subject: [PATCH 15/19] fix(marmot-interop): peel .result wrapper when reporting Test 10's B view MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Post-v0.2 wn wraps `--json groups show` output in {"result": {...}}, so `jq '{name, epoch}'` was reading the top level where those keys don't exist — Test 10 always logged "B state: {name: null, epoch: null}" and the operator couldn't tell whether the race produced the expected converged state on wn's side. Peel the .result wrapper before extracting the fields; same pattern already used everywhere else in the script that parses wn JSON. --- tools/marmot-interop/marmot-interop.sh | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tools/marmot-interop/marmot-interop.sh b/tools/marmot-interop/marmot-interop.sh index 966c3e30e..745baa844 100755 --- a/tools/marmot-interop/marmot-interop.sh +++ b/tools/marmot-interop/marmot-interop.sh @@ -1084,7 +1084,10 @@ test_10_concurrent_commits() { step "verifying B's view is consistent (name + epoch)" local b_view - b_view=$(wn_b --json groups show "$gid" 2>/dev/null | jq '{name, epoch}' 2>/dev/null || echo "{}") + # Post-v0.2 wn wraps `groups show` output in {"result": {...}}; peel + # that wrapper before extracting the fields so the info log isn't + # "name: null, epoch: null" on every run. + b_view=$(wn_b --json groups show "$gid" 2>/dev/null | jq '(.result // .) | {name, epoch}' 2>/dev/null || echo "{}") info "B state: $b_view" prompt_human "Read the group name now shown in Amethyst." From 3279c2463a05d7365ea0187916d81a9d95fdabd2 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 22 Apr 2026 22:58:21 +0000 Subject: [PATCH 16/19] =?UTF-8?q?fix(mls):=20use=20filtered=20direct=20pat?= =?UTF-8?q?h=20in=20UpdatePath=20per=20RFC=209420=20=C2=A77.9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Interop Test 06 (wn removes a member from a group Amethyst is in) was failing with: GroupEventHandler.add: ERROR Failed to apply commit: UpdatePath node count (1) doesn't match direct path length (2) Repro scenario: 3-member group [B=leaf 0, C=leaf 1, A=leaf 2]; B (admin, committer) removes C. After the Remove proposal applies, leaf 1 is blank but leaf 2 (A) is still occupied — so leaf_count stays at 3 per RFC 9420 §7.8 and the sender's direct path has length 2 ([node 1, root]). Per RFC 9420 §4.1.2 and §7.9 the **filtered direct path** drops any parent node whose child on the copath has an empty resolution: encrypting to such a parent is equivalent to encrypting to its only non-blank child, which is already on the direct path. In our scenario the parent at level 1 (parent of leaves 0, 1) has a blank leaf 1 on the copath — empty resolution — so it's filtered out. Filtered direct path length is 1, and that's what openmls / whitenoise-rs emit in `UpdatePath.nodes`. Quartz was always using the unfiltered direct path on both sides (send + receive + parent-hash chain + path-secret decrypt). That worked for quartz↔quartz (both sides agreed), but broke the moment a spec-correct sender (wn) sent a filtered UpdatePath into a quartz receiver. Parent-hash chain is further affected because §7.9.2 walks the filtered path — even if the size check passed, the hashes would not match. Fix it throughout: - RatchetTree.filteredDirectPath: new helper returning parallel (filteredDirectPath, filteredCopath) with the empty-resolution entries dropped. Uses the existing `resolution()` helper, which already handles unmerged_leaves per §4.1.2. - RatchetTree.applyUpdatePath: size-check + target the FILTERED path. - MlsGroup.commit (sender): emit one staged path node per filtered level; encrypt path secrets using the filtered copath; patch parent hashes into the filtered positions only. - MlsGroup.processCommitInner (receiver): the UpdatePath node index aligns to the filtered direct path, so the common-ancestor lookup must find the filtered index to pick the right ciphertext + copath resolution. Use the unfiltered index only for counting KDF steps to the root (path_secret chain advances one step per unfiltered level regardless of filtering — the chain is continuous; filtering only decides which levels emit a UpdatePathNode, not how many KDF steps separate them). - MlsGroup.computeSenderParentHashes: walk the filtered direct path. Map each filtered node back to its unfiltered level so the preUpdateSiblingHashes lookup (indexed by unfiltered level) still resolves. This makes the parent_hash chain agree with §7.9.2 and with what openmls computes on the other side. - MlsGroup.verifyParentHash: short-circuit on empty filtered path rather than empty unfiltered path. New regression test: testRemoveMiddleLeaf_ReceiverAcceptsCommit reproduces the 3-member remove-middle scenario end-to-end within quartz. It passed before this patch (because both sides were unfiltered and agreed with each other), and it passes after this patch (because both sides are now filtered and still agree) — the compatibility win is that quartz now also agrees with openmls/wn-produced commits of the same shape. Also fix an unrelated script bug in marmot-interop.sh: the discover_a_relays SQL probe was falsely reporting "wn has NO cached relay entries for A" even when welcome delivery was working, because the query assumed users.pubkey stored as BLOB but wn stores it as TEXT (hex) in the current schema. Accept either encoding. --- .../quartz/marmot/mls/group/MlsGroup.kt | 138 +++++++++++++----- .../quartz/marmot/mls/tree/RatchetTree.kt | 43 +++++- .../marmot/mls/MlsGroupLifecycleTest.kt | 60 ++++++++ tools/marmot-interop/marmot-interop.sh | 17 ++- 4 files changed, 209 insertions(+), 49 deletions(-) diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mls/group/MlsGroup.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mls/group/MlsGroup.kt index 3316be1d2..bdf98aba5 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mls/group/MlsGroup.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mls/group/MlsGroup.kt @@ -488,29 +488,50 @@ class MlsGroup private constructor( // the decryption into `UpdatePathError(UnableToDecrypt)`. val updatePath: UpdatePath? = if (needsPath && pathSecrets.isNotEmpty()) { - val copath = BinaryTree.copath(myLeafIndex, tree.leafCount) + // RFC 9420 §7.9: UpdatePath carries one node per entry in the + // **filtered** direct path — parents whose copath subtree has + // empty resolution are omitted (encrypting to them is + // equivalent to encrypting to their only non-blank child). + // Sender and receiver must agree on filtering or the + // UpdatePath length check fails and openmls/wn reject the + // commit with "UpdatePath node count doesn't match". + val (filteredDp, filteredCp) = tree.filteredDirectPath(myLeafIndex) + val directPath = BinaryTree.directPath(myLeafIndex, tree.leafCount) + + // path_secrets are derived one per UNFILTERED level so the + // KDF chain reaches the root regardless of filtering + // (commit_secret = DeriveSecret(root_path_secret, "path") + // must be computable even when the root is in a filtered-out + // position). Select the subset aligned to the filtered + // direct path for UpdatePath node generation. + val filteredPathSecrets = + directPath.indices.mapNotNull { i -> + val idxInFiltered = filteredDp.indexOf(directPath[i]) + if (idxInFiltered >= 0) pathSecrets[i] else null + } // Capture sibling tree hashes BEFORE applying the UpdatePath — // parent_hash computation (RFC 9420 §7.9.2) uses the - // ORIGINAL sibling-subtree tree hashes. + // ORIGINAL sibling-subtree tree hashes. Only needed at the + // filtered positions (those are the nodes whose parent_hash + // we're going to compute). val preUpdateSiblingHashes = capturePreUpdateSiblingHashes(myLeafIndex) // Stage path-keys into the tree so subsequent parent_hash / // tree_hash computations reflect the new keys. We'll fill in // the HPKE-encrypted secrets once we know the post-commit - // context bytes. + // context bytes. One staged node per FILTERED level. val stagedPathNodes = - pathSecrets.zip(copath).map { (pathKey, _) -> + filteredPathSecrets.map { pathKey -> UpdatePathNode(pathKey.publicKey, emptyList()) } tree.applyUpdatePath(myLeafIndex, stagedPathNodes) - // Compute parent_hash for every direct-path parent node and - // for the committer's leaf (RFC 9420 §7.9.2). + // Compute parent_hash for every FILTERED-direct-path parent + // node and for the committer's leaf (RFC 9420 §7.9.2). val (parentNodeHashes, leafParentHash) = computeSenderParentHashes(myLeafIndex, preUpdateSiblingHashes) - val directPath = BinaryTree.directPath(myLeafIndex, tree.leafCount) - for (nodeIdx in directPath) { + for (nodeIdx in filteredDp) { val existing = tree.getNode(nodeIdx) if (existing is com.vitorpamplona.quartz.marmot.mls.tree.TreeNode.Parent) { tree.setParent( @@ -559,7 +580,7 @@ class MlsGroup private constructor( ).toTlsBytes() val pathNodes = - stagedPathNodes.zip(copath).zip(pathSecrets) { (staged, copathNode), pathKey -> + stagedPathNodes.zip(filteredCp).zip(filteredPathSecrets) { (staged, copathNode), pathKey -> val resolution = tree.resolution(copathNode).filterNot { resNode -> BinaryTree.isLeaf(resNode) && @@ -1291,12 +1312,15 @@ class MlsGroup private constructor( } // After verification, patch the computed parent_hash values into - // the direct-path parent nodes. The sender's tree has these + // the FILTERED-direct-path parent nodes. The sender's tree has these // values filled in; receivers must match for treeHash() to agree - // (and thus for the epoch key schedule to converge). + // (and thus for the epoch key schedule to converge). Unfiltered + // parents have been blanked by the proposal or were never on the + // chain — skip them. val (recvParentHashes, _) = computeSenderParentHashes(senderLeafIndex, preUpdateSiblingHashes) - for (nodeIdx in BinaryTree.directPath(senderLeafIndex, tree.leafCount)) { + val (filteredDp, filteredCp) = tree.filteredDirectPath(senderLeafIndex) + for (nodeIdx in filteredDp) { val existing = tree.getNode(nodeIdx) if (existing is com.vitorpamplona.quartz.marmot.mls.tree.TreeNode.Parent) { tree.setParent( @@ -1308,8 +1332,15 @@ class MlsGroup private constructor( } } - // Decrypt path secret from our copath node - val directPath = BinaryTree.directPath(senderLeafIndex, tree.leafCount) + // Decrypt path secret from our copath node. + // + // RFC 9420 §7.9: UpdatePath nodes align to the sender's FILTERED + // direct path, so `commonAncestorIdx` must be computed against + // that filtered list (not the unfiltered directPath). Using the + // unfiltered index picks the wrong `updatePath.nodes[i]` and + // HPKE-decrypt fails silently — causing `ConfirmationTagMismatch` + // at best, or a completely wrong commit_secret at worst. + val unfilteredDirectPath = BinaryTree.directPath(senderLeafIndex, tree.leafCount) val myPath = BinaryTree.directPath(myLeafIndex, tree.leafCount) // Path-decryption context (RFC 9420 §7.6): matches what the @@ -1322,11 +1353,19 @@ class MlsGroup private constructor( treeHash = tree.treeHash(), ).toTlsBytes() - // Find the common ancestor - val commonAncestorIdx = directPath.indexOfFirst { it in myPath } - if (commonAncestorIdx >= 0 && commonAncestorIdx < updatePath.nodes.size) { - val pathNode = updatePath.nodes[commonAncestorIdx] - val copathNodeIdx = BinaryTree.copath(senderLeafIndex, tree.leafCount)[commonAncestorIdx] + // Find the unfiltered common-ancestor index (we need this for the + // KDF step count: commit_secret = DeriveSecret(path_secret[root]) + // requires walking one KDF step per unfiltered level from the + // common ancestor to the root). + val commonAncestorUnfilteredIdx = unfilteredDirectPath.indexOfFirst { it in myPath } + // Find the filtered common-ancestor index (for picking the right + // UpdatePath node + copath resolution). + val commonAncestorNode = + if (commonAncestorUnfilteredIdx >= 0) unfilteredDirectPath[commonAncestorUnfilteredIdx] else -1 + val commonAncestorFilteredIdx = filteredDp.indexOf(commonAncestorNode) + if (commonAncestorFilteredIdx >= 0 && commonAncestorFilteredIdx < updatePath.nodes.size) { + val pathNode = updatePath.nodes[commonAncestorFilteredIdx] + val copathNodeIdx = filteredCp[commonAncestorFilteredIdx] val resolution = tree.resolution(copathNodeIdx).filterNot { resNode -> BinaryTree.isLeaf(resNode) && @@ -1354,7 +1393,13 @@ class MlsGroup private constructor( // advances one step past the root; quartz was stopping at the root // and diverging — that's what caused `ConfirmationTagMismatch` on // every cross-impl commit. - val stepsToRoot = directPath.size - commonAncestorIdx - 1 + // + // Step count is measured against the UNFILTERED direct + // path — the path_secret chain advances one KDF step per + // level regardless of whether that level emits a + // UpdatePath node, so filtering changes which nodes carry + // ciphertext but not the number of KDF steps. + val stepsToRoot = unfilteredDirectPath.size - commonAncestorUnfilteredIdx - 1 var currentSecret = pathSecret repeat(stepsToRoot) { currentSecret = MlsCryptoProvider.deriveSecret(currentSecret, "path") @@ -1568,30 +1613,40 @@ class MlsGroup private constructor( senderLeafIndex: Int, preUpdateSiblingHashes: Map, ): Pair, ByteArray> { - val directPath = BinaryTree.directPath(senderLeafIndex, tree.leafCount) - if (directPath.isEmpty()) return Pair(emptyMap(), ByteArray(0)) + // RFC 9420 §7.9.2: parent_hash chain walks the FILTERED direct path. + // A filtered-out parent is never on the chain because its copath + // subtree is blank — there's no sibling tree hash to fold into the + // next hop. Using the unfiltered path here produces a chain that + // doesn't match what openmls/wn compute on the other side. + val (filteredDp, _) = tree.filteredDirectPath(senderLeafIndex) + val unfilteredDp = BinaryTree.directPath(senderLeafIndex, tree.leafCount) + if (filteredDp.isEmpty()) return Pair(emptyMap(), ByteArray(0)) + + // preUpdateSiblingHashes is indexed by level in the UNFILTERED path — + // map each filtered node to its unfiltered level so we pick the + // correct sibling tree hash. + val filteredLevels = filteredDp.map { unfilteredDp.indexOf(it) } val hashes = mutableMapOf() // Root's parent_hash is empty by convention (RFC 9420 §7.9.2). - hashes[directPath.last()] = ByteArray(0) + hashes[filteredDp.last()] = ByteArray(0) // Walk top-down (root has no parent → already set). For each node - // X below root, parent_hash(X) = Hash(encode(ParentHashInput{ - // encryption_key = parent(X).encryption_key, - // parent_hash = parent(X).parent_hash, - // original_sibling_tree_hash = tree_hash(sibling(X))_preUpdate, - // })). - for (i in directPath.size - 2 downTo 0) { - val xIdx = directPath[i] - val parentIdx = directPath[i + 1] + // X below root on the filtered path, parent_hash(X) is computed + // with parent(X)'s fields, where "parent" is the NEXT entry on the + // filtered direct path. + for (i in filteredDp.size - 2 downTo 0) { + val xIdx = filteredDp[i] + val parentIdx = filteredDp[i + 1] + val parentLevel = filteredLevels[i + 1] val parentNode = tree.getNode(parentIdx) if (parentNode !is com.vitorpamplona.quartz.marmot.mls.tree.TreeNode.Parent) { hashes[xIdx] = ByteArray(0) continue } val siblingTreeHash = - preUpdateSiblingHashes[i + 1] - ?: error("missing pre-update sibling tree hash at level ${i + 1}") + preUpdateSiblingHashes[parentLevel] + ?: error("missing pre-update sibling tree hash at level $parentLevel") hashes[xIdx] = MlsCryptoProvider.hash( encodeParentHashInput( @@ -1603,14 +1658,15 @@ class MlsGroup private constructor( } // The committer's leaf carries parent_hash(parent(leaf)) = parent_hash - // computed with the immediate parent's (directPath[0]) fields. - val immediateParentIdx = directPath.first() + // computed with the immediate parent's (filteredDp[0]) fields. + val immediateParentIdx = filteredDp.first() + val immediateParentLevel = filteredLevels.first() val immediateParent = tree.getNode(immediateParentIdx) val leafParentHash = if (immediateParent is com.vitorpamplona.quartz.marmot.mls.tree.TreeNode.Parent) { val siblingTreeHash = - preUpdateSiblingHashes[0] - ?: error("missing pre-update sibling tree hash at leaf level") + preUpdateSiblingHashes[immediateParentLevel] + ?: error("missing pre-update sibling tree hash at level $immediateParentLevel") MlsCryptoProvider.hash( encodeParentHashInput( encryptionKey = immediateParent.parentNode.encryptionKey, @@ -1676,8 +1732,12 @@ class MlsGroup private constructor( updatePath: UpdatePath, preUpdateSiblingHashes: Map, ): Boolean { - val directPath = BinaryTree.directPath(senderLeafIndex, tree.leafCount) - if (directPath.isEmpty()) return true + // Filtered direct path drives the parent_hash chain — a sender + // with an empty filtered direct path has no parents to hash so + // the leaf's parent_hash field should be an empty byte string + // and we can short-circuit verification. + val (filteredDp, _) = tree.filteredDirectPath(senderLeafIndex) + if (filteredDp.isEmpty()) return true // RFC 9420 §7.9.2: compute what the sender's parent_hash chain // SHOULD have been given the post-update tree state, then compare diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mls/tree/RatchetTree.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mls/tree/RatchetTree.kt index 39f70a786..77aba3bfe 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mls/tree/RatchetTree.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mls/tree/RatchetTree.kt @@ -266,22 +266,51 @@ class RatchetTree( return MlsCryptoProvider.hash(writer.toByteArray()) } + /** + * RFC 9420 §4.1.2 "filtered direct path": + * the direct path of a leaf node L, with any parent node removed whose + * child on the copath of L has an empty resolution (unmerged_leaves + * count toward the resolution). + * + * Returns parallel lists (filteredDirectPath, filteredCopath). This is + * what openmls / whitenoise-rs use for `UpdatePath.nodes` generation + * and application — omitting a parent whose copath subtree is entirely + * blank is spec-required because encrypting to that parent's key pair + * is equivalent to encrypting to its only non-blank child (which is + * already on the direct path). Relevant in, e.g., a 3-member group + * where the committer removes the middle leaf: the sibling subtree at + * the level above the committer becomes fully blank, and the parent at + * that level is dropped from the UpdatePath. + */ + fun filteredDirectPath(leafIndex: Int): Pair, List> { + val directPath = BinaryTree.directPath(leafIndex, _leafCount) + val copath = BinaryTree.copath(leafIndex, _leafCount) + val filteredDp = mutableListOf() + val filteredCp = mutableListOf() + for (i in directPath.indices) { + if (resolution(copath[i]).isNotEmpty()) { + filteredDp.add(directPath[i]) + filteredCp.add(copath[i]) + } + } + return filteredDp to filteredCp + } + /** * Apply an UpdatePath to the tree: update parent nodes along the sender's - * direct path with the provided path nodes. + * **filtered** direct path (RFC 9420 §7.9) with the provided path nodes. */ fun applyUpdatePath( senderLeafIndex: Int, pathNodes: List, ) { - val directPath = BinaryTree.directPath(senderLeafIndex, _leafCount) - - require(pathNodes.size == directPath.size) { - "UpdatePath node count (${pathNodes.size}) doesn't match direct path length (${directPath.size})" + val (filteredDp, _) = filteredDirectPath(senderLeafIndex) + require(pathNodes.size == filteredDp.size) { + "UpdatePath node count (${pathNodes.size}) doesn't match filtered direct path length (${filteredDp.size})" } - for (i in directPath.indices) { - val nodeIdx = directPath[i] + for (i in filteredDp.indices) { + val nodeIdx = filteredDp[i] val pathNode = pathNodes[i] setParent( diff --git a/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/mls/MlsGroupLifecycleTest.kt b/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/mls/MlsGroupLifecycleTest.kt index 0240d2cec..6609561c9 100644 --- a/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/mls/MlsGroupLifecycleTest.kt +++ b/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/mls/MlsGroupLifecycleTest.kt @@ -354,6 +354,66 @@ class MlsGroupLifecycleTest { assertEquals(bob2.leafIndex, aliceSees.senderLeafIndex) } + /** + * Regression: 3-member group, committer removes the MIDDLE leaf while the + * trailing leaf remains occupied. Interop harness Test 06 hit this with + * whitenoise-rs: the interop log showed quartz rejecting wn's commit + * with "Failed to apply commit: UpdatePath node count (1) doesn't match + * direct path length (2)". That error comes from + * `RatchetTree.applyUpdatePath` comparing the received node count against + * the CURRENT tree's direct-path length — the two sides disagree on the + * post-proposal leaf_count, so the path-length invariant breaks. + * + * Scenario Amethyst observed: + * Tree before: [B=leaf 0 (admin/committer), C=leaf 1, A=leaf 2] + * Commit: Remove(leaf 1) + * Tree after: [B=leaf 0, _blank_, A=leaf 2] (leaf 2 still occupied, + * so RFC §7.8 does NOT trim) + * + * Both sender and every receiver should end up at leaf_count=3 post- + * proposal with the sender's direct path at size 2 ([node 1, root 3]). + * If quartz's receiver processes the commit cleanly, the test passes. If + * this reproduction also reports "UpdatePath node count … doesn't match + * direct path length …", we've isolated the bug to the quartz side, not + * a wn-specific oddity. + */ + @Test + fun testRemoveMiddleLeaf_ReceiverAcceptsCommit() { + // Alice creates, adds Bob (→ leaf 1), adds Carol (→ leaf 2). + val alice = MlsGroup.create("alice".encodeToByteArray()) + + val bobBundle = createStandaloneKeyPackage("bob") + val addBobResult = alice.addMember(bobBundle.keyPackage.toTlsBytes()) + val bob = MlsGroup.processWelcome(addBobResult.welcomeBytes!!, bobBundle) + + val carolBundle = createStandaloneKeyPackage("carol") + val addCarolResult = alice.addMember(carolBundle.keyPackage.toTlsBytes()) + bob.processFramedCommit(addCarolResult.framedCommitBytes) + val carol = MlsGroup.processWelcome(addCarolResult.welcomeBytes!!, carolBundle) + + check(alice.memberCount == 3 && bob.memberCount == 3 && carol.memberCount == 3) { + "pre-condition: all three members must be present" + } + + // Alice removes Bob (leaf 1) — the MIDDLE leaf. Carol (leaf 2) stays, + // so trailing-blank trim does NOT fire and leaf_count stays at 3. + val removeResult = alice.removeMember(bob.leafIndex) + + // Carol, the remaining non-committer, must be able to apply the + // commit. This is where the interop regression surfaces. + carol.processFramedCommit(removeResult.framedCommitBytes) + + // After the Remove, Alice and Carol are the only members but the + // tree still has leaf_count=3 (leaf 1 blank, leaf 2 = Carol). + assertEquals(alice.epoch, carol.epoch, "Carol must be at Alice's epoch after remove") + assertEquals(2, alice.memberCount, "Alice: 2 active members after removing Bob") + assertEquals(2, carol.memberCount, "Carol: 2 active members after removing Bob") + + // Post-remove forward-secrecy round-trip: Alice sends, Carol decrypts. + val msg = "after removing bob".encodeToByteArray() + assertContentEquals(msg, carol.decrypt(alice.encrypt(msg)).content) + } + // ----------------------------------------------------------------------- // 8. Signing key rotation (Update proposal) // ----------------------------------------------------------------------- diff --git a/tools/marmot-interop/marmot-interop.sh b/tools/marmot-interop/marmot-interop.sh index 745baa844..60dfb9476 100755 --- a/tools/marmot-interop/marmot-interop.sh +++ b/tools/marmot-interop/marmot-interop.sh @@ -311,16 +311,27 @@ discover_a_relays() { step "wn_b's cached view of A's relay lists (SELECT from user_relays)" local rows + # wn's SQLite schema stores `users.pubkey` — we don't have a public + # CLI to confirm the column type, and in practice it's been observed + # stored as both BLOB (raw 32 bytes) and TEXT (lowercase hex) across + # versions. Try both forms so the probe doesn't falsely report "NO + # cached relay entries" when the cache is actually populated (symptom: + # this diagnostic warned loudly even though Tests 02/03/04/05 were + # delivering welcomes successfully — the welcomes were flowing, the + # probe was just querying the wrong column encoding). rows=$(sqlite3 -readonly "$db" \ "SELECT ur.relay_type, r.url FROM user_relays ur JOIN users u ON u.id = ur.user_id JOIN relays r ON r.id = ur.relay_id - WHERE LOWER(HEX(u.pubkey)) = LOWER('$A_HEX') + WHERE u.pubkey = X'$A_HEX' + OR LOWER(CAST(u.pubkey AS TEXT)) = LOWER('$A_HEX') ORDER BY ur.relay_type, r.url;" 2>>"$LOG_FILE" || true) if [[ -z "$rows" ]]; then warn "wn has NO cached relay entries for A ($A_HEX)." - warn " → Amethyst hasn't published kind:10002/10050/10051 to any relay wn_b can reach." - warn " → Welcomes and gift wraps will not reach Amethyst. Tests 03+ will fail." + warn " → Either Amethyst hasn't published kind:10002/10050/10051 to any relay wn_b" + warn " can reach, OR wn's SQLite schema changed pubkey column encoding again." + warn " → If later tests deliver welcomes successfully, the schema is the culprit;" + warn " grep the daemon's migration files for the users.pubkey column type." return fi From d05742a8b6697dd3f53bc407496becc9bdb98615 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 23 Apr 2026 00:18:56 +0000 Subject: [PATCH 17/19] debug(marmot): log publishMarmotKeyPackages path for Test 13 diagnosis MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Test 13 (KeyPackage rotation) fails with "no new KP event_id observed" — the interop log shows three `needsKeyPackageRotation=true` triggers (once per group join), each of which invokes `account.publishMarmotKeyPackages()` from `processMarmotWelcomeFlow`, but no subsequent `kind:30443` publish ever appears in the logcat. `publishMarmotKeyPackages` was completely silent: no-op guards, the needsRotation check, the rotated-events count, and the per-event publish all ran without a log line. We can't tell whether the rotation produces events that never reach a relay, produces zero events, short-circuits on needsRotation=false, or is never called at all. Mirror the addMarmotGroupMember log pattern: entry guards (manager null / not writeable), the needsRotation + relay count at decision time, the rotateConsumedKeyPackages produce-count, and a per-event publish line with id + target relay list. Next rerun of Test 13 will tell us which branch is silently dropping the rotation. --- .../vitorpamplona/amethyst/model/Account.kt | 24 ++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt index 7fd284b0c..e91446a4e 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt @@ -2167,15 +2167,33 @@ class Account( * Publish or rotate KeyPackage events. */ suspend fun publishMarmotKeyPackages() { - val manager = marmotManager ?: return - if (!isWriteable()) return + val manager = + marmotManager ?: run { + Log.w("MarmotDbg") { "publishMarmotKeyPackages: marmotManager is NULL — no-op" } + return + } + if (!isWriteable()) { + Log.w("MarmotDbg") { "publishMarmotKeyPackages: account is not writeable — no-op" } + return + } val relays = keyPackagePublishRelays() + val needsRotation = manager.needsKeyPackageRotation() + Log.d("MarmotDbg") { + "publishMarmotKeyPackages: needsRotation=$needsRotation relays=${relays.size}" + } - if (manager.needsKeyPackageRotation()) { + if (needsRotation) { val rotatedEvents = manager.rotateConsumedKeyPackages(relays.toList()) + Log.d("MarmotDbg") { + "publishMarmotKeyPackages: rotateConsumedKeyPackages produced ${rotatedEvents.size} event(s)" + } rotatedEvents.forEach { event -> cache.justConsumeMyOwnEvent(event) + Log.d("MarmotDbg") { + "publishMarmotKeyPackages: publishing rotated kind:${event.kind} id=${event.id.take(8)}… " + + "→ ${relays.size} relay(s): ${relays.map { it.url }}" + } client.publish(event, relays) } } From c88923a3d1d6235a84e5d423ae56be80257a3991 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 23 Apr 2026 00:30:57 +0000 Subject: [PATCH 18/19] fix(mls): decrypt retained-epoch messages correctly (Test 12 offline catchup) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `tryDecryptWithRetainedEpoch` fallback in `MlsGroupManager.decrypt` exists precisely to cover the interop harness's Test 12 scenario — application messages encrypted under epoch N arriving at a receiver that has already processed a commit advancing to N+1. The primary path throws "Message epoch X doesn't match current epoch Y", then the manager iterates the retained epoch window and tries each. Two bugs kept the fallback from ever succeeding: 1. sender-data ciphertext sample length. The primary decrypt path uses `MlsCryptoProvider.HASH_OUTPUT_LENGTH` (32 bytes, = KDF.Nh for HKDF-SHA256, per RFC 9420 §6.3.2); the retained path was using `AEAD_KEY_LENGTH` (16). The comment on the primary path — "Using AEAD.Nk here made sender-data decryption fail against every spec-compliant sender" — was the same fix just never propagated to this branch. sender-data AEAD always failed, the try/catch swallowed it, and the fallback returned null. 2. content plaintext was returned raw. AEAD output is a PrivateMessageContent struct (§6.3.1): `applicationData || signature || padding`. The primary decrypt path parses this with `pmcReader.readOpaqueVarInt()` to extract applicationData; the retained path returned the whole struct. Callers saw a length-prefixed blob with signature + zero padding glued on — obvious in a raw diff, invisible in a pass/fail test that only checked "decrypted is not null". Fix both in `tryDecryptWithRetainedEpoch`. Existing quartz↔quartz tests still pass because they don't exercise out-of-order decrypt — this branch was effectively dead code before. Add testDecryptRetainedEpoch_ApplicationMessageAfterCommit as a regression: Bob encrypts at epoch N, Alice advances to N+1 via add-member, Bob's epoch-N message then arrives at Alice — must round- trip through the retained-epoch fallback and match the original plaintext exactly. --- .../marmot/mls/group/MlsGroupManager.kt | 26 ++++++- .../quartz/marmot/MarmotPipelineTest.kt | 78 +++++++++++++++++++ 2 files changed, 101 insertions(+), 3 deletions(-) diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mls/group/MlsGroupManager.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mls/group/MlsGroupManager.kt index ca5f592c5..cc7ef8e2f 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mls/group/MlsGroupManager.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mls/group/MlsGroupManager.kt @@ -660,8 +660,18 @@ class MlsGroupManager( if (privMsg.epoch != retained.epoch) return null // Derive sender data key/nonce using ciphertext sample (RFC 9420 §6.3.1) + // RFC 9420 §6.3.2: ciphertext_sample is the first KDF.Nh bytes + // (32 for HKDF-SHA256), not AEAD.Nk (16). Using AEAD.Nk here made + // sender-data decryption silently fail for every retained-epoch + // message and turned the fallback path into a no-op — the + // symptom was interop Test 12 (offline catch-up): kind:9 + // messages encrypted under epoch N arriving after a 1→N+1 + // commit got rejected with "Message epoch X doesn't match + // current epoch Y" instead of being pulled through this path. + // Same fix already applied to MlsGroup.decrypt (line ~878); + // this branch was missed when that one was patched. val ciphertextSample = - privMsg.ciphertext.copyOfRange(0, minOf(privMsg.ciphertext.size, MlsCryptoProvider.AEAD_KEY_LENGTH)) + privMsg.ciphertext.copyOfRange(0, minOf(privMsg.ciphertext.size, MlsCryptoProvider.HASH_OUTPUT_LENGTH)) val senderDataKey = MlsCryptoProvider.expandWithLabel( retained.senderDataSecret, @@ -710,13 +720,23 @@ class MlsGroupManager( contentAad.putUint8(privMsg.contentType.value) contentAad.putOpaqueVarInt(privMsg.authenticatedData) - val plaintext = + val pmcBytes = MlsCryptoProvider.aeadDecrypt(kng.key, guardedNonce, contentAad.toByteArray(), privMsg.ciphertext) + // AEAD plaintext is a PrivateMessageContent struct (RFC 9420 + // §6.3.1): `applicationData || signature || padding`. The + // main decrypt path parses this and returns the inner + // applicationData; the retained-epoch branch was returning the + // raw struct, which made callers see a length-prefixed blob + // with signature + zero-padding glued onto the end. Extract the + // applicationData the same way. + val pmcReader = TlsReader(pmcBytes) + val applicationData = pmcReader.readOpaqueVarInt() + DecryptedMessage( senderLeafIndex = senderLeafIndex, contentType = privMsg.contentType, - content = plaintext, + content = applicationData, epoch = privMsg.epoch, ) } catch (_: Exception) { diff --git a/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/MarmotPipelineTest.kt b/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/MarmotPipelineTest.kt index ebfa6775d..9727705f2 100644 --- a/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/MarmotPipelineTest.kt +++ b/quartz/src/jvmAndroidTest/kotlin/com/vitorpamplona/quartz/marmot/MarmotPipelineTest.kt @@ -158,6 +158,84 @@ class MarmotPipelineTest { } } + /** + * Regression: the interop harness's Test 12 (offline catch-up) drove + * kind:9 application messages encrypted under epoch N into an Amethyst + * receiver that had already processed a 1→N+1 commit (e.g. add-member). + * Those epoch-N messages ought to decrypt through the retained-epoch + * fallback in [MlsGroupManager.decrypt], but the fallback was silently + * returning null because `tryDecryptWithRetainedEpoch` derived the + * sender-data sample using `AEAD_KEY_LENGTH` (16) instead of + * `HASH_OUTPUT_LENGTH` (32) — the same bug that was fixed on the + * main decrypt path earlier but never propagated to the retained + * branch. This test reproduces the flow and asserts the retained + * path actually decrypts. + */ + @Test + fun testDecryptRetainedEpoch_ApplicationMessageAfterCommit() { + runBlocking { + val aliceMgr = createGroupManager() + val bobMgr = createGroupManager() + + // 32-byte identities so they fit the MarmotGroupData 32-byte + // admin-pubkey slots. + val aliceId = ByteArray(32) { 0xA1.toByte() } + val bobId = ByteArray(32) { 0xB2.toByte() } + val carolId = ByteArray(32) { 0xC3.toByte() } + + aliceMgr.createGroup(groupId, aliceId) + // Install the MarmotGroupData extension so Welcome messages carry + // the NostrGroupData (MlsGroupManager.processWelcome requires it). + aliceMgr.updateGroupExtensions( + nostrGroupId = groupId, + extensions = + listOf( + com.vitorpamplona.quartz.marmot.mip01Groups + .MarmotGroupData( + nostrGroupId = groupId, + adminPubkeys = listOf(aliceId.toHexKey()), + ).toExtension(), + ), + ) + + // Bob joins at epoch 2 (epoch 0 = create, epoch 1 = GCE above, + // epoch 2 = add Bob). + val aliceGroup = aliceMgr.getGroup(groupId)!! + val bobBundle = aliceGroup.createKeyPackage(bobId, ByteArray(0)) + val addBob = aliceMgr.addMember(groupId, bobBundle.keyPackage.toTlsBytes()) + bobMgr.processWelcome(addBob.welcomeBytes!!, bobBundle) + val bobJoinEpoch = aliceMgr.getGroup(groupId)!!.epoch + assertEquals(bobJoinEpoch, bobMgr.getGroup(groupId)!!.epoch, "bob must be at alice's epoch after Welcome") + + // Bob encrypts a message at his current epoch — this would be + // held by a lossy relay and only reach Alice after she's + // already processed the next commit. + val bobStaleMsg = bobMgr.encrypt(groupId, "bob's offline epoch-N message".encodeToByteArray()) + + // Alice adds Carol, advancing to epoch N+1. Bob's earlier + // message is now out-of-order relative to Alice's tree state. + val carolBundle = aliceGroup.createKeyPackage(carolId, ByteArray(0)) + aliceMgr.addMember(groupId, carolBundle.keyPackage.toTlsBytes()) + assertEquals( + bobJoinEpoch + 1, + aliceMgr.getGroup(groupId)!!.epoch, + "alice must have advanced one epoch past bob's encrypt time", + ) + + // Bob's stale message now arrives at Alice. The primary decrypt + // path throws "Message epoch X doesn't match current epoch X+1"; + // the retained-epoch fallback inside MlsGroupManager.decrypt + // must pick it up. + val decrypted = aliceMgr.decrypt(groupId, bobStaleMsg) + assertEquals( + "bob's offline epoch-N message", + decrypted.content.decodeToString(), + "Alice must decrypt Bob's pre-commit message via retained-epoch fallback", + ) + assertEquals(bobJoinEpoch, decrypted.epoch, "message was encrypted at bob's join epoch") + } + } + @Test fun testInboundRejectsNonMemberGroup() { runBlocking { From 3a2068a7920fb6d672b594ebe72f68bbe17f3d9b Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 23 Apr 2026 01:44:32 +0000 Subject: [PATCH 19/19] fix(marmot): buffer + retry UndecryptableOuterLayer events after epoch advance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Test 12 (offline catch-up) was almost passing: the retained-epoch fallback (c88923a3) brought through the 5 epoch-1 application messages the user sent before adding C, but 3 epoch-2 messages + the rename commit still went missing. Only on app restart did they appear — the relay re-delivered and the replayed arrival order happened to put the epoch-2-advancing commit before the epoch-2 events. Root cause: kind:445 events arrive from the relay in subscription order, not epoch order. When B drives an offline-catch-up sequence like msg-1..5 (epoch 1) → add C (epoch 1→2) → msg-6..8 (epoch 2) → rename (epoch 2→3) the receiver can easily see {msg-6, msg-7, msg-8, rename, add-C, msg-1..5}. Everything from the rename onward arrives before the add-C commit, so the outer ChaCha20-Poly1305 layer fails with UndecryptableOuterLayer — we don't have the epoch-2 exporter yet. Quartz correctly reports the failure, but the receiver just logged "likely from before our join" and dropped the event on the floor. Nothing retried when the add-C commit eventually landed and the epoch caught up. Fix it at the GroupEventHandler level, where we can both see the original GroupEvent and hook into every CommitProcessed: - Keep a bounded per-group ArrayDeque of events that failed with UndecryptableOuterLayer. FIFO-evict at 64 entries so a genuinely pre-join stream can't pin unbounded memory. - On every CommitProcessed for a group, drain that group's queue and re-run each event through the normal add() path. A successful retry may itself emit another CommitProcessed and trigger recursive draining — that's fine, epochs are strictly monotonic so recursion depth is bounded by the queue size. - Concurrent-safe: all queue mutation is guarded by a Mutex. Ambient discriminator: we can't tell "epoch too old, from before our join" from "epoch too new, commit hasn't arrived yet" just from UndecryptableOuterLayer — both report it. Buffering both is fine: truly-pre-join events stay buffered until FIFO eviction pushes them out (no correctness problem, just a few dozen bytes of memory for a bounded time). --- .../loggedIn/DecryptAndIndexProcessor.kt | 89 ++++++++++++++++++- 1 file changed, 85 insertions(+), 4 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/DecryptAndIndexProcessor.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/DecryptAndIndexProcessor.kt index b2a5d834d..b1ab71975 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/DecryptAndIndexProcessor.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/DecryptAndIndexProcessor.kt @@ -50,6 +50,8 @@ import com.vitorpamplona.quartz.nipACWebRtcCalls.events.CallRejectEvent import com.vitorpamplona.quartz.nipACWebRtcCalls.events.CallRenegotiateEvent import com.vitorpamplona.quartz.utils.Log import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock class EventProcessor( private val account: Account, @@ -549,6 +551,29 @@ class GroupEventHandler( private val account: Account, private val cache: LocalCache, ) : EventHandler { + /** + * Per-group buffer of kind:445 events whose outer ChaCha20-Poly1305 + * layer couldn't be decrypted yet — typically because they carry an + * epoch the receiver hasn't advanced to. The relay subscription + * doesn't guarantee arrival order, so a run like + * + * 1) B sends msg-1..5 at epoch 1 + * 2) B adds C (commit advances B → epoch 2) + * 3) B sends msg-6..8 at epoch 2 + * 4) B renames (commit advances B → epoch 3) + * + * can reach a receiver in the order {msg-6, msg-7, msg-8, rename, + * add-C, msg-1..5} depending on relay ordering. Everything from step + * 3+ above is `UndecryptableOuterLayer` until the add-C commit lands; + * we re-run those pending events every time an epoch-advancing + * CommitProcessed fires for the same group. + * + * Bounded per-group so a stuck stream of "truly undecryptable" + * pre-join events can't grow unbounded. FIFO eviction on overflow. + */ + private val pendingUndecryptable: MutableMap> = mutableMapOf() + private val pendingMutex = Mutex() + override suspend fun add( event: GroupEvent, eventNote: Note, @@ -642,6 +667,10 @@ class GroupEventHandler( // Sync MIP-01 metadata after epoch advance (extensions may have changed) val chatroom = account.marmotGroupList.getOrCreateGroup(result.groupId) manager.syncMetadataTo(result.groupId, chatroom) + // Epoch just advanced — drain any kind:445 events that + // previously failed as UndecryptableOuterLayer for this + // group. See `pendingUndecryptable` for the scenario. + retryPendingFor(result.groupId, eventNote, publicNote) } is GroupEventResult.CommitPending -> { @@ -655,12 +684,31 @@ class GroupEventHandler( } is GroupEventResult.UndecryptableOuterLayer -> { - // Expected for commits + application messages from epochs - // that predate our join — per MLS forward secrecy we - // never held those keys. Not a bug, not a warning. + // Two common causes for this result: + // (a) event's epoch predates our join — we never held + // those exporter keys, nothing to retry later. + // (b) event's epoch is ahead of our current one — the + // epoch-advancing commit for this group hasn't + // arrived yet but will in a moment (relays don't + // guarantee arrival order across a kind:445 + // subscription). We can't tell (a) from (b) here, + // so buffer the event and retry after any future + // CommitProcessed for this group advances us. + // + // Bounded per-group: keep the most recent + // MAX_PENDING_PER_GROUP events, FIFO-evict on overflow. + // Prevents a flood of genuinely pre-join events from + // pinning unbounded memory. Log.d("MarmotDbg") { "GroupEventHandler.add: undecryptable outer layer for group=${result.groupId.take(8)}… " + - "(current + ${result.retainedEpochCount} retained epoch key(s) tried) — likely from before our join" + "(current + ${result.retainedEpochCount} retained epoch key(s) tried) — buffering for post-commit retry" + } + pendingMutex.withLock { + val queue = pendingUndecryptable.getOrPut(result.groupId) { ArrayDeque() } + if (queue.size >= MAX_PENDING_PER_GROUP) { + queue.removeFirst() + } + queue.addLast(event) } } @@ -673,4 +721,37 @@ class GroupEventHandler( Log.e("MarmotDbg", "GroupEventHandler.add: exception processing kind:445", e) } } + + /** + * Drain the pending-undecryptable queue for [groupId] and re-run each + * event through `add`. A successful retry may itself emit another + * CommitProcessed and trigger recursive draining — that's fine, MLS + * epoch advances are strictly monotonic so recursion depth is bounded + * by the number of queued events. + * + * `eventNote` / `publicNote` are re-used for the retry path; they only + * feed into relay-activity tracking and compose-note lookup, both of + * which are safe to re-fire with the triggering event's notes. + */ + private suspend fun retryPendingFor( + groupId: String, + eventNote: Note, + publicNote: Note, + ) { + val drained = + pendingMutex.withLock { + pendingUndecryptable.remove(groupId)?.toList() ?: return + } + if (drained.isEmpty()) return + Log.d("MarmotDbg") { + "GroupEventHandler.retryPendingFor: replaying ${drained.size} previously-undecryptable kind:445 event(s) for group=${groupId.take(8)}…" + } + for (pending in drained) { + add(pending, eventNote, publicNote) + } + } + + companion object { + private const val MAX_PENDING_PER_GROUP = 64 + } }