mirror of
https://relay.ngit.dev/npub15qydau2hjma6ngxkl2cyar74wzyjshvl65za5k5rl69264ar2exs5cyejr/ngit-grasp.git
synced 2026-10-05 23:18:24 +00:00
fix(sync): break reciprocal invitation bootstrap deadlock
A newly selected GRASP server receives the invitee's reciprocal announcement before it has any Git data. The inviter's announcement may already be in the rejected-event cache because, when first seen, the two maintainers did not yet list each other. That creates a cycle: the server needs the rejected announcement's clone URLs to fetch Git, but it waits for Git before it can promote the reciprocal repository. When a reciprocal owner announcement arrives, reprocess the cached maintainer announcements it names before attempting promotion. Reset pending repository sync when new or replacement announcement metadata appears, and immediately retry synced state or PR events from the hot cache so a previous backoff cannot hide the newly available source. This gives GRASP enough trusted metadata to discover the existing maintainer's state and Git data while keeping each maintainer's clone URLs personal to their own announcement.
This commit is contained in:
@@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fix maintainership invitation syncing by retrying the source maintainer announcement once reciprocal membership is known, allowing GRASP to discover and copy the existing repository.
|
||||
- Purgatory promotion now applies NIP-01's lowest-event-ID tie-break for same-second repository state replacements.
|
||||
|
||||
### Changed
|
||||
|
||||
@@ -194,6 +194,16 @@ impl AnnouncementPolicy {
|
||||
relays,
|
||||
);
|
||||
|
||||
if self
|
||||
.ctx
|
||||
.purgatory
|
||||
.has_pending_events(&announcement.identifier)
|
||||
{
|
||||
self.ctx
|
||||
.purgatory
|
||||
.enqueue_sync_immediate(&announcement.identifier);
|
||||
}
|
||||
|
||||
// Extend the announcement's expiry (reset to full 30 min window)
|
||||
self.ctx.purgatory.extend_announcement_expiry(
|
||||
&event.pubkey,
|
||||
@@ -313,6 +323,16 @@ impl AnnouncementPolicy {
|
||||
relays,
|
||||
);
|
||||
|
||||
if self
|
||||
.ctx
|
||||
.purgatory
|
||||
.has_pending_events(&announcement.identifier)
|
||||
{
|
||||
self.ctx
|
||||
.purgatory
|
||||
.enqueue_sync_immediate(&announcement.identifier);
|
||||
}
|
||||
|
||||
tracing::info!(
|
||||
identifier = %announcement.identifier,
|
||||
event_id = %event.id,
|
||||
|
||||
@@ -955,6 +955,18 @@ impl Purgatory {
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// Collect announcement events currently awaiting git data.
|
||||
///
|
||||
/// The proactive sync manager uses this snapshot to retry dependency events
|
||||
/// that may have arrived before a new owner announcement established their
|
||||
/// maintainer relationship.
|
||||
pub fn announcement_events_for_sync(&self) -> Vec<Event> {
|
||||
self.announcement_purgatory
|
||||
.iter()
|
||||
.map(|entry| entry.value().event.clone())
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// Get all event IDs currently stored in purgatory AND previously expired events.
|
||||
///
|
||||
/// Returns a HashSet of all event IDs for:
|
||||
|
||||
+136
@@ -2402,11 +2402,22 @@ impl SyncManager {
|
||||
|
||||
// Collect all purgatory announcements (snapshot - no async holds)
|
||||
let announcements = self.purgatory.announcements_for_sync();
|
||||
let announcement_events = self.purgatory.announcement_events_for_sync();
|
||||
|
||||
if announcements.is_empty() {
|
||||
return;
|
||||
}
|
||||
|
||||
// A maintainer announcement may have been fetched and rejected before the
|
||||
// reciprocal owner announcement reached this relay. Retry those cached
|
||||
// dependencies as soon as the owner announcement enters purgatory. Waiting
|
||||
// for owner promotion would deadlock: promotion needs git data, while the
|
||||
// source clone URL belongs to the rejected maintainer announcement.
|
||||
for event in announcement_events {
|
||||
self.reprocess_purgatory_announcement_dependencies(&event)
|
||||
.await;
|
||||
}
|
||||
|
||||
// Register any new entries in repo_sync_index as StateOnly
|
||||
let mut new_relay_urls: std::collections::HashSet<String> =
|
||||
std::collections::HashSet::new();
|
||||
@@ -2463,6 +2474,108 @@ impl SyncManager {
|
||||
}
|
||||
}
|
||||
|
||||
/// Retry events whose authorization depends on a newly admitted owner announcement.
|
||||
///
|
||||
/// This mirrors the accepted-announcement dependency handling in
|
||||
/// `process_event_static`, but runs while the owner announcement is still in
|
||||
/// purgatory. Announcement policy already treats purgatory announcements as
|
||||
/// maintainer authority; doing the retry here makes arrival order irrelevant.
|
||||
async fn reprocess_purgatory_announcement_dependencies(&self, event: &Event) {
|
||||
let announcement =
|
||||
match crate::nostr::events::RepositoryAnnouncement::from_event(event.clone()) {
|
||||
Ok(announcement) => announcement,
|
||||
Err(error) => {
|
||||
tracing::warn!(
|
||||
event_id = %event.id,
|
||||
error = %error,
|
||||
"Failed to parse purgatory announcement for dependency retry"
|
||||
);
|
||||
return;
|
||||
}
|
||||
};
|
||||
|
||||
let relay_url = format!("ws://{}", self.service_domain);
|
||||
|
||||
for maintainer_hex in &announcement.maintainers {
|
||||
let maintainer_pubkey = match PublicKey::from_hex(maintainer_hex) {
|
||||
Ok(pubkey) => pubkey,
|
||||
Err(error) => {
|
||||
tracing::warn!(
|
||||
maintainer_hex = %maintainer_hex,
|
||||
error = %error,
|
||||
"Invalid maintainer public key in purgatory announcement"
|
||||
);
|
||||
continue;
|
||||
}
|
||||
};
|
||||
|
||||
let (removed, hot_events) = self.rejected_events_index.invalidate_and_get(
|
||||
&maintainer_pubkey,
|
||||
&announcement.identifier,
|
||||
Some(rejected_index::EventType::Announcement),
|
||||
);
|
||||
|
||||
if removed > 0 {
|
||||
tracing::info!(
|
||||
maintainer = %maintainer_hex,
|
||||
identifier = %announcement.identifier,
|
||||
removed_from_cold_index = removed,
|
||||
hot_cache_events = hot_events.len(),
|
||||
"Invalidated rejected maintainer announcements from purgatory owner relationship"
|
||||
);
|
||||
}
|
||||
|
||||
// A non-zero cold-index invalidation makes this retry one-shot while
|
||||
// still allowing the next timer tick to catch a concurrent rejection.
|
||||
if removed > 0 && !hot_events.is_empty() {
|
||||
Self::reprocess_events_from_hot_cache(
|
||||
hot_events,
|
||||
"maintainer announcement",
|
||||
&maintainer_pubkey,
|
||||
&announcement.identifier,
|
||||
&relay_url,
|
||||
&self.database,
|
||||
&self.write_policy,
|
||||
&self.local_relay,
|
||||
&self.rejected_events_index,
|
||||
)
|
||||
.await;
|
||||
}
|
||||
}
|
||||
|
||||
// The owner's state may also have arrived before their announcement.
|
||||
let (removed, hot_events) = self.rejected_events_index.invalidate_and_get(
|
||||
&event.pubkey,
|
||||
&announcement.identifier,
|
||||
Some(rejected_index::EventType::State),
|
||||
);
|
||||
|
||||
if removed > 0 {
|
||||
tracing::info!(
|
||||
owner = %event.pubkey,
|
||||
identifier = %announcement.identifier,
|
||||
removed_from_cold_index = removed,
|
||||
hot_cache_events = hot_events.len(),
|
||||
"Invalidated rejected owner state from purgatory announcement"
|
||||
);
|
||||
}
|
||||
|
||||
if removed > 0 && !hot_events.is_empty() {
|
||||
Self::reprocess_events_from_hot_cache(
|
||||
hot_events,
|
||||
"state event",
|
||||
&event.pubkey,
|
||||
&announcement.identifier,
|
||||
&relay_url,
|
||||
&self.database,
|
||||
&self.write_policy,
|
||||
&self.local_relay,
|
||||
&self.rejected_events_index,
|
||||
)
|
||||
.await;
|
||||
}
|
||||
}
|
||||
|
||||
/// Handle a relay disconnection
|
||||
///
|
||||
/// This method is called when the event loop terminates and sends a disconnect notification.
|
||||
@@ -2668,6 +2781,29 @@ impl SyncManager {
|
||||
"{} added to purgatory (waiting for git data)",
|
||||
context
|
||||
);
|
||||
|
||||
// Hot-cache retries are sync-originated events. Reset any
|
||||
// existing backoff so newly available announcement/clone
|
||||
// metadata is acted on immediately rather than inheriting
|
||||
// the direct-submission three-minute delay.
|
||||
let identifier = if event.kind == Kind::RepoState {
|
||||
event
|
||||
.tags
|
||||
.iter()
|
||||
.find(|tag| tag.kind() == "d")
|
||||
.and_then(|tag| tag.content())
|
||||
.map(str::to_owned)
|
||||
} else if event.kind == Kind::GitPullRequest
|
||||
|| event.kind == Kind::GitPullRequestUpdate
|
||||
{
|
||||
crate::git::sync::extract_identifier_from_pr_event(&event)
|
||||
} else {
|
||||
None
|
||||
};
|
||||
|
||||
if let Some(identifier) = identifier {
|
||||
write_policy.purgatory().enqueue_sync_immediate(&identifier);
|
||||
}
|
||||
}
|
||||
ProcessResult::Rejected => {
|
||||
stats.rejected += 1;
|
||||
|
||||
@@ -22,12 +22,198 @@
|
||||
//! announcements are in relay_a's DB), wait briefly for the sync round-trip, then
|
||||
//! send the owner announcement + git push.
|
||||
|
||||
use std::process::Command;
|
||||
use std::time::Duration;
|
||||
|
||||
use nostr_sdk::prelude::*;
|
||||
|
||||
use crate::common::{sync_helpers::*, TestRelay};
|
||||
|
||||
/// A reciprocal owner announcement in purgatory must be enough to unlock an
|
||||
/// earlier rejected maintainer announcement and use that maintainer's clone URL.
|
||||
///
|
||||
/// The new owner deliberately advertises only their own empty target-repo clone
|
||||
/// URL. The source clone remains owned by the existing maintainer announcement.
|
||||
/// No client-side copy of another maintainer's `clone` tag is required.
|
||||
#[tokio::test]
|
||||
async fn test_purgatory_owner_uses_rejected_maintainer_clone_to_sync_git() {
|
||||
let source_relay = TestRelay::start().await;
|
||||
let maintainer_keys = Keys::generate();
|
||||
let owner_keys = Keys::generate();
|
||||
let identifier = "purgatory-owner-maintainer-source";
|
||||
|
||||
let maintainer_npub = maintainer_keys
|
||||
.public_key()
|
||||
.to_bech32()
|
||||
.expect("Failed to get maintainer npub");
|
||||
let maintainer_clone = format!(
|
||||
"http://{}/{}/{}.git",
|
||||
source_relay.domain(),
|
||||
maintainer_npub,
|
||||
identifier
|
||||
);
|
||||
let maintainer_announcement =
|
||||
EventBuilder::new(Kind::GitRepoAnnouncement, "Existing maintainer repository")
|
||||
.tags(vec![
|
||||
Tag::identifier(identifier),
|
||||
Tag::custom("clone", vec![maintainer_clone.clone()]),
|
||||
Tag::custom("relays", vec![source_relay.url().to_string()]),
|
||||
])
|
||||
.finalize(&maintainer_keys)
|
||||
.expect("Failed to create maintainer announcement");
|
||||
|
||||
send_to_relay(&source_relay, &maintainer_announcement)
|
||||
.await
|
||||
.expect("Failed to send maintainer announcement");
|
||||
let maintainer_git = push_git_data_to_relay(
|
||||
&source_relay,
|
||||
&maintainer_keys,
|
||||
identifier,
|
||||
&[&source_relay.domain()],
|
||||
)
|
||||
.await;
|
||||
let commit_output = Command::new("git")
|
||||
.args(["rev-parse", "HEAD"])
|
||||
.current_dir(maintainer_git.path())
|
||||
.output()
|
||||
.expect("Failed to read maintainer commit");
|
||||
assert!(commit_output.status.success());
|
||||
let maintainer_commit = String::from_utf8(commit_output.stdout)
|
||||
.expect("Commit hash should be UTF-8")
|
||||
.trim()
|
||||
.to_string();
|
||||
|
||||
let source_announcement_found = wait_for_event_on_relay(
|
||||
source_relay.url(),
|
||||
Filter::new()
|
||||
.kind(Kind::GitRepoAnnouncement)
|
||||
.author(maintainer_keys.public_key())
|
||||
.identifier(identifier),
|
||||
Duration::from_secs(5),
|
||||
)
|
||||
.await;
|
||||
assert!(
|
||||
source_announcement_found,
|
||||
"Maintainer announcement should be served by source relay"
|
||||
);
|
||||
|
||||
// Bootstrap first so the source announcement is rejected and cached before
|
||||
// the reciprocal owner relationship exists.
|
||||
let target_relay = TestRelay::start_with_sync(Some(source_relay.url().to_string())).await;
|
||||
tokio::time::sleep(Duration::from_secs(3)).await;
|
||||
|
||||
let owner_npub = owner_keys
|
||||
.public_key()
|
||||
.to_bech32()
|
||||
.expect("Failed to get owner npub");
|
||||
let owner_clone = format!(
|
||||
"http://{}/{}/{}.git",
|
||||
target_relay.domain(),
|
||||
owner_npub,
|
||||
identifier
|
||||
);
|
||||
let owner_announcement =
|
||||
EventBuilder::new(Kind::GitRepoAnnouncement, "New reciprocal owner repository")
|
||||
.tags(vec![
|
||||
Tag::identifier(identifier),
|
||||
Tag::custom("clone", vec![owner_clone.clone()]),
|
||||
Tag::custom(
|
||||
"relays",
|
||||
vec![
|
||||
source_relay.url().to_string(),
|
||||
target_relay.url().to_string(),
|
||||
],
|
||||
),
|
||||
Tag::custom("maintainers", vec![maintainer_keys.public_key().to_hex()]),
|
||||
])
|
||||
.finalize(&owner_keys)
|
||||
.expect("Failed to create owner announcement");
|
||||
|
||||
let owner_clone_values: Vec<String> = owner_announcement
|
||||
.tags
|
||||
.iter()
|
||||
.find(|tag| tag.kind() == "clone")
|
||||
.expect("Owner announcement should have clone tag")
|
||||
.clone()
|
||||
.to_vec()
|
||||
.into_iter()
|
||||
.skip(1)
|
||||
.collect();
|
||||
assert_eq!(owner_clone_values, vec![owner_clone]);
|
||||
assert!(
|
||||
!owner_clone_values.contains(&maintainer_clone),
|
||||
"Owner must not claim the maintainer's clone URL"
|
||||
);
|
||||
|
||||
let started = std::time::Instant::now();
|
||||
send_to_relay(&target_relay, &owner_announcement)
|
||||
.await
|
||||
.expect("Failed to send reciprocal owner announcement");
|
||||
|
||||
let owner_found = wait_for_event_on_relay(
|
||||
target_relay.url(),
|
||||
Filter::new()
|
||||
.kind(Kind::GitRepoAnnouncement)
|
||||
.author(owner_keys.public_key())
|
||||
.identifier(identifier),
|
||||
Duration::from_secs(30),
|
||||
)
|
||||
.await;
|
||||
assert!(
|
||||
owner_found,
|
||||
"Owner announcement should promote after fetching the maintainer's git data"
|
||||
);
|
||||
|
||||
let maintainer_found = wait_for_event_on_relay(
|
||||
target_relay.url(),
|
||||
Filter::new()
|
||||
.kind(Kind::GitRepoAnnouncement)
|
||||
.author(maintainer_keys.public_key())
|
||||
.identifier(identifier),
|
||||
Duration::from_secs(5),
|
||||
)
|
||||
.await;
|
||||
assert!(
|
||||
maintainer_found,
|
||||
"Rejected maintainer announcement should be reprocessed before owner promotion"
|
||||
);
|
||||
|
||||
let state_found = wait_for_event_on_relay(
|
||||
target_relay.url(),
|
||||
Filter::new()
|
||||
.kind(Kind::RepoState)
|
||||
.author(maintainer_keys.public_key())
|
||||
.identifier(identifier),
|
||||
Duration::from_secs(5),
|
||||
)
|
||||
.await;
|
||||
assert!(
|
||||
state_found,
|
||||
"Maintainer state should be served after git synchronization"
|
||||
);
|
||||
|
||||
let ref_aligned = crate::common::check_ref_at_commit(
|
||||
&target_relay.domain(),
|
||||
&owner_npub,
|
||||
identifier,
|
||||
"refs/heads/main",
|
||||
&maintainer_commit,
|
||||
)
|
||||
.await
|
||||
.expect("Failed to inspect target owner ref");
|
||||
assert!(
|
||||
ref_aligned,
|
||||
"Target owner repository should align to the maintainer's state"
|
||||
);
|
||||
assert!(
|
||||
started.elapsed() < Duration::from_secs(30),
|
||||
"Reciprocal owner synchronization should not inherit a long retry delay"
|
||||
);
|
||||
|
||||
target_relay.stop().await;
|
||||
source_relay.stop().await;
|
||||
}
|
||||
|
||||
/// Test that a maintainer announcement is re-processed immediately when the owner
|
||||
/// announcement is promoted from purgatory via a git push.
|
||||
///
|
||||
|
||||
Reference in New Issue
Block a user