nick/amethyst
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 1 Wiki Activity

fix(quartz/fs): port the same NIP correctness fixes from the sqlite store

Browse Source 
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
This commit is contained in:
Claude
2026-04-25 14:24:28 +00:00
parent 9f83feff47
commit 819322bbf9
8 changed files with 232 additions and 50 deletions
Download Patch File Download Diff File Expand all files Collapse all files
quartz/src/jvmMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/fs/FsEventStore.kt
+49 -25
View File
@@ -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<HexKey>()
query<Event>(filter) { ids.add(it.id) }
ids.forEach { deleteLocked(it) }
}
/** See [delete] for the empty-filter contract. */
override fun delete(filters: List<Filter>) =
lockManager.withWriteLock {
val nonEmpty = filters.filterNot { it.isEmpty() }
if (nonEmpty.isEmpty()) return@withWriteLock
val ids = HashSet<HexKey>()
query<Event>(filters) { ids.add(it.id) }
query<Event>(nonEmpty) { ids.add(it.id) }
ids.forEach { deleteLocked(it) }
}
quartz/src/jvmMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/fs/FsIndexer.kt
+9 -3
View File
@@ -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(
quartz/src/jvmMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/fs/FsSlots.kt
+11 -9
View File
@@ -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? {
quartz/src/jvmMain/kotlin/com/vitorpamplona/quartz/nip01Core/store/fs/FsTombstones.kt
+30 -4
View File
@@ -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(
quartz/src/jvmTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/fs/FsDeletionTest.kt
+40 -5
View File
@@ -67,6 +67,7 @@ class FsDeletionTest {
slug: String,
body: String,
ts: Long,
signer: NostrSignerSync = this.signer,
) = signer.sign<LongTextNoteEvent>(
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<TextNoteEvent>(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<TextNoteEvent>(Filter(ids = listOf(theirs.id))).map { it.id })
assertEquals(listOf(theirs.id), store.query<TextNoteEvent>(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<LongTextNoteEvent>(Filter(ids = listOf(v1.id))).map { it.id })
val strangerDel =
signer.sign<DeletionEvent>(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<LongTextNoteEvent>(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)
quartz/src/jvmTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/fs/FsEventStoreTest.kt
+17
View File
@@ -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>(TextNoteEvent.build("a", createdAt = 1))
val b = signer.sign<TextNoteEvent>(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")
quartz/src/jvmTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/fs/FsParityTest.kt
+52
View File
@@ -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<Event>(
createdAt = 100,
kind = 0,
tags = emptyArray(),
content = "{\"name\":\"a\"}",
)
val b =
signer.sign<Event>(
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)
// ------------------------------------------------------------------
quartz/src/jvmTest/kotlin/com/vitorpamplona/quartz/nip01Core/store/fs/FsSlotsTest.kt
+24 -4
View File
@@ -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<MetadataEvent>(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<MetadataEvent>(Filter(authors = listOf(signer.pubKey), kinds = listOf(MetadataEvent.KIND)))
assertEquals(1, got.size)
assertEquals(winner.id, got.single().id)
}
@Test