Merge pull request #4039 from vitorpamplona/claude/notify-block-relay-button-e6amne

Add relay blocking to NOTIFY payment prompts

amethyst/src/main/java/com/vitorpamplona/amethyst/model/Account.kt

@@ -3533,6 +3533,15 @@ class Account(
 
     suspend fun saveBlockedRelayList(blockedRelays: List<NormalizedRelayUrl>) = sendMyPublicAndPrivateOutbox(blockedRelayList.saveRelayList(blockedRelays))
 
+    /**
+     * Blocks a single relay, leaving the rest of the kind-10006 list alone.
+     *
+     * Once published, [com.vitorpamplona.amethyst.commons.relayClient.BlockedRelayFilteringClient]
+     * strips the relay from every REQ, COUNT and publish, so the pool drops the socket as soon as
+     * the subscriptions that wanted it are recomputed.
+     */
+    suspend fun blockRelay(relay: NormalizedRelayUrl) = sendMyPublicAndPrivateOutbox(blockedRelayList.addRelay(relay))
+
     /**
      * Returns all known signed replaceable events that configure this account
      * (profile, contact list, relay lists, mute list, bookmarks, etc.). Events

amethyst/src/main/java/com/vitorpamplona/amethyst/model/nip51Lists/blockedRelays/BlockedRelayListState.kt

@@ -73,6 +73,20 @@ class BlockedRelayListState(
                 emptySet(),
             )
 
+    /**
+     * Adds [relay] to the kind-10006 list, keeping whatever is already there.
+     *
+     * Callers that only want to block one relay (the NOTIFY prompt's "Block Relay" button, for
+     * instance) must not rebuild the list from a snapshot they captured earlier: the list is
+     * shared across clients and may have grown since. Reading the current note here keeps the
+     * add additive.
+     */
+    suspend fun addRelay(relay: NormalizedRelayUrl): BlockedRelayListEvent {
+        val current = normalizeBlockedRelayListWithBackup(blockedListNote).toMutableList()
+        if (relay !in current) current.add(relay)
+        return saveRelayList(current)
+    }
+
     suspend fun saveRelayList(blockedRelays: List<NormalizedRelayUrl>): BlockedRelayListEvent {
         if (!signer.isWriteable()) throw SignerExceptions.ReadOnlyException()
         val relayListForBlocked = getBlockedRelayList()

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

@@ -57,6 +57,26 @@ fun DisplayNotifyMessages(
             accountViewModel = accountViewModel,
             nav = nav,
             onDismiss = { requests.dismissPaymentRequest(request) },
+            onBlockRelay =
+                if (accountViewModel.isWriteable()) {
+                    {
+                        accountViewModel.launchSigner {
+                            accountViewModel.account.blockRelay(request.relayUrl)
+
+                            // Reached only once the block is signed and published, because
+                            // reportSignerErrors swallows a refused or timed-out signature without
+                            // a toast: dismissing up front would close the dialog on a relay that
+                            // is still unblocked and leave the user no sign anything failed. The
+                            // prompt staying up is the feedback.
+                            //
+                            // Every queued prompt from the relay goes at once, not just the one on
+                            // screen — a paid relay files one NOTIFY per rejected AUTH.
+                            requests.dismissAllFrom(request.relayUrl)
+                        }
+                    }
+                } else {
+                    null
+                },
         )
     }
 }

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

@@ -30,6 +30,7 @@ import androidx.compose.material3.ButtonColors
 import androidx.compose.material3.ButtonDefaults
 import androidx.compose.material3.MaterialTheme
 import androidx.compose.material3.Text
+import androidx.compose.material3.TextButton
 import androidx.compose.runtime.Composable
 import androidx.compose.runtime.mutableStateOf
 import androidx.compose.runtime.remember
@@ -55,6 +56,11 @@ fun NotifyRequestDialog(
     accountViewModel: AccountViewModel,
     nav: INav,
     onDismiss: () -> Unit,
+    /**
+     * Adds the relay that sent this message to the NIP-51 kind:10006 blocked list. Null hides the
+     * button — there is nothing to block for a read-only account, which cannot sign the list.
+     */
+    onBlockRelay: (() -> Unit)? = null,
 ) {
     AlertDialog(
         onDismissRequest = onDismiss,
@@ -94,5 +100,29 @@ fun NotifyRequestDialog(
                 }
             }
         },
+        dismissButton =
+            onBlockRelay?.let {
+                {
+                    TextButton(
+                        onClick = it,
+                        contentPadding = PaddingValues(horizontal = Size16dp),
+                    ) {
+                        Row(
+                            verticalAlignment = Alignment.CenterVertically,
+                        ) {
+                            Icon(
+                                symbol = MaterialSymbols.Block,
+                                contentDescription = null,
+                                tint = MaterialTheme.colorScheme.error,
+                            )
+                            Spacer(StdHorzSpacer)
+                            Text(
+                                text = stringRes(R.string.notify_block_relay),
+                                color = MaterialTheme.colorScheme.error,
+                            )
+                        }
+                    }
+                }
+            },
     )
 }

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

