fix(marmot): amy catches up after being offline and honors White Noise admin removals

Two of the three harness failures against MDK 0.10.4 were real bugs:

Test 12 (offline catch-up). amy's sync applied kind-445 events in the order the
relays returned them, newest first. A message or rename sent after a commit
failed to open (UndecryptableOuter / 'no canonical epoch') when it was tried
before that commit. The cursor then moved past it, so it was never fetched
again: an offline member came back missing the rename and every message after a
membership change. Sync now handles Welcomes first and group events oldest
first, and re-ingests events that failed that way whenever a commit lands in
the same pass. Test 12 now passes.

Test 27 (deletion wn->amy). White Noise deletes with kind 4891 ('removal',
{"v":1,"action":"remove"}) instead of kind 5 whenever the deleter is a group
admin, their own messages included. Neither amy nor the app knew the kind, so
an admin's deletion from White Noise never applied (the test passed alone and
failed after test 21 made wn an admin). A 4891 now retracts its targets when its
author is a current group admin: in amy's message list, and in the app live and
on restore. It never renders as a row. Test 27 now passes.

Test 29 (disband amy->wn) still fails: wn stays at epoch 1 and never applies
the disband. Our commit matches MDK's disband validation rules (inline
lifecycle + admin-policy updates, every other leaf removed), and a plain removal
of the last other member works (new harness test 34 passes), so it points to
the post-join window MDK uses for its own rotation. It is left open. Test 29 now
logs wn's full group view, and test 27 prints both epochs when it fails.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Vitor Pamplona
2026-09-27 23:43:32 -04:00
co-authored by Claude Opus 5.5
parent a3c5015ed0
commit d2ac376dfc
12 changed files with 193 additions and 6 deletions
@@ -4048,6 +4048,7 @@ class Account(
// kind:1009 edit is re-linked to its message too.
val innerNote = marmot.indexMarmotInnerEvent(innerEvent).note
marmotGroupList.addMessage(groupId, innerNote)
marmot.applyMarmotAdminRemoval(groupId, innerEvent)
} catch (e: Exception) {
Log.w(
"Account",
@@ -298,6 +298,21 @@ class AccountMarmotActions(
return IndexedInnerEvent(innerNote, isNew)
}
/**
* Apply a kind-4891 admin removal: drop the messages it names from the conversation
* when its author is an admin of the group (see [MarmotManager.adminRemovalTargets]).
* Runs on live delivery and on the restart replay, which re-adds every stored message.
*/
fun applyMarmotAdminRemoval(
nostrGroupId: HexKey,
innerEvent: Event,
) {
val manager = account.marmotManager ?: return
manager.adminRemovalTargets(nostrGroupId, innerEvent).forEach { targetId ->
account.cache.getNoteIfExists(targetId)?.let { account.marmotGroupList.removeMessage(nostrGroupId, it) }
}
}
/** [note] holds the inner event; [isNew] is true the first time this client indexed it. */
class IndexedInnerEvent(
val note: Note,
@@ -771,6 +771,7 @@ class GroupEventHandler(
// peer-sent kind:1210 is dropped inside addMessage — see
// `MarmotGroupList.isDisplayableFeedMessage`.
account.marmotGroupList.addMessage(result.groupId, innerNote)
account.marmot.applyMarmotAdminRemoval(result.groupId, innerEvent)
// Traffic is the natural clock for disappearing messages: a
// group being read is a group whose expired messages should
@@ -113,7 +113,14 @@ object MessageCommands {
// A deletion is not its own row either. The retracted body is
// blanked rather than the row dropped, so a harness (or a reader
// paging back) can tell "retracted" from "never arrived".
val deleted = ctx.marmot.deletedIds(parsed)
val deleted =
ctx.marmot.deletedIds(
parsed,
ctx.marmot
.groupView(gid)
?.adminPubkeys
?.toSet() ?: emptySet(),
)
// Pinned at persist time from the retention of the epoch that
// DELIVERED each message, so it is the message's own expiry and not
// a recomputation against whatever the group's setting is now.
@@ -216,6 +216,7 @@ ALL_TESTS=(
test_31_reaction_materializes_on_wn
test_32_amy_message_after_wn_commit
test_33_wn_leaves_amy_admin_group
test_34_amy_removes_last_other_member
)
# --tests runs a subset in the order given. Most tests read state a previous
+38
View File
@@ -398,3 +398,41 @@ test_33_wn_leaves_amy_admin_group() {
record_result "$id" fail "amy never committed wn's SelfRemove; wn is still in the tree"
fi
}
test_34_amy_removes_last_other_member() {
banner "Test 34 — amy removes the only other member; wn processes its own removal"
local id="34 amy removes wn from a 2-member group"
# Test 06 removes one of three members. Removing the only other member leaves
# the committer alone in the tree, which is the shape a device removal hit:
# White Noise never applied it and kept showing itself as a member.
local out gid mls_gid b_gid
out=$(amy_json marmot group create --name "Interop-34") || { record_result "$id" fail "amy group create failed"; return; }
gid=$(printf '%s' "$out" | jq -r '.group_id')
mls_gid=$(printf '%s' "$out" | jq -r '.mls_group_id')
amy_json marmot group add "$gid" "$B_NPUB" >/dev/null || { record_result "$id" fail "amy could not invite wn"; return; }
b_gid=$(wait_for_invite B 60) || { record_result "$id" fail "wn never received the Welcome"; return; }
wn_b groups accept "$b_gid" >/dev/null 2>&1 || true
wn_group_field_becomes "$mls_gid" '.group.group_id // empty' "$mls_gid" 120 || { record_result "$id" fail "wn never surfaced the group"; return; }
wn_b messages send "$mls_gid" "34 before removal" >/dev/null 2>&1 || true
amy_json marmot await message "$gid" --match "34 before removal" --timeout 90 >/dev/null || { record_result "$id" fail "amy never got wn's message"; return; }
amy_json marmot group remove "$gid" "$B_NPUB" >/dev/null || { record_result "$id" fail "amy remove failed"; return; }
local deadline=$(( $(date +%s) + 120 )) gone=0 view
while [[ $(date +%s) -lt $deadline ]]; do
wn_b sync >/dev/null 2>&1 || true
view=$(wn_b_json groups show "$mls_gid" 2>/dev/null || true)
if ! printf '%s' "$view" | jq -e --arg p "$B_HEX" '.result.members[]? | select((.member_id // .pubkey // .public_key) == $p)' >/dev/null 2>&1; then
gone=1; break
fi
sleep 3
done
printf 'test34 wn view after removal: %s\n' "$(printf '%s' "$view" | head -c 2000)" >>"$LOG_FILE"
if [[ "$gone" -eq 1 ]]; then
record_result "$id" pass
else
record_result "$id" fail "wn still lists itself as a member after amy removed it"
fi
}
+15 -2
View File
@@ -439,8 +439,15 @@ test_27_deletion_wn_to_amy() {
record_result "$id" fail "amy has no event id for wn's message"; return
fi
if ! wn_b messages delete "$mls_gid" "$target" >/dev/null 2>&1; then
# Keep wn's answer: "the command ran" and "the tombstone reached a relay" are
# different, and only the second one can reach amy.
local del
del=$(wn_b --json messages delete "$mls_gid" "$target" 2>>"$LOG_FILE") || {
record_result "$id" fail "wn messages delete failed"; return
}
printf 'wn messages delete %s -> %s\n' "$target" "$del" >>"$LOG_FILE"
if ! printf '%s' "$del" | jq -e '.result.published != false' >/dev/null 2>&1; then
record_result "$id" fail "wn did not publish the delete tombstone: $del"; return
fi
# The row stays, blanked and flagged: "retracted" and "never arrived" are
@@ -460,7 +467,12 @@ test_27_deletion_wn_to_amy() {
done
if [[ "$gone" -ne 1 ]]; then
record_result "$id" fail "amy never marked wn's message deleted"; return
# Tell a lost tombstone from a split group: if the two sides sit on different
# epochs, the delete was sent where amy cannot follow.
local wn_epoch amy_epoch
wn_epoch=$(wn_b_json groups show "$mls_gid" 2>/dev/null | jq -r '.result.mls.epoch // "?"')
amy_epoch=$(amy_json marmot group show "$gid" 2>/dev/null | jq -r '.epoch // "?"')
record_result "$id" fail "amy never marked wn's message deleted (wn epoch $wn_epoch, amy epoch $amy_epoch)"; return
fi
if [[ -n "$body" ]]; then
record_result "$id" fail "amy flagged the message deleted but still shows '$body'"; return
@@ -695,6 +707,7 @@ test_29_disband_amy_to_wn() {
wn_b sync >/dev/null 2>&1 || true
sleep 5
done
printf 'disband29 wn view: %s\n' "$(wn_b_json groups show "$mls_gid" 2>&1 | head -c 3000)" >>"$LOG_FILE"
printf 'disband29 epoch %s -> %s (amy now %s), wn at %s\n' \
"$before_epoch" "$after_epoch" "${amy_now:-?}" "${saw:-<none>}" >>"$LOG_FILE"
@@ -1598,25 +1598,51 @@ class MarmotManager(
* targets together, since a deletion is only authorized against the message
* it names.
*/
fun deletedIds(messages: List<Event>): Set<HexKey> {
fun deletedIds(
messages: List<Event>,
/** The group's admins: their kind-4891 removals apply to anyone's message. */
admins: Set<HexKey> = emptySet(),
): Set<HexKey> {
val authorOf = HashMap<HexKey, HexKey>(messages.size)
val claims = ArrayList<Pair<HexKey, HexKey>>()
val removed = HashSet<HexKey>()
for (event in messages) {
if (event.kind == DeletionRequestEvent.KIND) {
for (tag in event.tags) {
if (tag.size >= 2 && tag[0] == "e") claims.add(tag[1] to event.pubKey)
}
} else if (event.kind == MarmotAppEvent.KIND_REMOVE) {
if (event.pubKey in admins) removed.addAll(adminRemovalTargets(event))
} else {
authorOf[event.id] = event.pubKey
}
}
val deleted = HashSet<HexKey>(claims.size)
val deleted = HashSet<HexKey>(claims.size + removed.size)
for ((targetId, deleter) in claims) {
if (authorOf[targetId] == deleter) deleted.add(targetId)
}
deleted.addAll(removed.filter { it in authorOf })
return deleted
}
/**
* The messages a kind-4891 removal names, if its author may remove them: a current
* admin of [nostrGroupId]. Empty for anything else. White Noise sends 4891 instead of
* kind 5 whenever the deleter is an admin (their own messages included), so without
* this an admin's deletion from White Noise never reached us.
*/
fun adminRemovalTargets(
nostrGroupId: HexKey,
event: Event,
): List<HexKey> {
if (event.kind != MarmotAppEvent.KIND_REMOVE) return emptyList()
val admins = groupView(nostrGroupId)?.adminPubkeys ?: return emptyList()
if (event.pubKey !in admins) return emptyList()
return adminRemovalTargets(event)
}
private fun adminRemovalTargets(event: Event): List<HexKey> = event.tags.mapNotNull { tag -> if (tag.size >= 2 && tag[0] == "e") tag[1] else null }
/**
* The slice of canonical group state that kind:1210 rows are derived from,
* or null when this client is not in the group.
@@ -143,13 +143,24 @@ class MarmotSyncPolicy(
}
if (filterMap.isEmpty()) return
val events = drain(filterMap, timeoutMs)
// Relays answer newest-first and several relays interleave, but a kind:445 can only
// be opened at the epoch its predecessors built: a message sent after a commit fails
// (UndecryptableOuter, "no canonical epoch") if it is tried before that commit. The
// cursor then moves past it and it is never fetched again, which is how an offline
// member came back missing a rename and every message after a membership change.
// Welcomes first (they create the groups), then group events oldest first.
val events =
drain(filterMap, timeoutMs)
.distinctBy { it.second.id }
.sortedWith(compareBy({ if (it.second.kind == GiftWrapEvent.KIND) 0 else 1 }, { it.second.createdAt }))
var maxGwSeen = gwSince ?: 0L
val maxGroupSeen = perGroupFilters.keys.associateWith { cursors.groupSince(it) ?: 0L }.toMutableMap()
var sawGiftWrap = false
val sawGroupEvent = mutableSetOf<HexKey>()
val stagedProposals = mutableSetOf<HexKey>()
val waitingOnEpoch = mutableListOf<Event>()
var advancedEpoch = false
for ((relay, event) in events) {
// All the MLS/NIP-59 decryption + persistence lives in MarmotIngest —
@@ -163,6 +174,8 @@ class MarmotSyncPolicy(
}
log("ingest ${event.kind}/${event.id.take(8)} via $relay → ${result::class.simpleName}$detail")
if (result is MarmotIngestResult.ProposalStaged) stagedProposals.add(result.groupId)
if (event.kind == GroupEvent.KIND && result.couldOpenAfterACommit()) waitingOnEpoch.add(event)
if (result is MarmotIngestResult.Commit) advancedEpoch = true
when (event.kind) {
GiftWrapEvent.KIND -> {
@@ -179,6 +192,22 @@ class MarmotSyncPolicy(
}
}
// Same-second ties and cross-relay stragglers can still put an event ahead of the
// commit it needs; once a commit landed, give those another go, until a pass
// opens nothing new.
while (advancedEpoch && waitingOnEpoch.isNotEmpty()) {
advancedEpoch = false
val retry = waitingOnEpoch.toList()
waitingOnEpoch.clear()
for (event in retry) {
val result = marmot.ingest(event)
log("retry ${event.kind}/${event.id.take(8)} → ${result::class.simpleName}")
if (result is MarmotIngestResult.Commit) advancedEpoch = true
if (result is MarmotIngestResult.ProposalStaged) stagedProposals.add(result.groupId)
if (result.couldOpenAfterACommit()) waitingOnEpoch.add(event)
}
}
// A member's SelfRemove only takes effect once an admin commits it.
for (gid in stagedProposals) {
marmot.commitStagedProposalsIfAdmin(gid)?.let { log("committed staged proposals for ${gid.take(8)} → ${it.signedEvent.id.take(8)}") }
@@ -224,3 +253,8 @@ class MarmotSyncPolicy(
const val GIFT_WRAP_LOOKBACK_SECS: Long = 2L * 24 * 60 * 60
}
}
/** Failed only because the epoch it was sent at isn't reached yet; a later commit may open it. */
private fun MarmotIngestResult.couldOpenAfterACommit(): Boolean =
this is MarmotIngestResult.UndecryptableOuter ||
(this is MarmotIngestResult.Failure && message.startsWith(MarmotManager.NO_CANONICAL_EPOCH_ERROR))
@@ -177,6 +177,7 @@ class MarmotGroupList(
companion object {
private const val MARMOT_INNER_KIND_DELETION = 5
private const val MARMOT_INNER_KIND_ADMIN_REMOVAL = 4891
private const val MARMOT_INNER_KIND_REACTION = 7
private const val MARMOT_INNER_KIND_EDIT = 1009
private const val MARMOT_INNER_KIND_STREAM_START = 1200
@@ -192,6 +193,7 @@ class MarmotGroupList(
private val NON_CHAT_INNER_KINDS =
setOf(
MARMOT_INNER_KIND_DELETION,
MARMOT_INNER_KIND_ADMIN_REMOVAL,
MARMOT_INNER_KIND_REACTION,
MARMOT_INNER_KIND_EDIT,
MARMOT_INNER_KIND_STREAM_START,
@@ -140,6 +140,48 @@ class MarmotEditsAndSystemRowsTest {
assertTrue(mine.innerEvent.id !in f.manager.deletedIds(f.manager.storedEvents()))
}
@Test
fun `an admin's kind 4891 removal retracts a message, a non-admin's does not`() =
runBlocking {
// White Noise sends 4891 instead of kind 5 whenever the deleter is an admin,
// for their own messages too; missing it meant those deletions never applied.
val f = Fixture()
f.manager.createGroup(
nostrGroupId,
MarmotGroupData(
nostrGroupId = nostrGroupId,
name = "moderated",
relays = listOf("wss://relay.invalid"),
adminPubkeys = listOf(f.signer.pubKey),
),
)
val target = f.manager.buildTextMessage(nostrGroupId, "moderate me")
val admin = f.signer.pubKey
val stranger = "f".repeat(64)
fun removal(by: String) =
MarmotAppEvent.build(
pubKey = by,
kind = MarmotAppEvent.KIND_REMOVE,
content = """{"v":1,"action":"remove"}""",
createdAt = 1_800_000_000L,
tags = arrayOf(arrayOf("e", target.innerEvent.id)),
)
val byStranger = removal(stranger)
f.manager.persistDecryptedMessage(nostrGroupId, byStranger.toJson().dropLast(1) + ",\"sig\":\"\"}")
val admins = setOf(admin)
assertTrue(target.innerEvent.id !in f.manager.deletedIds(f.manager.storedEvents(), admins))
assertTrue(f.manager.adminRemovalTargets(nostrGroupId, Event.fromJson(byStranger.toJson().dropLast(1) + ",\"sig\":\"\"}")).isEmpty())
val byAdmin = removal(admin)
f.manager.persistDecryptedMessage(nostrGroupId, byAdmin.toJson().dropLast(1) + ",\"sig\":\"\"}")
assertTrue(target.innerEvent.id in f.manager.deletedIds(f.manager.storedEvents(), admins))
assertEquals(
listOf(target.innerEvent.id),
f.manager.adminRemovalTargets(nostrGroupId, Event.fromJson(byAdmin.toJson().dropLast(1) + ",\"sig\":\"\"}")),
)
}
@Test
fun `one kind 5 retracts every message it names`() =
runBlocking {
@@ -83,6 +83,13 @@ class MarmotAppEvent(
/** A durable group system row, synthesized from canonical state. */
const val KIND_SYSTEM = 1210
/**
* An admin's removal of a message (anyone's, their own included), with an `e` tag
* naming the target and `{"v":1,"action":"remove"}` as content. White Noise sends
* it instead of a kind-5 deletion whenever the deleter is a group admin.
*/
const val KIND_REMOVE = 4891
private val ALLOWED_MEMBERS = setOf("id", "pubkey", "created_at", "kind", "tags", "content")
val EMPTY_TAGS: TagArray = emptyArray()