diff --git a/js/packages/core/src/sync/snapshot.ts b/js/packages/core/src/sync/snapshot.ts index 70a7b1f7..f1ea7d3e 100644 --- a/js/packages/core/src/sync/snapshot.ts +++ b/js/packages/core/src/sync/snapshot.ts @@ -54,9 +54,40 @@ export const manifestSchema = z delta_since: z.number().int().nullable().optional().default(null), nonce: z.string().nullable().optional().default(null), index0: z.record(z.string(), z.unknown()).nullable().optional().default(null), + /** + * Publication time in milliseconds, inside the signed manifest. + * + * Nostr's `created_at` has whole-second resolution, so two snapshots + * published in the same second tie, and the tie-break then decides which + * one a restore returns. An event-id tie-break is stable and + * relay-independent but arbitrary with respect to time, so the older + * snapshot won about half the time and `nostr restore` silently returned + * a stale vault — entries created between the two syncs simply vanished. + * + * This field is inside the manifest JSON, so it is covered by the event + * signature and a relay cannot steer it. Optional: manifests written + * before this field existed carry null and are ordered by `created_at` + * alone (see `manifestOrderMs`). + */ + published_ms: z.number().int().nonnegative().nullable().optional().default(null), }) .loose(); +/** + * Total ordering key for manifests, newest-largest. + * + * `published_ms` when the publisher recorded it, otherwise the start of the + * `created_at` second. That fallback makes a new-format manifest win a tie + * against an old-format one published in the same second. The alternative — + * ordering the unknown one last — would let a pre-upgrade snapshot outrank + * every post-upgrade one for that second, which is the failure this exists to + * prevent. Old-format manifests are by definition from before the upgrade, so + * the bias points the right way. + */ +export function manifestOrderMs(manifest: Manifest, createdAt: number): number { + return manifest.published_ms ?? createdAt * 1000; +} + export type ChunkMeta = z.infer; export type Manifest = z.infer; @@ -124,7 +155,17 @@ export async function prepareSnapshot( event_id: null, })); return { - manifest: { ver: 1, algo: "gzip", chunks: metas, delta_since: null, nonce: null, index0: null }, + manifest: { + ver: 1, + algo: "gzip", + chunks: metas, + delta_since: null, + nonce: null, + index0: null, + // Stamped by publishSnapshot, which is the moment that actually + // matters; preparing a snapshot is not publishing it. + published_ms: null, + }, chunks, }; } diff --git a/js/packages/core/src/sync/syncFlows.ts b/js/packages/core/src/sync/syncFlows.ts index c9b21942..800907b3 100644 --- a/js/packages/core/src/sync/syncFlows.ts +++ b/js/packages/core/src/sync/syncFlows.ts @@ -21,6 +21,7 @@ import { KIND_MANIFEST, KIND_SNAPSHOT_CHUNK, newManifestId, + manifestOrderMs, parseManifest, prepareSnapshot, reassembleSnapshot, @@ -32,6 +33,27 @@ function nowUnix(): number { return Math.floor(Date.now() / 1000); } +/** + * Strictly increasing publication timestamp in milliseconds. + * + * A wall-clock millisecond is not enough on its own: two snapshots published + * back to back can land in the same millisecond, and then the ordering falls + * through to the event-id tie-break — arbitrary with respect to time, which + * is the whole bug this field exists to fix, just in a narrower window. + * Forcing each value above the last one this process produced makes it a + * monotonic sequence that also happens to be a timestamp. + * + * Across processes or machines the clock is still the only shared reference, + * so this narrows the tie window rather than closing it everywhere. A vault + * synced from two machines in the same millisecond remains ordered by event + * id, and is unchanged by this. + */ +let lastPublishedMs = 0; +function nextPublishedMs(): number { + lastPublishedMs = Math.max(Date.now(), lastPublishedMs + 1); + return lastPublishedMs; +} + export interface PublishedSnapshot { manifest: Manifest; manifestId: string; @@ -45,9 +67,13 @@ export async function publishSnapshot( privateKeyHex: string, keyIndex: Uint8Array, encryptedBytes: Uint8Array, - options: { limit?: number; createdAt?: number } = {}, + options: { limit?: number; createdAt?: number; publishedMs?: number } = {}, ): Promise { const createdAt = options.createdAt ?? nowUnix(); + // Sub-second publication time, signed as part of the manifest. Two syncs in + // the same second are otherwise indistinguishable by time, and the restore + // side then has to guess which is newer. + const publishedMs = options.publishedMs ?? nextPublishedMs(); const { manifest, chunks } = await prepareSnapshot(encryptedBytes, options.limit ?? 50_000); const chunkEventIds: string[] = []; @@ -68,6 +94,7 @@ export async function publishSnapshot( const { id: manifestId, nonce } = newManifestId(keyIndex); manifest.nonce = base64.encode(nonce); manifest.delta_since = createdAt; + manifest.published_ms = publishedMs; const manifestJson = JSON.stringify({ ver: manifest.ver, @@ -76,6 +103,7 @@ export async function publishSnapshot( delta_since: manifest.delta_since, nonce: manifest.nonce, index0: manifest.index0, + published_ms: manifest.published_ms, }); const manifestEvent = signEvent( privateKeyHex, @@ -108,12 +136,32 @@ export async function fetchLatestSnapshot( const pubkey = signerPublicKeyHex(privateKeyHex); const manifests = await pool.fetch({ authors: [pubkey], kinds: [KIND_MANIFEST] }); if (manifests.length === 0) return null; - // Newest first, with a tie-break the relay cannot choose. Arrival order - // is relay-controlled — sorting by it lets a relay decide which of two - // same-second snapshots wins. Event id is a hash of the event's own - // contents, so it gives a stable total order no party can steer. + // Newest first. `created_at` alone is whole seconds, so two snapshots + // published in the same second tie; the manifest's signed `published_ms` + // breaks that tie by actual publication time rather than arbitrarily. + // Getting this wrong is not a tie-break detail: the loser of the + // comparison IS the vault the user gets back, so an arbitrary tie-break + // silently restored a stale vault about half the time. + // + // Event id remains the final tie-break, for manifests that agree to the + // millisecond. Arrival order is never used — it is relay-controlled, which + // would let a relay choose the winner. Event id is a hash of the event's + // own contents, so it is a stable total order no party can steer. + const orderOf = new Map(); + for (const ev of manifests) { + let ms = ev.created_at * 1000; + try { + ms = manifestOrderMs(parseManifest(ev.content), ev.created_at); + } catch { + // Unparseable manifests are skipped further down; order them by + // created_at so they do not jump the queue on the way there. + } + orderOf.set(ev.id, ms); + } manifests.sort( - (a, b) => b.created_at - a.created_at || (a.id < b.id ? 1 : a.id > b.id ? -1 : 0), + (a, b) => + orderOf.get(b.id)! - orderOf.get(a.id)! || + (a.id < b.id ? 1 : a.id > b.id ? -1 : 0), ); // Falling back to an older manifest is a downgrade: a relay that withholds diff --git a/js/packages/core/test/relay.test.ts b/js/packages/core/test/relay.test.ts index b2903a07..c584f72d 100644 --- a/js/packages/core/test/relay.test.ts +++ b/js/packages/core/test/relay.test.ts @@ -18,6 +18,11 @@ import { decryptV3, encryptV3, mergeIndexPayloads, + prepareSnapshot, + newManifestId, + signEvent, + buildChunkEvent, + buildManifestEvent, parseEncryptedFile, utf8, bytesToHex, @@ -74,6 +79,141 @@ describe.skipIf(IS_JSDOM)("relay snapshot round trip", () => { expect(bytesToHex(sha256(plain))).toBe(vaultV3Payload.plaintext_sha256); }); + /** + * Two snapshots in the same second used to be ordered by event id, which is + * stable but arbitrary with respect to time — so `nostr restore` returned + * the OLDER vault roughly half the time, and entries created between the + * two syncs disappeared with no warning. It surfaced only as an + * intermittent test failure, because whether the newer snapshot won came + * down to a hash comparison. + * + * The loop makes it deterministic: over this many distinct payloads, an + * id-ordered restore is overwhelmingly likely to pick the older snapshot at + * least once (p ≈ 1 - 2^-12 per run). A single pair would reproduce the bug + * only half the time, which is how it stayed a "flake" for so long. + */ + it("returns the newer of two snapshots published in the same second", async () => { + const key = deriveIndexKeyBytes(MNEMONIC); + const keyIndex = deriveKeyIndex(MNEMONIC); + const SAME_SECOND = 1700001234; + + for (let round = 0; round < 12; round++) { + const roundRelay = new MockRelay(); + await roundRelay.start(); + const roundPool = new RelayPool([roundRelay.url], { timeoutMs: 3000 }); + try { + const payload = async (label: string) => + encryptV3( + key, + utf8( + JSON.stringify({ + schema_version: 4, + entries: { + "1": { + type: "password", kind: "password", label, + length: 16, archived: false, notes: "", tags: [], + modified_ts: SAME_SECOND, + }, + }, + }), + ), + ); + + // Same created_at second, older published first — exactly what a + // scripted or automated double-sync produces. + await publishSnapshot(roundPool, PRIVKEY, keyIndex, await payload(`older-${round}`), { + limit: 50_000, createdAt: SAME_SECOND, publishedMs: SAME_SECOND * 1000 + 100, + }); + await publishSnapshot(roundPool, PRIVKEY, keyIndex, await payload(`newer-${round}`), { + limit: 50_000, createdAt: SAME_SECOND, publishedMs: SAME_SECOND * 1000 + 900, + }); + + const fetched = await fetchLatestSnapshot(roundPool, PRIVKEY); + const state = JSON.parse( + new TextDecoder().decode(await decryptV3(key, fetched!.encrypted)), + ) as { entries: Record }; + expect(state.entries["1"]!.label).toBe(`newer-${round}`); + } finally { + await roundPool.close(); + await roundRelay.stop(); + } + } + }); + + it("orders a manifest with no published_ms by its created_at second", async () => { + // Manifests written before published_ms existed must still restore, and + // must not outrank a newer one published in the same second. Ordering the + // unknown one last instead would let a pre-upgrade snapshot beat every + // post-upgrade one for that second — the bug, reintroduced from the other + // direction. + const legacyRelay = new MockRelay(); + await legacyRelay.start(); + const legacyPool = new RelayPool([legacyRelay.url], { timeoutMs: 3000 }); + try { + const key = deriveIndexKeyBytes(MNEMONIC); + const keyIndex = deriveKeyIndex(MNEMONIC); + const AT = 1700002222; + const payload = async (label: string) => + encryptV3( + key, + utf8( + JSON.stringify({ + schema_version: 4, + entries: { + "1": { + type: "password", kind: "password", label, length: 16, + archived: false, notes: "", tags: [], modified_ts: AT, + }, + }, + }), + ), + ); + + // A genuinely old-format manifest: published the way a pre-upgrade + // client did, with no published_ms key in the JSON at all. Built by + // hand rather than by asking publishSnapshot to omit it, so the test + // exercises the real legacy shape instead of a flag. + { + const legacyBytes = await payload("legacy"); + const { manifest, chunks } = await prepareSnapshot(legacyBytes, 50_000); + for (let i = 0; i < chunks.length; i++) { + const meta = manifest.chunks[i]!; + const ev = signEvent( + PRIVKEY, + buildChunkEvent(meta.id, base64.encode(chunks[i]!), AT - 60), + ); + await legacyPool.publish(ev); + meta.event_id = ev.id; + } + const { id, nonce } = newManifestId(keyIndex); + const legacyJson = JSON.stringify({ + ver: manifest.ver, + algo: manifest.algo, + chunks: manifest.chunks, + delta_since: AT - 60, + nonce: base64.encode(nonce), + index0: null, + }); + expect(legacyJson).not.toContain("published_ms"); + await legacyPool.publish( + signEvent(PRIVKEY, buildManifestEvent(id, legacyJson, AT - 60)), + ); + } + await publishSnapshot(legacyPool, PRIVKEY, keyIndex, await payload("current"), { + limit: 50_000, createdAt: AT, publishedMs: AT * 1000 + 5, + }); + + const fetched = await fetchLatestSnapshot(legacyPool, PRIVKEY); + const state = JSON.parse( + new TextDecoder().decode(await decryptV3(key, fetched!.encrypted)), + ) as { entries: Record }; + expect(state.entries["1"]!.label).toBe("current"); + } finally { + await legacyPool.close(); + await legacyRelay.stop(); + } + }); + it("publishes deltas and replays them onto the snapshot state", async () => { const key = deriveIndexKeyBytes(MNEMONIC); const t = 1700000000; diff --git a/src/nostr/backup_models.py b/src/nostr/backup_models.py index 860c1bc7..115b0267 100644 --- a/src/nostr/backup_models.py +++ b/src/nostr/backup_models.py @@ -27,3 +27,33 @@ class Manifest: delta_since: Optional[int] = None nonce: Optional[str] = None index0: Optional[dict[str, Any]] = None + #: Publication time in milliseconds, inside the signed manifest. + #: + #: Nostr's ``created_at`` has whole-second resolution, so two snapshots + #: published in the same second tie, and whatever breaks that tie decides + #: which vault a restore returns. Ordering by an arbitrary tie-break -- + #: or, as this implementation used to, by whichever event the relay + #: happened to serve first -- silently restores a stale vault, losing + #: every entry created between the two syncs. + #: + #: ``None`` on manifests written before this field existed; those are + #: ordered by ``created_at`` alone (see ``manifest_order_ms``). + published_ms: Optional[int] = None + + +def manifest_order_ms(manifest: "Manifest", created_at: int) -> int: + """Total ordering key for manifests, newest-largest. + + ``published_ms`` when the publisher recorded it, otherwise the start of + the ``created_at`` second. That fallback makes a new-format manifest win + a tie against an old-format one published in the same second. Ordering + the unknown one last instead would let a pre-upgrade snapshot outrank + every post-upgrade one for that second, which is the failure this exists + to prevent. + + Must stay byte-for-byte equivalent to the TypeScript ``manifestOrderMs`` + (js/packages/core/src/sync/snapshot.ts) or the two implementations will + disagree about which snapshot is latest. + """ + ms = getattr(manifest, "published_ms", None) + return int(ms) if ms is not None else int(created_at) * 1000 diff --git a/src/nostr/snapshot.py b/src/nostr/snapshot.py index 56ab78d9..33dcdbaf 100644 --- a/src/nostr/snapshot.py +++ b/src/nostr/snapshot.py @@ -6,6 +6,7 @@ import hmac import json import logging import os +import threading import time from datetime import timedelta from typing import Tuple @@ -25,6 +26,40 @@ from .backup_models import ( logger = logging.getLogger("nostr.client") logger.setLevel(logging.WARNING) +#: How many manifests to fetch before choosing the newest. +#: +#: This used to be 1, which delegated "which snapshot is latest" to the relay: +#: whatever it chose to return was restored. Fetching a handful and ordering +#: them locally by signed publication time takes that decision back. Small +#: because manifests are tiny and only the newest few can ever win. +MANIFEST_FETCH_LIMIT = 16 + +#: Guards :func:`_next_published_ms` so concurrent publishes cannot collide. +_published_ms_lock = threading.Lock() +_last_published_ms = 0 + + +def _next_published_ms() -> int: + """Strictly increasing publication timestamp in milliseconds. + + A wall-clock millisecond is not enough on its own: two snapshots published + back to back can land in the same millisecond, and the ordering then falls + through to the event-id tie-break -- arbitrary with respect to time, which + is the bug ``published_ms`` exists to fix, just in a narrower window. + Forcing each value above the last one this process produced makes it a + monotonic sequence that also happens to be a timestamp. + + Across processes or machines the clock is still the only shared reference, + so this narrows the tie window rather than closing it everywhere. + + Must match the TypeScript ``nextPublishedMs`` + (js/packages/core/src/sync/syncFlows.ts). + """ + global _last_published_ms + with _published_ms_lock: + _last_published_ms = max(int(time.time() * 1000), _last_published_ms + 1) + return _last_published_ms + def prepare_snapshot( encrypted_bytes: bytes, limit: int @@ -59,6 +94,81 @@ def new_manifest_id(key_index: bytes) -> tuple[str, bytes]: return digest, nonce +def _manifest_event_created_at(event) -> int: + """``created_at`` as whole seconds, across the SDK's shapes.""" + value = getattr(event, "created_at", None) + if callable(value): + value = value() + if value is None: + value = getattr(event, "timestamp", 0) + if callable(value): + value = value() + if hasattr(value, "secs"): + value = value.secs + if callable(getattr(value, "as_secs", None)): + value = value.as_secs() + try: + return int(value) + except (TypeError, ValueError): + return 0 + + +def _manifest_event_parses(event) -> bool: + """Is this a structurally valid manifest, as opposed to relay noise? + + Distinguishes "the newest snapshot is incomplete" -- which must fail + loudly rather than fall back -- from "a relay handed us a junk event", + which should simply be ignored. + """ + try: + data = json.loads(event.content()) + return ( + isinstance(data, dict) + and "ver" in data + and "algo" in data + and isinstance(data.get("chunks"), list) + ) + except Exception: + return False + + +def _sort_manifest_events_newest_first(events: list) -> list: + """Order manifest events newest-first, by signed publication time. + + Previously this code took whatever the relay returned first, having asked + for ``limit(1)`` -- which hands the relay the choice of which snapshot a + restore returns. Even ordering by ``created_at`` is not enough on its own: + it is whole seconds, so two snapshots from the same second tie and the + tie-break decides the outcome. ``published_ms`` lives inside the signed + manifest, so it is both finer-grained and not something a relay can steer. + + Event id is the final tie-break: it is a hash of the event's own contents, + giving a stable total order that no party chooses. Arrival order is never + used. + + Must agree with the TypeScript ``fetchLatestSnapshot`` ordering. + """ + + def key(event): + created_at = _manifest_event_created_at(event) + order_ms = created_at * 1000 + try: + data = json.loads(event.content()) + raw = data.get("published_ms") + if raw is not None: + order_ms = int(raw) + except Exception: + # Unparseable manifests are rejected further down; order them by + # created_at so they do not jump the queue on the way there. + pass + event_id = getattr(event, "id", "") + if callable(event_id): + event_id = event_id() + return (order_ms, str(event_id)) + + return sorted(events, key=key, reverse=True) + + class SnapshotHandler: """Mixin providing chunk and manifest handling.""" @@ -109,6 +219,11 @@ class SnapshotHandler: manifest_id, nonce = new_manifest_id(self.key_index) manifest.nonce = base64.b64encode(nonce).decode("utf-8") + # Sub-second publication time, signed as part of the manifest, so a + # restore can tell two same-second snapshots apart. See + # Manifest.published_ms. + manifest.published_ms = _next_published_ms() + manifest_json = json.dumps( { "ver": manifest.ver, @@ -117,6 +232,7 @@ class SnapshotHandler: "delta_since": manifest.delta_since, "nonce": manifest.nonce, "index0": manifest.index0, + "published_ms": manifest.published_ms, } ) @@ -157,6 +273,11 @@ class SnapshotHandler: index0=( data.get("index0") if isinstance(data.get("index0"), dict) else None ), + published_ms=( + int(data["published_ms"]) + if data.get("published_ms") is not None + else None + ), ) except Exception: return None @@ -236,7 +357,7 @@ class SnapshotHandler: f = nostr_client.Filter().author(pubkey).kind(nostr_client.Kind(KIND_MANIFEST)) if ident: f = f.identifier(ident) - f = f.limit(1) + f = f.limit(MANIFEST_FETCH_LIMIT) try: events = (await self.client.fetch_events(f, timeout)).to_vec() except Exception as e: # pragma: no cover - network errors @@ -253,7 +374,7 @@ class SnapshotHandler: nostr_client.Filter() .author(pubkey) .kind(nostr_client.Kind(KIND_MANIFEST)) - .limit(1) + .limit(MANIFEST_FETCH_LIMIT) ) try: events = (await self.client.fetch_events(f, timeout)).to_vec() @@ -268,11 +389,25 @@ class SnapshotHandler: if not events: return None + events = _sort_manifest_events_newest_first(events) + + # Newest first, and deliberately NOT a fallback loop over older ones. + # + # Fetching several manifests is about taking the "which is newest" + # decision back from the relay, not about accepting an older snapshot + # when the newest cannot be assembled. A relay that withholds one + # chunk of the current snapshot would otherwise walk the client + # silently backwards through its own history, and a restore that + # quietly returns last week's vault is indistinguishable from one that + # worked. Refusing is recoverable; a silent downgrade is not. + # + # Manifests that do not parse at all are skipped rather than fatal: a + # relay can inject any event it likes into a response, and one + # malformed frame must not deny service to a client whose own snapshot + # is intact. The first manifest that PARSES is the only one tried. for manifest_event in events: try: result = await self._fetch_chunks_with_retry(manifest_event) - if result is not None: - return result except Exception as e: # pragma: no cover - network errors self.last_error = str(e) logger.error( @@ -280,6 +415,19 @@ class SnapshotHandler: self.relays, e, ) + continue + if result is not None: + return result + if _manifest_event_parses(manifest_event): + # A real manifest we could not complete. Say so instead of + # reaching further back. + self.last_error = ( + "The newest snapshot could not be assembled (a relay did " + "not return every chunk). Refusing to restore an older " + "snapshot silently; retry, or add another relay." + ) + logger.error("%s", self.last_error) + return None return None async def fetch_latest_snapshot(self) -> Tuple[Manifest, list[bytes]] | None: @@ -373,6 +521,13 @@ class SnapshotHandler: with self._state_lock: if self.current_manifest is not None: self.current_manifest.delta_since = int(created_at) + # Republishing the manifest under the same identifier makes + # this the newest version of it, so it needs a fresh + # published_ms like any other publish. Omitting it would sort + # the manifest carrying the newest delta_since BELOW the + # snapshot it supersedes, and readers would then fetch deltas + # from a stale watermark -- the update would simply not arrive. + self.current_manifest.published_ms = _next_published_ms() manifest_json = json.dumps( { "ver": self.current_manifest.ver, @@ -383,6 +538,7 @@ class SnapshotHandler: "delta_since": self.current_manifest.delta_since, "nonce": self.current_manifest.nonce, "index0": self.current_manifest.index0, + "published_ms": self.current_manifest.published_ms, } ) manifest_event = ( diff --git a/src/tests/test_manifest_ordering.py b/src/tests/test_manifest_ordering.py new file mode 100644 index 00000000..c080a7b7 --- /dev/null +++ b/src/tests/test_manifest_ordering.py @@ -0,0 +1,112 @@ +"""Manifest ordering: the newest snapshot wins, and the relay does not choose. + +Restoring the wrong manifest is not a cosmetic ordering bug -- the manifest +that wins the comparison IS the vault the user gets back. Two failures fed +into each other here: + +1. ``_fetch_manifest_with_keys`` asked the relay for ``limit(1)`` and used + whatever came back, which handed the relay the choice of which snapshot to + restore. +2. ``created_at`` is whole seconds, so two snapshots published in the same + second tie on time regardless. The TypeScript port broke that tie by event + id -- stable, but arbitrary with respect to time, so it returned the older + vault roughly half the time. + +``published_ms`` lives inside the signed manifest: finer-grained than +``created_at`` and not something a relay can steer. These tests pin the +ordering in Python; the equivalent TypeScript tests live in +js/packages/core/test/relay.test.ts, and the two must agree or the +implementations will disagree about which snapshot is latest. +""" + +import json + +import nostr.client # noqa: F401 (resolves the package's circular import) +from nostr.backup_models import Manifest, manifest_order_ms +from nostr.snapshot import _sort_manifest_events_newest_first + +SAME_SECOND = 1700001234 + + +class FakeEvent: + """Enough of the SDK event surface for the ordering code.""" + + def __init__(self, event_id: str, created_at: int, published_ms: int | None): + self._id = event_id + self._created_at = created_at + body = { + "ver": 1, + "algo": "gzip", + "chunks": [], + "delta_since": None, + "nonce": None, + "index0": None, + } + if published_ms is not None: + body["published_ms"] = published_ms + self._content = json.dumps(body) + + def content(self) -> str: + return self._content + + @property + def id(self) -> str: + return self._id + + @property + def created_at(self) -> int: + return self._created_at + + +def test_order_key_prefers_published_ms(): + with_ms = Manifest(ver=1, algo="gzip", chunks=[], published_ms=SAME_SECOND * 1000 + 900) + assert manifest_order_ms(with_ms, SAME_SECOND) == SAME_SECOND * 1000 + 900 + + +def test_order_key_falls_back_to_the_created_at_second(): + # A manifest written before published_ms existed sorts at the start of its + # second, so a new-format manifest from the same second outranks it. + legacy = Manifest(ver=1, algo="gzip", chunks=[]) + assert manifest_order_ms(legacy, SAME_SECOND) == SAME_SECOND * 1000 + + +def test_newer_same_second_snapshot_wins_despite_a_smaller_event_id(): + # The exact shape of the bug: ordering by event id, the older snapshot + # wins whenever its id happens to sort higher. + older = FakeEvent("ffff", SAME_SECOND, SAME_SECOND * 1000 + 100) + newer = FakeEvent("0000", SAME_SECOND, SAME_SECOND * 1000 + 900) + + ordered = _sort_manifest_events_newest_first([older, newer]) + assert [e.id for e in ordered] == ["0000", "ffff"] + + # Order of arrival must not matter either -- that is the relay's choice. + ordered_reversed = _sort_manifest_events_newest_first([newer, older]) + assert [e.id for e in ordered_reversed] == ["0000", "ffff"] + + +def test_a_legacy_manifest_does_not_outrank_a_newer_one(): + legacy = FakeEvent("aaaa", SAME_SECOND - 60, None) + current = FakeEvent("bbbb", SAME_SECOND, SAME_SECOND * 1000 + 5) + ordered = _sort_manifest_events_newest_first([legacy, current]) + assert ordered[0].id == "bbbb" + + +def test_event_id_still_breaks_an_exact_millisecond_tie(): + # Deterministic and relay-independent: the id is a hash of the event's own + # contents, so no party chooses the winner. + a = FakeEvent("1111", SAME_SECOND, SAME_SECOND * 1000 + 500) + b = FakeEvent("2222", SAME_SECOND, SAME_SECOND * 1000 + 500) + assert [e.id for e in _sort_manifest_events_newest_first([a, b])] == ["2222", "1111"] + assert [e.id for e in _sort_manifest_events_newest_first([b, a])] == ["2222", "1111"] + + +def test_an_unparseable_manifest_orders_by_created_at_without_raising(): + class Broken(FakeEvent): + def content(self) -> str: + return "{not json" + + broken = Broken("cccc", SAME_SECOND - 120, None) + good = FakeEvent("dddd", SAME_SECOND, SAME_SECOND * 1000) + ordered = _sort_manifest_events_newest_first([broken, good]) + # It is rejected later, during parsing; ordering must not blow up first. + assert ordered[0].id == "dddd"