fix(subgroups): open the ceremony a transcript line is about, not the room's newest

Diagnosed off the four connected devices. Three subgroup ceremonies of "one
(#admins)", all opened correctly by the coordinator, all sitting at
COLLECTING_HOST_KEYS with 1 of 3 host keys in. Nothing was dropped: every
participant device held the session, the ceremony room, its members and the
"your approval is needed" line. What none of them could do was reach it.

**A room does not have *a* ceremony, and every version of this screen has assumed
it does.** A subgroup whose admins are the whole group runs in the room the
group's own ceremony ran in -- that is the point of `1930d6aa`, and it is what the
devices did. Ask that room for its ceremony and you get whichever was opened last.
On the devices that was the group's own, already COMPLETE, in five of the six
room/device pairs; the subgroup's request for a host key sat underneath it,
unanswered, with the screen showing a finished ceremony and nothing to do.

Both previous attempts were the same mistake:

- unscoped "newest in room" -- picks the wrong ceremony whenever the other one is
  newer, which is what shipped and what the devices show;
- scoped by purpose (`1930d6aa`) -- invisible to every member who did not open the
  subgroup, since only the coordinator's route carries a parent (`98626a9e`).

Neither ordering can be right, because the question is wrong.

**A line knows which ceremony it is about; the room does not.** So `ChatMessage`
gains `dkgSessionId`, the pair to `frostSigningSessionId` and added for the same
reason one-at-a-time stopped being true -- `docs/frost-batch-signing.md` reached
this conclusion for signing sessions already, and the ceremony half was left on
the clock because "a room runs one at a time". It doesn't any more.

Every DKG line is stamped in `announce`, which all of them already funnel through.
`DkgRitualRoute`, the three approval routes and their screens carry the id, the
transcript's ritual notice passes the tapped line's, and `DkgRitualViewModel`
observes that session when given one and the room's newest otherwise -- which is
still the best a caller holding only a room can do, and is what the group's
details entry passes.

`ChatMessage.isAbout` uses it too, so `answeredRequests` stops crediting an answer
given to one ceremony as an answer to the other. Rows written before the column
read back null and fall back to the clock, which is correct for them: nothing that
predates subgroups ran two ceremonies in one room.

Schema 18, one nullable column, `AutoMigration(17, 18)`.

One test in `RobustRoomKeyCeremonyTest`: with two ceremonies in one room, every
line names one of them and a subgroup line resolves to the subgroup's ceremony. It
deliberately does not assert which the room's "newest" is -- two ceremonies opened
in the same second tie on `createdAt`, and the point is that nothing relies on
that ordering any more.

397 common tests, 716 jvm tests, `m3Audit` meets every budget.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Kgothatso Ngako
2026-09-09 01:25:12 +02:00
parent 98626a9ef9
commit 0b65d7025b
20 changed files with 5858 additions and 19 deletions

File diff suppressed because it is too large Load Diff

View File

@@ -177,7 +177,7 @@ val GENESIS_AT = Instant.fromEpochMilliseconds(1231006505000L)
UnsignedNostrEvent::class,
Zap::class
],
version = 17,
version = 18,
autoMigrations = [
// v2 only adds the DkgSession/DkgParticipantMessage tables, so Room can
// generate the migration itself — nothing existing changes shape.
@@ -288,6 +288,16 @@ val GENESIS_AT = Instant.fromEpochMilliseconds(1231006505000L)
// parent, so a dangling value is the normal state. Adding nullable
// columns is a shape Room migrates itself.
AutoMigration(from = 16, to = 17),
// v18 adds the nullable ChatMessage.dkgSessionId, the pair to
// frostSigningSessionId, because "the room's ceremony" stopped naming one
// thing. A subgroup whose admins are the whole group runs in the room the
// group's own ceremony ran in, so a transcript line has to say which of
// the two it is about -- otherwise a member tapping "your approval is
// needed" is shown whichever ceremony is newest, which is routinely the
// wrong one. Rows written before it read back null and fall back to the
// clock, which is correct for them: nothing that predates subgroups ran
// two ceremonies in one room.
AutoMigration(from = 17, to = 18),
]
)
@ColumnTypeConverters(MantraConverters::class)

View File

