fix: block the relay before dismissing its prompts, and make the cache atomic

Follow-up to the Block Relay button, from a review of that change.

Dismiss-before-block. The button called blockRelay() fire-and-forget and then
dismissed every prompt from the relay. reportSignerErrors swallows a refused or
timed-out signature (ManuallyUnauthorizedException, TimedOutException,
CouldNotPerformException) with a log line and no toast, so rejecting the signer
prompt closed the dialog, left the relay unblocked, and gave the user nothing to
tell them so — and the prompts were in the dismissal set for good. The dismissal
now runs from a callback that only fires after account.blockRelay() returns;
leaving the prompt up is the feedback when it doesn't. This also shrinks the
race window, since sendMyPublicAndPrivateOutbox consumes the kind-10006 into
LocalCache synchronously before publishing.

Non-atomic cache mutations. NOTIFYs are filed from the relay's socket coroutine
while dismissals run from the UI, so addPaymentRequestIfNew's `value +=`
read-modify-write could drop one of two concurrent edits, and dismissAllFrom
read the pending set before updating it — a prompt arriving in between was
removed without ever being recorded as dismissed. Both now go through
update/getAndUpdate.

Also avoids a copy on a hot path in BlockedRelayFilteringClient: every REQ,
COUNT and publish went through filterKeys/minus whenever the block list was
non-empty, allocating a full copy of the targets just to reproduce them
unchanged. A blocked relay is by definition one the app has stopped aiming at,
so it now checks whether any target is actually blocked before copying. This
matters more now that blocking is one tap from the dialog rather than a trip to
the settings screen, so non-empty block lists become the norm.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01U2GsUAheZmAXv6vk4m7m9T

amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/notifyCommand/compose/DisplayNotifyMessages.kt

