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 <noreply@anthropic.com>
This commit is contained in:
@@ -436,6 +436,152 @@ class SubgroupBirthCertificateEventTest {
|
|||||||
assertNull(SubgroupManager.classify(subgroupChatRoomId, aboutSomebodyElse))
|
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(
|
private fun certificate(
|
||||||
content: String = subgroupChatRoomId,
|
content: String = subgroupChatRoomId,
|
||||||
tags: Array<Array<String>> = SubgroupBirthCertificateEvent.assembleTags(
|
tags: Array<Array<String>> = SubgroupBirthCertificateEvent.assembleTags(
|
||||||
|
|||||||
@@ -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<DkgSession>,
|
||||||
|
pending: Map<String, DkgApprovalStep>,
|
||||||
|
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<List<DkgSession>> = 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,
|
||||||
|
)
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user