diff --git a/src/nostr/lifecycle/deletion/cascade/graph.rs b/src/nostr/lifecycle/deletion/cascade/graph.rs index 4d5a9e5..db1920d 100644 --- a/src/nostr/lifecycle/deletion/cascade/graph.rs +++ b/src/nostr/lifecycle/deletion/cascade/graph.rs @@ -1,8 +1,8 @@ -/// Event Dependency Graph Builder +/// Legacy Event Dependency Graph Builder /// -/// Builds a dependency graph for multi-maintainer deletion scenarios. -/// Tracks which events depend on which announcements, enabling selective -/// deletion when one maintainer removes their announcement but others remain. +/// Builds a dependency graph for offline cleanup reconciliation scenarios. +/// This module is retained for `cleanup-empty-repos`; it is not used by the +/// production NIP-09 announcement cascade, which lives in `orchestration.rs`. use std::collections::{HashMap, HashSet}; use nostr_relay_builder::prelude::{Event, EventId, Kind}; diff --git a/src/nostr/lifecycle/deletion/cascade/mod.rs b/src/nostr/lifecycle/deletion/cascade/mod.rs index 1107d00..334a2b4 100644 --- a/src/nostr/lifecycle/deletion/cascade/mod.rs +++ b/src/nostr/lifecycle/deletion/cascade/mod.rs @@ -1,3 +1,10 @@ +//! Announcement cascade deletion implementation. +//! +//! `orchestration` contains the production NIP-09/blacklist/whitelist cascade +//! algorithm. The `graph`, `reevaluation`, and `traversal` modules are legacy +//! reconciliation helpers retained for `cleanup-empty-repos`; their tests cover +//! that offline reconciliation behavior, not the live deletion cascade. + mod graph; mod orchestration; mod reevaluation; diff --git a/src/nostr/lifecycle/deletion/cascade/orchestration.rs b/src/nostr/lifecycle/deletion/cascade/orchestration.rs index 106b80c..1323083 100644 --- a/src/nostr/lifecycle/deletion/cascade/orchestration.rs +++ b/src/nostr/lifecycle/deletion/cascade/orchestration.rs @@ -33,7 +33,8 @@ struct ParsedAddressRef { struct CascadeGraph { nodes: HashMap, event_to_refs: HashMap>, - ref_to_events: HashMap>, + identity_to_event_ids: HashMap>, + ref_to_referencing_event_ids: HashMap>, } impl CascadeGraph { @@ -43,19 +44,19 @@ impl CascadeGraph { self.event_to_refs.insert(event_id, refs.clone()); for ref_key in refs { - self.ref_to_events + self.ref_to_referencing_event_ids .entry(ref_key) .or_default() .insert(event_id); } if let Some(address) = event_address(&event) { - self.ref_to_events + self.identity_to_event_ids .entry(RefKey::Address(address)) .or_default() .insert(event_id); } - self.ref_to_events + self.identity_to_event_ids .entry(RefKey::Event(event_id)) .or_default() .insert(event_id); @@ -64,7 +65,10 @@ impl CascadeGraph { } fn edge_count(&self) -> usize { - self.event_to_refs.values().map(HashSet::len).sum() + self.ref_to_referencing_event_ids + .values() + .map(HashSet::len) + .sum() } } @@ -120,22 +124,35 @@ impl DeletionPolicy { } let identifier = parts[2]; - let deleted_announcement_id = self - .query_address_events(announcement_addr) + let deletable_announcements = match self + .query_address_events_until(announcement_addr, deletion_created_at) .await - .ok() - .and_then(|events| { - events - .into_iter() - .find(|e| e.kind == Kind::GitRepoAnnouncement) - }) - .map(|event| event.id) - .unwrap_or_else(EventId::all_zeros); + { + Ok(events) => events + .into_iter() + .filter(|event| event.kind == Kind::GitRepoAnnouncement) + .collect::>(), + Err(e) => { + tracing::warn!(error = %e, announcement = %announcement_addr, "Cascade deletion: failed to verify announcement versions under deletion cutoff"); + return; + } + }; - let mut seed_event_ids = HashSet::new(); - if deleted_announcement_id != EventId::all_zeros() { - seed_event_ids.insert(deleted_announcement_id); + if deletable_announcements.is_empty() { + tracing::debug!( + announcement = %announcement_addr, + deletion_created_at = deletion_created_at.as_secs(), + "Cascade deletion skipped: no announcement version is deletable under NIP-09 cutoff" + ); + return; } + + let deleted_announcement_ids: HashSet = deletable_announcements + .iter() + .map(|event| event.id) + .collect(); + + let seed_event_ids = deleted_announcement_ids.clone(); let seed_addresses = HashSet::from([announcement_addr.to_string()]); let graph = match self @@ -158,18 +175,10 @@ impl DeletionPolicy { } }; - let kept_events = compute_retained_events( - &graph, - announcement_addr, - deleted_announcement_id, - &self.ctx.config.domain, - ); - let delete_events = compute_deletable_events( - &graph, - &kept_events, - announcement_addr, - deleted_announcement_id, - ); + let kept_events = + compute_retained_events(&graph, &deleted_announcement_ids, &self.ctx.config.domain); + let delete_events = + compute_deletable_events(&graph, &kept_events, &deleted_announcement_ids); tracing::info!( announcement = %announcement_addr, @@ -408,8 +417,12 @@ impl DeletionPolicy { expand_cascade_graph_from_db(&self.ctx.database, seed_event_ids, seed_addresses).await } - async fn query_address_events(&self, address: &str) -> Result, String> { - query_address_events(&self.ctx.database, address).await + async fn query_address_events_until( + &self, + address: &str, + until: Timestamp, + ) -> Result, String> { + query_address_events_with_until(&self.ctx.database, address, Some(until)).await } } @@ -459,7 +472,7 @@ async fn expand_cascade_graph_from_db( } for address in address_batch { loaded.extend( - query_address_events(database, &address) + query_address_events_with_until(database, &address, None) .await .map_err(CascadeExpansionError::Query)?, ); @@ -547,9 +560,10 @@ async fn query_events_by_ids( .map_err(|e| CascadeExpansionError::Query(format!("query ids failed: {e}"))) } -async fn query_address_events( +async fn query_address_events_with_until( database: &SharedDatabase, address: &str, + until: Option, ) -> Result, String> { let Some(parsed) = parse_accepted_address_ref(address) else { return Ok(Vec::new()); @@ -559,6 +573,9 @@ async fn query_address_events( if let Some(identifier) = parsed.identifier { filter = filter.custom_tag(SingleLetterTag::lowercase(Alphabet::D), identifier); } + if let Some(until) = until { + filter = filter.until(until); + } database .query(filter) @@ -651,21 +668,17 @@ async fn query_tag_variants( fn compute_retained_events( graph: &CascadeGraph, - deleted_address: &str, - deleted_announcement_id: EventId, + deleted_announcement_ids: &HashSet, domain: &str, ) -> HashSet { let mut kept = HashSet::new(); let mut vetoed_git_nodes = HashSet::new(); for event in graph.nodes.values() { - if event.id == deleted_announcement_id - || event_address(event).as_deref() == Some(deleted_address) - || vetoed_git_nodes.contains(&event.id) - { + if deleted_announcement_ids.contains(&event.id) || vetoed_git_nodes.contains(&event.id) { continue; } - if is_independent_anchor(event, deleted_address, domain) { + if is_independent_anchor(event, deleted_announcement_ids, domain) { kept.insert(event.id); } } @@ -677,8 +690,7 @@ fn compute_retained_events( for event in graph.nodes.values() { if kept.contains(&event.id) || vetoed_git_nodes.contains(&event.id) - || event.id == deleted_announcement_id - || event_address(event).as_deref() == Some(deleted_address) + || deleted_announcement_ids.contains(&event.id) || should_exclude_from_generic_graph(event) || should_not_recurse_through(event) { @@ -742,16 +754,14 @@ fn refs_for_kept_events(graph: &CascadeGraph, kept: &HashSet) -> KeptRe fn compute_deletable_events( graph: &CascadeGraph, kept: &HashSet, - deleted_address: &str, - deleted_announcement_id: EventId, + deleted_announcement_ids: &HashSet, ) -> HashSet { graph .nodes .values() .filter(|event| { !kept.contains(&event.id) - && event.id != deleted_announcement_id - && event_address(event).as_deref() != Some(deleted_address) + && !deleted_announcement_ids.contains(&event.id) && !should_exclude_from_generic_graph(event) && !should_not_recurse_through(event) }) @@ -759,9 +769,17 @@ fn compute_deletable_events( .collect() } -fn is_independent_anchor(event: &Event, deleted_address: &str, domain: &str) -> bool { +fn is_independent_anchor( + event: &Event, + deleted_announcement_ids: &HashSet, + domain: &str, +) -> bool { if event.kind == Kind::GitRepoAnnouncement { - return event_address(event).as_deref() != Some(deleted_address); + return !deleted_announcement_ids.contains(&event.id); + } + + if event.kind == Kind::GitUserGraspList { + return true; } crate::grasp06::policy::event_names_relays_prs_endpoint(event, domain) @@ -799,7 +817,7 @@ fn event_references_kept_repo_announcement( .into_iter() .flatten() .any(|ref_key| { - graph.ref_to_events.get(ref_key).is_some_and(|ids| { + graph.identity_to_event_ids.get(ref_key).is_some_and(|ids| { ids.iter().any(|id| { kept.contains(id) && graph diff --git a/src/nostr/lifecycle/deletion/cascade/reevaluation.rs b/src/nostr/lifecycle/deletion/cascade/reevaluation.rs index 309b118..fa5e61e 100644 --- a/src/nostr/lifecycle/deletion/cascade/reevaluation.rs +++ b/src/nostr/lifecycle/deletion/cascade/reevaluation.rs @@ -1,4 +1,4 @@ -/// Re-evaluation Engine - Determines event retention after announcement deletion +/// Legacy Re-evaluation Engine - Determines event retention after announcement deletion /// /// When a repository announcement is deleted, this module re-evaluates all dependent /// events to determine which should be kept (still acceptable through other announcements) @@ -6,6 +6,9 @@ /// /// This is critical for multi-maintainer scenarios where one maintainer deletes their /// announcement but other maintainers' announcements should keep the events alive. +/// +/// This module is retained for legacy/offline reconciliation callers. The +/// production NIP-09 announcement cascade algorithm lives in `orchestration.rs`. use std::collections::{HashMap, HashSet}; use nostr_relay_builder::prelude::{Event, EventId, Filter, Kind}; @@ -34,6 +37,9 @@ pub struct RetentionReason { /// Re-evaluate events that were dependent on a deleted announcement /// +/// Legacy reconciliation helper. Do not use this as a description of the +/// production NIP-09 announcement cascade. +/// /// This function determines which events should be kept vs deleted when an /// announcement is removed. It checks if events have alternative retention /// reasons through other valid announcements. diff --git a/src/nostr/lifecycle/deletion/cascade/traversal.rs b/src/nostr/lifecycle/deletion/cascade/traversal.rs index 482e333..4fc5d9c 100644 --- a/src/nostr/lifecycle/deletion/cascade/traversal.rs +++ b/src/nostr/lifecycle/deletion/cascade/traversal.rs @@ -1,10 +1,14 @@ -/// Graph Traversal and Circular Dependency Detection +/// Legacy Graph Traversal and Circular Dependency Detection /// -/// Determines which events to keep vs delete in multi-maintainer scenarios by: +/// Determines which events to keep vs delete for offline cleanup reconciliation +/// by: /// 1. Starting from kept repository announcements (kind 30617) /// 2. Traversing the dependency graph to find all reachable events /// 3. Detecting circular dependencies and handling them appropriately /// 4. Marking unreachable events for deletion +/// +/// This module is retained for `cleanup-empty-repos`; it is not used by the +/// production NIP-09 announcement cascade, which lives in `orchestration.rs`. use std::collections::{HashMap, HashSet, VecDeque}; use nostr_relay_builder::prelude::EventId; @@ -34,6 +38,9 @@ pub struct CircularDependency { /// Traverse the event graph and mark events for deletion /// +/// Legacy reconciliation helper for `cleanup-empty-repos`. Do not use this as a +/// description of the production NIP-09 announcement cascade. +/// /// This function implements a BFS traversal starting from kept repository announcements, /// marking all reachable events as KEEP and unreachable events as DELETE. /// diff --git a/tests/nip09_announcement_cascade.rs b/tests/nip09_announcement_cascade.rs index 2137d13..7da6648 100644 --- a/tests/nip09_announcement_cascade.rs +++ b/tests/nip09_announcement_cascade.rs @@ -246,6 +246,121 @@ async fn test_deleting_patch_by_event_id_does_not_delete_announcement_or_sibling assert!(!patch_survived, "patch must be deleted by its e-tag"); } +/// A stale NIP-09 coordinate deletion must not cascade through the current +/// announcement. The deletion's created_at predates the announcement, so no +/// kind-30617 version is actually deletable under NIP-09 replaceable semantics. +#[tokio::test] +async fn test_stale_announcement_coordinate_deletion_does_not_cascade() { + let relay = TestRelay::start().await; + let client = AuditClient::new(relay.url(), AuditConfig::isolated()) + .await + .expect("create audit client"); + + let (announcement, repo_id) = publish_served_repo(&client, "stale-coordinate-delete").await; + let coordinate = announcement_coordinate(&announcement, &repo_id); + + let issue = client + .create_issue(&announcement, "Stale delete issue", "must survive", vec![]) + .expect("build issue"); + client + .send_event(issue.clone()) + .await + .expect("relay should accept dependent event"); + + tokio::time::sleep(Duration::from_millis(300)).await; + + let stale_timestamp = Timestamp::from_secs(announcement.created_at.as_secs().saturating_sub(1)); + let stale_deletion = client + .event_builder(Kind::EventDeletion, "stale delete") + .custom_time(stale_timestamp) + .tag(Tag::custom("a", vec![coordinate.clone()])) + .build(client.keys()) + .expect("build stale deletion event"); + client + .send_event(stale_deletion) + .await + .expect("relay should accept stale deletion request"); + + tokio::time::sleep(Duration::from_millis(600)).await; + + let announcement_survived = client + .is_event_on_relay(announcement.id) + .await + .expect("query announcement after stale deletion"); + let issue_survived = client + .is_event_on_relay(issue.id) + .await + .expect("query issue after stale deletion"); + relay.stop().await; + + assert!( + announcement_survived, + "newer announcement must survive stale coordinate deletion" + ); + assert!( + issue_survived, + "dependent event must not be cascade-deleted when the announcement survives" + ); +} + +/// Kind 10317 user grasp lists are independently accepted by the write policy, +/// so cascade retention must treat them as independent anchors even if they are +/// connected to a deleted repository component. +#[tokio::test] +async fn test_user_grasp_list_survives_connected_announcement_cascade() { + let relay = TestRelay::start().await; + let client = AuditClient::new(relay.url(), AuditConfig::isolated()) + .await + .expect("create audit client"); + + let (announcement, repo_id) = publish_served_repo(&client, "user-grasp-list-anchor").await; + let coordinate = announcement_coordinate(&announcement, &repo_id); + + let grasp_list = client + .event_builder(Kind::GitUserGraspList, "") + .tag(Tag::custom("a", vec![coordinate.clone()])) + .build(client.keys()) + .expect("build 10317 user grasp list"); + client + .send_event(grasp_list.clone()) + .await + .expect("relay should accept 10317 user grasp list"); + + tokio::time::sleep(Duration::from_millis(300)).await; + + assert!( + client + .is_event_on_relay(grasp_list.id) + .await + .expect("query 10317 before deletion"), + "10317 precondition: event must be served before cascade" + ); + + let deletion = build_deletion(&client, &[], std::slice::from_ref(&coordinate)); + client + .send_event(deletion) + .await + .expect("relay should accept the announcement deletion"); + + tokio::time::sleep(Duration::from_millis(600)).await; + + let announcement_survived = client + .is_event_on_relay(announcement.id) + .await + .expect("query announcement after deletion"); + let grasp_list_survived = client + .is_event_on_relay(grasp_list.id) + .await + .expect("query 10317 after deletion"); + relay.stop().await; + + assert!(!announcement_survived, "announcement must still be deleted"); + assert!( + grasp_list_survived, + "independently accepted 10317 must survive connected announcement cascade" + ); +} + /// Deleting a comment by `e` tag must not trigger repository graph cascade: /// the parent issue and announcement must survive. #[tokio::test]