@@ -101,8 +101,15 @@ class NotifyCoordinator(
         // Consume the correlation so a later, unrelated NOTIFY can't reuse a stale attribution.
         // An unattributable NOTIFY (none of our auths were rejected here) is dropped rather than
         // risk surfacing it under the wrong account.
-        val account = billedPubkeyAt.remove(relay)?.let(accountForPubkey)
-        account?.relayNotifications?.addPaymentRequestIfNew(message, relay)
+        val account = billedPubkeyAt.remove(relay)?.let(accountForPubkey) ?: return
+
+        // A relay the user has already blocked doesn't get to keep prompting. The pool drops the
+        // socket once the subscriptions that wanted this relay are recomputed, but frames already
+        // in flight can still arrive in that window — and the whole point of the dialog's "Block
+        // Relay" button is that the dialog stops coming back.
+        if (relay in account.blockedRelayList.flow.value) return
+
+        account.relayNotifications.addPaymentRequestIfNew(message, relay)
     }
 
     init {

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) {
@@ -52,4 +53,23 @@ class NotifyRequestsCache {
             this.transientPaymentRequestDismissals.update { it + request }
         }
     }
+
+    /**
+     * Drops every pending prompt from [relayUrl] at once.
+     *
+     * Used when the user blocks the relay: a relay that asks for payment usually queues one NOTIFY
+     * per rejected AUTH, so dismissing them one at a time would keep re-showing the dialog for a
+     * relay the user just told us never to talk to again.
+     */
+    fun dismissAllFrom(relayUrl: NormalizedRelayUrl) {
+        // 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 } }
+
+        val dismissed = before.filterTo(mutableSetOf()) { it.relayUrl == relayUrl }
+        if (dismissed.isEmpty()) return
+
+        this.transientPaymentRequestDismissals.update { it + dismissed }
+    }
 }

amethyst/src/main/res/values/strings.xml

@@ -3123,6 +3123,7 @@
     </string>
 
     <string name="payment_required_title">Message from %1$s</string>
+    <string name="notify_block_relay">Block Relay</string>
     <string name="thread_title">Thread</string>
     <string name="send_the_seller_a_message">Send the seller a message</string>
     <string name="hi_seller_is_this_still_available">Hi %1$s, is this still available?</string>

amethyst/src/test/java/com/vitorpamplona/amethyst/model/nip51Lists/BlockedRelayListAddRelayTest.kt

