From b6c3e3ddc0073c24d7c958f941d8128318e7e572 Mon Sep 17 00:00:00 2001 From: Kgothatso Ngako Date: Fri, 2 Oct 2026 20:22:17 +0200 Subject: [PATCH] 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, + ) +}