Drop dead relays from the subscription pool, not just reconnect backoff

The previous change only stopped the logger listener from scheduling
reconnects, but Quartz's relay pool is driven by the relays referenced in
active subscriptions. NotificationSubscription.updateFilter re-subscribed
every 30s with the full relay set, so the pool kept opening a socket to an
unreachable relay on every refresh.

Move the per-relay failure tracking into a shared RelayHealthTracker and
exclude dead relays from the subscription relay sets in updateFilter, so a
dead relay leaves the pool entirely and is no longer reconnected. Dead-relay
state is cleared on a successful connection, on every OS network change
(ConnectivityService), and on a manual reconnect (ReconnectReceiver), so
relays get a fresh chance to recover.

https://claude.ai/code/session_01SLv8ZEooFiXErAvpARtz2Y

app/src/main/java/com/greenart7c3/nostrsigner/relays/NostrClientLoggerListener.kt

@@ -17,7 +17,7 @@ import com.vitorpamplona.quartz.nip01Core.relay.commands.toClient.OkMessage
 import com.vitorpamplona.quartz.nip01Core.relay.commands.toRelay.Command
 import com.vitorpamplona.quartz.nip01Core.relay.commands.toRelay.EventCmd
 import com.vitorpamplona.quartz.nip01Core.relay.commands.toRelay.ReqCmd
-import java.util.concurrent.ConcurrentHashMap
+import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Job
 import kotlinx.coroutines.delay