@@ -122,6 +122,24 @@ data class ChatMessage(
*/
val frostSigningSessionId: String? = null,
/**
* The ceremony a [DKG_TYPES] line belongs to, or null for anything else.
*
* The pair to [frostSigningSessionId], and added for the same reason one
* ceremony at a time stopped being true. A room's ceremony room is derived
* from its members, so a subgroup whose admins are the whole group runs in
* the room the group's own ceremony ran in -- and then "the room's ceremony"
* names two things. A screen opened from one of these lines has to reach the
* ceremony the line is *about*, not the newest one in the room, or a member
* asked to join a subgroup is shown a ceremony that finished last week and
* told there is nothing to do.
*
* Null on every line written before this column existed, which the readers
* below fall back to the clock for. Correct for those rooms too: nothing that
* predates subgroups ran two ceremonies at once.
*/
val dkgSessionId: String? = null,
override val createdAt: Instant = Clock.System.now(),
override val updatedAt: Instant = createdAt,
override val savedAt: Instant = Clock.System.now(),
@@ -521,12 +539,19 @@ data class ChatMessage(
* and it is what a line written before [frostSigningSessionId] existed
* has to be read by.
*/
private fun ChatMessage.isAbout(request: ChatMessage): Boolean =
if (frostSigningSessionId != null && request.frostSigningSessionId != null) {
private fun ChatMessage.isAbout(request: ChatMessage): Boolean = when {
frostSigningSessionId != null && request.frostSigningSessionId != null ->
frostSigningSessionId == request.frostSigningSessionId
} else {
createdAt >= request.createdAt
}
// Ceremonies say which one they are now too. The clock used to be
// right here because a room ran one at a time; a room that holds a
// group's own ceremony and a subgroup's would otherwise credit an
// answer given to one as an answer to the other.
dkgSessionId != null && request.dkgSessionId != null ->
dkgSessionId == request.dkgSessionId
else -> createdAt >= request.createdAt
}
/**
* The signing requests that are no longer open, by row id.

View File

@@ -28,6 +28,9 @@ class DatabaseDkgRepository(
): DkgRepository {
private val logger = Logger.withTag(TAG)
override fun observeSessionById(sessionId: String): Flow<DkgSession?> =
database.dkgSessionDao().observeSessionById(sessionId)
override fun observeLatestSessionForChatRoom(
chatRoomId: String,
parentChatRoomId: String?,

View File

@@ -1095,7 +1095,11 @@ object ChillDkgRitualManager {
marmotInnerEventId = null,
chatRoomId = session.chatRoomId,
content = content,
messageType = messageType
messageType = messageType,
// Which ceremony this line is about. A room can hold more than
// one -- see `ChatMessage.dkgSessionId` -- and the transcript is
// what a member taps to reach the one that wants them.
dkgSessionId = session.id
)
)
}

View File

@@ -36,6 +36,14 @@ interface DkgRepository {
fun observeMessages(sessionId: String): Flow<List<DkgParticipantMessage>>
/**
* One named ceremony, however many the room holds.
*
* What a transcript line opens: the line knows which ceremony it is about,
* and the room does not.
*/
fun observeSessionById(sessionId: String): Flow<DkgSession?>
suspend fun getLatestSessionForChatRoom(chatRoomId: String): DkgSession?
/**
@@ -161,6 +169,8 @@ interface DkgRepository {
override fun observeMessages(sessionId: String): Flow<List<DkgParticipantMessage>> = flowOf(emptyList())
override fun observeSessionById(sessionId: String): Flow<DkgSession?> = flowOf(null)
override suspend fun getLatestSessionForChatRoom(chatRoomId: String): DkgSession? = null
override suspend fun proposeRitual(

View File

@@ -161,11 +161,12 @@ fun ChatRoomMessagingScreen(
key(true) {
ChatTranscript(
viewModel = chatMessageListViewModel,
onOpenSharedKey = {
onOpenSharedKey = { dkgSessionId ->
onNavigateToRoute.invoke(
DkgRitualRoute(
activeUserPublicKey = activeUserPublicKey,
chatRoomId = chatRoomId
chatRoomId = chatRoomId,
dkgSessionId = dkgSessionId
)
)
},

View File

@@ -64,6 +64,7 @@ internal fun DkgApprovalScaffold(
approveLabel: String,
activeUserPublicKey: HexKey,
chatRoomId: String,
dkgSessionId: String? = null,
activeWalletStateFlow: StateFlow<ActiveWallet?>,
chatRepository: ChatRepository,
dkgRepository: DkgRepository,
@@ -73,6 +74,7 @@ internal fun DkgApprovalScaffold(
val dkgRitualViewModel: DkgRitualViewModel = viewModel(
factory = DkgRitualViewModel.factory(
chatRoomId = chatRoomId,
dkgSessionId = dkgSessionId,
activeUserPublicKey = activeUserPublicKey,
activeWalletStateFlow = activeWalletStateFlow,
chatRepository = chatRepository,

View File

@@ -32,6 +32,7 @@ import press.mantra.compose.ui.theme.spacing
fun DkgJoinApprovalScreen(
activeUserPublicKey: HexKey,
chatRoomId: String,
dkgSessionId: String? = null,
activeWalletStateFlow: StateFlow<ActiveWallet?>,
chatRepository: ChatRepository,
dkgRepository: DkgRepository,
@@ -43,6 +44,7 @@ fun DkgJoinApprovalScreen(
approveLabel = "Join the ceremony",
activeUserPublicKey = activeUserPublicKey,
chatRoomId = chatRoomId,
dkgSessionId = dkgSessionId,
activeWalletStateFlow = activeWalletStateFlow,
chatRepository = chatRepository,
dkgRepository = dkgRepository,

View File

@@ -137,6 +137,8 @@ fun DkgRitualScreen(
* rung: the parent's birth certificate, between the key and the key state.
*/
parentChatRoomId: String? = null,
/** Which of the room's ceremonies to show; null takes the newest. */
dkgSessionId: String? = null,
initialDkgRitualUIState: DkgRitualUIState = DkgRitualUIState.Loading,
activeWalletStateFlow: StateFlow<ActiveWallet?>,
chatRepository: ChatRepository,
@@ -148,6 +150,7 @@ fun DkgRitualScreen(
chatRoomId = chatRoomId,
activeUserPublicKey = activeUserPublicKey,
parentChatRoomId = parentChatRoomId,
dkgSessionId = dkgSessionId,
initialDkgRitualUIState = initialDkgRitualUIState,
activeWalletStateFlow = activeWalletStateFlow,
chatRepository = chatRepository,
@@ -200,15 +203,18 @@ fun DkgRitualScreen(
when (pending) {
DkgApprovalStep.HOST_KEY -> DkgJoinApprovalRoute(
activeUserPublicKey = activeUserPublicKey,
chatRoomId = chatRoomId
chatRoomId = chatRoomId,
dkgSessionId = dkgRitualUIState.session?.id
)
DkgApprovalStep.ROUND_1 -> DkgRound1ApprovalRoute(
activeUserPublicKey = activeUserPublicKey,
chatRoomId = chatRoomId
chatRoomId = chatRoomId,
dkgSessionId = dkgRitualUIState.session?.id
)
DkgApprovalStep.ROUND_2 -> DkgRound2ApprovalRoute(
activeUserPublicKey = activeUserPublicKey,
chatRoomId = chatRoomId
chatRoomId = chatRoomId,
dkgSessionId = dkgRitualUIState.session?.id
)
}
)

View File

@@ -36,6 +36,7 @@ import mantra.composeapp.generated.resources.you
fun DkgRound1ApprovalScreen(
activeUserPublicKey: HexKey,
chatRoomId: String,
dkgSessionId: String? = null,
activeWalletStateFlow: StateFlow<ActiveWallet?>,
chatRepository: ChatRepository,
dkgRepository: DkgRepository,
@@ -47,6 +48,7 @@ fun DkgRound1ApprovalScreen(
approveLabel = "Contribute",
activeUserPublicKey = activeUserPublicKey,
chatRoomId = chatRoomId,
dkgSessionId = dkgSessionId,
activeWalletStateFlow = activeWalletStateFlow,
chatRepository = chatRepository,
dkgRepository = dkgRepository,

View File

@@ -37,6 +37,7 @@ import mantra.composeapp.generated.resources.this_is_the_last_thing_the_ceremony
fun DkgRound2ApprovalScreen(
activeUserPublicKey: HexKey,
chatRoomId: String,
dkgSessionId: String? = null,
activeWalletStateFlow: StateFlow<ActiveWallet?>,
chatRepository: ChatRepository,
dkgRepository: DkgRepository,
@@ -48,6 +49,7 @@ fun DkgRound2ApprovalScreen(
approveLabel = "Confirm",
activeUserPublicKey = activeUserPublicKey,
chatRoomId = chatRoomId,
dkgSessionId = dkgSessionId,
activeWalletStateFlow = activeWalletStateFlow,
chatRepository = chatRepository,
dkgRepository = dkgRepository,

View File

@@ -553,6 +553,7 @@ fun MantraNavHost(
activeUserPublicKey = route.activeUserPublicKey,
chatRoomId = route.chatRoomId,
parentChatRoomId = route.parentChatRoomId,
dkgSessionId = route.dkgSessionId,
activeWalletStateFlow = sovereignWalletViewModel.activeWalletInUI,
chatRepository = databaseChatRepository,
dkgRepository = databaseDkgRepository,
@@ -567,6 +568,7 @@ fun MantraNavHost(
DkgJoinApprovalScreen(
activeUserPublicKey = route.activeUserPublicKey,
chatRoomId = route.chatRoomId,
dkgSessionId = route.dkgSessionId,
activeWalletStateFlow = sovereignWalletViewModel.activeWalletInUI,
chatRepository = databaseChatRepository,
dkgRepository = databaseDkgRepository,
@@ -579,6 +581,7 @@ fun MantraNavHost(
DkgRound1ApprovalScreen(
activeUserPublicKey = route.activeUserPublicKey,
chatRoomId = route.chatRoomId,
dkgSessionId = route.dkgSessionId,
activeWalletStateFlow = sovereignWalletViewModel.activeWalletInUI,
chatRepository = databaseChatRepository,
dkgRepository = databaseDkgRepository,
@@ -591,6 +594,7 @@ fun MantraNavHost(
DkgRound2ApprovalScreen(
activeUserPublicKey = route.activeUserPublicKey,
chatRoomId = route.chatRoomId,
dkgSessionId = route.dkgSessionId,
activeWalletStateFlow = sovereignWalletViewModel.activeWalletInUI,
chatRepository = databaseChatRepository,
dkgRepository = databaseDkgRepository,

View File

@@ -13,5 +13,7 @@ import kotlinx.serialization.Serializable
@Serializable
data class DkgJoinApprovalRoute(
val activeUserPublicKey: String,
val chatRoomId: String
val chatRoomId: String,
/** The ceremony being approved; null takes the room's newest. */
val dkgSessionId: String? = null
): Route()

View File

@@ -15,5 +15,15 @@ import kotlinx.serialization.Serializable
data class DkgRitualRoute(
val activeUserPublicKey: String,
val chatRoomId: String,
val parentChatRoomId: String? = null
val parentChatRoomId: String? = null,
/**
* Which of the room's ceremonies to show, when the caller knows.
*
* A room can hold more than one -- a subgroup whose admins are the whole
* group runs in the room the group's own ceremony ran in -- so "the room's
* ceremony" does not name one thing. A transcript line knows which it is
* about and passes it; a caller with only a room passes null and gets the
* newest, which is the best answer available to it.
*/
val dkgSessionId: String? = null
): Route()

View File

@@ -13,5 +13,7 @@ import kotlinx.serialization.Serializable
@Serializable
data class DkgRound1ApprovalRoute(
val activeUserPublicKey: String,
val chatRoomId: String
val chatRoomId: String,
/** The ceremony being approved; null takes the room's newest. */
val dkgSessionId: String? = null
): Route()

View File

@@ -13,5 +13,7 @@ import kotlinx.serialization.Serializable
@Serializable
data class DkgRound2ApprovalRoute(
val activeUserPublicKey: String,
val chatRoomId: String
val chatRoomId: String,
/** The ceremony being approved; null takes the room's newest. */
val dkgSessionId: String? = null
): Route()

View File

@@ -99,7 +99,7 @@ import press.mantra.compose.ui.composable.widgets.ErrorState
@Composable
fun ChatTranscript(
viewModel: ChatMessageListViewModel,
onOpenSharedKey: () -> Unit,
onOpenSharedKey: (dkgSessionId: String?) -> Unit,
/**
* Opens the signing this line is about, by session id -- or the room's
* whole list of proposals when the line predates [ChatMessage.frostSigningSessionId]
@@ -233,7 +233,13 @@ fun ChatTranscript(
localChatMessage = localChatMessage,
isAnswered = localChatMessage.chatMessage.id in answeredRequests,
isSettled = localChatMessage.chatMessage.id in settledRequests,
onClick = onOpenSharedKey
// The line knows which ceremony it is
// about; the room does not. Passing it is
// what stops a subgroup's request opening
// the group's own finished ceremony.
onClick = {
onOpenSharedKey(localChatMessage.chatMessage.dkgSessionId)
}
)
return@items
}

View File

@@ -55,6 +55,11 @@ class DkgRitualViewModel(
* one. Off the route, which got it off the proposal -- a claim either way.
*/
val parentChatRoomId: HexKey? = null,
/**
* The ceremony to show, when the caller knew which. Null falls back to the
* room's newest -- see [DkgRitualRoute.dkgSessionId].
*/
val dkgSessionId: String? = null,
initialDkgRitualUIState: DkgRitualUIState,
val activeWalletStateFlow: StateFlow<ActiveWallet?>,
val chatRepository: ChatRepository,
@@ -154,7 +159,16 @@ class DkgRitualViewModel(
// it. `getLatestSessionFor` is still scoped where scoping is the
// question being asked -- "may I open another" -- in `proposeRitual`
// and `refuseCeremonyRoom`.
dkgRepository.observeLatestSessionForChatRoom(chatRoomId).collect { session ->
// The named ceremony where the caller knew which, and the room's
// newest otherwise. A room holds more than one the moment a subgroup's
// admins are the whole group, and then "the room's newest" is whichever
// ceremony was opened last -- routinely the group's own, already
// finished, while the subgroup's request goes unanswered underneath it.
val sessions = dkgSessionId
?.let { dkgRepository.observeSessionById(it) }
?: dkgRepository.observeLatestSessionForChatRoom(chatRoomId)
sessions.collect { session ->
val loaded = (dkgRitualUIState as? DkgRitualUIState.Loaded)
?: DkgRitualUIState.Loaded(
localChatRoom = localChatRoom,
@@ -632,6 +646,7 @@ class DkgRitualViewModel(
chatRoomId: String,
activeUserPublicKey: HexKey,
parentChatRoomId: HexKey? = null,
dkgSessionId: String? = null,
initialDkgRitualUIState: DkgRitualUIState = DkgRitualUIState.Loading,
activeWalletStateFlow: StateFlow<ActiveWallet?>,
chatRepository: ChatRepository,
@@ -642,6 +657,7 @@ class DkgRitualViewModel(
chatRoomId = chatRoomId,
activeUserPublicKey = activeUserPublicKey,
parentChatRoomId = parentChatRoomId,
dkgSessionId = dkgSessionId,
initialDkgRitualUIState = initialDkgRitualUIState,
activeWalletStateFlow = activeWalletStateFlow,
chatRepository = chatRepository,

View File

@@ -369,4 +369,56 @@ class RobustRoomKeyCeremonyTest {
assertEquals(subgroup.id, openCeremony(room, parentChatRoomId = parent).id)
assertEquals(own.id, openCeremony(room).id)
}
/**
* The failure the connected devices actually showed.
*
* A subgroup whose admins are the whole group runs in the room the group's
* own ceremony ran in. Ask that room for "its ceremony" and you get whichever
* was opened last -- on four real devices that was the group's own, already
* finished, while the subgroup's request for a host key sat underneath it
* unanswered. Every participant had the session, the room and the approval
* line, and the screen showed them a ceremony with nothing to do.
*
* Neither ordering fixes it, because the question is wrong: a room does not
* have *a* ceremony. The line a member taps knows which one it is about, so
* that is what the screen is opened with.
*/
@Test
fun `a transcript line names the ceremony it belongs to`() = runBlocking {
val room = robustRoom()
val parent = "ab".repeat(32)
val subgroup = openCeremony(room, parentChatRoomId = parent)
val own = openCeremony(room)
// Asking the room hands back one of the two and cannot say which is
// wanted -- on the devices it was the group's own, already finished,
// while the subgroup's request sat underneath it. Not asserted on here
// because two ceremonies opened in the same second tie on `createdAt`;
// what matters is that nothing has to rely on that ordering at all.
assertNotEquals(own.id, subgroup.id)
// Every line says which ceremony it is about, so the subgroup's request
// opens the subgroup's ceremony however many the room holds.
val lines = chatMessages(room)
assertTrue(
lines.any { it.dkgSessionId == subgroup.id },
"the subgroup's ceremony has to have written lines of its own",
)
assertTrue(lines.any { it.dkgSessionId == own.id })
assertTrue(
lines.all { it.dkgSessionId != null },
"a line with no ceremony on it is a line that cannot be opened",
)
lines.filter { it.dkgSessionId == subgroup.id }.forEach {
assertEquals(
subgroup.id,
db.dkgSessionDao().observeSessionById(it.dkgSessionId!!).first()?.id,
"tapping a subgroup line reaches the subgroup's ceremony",
)
}
}
}