Skip to content

fix(relay): serialize NIP-IA snapshot timestamps - #3849

Open
CryptoJones wants to merge 4 commits into
block:mainfrom
CryptoJones:hal/fix-archive-snapshot-collision
Open

fix(relay): serialize NIP-IA snapshot timestamps#3849
CryptoJones wants to merge 4 commits into
block:mainfrom
CryptoJones:hal/fix-archive-snapshot-collision

Conversation

@CryptoJones

Copy link
Copy Markdown

Summary

  • make relay-authored NIP-IA kind:13535 snapshot timestamps strictly monotonic
  • prevent same-second NIP-01 event-ID tie-breaking from retaining an intermediate archive set
  • add regression coverage for six rapid snapshot replacements and wall-clock catch-up

Fixes #3848.

Verification

  • cargo fmt --all -- --check
  • cargo clippy -p buzz-relay --all-targets -- -D warnings
  • cargo test -p buzz-relay — 794 passed, including both new regression tests; 10 unrelated environment-dependent failures because the local Postgres instance lacks role buzz, plus one mesh demo timeout

Proudly Made in Nebraska. Go Big Red! 🌽 https://xkcd.com/2347/

Co-authored-by: Aaron K. Clark (CryptoJones) <cryptojones@owasp.org>
Signed-off-by: Aaron K. Clark (CryptoJones) <cryptojones@owasp.org>
@CryptoJones
CryptoJones requested a review from a team as a code owner July 31, 2026 03:36
@Chessing234

Copy link
Copy Markdown
Contributor

makes sense for #3848. maybe assert the wall-clock catch-up path in the test name so future readers know why sleep is there

@Chessing234

Copy link
Copy Markdown
Contributor

serializing nip-ia snapshot timestamps is a good fix for #3848. a tiny test with two same-second updates would lock it in.

@Chessing234 Chessing234 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the right fix for #3848 — same-second replaceable ties keeping the lowest id was eating intermediate nip-ia snapshots. the unit tests for strictly increasing timestamps are exactly what i'd want. one ask: if six archives land in one wall-clock second, created_at jumps into the future by up to 5s; confirm clients tolerate created_at > now on relay-signed snapshots (most do) so you don't trade lost updates for 'future event' filters.

Co-authored-by: Aaron K. Clark (CryptoJones) <cryptojones@owasp.org>
Signed-off-by: Aaron K. Clark (CryptoJones) <cryptojones@owasp.org>
@CryptoJones

Copy link
Copy Markdown
Author

Hi @Chessing234 — addressed your feedback from this thread. I renamed the test covering the wall-clock catch-up path from replaceable_snapshot_timestamp_does_not_regress_behind_wall_clock to wall_clock_catch_up_advances_snapshot_timestamp so future readers immediately know which code path it exercises (and why a sleep might be added there for real-time testing). Both regression tests pass.

…hot-collision

Co-authored-by: Aaron K. Clark (CryptoJones) <cryptojones@owasp.org>
Signed-off-by: Aaron K. Clark (CryptoJones) <cryptojones@owasp.org>
@CryptoJones

Copy link
Copy Markdown
Author

@Chessing234 Thanks for flagging the future-timestamp case. The updated head 66df23a0f now closes all three points:

  • The wall-clock catch-up test name is explicit.
  • Same-second snapshots use strictly increasing timestamps.
  • First-party desktop and CLI snapshot queries do not apply an until filter or reject created_at > now. To keep that behavior bounded for other clients, snapshot publication now pauses at a 60-second future-skew ceiling, and docs/nips/NIP-IA.md documents that client contract.

The update also closes a concurrency gap found during follow-up review: if two publishers calculate the same replacement timestamp, the loser now logs the collision, re-reads both the canonical archive rows and latest snapshot, and retries rather than silently dropping or republishing stale tags.

Local validation on the refreshed branch: cargo clippy -p buzz-relay --all-targets -- -D warnings passes. The full relay suite reports 835 passing tests; its one remaining failure is the pre-existing mesh-demo echo timeout (504 vs 200), outside this diff.

Proudly Made in Nebraska. Go Big Red! 🌽 https://xkcd.com/2347/

Co-authored-by: Aaron K. Clark (CryptoJones) <cryptojones@owasp.org>
Signed-off-by: Aaron K. Clark (CryptoJones) <cryptojones@owasp.org>
@CryptoJones
CryptoJones force-pushed the hal/fix-archive-snapshot-collision branch from 66df23a to 46465fe Compare August 1, 2026 06:29
@CryptoJones

Copy link
Copy Markdown
Author

Follow-up: final head 46465fe43 also bounds the collision/skew retry loop to eight attempts. A backward clock step therefore cannot retain an ingest-handler permit indefinitely; skew pauses are logged and exhaustion returns through the caller's existing warning path. The added retry-budget regression test brings the full relay run to 836 passing tests, with the same unrelated mesh-demo timeout remaining.

Proudly Made in Nebraska. Go Big Red! 🌽 https://xkcd.com/2347/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

NIP-IA archive snapshot can lose rapid same-second updates

2 participants