@@ -56,22 +56,15 @@ class NostrClientLoggerListener(
     private var reconnectDelay = 5_000L
     private var lastDisconnectTime = 0L
 
-    // Per-relay consecutive connection-failure counts. A relay that fails to
-    // connect MAX_RECONNECT_ATTEMPTS times in a row is treated as permanently
-    // dead and stops scheduling reconnects, so an unreachable relay can't keep
-    // waking the radio every minute forever and draining the battery. The count
-    // is reset whenever the relay connects successfully (onConnected), and a dead
-    // relay still gets a fresh chance on the next OS network change, because the
-    // ConnectivityService network callback calls client.connect(), which retries
-    // every relay regardless of this backoff state.
-    private val failureCounts = ConcurrentHashMap<String, Int>()
-
-    private fun scheduleReconnect(relayUrl: String) {
-        val failures = (failureCounts[relayUrl] ?: 0) + 1
-        failureCounts[relayUrl] = failures
-        if (failures > MAX_RECONNECT_ATTEMPTS) {
+    // Counts the failure against the relay and only schedules a reconnect while it
+    // is still worth retrying. Once a relay is dead, RelayHealthTracker also makes
+    // NotificationSubscription.updateFilter drop it from the subscription relay
+    // set, so Quartz stops opening sockets to it every 30s. The streak resets on a
+    // successful connection (onConnected) or a network change / manual reconnect.
+    private fun scheduleReconnect(relay: NormalizedRelayUrl) {
+        if (!RelayHealthTracker.recordFailure(relay)) {
             if (BuildConfig.DEBUG) {
-                Log.d(Amber.TAG, "Relay $relayUrl marked dead after $failures failed attempts; skipping reconnect")
+                Log.d(Amber.TAG, "Relay ${relay.url} marked dead; skipping reconnect")
             }
             return
         }
@@ -138,7 +131,7 @@ class NostrClientLoggerListener(
             return
         }
 
-        scheduleReconnect(relay.url.url)
+        scheduleReconnect(relay.url)
         super.onCannotConnect(relay, errorMessage)
     }
 
@@ -189,7 +182,7 @@ class NostrClientLoggerListener(
             return
         }
 
-        scheduleReconnect(relay.url.url)
+        scheduleReconnect(relay.url)
         super.onDisconnected(relay)
     }
 
@@ -203,16 +196,8 @@ class NostrClientLoggerListener(
         if (BuildConfig.DEBUG) Log.d(Amber.TAG, "onConnected: ${relay.url.url} ping: ${pingMillis}ms compressed: $compressed")
         saveLog(relay.url.url, "onConnected", "Connected")
         // Relay recovered: clear its failure streak so it is eligible for the
-        // normal reconnect-with-backoff path again.
-        failureCounts.remove(relay.url.url)
+        // normal reconnect-with-backoff path (and the subscription) again.
+        RelayHealthTracker.recordSuccess(relay.url)
         super.onConnected(relay, pingMillis, compressed)
     }
-
-    companion object {
-        // After this many consecutive failures a relay is considered permanently
-        // dead and is no longer scheduled for reconnection until it recovers on a
-        // network change. With the 60s backoff cap this is roughly 10 minutes of
-        // retrying before giving up.
-        private const val MAX_RECONNECT_ATTEMPTS = 10
-    }
 }

app/src/main/java/com/greenart7c3/nostrsigner/relays/RelayHealthTracker.kt

@@ -0,0 +1,45 @@
+package com.greenart7c3.nostrsigner.relays
+
+import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl
+import java.util.concurrent.ConcurrentHashMap
+
+/**
+ * Tracks consecutive connection failures per relay so the app can stop retrying
+ * relays that are permanently unreachable.
+ *
+ * Quartz's relay pool is driven by the relays referenced in active subscriptions:
+ * [NotificationSubscription.updateFilter] re-subscribes every 30s, and any relay
+ * present in that map is (re)connected by the pool. Without this tracker an
+ * offline relay stays in the map forever, so a socket is opened to it on every
+ * refresh, needlessly waking the radio and draining the battery.
+ *
+ * A relay is considered dead after [MAX_RECONNECT_ATTEMPTS] consecutive failures.
+ * Dead relays are excluded from the subscription relay sets so the pool drops
+ * them entirely. The streak is cleared when a relay connects successfully
+ * ([recordSuccess]), and [reset] is called on every OS network change and on a
+ * manual reconnect so previously-dead relays get a fresh chance.
+ */
+object RelayHealthTracker {
+    private const val MAX_RECONNECT_ATTEMPTS = 10
+
+    private val failureCounts = ConcurrentHashMap<NormalizedRelayUrl, Int>()
+
+    /** Records a failed connection attempt. Returns true while the relay is still worth retrying. */
+    fun recordFailure(relay: NormalizedRelayUrl): Boolean {
+        val failures = failureCounts.merge(relay, 1, Int::plus) ?: 1
+        return failures <= MAX_RECONNECT_ATTEMPTS
+    }
+
+    /** Clears the failure streak after a successful connection. */
+    fun recordSuccess(relay: NormalizedRelayUrl) {
+        failureCounts.remove(relay)
+    }
+
+    /** True once a relay has failed [MAX_RECONNECT_ATTEMPTS] times in a row without recovering. */
+    fun isDead(relay: NormalizedRelayUrl): Boolean = (failureCounts[relay] ?: 0) > MAX_RECONNECT_ATTEMPTS
+
+    /** Forgets all failure history so every relay is eligible to be retried again. */
+    fun reset() {
+        failureCounts.clear()
+    }
+}

app/src/main/java/com/greenart7c3/nostrsigner/service/ConnectivityService.kt

@@ -15,6 +15,7 @@ import com.greenart7c3.nostrsigner.BuildConfig
 import com.greenart7c3.nostrsigner.BuildFlavorChecker
 import com.greenart7c3.nostrsigner.LocalPreferences
 import com.greenart7c3.nostrsigner.okhttp.HttpClientManager
+import com.greenart7c3.nostrsigner.relays.RelayHealthTracker
 import java.util.Timer
 import java.util.TimerTask
 import kotlin.collections.set
@@ -37,6 +38,9 @@ class ConnectivityService : Service() {
                 if (Amber.instance.settings.killSwitch.value) return
 
                 if (lastNetwork != null && lastNetwork != network) {
+                    // New network: give previously-dead relays a fresh chance. The
+                    // 30s updateFilter tick re-adds them to the subscription set.
+                    RelayHealthTracker.reset()
                     scope.launch(Dispatchers.IO) {
                         if (!Amber.instance.client.isActive()) {
                             Amber.instance.client.connect()
@@ -58,6 +62,10 @@ class ConnectivityService : Service() {
                 if (Amber.instance.settings.killSwitch.value) return
 
                 val changed = Amber.instance.updateNetworkCapabilities(networkCapabilities)
+                if (changed) {
+                    // Transport changed (e.g. wifi <-> mobile): retry dead relays.
+                    RelayHealthTracker.reset()
+                }
 
                 scope.launch(Dispatchers.IO) {
                     Log.d(

app/src/main/java/com/greenart7c3/nostrsigner/service/NotificationSubscription.kt

@@ -25,6 +25,7 @@ import android.util.Log
 import com.greenart7c3.nostrsigner.Amber
 import com.greenart7c3.nostrsigner.BuildFlavorChecker
 import com.greenart7c3.nostrsigner.LocalPreferences
+import com.greenart7c3.nostrsigner.relays.RelayHealthTracker
 import com.vitorpamplona.quartz.nip01Core.relay.client.NostrClient
 import com.vitorpamplona.quartz.nip01Core.relay.client.listeners.RelayConnectionListener
 import com.vitorpamplona.quartz.nip01Core.relay.client.single.IRelayClient
@@ -90,11 +91,19 @@ class NotificationSubscription(
                 val connPubKey = conn.localPubKey
                 indexEntries[connPubKey] = LocalKeyAccountIndex.Match(account.npub, conn.localKey)
                 val subKey = "${account.hexKey}_$connPubKey"
+
+                // Exclude relays that have been declared dead so Quartz stops opening
+                // a socket to them on every 30s refresh. They are re-added once
+                // RelayHealthTracker is reset (network change / manual reconnect) or
+                // they connect successfully again.
+                val connRelays = conn.relays.ifEmpty { Amber.instance.getSavedRelays(account) }
+                    .filterNot { RelayHealthTracker.isDead(it) }
+                if (connRelays.isEmpty()) continue
+
                 activeSubKeys.add(subKey)
                 if (!subIds.containsKey(subKey)) {
                     subIds[subKey] = UUID.randomUUID().toString()
                 }
-                val connRelays = conn.relays.ifEmpty { Amber.instance.getSavedRelays(account) }
                 client.subscribe(
                     subIds[subKey]!!,
                     connRelays.associateWith {
@@ -112,24 +121,27 @@ class NotificationSubscription(
 
             // Main account subscription only for legacy connections (no localKey)
             if (hasLegacyConnections) {
-                activeSubKeys.add(account.hexKey)
-                if (!subIds.containsKey(account.hexKey)) {
-                    subIds[account.hexKey] = UUID.randomUUID().toString()
-                }
                 val relays = Amber.instance.getSavedRelays(account)
-                client.subscribe(
-                    subIds[account.hexKey]!!,
-                    relays.associateWith {
-                        listOf(
-                            Filter(
-                                kinds = listOf(NostrConnectEvent.KIND),
-                                tags = mapOf("p" to listOf(account.hexKey)),
-                                limit = 1,
-                                since = since,
-                            ),
-                        )
-                    },
-                )
+                    .filterNot { RelayHealthTracker.isDead(it) }
+                if (relays.isNotEmpty()) {
+                    activeSubKeys.add(account.hexKey)
+                    if (!subIds.containsKey(account.hexKey)) {
+                        subIds[account.hexKey] = UUID.randomUUID().toString()
+                    }
+                    client.subscribe(
+                        subIds[account.hexKey]!!,
+                        relays.associateWith {
+                            listOf(
+                                Filter(
+                                    kinds = listOf(NostrConnectEvent.KIND),
+                                    tags = mapOf("p" to listOf(account.hexKey)),
+                                    limit = 1,
+                                    since = since,
+                                ),
+                            )
+                        },
+                    )
+                }
             }
         }
 

app/src/main/java/com/greenart7c3/nostrsigner/service/ReconnectReceiver.kt

@@ -4,12 +4,16 @@ import android.content.BroadcastReceiver
 import android.content.Context
 import android.content.Intent
 import com.greenart7c3.nostrsigner.Amber
+import com.greenart7c3.nostrsigner.relays.RelayHealthTracker
 import kotlinx.coroutines.launch
 
 class ReconnectReceiver : BroadcastReceiver() {
     override fun onReceive(context: Context, intent: Intent) {
         Amber.instance.applicationIOScope.launch {
-            Amber.instance.reconnect()
+            // Manual reconnect: clear dead-relay state and re-run the filters so any
+            // relay previously dropped from the pool is re-added before reconnecting.
+            RelayHealthTracker.reset()
+            Amber.instance.checkForNewRelaysAndUpdateAllFilters(shouldReconnect = true)
         }
     }
 }