From 6bdcd46b33de4bbcb0c38aadee5688217a7c151a Mon Sep 17 00:00:00 2001 From: Taksh Date: Mon, 24 Aug 2026 21:32:36 +0530 Subject: [PATCH 1/2] fix(nostr): keep retrying relays instead of giving up on them forever A relay that fails is retried on an exponential backoff, then abandoned for the lifetime of the process. Nothing brings it back: the relay layer registers no connectivity callback, the periodic subscription validator only repairs subscriptions on sockets that are already open (and returns immediately when connectedRelayCount is 0, which is exactly the state after an outage), and connect() runs once from NostrClient.initialize(). The remaining paths that reset reconnectAttempts are a manual retry, a Tor state change, and a successful open. Two ways a relay died permanently: - Any error whose message mentioned DNS returned before scheduling anything at all. "Unable to resolve host" is what this device reports when it simply has no network, so a moment in a tunnel killed every relay at once, with no retry ever. - Otherwise the schedule stopped at MAX_RECONNECT_ATTEMPTS. With INITIAL=1s and MULTIPLIER=2 that is nine waits totalling about eight and a half minutes, after which the relay was dead. MAX_BACKOFF_INTERVAL was unreachable: attempt 9 asks for 256s and attempt 10 gave up, so the five-minute ceiling the constant defines never applied to anything. Let the backoff saturate at MAX_BACKOFF_INTERVAL and keep retrying there. A name-resolution failure now backs off like any other error. Steady state costs one connection attempt per relay per five minutes; the previous behaviour cost the user every internet DM, delivery receipt and geohash channel until they noticed and restarted the app. The schedule moves into RelayReconnectPolicy so it is unit-testable without OkHttp or a Context. --- .../android/nostr/NostrRelayManager.kt | 47 ++------- .../android/nostr/RelayReconnectPolicy.kt | 45 +++++++++ .../android/nostr/RelayReconnectPolicyTest.kt | 96 +++++++++++++++++++ 3 files changed, 151 insertions(+), 37 deletions(-) create mode 100644 app/src/main/java/com/bitchat/android/nostr/RelayReconnectPolicy.kt create mode 100644 app/src/test/kotlin/com/bitchat/android/nostr/RelayReconnectPolicyTest.kt diff --git a/app/src/main/java/com/bitchat/android/nostr/NostrRelayManager.kt b/app/src/main/java/com/bitchat/android/nostr/NostrRelayManager.kt index 772e2a60..81c1cac6 100644 --- a/app/src/main/java/com/bitchat/android/nostr/NostrRelayManager.kt +++ b/app/src/main/java/com/bitchat/android/nostr/NostrRelayManager.kt @@ -14,8 +14,6 @@ import okhttp3.* import java.util.UUID import java.util.concurrent.ConcurrentHashMap import java.util.concurrent.atomic.AtomicBoolean -import kotlin.math.min -import kotlin.math.pow /** * Manages WebSocket connections to Nostr relays @@ -48,12 +46,8 @@ class NostrRelayManager private constructor() { "wss://nostr21.com" ) - // Exponential backoff configuration (same as iOS) - private const val INITIAL_BACKOFF_INTERVAL = com.bitchat.android.util.AppConstants.Nostr.INITIAL_BACKOFF_INTERVAL_MS // 1 second - private const val MAX_BACKOFF_INTERVAL = com.bitchat.android.util.AppConstants.Nostr.MAX_BACKOFF_INTERVAL_MS // 5 minutes - private const val BACKOFF_MULTIPLIER = com.bitchat.android.util.AppConstants.Nostr.BACKOFF_MULTIPLIER - private const val MAX_RECONNECT_ATTEMPTS = com.bitchat.android.util.AppConstants.Nostr.MAX_RECONNECT_ATTEMPTS - + // Reconnect backoff lives in RelayReconnectPolicy. + // Track gift-wraps we initiated for logging private val pendingGiftWrapIDs = ConcurrentHashMap.newKeySet() @@ -1018,36 +1012,15 @@ class NostrRelayManager private constructor() { if (!desiredConnected.get() || !isNetworkActionAllowed(connectionToken) ) return - - // Check if this is a DNS error - val errorMessage = error.message?.lowercase() ?: "" - if (errorMessage.contains("hostname could not be found") || - errorMessage.contains("dns") || - errorMessage.contains("unable to resolve host")) { - - val relay = relaysList.find { it.url == relayUrl } - if (relay?.lastError == null) { - Log.w(TAG, "Nostr relay DNS failure; not retrying") - } - return - } - - // Implement exponential backoff for non-DNS errors + + // Every failure backs off and retries, including a name-resolution + // failure: "unable to resolve host" is what this device reports when it + // simply has no network, so treating it as permanent turns a walk + // through a tunnel into a dead relay layer for the rest of the process. val relay = relaysList.find { it.url == relayUrl } ?: return - relay.reconnectAttempts++ - - // Stop attempting after max attempts - if (relay.reconnectAttempts >= MAX_RECONNECT_ATTEMPTS) { - Log.w(TAG, "Max Nostr relay reconnection attempts reached") - return - } - - // Calculate backoff interval - val backoffInterval = min( - INITIAL_BACKOFF_INTERVAL * BACKOFF_MULTIPLIER.pow(relay.reconnectAttempts - 1.0), - MAX_BACKOFF_INTERVAL.toDouble() - ).toLong() - + relay.reconnectAttempts = RelayReconnectPolicy.nextAttempt(relay.reconnectAttempts) + val backoffInterval = RelayReconnectPolicy.backoffMs(relay.reconnectAttempts) + relay.nextReconnectTime = System.currentTimeMillis() + backoffInterval Log.d(TAG, "Scheduling Nostr relay reconnection") diff --git a/app/src/main/java/com/bitchat/android/nostr/RelayReconnectPolicy.kt b/app/src/main/java/com/bitchat/android/nostr/RelayReconnectPolicy.kt new file mode 100644 index 00000000..199e0dcf --- /dev/null +++ b/app/src/main/java/com/bitchat/android/nostr/RelayReconnectPolicy.kt @@ -0,0 +1,45 @@ +package com.bitchat.android.nostr + +import com.bitchat.android.util.AppConstants +import kotlin.math.min +import kotlin.math.pow + +/** + * Reconnect schedule for a Nostr relay socket. + * + * A phone loses its data connection constantly — airplane mode, a tunnel, a + * Wi-Fi to cellular handover, a dead zone — and every relay fails at once when + * it does. The schedule therefore has to survive an outage of arbitrary length + * and heal on its own, because nothing else will: the relay layer registers no + * connectivity callback, the periodic subscription validator only repairs + * subscriptions on sockets that are already open, and `connect()` runs once at + * startup. + * + * So the backoff grows exponentially and then *saturates* rather than + * terminating. Retrying forever at the ceiling costs one connection attempt per + * relay per [AppConstants.Nostr.MAX_BACKOFF_INTERVAL_MS]; giving up costs the + * user every internet DM, delivery receipt and geohash channel until they + * notice and restart the app. + */ +internal object RelayReconnectPolicy { + + /** + * Attempt count past which the interval stops growing. Beyond this the + * exponential term is already above the ceiling, so pinning it keeps the + * schedule at a steady [AppConstants.Nostr.MAX_BACKOFF_INTERVAL_MS] and + * keeps the exponent from running away over a long outage. + */ + const val SATURATION_ATTEMPTS: Int = AppConstants.Nostr.MAX_RECONNECT_ATTEMPTS + + /** Attempt number to record after a failure at [previousAttempts]. */ + fun nextAttempt(previousAttempts: Int): Int = + (previousAttempts.coerceAtLeast(0) + 1).coerceAtMost(SATURATION_ATTEMPTS) + + /** Delay before the reconnect for a given attempt number (1-based). */ + fun backoffMs(attempt: Int): Long { + val step = attempt.coerceAtLeast(1) + val exponential = AppConstants.Nostr.INITIAL_BACKOFF_INTERVAL_MS * + AppConstants.Nostr.BACKOFF_MULTIPLIER.pow(step - 1.0) + return min(exponential, AppConstants.Nostr.MAX_BACKOFF_INTERVAL_MS.toDouble()).toLong() + } +} diff --git a/app/src/test/kotlin/com/bitchat/android/nostr/RelayReconnectPolicyTest.kt b/app/src/test/kotlin/com/bitchat/android/nostr/RelayReconnectPolicyTest.kt new file mode 100644 index 00000000..f5e2581a --- /dev/null +++ b/app/src/test/kotlin/com/bitchat/android/nostr/RelayReconnectPolicyTest.kt @@ -0,0 +1,96 @@ +package com.bitchat.android.nostr + +import com.bitchat.android.util.AppConstants +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * The relay layer has no connectivity callback, its periodic validator only + * repairs subscriptions on sockets that are already open, and `connect()` runs + * once at startup. The backoff schedule is therefore the only thing that can + * bring relays back after the phone loses its data connection, so it has to + * saturate rather than terminate. + */ +class RelayReconnectPolicyTest { + + private val initial = AppConstants.Nostr.INITIAL_BACKOFF_INTERVAL_MS + private val ceiling = AppConstants.Nostr.MAX_BACKOFF_INTERVAL_MS + + @Test + fun `the first retry waits the initial interval`() { + assertEquals(initial, RelayReconnectPolicy.backoffMs(RelayReconnectPolicy.nextAttempt(0))) + } + + @Test + fun `the interval doubles per attempt until it reaches the ceiling`() { + var attempt = 0 + var previous = 0L + var sawCeiling = false + + repeat(RelayReconnectPolicy.SATURATION_ATTEMPTS) { + attempt = RelayReconnectPolicy.nextAttempt(attempt) + val delay = RelayReconnectPolicy.backoffMs(attempt) + + assertTrue("delay must never exceed the ceiling", delay <= ceiling) + if (delay == ceiling) { + sawCeiling = true + } else { + assertEquals("expected doubling below the ceiling", maxOf(initial, previous * 2), delay) + } + previous = delay + } + + assertTrue("the schedule must actually reach the ceiling", sawCeiling) + } + + @Test + fun `an outage longer than the schedule keeps retrying at the ceiling`() { + var attempt = 0 + // Far past the old give-up point; a real outage can last hours. + repeat(500) { attempt = RelayReconnectPolicy.nextAttempt(attempt) } + + assertEquals(RelayReconnectPolicy.SATURATION_ATTEMPTS, attempt) + assertEquals(ceiling, RelayReconnectPolicy.backoffMs(attempt)) + } + + @Test + fun `a long outage cannot run the attempt counter or the exponent away`() { + var attempt = 0 + repeat(10_000) { attempt = RelayReconnectPolicy.nextAttempt(attempt) } + + val delay = RelayReconnectPolicy.backoffMs(attempt) + assertTrue("delay must stay finite and bounded", delay in 1..ceiling) + } + + @Test + fun `a successful connection resets the schedule to the initial interval`() { + var attempt = 0 + repeat(6) { attempt = RelayReconnectPolicy.nextAttempt(attempt) } + assertTrue(RelayReconnectPolicy.backoffMs(attempt) > initial) + + // updateRelayStatus zeroes reconnectAttempts on a successful open. + attempt = 0 + + assertEquals(initial, RelayReconnectPolicy.backoffMs(RelayReconnectPolicy.nextAttempt(attempt))) + } + + @Test + fun `a nonsensical stored attempt count still yields a usable delay`() { + assertEquals(initial, RelayReconnectPolicy.backoffMs(RelayReconnectPolicy.nextAttempt(-5))) + assertTrue(RelayReconnectPolicy.backoffMs(0) in 1..ceiling) + assertTrue(RelayReconnectPolicy.backoffMs(Int.MAX_VALUE) in 1..ceiling) + } + + @Test + fun `the whole schedule stays under an hour of total wait before the ceiling`() { + var attempt = 0 + var total = 0L + repeat(RelayReconnectPolicy.SATURATION_ATTEMPTS) { + attempt = RelayReconnectPolicy.nextAttempt(attempt) + total += RelayReconnectPolicy.backoffMs(attempt) + } + + assertTrue("reaching the steady state must not take an hour", total < 60 * 60 * 1000L) + } +}