diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/DkgRitualScreen.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/DkgRitualScreen.kt index 3e063723..14ee441f 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/DkgRitualScreen.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/DkgRitualScreen.kt @@ -132,11 +132,6 @@ import press.mantra.compose.ui.theme.ConformancePreviews fun DkgRitualScreen( activeUserPublicKey: HexKey, chatRoomId: String, - /** - * Set when this ceremony is making a subgroup, which grows the ladder by one - * rung: the parent's birth certificate, between the key and the key state. - */ - parentChatRoomId: String? = null, /** Which of the room's ceremonies to show; null takes the newest. */ dkgSessionId: String? = null, initialDkgRitualUIState: DkgRitualUIState = DkgRitualUIState.Loading, @@ -149,7 +144,6 @@ fun DkgRitualScreen( factory = DkgRitualViewModel.factory( chatRoomId = chatRoomId, activeUserPublicKey = activeUserPublicKey, - parentChatRoomId = parentChatRoomId, dkgSessionId = dkgSessionId, initialDkgRitualUIState = initialDkgRitualUIState, activeWalletStateFlow = activeWalletStateFlow, diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/navigation/MantraNavHost.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/navigation/MantraNavHost.kt index 83f00db7..6ffc270a 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/navigation/MantraNavHost.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/navigation/MantraNavHost.kt @@ -552,7 +552,6 @@ fun MantraNavHost( DkgRitualScreen( activeUserPublicKey = route.activeUserPublicKey, chatRoomId = route.chatRoomId, - parentChatRoomId = route.parentChatRoomId, dkgSessionId = route.dkgSessionId, activeWalletStateFlow = sovereignWalletViewModel.activeWalletInUI, chatRepository = databaseChatRepository, diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/navigation/routes/DkgRitualRoute.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/navigation/routes/DkgRitualRoute.kt index 9181455e..9f3e3e80 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/navigation/routes/DkgRitualRoute.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/navigation/routes/DkgRitualRoute.kt @@ -5,17 +5,17 @@ import kotlinx.serialization.Serializable /** * The shared-key ceremony for one NIP-17 group. * - * [parentChatRoomId] is set when the ceremony is making a subgroup, and turns the - * screen from a three-step ladder into a four-step one -- the extra rung being the - * parent's birth certificate. It is a claim rather than evidence, in the route as - * on the wire: what makes a lineage real is the certificate the parent's quorum - * signs, two steps later. See `docs/subgroups.md`. + * Deliberately says nothing about *what the ceremony is for*. A subgroup's + * ceremony grows the ladder by a rung -- the parent's birth certificate -- and + * that is read off `DkgSession.parentChatRoomId`, never off the route. Only the + * member who opened a subgroup could ever have put it here, and every other way + * into this screen is from a room; a route that knows more than the room does is + * a route that is right for one caller and silently wrong for the rest. */ @Serializable data class DkgRitualRoute( val activeUserPublicKey: String, val chatRoomId: String, - val parentChatRoomId: String? = null, /** * Which of the room's ceremonies to show, when the caller knows. * 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 0e1d05fe..f4ceb058 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 @@ -50,11 +50,6 @@ import kotlinx.coroutines.launch class DkgRitualViewModel( val chatRoomId: String, val activeUserPublicKey: HexKey, - /** - * The group this ceremony is making a subgroup of, or null for an ordinary - * one. Off the route, which got it off the proposal -- a claim either way. - */ - val parentChatRoomId: HexKey? = null, /** * The ceremony to show, when the caller knew which. Null falls back to the * room's newest -- see [DkgRitualRoute.dkgSessionId]. @@ -113,6 +108,12 @@ class DkgRitualViewModel( if (loaded.session != null && loaded.session.stage != DkgRitualStage.FAILED) return false + // Not just the ceremony on screen. A room can hold more than one, and this + // screen exists to offer one to a room that has none -- so a room with a + // ceremony running under a different purpose must not be offered another, + // whatever this screen happens to be showing. + if (loaded.roomHasCeremony) return false + return ChillDkgRitualManager.canRunRitual(loaded.localChatRoom) } @@ -141,10 +142,7 @@ class DkgRitualViewModel( return@launch } - dkgRitualUIState = DkgRitualUIState.Loaded( - localChatRoom = localChatRoom, - parentChatRoomId = parentChatRoomId - ) + dkgRitualUIState = DkgRitualUIState.Loaded(localChatRoom = localChatRoom) threshold.value = ChatRoomType .defaultQuorum(ChillDkgRitualManager.memberPublicKeys(localChatRoom).size) .coerceIn(quorumRange()) @@ -171,17 +169,27 @@ class DkgRitualViewModel( ?.let { dkgRepository.observeSessionById(it) } ?: dkgRepository.observeLatestSessionForChatRoom(chatRoomId) + // Whether the room has any ceremony at all, which is a different + // question from which one is being shown -- see + // `DkgRitualUIState.roomHasCeremony`. + launch { + dkgRepository.observeLatestSessionForChatRoom(chatRoomId).collect { newest -> + val current = dkgRitualUIState as? DkgRitualUIState.Loaded ?: return@collect + + dkgRitualUIState = current.copy( + roomHasCeremony = newest != null && newest.stage != DkgRitualStage.FAILED + ) + } + } + sessions.collect { session -> val loaded = (dkgRitualUIState as? DkgRitualUIState.Loaded) - ?: DkgRitualUIState.Loaded( - localChatRoom = localChatRoom, - parentChatRoomId = parentChatRoomId - ) + ?: DkgRitualUIState.Loaded(localChatRoom = localChatRoom) - // The session's own claim wins over the route's. They agree for the - // coordinator; for everybody else the route has nothing and the - // session is the only thing that knows. - val parent = session?.parentChatRoomId ?: parentChatRoomId + // The session is the only thing that knows what the ceremony is + // for. The route used to carry it too, which was right for the one + // member who opened the subgroup and null for everybody else. + val parent = session?.parentChatRoomId dkgRitualUIState = loaded.copy( session = session, @@ -661,7 +669,6 @@ class DkgRitualViewModel( fun factory( chatRoomId: String, activeUserPublicKey: HexKey, - parentChatRoomId: HexKey? = null, dkgSessionId: String? = null, initialDkgRitualUIState: DkgRitualUIState = DkgRitualUIState.Loading, activeWalletStateFlow: StateFlow, @@ -672,7 +679,6 @@ class DkgRitualViewModel( DkgRitualViewModel( chatRoomId = chatRoomId, activeUserPublicKey = activeUserPublicKey, - parentChatRoomId = parentChatRoomId, dkgSessionId = dkgSessionId, initialDkgRitualUIState = initialDkgRitualUIState, activeWalletStateFlow = activeWalletStateFlow, diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/SelectSubgroupAdminsViewModel.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/SelectSubgroupAdminsViewModel.kt index 566cb931..df7fc00d 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/SelectSubgroupAdminsViewModel.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/SelectSubgroupAdminsViewModel.kt @@ -24,7 +24,7 @@ import press.mantra.compose.managers.ChillDkgRitualManager import press.mantra.compose.managers.MarmotGroupCreation import press.mantra.compose.repository.ChatRepository import press.mantra.compose.repository.DkgRepository -import press.mantra.compose.ui.composable.navigation.routes.DkgRitualRoute +import press.mantra.compose.ui.composable.navigation.routes.ChatRoomMessagingRoute import press.mantra.compose.ui.composable.navigation.routes.Route import press.mantra.compose.ui.view.state.SelectSubgroupAdminsUIState @@ -248,11 +248,24 @@ class SelectSubgroupAdminsViewModel( } withContext(Dispatchers.Main) { + // The transcript, not the shared-key screen. The ceremony is + // already open and its first line is already in this room, so what + // the coordinator needs is to watch it and answer the requests as + // they arrive -- which is the same path every other member takes, + // through the ritual notices that name their own ceremony. + // + // Sending them to the shared-key screen instead was actively + // harmful: that screen's job is to *offer* a ceremony to a room + // that has none, so the coordinator arrived at a "Start key + // ceremony" button seconds after starting one, and pressing it + // opened a second -- for no subgroup, so `proposeRitual`'s guard + // did not recognise it as a duplicate -- which then buried the + // subgroup's under the room's newest. onNavigateToRoute( - DkgRitualRoute( + ChatRoomMessagingRoute( activeUserPublicKey = activeUserPublicKey, chatRoomId = ceremonyRoom.chatRoom.id, - parentChatRoomId = parentChatRoomId + relayHint = null ) ) } diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/state/DkgRitualUIState.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/state/DkgRitualUIState.kt index 11dee98a..e79b867d 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/state/DkgRitualUIState.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/state/DkgRitualUIState.kt @@ -63,6 +63,18 @@ sealed interface DkgRitualUIState { * signed first. It is what gates creating the room. */ val isKeyStateSigned: Boolean = false, + /** + * Whether this room already holds a ceremony that has not failed -- + * whichever one [session] happens to be showing. + * + * Two different questions, and conflating them opened a second ceremony + * in a room that already had one. [session] is the ceremony being looked + * at; this is whether the room has any. `proposeRitual`'s guard is scoped + * by purpose, so a room holding a subgroup's ceremony would happily take + * a second one for no subgroup -- and then "the room's newest ceremony" + * is the new empty one and the subgroup's is buried under it. + */ + val roomHasCeremony: Boolean = false, /** * The group this ceremony is making a subgroup of, or null for an * ordinary one. diff --git a/composeApp/src/jvmTest/kotlin/press/mantra/compose/managers/RobustRoomKeyCeremonyTest.kt b/composeApp/src/jvmTest/kotlin/press/mantra/compose/managers/RobustRoomKeyCeremonyTest.kt index c066e7ae..a2863aa8 100644 --- a/composeApp/src/jvmTest/kotlin/press/mantra/compose/managers/RobustRoomKeyCeremonyTest.kt +++ b/composeApp/src/jvmTest/kotlin/press/mantra/compose/managers/RobustRoomKeyCeremonyTest.kt @@ -421,4 +421,38 @@ class RobustRoomKeyCeremonyTest { ) } } + + /** + * The second ceremony a coordinator was able to open seconds after the first. + * + * `proposeRitual`'s guard is scoped by purpose, deliberately -- a room has to + * be able to hold the group's own ceremony and a subgroup's. But that means a + * room with a subgroup's ceremony in it will take a second one for *no* + * subgroup without complaint, and then "the room's newest ceremony" is the new + * empty one and the subgroup's is buried under it. + * + * Nothing on the wire stops that and nothing should: two purposes are two + * ceremonies. What stops it is not offering it -- the shared-key screen exists + * to give a ceremony to a room that has none, and this room has one. + */ + @Test + fun `a room holding a ceremony will still take one for another purpose`() = runBlocking { + val room = robustRoom() + + val subgroup = openCeremony(room, parentChatRoomId = "ab".repeat(32)) + val own = openCeremony(room) + + // Both exist, which is correct at this level: the guard is about purpose. + assertNotEquals(subgroup.id, own.id) + + // So the refusal has to live in what the screen offers, which is why + // `DkgRitualUIState.roomHasCeremony` is asked separately from which + // ceremony is being shown. + assertEquals( + 2, + db.dkgSessionDao().getByParentChatRoomId("ab".repeat(32)).size + + (if (db.dkgSessionDao().getLatestSessionFor(room.chatRoom.id, null) != null) 1 else 0), + "a room can end up holding two ceremonies, so the UI must not offer a third", + ) + } }