Merge pull request #3480 from vitorpamplona/claude/negentropysynch-nip42-hang-1m4hxb

Fix negentropy refusal handling to prevent window-split storm
2026-10-05 19:28:25 +00:00 · 2026-07-05 12:09:55 -04:00
parent 4a13a15f6c 5927c1837e
commit c27e977b7a
2 changed files with 84 additions and 5 deletions
@@ -937,13 +937,23 @@ private sealed interface NegFrame {
 /**
 * strfry sends `["NEG-ERR", subId, "blocked: too many query results"]` when a
 * NEG-OPEN matches more than `relay__negentropy__maxSyncEvents`. Match that
- * verbatim, plus a looser contains-check so equivalent wording from other relays
- * still triggers the window split rather than aborting.
+ * verbatim, plus a looser contains-check for equivalent "result set too large"
+ * wording from other relays, so it still triggers the window split rather than
+ * aborting.
+ *
+ * This MUST stay narrow: only a genuine *set-too-large* signal may be treated as
+ * overflow, because overflow triggers `created_at` window-splitting. A NEG-ERR
+ * that is really a hard refusal — negentropy disabled, `auth-required`, a ban —
+ * must NOT match, or every split re-opens, is refused again, and the splitter
+ * fans out across the whole `created_at` range (a ~2^31-window storm) instead of
+ * failing over to paging. In particular a bare `blocked: …` prefix is such a
+ * refusal (e.g. strfry-style `"blocked: Negentropy sync is disabled"`) and is
+ * deliberately excluded — only the specific overflow wording counts.
 */
 private fun isOverflow(reason: String): Boolean =
-    reason == "blocked: too many query results" ||
-        reason.contains("too many", ignoreCase = true) ||
-        reason.startsWith("blocked", ignoreCase = true)
+    reason.contains("too many", ignoreCase = true) ||
+        reason.contains("too large", ignoreCase = true) ||
+        reason.contains("max_sync_events", ignoreCase = true)

 /**
 * One `REQ` for [batch] ids; collects the matching events and returns them on
@@ -31,8 +31,11 @@ import com.vitorpamplona.quartz.nip01Core.relay.client.accessories.fetchAllPages
 import com.vitorpamplona.quartz.nip01Core.relay.client.accessories.negentropySync
 import com.vitorpamplona.quartz.nip01Core.relay.client.accessories.negentropySyncEvents
 import com.vitorpamplona.quartz.nip01Core.relay.client.accessories.negentropySyncOrFetch
+import com.vitorpamplona.quartz.nip01Core.relay.commands.toRelay.ReqCmd
 import com.vitorpamplona.quartz.nip01Core.relay.filters.Filter
 import com.vitorpamplona.quartz.nip01Core.relay.normalizer.RelayUrlNormalizer
+import com.vitorpamplona.quartz.nip01Core.relay.server.policies.PassThroughPolicy
+import com.vitorpamplona.quartz.nip01Core.relay.server.policies.PolicyResult
 import com.vitorpamplona.quartz.nip77Negentropy.NegentropySettings
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
@@ -284,6 +287,72 @@ class NostrClientNegentropySyncTest : RelayClientTest() {
            }
        }

+    /**
+     * A relay that refuses negentropy with a `blocked: …` NEG-ERR that is NOT an
+     * over-cap overflow (here: NIP-77 disabled) but still serves plain REQ — the
+     * live shape of `wss://nostr-pr02.redscrypt.org`.
+     *
+     * Regression for the window-split storm: the refusal string starts with
+     * `blocked`, which [isOverflow] used to treat as a `max_sync_events` overflow.
+     * Every window then re-opened, was refused again, and the splitter fanned out
+     * across the whole `created_at` range (~2^31 windows) — so it neither threw
+     * [NegentropySyncException] (no paging fallback) nor tripped the idle watchdog
+     * (the relay answered every NEG-OPEN promptly), and the call hung indefinitely.
+     * With the fix the refusal is a hard failure: negentropy aborts on the first
+     * window and [negentropySyncOrFetch] pages instead, delivering every event.
+     */
+    @Test
+    fun orFetchPagesWhenNegentropyRefusedWithBlockedError() =
+        runBlocking {
+            // geode routes NEG-OPEN and REQ through the same accept(ReqCmd) hook, so
+            // we refuse only the NEG-OPEN / window filters (kinds, no limit) and let
+            // the paging fallback REQ through — it always carries a `limit`.
+            val negDisabled =
+                object : PassThroughPolicy() {
+                    override fun accept(cmd: ReqCmd): PolicyResult<ReqCmd> =
+                        if (cmd.filters.any { it.kinds != null && it.limit == null && it.ids == null }) {
+                            PolicyResult.Rejected("blocked: Negentropy sync is disabled")
+                        } else {
+                            PolicyResult.Accepted(cmd)
+                        }
+                }
+
+            val hub = InProcessRelays(defaultPolicy = { negDisabled })
+            val scope = CoroutineScope(Dispatchers.Default + SupervisorJob())
+            val client = NostrClient(hub, scope)
+            try {
+                val url = RelayUrlNormalizer.normalize("ws://127.0.0.1:7785/")
+                // Spread across created_at so a naive splitter would have many
+                // windows to churn through; the fix must abort on the first one.
+                // Kind 1 (regular, not replaceable) so all 10 distinct events persist.
+                val events = (1..10).map { SyntheticEvents.fakeEvent(idSeed = it, kind = 1, createdAt = it * 1_000_000L) }
+                hub.getOrCreate(url).preload(events)
+
+                val got = mutableListOf<Event>()
+                val result =
+                    withTimeout(30_000) {
+                        client.negentropySyncOrFetch(
+                            relay = url,
+                            filter = Filter(kinds = listOf(1)),
+                            maxEvents = 5000,
+                            idleTimeoutMs = 10_000L,
+                        ) { got.add(it) }
+                    }
+
+                assertTrue(result.pagedFallback, "a non-overflow blocked NEG-ERR must fail over to paging, not window-split")
+                assertEquals(
+                    NegentropySyncException.Reason.UNAVAILABLE,
+                    result.fallbackCause?.reason,
+                    "the refusal is a hard failure, not an over-cap overflow",
+                )
+                assertEquals(10, got.map { it.id }.toSet().size, "every event delivered via the paging fallback")
+            } finally {
+                client.disconnect()
+                scope.cancel()
+                hub.close()
+            }
+        }
+
    /**
     * On a relay that reconciles fine, [negentropySyncOrFetch] uses negentropy and
     * does not page.