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 192e9523c1..eb0d4e75f4 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt @@ -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", diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountMarmotActions.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountMarmotActions.kt index 3ba76a367c..d1b3a20cad 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountMarmotActions.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountMarmotActions.kt @@ -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, 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 b22771664f..71178e26dc 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 @@ -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 diff --git a/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/commands/MessageCommands.kt b/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/commands/MessageCommands.kt index 8c66e70686..3b56df46df 100644 --- a/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/commands/MessageCommands.kt +++ b/cli/src/main/kotlin/com/vitorpamplona/amethyst/cli/commands/MessageCommands.kt @@ -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. diff --git a/cli/tests/marmot/marmot-interop-headless.sh b/cli/tests/marmot/marmot-interop-headless.sh index 966303d14a..37b233460e 100755 --- a/cli/tests/marmot/marmot-interop-headless.sh +++ b/cli/tests/marmot/marmot-interop-headless.sh @@ -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 diff --git a/cli/tests/marmot/tests-manage.sh b/cli/tests/marmot/tests-manage.sh index 912eeefbc7..84a653b289 100644 --- a/cli/tests/marmot/tests-manage.sh +++ b/cli/tests/marmot/tests-manage.sh @@ -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 +} diff --git a/cli/tests/marmot/tests-media.sh b/cli/tests/marmot/tests-media.sh index 1779cc4ee4..73a285687b 100644 --- a/cli/tests/marmot/tests-media.sh +++ b/cli/tests/marmot/tests-media.sh @@ -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:-}" >>"$LOG_FILE" 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 5796ed9978..7c6da904cf 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 @@ -1598,25 +1598,51 @@ class MarmotManager( * targets together, since a deletion is only authorized against the message * it names. */ - fun deletedIds(messages: List): Set { + fun deletedIds( + messages: List, + /** The group's admins: their kind-4891 removals apply to anyone's message. */ + admins: Set = emptySet(), + ): Set { val authorOf = HashMap(messages.size) val claims = ArrayList>() + val removed = HashSet() 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(claims.size) + val deleted = HashSet(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 { + 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 = 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. diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotSyncPolicy.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotSyncPolicy.kt index 99bafa11b1..3fc71f2066 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotSyncPolicy.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotSyncPolicy.kt @@ -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() val stagedProposals = mutableSetOf() + val waitingOnEpoch = mutableListOf() + 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)) diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/marmotGroups/MarmotGroupList.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/marmotGroups/MarmotGroupList.kt index 2d767a6aa8..59059005c9 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/marmotGroups/MarmotGroupList.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/marmotGroups/MarmotGroupList.kt @@ -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, diff --git a/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotEditsAndSystemRowsTest.kt b/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotEditsAndSystemRowsTest.kt index e2ea1de62e..0dd1ca3c03 100644 --- a/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotEditsAndSystemRowsTest.kt +++ b/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotEditsAndSystemRowsTest.kt @@ -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 { diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/foundation/appEvents/MarmotAppEvent.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/foundation/appEvents/MarmotAppEvent.kt index e63c2f7d2f..b23f7f1e65 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/foundation/appEvents/MarmotAppEvent.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/foundation/appEvents/MarmotAppEvent.kt @@ -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()