From ab5119b75ba53f897fc543fa19dcd39b3a9330d6 Mon Sep 17 00:00:00 2001 From: Vitor Pamplona Date: Sun, 27 Sep 2026 15:38:31 -0400 Subject: [PATCH] fix(marmot): send reactions and deletions inside the group, not as NIP-17 DMs A Marmot message is an unsigned rumor, so the app's generic reaction and deletion paths treated it as a NIP-17 private note: a reaction was gift-wrapped to the author as a DM, and a deletion went the same way (or to nobody, for our own message). Neither reached the MLS group. White Noise never showed an Amethyst reaction or deletion, and group activity left the group's channel as DMs. Only the CLI used the in-group builders, which is why the interop harness passed. Account.reactTo, delete and deletePrivately now check whether the target is a Marmot message (MarmotGroupList.groupIdForNote) and send an inner kind:7 or kind:5 through sendMarmotGroupMessage, like any other group message. Unreact already goes through deletePrivately with the reacted-to message as its target, so it is covered. Custom-emoji reactions carry their emoji tag, as on the public path. Verified on device (Amethyst tablet <-> White Noise Android, MDK 0.10.4): react, unreact and delete from Amethyst all show in White Noise. No gift wraps were sent for them. The White Noise app only shows an incoming reaction after its chat is reopened; wn's materialized timeline has it immediately, so that is White Noise's live refresh. Harness test 31 checks that an amy reaction appears in wn's MATERIALIZED timeline (test 09 only checks the raw event log, which the app does not render). Builder unit tests added. Co-Authored-By: Claude Opus 5.5 --- .../vitorpamplona/amethyst/model/Account.kt | 21 ++++- .../amethyst/model/AccountMarmotActions.kt | 49 +++++++++++ cli/tests/marmot/marmot-interop-headless.sh | 1 + cli/tests/marmot/tests-manage.sh | 36 ++++++++ .../amethyst/commons/marmot/MarmotManager.kt | 30 +++++-- .../marmot/MarmotInnerRumorBuildersTest.kt | 83 +++++++++++++++++++ 6 files changed, 210 insertions(+), 10 deletions(-) create mode 100644 commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotInnerRumorBuildersTest.kt 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 e2a51f8237..7f149f0610 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt @@ -1538,6 +1538,12 @@ class Account( note: Note, reaction: String, ) { + // A Marmot message reacts inside its MLS group; see AccountMarmotActions.reactToMarmotMessage. + marmot.marmotGroupOf(note)?.let { groupId -> + marmot.reactToMarmotMessage(groupId, note, reaction) + return + } + // Reactions to NIP-17 groups and unsealed rumors are gift-wrapped: the // inner kind-7 only ever travels as ciphertext, so mining it is pure // waste — those targets skip the queue and sign with the plain signer. @@ -1703,7 +1709,14 @@ class Account( suspend fun delete(notes: List) { if (!isWriteable()) return - val myNotes = notes.filter { it.author == userProfile() && it.event != null } + // Marmot messages are retracted inside their group. A public NIP-09 here would e-tag + // the group's private rumor ids onto public relays. + val (marmotNotes, otherNotes) = notes.partition { marmot.marmotGroupOf(it) != null } + marmotNotes.groupBy { marmot.marmotGroupOf(it)!! }.forEach { (groupId, groupNotes) -> + marmot.deleteMarmotMessages(groupId, groupNotes) + } + + val myNotes = otherNotes.filter { it.author == userProfile() && it.event != null } if (myNotes.isNotEmpty()) { // chunks in 200 elements to avoid going over the 65KB limit for events. myNotes.chunked(200).forEach { chunkedList -> @@ -1733,6 +1746,12 @@ class Account( if (!isWriteable()) return val targetEvent = target.event ?: return + // In a Marmot group the deletion goes to the group, not to the target's author as a DM. + marmot.marmotGroupOf(target)?.let { groupId -> + marmot.deleteMarmotMessages(groupId, notes) + return + } + val myRumors = notes.filter { it.author == userProfile() }.mapNotNull { it.event } if (myRumors.isEmpty()) return 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 68c223da21..bdb1f9a9e6 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountMarmotActions.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/AccountMarmotActions.kt @@ -20,6 +20,7 @@ */ package com.vitorpamplona.amethyst.model +import com.vitorpamplona.amethyst.commons.model.Note import com.vitorpamplona.quartz.marmot.appComponents.BlobStoreEndpointV2 import com.vitorpamplona.quartz.marmot.appComponents.EncryptedMediaPolicyV2 import com.vitorpamplona.quartz.marmot.appComponents.GroupAvatarUrlV1 @@ -246,6 +247,54 @@ class AccountMarmotActions( manager.persistDecryptedMessage(nostrGroupId, innerEvent.toJson()) } + /** + * The Marmot group [note] was received or sent in, or null when it is not a + * Marmot message. + * + * Only chat rows are indexed, so pass the MESSAGE a reaction or deletion is + * about, not the reaction itself. + */ + fun marmotGroupOf(note: Note): HexKey? = account.marmotGroupList.groupIdForNote(note.idHex) + + /** + * React to a Marmot message inside its group. + * + * A Marmot message is an unsigned rumor, which the generic reaction path + * handles as a NIP-17 private note: it gift-wrapped the kind:7 to the + * author as a DM. That never reached the group, so no other client showed + * it, and it moved group activity out of the group's channel. The reaction + * is an ordinary inner kind:7 (MIP-03), encrypted to the group like any + * message. + */ + suspend fun reactToMarmotMessage( + nostrGroupId: HexKey, + target: Note, + reaction: String, + ) { + val manager = account.marmotManager ?: return + val targetEvent = target.event ?: return + if (target.hasReacted(account.userProfile(), reaction)) return + val rumor = manager.buildReactionRumor(targetEvent, reaction) + sendMarmotGroupMessage(nostrGroupId, rumor, marmotGroupRelays(nostrGroupId)) + } + + /** + * Retract our own messages or reactions in a Marmot group with an inner + * kind:5, for the same reason [reactToMarmotMessage] exists: the generic + * private path sent a gift-wrapped NIP-09 to the target's author, so the + * other members never saw the deletion. + */ + suspend fun deleteMarmotMessages( + nostrGroupId: HexKey, + notes: List, + ) { + val manager = account.marmotManager ?: return + val mine = notes.filter { it.author == account.userProfile() }.mapNotNull { it.event } + if (mine.isEmpty()) return + val rumor = manager.buildDeletionRumor(mine) + sendMarmotGroupMessage(nostrGroupId, rumor, marmotGroupRelays(nostrGroupId)) + } + /** * Fetch a user's KeyPackage from relays and add them to a Marmot group. * Returns a status message describing the outcome. diff --git a/cli/tests/marmot/marmot-interop-headless.sh b/cli/tests/marmot/marmot-interop-headless.sh index e1d7e8d2e8..afd2fbb738 100755 --- a/cli/tests/marmot/marmot-interop-headless.sh +++ b/cli/tests/marmot/marmot-interop-headless.sh @@ -210,6 +210,7 @@ ALL_TESTS=( test_27_deletion_wn_to_amy test_28_retention_wn_to_amy test_29_disband_amy_to_wn + test_31_reaction_materializes_on_wn ) # --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 9a8fcf5fb1..29d8eff84e 100644 --- a/cli/tests/marmot/tests-manage.sh +++ b/cli/tests/marmot/tests-manage.sh @@ -250,3 +250,39 @@ test_17_group_image_commit() { record_result "$id" fail "wn could not decrypt A's post-image message — image commit not applied" fi } + +# A reaction White Noise can SEE. Test 09 only proves wn's raw event log holds a +# kind:7; the app renders the materialized timeline, which attaches a reaction +# to its target by its own rules. An amy reaction the raw log kept but the +# timeline dropped read as a pass there and as "no reaction" in the app. +test_31_reaction_materializes_on_wn() { + banner "Test 31 — amy's reaction shows in wn's materialized timeline" + local id="31 reaction materialized" + + local out gid mls_gid b_gid anchor_id + out=$(amy_json marmot group create --name "Interop-31") || { 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" "31 anchor" >/dev/null 2>&1 || true + amy_json marmot await message "$gid" --match "31 anchor" --timeout 90 >/dev/null || { record_result "$id" fail "amy never got the anchor"; return; } + anchor_id=$(amy_json marmot message list "$gid" --limit 50 2>/dev/null | jq_list messages \ + | jq -r 'select((.plaintext // .content // "") == "31 anchor") | (.message_id // .event_id)' | head -n 1) + [[ -n "$anchor_id" && "$anchor_id" != "null" ]] || { record_result "$id" fail "no anchor id in amy's log"; return; } + amy_json marmot message react "$gid" "$anchor_id" "🍕" >/dev/null || { record_result "$id" fail "amy react failed"; return; } + + local deadline=$(( $(date +%s) + 90 )) tl="" + while [[ $(date +%s) -lt $deadline ]]; do + tl=$(wn_b --json messages timeline list "$mls_gid" --limit 50 2>/dev/null || true) + if printf '%s' "$tl" | grep -q '🍕'; then + record_result "$id" pass; return + fi + sleep 3 + done + printf '%s\n' "$tl" >> "$LOG_FILE" + record_result "$id" fail "wn's timeline never showed amy's reaction (timeline JSON in the log)" +} 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 b100491b67..f145024fa1 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 @@ -87,6 +87,7 @@ import com.vitorpamplona.quartz.nip01Core.tags.people.pTags import com.vitorpamplona.quartz.nip09Deletions.DeletionRequestEvent import com.vitorpamplona.quartz.nip18Reposts.quotes.QEventTag import com.vitorpamplona.quartz.nip18Reposts.quotes.quote +import com.vitorpamplona.quartz.nip30CustomEmoji.EmojiUrlTag import com.vitorpamplona.quartz.nip59Giftwrap.rumors.RumorAssembler import com.vitorpamplona.quartz.utils.Log import com.vitorpamplona.quartz.utils.TimeUtils @@ -570,13 +571,20 @@ class MarmotManager( targetEvent: Event, reaction: String, ): Event { + val hint = + com.vitorpamplona.quartz.nip01Core.hints + .EventHintBundle(targetEvent) + // A custom-emoji reaction (":name:url") carries its image in an emoji + // tag, exactly as the public NIP-25 path builds it. + val emojiUrl = if (reaction.startsWith(":")) EmojiUrlTag.decode(reaction) else null val template = - com.vitorpamplona.quartz.nip25Reactions.ReactionEvent - .build( - reaction, - com.vitorpamplona.quartz.nip01Core.hints - .EventHintBundle(targetEvent), - ) + if (emojiUrl != null) { + com.vitorpamplona.quartz.nip25Reactions.ReactionEvent + .build(emojiUrl, hint) + } else { + com.vitorpamplona.quartz.nip25Reactions.ReactionEvent + .build(reaction, hint) + } return com.vitorpamplona.quartz.nip59Giftwrap.rumors.RumorAssembler .assembleRumor( signer.pubKey, @@ -739,14 +747,18 @@ class MarmotManager( targetEvents: List, persistOwn: Boolean = true, ): TextMessageBundle { - require(targetEvents.isNotEmpty()) { "buildDeletionMessage: targetEvents must not be empty" } - val template = DeletionRequestEvent.build(targetEvents) - val innerEvent = RumorAssembler.assembleRumor(signer.pubKey, template) + val innerEvent = buildDeletionRumor(targetEvents) val outbound = buildGroupMessage(nostrGroupId, innerEvent) if (persistOwn) persistDecryptedMessage(nostrGroupId, innerEvent.toJson()) return TextMessageBundle(outbound = outbound, innerEvent = innerEvent) } + /** The inner kind:5 deletion rumor alone. See [buildTextRumor]. */ + suspend fun buildDeletionRumor(targetEvents: List): DeletionRequestEvent { + require(targetEvents.isNotEmpty()) { "buildDeletionRumor: targetEvents must not be empty" } + return RumorAssembler.assembleRumor(signer.pubKey, DeletionRequestEvent.build(targetEvents)) + } + /** * A single invitee for [addMemberInvites]: whose KeyPackage is consumed, the bare * KeyPackage bytes as published, and the id of the event that carried them diff --git a/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotInnerRumorBuildersTest.kt b/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotInnerRumorBuildersTest.kt new file mode 100644 index 0000000000..3ce3da21f8 --- /dev/null +++ b/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotInnerRumorBuildersTest.kt @@ -0,0 +1,83 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.commons.marmot + +import com.vitorpamplona.quartz.nip01Core.core.Event +import com.vitorpamplona.quartz.nip01Core.crypto.KeyPair +import com.vitorpamplona.quartz.nip01Core.signers.NostrSignerInternal +import com.vitorpamplona.quartz.nip09Deletions.DeletionRequestEvent +import com.vitorpamplona.quartz.nip25Reactions.ReactionEvent +import kotlinx.coroutines.runBlocking +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +/** + * The inner rumors the app sends for reactions and deletions in a Marmot group. + * Both used to leave the group as NIP-17 gift wraps to the target's author; they + * are now plain MIP-03 inner events, so their shape is what peers validate. + */ +class MarmotInnerRumorBuildersTest { + private val signer = NostrSignerInternal(KeyPair()) + private val manager = MarmotManager(signer, SnapshotStateStore()) + + private val target = + Event( + id = "b".repeat(64), + pubKey = "c".repeat(64), + createdAt = 1_790_000_000, + kind = 9, + tags = emptyArray(), + content = "react to me", + sig = "", + ) + + @Test + fun aReactionNamesItsTargetFirstAndIsAnUnsignedRumor() = + runBlocking { + val rumor = manager.buildReactionRumor(target, "🎉") + assertEquals(ReactionEvent.KIND, rumor.kind) + assertEquals("🎉", rumor.content) + assertEquals(signer.pubKey, rumor.pubKey) + assertTrue(rumor.sig.isEmpty(), "MIP-03 inner events are unsigned rumors") + // MDK attaches a reaction to the FIRST e tag. + assertEquals(target.id, rumor.tags.first { it[0] == "e" }[1]) + } + + @Test + fun aCustomEmojiReactionCarriesItsImage() = + runBlocking { + val rumor = manager.buildReactionRumor(target, ":soapbox:https://example.com/soapbox.png") + assertEquals(":soapbox:", rumor.content) + val emoji = rumor.tags.first { it[0] == "emoji" } + assertEquals(listOf("emoji", "soapbox", "https://example.com/soapbox.png"), emoji.take(3)) + } + + @Test + fun aDeletionTargetsEachRetractedEvent() = + runBlocking { + val rumor = manager.buildDeletionRumor(listOf(target)) + assertEquals(DeletionRequestEvent.KIND, rumor.kind) + assertEquals(signer.pubKey, rumor.pubKey) + assertTrue(rumor.sig.isEmpty()) + assertEquals(listOf(target.id), rumor.tags.filter { it[0] == "e" }.map { it[1] }) + } +}