@@ -60,12 +60,19 @@ fun DisplayNotifyMessages(
             onBlockRelay =
                 if (accountViewModel.isWriteable()) {
                     {
-                        accountViewModel.blockRelay(request.relayUrl)
-                        // Every queued prompt from this relay goes with the block, not just the one
-                        // on screen: a paid relay files one NOTIFY per rejected AUTH, so dismissing
-                        // only [request] would immediately re-open the dialog for a relay the user
-                        // just asked us to stop talking to.
-                        requests.dismissAllFrom(request.relayUrl)
+                        accountViewModel.blockRelay(request.relayUrl) {
+                            // Only after the block is signed and published, never before: a refused
+                            // or timed-out signature is swallowed without a toast, so dismissing up
+                            // front would close the dialog on a relay that is still unblocked and
+                            // leave the user no sign that anything failed. Leaving the prompt up is
+                            // the feedback.
+                            //
+                            // Every queued prompt from this relay goes at once, not just the one on
+                            // screen: a paid relay files one NOTIFY per rejected AUTH, so dismissing
+                            // only [request] would immediately re-open the dialog for a relay the
+                            // user just asked us to stop talking to.
+                            requests.dismissAllFrom(request.relayUrl)
+                        }
                     }
                 } else {
                     null

amethyst/src/main/java/com/vitorpamplona/amethyst/service/relayClient/notifyCommand/model/NotifyRequestsCache.kt

@@ -23,6 +23,7 @@ package com.vitorpamplona.amethyst.service.relayClient.notifyCommand.model
 import androidx.compose.runtime.Stable
 import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl
 import kotlinx.coroutines.flow.MutableStateFlow
+import kotlinx.coroutines.flow.getAndUpdate
 import kotlinx.coroutines.flow.update
 
 @Stable
@@ -38,12 +39,12 @@ class NotifyRequestsCache {
     }
 
     fun addPaymentRequestIfNew(paymentRequest: NotifyRequest) {
-        if (
-            !this.transientPaymentRequests.value.contains(paymentRequest) &&
-            !this.transientPaymentRequestDismissals.value.contains(paymentRequest)
-        ) {
-            this.transientPaymentRequests.value += paymentRequest
-        }
+        if (this.transientPaymentRequestDismissals.value.contains(paymentRequest)) return
+
+        // `update` rather than `value +=`: NOTIFYs are filed from the relay's socket coroutine
+        // while dismissals run from the UI, and a plain read-modify-write silently drops one of
+        // two concurrent edits — either losing a prompt or resurrecting a dismissed one.
+        this.transientPaymentRequests.update { if (paymentRequest in it) it else it + paymentRequest }
     }
 
     fun dismissPaymentRequest(request: NotifyRequest) {
@@ -61,10 +62,14 @@ class NotifyRequestsCache {
      * relay the user just told us never to talk to again.
      */
     fun dismissAllFrom(relayUrl: NormalizedRelayUrl) {
-        val fromRelay = this.transientPaymentRequests.value.filterTo(mutableSetOf()) { it.relayUrl == relayUrl }
-        if (fromRelay.isEmpty()) return
+        // getAndUpdate so the drain and the snapshot of what was drained are one atomic step: a
+        // NOTIFY filed by the socket coroutine between a separate read and write would otherwise
+        // be dropped from the pending set without ever being recorded as dismissed.
+        val before = this.transientPaymentRequests.getAndUpdate { pending -> pending.filterNotTo(mutableSetOf()) { it.relayUrl == relayUrl } }
 
-        this.transientPaymentRequests.update { it - fromRelay }
-        this.transientPaymentRequestDismissals.update { it + fromRelay }
+        val dismissed = before.filterTo(mutableSetOf()) { it.relayUrl == relayUrl }
+        if (dismissed.isEmpty()) return
+
+        this.transientPaymentRequestDismissals.update { it + dismissed }
     }
 }

amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt

@@ -1919,7 +1919,22 @@ class AccountViewModel(
 
     fun unfollowRelayFeed(url: NormalizedRelayUrl) = launchSigner { account.unfollowRelayFeed(url) }
 
-    fun blockRelay(url: NormalizedRelayUrl) = launchSigner { account.blockRelay(url) }
+    /**
+     * Blocks [url], running [onBlocked] only once the kind-10006 has actually been signed and
+     * published.
+     *
+     * The ordering matters: [reportSignerErrors] swallows a refused or timed-out signature
+     * (ManuallyUnauthorizedException, TimedOutException, CouldNotPerformException) with nothing but
+     * a log line, so a caller that cleaned up before the block landed would leave the user with an
+     * unblocked relay, no feedback, and whatever UI state it tore down already gone.
+     */
+    fun blockRelay(
+        url: NormalizedRelayUrl,
+        onBlocked: () -> Unit = {},
+    ) = launchSigner {
+        account.blockRelay(url)
+        onBlocked()
+    }
 
     fun showWord(word: String) = launchSigner { account.showWord(word) }
 

amethyst/src/test/java/com/vitorpamplona/amethyst/service/relayClient/notifyCommand/model/NotifyRequestsCacheTest.kt

@@ -55,6 +55,23 @@ class NotifyRequestsCacheTest {
         assertTrue(cache.transientPaymentRequests.value.isEmpty())
     }
 
+    @Test
+    fun aConcurrentPromptFromTheSameRelayIsNotSilentlySwallowed() {
+        val cache = NotifyRequestsCache()
+        cache.addPaymentRequestIfNew("Pay up", paid)
+
+        // Stands in for a NOTIFY landing on the socket coroutine while the UI drains the relay:
+        // whatever survives the drain must still be reachable, never removed-but-unrecorded.
+        cache.dismissAllFrom(paid)
+        cache.addPaymentRequestIfNew("A different demand", paid)
+
+        val pending = cache.transientPaymentRequests.value
+        val dismissed = cache.transientPaymentRequestDismissals.value
+
+        assertEquals(setOf(NotifyRequest(paid, "Pay up")), dismissed)
+        assertEquals(setOf(NotifyRequest(paid, "A different demand")), pending)
+    }
+
     @Test
     fun dismissingARelayWithNoPromptsChangesNothing() {
         val cache = NotifyRequestsCache()

commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/relayClient/BlockedRelayFilteringClient.kt

@@ -73,12 +73,25 @@ class BlockedRelayFilteringClient(
         relayList: Set<NormalizedRelayUrl>,
     ) {
         val blocked = blockedRelays()
-        delegate.publish(event, if (blocked.isEmpty()) relayList else relayList - blocked)
+        delegate.publish(event, if (blocked.hitsNoneOf(relayList)) relayList else relayList - blocked)
     }
 
     private fun Map<NormalizedRelayUrl, List<Filter>>.withoutBlocked(): Map<NormalizedRelayUrl, List<Filter>> {
         val blocked = blockedRelays()
-        if (blocked.isEmpty()) return this
+        if (blocked.hitsNoneOf(keys)) return this
         return filterKeys { it !in blocked }
     }
+
+    /**
+     * Whether none of the blocked relays appear in [targets] — i.e. whether the caller can be
+     * handed its own collection back untouched.
+     *
+     * Worth the extra scan because this runs on every REQ, COUNT and publish (filter assemblers
+     * rebuild their per-relay targets constantly) while a blocked relay is, by definition, one the
+     * app has stopped aiming at — so "the block list is non-empty but irrelevant to this call" is
+     * the overwhelmingly common case. Without the check, `filterKeys` / `minus` allocate and copy
+     * the whole collection every time just to reproduce it unchanged. A hash lookup per target and
+     * no allocation beats an allocation plus a full copy.
+     */
+    private fun Set<NormalizedRelayUrl>.hitsNoneOf(targets: Collection<NormalizedRelayUrl>): Boolean = isEmpty() || targets.none { it in this }
 }

commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/relayClient/BlockedRelayFilteringClientTest.kt

@@ -28,6 +28,7 @@ import com.vitorpamplona.quartz.nip01Core.relay.filters.Filter
 import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl
 import kotlin.test.Test
 import kotlin.test.assertEquals
+import kotlin.test.assertSame
 
 class BlockedRelayFilteringClientTest {
     private val good = NormalizedRelayUrl("wss://good.example/")
@@ -133,6 +134,24 @@ class BlockedRelayFilteringClientTest {
         assertEquals(emptySet(), inner.publishedRelays)
     }
 
+    @Test
+    fun aBlockListThatTouchesNothingHereIsPassedThroughWithoutCopying() {
+        val inner = RecordingClient()
+        // Non-empty block list, but none of it is aimed at on this call — the common case once the
+        // user has blocked anything at all.
+        val client = BlockedRelayFilteringClient(inner) { setOf(blocked) }
+
+        val filters = mapOf(good to listOf(Filter()), alsoGood to listOf(Filter()))
+        val relays = setOf(good, alsoGood)
+        client.subscribe("sub", filters, null)
+        client.publish(event(), relays)
+
+        // Identity, not just equality: reproducing an unchanged map/set costs an allocation and a
+        // full copy on a path that runs on every REQ and publish.
+        assertSame(filters, inner.subscribedFilters)
+        assertSame(relays, inner.publishedRelays)
+    }
+
     @Test
     fun blockSetIsReadPerCallSoLaterChangesApply() {
         val inner = RecordingClient()