diff --git a/js/nostr/client.js b/js/nostr/client.js index d4f37555..4437e117 100644 --- a/js/nostr/client.js +++ b/js/nostr/client.js @@ -1167,6 +1167,26 @@ export class NostrClient { return this.connectionManager.getHealthyRelays(candidates); } + /** + * Resolve the relay set for deletion/tombstone events. + * + * A delete MUST reach every relay the video could have been published to, + * otherwise relays outside the set keep serving the original event and the + * "deleted" video resurrects on the next load. Normal publishes use the + * (uncapped) write set, but the delete paths historically used `this.relays` + * — the CAPPED read set (≤8) — so a video published broadly was only + * tombstoned on a subset. Mirror publishVideo: prefer the write set, fall + * back to the read set, then the bundled defaults. + */ + getDeletePublishRelays() { + const write = sanitizeRelayList( + Array.isArray(this.writeRelays) && this.writeRelays.length + ? this.writeRelays + : this.relays, + ); + return write.length ? write : Array.from(RELAY_URLS); + } + getDmDecryptCacheLimit() { return DM_DECRYPT_CACHE_LIMIT; } @@ -3038,7 +3058,7 @@ export class NostrClient { const signedEvent = await queueSignEvent(signer, event); const publishResults = await publishEventToRelays( this.pool, - this.relays, + this.getDeletePublishRelays(), signedEvent ); @@ -3366,7 +3386,7 @@ export class NostrClient { const signedDelete = await queueSignEvent(signer, deleteEvent); const publishResults = await publishEventToRelays( this.pool, - this.relays, + this.getDeletePublishRelays(), signedDelete, { waitForAll: true } ); @@ -4308,4 +4328,3 @@ export { resolveActiveSigner, shouldRequestExtensionPermissions, }; -const a = 1 diff --git a/tests/delete-publishes-to-write-relays.test.mjs b/tests/delete-publishes-to-write-relays.test.mjs new file mode 100644 index 00000000..f2542315 --- /dev/null +++ b/tests/delete-publishes-to-write-relays.test.mjs @@ -0,0 +1,97 @@ +// A delete must reach every relay the video could have been published to. +// +// Bug (SCN-delete-reaches-write-relays): normal video publishes fan out to the +// (uncapped) WRITE relay set, but the soft-delete tombstone (revertVideo) and +// the NIP-09 deletion both published only to `this.relays` — the CAPPED read +// set (<=8). A video published broadly was therefore tombstoned on just a +// subset; relays outside that subset kept serving the original event and the +// "deleted" video resurrected on the next load. +// +// Observable outcome asserted at the boundary: the relay URLs passed to +// pool.publish() when a delete is published. + +import assert from "node:assert/strict"; +import test from "node:test"; +import { NostrClient } from "../js/nostr/client.js"; + +if (!globalThis.WebSocket) { + globalThis.WebSocket = class MockWebSocket {}; +} + +const PK = "a".repeat(64); + +function makeClient(writeRelays, readRelays) { + const client = new NostrClient(); + client.writeRelays = writeRelays; + client.relays = readRelays; + + const published = []; + client.pool = { + // nostr-tools SimplePool.publish(urls, event) returns one promise per url. + publish: (urls) => { + published.push(...urls); + return urls.map(() => Promise.resolve("ok")); + }, + }; + // Stub the signer path so the test is hermetic (no extension / network). + client.ensureActiveSignerForPubkey = async () => {}; + client.signerManager = { + resolveActiveSigner: () => ({ + signEvent: async (evt) => ({ ...evt, id: "signed-tombstone", sig: "sig" }), + }), + }; + return { client, published }; +} + +const ORIGINAL = { + id: "orig-event-id", + pubkey: PK, + created_at: 1000, + content: JSON.stringify({ + version: 3, + deleted: false, + title: "Doomed video", + videoRootId: "root-1", + }), + tags: [["d", "video-d-tag"]], +}; + +test("getDeletePublishRelays prefers the full write set over the capped read set", () => { + const writeRelays = ["wss://w1", "wss://w2", "wss://w3"]; + const { client } = makeClient(writeRelays, ["wss://w1"]); + assert.deepEqual( + client.getDeletePublishRelays().sort(), + writeRelays.slice().sort(), + ); +}); + +test("getDeletePublishRelays falls back to the read set when there are no write relays", () => { + const readRelays = ["wss://r1", "wss://r2"]; + const { client } = makeClient([], readRelays); + assert.deepEqual(client.getDeletePublishRelays().sort(), readRelays.slice().sort()); +}); + +test("a soft-delete tombstone publishes to the WHOLE write set, not just the capped read subset", async () => { + const writeRelays = Array.from( + { length: 12 }, + (_, i) => `wss://write-${i}.example.com`, + ); + // The capped read set is a small subset — the bug published deletes here only. + const readRelays = writeRelays.slice(0, 2); + const { client, published } = makeClient(writeRelays, readRelays); + + await client.revertVideo(ORIGINAL, PK); + + for (const url of writeRelays) { + assert.ok( + published.includes(url), + `tombstone must reach write relay ${url} so the delete propagates everywhere`, + ); + } + // It must reach more than just the 2-relay capped read subset. + const distinct = new Set(published); + assert.ok( + distinct.size > readRelays.length, + `delete must not be limited to the capped read subset (hit ${distinct.size} relays)`, + ); +});