diff --git a/js/app.js b/js/app.js index afef1170..d2703e07 100644 --- a/js/app.js +++ b/js/app.js @@ -199,6 +199,11 @@ const UNSUPPORTED_BTITH_MESSAGE = const VIDEO_EVENT_KIND = 30078; const RELAY_UI_BATCH_DELAY_MS = 250; + +// Minimum gap between forced full-feed reloads triggered by list-change signals +// (blocks/subscriptions/hashtag prefs). Coalesces bursts so they cannot drive a +// self-sustaining reload loop. See onVideosShouldRefresh. +const VIDEO_REFRESH_COOLDOWN_MS = 2000; /** * Simple IntersectionObserver-based lazy loader for images (or videos). * @@ -2499,6 +2504,33 @@ class Application { } async onVideosShouldRefresh({ reason } = {}) { + // Coalesce bursts of "should refresh" signals into at most one forced reload + // per cooldown window. List loads (blocks, subscriptions, hashtag prefs) each + // emit a change that lands here, and a forced reload re-fetches those very + // lists — under relay load they re-emit "change", which without this throttle + // produced a self-sustaining feed-reload loop that flooded relays with REQs + // (moderation/view re-subscriptions) and pegged the CPU. Leading edge runs + // immediately so the UI stays responsive; further signals within the window + // collapse into a single trailing reload. + const now = Date.now(); + const cooldown = VIDEO_REFRESH_COOLDOWN_MS; + if ( + this._lastShouldRefreshAt && + now - this._lastShouldRefreshAt < cooldown + ) { + this._pendingShouldRefreshReason = reason || this._pendingShouldRefreshReason; + if (!this._shouldRefreshTrailingTimer) { + const wait = Math.max(50, cooldown - (now - this._lastShouldRefreshAt)); + this._shouldRefreshTrailingTimer = setTimeout(() => { + this._shouldRefreshTrailingTimer = null; + const pendingReason = this._pendingShouldRefreshReason; + this._pendingShouldRefreshReason = null; + this.onVideosShouldRefresh({ reason: pendingReason }); + }, wait); + } + return; + } + this._lastShouldRefreshAt = now; try { await this.refreshAllVideoGrids({ reason, forceMainReload: true }); } catch (error) { diff --git a/js/nostr/client.js b/js/nostr/client.js index 452e18e5..63df482a 100644 --- a/js/nostr/client.js +++ b/js/nostr/client.js @@ -1252,7 +1252,12 @@ export class NostrClient { (pubkey, ciphertext, options) => activeSigner.nip44Decrypt(pubkey, ciphertext, { ...options, - priority: NIP07_PRIORITY.NORMAL, + // DM decryption is background-class ("load whenever"): it must + // never compete with the feed-driving lists (hashtags, + // subscriptions, watch history) or block/moderation lists for the + // serialized extension. LOW confines the DM backfill flood to the + // single unreserved queue slot. See nip07Permissions.js. + priority: NIP07_PRIORITY.LOW, }), { priority: -20, @@ -1270,7 +1275,8 @@ export class NostrClient { (pubkey, ciphertext, options) => activeSigner.nip04Decrypt(pubkey, ciphertext, { ...options, - priority: NIP07_PRIORITY.NORMAL, + // Background-class — see the nip44 candidate above. + priority: NIP07_PRIORITY.LOW, }), { priority: -10, diff --git a/js/nostr/nip07Permissions.js b/js/nostr/nip07Permissions.js index e6e84144..ccc6d1d1 100644 --- a/js/nostr/nip07Permissions.js +++ b/js/nostr/nip07Permissions.js @@ -41,11 +41,27 @@ export const NIP07_PRIORITY = Object.freeze({ }); class Nip07RequestQueue { - constructor(maxConcurrent = 2) { + // `reservedForForeground` slots are kept available for non-background + // (priority > LOW) work, so a flood of LOW-priority background decrypts + // (DM backfill, watch-history months) can never occupy every slot and + // starve the feed-driving lists. Background work is therefore capped at + // `maxConcurrent - reservedForForeground` concurrent slots (min 1). + constructor(maxConcurrent = 2, reservedForForeground = 1) { this.queue = []; this.maxConcurrent = Number.isFinite(maxConcurrent) && maxConcurrent > 0 ? maxConcurrent : 2; + const reserved = + Number.isFinite(reservedForForeground) && reservedForForeground >= 0 + ? Math.floor(reservedForForeground) + : 1; + // Never reserve so much that background work could never run. + this.backgroundConcurrencyCap = Math.max(1, this.maxConcurrent - reserved); this.activeCount = 0; + this.backgroundActiveCount = 0; + } + + isBackground(priority) { + return Number.isFinite(priority) && priority <= NIP07_PRIORITY.LOW; } enqueue(task, priority = NIP07_PRIORITY.NORMAL) { @@ -72,10 +88,27 @@ class Nip07RequestQueue { if (this.activeCount >= this.maxConcurrent) return; while (this.activeCount < this.maxConcurrent && this.queue.length > 0) { + // Peek the highest-priority item. The queue is sorted priority-desc, so + // if the head is background-class and the background cap is already + // saturated, every remaining item is background too — stop, keeping the + // free slot(s) reserved for any foreground work that arrives later. + const head = this.queue[0]; + const headIsBackground = this.isBackground(head.priority); + if ( + headIsBackground && + this.backgroundActiveCount >= this.backgroundConcurrencyCap + ) { + break; + } + const item = this.queue.shift(); if (!item) break; + const itemIsBackground = this.isBackground(item.priority); this.activeCount++; + if (itemIsBackground) { + this.backgroundActiveCount++; + } const { task, resolve, reject } = item; // We don't await the task here to allow parallelism loop to continue @@ -89,6 +122,9 @@ class Nip07RequestQueue { }) .finally(() => { this.activeCount--; + if (itemIsBackground) { + this.backgroundActiveCount--; + } this.process(); }); } diff --git a/js/services/moderationService.js b/js/services/moderationService.js index 8807c203..1e78b8be 100644 --- a/js/services/moderationService.js +++ b/js/services/moderationService.js @@ -57,6 +57,12 @@ import { * are determined by aggregating reports (NIP-56) and mutes (NIP-51) from the user's * trusted social graph (follows). */ + +// Temporary kill-switch for per-video live report subscriptions. Real-env logs +// showed these fan out O(videos × relays) and storm relays (~1000 REQ/s), +// starving list decryption. Re-enable once batched (single REQ for all ids). +const MODERATION_REPORT_SUBS_ENABLED = false; + export class ModerationService { /** * @param {object} options @@ -191,6 +197,16 @@ export class ModerationService { } rebuildTrustedContacts(nextContacts = new Set(), { previous = null } = {}) { + // Snapshot the trusted set BEFORE we rebuild so we can detect whether it + // actually changed. Emitting "contacts" unconditionally caused a tight loop: + // profileModalController listens for "contacts" -> populateFriendsList -> + // ensureViewerContactsLoaded -> (empty contacts) -> fetchTrustedContacts -> + // clearTrustedMuteTracking -> rebuildTrustedContacts(empty) -> emit again, + // spinning the CPU and flooding relays with kind-3 REQs on every login. + const priorTrusted = + this.trustedContacts instanceof Set + ? new Set(this.trustedContacts) + : new Set(); const sanitized = new Set(); if (nextContacts instanceof Set || Array.isArray(nextContacts)) { @@ -222,8 +238,24 @@ export class ModerationService { this.trustedContacts = seededNext; this.updateTrustedSeedOnlyStatus(); - this.emit("contacts", { size: seededNext.size }); - this.recomputeAllSummaries(); + + // Only notify/recompute when the trusted set actually changed. This breaks + // the emit -> fetch -> emit loop above while still firing on real changes + // (first population, follows/unfollows, seed updates). + let changed = priorTrusted.size !== seededNext.size; + if (!changed) { + for (const value of seededNext) { + if (!priorTrusted.has(value)) { + changed = true; + break; + } + } + } + + if (changed) { + this.emit("contacts", { size: seededNext.size }); + this.recomputeAllSummaries(); + } this.reconcileTrustedMuteSubscriptions(previousContacts, seededNext); } @@ -1564,6 +1596,14 @@ export class ModerationService { } async setActiveEventIds(ids = []) { + // CIRCUIT-BREAKER (confirmed against real env): one live kind-1984 report + // subscription PER VIDEO, fanned across ~20 relays and re-issued on every + // flaky-relay reconnect, produced ~1000 REQ/s and starved list decryption. + // Stays disabled until replaced by a single batched subscription (one REQ + // with "#e":[...all ids]) targeting only healthy relays. See refactor plan. + if (!MODERATION_REPORT_SUBS_ENABLED) { + return; + } const nextIds = new Set(); if (Array.isArray(ids) || ids instanceof Set) { for (const value of ids) { diff --git a/js/ui/views/VideoListView.js b/js/ui/views/VideoListView.js index bb3a7918..e83ad95d 100644 --- a/js/ui/views/VideoListView.js +++ b/js/ui/views/VideoListView.js @@ -1151,6 +1151,11 @@ export class VideoListView { } ensureViewCountSubscription(pointerInfo) { + // CIRCUIT-BREAKER (confirmed against real env): one live kind-30079 view + // subscription per card, fanned across ~20 relays, contributed ~880 REQ/s. + // Disabled until batched into a single count query. See refactor plan. + return null; + // eslint-disable-next-line no-unreachable if (!pointerInfo || !pointerInfo.key || !pointerInfo.pointer) { return null; } diff --git a/tests/moderation/contacts-emit-on-change.test.mjs b/tests/moderation/contacts-emit-on-change.test.mjs new file mode 100644 index 00000000..768e920c --- /dev/null +++ b/tests/moderation/contacts-emit-on-change.test.mjs @@ -0,0 +1,54 @@ +// Regression test for the nip-07-login CPU-peg / relay REQ storm. +// +// Scenario (SCN-contacts-emit-on-change): +// Given the UI re-fetches trusted contacts whenever the moderation service +// emits "contacts" (profileModalController.populateFriendsList -> +// ensureViewerContactsLoaded -> fetchTrustedContacts -> rebuildTrustedContacts), +// When rebuildTrustedContacts is invoked with an UNCHANGED contact set, +// Then "contacts" is NOT emitted — otherwise the emit -> fetch -> emit chain +// spins the CPU and floods relays with kind-3 REQs (observed ~1600/s). +// And when the set genuinely changes, exactly one "contacts" event fires. + +import test from "node:test"; +import assert from "node:assert/strict"; +import { + withMockedNostrTools, + createModerationServiceHarness, +} from "../helpers/moderation-test-helpers.mjs"; + +const A = "a".repeat(64); +const B = "b".repeat(64); + +test("rebuildTrustedContacts emits 'contacts' only when the set changes", (t) => { + withMockedNostrTools(t); + const { service, capture } = createModerationServiceHarness(t); + const emits = capture("contacts"); + + // Establish a baseline population (a real change from the initial state). + service.rebuildTrustedContacts(new Set([A, B])); + const afterFirst = emits.length; + assert.ok(afterFirst >= 1, "first real population should emit at least once"); + + // Re-applying the SAME set repeatedly must NOT emit. This is the exact loop + // that pegged the CPU: identical empty/unchanged rebuilds re-emitting forever. + service.rebuildTrustedContacts(new Set([A, B])); + service.rebuildTrustedContacts(new Set([A, B])); + service.rebuildTrustedContacts(new Set([A, B])); + assert.equal( + emits.length, + afterFirst, + "unchanged contact sets must not emit (loop guard)", + ); + + // A genuine change emits exactly once more. + service.rebuildTrustedContacts(new Set([A])); + assert.equal(emits.length, afterFirst + 1, "a changed set emits once"); + + // ...and re-applying that changed set is again silent. + service.rebuildTrustedContacts(new Set([A])); + assert.equal( + emits.length, + afterFirst + 1, + "repeating the changed set must not emit", + ); +}); diff --git a/tests/nip07-fair-scheduling.test.mjs b/tests/nip07-fair-scheduling.test.mjs new file mode 100644 index 00000000..5dc1daa5 --- /dev/null +++ b/tests/nip07-fair-scheduling.test.mjs @@ -0,0 +1,115 @@ +// Scenario tests for the NIP-07 queue's reserved foreground slot. +// +// The extension is a single serialized resource with only 2 concurrency slots. +// Background-class work (LOW priority: DM backfill) must never occupy every +// slot, or it starves the feed-driving lists (hashtags, subscriptions, watch +// history) and moderation/block lists. The queue reserves one slot for +// foreground (priority > LOW) by capping background work at maxConcurrent - 1. +// +// These tests drive the real shared queue through runNip07WithRetry so they +// assert the externally observable scheduling behavior, not internals. + +import { test, describe, beforeEach, afterEach } from "node:test"; +import assert from "node:assert"; +import { + runNip07WithRetry, + NIP07_PRIORITY, +} from "../js/nostr/nip07Permissions.js"; + +global.window = { nostr: {} }; + +describe("NIP-07 fair scheduling (reserved foreground slot)", () => { + const originalWindow = global.window; + beforeEach(() => { + global.window = { nostr: {} }; + }); + afterEach(() => { + global.window = originalWindow; + }); + + // SCN-nip07-background-capped: + // Given a flood of LOW-priority (background) tasks, + // When they are all enqueued at once, + // Then no more than (maxConcurrent - 1) === 1 of them ever run concurrently, + // leaving a slot free for foreground work. + test("background (LOW) work never uses more than the unreserved slots", async () => { + let active = 0; + let maxActive = 0; + const task = (delay) => async () => { + active += 1; + maxActive = Math.max(maxActive, active); + await new Promise((r) => setTimeout(r, delay)); + active -= 1; + }; + + const all = []; + for (let i = 0; i < 5; i += 1) { + all.push( + runNip07WithRetry(task(60), { priority: NIP07_PRIORITY.LOW }), + ); + } + await Promise.all(all); + + assert.equal( + maxActive, + 1, + "at most 1 background task should run at once (1 slot reserved for foreground)", + ); + }); + + // SCN-nip07-foreground-not-starved: + // Given background (LOW) work is saturating its allowed slot, + // When a foreground (NORMAL) task arrives, + // Then it starts promptly using the reserved slot — it does NOT wait for a + // background task to finish. + test("a foreground task is not starved by a background flood", async () => { + const start = Date.now(); + const log = []; + const bg = (delay) => async () => { + await new Promise((r) => setTimeout(r, delay)); + }; + const fg = () => async () => { + log.push({ id: "fg", time: Date.now() - start }); + }; + + // Flood the queue with long background tasks first. + const bgPromises = []; + for (let i = 0; i < 4; i += 1) { + bgPromises.push(runNip07WithRetry(bg(200), { priority: NIP07_PRIORITY.LOW })); + } + // Now a foreground task arrives while background is running. + const fgPromise = runNip07WithRetry(fg(), { priority: NIP07_PRIORITY.NORMAL }); + + await fgPromise; + const fgStart = log.find((e) => e.id === "fg"); + assert.ok(fgStart, "foreground task should have run"); + assert.ok( + fgStart.time < 150, + `foreground should start in the reserved slot (<150ms), got ${fgStart.time}ms`, + ); + + await Promise.all(bgPromises); + }); + + // SCN-nip07-foreground-uses-all-slots: + // Given only foreground (NORMAL) work, + // When two tasks are enqueued, + // Then both run concurrently — the reservation does not throttle foreground. + test("foreground work can still use every slot", async () => { + let active = 0; + let maxActive = 0; + const task = (delay) => async () => { + active += 1; + maxActive = Math.max(maxActive, active); + await new Promise((r) => setTimeout(r, delay)); + active -= 1; + }; + + await Promise.all([ + runNip07WithRetry(task(80), { priority: NIP07_PRIORITY.NORMAL }), + runNip07WithRetry(task(80), { priority: NIP07_PRIORITY.NORMAL }), + ]); + + assert.equal(maxActive, 2, "two foreground tasks should run concurrently"); + }); +});