From 65182ac8920bc2a69f6f8f12bf3cfa98c9274d01 Mon Sep 17 00:00:00 2001 From: Kgothatso Ngako Date: Sat, 29 Aug 2026 18:38:39 +0200 Subject: [PATCH] refactor: route every MLS group restore through ChatRoom.toMlsGroup() Four places restored an MlsGroup from a chat room's persisted state, each spelling out the same MlsGroup.restore(MlsGroupState.decodeTls(hex)) chain by hand. Three of them could not have called the shared helper even if they wanted to: until ea8b2b2, ChatRoom.toMlsGroup() demanded a pastGenerations count so it could replay the sender ratchet, which is a question none of these callers had any business answering. Dropping that parameter left the helper callable everywhere, so call it everywhere. database/repository/DatabaseChatRepository.kt * sendChatMessage() restores via toMlsGroup(). It only wants to know whether the room is MLS-backed or gift-wrapped, which is exactly what a null return says. ui/view/model/AddArtifactViewModel.kt * Same collapse: the ?.let { restore(...) }?.let { mlsGroup -> ... } double-let becomes toMlsGroup()?.let { mlsGroup -> ... }. database/dao/NostrDao.kt * The fourth copy, and the one easiest to miss: it decoded the hex into a mlsGroupStateByteArray temp and then restored under an explicit null check on the bytes, rather than the ?.let the others used. Same operation wearing different clothes. Now toMlsGroup() with a null check on the group itself, which is what the branch actually meant. None of these three encrypt, so none of them ever needed the generation replay the old signature forced on them -- NostrDao is the inbound path, AddArtifactViewModel only gates on the group existing, and sendChatMessage writes a rumor for the outbound pipeline to encrypt later. The only caller that does encrypt, encryptAndSendMarmotInnerEvent, persists mlsGroup.saveState() immediately afterwards, so the ratchet position now round-trips through storage on its own. Unused imports go with them: MlsGroup and MlsGroupState from the first two files, MlsGroupState from NostrDao (which still needs MlsGroup for processWelcome). MlsGroup.restore now appears exactly once in the codebase, inside toMlsGroup() itself. Verified: ./gradlew :composeApp:compileCommonMainKotlinMetadata ./gradlew :composeApp:compileDebugKotlinAndroid --rerun-tasks Co-Authored-By: Claude Opus 5 --- .../press/mantra/compose/database/dao/NostrDao.kt | 11 ++--------- .../database/repository/DatabaseChatRepository.kt | 10 +--------- .../compose/ui/view/model/AddArtifactViewModel.kt | 10 +--------- 3 files changed, 4 insertions(+), 27 deletions(-) diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/database/dao/NostrDao.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/database/dao/NostrDao.kt index fd8cc712..9f6f6b09 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/database/dao/NostrDao.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/database/dao/NostrDao.kt @@ -36,7 +36,6 @@ import com.vitorpamplona.quartz.marmot.mip00KeyPackages.KeyPackageRelayListEvent import com.vitorpamplona.quartz.marmot.mip02Welcome.WelcomeEvent import com.vitorpamplona.quartz.marmot.mls.codec.TlsReader import com.vitorpamplona.quartz.marmot.mls.group.MlsGroup -import com.vitorpamplona.quartz.marmot.mls.group.MlsGroupState import com.vitorpamplona.quartz.marmot.mls.messages.KeyPackageBundle import com.vitorpamplona.quartz.marmot.mls.messages.MlsKeyPackage import com.vitorpamplona.quartz.marmot.mls.tree.Credential @@ -382,15 +381,9 @@ abstract class NostrDao( val localChatRoom = database.chatRoomDao().findChatRoomById(chatRoomId) if (localChatRoom != null) { - val mlsGroupStateByteArray = localChatRoom.chatRoom.mlsGroupState?.hexToByteArray() - - if (mlsGroupStateByteArray != null) { - val mlsGroup = MlsGroup.restore( - MlsGroupState.decodeTls( - mlsGroupStateByteArray - ) - ) + val mlsGroup = localChatRoom.chatRoom.toMlsGroup() + if (mlsGroup != null) { val memberPubkeys = mlsGroup.members().mapNotNull { (leafIndex, leafNode) -> val pubkey = when (val cred = leafNode.credential) { diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/database/repository/DatabaseChatRepository.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/database/repository/DatabaseChatRepository.kt index 9ca036b7..fa1955df 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/database/repository/DatabaseChatRepository.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/database/repository/DatabaseChatRepository.kt @@ -12,8 +12,6 @@ import press.mantra.compose.database.model.intermdiate.LocalChatMessage import press.mantra.compose.database.model.intermdiate.LocalChatRoom import co.touchlab.kermit.Logger import com.vitorpamplona.quartz.marmot.mip02Welcome.WelcomeEvent -import com.vitorpamplona.quartz.marmot.mls.group.MlsGroup -import com.vitorpamplona.quartz.marmot.mls.group.MlsGroupState import com.vitorpamplona.quartz.nip01Core.core.HexKey import com.vitorpamplona.quartz.nip01Core.core.Kind import com.vitorpamplona.quartz.nip01Core.crypto.EventHasher @@ -153,13 +151,7 @@ class DatabaseChatRepository( messageType: Kind ) { logger.i("sendChatMessage($text): $localChatRoom") - val mlsGroup = localChatRoom.chatRoom.mlsGroupState?.let { mlsGroupState -> - MlsGroup.restore( - MlsGroupState.decodeTls( - mlsGroupState.hexToByteArray() - ) - ) - } + val mlsGroup = localChatRoom.chatRoom.toMlsGroup() if (mlsGroup != null) { // Create ChatRumor... diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/AddArtifactViewModel.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/AddArtifactViewModel.kt index 87ea4f26..a84ec5a8 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/AddArtifactViewModel.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/AddArtifactViewModel.kt @@ -14,8 +14,6 @@ import androidx.lifecycle.viewmodel.viewModelFactory import press.mantra.compose.repository.ChatRepository import press.mantra.compose.repository.NostrRepository import co.touchlab.kermit.Logger -import com.vitorpamplona.quartz.marmot.mls.group.MlsGroup -import com.vitorpamplona.quartz.marmot.mls.group.MlsGroupState import com.vitorpamplona.quartz.nip01Core.core.HexKey import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.IO @@ -73,13 +71,7 @@ class AddArtifactViewModel( onSuccess: (artifactId: String) -> Unit, onFailure: () -> Unit ) { - localChatRoom.chatRoom.mlsGroupState?.let { mlsGroupState -> - MlsGroup.restore( - MlsGroupState.decodeTls( - mlsGroupState.hexToByteArray() - ) - ) - }?.let { mlsGroup -> + localChatRoom.chatRoom.toMlsGroup()?.let { mlsGroup -> val name = nameField.text.toString() val url = urlField.text.toString() val versionLabel = versionLabelField.text.toString()