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 6bc6fde0..9496c6a2 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 @@ -737,16 +737,23 @@ abstract class NostrDao( participants.forEach { participant -> val giftWrapParticipantProfile = database.profileDao().getProfileByPublicKey(participant.participantPublicKey) - if (giftWrapParticipantProfile == null) { - // Save a placeholder - database.profileDao().insertPlaceholderProfile( - Profile( - displayName = "LOADING...", - publicKey = participant.participantPublicKey, - createdAt = GENESIS_AT, - nostrEventId = nostrEvent.id, // Will get overwriting by sync, + // A placeholder row is not a profile -- it is the + // "LOADING..." name waiting on a kind:0 -- so a member we + // already have one for (seen in another room, say) needs + // asking about just as much as one we have never seen. + // Only a resolved profile takes this member off the list. + if (giftWrapParticipantProfile == null || giftWrapParticipantProfile.createdAt == GENESIS_AT) { + if (giftWrapParticipantProfile == null) { + // Save a placeholder + database.profileDao().insertPlaceholderProfile( + Profile( + displayName = "LOADING...", + publicKey = participant.participantPublicKey, + createdAt = GENESIS_AT, + nostrEventId = nostrEvent.id, // Will get overwriting by sync, + ) ) - ) + } val recommendRelayUrl = giftWrapMessage.receiverRelayHit diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/managers/LiveSubscriptionManager.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/managers/LiveSubscriptionManager.kt index 7627fcfc..aa65b08d 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/managers/LiveSubscriptionManager.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/managers/LiveSubscriptionManager.kt @@ -34,6 +34,7 @@ import press.mantra.compose.network.relays.LiveSubscriptionTransport import press.mantra.compose.network.relays.isRelayBackPressure import press.mantra.compose.network.sockets.NostrIncomingMessage import press.mantra.compose.network.sockets.endsLiveSubscription +import press.mantra.compose.nostr.MemberProfileSync import press.mantra.compose.nostr.Nip17Filters import press.mantra.compose.nostr.Relays import press.mantra.compose.repository.ChatRepository @@ -249,7 +250,8 @@ class LiveSubscriptionManager( * guarantee. */ private suspend fun queueCatchUpSynchronization(publicKey: HexKey) { - val groupIds = liveGroupIds(publicKey) + val rooms = chatRepository.getChatRoomListByPublicKey(publicKey) + val groupIds = groupIdsFrom(rooms) val giftWrapFilter = Nip17Filters.inbox(publicKey) @@ -285,13 +287,20 @@ class LiveSubscriptionManager( ) } - logger.i("Queueing ${requests.size} catch-up reconciliation(s) over ${groupIds.size} group(s)") + // The members none of our rooms can name yet. Asked for here, where every room + // this account has is already in hand, because the ask made where a placeholder + // is first minted happens once and a Marmot group gives it no second chance -- + // see [MemberProfileSync]. + val unnamedMembers = MemberProfileSync.unresolvedMemberPublicKeys(rooms, publicKey) + requests += MemberProfileSync.requests(unnamedMembers) + + logger.i( + "Queueing ${requests.size} catch-up reconciliation(s) over ${groupIds.size} group(s)" + + " and ${unnamedMembers.size} unnamed member(s)" + ) nostrRepository.queueNegentropySynchronizeRequest(requests) } - private suspend fun liveGroupIds(publicKey: HexKey): List = - groupIdsFrom(chatRepository.getChatRoomListByPublicKey(publicKey)) - /** * Keeps the group subscriptions matching the groups we are actually in. * diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/managers/MarmotInboundManager.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/managers/MarmotInboundManager.kt index d278c66a..55efd3a7 100644 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/managers/MarmotInboundManager.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/managers/MarmotInboundManager.kt @@ -160,6 +160,13 @@ object MarmotInboundManager { logger.d("New Participant: ${newParticipant.participantPublicKey}") val profile = database.profileDao().getProfileByPublicKey(newParticipant.participantPublicKey) + // A placeholder row is not a profile: it is the "LOADING..." name and a + // GENESIS_AT stamp, standing in until a kind:0 arrives. So a member we + // already hold one for still needs asking about -- one removed and added + // again, or one whose last ask came back empty, would otherwise keep the + // placeholder for the life of the room. Only a resolved profile is done. + if (profile != null && profile.createdAt > GENESIS_AT) return@forEach + if (profile == null) { // Create placeholder profile... val nostrEvent = database.nostrEventDao().getNostrEvents( @@ -168,45 +175,43 @@ object MarmotInboundManager { limit = 1 ) - val placeholderProfile = Profile( - displayName = "LOADING...", - publicKey = newParticipant.participantPublicKey, - createdAt = GENESIS_AT, - nostrEventId = nostrEvent.first().id, // Will hopefully get overwriting by sync, - ) - database.profileDao().insertPlaceholderProfile( - placeholderProfile - ) - - // Schedule sync for profile... - val synchronizationFilter = SynchronizationFilter( - authors = arrayOf( - placeholderProfile.publicKey - ), - kinds = arrayOf( - MetadataEvent.KIND, - ChatMessageRelayListEvent.KIND, - KeyPackageEvent.KIND, - KeyPackageRelayListEvent.KIND, + Profile( + displayName = "LOADING...", + publicKey = newParticipant.participantPublicKey, + createdAt = GENESIS_AT, + nostrEventId = nostrEvent.first().id, // Will hopefully get overwriting by sync, ) ) - - database.negentropySynchronizeRequestDao().insert( - Relays.DefaultDMRelayList.map { relayUrl -> - NegentropySynchronizeRequest( - id = NegentropySynchronizeRequest.computeId( - relayURL = relayUrl.url, - synchronizationFilter - ), - purpose = "synchronization", - synchronizationFilter = synchronizationFilter, - relayURL = relayUrl.url, - level = 0 - ) - } - ) } + + // Schedule sync for profile... + val synchronizationFilter = SynchronizationFilter( + authors = arrayOf( + newParticipant.participantPublicKey + ), + kinds = arrayOf( + MetadataEvent.KIND, + ChatMessageRelayListEvent.KIND, + KeyPackageEvent.KIND, + KeyPackageRelayListEvent.KIND, + ) + ) + + database.negentropySynchronizeRequestDao().insert( + Relays.DefaultDMRelayList.map { relayUrl -> + NegentropySynchronizeRequest( + id = NegentropySynchronizeRequest.computeId( + relayURL = relayUrl.url, + synchronizationFilter + ), + purpose = "synchronization", + synchronizationFilter = synchronizationFilter, + relayURL = relayUrl.url, + level = 0 + ) + } + ) } // Handle membership changes... database.participantDao().upsert(removedAdmins) diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/nostr/MemberProfileSync.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/nostr/MemberProfileSync.kt new file mode 100644 index 00000000..f245e9a6 --- /dev/null +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/nostr/MemberProfileSync.kt @@ -0,0 +1,93 @@ +package press.mantra.compose.nostr + +import com.vitorpamplona.quartz.nip01Core.core.HexKey +import com.vitorpamplona.quartz.nip01Core.metadata.MetadataEvent +import press.mantra.compose.database.GENESIS_AT +import press.mantra.compose.database.model.NegentropySynchronizeRequest +import press.mantra.compose.database.model.intermdiate.LocalChatRoom +import press.mantra.compose.database.model.types.SynchronizationFilter + +/** + * Asking the relays who a room's members are, for the members this device cannot name. + * + * A pubkey gets a Profile row the moment it is first seen, and that row is a + * placeholder: the "LOADING..." display name, stamped [GENESIS_AT] so it is + * distinguishable from a profile actually read off a kind:0. What replaces it is a + * kind:0, and every path that mints a placeholder asks for one -- exactly once, on + * whichever relay that pubkey happened to arrive on. + * + * One ask is enough for a NIP-17 room only by accident. Opening one fetches each + * member's kind:10050, and indexing any event authored by a pubkey we hold a + * placeholder for re-queues the full profile sync (see `NostrDao.indexNostrEvent`), + * so the name still arrives on the second bounce when the first ask came back empty. + * + * A Marmot group gets no such bounce. Its members never author anything this device + * indexes -- a kind:445 is signed by a throwaway key and the sender's real identity + * lives inside the MLS frame -- so the single ask made at the Welcome, or at the + * commit that added them, is the only one there will ever be. Lose it to a relay that + * did not hold their kind:0 yet and that member reads "LOADING..." for as long as the + * room does. + * + * Hence asking again from what is on screen, rather than only from what just arrived. + */ +object MemberProfileSync { + + /** Tells these requests apart in the queue and in the logs. */ + const val PURPOSE = "member-profiles" + + /** + * Authors per request. A relay is entitled to refuse a filter it considers too + * large, and one over-long request being refused takes every member in it down + * with it. + */ + const val MAX_AUTHORS_PER_REQUEST = 100 + + /** + * The members of [rooms] this device still cannot put a name to. + * + * A missing row and a placeholder row mean the same thing here: nobody has read a + * kind:0 for that pubkey. Ourselves excluded -- our own profile is not something a + * relay teaches us. + */ + fun unresolvedMemberPublicKeys( + rooms: List, + activeUserPublicKey: HexKey, + ): List = + rooms + .flatMap { it.localParticipants } + .filter { participant -> participant.profile?.let { it.createdAt > GENESIS_AT } != true } + .map { it.participant.participantPublicKey } + .filter { it != activeUserPublicKey } + .distinct() + .sorted() + + /** + * One kind:0 request per DM relay per chunk of [publicKeys], and nothing at all for + * nobody. + * + * Level 0 on purpose. It is what marks a request as one somebody is waiting on, and + * it is what lets an arriving kind:0 pull the rest of a member with it: indexing it + * finds the placeholder still in place and queues that member's DM relay list and + * key packages off the back of it. + */ + fun requests(publicKeys: List): List = + publicKeys.chunked(MAX_AUTHORS_PER_REQUEST).flatMap { chunk -> + val synchronizationFilter = SynchronizationFilter( + kinds = arrayOf(MetadataEvent.KIND), + authors = chunk.toTypedArray(), + ) + + Relays.DefaultDMRelayList.map { relay -> + NegentropySynchronizeRequest( + id = NegentropySynchronizeRequest.computeId( + relayURL = relay.url, + synchronizationFilter = synchronizationFilter, + ), + purpose = PURPOSE, + synchronizationFilter = synchronizationFilter, + relayURL = relay.url, + level = 0, + ) + } + } +} diff --git a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/ChatMessageListViewModel.kt b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/ChatMessageListViewModel.kt index 817bd627..d8f1834b 100755 --- a/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/ChatMessageListViewModel.kt +++ b/composeApp/src/commonMain/kotlin/press/mantra/compose/ui/view/model/ChatMessageListViewModel.kt @@ -74,6 +74,7 @@ import press.mantra.compose.database.model.types.SynchronizationFilter import press.mantra.compose.extensions.shortened import press.mantra.compose.extensions.toFormattedTimeAndDateString import press.mantra.compose.managers.FrostSigningManager +import press.mantra.compose.nostr.MemberProfileSync import press.mantra.compose.nostr.Nip17Filters import press.mantra.compose.nostr.Relays import press.mantra.compose.repository.ChatRepository @@ -251,21 +252,39 @@ class ChatMessageListViewModel( } /** - * Finds out where this room's participants read their messages. + * Finds out who this room's members are, and where they read their messages. * * This used to schedule the room's message sync as well — gift wraps for a NIP-17 room, * group events for an MLS one. Both are now covered by the subscriptions * LiveSubscriptionManager holds open for the whole account, so opening a chat no longer * asks for its messages; they are already arriving. * - * What is left is discovery, not message sync: if we do not hold a participant's - * kind-10050 we cannot address a message to them, and that is worth resolving the moment - * a chat is opened rather than whenever a background pass gets to it. + * What is left is discovery, not message sync, and it is worth doing the moment a chat is + * opened rather than whenever a background pass gets to it: + * + * - Any member still showing as "LOADING..." — every room, not just NIP-17 ones. See + * [MemberProfileSync] for why a Marmot group's members otherwise stay that way: the + * only ask for their kind:0 was made once, when the Welcome or the commit that added + * them arrived, and nothing they subsequently do prompts another. + * - A NIP-17 participant's kind-10050, without which we cannot address a message to + * them at all. An MLS room needs none: its messages go to the group's own relays. */ fun scheduleSynchronization() { logger.d("scheduleSynchronization") viewModelScope.launch(Dispatchers.IO) { + val unnamedMembers = MemberProfileSync.unresolvedMemberPublicKeys( + rooms = listOf(localChatRoom), + activeUserPublicKey = localChatRoom.chatRoom.userPublicKey, + ) + + if (unnamedMembers.isNotEmpty()) { + logger.i("Members of ${localChatRoom.chatRoom.id} we cannot name yet: $unnamedMembers") + nostrRepository.queueNegentropySynchronizeRequest( + MemberProfileSync.requests(unnamedMembers) + ) + } + if (localChatRoom.chatRoom.mlsGroupState == null) { localChatRoom.localParticipants.filter { it.participant.participantPublicKey != localChatRoom.chatRoom.userPublicKey }.forEach { recipients -> diff --git a/composeApp/src/commonTest/kotlin/press/mantra/compose/nostr/MemberProfileSyncTest.kt b/composeApp/src/commonTest/kotlin/press/mantra/compose/nostr/MemberProfileSyncTest.kt new file mode 100644 index 00000000..b3ac1227 --- /dev/null +++ b/composeApp/src/commonTest/kotlin/press/mantra/compose/nostr/MemberProfileSyncTest.kt @@ -0,0 +1,151 @@ +package press.mantra.compose.nostr + +import com.vitorpamplona.quartz.nip01Core.core.HexKey +import com.vitorpamplona.quartz.nip01Core.metadata.MetadataEvent +import press.mantra.compose.database.GENESIS_AT +import press.mantra.compose.database.model.ChatRoom +import press.mantra.compose.database.model.Participant +import press.mantra.compose.database.model.Profile +import press.mantra.compose.database.model.intermdiate.LocalChatRoom +import press.mantra.compose.database.model.intermdiate.LocalParticipant +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue +import kotlin.time.Instant + +class MemberProfileSyncTest { + + private val me = "a".repeat(64) + private val alice = "b".repeat(64) + private val bob = "c".repeat(64) + + // --- who gets asked about ----------------------------------------------------------- + + /** + * The whole point. A member with no row at all and a member holding the "LOADING..." + * placeholder are in the same position — nobody has read a kind:0 for either — and the + * placeholder is the one that looks resolved if you only check for null. That is the + * check every mint site makes, which is why a Marmot group's members stay "LOADING..." + * once their single ask comes back empty. + */ + @Test + fun `a placeholder counts as unknown, the same as no row at all`() { + val rooms = listOf( + room( + members = listOf( + member(me, resolved(me)), + member(alice, placeholder(alice)), + member(bob, null), + ) + ) + ) + + assertEquals(listOf(alice, bob).sorted(), MemberProfileSync.unresolvedMemberPublicKeys(rooms, me)) + } + + @Test + fun `a member whose profile has arrived is not asked about again`() { + val rooms = listOf(room(members = listOf(member(me, resolved(me)), member(alice, resolved(alice))))) + + assertTrue(MemberProfileSync.unresolvedMemberPublicKeys(rooms, me).isEmpty()) + } + + /** + * Ours is the one profile a relay cannot teach us, and on a fresh device it is a + * placeholder like any other — so without this every foreground would ask the relays + * about us. + */ + @Test + fun `we are never asked about`() { + val rooms = listOf(room(members = listOf(member(me, placeholder(me))))) + + assertTrue(MemberProfileSync.unresolvedMemberPublicKeys(rooms, me).isEmpty()) + } + + /** Someone in three groups is still one kind:0. */ + @Test + fun `a member in several rooms is asked about once`() { + val rooms = listOf( + room(id = "one", members = listOf(member(alice, placeholder(alice)))), + room(id = "two", members = listOf(member(alice, null))), + ) + + assertEquals(listOf(alice), MemberProfileSync.unresolvedMemberPublicKeys(rooms, me)) + } + + // --- what gets asked ---------------------------------------------------------------- + + @Test + fun `nobody to ask about means no requests`() { + assertTrue(MemberProfileSync.requests(emptyList()).isEmpty()) + } + + @Test + fun `one request per relay, asking those authors for kind zero`() { + val requests = MemberProfileSync.requests(listOf(alice, bob)) + + assertEquals(Relays.DefaultDMRelayList.size, requests.size) + assertEquals(Relays.DefaultDMRelayList.map { it.url }.sorted(), requests.map { it.relayURL }.sorted()) + + requests.forEach { request -> + assertEquals(MemberProfileSync.PURPOSE, request.purpose) + // Level 0 is what marks a request as one somebody is waiting on, and what lets + // the arriving kind:0 pull the rest of the member in behind it. + assertEquals(0, request.level) + assertEquals(listOf(MetadataEvent.KIND), request.synchronizationFilter.kinds?.toList()) + assertEquals(listOf(alice, bob), request.synchronizationFilter.authors?.toList()) + } + } + + /** One over-long filter a relay refuses would take every member in it down with it. */ + @Test + fun `authors are chunked so one refused filter cannot lose everybody`() { + val many = (1..250).map { it.toString().padStart(64, '0') } + + val chunkSizes = MemberProfileSync.requests(many) + .filter { it.relayURL == Relays.DefaultDMRelayList.first().url } + .map { it.synchronizationFilter.authors?.size } + + assertEquals(listOf(100, 100, 50), chunkSizes) + } + + // --- fixtures ----------------------------------------------------------------------- + + private fun room( + id: String = "room", + members: List, + ) = LocalChatRoom( + chatRoom = ChatRoom( + id = id, + userPublicKey = me, + subject = null, + description = null, + mlsGroupState = "state", + ), + localParticipants = members, + ) + + private fun member(publicKey: HexKey, profile: Profile?) = LocalParticipant( + participant = Participant( + participantPublicKey = publicKey, + chatRoomId = "room", + relayHint = null, + ), + profile = profile, + ) + + /** What every mint site writes: the "LOADING..." name, stamped [GENESIS_AT]. */ + private fun placeholder(publicKey: HexKey) = Profile( + publicKey = publicKey, + displayName = "LOADING...", + nostrEventId = "event", + createdAt = GENESIS_AT, + ) + + private fun resolved(publicKey: HexKey) = Profile( + publicKey = publicKey, + displayName = "Somebody", + nostrEventId = "event", + createdAt = Instant.fromEpochSeconds(1_700_000_000), + ) +}