mantra/mantra-kmp
1
0
Fork 0
Code Issues 2 Pull Requests Actions 1 Packages Projects Releases 1 Wiki Activity

fix: send a group event because it was queued, not because the chat mentions it

Browse Source 
No FROST signing message has ever reached another participant. The proposal
was built, MLS-encrypted, wrapped under the exporter secret, signed as a
kind:445, written to NostrEvent and MarmotGroupEvent, and its queue row
marked processed -- and then never handed to a relay, by a branch that was
never about delivery at all.

**The gate.** The tail of MarmotOutboundDao.encryptAndSendMarmotInnerEvent
looked up the transcript row for the queued rumor and did everything else
inside it:

    val chatMessageOrNull = database.chatMessageDao()
        .getChatMessagesByMarmotInnerEventId(marmotInnerEvent.id)
    chatMessageOrNull?.let { chatMessage ->
        ... relation, marmotGroupEventId ...
        val ids = database.broadcastNostrEventRequestDao().insert(...)
    }

The BroadcastNostrEventRequest rows are the only thing that puts a kind:445
on a relay -- observeBroadcastNostrEventRequestsByStatus("pending") is what
the broadcaster watches, and nothing else inserts them for this path. So the
question "does the chat have a line for this?" was silently answering the
question "should the group receive this?".

**Why FROST always lost.** A signing message has no ChatMessage by design.
FrostSigningManager.broadcast queues the rumor alone, and announce() writes
its milestone lines with marmotInnerEventId = null on purpose: each device
writes its own transcript from the messages it has already received, so the
lines cost no traffic and cannot disagree with the session they describe.
The inbound half states the same intent from the other side --
ChatMessage.applyInnerEvent returns null for every FrostSigningEvents kind,
because a row there would be a second, worse account of what the manager
already narrates.

That is every kind in the family, not just the proposal: nonces, the signer
set, partial signatures, the finished signature and the failure notice all
go through the same broadcast(). A session could not have completed even if
a proposal had somehow arrived.

**GroupKeyStateManager.announce had it too.** Same shape, same silence: a
room's kind:30326 announcement of which key it signs with was queued,
encrypted and dropped. a909108 added it so members would stop rederiving;
no member has ever received one.

**Why the neighbours worked, and hid it.** The DKG rides NIP-17 gift wraps
through a different path entirely, so a room could finish a ceremony, hold a
real shared key, and report canSign() == true with the signing transport
dead beneath it. nip30303 submissions work because MantraDao.sendMarmotInnerEvent
pairs every queued rumor with a ChatMessage carrying its id -- not as a
delivery mechanism, just because a submission is also something a member did.
FROST was the first traffic to use the group path without a chat line, which
is why this reads as a FROST bug and is not one.

**Why it went unnoticed.** Nothing failed. The coordinator's own device is
fully convinced: proposeSigning writes the session, announceStarted puts a
line in the chat, advance() runs, and publishOwn records the coordinator's
own nonce and announces that step too. From the proposer's side a session
nobody else can see is indistinguishable from one waiting on slow peers.

Unlike 65e4a3a, the queue did not block. marmotGroupEventId is set before
the transcript lookup, so the row left the queue cleanly and the next one
was picked up. Every message was lost individually, in silence, with no
backlog to notice.

**The fix.** The broadcast insert is hoisted out of the branch, and the
decision it was tangled with is lifted into MarmotDelivery.plan: given a
group event, a relay list, and a chat message or null, what has to be
written. A group event is sent because it was queued; a chat line is linked
because a member said something. The DAO now computes that plan and executes
it, with the insert as a plain unconditional statement ahead of the
bookkeeping that legitimately does depend on there being a line.

The extraction is not decoration. encryptAndSendMarmotInnerEvent is
Room-backed and cannot be stood up in a unit test, which is exactly how the
gate survived; separating the decision from the filing of it is the same
move MarmotDirectMessage.classify exists for, and for the same stated
reason.

**Tests.** MarmotDeliveryTest, six of them. The two that matter are "a
signing proposal goes out, though nothing in the chat points at it" and "the
send does not depend on the transcript", which asserts the broadcast list is
identical with and without a chat message. The rest pin the supporting
facts: one request per relay naming the event, every request written pending
because that is the only status the broadcaster looks at, the linkage that
does depend on a chat line, and an empty relay list as the sole legitimate
way to produce an empty broadcast list -- so that an empty list always reads
as "nowhere to send it" and never as "nothing to send".

**Not covered, deliberately.** These pin the decision, not the call site. Re-
nesting the insert inside chatMessageOrNull?.let would leave MarmotDelivery
correct and every test passing. Closing that needs the DAO itself under
test: BundledSQLiteDriver is on the classpath and getInMemoryDatabaseBuilder
exists, but its android actual wants a real Context, testDebugUnitTest is
plain JVM, and there is no androidUnitTest source set or Robolectric. That
is its own change, not one to smuggle in here.

Verified: :composeApp:compileDebugKotlinAndroid succeeds; 160 tests pass,
154 before these six. The inbound half was read rather than assumed --
NostrDao dispatches FrostSigningEvents kinds to processSigningPayload,
inbound rumors are stored with marmotGroupEventId set so they cannot re-enter
the outbound queue, and the out-of-order replay path is intact. Outbound was
the only break. That two participants now actually see a proposal is
inference from the code, not an observation: it wants two devices.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Kgothatso Ngako
2026-09-06 01:06:05 +02:00
parent 65e4a3acc0
commit 02117643c4
3 changed files with 183 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

30
composeApp/src/commonMain/kotlin/press/mantra/compose/database/dao/MarmotOutboundDao.kt
View File

@@ -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(

60
composeApp/src/commonMain/kotlin/press/mantra/compose/nostr/MarmotDelivery.kt Normal file
View File

@@ -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<BroadcastNostrEventRequest>,
/**
* 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<NormalizedRelayUrl>,
chatMessage: ChatMessage?,
): MarmotDelivery = MarmotDelivery(
broadcasts = relays.map { relay ->
BroadcastNostrEventRequest(
nostrEventId = groupEventId,
relayURL = relay.url,
)
},
transcriptChatMessageId = chatMessage?.id,
)
}
}

107
composeApp/src/commonTest/kotlin/press/mantra/compose/nostr/MarmotDeliveryTest.kt Normal file
View File

@@ -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)
}
}