From 1cffd8db18d1e76fba576f2a7cf58944fe86609b Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 10 Jul 2026 22:29:41 +0000 Subject: [PATCH] fix: propagate relay acceptances into gift-wrap chains for chat relay icons Chat rows render the inner rumor note of a NIP-17 message, but relay attribution only landed there through narrow windows, so accepted relays often never showed as icons: - Duplicate deliveries were stranded on the wrap: a gift wrap re-delivered by a second relay hit the duplicate branch of consumeRegularEvent, which tagged the outer wrap note only and never re-processed the event. Extract the OK-path drilling (wrap -> seal -> rumor) into LocalCache.addRelayToNoteAndInners and call it from both the OK confirmation path (markAsSeen) and the duplicate EVENT path, replacing CacheClientConnector's private copy. - Cross-thread visibility: Note.event, Note.relays, Note.flowSet and the innerEventId of GiftWrapEvent/SealedRumorEvent are written by decrypt/index coroutines and read lock-free on relay socket threads; a stale read parks an acceptance on the outer envelope permanently. Mark them @Volatile. - Orphaned UI flows: RenderClosedRelayList/RenderAllRelayList and createMustShowExpandButtonFlows captured note.flow().relays.stateFlow once in remember/stateIn; MemoryTrimmingService.cleanObservers destroys the unobserved NoteFlowSet while the lifecycle is stopped, so resumed rows never saw another relay update. Wrap in a cold flow that re-resolves flow() on every collection start. - Indexing latency: sent DMs waited for the ~1s newEventBundles batcher before the self-wrap was unwrapped and the message reached the chatroom, parking early OKs on the wrap. broadcastPrivately and sendNip04PrivateMessage now run the EventProcessor on the freshly consumed note immediately; the batched re-delivery is idempotent. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01YEiq1NMK3q12KGQ2yYhPEp --- .../vitorpamplona/amethyst/model/Account.kt | 13 +++++ .../amethyst/model/LocalCache.kt | 30 +++++++++++- .../relayClient/CacheClientConnector.kt | 48 ++----------------- .../amethyst/ui/note/RelayListBox.kt | 15 +++--- .../ui/screen/loggedIn/AccountViewModel.kt | 10 ++-- .../amethyst/commons/model/Note.kt | 10 ++++ .../nip59Giftwrap/seals/SealedRumorEvent.kt | 4 ++ .../nip59Giftwrap/wraps/GiftWrapEvent.kt | 4 ++ 8 files changed, 77 insertions(+), 57 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt index b8f6eb22d3..91234aadf7 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt @@ -2521,6 +2521,11 @@ class Account( cache.justConsumeMyOwnEvent(newEvent) client.publish(newEvent, outboxRelays.flow.value + destinationRelays) + + // Index into the chatroom immediately (same rationale as + // broadcastPrivately) instead of waiting for the newEventBundles + // batcher; the later batched re-delivery is deduped by the chatroom. + cache.getNoteIfExists(newEvent.id)?.let { newNotesPreProcessor.consume(it) } } override suspend fun sendNip17EncryptedFile(template: EventTemplate) { @@ -2573,6 +2578,14 @@ class Account( val relayList = computeRelayListToBroadcast(wrap) client.publish(wrap, relayList) } + + // Unwrap and index the self-copy right away instead of waiting for the + // newEventBundles batcher (up to ~1s): the sent message reaches the + // chatroom before the first relay OK, so acceptances land directly on + // the rumor note the chat renders instead of parking on the wrap. The + // batcher re-delivers this note later; the processor's replay path and + // the chatroom add are both idempotent. + mineNote?.let { newNotesPreProcessor.consume(it) } } // --- Marmot Group Messaging --- diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/LocalCache.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/LocalCache.kt index 09bbdc0e5f..c2473ad474 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/LocalCache.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/LocalCache.kt @@ -237,6 +237,7 @@ import com.vitorpamplona.quartz.nip58Badges.accepted.AcceptedBadgeSetEvent import com.vitorpamplona.quartz.nip58Badges.award.BadgeAwardEvent import com.vitorpamplona.quartz.nip58Badges.definition.BadgeDefinitionEvent import com.vitorpamplona.quartz.nip58Badges.profile.ProfileBadgesEvent +import com.vitorpamplona.quartz.nip59Giftwrap.HasInnerEvent import com.vitorpamplona.quartz.nip59Giftwrap.seals.SealedRumorEvent import com.vitorpamplona.quartz.nip59Giftwrap.wraps.GiftWrapEvent import com.vitorpamplona.quartz.nip5aStaticWebsites.NamedSiteEvent @@ -807,7 +808,11 @@ object LocalCache : ILocalCache, ICacheProvider { if (relay != null) { author.addRelayBeingUsed(relay, event.createdAt) - note.addRelay(relay) + // A gift wrap re-delivered by another relay is a duplicate (returns + // false below and is never re-processed), so drill into the already + // unwrapped chain here — otherwise the relay never reaches the + // rumor note that the chat UI actually renders. + addRelayToNoteAndInners(note, relay) } // Already processed this event. @@ -3145,7 +3150,28 @@ object LocalCache : ILocalCache, ICacheProvider { } } - note?.addRelay(relay) + note?.let { addRelayToNoteAndInners(it, relay) } + } + + /** + * Adds [relay] to [note] and to every already-unwrapped inner note of its + * gift-wrap chain (wrap → seal → rumor). The chat UI renders the inner + * rumor, so a relay recorded only on the outer envelope never surfaces as + * an icon. Inner notes that don't exist yet are not lost: the unwrap path + * copies the envelope's relays down via [copyRelaysFromTo] when it runs. + */ + fun addRelayToNoteAndInners( + note: Note, + relay: NormalizedRelayUrl, + ) { + note.addRelay(relay) + + val noteEvent = note.event + if (noteEvent is HasInnerEvent) { + noteEvent.innerEventId?.let { innerId -> + getNoteIfExists(innerId)?.let { addRelayToNoteAndInners(it, relay) } + } + } } // Observers line up here. diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/CacheClientConnector.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/CacheClientConnector.kt index 6e03d26c7e..8a6e418b7e 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/CacheClientConnector.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/CacheClientConnector.kt @@ -21,14 +21,9 @@ package com.vitorpamplona.amethyst.service.relayClient import com.vitorpamplona.amethyst.model.LocalCache -import com.vitorpamplona.amethyst.model.Note -import com.vitorpamplona.quartz.nip01Core.core.HexKey import com.vitorpamplona.quartz.nip01Core.relay.client.INostrClient import com.vitorpamplona.quartz.nip01Core.relay.client.accessories.EventCollector import com.vitorpamplona.quartz.nip01Core.relay.client.accessories.RelayInsertConfirmationCollector -import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl -import com.vitorpamplona.quartz.nip59Giftwrap.seals.SealedRumorEvent -import com.vitorpamplona.quartz.nip59Giftwrap.wraps.GiftWrapEvent class CacheClientConnector( val client: INostrClient, @@ -39,53 +34,16 @@ class CacheClientConnector( cache.justConsume(event, relay, false) } + // markAsSeen drills into the gift-wrap chain (wrap → seal → rumor) via + // LocalCache.addRelayToNoteAndInners, so an OK acceptance for a wrap also + // tags the inner rumor note the chat UI renders. val confirmationWatcher = RelayInsertConfirmationCollector(client) { eventId, relay -> cache.markAsSeen(eventId, relay.url) - markAsSeen(eventId, relay.url) } fun destroy() { receiver.destroy() confirmationWatcher.destroy() } - - private fun markAsSeen( - eventId: HexKey, - info: NormalizedRelayUrl, - ) { - val note = LocalCache.getNoteIfExists(eventId) - if (note != null) { - note.addRelay(info) - markAsSeenInner(note, info) - } - } - - private fun markAsSeenInner( - note: Note, - info: NormalizedRelayUrl, - ) { - val noteEvent = note.event - if (noteEvent is GiftWrapEvent) { - val innerEvent = noteEvent.innerEventId - if (innerEvent != null) { - val innerNote = cache.getNoteIfExists(innerEvent) - if (innerNote != null) { - innerNote.addRelay(info) - markAsSeenInner(innerNote, info) - } - } - } - - if (noteEvent is SealedRumorEvent) { - val innerEvent = noteEvent.innerEventId - if (innerEvent != null) { - val innerNote = cache.getNoteIfExists(innerEvent) - if (innerNote != null) { - innerNote.addRelay(info) - markAsSeenInner(innerNote, info) - } - } - } - } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/RelayListBox.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/RelayListBox.kt index 8a3d5551ab..960c887c81 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/RelayListBox.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/RelayListBox.kt @@ -62,6 +62,8 @@ import com.vitorpamplona.amethyst.ui.theme.placeholderText import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl import kotlinx.coroutines.FlowPreview import kotlinx.coroutines.flow.distinctUntilChanged +import kotlinx.coroutines.flow.emitAll +import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.sample @@ -94,11 +96,13 @@ fun RenderAllRelayList( accountViewModel: AccountViewModel, nav: INav, ) { + // Cold wrapper: `flow()` must be re-resolved on every collection start. + // A memory trim destroys the NoteFlowSet while the lifecycle is stopped; + // a stateFlow captured in remember would then be orphaned and never see + // another relay update. val flow = remember(baseNote) { - baseNote - .flow() - .relays.stateFlow + flow { emitAll(baseNote.flow().relays.stateFlow) } .sample(500) .map { it.note.relays } .distinctUntilChanged() @@ -122,11 +126,10 @@ fun RenderClosedRelayList( accountViewModel: AccountViewModel, nav: INav, ) { + // Cold wrapper for the same trim-survival reason as RenderAllRelayList. val flow = remember(baseNote) { - baseNote - .flow() - .relays.stateFlow + flow { emitAll(baseNote.flow().relays.stateFlow) } .sample(500) .map { it.note.relays.take(3) } .distinctUntilChanged() diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt index 9eb163a241..5bbe77ea39 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt @@ -188,7 +188,9 @@ import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.combineTransform +import kotlinx.coroutines.flow.emitAll import kotlinx.coroutines.flow.flatMapLatest +import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.flowOn import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.onStart @@ -652,10 +654,10 @@ class AccountViewModel( fun createMustShowExpandButtonFlows(note: Note): StateFlow = noteMustShowExpandButtonFlows.get(note) - ?: note - .flow() - .relays - .stateFlow + // Cold wrapper: WhileSubscribed drops the upstream when idle and a + // memory trim may destroy the NoteFlowSet in between; re-resolving + // `flow()` on every restart keeps this cached StateFlow alive. + ?: flow { emitAll(note.flow().relays.stateFlow) } .map { it.note.relays.size > 3 } .flowOn(Dispatchers.IO) .stateIn( diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/Note.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/Note.kt index bb5a045ee5..61cec122e3 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/Note.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/Note.kt @@ -115,6 +115,10 @@ open class Note( // These fields are only available after the Text Note event is received. // They are immutable after that. + // `@Volatile`: written by the decrypt/index pipeline (IO coroutines) and + // read by the relay socket thread (OK confirmations drilling into the + // gift-wrap chain) — a stale read strands a relay on the outer wrap. + @Volatile var event: Event? = null var author: User? = null var replyTo: List? = null @@ -248,6 +252,9 @@ open class Note( var zapPayments = mapOf() private set + // `@Volatile`: written under [syncLock] but read lock-free from the relay + // socket thread and from [LocalCache.copyRelaysFromTo] on IO coroutines. + @Volatile var relays = listOf() private set @@ -1321,6 +1328,9 @@ open class Note( return false } + // `@Volatile`: created/destroyed under [syncLock] but read lock-free by + // every `flowSet?.x?.invalidateData()` call on writer threads. + @Volatile var flowSet: NoteFlowSet? = null fun createOrDestroyFlowSync(create: Boolean) = diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip59Giftwrap/seals/SealedRumorEvent.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip59Giftwrap/seals/SealedRumorEvent.kt index 4dfbf62037..d37a771984 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip59Giftwrap/seals/SealedRumorEvent.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip59Giftwrap/seals/SealedRumorEvent.kt @@ -29,6 +29,7 @@ import com.vitorpamplona.quartz.nip59Giftwrap.HasInnerEvent import com.vitorpamplona.quartz.nip59Giftwrap.rumors.Rumor import com.vitorpamplona.quartz.utils.Log import com.vitorpamplona.quartz.utils.TimeUtils +import kotlin.concurrent.Volatile @Immutable class SealedRumorEvent( @@ -40,8 +41,11 @@ class SealedRumorEvent( sig: HexKey, ) : Event(id, pubKey, createdAt, KIND, tags, content, sig), HasInnerEvent { + // `@Volatile`: set by the decrypting coroutine in [unsealThrowing], read + // by relay socket threads walking the wrap → seal → rumor chain. @kotlinx.serialization.Transient @kotlin.jvm.Transient + @Volatile override var innerEventId: HexKey? = null fun copyNoContent(): SealedRumorEvent { diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip59Giftwrap/wraps/GiftWrapEvent.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip59Giftwrap/wraps/GiftWrapEvent.kt index 881e344cd2..ad9c91bb67 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip59Giftwrap/wraps/GiftWrapEvent.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip59Giftwrap/wraps/GiftWrapEvent.kt @@ -34,6 +34,7 @@ import com.vitorpamplona.quartz.nip40Expiration.ExpirationTag import com.vitorpamplona.quartz.nip59Giftwrap.HasInnerEvent import com.vitorpamplona.quartz.utils.Log import com.vitorpamplona.quartz.utils.TimeUtils +import kotlin.concurrent.Volatile @Immutable open class GiftWrapEvent( @@ -46,8 +47,11 @@ open class GiftWrapEvent( kind: Int = KIND, ) : Event(id, pubKey, createdAt, kind, tags, content, sig), HasInnerEvent { + // `@Volatile`: set by the decrypting coroutine in [unwrapThrowing], read + // by relay socket threads walking the wrap → seal → rumor chain. @kotlinx.serialization.Transient @kotlin.jvm.Transient + @Volatile override var innerEventId: HexKey? = null open fun copyNoContent(): GiftWrapEvent {