From 51106874133734c88afb78b2108df68323853fe8 Mon Sep 17 00:00:00 2001 From: Kgothatso Ngako Date: Wed, 9 Sep 2026 16:27:57 +0200 Subject: [PATCH] feat(subgroups): run the child's ceremony in the parent's room, not beside it The switch. A subgroup's ChillDKG and its key state now happen in the parent's Marmot room; the sibling NIP-17 room derived from the child's admins is gone, and so is the rule that one admin set could hold one subgroup forever. This is what `docs/subgroups.md` deferred in "Why not the parent's Marmot room" and said to revisit. The reason to wait was `docs/mls-skipped-keys.md` -- a DKG cannot finish until every participant takes part, so one group event lost to the skipped-keys bug stalls it for everybody -- and that is being fixed. The two transports and the two guards this needs landed in the three commits before it. **`SelectSubgroupAdminsViewModel` stops standing up a room.** It opens the ritual in `loaded.parentRoom`, p-tagged to the picked admins, and hands back to the parent's transcript. That transcript is where the other admins were going to answer from anyway -- `observeCeremoniesAwaitingYou` and `observeProposalsAwaitingYou` are room-scoped and were already looking there -- so the coordinator now lands in the same place as everybody else rather than in a room only they knew was coming. The name and the admin set ride on the proposal and land on the session, which is what `1e7ddd84`'s two columns are for. `MarmotGroupName.of` still puts the `#` on at the two sites that use the name, and is still idempotent, so the certificate the parent signs and the room `MarmotGroupCreation` creates are called the same thing. **`SubgroupManager.ceremonyRoomIdFor` is deleted and `refuseCeremonyRoom` becomes `refuseSubgroup`.** There is no ceremony room to derive or to name a refusal after. The refusal it made -- "this group already has a subgroup run by exactly these members", forever -- narrows to a ceremony over those admins under that parent that is still *running*, on `getLatestSubgroupSessionFor`. That refusal is worth keeping for a reason the old one did not have. It is not that two subgroups over the same people are forbidden; it is that nobody can answer for two live ceremonies at once. Every admin would be asked twice, on two ladders, for two keys, one of which nobody will make a room from. A *finished* one means that subgroup exists, and asking for another is a legitimate ask that the derived room made impossible. **Three reads move from the room to the ceremony**, all of them in the ritual screen, and each would have been silently wrong in the parent's room: - `proposeBirthCertificate`'s `adminPublicKeys` -- the room's roster would ask the parent to certify itself as its own child; - `createAdminGroup`'s `members` -- it would welcome the whole parent into the subgroup; - the name both of those carry -- it would be the parent's. `DkgRitualUIState.ritualMembers` follows, so the progress ladder draws the ceremony's participants rather than a row per parent member, and does not report a ceremony waiting on people it was never with. Profiles still come off the room, which for a subgroup is the parent and holds every admin the ceremony can have. **Nothing new is offered on a Marmot room's detail screen.** The shared-key button is still gated on `mlsGroupState == null`, and the comment there now says why that is still right: offering a ceremony to a room is offering it a key of its own, and a Marmot room's id is derived from a key it already has. Its subgroups' ceremonies are reached from the transcript line that names the one they are about, which is the only thing that can say which. `SubgroupManagerJvmTest` follows the refusal. The two tests built on `ceremonyRoomIdFor` become tests of what actually distinguishes ceremonies now -- another subgroup's and the room's own do not block one, a live one over the same admins does, a finished one does not -- and the ordering test moves to `DkgSession.formatParticipants`, which is where "the same set however it was assembled" now has to hold. Co-Authored-By: Claude Opus 5 --- .../repository/DatabaseChatRepository.kt | 2 +- .../repository/DatabaseDkgRepository.kt | 8 +- .../compose/managers/SubgroupManager.kt | 84 ++++++------- .../compose/repository/ChatRepository.kt | 2 +- .../compose/repository/DkgRepository.kt | 42 ++++--- .../ui/composable/ChatRoomDetailScreen.kt | 22 ++-- .../ui/view/model/DkgRitualViewModel.kt | 43 ++++--- .../model/SelectSubgroupAdminsViewModel.kt | 83 ++++++------- .../compose/ui/view/state/DkgRitualUIState.kt | 43 +++++-- .../managers/SubgroupManagerJvmTest.kt | 115 +++++++++--------- 10 files changed, 245 insertions(+), 199 deletions(-) diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/database/repository/DatabaseChatRepository.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/database/repository/DatabaseChatRepository.kt index c4f36b34..b931ad84 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/database/repository/DatabaseChatRepository.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/database/repository/DatabaseChatRepository.kt @@ -135,7 +135,7 @@ class DatabaseChatRepository( val parentRoom = database.chatRoomDao().findChatRoomById(parentChatRoomId) ?: return "Couldn't load this group." - SubgroupManager.refuseCeremonyRoom( + SubgroupManager.refuseSubgroup( database = database, parentRoom = parentRoom, adminPublicKeys = adminPublicKeys, 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 d436d3ef..e1674fc5 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 @@ -62,7 +62,9 @@ class DatabaseDkgRepository( userPublicKey: HexKey, nostrPrivateKey: ByteArray, threshold: Int, - parentChatRoomId: HexKey? + parentChatRoomId: HexKey?, + participantPublicKeys: Set?, + subject: String? ): DkgSession? = try { ChillDkgRitualManager.proposeRitual( database = database, @@ -70,7 +72,9 @@ class DatabaseDkgRepository( userPublicKey = userPublicKey, nostrPrivateKey = nostrPrivateKey, threshold = threshold, - parentChatRoomId = parentChatRoomId + parentChatRoomId = parentChatRoomId, + participantPublicKeys = participantPublicKeys, + subject = subject ) } catch (e: Throwable) { logger.e("Error proposing DKG ritual for ${localChatRoom.chatRoom.id}", e) diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/managers/SubgroupManager.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/managers/SubgroupManager.kt index c176d25a..79c3ab7a 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/managers/SubgroupManager.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/managers/SubgroupManager.kt @@ -4,7 +4,6 @@ import co.touchlab.kermit.Logger import com.vitorpamplona.quartz.nip01Core.core.Event import com.vitorpamplona.quartz.nip01Core.core.HexKey import press.mantra.compose.database.MantraDatabase -import press.mantra.compose.database.model.ChatRoom import press.mantra.compose.database.model.DkgSession import press.mantra.compose.database.model.FrostSigningSession import press.mantra.compose.database.model.GroupKeyState @@ -24,6 +23,11 @@ import press.mantra.compose.nostr.subgroup.SubgroupParentage * 4. the Marmot room -> somebody creates C and welcomes the admins * ``` * + * **All four happen in the parent's room**, bar the last, which makes a room of + * its own. Steps 1 and 3 are held there because the child has nowhere else to be: + * its id is derived from the key step 1 produces, so it cannot exist until + * afterwards. Step 2 is there because the signature is the parent's. + * * None of that order is a policy choice. Step 2 needs `C`, which does not exist * until step 1 produces `K`; step 3 carries the certificate, so it needs step 2; * and step 4 is gated on the key state for the same reason @@ -67,23 +71,6 @@ object SubgroupManager { */ const val MINIMUM_ADMINS: Int = 3 - /** - * The NIP-17 room a ceremony for these admins would run in. - * - * Pure, and the same aggregate `NostrNip17Dao.createNip17ChatRoom` and the - * inbound path both derive -- which is what makes a ceremony room findable - * without being announced, and what makes two members starting the same - * subgroup land in one room rather than two. - * - * It is also where the one hard limit comes from. A NIP-17 room's id is a - * pure function of its members, so **one admin set gets one ceremony room, - * forever**: picking a set that already holds a completed ceremony would hand - * back that ceremony's key, and the "new" subgroup would be the old group - * under a new name. [refuseCeremonyRoom] is where that is caught. - */ - fun ceremonyRoomIdFor(adminPublicKeys: Set, coordinatorPublicKey: HexKey): String = - ChatRoom.deriveChatRoomId(adminPublicKeys + coordinatorPublicKey) - /** * Why these admins cannot hold a ceremony together, or null when they can. * @@ -94,7 +81,7 @@ object SubgroupManager { * The message is the user's, so each one says what to do rather than what is * wrong. */ - suspend fun refuseCeremonyRoom( + suspend fun refuseSubgroup( database: MantraDatabase, parentRoom: LocalChatRoom, adminPublicKeys: Set, @@ -130,8 +117,8 @@ object SubgroupManager { // group agreed, and is right on both sides from the first moment. // Named for the parent, because `adminPublicKeys` in this function is the // *subgroup's* picked admins and the two must never be confused -- the - // ceremony room is derived from the second, and deriving it from the - // first would silently make a different room. + // ceremony is opened over the second while running in a room governed by + // the first. val parentAdminPublicKeys = runCatching { parentRoom.chatRoom.toMlsGroup()?.currentMarmotData()?.adminPubkeys }.getOrNull() @@ -160,23 +147,30 @@ object SubgroupManager { // A subgroup *may* be the whole group. It is a logical division -- a group // deciding that some of its work belongs to a differently-keyed room -- // rather than a smaller membership, so "everybody" is a normal answer and - // was never this function's business to refuse. + // never this function's business to refuse. // - // What the old rule was standing in for is real and is checked below, - // precisely: the ceremony room is derived from its admins, so a subgroup - // over everybody lands in the room the group's own ceremony was held in. - // Sharing the room is fine; sharing a *ceremony* is not, and - // `DkgSession.parentChatRoomId` is what keeps them apart. + // The only thing left to refuse is a ceremony already under way for the + // same subgroup, and it is refused because a person cannot answer for two + // of them: every admin would be asked twice, on two ladders, for two keys, + // one of which nobody will ever make a room from. // - // Checking on set size was wrong twice over. It refused a legitimate - // subgroup, and it refused it on a proxy: a parent whose membership has - // changed since its own ceremony derives a different room, so the sizes - // could match with no collision at all, and could differ with one. - val ceremonyRoomId = ceremonyRoomIdFor(adminPublicKeys, coordinatorPublicKey) - val running = database.dkgSessionDao() - .getLatestSessionFor(ceremonyRoomId, parentChatRoomId) - if (running != null && running.stage != DkgRitualStage.FAILED) { - return "This group already has a subgroup run by exactly these members." + // A *finished* one is not refused. It means that subgroup was made, and a + // group is entitled to a second run by the same people -- which was + // impossible while the ceremony ran in a room derived from its admins, + // because asking again handed back the first ceremony's key and the "new" + // subgroup was the old one renamed. Moving the ceremony into the parent's + // room is what retired that, and this is the whole of what is left. + val running = database.dkgSessionDao().getLatestSubgroupSessionFor( + chatRoomId = parentChatRoomId, + parentChatRoomId = parentChatRoomId, + participantPublicKeys = DkgSession.formatParticipants(admins).orEmpty() + ) + if (running != null && + running.stage != DkgRitualStage.FAILED && + running.stage != DkgRitualStage.COMPLETE + ) { + return "A key ceremony for a subgroup run by exactly these members is " + + "already under way." } return null @@ -258,9 +252,9 @@ object SubgroupManager { * [subgroupChatRoomId], or null if this device holds none. * * **Two certificates for one child is a normal outcome, not a conflict.** Two - * parent admins can press the button on the same admin set: the second lands - * in the same ceremony room and gets the first's ceremony back, but both may - * go on to propose a certificate and both sessions can complete. + * parent admins can press the button on the same admin set: the second's + * `proposeRitual` folds into the ceremony the first opened, but both may go on + * to propose a certificate and both sessions can complete. * `GroupSignedEvent` is keyed on the event id, so the rows coexist -- and * because both say the same true thing about the same child, which one wins * does not matter. The `d` tag makes them replacements of each other rather @@ -286,9 +280,15 @@ object SubgroupManager { * Step 3: asks the child's quorum to sign its key state, carrying the * certificate. * - * Runs where the ceremony ran, because it has to: `GroupKeyStateManager.propose` - * and `FrostSigningManager.signingPath` both tie a key state to the room its - * ceremony was held in, and the room the state is *about* does not exist yet. + * Runs where the ceremony ran -- the parent's room -- because it has to: + * `GroupKeyStateManager.propose` and `FrostSigningManager.signingPath` both tie + * a key state to the room its ceremony was held in, and the room the state is + * *about* does not exist yet. + * + * Signed by the **child's** quorum for all that, not the parent's. The room is + * only the transport; who signs comes from the ceremony's host keys, and the + * key it signs with is the one that ceremony produced. A parent admin outside + * the subgroup reads the messages and holds no share to answer them with. * * Refuses without a certificate that passes `certifies`. Every device that * receives the state runs the same check and drops it when it fails, so diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/repository/ChatRepository.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/repository/ChatRepository.kt index de8442ad..8fbfa95e 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/repository/ChatRepository.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/repository/ChatRepository.kt @@ -95,7 +95,7 @@ interface ChatRepository { * * Read by the picker so a subgroup that could only fail is never offered, and * again on confirm, because a screen not drawing something is not a guard. - * See `SubgroupManager.refuseCeremonyRoom`. + * See `SubgroupManager.refuseSubgroup`. */ suspend fun refuseSubgroup( parentChatRoomId: String, 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 e60ec7f0..cb4da4eb 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/repository/DkgRepository.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/repository/DkgRepository.kt @@ -25,9 +25,8 @@ interface DkgRepository { * [parentChatRoomId] is null, or that parent's subgroup when it is not. * Abandoned attempts are superseded by the newest. * - * A room can hold two: a subgroup whose admins are the whole group derives - * the room the group's own ceremony ran in. See - * `DkgSessionDao.getLatestSessionFor`. + * A room routinely holds several: a parent's room hosts the ceremony of every + * subgroup the group makes. See `DkgSessionDao.getLatestSessionFor`. */ fun observeLatestSessionForChatRoom( chatRoomId: String, @@ -67,18 +66,26 @@ interface DkgRepository { suspend fun getLatestSessionForChatRoom(chatRoomId: String): DkgSession? /** - * Opens a ritual. Only meaningful for the room's creator. + * Opens a ritual in [localChatRoom], making this device the coordinator. * * [parentChatRoomId] says what the ceremony is for when it is for a subgroup. * It rides on the proposal and is believed by nobody -- see * `DkgSession.parentChatRoomId`. + * + * [participantPublicKeys] and [subject] are what a subgroup needs and a + * group's own ceremony does not: the room it runs in is the *parent's*, so + * the room can say neither who the ceremony is with nor what the room it + * makes is to be called. Both default to the room, which is the right answer + * for a group opening its own. */ suspend fun proposeRitual( localChatRoom: LocalChatRoom, userPublicKey: HexKey, nostrPrivateKey: ByteArray, threshold: Int, - parentChatRoomId: HexKey? = null + parentChatRoomId: HexKey? = null, + participantPublicKeys: Set? = null, + subject: String? = null ): DkgSession? /** What the ritual is waiting on this device's owner for, if anything. */ @@ -96,8 +103,8 @@ interface DkgRepository { * Asks the group to sign a statement of which ceremony's key its admin * room will sign with, before that room is created. * - * Called once by the ceremony's coordinator, in the NIP-17 room the ceremony - * ran in. What comes back is the signing session, not the state: the state + * Called once by the ceremony's coordinator, in the room the ceremony ran + * in. What comes back is the signing session, not the state: the state * exists when a quorum has signed, which is the point of proposing it rather * than announcing it. Null if the ritual has produced no key yet, or if this * room has no standing to propose one -- both of which mean there is nothing @@ -112,10 +119,11 @@ interface DkgRepository { /** * The signing sessions open in a room, newest first, with what each signs. * - * Read by the ritual screen to follow the one session a NIP-17 room ever - * holds: the group agreeing its key state. The items travel with the session - * because "which session is this" is answered by the kind of the event it is - * signing, not by the row. + * Read by the ritual screen to follow the session the group's key state is + * agreed in. The items travel with the session because "which session is + * this" is answered by the kind of the event it is signing, not by the row -- + * which is what lets the same reading work in a parent's room, where a + * subgroup's key state is one session among however many the room is running. */ fun observeSigningSessions(chatRoomId: String): Flow> @@ -149,10 +157,10 @@ interface DkgRepository { /** * Asks the parent's quorum to certify the subgroup this ceremony produced. * - * Runs in the **parent's** room, not the ceremony's, so what comes back is - * signed by the parent's key and authored by the parent's own room id -- the - * one thing a lineage can be checked against. See - * `SubgroupManager.proposeBirthCertificate`. + * Signed by the parent's key and authored by the parent's own room id -- the + * one thing a lineage can be checked against. The ceremony runs in that room + * too now, so this is no longer a change of venue, only of whose quorum is + * being asked. See `SubgroupManager.proposeBirthCertificate`. */ suspend fun proposeBirthCertificate( parentChatRoomId: String, @@ -210,7 +218,9 @@ interface DkgRepository { userPublicKey: HexKey, nostrPrivateKey: ByteArray, threshold: Int, - parentChatRoomId: HexKey? + parentChatRoomId: HexKey?, + participantPublicKeys: Set?, + subject: String? ): DkgSession? = null override suspend fun pendingApproval(session: DkgSession): DkgApprovalStep? = null diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/ChatRoomDetailScreen.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/ChatRoomDetailScreen.kt index 1edfffee..70340da0 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/ChatRoomDetailScreen.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/ChatRoomDetailScreen.kt @@ -590,9 +590,16 @@ fun ChatRoomDetailScreen( ) } - // A shared threshold key only means something where every - // member is an equal admin, which is the NIP-17 (robust) case. - // MLS rooms have one admin and no group key to share. + // Offering a ceremony to a room is offering it a key of + // its own, and only a NIP-17 room is in a position to + // want one: a Marmot room's id is derived from a key it + // already has, so a second ceremony there would produce + // a key the room could never answer to. + // + // A Marmot room does host ceremonies -- its subgroups' + // -- and those are reached from the transcript line that + // names the ceremony they are about, never from here, + // which has no way to say which. if (chatRoomDetailUIState.localChatRoom.chatRoom.mlsGroupState == null) { item { TextButton( @@ -619,11 +626,10 @@ fun ChatRoomDetailScreen( } } - // Every room, not only the one that holds the key. A - // ceremony needs a NIP-17 group, but a signing message is - // a marmot inner event -- see `FrostSigningManager.broadcast` - // -- so the room a group actually proposes in is the MLS - // one, which is the last place this should be missing from. + // Every room, not only the one that holds the key. Most + // of what a group signs it signs in its Marmot room -- + // its subgroups' certificates and key states included -- + // so that is the last place this should be missing from. // // The transcript carries a proposal past as it happens; // this is where a member goes to find one that has scrolled 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 f452df8c..791a0414 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 @@ -443,13 +443,17 @@ class DkgRitualViewModel( parentChatRoomId = parent, userPublicKey = activeUserPublicKey, session = session, + // The ceremony's participants, not the room's members. The + // ceremony runs in the parent's room, so the room's roster is + // everybody the subgroup is *not* -- putting it here would ask the + // parent to certify itself as its own child. adminPublicKeys = ChillDkgRitualManager - .memberPublicKeys(loaded.localChatRoom) + .participantsOf(session, loaded.localChatRoom) .toList(), - // The ceremony room was created with the name the coordinator - // typed, and the proposal carried it to everyone else -- so this - // is the same name every admin has been looking at. - name = loaded.localChatRoom.chatRoom.subject ?: "Subgroup" + // The name the coordinator typed, carried to every other admin on + // the proposal -- so this is the same name they have all been + // looking at, and the one the parent's quorum is about to sign. + name = session.subject ?: "Subgroup" ) isActionPending.value = false @@ -535,9 +539,10 @@ class DkgRitualViewModel( * Opens the group's admin room: a Marmot/MLS chat whose every member is an * admin, keyed on an id derived from the shared key. * - * The room the ceremony ran in is NIP-17, where nobody administers anything. - * This gives the same people a room where every one of them can act, which is - * the shape a group that has just made a t-of-n key is asking for. + * The room the ceremony ran in is somebody else's -- a NIP-17 room where + * nobody administers anything, or the parent whose subgroup this is. This + * gives the people who hold shares a room where every one of them can act, + * which is the shape a group that has just made a t-of-n key is asking for. * * Derived rather than random, unlike every other Marmot room. Every member's * device can compute the id from the ceremony they all took part in, so the @@ -546,10 +551,10 @@ class DkgRitualViewModel( * early to the existing room rather than minting a second one. * * Only once the group has signed what the room signs with. That statement is - * agreed in the NIP-17 room the ceremony ran in, before there is a Marmot - * room to hold it -- see `GroupKeyStateManager.propose` -- so by here it is - * a fact the room is created *knowing*, rather than the first thing it has - * to go and ask about. + * agreed in the room the ceremony ran in, before there is a room of its own + * to hold it -- see `GroupKeyStateManager.propose` -- so by here it is a fact + * the room is created *knowing*, rather than the first thing it has to go and + * ask about. */ fun createAdminGroup(onNavigateToRoute: (Route) -> Unit) { if (isActionPending.value) return @@ -574,17 +579,19 @@ class DkgRitualViewModel( // Everyone the ceremony was run with, this device included. These are the // admins: the whole point of the room is that the members who hold shares of // the key can all act in it. - val members = ChillDkgRitualManager.memberPublicKeys(loaded.localChatRoom) + // + // Off the ceremony rather than off the room, because for a subgroup the + // room is the parent's and its roster is the wrong set entirely. + val members = ChillDkgRitualManager.participantsOf(session, loaded.localChatRoom) // A subgroup is called what the coordinator called it; an admin room is // called after the group it administers, because it has no name of its // own to be given. Either way the bare name goes down and - // `MarmotGroupCreation` puts it under `MarmotGroupName`, so the NIP-17 - // room the ceremony ran in stays `Ekklesia` and the Marmot room it stands - // up is `#Ekklesia`. + // `MarmotGroupCreation` puts it under `MarmotGroupName`, so a NIP-17 room + // called `Ekklesia` stands up a Marmot room called `#Ekklesia`. val subgroupParent = loaded.parentChatRoomId - val name = loaded.localChatRoom.chatRoom.subject + val name = session.subject ?: if (subgroupParent != null) "Subgroup" else "Group" isActionPending.value = true @@ -597,7 +604,7 @@ class DkgRitualViewModel( purpose = if (subgroupParent != null) { "A subgroup of the group that certified it." } else { - "Admins of ${loaded.localChatRoom.chatRoom.subject ?: "the group"}." + "Admins of ${session.subject ?: "the group"}." }, adminPublicKeys = members, userPublicKey = activeUserPublicKey, 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 183578e1..ae4e93b2 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 @@ -20,9 +20,7 @@ import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.launch import kotlinx.coroutines.withContext import press.mantra.compose.database.model.types.ChatRoomType -import press.mantra.compose.managers.ChillDkgRitualManager import press.mantra.compose.managers.MarmotGroupCreation -import press.mantra.compose.managers.MarmotGroupName import press.mantra.compose.repository.ChatRepository import press.mantra.compose.repository.DkgRepository import press.mantra.compose.ui.composable.navigation.routes.ChatRoomMessagingRoute @@ -44,9 +42,10 @@ import press.mantra.compose.ui.view.state.SelectSubgroupAdminsUIState * `quorumRange` and drops a proposal outside it, and the host-key approval gate is * where each of them agrees to the `t`-of-`n` they can now see. * - * Confirming opens the ceremony -- a NIP-17 room over the picked admins, and a - * ritual proposal in it -- and hands over to `DkgRitualViewModel`, which drives - * the remaining three steps. See `docs/subgroups.md`. + * Confirming opens the ceremony -- a ritual proposal in the **parent's** room, + * p-tagged to the picked admins -- and hands back to the parent's transcript, + * where every admin answers the ceremony from the same place. See + * `docs/subgroups.md`. */ class SelectSubgroupAdminsViewModel( val activeUserPublicKey: HexKey, @@ -191,12 +190,14 @@ class SelectSubgroupAdminsViewModel( } /** - * Opens the ceremony and hands over to the ritual screen. + * Opens the ceremony in the parent's room and hands back to its transcript. * - * The room is stood up first and the proposal is the first thing out of it, - * exactly as a robust group is created: standing up a NIP-17 room sends - * nothing to anybody, so the proposal is how the other admins hear of it at - * all -- which is why it carries the name and the parent. + * No sibling room is made. The ceremony runs where the parent's members + * already are: the proposal names the picked admins in its p-tags, which is + * the participant set every device derives `n` from, and a parent member who + * is not in it drops the proposal rather than joining. It carries the name + * and the parent for the same reason it always did -- the child has no room + * of its own to read either off until the ceremony has produced its key. */ fun confirm(onNavigateToRoute: (Route) -> Unit) { if (isActionPending.value) return @@ -223,45 +224,36 @@ class SelectSubgroupAdminsViewModel( return@launch } - // The ceremony room keeps the bare name. It is a NIP-17 room -- where - // the subgroup is made, not the subgroup -- and the `#` is what tells - // those two rows apart afterwards, so putting it on both would spend - // the mark to say nothing. + // The bare name goes on the ceremony, and the `#` is added where the + // name is used: `proposeBirthCertificate` normalises the name the + // parent signs and `MarmotGroupCreation` normalises the name the room + // carries, both reading it back off this session. + // `MarmotGroupName.of` is idempotent, so those are the same string -- + // which they have to be, or the room is not called what its parent + // certified. // - // The subgroup gets it twice over, and never from here: - // `proposeBirthCertificate` normalises the name the parent signs and - // `MarmotGroupCreation` normalises the name the room carries, both - // reading this subject. `MarmotGroupName.of` is idempotent, so those - // are the same string -- which they have to be, or the room is not - // called what its parent certified. - val ceremonyRoom = chatRepository.createNip17ChatRoom( - userPublicKey = activeUserPublicKey, - participantPublicKeys = selectedPublicKeys.toList(), - subject = name.value.trim(), - description = "Making ${MarmotGroupName.of(name.value)} a subgroup of " + - (loaded.parentRoom.chatRoom.subject ?: "this group") + "." - ) - - if (ceremonyRoom == null) { - isActionPending.value = false - uiState = SelectSubgroupAdminsUIState.Error( - "Couldn't start the subgroup's key ceremony. Please try again." - ) - return@launch - } - + // Carried on the session rather than on a room, because there is no + // longer a room to carry it: the ceremony is held in the parent's, + // and the parent's name is the one name a child must not take. val session = dkgRepository.proposeRitual( - localChatRoom = ceremonyRoom, + localChatRoom = loaded.parentRoom, userPublicKey = activeUserPublicKey, nostrPrivateKey = nostrPrivateKey.value.toByteArray(), threshold = threshold.value, - parentChatRoomId = parentChatRoomId + parentChatRoomId = parentChatRoomId, + // The admins, and only them. The room is the parent's, so the + // ceremony has to say who it is with -- this is the set that + // becomes the proposal's p-tags and every device's `n`. The + // coordinator is added by `proposeRitual`: opening a ceremony is + // being in it, and is not theirs to tick. + participantPublicKeys = selectedPublicKeys.toSet(), + subject = name.value.trim() ) isActionPending.value = false if (session == null) { - logger.e("Created ${ceremonyRoom.chatRoom.id} without a key ceremony") + logger.e("Could not open a subgroup ceremony in $parentChatRoomId") uiState = SelectSubgroupAdminsUIState.Error( "Couldn't start the subgroup's key ceremony. Please try again." ) @@ -269,11 +261,12 @@ 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. + // The parent's transcript, not the shared-key screen. The ceremony + // is already open and its first line is already there, so what the + // coordinator needs is to watch it and answer the requests as they + // arrive -- which is now literally the same place every other + // admin answers from, 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 @@ -285,7 +278,7 @@ class SelectSubgroupAdminsViewModel( onNavigateToRoute( ChatRoomMessagingRoute( activeUserPublicKey = activeUserPublicKey, - chatRoomId = ceremonyRoom.chatRoom.id, + chatRoomId = 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 52be6cb9..cf422002 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 @@ -102,20 +102,43 @@ sealed interface DkgRitualUIState { /** * Everyone the ceremony involves, as far as this device can tell. * - * The room's members, plus anyone who has actually taken part. The union - * matters because the two can disagree: `n` is fixed from the proposal's - * p-tags, while the room's rows are local and can drift, and a participant - * who has published a host key is in the ceremony whatever the room says. + * The ceremony's own participants, plus anyone who has actually taken + * part. The union matters because the two can disagree: `n` is fixed from + * the proposal's p-tags, and a participant who has published a host key is + * in the ceremony whatever any local row says. + * + * The **ceremony's** set, not the room's, and the difference is the whole + * ladder. A subgroup's ceremony runs in the parent's room, so the room's + * roster would draw a row per parent member and report the ceremony + * waiting on people it was never with. `participantsOf` falls back to the + * room for a ceremony recorded before that set was stored, which is right + * for those: they ran in a room whose members were exactly their + * participants. + * + * Profiles still come off the room, since the room is where the rows with + * names in them are -- for a subgroup that is the parent, which holds + * every admin the ceremony can have. */ val ritualMembers: List get() { - val known = localChatRoom.localParticipants.distinctBy { it.participant.participantPublicKey } - val knownKeys = known.map { it.participant.participantPublicKey }.toSet() + val roomMembers = localChatRoom.localParticipants + .distinctBy { it.participant.participantPublicKey } + .associateBy { it.participant.participantPublicKey } - val strangers = (hostKeyParticipants + round1Participants + round2Participants) - .filterNot { it in knownKeys } - .map { LocalParticipant(Participant(participantPublicKey = it, chatRoomId = localChatRoom.chatRoom.id, relayHint = null), null) } + val participants = session + ?.let { ChillDkgRitualManager.participantsOf(it, localChatRoom) } + ?: roomMembers.keys - return known + strangers + return (participants + hostKeyParticipants + round1Participants + round2Participants) + .map { publicKey -> + roomMembers[publicKey] ?: LocalParticipant( + Participant( + participantPublicKey = publicKey, + chatRoomId = localChatRoom.chatRoom.id, + relayHint = null + ), + null + ) + } } /** diff --git a/composeApp/src/jvmTest/kotlin/press/mantra/compose/managers/SubgroupManagerJvmTest.kt b/composeApp/src/jvmTest/kotlin/press/mantra/compose/managers/SubgroupManagerJvmTest.kt index e1400675..5953e669 100644 --- a/composeApp/src/jvmTest/kotlin/press/mantra/compose/managers/SubgroupManagerJvmTest.kt +++ b/composeApp/src/jvmTest/kotlin/press/mantra/compose/managers/SubgroupManagerJvmTest.kt @@ -346,61 +346,73 @@ class SubgroupManagerJvmTest { } @Test - fun `the group's own ceremony does not block a subgroup in the same room`(): Unit = + fun `a ceremony in the parent's room for something else does not block one`(): Unit = runBlocking { - // The collision the size rule was standing in for, made concrete. A - // subgroup over everybody derives the room the group's own ceremony was - // held in -- so the room is shared, and only `parentChatRoomId` keeps - // the two ceremonies apart. Without that scoping `proposeRitual` would - // hand back the *group's* key and quietly make the child the parent. - val parent = parentWith(listOf(alice, bob)) - val ceremonyRoomId = SubgroupManager.ceremonyRoomIdFor(setOf(alice, bob), user) + // The parent's room hosts every ceremony the group ever runs, so "is + // this room already running one" stopped being a question worth + // asking the moment the ceremony moved here. Scoping on the room + // would refuse a group's second subgroup on the strength of its + // first, which is most of what the move was for. + val parent = parentWith(listOf(alice, bob, carol)) - seedCeremonyRoom(ceremonyRoomId, listOf(alice, bob, user)) - db.dkgSessionDao().upsert(ownCeremony(ceremonyRoomId)) + db.dkgSessionDao().upsert( + subgroupCeremony(id = "other-admins", admins = setOf(alice, carol, user)) + ) + db.dkgSessionDao().upsert(ownCeremony(parentRoomId)) assertNull( refusal(parent, setOf(alice, bob)), - "the group's own ceremony is not this subgroup's", + "another subgroup's ceremony, and the room's own, are not this one", ) } @Test - fun `the same parent cannot have two subgroups over the same admins`(): Unit = runBlocking { + fun `a subgroup ceremony already under way over the same admins is refused`(): Unit = + runBlocking { + val parent = parentWith(listOf(alice, bob, carol)) + + assertNull(refusal(parent, setOf(alice, bob))) + + db.dkgSessionDao().upsert( + subgroupCeremony(id = "running", admins = setOf(alice, bob, user)) + ) + + // Not because two subgroups over the same people are forbidden -- + // they are not, since the move -- but because nobody can answer for + // two live ceremonies at once: every admin would be asked twice, on + // two ladders, for two keys, one of which nobody will ever make a + // room from. + assertNotNull(refusal(parent, setOf(alice, bob))) + + // A different admin set is a different subgroup, and untouched. + assertNull(refusal(parent, setOf(alice, carol))) + } + + @Test + fun `a finished subgroup does not stop a second over the same admins`(): Unit = runBlocking { + // The limitation that went away with the sibling room. A ceremony room + // derived from its admins meant one admin set got one subgroup forever: + // asking again handed back the first ceremony's key, so the "new" + // subgroup was the old one under a new name. In the parent's room a + // second ask is a second ceremony, with a key of its own. val parent = parentWith(listOf(alice, bob, carol)) - assertNull(refusal(parent, setOf(alice, bob))) - - // The one collision that survives, and it is degenerate: same parent, - // same admins. The ceremony room is derived from the admins and the - // ceremonies in it are told apart by their parent, so this pair has - // nothing left to distinguish them -- which is another way of saying it - // is one subgroup asked for twice. - val ceremonyRoomId = SubgroupManager.ceremonyRoomIdFor(setOf(alice, bob), user) - seedCeremonyRoom(ceremonyRoomId, listOf(alice, bob, user)) db.dkgSessionDao().upsert( - ownCeremony(ceremonyRoomId).copy(id = "s1", parentChatRoomId = parentRoomId) + subgroupCeremony(id = "done", admins = setOf(alice, bob, user)) + .copy(stage = DkgRitualStage.COMPLETE) ) - assertNotNull(refusal(parent, setOf(alice, bob))) - - // And a different admin set is a different room, so it is untouched. - assertNull(refusal(parent, setOf(alice, carol))) + assertNull(refusal(parent, setOf(alice, bob))) } @Test - fun `the ceremony room is the same whichever way the admins are ordered`() { - // Every device has to land on the same room without being told, which is - // what makes a ceremony findable at all -- and what makes two members - // starting the same subgroup meet rather than fork. + fun `the stored participant set is the same whichever way the admins were picked`() { + // The coordinator assembles the set from a picker and every other device + // from the proposal's p-tags. They have to match as strings or the guard + // above finds nothing and every ask opens a ceremony. assertEquals( - SubgroupManager.ceremonyRoomIdFor(setOf(alice, bob), user), - SubgroupManager.ceremonyRoomIdFor(setOf(bob, alice), user) - ) - // And the coordinator is in it whether or not they were ticked. - assertEquals( - SubgroupManager.ceremonyRoomIdFor(setOf(alice, bob), user), - SubgroupManager.ceremonyRoomIdFor(setOf(alice, bob, user), user) + DkgSession.formatParticipants(setOf(alice, bob, user)), + DkgSession.formatParticipants(setOf(user, bob, alice)) ) } @@ -488,24 +500,15 @@ class SubgroupManagerJvmTest { * The NIP-17 room a ceremony runs in, stood up the way the picker does it -- * room first, because `DkgSession.chatRoomId` is a foreign key onto it. */ - private suspend fun seedCeremonyRoom(expectedId: String, members: List) { - members.forEach { seedProfile(it) } - - val room = assertNotNull( - db.nostrNip17Dao().createNip17ChatRoom( - userPublicKey = user, - participantPublicKeys = members.filterNot { it == user }, - subject = "Translation team", - ) + /** A ceremony held in the parent's room to make a subgroup run by [admins]. */ + private fun subgroupCeremony(id: String, admins: Set) = + ownCeremony(parentRoomId).copy( + id = id, + parentChatRoomId = parentRoomId, + participantPublicKeys = DkgSession.formatParticipants(admins), + participantCount = admins.size, ) - assertEquals( - expectedId, - room.chatRoom.id, - "the derived ceremony room is the one createNip17ChatRoom lands on", - ) - } - /** A ceremony held in [chatRoomId] for no subgroup: the group's own. */ private fun ownCeremony(chatRoomId: String) = DkgSession( id = "own-ceremony", @@ -546,7 +549,7 @@ class SubgroupManagerJvmTest { SharedKeyDerivation.marmotGroupId(material.thresholdPublicKey.value.toHex(), path) private suspend fun refusal(parent: LocalChatRoom, admins: Set): String? = - SubgroupManager.refuseCeremonyRoom( + SubgroupManager.refuseSubgroup( database = db, parentRoom = parent, adminPublicKeys = admins, @@ -557,7 +560,7 @@ class SubgroupManagerJvmTest { * A parent this device could actually make a subgroup of: an admin of it, and * holding a share of its key. * - * Both are conditions of `refuseCeremonyRoom` before it looks at the picking + * Both are conditions of `refuseSubgroup` before it looks at the picking * at all, so a fixture without them tests only the first refusal. [canSign] * and [isAdmin] are here so the tests that are *about* those two can turn * each off on its own. @@ -574,7 +577,7 @@ class SubgroupManagerJvmTest { * that null. * * [canSign] and [isAdmin] are switchable so the tests about those two can turn - * each off on its own; both are conditions `refuseCeremonyRoom` checks before + * each off on its own; both are conditions `refuseSubgroup` checks before * it looks at the picking at all. */ private suspend fun parentWith(