From fff2bef2d48e05088492afdb3c75ab5bbbfa9368 Mon Sep 17 00:00:00 2001 From: thePR0M3TH3AN <53631862+PR0M3TH3AN@users.noreply.github.com> Date: Sat, 20 Jun 2026 17:29:35 -0400 Subject: [PATCH] fix(watch-history): take newest version per month on read (delete sticks) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second root cause of "watch history delete doesn't work / list isn't getting overwritten": reads UNION items across every event the relays return, and a publish only reports success when ALL relays accept. With a flaky relay list the reduced (post-removal) event reaches some relays while others keep serving the old, larger copy — and the reader deduped only by event id, so both the stale and fresh versions of the same month (d-tag) were unioned, resurrecting removed items. Add dedupeNewestPerReplaceableAddress and apply it at both read union sites (the chunk-events path and the pointer-events path): keep only the newest event per d-tag (month) before unioning, so a lagging relay can't reintroduce removed items. Distinct months are all preserved; events without a d-tag are kept by id. Tests + mutation-verified: newest-per-month wins (incl. the fewer-items removal case), distinct months preserved, newest-first ordering, no-d-tag kept by id. Reverting to id-dedup fails the resurrection scenarios. Build + full lint green (watchHistory.js back under file-size budget). Co-Authored-By: Claude Opus 4.8 --- js/nostr/watchHistory.js | 19 ++++----- js/nostr/watchHistoryDedup.js | 39 ++++++++++++++++++ tests/watch-history-dedup.test.mjs | 64 ++++++++++++++++++++++++++++++ 3 files changed, 113 insertions(+), 9 deletions(-) create mode 100644 js/nostr/watchHistoryDedup.js create mode 100644 tests/watch-history-dedup.test.mjs diff --git a/js/nostr/watchHistory.js b/js/nostr/watchHistory.js index 67d678e8..6b232cf3 100644 --- a/js/nostr/watchHistory.js +++ b/js/nostr/watchHistory.js @@ -40,6 +40,7 @@ import { } from "../utils/pointerNormalization.js"; import { pMap } from "../utils/asyncUtils.js"; import { selectNewestListEvent } from "./listEventOrdering.js"; +import { dedupeNewestPerReplaceableAddress } from "./watchHistoryDedup.js"; import { getCachedChunkPlaintext, setCachedChunkPlaintext, @@ -2030,9 +2031,10 @@ class WatchHistoryManager { // count against the limit. Only uncached, genuinely-encrypted chunks // consume the budget; any beyond it are deferred (the caller can request // older chunks later, which re-uses the chunk cache for the rest). - const sortedChunkEvents = [...allChunkEvents].sort( - (a, b) => (Number(b?.created_at) || 0) - (Number(a?.created_at) || 0), - ); + // Keep only the NEWEST event per month (d-tag) so a lagging relay's + // stale copy can't resurrect removed items in the union. Newest-first, + // so the decrypt-budget ordering below is preserved. + const sortedChunkEvents = dedupeNewestPerReplaceableAddress(allChunkEvents); const chunkEvents = []; let uncachedBudget = chunkDecryptLimit; for (const event of sortedChunkEvents) { @@ -2105,13 +2107,12 @@ class WatchHistoryManager { const eventsToProcess = Array.isArray(eventToProcess) ? eventToProcess : [eventToProcess]; const collectedItems = []; - // Deduplicate events by ID - const uniqueEvents = new Map(); - for (const ev of eventsToProcess) { - if (ev && ev.id) uniqueEvents.set(ev.id, ev); - } + // Deduplicate by replaceable address (newest per d-tag), NOT by id — a stale + // version of the same month from a lagging relay must not be unioned in + // alongside the fresh one (that resurrects removed items). + const uniqueEvents = dedupeNewestPerReplaceableAddress(eventsToProcess); - for (const event of uniqueEvents.values()) { + for (const event of uniqueEvents) { const fallbackPointers = extractPointerItemsFromEvent(event); const ciphertext = typeof event.content === "string" ? event.content : ""; diff --git a/js/nostr/watchHistoryDedup.js b/js/nostr/watchHistoryDedup.js new file mode 100644 index 00000000..6f26b49a --- /dev/null +++ b/js/nostr/watchHistoryDedup.js @@ -0,0 +1,39 @@ +// Watch-history events are PARAMETERIZED REPLACEABLE (kind + pubkey + d-tag, +// where the d-tag is the month). When reading across multiple relays we can +// receive several versions of the same month — e.g. a relay that missed the +// latest removal/clear publish still serves the old, larger copy. The reader +// unions items across events, so unless we keep only the NEWEST version per +// address, a lagging relay resurrects items the user just removed. +// +// This collapses a set of events to the newest per replaceable address (by +// created_at), so the union downstream reflects the user's latest intent. + +/** + * @param {Array} events raw nostr events + * @returns {Array} newest event per d-tag (events without a d-tag are + * kept individually by id), sorted newest-first + */ +export function dedupeNewestPerReplaceableAddress(events) { + const sorted = (Array.isArray(events) ? events : []) + .filter((event) => event && typeof event === "object") + .slice() + .sort((a, b) => (Number(b?.created_at) || 0) - (Number(a?.created_at) || 0)); + + const seen = new Set(); + const out = []; + for (const event of sorted) { + const dTag = Array.isArray(event.tags) + ? event.tags.find((tag) => Array.isArray(tag) && tag[0] === "d")?.[1] || "" + : ""; + // Distinct months have distinct d-tags, so deduping by d-tag keeps one + // (newest) per month while preserving every month. Events with no d-tag + // can't be addressed, so they're kept individually by id. + const key = dTag ? `d:${dTag}` : `id:${event.id}`; + if (seen.has(key)) { + continue; + } + seen.add(key); + out.push(event); + } + return out; +} diff --git a/tests/watch-history-dedup.test.mjs b/tests/watch-history-dedup.test.mjs new file mode 100644 index 00000000..72a01b57 --- /dev/null +++ b/tests/watch-history-dedup.test.mjs @@ -0,0 +1,64 @@ +// Reading watch history unions items across the events relays return. Watch- +// history months are replaceable (d-tag = month), so when relays disagree — a +// lagging relay still serving the pre-removal copy — the reader must keep only +// the NEWEST version per month, or removed items get resurrected by the union. + +import assert from "node:assert/strict"; +import test from "node:test"; +import { dedupeNewestPerReplaceableAddress } from "../js/nostr/watchHistoryDedup.js"; + +const ev = (id, dTag, created_at) => ({ + id, + created_at, + tags: [["d", dTag]], +}); + +test("keeps only the newest event per month (d-tag)", () => { + // Old full copy (created 100) and new reduced copy (created 200) of 2026-06, + // as if a lagging relay still served the old one alongside the fresh one. + const out = dedupeNewestPerReplaceableAddress([ + ev("old", "2026-06", 100), + ev("new", "2026-06", 200), + ]); + assert.equal(out.length, 1); + assert.equal(out[0].id, "new", "the newest version of the month wins"); +}); + +test("preserves distinct months", () => { + const out = dedupeNewestPerReplaceableAddress([ + ev("a", "2026-06", 200), + ev("b", "2026-05", 100), + ]); + assert.deepEqual( + out.map((e) => e.id).sort(), + ["a", "b"], + "different months are all kept", + ); +}); + +test("result is sorted newest-first", () => { + const out = dedupeNewestPerReplaceableAddress([ + ev("older", "2026-05", 100), + ev("newer", "2026-06", 300), + ev("mid", "2026-04", 200), + ]); + assert.deepEqual(out.map((e) => e.id), ["newer", "mid", "older"]); +}); + +test("events without a d-tag are kept individually (by id)", () => { + const out = dedupeNewestPerReplaceableAddress([ + { id: "x", created_at: 1, tags: [] }, + { id: "y", created_at: 2, tags: [] }, + ]); + assert.equal(out.length, 2); +}); + +test("the newest month copy with FEWER items is what survives (removal sticks)", () => { + // Models the real bug: old copy has [a,b]; the removal published [a] to some + // relays. Dedup must surface only the [a] event so the union drops b. + const old = { id: "old", created_at: 100, tags: [["d", "2026-06"]], items: ["a", "b"] }; + const fresh = { id: "fresh", created_at: 200, tags: [["d", "2026-06"]], items: ["a"] }; + const out = dedupeNewestPerReplaceableAddress([old, fresh]); + assert.equal(out.length, 1); + assert.deepEqual(out[0].items, ["a"], "only the post-removal copy contributes"); +});