From e22a8ae4cdf9f48590fb8919d794c69faf0ba405 Mon Sep 17 00:00:00 2001 From: Kgothatso Ngako Date: Sun, 6 Sep 2026 01:13:42 +0200 Subject: [PATCH 1/2] fix: stop asking a member to review a signature the group has settled The transcript's "Review" affordance is a promise: tapping it leads to a decision still there to be made. For a FROST signing proposal it was only ever withdrawn one way -- and a proposal can be processed three. **How a request was closed.** RitualNotice drops the tint and the call to action when the request is answered, and a request counts as answered when the step it asked for has since been published by this device: FROST_REQUEST_FULFILMENTS = mapOf(TYPE_FROST_APPROVAL_NEEDED to TYPE_FROST_NONCE) Approving publishes a nonce, so approving closes it. Nothing else does. **Declining.** decline() fails the session and broadcasts a FAILURE. It publishes nothing of the member's own, by design -- a refusal is a refusal. So no fulfilment line is ever written, and the request went on asking, in primary tint, for a decision the member had already made. Tapping it reached a screen with no buttons on it, which was the screen being right. **A quorum that did not need them.** A t-of-n key finishes without everybody. The coordinator takes the first t nonces, and a member whose phone was in a pocket is simply not among them -- but advance() returned at the approval gate on their device, so the arriving SIGNATURE was stored and nothing was done with it. Their session sat at COLLECTING_NONCES forever. The request stayed lit, the screen still offered Sign and Don't sign, and both answers were wrong: a nonce nobody was waiting for, or a refusal that would flip a COMPLETE session to FAILED on every device and announce "Nothing was signed" to a group holding the signature. fail() writes the stage with update() rather than moveTo(), so that last one was reachable. **The transcript.** A request is now closed by being *answered* or by being *settled* -- a frostComplete or frostFailed line after it. The two are kept apart deliberately. Answered keeps the tick; settled does not, because the member never answered and crediting them with a signature they refused, or were never asked for, is worse than the summons was. Both rules moved out of the composable onto ChatMessage, where they are stated once and tested. Settlement is signing-only: a ceremony step can only be taken or waited for, so a DKG request has no equivalent and reading one from a signing session's end would drop a summons the ritual is still stalled on. **The session.** The transcript alone could not close the third case: the device that never approved wrote no terminal line to read. advance() now completes on a signature that has already arrived, ahead of the approval gate rather than below it. That gate is there to keep this device's own material off the wire, and finishing puts none there -- it verifies the aggregate, applies the event and announces, all from what is already stored. Everything it now skips on that path is work the signature made pointless anyway: a late nonce, a partial signature nobody will aggregate. Three things follow. isAwaitingApproval reports false, so FrostSigningScreen hides the buttons -- it now asks the manager rather than re-deriving the rule, which had drifted into a second copy of it. A late "Don't sign" cannot abandon a signature that exists. And the signed event finally lands locally for a member who never approved: applySignedEvent sat below the gate and was being skipped, so a dialect the group signed without them never reached their store. Verified: :composeApp:compileDebugKotlinAndroid succeeds, and :composeApp:testDebugUnitTest passes -- 165 tests, 16 of them new. Eight cover the transcript rules against a hand-built row list; eight cover isAwaitingApproval, including the settled-signature case. What stays uncovered is advance() itself, which is Room-backed. Co-Authored-By: Claude Opus 5 --- .../compose/database/model/ChatMessage.kt | 58 ++++++++ .../compose/managers/FrostSigningManager.kt | 105 +++++++++----- .../ui/composable/FrostSigningScreen.kt | 9 +- .../ui/view/model/ChatMessageListViewModel.kt | 39 +++--- .../model/TranscriptRequestStateTest.kt | 131 ++++++++++++++++++ .../compose/managers/FrostSigningRoundTest.kt | 35 +++++ 6 files changed, 313 insertions(+), 64 deletions(-) create mode 100644 composeApp/src/commonTest/kotlin/press/mantra/compose/database/model/TranscriptRequestStateTest.kt diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/ChatMessage.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/ChatMessage.kt index 00104db4..ddb88a63 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/ChatMessage.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/database/model/ChatMessage.kt @@ -241,6 +241,21 @@ data class ChatMessage( /** The signing lines that ask rather than report. */ val FROST_REQUEST_TYPES = setOf(TYPE_FROST_APPROVAL_NEEDED) + /** + * The signing lines that end a session, whichever way it went. + * + * A request usually stops being one by being answered, but that is not + * the only way. A member who declines publishes nothing, and a quorum + * that signs without them asks them for nothing further -- from the + * transcript both look the same, nothing of the reader's own either + * side of the request, so the session's own ending is the only thing + * left to read it from. + * + * A ceremony has no equivalent because a ceremony step can only be + * taken or waited for. Signing is the one thing a member can refuse. + */ + val FROST_SETTLEMENTS = setOf(TYPE_FROST_COMPLETE, TYPE_FROST_FAILED) + /** Every signing line, for rendering them as system lines rather than bubbles. */ val FROST_TYPES = setOf( TYPE_FROST_STARTED, @@ -278,6 +293,49 @@ data class ChatMessage( TYPE_DKG_FAILED, ) + /** + * The request lines this device has since answered, by row id. + * + * A request is answered when the step it asked for has been published by + * this device -- which is exactly what approving it does. The transcript + * already records that as an authored line, so the answer is read from + * the list rather than from the session, and it stays right for a room + * that has run more than one ceremony. + */ + fun answeredRequests(messages: List): Set = messages + .mapNotNull { request -> + val published = (DKG_REQUEST_FULFILMENTS + FROST_REQUEST_FULFILMENTS)[request.messageType] + ?: return@mapNotNull null + + val done = messages.any { + it.messageType == published && + it.isUserMessage && + it.createdAt >= request.createdAt + } + + request.id.takeIf { done } + } + .toSet() + + /** + * The signing requests that are no longer open, by row id. + * + * Not the same thing as [answeredRequests], and deliberately kept apart + * from it: these are requests the reader never answered, so they have + * earned no tick and claiming otherwise would credit them with a + * signature they refused or were never needed for. What they have run + * out of is a decision to make -- see [FROST_SETTLEMENTS]. + */ + fun settledRequests(messages: List): Set = messages + .filter { request -> + request.messageType in FROST_REQUEST_TYPES && + messages.any { + it.messageType in FROST_SETTLEMENTS && it.createdAt >= request.createdAt + } + } + .map { it.id } + .toSet() + /** * Files one direct message, from whichever side of it this device is on. * 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 5fbdbbaf..09a7f085 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/managers/FrostSigningManager.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/managers/FrostSigningManager.kt @@ -464,6 +464,19 @@ object FrostSigningManager { ?: return try { + // A signature the group has already made settles this session whether + // or not its owner ever answered: a t-of-n key does not need everybody, + // so a quorum can finish while one member's phone is still in a pocket. + // Ahead of the gate below because that gate is about keeping this + // device's own material off the wire, and finishing puts none of it + // there -- while going on to ask would be asking for a decision that + // can no longer change anything, and would let a late "don't sign" + // abandon a signature that exists. + if (session.signature != null) { + complete(database, session) + return + } + // Nothing of this device's own goes out before its owner has said so. // Returns rather than throws: the session is not failing, it is waiting // on a person, and everything received stays stored so it resumes the @@ -580,43 +593,7 @@ object FrostSigningManager { announceStep(database, session, FrostSigningEvents.SIGNATURE, session.userPublicKey) } - val signature = session.signature ?: return - - // The payoff: a signature that verifies is one the group made, whoever - // relayed it. Checking rather than trusting is what keeps a faulty or - // dishonest coordinator from passing off something that will be - // rejected by every relay it reaches. - val signedEvent = signedEvent(session, signature) - val verified = Nip01Crypto.verify( - signature = signature.hexToByteArray(), - hash = session.eventId.hexToByteArray(), - pubKey = signedEvent.pubKey.hexToByteArray() - ) - if (!verified) { - throw IllegalStateException("The aggregated signature does not verify against ${session.eventId}") - } - - update(database, session) { it.copy(stage = FrostSigningStage.COMPLETE) } - - // A signature exists to be used. Every device has the event and the - // signature by now, so each applies the result itself rather than - // waiting to be sent something it can already build -- the same - // reasoning the transcript lines are written on. Nothing goes on the - // wire: a signed event authored by the threshold key cannot travel as - // an inner event anyway, because the outbound pipeline re-authors - // rumors as their sender and would strip the group's signature off. - applySignedEvent(database, session, signedEvent) - - announce( - database = database, - session = session, - messageType = ChatMessage.TYPE_FROST_COMPLETE, - content = "The group signed the event. It took ${session.threshold} of " + - "${session.participantCount} members.", - actor = session.coordinatorPublicKey - ) - - logger.i("Signing session $sessionId complete") + complete(database, session) } catch (e: CancellationException) { // The sync was torn down mid-step, which says nothing about the session. throw e @@ -633,6 +610,54 @@ object FrostSigningManager { } } + /** + * Verifies the group's signature, uses the event, and closes the session. + * + * The payoff: a signature that verifies is one the group made, whoever + * relayed it. Checking rather than trusting is what keeps a faulty or + * dishonest coordinator from passing off something that will be rejected by + * every relay it reaches. + * + * Reached only with the session still open -- [advance] returns above this + * on a settled one -- so the milestone is written once by construction, + * like every other line in the transcript. + */ + private suspend fun complete(database: MantraDatabase, session: FrostSigningSession) { + val signature = session.signature ?: return + + val signedEvent = signedEvent(session, signature) + val verified = Nip01Crypto.verify( + signature = signature.hexToByteArray(), + hash = session.eventId.hexToByteArray(), + pubKey = signedEvent.pubKey.hexToByteArray() + ) + if (!verified) { + throw IllegalStateException("The aggregated signature does not verify against ${session.eventId}") + } + + update(database, session) { it.copy(stage = FrostSigningStage.COMPLETE) } + + // A signature exists to be used. Every device has the event and the + // signature by now, so each applies the result itself rather than + // waiting to be sent something it can already build -- the same + // reasoning the transcript lines are written on. Nothing goes on the + // wire: a signed event authored by the threshold key cannot travel as + // an inner event anyway, because the outbound pipeline re-authors + // rumors as their sender and would strip the group's signature off. + applySignedEvent(database, session, signedEvent) + + announce( + database = database, + session = session, + messageType = ChatMessage.TYPE_FROST_COMPLETE, + content = "The group signed the event. It took ${session.threshold} of " + + "${session.participantCount} members.", + actor = session.coordinatorPublicKey + ) + + logger.i("Signing session ${session.id} complete") + } + /** * Turns the signed event into whatever it is: a dialect, an artifact, a * chapter. @@ -974,6 +999,12 @@ object FrostSigningManager { return false } + // The group signed it without needing this member, and [advance] closes + // the session on its next pass without asking them anything. Offering the + // decision anyway would be offering two bad answers: a nonce nobody is + // waiting for, or a refusal that abandons a signature already made. + if (session.signature != null) return false + return session.signApprovedAt == null } diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/FrostSigningScreen.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/FrostSigningScreen.kt index 38282f75..a495ad9c 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/FrostSigningScreen.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/FrostSigningScreen.kt @@ -48,6 +48,7 @@ import press.mantra.compose.database.model.ChatRoom import press.mantra.compose.database.model.FrostSigningSession import press.mantra.compose.database.model.intermdiate.LocalChatRoom import press.mantra.compose.database.model.types.FrostSigningStage +import press.mantra.compose.managers.FrostSigningManager import press.mantra.compose.nostr.nip30303.ArtifactEvent import press.mantra.compose.nostr.nip30303.ChapterEvent import press.mantra.compose.nostr.nip30303.DialectEvent @@ -223,10 +224,10 @@ fun FrostSigningScreen( } } - if (session.signApprovedAt == null && - session.stage != FrostSigningStage.COMPLETE && - session.stage != FrostSigningStage.FAILED - ) { + // Asked rather than re-derived: the manager owns when a session + // is still waiting on its owner, and a second copy of that rule + // here is a second copy to keep in step. + if (FrostSigningManager.isAwaitingApproval(session)) { HorizontalDivider() Text( diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/ChatMessageListViewModel.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/ChatMessageListViewModel.kt index 2a9c0b93..4ba70a91 100755 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/ChatMessageListViewModel.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/ChatMessageListViewModel.kt @@ -283,30 +283,17 @@ class ChatMessageListViewModel( ) } else { - // A request line is answered when the step it asked for - // has since been published by this device -- which is - // exactly what approving it does. The transcript already - // records that as an authored line, so the answer is here - // in the list rather than in the session, and it stays - // right for a room that has run more than one ceremony. - val answeredRequests = chatRoomDetailMessageListUIState + // Which request lines are still asking something of the + // reader. Read off the transcript rather than the session + // -- the rows are what a line rendered days later has -- + // and both rules live on ChatMessage, where they can be + // stated once and tested. + val messages = chatRoomDetailMessageListUIState .chatMessageList - .mapNotNull { request -> - val published = ( - ChatMessage.DKG_REQUEST_FULFILMENTS + - ChatMessage.FROST_REQUEST_FULFILMENTS - )[request.chatMessage.messageType] - ?: return@mapNotNull null + .map { it.chatMessage } - val done = chatRoomDetailMessageListUIState.chatMessageList.any { - it.chatMessage.messageType == published && - it.chatMessage.isUserMessage && - it.chatMessage.createdAt >= request.chatMessage.createdAt - } - - request.chatMessage.id.takeIf { done } - } - .toSet() + val answeredRequests = ChatMessage.answeredRequests(messages) + val settledRequests = ChatMessage.settledRequests(messages) LazyColumn( modifier = Modifier.fillMaxWidth().padding(5.dp), @@ -366,6 +353,7 @@ class ChatMessageListViewModel( RitualNotice( localChatMessage = localChatMessage, isAnswered = localChatMessage.chatMessage.id in answeredRequests, + isSettled = localChatMessage.chatMessage.id in settledRequests, onClick = onOpenSharedKey ) return@items @@ -394,6 +382,7 @@ class ChatMessageListViewModel( RitualNotice( localChatMessage = localChatMessage, isAnswered = localChatMessage.chatMessage.id in answeredRequests, + isSettled = localChatMessage.chatMessage.id in settledRequests, onClick = onOpenSigning ) return@items @@ -663,6 +652,7 @@ private fun PrivateMessageNotice( private fun RitualNotice( localChatMessage: LocalChatMessage, isAnswered: Boolean, + isSettled: Boolean, onClick: () -> Unit, ) { val chatMessage = localChatMessage.chatMessage @@ -704,10 +694,13 @@ private fun RitualNotice( // quiet; these are not. // An answered request is history, not a summons: it keeps its stage's icon so // the step is still recognisable, but drops the colour and the call to action. + // So is a settled one -- declined, or signed by a quorum that did not need this + // member. Nothing was answered there, so it gets no tick, but offering to + // review it would be offering a decision that has already gone by. val isRequest = ( chatMessage.messageType in ChatMessage.DKG_REQUEST_TYPES || chatMessage.messageType in ChatMessage.FROST_REQUEST_TYPES - ) && !isAnswered + ) && !isAnswered && !isSettled val tint = when { chatMessage.messageType == ChatMessage.TYPE_DKG_FAILED || diff --git a/composeApp/src/commonTest/kotlin/press/mantra/compose/database/model/TranscriptRequestStateTest.kt b/composeApp/src/commonTest/kotlin/press/mantra/compose/database/model/TranscriptRequestStateTest.kt new file mode 100644 index 00000000..4086c3d1 --- /dev/null +++ b/composeApp/src/commonTest/kotlin/press/mantra/compose/database/model/TranscriptRequestStateTest.kt @@ -0,0 +1,131 @@ +package press.mantra.compose.database.model + +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.time.Instant + +/** + * When a request line in a transcript stops asking for something. + * + * The transcript renders a request with a tint and a "Review" affordance, and + * that is a promise: tapping it leads to a decision still there to be made. + * Keeping the promise means knowing when the decision has gone, and the rows are + * all there is to know it from -- a line rendered days later has no session to + * ask, and the room may have signed several things since. + * + * Two ways for a request to be over, and they are not the same. Answering it + * leaves a line of the reader's own and earns the tick. A session ending + * underneath it leaves nothing of theirs at all: a member who declined published + * nothing, and a quorum that signed without them wanted nothing. Both must drop + * the summons; neither may claim the member signed. + */ +class TranscriptRequestStateTest { + private val user = "u".repeat(64) + private val other = "o".repeat(64) + + private var lastId = 0L + + /** One transcript row, with only the four fields either rule reads. */ + private fun line( + type: String, + at: Long, + sender: String = user + ) = ChatMessage( + id = ++lastId, + senderPublicKey = sender, + isUserMessage = sender == user, + giftWrapPayloadId = null, + marmotGroupEventId = null, + marmotInnerEventId = null, + chatRoomId = "room", + content = "", + messageType = type, + createdAt = Instant.fromEpochSeconds(at) + ) + + @Test + fun `a signing request nobody has acted on is still asking`() { + val request = line(ChatMessage.TYPE_FROST_APPROVAL_NEEDED, at = 10) + val transcript = listOf(line(ChatMessage.TYPE_FROST_STARTED, at = 9, sender = other), request) + + assertEquals(emptySet(), ChatMessage.answeredRequests(transcript)) + assertEquals(emptySet(), ChatMessage.settledRequests(transcript)) + } + + @Test + fun `publishing the nonce answers the request that asked for it`() { + val request = line(ChatMessage.TYPE_FROST_APPROVAL_NEEDED, at = 10) + val transcript = listOf(request, line(ChatMessage.TYPE_FROST_NONCE, at = 11)) + + assertEquals(setOf(request.id), ChatMessage.answeredRequests(transcript)) + } + + @Test + fun `somebody else's nonce answers nothing`() { + // The fulfilment has to be this device's own: a transcript is full of other + // members taking the step this reader has yet to take. + val request = line(ChatMessage.TYPE_FROST_APPROVAL_NEEDED, at = 10) + val transcript = listOf(request, line(ChatMessage.TYPE_FROST_NONCE, at = 11, sender = other)) + + assertEquals(emptySet(), ChatMessage.answeredRequests(transcript)) + } + + @Test + fun `declining settles the request without claiming it was signed`() { + // Declining publishes nothing, so there is no fulfilment to find. The + // failure the refusal writes is the only trace, and it has to be enough -- + // otherwise the line goes on offering a decision already made. + val request = line(ChatMessage.TYPE_FROST_APPROVAL_NEEDED, at = 10) + val transcript = listOf(request, line(ChatMessage.TYPE_FROST_FAILED, at = 11)) + + assertEquals(setOf(request.id), ChatMessage.settledRequests(transcript)) + assertEquals(emptySet(), ChatMessage.answeredRequests(transcript)) + } + + @Test + fun `a group that signs without this member settles their request`() { + // A t-of-n key does not need everybody. Nothing of this member's is in the + // signature and nothing of theirs was ever published, so answered stays + // empty -- but there is no longer anything for them to decide. + val request = line(ChatMessage.TYPE_FROST_APPROVAL_NEEDED, at = 10) + val transcript = listOf(request, line(ChatMessage.TYPE_FROST_COMPLETE, at = 12, sender = other)) + + assertEquals(setOf(request.id), ChatMessage.settledRequests(transcript)) + assertEquals(emptySet(), ChatMessage.answeredRequests(transcript)) + } + + @Test + fun `an earlier session's ending does not close a later request`() { + // Rooms sign more than once, and the previous session's last line sits + // above this one's first. + val request = line(ChatMessage.TYPE_FROST_APPROVAL_NEEDED, at = 20) + val transcript = listOf(line(ChatMessage.TYPE_FROST_COMPLETE, at = 9, sender = other), request) + + assertEquals(emptySet(), ChatMessage.settledRequests(transcript)) + } + + @Test + fun `a ceremony step is settled by nothing`() { + // Signing is the one thing a member can refuse, so it is the only place a + // request can be over without them having answered it. A ceremony step is + // either taken or still waited on, and reading either ending as the end of + // one would drop a summons the ritual is still stalled on. + val request = line(ChatMessage.TYPE_DKG_APPROVAL_NEEDED_ROUND_1, at = 10) + val transcript = listOf( + request, + line(ChatMessage.TYPE_FROST_FAILED, at = 11), + line(ChatMessage.TYPE_DKG_FAILED, at = 12, sender = other) + ) + + assertEquals(emptySet(), ChatMessage.settledRequests(transcript)) + } + + @Test + fun `each ceremony step is answered only by its own`() { + val hostKey = line(ChatMessage.TYPE_DKG_APPROVAL_NEEDED_HOST_KEY, at = 10) + val roundOne = line(ChatMessage.TYPE_DKG_APPROVAL_NEEDED_ROUND_1, at = 12) + val transcript = listOf(hostKey, line(ChatMessage.TYPE_DKG_HOST_KEY, at = 11), roundOne) + + assertEquals(setOf(hostKey.id), ChatMessage.answeredRequests(transcript)) + } +} diff --git a/composeApp/src/commonTest/kotlin/press/mantra/compose/managers/FrostSigningRoundTest.kt b/composeApp/src/commonTest/kotlin/press/mantra/compose/managers/FrostSigningRoundTest.kt index 0d4fc4c5..391c5998 100644 --- a/composeApp/src/commonTest/kotlin/press/mantra/compose/managers/FrostSigningRoundTest.kt +++ b/composeApp/src/commonTest/kotlin/press/mantra/compose/managers/FrostSigningRoundTest.kt @@ -14,11 +14,13 @@ import fr.acinq.bitcoin.crypto.frost.Session import fr.acinq.bitcoin.crypto.frost.TweakCache import fr.acinq.secp256k1.Hex import kotlin.test.Test +import kotlin.time.Instant import kotlin.test.assertEquals import kotlin.test.assertFalse import kotlin.test.assertTrue import press.mantra.compose.database.model.DkgSession import press.mantra.compose.database.model.FrostSigningSession +import press.mantra.compose.database.model.types.FrostSigningStage import press.mantra.compose.extensions.toHex import press.mantra.compose.nostr.frost.FrostSigningEvents @@ -217,6 +219,39 @@ class FrostSigningSessionTest { signerIds = signerIds ) + @Test + fun `a session waits on its owner until they answer`() { + val open = session(signerId = 2, signerIds = null) + + assertTrue(FrostSigningManager.isAwaitingApproval(open)) + assertFalse( + FrostSigningManager.isAwaitingApproval( + open.copy(signApprovedAt = Instant.fromEpochSeconds(1)) + ) + ) + } + + @Test + fun `a session that has settled asks its owner nothing`() { + val open = session(signerId = 2, signerIds = null) + + assertFalse(FrostSigningManager.isAwaitingApproval(open.copy(stage = FrostSigningStage.COMPLETE))) + assertFalse(FrostSigningManager.isAwaitingApproval(open.copy(stage = FrostSigningStage.FAILED))) + } + + @Test + fun `a signature the group already made asks its owner nothing either`() { + // A t-of-n key does not need everybody, so a quorum can finish while one + // member's phone is still in a pocket. The session stays at its opening + // stage on their device until it next advances, and offering them the + // decision in that window offers two bad answers: a nonce nobody is + // waiting for, or a refusal that abandons a signature that exists. + val signedWithoutThem = session(signerId = 2, signerIds = "0,1") + .copy(signature = "a".repeat(128)) + + assertFalse(FrostSigningManager.isAwaitingApproval(signedWithoutThem)) + } + @Test fun `a member left out of the signer set is not a signer`() { assertTrue(session(signerId = 1, signerIds = "0,1").isSigner()) From 024da9940434fdc1c201553e7982342a1198de78 Mon Sep 17 00:00:00 2001 From: Kgothatso Ngako Date: Sun, 6 Sep 2026 01:26:52 +0200 Subject: [PATCH 2/2] test: pin what a member who never took part needs to finish a session e22a8ae hoisted completion above the approval gate on the strength of one claim: closing a session needs nothing secret, and nothing the member would have had to publish. Were that false -- were the aggregated nonce, the signer set or a share needed to check the result -- the gate would have to stay where it was, and the member a quorum did not need would go on being asked to sign something already signed. Nothing checked the claim. FrostSigningCompletionTest builds a real 2-of-3 signature from members 0 and 1, then works entirely from member 2's row: never approved, not in the signer set, aggregatedNonce and signerIds deliberately null. From that alone it pins that they can verify what the group signed, that the finished event is the proposed one unaltered rather than rebuilt or rehashed, that there is no finished event before the signature arrives, that the arrived signature is what stops the session asking, and that a signature over a different event is refused -- which is what the check in complete() is for. Both new assertions about isAwaitingApproval were mutation-checked: with the `signature != null` guard removed, exactly two tests fail and the rest of the suite still passes, so they guard the change rather than restating it. Not covered, and not coverable here: advance() itself -- that the branch fires on an inbound SIGNATURE rather than stopping at the gate. It is Room-backed, and this project has no harness for that (no Robolectric, and the in-memory builder's android actual needs a Context). The pure half of the claim is what this pins instead. Also corrects e22a8ae's message, which said sixteen new tests. It was eleven: eight in TranscriptRequestStateTest and three added to FrostSigningSessionTest, which has eight in total. Verified: :composeApp:compileDebugKotlinAndroid succeeds, and :composeApp:testDebugUnitTest passes -- 176 tests across 24 classes. Co-Authored-By: Claude Opus 5 --- .../compose/managers/FrostSigningRoundTest.kt | 175 +++++++++++++++++- 1 file changed, 174 insertions(+), 1 deletion(-) diff --git a/composeApp/src/commonTest/kotlin/press/mantra/compose/managers/FrostSigningRoundTest.kt b/composeApp/src/commonTest/kotlin/press/mantra/compose/managers/FrostSigningRoundTest.kt index 391c5998..ca9d3690 100644 --- a/composeApp/src/commonTest/kotlin/press/mantra/compose/managers/FrostSigningRoundTest.kt +++ b/composeApp/src/commonTest/kotlin/press/mantra/compose/managers/FrostSigningRoundTest.kt @@ -1,5 +1,6 @@ package press.mantra.compose.managers +import com.vitorpamplona.quartz.nip01Core.core.Event import com.vitorpamplona.quartz.nip01Core.crypto.EventHasher import com.vitorpamplona.quartz.nip01Core.crypto.Nip01Crypto import com.vitorpamplona.quartz.nip01Core.core.hexToByteArray @@ -14,10 +15,10 @@ import fr.acinq.bitcoin.crypto.frost.Session import fr.acinq.bitcoin.crypto.frost.TweakCache import fr.acinq.secp256k1.Hex import kotlin.test.Test -import kotlin.time.Instant import kotlin.test.assertEquals import kotlin.test.assertFalse import kotlin.test.assertTrue +import kotlin.time.Instant import press.mantra.compose.database.model.DkgSession import press.mantra.compose.database.model.FrostSigningSession import press.mantra.compose.database.model.types.FrostSigningStage @@ -311,3 +312,175 @@ class FrostSigningSessionTest { ) } } + +/** + * What a device needs on its row to finish a session it never took part in. + * + * `FrostSigningManager.advance` completes on an arrived signature ahead of the + * approval gate, and that hoist rests on one claim: closing a session needs + * nothing secret and nothing the member would have had to publish. Were it + * false -- were the aggregated nonce, the signer set or a share needed to check + * the result -- the gate would have to stay where it was, and a member the + * quorum did not need would be stuck being asked to sign something already + * signed. + * + * So the claim is spelled out here against a real 2-of-3 signature, from the + * row of the member who was left out of it. + */ +class FrostSigningCompletionTest { + private val keyMaterial: KeyMaterial = Frost.trustedDealerKeygen( + thresholdSecretKey = PrivateKey( + ByteVector32("2decade0000000000000000000000000000000000000000000000000000000b2") + ), + nParticipants = 3, + threshold = 2 + ) + + private val tweakCache: TweakCache = TweakCache.create(keyMaterial.thresholdPublicKey) + + /** The group's nostr identity, exactly as `unsignedEventOf` derives it. */ + private val groupPubKey = tweakCache.tweakedPublicKey.value.toHex() + + /** The event the group is asked to sign, built the way the manager builds it. */ + private val unsignedEvent = Event( + id = EventHasher.hashId( + pubKey = groupPubKey, + createdAt = 1_700_000_000L, + kind = 1, + tags = arrayOf(), + content = "a dialect the group agreed on" + ), + pubKey = groupPubKey, + createdAt = 1_700_000_000L, + kind = 1, + tags = arrayOf(), + content = "a dialect the group agreed on", + sig = "" + ) + + /** A real signature from members 0 and 1. Member 2 is not in it and never was. */ + private val signature: String = run { + val message = ByteVector(unsignedEvent.id.hexToByteArray()) + val signerIds = listOf(0, 1) + + val nonces = signerIds.map { signerId -> + SecretNonce.generate( + sessionRandom = ByteVector32("c".repeat(63) + "${signerId + 1}"), + secretShare = keyMaterial.secretShares[signerId], + publicShare = keyMaterial.publicShares[signerId], + tweakedThresholdPublicKey = tweakCache.tweakedPublicKey, + message = message, + extraInput = null + ) + } + + val session = Session.create( + aggregatedNonce = IndividualNonce.aggregate(nonces.map { it.second }).right!!, + signerIds = signerIds.map { it.toUInt() }, + signerPublicShares = signerIds.map { keyMaterial.publicShares[it] }, + nParticipants = 3, + threshold = 2, + tweakCache = tweakCache, + message = message + ) + + val partials = signerIds.mapIndexed { position, signerId -> + session.sign(nonces[position].first, keyMaterial.secretShares[signerId], signerId.toUInt()).right!! + } + + session.aggregateSigs(partials).right!!.toHex() + } + + /** + * Member 2's row, as it stands when the signature reaches them: they never + * approved, so nothing of theirs was ever published, and the coordinator + * never named them. Every column the completion path reads is here; the ones + * it must not need are deliberately left null. + */ + private fun leftOutMemberSession(signature: String? = null) = FrostSigningSession( + id = "s".repeat(64), + chatRoomId = "room", + coordinatorPublicKey = "c".repeat(64), + userPublicKey = "u".repeat(64), + dkgSessionId = "k".repeat(64), + threshold = 2, + participantCount = 3, + signerId = 2, + unsignedEventJson = unsignedEvent.toJson(), + eventId = unsignedEvent.id, + nonceRandom = "f".repeat(64), + aggregatedNonce = null, + signerIds = null, + signature = signature, + signApprovedAt = null + ) + + @Test + fun `a member who never took part can still check what the group signed`() { + val session = leftOutMemberSession(signature) + val signed = FrostSigningManager.signedEvent(session)!! + + assertTrue( + Nip01Crypto.verify( + signature = signed.sig.hexToByteArray(), + hash = session.eventId.hexToByteArray(), + pubKey = signed.pubKey.hexToByteArray() + ), + "completing must need only the row: the event, its id and the signature" + ) + } + + @Test + fun `the finished event is the one that was proposed, with a signature on it`() { + // Not rebuilt and not rehashed: the id a session is pinned to is the id + // the signature is over, so anything that changed here would produce an + // event whose signature verifies against nothing. + val signed = FrostSigningManager.signedEvent(leftOutMemberSession(signature))!! + + assertEquals(unsignedEvent.id, signed.id) + assertEquals(unsignedEvent.pubKey, signed.pubKey) + assertEquals(unsignedEvent.createdAt, signed.createdAt) + assertEquals(unsignedEvent.kind, signed.kind) + assertEquals(unsignedEvent.content, signed.content) + assertEquals(signature, signed.sig) + } + + @Test + fun `there is no finished event until the signature arrives`() { + assertEquals(null, FrostSigningManager.signedEvent(leftOutMemberSession())) + } + + @Test + fun `the arrived signature is what stops the session asking`() { + // The pair that matters to the screen and the transcript: the same row, + // before and after the group finished without this member. + assertTrue(FrostSigningManager.isAwaitingApproval(leftOutMemberSession())) + assertFalse(FrostSigningManager.isAwaitingApproval(leftOutMemberSession(signature))) + } + + @Test + fun `a signature over a different event is refused`() { + // What the check is for. A coordinator passing off something else must not + // get it applied and announced as the group's, and the row is all there is + // to catch it with. + val other = leftOutMemberSession(signature).copy( + eventId = EventHasher.hashId( + pubKey = groupPubKey, + createdAt = 1_700_000_000L, + kind = 1, + tags = arrayOf(), + content = "something else entirely" + ) + ) + + val signed = FrostSigningManager.signedEvent(other)!! + + assertFalse( + Nip01Crypto.verify( + signature = signed.sig.hexToByteArray(), + hash = other.eventId.hexToByteArray(), + pubKey = signed.pubKey.hexToByteArray() + ) + ) + } +}