From 23ddeba8ec2038304d0ee29959402d0a236a8940 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 4 Jun 2026 17:42:29 +0000 Subject: [PATCH] fix: sever child back-references when deleting a Note (NIP-09) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit deleteNote() removed the target from its parents, gatherers, and the cache map, but never cleared its own child collections nor dropped itself from its children's replyTo. That left a partial deletion: every child kept the removed shell alive through replyTo (a leak), and a reply resolved later via computeReplyTo would getOrCreateNote a *second* Note for the same id — breaking the one-Note-per-id invariant. Adds Note.detachFromChildren(), which clears the note's forward child collections (via removeAllChildNotes) and severs this note from each child's replyTo (keeping any other parents). deleteNote() now calls it before notes.remove(), so once the note leaves the map nothing points at the dead shell. Orphaned replies become roots, which is correct once their parent is hard-deleted from the cache. Adds detachFromChildren coverage to NotePruningReferenceTest. https://claude.ai/code/session_01RqJPYzmjb1pR3NBeH2yY3s --- .../amethyst/model/LocalCache.kt | 6 +++ .../amethyst/commons/model/Note.kt | 25 ++++++++++ .../commons/model/NotePruningReferenceTest.kt | 48 +++++++++++++++++++ 3 files changed, 79 insertions(+) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/LocalCache.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/LocalCache.kt index c9372f62d4..878b7fa0ca 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/LocalCache.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/LocalCache.kt @@ -1379,6 +1379,12 @@ object LocalCache : ILocalCache, ICacheProvider { getAnyChannel(deleteNote)?.removeNote(deleteNote) + // Sever the back-references from this note's children before it leaves the + // map. Otherwise each child keeps the removed shell alive through its replyTo + // (a partial deletion / leak) and a reply resolved later via computeReplyTo + // would resurrect a second Note for the same id. + deleteNote.detachFromChildren() + notes.remove(deleteNote.idHex) deleteNote.clearFlow() diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/Note.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/Note.kt index c3f469f1c3..f1bbaaeb7c 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/Note.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/model/Note.kt @@ -417,6 +417,31 @@ open class Note( return toBeRemoved } + /** + * Fully detach this note from the notes below it in the graph so it can be + * removed from the cache without leaving a partial deletion behind. It both + * clears this note's own child collections (via [removeAllChildNotes]) and + * drops this note from every child's [replyTo], so once this note leaves the + * cache map nothing keeps the dead shell alive. + * + * This matters for the NIP-09 delete path: without severing the child → + * parent `replyTo` links, the removed note leaks (held by each child) and a + * later reply resolved through `computeReplyTo` would `getOrCreateNote` a + * *second* Note for the same id — breaking the one-Note-per-id invariant. + * + * Returns the now-orphaned children (their other parents, if any, are kept). + */ + fun detachFromChildren(): List { + val children = removeAllChildNotes() + children.forEach { child -> + val parents = child.replyTo + if (parents != null && this in parents) { + child.replyTo = parents - this + } + } + return children + } + fun removeReaction(note: Note) { val tags = note.event?.tags ?: emptyArray() val reaction = note.event?.content?.firstFullCharOrEmoji(ImmutableListOfLists(tags)) ?: "+" diff --git a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/NotePruningReferenceTest.kt b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/NotePruningReferenceTest.kt index 1b065ac826..6618e33716 100644 --- a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/NotePruningReferenceTest.kt +++ b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/model/NotePruningReferenceTest.kt @@ -162,4 +162,52 @@ class NotePruningReferenceTest { assertTrue(newer.labels["nostr"]?.contains(labelNote) == true, "label must move to the newer version") assertSame(newer, labelNote.replyTo?.single()) } + + // ── Fix 4: deleteNote severs child back-references (no partial deletion) ── + + @Test + fun detachFromChildrenSeversReplyToAndClearsCollections() { + val parent = note("a1".repeat(32)) + val reply = note("b1".repeat(32)).apply { replyTo = listOf(parent) } + parent.addReply(reply) + + val detached = parent.detachFromChildren() + + assertTrue(reply in detached, "the child must be returned as detached") + assertTrue(parent.replies.isEmpty(), "parent must release its forward child references") + assertTrue( + reply.replyTo?.contains(parent) != true, + "child must no longer point back at the removed parent", + ) + } + + @Test + fun detachFromChildrenKeepsOtherParents() { + val deleted = note("a2".repeat(32)) + val survivor = note("c2".repeat(32)) + val reply = note("b2".repeat(32)).apply { replyTo = listOf(deleted, survivor) } + deleted.addReply(reply) + survivor.addReply(reply) + + deleted.detachFromChildren() + + assertEquals(listOf(survivor), reply.replyTo, "only the removed parent must be dropped from replyTo") + } + + @Test + fun detachFromChildrenSeversReactionAndZapSources() { + val parent = note("a3".repeat(32)) + val reaction = sourceNote("31".repeat(32)).apply { replyTo = listOf(parent) } + val zapSource = sourceNote("32".repeat(32)).apply { replyTo = listOf(parent) } + + parent.addOnchainZap(zapSource, "tx1", claimedSats = 1L, verifiedSats = 1L, status = OnchainZapStatus.CONFIRMED) + parent.addBoost(reaction) + + parent.detachFromChildren() + + assertTrue(parent.boosts.isEmpty()) + assertTrue(parent.onchainZaps.isEmpty()) + assertTrue(reaction.replyTo?.contains(parent) != true) + assertTrue(zapSource.replyTo?.contains(parent) != true) + } }