Fix: nip01 replaceable tiebreaker#416
Merged
phoenix-server merged 3 commits intocameri:mainfrom Apr 10, 2026
Merged
Conversation
| const query = repository.upsert(event).toString() | ||
|
|
||
| expect(query).to.equal('insert into "events" ("deleted_at", "event_content", "event_created_at", "event_deduplication", "event_id", "event_kind", "event_pubkey", "event_signature", "event_tags", "expires_at", "remote_address") values (NULL, \'{"name":"ottman@minds.io","about":"","picture":"https://feat-2311-nostr.minds.io/icon/1002952989368913934/medium/1564498626/1564498626/1653379539"}\', 1564498626, \'["55b702c167c85eb1c2d5ab35d68bedd1a35b94c01147364d2395c2f66f35a503",0]\', X\'e527fe8b0f64a38c6877f943a9e8841074056ba72aceb31a4c85e6d10b27095a\', 0, X\'55b702c167c85eb1c2d5ab35d68bedd1a35b94c01147364d2395c2f66f35a503\', X\'d1de98733de2b412549aa64454722d9b66ab3c68e9e0d0f9c5d42e7bd54c30a06174364b683d2c8dbb386ff47f31e6cb7e2f3c3498d8819ee80421216c8309a9\', \'[]\', NULL, \'::1\') on conflict (event_pubkey, event_kind, event_deduplication) WHERE (event_kind = 0 OR event_kind = 3 OR event_kind = 41 OR (event_kind >= 10000 AND event_kind < 20000)) OR (event_kind >= 30000 AND event_kind < 40000) do update set "event_id" = X\'e527fe8b0f64a38c6877f943a9e8841074056ba72aceb31a4c85e6d10b27095a\',"event_created_at" = 1564498626,"event_tags" = \'[]\',"event_content" = \'{"name":"ottman@minds.io","about":"","picture":"https://feat-2311-nostr.minds.io/icon/1002952989368913934/medium/1564498626/1564498626/1653379539"}\',"event_signature" = X\'d1de98733de2b412549aa64454722d9b66ab3c68e9e0d0f9c5d42e7bd54c30a06174364b683d2c8dbb386ff47f31e6cb7e2f3c3498d8819ee80421216c8309a9\',"remote_address" = \'::1\',"expires_at" = NULL,"deleted_at" = NULL where "events"."event_created_at" < 1564498626') | ||
| expect(query).to.equal('insert into "events" ("deleted_at", "event_content", "event_created_at", "event_deduplication", "event_id", "event_kind", "event_pubkey", "event_signature", "event_tags", "expires_at", "remote_address") values (NULL, \'{"name":"ottman@minds.io","about":"","picture":"https://feat-2311-nostr.minds.io/icon/1002952989368913934/medium/1564498626/1564498626/1653379539"}\', 1564498626, \'["55b702c167c85eb1c2d5ab35d68bedd1a35b94c01147364d2395c2f66f35a503",0]\', X\'e527fe8b0f64a38c6877f943a9e8841074056ba72aceb31a4c85e6d10b27095a\', 0, X\'55b702c167c85eb1c2d5ab35d68bedd1a35b94c01147364d2395c2f66f35a503\', X\'d1de98733de2b412549aa64454722d9b66ab3c68e9e0d0f9c5d42e7bd54c30a06174364b683d2c8dbb386ff47f31e6cb7e2f3c3498d8819ee80421216c8309a9\', \'[]\', NULL, \'::1\') on conflict (event_pubkey, event_kind, event_deduplication) WHERE (event_kind = 0 OR event_kind = 3 OR event_kind = 41 OR (event_kind >= 10000 AND event_kind < 20000)) OR (event_kind >= 30000 AND event_kind < 40000) do update set "event_id" = X\'e527fe8b0f64a38c6877f943a9e8841074056ba72aceb31a4c85e6d10b27095a\',"event_created_at" = 1564498626,"event_tags" = \'[]\',"event_content" = \'{"name":"ottman@minds.io","about":"","picture":"https://feat-2311-nostr.minds.io/icon/1002952989368913934/medium/1564498626/1564498626/1653379539"}\',"event_signature" = X\'d1de98733de2b412549aa64454722d9b66ab3c68e9e0d0f9c5d42e7bd54c30a06174364b683d2c8dbb386ff47f31e6cb7e2f3c3498d8819ee80421216c8309a9\',"remote_address" = \'::1\',"expires_at" = NULL,"deleted_at" = NULL where ("events"."event_created_at" < 1564498626 or ("events"."event_created_at" = 1564498626 and "events"."event_id" > X\'e527fe8b0f64a38c6877f943a9e8841074056ba72aceb31a4c85e6d10b27095a\'))') |
Collaborator
There was a problem hiding this comment.
do any of these test cases cover the tie-breaker or this just updates existing queries?
Let's make sure we have a test case for the tie-breaker per the spec.
Contributor
Author
There was a problem hiding this comment.
You're right! Those updates in event-repository.spec.ts are strictly unit "snapshot" tests to ensure the new nested Knex logic generates correctly formatted SQL syntax under the hood.
I completely agree that an actual behavioral integration test is crucial here. I will add a dedicated scenario to the Cucumber test suite test/integration/features/nip-16/nip-16.feature strictly verifying the NIP-01 lexical ID tie-breaker is respected during identical timestamp collisions. I’ll push that commit up shortly!
…vent ID comparison
YashIIT0909
force-pushed
the
fix/nip01-replaceable-tiebreaker
branch
from
a3fe7a3 to
3170e5f
Compare
YashIIT0909
changed the title
cameri
approved these changes
Apr 9, 2026
Owner
|
@YashIIT0909 Please fix the linting issues. |
YashIIT0909
force-pushed
the
fix/nip01-replaceable-tiebreaker
branch
from
a31ef2e to
db13bd6
Compare
cameri
approved these changes
Apr 10, 2026
Mohit-Davar
pushed a commit
to Mohit-Davar/nostream
that referenced
this pull request
Apr 11, 2026
* fix: update event upsert query to handle duplicate timestamps using event ID comparison * test: add NIP-01 tie-breaker integration test for identical timestamps
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Title:
fix: NIP-01 replaceable event tie-breaker on identical timestampsDescription
This PR fixes a bug in event-repository.ts where the upsert mechanism failed to properly implement the NIP-01 (formerly NIP-16) tie-breaking rule for replaceable events.
Previously, if two replaceable events with the exact same
created_attimestamp were sent out, the Knex upsert query rigorously enforced<on the timestamps. This caused the event processed second to be silently bypassed withrowCount=0, ignoring the fallback rule that states the event with the lower lexicalidmust be retained.This PR modifies the
wherecondition within theON CONFLICT DO UPDATEclause to properly fallback to lexical ID comparison when timestamps are identical. The unit tests in event-repository.spec.ts have also been updated to strictly assert against the new nested query structure.Related Issue
Fixes #411
Motivation and Context
Nostr
created_attimestamps only possess second-level precision. High concurrency (bots, migrations, network batches) easily results in users publishing replaceable events with perfectly identical timestamps. Without the lexicalidtie-breaker, relays drop differing events depending on network race conditions, causing fragmented user metadata across the Nostr ecosystem.How Has This Been Tested?
rowCountoutcomes for high-concurrency partial-index constraint insertions.Event Bhas an identically set timestamp asEvent Abut possesses a lowerid,Event Bsuccessfully overwritesEvent A.npm run test:unit, specifically testing the heavily mocked assertions under test/unit/repositories/event-repository.spec.ts.npm run build:checkandnpm run lint.Types of changes
Checklist: