From 8950bf67feb50e1a3537c70834117abab9cba311 Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Thu, 23 Jul 2026 03:11:47 +0000 Subject: [PATCH] fix: apply NIP-01 state ordering in purgatory --- CHANGELOG.md | 4 ++ src/git/sync.rs | 46 ++++++++++------ tests/purgatory_sync.rs | 117 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 150 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f61bb26..bc62f4e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Added four configuration options for bounded retention of NIP-09 deletion requests and NIP-62 request-to-vanish events, together with cleanup telemetry for operators. +### Fixed + +- Purgatory promotion now applies NIP-01's lowest-event-ID tie-break for same-second repository state replacements. + ### Changed - Addressed a production storage imbalance where roughly 50k of 60k stored events were deletion requests. Deletion requests now have a bounded lifecycle, so requests that are no longer relevant are reconciled and retired while requests that may still affect valid event handling are preserved. diff --git a/src/git/sync.rs b/src/git/sync.rs index 2e0d796..ac729b9 100644 --- a/src/git/sync.rs +++ b/src/git/sync.rs @@ -1055,10 +1055,15 @@ async fn process_purgatory_state_events( return result; } - // Sort by created_at (oldest first) so we process events in chronological order. - // This ensures that when multiple state events are in purgatory, older ones - // get processed first, allowing newer ones to correctly supersede them. - purgatory_states.sort_by_key(|entry| entry.event.created_at); + // Process oldest states first. NIP-01 breaks equal-timestamp replaceable + // event ties by retaining the lexicographically lowest event ID, so include + // that ordering here instead of relying on insertion order. + purgatory_states.sort_by(|a, b| { + a.event + .created_at + .cmp(&b.event.created_at) + .then_with(|| a.event.id.cmp(&b.event.id)) + }); debug!( identifier = %identifier, @@ -1161,23 +1166,26 @@ async fn process_purgatory_state_events( result.refs_deleted += process_result.refs_deleted; result.errors.extend(process_result.errors); - // Check if there's a newer state from the same author in the database - let has_newer_from_same_author = db_repo_data.states.iter().any(|s| { + // Check whether the database already has the NIP-01-preferred state + // for this author and coordinate. For equal timestamps, the lower + // event ID wins. + let has_preferred_state_from_same_author = db_repo_data.states.iter().any(|s| { s.event.pubkey == state.event.pubkey && (s.event.created_at > state.event.created_at || (s.event.created_at == state.event.created_at - && s.event.id > state.event.id)) + && s.event.id < state.event.id)) }); - if has_newer_from_same_author { - // Just remove from purgatory without saving - a newer event from same author exists + if has_preferred_state_from_same_author { + // Just remove from purgatory without saving: a newer or NIP-01 + // tie-break-preferred state from the same author already exists. purgatory.remove_state_event(identifier, &entry.event.id); result.states_released += 1; debug!( identifier = %identifier, event_id = %entry.event.id, - "Removed older state event from purgatory - newer event from same author exists in DB" + "Removed superseded state event from purgatory - preferred event from same author exists in DB" ); } else { // Save to database @@ -1330,25 +1338,28 @@ fn is_latest_authorized_state( maintainers: &[String], db_states: &[RepositoryState], ) -> bool { - // Find the latest authorized state from database + // Find the NIP-01-preferred authorized state from the database: newest + // timestamp wins, and the lowest event ID wins an equal-timestamp tie. let latest_db_state = db_states .iter() .filter(|s| maintainers.contains(&s.event.pubkey.to_hex())) .max_by(|a, b| { - // Compare by created_at, then by event id for tie-breaking + // Compare by created_at, then reverse event-ID order so `max_by` + // selects the lexicographically lowest ID on equal timestamps. a.event .created_at .cmp(&b.event.created_at) - .then_with(|| a.event.id.cmp(&b.event.id)) + .then_with(|| b.event.id.cmp(&a.event.id)) }); match latest_db_state { None => true, // No other states exist in DB, this is the latest Some(latest) => { - // This state is latest if it's newer, or if equal timestamp with larger event id + // This state is preferred if it's newer, or if an equal timestamp + // has a lower event ID under NIP-01's replaceable-event tie-break. state.event.created_at > latest.event.created_at || (state.event.created_at == latest.event.created_at - && state.event.id >= latest.event.id) + && state.event.id <= latest.event.id) } } } @@ -2157,8 +2168,9 @@ mod tests { let maintainers = vec![keys.public_key().to_hex()]; - // The one with larger event ID should be considered latest - let (latest, older) = if state1.event.id > state2.event.id { + // NIP-01 retains the lexicographically lower event ID on an + // equal-timestamp replaceable-event tie. + let (latest, older) = if state1.event.id < state2.event.id { (state1, state2) } else { (state2, state1) diff --git a/tests/purgatory_sync.rs b/tests/purgatory_sync.rs index 933884e..eb13b9a 100644 --- a/tests/purgatory_sync.rs +++ b/tests/purgatory_sync.rs @@ -37,6 +37,38 @@ use common::{ use nostr_sdk::prelude::*; use std::time::Duration; +/// Build a state event with a nonce that changes its ID without changing its +/// replaceable coordinate or declared repository state. +fn build_grinded_state_event( + keys: &Keys, + identifier: &str, + commit_hash: &str, + clone_url: &str, + relay_url: &str, + created_at: Timestamp, + nonce: u64, +) -> Event { + EventBuilder::new(Kind::RepoState, "") + .tags(vec![ + Tag::identifier(identifier), + Tag::custom("clone", vec![clone_url.to_string()]), + Tag::custom("relays", vec![relay_url.to_string()]), + Tag::custom("refs/heads/main", vec![commit_hash.to_string()]), + Tag::custom("HEAD", vec!["refs/heads/main".to_string()]), + Tag::custom( + "nonce", + vec![ + nonce.to_string(), + "0".to_string(), + "ngit-created-at-tiebreak".to_string(), + ], + ), + ]) + .custom_created_at(created_at) + .finalize(keys) + .expect("build grinded state event") +} + /// Test that a git push triggers `process_newly_available_git_data` and /// releases state events from purgatory. /// @@ -123,6 +155,91 @@ async fn test_push_triggers_unified_processing() { relay.stop().await; } +/// Equal-timestamp state events from one author share a replaceable +/// coordinate. Both must be held while their commit is missing, then only the +/// NIP-01 winner (the lower event ID) may be promoted after the push. +#[tokio::test] +async fn same_second_same_author_states_in_purgatory_keep_nip01_lowest_id() { + let relay = TestRelay::start().await; + let keys = Keys::generate(); + let identifier = "same-second-purgatory-state"; + let temp_dir = tempfile::tempdir().expect("create test repository directory"); + let commit_hash = create_test_repo_with_commit(temp_dir.path(), CommitVariant::StateTest) + .expect("create test repository"); + let announcement = create_repo_announcement(&keys, &[&relay.domain()], identifier); + + let client = Client::builder() + .authenticator(SignerAuthenticator::new(keys.clone())) + .build(); + client.add_relay(relay.url()).await.expect("add relay"); + client.connect().await; + client + .send_event(&announcement) + .await + .expect("send announcement"); + + let npub = keys.public_key().to_bech32().expect("encode public key"); + let clone_url = format!("http://{}/{}/{}.git", relay.domain(), npub, identifier); + let created_at = Timestamp::from_secs(Timestamp::now().as_secs() + 1); + let losing_state = build_grinded_state_event( + &keys, + identifier, + &commit_hash, + &clone_url, + relay.url(), + created_at, + 0, + ); + let winning_state = (1..100_000) + .map(|nonce| { + build_grinded_state_event( + &keys, + identifier, + &commit_hash, + &clone_url, + relay.url(), + created_at, + nonce, + ) + }) + .find(|candidate| candidate.id < losing_state.id) + .expect("nonce grinding must produce a lower NIP-01 event ID"); + + assert_eq!(losing_state.created_at, winning_state.created_at); + assert!(winning_state.id < losing_state.id); + + // Submit the losing state first to prove promotion does not depend on + // arrival order when purgatory releases an equal-timestamp pair. + client + .send_event(&losing_state) + .await + .expect("send higher-ID state event"); + client + .send_event(&winning_state) + .await + .expect("send lower-ID state event"); + + for event_id in [losing_state.id, winning_state.id] { + verify_event_not_served(relay.url(), &event_id, Duration::from_secs(1)) + .await + .expect("both state events must remain in purgatory before the push"); + } + + push_to_relay(temp_dir.path(), &relay.domain(), &npub, identifier) + .expect("push state commit to relay"); + + let promoted = wait_for_event_served(relay.url(), &winning_state.id, Duration::from_secs(5)) + .await + .expect("lower-ID state event must be promoted"); + assert_eq!(promoted.id, winning_state.id); + verify_event_not_served(relay.url(), &losing_state.id, Duration::from_secs(1)) + .await + .expect("higher-ID state event must not be promoted"); + + client.disconnect().await; + relay.stop().await; +} + /// Test that a state event entering purgatory triggers remote git fetch /// and is released once the git data is available. ///