From 28b23b5e63b2ac925dfaba34cebaffc9982b0e85 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 25 Apr 2026 12:32:11 +0000 Subject: [PATCH 1/5] fix(quartz/sqlite): NIP compliance and migration safety in event store MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - NIP-09 (#2): a-tag and replaceable deletes now respect `created_at <= deletion.created_at` so a stale deletion request cannot remove a newer addressable that legitimately replaced it. `+created_at` hint on the addressable path keeps the d_tag-selective index in use. - NIP-01 (#1): replaceable / addressable triggers now apply the lexical-id tiebreaker — when two events share `created_at`, the one with the lexicographically smaller id wins, matching the spec. - Schema (#5): FullTextSearchModule.versionFinder probes FTS support with a dummy table, but used to leave it behind. The first v1->v2 upgrade then failed because re-running create() would hit "already exists". Now we drop the probe table immediately and defensively clean up any stragglers. - Schema (#6): onCreate / onUpgrade and the matching `setUserVersion` are now wrapped in a single transaction so a partial migration cannot leave the DB with mismatched user_version and schema. - SQL DSL (#4): Condition.NotEquals(null) now produces `IS NOT NULL` instead of `IS NULL`. - Doc fix (#13): swapped vacuum/analyse comments now describe the right command. Tests: same-`created_at` tiebreaker for replaceables and addressables, NIP-09 created_at window for a-tag deletes, schema drop+recreate idempotency (covers the FTS dummy-table regression). https://claude.ai/code/session_01X163Nr31vGkvAXoJ3JgMov --- .../store/sqlite/AddressableModule.kt | 21 +++-- .../store/sqlite/DeletionRequestModule.kt | 18 +++-- .../store/sqlite/FullTextSearchModule.kt | 37 ++++++--- .../store/sqlite/ReplaceableModule.kt | 17 ++-- .../store/sqlite/SQLiteEventStore.kt | 20 +++-- .../store/sqlite/sql/SqlSelectionBuilder.kt | 2 +- .../nip01Core/store/sqlite/AddressableTest.kt | 66 ++++++++++++++++ .../nip01Core/store/sqlite/BasicTest.kt | 19 +++++ .../nip01Core/store/sqlite/DeletionTest.kt | 79 ++++++++++++++++--- .../nip01Core/store/sqlite/ReplaceableTest.kt | 37 +++++++++ 10 files changed, 265 insertions(+), 51 deletions(-) diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/AddressableModule.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/AddressableModule.kt index 4181a34d3..a27a11535 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/AddressableModule.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/AddressableModule.kt @@ -32,12 +32,14 @@ class AddressableModule : IModule { """.trimIndent(), ) - // deletes older addressables when inserting new ones - // if a newer addressable is inserted the unique index - // above will be triggered. Delete cascade will take - // care of the event_tags table - // the duplicate: kind >= 30000 AND kind < 40000 - // helps SQLlite find the index above + // deletes older addressables when inserting new ones. + // Per NIP-01, the "older" version of an addressable is the one + // with the smaller created_at, OR with equal created_at and + // a lexicographically larger id (lowest id wins). + // If a newer addressable is inserted the unique index above + // will be triggered. Delete cascade will take care of the + // event_tags table. The duplicate `kind >= 30000 AND kind < 40000` + // clause helps SQLite pick the partial unique index above. db.execSQL( """ CREATE TRIGGER delete_older_addressable_event @@ -50,8 +52,11 @@ class AddressableModule : IModule { event_headers.kind = NEW.kind AND event_headers.pubkey = NEW.pubkey AND event_headers.d_tag = NEW.d_tag AND - event_headers.created_at < NEW.created_at AND - event_headers.kind >= 30000 AND event_headers.kind < 40000; + event_headers.kind >= 30000 AND event_headers.kind < 40000 AND + ( + event_headers.created_at < NEW.created_at OR + (event_headers.created_at = NEW.created_at AND event_headers.id > NEW.id) + ); END; """.trimIndent(), ) diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/DeletionRequestModule.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/DeletionRequestModule.kt index 1b50598c1..30bb25f99 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/DeletionRequestModule.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/DeletionRequestModule.kt @@ -87,7 +87,7 @@ class DeletionRequestModule( val idValues = event.deleteEventIds() val addresses = event.deleteAddresses() - deleteSQL(event.pubKey, idValues, addresses, hasher(db)).forEach { delete -> + deleteSQL(event.pubKey, event.createdAt, idValues, addresses, hasher(db)).forEach { delete -> db.execSQL(delete.sql, delete.args) } } @@ -97,10 +97,14 @@ class DeletionRequestModule( * Creates a Delete statement that correctly deletes by id, * by address and by replaceable (no d-tag) using each index * appropriately, including GiftWraps where the owner is the - * p-tag (via event_header.pubkey_owner_hash) + * p-tag (via event_header.pubkey_owner_hash). + * + * Per NIP-09, address-based deletes (a-tag) MUST only remove + * events with `created_at <= deletion.created_at`. */ fun deleteSQL( pubkey: HexKey, + createdAt: Long, idValues: List, addresses: List
, hasher: com.vitorpamplona.quartz.nip01Core.store.sqlite.TagNameValueHasher, @@ -156,9 +160,10 @@ class DeletionRequestModule( WHERE ( $addressableParams ) AND - kind >= 30000 AND kind < 40000 + kind >= 30000 AND kind < 40000 AND + +created_at <= ? """.trimIndent(), - addressableValues.toTypedArray(), + (addressableValues + createdAt).toTypedArray(), ) } else { null @@ -172,9 +177,10 @@ class DeletionRequestModule( WHERE kind IN ($replaceableKindsParam) AND pubkey = ? AND - ((kind in (0,3)) OR (kind >= 10000 AND kind < 20000)) + ((kind in (0,3)) OR (kind >= 10000 AND kind < 20000)) AND + created_at <= ? """.trimIndent(), - replaceableKindsValues.plus(pubkey).toTypedArray(), + (replaceableKindsValues.plus(pubkey) + createdAt).toTypedArray(), ) } else { null diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/FullTextSearchModule.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/FullTextSearchModule.kt index a7884a224..f39da4fe8 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/FullTextSearchModule.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/FullTextSearchModule.kt @@ -31,7 +31,7 @@ class FullTextSearchModule : IModule { val contentName = "content" override fun create(db: SQLiteConnection) { - val ftsVersion = FullTextSearchModule().versionFinder(db) + val ftsVersion = versionFinder(db) db.execSQL( """ CREATE VIRTUAL TABLE $tableName @@ -77,19 +77,34 @@ class FullTextSearchModule : IModule { } } - fun versionFinder(db: SQLiteConnection): Int = - try { + fun versionFinder(db: SQLiteConnection): Int { + // Defensive cleanup in case a previous probe left these behind + // (e.g. a partial create() during an upgrade) — without this, + // the next CREATE VIRTUAL TABLE would fail with "already exists". + db.execSQL("DROP TABLE IF EXISTS dummy_fts5") + db.execSQL("DROP TABLE IF EXISTS dummy_fts4") + db.execSQL("DROP TABLE IF EXISTS dummy_fts3") + + val (table, version) = try { - db.execSQL("CREATE VIRTUAL TABLE dummy_fts5 USING fts5(dummy)") - 5 + try { + db.execSQL("CREATE VIRTUAL TABLE dummy_fts5 USING fts5(dummy)") + "dummy_fts5" to 5 + } catch (e: SQLiteException) { + db.execSQL("CREATE VIRTUAL TABLE dummy_fts4 USING fts4(dummy)") + "dummy_fts4" to 4 + } } catch (e: SQLiteException) { - db.execSQL("CREATE VIRTUAL TABLE dummy_fts4 USING fts4(dummy)") - 4 + db.execSQL("CREATE VIRTUAL TABLE dummy_fts3 USING fts3(dummy)") + "dummy_fts3" to 3 } - } catch (e: SQLiteException) { - db.execSQL("CREATE VIRTUAL TABLE dummy_fts3 USING fts3(dummy)") - 3 - } + + // We only needed the table to probe FTS support — drop it now so + // re-running create() on an already-probed DB stays idempotent. + db.execSQL("DROP TABLE $table") + + return version + } override fun deleteAll(db: SQLiteConnection) { db.execSQL("DELETE FROM event_fts") diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/ReplaceableModule.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/ReplaceableModule.kt index 4ed253b13..cdd898e9e 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/ReplaceableModule.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/ReplaceableModule.kt @@ -32,10 +32,13 @@ class ReplaceableModule : IModule { """.trimIndent(), ) - // deletes older addressables when inserting new ones - // if a newer addressable is inserted the unique index - // above will be triggered. Delete cascade will take - // care of the event_tags table + // deletes older replaceables when inserting new ones. + // Per NIP-01, the "older" version of a replaceable is the one + // with the smaller created_at, OR with equal created_at and + // a lexicographically larger id (lowest id wins). + // If a newer replaceable is inserted the unique index above + // will be triggered. Delete cascade will take care of the + // event_tags table. db.execSQL( """ CREATE TRIGGER delete_older_replaceable_event @@ -43,12 +46,14 @@ class ReplaceableModule : IModule { FOR EACH ROW WHEN (NEW.kind IN (0, 3)) OR (NEW.kind >= 10000 AND NEW.kind < 20000) BEGIN - -- Delete older records if this is the newest DELETE FROM event_headers WHERE event_headers.kind = NEW.kind AND event_headers.pubkey = NEW.pubkey AND - event_headers.created_at < NEW.created_at; + ( + event_headers.created_at < NEW.created_at OR + (event_headers.created_at = NEW.created_at AND event_headers.id > NEW.id) + ); END; """.trimIndent(), ) diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/SQLiteEventStore.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/SQLiteEventStore.kt index faa9e7cf3..60da85fe9 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/SQLiteEventStore.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/SQLiteEventStore.kt @@ -108,11 +108,15 @@ class SQLiteEventStore( val currentVersion = getUserVersion(db) if (currentVersion == 0) { - onCreate(db) - setUserVersion(db, DATABASE_VERSION) + db.transaction { + onCreate(this) + setUserVersion(this, DATABASE_VERSION) + } } else if (currentVersion < DATABASE_VERSION) { - onUpgrade(db, currentVersion, DATABASE_VERSION) - setUserVersion(db, DATABASE_VERSION) + db.transaction { + onUpgrade(this, currentVersion, DATABASE_VERSION) + setUserVersion(this, DATABASE_VERSION) + } } return db @@ -160,16 +164,16 @@ class SQLiteEventStore( } suspend fun vacuum() { - // 1. ANALYZE: Collects statistics about tables and indices - // to help the query planner optimize queries. + // VACUUM: Rebuilds the database file, reclaiming unused space + // and reducing fragmentation. withContext(Dispatchers.IO) { connection.execSQL("VACUUM") } } suspend fun analyse() { - // 2. VACUUM: Rebuilds the database file, reclaiming unused space - // and reducing fragmentation. + // ANALYZE: Collects statistics about tables and indices + // to help the query planner optimize queries. withContext(Dispatchers.IO) { connection.execSQL("ANALYZE") } diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/sql/SqlSelectionBuilder.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/sql/SqlSelectionBuilder.kt index be9bf59c0..949ceb9c0 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/sql/SqlSelectionBuilder.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/sql/SqlSelectionBuilder.kt @@ -60,7 +60,7 @@ class SqlSelectionBuilder( is Condition.NotEquals -> { if (cond.value == null) { - "${cond.column} IS NULL" + "${cond.column} IS NOT NULL" } else { selectionArgs.add(cond.value.toString()) "${cond.column} != ?" diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/AddressableTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/AddressableTest.kt index 12c1209b5..759d1b223 100644 --- a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/AddressableTest.kt +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/AddressableTest.kt @@ -89,6 +89,72 @@ class AddressableTest : BaseDBTest() { db.assertQuery(null, Filter(ids = listOf(version1.id))) } + @Test + fun testReplacingSameCreatedAtLowerIdWins() = + forEachDB { db -> + val time = TimeUtils.now() + // Two events with the same (kind, pubkey, d_tag, created_at) but + // different content -> different ids. NIP-01 says the lower-id wins. + val a = + signer.sign( + LongTextNoteEvent.build( + "version A", + "title", + dTag = "tie", + createdAt = time, + ), + ) + val b = + signer.sign( + LongTextNoteEvent.build( + "version B", + "title", + dTag = "tie", + createdAt = time, + ), + ) + val (winner, loser) = if (a.id < b.id) a to b else b to a + + db.insert(loser) + db.assertQuery(loser, Filter(ids = listOf(loser.id))) + + db.insert(winner) + db.assertQuery(null, Filter(ids = listOf(loser.id))) + db.assertQuery(winner, Filter(ids = listOf(winner.id))) + } + + @Test + fun testReplacingSameCreatedAtHigherIdRejected() = + forEachDB { db -> + val time = TimeUtils.now() + val a = + signer.sign( + LongTextNoteEvent.build( + "version A", + "title", + dTag = "tie", + createdAt = time, + ), + ) + val b = + signer.sign( + LongTextNoteEvent.build( + "version B", + "title", + dTag = "tie", + createdAt = time, + ), + ) + val (winner, loser) = if (a.id < b.id) a to b else b to a + + db.insert(winner) + assertFailsWith { + db.insert(loser) + } + db.assertQuery(winner, Filter(ids = listOf(winner.id))) + db.assertQuery(null, Filter(ids = listOf(loser.id))) + } + @Test fun testTriggersIndexUsage() = forEachDB { db -> diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/BasicTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/BasicTest.kt index 56216d2db..0968e4808 100644 --- a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/BasicTest.kt +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/BasicTest.kt @@ -200,6 +200,25 @@ class BasicTest : BaseDBTest() { } } + @Test + fun testSchemaRecreateIsIdempotent() = + forEachDB { db -> + // The v1->v2 upgrade in SQLiteEventStore.onUpgrade does + // modules.reversed().forEach { it.drop(db) } then + // modules.forEach { it.create(db) }. Pre-fix, FullTextSearchModule + // left dummy_fts3/4/5 tables behind on first probe, so the + // second create() would throw "already exists". + db.store.modules + .reversed() + .forEach { it.drop(db.store.connection) } + db.store.modules.forEach { it.create(db.store.connection) } + + // After re-creation the store is still usable. + val note = signer.sign(TextNoteEvent.build("test1")) + db.store.insertEvent(note) + db.store.assertQuery(note, Filter(ids = listOf(note.id))) + } + @Test fun hashCodeTest() = forEachDB { db -> diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/DeletionTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/DeletionTest.kt index d387d5bee..9eb6f2844 100644 --- a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/DeletionTest.kt +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/DeletionTest.kt @@ -93,7 +93,9 @@ class DeletionTest : BaseDBTest() { db.assertQuery(null, Filter(ids = listOf(note2.id))) db.assertQuery(note3, Filter(ids = listOf(note3.id))) - val deletion = signer.sign(DeletionEvent.build(listOf(note1))) + // Deletion must be timestamped >= note3.createdAt for it to + // remove the latest addressable per NIP-09. + val deletion = signer.sign(DeletionEvent.build(listOf(note1), createdAt = time + 100)) db.insert(deletion) @@ -128,7 +130,7 @@ class DeletionTest : BaseDBTest() { db.assertQuery(null, Filter(ids = listOf(note2.id))) db.assertQuery(note3, Filter(ids = listOf(note3.id))) - val deletion = signer.sign(DeletionEvent.buildAddressOnly(listOf(note1))) + val deletion = signer.sign(DeletionEvent.buildAddressOnly(listOf(note1), createdAt = time + 100)) db.insert(deletion) @@ -226,6 +228,7 @@ class DeletionTest : BaseDBTest() { db.store.deletionModule .deleteSQL( pubkey = "key1", + createdAt = 1766686500, idValues = listOf("ca29c211f", "ca29c211d"), addresses = emptyList(), hasher = @@ -256,6 +259,7 @@ class DeletionTest : BaseDBTest() { db.store.deletionModule .deleteSQL( pubkey = "key1", + createdAt = 1766686500, idValues = emptyList(), addresses = listOf( @@ -273,8 +277,9 @@ class DeletionTest : BaseDBTest() { WHERE ( (kind = "30000" AND pubkey = "key1" AND d_tag = "a") ) AND - kind >= 30000 AND kind < 40000 - ├── SEARCH event_headers USING COVERING INDEX addressable_idx (kind=? AND pubkey=? AND d_tag=?) + kind >= 30000 AND kind < 40000 AND + +created_at <= "1766686500" + ├── SEARCH event_headers USING INDEX addressable_idx (kind=? AND pubkey=? AND d_tag=?) ├── SEARCH event_vanish USING INTEGER PRIMARY KEY (rowid=?) ├── SEARCH event_expirations USING INTEGER PRIMARY KEY (rowid=?) └── SEARCH event_tags USING COVERING INDEX fk_event_tags_header_id (event_header_row_id=?) @@ -290,6 +295,7 @@ class DeletionTest : BaseDBTest() { db.store.deletionModule .deleteSQL( pubkey = "key1", + createdAt = 1766686500, idValues = emptyList(), addresses = listOf( @@ -310,8 +316,9 @@ class DeletionTest : BaseDBTest() { WHERE ( (kind = "30000" AND pubkey = "key1" AND d_tag IN ("a","b","c","d")) ) AND - kind >= 30000 AND kind < 40000 - ├── SEARCH event_headers USING COVERING INDEX addressable_idx (kind=? AND pubkey=? AND d_tag=?) + kind >= 30000 AND kind < 40000 AND + +created_at <= "1766686500" + ├── SEARCH event_headers USING INDEX addressable_idx (kind=? AND pubkey=? AND d_tag=?) ├── SEARCH event_vanish USING INTEGER PRIMARY KEY (rowid=?) ├── SEARCH event_expirations USING INTEGER PRIMARY KEY (rowid=?) └── SEARCH event_tags USING COVERING INDEX fk_event_tags_header_id (event_header_row_id=?) @@ -327,6 +334,7 @@ class DeletionTest : BaseDBTest() { db.store.deletionModule .deleteSQL( pubkey = "key1", + createdAt = 1766686500, idValues = emptyList(), addresses = listOf( @@ -351,12 +359,13 @@ class DeletionTest : BaseDBTest() { OR (kind = "30101" AND pubkey = "key1" AND d_tag IN ("c","d")) ) AND - kind >= 30000 AND kind < 40000 + kind >= 30000 AND kind < 40000 AND + +created_at <= "1766686500" ├── MULTI-INDEX OR │ ├── INDEX 1 - │ │ └── SEARCH event_headers USING COVERING INDEX addressable_idx (kind=? AND pubkey=? AND d_tag=?) + │ │ └── SEARCH event_headers USING INDEX addressable_idx (kind=? AND pubkey=? AND d_tag=?) │ └── INDEX 2 - │ └── SEARCH event_headers USING COVERING INDEX addressable_idx (kind=? AND pubkey=? AND d_tag=?) + │ └── SEARCH event_headers USING INDEX addressable_idx (kind=? AND pubkey=? AND d_tag=?) ├── SEARCH event_vanish USING INTEGER PRIMARY KEY (rowid=?) ├── SEARCH event_expirations USING INTEGER PRIMARY KEY (rowid=?) └── SEARCH event_tags USING COVERING INDEX fk_event_tags_header_id (event_header_row_id=?) @@ -372,6 +381,7 @@ class DeletionTest : BaseDBTest() { db.store.deletionModule .deleteSQL( pubkey = "key1", + createdAt = 1766686500, idValues = emptyList(), addresses = listOf( @@ -394,8 +404,9 @@ class DeletionTest : BaseDBTest() { WHERE kind IN ("10000","10001") AND pubkey = "key1" AND - ((kind in (0,3)) OR (kind >= 10000 AND kind < 20000)) - ├── SEARCH event_headers USING COVERING INDEX replaceable_idx (kind=? AND pubkey=?) + ((kind in (0,3)) OR (kind >= 10000 AND kind < 20000)) AND + created_at <= "1766686500" + ├── SEARCH event_headers USING COVERING INDEX query_by_kind_pubkey_created (kind=? AND pubkey=? AND created_at + // Old addressable (will be deleted by deletion request) + val old = + signer.sign( + LongTextNoteEvent.build( + "version 1", + "title", + dTag = "blog-1", + createdAt = 1000, + ), + ) + + db.insert(old) + db.assertQuery(old, Filter(ids = listOf(old.id))) + + // Deletion targeting the address with created_at = 1500 + val deletion = + signer.sign( + DeletionEvent.buildAddressOnly( + listOf(old), + createdAt = 1500, + ), + ) + db.insert(deletion) + + // The old version is gone + db.assertQuery(null, Filter(ids = listOf(old.id))) + + // Now a NEWER version of the same address arrives, with + // created_at AFTER the deletion. NIP-09 says it must be kept. + val newer = + signer.sign( + LongTextNoteEvent.build( + "version 2", + "title", + dTag = "blog-1", + createdAt = 2000, + ), + ) + + db.insert(newer) + db.assertQuery(newer, Filter(ids = listOf(newer.id))) + } } diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/ReplaceableTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/ReplaceableTest.kt index d5a1085f9..1edd6759e 100644 --- a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/ReplaceableTest.kt +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/ReplaceableTest.kt @@ -89,6 +89,43 @@ class ReplaceableTest : BaseDBTest() { db.assertQuery(null, Filter(ids = listOf(version1.id))) } + @Test + fun testReplacingSameCreatedAtLowerIdWins() = + forEachDB { db -> + val time = TimeUtils.now() + // Two events with the same (kind, pubkey, created_at) but different + // content -> different ids. NIP-01 says the lower-id wins. + val a = signer.sign(MetadataEvent.createNew("Vitor A", createdAt = time)) + val b = signer.sign(MetadataEvent.createNew("Vitor B", createdAt = time)) + val (winner, loser) = if (a.id < b.id) a to b else b to a + + // Insert the higher-id (loser) first, then the lower-id (winner). + // The trigger should delete the loser and let the winner in. + db.insert(loser) + db.assertQuery(loser, Filter(ids = listOf(loser.id))) + + db.insert(winner) + db.assertQuery(null, Filter(ids = listOf(loser.id))) + db.assertQuery(winner, Filter(ids = listOf(winner.id))) + } + + @Test + fun testReplacingSameCreatedAtHigherIdRejected() = + forEachDB { db -> + val time = TimeUtils.now() + val a = signer.sign(MetadataEvent.createNew("Vitor A", createdAt = time)) + val b = signer.sign(MetadataEvent.createNew("Vitor B", createdAt = time)) + val (winner, loser) = if (a.id < b.id) a to b else b to a + + // Winner already in DB; the higher-id duplicate must be rejected. + db.insert(winner) + assertFailsWith { + db.insert(loser) + } + db.assertQuery(winner, Filter(ids = listOf(winner.id))) + db.assertQuery(null, Filter(ids = listOf(loser.id))) + } + @Test fun testTriggersIndexUsageKind0() = forEachDB { db -> From e9e994fff476dc505739aed6ace0ecbe4f554012 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 25 Apr 2026 13:31:16 +0000 Subject: [PATCH 2/5] fix(quartz/sqlite): Batch A correctness/consistency fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - isExpired (#10): NIP-40 says an event is expired *once* `expiration` is reached, and the SQL trigger uses `<= unixepoch()`. The Kotlin pre-check used `<` (strict) so an event with `expiration == now` passed the Kotlin check then failed in the trigger. Both layers now use `<=`. Also applies to `isExpirationBefore` for consistency. - transaction extension (#7): if the body throws *and* ROLLBACK also throws, we now attach the rollback failure as a suppressed exception instead of letting it mask the original cause. COMMIT is moved outside the catch so a commit failure doesn't trigger a second ROLLBACK on already-finalized transaction state. - SeedModule.hasher (#8): the cache field is now a `kotlin.concurrent. atomics.AtomicReference` (matches the pattern used in BleChunkAssembler and BasicRelayClient) so the hasher publication is visible across threads. The race itself is benign — the seed is stable, so two concurrent computations produce identical hashers — but the prior plain `var` had no visibility guarantee. - delete(Filter()) (#12): documents the intentional asymmetry — `query` on an empty filter returns everything, but `delete` on an empty filter is a no-op (safe-by-default). New test pins the contract. Tests: - testInsertingEventExpiringExactlyNow: events with `expiration == now` are rejected by both Kotlin and the trigger. - testTransactionRollsBackOnException: a user transaction whose body throws leaves the DB unchanged and still accepts new writes. - testDeleteWithEmptyFilterIsSafe: empty-filter delete is a no-op. https://claude.ai/code/session_01X163Nr31vGkvAXoJ3JgMov --- .../nip01Core/store/sqlite/QueryBuilder.kt | 8 +++ .../store/sqlite/SQLiteConnectionExt.kt | 18 +++++-- .../nip01Core/store/sqlite/SeedModule.kt | 16 +++++- .../quartz/nip40Expiration/TagArrayExt.kt | 4 +- .../nip01Core/store/sqlite/BasicTest.kt | 53 +++++++++++++++++++ .../nip01Core/store/sqlite/ExpirationTest.kt | 22 ++++++++ 6 files changed, 113 insertions(+), 8 deletions(-) diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/QueryBuilder.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/QueryBuilder.kt index a88a12ceb..fa6ad1ffe 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/QueryBuilder.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/QueryBuilder.kt @@ -318,6 +318,13 @@ class QueryBuilder( // -------------- // Deletes // ------------- + + /** + * Safe-by-default: an empty filter (or a list of only empty filters) + * deletes nothing and returns 0, so a stray `delete(Filter())` cannot + * wipe the entire store. This is asymmetric with `query(Filter())`, + * which intentionally returns every event. + */ fun delete( filter: Filter, db: SQLiteConnection, @@ -331,6 +338,7 @@ class QueryBuilder( } } + /** See [delete] for the empty-filter contract. */ fun delete( filters: List, db: SQLiteConnection, diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/SQLiteConnectionExt.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/SQLiteConnectionExt.kt index 0d5a8464c..f56978838 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/SQLiteConnectionExt.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/SQLiteConnectionExt.kt @@ -70,14 +70,24 @@ fun SQLiteConnection.changes(): Int = inline fun SQLiteConnection.transaction(body: SQLiteConnection.() -> T): T { execSQL("BEGIN IMMEDIATE TRANSACTION") + val result: T try { - val result = body() - execSQL("END TRANSACTION") - return result + result = body() } catch (e: Throwable) { - execSQL("ROLLBACK TRANSACTION") + // Attempt rollback, but never let a rollback failure mask the + // original cause — attach it as a suppressed exception instead. + try { + execSQL("ROLLBACK TRANSACTION") + } catch (rollbackError: Throwable) { + e.addSuppressed(rollbackError) + } throw e } + // Commit is intentionally outside the catch: if COMMIT fails SQLite + // has already finalized the transaction state, so we must not also + // try to ROLLBACK on top of it. + execSQL("END TRANSACTION") + return result } fun SQLiteStatement.bindAny( diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/SeedModule.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/SeedModule.kt index a997497a2..d17d65b89 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/SeedModule.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/SeedModule.kt @@ -22,7 +22,10 @@ package com.vitorpamplona.quartz.nip01Core.store.sqlite import androidx.sqlite.SQLiteConnection import com.vitorpamplona.quartz.utils.RandomInstance +import kotlin.concurrent.atomics.AtomicReference +import kotlin.concurrent.atomics.ExperimentalAtomicApi +@OptIn(ExperimentalAtomicApi::class) class SeedModule : IModule { override fun create(db: SQLiteConnection) { db.execSQL("CREATE TABLE seeds (seed_value INTEGER)") @@ -78,7 +81,16 @@ class SeedModule : IModule { override fun deleteAll(db: SQLiteConnection) {} - private var hasherCache: TagNameValueHasher? = null + // The hasher is keyed off a stable per-DB seed, so two concurrent + // computations would produce identical hashers — the race is benign. + // We still publish via AtomicReference so the field write is visible + // across threads (commonMain has no @Volatile equivalent). + private val hasherCache = AtomicReference(null) - fun hasher(db: SQLiteConnection): TagNameValueHasher = hasherCache ?: TagNameValueHasher(getSeed(db)).also { hasherCache = it } + fun hasher(db: SQLiteConnection): TagNameValueHasher { + hasherCache.load()?.let { return it } + val fresh = TagNameValueHasher(getSeed(db)) + hasherCache.compareAndSet(null, fresh) + return hasherCache.load() ?: fresh + } } diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip40Expiration/TagArrayExt.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip40Expiration/TagArrayExt.kt index 14b0215b7..625708aac 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip40Expiration/TagArrayExt.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip40Expiration/TagArrayExt.kt @@ -27,10 +27,10 @@ fun TagArray.expiration() = this.firstNotNullOfOrNull(ExpirationTag::parse) fun TagArray.isExpired(): Boolean { val exp = expiration() ?: return false - return exp < TimeUtils.now() + return exp <= TimeUtils.now() } fun TagArray.isExpirationBefore(time: Long): Boolean { val exp = expiration() ?: return false - return exp < time + return exp <= time } diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/BasicTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/BasicTest.kt index 0968e4808..dd0e74631 100644 --- a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/BasicTest.kt +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/BasicTest.kt @@ -200,6 +200,59 @@ class BasicTest : BaseDBTest() { } } + @Test + fun testDeleteWithEmptyFilterIsSafe() = + forEachDB { db -> + val note1 = signer.sign(TextNoteEvent.build("test1", createdAt = 1)) + val note2 = signer.sign(TextNoteEvent.build("test2", createdAt = 2)) + + db.insert(note1) + db.insert(note2) + + // Empty filter: query returns everything, but delete must NOT + // wipe the store. This asymmetry is intentional safe-by-default. + assertEquals(2, db.count(Filter())) + db.delete(Filter()) + + db.assertQuery(note1, Filter(ids = listOf(note1.id))) + db.assertQuery(note2, Filter(ids = listOf(note2.id))) + + // Same applies to a list of empty filters. + db.delete(listOf(Filter(), Filter())) + assertEquals(2, db.count(Filter())) + } + + @Test + fun testTransactionRollsBackOnException() = + forEachDB { db -> + val note1 = signer.sign(TextNoteEvent.build("kept", createdAt = 1)) + val note2 = signer.sign(TextNoteEvent.build("rolled-back", createdAt = 2)) + + // Pre-existing event the test should not disturb. + db.insert(note1) + db.assertQuery(note1, Filter(ids = listOf(note1.id))) + + // A user-level transaction that inserts note2 then throws should + // leave the DB exactly as it was before — note2 must NOT remain. + val sentinel = RuntimeException("boom") + try { + db.transaction { + insert(note2) + throw sentinel + } + error("transaction should have rethrown") + } catch (e: RuntimeException) { + kotlin.test.assertEquals(sentinel, e) + } + + db.assertQuery(note1, Filter(ids = listOf(note1.id))) + db.assertQuery(null, Filter(ids = listOf(note2.id))) + + // After a rollback, the DB must still accept new writes. + db.insert(note2) + db.assertQuery(note2, Filter(ids = listOf(note2.id))) + } + @Test fun testSchemaRecreateIsIdempotent() = forEachDB { db -> diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/ExpirationTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/ExpirationTest.kt index 793470e3b..406b97418 100644 --- a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/ExpirationTest.kt +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/ExpirationTest.kt @@ -82,4 +82,26 @@ class ExpirationTest : BaseDBTest() { db.insert(note1) } } + + @Test + fun testInsertingEventExpiringExactlyNow() = + forEachDB { db -> + val time = TimeUtils.now() + + // Per NIP-40 the event is expired once `expiration` is reached; + // the SQL trigger uses `<= unixepoch()` and Kotlin's isExpired + // now agrees, so an event whose expiration equals "now" must be + // rejected by both layers — not silently let through the Kotlin + // pre-check only to fail in the trigger. + val note = + signer.sign( + TextNoteEvent.build("expires-now", createdAt = time - 1) { + expiration(time) + }, + ) + + assertFailsWith { + db.insert(note) + } + } } From 9f83feff47d16900eb3c21f60efef6d9ee736e69 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 25 Apr 2026 13:56:28 +0000 Subject: [PATCH 3/5] test(quartz/sqlite): cover transaction rollback, deletion permissions, vanish scope, FTS rotation, vacuum, SQL DSL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Batch B test-coverage gaps from the review: - testTransactionRollsBackOnTriggerAbort: a multi-insert transaction whose middle statement is rejected by `reject_deleted_events` rolls back ALL inserts in the transaction, not just the failing one. - testDeletionByThirdPartyDoesNothing: NIP-09 author check — another user's deletion event must not remove the original. - testKind5CanBeDeletedByAnotherKind5OfSameAuthor: pins the current behavior that kind-5 events are not specially protected; a same-author follow-up deletion can remove a previous one, and re-insertion of the removed deletion is then blocked. - testGiftWrapDeletionRequiresRecipient: NIP-59 GiftWraps key on the recipient (p-tag), not the (encrypted) inner author. Sender and unrelated third parties cannot delete; the recipient can. - testVanishForDifferentRelayIsNoOp: a kind-62 vanish naming a different relay must be stored but must not delete events on this relay nor block new inserts (RightToVanishModule.shouldVanishFrom contract). - testFtsCleanedUpAfterReplaceableRotation: FTS rows are cleaned via the AFTER DELETE trigger when an addressable is superseded — old content must no longer match search. - testVacuumAndAnalyseSmoke: VACUUM and ANALYZE run on a populated DB without throwing and preserve existing rows. - SqlSelectionBuilderTest: pins NotEquals(null) → IS NOT NULL, empty IN → "1 = 0", equalsOrIn singleton/multiple paths. https://claude.ai/code/session_01X163Nr31vGkvAXoJ3JgMov --- .../nip01Core/store/sqlite/BasicTest.kt | 46 +++++++++ .../nip01Core/store/sqlite/DeletionTest.kt | 93 +++++++++++++++++++ .../store/sqlite/RightToVanishTest.kt | 31 +++++++ .../nip01Core/store/sqlite/SearchTest.kt | 40 ++++++++ .../sqlite/sql/SqlSelectionBuilderTest.kt | 68 ++++++++++++++ 5 files changed, 278 insertions(+) create mode 100644 quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/sql/SqlSelectionBuilderTest.kt diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/BasicTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/BasicTest.kt index dd0e74631..e3cd10d98 100644 --- a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/BasicTest.kt +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/BasicTest.kt @@ -20,16 +20,19 @@ */ package com.vitorpamplona.quartz.nip01Core.store.sqlite +import androidx.sqlite.SQLiteException import com.vitorpamplona.quartz.nip01Core.core.Event import com.vitorpamplona.quartz.nip01Core.metadata.MetadataEvent import com.vitorpamplona.quartz.nip01Core.relay.filters.Filter import com.vitorpamplona.quartz.nip01Core.signers.NostrSignerSync import com.vitorpamplona.quartz.nip01Core.tags.hashtags.hashtag import com.vitorpamplona.quartz.nip01Core.tags.hashtags.isTaggedHash +import com.vitorpamplona.quartz.nip09Deletions.DeletionEvent import com.vitorpamplona.quartz.nip10Notes.TextNoteEvent import com.vitorpamplona.quartz.nip22Comments.CommentEvent import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertFailsWith import kotlin.test.assertTrue class BasicTest : BaseDBTest() { @@ -253,6 +256,49 @@ class BasicTest : BaseDBTest() { db.assertQuery(note2, Filter(ids = listOf(note2.id))) } + @Test + fun testTransactionRollsBackOnTriggerAbort() = + forEachDB { db -> + // Pre-load a target event and a deletion that blocks re-insertion. + val deletedTarget = signer.sign(TextNoteEvent.build("target", createdAt = 1)) + val deletion = signer.sign(DeletionEvent.build(listOf(deletedTarget), createdAt = 100)) + db.insert(deletion) + db.assertQuery(deletion, Filter(ids = listOf(deletion.id))) + + val keptInThisTx = signer.sign(TextNoteEvent.build("kept-in-tx", createdAt = 2)) + + // A trigger ABORT in the middle of a multi-insert transaction must + // roll BOTH inserts back — `reject_deleted_events` only undoes its + // own statement, but the wrapping `connection.transaction` extension + // is responsible for ROLLBACK-ing the whole batch. + assertFailsWith { + db.transaction { + insert(keptInThisTx) + insert(deletedTarget) // blocked by reject_deleted_events + } + } + + db.assertQuery(null, Filter(ids = listOf(keptInThisTx.id))) + db.assertQuery(null, Filter(ids = listOf(deletedTarget.id))) + // The deletion stored before the failed transaction must remain. + db.assertQuery(deletion, Filter(ids = listOf(deletion.id))) + } + + @Test + fun testVacuumAndAnalyseSmoke() = + forEachDB { db -> + // Smoke test: VACUUM and ANALYZE must not throw on a populated DB + // and must leave existing rows intact. Also catches the comments + // for these two functions getting swapped again. + val note = signer.sign(TextNoteEvent.build("vacuum me")) + db.insert(note) + + db.store.analyse() + db.store.vacuum() + + db.assertQuery(note, Filter(ids = listOf(note.id))) + } + @Test fun testSchemaRecreateIsIdempotent() = forEachDB { db -> diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/DeletionTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/DeletionTest.kt index 9eb6f2844..811e9b532 100644 --- a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/DeletionTest.kt +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/DeletionTest.kt @@ -415,6 +415,99 @@ class DeletionTest : BaseDBTest() { ) } + @Test + fun testDeletionByThirdPartyDoesNothing() = + forEachDB { db -> + val owner = NostrSignerSync() + val stranger = NostrSignerSync() + + val target = owner.sign(TextNoteEvent.build("private")) + db.insert(target) + db.assertQuery(target, Filter(ids = listOf(target.id))) + + // A deletion event from a *different* author must not remove the + // target — only the original author can delete via NIP-09. + val strangerDeletion = stranger.sign(DeletionEvent.build(listOf(target))) + db.insert(strangerDeletion) + + db.assertQuery(target, Filter(ids = listOf(target.id))) + db.assertQuery(strangerDeletion, Filter(ids = listOf(strangerDeletion.id))) + + // The owner's deletion still works after a failed third-party attempt. + val ownerDeletion = owner.sign(DeletionEvent.build(listOf(target))) + db.insert(ownerDeletion) + db.assertQuery(null, Filter(ids = listOf(target.id))) + } + + @Test + fun testKind5CanBeDeletedByAnotherKind5OfSameAuthor() = + forEachDB { db -> + // Pinning current behavior: a kind-5 event is treated like any + // other event for ownership purposes — the same author can issue + // a second deletion that removes the first. Cross-author + // deletion of a kind-5 must NOT work. + val target = signer.sign(TextNoteEvent.build("target")) + val firstDeletion = signer.sign(DeletionEvent.build(listOf(target), createdAt = 100)) + + db.insert(target) + db.insert(firstDeletion) + db.assertQuery(null, Filter(ids = listOf(target.id))) + db.assertQuery(firstDeletion, Filter(ids = listOf(firstDeletion.id))) + + // Stranger cannot delete the kind-5. + val stranger = NostrSignerSync() + val strangerDeletion = + stranger.sign(DeletionEvent.build(listOf(firstDeletion), createdAt = 200)) + db.insert(strangerDeletion) + db.assertQuery(firstDeletion, Filter(ids = listOf(firstDeletion.id))) + + // Same author can delete their previous kind-5. + val secondDeletion = + signer.sign(DeletionEvent.build(listOf(firstDeletion), createdAt = 300)) + db.insert(secondDeletion) + db.assertQuery(null, Filter(ids = listOf(firstDeletion.id))) + db.assertQuery(secondDeletion, Filter(ids = listOf(secondDeletion.id))) + + // First deletion now blocked from re-insertion (own e-tag in event_tags). + assertFailsWith { + db.insert(firstDeletion) + } + } + + @Test + fun testGiftWrapDeletionRequiresRecipient() = + forEachDB { db -> + // GiftWraps key their owner off the p-tag (recipient), since the + // outer wrap is signed by an ephemeral random key per NIP-59. The + // inner author isn't visible to the relay, so only the recipient + // can delete the wrap. + val recipient = NostrSignerSync() + val sender = NostrSignerSync() + val unrelated = NostrSignerSync() + + val inner = sender.sign(TextNoteEvent.build("secret")) + val wrap = GiftWrapEvent.create(inner, recipient.pubKey) + + db.insert(wrap) + db.assertQuery(wrap, Filter(ids = listOf(wrap.id))) + + // Inner author (the "sender") cannot delete a wrap addressed to + // someone else — sender pubkey != wrap's pubkey_owner_hash. + val senderDeletion = sender.sign(DeletionEvent.build(listOf(wrap))) + db.insert(senderDeletion) + db.assertQuery(wrap, Filter(ids = listOf(wrap.id))) + + // An unrelated third party can't delete it either. + val unrelatedDeletion = unrelated.sign(DeletionEvent.build(listOf(wrap))) + db.insert(unrelatedDeletion) + db.assertQuery(wrap, Filter(ids = listOf(wrap.id))) + + // The recipient (whose pubkey matches pubkey_owner_hash) can. + val recipientDeletion = recipient.sign(DeletionEvent.build(listOf(wrap))) + db.insert(recipientDeletion) + db.assertQuery(null, Filter(ids = listOf(wrap.id))) + } + @Test fun testDeletionDoesNotRemoveNewerAddressable() = forEachDB { db -> diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/RightToVanishTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/RightToVanishTest.kt index 8ba8e4144..c052d6190 100644 --- a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/RightToVanishTest.kt +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/RightToVanishTest.kt @@ -69,6 +69,37 @@ class RightToVanishTest : BaseDBTest() { db.assertQuery(note3, Filter(ids = listOf(note3.id))) } + @Test + fun testVanishForDifferentRelayIsNoOp() = + forEachDB { db -> + // The default EventStore is bound to wss://quartz.local. A vanish + // request that names a *different* relay must not delete the + // user's events on this relay — RightToVanishModule.insert checks + // shouldVanishFrom(relay) and returns without writing event_vanish. + val time = TimeUtils.now() + val note = signer.sign(TextNoteEvent.build("kept", createdAt = time)) + db.insert(note) + db.assertQuery(note, Filter(ids = listOf(note.id))) + + val vanishElsewhere = + signer.sign( + RequestToVanishEvent.build( + "wss://some-other-relay.example".normalizeRelayUrl(), + createdAt = time + 1, + ), + ) + db.insert(vanishElsewhere) + + // The vanish event itself is stored, but it must not delete + // events from this relay nor block re-insertion of new ones. + db.assertQuery(vanishElsewhere, Filter(ids = listOf(vanishElsewhere.id))) + db.assertQuery(note, Filter(ids = listOf(note.id))) + + val newNote = signer.sign(TextNoteEvent.build("still accepted", createdAt = time + 2)) + db.insert(newNote) + db.assertQuery(newNote, Filter(ids = listOf(newNote.id))) + } + @Test fun testInsertDeleteGiftWrap() = forEachDB { db -> diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/SearchTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/SearchTest.kt index 786d1a756..dae55cfa6 100644 --- a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/SearchTest.kt +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/SearchTest.kt @@ -22,11 +22,16 @@ package com.vitorpamplona.quartz.nip01Core.store.sqlite import com.vitorpamplona.quartz.nip01Core.metadata.MetadataEvent import com.vitorpamplona.quartz.nip01Core.relay.filters.Filter +import com.vitorpamplona.quartz.nip01Core.signers.NostrSignerSync import com.vitorpamplona.quartz.nip10Notes.TextNoteEvent import com.vitorpamplona.quartz.nip22Comments.CommentEvent +import com.vitorpamplona.quartz.nip23LongContent.LongTextNoteEvent +import com.vitorpamplona.quartz.utils.TimeUtils import kotlin.test.Test class SearchTest : BaseDBTest() { + val signer = NostrSignerSync() + companion object { val profile = MetadataEvent( @@ -89,4 +94,39 @@ class SearchTest : BaseDBTest() { db.assertQuery(comment, Filter(search = "testing")) db.assertQuery(comment, Filter(kinds = listOf(CommentEvent.KIND), search = "testing")) } + + @Test + fun testFtsCleanedUpAfterReplaceableRotation() = + forEachDB { db -> + // The fts_foreign_key trigger fires on event_headers DELETE, so + // when an addressable is superseded its FTS row should also be + // removed. Otherwise stale content would keep matching searches. + val time = TimeUtils.now() + val v1 = + signer.sign( + LongTextNoteEvent.build( + "first version uniqalpha", + title = "blog title", + dTag = "fts-rotation", + createdAt = time, + ), + ) + val v2 = + signer.sign( + LongTextNoteEvent.build( + "second version uniqbeta", + title = "blog title", + dTag = "fts-rotation", + createdAt = time + 1, + ), + ) + + db.store.insertEvent(v1) + db.assertQuery(v1, Filter(search = "uniqalpha")) + + db.store.insertEvent(v2) + // v1's FTS row must be gone; v2's content takes over. + db.assertQuery(null, Filter(search = "uniqalpha")) + db.assertQuery(v2, Filter(search = "uniqbeta")) + } } diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/sql/SqlSelectionBuilderTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/sql/SqlSelectionBuilderTest.kt new file mode 100644 index 000000000..3de7d0063 --- /dev/null +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/sql/SqlSelectionBuilderTest.kt @@ -0,0 +1,68 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.quartz.nip01Core.store.sqlite.sql + +import kotlin.test.Test +import kotlin.test.assertEquals + +class SqlSelectionBuilderTest { + @Test + fun equalsWithNullProducesIsNull() { + val clause = where { equals("col", null) } + assertEquals("col IS NULL", clause.conditions) + assertEquals(emptyList(), clause.args) + } + + @Test + fun notEqualsWithNullProducesIsNotNull() { + val clause = where { notEquals("col", null) } + assertEquals("col IS NOT NULL", clause.conditions) + assertEquals(emptyList(), clause.args) + } + + @Test + fun notEqualsWithValueProducesPlaceholder() { + val clause = where { notEquals("col", "x") } + assertEquals("col != ?", clause.conditions) + assertEquals(listOf("x"), clause.args) + } + + @Test + fun emptyInBecomesAlwaysFalse() { + val clause = where { isIn("col", emptyList()) } + assertEquals("1 = 0", clause.conditions) + assertEquals(emptyList(), clause.args) + } + + @Test + fun equalsOrInPicksEqualsForSingleton() { + val clause = where { equalsOrIn("col", listOf("x")) } + assertEquals("col = ?", clause.conditions) + assertEquals(listOf("x"), clause.args) + } + + @Test + fun equalsOrInPicksInForMultiple() { + val clause = where { equalsOrIn("col", listOf("a", "b", "c")) } + assertEquals("col IN (?, ?, ?)", clause.conditions) + assertEquals(listOf("a", "b", "c"), clause.args) + } +} From 819322bbf91fde00a066827bb2a15f54b9d7e0df Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 25 Apr 2026 14:24:28 +0000 Subject: [PATCH 4/5] fix(quartz/fs): port the same NIP correctness fixes from the sqlite store MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three of the bugs that the SQLite review surfaced also live in the filesystem-backed event store. Bringing both stores back to parity: - NIP-01 lexical-id tiebreaker (FsSlots, FsEventStore): when two replaceables / addressables share `createdAt`, the lexically smaller id wins. The previous `existing.createdAt >= incoming.createdAt` check rejected equal-timestamp inserts unconditionally — which blocks the legitimate winner whenever it arrived second. - delete(Filter()) safe-by-default (FsEventStore): an empty filter used to enumerate every event and delete each one, so a stray `delete(Filter())` would wipe the entire on-disk store. Now both the single-filter and list-of-filters overloads short-circuit when every filter is empty, matching the SQLiteEventStore contract. - NIP-09 author check on tombstone install (FsEventStore, FsTombstones): SQLite's `reject_deleted_events` trigger checks `event_tags.pubkey_hash = NEW.pubkey_owner_hash`, so a stranger's kind-5 with an `e`/`a` tag pointing at someone else's event must not block them from re-publishing. The FS store used to install the tombstone unconditionally and then read it back without an author check. - id tombstones still install (so they can fire when the deletion arrives before its target), but `idTombstoneOwnerPubKey` is now compared against the candidate event's owner pubkey at insert time. GiftWrap parity preserved via FsIndexer.ownerPubKey, which returns the recipient like the SQLite `pubkey_owner_hash`. - addr tombstones are now only installed when `addr.pubKeyHex == deletion.pubKey`, since the address itself carries the owner identity. Tests: - FsSlotsTest: replaces "equal timestamp replaceable is rejected" (which pinned the buggy behaviour) with two tests covering the lexical-id tiebreaker in both insertion orders. - FsParityTest: new same-`createdAt` tiebreaker tests for both replaceable and addressable kinds, asserting FS and SQLite agree. - FsDeletionTest: - inverts "deletion by non-author does not cascade but still installs id tombstone" — the legitimate owner must be able to re-insert after the stranger's deletion. - new test that a stranger's `a`-tag deletion does not block the legitimate addressable owner from publishing a new version. - FsEventStoreTest: new `delete with empty filter is safe` test matching the SQLite-side contract added in batch A. https://claude.ai/code/session_01X163Nr31vGkvAXoJ3JgMov --- .../quartz/nip01Core/store/fs/FsEventStore.kt | 74 ++++++++++++------- .../quartz/nip01Core/store/fs/FsIndexer.kt | 12 ++- .../quartz/nip01Core/store/fs/FsSlots.kt | 20 ++--- .../quartz/nip01Core/store/fs/FsTombstones.kt | 34 ++++++++- .../nip01Core/store/fs/FsDeletionTest.kt | 45 +++++++++-- .../nip01Core/store/fs/FsEventStoreTest.kt | 17 +++++ .../quartz/nip01Core/store/fs/FsParityTest.kt | 52 +++++++++++++ .../quartz/nip01Core/store/fs/FsSlotsTest.kt | 28 ++++++- 8 files changed, 232 insertions(+), 50 deletions(-) diff --git a/quartz/src/jvmMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/fs/FsEventStore.kt b/quartz/src/jvmMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/fs/FsEventStore.kt index c5b2aefd1..7bd8ac2ba 100644 --- a/quartz/src/jvmMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/fs/FsEventStore.kt +++ b/quartz/src/jvmMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/fs/FsEventStore.kt @@ -111,10 +111,16 @@ open class FsEventStore( val slot = slots.slotPathFor(event) val existingSlot = slot?.let { slots.readSlot(it) } - if (existingSlot != null && existingSlot.createdAt >= event.createdAt) { - // Newer or equal-timestamp version already owns this slot. - // Matches ReplaceableModule / AddressableModule blocking - // behaviour in SQLite. + if (existingSlot != null && + ( + existingSlot.createdAt > event.createdAt || + (existingSlot.createdAt == event.createdAt && existingSlot.id <= event.id) + ) + ) { + // Existing slot winner outranks the incoming event under NIP-01 + // (later createdAt, or same createdAt with the lexically smaller + // id). Matches the ReplaceableModule / AddressableModule trigger + // condition in SQLite. return } @@ -178,12 +184,17 @@ open class FsEventStore( /** * NIP-09 pre-insert guard. Parity with SQLite's `reject_deleted_events` - * BEFORE INSERT trigger: id-scoped tombstones always block; address- - * scoped tombstones block when the existing cutoff is `>=` the new - * event's createdAt. + * BEFORE INSERT trigger: id and addr tombstones only block when the + * deletion's author matches the candidate event's owner pubkey — + * SQLite checks `event_tags.pubkey_hash = NEW.pubkey_owner_hash`. For + * GiftWraps the owner is the recipient (p-tag), matching `pubkey_ + * owner_hash`. Addr tombstones additionally enforce the + * `event.createdAt <= cutoff` window. */ private fun isBlockedByTombstone(event: Event): Boolean { - if (tombstones.hasIdTombstone(event.id)) return true + val ownerPubKey = indexer.ownerPubKey(event) + val tombstoneAuthor = tombstones.idTombstoneOwnerPubKey(event.id) + if (tombstoneAuthor != null && tombstoneAuthor == ownerPubKey) return true if (event is AddressableEvent && event.kind.isAddressable()) { val cutoff = tombstones.addrTombstoneCutoff(event.kind, event.pubKey, event.dTag()) if (cutoff != null && event.createdAt <= cutoff) return true @@ -227,22 +238,25 @@ open class FsEventStore( } for (addr in deletion.deleteAddresses()) { - // Cascade is only honoured when the address's author matches - // the deletion author — matching SQLite's WHERE pubkey = ?. - if (addr.pubKeyHex == deletion.pubKey) { - val slot = - when { - addr.kind.isReplaceable() -> layout.replaceableSlot(addr.kind, addr.pubKeyHex) - addr.kind.isAddressable() -> layout.addressableSlot(addr.kind, addr.pubKeyHex, addr.dTag) - else -> null - } - if (slot != null) { - val winner = slots.readSlot(slot) - if (winner != null && winner.createdAt <= deletion.createdAt) { - indexer.unlink(winner) - layout.canonical(winner.id).deleteIfExists() - slots.clear(slot) - } + // Per NIP-09 only the original author can delete their own + // addressable. If the deletion's author doesn't own the + // address, ignore both the cascade AND the tombstone install + // — otherwise a stranger's stray `a`-tag would block the + // legitimate owner from re-publishing. + if (addr.pubKeyHex != deletion.pubKey) continue + + val slot = + when { + addr.kind.isReplaceable() -> layout.replaceableSlot(addr.kind, addr.pubKeyHex) + addr.kind.isAddressable() -> layout.addressableSlot(addr.kind, addr.pubKeyHex, addr.dTag) + else -> null + } + if (slot != null) { + val winner = slots.readSlot(slot) + if (winner != null && winner.createdAt <= deletion.createdAt) { + indexer.unlink(winner) + layout.canonical(winner.id).deleteIfExists() + slots.clear(slot) } } tombstones.installAddr(addr.kind, addr.pubKeyHex, addr.dTag, deletion, deletionCanonical) @@ -323,17 +337,27 @@ open class FsEventStore( // Delete // ------------------------------------------------------------------ + /** + * Safe-by-default: an empty filter (or a list of only empty filters) + * deletes nothing, so a stray `delete(Filter())` cannot wipe the + * entire store. This is asymmetric with `query(Filter())` which + * intentionally returns every event — same contract as `SQLiteEventStore`. + */ override fun delete(filter: Filter) = lockManager.withWriteLock { + if (filter.isEmpty()) return@withWriteLock val ids = ArrayList() query(filter) { ids.add(it.id) } ids.forEach { deleteLocked(it) } } + /** See [delete] for the empty-filter contract. */ override fun delete(filters: List) = lockManager.withWriteLock { + val nonEmpty = filters.filterNot { it.isEmpty() } + if (nonEmpty.isEmpty()) return@withWriteLock val ids = HashSet() - query(filters) { ids.add(it.id) } + query(nonEmpty) { ids.add(it.id) } ids.forEach { deleteLocked(it) } } diff --git a/quartz/src/jvmMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/fs/FsIndexer.kt b/quartz/src/jvmMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/fs/FsIndexer.kt index 0fbb233c3..6a99bec6c 100644 --- a/quartz/src/jvmMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/fs/FsIndexer.kt +++ b/quartz/src/jvmMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/fs/FsIndexer.kt @@ -93,11 +93,17 @@ internal class FsIndexer( } } - fun ownerHash(event: Event): Long = + fun ownerHash(event: Event): Long = hasher.hash(ownerPubKey(event)) + + /** + * Pubkey-form counterpart of [ownerHash]. Same SQLite parity rule — + * recipient (p-tag) for GiftWraps, sender pubkey otherwise. + */ + fun ownerPubKey(event: Event): String = if (event is GiftWrapEvent) { - event.recipientPubKey()?.let { hasher.hash(it) } ?: hasher.hash(event.pubKey) + event.recipientPubKey() ?: event.pubKey } else { - hasher.hash(event.pubKey) + event.pubKey } private fun createLink( diff --git a/quartz/src/jvmMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/fs/FsSlots.kt b/quartz/src/jvmMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/fs/FsSlots.kt index 2d9d2b6df..e6755cddb 100644 --- a/quartz/src/jvmMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/fs/FsSlots.kt +++ b/quartz/src/jvmMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/fs/FsSlots.kt @@ -48,10 +48,12 @@ import kotlin.io.path.readText * Tnew = incoming event.createdAt * Told = current slot winner's createdAt (if any) * - * Tnew > Told → atomically install new slot, evict old - * Tnew <= Told → reject the insert entirely - * no existing slot → install new slot - * not replaceable/addr. → no-op (kinds that don't have slot semantics) + * Tnew > Told → atomically install new slot, evict old + * Tnew == Told && id_new < id_old → install new (NIP-01 lexical id tiebreaker) + * Tnew == Told && id_new >= id_old → reject the insert entirely + * Tnew < Told → reject the insert entirely + * no existing slot → install new slot + * not replaceable/addr. → no-op (kinds that don't have slot semantics) * * Eviction deletes the old canonical file and all its index hardlinks. */ @@ -76,17 +78,17 @@ internal class FsSlots( } /** - * Pre-insert check. Returns the *existing* winner if it blocks the - * insert (i.e. its createdAt is >= incoming.createdAt). Returns null - * when insertion may proceed — possibly with an evictable older winner, - * which the caller looks up separately via [readSlot]. + * Pre-insert check. Returns true when the existing slot winner + * outranks the incoming event. NIP-01 ranking is `(createdAt DESC, id ASC)` — + * later timestamp wins, and on a tie the lexicographically smaller id wins. */ fun shouldBlock( event: Event, slot: Path, ): Boolean { val existing = readSlot(slot) ?: return false - return existing.createdAt >= event.createdAt + return existing.createdAt > event.createdAt || + (existing.createdAt == event.createdAt && existing.id <= event.id) } fun readSlot(slot: Path): Event? { diff --git a/quartz/src/jvmMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/fs/FsTombstones.kt b/quartz/src/jvmMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/fs/FsTombstones.kt index 15095a479..e99f12b3c 100644 --- a/quartz/src/jvmMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/fs/FsTombstones.kt +++ b/quartz/src/jvmMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/fs/FsTombstones.kt @@ -42,12 +42,20 @@ import kotlin.io.path.readText * empty d for replaceables) * * Semantics: - * - An `id` tombstone unconditionally blocks re-insertion of the exact - * event id (matching SQLite's ExistsCheck on etag_hash). + * - An `id` tombstone blocks re-insertion of the exact event id only + * when the deletion's author matches the candidate event's owner — + * matching SQLite's `event_tags.pubkey_hash = NEW.pubkey_owner_hash` + * condition in `reject_deleted_events`. The id tombstone is therefore + * stored unconditionally (so the check survives "deletion arrives + * before its target") and authority is checked at lookup time via + * [idTombstoneOwnerPubKey]. * - An `addr` tombstone holds the kind-5 `createdAt` as a cutoff. An * insert is blocked iff `event.createdAt <= tombstone.createdAt`. * Newer events at the same address may pass (matching SQLite's - * `created_at >= NEW.created_at` trigger condition). + * `created_at >= NEW.created_at` trigger condition). Addr tombstones + * are only installed when `addr.pubkey == deletion.author`, since + * the address itself carries the owner identity (NIP-09: only the + * original author may delete their addressable). * - When multiple kind-5s target the same tombstone path, the one * with the larger `createdAt` wins (strongest cutoff). Atomic * rename swaps it in. @@ -58,7 +66,25 @@ import kotlin.io.path.readText internal class FsTombstones( private val layout: FsLayout, ) { - fun hasIdTombstone(id: HexKey): Boolean = layout.idTombstonePath(id).exists() + /** + * Returns the kind-5 deletion's author pubkey for an id tombstone, or + * null if there is no tombstone (or it can't be parsed). The caller + * must compare this against the candidate event's owner pubkey to + * decide whether to honour the tombstone — a stranger's kind-5 with + * an `e`-tag pointing at someone else's event MUST NOT block that + * event from being (re-)inserted. + */ + fun idTombstoneOwnerPubKey(id: HexKey): HexKey? { + val path = layout.idTombstonePath(id) + if (!path.exists()) return null + return try { + Event.fromJson(path.readText()).pubKey + } catch (_: java.io.IOException) { + null + } catch (_: com.fasterxml.jackson.core.JacksonException) { + null + } + } /** Returns the `createdAt` cutoff from the tombstone, or null if none exists. */ fun addrTombstoneCutoff( diff --git a/quartz/src/jvmTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/fs/FsDeletionTest.kt b/quartz/src/jvmTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/fs/FsDeletionTest.kt index 2d475ecd6..88f40c252 100644 --- a/quartz/src/jvmTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/fs/FsDeletionTest.kt +++ b/quartz/src/jvmTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/fs/FsDeletionTest.kt @@ -67,6 +67,7 @@ class FsDeletionTest { slug: String, body: String, ts: Long, + signer: NostrSignerSync = this.signer, ) = signer.sign( createdAt = ts, kind = LongTextNoteEvent.KIND, @@ -74,6 +75,12 @@ class FsDeletionTest { content = body, ) + private fun otherArticle( + slug: String, + body: String, + ts: Long, + ) = article(slug, body, ts, signer = otherSigner) + // ------------------------------------------------------------------ // Delete by id // ------------------------------------------------------------------ @@ -106,7 +113,7 @@ class FsDeletionTest { } @Test - fun `deletion by non-author does not cascade but still installs id tombstone`() { + fun `deletion by non-author neither cascades nor blocks legitimate re-insertion`() { // other signer authors a note val theirs = note("not yours", 10, signer = otherSigner) store.insert(theirs) @@ -118,12 +125,15 @@ class FsDeletionTest { // Cascade did NOT run — not our author. assertEquals(listOf(theirs.id), store.query(Filter(ids = listOf(theirs.id))).map { it.id }) - // But a tombstone is installed for the id, blocking future re-inserts. - // Re-inserting the note is effectively a no-op because it already - // exists. Instead we delete then try to re-insert. + // The id tombstone *is* installed (so it can fire if and when a + // future event with that id is owned by the deletion's author), + // but when the legitimate owner deletes the local copy and the + // event re-arrives from another relay, the tombstone must NOT + // block it — only same-author deletions can block re-insertion. + // Matches SQLite's `event_tags.pubkey_hash = NEW.pubkey_owner_hash`. store.delete(theirs.id) store.insert(theirs) - assertEquals(emptyList(), store.query(Filter(ids = listOf(theirs.id))).map { it.id }) + assertEquals(listOf(theirs.id), store.query(Filter(ids = listOf(theirs.id))).map { it.id }) } // ------------------------------------------------------------------ @@ -258,6 +268,31 @@ class FsDeletionTest { // Tombstone files use hardlinks to the kind-5 canonical // ------------------------------------------------------------------ + @Test + fun `non-author address deletion does not block legitimate addressable inserts`() { + // `otherSigner` (call them Bob) authors an addressable; the + // default `signer` (a stranger relative to Bob) then publishes a + // kind-5 with an `a` tag pointing at Bob's address. NIP-09 says + // only the address owner may delete it, so the stranger's event + // must NOT install an address tombstone — otherwise Bob couldn't + // publish a new version at the same address. Matches SQLite's + // `event_tags.pubkey_hash = NEW.pubkey_owner_hash` guard. + val v1 = otherArticle("shared", "v1", 10) + store.insert(v1) + assertEquals(listOf(v1.id), store.query(Filter(ids = listOf(v1.id))).map { it.id }) + + val strangerDel = + signer.sign(DeletionEvent.build(listOf(v1), createdAt = 20)) + store.insert(strangerDel) + + // Bob can still publish a newer version at the same address. Since + // the stranger's deletion was non-authoritative, no addr tombstone + // exists to block. + val v2 = otherArticle("shared", "v2", 30) + store.insert(v2) + assertEquals(listOf(v2.id), store.query(Filter(ids = listOf(v2.id))).map { it.id }) + } + @Test fun `id tombstone is a hardlink to the kind-5 event`() { val n = note("x", 10) diff --git a/quartz/src/jvmTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/fs/FsEventStoreTest.kt b/quartz/src/jvmTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/fs/FsEventStoreTest.kt index 1df0509ba..946123faf 100644 --- a/quartz/src/jvmTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/fs/FsEventStoreTest.kt +++ b/quartz/src/jvmTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/fs/FsEventStoreTest.kt @@ -169,6 +169,23 @@ class FsEventStoreTest { assertTrue(store.count(Filter(ids = listOf(b.id))) == 1) } + @Test + fun `delete with empty filter is safe`() { + val a = signer.sign(TextNoteEvent.build("a", createdAt = 1)) + val b = signer.sign(TextNoteEvent.build("b", createdAt = 2)) + store.insert(a) + store.insert(b) + + // Empty filter: query returns everything, but delete must NOT + // wipe the store. Same safe-by-default contract as SQLiteEventStore. + assertEquals(2, store.count(Filter())) + store.delete(Filter()) + assertEquals(2, store.count(Filter())) + + store.delete(listOf(Filter(), Filter())) + assertEquals(2, store.count(Filter())) + } + @Test fun `staging dir is cleared on init`() { val staging = root.resolve(".staging") diff --git a/quartz/src/jvmTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/fs/FsParityTest.kt b/quartz/src/jvmTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/fs/FsParityTest.kt index 4fbf749b7..7b18122f2 100644 --- a/quartz/src/jvmTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/fs/FsParityTest.kt +++ b/quartz/src/jvmTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/fs/FsParityTest.kt @@ -231,6 +231,58 @@ class FsParityTest { assertParity(Filter(authors = listOf(signer.pubKey), kinds = listOf(LongTextNoteEvent.KIND))) } + @Test + fun `replaceable same-createdAt lexical id tiebreaker parity`() { + // Two kind-0 events with identical createdAt produce different ids + // because their content differs. NIP-01 says the lexically smaller + // id wins on a tie. Both stores must agree, regardless of insertion + // order. + val a = + signer.sign( + createdAt = 100, + kind = 0, + tags = emptyArray(), + content = "{\"name\":\"a\"}", + ) + val b = + signer.sign( + createdAt = 100, + kind = 0, + tags = emptyArray(), + content = "{\"name\":\"b\"}", + ) + + insertBoth(a) + insertBoth(b) + + assertParity( + Filter(authors = listOf(signer.pubKey), kinds = listOf(0)), + "loser-then-winner: lexically smaller id should win", + ) + assertParity( + Filter(ids = listOf(a.id, b.id)), + "the loser must not survive in the by-id query", + ) + } + + @Test + fun `addressable same-createdAt lexical id tiebreaker parity`() { + val a = article("tie", "version a", 100) + val b = article("tie", "version b", 100) + + insertBoth(a) + insertBoth(b) + + assertParity( + Filter( + authors = listOf(signer.pubKey), + kinds = listOf(LongTextNoteEvent.KIND), + tags = mapOf("d" to listOf("tie")), + ), + ) + assertParity(Filter(ids = listOf(a.id, b.id))) + } + // ------------------------------------------------------------------ // Deletion (NIP-09) // ------------------------------------------------------------------ diff --git a/quartz/src/jvmTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/fs/FsSlotsTest.kt b/quartz/src/jvmTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/fs/FsSlotsTest.kt index 9ded5a0fb..fd8eedeb0 100644 --- a/quartz/src/jvmTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/fs/FsSlotsTest.kt +++ b/quartz/src/jvmTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/fs/FsSlotsTest.kt @@ -102,15 +102,35 @@ class FsSlotsTest { } @Test - fun `equal timestamp replaceable is rejected`() { + fun `equal timestamp replaceable resolves by lexical id (NIP-01)`() { val a = metadata("a", 100) val b = metadata("b", 100) - store.insert(a) - store.insert(b) + // NIP-01 tiebreaker: when createdAt ties, the lexically smaller + // id wins, regardless of insertion order. + val (winner, loser) = if (a.id < b.id) a to b else b to a + + // Loser inserted first, then winner — winner must replace. + store.insert(loser) + store.insert(winner) val got = store.query(Filter(authors = listOf(signer.pubKey), kinds = listOf(MetadataEvent.KIND))) assertEquals(1, got.size) - assertEquals(a.id, got.single().id) + assertEquals(winner.id, got.single().id) + } + + @Test + fun `equal timestamp replaceable rejects higher id when winner already present`() { + val a = metadata("a", 100) + val b = metadata("b", 100) + val (winner, loser) = if (a.id < b.id) a to b else b to a + + // Winner inserted first — loser must NOT take the slot. + store.insert(winner) + store.insert(loser) + + val got = store.query(Filter(authors = listOf(signer.pubKey), kinds = listOf(MetadataEvent.KIND))) + assertEquals(1, got.size) + assertEquals(winner.id, got.single().id) } @Test From 75bcd779147945e93671ad6aec42533970e36e1c Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 25 Apr 2026 14:30:59 +0000 Subject: [PATCH 5/5] docs(quartz/store): note the NIP-01 tiebreaker, NIP-09 created_at window, and author-check on deletion in both store READMEs https://claude.ai/code/session_01X163Nr31vGkvAXoJ3JgMov --- .../vitorpamplona/quartz/nip01Core/store/sqlite/README.md | 5 ++++- .../com/vitorpamplona/quartz/nip01Core/store/fs/README.md | 6 +++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/README.md b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/README.md index deee5cee3..3c3d5c610 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/README.md +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/sqlite/README.md @@ -14,11 +14,13 @@ consistency can be sacrificed, but a database that will never crash and never go - Forces unique constraint by kind, pubkey - Old versions are removed when newer versions arrive. - Old versions are blocked if newer versions exist. + - Same `created_at`: NIP-01 lexical-id tiebreaker (lowest id wins). - **Addressable Events**: - Forces unique constraint by kind, pubkey, d-tag - Old versions are removed when newer versions arrive. - Old versions are blocked if newer versions exist. + - Same `created_at`: NIP-01 lexical-id tiebreaker (lowest id wins). - **Ephemeral Events** - Ephemeral events never stored. @@ -29,8 +31,9 @@ consistency can be sacrificed, but a database that will never crash and never go - **NIP-09 Deletion Events** - Deletes by event id - - Deletes by address until the `created_at` + - Deletes by address up to and including the deletion's `created_at` (newer versions are kept). - Blocks deleted events from being re-inserted. + - Only the original author's deletions take effect; cross-author kind-5s are stored but inert. - GiftWraps are deleted by p-tag - **NIP-62 Right to Vanish** diff --git a/quartz/src/jvmMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/fs/README.md b/quartz/src/jvmMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/fs/README.md index b15074ed3..15515c1cf 100644 --- a/quartz/src/jvmMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/fs/README.md +++ b/quartz/src/jvmMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/fs/README.md @@ -35,10 +35,10 @@ Everything documented in [`../sqlite/README.md`](../sqlite/README.md): | SQLite feature | This store | |---|---| | Insert + retrieve by Nostr filter | ✓ | -| Replaceable events (kinds 0, 3, 10000-19999) — newer wins, older blocked | ✓ via `replaceable//.json` slot | -| Addressable events (kinds 30000-39999) — `(kind,pubkey,d)` uniqueness | ✓ via `addressable///.json` slot | +| Replaceable events (kinds 0, 3, 10000-19999) — newer wins, older blocked, NIP-01 lexical-id tiebreaker on equal `created_at` | ✓ via `replaceable//.json` slot | +| Addressable events (kinds 30000-39999) — `(kind,pubkey,d)` uniqueness, same NIP-01 tiebreaker | ✓ via `addressable///.json` slot | | Ephemeral events never stored | ✓ rejected pre-write | -| NIP-09 deletions — by id, by address, gift-wrap by `p`-tag, blocks re-insert | ✓ via `tombstones/id/` and `tombstones/addr/` (each tombstone is a hardlink to the kind-5 event) | +| NIP-09 deletions — by id, by address (up to deletion's `created_at`), gift-wrap by `p`-tag, only the original author's deletions take effect | ✓ via `tombstones/id/` and `tombstones/addr/` (each tombstone is a hardlink to the kind-5 event) | | NIP-40 expirations — reject expired-on-insert, periodic sweep | ✓ via `idx/expires_at/` index + `deleteExpiredEvents()` | | NIP-45 counts | ✓ same planner, just count results | | NIP-50 full-text search — content tokenisation, AND of tokens | ✓ via `idx/fts//` |