fix(subgroups): ask whether the key state is signed by ceremony, not by room
Subgroups "2.0" and "2.1" on the connected devices had finished everything. A
ChillDKG COMPLETE for each, a birth certificate signed by the parent's quorum for
each, a key state signed by their own quorum for each -- all four events sitting in
the coordinator's database. The coordinator was offered no way to create the room.
`observeSignedGroupKeyState` resolved which key to ask about by looking the
ceremony up **from the room**:
getLatestSessionFor(chatRoomId, parentChatRoomId) // parent was null here
The view model calls it with no parent, so that reads "the ceremony in this room
that is *not* for a subgroup". A subgroup's ceremony room holds only the
subgroup's ceremony, so it matched nothing, derived no key, and reported "not
signed" over a state in the same database. The button is gated on that boolean, so
it never appeared.
This is the same mistake as `0b65d702` one layer down. That commit fixed the
*session* lookup by having the transcript name the ceremony; it left the key-state
and certificate lookups still re-deriving a ceremony from the room. A room does not
have one, and every lookup that assumes it does is wrong in a different way: by
room it finds none here, and by "newest in room" it would have found the wrong one
in the rooms from the previous report.
So both observers now take the ceremony they are about.
`observeSignedGroupKeyState(dkgSessionId)` derives the subject room from that
ceremony's own threshold key, and `observeBirthCertificate(dkgSessionId, parent)`
does the same. Neither can disagree with the ceremony the screen is showing,
because it is handed the same one.
In the view model they move into `observeForCeremony`, re-pointed when the
ceremony changes the way the message watcher already is -- previously the key-state
watcher was started once at init against a room, which is what let it drift from
the session on screen. `observeKeyState` keeps only the room-scoped watch of
signing sessions, which is genuinely a room question.
One test in `SignedGroupKeyStateTest`: a key state the group really signed is
reachable from the ceremony that produced it, and a ceremony with no key answers
null rather than throwing -- this flow runs from before a ceremony finishes.
397 common tests, 717 jvm tests, `m3Audit` meets every budget.
The two stuck subgroups will offer "create the subgroup" on the next build: their
key states are already signed and on file, and nothing about them has to be redone.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -92,22 +92,26 @@ class DatabaseDkgRepository(
|
||||
): Flow<List<LocalFrostSigningSession>> =
|
||||
database.frostSigningSessionDao().observeSessionsForChatRoom(chatRoomId)
|
||||
|
||||
override fun observeSignedGroupKeyState(
|
||||
chatRoomId: String,
|
||||
parentChatRoomId: String?,
|
||||
): Flow<GroupKeyState?> =
|
||||
override fun observeSignedGroupKeyState(dkgSessionId: String): Flow<GroupKeyState?> =
|
||||
database.groupSignedEventDao()
|
||||
.observeByKind(GroupKeyStateEvent.KIND)
|
||||
.map { signedEvents ->
|
||||
// The room the ceremony's key derives, resolved per emission
|
||||
// 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.
|
||||
val adminRoomId = database.dkgSessionDao()
|
||||
.getLatestSessionFor(chatRoomId, parentChatRoomId)
|
||||
//
|
||||
// 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() }
|
||||
|
||||
adminRoomId?.let { GroupKeyStateManager.stateAmong(signedEvents, it) }
|
||||
subjectRoomId?.let { GroupKeyStateManager.stateAmong(signedEvents, it) }
|
||||
}
|
||||
|
||||
override suspend fun adoptGroupKeyState(chatRoomId: String): GroupKeyState? = try {
|
||||
@@ -148,7 +152,7 @@ class DatabaseDkgRepository(
|
||||
}
|
||||
|
||||
override fun observeBirthCertificate(
|
||||
ceremonyChatRoomId: String,
|
||||
dkgSessionId: String,
|
||||
parentChatRoomId: String,
|
||||
): Flow<GroupSignedEvent?> =
|
||||
database.groupSignedEventDao()
|
||||
@@ -158,7 +162,7 @@ class DatabaseDkgRepository(
|
||||
// rather than once: the ceremony has no key until it finishes,
|
||||
// and this flow is running before it does.
|
||||
val subgroupChatRoomId = database.dkgSessionDao()
|
||||
.getLatestSessionFor(ceremonyChatRoomId, parentChatRoomId)
|
||||
.getSessionById(dkgSessionId)
|
||||
?.thresholdPublicKey
|
||||
?.let { runCatching { SharedKeyDerivation.marmotGroupId(it) }.getOrNull() }
|
||||
?: return@map null
|
||||
|
||||
@@ -111,10 +111,17 @@ interface DkgRepository {
|
||||
* changes is the moment there is a room to make -- and the state it is
|
||||
* about does not exist as a row until somebody makes one.
|
||||
*/
|
||||
fun observeSignedGroupKeyState(
|
||||
chatRoomId: String,
|
||||
parentChatRoomId: String? = null,
|
||||
): Flow<GroupKeyState?>
|
||||
/**
|
||||
* Whether the group has signed a key state for the room [dkgSessionId]'s key
|
||||
* derives.
|
||||
*
|
||||
* Named by ceremony rather than by room, because the room a ceremony runs in
|
||||
* can hold more than one and re-deriving "the room's ceremony" here would
|
||||
* answer about a different key than the screen is showing. That is what left
|
||||
* a finished subgroup with its key state signed, its certificate signed, and
|
||||
* no way to create the room -- the lookup found no ceremony at all.
|
||||
*/
|
||||
fun observeSignedGroupKeyState(dkgSessionId: String): Flow<GroupKeyState?>
|
||||
|
||||
/** Files the state the group signed for a room that now exists. */
|
||||
suspend fun adoptGroupKeyState(chatRoomId: String): GroupKeyState?
|
||||
@@ -142,7 +149,7 @@ interface DkgRepository {
|
||||
* the key state, which is itself signed before the room is created.
|
||||
*/
|
||||
fun observeBirthCertificate(
|
||||
ceremonyChatRoomId: String,
|
||||
dkgSessionId: String,
|
||||
parentChatRoomId: String,
|
||||
): Flow<GroupSignedEvent?>
|
||||
|
||||
@@ -200,10 +207,7 @@ interface DkgRepository {
|
||||
chatRoomId: String
|
||||
): Flow<List<LocalFrostSigningSession>> = flowOf(emptyList())
|
||||
|
||||
override fun observeSignedGroupKeyState(
|
||||
chatRoomId: String,
|
||||
parentChatRoomId: String?,
|
||||
): Flow<GroupKeyState?> =
|
||||
override fun observeSignedGroupKeyState(dkgSessionId: String): Flow<GroupKeyState?> =
|
||||
flowOf(null)
|
||||
|
||||
override suspend fun adoptGroupKeyState(chatRoomId: String): GroupKeyState? = null
|
||||
@@ -217,7 +221,7 @@ interface DkgRepository {
|
||||
): FrostSigningSession? = null
|
||||
|
||||
override fun observeBirthCertificate(
|
||||
ceremonyChatRoomId: String,
|
||||
dkgSessionId: String,
|
||||
parentChatRoomId: String,
|
||||
): Flow<GroupSignedEvent?> = flowOf(null)
|
||||
|
||||
|
||||
@@ -88,6 +88,9 @@ class DkgRitualViewModel(
|
||||
*/
|
||||
private var watchedParentChatRoomId: HexKey? = null
|
||||
|
||||
/** The ceremony [observeForCeremony] is currently watching. */
|
||||
private var watchedSessionId: String? = null
|
||||
|
||||
/**
|
||||
* The quorum the ritual will generate a key for. Pre-filled with the same
|
||||
* majority default the group-creation screen offers.
|
||||
@@ -186,12 +189,14 @@ class DkgRitualViewModel(
|
||||
pendingApproval = session?.let { dkgRepository.pendingApproval(it) }
|
||||
)
|
||||
|
||||
// Re-point the certificate watchers whenever the purpose changes,
|
||||
// which for a member arriving cold is once: from nothing to the
|
||||
// parent the proposal named.
|
||||
if (parent != watchedParentChatRoomId) {
|
||||
// Re-point the per-ceremony watchers whenever the ceremony or its
|
||||
// purpose changes, which for a member arriving cold is once.
|
||||
// Everything downstream of this -- the key state, the certificate
|
||||
// -- is about one ceremony, and the room cannot say which.
|
||||
if (session?.id != watchedSessionId || parent != watchedParentChatRoomId) {
|
||||
watchedSessionId = session?.id
|
||||
watchedParentChatRoomId = parent
|
||||
observeCertificate(parent)
|
||||
observeForCeremony(session?.id, parent)
|
||||
}
|
||||
|
||||
// Re-point the message watcher at whatever session is current. An
|
||||
@@ -259,21 +264,13 @@ class DkgRitualViewModel(
|
||||
}
|
||||
}
|
||||
|
||||
launch {
|
||||
dkgRepository.observeSignedGroupKeyState(chatRoomId)
|
||||
.collect { state ->
|
||||
val loaded = dkgRitualUIState as? DkgRitualUIState.Loaded ?: return@collect
|
||||
|
||||
dkgRitualUIState = loaded.copy(isKeyStateSigned = state != null)
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Follows the parent certifying this subgroup, once there is a parent to
|
||||
* follow.
|
||||
* Follows one ceremony: whether its key state has been signed, and -- for a
|
||||
* subgroup -- whether its parent has certified it.
|
||||
*
|
||||
* Off the *parent's* signed events and sessions rather than this room's: the
|
||||
* certificate is made where the parent's key can sign it, which is never the
|
||||
@@ -285,19 +282,38 @@ class DkgRitualViewModel(
|
||||
* message watcher is -- an ordinary ceremony passes null and gets nothing
|
||||
* watched, which is exactly right.
|
||||
*/
|
||||
private fun observeCertificate(parentChatRoomId: HexKey?) {
|
||||
private fun observeForCeremony(dkgSessionId: String?, parentChatRoomId: HexKey?) {
|
||||
certificateObserver?.cancel()
|
||||
|
||||
if (parentChatRoomId == null) {
|
||||
if (dkgSessionId == null) {
|
||||
dkgRitualUIState = (dkgRitualUIState as? DkgRitualUIState.Loaded)
|
||||
?.copy(isCertified = false, certificateSession = null)
|
||||
?.copy(isKeyStateSigned = false, isCertified = false, certificateSession = null)
|
||||
?: dkgRitualUIState
|
||||
return
|
||||
}
|
||||
|
||||
certificateObserver = viewModelScope.launch(Dispatchers.IO) {
|
||||
// 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
|
||||
// about this one's key.
|
||||
launch {
|
||||
dkgRepository.observeBirthCertificate(chatRoomId, parentChatRoomId)
|
||||
dkgRepository.observeSignedGroupKeyState(dkgSessionId).collect { state ->
|
||||
val loaded = dkgRitualUIState as? DkgRitualUIState.Loaded ?: return@collect
|
||||
|
||||
dkgRitualUIState = loaded.copy(isKeyStateSigned = state != null)
|
||||
}
|
||||
}
|
||||
|
||||
if (parentChatRoomId == null) {
|
||||
dkgRitualUIState = (dkgRitualUIState as? DkgRitualUIState.Loaded)
|
||||
?.copy(isCertified = false, certificateSession = null)
|
||||
?: dkgRitualUIState
|
||||
return@launch
|
||||
}
|
||||
|
||||
launch {
|
||||
dkgRepository.observeBirthCertificate(dkgSessionId, parentChatRoomId)
|
||||
.collect { certificate ->
|
||||
val loaded = dkgRitualUIState as? DkgRitualUIState.Loaded ?: return@collect
|
||||
|
||||
|
||||
@@ -25,10 +25,12 @@ import kotlin.test.assertNotNull
|
||||
import kotlin.test.assertNull
|
||||
import kotlin.test.assertTrue
|
||||
import kotlin.time.Instant
|
||||
import kotlinx.coroutines.flow.first
|
||||
import kotlinx.coroutines.runBlocking
|
||||
import press.mantra.compose.database.MantraDatabase
|
||||
import press.mantra.compose.database.model.ChatMessage
|
||||
import press.mantra.compose.database.builder.getRoomDatabase
|
||||
import press.mantra.compose.database.repository.DatabaseDkgRepository
|
||||
import press.mantra.compose.database.model.ChatRoom
|
||||
import press.mantra.compose.database.model.DkgParticipantMessage
|
||||
import press.mantra.compose.database.model.DkgSession
|
||||
@@ -1623,4 +1625,54 @@ class SignedGroupKeyStateTest {
|
||||
assertNull(state.birthCertificateJson)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The state two real subgroups were left in: everything signed, and no way to
|
||||
* create the room.
|
||||
*
|
||||
* "2.0" and "2.1" each finished a ChillDKG, got a birth certificate out of the
|
||||
* parent's quorum, and got their key state signed by their own. The coordinator
|
||||
* was then offered nothing, because the screen asked whether a key state was
|
||||
* signed **by room** -- and a subgroup's ceremony room holds only that
|
||||
* subgroup's ceremony, so the lookup for the room's *own* ceremony found
|
||||
* nothing, derived no key, and reported "not signed" over a state sitting in
|
||||
* the same database.
|
||||
*
|
||||
* Asked by ceremony it is answerable, and the answer does not depend on how
|
||||
* many ceremonies the room holds.
|
||||
*/
|
||||
@Test
|
||||
fun `a signed key state is found by the ceremony that produced it`() = runBlocking {
|
||||
val creator = device(members[0], signerIndex = 0)
|
||||
val other = device(members[1], signerIndex = 1)
|
||||
|
||||
val session = GroupKeyStateManager.propose(
|
||||
database = creator.db,
|
||||
localChatRoom = creator.room,
|
||||
userPublicKey = creator.publicKey,
|
||||
key = ceremonyOn(creator)
|
||||
)
|
||||
pump(creator, other)
|
||||
FrostSigningManager.approve(other.db, other.room, session.id)
|
||||
pump(creator, other)
|
||||
|
||||
assertNotNull(creator.keyState(), "the group signed it")
|
||||
|
||||
// What the screen asks, and what it used to ask. The ceremony knows which
|
||||
// key it made; the room it ran in does not.
|
||||
assertNotNull(
|
||||
DatabaseDkgRepository(creator.db, this)
|
||||
.observeSignedGroupKeyState(ceremonyId)
|
||||
.first(),
|
||||
"the key state has to be reachable from the ceremony that produced it",
|
||||
)
|
||||
|
||||
// And a ceremony that produced no key yet answers null rather than
|
||||
// throwing -- this flow runs from before the ceremony finishes.
|
||||
assertNull(
|
||||
DatabaseDkgRepository(creator.db, this)
|
||||
.observeSignedGroupKeyState("no-such-ceremony")
|
||||
.first(),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user