mirror of
https://github.com/PR0M3TH3AN/bitvid.git
synced 2026-10-27 07:52:24 +00:00
fix(subscriptions): guard the subscription-list refresh loop
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 <[email protected]>
This commit is contained in:
1 parent
1f07538c66
commit
10ea4f6a22
3 files changed
+77
No files matched your search
@@ -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(
|
||||
|
||||
@@ -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}`);
|
||||
|
||||
@@ -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");
|
||||
});
|
||||
Reference in new issue
Block a user