@@ -0,0 +1,154 @@
+/*
+ * 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.model.nip51Lists
+
+import android.os.Looper
+import com.vitorpamplona.amethyst.model.AccountSettings
+import com.vitorpamplona.amethyst.model.LocalCache
+import com.vitorpamplona.amethyst.model.nip51Lists.blockedRelays.BlockedRelayListDecryptionCache
+import com.vitorpamplona.amethyst.model.nip51Lists.blockedRelays.BlockedRelayListState
+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.nip51Lists.relayLists.BlockedRelayListEvent
+import io.mockk.every
+import io.mockk.mockk
+import io.mockk.mockkStatic
+import io.mockk.unmockkStatic
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.SupervisorJob
+import kotlinx.coroutines.test.runTest
+import org.junit.After
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertTrue
+import org.junit.Before
+import org.junit.Test
+
+/**
+ * Drives the real kind-10006 mutation behind the NOTIFY dialog's "Block Relay" button.
+ *
+ * The button hands a single relay to [BlockedRelayListState.addRelay], which read-modify-writes the
+ * whole list — and [BlockedRelayListEvent.updateRelayList] *replaces* every relay tag with what it
+ * is given. So "does blocking work" is really two questions this pins: the new relay lands, and the
+ * relays already blocked survive. A regression here silently wipes the user's block list on their
+ * next tap, which no amount of UI testing would make obvious.
+ *
+ * Each case uses a fresh keypair so its kind-10006 lives at its own address in the process-wide
+ * [LocalCache].
+ */
+class BlockedRelayListAddRelayTest {
+    private val paid = RelayUrlNormalizer.normalize("wss://paid.example.com")
+    private val alsoPaid = RelayUrlNormalizer.normalize("wss://other.example.com")
+
+    @Before
+    fun setup() {
+        // LocalCache.consume refuses the main thread; plain JVM tests have no Looper,
+        // where null == null reads as "main". Distinct mocks make it a worker thread.
+        mockkStatic(Looper::class)
+        every { Looper.myLooper() } returns mockk<Looper>()
+        every { Looper.getMainLooper() } returns mockk<Looper>()
+    }
+
+    @After
+    fun tearDown() {
+        unmockkStatic(Looper::class)
+    }
+
+    private class Fixture {
+        val keyPair = KeyPair()
+        val signer = NostrSignerInternal(keyPair)
+        val decryptionCache = BlockedRelayListDecryptionCache(signer)
+
+        // Stubbed rather than real: AccountSettings reaches for Resources.getSystem() to read the
+        // user's spoken languages, which is null outside an Android runtime. The state only asks it
+        // for the backup list (none here) and hands it updates to persist.
+        val settings = mockk<AccountSettings>(relaxed = true) { every { backupBlockedRelayList } returns null }
+
+        val state =
+            BlockedRelayListState(
+                signer = signer,
+                cache = LocalCache,
+                decryptionCache = decryptionCache,
+                scope = CoroutineScope(Dispatchers.IO + SupervisorJob()),
+                settings = settings,
+            )
+
+        /** Mirrors what the publish path does: the signed event goes into the cache it came from. */
+        fun land(event: BlockedRelayListEvent) = LocalCache.justConsumeMyOwnEvent(event)
+
+        suspend fun blockedIn(event: BlockedRelayListEvent) = BlockedRelayListDecryptionCache(signer).relays(event)
+    }
+
+    @Test
+    fun blockingTheFirstRelayCreatesTheList() =
+        runTest {
+            val f = Fixture()
+
+            val event = f.state.addRelay(paid)
+
+            assertEquals(BlockedRelayListEvent.KIND, event.kind)
+            assertEquals(setOf(paid), f.blockedIn(event))
+        }
+
+    /** The one that matters: a second tap must not throw away the first block. */
+    @Test
+    fun blockingASecondRelayKeepsTheFirst() =
+        runTest {
+            val f = Fixture()
+            f.land(f.state.addRelay(paid))
+
+            val event = f.state.addRelay(alsoPaid)
+
+            assertEquals(setOf(paid, alsoPaid), f.blockedIn(event))
+        }
+
+    @Test
+    fun blockingAnAlreadyBlockedRelayDoesNotDuplicateIt() =
+        runTest {
+            val f = Fixture()
+            f.land(f.state.addRelay(paid))
+
+            val event = f.state.addRelay(paid)
+
+            assertEquals(setOf(paid), f.blockedIn(event))
+            assertEquals(1, BlockedRelayListDecryptionCache(f.signer).relays(event).size)
+        }
+
+    /**
+     * Blocked relays are private tags. Leaking them to public tags would publish which paid relays
+     * the user walked away from to anyone who reads their kind-10006.
+     */
+    @Test
+    fun blockedRelaysStayInEncryptedPrivateTags() =
+        runTest {
+            val f = Fixture()
+            f.land(f.state.addRelay(paid))
+
+            val event = f.state.addRelay(alsoPaid)
+
+            assertTrue("blocked relays must not appear in public tags", event.publicRelays().isEmpty())
+            assertTrue(
+                "no relay url may appear in the cleartext tag array",
+                event.tags.none { tag -> tag.any { it.contains("paid.example.com") || it.contains("other.example.com") } },
+            )
+        }
+}

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

@@ -0,0 +1,88 @@
+/*
+ * 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.service.relayClient.notifyCommand.model
+
+import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertTrue
+import org.junit.Test
+
+class NotifyRequestsCacheTest {
+    private val paid = NormalizedRelayUrl("wss://paid.relay/")
+    private val other = NormalizedRelayUrl("wss://other.relay/")
+
+    @Test
+    fun blockingARelayDropsEveryPendingPromptFromIt() {
+        val cache = NotifyRequestsCache()
+        cache.addPaymentRequestIfNew("Pay up", paid)
+        cache.addPaymentRequestIfNew("Still unpaid", paid)
+        cache.addPaymentRequestIfNew("Unrelated", other)
+
+        cache.dismissAllFrom(paid)
+
+        assertEquals(
+            setOf(NotifyRequest(other, "Unrelated")),
+            cache.transientPaymentRequests.value,
+        )
+    }
+
+    @Test
+    fun aDismissedPromptStaysDismissedWhenTheRelayRepeatsIt() {
+        val cache = NotifyRequestsCache()
+        cache.addPaymentRequestIfNew("Pay up", paid)
+
+        cache.dismissAllFrom(paid)
+        cache.addPaymentRequestIfNew("Pay up", paid)
+
+        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()
+        cache.addPaymentRequestIfNew("Unrelated", other)
+
+        cache.dismissAllFrom(paid)
+
+        assertEquals(
+            setOf(NotifyRequest(other, "Unrelated")),
+            cache.transientPaymentRequests.value,
+        )
+        assertTrue(cache.transientPaymentRequestDismissals.value.isEmpty())
+    }
+}

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()