diff --git a/CHANGELOG.md b/CHANGELOG.md index ed7f326..b88ec9b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Apply the lower-event-ID tie-break to same-second purgatory replacements. + Git push authorization now selects the preferred matching state regardless + of arrival order, and a winning announcement that removes this service + evicts its purgatory entry and associated repository data. + - Preserve all persisted root events when rebuilding sync state at startup. Purgatory cleanup can no longer remove a partially reconstructed repository and leave its older threads untracked until another restart. diff --git a/docs/explanation/architecture.md b/docs/explanation/architecture.md index a0b61d6..c70bc9d 100644 --- a/docs/explanation/architecture.md +++ b/docs/explanation/architecture.md @@ -419,9 +419,13 @@ pub struct Purgatory { - Bare repo created immediately so pushes can succeed - Announcement promoted to database only when git data proves content exists - Two-phase soft expiry: bare repo deleted at 30 min, event retained 24h for revival + - De-list replacements evict an owner's entry only when they win NIP-01 + ordering: later timestamp, or lower event ID at the same timestamp. 3. **Late Binding**: State event refs are extracted at git push time, not event arrival - Enables flexible matching when pushes arrive out-of-order + - Among matching states from authorized authors, push authorization selects + the latest timestamp, breaking ties with the lowest event ID. - Helper functions in [`helpers.rs`](../../src/purgatory/helpers.rs) handle ref extraction 4. **Bidirectional Waiting**: Either side can arrive first diff --git a/src/git/authorization.rs b/src/git/authorization.rs index a2fc668..dc4ac29 100644 --- a/src/git/authorization.rs +++ b/src/git/authorization.rs @@ -685,10 +685,14 @@ pub async fn get_state_authorization_for_selected_repo( .collect(); if !authorized_events.is_empty() { - // Find the latest event + // NIP-01 prefers the lower event ID when timestamps tie. let latest_authorized = authorized_events .iter() - .max_by_key(|event| event.created_at) + .max_by(|left, right| { + left.created_at + .cmp(&right.created_at) + .then_with(|| right.id.cmp(&left.id)) + }) .unwrap(); // Safe because we checked the vec is not empty // Parse the event into RepositoryState @@ -1377,6 +1381,80 @@ mod tests { use super::*; use nostr_sdk::prelude::{EventBuilder, FinalizeEvent, Keys, Tag}; + #[tokio::test] + async fn nip01_purgatory_state_selection_is_independent_of_arrival_order() { + let keys = Keys::generate(); + let identifier = "same-second-state"; + let commit = "1234567890abcdef1234567890abcdef12345678"; + let timestamp = Timestamp::from_secs(100); + let make_state = |signer: &Keys, content: &str, created_at| { + EventBuilder::new(Kind::RepoState, content) + .tags([ + Tag::identifier(identifier), + Tag::custom("refs/heads/main", [commit]), + ]) + .custom_created_at(created_at) + .finalize(signer) + .unwrap() + }; + let mut states = [ + make_state(&keys, "a", timestamp), + make_state(&keys, "b", timestamp), + ]; + states.sort_by_key(|event| event.id); + let [winner, loser] = states; + let outsider = make_state(&Keys::generate(), "unauthorized", Timestamp::from_secs(200)); + let newer = make_state(&keys, "newer", Timestamp::from_secs(101)); + + for candidates in [[&winner, &loser], [&loser, &winner]] { + let directory = tempfile::tempdir().unwrap(); + let purgatory = Arc::new(Purgatory::new(directory.path().to_path_buf())); + let database: SharedDatabase = Arc::new(nostr_memory::MemoryDatabase::unbounded()); + database + .save_event(&create_announcement_event(&keys, identifier, &[])) + .await + .unwrap(); + for event in candidates.into_iter().chain([&outsider]) { + purgatory.add_state(event.clone(), identifier.into(), event.pubkey, false); + } + let pushed = vec![("0".repeat(40), commit.into(), "refs/heads/main".into())]; + let selected = keys.public_key().to_hex(); + let result = get_state_authorization_for_selected_repo( + &database, + identifier, + &selected, + &purgatory, + &pushed, + directory.path(), + ) + .await + .unwrap(); + assert!(result.authorized); + assert_eq!(result.state.unwrap().event.id, winner.id); + assert_eq!( + result + .purgatory_events + .iter() + .map(|event| event.id) + .collect::>(), + vec![winner.id] + ); + + purgatory.add_state(newer.clone(), identifier.into(), keys.public_key(), false); + let result = get_state_authorization_for_selected_repo( + &database, + identifier, + &selected, + &purgatory, + &pushed, + directory.path(), + ) + .await + .unwrap(); + assert_eq!(result.state.unwrap().event.id, newer.id); + } + } + fn create_test_keys() -> Keys { Keys::generate() } diff --git a/src/nostr/policy/announcement.rs b/src/nostr/policy/announcement.rs index 832ebab..1bc6f1a 100644 --- a/src/nostr/policy/announcement.rs +++ b/src/nostr/policy/announcement.rs @@ -80,7 +80,7 @@ impl AnnouncementPolicy { match RepositoryAnnouncement::from_event(event.clone()) { Ok(announcement) => { // If this pubkey+identifier has a purgatory entry AND the incoming - // event is strictly newer, the owner is sending a replacement that + // event wins NIP-01 ordering, the owner is sending a replacement that // removes our service. Clear the purgatory entry and its bare repo. // // If the incoming event is older than the purgatory entry (e.g. a @@ -90,7 +90,11 @@ impl AnnouncementPolicy { .ctx .purgatory .find_announcement(&event.pubkey, &announcement.identifier) - .is_some_and(|entry| event.created_at > entry.event.created_at); + .is_some_and(|entry| { + event.created_at > entry.event.created_at + || (event.created_at == entry.event.created_at + && event.id < entry.event.id) + }); if should_evict { self.remove_purgatory_announcement( @@ -526,6 +530,74 @@ mod tests { .expect("signed announcement") } + #[tokio::test] + async fn nip01_purgatory_delisting_evicts_only_the_winning_replacement() { + let keys = Keys::generate(); + let identifier = "membership-gate-repo"; + let mut candidates = ["first.example", "second.example"].map(|domain| { + let event = service_announcement(&keys, domain); + let event = EventBuilder::new(event.kind, event.content) + .tags(event.tags.to_vec()) + .custom_created_at(nostr_sdk::prelude::Timestamp::from_secs(100)) + .finalize(&keys) + .unwrap(); + (event, domain) + }); + candidates.sort_by_key(|(event, _)| event.id); + let [(winner, winner_domain), (loser, loser_domain)] = candidates; + + for (stored, domain, incoming, evicted) in [ + (&loser, loser_domain, &winner, true), + (&winner, winner_domain, &loser, false), + ] { + let directory = tempfile::tempdir().unwrap(); + let repo_path = directory.path().join("repo.git"); + std::fs::create_dir(&repo_path).unwrap(); + let mut config = Config::for_testing(); + config.domain = domain.into(); + let purgatory = Arc::new(crate::purgatory::Purgatory::new( + directory.path().to_path_buf(), + )); + purgatory.add_announcement( + stored.clone(), + identifier.into(), + keys.public_key(), + repo_path.clone(), + HashSet::new(), + ); + let state = EventBuilder::new(Kind::RepoState, "") + .tag(Tag::identifier(identifier)) + .finalize(&keys) + .unwrap(); + purgatory.add_state(state, identifier.into(), keys.public_key(), false); + let ctx = PolicyContext::new_for_test( + domain.to_string(), + Arc::new(nostr_memory::MemoryDatabase::unbounded()), + directory.path().to_path_buf(), + Arc::clone(&purgatory), + config.clone(), + ); + let policy = AnnouncementPolicy::new(ctx, config, None); + policy.validate(incoming).await; + assert_eq!( + purgatory + .find_announcement(&keys.public_key(), identifier) + .is_none(), + evicted + ); + assert_eq!(!repo_path.exists(), evicted); + assert_eq!(purgatory.find_state(identifier).is_empty(), evicted); + // Replaying a non-winning de-list must remain harmless. + policy.validate(incoming).await; + assert_eq!( + purgatory + .find_announcement(&keys.public_key(), identifier) + .is_none(), + evicted + ); + } + } + #[tokio::test] async fn private_mode_rejects_announcement_from_nonmember_author() { let member = Keys::generate();