From 226653d2828b1817d671f807b03fcb84939556d0 Mon Sep 17 00:00:00 2001 From: Kgothatso Ngako Date: Fri, 2 Oct 2026 22:59:14 +0200 Subject: [PATCH] fix(frost): one transcript line for a wave, not one per dialect A signed event files its chat line unless the batch holds more than one event of its kind, in which case the session's completion line speaks for them. Needed by the previous commit. A wave of fifty dialects would otherwise post fifty "Added X to dialects" lines into the group's conversation, one after another, followed by a line saying the group signed fifty events -- a single act burying the conversation it was announced in. Every kind that could already appear many-per-batch returns null from applyInnerEvent for precisely this reason: a chunk is part of a chapter and the chapter's line speaks for it, and likewise a translation chapter, a contributor list and both project memberships. DialectEvent is the first kind where a batch holds many peers each of which wants a line. So checked against every batch the app builds today, this changes nothing: the leading kind of each is one of its kind and keeps its line. It is not a rule about batch size, which is the mistake available here. A chapter and its sixty-four chunks is one ChapterEvent, and gating on the batch being larger than one event would have taken the chapter's line away. Decided at the call site because that is the only place that can see the batch. applyInnerEvent is untouched -- it still writes the row, which is the part that matters -- and only the chatMessageDao upsert is skipped. ChronicleManager needed nothing: it already drops the line a payload returns, and says why. Three tests on SignedGroupKeyStateTest, which runs real two-device sessions: one dialect still files its line, three file none and still land three rows, and a dialect with the artifact that names it files both. The first two are the same mechanism asserted at 1 and at 0, so neither can pass vacuously. Co-Authored-By: Claude Opus 5 --- .../compose/managers/FrostSigningManager.kt | 37 ++++- .../managers/SignedGroupKeyStateTest.kt | 126 ++++++++++++++++++ 2 files changed, 160 insertions(+), 3 deletions(-) diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/managers/FrostSigningManager.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/managers/FrostSigningManager.kt index 59798fd1..bdcc7bf9 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/managers/FrostSigningManager.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/managers/FrostSigningManager.kt @@ -975,7 +975,32 @@ object FrostSigningManager { // re-authors rumors as their sender and would strip the group's // signature off. Sending one to relays is a member's separate act, // from `BroadcastGroupSignedEventScreen`, off the recorded row. - signedEvents.forEach { applySignedEvent(database, session, it, recorded) } + // + // A line per event, *unless the batch holds more than one of its kind*, + // in which case the completion line below speaks for them. Peers in a + // batch are the case that needs it: a wave of fifty dialects would + // otherwise post fifty "Added X to dialects" lines into the group's + // conversation, one after the other, followed by a line saying the group + // signed fifty events. + // + // Every kind that could already appear many-per-batch returns null from + // `applyInnerEvent` for precisely this reason -- a chunk is part of a + // chapter and the chapter's line speaks for it, and likewise a + // translation chapter, a contributor list and both project memberships -- + // so this changes nothing for any batch built before the dialect wave. + // What it is not is a rule about batch *size*: a chapter and its + // sixty-four chunks is one ChapterEvent, and its line stays. + val kinds = signedEvents.groupingBy { it.kind }.eachCount() + + signedEvents.forEach { signedEvent -> + applySignedEvent( + database = database, + session = session, + signedEvent = signedEvent, + recorded = recorded, + fileChatLine = kinds[signedEvent.kind] == 1, + ) + } announce( database = database, @@ -1040,12 +1065,18 @@ object FrostSigningManager { * A failure here is not the session's: the signature is made and valid, and * saying otherwise would tell the group to abandon a ceremony that * succeeded. It is logged and the session still completes. + * + * [fileChatLine] is false for an event the batch holds several of a kind of; + * the row is written either way, and only the transcript line is skipped. + * See the rule where it is decided, at the call site -- which is the only + * place that can see the batch, and so the only place that can know. */ private suspend fun applySignedEvent( database: MantraDatabase, session: FrostSigningSession, signedEvent: Event, - recorded: Boolean + recorded: Boolean, + fileChatLine: Boolean = true ) { try { ChatMessage.applyInnerEvent( @@ -1058,7 +1089,7 @@ object FrostSigningManager { isUserMessage = session.isCoordinator(), createdAt = Clock.System.now(), groupSignedEventId = signedEvent.id.takeIf { recorded } - )?.let { database.chatMessageDao().upsert(it) } + )?.takeIf { fileChatLine }?.let { database.chatMessageDao().upsert(it) } } catch (e: Throwable) { logger.e("Signed ${signedEvent.id} but could not apply it locally", e) } diff --git a/composeApp/src/jvmTest/kotlin/press/mantra/compose/managers/SignedGroupKeyStateTest.kt b/composeApp/src/jvmTest/kotlin/press/mantra/compose/managers/SignedGroupKeyStateTest.kt index 1e85b6a7..8804a231 100644 --- a/composeApp/src/jvmTest/kotlin/press/mantra/compose/managers/SignedGroupKeyStateTest.kt +++ b/composeApp/src/jvmTest/kotlin/press/mantra/compose/managers/SignedGroupKeyStateTest.kt @@ -48,6 +48,7 @@ import press.mantra.compose.extensions.toHex import press.mantra.compose.nostr.dkg.DkgRitualEvents import press.mantra.compose.nostr.frost.FrostSigningEvents import press.mantra.compose.nostr.frost.GroupKeyStateEvent +import press.mantra.compose.nostr.nip30303.ArtifactEvent import press.mantra.compose.nostr.nip30303.DialectEvent /** @@ -198,6 +199,13 @@ class SignedGroupKeyStateTest { /** Everything the group has signed here, as the group signed it. */ suspend fun signedEvents() = db.groupSignedEventDao().getByChatRoomId(roomId) + + /** How many transcript lines of [messageType] this room holds. */ + suspend fun linesOfType(messageType: String) = + db.chatMessageDao().getChatMessagesByChatRoomId(roomId) + .count { it.chatMessage.messageType == messageType } + + suspend fun dialectLines() = linesOfType("dialect") } private suspend fun device( @@ -1071,6 +1079,124 @@ class SignedGroupKeyStateTest { } } + // ---- What a batch says in the transcript ------------------------------ + + /** + * A dialect signed on its own still announces itself. + * + * The other half of the rule below, and the half that has to keep working: + * the hand-typed form proposes exactly this, and a group that adds one + * dialect should see a line saying so. + */ + @Test + fun `a dialect signed on its own files its line`() = runBlocking { + val creator = device(members[0], signerIndex = 0) + val other = device(members[1], signerIndex = 1) + + val session = FrostSigningManager.proposeSigningBatch( + database = creator.db, + localChatRoom = creator.room, + userPublicKey = creator.publicKey, + events = dialects().take(1) + ) + pump(creator, other) + FrostSigningManager.approve(other.db, other.room, session.id) + pump(creator, other) + + assertEquals(1, creator.dialectLines(), "one dialect, one line") + } + + /** + * A wave says it once. + * + * Fifty dialects filing fifty "Added X to dialects" lines would bury the + * group's conversation under a single act, so a batch holding more than one + * event of a kind leaves the talking to its completion line. The rows are + * written either way -- that is the part a member came for, and it is + * asserted here rather than taken on trust. + */ + @Test + fun `a wave of dialects files one completion line and not one per dialect`() = runBlocking { + val creator = device(members[0], signerIndex = 0) + val other = device(members[1], signerIndex = 1) + + val wave = dialects() + val session = FrostSigningManager.proposeSigningBatch( + database = creator.db, + localChatRoom = creator.room, + userPublicKey = creator.publicKey, + events = wave + ) + pump(creator, other) + FrostSigningManager.approve(other.db, other.room, session.id) + pump(creator, other) + + listOf(creator, other).forEach { device -> + assertEquals( + 0, + device.dialectLines(), + "a wave of ${wave.size} should not file a line each" + ) + + assertEquals( + 1, + device.linesOfType(ChatMessage.TYPE_FROST_COMPLETE), + "the session's own line is what speaks for the wave" + ) + + // The dialects themselves still arrive. Only the transcript is quiet. + assertEquals( + wave.size, + device.items(session.id).count { + device.db.mantraDialectDao().getDialectById(it.eventId) != null + } + ) + } + } + + /** + * A batch of different kinds keeps every line. + * + * This is the shape every batch built before the dialect wave has -- one + * thing and its parts, where the parts file nothing of their own -- so it is + * the case the rule must not have broken. A dialect and the artifact that + * names it: two kinds, one of each, two lines. + * + * The artifact has to be built from what the proposal hands back, since its + * `dialectId` is a hash over the group's key at the room's path and the + * dialect is not a row until item 0 is applied. + */ + @Test + fun `a batch of one dialect and one artifact files both lines`() = runBlocking { + val creator = device(members[0], signerIndex = 0) + val other = device(members[1], signerIndex = 1) + + val session = FrostSigningManager.proposeSigningBatch( + database = creator.db, + localChatRoom = creator.room, + userPublicKey = creator.publicKey, + lead = dialects().first(), + dependents = { dialect -> + listOf( + ArtifactEvent.build( + name = "Frank Talk", + url = "https://example.invalid/frank-talk", + visibility = "public", + license = "CC0", + dialectId = dialect.id, + versionLabel = "1", + ) + ) + } + ) + pump(creator, other) + FrostSigningManager.approve(other.db, other.room, session.id) + pump(creator, other) + + assertEquals(1, creator.dialectLines(), "the dialect is one of its kind here") + assertEquals(1, creator.linesOfType("artifact"), "and so is the artifact") + } + /** * A proposal is the one place a remote party decides how much work everybody * else does, so the size cap is checked on arrival and not only when