From 31e6fc6425d2d9c310be63ca11f27163786887bd Mon Sep 17 00:00:00 2001 From: Kgothatso Ngako Date: Wed, 9 Sep 2026 01:54:01 +0200 Subject: [PATCH] fix(subgroups): ask whether the key state is signed by ceremony, not by room Subgroups "2.0" and "2.1" on the connected devices had finished everything. A ChillDKG COMPLETE for each, a birth certificate signed by the parent's quorum for each, a key state signed by their own quorum for each -- all four events sitting in the coordinator's database. The coordinator was offered no way to create the room. `observeSignedGroupKeyState` resolved which key to ask about by looking the ceremony up **from the room**: getLatestSessionFor(chatRoomId, parentChatRoomId) // parent was null here The view model calls it with no parent, so that reads "the ceremony in this room that is *not* for a subgroup". A subgroup's ceremony room holds only the subgroup's ceremony, so it matched nothing, derived no key, and reported "not signed" over a state in the same database. The button is gated on that boolean, so it never appeared. This is the same mistake as `0b65d702` one layer down. That commit fixed the *session* lookup by having the transcript name the ceremony; it left the key-state and certificate lookups still re-deriving a ceremony from the room. A room does not have one, and every lookup that assumes it does is wrong in a different way: by room it finds none here, and by "newest in room" it would have found the wrong one in the rooms from the previous report. So both observers now take the ceremony they are about. `observeSignedGroupKeyState(dkgSessionId)` derives the subject room from that ceremony's own threshold key, and `observeBirthCertificate(dkgSessionId, parent)` does the same. Neither can disagree with the ceremony the screen is showing, because it is handed the same one. In the view model they move into `observeForCeremony`, re-pointed when the ceremony changes the way the message watcher already is -- previously the key-state watcher was started once at init against a room, which is what let it drift from the session on screen. `observeKeyState` keeps only the room-scoped watch of signing sessions, which is genuinely a room question. One test in `SignedGroupKeyStateTest`: a key state the group really signed is reachable from the ceremony that produced it, and a ceremony with no key answers null rather than throwing -- this flow runs from before a ceremony finishes. 397 common tests, 717 jvm tests, `m3Audit` meets every budget. The two stuck subgroups will offer "create the subgroup" on the next build: their key states are already signed and on file, and nothing about them has to be redone. Co-Authored-By: Claude Opus 5 --- .../repository/DatabaseDkgRepository.kt | 24 +++++---- .../compose/repository/DkgRepository.kt | 24 +++++---- .../ui/view/model/DkgRitualViewModel.kt | 54 ++++++++++++------- .../managers/SignedGroupKeyStateTest.kt | 52 ++++++++++++++++++ 4 files changed, 115 insertions(+), 39 deletions(-) diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/database/repository/DatabaseDkgRepository.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/database/repository/DatabaseDkgRepository.kt index 733fae50..da023200 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/database/repository/DatabaseDkgRepository.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/database/repository/DatabaseDkgRepository.kt @@ -92,22 +92,26 @@ class DatabaseDkgRepository( ): Flow> = database.frostSigningSessionDao().observeSessionsForChatRoom(chatRoomId) - override fun observeSignedGroupKeyState( - chatRoomId: String, - parentChatRoomId: String?, - ): Flow = + override fun observeSignedGroupKeyState(dkgSessionId: String): Flow = database.groupSignedEventDao() .observeByKind(GroupKeyStateEvent.KIND) .map { signedEvents -> - // The room the ceremony's key derives, resolved per emission + // The room this ceremony's key derives, resolved per emission // rather than once: the ceremony has no key until it finishes, // and this flow is running before it does. - val adminRoomId = database.dkgSessionDao() - .getLatestSessionFor(chatRoomId, parentChatRoomId) + // + // By session id, not by room. A ceremony room holds more than one + // ceremony the moment a subgroup's admins are the whole group, and + // a lookup by room answered about whichever the room's rule picked + // -- for a room holding only a subgroup's ceremony, asking for the + // room's *own* found nothing at all, and a finished subgroup with + // a signed key state could never be created. + val subjectRoomId = database.dkgSessionDao() + .getSessionById(dkgSessionId) ?.thresholdPublicKey ?.let { runCatching { SharedKeyDerivation.marmotGroupId(it) }.getOrNull() } - adminRoomId?.let { GroupKeyStateManager.stateAmong(signedEvents, it) } + subjectRoomId?.let { GroupKeyStateManager.stateAmong(signedEvents, it) } } override suspend fun adoptGroupKeyState(chatRoomId: String): GroupKeyState? = try { @@ -148,7 +152,7 @@ class DatabaseDkgRepository( } override fun observeBirthCertificate( - ceremonyChatRoomId: String, + dkgSessionId: String, parentChatRoomId: String, ): Flow = database.groupSignedEventDao() @@ -158,7 +162,7 @@ class DatabaseDkgRepository( // rather than once: the ceremony has no key until it finishes, // and this flow is running before it does. val subgroupChatRoomId = database.dkgSessionDao() - .getLatestSessionFor(ceremonyChatRoomId, parentChatRoomId) + .getSessionById(dkgSessionId) ?.thresholdPublicKey ?.let { runCatching { SharedKeyDerivation.marmotGroupId(it) }.getOrNull() } ?: return@map null diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/repository/DkgRepository.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/repository/DkgRepository.kt index 36c4aa53..01e55111 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/repository/DkgRepository.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/repository/DkgRepository.kt @@ -111,10 +111,17 @@ interface DkgRepository { * changes is the moment there is a room to make -- and the state it is * about does not exist as a row until somebody makes one. */ - fun observeSignedGroupKeyState( - chatRoomId: String, - parentChatRoomId: String? = null, - ): Flow + /** + * Whether the group has signed a key state for the room [dkgSessionId]'s key + * derives. + * + * Named by ceremony rather than by room, because the room a ceremony runs in + * can hold more than one and re-deriving "the room's ceremony" here would + * answer about a different key than the screen is showing. That is what left + * a finished subgroup with its key state signed, its certificate signed, and + * no way to create the room -- the lookup found no ceremony at all. + */ + fun observeSignedGroupKeyState(dkgSessionId: String): Flow /** Files the state the group signed for a room that now exists. */ suspend fun adoptGroupKeyState(chatRoomId: String): GroupKeyState? @@ -142,7 +149,7 @@ interface DkgRepository { * the key state, which is itself signed before the room is created. */ fun observeBirthCertificate( - ceremonyChatRoomId: String, + dkgSessionId: String, parentChatRoomId: String, ): Flow @@ -200,10 +207,7 @@ interface DkgRepository { chatRoomId: String ): Flow> = flowOf(emptyList()) - override fun observeSignedGroupKeyState( - chatRoomId: String, - parentChatRoomId: String?, - ): Flow = + override fun observeSignedGroupKeyState(dkgSessionId: String): Flow = flowOf(null) override suspend fun adoptGroupKeyState(chatRoomId: String): GroupKeyState? = null @@ -217,7 +221,7 @@ interface DkgRepository { ): FrostSigningSession? = null override fun observeBirthCertificate( - ceremonyChatRoomId: String, + dkgSessionId: String, parentChatRoomId: String, ): Flow = flowOf(null) diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/DkgRitualViewModel.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/DkgRitualViewModel.kt index 44408afe..0e1d05fe 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/DkgRitualViewModel.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/DkgRitualViewModel.kt @@ -88,6 +88,9 @@ class DkgRitualViewModel( */ private var watchedParentChatRoomId: HexKey? = null + /** The ceremony [observeForCeremony] is currently watching. */ + private var watchedSessionId: String? = null + /** * The quorum the ritual will generate a key for. Pre-filled with the same * majority default the group-creation screen offers. @@ -186,12 +189,14 @@ class DkgRitualViewModel( pendingApproval = session?.let { dkgRepository.pendingApproval(it) } ) - // Re-point the certificate watchers whenever the purpose changes, - // which for a member arriving cold is once: from nothing to the - // parent the proposal named. - if (parent != watchedParentChatRoomId) { + // Re-point the per-ceremony watchers whenever the ceremony or its + // purpose changes, which for a member arriving cold is once. + // Everything downstream of this -- the key state, the certificate + // -- is about one ceremony, and the room cannot say which. + if (session?.id != watchedSessionId || parent != watchedParentChatRoomId) { + watchedSessionId = session?.id watchedParentChatRoomId = parent - observeCertificate(parent) + observeForCeremony(session?.id, parent) } // Re-point the message watcher at whatever session is current. An @@ -259,21 +264,13 @@ class DkgRitualViewModel( } } - launch { - dkgRepository.observeSignedGroupKeyState(chatRoomId) - .collect { state -> - val loaded = dkgRitualUIState as? DkgRitualUIState.Loaded ?: return@collect - - dkgRitualUIState = loaded.copy(isKeyStateSigned = state != null) - } - } } } /** - * Follows the parent certifying this subgroup, once there is a parent to - * follow. + * Follows one ceremony: whether its key state has been signed, and -- for a + * subgroup -- whether its parent has certified it. * * Off the *parent's* signed events and sessions rather than this room's: the * certificate is made where the parent's key can sign it, which is never the @@ -285,19 +282,38 @@ class DkgRitualViewModel( * message watcher is -- an ordinary ceremony passes null and gets nothing * watched, which is exactly right. */ - private fun observeCertificate(parentChatRoomId: HexKey?) { + private fun observeForCeremony(dkgSessionId: String?, parentChatRoomId: HexKey?) { certificateObserver?.cancel() - if (parentChatRoomId == null) { + if (dkgSessionId == null) { dkgRitualUIState = (dkgRitualUIState as? DkgRitualUIState.Loaded) - ?.copy(isCertified = false, certificateSession = null) + ?.copy(isKeyStateSigned = false, isCertified = false, certificateSession = null) ?: dkgRitualUIState return } certificateObserver = viewModelScope.launch(Dispatchers.IO) { + // Whether the group has agreed what the room this ceremony's key + // derives will sign with. Asked by ceremony rather than by room: the + // room holds however many ceremonies it holds, and the answer is + // about this one's key. launch { - dkgRepository.observeBirthCertificate(chatRoomId, parentChatRoomId) + dkgRepository.observeSignedGroupKeyState(dkgSessionId).collect { state -> + val loaded = dkgRitualUIState as? DkgRitualUIState.Loaded ?: return@collect + + dkgRitualUIState = loaded.copy(isKeyStateSigned = state != null) + } + } + + if (parentChatRoomId == null) { + dkgRitualUIState = (dkgRitualUIState as? DkgRitualUIState.Loaded) + ?.copy(isCertified = false, certificateSession = null) + ?: dkgRitualUIState + return@launch + } + + launch { + dkgRepository.observeBirthCertificate(dkgSessionId, parentChatRoomId) .collect { certificate -> val loaded = dkgRitualUIState as? DkgRitualUIState.Loaded ?: return@collect 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 a40b92fd..79c0f283 100644 --- a/composeApp/src/jvmTest/kotlin/press/mantra/compose/managers/SignedGroupKeyStateTest.kt +++ b/composeApp/src/jvmTest/kotlin/press/mantra/compose/managers/SignedGroupKeyStateTest.kt @@ -25,10 +25,12 @@ import kotlin.test.assertNotNull import kotlin.test.assertNull import kotlin.test.assertTrue import kotlin.time.Instant +import kotlinx.coroutines.flow.first import kotlinx.coroutines.runBlocking import press.mantra.compose.database.MantraDatabase import press.mantra.compose.database.model.ChatMessage import press.mantra.compose.database.builder.getRoomDatabase +import press.mantra.compose.database.repository.DatabaseDkgRepository import press.mantra.compose.database.model.ChatRoom import press.mantra.compose.database.model.DkgParticipantMessage import press.mantra.compose.database.model.DkgSession @@ -1623,4 +1625,54 @@ class SignedGroupKeyStateTest { assertNull(state.birthCertificateJson) } } + + /** + * The state two real subgroups were left in: everything signed, and no way to + * create the room. + * + * "2.0" and "2.1" each finished a ChillDKG, got a birth certificate out of the + * parent's quorum, and got their key state signed by their own. The coordinator + * was then offered nothing, because the screen asked whether a key state was + * signed **by room** -- and a subgroup's ceremony room holds only that + * subgroup's ceremony, so the lookup for the room's *own* ceremony found + * nothing, derived no key, and reported "not signed" over a state sitting in + * the same database. + * + * Asked by ceremony it is answerable, and the answer does not depend on how + * many ceremonies the room holds. + */ + @Test + fun `a signed key state is found by the ceremony that produced it`() = runBlocking { + val creator = device(members[0], signerIndex = 0) + val other = device(members[1], signerIndex = 1) + + val session = GroupKeyStateManager.propose( + database = creator.db, + localChatRoom = creator.room, + userPublicKey = creator.publicKey, + key = ceremonyOn(creator) + ) + pump(creator, other) + FrostSigningManager.approve(other.db, other.room, session.id) + pump(creator, other) + + assertNotNull(creator.keyState(), "the group signed it") + + // What the screen asks, and what it used to ask. The ceremony knows which + // key it made; the room it ran in does not. + assertNotNull( + DatabaseDkgRepository(creator.db, this) + .observeSignedGroupKeyState(ceremonyId) + .first(), + "the key state has to be reachable from the ceremony that produced it", + ) + + // And a ceremony that produced no key yet answers null rather than + // throwing -- this flow runs from before the ceremony finishes. + assertNull( + DatabaseDkgRepository(creator.db, this) + .observeSignedGroupKeyState("no-such-ceremony") + .first(), + ) + } }