diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/relays/NostrClientLoggerListener.kt b/app/src/main/java/com/greenart7c3/nostrsigner/relays/NostrClientLoggerListener.kt index b7300039..2a202698 100644 --- a/app/src/main/java/com/greenart7c3/nostrsigner/relays/NostrClientLoggerListener.kt +++ b/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() - - 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 - } } diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/relays/RelayHealthTracker.kt b/app/src/main/java/com/greenart7c3/nostrsigner/relays/RelayHealthTracker.kt new file mode 100644 index 00000000..d395f35f --- /dev/null +++ b/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() + + /** 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() + } +} diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/service/ConnectivityService.kt b/app/src/main/java/com/greenart7c3/nostrsigner/service/ConnectivityService.kt index 475ba367..20f9a5f4 100644 --- a/app/src/main/java/com/greenart7c3/nostrsigner/service/ConnectivityService.kt +++ b/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( diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/service/NotificationSubscription.kt b/app/src/main/java/com/greenart7c3/nostrsigner/service/NotificationSubscription.kt index d82431f2..f6bd20f8 100644 --- a/app/src/main/java/com/greenart7c3/nostrsigner/service/NotificationSubscription.kt +++ b/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, + ), + ) + }, + ) + } } } diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/service/ReconnectReceiver.kt b/app/src/main/java/com/greenart7c3/nostrsigner/service/ReconnectReceiver.kt index ac927236..44801c36 100644 --- a/app/src/main/java/com/greenart7c3/nostrsigner/service/ReconnectReceiver.kt +++ b/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) } } }