fix: create the #admins room only once every member can be added
Every member's key package is now resolved before anything is created. If one is missing the room is not created at all, and the coordinator is told which member to go and ask rather than being handed a room quietly short of people. Previously the room was created and then whoever could be added was added, with the rest collected into a list that only reached the log. Two things make that the wrong trade here, and neither applies to ordinary group creation: MarmotGroupData.adminPubkeys is baked into the epoch-0 GroupContext and names every member of the ceremony. A room created without one of them therefore lists an admin who is not in the MLS tree -- a group that disagrees with itself from its first epoch, and MIP-01 leans on that list for most group operations. And the id is derived from the shared key, so there is exactly one room per group at this path. A half-created one occupies that address permanently; unlike a random id there is no second one to retry with. Creating nothing leaves the retry clean. The lookup moves ahead of group creation, which also means the batched add now receives a list it knows is complete -- `addMembers` no longer has to reason about absent key packages on this path. `inviteAdmins` goes with it. Its job was resolving key packages and then adding whoever it could; the first half moved into the precondition and the second is a direct `addMembers` call. The blocked members surface as `DkgRitualUIState.adminGroupBlockedOn`, carrying names rather than public keys -- the action this prompts is asking a particular person to open the app, so a name is what the coordinator needs. Cleared when the button is pressed again, so a retry does not show the previous answer. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -253,6 +253,7 @@ fun DkgRitualScreen(
|
||||
round1Participants = dkgRitualUIState.round1Participants,
|
||||
round2Participants = dkgRitualUIState.round2Participants,
|
||||
isActionPending = isActionPending,
|
||||
adminGroupBlockedOn = dkgRitualUIState.adminGroupBlockedOn,
|
||||
onCreateAdminGroup = {
|
||||
dkgRitualViewModel.createAdminGroup(onNavigateToRoute)
|
||||
}
|
||||
@@ -297,6 +298,7 @@ private fun RitualProgress(
|
||||
round1Participants: Set<HexKey>,
|
||||
round2Participants: Set<HexKey>,
|
||||
isActionPending: Boolean,
|
||||
adminGroupBlockedOn: List<String>,
|
||||
onCreateAdminGroup: () -> Unit,
|
||||
) {
|
||||
val stage = session.stage
|
||||
@@ -450,6 +452,18 @@ private fun RitualProgress(
|
||||
// first, and the coordinator is the member the group already watched
|
||||
// do the work.
|
||||
if (session.isCoordinator()) {
|
||||
// Nothing is created until every member's key package is in
|
||||
// hand, so this is the whole reason there is no room yet.
|
||||
if (adminGroupBlockedOn.isNotEmpty()) {
|
||||
Text(
|
||||
text = "Waiting on ${adminGroupBlockedOn.joinToString(", ")} to " +
|
||||
"open the app, so their device can publish the key it needs " +
|
||||
"to be added.",
|
||||
style = MaterialTheme.typography.labelMedium,
|
||||
color = MaterialTheme.colorScheme.error
|
||||
)
|
||||
}
|
||||
|
||||
Button(
|
||||
onClick = { onCreateAdminGroup() },
|
||||
enabled = !isActionPending,
|
||||
|
||||
@@ -269,6 +269,7 @@ class DkgRitualViewModel(
|
||||
val name = "${loaded.localChatRoom.chatRoom.subject ?: "Group"} (#admins)"
|
||||
|
||||
isActionPending.value = true
|
||||
dkgRitualUIState = loaded.copy(adminGroupBlockedOn = emptyList())
|
||||
|
||||
viewModelScope.launch(Dispatchers.IO) {
|
||||
// Derived ids make this reachable twice -- a second tap, or another
|
||||
@@ -289,6 +290,52 @@ class DkgRitualViewModel(
|
||||
return@launch
|
||||
}
|
||||
|
||||
// Every key package, before anything exists. Two reasons this is a
|
||||
// precondition rather than a best effort:
|
||||
//
|
||||
// MarmotGroupData.adminPubkeys is baked into the epoch-0 GroupContext
|
||||
// and names every member, so a room created without one of them lists an
|
||||
// admin who is not in the MLS tree -- a group that disagrees with itself
|
||||
// from its first epoch.
|
||||
//
|
||||
// And the id is derived, so there is exactly one room per group at this
|
||||
// path. A half-created one occupies that address permanently; there is no
|
||||
// second id to retry with. Better to create nothing and say who is
|
||||
// missing.
|
||||
val peers = members.filterNot { it == activeUserPublicKey }
|
||||
val keyPackages = coroutineScope {
|
||||
peers.map { publicKey ->
|
||||
async {
|
||||
publicKey to withTimeoutOrNull(KEY_PACKAGE_LOOKUP_TIMEOUT) {
|
||||
chatRepository.observeMarmotKeyPackageForPublicKey(publicKey)
|
||||
.filterNotNull()
|
||||
.first()
|
||||
}
|
||||
}
|
||||
}.awaitAll()
|
||||
}
|
||||
|
||||
val missing = keyPackages.filter { it.second == null }.map { it.first }
|
||||
if (missing.isNotEmpty()) {
|
||||
logger.w("Not creating admin group $groupId: no key package for $missing")
|
||||
|
||||
isActionPending.value = false
|
||||
dkgRitualUIState = loaded.copy(
|
||||
adminGroupBlockedOn = missing.map { publicKey ->
|
||||
loaded.ritualMembers
|
||||
.firstOrNull { it.participant.participantPublicKey == publicKey }
|
||||
?.profile
|
||||
?.humanReadableNameOrPubkey()
|
||||
?: publicKey.take(12)
|
||||
}
|
||||
)
|
||||
return@launch
|
||||
}
|
||||
|
||||
val addable = keyPackages.mapNotNull { (publicKey, keyPackage) ->
|
||||
keyPackage?.let { publicKey to it }
|
||||
}
|
||||
|
||||
val relays = Relays.DefaultDMRelayList.map { it.url }
|
||||
|
||||
// Built directly rather than through MarmotGroupData.bootstrap, which
|
||||
@@ -330,7 +377,11 @@ class DkgRitualViewModel(
|
||||
return@launch
|
||||
}
|
||||
|
||||
val notAdded = inviteAdmins(groupId, members.filterNot { it == activeUserPublicKey })
|
||||
val notAdded = runCatching {
|
||||
chatRepository.addMembers(localChatRoom = localChatRoom, peers = addable)
|
||||
}.onFailure {
|
||||
logger.e("Failed to add members to admin group $groupId", it)
|
||||
}.getOrElse { addable.map { (publicKey, _) -> publicKey } }
|
||||
|
||||
isActionPending.value = false
|
||||
|
||||
@@ -359,55 +410,6 @@ class DkgRitualViewModel(
|
||||
* reported rather than treated as failure: a room with most of the group in it
|
||||
* is more useful than no room.
|
||||
*/
|
||||
/**
|
||||
* Adds every admin to the freshly created room in one commit, returning those
|
||||
* that could not be added.
|
||||
*
|
||||
* Batched rather than invited one at a time: the whole membership is known here,
|
||||
* and inviting sequentially creates an epoch per member, each of whose commits
|
||||
* races the previous member's Welcome. A member who loses that race is silently
|
||||
* stuck an epoch behind. See docs/marmot-membership.md.
|
||||
*
|
||||
* A Marmot invite still needs the invitee's published key package, so a member
|
||||
* who has never published one cannot be added and has to be invited later. That
|
||||
* is reported rather than treated as failure: a room with most of the group in
|
||||
* it is more useful than no room.
|
||||
*/
|
||||
private suspend fun inviteAdmins(groupId: String, peers: List<HexKey>): List<HexKey> {
|
||||
val keyPackages = coroutineScope {
|
||||
peers.map { publicKey ->
|
||||
async {
|
||||
publicKey to withTimeoutOrNull(KEY_PACKAGE_LOOKUP_TIMEOUT) {
|
||||
chatRepository.observeMarmotKeyPackageForPublicKey(publicKey)
|
||||
.filterNotNull()
|
||||
.first()
|
||||
}
|
||||
}
|
||||
}.awaitAll()
|
||||
}
|
||||
|
||||
val withoutKeyPackage = keyPackages.filter { it.second == null }.map { it.first }
|
||||
withoutKeyPackage.forEach { logger.w("No key package for $it; leaving them out of $groupId") }
|
||||
|
||||
val addable = keyPackages.mapNotNull { (publicKey, keyPackage) ->
|
||||
keyPackage?.let { publicKey to it }
|
||||
}
|
||||
if (addable.isEmpty()) return withoutKeyPackage
|
||||
|
||||
val localChatRoom = chatRepository.getChatRoomByIdentifier(groupId)
|
||||
if (localChatRoom == null) {
|
||||
logger.e("Admin group $groupId disappeared before its members were added")
|
||||
return peers
|
||||
}
|
||||
|
||||
val notAdded = runCatching {
|
||||
chatRepository.addMembers(localChatRoom = localChatRoom, peers = addable)
|
||||
}.onFailure {
|
||||
logger.e("Failed to add members to admin group $groupId", it)
|
||||
}.getOrElse { addable.map { (publicKey, _) -> publicKey } }
|
||||
|
||||
return withoutKeyPackage + notAdded
|
||||
}
|
||||
|
||||
override fun onCleared() {
|
||||
messageObserver?.cancel()
|
||||
|
||||
@@ -33,6 +33,15 @@ sealed interface DkgRitualUIState {
|
||||
* derive is this, which needs the stored messages as well as the row.
|
||||
*/
|
||||
val pendingApproval: DkgApprovalStep? = null,
|
||||
/**
|
||||
* Members whose key package the coordinator could not find, and so who stop
|
||||
* the #admins room being created at all.
|
||||
*
|
||||
* Names rather than keys, because the answer to this is to go and ask that
|
||||
* person to open the app. Empty when nothing is blocked, which is also the
|
||||
* state a retry starts from.
|
||||
*/
|
||||
val adminGroupBlockedOn: List<String> = emptyList(),
|
||||
): DkgRitualUIState {
|
||||
val hostKeyCount: Int get() = hostKeyParticipants.size
|
||||
val round1Count: Int get() = round1Participants.size
|
||||
|
||||
Reference in New Issue
Block a user