fix: stop giftwrap/rooms loads completing before their REQs are sent

The silence + connect-grace backstops are only meaningful when the owner
feeds onReqSent, which only the convo NIP-04 path does. But connectStalled
keyed off the ABSENCE of a recorded REQ, so for giftwrap/rooms (which never
call onReqSent) every relay looked connect-stalled after connectGrace —
the window completed at ~15s before its REQs had even gone out during a
slow connect storm ('giftwrap.live load done: settled/silent' 45s before
the REQ), prematurely declaring an empty round done and tripping the
no-progress guard, so giftwraps stopped loading.

Gate both REQ-aware backstops behind a tracksReqSends flag that only the
convo manager sets; everyone else keeps the plain settle / idle / cap
behavior. Adds a regression test that a non-tracking tracker keeps
blocking a never-heard-from relay until it actually settles.

https://claude.ai/code/session_01B1fmmmX8JjQWH3amMLdvcW
This commit is contained in:
Claude
2026-06-03 00:04:09 +00:00
parent 0d27477c4d
commit 07b0c87c38
3 changed files with 45 additions and 4 deletions
@@ -70,10 +70,19 @@ import kotlin.time.Duration.Companion.seconds
* blocking the round after [connectGrace] from the load start — but it is NOT given up (it may be a
* genuinely slow connect), so the owner keeps it and retries it next round. And an [absoluteCap] is
* the final ceiling on a window that somehow defeats all of the above.
*
* The two REQ-aware backstops (silence + connect-grace) only make sense when the owner actually feeds
* [onReqSent], so they are gated behind [tracksReqSends]. A tracker that does NOT track REQ sends keeps
* the plain settle / idle / cap behavior — otherwise, with an always-empty [reqSentAt], EVERY relay
* would look "connect-stalled" after [connectGrace] and the window would complete before its REQs even
* went out (e.g. during a slow connect storm), prematurely declaring an empty round done.
*/
class WindowLoadTracker(
// Short label for the DMPagination logs (e.g. "giftwrap", "rooms.nip04", "convo.nip04").
private val name: String = "dm",
// Whether the owner feeds [onReqSent]; enables the silence + connect-grace backstops. Off by
// default so trackers that don't track REQ sends are unaffected by them.
private val tracksReqSends: Boolean = false,
private val idleTimeout: Duration = 3.seconds,
private val silenceTimeout: Duration = 10.seconds,
private val connectGrace: Duration = 15.seconds,
@@ -187,7 +196,7 @@ class WindowLoadTracker(
private fun silencedOut(
relay: NormalizedRelayUrl,
now: Long,
): Boolean = relay !in heardFrom && (reqSentAt[relay]?.let { now - it >= silenceTimeout.inWholeMilliseconds } ?: false)
): Boolean = tracksReqSends && relay !in heardFrom && (reqSentAt[relay]?.let { now - it >= silenceTimeout.inWholeMilliseconds } ?: false)
// A relay that is still expected but has neither been heard from nor even received its REQ within
// [connectGrace] of the load start — i.e. stuck connecting / reconnecting. It stops blocking the
@@ -195,7 +204,7 @@ class WindowLoadTracker(
private fun connectStalled(
relay: NormalizedRelayUrl,
now: Long,
): Boolean = relay !in heardFrom && !reqSentAt.containsKey(relay) && now - loadStartMs >= connectGrace.inWholeMilliseconds
): Boolean = tracksReqSends && relay !in heardFrom && !reqSentAt.containsKey(relay) && now - loadStartMs >= connectGrace.inWholeMilliseconds
/** Records which relays the current REQ was sent to. Completes immediately if there are none. */
@Synchronized
@@ -69,7 +69,7 @@ class ChatroomNip04HistorySubAssembler(
private val started = ConcurrentHashMap.newKeySet<ConvoKey>()
private val askedRelays = ConcurrentHashMap<ConvoKey, Set<NormalizedRelayUrl>>()
private val windowLoad = WindowLoadTracker("convo.nip04.history", onAbandoned = ::onRelaysAbandoned)
private val windowLoad = WindowLoadTracker("convo.nip04.history", tracksReqSends = true, onAbandoned = ::onRelaysAbandoned)
val loadingMore: StateFlow<Boolean> = windowLoad.loading
private val _exhausted = MutableStateFlow(false)
@@ -50,6 +50,7 @@ class WindowLoadTrackerSilenceTest {
val tracker =
WindowLoadTracker(
name = "test",
tracksReqSends = true,
silenceTimeout = 50.milliseconds,
onAbandoned = { abandoned.set(it) },
)
@@ -77,6 +78,7 @@ class WindowLoadTrackerSilenceTest {
val tracker =
WindowLoadTracker(
name = "test",
tracksReqSends = true,
connectGrace = 50.milliseconds,
onAbandoned = { abandoned.set(it) },
)
@@ -94,11 +96,41 @@ class WindowLoadTrackerSilenceTest {
scope.cancel()
}
@Test
fun withoutReqTrackingAStalledRelayKeepsBlockingUntilItSettles() =
runBlocking {
val scope = CoroutineScope(Dispatchers.Default + SupervisorJob())
// A tracker that does NOT feed onReqSent (giftwrap / rooms): the REQ-aware backstops must
// stay off, or every never-heard-from relay would look stalled and the window would finish
// before its REQs even went out (the connect-storm regression).
val tracker =
WindowLoadTracker(
name = "test",
tracksReqSends = false,
silenceTimeout = 50.milliseconds,
connectGrace = 50.milliseconds,
)
tracker.startLoading(scope)
tracker.setExpectedRelays(setOf(good, silent))
tracker.onRelaySettled(good)
// `silent` was never heard from and never got a REQ; well past both short backstops it must
// STILL block, because this tracker doesn't track REQ sends.
Thread.sleep(400)
assertTrue("non-req-tracking tracker must not abandon a stalled relay", tracker.loading.value)
// Only an actual terminal signal completes it.
tracker.onRelaySettled(silent)
withTimeout(3000) { tracker.loading.first { !it } }
scope.cancel()
}
@Test
fun aSilentRelayThatNeverGotAReqStillBlocksUntilItSettles() =
runBlocking {
val scope = CoroutineScope(Dispatchers.Default + SupervisorJob())
val tracker = WindowLoadTracker(name = "test", silenceTimeout = 50.milliseconds)
val tracker = WindowLoadTracker(name = "test", tracksReqSends = true, silenceTimeout = 50.milliseconds)
tracker.startLoading(scope)
tracker.setExpectedRelays(setOf(good, silent))