From 10ea4f6a220455a3b68784d88dc39cbd115e9264 Mon Sep 17 00:00:00 2001 From: thePR0M3TH3AN <53631862+PR0M3TH3AN@users.noreply.github.com> Date: Tue, 16 Jun 2026 09:53:52 -0400 Subject: [PATCH] fix(subscriptions): guard the subscription-list refresh loop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The live subscription-list subscription (kind 30000) delivers our OWN list event back through handleSubscriptionListEvent, which called updateFromRelays on EVERY event. updateFromRelays re-fetches the list, the relay echoes the same event through the live sub, and the handler fires again — a self-sustaining loop that floods the cold-login burst with redundant kind-30000 REQs (the kind 30000=19 storm in the real-env console, KNOWN_BUGS #0). Same shape as the contact-list emit→fetch→emit loop fixed earlier. Add an emit-on-change guard: skip the refresh when the event is one we've already acted on (by id — matters during the pre-decrypt login window where subsEventCreatedAt isn't set yet) or is not newer than what we've applied. A genuinely newer list event still refreshes exactly once. Also extend the channel-sim harness to capture per-filter REQ signatures and kind-30000 REQ timing, which is how this loop was isolated. Test: tests/subscriptions-manager.test.mjs — echo of the same event id does not re-refresh; an older event is ignored; a newer event refreshes once. Co-Authored-By: Claude Opus 4.8 --- js/subscriptions.js | 30 ++++++++++++++++++++++ scripts/perf/nip07-channel-sim.mjs | 10 ++++++++ tests/subscriptions-manager.test.mjs | 37 ++++++++++++++++++++++++++++ 3 files changed, 77 insertions(+) diff --git a/js/subscriptions.js b/js/subscriptions.js index 47ff7521..b72532d3 100644 --- a/js/subscriptions.js +++ b/js/subscriptions.js @@ -396,6 +396,8 @@ class SubscriptionsManager { this.subscribedPubkeys = new Set(); this.subsEventId = null; this.subsEventCreatedAt = null; + // Last live-subscription event id we triggered a refresh for (loop guard). + this._lastHandledListEventId = null; this.currentUserPubkey = null; this.uiReady = false; this.dataReady = false; @@ -867,6 +869,7 @@ class SubscriptionsManager { this.subscribedPubkeys.clear(); this.subsEventId = null; this.subsEventCreatedAt = null; + this._lastHandledListEventId = null; this.currentUserPubkey = null; this.uiReady = false; this.dataReady = false; @@ -953,6 +956,33 @@ class SubscriptionsManager { return; } + // Emit-on-change guard: the live subscription re-delivers our OWN list event + // every time updateFromRelays re-fetches it, so refreshing on every event + // forms a self-sustaining loop (fetch → relay echoes the event → handler → + // fetch …) that floods the login burst with redundant kind-30000 REQs + // (KNOWN_BUGS #0). Skip when the event is one we've already acted on (by id) + // or is not newer than what we've applied. The id check matters DURING the + // cold-login window where subsEventCreatedAt isn't set yet (decrypt still in + // flight) — without it the loop iterates several times before settling. + const incomingCreatedAt = Number.isFinite(event?.created_at) + ? event.created_at + : 0; + const knownCreatedAt = Number.isFinite(this.subsEventCreatedAt) + ? this.subsEventCreatedAt + : 0; + if (event?.id && event.id === this.subsEventId) { + return; // exact event we already hold + } + if (event?.id && event.id === this._lastHandledListEventId) { + return; // already triggered a refresh for this exact event + } + if (incomingCreatedAt <= knownCreatedAt && knownCreatedAt > 0) { + return; // not newer than what we've already applied + } + if (event?.id) { + this._lastHandledListEventId = event.id; + } + this.updateFromRelays(normalized, { allowPermissionPrompt: false }) .catch((error) => { devLogger.warn( diff --git a/scripts/perf/nip07-channel-sim.mjs b/scripts/perf/nip07-channel-sim.mjs index a0ae4e54..ce9d304b 100644 --- a/scripts/perf/nip07-channel-sim.mjs +++ b/scripts/perf/nip07-channel-sim.mjs @@ -209,6 +209,8 @@ async function main() { dmHelpersUnavailable: 0, }; const reqBurst = {}; // kind -> count, captured during the login window + const reqSig = {}; // filter signature -> count, to surface redundant queries + const subsReqTimes = []; // ms-since-login of each kind-30000 REQ (redundancy timing) let loginAt = Date.now(); let measuringReq = true; @@ -238,6 +240,10 @@ async function main() { const msg = JSON.parse(p); for (const f of msg.slice(2)) { for (const k of f.kinds || ["?"]) reqBurst[k] = (reqBurst[k] || 0) + 1; + const otherKeys = Object.keys(f).filter((x) => x !== "kinds" && x !== "authors").sort().join(","); + const sig = `kinds=${(f.kinds || []).join("|")}${otherKeys ? " " + otherKeys : ""}${f.authors ? " authors=" + f.authors.length : ""}`; + reqSig[sig] = (reqSig[sig] || 0) + 1; + if ((f.kinds || []).includes(30000)) subsReqTimes.push(Date.now() - loginAt); } } catch (_) {} }); @@ -298,6 +304,10 @@ async function main() { report(`time -> signer-ready : ${signals.signerReadyAt ?? "NEVER"} ms`); report(`time -> lists-sync done: ${signals.listsSyncCompleteAt ?? "NEVER"} ms`); report(`login REQ burst (~10s) : ${reqTotal} frames [${reqTop}]`); + const sigTop = Object.entries(reqSig).sort((a, b) => b[1] - a[1]).slice(0, 12); + report("top REQ filters (count → filter):"); + for (const [sig, n] of sigTop) report(` ${String(n).padStart(4)} ${sig}`); + report(`kind-30000 REQ times(ms): [${subsReqTimes.join(", ")}]`); report(`decrypt timeouts logged: ${signals.decryptTimeouts}`); report(`channel drops injected : ${drops}`); report(`breaker opened / recov : ${signals.breakerOpened} / ${signals.breakerRecovered}`); diff --git a/tests/subscriptions-manager.test.mjs b/tests/subscriptions-manager.test.mjs index 78c936ef..5a7dffa4 100644 --- a/tests/subscriptions-manager.test.mjs +++ b/tests/subscriptions-manager.test.mjs @@ -1377,3 +1377,40 @@ test("loadSubscriptions uses overlap window and deterministic latest-event tie-b profileCache.activePubkey = null; } }); + +test("handleSubscriptionListEvent does not re-trigger on the live-sub echo (loop guard)", () => { + const SubscriptionsManager = subscriptions.constructor; + const manager = new SubscriptionsManager(); + const actor = "1".repeat(64); + manager.currentUserPubkey = actor; + + // Stand in for the relay fetch/decrypt refresh so we can count invocations + // without touching the network. The real loop is: live sub delivers our own + // kind-30000 event -> handler -> updateFromRelays -> re-fetch -> relay echoes + // the SAME event -> handler -> ... The guard must break that. + let refreshCalls = 0; + manager.updateFromRelays = async () => { + refreshCalls += 1; + }; + + const listEvent = { id: "evt-A", pubkey: actor, created_at: 1000 }; + + // First delivery of a brand-new list event => exactly one refresh. + manager.handleSubscriptionListEvent(listEvent); + assert.equal(refreshCalls, 1, "first delivery of a new list event refreshes once"); + + // The live subscription re-delivers the SAME event (the echo). Must NOT refresh. + manager.handleSubscriptionListEvent({ ...listEvent }); + manager.handleSubscriptionListEvent({ ...listEvent }); + assert.equal(refreshCalls, 1, "echoes of the same event id must not re-refresh"); + + // An older event must also be ignored once we've applied a newer one. + manager.subsEventId = "evt-A"; + manager.subsEventCreatedAt = 1000; + manager.handleSubscriptionListEvent({ id: "evt-older", pubkey: actor, created_at: 500 }); + assert.equal(refreshCalls, 1, "an older list event must not trigger a refresh"); + + // A genuinely NEWER event DOES refresh again. + manager.handleSubscriptionListEvent({ id: "evt-B", pubkey: actor, created_at: 2000 }); + assert.equal(refreshCalls, 2, "a newer list event triggers exactly one more refresh"); +});