From 2acffbcc54fd6a9eefb74812cf9e1ed736244a93 Mon Sep 17 00:00:00 2001 From: orveth Date: Fri, 17 Apr 2026 18:08:47 -0700 Subject: [PATCH 1/3] =?UTF-8?q?fix(core/cache):=20NIP-01=20tiebreak=20?= =?UTF-8?q?=E2=80=94=20lowest=20eventId=20wins=20on=20createdAt=20tie?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per NIP-01 replaceable-event semantics: "In case of replaceable events with the same timestamp, the event with the lowest id (first in lexical order) should be retained, and the other discarded." The `nextWins` CAS predicate compared `next.eventId > prev.eventId`, so the higher eventId won on a createdAt tie. Every well-behaved relay peer converges to the lowest eventId on the same tie, so v2 would diverge from the network for any same-`createdAt` collision on a parameterized-replaceable key. Flip to `next.eventId < prev.eventId`: - on a tie, `next` supersedes `prev` iff next's id is lex-lower - equal ids remain a no-op (strict `<` is false on equal, so the same-eventId-3x idempotent test still holds) Flagged by the 2026-04-17 replaceable-events audit (P0). Test rebaseline + comment fixes follow in separate commits. Co-Authored-By: Claude Opus 4.7 --- packages/core/src/cache/upsert.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/core/src/cache/upsert.ts b/packages/core/src/cache/upsert.ts index f5e20bc..c8e8706 100644 --- a/packages/core/src/cache/upsert.ts +++ b/packages/core/src/cache/upsert.ts @@ -54,8 +54,10 @@ function nextWins( ): boolean { if (next.createdAt > prev.createdAt) return true; if (next.createdAt < prev.createdAt) return false; - // Tiebreak: lexicographically higher eventId wins. - return next.eventId > prev.eventId; + // Tiebreak: per NIP-01, the event with the LOWEST id (first in lexical + // order) is retained. So `next` supersedes `prev` only when next.eventId + // is lex-lower than prev.eventId. Equal ids are a no-op (false). + return next.eventId < prev.eventId; } /** Upsert a kind:38172 or kind:38173 announcement with Layer A gating on both kinds. */ From 3411c05c759d9f5d0fc51f4637cd875ef55cb6c1 Mon Sep 17 00:00:00 2001 From: orveth Date: Fri, 17 Apr 2026 18:10:37 -0700 Subject: [PATCH 2/3] test(core/cache): rebaseline tiebreak assertions per NIP-01 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After flipping the CAS tiebreak comparator to match NIP-01, 9 test assertions that codified the previous (inverted) behavior now fail. This commit rebaselines them and anchors each in the NIP-01 spec quote so they don't drift back: - packages/core/src/cache/upsert.test.ts (6 tests) - upsertAnnouncement: "lower eventId replaces on tie" + "higher rejected as stale on tie" - upsertReview: same pair, flipped - upsertProfile: tiebreak assertion flipped - upsertRelayList: tiebreak assertion flipped - packages/core/src/integration.test.ts (1 test) - tiebreak-under-race 3-variant shuffle: lowest eventId is the deterministic winner, not highest - packages/core/src/reviews/upsert.test.ts (2 tests) - upsertReviewWithAggregate tiebreak pair: aggregate now reflects the lower-eventId rating Also fixed a stale inline comment in integration.test.ts (equal-eventId loses because `next < prev` is false on equal, not `next > prev`). Test count: 247 before, 247 after. No new assertions added — purely direction-flipping rebaseline. New tests (positive-only NIP-01 anchor and same-createdAt shuffle) are out of scope for this PR per the audit's "P0 fix only, punt coverage gaps to backlog" decision. Co-Authored-By: Claude Opus 4.7 --- packages/core/src/cache/upsert.test.ts | 95 +++++++++++++----------- packages/core/src/integration.test.ts | 17 +++-- packages/core/src/reviews/upsert.test.ts | 21 +++--- 3 files changed, 71 insertions(+), 62 deletions(-) diff --git a/packages/core/src/cache/upsert.test.ts b/packages/core/src/cache/upsert.test.ts index 6723f9a..43179c2 100644 --- a/packages/core/src/cache/upsert.test.ts +++ b/packages/core/src/cache/upsert.test.ts @@ -165,29 +165,31 @@ describe("upsertAnnouncement", () => { expect(fetched?.content).toBe("newer"); }); - it("replaces when createdAt ties and eventId is higher lexicographically", async () => { + it("replaces when createdAt ties and eventId is lower lexicographically (NIP-01: lowest id wins)", async () => { + // NIP-01: "In case of replaceable events with the same timestamp, the + // event with the lowest id (first in lexical order) should be retained." const db = await freshDB(); - const loEid = makeAnnouncement({ eventId: EID_LOW, content: "lo" }); const hiEid = makeAnnouncement({ eventId: EID_HIGH, content: "hi" }); + const loEid = makeAnnouncement({ eventId: EID_LOW, content: "lo" }); - expect(await upsertAnnouncement(db, loEid)).toBe("inserted"); - expect(await upsertAnnouncement(db, hiEid)).toBe("replaced"); + expect(await upsertAnnouncement(db, hiEid)).toBe("inserted"); + expect(await upsertAnnouncement(db, loEid)).toBe("replaced"); - const fetched = await db.announcements.get([loEid.pubkey, loEid.kind, loEid.d]); - expect(fetched?.eventId).toBe(EID_HIGH); - expect(fetched?.content).toBe("hi"); + const fetched = await db.announcements.get([hiEid.pubkey, hiEid.kind, hiEid.d]); + expect(fetched?.eventId).toBe(EID_LOW); + expect(fetched?.content).toBe("lo"); }); - it("rejects as stale when createdAt ties and eventId is lower lexicographically", async () => { + it("rejects as stale when createdAt ties and eventId is higher lexicographically (NIP-01: lowest id wins)", async () => { const db = await freshDB(); - const hiEid = makeAnnouncement({ eventId: EID_HIGH, content: "hi" }); const loEid = makeAnnouncement({ eventId: EID_LOW, content: "lo" }); + const hiEid = makeAnnouncement({ eventId: EID_HIGH, content: "hi" }); - expect(await upsertAnnouncement(db, hiEid)).toBe("inserted"); - expect(await upsertAnnouncement(db, loEid)).toBe("rejected-stale"); + expect(await upsertAnnouncement(db, loEid)).toBe("inserted"); + expect(await upsertAnnouncement(db, hiEid)).toBe("rejected-stale"); - const fetched = await db.announcements.get([hiEid.pubkey, hiEid.kind, hiEid.d]); - expect(fetched?.eventId).toBe(EID_HIGH); + const fetched = await db.announcements.get([loEid.pubkey, loEid.kind, loEid.d]); + expect(fetched?.eventId).toBe(EID_LOW); }); it("rejects as invalid when kind:38172 has a 16-char bot-spam d-tag", async () => { @@ -283,7 +285,7 @@ describe("upsertAnnouncement", () => { const r2 = await upsertAnnouncement(db, row); const r3 = await upsertAnnouncement(db, row); - // First wins, subsequent dupes lose tiebreak (next.eventId > prev.eventId is false on equal). + // First wins, subsequent dupes lose tiebreak (next.eventId < prev.eventId is false on equal). expect(r1).toBe("inserted"); expect(r2).toBe("rejected-stale"); expect(r3).toBe("rejected-stale"); @@ -399,28 +401,31 @@ describe("upsertReview", () => { expect(fetched?.rating).toBe(5); }); - it("tiebreak: higher eventId replaces on createdAt tie", async () => { + it("tiebreak: lower eventId replaces on createdAt tie (NIP-01: lowest id wins)", async () => { + // NIP-01: "In case of replaceable events with the same timestamp, the + // event with the lowest id (first in lexical order) should be retained." const db = await freshDB(); - const loEid = makeReview({ eventId: EID_LOW, rating: 1 }); const hiEid = makeReview({ eventId: EID_HIGH, rating: 5 }); + const loEid = makeReview({ eventId: EID_LOW, rating: 1 }); - expect(await upsertReview(db, loEid)).toBe("inserted"); - expect(await upsertReview(db, hiEid)).toBe("replaced"); + expect(await upsertReview(db, hiEid)).toBe("inserted"); + expect(await upsertReview(db, loEid)).toBe("replaced"); - const fetched = await db.reviews.get([loEid.pubkey, loEid.kind, loEid.d]); - expect(fetched?.rating).toBe(5); + const fetched = await db.reviews.get([hiEid.pubkey, hiEid.kind, hiEid.d]); + expect(fetched?.rating).toBe(1); + expect(fetched?.eventId).toBe(EID_LOW); }); - it("tiebreak: lower eventId is rejected as stale on createdAt tie", async () => { + it("tiebreak: higher eventId is rejected as stale on createdAt tie (NIP-01: lowest id wins)", async () => { const db = await freshDB(); - const hiEid = makeReview({ eventId: EID_HIGH, rating: 5 }); const loEid = makeReview({ eventId: EID_LOW, rating: 1 }); + const hiEid = makeReview({ eventId: EID_HIGH, rating: 5 }); - expect(await upsertReview(db, hiEid)).toBe("inserted"); - expect(await upsertReview(db, loEid)).toBe("rejected-stale"); + expect(await upsertReview(db, loEid)).toBe("inserted"); + expect(await upsertReview(db, hiEid)).toBe("rejected-stale"); - const fetched = await db.reviews.get([hiEid.pubkey, hiEid.kind, hiEid.d]); - expect(fetched?.rating).toBe(5); + const fetched = await db.reviews.get([loEid.pubkey, loEid.kind, loEid.d]); + expect(fetched?.rating).toBe(1); }); it("keeps separate rows when the same reviewer reviews different mints", async () => { @@ -461,19 +466,21 @@ describe("upsertProfile", () => { expect(fetched?.name).toBe("alice-v2"); }); - it("tiebreak on eventId when createdAt ties", async () => { + it("tiebreak on eventId when createdAt ties (NIP-01: lowest id wins)", async () => { + // NIP-01: "In case of replaceable events with the same timestamp, the + // event with the lowest id (first in lexical order) should be retained." const db = await freshDB(); - const lo = makeProfile({ eventId: EID_LOW, name: "lo" }); const hi = makeProfile({ eventId: EID_HIGH, name: "hi" }); + const lo = makeProfile({ eventId: EID_LOW, name: "lo" }); - expect(await upsertProfile(db, lo)).toBe("inserted"); - expect(await upsertProfile(db, hi)).toBe("replaced"); + expect(await upsertProfile(db, hi)).toBe("inserted"); + expect(await upsertProfile(db, lo)).toBe("replaced"); - const backToLo = makeProfile({ eventId: EID_LOW, name: "back-to-lo" }); - expect(await upsertProfile(db, backToLo)).toBe("rejected-stale"); + const backToHi = makeProfile({ eventId: EID_HIGH, name: "back-to-hi" }); + expect(await upsertProfile(db, backToHi)).toBe("rejected-stale"); const fetched = await db.profiles.get(lo.pubkey); - expect(fetched?.name).toBe("hi"); + expect(fetched?.name).toBe("lo"); }); it("keeps one row per pubkey; different pubkeys are independent", async () => { @@ -506,20 +513,22 @@ describe("upsertRelayList", () => { expect(fetched?.relays[0]?.write).toBe(false); }); - it("tiebreak on eventId when createdAt ties", async () => { + it("tiebreak on eventId when createdAt ties (NIP-01: lowest id wins)", async () => { + // NIP-01: "In case of replaceable events with the same timestamp, the + // event with the lowest id (first in lexical order) should be retained." const db = await freshDB(); - const lo = makeRelayList({ eventId: EID_LOW }); - const hi = makeRelayList({ - eventId: EID_HIGH, - relays: [{ url: "wss://hi.example", read: true, write: true }], + const hi = makeRelayList({ eventId: EID_HIGH }); + const lo = makeRelayList({ + eventId: EID_LOW, + relays: [{ url: "wss://lo.example", read: true, write: true }], }); - expect(await upsertRelayList(db, lo)).toBe("inserted"); - expect(await upsertRelayList(db, hi)).toBe("replaced"); + expect(await upsertRelayList(db, hi)).toBe("inserted"); + expect(await upsertRelayList(db, lo)).toBe("replaced"); - const fetched = await db.relayLists.get(lo.pubkey); - expect(fetched?.eventId).toBe(EID_HIGH); - expect(fetched?.relays[0]?.url).toBe("wss://hi.example"); + const fetched = await db.relayLists.get(hi.pubkey); + expect(fetched?.eventId).toBe(EID_LOW); + expect(fetched?.relays[0]?.url).toBe("wss://lo.example"); }); }); diff --git a/packages/core/src/integration.test.ts b/packages/core/src/integration.test.ts index 9d937d2..d73b084 100644 --- a/packages/core/src/integration.test.ts +++ b/packages/core/src/integration.test.ts @@ -184,8 +184,9 @@ describe("integration: corpus replay → parse → cache", () => { describe("integration: CAS convergence under simulated multi-relay race", () => { it("multi-relay echo of the same event id: 1 row, 1 inserted + 2 rejected-stale, deterministic", async () => { // Three relays publish the same canonical event. Same id, same key, - // same createdAt — the equal-eventId loses the tiebreak (next > prev is - // false), so re-broadcasts always end as 'rejected-stale'. No churn. + // same createdAt — the equal-eventId loses the tiebreak (next < prev is + // false on equal), so re-broadcasts always end as 'rejected-stale'. No + // churn. const legacy = f.cashu38172Legacy[0]; expect(legacy).toBeDefined(); if (!legacy) return; @@ -208,12 +209,12 @@ describe("integration: CAS convergence under simulated multi-relay race", () => expect(stale.length).toBe(2); }); - it("tiebreak under race: 3 events with same [pubkey,kind,d,createdAt] but different eventIds — highest eventId always wins, 10 shuffled trials", async () => { + it("tiebreak under race: 3 events with same [pubkey,kind,d,createdAt] but different eventIds — lowest eventId always wins (NIP-01), 10 shuffled trials", async () => { // Real-world: same logical replaceable event published by the same // signer at the same second but with different ids (e.g. retried after - // a sig collision, or re-emitted by a buggy client). The lex-highest - // eventId must win deterministically every time, regardless of arrival - // order. + // a sig collision, or re-emitted by a buggy client). NIP-01: the + // lex-LOWEST eventId must win deterministically every time, regardless + // of arrival order. const legacy = f.cashu38172Legacy[0]; expect(legacy).toBeDefined(); if (!legacy) return; @@ -244,8 +245,8 @@ describe("integration: CAS convergence under simulated multi-relay race", () => expect(await db.announcements.count()).toBe(1); const fetched = await db.announcements.get([baseRow.pubkey, baseRow.kind, baseRow.d]); - expect(fetched?.eventId).toBe(eidHigh); - expect(fetched?.content).toBe("hi"); + expect(fetched?.eventId).toBe(eidLow); + expect(fetched?.content).toBe("lo"); } }); }); diff --git a/packages/core/src/reviews/upsert.test.ts b/packages/core/src/reviews/upsert.test.ts index 45dfeeb..8a167d2 100644 --- a/packages/core/src/reviews/upsert.test.ts +++ b/packages/core/src/reviews/upsert.test.ts @@ -135,32 +135,31 @@ describe("upsertReviewWithAggregate — CAS + aggregate-stays-in-sync", () => { expect(after?.avgRating).toBe(5); }); - it("tiebreak on eventId: same createdAt, higher eventId wins, aggregate reflects new rating", async () => { + it("tiebreak on eventId: same createdAt, lower eventId wins (NIP-01), aggregate reflects new rating", async () => { + // NIP-01: "In case of replaceable events with the same timestamp, the + // event with the lowest id (first in lexical order) should be retained." const db = await freshDB(); - await upsertReviewWithAggregate(db, makeReview({ eventId: EID_LOW, rating: 1 })); - const result = await upsertReviewWithAggregate( - db, - makeReview({ eventId: EID_HIGH, rating: 5 }), - ); + await upsertReviewWithAggregate(db, makeReview({ eventId: EID_HIGH, rating: 5 })); + const result = await upsertReviewWithAggregate(db, makeReview({ eventId: EID_LOW, rating: 1 })); expect(result).toBe("replaced"); const agg = await db.mintAggregate.get(D_VALID); - expect(agg?.avgRating).toBe(5); + expect(agg?.avgRating).toBe(1); }); - it("tiebreak rejects lower eventId: aggregate NOT updated", async () => { + it("tiebreak rejects higher eventId (NIP-01): aggregate NOT updated", async () => { const db = await freshDB(); - await upsertReviewWithAggregate(db, makeReview({ eventId: EID_HIGH, rating: 5 }), () => 1000); + await upsertReviewWithAggregate(db, makeReview({ eventId: EID_LOW, rating: 1 }), () => 1000); const result = await upsertReviewWithAggregate( db, - makeReview({ eventId: EID_LOW, rating: 1 }), + makeReview({ eventId: EID_HIGH, rating: 5 }), () => 9999, ); expect(result).toBe("rejected-stale"); const agg = await db.mintAggregate.get(D_VALID); expect(agg?.updatedAt).toBe(1000); - expect(agg?.avgRating).toBe(5); + expect(agg?.avgRating).toBe(1); }); it("replacing a rated review with an unrated one → aggregate flips to avg=null", async () => { From d956b3d6d55423f21bd3cf57202ec6bc3aca7b0a Mon Sep 17 00:00:00 2001 From: orveth Date: Fri, 17 Apr 2026 18:10:59 -0700 Subject: [PATCH 3/3] docs(core): correct NIP-01 tiebreak comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After the CAS comparator flip, the file-level docstring for cache/upsert.ts still described the old (inverted) behavior as the "standard" tiebreak. That narrative was precisely the bug — the actual NIP-01 standard is the opposite direction. Replace with the NIP-01 quote and note that divergence on same- `createdAt` collisions is the cost of getting the direction wrong. The inline comment above the `nextWins` return (cache/upsert.ts:57) and the parenthetical in integration.test.ts multi-relay echo case were already fixed by the flip + rebaseline commits. This commit finishes the narrative cleanup. Co-Authored-By: Claude Opus 4.7 --- packages/core/src/cache/upsert.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/core/src/cache/upsert.ts b/packages/core/src/cache/upsert.ts index c8e8706..c3c83ab 100644 --- a/packages/core/src/cache/upsert.ts +++ b/packages/core/src/cache/upsert.ts @@ -8,9 +8,12 @@ * * Replaceable-event ordering rules (per NIP-01 §7.3 / NIP-33): * 1. Higher `createdAt` wins. - * 2. Tiebreak — when `createdAt` is equal, the event with the higher - * `eventId` (string compare on lowercase hex) wins. This is the - * standard replaceable-event tiebreak clients converge on. + * 2. Tiebreak — when `createdAt` is equal, the event with the LOWEST + * `eventId` (first in lexical order, string compare on lowercase hex) + * is retained. NIP-01: "the event with the lowest id (first in + * lexical order) should be retained, and the other discarded." This + * is what well-behaved relay peers converge on; any other direction + * produces silent divergence on same-`createdAt` collisions. * * Layer A gate for kind:38172: before writing an announcement we check * isValidCashuDTag(d). Invalid shapes (bot spam, non-hex garbage) are