mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-10-05 11:18:24 +00:00
fix(quartz): a superseded replaceable is REPLACED (OK false), not a duplicate
A replaceable/addressable version that a stored one already beats is not written. Since18c576a068the SQLite store has reported it as SUPERSEDED, "duplicate: a newer version ...", and RelaySession acks every `duplicate:` reason with OK true. So a client was told its event is on the relay when no REQ will ever return it. NIP-01's third OK field is `true` when the event was accepted. classifyRowError now answers RejectionReason.REPLACED, `replaced: a newer version exists`, which RelaySession sends as OK false. That is strfry's answer too (`false, "replaced: have newer event"`). It also names the case correctly: the relay does not have THIS event, it has a newer one. The W09 work is untouched. Classification still asks the database rather than the driver's message, so Android's null-message constraint failures still classify correctly, and a byte-for-byte re-offer of a stored version is still DUPLICATE (OK true). The MDK `wn` loop18c576a068was fixing (a second KeyPackage minted in the same second) now gets `OK false replaced:` instead of raw constraint text, which was the unclassifiable part. Retrying a `replaced:` rejection cannot succeed, so that is for the client to stop doing, not for the relay to paper over with OK true. RejectionReason.SUPERSEDED stays, @Deprecated in favour of REPLACED, for source compatibility. NostrServerTest and InsertOutcomeClassificationTest flip their expectations, and the event-store-semantics skill updates W01/W02 and W09 and adds a changelog entry. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AdSGU88PYrRVyVHjiEc5sw
This commit is contained in:
Claude
@@ -143,11 +143,13 @@ messages quoted below (they surface as the NIP-01 `OK false` reason).
|
||||
kinds. A `BEFORE INSERT` trigger deletes any stored version that is *older* — meaning
|
||||
`created_at` smaller, **or equal `created_at` with lexicographically larger id** (NIP-01
|
||||
lowest-id-wins). Inserting a version that is *not* newer under that ordering leaves the stored
|
||||
row in place and fails the unique index → rejected with `RejectionReason.SUPERSEDED`
|
||||
(`duplicate: a newer version of this replaceable event is already stored`), which the relay
|
||||
session answers with `OK true` exactly like an id duplicate (NIP-01 `duplicate:` prefix; same
|
||||
reply nostr-rs-relay gives). Net contract: exactly one version stored; newest wins; ties broken
|
||||
by lowest id; older re-inserts blocked but acknowledged as already covered.
|
||||
row in place and fails the unique index → rejected with `RejectionReason.REPLACED`
|
||||
(`replaced: a newer version exists`), which the relay session answers with `OK false`: the
|
||||
event was not written, and NIP-01's `true` means accepted. Same reply strfry gives
|
||||
(`false, "replaced: have newer event"`); nostr-rs-relay answers `true, "duplicate:"` instead,
|
||||
and `RejectionReason.SUPERSEDED`, which did the same, is deprecated. Net contract: exactly one
|
||||
version stored; newest wins; ties broken by lowest id; older re-inserts blocked and reported as
|
||||
such.
|
||||
|
||||
**STORE-W02 — addressable supersession.** Same as W01 with unique index
|
||||
`(kind, pubkey, d_tag)` over `30000 ≤ kind < 40000`. Nuance: `d_tag` is populated from the
|
||||
@@ -193,16 +195,15 @@ in input order; OK frames pair by event id, not order.
|
||||
exception text.** `SQLiteEventStore.classifyRowError` rolls the row's savepoint back and then
|
||||
asks the connection (which now shows pre-insert state): id already present → `DUPLICATE`;
|
||||
a stored version that beats this one at the replaceable/addressable coordinate (the exact
|
||||
complement of the supersession predicate in W01/W02) → `SUPERSEDED`; otherwise `Failed`.
|
||||
complement of the supersession predicate in W01/W02) → `REPLACED`; otherwise `Failed`.
|
||||
Message text is only a fast path and a fallback for trigger RAISEs (`blocked:`, `not allowed`),
|
||||
which leave no database-visible trace. This matters because the message is driver-specific —
|
||||
the bundled JVM driver writes `UNIQUE constraint failed: event_headers.id`, Android's throws an
|
||||
`android.database.SQLException` with a **null** message — so a text-only classifier answered
|
||||
`OK false` on Android for events the store already held. Corollary: re-offering a stored
|
||||
replaceable/addressable event **byte-for-byte** is `DUPLICATE`, not `SUPERSEDED` (it violates
|
||||
both indexes and only the id answer is driver-independent); a stale *different* version is
|
||||
still `SUPERSEDED`. Both carry the `duplicate:` prefix, so the relay reply is `OK true` either
|
||||
way.
|
||||
replaceable/addressable event **byte-for-byte** is `DUPLICATE` (`OK true`: it is stored), not
|
||||
`REPLACED` (it violates both indexes and only the id answer is driver-independent); a stale
|
||||
*different* version is `REPLACED` (`OK false`: it is not).
|
||||
|
||||
---
|
||||
|
||||
@@ -338,6 +339,10 @@ non-itemizable cases).
|
||||
|
||||
Add one line per behavior change, newest first: `YYYY-MM-DD <short sha> <rule id> — what changed`.
|
||||
|
||||
- 2026-09-27 (pending) W01/W02, W09 — a stale replaceable/addressable version is `REPLACED`
|
||||
(`replaced:` → `OK false`) again, not `SUPERSEDED` (`duplicate:` → `OK true`): it is not
|
||||
written, and `OK true` told the client it was. `SUPERSEDED` is deprecated. Byte-for-byte
|
||||
re-offers stay `DUPLICATE`.
|
||||
- 2026-09-18 (pending) W09, W01/W02 — insert-failure classification now queries the database
|
||||
instead of parsing the driver's exception message (Android's is null, so duplicates were
|
||||
reported as `Failed`/`OK false`). A byte-for-byte re-offer of a stored replaceable/addressable
|
||||
|
||||
+19
-6
@@ -51,17 +51,30 @@ object RejectionReason {
|
||||
const val DUPLICATE = "duplicate: already have this event"
|
||||
|
||||
/**
|
||||
* A replaceable or addressable event that a stored version already supersedes
|
||||
* (newer `created_at`, or the same `created_at` and a lower id). Nothing is
|
||||
* written, and — like [DUPLICATE] — the relay answers `OK true`: NIP-01 keeps
|
||||
* `duplicate:` as the machine-readable prefix for "already covered", and that
|
||||
* is what nostr-rs-relay sends here too, so clients that retry on anything
|
||||
* else (MDK's `wn`) settle instead of re-offering the same event forever.
|
||||
* No longer produced by any store: a superseded version is [REPLACED]. Its
|
||||
* `duplicate:` prefix made the relay answer `OK true` for an event that was
|
||||
* never written, which NIP-01 reserves for an accepted one.
|
||||
*/
|
||||
@Deprecated(
|
||||
"A stale replaceable/addressable version is not written, so it is REPLACED (OK false), not a duplicate.",
|
||||
ReplaceWith("RejectionReason.REPLACED", "com.vitorpamplona.quartz.nip01Core.store.RejectionReason"),
|
||||
)
|
||||
const val SUPERSEDED = "duplicate: a newer version of this replaceable event is already stored"
|
||||
const val EXPIRED = "blocked: Cannot insert an expired event"
|
||||
const val DELETED = "blocked: a deletion event exists"
|
||||
const val VANISHED = "blocked: a request to vanish event exists"
|
||||
|
||||
/**
|
||||
* A replaceable or addressable event that a stored version already supersedes
|
||||
* (newer `created_at`, or the same `created_at` and a lower id): STORE-W01/W02.
|
||||
* Nothing is written, so the relay answers `OK false` — NIP-01's third field is
|
||||
* `true` only when the event was accepted, and acking it would tell the client
|
||||
* its event is on this relay when no REQ will ever return it. `replaced:` is
|
||||
* also strfry's answer (`false, "replaced: have newer event"`), and it does not
|
||||
* misname the case the way `duplicate:` would: the relay does not have THIS
|
||||
* event, it has a newer one. A client should not retry it; nothing a retry can
|
||||
* change. A byte-for-byte re-offer of the stored version is [DUPLICATE] instead.
|
||||
*/
|
||||
const val REPLACED = "replaced: a newer version exists"
|
||||
const val INSERT_FAILED = "error: insert failed"
|
||||
}
|
||||
|
||||
+3
-3
@@ -581,9 +581,9 @@ class SQLiteEventStore(
|
||||
}
|
||||
// The replaceable / addressable unique indexes fire only when the supersession
|
||||
// trigger found nothing older to delete, i.e. the stored version already wins
|
||||
// (STORE-W01/W02). Same shape as a duplicate: nothing to write, `OK true`.
|
||||
// (STORE-W01/W02). Nothing was written, so REPLACED: `OK false`, not a duplicate.
|
||||
if (message.contains(SUPERSEDED_CONSTRAINT) || isSupersededByStored(event, db)) {
|
||||
return IEventStore.InsertOutcome.Rejected(RejectionReason.SUPERSEDED)
|
||||
return IEventStore.InsertOutcome.Rejected(RejectionReason.REPLACED)
|
||||
}
|
||||
|
||||
return if (message.contains("constraint", ignoreCase = true)) {
|
||||
@@ -618,7 +618,7 @@ class SQLiteEventStore(
|
||||
* trigger *would* have deleted doesn't count. That precision matters
|
||||
* here: this is the fallback for unrecognized failures, and a disk
|
||||
* error while inserting a winning replaceable event must stay `Failed`
|
||||
* rather than turn into a silent `OK true`. An equal id is the
|
||||
* rather than be reported as having lost to a stored version. An equal id is the
|
||||
* duplicate case and is answered before this one.
|
||||
*/
|
||||
private fun isSupersededByStored(
|
||||
|
||||
+14
-12
@@ -29,6 +29,7 @@ import com.vitorpamplona.quartz.nip01Core.relay.filters.Filter
|
||||
import com.vitorpamplona.quartz.nip01Core.relay.server.policies.EmptyPolicy
|
||||
import com.vitorpamplona.quartz.nip01Core.relay.server.policies.IRelayPolicy
|
||||
import com.vitorpamplona.quartz.nip01Core.store.IEventStore
|
||||
import com.vitorpamplona.quartz.nip01Core.store.RejectionReason
|
||||
import com.vitorpamplona.quartz.nip01Core.store.sqlite.EventStore
|
||||
import com.vitorpamplona.quartz.utils.EventFactory
|
||||
import kotlinx.coroutines.ExperimentalCoroutinesApi
|
||||
@@ -151,12 +152,12 @@ class NostrServerTest {
|
||||
|
||||
/**
|
||||
* STORE-W01: a replaceable event older than the stored version is not written,
|
||||
* and the relay acknowledges it the way nostr-rs-relay does — `OK true` with the
|
||||
* NIP-01 `duplicate:` prefix — rather than leaking the unique-index text as a
|
||||
* rejection the client would keep retrying.
|
||||
* so the relay answers `OK false` with `replaced:` (strfry's answer) — never
|
||||
* `OK true`, which NIP-01 keeps for an accepted event, and never the raw
|
||||
* unique-index text, which no client can classify.
|
||||
*/
|
||||
@Test
|
||||
fun olderReplaceableIsAcknowledgedAsDuplicateNotRejected() =
|
||||
fun olderReplaceableIsRejectedAsReplaced() =
|
||||
runTest {
|
||||
val dispatcher = UnconfinedTestDispatcher(testScheduler)
|
||||
val store = EventStore(null)
|
||||
@@ -172,8 +173,8 @@ class NostrServerTest {
|
||||
val okMessages = collector.rawMessagesContaining("OK")
|
||||
assertEquals(2, okMessages.size)
|
||||
assertTrue(okMessages[0].contains(",true,"))
|
||||
assertTrue(okMessages[1].contains(",true,"), "older version must be acked, got ${okMessages[1]}")
|
||||
assertTrue(okMessages[1].contains("duplicate:"), "older version must carry the duplicate: prefix")
|
||||
assertTrue(okMessages[1].contains(",false,"), "older version was not stored, so it must not be acked, got ${okMessages[1]}")
|
||||
assertTrue(okMessages[1].contains(RejectionReason.PREFIX_REPLACED), "older version must carry the replaced: prefix")
|
||||
|
||||
val stored = store.query<Event>(Filter(kinds = listOf(0)))
|
||||
assertEquals(listOf(newer.id), stored.map { it.id }, "the newer version stays the only stored one")
|
||||
@@ -183,12 +184,13 @@ class NostrServerTest {
|
||||
|
||||
/**
|
||||
* STORE-W02 tie: two addressable events with the same `d` tag and the same
|
||||
* `created_at` — the lower id wins, the other is acknowledged as superseded.
|
||||
* This is the exact shape MDK's `wn keys publish` produces when it mints a
|
||||
* second KeyPackage within the same second as the first.
|
||||
* `created_at` — the lower id wins, the other is rejected as `replaced:`. This
|
||||
* is the shape MDK's `wn keys publish` produces when it mints a second
|
||||
* KeyPackage within the same second as the first; the loser is not stored,
|
||||
* so the client must learn that rather than be told it was accepted.
|
||||
*/
|
||||
@Test
|
||||
fun sameSecondAddressableTieLoserIsAcknowledgedAsDuplicate() =
|
||||
fun sameSecondAddressableTieLoserIsRejectedAsReplaced() =
|
||||
runTest {
|
||||
val dispatcher = UnconfinedTestDispatcher(testScheduler)
|
||||
val store = EventStore(null)
|
||||
@@ -204,8 +206,8 @@ class NostrServerTest {
|
||||
|
||||
val okMessages = collector.rawMessagesContaining("OK")
|
||||
assertEquals(2, okMessages.size)
|
||||
assertTrue(okMessages[1].contains(",true,"), "tie loser must be acked, got ${okMessages[1]}")
|
||||
assertTrue(okMessages[1].contains("duplicate:"))
|
||||
assertTrue(okMessages[1].contains(",false,"), "tie loser was not stored, so it must not be acked, got ${okMessages[1]}")
|
||||
assertTrue(okMessages[1].contains(RejectionReason.PREFIX_REPLACED))
|
||||
|
||||
val stored = store.query<Event>(Filter(kinds = listOf(30443)))
|
||||
assertEquals(listOf(lowerId.id), stored.map { it.id }, "lowest id wins the tie")
|
||||
|
||||
+9
-9
@@ -34,9 +34,9 @@ import kotlin.test.assertTrue
|
||||
|
||||
/**
|
||||
* `batchInsert` must name *why* a row didn't land, because the relay turns
|
||||
* that reason into the NIP-01 answer: [RejectionReason.DUPLICATE] and
|
||||
* [RejectionReason.SUPERSEDED] are `OK true` ("already covered"), everything
|
||||
* else is `OK false`, which clients retry.
|
||||
* that reason into the NIP-01 answer: [RejectionReason.DUPLICATE] is `OK true`
|
||||
* (the event is already here), everything else is `OK false` — including
|
||||
* [RejectionReason.REPLACED], a stale version that was not written.
|
||||
*
|
||||
* This suite pins the classification **independently of the SQLite driver's
|
||||
* exception text**. The bundled JVM driver raises
|
||||
@@ -81,18 +81,18 @@ class InsertOutcomeClassificationTest : BaseDBTest() {
|
||||
}
|
||||
|
||||
@Test
|
||||
fun olderReplaceableIsRejectedAsSuperseded() =
|
||||
fun olderReplaceableIsRejectedAsReplaced() =
|
||||
forEachDB { db ->
|
||||
val time = TimeUtils.now()
|
||||
val older = signer.sign(MetadataEvent.createNew("Vitor 1", createdAt = time))
|
||||
val newer = signer.sign(MetadataEvent.createNew("Vitor 2", createdAt = time + 1))
|
||||
|
||||
assertEquals(IEventStore.InsertOutcome.Accepted, db.batchInsert(listOf<Event>(newer))[0])
|
||||
assertRejected(RejectionReason.SUPERSEDED, db.batchInsert(listOf<Event>(older))[0])
|
||||
assertRejected(RejectionReason.REPLACED, db.batchInsert(listOf<Event>(older))[0])
|
||||
}
|
||||
|
||||
@Test
|
||||
fun sameSecondReplaceableTieLoserIsRejectedAsSuperseded() =
|
||||
fun sameSecondReplaceableTieLoserIsRejectedAsReplaced() =
|
||||
forEachDB { db ->
|
||||
val time = TimeUtils.now()
|
||||
// NIP-01 breaks a created_at tie by lowest id, so the winner is
|
||||
@@ -104,18 +104,18 @@ class InsertOutcomeClassificationTest : BaseDBTest() {
|
||||
).sortedBy { it.id }
|
||||
|
||||
assertEquals(IEventStore.InsertOutcome.Accepted, db.batchInsert(listOf<Event>(winner))[0])
|
||||
assertRejected(RejectionReason.SUPERSEDED, db.batchInsert(listOf<Event>(loser))[0])
|
||||
assertRejected(RejectionReason.REPLACED, db.batchInsert(listOf<Event>(loser))[0])
|
||||
}
|
||||
|
||||
@Test
|
||||
fun olderAddressableIsRejectedAsSuperseded() =
|
||||
fun olderAddressableIsRejectedAsReplaced() =
|
||||
forEachDB { db ->
|
||||
val time = TimeUtils.now()
|
||||
val older = signer.sign(LongFormContentEvent.build("v1", "title", dTag = "blog", createdAt = time))
|
||||
val newer = signer.sign(LongFormContentEvent.build("v2", "title", dTag = "blog", createdAt = time + 1))
|
||||
|
||||
assertEquals(IEventStore.InsertOutcome.Accepted, db.batchInsert(listOf<Event>(newer))[0])
|
||||
assertRejected(RejectionReason.SUPERSEDED, db.batchInsert(listOf<Event>(older))[0])
|
||||
assertRejected(RejectionReason.REPLACED, db.batchInsert(listOf<Event>(older))[0])
|
||||
}
|
||||
|
||||
@Test
|
||||
|
||||
Reference in New Issue
Block a user