From afb6200b40e480410aa352d59c34692265eea3a4 Mon Sep 17 00:00:00 2001 From: thePR0M3TH3AN <53631862+PR0M3TH3AN@users.noreply.github.com> Date: Sat, 20 Jun 2026 12:38:51 -0400 Subject: [PATCH] fix(delete): publish tombstones to the write relays so deletes propagate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause of "deleted/tombstoned videos still show in the UI": normal video publishes fan out to the uncapped WRITE relay set, but both delete paths — the soft-delete tombstone (revertVideo) and the NIP-09 deletion — published only to this.relays, the CAPPED read set (<=8). A video published broadly was therefore tombstoned on just a subset; every relay outside that subset kept serving the original event, so the video resurrected on the next load (the tombstone's created_at was already bumped strictly-newer, so replacement worked — it just never reached the relays that mattered). Add getDeletePublishRelays() (prefer write set, fall back to read set, then bundled defaults) and use it for both delete publishes, mirroring publishVideo. Also removed a stray `const a = 1` debug leftover at end of client.js. Tests: getDeletePublishRelays prefers the write set / falls back to read; and a call-site test that revertVideo publishes the tombstone to the full 12-relay write set rather than the 2-relay capped read subset. Mutation-verified — restoring `this.relays` at the call sites fails the propagation test. Build + full lint green. Co-Authored-By: Claude Opus 4.8 --- js/nostr/client.js | 25 ++++- .../delete-publishes-to-write-relays.test.mjs | 97 +++++++++++++++++++ 2 files changed, 119 insertions(+), 3 deletions(-) create mode 100644 tests/delete-publishes-to-write-relays.test.mjs 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)`, + ); +});