diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/database/dao/MarmotOutboundDao.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/database/dao/MarmotOutboundDao.kt index 653d12ee..e6c5049f 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/database/dao/MarmotOutboundDao.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/database/dao/MarmotOutboundDao.kt @@ -22,6 +22,7 @@ import press.mantra.compose.database.model.intermdiate.LocalChatRoom import press.mantra.compose.extensions.exporterSecret import press.mantra.compose.extensions.toHex import press.mantra.compose.managers.MarmotInboundManager.EPOCH_RETENTION_WINDOW +import press.mantra.compose.nostr.MarmotDelivery import press.mantra.compose.nostr.MarmotDirectMessage import press.mantra.compose.nostr.Relays import co.touchlab.kermit.Logger @@ -653,13 +654,24 @@ abstract class MarmotOutboundDao( // Keyed on the queued row, not on `innerEvent.id`. For a direct message those // differ -- the row is the rumor, the wire event is the wrap built around it -- - // and the wrap's id matches no ChatMessage, so this lookup would come back null, - // the message would never be linked, and no BroadcastNostrEventRequest would ever - // be inserted. Encrypted, stored, and silently never sent. They are the same value - // for every other kind of message. + // and the wrap's id matches no ChatMessage, so this lookup comes back null. That + // costs only the transcript linkage, not the send. val chatMessageOrNull = database.chatMessageDao().getChatMessagesByMarmotInnerEventId(marmotInnerEvent.id) logger.d("chatMessageOrNull: $chatMessageOrNull") + + val delivery = MarmotDelivery.plan( + groupEventId = groupEvent.id, + relays = Relays.DefaultDMRelayList, // TODO: Get these from localChatRoom... + chatMessage = chatMessageOrNull, + ) + + // Sync broadcast to all the required relays. Unconditional, and before any + // transcript bookkeeping: see MarmotDelivery for what gating it on a chat line + // cost the signing sessions that have none. + val broadcastNostrEventRequestIds = + database.broadcastNostrEventRequestDao().insert(delivery.broadcasts) + chatMessageOrNull?.let { chatMessage -> database.chatMessageNostrEventRelationDao().upsert( ChatMessageNostrEventRelation( @@ -674,16 +686,6 @@ abstract class MarmotOutboundDao( ) ) - // Sync broadcast to all the required relays... - val broadcastNostrEventRequestIds = database.broadcastNostrEventRequestDao().insert( - Relays.DefaultDMRelayList.map { // TODO: Get these from localChatRoom... - BroadcastNostrEventRequest( - nostrEventId = groupEvent.id, - relayURL = it.url - ) - } - ) - broadcastNostrEventRequestIds.forEach { broadcastNostrEventRequestId -> database.chatMessageBroadcastNostrEventRequestRelationDao().upsert( ChatMessageBroadcastNostrEventRequestRelation( diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/nostr/MarmotDelivery.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/nostr/MarmotDelivery.kt new file mode 100644 index 00000000..c41d8a76 --- /dev/null +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/nostr/MarmotDelivery.kt @@ -0,0 +1,60 @@ +package press.mantra.compose.nostr + +import com.vitorpamplona.quartz.nip01Core.core.HexKey +import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl +import press.mantra.compose.database.model.BroadcastNostrEventRequest +import press.mantra.compose.database.model.ChatMessage + +/** + * What has to be written once a queued inner event has been encrypted into a + * group event: where it is sent, and which transcript row -- if any -- the send + * belongs to. + * + * The two are independent, and this exists to say so. A group event is sent + * because it was queued; a chat line is linked because a member said something. + * Letting the second decide the first is what silenced FROST signing: the + * broadcast rows were written inside `chatMessageOrNull?.let { }`, and protocol + * traffic has no ChatMessage -- `ChatMessage.applyInnerEvent` returns null for + * every [press.mantra.compose.nostr.frost.FrostSigningEvents] kind and for + * [press.mantra.compose.nostr.frost.GroupKeyStateEvent], and + * `FrostSigningManager` queues its messages without one. So every signing + * proposal was MLS-encrypted, wrapped, persisted and marked processed, and then + * never sent to a relay. Nothing failed; the other participants simply never saw + * it. + * + * Pulled out of `MarmotOutboundDao.encryptAndSendMarmotInnerEvent` because that + * function is Room-backed and cannot be unit tested, which is precisely how the + * gate survived. The decision is separated from the filing of it for the same + * reason [MarmotDirectMessage.classify] is. + */ +data class MarmotDelivery( + /** + * One request per relay, always. An empty list here means the event is on + * disk and going nowhere. + */ + val broadcasts: List, + /** + * The chat line this send belongs to, or null when the row is protocol + * traffic -- something the room did rather than something a member said. + */ + val transcriptChatMessageId: Long?, +) { + /** True when nothing in the chat points at this event, which is not a reason to withhold it. */ + val isProtocolTraffic: Boolean get() = transcriptChatMessageId == null + + companion object { + fun plan( + groupEventId: HexKey, + relays: Collection, + chatMessage: ChatMessage?, + ): MarmotDelivery = MarmotDelivery( + broadcasts = relays.map { relay -> + BroadcastNostrEventRequest( + nostrEventId = groupEventId, + relayURL = relay.url, + ) + }, + transcriptChatMessageId = chatMessage?.id, + ) + } +} diff --git a/composeApp/src/commonTest/kotlin/press/mantra/compose/nostr/MarmotDeliveryTest.kt b/composeApp/src/commonTest/kotlin/press/mantra/compose/nostr/MarmotDeliveryTest.kt new file mode 100644 index 00000000..5df982dc --- /dev/null +++ b/composeApp/src/commonTest/kotlin/press/mantra/compose/nostr/MarmotDeliveryTest.kt @@ -0,0 +1,107 @@ +package press.mantra.compose.nostr + +import press.mantra.compose.database.model.ChatMessage +import kotlin.test.Test +import kotlin.test.assertContentEquals +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +/** + * Whether an encrypted group event actually leaves the device. + * + * The bug this pins did not throw, log, or fail a build. `MarmotOutboundDao` + * wrote the broadcast rows inside `chatMessageOrNull?.let { }`, so an event with + * no chat line pointing at it was MLS-encrypted, wrapped, persisted, its queue + * row marked processed -- and never sent. FROST signing proposals and the room's + * key announcement are exactly that: traffic the room generates, which + * deliberately writes no ChatMessage of its own. Every proposal was silently + * delivered to nobody. + * + * The DAO around this is Room-backed and cannot be stood up here, which is how + * the gate survived unnoticed in the first place. So the decision is tested + * where it can be seen, and the DAO does nothing with it but write what it says. + */ +class MarmotDeliveryTest { + private val groupEventId = "a".repeat(64) + private val relays = Relays.DefaultDMRelayList + + private val chatLine = ChatMessage( + id = 42L, + senderPublicKey = "b".repeat(64), + isUserMessage = true, + giftWrapPayloadId = null, + marmotGroupEventId = null, + marmotInnerEventId = "c".repeat(64), + chatRoomId = "room", + content = "the vote is at six", + ) + + @Test + fun `a signing proposal goes out, though nothing in the chat points at it`() { + // The regression. A FROST message has no ChatMessage by design -- the manager + // writes its own transcript lines from what arrives, so a row here would be a + // second, worse account of the same thing -- and that must not be the reason the + // group never hears about it. + val delivery = MarmotDelivery.plan(groupEventId, relays, chatMessage = null) + + assertTrue(delivery.isProtocolTraffic) + assertEquals(relays.size, delivery.broadcasts.size) + assertTrue(delivery.broadcasts.isNotEmpty(), "an event on disk and going nowhere") + } + + @Test + fun `the send does not depend on the transcript`() { + // Said as directly as it can be said: the two questions are independent. Anything + // that makes a broadcast conditional on a chat line fails here. + val protocol = MarmotDelivery.plan(groupEventId, relays, chatMessage = null) + val spoken = MarmotDelivery.plan(groupEventId, relays, chatMessage = chatLine) + + assertContentEquals( + protocol.broadcasts.map { it.relayURL }, + spoken.broadcasts.map { it.relayURL }, + ) + assertEquals(protocol.broadcasts.size, spoken.broadcasts.size) + } + + @Test + fun `every relay gets a request, naming the event`() { + val delivery = MarmotDelivery.plan(groupEventId, relays, chatMessage = chatLine) + + assertContentEquals( + relays.map { it.url }, + delivery.broadcasts.map { it.relayURL }, + ) + assertTrue(delivery.broadcasts.all { it.nostrEventId == groupEventId }) + } + + @Test + fun `requests are queued pending, which is all the broadcaster looks at`() { + // `observeBroadcastNostrEventRequestsByStatus("pending")` is the only thing that + // picks these up. A request written in any other state is as unsent as no request. + val delivery = MarmotDelivery.plan(groupEventId, relays, chatMessage = null) + + assertTrue(delivery.broadcasts.all { it.status == "pending" }) + } + + @Test + fun `a member's message is linked to its chat line`() { + // The bookkeeping that legitimately does depend on there being a chat line: the + // transcript needs to know which event carried the words, so a sent message can + // be shown as sent. + val delivery = MarmotDelivery.plan(groupEventId, relays, chatMessage = chatLine) + + assertFalse(delivery.isProtocolTraffic) + assertEquals(chatLine.id, delivery.transcriptChatMessageId) + } + + @Test + fun `no relays is the only way an event stays home`() { + // Worth pinning as the single legitimate empty case, so an empty broadcast list + // is always read as "nowhere to send it" and never as "nothing to send". + val delivery = MarmotDelivery.plan(groupEventId, relays = emptyList(), chatMessage = chatLine) + + assertTrue(delivery.broadcasts.isEmpty()) + assertEquals(chatLine.id, delivery.transcriptChatMessageId) + } +}