diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupChatView.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupChatView.kt index 2ffe74d75a..1c4575065d 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupChatView.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupChatView.kt @@ -129,7 +129,7 @@ fun MarmotGroupChatView( feedContentState = feedViewModel.feedState, accountViewModel = accountViewModel, nav = nav, - routeForLastRead = "MarmotGroup/$nostrGroupId", + routeForLastRead = marmotGroupLastReadRoute(nostrGroupId), onWantsToReply = { note -> newMessageModel.reply(note) }, onWantsToEditDraft = { }, ) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupLastRead.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupLastRead.kt new file mode 100644 index 0000000000..252fb79e8a --- /dev/null +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupLastRead.kt @@ -0,0 +1,31 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.marmotGroup + +import com.vitorpamplona.quartz.nip01Core.core.HexKey + +/** + * Key into the account's persisted last-read map (`lastReadPerRoute`) for a + * Marmot MLS group. The mark-as-read side (the open chat's feed) and the + * unread indicators (Messages screen row, group list row) must use the + * identical key, or read state silently stops persisting for one of them. + */ +fun marmotGroupLastReadRoute(nostrGroupId: HexKey): String = "MarmotGroup/$nostrGroupId" diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupListScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupListScreen.kt index 7eff984dbf..1a4201cdb4 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupListScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/marmotGroup/MarmotGroupListScreen.kt @@ -226,11 +226,12 @@ fun MarmotGroupListItem( val memberPubkeys = remember(members) { members.map { it.pubkey } } val newestMessage = chatroom.newestMessage - val lastReadTime by accountViewModel.account.loadLastReadFlow("MarmotGroup/$groupId").collectAsStateWithLifecycle() - val unread = - remember(newestMessage, lastReadTime) { - chatroom.messages.count { (it.createdAt() ?: Long.MIN_VALUE) > lastReadTime } - } + val lastReadTime by accountViewModel.account.loadLastReadFlow(marmotGroupLastReadRoute(groupId)).collectAsStateWithLifecycle() + // Not remembered: chatroom.messages can shrink without newestMessage or + // lastReadTime changing (pruning, kind:5 deletion of an older message), + // so caching on those keys would serve a stale count. The set is pruned + // to ~100 entries, so counting per recomposition is cheap. + val unread = chatroom.messages.count { (it.createdAt() ?: Long.MIN_VALUE) > lastReadTime } Row( modifier = diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/rooms/ChatroomHeaderCompose.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/rooms/ChatroomHeaderCompose.kt index e09ef9f793..66409b41a9 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/rooms/ChatroomHeaderCompose.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/chats/rooms/ChatroomHeaderCompose.kt @@ -63,6 +63,7 @@ import com.vitorpamplona.amethyst.ui.note.ObserveDraftEvent import com.vitorpamplona.amethyst.ui.note.elements.TimeAgoStyle import com.vitorpamplona.amethyst.ui.note.elements.ToggleableTimeAgoText import com.vitorpamplona.amethyst.ui.screen.loggedIn.AccountViewModel +import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.marmotGroup.marmotGroupLastReadRoute import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.privateDM.header.RoomNameDisplay import com.vitorpamplona.amethyst.ui.screen.loggedIn.chats.publicChannels.ephemChat.LoadEphemeralChatChannel import com.vitorpamplona.amethyst.ui.stringRes @@ -273,7 +274,7 @@ private fun MarmotGroupRoomCompose( stringRes(R.string.marmot_group_no_messages_yet) } - val lastReadTime by accountViewModel.account.loadLastReadFlow("MarmotGroup/${chatroom.nostrGroupId}").collectAsStateWithLifecycle() + val lastReadTime by accountViewModel.account.loadLastReadFlow(marmotGroupLastReadRoute(chatroom.nostrGroupId)).collectAsStateWithLifecycle() ChannelName( channelIdHex = chatroom.nostrGroupId, diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt index 70afbc664d..aabe40ffe0 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManager.kt @@ -51,6 +51,7 @@ import com.vitorpamplona.quartz.nip01Core.tags.people.pTags import com.vitorpamplona.quartz.nip18Reposts.quotes.QEventTag import com.vitorpamplona.quartz.nip18Reposts.quotes.quote import com.vitorpamplona.quartz.utils.Log +import com.vitorpamplona.quartz.utils.TimeUtils import kotlin.io.encoding.Base64 import kotlin.io.encoding.ExperimentalEncodingApi @@ -124,7 +125,7 @@ class MarmotManager( if (messageStore == null) return emptyMap() val result = mutableMapOf() for (groupId in groupIds) { - val newest = + val newestStored = loadStoredMessages(groupId).maxOfOrNull { json -> try { Event.fromJson(json).createdAt @@ -133,6 +134,11 @@ class MarmotManager( 0L } } ?: continue + // Clamp at wall-clock now: inner createdAt is sender-controlled, + // and a single future-dated message must not push `since` past + // the present — that would skip genuinely new events on every + // restart until a fresher message arrives. + val newest = minOf(newestStored, TimeUtils.now()) if (newest > GROUP_EVENT_REFETCH_OVERLAP_SEC) { result[groupId] = newest - GROUP_EVENT_REFETCH_OVERLAP_SEC } @@ -755,7 +761,7 @@ class MarmotManager( * are timestamped at the same send, so the window only needs to * absorb relay/system clock skew and out-of-order publishes. */ - internal const val GROUP_EVENT_REFETCH_OVERLAP_SEC = 24L * 60 * 60 + internal val GROUP_EVENT_REFETCH_OVERLAP_SEC: Long = TimeUtils.ONE_DAY.toLong() } } diff --git a/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManagerRestoreTest.kt b/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManagerRestoreTest.kt index 894a6190c3..a923b2ec31 100644 --- a/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManagerRestoreTest.kt +++ b/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/marmot/MarmotManagerRestoreTest.kt @@ -27,10 +27,13 @@ import com.vitorpamplona.quartz.marmot.mls.group.MlsGroupStateStore import com.vitorpamplona.quartz.nip01Core.crypto.KeyPair import com.vitorpamplona.quartz.nip01Core.relay.normalizer.RelayUrlNormalizer import com.vitorpamplona.quartz.nip01Core.signers.NostrSignerInternal +import com.vitorpamplona.quartz.utils.TimeUtils import kotlinx.coroutines.runBlocking import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertNotNull import kotlin.test.assertNull +import kotlin.test.assertTrue /** * Restart behavior of [MarmotManager.restoreAll]. @@ -84,6 +87,50 @@ class MarmotManagerRestoreTest { } } + @Test + fun testRestoreAllClampsFutureDatedMessagesAtNow() { + runBlocking { + val signer = NostrSignerInternal(KeyPair()) + val mlsStore = InMemoryStateStore() + val messageStore = InMemoryMessageStore() + val kpStore = InMemoryBundleStore() + + val manager = MarmotManager(signer, mlsStore, messageStore, kpStore) + manager.createGroup( + nostrGroupId, + MarmotGroupData( + nostrGroupId = nostrGroupId, + name = "restore-clamp", + relays = listOf(relay.url), + ), + ) + // The inner createdAt is sender-controlled: a single future-dated + // message must not push the seeded `since` past wall-clock now, + // or the filter would skip genuinely new events after restart. + val futureCreatedAt = TimeUtils.now() + 30L * 24 * 60 * 60 + val futureJson = + """{"id":"${"c".repeat(64)}","pubkey":"${"d".repeat(64)}","created_at":$futureCreatedAt,"kind":9,"tags":[],"content":"from the future","sig":""}""" + manager.persistDecryptedMessage(nostrGroupId, futureJson) + + val beforeRestore = TimeUtils.now() + val restarted = MarmotManager(signer, mlsStore, messageStore, kpStore) + restarted.restoreAll() + val afterRestore = TimeUtils.now() + + val since = + restarted.subscriptionManager + .activeGroupFilters() + .single() + .since + assertNotNull(since, "a future-dated message must still seed a clamped since") + assertTrue( + since >= beforeRestore - MarmotManager.GROUP_EVENT_REFETCH_OVERLAP_SEC && + since <= afterRestore - MarmotManager.GROUP_EVENT_REFETCH_OVERLAP_SEC, + "since must be clamped to now - overlap, was $since", + ) + } + } + @Test fun testRestoreAllLeavesSinceUnsetWithoutStoredMessages() { runBlocking {