From 4e6ca0a08032b0405fea7ca87a54420557476a09 Mon Sep 17 00:00:00 2001 From: Kgothatso Ngako Date: Fri, 2 Oct 2026 19:45:28 +0200 Subject: [PATCH 1/4] fix(dkg): stop the key ceremony screen offering to start the ceremony it is showing "Start key ceremony" is not a button, it is the whole no-ceremony screen -- the explainer, the quorum picker and the FAB together -- so anything that leaves `DkgRitualUIState.Loaded` saying there is no session puts that offer in front of a member watching one run. Two unrelated things did, and they fail differently: one flashes, one sticks. **The state went up before the rows were read.** `initiate()` published a bare `Loaded(localChatRoom)` -- session null, `roomHasCeremony` false -- and *then* started the two flows that answer both questions. A cold open is the only time those queries are slow, and it is exactly when a member arrives from the transcript's notice, so the offer was drawn over a running ceremony for as long as they took. Both answers are a row lookup, so they are now read with `first()` before anything is published and the state goes up already correct. The flows still run; they only keep it current. **Seven flows were folding into one `var ... by mutableStateOf`.** The ceremony, the room's other ceremonies, the ceremony's messages, the key state, whether the key state is signed, the certificate and the certificate's signing -- each of them a read, a `copy` and an assignment, every one emitting on Room's own executor. Two emissions arriving together silently dropped one: whichever read first wrote last, over the top of the other's change. The field that went missing most visibly was `session`, copied back to null by a watcher that had read the state before the ceremony landed in it. That is not a flash. The session flow only re-emits when the `DkgSession` table changes, and a ceremony waiting on a member who has not opened the app changes nothing, so the screen sat there offering to start the ceremony it was showing. `MutableStateFlow` with every write through `update { }` -- a compare-and-set that retries on a loss, so a change can only ever be folded into the state that is current. The one rule it imposes is that the block re-runs, so it must be pure: `pendingApproval` is a suspending database lookup that used to sit *inside* the `copy`, and it now happens above the call. `updateLoaded` carries the `as? Loaded ?: return` that all twenty call sites used to write by hand. **Two knock-ons.** `canStartRitual()` and `quorumRange()` were read from the composable and are now passed the state, because reading a flow's `.value` inside composition observes nothing -- the offer would have been decided once and never revised. Passing the rendered state is also more correct while `ScreenStateTransition` still holds the outgoing one. `DkgRitualScreen` and `DkgApprovalScaffold` collect with `collectAsState()`. **The button itself is right and stays.** Creating a robust group already opens the ceremony -- `SelectChatRoomTypeViewModel.createNip17ChatRoom` calls `proposeRitual` immediately after `createNip17ChatRoom` -- and this screen's offer is the deliberate fallback for the case where that failed (`keyCeremonyNotStarted`) and for rooms created before it existed. What was broken is when it appeared, not that it exists. **Tests.** `DkgRitualStartOfferJvmTest`, two cases, and each was checked against its own regression reinstated rather than only against the fix. The first asserts the *first* `Loaded` already knows the room is running a ceremony, with a deliberately slow query so the window is proved gone rather than hidden. The second forces the lost update to happen rather than racing into it: the session watcher is parked inside `pendingApproval` while the key-state watcher writes, and the write that follows must not carry the stale copy over it. **Not done.** `HomeViewModel`, `ChatMessageListViewModel`, `ProposalListViewModel` and others still declare `by mutableStateOf` in a view model constructor. They are only at risk where more than one flow folds into one state object; none was audited here. 964 jvm tests, 454 common tests, `m3Audit` meeting every budget. Co-Authored-By: Claude Opus 5 --- .../ui/composable/DkgApprovalScaffold.kt | 3 +- .../compose/ui/composable/DkgRitualScreen.kt | 7 +- .../ui/view/model/DkgRitualViewModel.kt | 271 +++++++++++------- .../view/model/DkgRitualStartOfferJvmTest.kt | 247 ++++++++++++++++ 4 files changed, 428 insertions(+), 100 deletions(-) create mode 100644 composeApp/src/jvmTest/kotlin/press/mantra/compose/ui/view/model/DkgRitualStartOfferJvmTest.kt diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/DkgApprovalScaffold.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/DkgApprovalScaffold.kt index cc358aa5..d7513e87 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/DkgApprovalScaffold.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/DkgApprovalScaffold.kt @@ -22,6 +22,7 @@ import androidx.compose.material3.Text import androidx.compose.material3.TextButton import androidx.compose.material3.TopAppBar import androidx.compose.runtime.Composable +import androidx.compose.runtime.collectAsState import androidx.compose.runtime.LaunchedEffect import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier @@ -100,7 +101,7 @@ internal fun DkgApprovalScaffold( ) } ) { padding -> - when (val state = dkgRitualViewModel.dkgRitualUIState) { + when (val state = dkgRitualViewModel.dkgRitualUIState.collectAsState().value) { is DkgRitualUIState.Loading -> Column( modifier = Modifier.fillMaxWidth().padding(padding).readableContent().padding(MaterialTheme.spacing.space250), horizontalAlignment = Alignment.CenterHorizontally 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 f669285c..3ea26e01 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 @@ -37,6 +37,7 @@ import androidx.compose.material3.Surface import androidx.compose.material3.Text import androidx.compose.material3.TopAppBar import androidx.compose.runtime.Composable +import androidx.compose.runtime.collectAsState import androidx.compose.runtime.LaunchedEffect import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier @@ -154,7 +155,7 @@ fun DkgRitualScreen( ) ) - ScreenStateTransition(dkgRitualViewModel.dkgRitualUIState) { uiState -> + ScreenStateTransition(dkgRitualViewModel.dkgRitualUIState.collectAsState().value) { uiState -> when (val dkgRitualUIState = uiState) { is DkgRitualUIState.Error -> { Column( @@ -168,7 +169,7 @@ fun DkgRitualScreen( is DkgRitualUIState.Loaded -> { val session = dkgRitualUIState.session val participantCount = dkgRitualUIState.participantCount - val canStartRitual = dkgRitualViewModel.canStartRitual() + val canStartRitual = dkgRitualViewModel.canStartRitual(dkgRitualUIState) val isActionPending = dkgRitualViewModel.isActionPending.value Scaffold( @@ -286,7 +287,7 @@ fun DkgRitualScreen( QuorumStepper( threshold = dkgRitualViewModel.threshold.value, participantCount = participantCount, - quorumRange = dkgRitualViewModel.quorumRange(), + quorumRange = dkgRitualViewModel.quorumRange(dkgRitualUIState), onThresholdChange = dkgRitualViewModel::setThreshold ) } else { 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 76d61b2b..feb4316f 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 @@ -1,9 +1,7 @@ package press.mantra.compose.ui.view.model import androidx.compose.runtime.MutableState -import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf -import androidx.compose.runtime.setValue import androidx.lifecycle.ViewModel import androidx.lifecycle.ViewModelProvider import androidx.lifecycle.viewModelScope @@ -36,7 +34,10 @@ import press.mantra.compose.identity.Identity import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.IO import kotlinx.coroutines.Job +import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch /** @@ -60,8 +61,51 @@ class DkgRitualViewModel( val dkgRepository: DkgRepository, ): ViewModel() { - var dkgRitualUIState: DkgRitualUIState by mutableStateOf(initialDkgRitualUIState) - private set + private val _dkgRitualUIState = MutableStateFlow(initialDkgRitualUIState) + + /** + * What the screen draws -- a flow rather than a `mutableStateOf`, because + * seven flows write it. + * + * The ceremony, the room's other ceremonies, the ceremony's messages, the + * key state, whether the key state is signed, the certificate and the + * certificate's signing all fold into this one object, and every one of + * them emits on Room's own executor. Held + * as a `var ... by mutableStateOf`, each of those was a read, a `copy` and + * an assignment with a suspension point in the middle, so two emissions + * arriving together silently dropped one: whichever read first wrote last, + * over the top of the other's change. + * + * The change that went missing most visibly was [DkgRitualUIState.Loaded.session], + * copied back to null by a watcher that had read the state before the + * ceremony landed in it -- and a screen with no session is the screen that + * offers to start one. It stayed that way until some other table changed and + * the session flow emitted again, which for a ceremony waiting on an absent + * member is never. + * + * [MutableStateFlow.update] retries on a losing compare-and-set, so a change + * can only be folded into the state that is actually current. Every write + * below goes through it -- see [updateLoaded]. It is the same rule the + * wallet view models already keep, for the same reason. + */ + val dkgRitualUIState: StateFlow = _dkgRitualUIState.asStateFlow() + + /** + * Folds a change into the loaded state, atomically, and does nothing when + * the screen is not in it -- the `as? Loaded ?: return` that every one of + * these call sites used to write by hand. + * + * [block] is re-run when it loses the race, so it has to be a pure function + * of the state it is handed: read nothing else out of the view model inside + * it, and do the suspending work above the call. + */ + private inline fun updateLoaded( + block: (DkgRitualUIState.Loaded) -> DkgRitualUIState.Loaded + ) { + _dkgRitualUIState.update { current -> + if (current is DkgRitualUIState.Loaded) block(current) else current + } + } private val logger = Logger.withTag(TAG) @@ -101,9 +145,15 @@ class DkgRitualViewModel( * Any member may: ChillDKG gives the coordinator no say in the outcome, and a * NIP-17 room records no creator to reserve the job for. What rules it out is * a ceremony already running, or a group too small to split a key across. + * + * [state] is passed in by the screen rather than read off the view model, + * because the state is a flow now and reading `.value` inside composition + * observes nothing -- the offer would be decided once and never revised. The + * screen hands in the state it is drawing, which is also the only state the + * answer should be about while a transition still holds the outgoing one. */ - fun canStartRitual(): Boolean { - val loaded = dkgRitualUIState as? DkgRitualUIState.Loaded ?: return false + fun canStartRitual(state: DkgRitualUIState = dkgRitualUIState.value): Boolean { + val loaded = state as? DkgRitualUIState.Loaded ?: return false if (loaded.session != null && loaded.session.stage != DkgRitualStage.FAILED) return false @@ -116,8 +166,8 @@ class DkgRitualViewModel( return ChillDkgRitualManager.canRunRitual(loaded.localChatRoom) } - fun quorumRange(): IntRange { - val loaded = dkgRitualUIState as? DkgRitualUIState.Loaded + fun quorumRange(state: DkgRitualUIState = dkgRitualUIState.value): IntRange { + val loaded = state as? DkgRitualUIState.Loaded ?: return ChatRoomType.quorumRange(ChatRoomType.MINIMUM_QUORUM) // Below the minimum the range runs backwards and is empty, which `coerceIn` @@ -137,17 +187,10 @@ class DkgRitualViewModel( viewModelScope.launch(Dispatchers.IO) { val localChatRoom = chatRepository.getChatRoomByIdentifier(chatRoomId) if (localChatRoom == null) { - dkgRitualUIState = DkgRitualUIState.Error("Couldn't load this chat") + _dkgRitualUIState.value = DkgRitualUIState.Error("Couldn't load this chat") return@launch } - dkgRitualUIState = DkgRitualUIState.Loaded(localChatRoom = localChatRoom) - threshold.value = ChatRoomType - .defaultQuorum(ChillDkgRitualManager.memberPublicKeys(localChatRoom).size) - .coerceIn(quorumRange()) - - observeKeyState() - // Room-scoped, deliberately. Only the member who opened a subgroup // arrives here knowing it is one -- everybody else reaches this screen // from the room, through the transcript's ritual notice or the group's @@ -171,30 +214,65 @@ class DkgRitualViewModel( // 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 + val roomSessions = dkgRepository.observeLatestSessionForChatRoom(chatRoomId) - dkgRitualUIState = current.copy( - roomHasCeremony = newest != null && newest.stage != DkgRitualStage.FAILED - ) + // Both answers are read here, before anything is published, rather + // than awaited after a bare `Loaded` has gone up. A `Loaded` with no + // session and no ceremony *is* the screen that offers to start one, + // so publishing it first flashed "start key ceremony" over a ceremony + // already running -- for as long as the two queries took, which is + // long enough to read and long enough to tap. The flows below keep + // both current from here on; this only settles what the first frame + // says. + val shownSession = sessions.first() + val newestSession = roomSessions.first() + + _dkgRitualUIState.value = DkgRitualUIState.Loaded( + localChatRoom = localChatRoom, + session = shownSession, + parentChatRoomId = shownSession?.parentChatRoomId, + pendingApproval = shownSession?.let { dkgRepository.pendingApproval(it) }, + roomHasCeremony = newestSession != null && + newestSession.stage != DkgRitualStage.FAILED + ) + + threshold.value = ChatRoomType + .defaultQuorum(ChillDkgRitualManager.memberPublicKeys(localChatRoom).size) + .coerceIn(quorumRange()) + + observeKeyState() + + launch { + roomSessions.collect { newest -> + updateLoaded { current -> + current.copy( + roomHasCeremony = newest != null && + newest.stage != DkgRitualStage.FAILED + ) + } } } sessions.collect { session -> - val loaded = (dkgRitualUIState as? DkgRitualUIState.Loaded) - ?: DkgRitualUIState.Loaded(localChatRoom = localChatRoom) - // 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, - parentChatRoomId = parent, - pendingApproval = session?.let { dkgRepository.pendingApproval(it) } - ) + // Above the update, not inside it: `update` re-runs its block when + // it loses the race, and a block that re-runs cannot suspend. + val pending = session?.let { dkgRepository.pendingApproval(it) } + + _dkgRitualUIState.update { current -> + val loaded = current as? DkgRitualUIState.Loaded + ?: DkgRitualUIState.Loaded(localChatRoom = localChatRoom) + + loaded.copy( + session = session, + parentChatRoomId = parent, + pendingApproval = pending + ) + } // Re-point the per-ceremony watchers whenever the ceremony or its // purpose changes, which for a member arriving cold is once. @@ -212,23 +290,32 @@ class DkgRitualViewModel( if (session != null) { messageObserver = viewModelScope.launch(Dispatchers.IO) { dkgRepository.observeMessages(session.id).collect { messages -> - val current = dkgRitualUIState as? DkgRitualUIState.Loaded ?: return@collect - fun senders(kind: Kind) = messages .filter { it.kind == kind } .map { it.participantPublicKey } .toSet() - dkgRitualUIState = current.copy( - hostKeyParticipants = senders(DkgRitualEvents.HOST_KEY), - round1Participants = senders(DkgRitualEvents.ROUND_1), - round2Participants = senders(DkgRitualEvents.ROUND_2), - // Recomputed here as well as on the session, because - // whether round 1 can be asked for depends on how many - // host keys have arrived, which only this flow sees. - pendingApproval = current.session - ?.let { dkgRepository.pendingApproval(it) } - ) + // Recomputed here as well as on the session, because + // whether round 1 can be asked for depends on how many + // host keys have arrived, which only this flow sees. + val pending = dkgRepository.pendingApproval(session) + + updateLoaded { current -> + // This watcher belongs to one ceremony, and + // cancelling it does not reach into a body already + // running -- so say which ceremony these counts are + // for rather than letting them land on the next. + if (current.session?.id != session.id) { + return@updateLoaded current + } + + current.copy( + hostKeyParticipants = senders(DkgRitualEvents.HOST_KEY), + round1Participants = senders(DkgRitualEvents.ROUND_1), + round2Participants = senders(DkgRitualEvents.ROUND_2), + pendingApproval = pending + ) + } } } } @@ -257,17 +344,15 @@ class DkgRitualViewModel( // before there is a room -- which is the whole point of asking it. launch { dkgRepository.observeSigningSessions(chatRoomId).collect { sessions -> - val loaded = dkgRitualUIState as? DkgRitualUIState.Loaded ?: return@collect + val keyState = sessions.firstOrNull { local -> + local.items.any { item -> + runCatching { + Event.fromJson(item.unsignedEventJson).kind + }.getOrNull() == GroupKeyStateEvent.KIND + } + }?.session - dkgRitualUIState = loaded.copy( - keyStateSession = sessions.firstOrNull { local -> - local.items.any { item -> - runCatching { - Event.fromJson(item.unsignedEventJson).kind - }.getOrNull() == GroupKeyStateEvent.KIND - } - }?.session - ) + updateLoaded { it.copy(keyStateSession = keyState) } } } @@ -293,9 +378,9 @@ class DkgRitualViewModel( certificateObserver?.cancel() if (dkgSessionId == null) { - dkgRitualUIState = (dkgRitualUIState as? DkgRitualUIState.Loaded) - ?.copy(isKeyStateSigned = false, isCertified = false, certificateSession = null) - ?: dkgRitualUIState + updateLoaded { + it.copy(isKeyStateSigned = false, isCertified = false, certificateSession = null) + } return } @@ -306,41 +391,33 @@ class DkgRitualViewModel( // about this one's key. launch { dkgRepository.observeSignedGroupKeyState(dkgSessionId).collect { state -> - val loaded = dkgRitualUIState as? DkgRitualUIState.Loaded ?: return@collect - - dkgRitualUIState = loaded.copy(isKeyStateSigned = state != null) + updateLoaded { it.copy(isKeyStateSigned = state != null) } } } if (parentChatRoomId == null) { - dkgRitualUIState = (dkgRitualUIState as? DkgRitualUIState.Loaded) - ?.copy(isCertified = false, certificateSession = null) - ?: dkgRitualUIState + updateLoaded { it.copy(isCertified = false, certificateSession = null) } return@launch } launch { dkgRepository.observeBirthCertificate(dkgSessionId, parentChatRoomId) .collect { certificate -> - val loaded = dkgRitualUIState as? DkgRitualUIState.Loaded ?: return@collect - - dkgRitualUIState = loaded.copy(isCertified = certificate != null) + updateLoaded { it.copy(isCertified = certificate != null) } } } launch { dkgRepository.observeSigningSessions(parentChatRoomId).collect { sessions -> - val loaded = dkgRitualUIState as? DkgRitualUIState.Loaded ?: return@collect + val certificate = sessions.firstOrNull { local -> + local.items.any { item -> + runCatching { + Event.fromJson(item.unsignedEventJson).kind + }.getOrNull() == SubgroupBirthCertificateEvent.KIND + } + }?.session - dkgRitualUIState = loaded.copy( - certificateSession = sessions.firstOrNull { local -> - local.items.any { item -> - runCatching { - Event.fromJson(item.unsignedEventJson).kind - }.getOrNull() == SubgroupBirthCertificateEvent.KIND - } - }?.session - ) + updateLoaded { it.copy(certificateSession = certificate) } } } } @@ -359,7 +436,7 @@ class DkgRitualViewModel( fun proposeKeyState() { if (isActionPending.value) return - val loaded = dkgRitualUIState as? DkgRitualUIState.Loaded ?: return + val loaded = dkgRitualUIState.value as? DkgRitualUIState.Loaded ?: return val session = loaded.session ?: return if (session.thresholdPublicKey == null) return @@ -398,7 +475,7 @@ class DkgRitualViewModel( if (signing == null) { logger.e("Failed to propose the key state for $chatRoomId") - dkgRitualUIState = DkgRitualUIState.Error( + _dkgRitualUIState.value = DkgRitualUIState.Error( "Couldn't ask the group to agree its signing key. Please try again." ) } @@ -421,7 +498,7 @@ class DkgRitualViewModel( fun proposeBirthCertificate() { if (isActionPending.value) return - val loaded = dkgRitualUIState as? DkgRitualUIState.Loaded ?: return + val loaded = dkgRitualUIState.value as? DkgRitualUIState.Loaded ?: return // The state's, not the route's. Every step after the ceremony is open to // any of the subgroup's admins -- see `SubgroupManager` -- and they reach // this screen from the room, with no route to have told them anything. @@ -459,7 +536,7 @@ class DkgRitualViewModel( if (signing == null) { logger.e("Failed to propose a birth certificate in $parent") - dkgRitualUIState = DkgRitualUIState.Error( + _dkgRitualUIState.value = DkgRitualUIState.Error( "Couldn't ask the group to certify this subgroup. Please try again." ) } @@ -470,11 +547,11 @@ class DkgRitualViewModel( fun startRitual() { if (isActionPending.value) return - val loaded = dkgRitualUIState as? DkgRitualUIState.Loaded ?: return + val loaded = dkgRitualUIState.value as? DkgRitualUIState.Loaded ?: return val nostrPrivateKey = activeIdentityStateFlow.value?.nostrPrivateKey if (nostrPrivateKey == null) { - dkgRitualUIState = DkgRitualUIState.Error("Couldn't read your keys. Please try again.") + _dkgRitualUIState.value = DkgRitualUIState.Error("Couldn't read your keys. Please try again.") return } @@ -492,7 +569,7 @@ class DkgRitualViewModel( if (session == null) { logger.e("Failed to start DKG ritual for $chatRoomId") - dkgRitualUIState = DkgRitualUIState.Error("Couldn't start the key ceremony. Please try again.") + _dkgRitualUIState.value = DkgRitualUIState.Error("Couldn't start the key ceremony. Please try again.") } // On success the session flow above delivers the new row; no need to // set it here and risk racing the observer. @@ -509,12 +586,12 @@ class DkgRitualViewModel( fun approve(step: DkgApprovalStep) { if (isActionPending.value) return - val loaded = dkgRitualUIState as? DkgRitualUIState.Loaded ?: return + val loaded = dkgRitualUIState.value as? DkgRitualUIState.Loaded ?: return val session = loaded.session ?: return val nostrPrivateKey = activeIdentityStateFlow.value?.nostrPrivateKey if (nostrPrivateKey == null) { - dkgRitualUIState = DkgRitualUIState.Error("Couldn't read your keys. Please try again.") + _dkgRitualUIState.value = DkgRitualUIState.Error("Couldn't read your keys. Please try again.") return } @@ -558,7 +635,7 @@ class DkgRitualViewModel( fun createAdminGroup(onNavigateToRoute: (Route) -> Unit) { if (isActionPending.value) return - val loaded = dkgRitualUIState as? DkgRitualUIState.Loaded ?: return + val loaded = dkgRitualUIState.value as? DkgRitualUIState.Loaded ?: return val session = loaded.session ?: return val thresholdPublicKey = session.thresholdPublicKey ?: return @@ -568,7 +645,7 @@ class DkgRitualViewModel( val nostrPrivateKey = activeIdentityStateFlow.value?.nostrPrivateKey if (nostrPrivateKey == null) { - dkgRitualUIState = DkgRitualUIState.Error("Couldn't read your keys. Please try again.") + _dkgRitualUIState.value = DkgRitualUIState.Error("Couldn't read your keys. Please try again.") return } @@ -594,7 +671,7 @@ class DkgRitualViewModel( ?: if (subgroupParent != null) "Subgroup" else "Group" isActionPending.value = true - dkgRitualUIState = loaded.copy(adminGroupBlockedOn = emptyList()) + updateLoaded { it.copy(adminGroupBlockedOn = emptyList()) } viewModelScope.launch(Dispatchers.IO) { val outcome = chatRepository.createMarmotGroup( @@ -617,19 +694,21 @@ class DkgRitualViewModel( // Named rather than counted. A member cannot publish a key // package on somebody else's behalf, so the only useful thing // to say is whose door to knock on. - dkgRitualUIState = loaded.copy( - adminGroupBlockedOn = outcome.missing.map { publicKey -> - loaded.ritualMembers - .firstOrNull { it.participant.participantPublicKey == publicKey } - ?.profile - ?.humanReadableNameOrPubkey() - ?: publicKey.take(12) - } - ) + updateLoaded { current -> + current.copy( + adminGroupBlockedOn = outcome.missing.map { publicKey -> + current.ritualMembers + .firstOrNull { it.participant.participantPublicKey == publicKey } + ?.profile + ?.humanReadableNameOrPubkey() + ?: publicKey.take(12) + } + ) + } } is MarmotGroupCreation.Outcome.Failed -> { - dkgRitualUIState = DkgRitualUIState.Error(outcome.reason) + _dkgRitualUIState.value = DkgRitualUIState.Error(outcome.reason) } is MarmotGroupCreation.Outcome.Created -> withContext(Main) { diff --git a/composeApp/src/jvmTest/kotlin/press/mantra/compose/ui/view/model/DkgRitualStartOfferJvmTest.kt b/composeApp/src/jvmTest/kotlin/press/mantra/compose/ui/view/model/DkgRitualStartOfferJvmTest.kt new file mode 100644 index 00000000..e82b816e --- /dev/null +++ b/composeApp/src/jvmTest/kotlin/press/mantra/compose/ui/view/model/DkgRitualStartOfferJvmTest.kt @@ -0,0 +1,247 @@ +package press.mantra.compose.ui.view.model + +import com.vitorpamplona.quartz.nip01Core.core.HexKey +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.delay +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.flow.flow +import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.resetMain +import kotlinx.coroutines.test.setMain +import kotlinx.coroutines.withTimeout +import press.mantra.compose.database.model.ChatRoom +import press.mantra.compose.database.model.DkgSession +import press.mantra.compose.database.model.FrostSigningItem +import press.mantra.compose.database.model.FrostSigningSession +import press.mantra.compose.database.model.Participant +import press.mantra.compose.database.model.intermdiate.LocalChatRoom +import press.mantra.compose.database.model.intermdiate.LocalFrostSigningSession +import press.mantra.compose.database.model.intermdiate.LocalParticipant +import press.mantra.compose.database.model.types.DkgApprovalStep +import press.mantra.compose.database.model.types.DkgRitualStage +import press.mantra.compose.repository.ChatRepository +import press.mantra.compose.repository.DkgRepository +import press.mantra.compose.ui.view.state.DkgRitualUIState +import kotlin.test.AfterTest +import kotlin.test.BeforeTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertNotNull + +/** + * The two ways the key-ceremony screen came to offer to start the ceremony it + * was already showing. + * + * "Start key ceremony" is the whole no-ceremony screen -- the explainer, the + * quorum picker and the button -- so anything that leaves the state saying + * there is no ceremony puts that offer in front of a member watching one run. + * Both of these did, and both are invisible to a test that only asks the view + * model what it settles on: the state was right before and right after, and + * wrong in between for long enough to read and to tap. + * + * Neither is about ChillDKG. They are about one `DkgRitualUIState` being + * assembled by seven flows at once, which is why they are tested here rather + * than against the ritual. + */ +@OptIn(ExperimentalCoroutinesApi::class) +class DkgRitualStartOfferJvmTest { + private val chatRoomId = "a1b2c3d4".repeat(8) + private val me = "11".repeat(32) + private val peer = "22".repeat(32) + + @BeforeTest + fun setUp() { + Dispatchers.setMain(UnconfinedTestDispatcher()) + } + + @AfterTest + fun tearDown() { + Dispatchers.resetMain() + } + + /** + * The first loaded state already knows the room is running a ceremony. + * + * The screen used to publish a bare `Loaded` and then wait for the rows, + * and a `Loaded` with no session and no ceremony *is* the offer to start + * one. On a cold open -- the only time the queries are slow -- that is + * exactly when it was shown. + */ + @Test + fun `the offer to start is not published while the rows are still being read`() = runBlocking { + val session = session(DkgRitualStage.COLLECTING_HOST_KEYS) + + val viewModel = viewModel( + object : DkgRepository by DkgRepository.NO_OP_DKG_REPOSITORY { + // Slow on purpose: a Room query answers in microseconds and + // would hide the window rather than prove it is gone. + override fun observeLatestSessionForChatRoom( + chatRoomId: String, + parentChatRoomId: String?, + ): Flow = flow { + delay(50) + emit(session) + } + } + ) + + viewModel.initiate() + + val firstLoaded = withTimeout(5_000) { + viewModel.dkgRitualUIState.first { it is DkgRitualUIState.Loaded } + } as DkgRitualUIState.Loaded + + assertEquals(session.id, firstLoaded.session?.id, "the first loaded state missed the ceremony") + assertFalse( + viewModel.canStartRitual(firstLoaded), + "offered to start a ceremony on a room already running one" + ) + } + + /** + * A write that suspends between reading the state and writing it back does + * not carry the stale copy over somebody else's change. + * + * Every flow folding into this state used to read it, build a `copy` and + * assign -- and the session watcher asks the database what this member owes + * in the middle of that. The key-state watcher writing during that pause had + * its change thrown away by the assignment that followed. The same pause in + * the other direction is what put `session` back to null and offered to + * start the ceremony on screen; this is the half of it that can be forced to + * happen rather than raced into. + */ + @Test + fun `a change written during a suspension survives the write that follows it`() = runBlocking { + val session = session(DkgRitualStage.COLLECTING_ROUND_1) + val signingSessions = MutableStateFlow>(emptyList()) + + // Held open until the test has had the key-state watcher write. The + // seeding call runs before anything else is collecting, so it is let + // through; the session watcher's call is the one that waits. + val gate = CompletableDeferred() + var calls = 0 + + val viewModel = viewModel( + object : DkgRepository by DkgRepository.NO_OP_DKG_REPOSITORY { + override fun observeLatestSessionForChatRoom( + chatRoomId: String, + parentChatRoomId: String?, + ): Flow = flowOf(session) + + override fun observeSigningSessions( + chatRoomId: String + ): Flow> = signingSessions + + override suspend fun pendingApproval(session: DkgSession): DkgApprovalStep? { + calls += 1 + if (calls == 1) return null + + gate.await() + return DkgApprovalStep.ROUND_1 + } + } + ) + + viewModel.initiate() + + withTimeout(5_000) { viewModel.dkgRitualUIState.first { it is DkgRitualUIState.Loaded } } + + // The watcher is now parked inside `pendingApproval`, holding a copy of + // the state as it was before this line. + signingSessions.value = listOf(keyStateSigningSession()) + + withTimeout(5_000) { + viewModel.dkgRitualUIState.first { + (it as? DkgRitualUIState.Loaded)?.keyStateSession != null + } + } + + gate.complete(Unit) + + val settled = withTimeout(5_000) { + viewModel.dkgRitualUIState.first { + (it as? DkgRitualUIState.Loaded)?.pendingApproval == DkgApprovalStep.ROUND_1 + } + } as DkgRitualUIState.Loaded + + assertNotNull( + settled.keyStateSession, + "the session watcher wrote back a copy taken before the key state arrived" + ) + assertEquals(session.id, settled.session?.id) + } + + private fun viewModel(dkgRepository: DkgRepository) = DkgRitualViewModel( + chatRoomId = chatRoomId, + activeUserPublicKey = me, + initialDkgRitualUIState = DkgRitualUIState.Loading, + activeIdentityStateFlow = MutableStateFlow(null), + chatRepository = object : ChatRepository by ChatRepository.NO_OP_CHAT_REPOSITORY { + override suspend fun getChatRoomByIdentifier(id: HexKey): LocalChatRoom? = room() + }, + dkgRepository = dkgRepository + ) + + private fun room() = LocalChatRoom( + chatRoom = ChatRoom( + id = chatRoomId, + userPublicKey = me, + subject = "Ekklesia", + description = null, + mlsGroupState = null, + ), + localParticipants = listOf(me, peer).map { publicKey -> + LocalParticipant( + participant = Participant( + participantPublicKey = publicKey, + chatRoomId = chatRoomId, + relayHint = null, + ), + profile = null, + ) + }, + ) + + private fun session(stage: DkgRitualStage) = DkgSession( + id = "ceremony", + chatRoomId = chatRoomId, + coordinatorPublicKey = me, + userPublicKey = me, + threshold = 2, + participantCount = 2, + hostPublicKey = me, + round1Random = "66".repeat(32), + round2AuxRandom = "77".repeat(32), + stage = stage, + ) + + private fun keyStateSigningSession() = LocalFrostSigningSession( + session = FrostSigningSession( + id = "signing", + chatRoomId = chatRoomId, + coordinatorPublicKey = me, + userPublicKey = me, + dkgSessionId = "ceremony", + threshold = 2, + participantCount = 2, + signerId = 0, + ), + unorderedItems = listOf( + FrostSigningItem( + sessionId = "signing", + itemIndex = 0, + unsignedEventJson = """{"id":"${"33".repeat(32)}","pubkey":"$me",""" + + """"created_at":0,"kind":30326,"tags":[],"content":"","sig":"${"44".repeat(64)}"}""", + eventId = "33".repeat(32), + nonceRandom = "55".repeat(32), + ) + ), + ) +} From a14ea3d9d3cbf6293efac012447424841efb7f49 Mon Sep 17 00:00:00 2001 From: Kgothatso Ngako Date: Fri, 2 Oct 2026 19:46:06 +0200 Subject: [PATCH 2/4] feat(dkg): every key ceremony a room holds, pending first A room holds more than one ceremony as a matter of course, and has done since a subgroup's ChillDKG moved into its parent's Marmot room: the parent's own, long finished, plus one for every subgroup the group has ever made. Nothing in the app could name the second of them. `DkgRitualRoute` without a session id shows the room's *newest*, which is routinely the finished one with the subgroup's request buried underneath it. The group's details reached only that. The transcript could name one, by scrolling to the line it was about -- which for a subgroup means scrolling past whatever else the room has been used for, and is impossible for a ceremony whose line never arrived. **`DkgRitualListScreen`, deliberately the same shape as `ProposalListScreen`.** One row per ceremony off `observeSessionsForChatRoom`, split into "waiting for you" and "everything else". Split rather than sorted, for the reason `ProposalListUIState` gives: the rest of the list is what the room has done, these are what it is waiting on the reader for, and they are a different kind of thing to read. The finished ones are kept, because how this group came to hold the keys it holds is the other half of what the screen is for. **What "waiting on you" means is two things, and they are owed by different people.** An approval gate this member cannot delegate, and a finished ceremony whose room only its coordinator can create -- a ceremony reaching COMPLETE finishes nothing on its own, and nothing else in the app would tell the coordinator so. Both are asked of `DkgRepository` rather than re-derived, the same readings the transcript's standing notice and the ceremony screen make, so the three cannot disagree. An abandoned ceremony is asked neither: there is no step left to approve and no key to build a room from, and a row claiming otherwise would send a member to a screen with nothing on it. **A row says what the key is for before it says where it has got to**, because a room holding two holds them for different groups and a row that only reported progress would leave a member guessing which. A subgroup is named by its proposal, so there is a third branch for the member who arrives before that name has -- it still says which of the two kinds of key this is. The quorum is on the row too: a ceremony fixes it and it cannot be changed afterwards. **Three entry points, two of which were gaps.** `ChatRoomDetailScreen` gets a "Key ceremonies" row in *every* room, including the Marmot ones the `mlsGroupState` check keeps "Shared key" out of. The two are different questions and both are kept: "Shared key" offers a room a key of its own, which only a NIP-17 room can want; this lists the ceremonies a room *holds*. A parent's Marmot room has never had a door to its subgroups' ceremonies at all. The transcript's `CeremoniesAwaitingYouNotice` says "3 key ceremonies need you" and then opened `ceremonies.first()` -- answering a different question from the one it asked, and leaving the other two exactly as hard to find as before. It now opens the list whenever it stands for more than one, which is what the signing notice beside it already did. A ceremony line in the transcript routes to the list when it cannot name a session -- a line written before chat rows carried one -- or when the ceremony it names is not the only one waiting on this member. `hidesOtherCeremonies` mirrors `hidesOtherDecisions` exactly, including its restraint: only for a ceremony that is itself waiting on them, because a line about a finished one is history and a reader who taps it is asking to see that, not to be handed a queue. **An empty list offers the way out of itself**, rather than ending in a blank page that sends a member back to the group's details to find the other door. It only offers to ask; the ceremony screen still decides whether the group is in a position to hold one. `DkgRitualListViewModel` is a `MutableStateFlow` from the start rather than a `by mutableStateOf`, for the reason the commit before this one found. **Tests, 6 cases in 2 files.** `DkgRitualListViewModelJvmTest` covers the three decisions nothing else can check: that a ceremony wanting something is at the top even when its date is not, that only the coordinator is told to finish setting up a finished one, and that an abandoned ceremony is waiting on nobody even when both lookups say it is. `DkgRitualListScreenJvmTest` renders it and checks the thing that is only true once drawn -- that a row opens the ceremony it names rather than the room's newest, which is the whole reason the screen exists -- plus the two headings and the empty state's offer. **Not done.** Nobody has looked at this screen running; the render tests prove it composes, sections and routes, which is not the same thing. 970 jvm tests, 454 common tests, `m3Audit` meeting every budget. Co-Authored-By: Claude Opus 5 --- .../composeResources/values/strings.xml | 13 + .../ui/composable/ChatRoomDetailScreen.kt | 35 ++ .../ui/composable/ChatRoomMessagingScreen.kt | 36 +- .../ui/composable/DkgRitualListScreen.kt | 504 ++++++++++++++++++ .../ui/composable/navigation/MantraNavHost.kt | 18 + .../navigation/routes/DkgRitualListRoute.kt | 23 + .../composable/widgets/chat/ChatTranscript.kt | 9 +- .../ui/view/model/ChatMessageListViewModel.kt | 19 + .../ui/view/model/DkgRitualListViewModel.kt | 140 +++++ .../ui/view/state/DkgRitualListUIState.kt | 68 +++ .../composable/DkgRitualListScreenJvmTest.kt | 157 ++++++ .../model/DkgRitualListViewModelJvmTest.kt | 198 +++++++ 12 files changed, 1213 insertions(+), 7 deletions(-) create mode 100644 composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/DkgRitualListScreen.kt create mode 100644 composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/navigation/routes/DkgRitualListRoute.kt create mode 100644 composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/DkgRitualListViewModel.kt create mode 100644 composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/state/DkgRitualListUIState.kt create mode 100644 composeApp/src/jvmTest/kotlin/press/mantra/compose/ui/composable/DkgRitualListScreenJvmTest.kt create mode 100644 composeApp/src/jvmTest/kotlin/press/mantra/compose/ui/view/model/DkgRitualListViewModelJvmTest.kt diff --git a/composeApp/src/commonMain/composeResources/values/strings.xml b/composeApp/src/commonMain/composeResources/values/strings.xml index 771819ef..2f5f0b2d 100644 --- a/composeApp/src/commonMain/composeResources/values/strings.xml +++ b/composeApp/src/commonMain/composeResources/values/strings.xml @@ -501,4 +501,17 @@ Accept as a parent A group is a subgroup of another only once its own quorum says so. Accepting asks this group to agree what it signs with again, naming them. Asked this group to accept that parent + + Key ceremonies + This group has not held a key ceremony. + The group's shared key + A subgroup's key + A subgroup: %1$s + Waiting for members to join. + Waiting for everyone to commit their part. + Waiting for everyone to confirm the result. + The group has a key from this ceremony. + Abandoned. No key was created. + %1$s of %2$s members will sign with it + Set up a shared key 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 b490e05d..9125dfbb 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 @@ -70,6 +70,7 @@ import press.mantra.compose.ui.composable.navigation.routes.ImplementationPendin import press.mantra.compose.ui.composable.navigation.routes.ProfilePreviewRoute import press.mantra.compose.ui.composable.navigation.routes.Route import press.mantra.compose.ui.composable.navigation.routes.DkgRitualRoute +import press.mantra.compose.ui.composable.navigation.routes.DkgRitualListRoute import press.mantra.compose.ui.composable.navigation.routes.ProposalListRoute import press.mantra.compose.ui.composable.navigation.routes.SearchMemberToAddToChatRoomRoute import press.mantra.compose.extensions.toFormattedTimeAndDateString @@ -123,6 +124,7 @@ import mantra.composeapp.generated.resources.no_projects_exists_in_this_group import mantra.composeapp.generated.resources.projects import mantra.composeapp.generated.resources.proposals import mantra.composeapp.generated.resources.reindex_events +import mantra.composeapp.generated.resources.key_ceremonies import mantra.composeapp.generated.resources.shared_key import mantra.composeapp.generated.resources.signing_key import mantra.composeapp.generated.resources.the_key_this_room_signs_as @@ -742,6 +744,39 @@ fun ChatRoomDetailScreen( } } + // Every room, including the Marmot ones the `mlsGroupState` + // check above keeps "Shared key" out of. That button offers + // a room a key of its own, which only a NIP-17 room can + // want; this one lists the ceremonies a room *holds*, and a + // parent's Marmot room holds one per subgroup the group has + // ever made. Until this there was no way to reach the second + // of them except by scrolling the transcript to its request + // line, and no way at all to reach one whose line had never + // arrived. + item { + TextButton( + onClick = { + onNavigateToRoute.invoke( + DkgRitualListRoute( + activeUserPublicKey = activeUserPublicKey, + chatRoomId = chatRoomId + ) + ) + } + ) { + Icon( + Icons.Default.Key, + contentDescription = "Key ceremonies" + ) + + Spacer( + modifier = Modifier.width(MaterialTheme.spacing.space125) + ) + + Text(stringResource(Res.string.key_ceremonies)) + } + } + // 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 -- diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/ChatRoomMessagingScreen.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/ChatRoomMessagingScreen.kt index e678a623..e5b842ae 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/ChatRoomMessagingScreen.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/ChatRoomMessagingScreen.kt @@ -49,6 +49,7 @@ import press.mantra.compose.repository.NostrRepository import press.mantra.compose.ui.composable.navigation.routes.ChatRoomDetailRoute import press.mantra.compose.ui.composable.navigation.routes.DkgRitualRoute import press.mantra.compose.ui.composable.navigation.routes.FrostSigningRoute +import press.mantra.compose.ui.composable.navigation.routes.DkgRitualListRoute import press.mantra.compose.ui.composable.navigation.routes.ProposalListRoute import press.mantra.compose.ui.composable.navigation.routes.Route import press.mantra.compose.ui.composable.widgets.LoadingDataIndicator @@ -176,12 +177,37 @@ fun ChatRoomMessagingScreen( ChatTranscript( viewModel = chatMessageListViewModel, onOpenSharedKey = { dkgSessionId -> + // The same two reasons the signing lines open + // the queue instead of one proposal. A line + // written before chat rows named their ceremony + // cannot say which it is about, and the room may + // hold several -- the list is the honest answer + // there, showing all of them with their own + // state rather than guessing at the newest, + // which since subgroups moved into the parent's + // room is routinely the finished one. + // + // The other is that the ceremony they tapped is + // not the only one waiting on them, in which + // case it is not the whole of what is being + // asked and a screen showing only it would say + // it was. onNavigateToRoute.invoke( - DkgRitualRoute( - activeUserPublicKey = activeUserPublicKey, - chatRoomId = chatRoomId, - dkgSessionId = dkgSessionId - ) + if (dkgSessionId == null || + chatMessageListViewModel + .hidesOtherCeremonies(dkgSessionId) + ) { + DkgRitualListRoute( + activeUserPublicKey = activeUserPublicKey, + chatRoomId = chatRoomId + ) + } else { + DkgRitualRoute( + activeUserPublicKey = activeUserPublicKey, + chatRoomId = chatRoomId, + dkgSessionId = dkgSessionId + ) + } ) }, onOpenSigning = { sessionId -> diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/DkgRitualListScreen.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/DkgRitualListScreen.kt new file mode 100644 index 00000000..aa7121b6 --- /dev/null +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/DkgRitualListScreen.kt @@ -0,0 +1,504 @@ +package press.mantra.compose.ui.composable + +import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.Spacer +import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.height +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.width +import androidx.compose.foundation.lazy.LazyColumn +import androidx.compose.foundation.lazy.items +import androidx.compose.material.icons.Icons +import androidx.compose.material.icons.filled.CheckCircle +import androidx.compose.material.icons.filled.ChevronRight +import androidx.compose.material.icons.filled.ErrorOutline +import androidx.compose.material.icons.filled.HourglassEmpty +import androidx.compose.material.icons.filled.Key +import androidx.compose.material3.Card +import androidx.compose.material3.CardDefaults +import androidx.compose.material3.CircularProgressIndicator +import androidx.compose.material3.ExperimentalMaterial3Api +import androidx.compose.material3.FilledTonalButton +import androidx.compose.material3.Icon +import androidx.compose.material3.ListItem +import androidx.compose.material3.ListItemDefaults +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.Scaffold +import androidx.compose.material3.SnackbarHost +import androidx.compose.material3.Surface +import androidx.compose.material3.Text +import androidx.compose.material3.TopAppBar +import androidx.compose.runtime.Composable +import androidx.compose.runtime.collectAsState +import androidx.compose.runtime.LaunchedEffect +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.graphics.Color +import androidx.compose.ui.text.style.TextAlign +import androidx.compose.ui.text.style.TextOverflow +import androidx.lifecycle.viewmodel.compose.viewModel +import com.vitorpamplona.quartz.nip01Core.core.HexKey +import mantra.composeapp.generated.resources.Res +import mantra.composeapp.generated.resources.a_subgroup_named_s +import mantra.composeapp.generated.resources.a_subgroups_key +import mantra.composeapp.generated.resources.abandoned_no_key_was_created +import mantra.composeapp.generated.resources.check_the_combined_result +import mantra.composeapp.generated.resources.everything_else +import mantra.composeapp.generated.resources.join_the_ceremony_by_publishing_your_key +import mantra.composeapp.generated.resources.key_ceremonies +import mantra.composeapp.generated.resources.open +import mantra.composeapp.generated.resources.review +import mantra.composeapp.generated.resources.s_of_s_members_will_sign_with_it +import mantra.composeapp.generated.resources.send_your_contribution_to_the_key +import mantra.composeapp.generated.resources.set_up_a_shared_key +import mantra.composeapp.generated.resources.the_group_has_a_key_from_this_ceremony +import mantra.composeapp.generated.resources.the_group_has_its_key_finish_setting_it_up +import mantra.composeapp.generated.resources.the_groups_shared_key +import mantra.composeapp.generated.resources.this_group_has_held_no_key_ceremony +import mantra.composeapp.generated.resources.waiting_for_everyone_to_commit_their_part +import mantra.composeapp.generated.resources.waiting_for_everyone_to_confirm_the_result +import mantra.composeapp.generated.resources.waiting_for_members_to_join +import mantra.composeapp.generated.resources.waiting_for_you +import org.jetbrains.compose.resources.stringResource +import press.mantra.compose.database.model.ChatRoom +import press.mantra.compose.database.model.DkgSession +import press.mantra.compose.database.model.intermdiate.LocalChatRoom +import press.mantra.compose.database.model.types.DkgApprovalStep +import press.mantra.compose.database.model.types.DkgRitualStage +import press.mantra.compose.extensions.toFormattedTimeAndDateString +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.Route +import press.mantra.compose.ui.composable.widgets.Decorative +import press.mantra.compose.ui.composable.widgets.LocalSnackbarHostState +import press.mantra.compose.ui.composable.widgets.NavigateBackButton +import press.mantra.compose.ui.theme.ConformancePreviews +import press.mantra.compose.ui.theme.MantraTheme +import press.mantra.compose.ui.theme.readableContent +import press.mantra.compose.ui.theme.spacing +import press.mantra.compose.ui.view.model.DkgRitualListViewModel +import press.mantra.compose.ui.view.state.DkgRitualListUIState + +/** + * Every key ceremony a room holds. + * + * The transcript shows a ceremony as it happens, in among everything else that + * was said, and the standing notice above the composer says when one is waiting + * on the reader. Neither can say *which*, once there are two -- and since a + * subgroup's ChillDKG moved into its parent's room there are routinely two: the + * group's own, long finished, and whatever subgroup is being made this week. + * + * So: one row per ceremony, each carrying its own state, the finished ones kept + * rather than dropped. The ones still waiting on the reader come first, because + * those are the only ones with anything to do in them; the rest are the record + * of how this group came to hold the keys it holds, which is the other half of + * what this screen is for. + */ +@OptIn(ExperimentalMaterial3Api::class) +@Composable +fun DkgRitualListScreen( + activeUserPublicKey: HexKey, + chatRoomId: String, + initialDkgRitualListUIState: DkgRitualListUIState = DkgRitualListUIState.Loading, + chatRepository: ChatRepository, + dkgRepository: DkgRepository, + onNavigateBack: () -> Unit, + onNavigateToRoute: (Route) -> Unit, +) { + val dkgRitualListViewModel: DkgRitualListViewModel = viewModel( + factory = DkgRitualListViewModel.factory( + chatRoomId = chatRoomId, + activeUserPublicKey = activeUserPublicKey, + initialDkgRitualListUIState = initialDkgRitualListUIState, + chatRepository = chatRepository, + dkgRepository = dkgRepository + ) + ) + + LaunchedEffect(true) { + if (initialDkgRitualListUIState == DkgRitualListUIState.Loading) { + dkgRitualListViewModel.initiate() + } + } + + Scaffold( + snackbarHost = { SnackbarHost(LocalSnackbarHostState.current) }, + topBar = { + TopAppBar( + title = { + Text( + text = stringResource(Res.string.key_ceremonies), + maxLines = 1, + overflow = TextOverflow.Ellipsis + ) + }, + navigationIcon = { NavigateBackButton(onNavigateBack) } + ) + } + ) { padding -> + when (val state = dkgRitualListViewModel.dkgRitualListUIState.collectAsState().value) { + is DkgRitualListUIState.Loading -> Column( + modifier = Modifier.fillMaxWidth().padding(padding).readableContent() + .padding(MaterialTheme.spacing.space250), + horizontalAlignment = Alignment.CenterHorizontally + ) { + Spacer(modifier = Modifier.height(MaterialTheme.spacing.space600)) + CircularProgressIndicator() + } + + is DkgRitualListUIState.Error -> Column( + modifier = Modifier.fillMaxWidth().padding(padding).readableContent() + .padding(MaterialTheme.spacing.space250), + horizontalAlignment = Alignment.CenterHorizontally + ) { + Spacer(modifier = Modifier.height(MaterialTheme.spacing.space600)) + Text(text = state.message, textAlign = TextAlign.Center) + } + + is DkgRitualListUIState.Loaded -> { + val open: (DkgRitualListUIState.Ceremony) -> Unit = { ceremony -> + onNavigateToRoute.invoke( + DkgRitualRoute( + activeUserPublicKey = activeUserPublicKey, + chatRoomId = chatRoomId, + dkgSessionId = ceremony.session.id + ) + ) + } + + LazyColumn( + modifier = Modifier.padding(padding).readableContent().fillMaxSize() + .padding(MaterialTheme.spacing.space250), + verticalArrangement = Arrangement.spacedBy(MaterialTheme.spacing.space125) + ) { + if (state.ceremonies.isEmpty()) { + item { + Column( + modifier = Modifier.fillMaxWidth() + .padding(top = MaterialTheme.spacing.space600), + horizontalAlignment = Alignment.CenterHorizontally, + verticalArrangement = Arrangement.spacedBy( + MaterialTheme.spacing.sectionGap + ) + ) { + Text( + text = stringResource( + Res.string.this_group_has_held_no_key_ceremony + ), + textAlign = TextAlign.Center + ) + + // The way out of an empty list, rather than a + // dead end that sends a member back to the + // group's details to find the other door. The + // ceremony screen decides whether the group is + // in a position to hold one; this only offers + // to ask it. + FilledTonalButton(onClick = { + onNavigateToRoute.invoke( + DkgRitualRoute( + activeUserPublicKey = activeUserPublicKey, + chatRoomId = chatRoomId + ) + ) + }) { + Icon(Icons.Default.Key, contentDescription = Decorative) + + Spacer( + modifier = Modifier.width( + MaterialTheme.spacing.relatedGap + ) + ) + + Text(stringResource(Res.string.set_up_a_shared_key)) + } + } + } + } + + if (state.waitingForYou.isNotEmpty()) { + item { + Text( + text = stringResource(Res.string.waiting_for_you), + style = MaterialTheme.typography.labelMedium, + color = MaterialTheme.colorScheme.primary + ) + } + + items( + items = state.waitingForYou, + key = { it.session.id } + ) { ceremony -> + CeremonyCard(ceremony = ceremony, onClick = { open(ceremony) }) + } + } + + if (state.rest.isNotEmpty()) { + // Headed only when there is a section above to tell it + // apart from; on its own it is the whole screen, which + // the title already says. + if (state.waitingForYou.isNotEmpty()) { + item { + Text( + text = stringResource(Res.string.everything_else), + style = MaterialTheme.typography.labelMedium + ) + } + } + + items( + items = state.rest, + key = { it.session.id } + ) { ceremony -> + CeremonyCard(ceremony = ceremony, onClick = { open(ceremony) }) + } + } + } + } + } + } +} + +/** + * One ceremony, named after the key it is making. + * + * What a ceremony is *for* is the first thing to say about it, because a room + * holding two holds them for different groups: one is this room's own key and + * the other belongs to a subgroup that does not exist yet. A row that only said + * how far along it was would leave a member guessing which of the two they were + * looking at. + */ +@Composable +private fun CeremonyCard( + ceremony: DkgRitualListUIState.Ceremony, + onClick: () -> Unit, +) { + val session = ceremony.session + + // Every colour inside this card is derived from the card rather than reached + // for independently, because ListItem does not inherit LocalContentColor -- + // its headline comes from `onSurface` whatever the container is. See + // ProposalListScreen, where that combination drew #1B1B1B on #1B1B1B. + val cardColors = if (ceremony.awaitsYou) { + CardDefaults.cardColors( + containerColor = MaterialTheme.colorScheme.primaryContainer + ) + } else { + CardDefaults.cardColors() + } + val cardContentColor = cardColors.contentColor + + // A subgroup is named by its proposal, and a member who did not open it may + // be looking at the ceremony before that name has arrived -- hence the third + // branch, which still says which of the two kinds of key this is. + val subject = session.subject + + val label = when { + !ceremony.isSubgroup -> stringResource(Res.string.the_groups_shared_key) + subject != null -> stringResource(Res.string.a_subgroup_named_s, subject) + else -> stringResource(Res.string.a_subgroups_key) + } + + Card(onClick = onClick, colors = cardColors) { + ListItem( + colors = ListItemDefaults.colors( + containerColor = Color.Transparent, + headlineColor = cardContentColor, + overlineColor = cardContentColor, + supportingColor = cardContentColor, + leadingIconColor = cardContentColor, + trailingIconColor = cardContentColor, + ), + leadingContent = { + Icon( + imageVector = when { + ceremony.awaitsYou -> Icons.Default.Key + session.stage == DkgRitualStage.COMPLETE -> Icons.Default.CheckCircle + session.stage == DkgRitualStage.FAILED -> Icons.Default.ErrorOutline + else -> Icons.Default.HourglassEmpty + }, + // Carries the stage on its own, and carries it alone on the + // highlighted card where the failure state has no colour to + // spare. + contentDescription = when { + ceremony.awaitsYou -> "Waiting on you" + session.stage == DkgRitualStage.COMPLETE -> "Finished" + session.stage == DkgRitualStage.FAILED -> "Abandoned" + else -> "Waiting on others" + }, + tint = when { + // `error` is 2.67:1 on the highlighted card, so there the + // failure is carried by the icon's shape and by the status + // line rather than by colour. + session.stage == DkgRitualStage.FAILED && + !ceremony.awaitsYou -> MaterialTheme.colorScheme.error + else -> cardContentColor + } + ) + }, + trailingContent = { + Row(verticalAlignment = Alignment.CenterVertically) { + // The same word the transcript's notice uses for the same + // thing, so a member who came here looking for the card they + // tapped finds it saying what it said there. + Text( + text = if (ceremony.awaitsYou) { + stringResource(Res.string.review) + } else { + stringResource(Res.string.open) + }, + style = MaterialTheme.typography.labelLarge, + color = cardContentColor + ) + + Icon( + Icons.Default.ChevronRight, + contentDescription = "Open this ceremony" + ) + } + }, + overlineContent = { Text(text = label) }, + headlineContent = { + Text( + text = statusOf(ceremony), + maxLines = 2, + overflow = TextOverflow.Ellipsis + ) + }, + supportingContent = { + Column( + verticalArrangement = Arrangement.spacedBy(MaterialTheme.spacing.space25) + ) { + // The quorum a ceremony settles is fixed by it and cannot be + // changed afterwards, so it belongs on the row rather than + // only on the screen behind it. + Text( + text = stringResource( + Res.string.s_of_s_members_will_sign_with_it, + session.threshold, + session.participantCount + ) + ) + + Text( + text = "${ceremony.coordinatorName} · " + + session.createdAt.toFormattedTimeAndDateString(), + style = MaterialTheme.typography.labelSmall, + color = MaterialTheme.colorScheme.onSurfaceVariant + ) + } + } + ) + } +} + +/** + * Where a ceremony has got to, in a line. + * + * Shorter than the ceremony screen's ladder, and it has to be: this is one row + * of a list somebody is scanning, not the page they are acting on. What it must + * never do is call a ceremony finished when it is not, or waiting when there is + * nothing left to wait for. + * + * What it owes this member outranks the stage, in the words the approval screens + * use for the same step, so nobody is asked twice in two vocabularies. + */ +@Composable +private fun statusOf(ceremony: DkgRitualListUIState.Ceremony): String { + ceremony.pendingApproval?.let { + return when (it) { + DkgApprovalStep.HOST_KEY -> + stringResource(Res.string.join_the_ceremony_by_publishing_your_key) + DkgApprovalStep.ROUND_1 -> + stringResource(Res.string.send_your_contribution_to_the_key) + DkgApprovalStep.ROUND_2 -> + stringResource(Res.string.check_the_combined_result) + } + } + + if (ceremony.roomAwaitingCreation != null) { + return stringResource(Res.string.the_group_has_its_key_finish_setting_it_up) + } + + return when (ceremony.session.stage) { + DkgRitualStage.COLLECTING_HOST_KEYS -> + stringResource(Res.string.waiting_for_members_to_join) + + DkgRitualStage.COLLECTING_ROUND_1 -> + stringResource(Res.string.waiting_for_everyone_to_commit_their_part) + + DkgRitualStage.COLLECTING_ROUND_2 -> + stringResource(Res.string.waiting_for_everyone_to_confirm_the_result) + + DkgRitualStage.COMPLETE -> + stringResource(Res.string.the_group_has_a_key_from_this_ceremony) + + // The reason is the whole content of a failure -- "a member refused" and + // "the group could not agree" are different things to have happened -- so + // it stands in for the bare word whenever there is one. + DkgRitualStage.FAILED -> + ceremony.session.failureReason + ?: stringResource(Res.string.abandoned_no_key_was_created) + } +} + +@ConformancePreviews +@Composable +private fun DkgRitualListScreenPreview() { + val session = DkgSession( + id = "sessionId", + chatRoomId = "chatRoomId", + coordinatorPublicKey = "c".repeat(64), + userPublicKey = "u".repeat(64), + threshold = 2, + participantCount = 3, + hostPublicKey = "h".repeat(64), + round1Random = "1".repeat(64), + round2AuxRandom = "2".repeat(64), + ) + + MantraTheme { + Surface(modifier = Modifier.fillMaxSize()) { + DkgRitualListScreen( + activeUserPublicKey = "u".repeat(64), + chatRoomId = "chatRoomId", + initialDkgRitualListUIState = DkgRitualListUIState.Loaded( + localChatRoom = LocalChatRoom( + chatRoom = ChatRoom( + id = "chatRoomId", + userPublicKey = "u".repeat(64), + subject = "#Ekklesia", + description = null, + initialGiftWrapPayloadId = "sdfaer", + mlsGroupState = null + ) + ), + ceremonies = listOf( + DkgRitualListUIState.Ceremony( + session = session.copy( + id = "subgroupSessionId", + parentChatRoomId = "parentChatRoomId", + subject = "Translators", + stage = DkgRitualStage.COLLECTING_ROUND_1 + ), + coordinatorName = "Ada", + pendingApproval = DkgApprovalStep.ROUND_1 + ), + DkgRitualListUIState.Ceremony( + session = session.copy(stage = DkgRitualStage.COMPLETE), + coordinatorName = "You" + ) + ) + ), + chatRepository = ChatRepository.NO_OP_CHAT_REPOSITORY, + dkgRepository = DkgRepository.NO_OP_DKG_REPOSITORY, + onNavigateBack = {}, + onNavigateToRoute = {} + ) + } + } +} 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 ae13c349..a5d67444 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 @@ -38,6 +38,7 @@ import press.mantra.compose.ui.composable.ChatRoomDetailScreen import press.mantra.compose.ui.composable.ChatRoomMessagingScreen import press.mantra.compose.ui.composable.CreateProfileScreen import press.mantra.compose.ui.composable.DkgJoinApprovalScreen +import press.mantra.compose.ui.composable.DkgRitualListScreen import press.mantra.compose.ui.composable.DkgRitualScreen import press.mantra.compose.ui.composable.DkgRound1ApprovalScreen import press.mantra.compose.ui.composable.DkgRound2ApprovalScreen @@ -140,6 +141,7 @@ import press.mantra.compose.ui.composable.navigation.routes.AddArtifactRoute import press.mantra.compose.ui.composable.navigation.routes.AddDialectRoute import press.mantra.compose.ui.composable.navigation.routes.BroadcastGroupSignedEventRoute import press.mantra.compose.ui.composable.navigation.routes.FrostSigningRoute +import press.mantra.compose.ui.composable.navigation.routes.DkgRitualListRoute import press.mantra.compose.ui.composable.navigation.routes.ProposalListRoute import press.mantra.compose.ui.composable.navigation.routes.AddChapterRoute import press.mantra.compose.ui.composable.navigation.routes.AddTranslationRoute @@ -1198,6 +1200,22 @@ fun MantraNavHost( } ) } + composable { backStackEntry -> + val route = backStackEntry.toRoute() + + DkgRitualListScreen( + activeUserPublicKey = route.activeUserPublicKey, + chatRoomId = route.chatRoomId, + chatRepository = databaseChatRepository, + dkgRepository = databaseDkgRepository, + onNavigateBack = { + navController.popBackStack() + }, + onNavigateToRoute = { ceremonyRoute -> + navController.navigate(route = ceremonyRoute) + } + ) + } composable { backStackEntry -> val route = backStackEntry.toRoute() diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/navigation/routes/DkgRitualListRoute.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/navigation/routes/DkgRitualListRoute.kt new file mode 100644 index 00000000..e23efb54 --- /dev/null +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/navigation/routes/DkgRitualListRoute.kt @@ -0,0 +1,23 @@ +package press.mantra.compose.ui.composable.navigation.routes + +import kotlinx.serialization.Serializable + +/** + * Every key ceremony a room holds. + * + * The room's list, not the reader's: a ceremony is run by the whole group, and + * which of them are waiting on this member is a status on each row rather than + * a different list -- the same shape as [ProposalListRoute], for the same + * reason. + * + * A room holds more than one as a matter of course since a subgroup's ChillDKG + * moved into its parent's room, and until this there was no screen that could + * name the second: [DkgRitualRoute] without a session id shows the newest, and + * the newest is routinely the group's own, long finished, with the subgroup's + * request buried underneath it. + */ +@Serializable +data class DkgRitualListRoute( + val activeUserPublicKey: String, + val chatRoomId: String +): Route() diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/widgets/chat/ChatTranscript.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/widgets/chat/ChatTranscript.kt index bfefd1b6..a69c8b8d 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/widgets/chat/ChatTranscript.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/composable/widgets/chat/ChatTranscript.kt @@ -578,7 +578,8 @@ fun ChatTranscript( @Composable private fun CeremoniesAwaitingYouNotice( ceremonies: List, - onClick: (dkgSessionId: String) -> Unit, + /** Null where the notice stands for more than one, which no single screen can show. */ + onClick: (dkgSessionId: String?) -> Unit, ) { val single = ceremonies.singleOrNull() @@ -602,7 +603,11 @@ private fun CeremoniesAwaitingYouNotice( } Card( - onClick = { onClick(ceremonies.first().sessionId) }, + // The one it names when it names one, and the list otherwise. A notice + // reading "3 key ceremonies need you" that opened the first of them -- + // which is what this did -- answered a different question from the one + // it asked, and left the other two exactly as hard to find as before. + onClick = { onClick(single?.sessionId) }, modifier = Modifier.fillMaxWidth().padding(horizontal = MaterialTheme.spacing.space50), colors = CardDefaults.cardColors( containerColor = MaterialTheme.colorScheme.primaryContainer, 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 7ef1bdb0..16eb4c5c 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 @@ -156,6 +156,25 @@ class ChatMessageListViewModel( proposalsAwaitingYou.size > 1 && proposalsAwaitingYou.any { it.sessionId == sessionId } + /** + * Whether opening [sessionId] by itself would hide other ceremonies the room + * is waiting on this member for. + * + * [hidesOtherDecisions]'s counterpart, and the same argument: a ceremony line + * names one ceremony, and opening it is right while that is the only one + * asking. A room holds more than one as a matter of course since a subgroup's + * ChillDKG moved into its parent's room, and the second is not in the + * transcript beside the first -- so answering the one they tapped and leaving + * would look exactly like being done. + * + * Only for a ceremony that is itself waiting on them. A line about a ceremony + * the group finished is history, and a reader who taps one is asking to see + * that, not to be handed the queue. + */ + fun hidesOtherCeremonies(sessionId: String): Boolean = + ceremoniesAwaitingYou.size > 1 && + ceremoniesAwaitingYou.any { it.sessionId == sessionId } + fun startDirectMessage(participant: Participant) { logger.d("Arming a direct message to ${participant.participantPublicKey}") directMessageRecipient = participant diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/DkgRitualListViewModel.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/DkgRitualListViewModel.kt new file mode 100644 index 00000000..38677d1c --- /dev/null +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/DkgRitualListViewModel.kt @@ -0,0 +1,140 @@ +package press.mantra.compose.ui.view.model + +import androidx.lifecycle.ViewModel +import androidx.lifecycle.ViewModelProvider +import androidx.lifecycle.viewModelScope +import androidx.lifecycle.viewmodel.initializer +import androidx.lifecycle.viewmodel.viewModelFactory +import co.touchlab.kermit.Logger +import com.vitorpamplona.quartz.nip01Core.core.HexKey +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.IO +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.launch +import press.mantra.compose.database.model.DkgSession +import press.mantra.compose.database.model.intermdiate.LocalChatRoom +import press.mantra.compose.database.model.types.DkgRitualStage +import press.mantra.compose.extensions.memberName +import press.mantra.compose.repository.ChatRepository +import press.mantra.compose.repository.DkgRepository +import press.mantra.compose.ui.view.state.DkgRitualListUIState + +/** + * Every key ceremony a room holds. + * + * A room routinely holds more than one: a parent's Marmot room hosts the + * ceremony of every subgroup the group ever makes, alongside whatever it holds + * of its own. Watching the whole list rather than the newest is the point -- + * which of them is waiting on the reader is a fact about each one, and the + * newest is routinely the finished one. + * + * The counterpart to [ProposalListViewModel], and deliberately the same shape. + */ +class DkgRitualListViewModel( + val chatRoomId: String, + val activeUserPublicKey: HexKey, + initialDkgRitualListUIState: DkgRitualListUIState, + val chatRepository: ChatRepository, + val dkgRepository: DkgRepository, +): ViewModel() { + + private val _dkgRitualListUIState = MutableStateFlow(initialDkgRitualListUIState) + + /** + * A flow rather than a `mutableStateOf`: this is written from + * `Dispatchers.IO`, and a `mutableStateOf` created in a view model's + * constructor silently drops a write that lands during the composition that + * created it. See [DkgRitualViewModel.dkgRitualUIState]. + */ + val dkgRitualListUIState: StateFlow = + _dkgRitualListUIState.asStateFlow() + + private val logger = Logger.withTag(TAG) + + /** + * Loads the room, then watches its ceremonies for as long as the screen + * lives. + * + * Every one of them moves on messages arriving from other members, so a + * screen that read the list once would go stale the moment somebody else + * took their step -- which is exactly when a member is likely to be looking + * at it. + */ + fun initiate() { + viewModelScope.launch(Dispatchers.IO) { + val localChatRoom = chatRepository.getChatRoomByIdentifier(chatRoomId) + if (localChatRoom == null) { + _dkgRitualListUIState.value = + DkgRitualListUIState.Error("Couldn't find the group") + return@launch + } + + dkgRepository.observeSessionsForChatRoom(chatRoomId).collect { sessions -> + logger.d("${sessions.size} key ceremonies in $chatRoomId") + + _dkgRitualListUIState.value = DkgRitualListUIState.Loaded( + localChatRoom = localChatRoom, + ceremonies = sessions.map { asCeremony(it, localChatRoom) } + ) + } + } + } + + /** + * One ceremony as a row. + * + * What it wants is asked of the repository rather than worked out here, so + * this list, the transcript's standing notice and the ceremony screen cannot + * disagree about which ceremonies still have something in them -- the same + * rule the proposal list keeps. + * + * An abandoned ceremony is asked nothing. It can want nothing: there is no + * step left to approve and no key to build a room from, and a row claiming + * otherwise would send a member to a screen with nothing on it. + */ + private suspend fun asCeremony( + session: DkgSession, + localChatRoom: LocalChatRoom + ): DkgRitualListUIState.Ceremony { + val live = session.takeIf { it.stage != DkgRitualStage.FAILED } + + return DkgRitualListUIState.Ceremony( + session = session, + coordinatorName = session.coordinatorPublicKey.memberName( + members = localChatRoom.localParticipants, + activeUserPublicKey = activeUserPublicKey + ), + pendingApproval = live?.let { dkgRepository.pendingApproval(it) }, + // Only of the member who opened it: creating the room is the + // coordinator's job, and a row telling everybody else to do it would + // be telling them to do something they cannot. + roomAwaitingCreation = live + ?.takeIf { it.isCoordinator() } + ?.let { dkgRepository.roomAwaitingCreation(it) } + ) + } + + companion object { + private const val TAG = "DkgRitualListViewModel" + + fun factory( + chatRoomId: String, + activeUserPublicKey: HexKey, + initialDkgRitualListUIState: DkgRitualListUIState = DkgRitualListUIState.Loading, + chatRepository: ChatRepository, + dkgRepository: DkgRepository, + ): ViewModelProvider.Factory = viewModelFactory { + initializer { + DkgRitualListViewModel( + chatRoomId = chatRoomId, + activeUserPublicKey = activeUserPublicKey, + initialDkgRitualListUIState = initialDkgRitualListUIState, + chatRepository = chatRepository, + dkgRepository = dkgRepository + ) + } + } + } +} diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/state/DkgRitualListUIState.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/state/DkgRitualListUIState.kt new file mode 100644 index 00000000..0e9e533e --- /dev/null +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/state/DkgRitualListUIState.kt @@ -0,0 +1,68 @@ +package press.mantra.compose.ui.view.state + +import press.mantra.compose.database.model.DkgSession +import press.mantra.compose.database.model.intermdiate.LocalChatRoom +import press.mantra.compose.database.model.types.DkgApprovalStep + +sealed interface DkgRitualListUIState { + data class Loaded( + val localChatRoom: LocalChatRoom, + + /** Every ceremony the room holds, newest first. */ + val ceremonies: List = emptyList(), + ): DkgRitualListUIState { + /** + * The ones that cannot move until this member acts. + * + * Split out rather than sorted to the top, the way the proposal list + * splits its own -- the rest of the list is what the room has done, + * these are what it is waiting on the reader for. A room can have two + * at once the moment a subgroup's admins are the whole group, and + * burying the second under the first one's history is the whole reason + * this screen exists. + */ + val waitingForYou: List = ceremonies.filter { it.awaitsYou } + + val rest: List = ceremonies.filterNot { it.awaitsYou } + } + + /** + * One ceremony, as a row. + * + * Both reasons a ceremony can want something are carried separately rather + * than folded into one flag, because they are owed by different people and + * read differently: an approval is this member's gate and nobody else can + * pass it, while finishing the setup is work the coordinator alone can come + * back to. The row says which. + */ + data class Ceremony( + val session: DkgSession, + + /** Whoever opened it, as a name to read. */ + val coordinatorName: String, + + /** What the ceremony is waiting on this member for, if anything. */ + val pendingApproval: DkgApprovalStep? = null, + + /** + * The room this finished ceremony's key derives, when it does not exist + * yet and this member is the one who would make it. + * + * Reaching COMPLETE finishes nothing on its own: the group has a key and + * somebody still has to get its state signed and create the room. The + * same reading the transcript's standing notice makes. + */ + val roomAwaitingCreation: String? = null, + ) { + /** Whether this ceremony is making a subgroup rather than the room's own key. */ + val isSubgroup: Boolean get() = session.parentChatRoomId != null + + val awaitsYou: Boolean get() = pendingApproval != null || roomAwaitingCreation != null + } + + data class Error( + val message: String + ): DkgRitualListUIState + + data object Loading: DkgRitualListUIState +} diff --git a/composeApp/src/jvmTest/kotlin/press/mantra/compose/ui/composable/DkgRitualListScreenJvmTest.kt b/composeApp/src/jvmTest/kotlin/press/mantra/compose/ui/composable/DkgRitualListScreenJvmTest.kt new file mode 100644 index 00000000..7d49853d --- /dev/null +++ b/composeApp/src/jvmTest/kotlin/press/mantra/compose/ui/composable/DkgRitualListScreenJvmTest.kt @@ -0,0 +1,157 @@ +package press.mantra.compose.ui.composable + +import androidx.compose.runtime.Composable +import androidx.compose.ui.test.ExperimentalTestApi +import androidx.compose.ui.test.assertIsDisplayed +import androidx.compose.ui.test.onNodeWithText +import androidx.compose.ui.test.performClick +import androidx.compose.ui.test.runDesktopComposeUiTest +import press.mantra.compose.database.model.ChatRoom +import press.mantra.compose.database.model.DkgSession +import press.mantra.compose.database.model.intermdiate.LocalChatRoom +import press.mantra.compose.database.model.types.DkgApprovalStep +import press.mantra.compose.database.model.types.DkgRitualStage +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.Route +import press.mantra.compose.ui.composable.widgets.ProvideSnackbarHost +import press.mantra.compose.ui.theme.MantraTheme +import press.mantra.compose.ui.view.state.DkgRitualListUIState +import kotlin.test.Test +import kotlin.test.assertEquals + +/** + * What the ceremony list is for, read off the rendered screen. + * + * The ordering is settled in [press.mantra.compose.ui.view.model.DkgRitualListViewModel] + * and tested there. What is only true once it is drawn is that a row goes to + * the ceremony it names -- the whole reason this screen exists is that the + * screens before it could not say which of a room's ceremonies they meant -- + * and that a room with none offers the way to hold one rather than ending in a + * blank page. + */ +@OptIn(ExperimentalTestApi::class) +class DkgRitualListScreenJvmTest { + private val me = "11".repeat(32) + private val chatRoomId = "22".repeat(32) + + @Test + fun `a row opens the ceremony it names, not the room's newest`() = + runDesktopComposeUiTest(500, 1200) { + var opened: Route? = null + + setContent { + Screen( + state = loaded( + ceremony("subgroup", DkgRitualStage.COLLECTING_HOST_KEYS).copy( + pendingApproval = DkgApprovalStep.HOST_KEY + ), + ceremony("newest", DkgRitualStage.COMPLETE) + ), + onNavigateToRoute = { opened = it } + ) + } + + onNodeWithText("Join it by publishing your device's key") + .assertIsDisplayed() + .performClick() + + assertEquals( + DkgRitualRoute( + activeUserPublicKey = me, + chatRoomId = chatRoomId, + dkgSessionId = "subgroup" + ), + opened + ) + } + + /** The two sections, so a ceremony owing nothing cannot be mistaken for one that does. */ + @Test + fun `the ones waiting on you are headed apart from the rest`() = + runDesktopComposeUiTest(500, 1200) { + setContent { + Screen( + state = loaded( + ceremony("subgroup", DkgRitualStage.COLLECTING_HOST_KEYS).copy( + pendingApproval = DkgApprovalStep.HOST_KEY + ), + ceremony("newest", DkgRitualStage.COMPLETE) + ) + ) + } + + onNodeWithText("Waiting for you").assertIsDisplayed() + onNodeWithText("Everything else").assertIsDisplayed() + onNodeWithText("The group has a key from this ceremony.").assertIsDisplayed() + } + + @Test + fun `a room that has held none offers to hold one`() = + runDesktopComposeUiTest(500, 1200) { + var opened: Route? = null + + setContent { Screen(state = loaded(), onNavigateToRoute = { opened = it }) } + + onNodeWithText("This group has not held a key ceremony.").assertIsDisplayed() + onNodeWithText("Set up a shared key").assertIsDisplayed().performClick() + + // No session id: there is no ceremony to name, and the ceremony + // screen is what decides whether the group can hold one at all. + assertEquals( + DkgRitualRoute(activeUserPublicKey = me, chatRoomId = chatRoomId), + opened + ) + } + + @Composable + private fun Screen( + state: DkgRitualListUIState, + onNavigateToRoute: (Route) -> Unit = {}, + ) { + MantraTheme { + ProvideSnackbarHost { + DkgRitualListScreen( + activeUserPublicKey = me, + chatRoomId = chatRoomId, + initialDkgRitualListUIState = state, + chatRepository = ChatRepository.NO_OP_CHAT_REPOSITORY, + dkgRepository = DkgRepository.NO_OP_DKG_REPOSITORY, + onNavigateBack = {}, + onNavigateToRoute = onNavigateToRoute, + ) + } + } + } + + private fun loaded(vararg ceremonies: DkgRitualListUIState.Ceremony) = + DkgRitualListUIState.Loaded( + localChatRoom = LocalChatRoom( + chatRoom = ChatRoom( + id = chatRoomId, + userPublicKey = me, + subject = "Ekklesia", + description = null, + mlsGroupState = null, + ) + ), + ceremonies = ceremonies.toList() + ) + + private fun ceremony(id: String, stage: DkgRitualStage) = DkgRitualListUIState.Ceremony( + session = DkgSession( + id = id, + chatRoomId = chatRoomId, + coordinatorPublicKey = me, + userPublicKey = me, + threshold = 2, + participantCount = 2, + hostPublicKey = me, + round1Random = "66".repeat(32), + round2AuxRandom = "77".repeat(32), + stage = stage, + ), + coordinatorName = "You", + ) +} diff --git a/composeApp/src/jvmTest/kotlin/press/mantra/compose/ui/view/model/DkgRitualListViewModelJvmTest.kt b/composeApp/src/jvmTest/kotlin/press/mantra/compose/ui/view/model/DkgRitualListViewModelJvmTest.kt new file mode 100644 index 00000000..80798701 --- /dev/null +++ b/composeApp/src/jvmTest/kotlin/press/mantra/compose/ui/view/model/DkgRitualListViewModelJvmTest.kt @@ -0,0 +1,198 @@ +package press.mantra.compose.ui.view.model + +import com.vitorpamplona.quartz.nip01Core.core.HexKey +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.resetMain +import kotlinx.coroutines.test.setMain +import kotlinx.coroutines.withTimeout +import press.mantra.compose.database.model.ChatRoom +import press.mantra.compose.database.model.DkgSession +import press.mantra.compose.database.model.Participant +import press.mantra.compose.database.model.Profile +import press.mantra.compose.database.model.intermdiate.LocalChatRoom +import press.mantra.compose.database.model.intermdiate.LocalParticipant +import press.mantra.compose.database.model.types.DkgApprovalStep +import press.mantra.compose.database.model.types.DkgRitualStage +import press.mantra.compose.repository.ChatRepository +import press.mantra.compose.repository.DkgRepository +import press.mantra.compose.ui.view.state.DkgRitualListUIState +import kotlin.test.AfterTest +import kotlin.test.BeforeTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNull +import kotlin.test.assertTrue + +/** + * What the key-ceremony list decides that nothing else can check. + * + * **Which ceremonies come first.** A room holds one per subgroup the group has + * ever made plus its own, and the newest is routinely the finished one -- so + * the list is only worth having if the two that want something from the reader + * are at the top of it rather than wherever their dates put them. + * + * **What counts as wanting something.** Two different things do, owed by + * different people: an approval gate this member cannot delegate, and a + * finished ceremony whose room only its coordinator can create. An abandoned + * ceremony wants neither, whatever the rows underneath it say. + */ +@OptIn(ExperimentalCoroutinesApi::class) +class DkgRitualListViewModelJvmTest { + private val chatRoomId = "a1b2c3d4".repeat(8) + private val me = "11".repeat(32) + private val ada = "22".repeat(32) + + @BeforeTest + fun setUp() { + Dispatchers.setMain(UnconfinedTestDispatcher()) + } + + @AfterTest + fun tearDown() { + Dispatchers.resetMain() + } + + @Test + fun `the ceremonies waiting on this member are separated from the rest`() = runBlocking { + val mine = session("mine", DkgRitualStage.COLLECTING_ROUND_1) + val finished = session("finished", DkgRitualStage.COMPLETE) + val subgroup = session("subgroup", DkgRitualStage.COLLECTING_HOST_KEYS) + .copy(parentChatRoomId = "parent", subject = "Translators") + + val loaded = load( + // Newest first, the way the query returns them -- and the one that + // wants something is deliberately not the newest. + sessions = listOf(finished, subgroup, mine), + pending = mapOf( + "mine" to DkgApprovalStep.ROUND_1, + "subgroup" to DkgApprovalStep.HOST_KEY + ) + ) + + assertEquals( + listOf("subgroup", "mine"), + loaded.waitingForYou.map { it.session.id }, + "a ceremony waiting on this member was left down the list" + ) + assertEquals(listOf("finished"), loaded.rest.map { it.session.id }) + assertTrue(loaded.waitingForYou.single { it.session.id == "subgroup" }.isSubgroup) + } + + /** + * A finished ceremony still owes its coordinator a room, and owes everybody + * else nothing -- a row telling the rest of the group to create it would be + * telling them to do something they cannot. + */ + @Test + fun `only the coordinator is told to finish setting up a finished ceremony`() = runBlocking { + val theirs = session("theirs", DkgRitualStage.COMPLETE).copy(coordinatorPublicKey = ada) + val ours = session("ours", DkgRitualStage.COMPLETE) + + val loaded = load( + sessions = listOf(theirs, ours), + roomAwaitingCreation = "theRoomItDerives" + ) + + assertEquals(listOf("ours"), loaded.waitingForYou.map { it.session.id }) + assertEquals("Ada", loaded.rest.single().coordinatorName) + assertEquals("You", loaded.waitingForYou.single().coordinatorName) + } + + /** + * An abandoned ceremony is asked nothing at all. There is no step left to + * approve and no key to build a room from, so a row that claimed otherwise + * would send a member to a screen with nothing on it. + */ + @Test + fun `an abandoned ceremony is never waiting on anybody`() = runBlocking { + val loaded = load( + sessions = listOf(session("abandoned", DkgRitualStage.FAILED)), + // Both answers say yes, and both have to be ignored on a failure. + pending = mapOf("abandoned" to DkgApprovalStep.ROUND_1), + roomAwaitingCreation = "theRoomItDerives" + ) + + assertTrue(loaded.waitingForYou.isEmpty()) + + val row = loaded.rest.single() + assertNull(row.pendingApproval) + assertNull(row.roomAwaitingCreation) + } + + private suspend fun load( + sessions: List, + pending: Map = emptyMap(), + roomAwaitingCreation: String? = null, + ): DkgRitualListUIState.Loaded { + val viewModel = DkgRitualListViewModel( + chatRoomId = chatRoomId, + activeUserPublicKey = me, + initialDkgRitualListUIState = DkgRitualListUIState.Loading, + chatRepository = object : ChatRepository by ChatRepository.NO_OP_CHAT_REPOSITORY { + override suspend fun getChatRoomByIdentifier(id: HexKey): LocalChatRoom? = room() + }, + dkgRepository = object : DkgRepository by DkgRepository.NO_OP_DKG_REPOSITORY { + override fun observeSessionsForChatRoom( + chatRoomId: String + ): Flow> = flowOf(sessions) + + override suspend fun pendingApproval(session: DkgSession): DkgApprovalStep? = + pending[session.id] + + override suspend fun roomAwaitingCreation(session: DkgSession): String? = + roomAwaitingCreation + } + ) + + viewModel.initiate() + + return withTimeout(5_000) { + viewModel.dkgRitualListUIState.first { it is DkgRitualListUIState.Loaded } + } as DkgRitualListUIState.Loaded + } + + private fun room() = LocalChatRoom( + chatRoom = ChatRoom( + id = chatRoomId, + userPublicKey = me, + subject = "Ekklesia", + description = null, + mlsGroupState = null, + ), + localParticipants = listOf(me to null, ada to "Ada").map { (publicKey, name) -> + LocalParticipant( + participant = Participant( + participantPublicKey = publicKey, + chatRoomId = chatRoomId, + relayHint = null, + ), + profile = name?.let { + Profile( + publicKey = publicKey, + displayName = it, + nostrEventId = "ee".repeat(32), + ) + }, + ) + }, + ) + + private fun session(id: String, stage: DkgRitualStage) = DkgSession( + id = id, + chatRoomId = chatRoomId, + coordinatorPublicKey = me, + userPublicKey = me, + threshold = 2, + participantCount = 2, + hostPublicKey = me, + round1Random = "66".repeat(32), + round2AuxRandom = "77".repeat(32), + stage = stage, + ) +} From 26d305bc143a94d41ae8957ad99659e7c294d8a5 Mon Sep 17 00:00:00 2001 From: Kgothatso Ngako Date: Fri, 2 Oct 2026 20:12:43 +0200 Subject: [PATCH 3/4] fix(subgroups): a parent's second subgroup can be certified, and sign its own key state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both rungs after a subgroup's ceremony run in the **parent's** room. The birth certificate is the parent's signature, so it is signed where the parent's key is; the key state is signed over the parent's room because the room it is *about* does not exist yet -- that is the whole point of signing it first. So a parent making its second subgroup has two certificate signings and two key-state signings in one room, and the ceremony screen read them out by event kind alone. `observeSessionsForChatRoom` is `ORDER BY createdAt DESC`, so `firstOrNull` on a kind match is "the newest of that kind in this room" -- which is some other subgroup's. The screen for one child followed another child's progress. **The display was not the damage.** Both rungs disable their button while a signing of that kind is running and un-failed: ```kotlin enabled = … && (certificateSession == null || certificateFailed) ``` and a *completed* signing is un-failed forever. So once a parent had certified one subgroup, it could never be asked to certify another: the rung sat there disabled with no way out, and `proposeBirthCertificate` returned early on the same reading. The key state blocked the same way, and `createAdminGroup` is gated behind it, so the second subgroup could not be made at all. **It did not need two subgroups.** Adoption signs a key state in the child's own room (`proposeAcceptance`, `localChatRoom = childRoom`), so a group that has ever been adopted already holds a `GroupKeyStateEvent` session in its room -- enough to block the first subgroup it tries to make. What makes this a half-finished job rather than an oversight is that the *signed* facts were already right. `isCertified` goes through `observeBirthCertificate`, scoped by session id, and `isKeyStateSigned` through `observeSignedGroupKeyState`, whose own comment records this exact lesson being learned once already: "A ceremony room holds more than one ceremony the moment a subgroup's admins are the whole group." The in-flight lookups never got the same treatment, and the comment above the key-state watcher still asserted the premise that had stopped being true -- "the room a ceremony runs in signs nothing else". **The key state is matched on the ceremony the proposal names.** `GroupKeyStateEvent` carries `FrostKeyTag` from the moment the proposal is assembled, so `parseDkgSessionId(tags) == dkgSessionId` discriminates before the ceremony has a key and before anybody signs. The watcher moves out of `observeKeyState` -- deleted, with its `Job` -- and into `observeForCeremony`, which is already cancelled and re-pointed per ceremony and is where every other per-ceremony watcher lives. `observeForCeremony`'s null branch now clears `keyStateSession` too, which it could not before because the field was not its to clear. **The certificate is matched on the child it names, through a new `addresses`.** `certifies` could not be reused: it ends on `GroupKeyStateEvent.isSignedByRoom`, a real signature check by the parent's quorum, and the thing being matched here is a proposal that quorum has not signed yet -- that is what the rung is waiting for. `SubgroupBirthCertificateEvent.addresses` is the structural half (kind, content, the `d` tag, the parent tag), `certifies` now calls it so the two cannot drift, and its doc says outright that nothing may decide a lineage on it. The three values it reads come out of `assembleTags` together, so they agree by construction rather than by agreement. Correctness is not at stake in either: these two fields only decide which progress bar is drawn and which button is live. Whether the parent actually agreed is `isCertified`, which does check the signature. **`DkgRepository.roomDerivedBy(dkgSessionId)`** is the session-id-to-room derivation that `observeBirthCertificate` and `observeSignedGroupKeyState` each carried inline. Three callers now, one of them new, and the no-op repository answers null -- which is also the honest answer for a ceremony that has not finished, and the reason all three resolve it per emission rather than once. **Tests: `DkgRitualSecondSubgroupJvmTest`, 3 cases, each checked against the bug reinstated rather than only against the fix.** Two assert that the screen follows its own ceremony's signing when a sibling's is newer. The third is the permanent block, and it is driven in two emissions on a hot flow for a reason worth recording: written as a single emission it asserted "still null" and **passed against the bug**, because at that moment the watcher had not started. Nothing is proved by a field that is null before anybody has looked at it. **One test from the commit before this had to be rewritten, and the change is this commit's fault.** `DkgRitualStartOfferJvmTest`'s lost-update case forced its interleaving by parking the session watcher inside `pendingApproval` while the key-state watcher wrote. Those two are no longer concurrent -- the key-state watcher now starts *from* the session collector, after the park -- so the setup could not happen. The claim was untouched by that, so it is now made against the session watcher versus the room watcher, which are genuinely concurrent: both collect before either writes, neither waits on the other, and the field they contend over is `roomHasCeremony`, which is the one that decides whether the offer to start appears. Re-confirmed failing against the lost update reinstated. 973 jvm tests, 454 common tests, `m3Audit` meeting every budget. Co-Authored-By: Claude Opus 5 --- .../repository/DatabaseDkgRepository.kt | 44 ++- .../subgroup/SubgroupBirthCertificateEvent.kt | 42 ++- .../compose/repository/DkgRepository.kt | 15 + .../ui/view/model/DkgRitualViewModel.kt | 126 ++++--- .../model/DkgRitualSecondSubgroupJvmTest.kt | 322 ++++++++++++++++++ .../view/model/DkgRitualStartOfferJvmTest.kt | 79 +++-- 6 files changed, 511 insertions(+), 117 deletions(-) create mode 100644 composeApp/src/jvmTest/kotlin/press/mantra/compose/ui/view/model/DkgRitualSecondSubgroupJvmTest.kt 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 1353970d..fcb53e1f 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 @@ -110,26 +110,27 @@ class DatabaseDkgRepository( ): Flow> = database.frostSigningSessionDao().observeSessionsForChatRoom(chatRoomId) + /** + * 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. + */ + override suspend fun roomDerivedBy(dkgSessionId: String): String? = + database.dkgSessionDao() + .getSessionById(dkgSessionId) + ?.thresholdPublicKey + ?.let { runCatching { SharedKeyDerivation.marmotGroupId(it) }.getOrNull() } + override fun observeSignedGroupKeyState(dkgSessionId: String): Flow = database.groupSignedEventDao() .observeByKind(GroupKeyStateEvent.KIND) .map { signedEvents -> - // 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. - // - // 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() } - - subjectRoomId?.let { GroupKeyStateManager.stateAmong(signedEvents, it) } + // Resolved per emission rather than once: the ceremony has no key + // until it finishes, and this flow is running before it does. + roomDerivedBy(dkgSessionId) + ?.let { GroupKeyStateManager.stateAmong(signedEvents, it) } } override suspend fun adoptGroupKeyState(chatRoomId: String): LocalGroupKeyState? = try { @@ -176,14 +177,9 @@ class DatabaseDkgRepository( database.groupSignedEventDao() .observeByKind(SubgroupBirthCertificateEvent.KIND) .map { signedEvents -> - // The room the 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 subgroupChatRoomId = database.dkgSessionDao() - .getSessionById(dkgSessionId) - ?.thresholdPublicKey - ?.let { runCatching { SharedKeyDerivation.marmotGroupId(it) }.getOrNull() } - ?: return@map null + // Resolved per emission rather than once: the ceremony has no key + // until it finishes, and this flow is running before it does. + val subgroupChatRoomId = roomDerivedBy(dkgSessionId) ?: return@map null signedEvents .filter { diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/nostr/subgroup/SubgroupBirthCertificateEvent.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/nostr/subgroup/SubgroupBirthCertificateEvent.kt index 9c1f8155..0554151d 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/nostr/subgroup/SubgroupBirthCertificateEvent.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/nostr/subgroup/SubgroupBirthCertificateEvent.kt @@ -170,15 +170,7 @@ object SubgroupBirthCertificateEvent { subgroupChatRoomId: String, parentChatRoomId: String ): Boolean = runCatching { - if (event.kind != KIND) return false - - if (!event.content.trim().equals(subgroupChatRoomId, ignoreCase = true)) return false - - val addressed = parseSubgroupChatRoomId(event.tags) ?: return false - if (!addressed.equals(subgroupChatRoomId, ignoreCase = true)) return false - - val named = parseParentChatRoomId(event.tags) ?: return false - if (!named.equals(parentChatRoomId, ignoreCase = true)) return false + if (!addresses(event, subgroupChatRoomId, parentChatRoomId)) return false val thresholdPublicKey = parseThresholdPublicKey(event.tags) ?: return false val path = parsePath(event.tags) ?: return false @@ -189,6 +181,38 @@ object SubgroupBirthCertificateEvent { GroupKeyStateEvent.isSignedByRoom(event, parentChatRoomId) }.getOrDefault(false) + /** + * Whether this event is *addressed to* that child by that parent -- the + * half of [certifies] that can be asked of a proposal nobody has signed. + * + * **Not a weaker [certifies].** It says who the event is about, not that + * the parent's quorum agreed to it: no derivation and no signature, so an + * event that merely claims the right pair passes. Nothing may decide a + * lineage on this. + * + * What it is for is telling one *in-flight* certificate signing from + * another in the same room. A parent certifies every subgroup it ever makes + * from one room, so the ceremony screen has several to choose between and + * no signature to choose by -- the proposal is what the quorum has not yet + * signed. The three values it reads are assembled together by + * [assembleTags], so they agree by construction. + */ + fun addresses( + event: Event, + subgroupChatRoomId: String, + parentChatRoomId: String + ): Boolean = runCatching { + if (event.kind != KIND) return false + + if (!event.content.trim().equals(subgroupChatRoomId, ignoreCase = true)) return false + + val addressed = parseSubgroupChatRoomId(event.tags) ?: return false + if (!addressed.equals(subgroupChatRoomId, ignoreCase = true)) return false + + val named = parseParentChatRoomId(event.tags) ?: return false + named.equals(parentChatRoomId, ignoreCase = true) + }.getOrDefault(false) + /** * The name tag. A bare two-element tag with no class of its own, because * nothing but the proposal screen and the subgroup list ever reads it and 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 1e6c16d6..c61a9e85 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/repository/DkgRepository.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/repository/DkgRepository.kt @@ -65,6 +65,19 @@ interface DkgRepository { suspend fun getLatestSessionForChatRoom(chatRoomId: String): DkgSession? + /** + * The id of the room a ceremony's key derives, or null while it has no key. + * + * The one name a ceremony and the room it makes have in common, and the only + * way to tell two of a parent's subgroups apart in anything written about + * the room rather than about the ceremony -- a birth certificate names the + * child, not the ceremony that produced it. + * + * Null is the ordinary answer rather than a failure: a ceremony has no key + * until it finishes, and every caller here is watching from before that. + */ + suspend fun roomDerivedBy(dkgSessionId: String): String? + /** * Opens a ritual in [localChatRoom], making this device the coordinator. * @@ -213,6 +226,8 @@ interface DkgRepository { override suspend fun getLatestSessionForChatRoom(chatRoomId: String): DkgSession? = null + override suspend fun roomDerivedBy(dkgSessionId: String): String? = null + override suspend fun proposeRitual( localChatRoom: LocalChatRoom, userPublicKey: HexKey, 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 feb4316f..5f6858ac 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 @@ -114,9 +114,6 @@ class DkgRitualViewModel( /** Watches the current session's messages; replaced whenever the session changes. */ private var messageObserver: Job? = null - /** Watches the room's signing sessions, which is where the key state is agreed. */ - private var keyStateObserver: Job? = null - /** Watches the parent certifying this subgroup; re-pointed as the purpose is learned. */ private var certificateObserver: Job? = null @@ -240,8 +237,6 @@ class DkgRitualViewModel( .defaultQuorum(ChillDkgRitualManager.memberPublicKeys(localChatRoom).size) .coerceIn(quorumRange()) - observeKeyState() - launch { roomSessions.collect { newest -> updateLoaded { current -> @@ -323,43 +318,6 @@ class DkgRitualViewModel( } } - /** - * Follows the group agreeing what its admin room will sign with. - * - * Nothing here watches a [GroupKeyState] row, because there is no room yet - * for one to belong to -- being able to follow this before the room exists - * is the point. What is watched instead is the session, for how the signing - * is going, and the signed event, for whether it finished. - * - * The room a ceremony runs in signs nothing else, so the newest session in - * it is the key state's -- but it is matched on the kind it carries anyway, - * so a room that one day signs something else does not confuse the two. - */ - private fun observeKeyState() { - keyStateObserver?.cancel() - keyStateObserver = viewModelScope.launch(Dispatchers.IO) { - // Two flows because they answer two questions off two tables. How - // the signing is going is the session row; whether it finished is - // the signed event, and only that second one can be asked at all - // before there is a room -- which is the whole point of asking it. - launch { - dkgRepository.observeSigningSessions(chatRoomId).collect { sessions -> - val keyState = sessions.firstOrNull { local -> - local.items.any { item -> - runCatching { - Event.fromJson(item.unsignedEventJson).kind - }.getOrNull() == GroupKeyStateEvent.KIND - } - }?.session - - updateLoaded { it.copy(keyStateSession = keyState) } - } - } - - - } - } - /** * Follows one ceremony: whether its key state has been signed, and -- for a * subgroup -- whether its parent has certified it. @@ -379,12 +337,50 @@ class DkgRitualViewModel( if (dkgSessionId == null) { updateLoaded { - it.copy(isKeyStateSigned = false, isCertified = false, certificateSession = null) + it.copy( + isKeyStateSigned = false, + keyStateSession = null, + isCertified = false, + certificateSession = null + ) } return } certificateObserver = viewModelScope.launch(Dispatchers.IO) { + // How the agreement is going, as opposed to whether it finished -- + // two questions off two tables, and only the second can be asked + // before there is a room at all, which is the point of asking it. + // + // By the ceremony the proposal names, not by kind in this room. The + // room a ceremony runs in used to sign nothing else, and that is + // what this read assumed; a parent's Marmot room now hosts the + // ceremony of every subgroup the group makes, so it holds one of + // these per subgroup. Matching on kind alone handed the screen the + // newest of them -- which is some other subgroup's -- and with it a + // progress bar for a signing this ceremony is not in and a button + // disabled on the strength of it. Permanently, once that other one + // completed: the guard only reopens on a failure. + // + // `FrostKeyTag` is on the proposal from the moment it is made, so + // this works before the ceremony has a key and before anybody signs. + launch { + dkgRepository.observeSigningSessions(chatRoomId).collect { sessions -> + val keyState = sessions.firstOrNull { local -> + local.items.any { item -> + val event = runCatching { + Event.fromJson(item.unsignedEventJson) + }.getOrNull() ?: return@any false + + event.kind == GroupKeyStateEvent.KIND && + GroupKeyStateEvent.parseDkgSessionId(event.tags) == dkgSessionId + } + }?.session + + updateLoaded { it.copy(keyStateSession = keyState) } + } + } + // 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 @@ -408,14 +404,43 @@ class DkgRitualViewModel( } launch { + // The child this ceremony is making, which is what a certificate + // names. Resolved per emission rather than once, because the + // ceremony has no key until it finishes and this flow is running + // before it does -- the same reason `observeBirthCertificate` + // resolves it per emission. + // + // Scoped to that child, not to the kind. A parent certifies every + // subgroup it ever makes from the same room, so by kind alone this + // found the newest certificate signing in the parent -- some other + // child's -- and read its progress as this one's. Worse, the rung + // disables "ask the group to certify" while a certificate session + // is running and un-failed, so once a parent had certified one + // subgroup it could never be asked to certify another. dkgRepository.observeSigningSessions(parentChatRoomId).collect { sessions -> - val certificate = sessions.firstOrNull { local -> - local.items.any { item -> - runCatching { - Event.fromJson(item.unsignedEventJson).kind - }.getOrNull() == SubgroupBirthCertificateEvent.KIND - } - }?.session + val subgroupChatRoomId = dkgRepository.roomDerivedBy(dkgSessionId) + + val certificate = subgroupChatRoomId?.let { child -> + sessions.firstOrNull { local -> + local.items.any { item -> + val event = runCatching { + Event.fromJson(item.unsignedEventJson) + }.getOrNull() ?: return@any false + + // `addresses`, not `certifies`: the thing being + // matched is a proposal, and `certifies` ends on + // a signature by the parent's quorum, which is + // precisely what has not happened yet. Whether + // the parent actually agreed is `isCertified`, + // which does check it. + SubgroupBirthCertificateEvent.addresses( + event = event, + subgroupChatRoomId = child, + parentChatRoomId = parentChatRoomId, + ) + } + }?.session + } updateLoaded { it.copy(certificateSession = certificate) } } @@ -736,7 +761,6 @@ class DkgRitualViewModel( override fun onCleared() { messageObserver?.cancel() - keyStateObserver?.cancel() certificateObserver?.cancel() super.onCleared() } diff --git a/composeApp/src/jvmTest/kotlin/press/mantra/compose/ui/view/model/DkgRitualSecondSubgroupJvmTest.kt b/composeApp/src/jvmTest/kotlin/press/mantra/compose/ui/view/model/DkgRitualSecondSubgroupJvmTest.kt new file mode 100644 index 00000000..84815488 --- /dev/null +++ b/composeApp/src/jvmTest/kotlin/press/mantra/compose/ui/view/model/DkgRitualSecondSubgroupJvmTest.kt @@ -0,0 +1,322 @@ +package press.mantra.compose.ui.view.model + +import com.vitorpamplona.quartz.nip01Core.core.Event +import com.vitorpamplona.quartz.nip01Core.core.HexKey +import com.vitorpamplona.quartz.nip01Core.core.toHexKey +import com.vitorpamplona.quartz.nip01Core.crypto.KeyPair +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.resetMain +import kotlinx.coroutines.test.setMain +import kotlinx.coroutines.withTimeout +import press.mantra.compose.database.model.ChatRoom +import press.mantra.compose.database.model.DkgSession +import press.mantra.compose.database.model.FrostSigningItem +import press.mantra.compose.database.model.FrostSigningSession +import press.mantra.compose.database.model.Participant +import press.mantra.compose.database.model.intermdiate.LocalChatRoom +import press.mantra.compose.database.model.intermdiate.LocalFrostSigningSession +import press.mantra.compose.database.model.intermdiate.LocalParticipant +import press.mantra.compose.database.model.types.DkgRitualStage +import press.mantra.compose.database.model.types.FrostSigningStage +import press.mantra.compose.nostr.frost.GroupKeyStateEvent +import press.mantra.compose.nostr.subgroup.SubgroupBirthCertificateEvent +import press.mantra.compose.repository.ChatRepository +import press.mantra.compose.repository.DkgRepository +import press.mantra.compose.ui.view.state.DkgRitualUIState +import kotlin.test.AfterTest +import kotlin.test.BeforeTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNull + +/** + * A parent's room holds every one of its subgroups' signings, and the ceremony + * screen has to pick out the one it is showing. + * + * Since a subgroup's ChillDKG moved into its parent's Marmot room, both of the + * rungs after the ceremony run there too: the birth certificate is the parent's + * signature, and the key state is signed over the parent's room because the + * subgroup's own room does not exist yet. So a parent making a second subgroup + * has two of each in one room, told apart by nothing but the ceremony they + * belong to. + */ +@OptIn(ExperimentalCoroutinesApi::class) +class DkgRitualSecondSubgroupJvmTest { + private val parentChatRoomId = "aa".repeat(32) + private val me = "11".repeat(32) + private val peer = "22".repeat(32) + + // Real points, in the compressed form `SharedKeyDerivation` reads: it derives + // a room id off the curve, and a made-up 32 bytes is not on it. + private val firstKey = "02" + KeyPair().pubKey.toHexKey() + private val secondKey = "02" + KeyPair().pubKey.toHexKey() + + @BeforeTest + fun setUp() { + Dispatchers.setMain(UnconfinedTestDispatcher()) + } + + @AfterTest + fun tearDown() { + Dispatchers.resetMain() + } + + @Test + fun `the certificate shown is this subgroup's, not the parent's newest`() = runBlocking { + val loaded = load( + signingSessions = listOf( + // Newest first, the way the query returns them. + certificateSigning("forSecond", subgroupOf(secondKey)), + certificateSigning("forFirst", subgroupOf(firstKey)), + ), + until = { it.certificateSession != null } + ) + + assertEquals( + "forFirst", + loaded.certificateSession?.id, + "the screen for one subgroup is following another subgroup's certificate" + ) + } + + @Test + fun `the key state shown is this ceremony's, not the room's newest`() = runBlocking { + val loaded = load( + signingSessions = listOf( + keyStateSigning("forSecond", dkgSessionId = "secondCeremony"), + keyStateSigning("forFirst", dkgSessionId = "firstCeremony"), + ), + until = { it.keyStateSession != null } + ) + + assertEquals( + "forFirst", + loaded.keyStateSession?.id, + "the screen for one ceremony is following another ceremony's key state" + ) + } + + /** + * The damaging half, and the reason this is a block rather than a cosmetic + * mix-up. Both rungs disable their button while a signing of that kind is + * running and un-failed, and a *completed* signing is un-failed forever -- + * so by kind alone, a parent that had certified one subgroup could never be + * asked to certify another, and the rung sat there disabled with no way out. + */ + @Test + fun `a sibling's finished signings leave this ceremony's rungs open`() = runBlocking { + // Driven in two emissions rather than one, so the assertion is about a + // watcher that demonstrably ran. Asserting "still null" against a single + // emission passes before the watcher has started, which is how a version + // of this test passed against the bug it was written for. + val signingSessions = MutableStateFlow( + listOf( + certificateSigning("mine", subgroupOf(firstKey)), + keyStateSigning("myKeyState", dkgSessionId = "firstCeremony"), + ) + ) + + val viewModel = viewModel(signingSessions) + viewModel.initiate() + + awaitLoaded(viewModel) { + it.certificateSession?.id == "mine" && it.keyStateSession?.id == "myKeyState" + } + + // Now the room holds only a sibling's, both finished -- which is every + // parent that has already made one subgroup and is making its second. + signingSessions.value = listOf( + certificateSigning( + "siblingCertificate", + subgroupOf(secondKey), + stage = FrostSigningStage.COMPLETE + ), + // A key state naming another ceremony: the sibling's, or this parent's + // own from an adoption, which is signed in this room too. + keyStateSigning( + "siblingKeyState", + dkgSessionId = "secondCeremony", + stage = FrostSigningStage.COMPLETE + ), + ) + + val loaded = awaitLoaded(viewModel) { + it.certificateSession == null && it.keyStateSession == null + } + + // Both rungs gate on these being null or failed, and a completed signing + // is never failed -- so a sibling left in either of them shuts the rung + // for good. + assertNull(loaded.certificateSession) + assertNull(loaded.keyStateSession) + } + + private suspend fun load( + signingSessions: List, + until: (DkgRitualUIState.Loaded) -> Boolean, + ): DkgRitualUIState.Loaded { + val viewModel = viewModel(MutableStateFlow(signingSessions)) + viewModel.initiate() + return awaitLoaded(viewModel, until) + } + + private suspend fun awaitLoaded( + viewModel: DkgRitualViewModel, + until: (DkgRitualUIState.Loaded) -> Boolean, + ): DkgRitualUIState.Loaded = withTimeout(5_000) { + viewModel.dkgRitualUIState.first { + (it as? DkgRitualUIState.Loaded)?.let(until) == true + } + } as DkgRitualUIState.Loaded + + /** The screen is always showing the FIRST subgroup's ceremony. */ + private fun viewModel( + signingSessions: MutableStateFlow> + ): DkgRitualViewModel { + val ceremony = DkgSession( + id = "firstCeremony", + chatRoomId = parentChatRoomId, + coordinatorPublicKey = me, + userPublicKey = me, + threshold = 2, + participantCount = 2, + hostPublicKey = me, + round1Random = "66".repeat(32), + round2AuxRandom = "77".repeat(32), + stage = DkgRitualStage.COMPLETE, + thresholdPublicKey = firstKey, + parentChatRoomId = parentChatRoomId, + subject = "Translators", + ) + + return DkgRitualViewModel( + chatRoomId = parentChatRoomId, + activeUserPublicKey = me, + dkgSessionId = ceremony.id, + initialDkgRitualUIState = DkgRitualUIState.Loading, + activeIdentityStateFlow = MutableStateFlow(null), + chatRepository = object : ChatRepository by ChatRepository.NO_OP_CHAT_REPOSITORY { + override suspend fun getChatRoomByIdentifier(id: HexKey): LocalChatRoom? = room() + }, + dkgRepository = object : DkgRepository by DkgRepository.NO_OP_DKG_REPOSITORY { + override suspend fun roomDerivedBy(dkgSessionId: String): String? = + if (dkgSessionId == ceremony.id) subgroupOf(firstKey) else null + + override fun observeSessionById(sessionId: String): Flow = + flowOf(ceremony) + + override fun observeLatestSessionForChatRoom( + chatRoomId: String, + parentChatRoomId: String?, + ): Flow = flowOf(ceremony) + + override fun observeSigningSessions( + chatRoomId: String + ): Flow> = signingSessions + } + ) + } + + /** The room id a ceremony's key derives, which is what a certificate names. */ + private fun subgroupOf(thresholdPublicKey: HexKey): String = + press.mantra.compose.managers.SharedKeyDerivation.marmotGroupId(thresholdPublicKey) + + private fun certificateSigning( + id: String, + subgroupChatRoomId: String, + stage: FrostSigningStage = FrostSigningStage.COLLECTING_NONCES, + ) = signing( + id = id, + stage = stage, + event = Event( + id = "ee".repeat(32), + pubKey = me, + createdAt = 0, + kind = SubgroupBirthCertificateEvent.KIND, + tags = SubgroupBirthCertificateEvent.assembleTags( + subgroupChatRoomId = subgroupChatRoomId, + parentChatRoomId = parentChatRoomId, + thresholdPublicKey = firstKey, + adminPublicKeys = listOf(me, peer), + name = "Translators", + ), + content = subgroupChatRoomId, + sig = "ff".repeat(64), + ) + ) + + private fun keyStateSigning( + id: String, + dkgSessionId: String, + stage: FrostSigningStage = FrostSigningStage.COLLECTING_NONCES, + ) = signing( + id = id, + stage = stage, + event = Event( + id = "ee".repeat(32), + pubKey = me, + createdAt = 0, + kind = GroupKeyStateEvent.KIND, + tags = GroupKeyStateEvent.assembleTags( + chatRoomId = subgroupOf(firstKey), + dkgSessionId = dkgSessionId, + ), + content = firstKey, + sig = "ff".repeat(64), + ) + ) + + private fun signing( + id: String, + event: Event, + stage: FrostSigningStage = FrostSigningStage.COLLECTING_NONCES, + ) = LocalFrostSigningSession( + session = FrostSigningSession( + id = id, + stage = stage, + chatRoomId = parentChatRoomId, + coordinatorPublicKey = me, + userPublicKey = me, + dkgSessionId = "firstCeremony", + threshold = 2, + participantCount = 2, + signerId = 0, + ), + unorderedItems = listOf( + FrostSigningItem( + sessionId = id, + itemIndex = 0, + unsignedEventJson = event.toJson(), + eventId = event.id, + nonceRandom = "55".repeat(32), + ) + ), + ) + + private fun room() = LocalChatRoom( + chatRoom = ChatRoom( + id = parentChatRoomId, + userPublicKey = me, + subject = "Ekklesia", + description = null, + mlsGroupState = "state", + ), + localParticipants = listOf(me, peer).map { publicKey -> + LocalParticipant( + participant = Participant( + participantPublicKey = publicKey, + chatRoomId = parentChatRoomId, + relayHint = null, + ), + profile = null, + ) + }, + ) +} diff --git a/composeApp/src/jvmTest/kotlin/press/mantra/compose/ui/view/model/DkgRitualStartOfferJvmTest.kt b/composeApp/src/jvmTest/kotlin/press/mantra/compose/ui/view/model/DkgRitualStartOfferJvmTest.kt index e82b816e..e811acb0 100644 --- a/composeApp/src/jvmTest/kotlin/press/mantra/compose/ui/view/model/DkgRitualStartOfferJvmTest.kt +++ b/composeApp/src/jvmTest/kotlin/press/mantra/compose/ui/view/model/DkgRitualStartOfferJvmTest.kt @@ -111,21 +111,30 @@ class DkgRitualStartOfferJvmTest { * * Every flow folding into this state used to read it, build a `copy` and * assign -- and the session watcher asks the database what this member owes - * in the middle of that. The key-state watcher writing during that pause had - * its change thrown away by the assignment that followed. The same pause in - * the other direction is what put `session` back to null and offered to - * start the ceremony on screen; this is the half of it that can be forced to - * happen rather than raced into. + * in the middle of that. Whoever wrote during that pause had their change + * thrown away by the assignment that followed. The same pause in the other + * direction is what put `session` back to null and offered to start the + * ceremony on screen; this is the half of it that can be forced to happen + * rather than raced into. + * + * The pair used here is the session watcher against the room watcher, which + * are the two that are genuinely concurrent: both are collecting before + * either has written, and neither waits on the other. `roomHasCeremony` is + * the field they fight over, and it is the one that decides the offer. */ @Test fun `a change written during a suspension survives the write that follows it`() = runBlocking { - val session = session(DkgRitualStage.COLLECTING_ROUND_1) - val signingSessions = MutableStateFlow>(emptyList()) + val live = session(DkgRitualStage.COLLECTING_ROUND_1) + val abandoned = session(DkgRitualStage.FAILED) - // Held open until the test has had the key-state watcher write. The - // seeding call runs before anything else is collecting, so it is let - // through; the session watcher's call is the one that waits. + val roomSessions = MutableStateFlow(live) + + // Opened once the test has had the room watcher write. The seeding call + // runs before anything else is collecting, so it is let through; the + // session watcher's call is the one that waits, and `parked` says when + // it is in there. val gate = CompletableDeferred() + val parked = CompletableDeferred() var calls = 0 val viewModel = viewModel( @@ -133,16 +142,16 @@ class DkgRitualStartOfferJvmTest { override fun observeLatestSessionForChatRoom( chatRoomId: String, parentChatRoomId: String?, - ): Flow = flowOf(session) + ): Flow = roomSessions - override fun observeSigningSessions( - chatRoomId: String - ): Flow> = signingSessions + override fun observeSessionById(sessionId: String): Flow = + flowOf(live) override suspend fun pendingApproval(session: DkgSession): DkgApprovalStep? { calls += 1 if (calls == 1) return null + parked.complete(Unit) gate.await() return DkgApprovalStep.ROUND_1 } @@ -151,33 +160,37 @@ class DkgRitualStartOfferJvmTest { viewModel.initiate() - withTimeout(5_000) { viewModel.dkgRitualUIState.first { it is DkgRitualUIState.Loaded } } + awaitLoaded(viewModel) { it.roomHasCeremony } - // The watcher is now parked inside `pendingApproval`, holding a copy of - // the state as it was before this line. - signingSessions.value = listOf(keyStateSigningSession()) - - withTimeout(5_000) { - viewModel.dkgRitualUIState.first { - (it as? DkgRitualUIState.Loaded)?.keyStateSession != null - } - } + // The session watcher is now inside `pendingApproval`, holding -- in the + // shape this used to be written in -- a copy of the state as it was + // before the next two lines. + withTimeout(5_000) { parked.await() } + roomSessions.value = abandoned + awaitLoaded(viewModel) { !it.roomHasCeremony } gate.complete(Unit) - val settled = withTimeout(5_000) { - viewModel.dkgRitualUIState.first { - (it as? DkgRitualUIState.Loaded)?.pendingApproval == DkgApprovalStep.ROUND_1 - } - } as DkgRitualUIState.Loaded + val settled = awaitLoaded(viewModel) { + it.pendingApproval == DkgApprovalStep.ROUND_1 + } - assertNotNull( - settled.keyStateSession, - "the session watcher wrote back a copy taken before the key state arrived" + assertFalse( + settled.roomHasCeremony, + "the session watcher wrote back a copy taken before the room watcher's change" ) - assertEquals(session.id, settled.session?.id) + assertEquals(live.id, settled.session?.id) } + private suspend fun awaitLoaded( + viewModel: DkgRitualViewModel, + until: (DkgRitualUIState.Loaded) -> Boolean, + ): DkgRitualUIState.Loaded = withTimeout(5_000) { + viewModel.dkgRitualUIState.first { + (it as? DkgRitualUIState.Loaded)?.let(until) == true + } + } as DkgRitualUIState.Loaded + private fun viewModel(dkgRepository: DkgRepository) = DkgRitualViewModel( chatRoomId = chatRoomId, activeUserPublicKey = me, From b6c3e3ddc0073c24d7c958f941d8128318e7e572 Mon Sep 17 00:00:00 2001 From: Kgothatso Ngako Date: Fri, 2 Oct 2026 20:22:17 +0200 Subject: [PATCH 4/4] test(subgroups): pin what `addresses` is worth, and when one ceremony is the whole question Two surfaces the last three commits added and left resting on indirect coverage. Both are decisions rather than plumbing, and neither had a test that would notice it changing. **`SubgroupBirthCertificateEvent.addresses` had none at all.** Its sibling `certifies` has nineteen, several of them real FROST forgeries, because a certificate is the whole of what a subgroup relationship is. `addresses` was then added *beside* it as a deliberately weaker reading -- no derivation, no signature -- and exercised only through a view model, which is the wrong place to learn what it is worth. The invariant worth having is the one a future reader can get wrong: **an event can pass `addresses` and fail `certifies`.** Two cases say so in the two ways it happens -- an unsigned proposal, which is exactly what the ceremony screen holds and asks about, and a forgery authored and signed by the child's own quorum, which is anybody at all. A third asserts `certifies` implies `addresses`, so the weaker one cannot drift into being the stricter one while both are called "which child is this about". The other two are the case the predicate exists for: two subgroups of one parent. A sibling's well-formed proposal is not addressed to this child, *and is* addressed to the sibling -- told apart rather than both refused, which a predicate that merely failed closed would also satisfy. Checked by giving `addresses` a signature check: the two "and certifies nothing" cases fail, the other three do not. **`hidesOtherCeremonies` had none either**, and it decides where a tap in the transcript goes. `CeremonyQueueRoutingJvmTest` pins both of its guards separately, which is the point of four cases rather than two: - one ceremony owed -> open it. Drop the `size > 1` and this fails. - two owed -> the list, from either tap, because neither is the whole of what the room is waiting on this member for. - a *finished* ceremony's line -> open it, even while two others are owed. This is the restraint, and dropping the "is itself awaiting" clause is what it catches: a reader tapping history is asking to see history, not to be handed somebody else's queue. - an abandoned one is never owed, even when `pendingApproval` says it is. Each regression was reinstated separately and each is caught by exactly one case. One thing the empty expectation needed care over, and it is the same trap a test in `26d305bc` fell into: a watcher that writes once cannot be waited for by its own empty value, because empty is indistinguishable from not having run. The abandoned case waits on a clock instead and says so. **`hidesOtherDecisions`, the proposal counterpart this mirrors, still has no test.** It predates all of this and is left alone, but the asymmetry now runs the other way and is worth knowing about. **Still untested, and deliberately so rather than overlooked:** the three entry points into the ceremony list -- the "Key ceremonies" row on the group's details, which exists to be reachable in a *Marmot* room where "Shared key" is not, the standing notice handing back null for several, and the transcript line's routing. Those are desktop compose-ui tests over `ChatRoomDetailScreen` and `ChatTranscript`, both of which stand up considerably more than anything here does. 982 jvm tests, 459 common tests, `m3Audit` meeting every budget. Co-Authored-By: Claude Opus 5 --- .../SubgroupBirthCertificateEventTest.kt | 146 ++++++++++++ .../view/model/CeremonyQueueRoutingJvmTest.kt | 216 ++++++++++++++++++ 2 files changed, 362 insertions(+) create mode 100644 composeApp/src/jvmTest/kotlin/press/mantra/compose/ui/view/model/CeremonyQueueRoutingJvmTest.kt diff --git a/composeApp/src/commonTest/kotlin/press/mantra/compose/nostr/subgroup/SubgroupBirthCertificateEventTest.kt b/composeApp/src/commonTest/kotlin/press/mantra/compose/nostr/subgroup/SubgroupBirthCertificateEventTest.kt index a625bb39..ada04fb2 100644 --- a/composeApp/src/commonTest/kotlin/press/mantra/compose/nostr/subgroup/SubgroupBirthCertificateEventTest.kt +++ b/composeApp/src/commonTest/kotlin/press/mantra/compose/nostr/subgroup/SubgroupBirthCertificateEventTest.kt @@ -436,6 +436,152 @@ class SubgroupBirthCertificateEventTest { assertNull(SubgroupManager.classify(subgroupChatRoomId, aboutSomebodyElse)) } + // ---- `addresses`: the half that can be asked of a proposal ------------ + // + // The ceremony screen has to tell one *in-flight* certificate signing from + // another in the parent's room, and a parent certifies every subgroup it + // ever makes from that one room. `certifies` cannot answer it: it ends on a + // signature by the parent's quorum, which is exactly what the screen is + // waiting for. `addresses` is the structural half, and the only thing worth + // asserting about it is where it stops. + + @Test + fun `an unsigned proposal addresses its child, and certifies nothing`() { + // What the ceremony screen is actually holding: the event as it was + // assembled, before anybody signed it. Any string in `sig` serves -- + // the point is that no quorum has put one there. + val proposal = certificate(signature = "0".repeat(128)) + + assertTrue( + SubgroupBirthCertificateEvent.addresses( + event = proposal, + subgroupChatRoomId = subgroupChatRoomId, + parentChatRoomId = parentChatRoomId + ), + "the screen could not tell which subgroup its own proposal is about" + ) + + assertFalse( + SubgroupBirthCertificateEvent.certifies( + event = proposal, + subgroupChatRoomId = subgroupChatRoomId, + parentChatRoomId = parentChatRoomId + ), + "an unsigned proposal is being read as a parent's certificate" + ) + } + + @Test + fun `a forgery by another group addresses the pair it names, and certifies nothing`() { + // The reason `addresses` must never decide a lineage: anybody can author + // an event naming any pair, and this says yes to it. The guard is that + // nothing downstream of a lineage asks `addresses` -- only the screen + // deciding which of its own room's signings to draw does. + val forged = certificate(author = childMaterial) + + assertTrue( + SubgroupBirthCertificateEvent.addresses( + event = forged, + subgroupChatRoomId = subgroupChatRoomId, + parentChatRoomId = parentChatRoomId + ) + ) + + assertFalse( + SubgroupBirthCertificateEvent.certifies( + event = forged, + subgroupChatRoomId = subgroupChatRoomId, + parentChatRoomId = parentChatRoomId + ) + ) + } + + @Test + fun `a sibling's proposal is not addressed to this child`() { + // Two subgroups of one parent, which is the case the screen exists to + // tell apart. The sibling's proposal is a real, well-formed certificate + // request; it is simply about somebody else. + val siblingRoomId = SharedKeyDerivation + .marmotGroupId(strangerMaterial.thresholdPublicKey.value.toHex(), path) + + val sibling = certificate( + content = siblingRoomId, + tags = SubgroupBirthCertificateEvent.assembleTags( + subgroupChatRoomId = siblingRoomId, + parentChatRoomId = parentChatRoomId, + thresholdPublicKey = strangerMaterial.thresholdPublicKey.value.toHex(), + adminPublicKeys = admins, + name = "Another team", + path = path + ) + ) + + assertFalse( + SubgroupBirthCertificateEvent.addresses( + event = sibling, + subgroupChatRoomId = subgroupChatRoomId, + parentChatRoomId = parentChatRoomId + ), + "one subgroup's rung is following a sibling's proposal" + ) + + // And it is addressed to the sibling, so the sibling's own screen finds + // it -- the two are told apart rather than both being refused. + assertTrue( + SubgroupBirthCertificateEvent.addresses( + event = sibling, + subgroupChatRoomId = siblingRoomId, + parentChatRoomId = parentChatRoomId + ) + ) + } + + @Test + fun `a proposal naming another parent is not addressed by this one`() { + val fromElsewhere = certificate( + tags = SubgroupBirthCertificateEvent.assembleTags( + subgroupChatRoomId = subgroupChatRoomId, + parentChatRoomId = SharedKeyDerivation + .marmotGroupId(strangerMaterial.thresholdPublicKey.value.toHex(), path), + thresholdPublicKey = childKey, + adminPublicKeys = admins, + name = name, + path = path + ) + ) + + assertFalse( + SubgroupBirthCertificateEvent.addresses( + event = fromElsewhere, + subgroupChatRoomId = subgroupChatRoomId, + parentChatRoomId = parentChatRoomId + ) + ) + } + + @Test + fun `everything certifies addresses, so the weaker one cannot be the stricter`() { + // `certifies` calls `addresses` and adds to it, and this is what keeps + // that true: a change that let one pass where the other refuses would + // mean two different answers to "which child is this about". + val valid = certificate() + + assertTrue( + SubgroupBirthCertificateEvent.certifies( + event = valid, + subgroupChatRoomId = subgroupChatRoomId, + parentChatRoomId = parentChatRoomId + ) + ) + assertTrue( + SubgroupBirthCertificateEvent.addresses( + event = valid, + subgroupChatRoomId = subgroupChatRoomId, + parentChatRoomId = parentChatRoomId + ) + ) + } + private fun certificate( content: String = subgroupChatRoomId, tags: Array> = SubgroupBirthCertificateEvent.assembleTags( diff --git a/composeApp/src/jvmTest/kotlin/press/mantra/compose/ui/view/model/CeremonyQueueRoutingJvmTest.kt b/composeApp/src/jvmTest/kotlin/press/mantra/compose/ui/view/model/CeremonyQueueRoutingJvmTest.kt new file mode 100644 index 00000000..f6c876cb --- /dev/null +++ b/composeApp/src/jvmTest/kotlin/press/mantra/compose/ui/view/model/CeremonyQueueRoutingJvmTest.kt @@ -0,0 +1,216 @@ +package press.mantra.compose.ui.view.model + +import com.vitorpamplona.quartz.nip01Core.core.HexKey +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.resetMain +import kotlinx.coroutines.test.setMain +import kotlinx.coroutines.withTimeout +import kotlinx.coroutines.delay +import press.mantra.compose.database.model.ChatRoom +import press.mantra.compose.database.model.DkgSession +import press.mantra.compose.database.model.Participant +import press.mantra.compose.database.model.intermdiate.LocalChatRoom +import press.mantra.compose.database.model.intermdiate.LocalParticipant +import press.mantra.compose.database.model.types.DkgApprovalStep +import press.mantra.compose.database.model.types.DkgRitualStage +import press.mantra.compose.repository.ChatRepository +import press.mantra.compose.repository.DkgRepository +import press.mantra.compose.repository.FrostSigningRepository +import press.mantra.compose.repository.NostrRepository +import press.mantra.compose.ui.view.state.ChatMessageListUIState +import kotlin.test.AfterTest +import kotlin.test.BeforeTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +/** + * When tapping one ceremony in the transcript is answering the whole question, + * and when it is answering a third of it. + * + * The transcript names one ceremony per line and its standing notice stands for + * however many are owed. Opening the named one is right while it is the only + * thing being asked; it stops being right the moment there are two, because the + * second is not on screen beside the first -- it may be pages up, or have + * arrived while the member was reading -- so answering the one they tapped and + * leaving would look exactly like being done. + * + * `hidesOtherDecisions` has made this call for proposals since before subgroups + * existed. [ChatMessageListViewModel.hidesOtherCeremonies] is its counterpart, + * and a room holds several ceremonies as a matter of course now that a + * subgroup's ChillDKG runs in its parent's room. + */ +@OptIn(ExperimentalCoroutinesApi::class) +class CeremonyQueueRoutingJvmTest { + private val chatRoomId = "aa".repeat(32) + private val me = "11".repeat(32) + private val peer = "22".repeat(32) + + @BeforeTest + fun setUp() { + Dispatchers.setMain(UnconfinedTestDispatcher()) + } + + @AfterTest + fun tearDown() { + Dispatchers.resetMain() + } + + @Test + fun `one ceremony owed is answered by opening it`() = runBlocking { + val viewModel = awaiting( + sessions = listOf(session("only", DkgRitualStage.COLLECTING_ROUND_1)), + pending = mapOf("only" to DkgApprovalStep.ROUND_1), + expected = 1 + ) + + assertFalse( + viewModel.hidesOtherCeremonies("only"), + "the one ceremony owed was sent to a queue of itself" + ) + } + + @Test + fun `two ceremonies owed send the member to the list instead`() = runBlocking { + val viewModel = awaiting( + sessions = listOf( + session("ours", DkgRitualStage.COLLECTING_ROUND_1), + session("subgroup", DkgRitualStage.COLLECTING_HOST_KEYS) + .copy(parentChatRoomId = chatRoomId, subject = "Translators"), + ), + pending = mapOf( + "ours" to DkgApprovalStep.ROUND_1, + "subgroup" to DkgApprovalStep.HOST_KEY + ), + expected = 2 + ) + + // Either tap: the queue is the honest answer to both, because neither is + // the whole of what the room is waiting on this member for. + assertTrue(viewModel.hidesOtherCeremonies("ours")) + assertTrue(viewModel.hidesOtherCeremonies("subgroup")) + } + + /** + * The restraint, and the reason this is not just "does the room owe two". + * A line about a ceremony the group finished is history, and a reader who + * taps one is asking to see that -- not to be handed a queue of other + * people's business. + */ + @Test + fun `a finished ceremony is opened on its own even while others are owed`() = runBlocking { + val viewModel = awaiting( + sessions = listOf( + session("done", DkgRitualStage.COMPLETE), + session("ours", DkgRitualStage.COLLECTING_ROUND_1), + session("subgroup", DkgRitualStage.COLLECTING_HOST_KEYS), + ), + pending = mapOf( + "ours" to DkgApprovalStep.ROUND_1, + "subgroup" to DkgApprovalStep.HOST_KEY + ), + expected = 2 + ) + + assertFalse( + viewModel.hidesOtherCeremonies("done"), + "a finished ceremony's line was answered with somebody else's queue" + ) + } + + /** + * An abandoned one is not owed either, which matters because the room's + * watcher filters on it and a row that slipped through would put a dead + * ceremony at the top of the notice. + */ + @Test + fun `an abandoned ceremony is never counted as owed`() = runBlocking { + val viewModel = awaiting( + sessions = listOf(session("abandoned", DkgRitualStage.FAILED)), + // Says yes, and has to be ignored on a failure. + pending = mapOf("abandoned" to DkgApprovalStep.ROUND_1), + expected = 0 + ) + + assertTrue(viewModel.ceremoniesAwaitingYou.isEmpty()) + assertFalse(viewModel.hidesOtherCeremonies("abandoned")) + } + + /** Builds the view model and waits until its watcher has settled on [expected]. */ + private suspend fun awaiting( + sessions: List, + pending: Map, + expected: Int, + ): ChatMessageListViewModel { + val viewModel = ChatMessageListViewModel( + initialChatMessageListUIState = ChatMessageListUIState.Loading, + localChatRoom = room(), + nostrRepository = NostrRepository.NO_OP_NOSTR_REPOSITORY, + chatRepository = ChatRepository.NO_OP_CHAT_REPOSITORY, + frostSigningRepository = FrostSigningRepository.NO_OP_FROST_SIGNING_REPOSITORY, + dkgRepository = object : DkgRepository by DkgRepository.NO_OP_DKG_REPOSITORY { + override fun observeSessionsForChatRoom( + chatRoomId: String + ): Flow> = flowOf(sessions) + + override suspend fun pendingApproval(session: DkgSession): DkgApprovalStep? = + pending[session.id] + } + ) + + viewModel.initiate() + + // The watcher writes once, so an empty expectation cannot be waited for + // by its own value -- it is indistinguishable from not having run. Wait + // for a non-empty one, and give an empty one a moment to be wrong in. + withTimeout(5_000) { + if (expected == 0) { + delay(200) + } else { + while (viewModel.ceremoniesAwaitingYou.size != expected) delay(5) + } + } + + assertEquals(expected, viewModel.ceremoniesAwaitingYou.size) + return viewModel + } + + private fun room() = LocalChatRoom( + chatRoom = ChatRoom( + id = chatRoomId, + userPublicKey = me, + subject = "Ekklesia", + description = null, + mlsGroupState = null, + ), + localParticipants = listOf(me, peer).map { publicKey -> + LocalParticipant( + participant = Participant( + participantPublicKey = publicKey, + chatRoomId = chatRoomId, + relayHint = null, + ), + profile = null, + ) + }, + ) + + private fun session(id: String, stage: DkgRitualStage) = DkgSession( + id = id, + chatRoomId = chatRoomId, + coordinatorPublicKey = peer, + userPublicKey = me, + threshold = 2, + participantCount = 2, + hostPublicKey = peer, + round1Random = "66".repeat(32), + round2AuxRandom = "77".repeat(32), + stage = stage, + ) +}