From 9621d88807005b5b62967f705fec550ea8c51f22 Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Thu, 16 Jul 2026 17:46:20 +0100 Subject: [PATCH] feat: measure deletion outcomes --- docs/explanation/repository-lifecycle.md | 18 ++- src/nostr/lifecycle/deletion/archival.rs | 137 ++++++++++++------- src/nostr/lifecycle/deletion/cascade.rs | 152 ++++++++++++---------- src/nostr/lifecycle/deletion/mod.rs | 62 +++++++++ src/nostr/lifecycle/deletion/policy.rs | 77 ++++++++++- src/nostr/lifecycle/deletion/purgatory.rs | 72 +++++++--- src/nostr/lifecycle/deletion/service.rs | 53 +++++++- 7 files changed, 424 insertions(+), 147 deletions(-) diff --git a/docs/explanation/repository-lifecycle.md b/docs/explanation/repository-lifecycle.md index f2d8349..c1a299e 100644 --- a/docs/explanation/repository-lifecycle.md +++ b/docs/explanation/repository-lifecycle.md @@ -8,12 +8,12 @@ NIP-09 deletion requests, NIP-62 request-to-vanish events, operator blacklist an whitelist reconciliation, service de-listing, holding/archive retention, recovery, and purgatory transitions. -## Proposed bounded request retention +## Bounded request retention -> **Status:** Desired architecture, not yet implemented. The current code keeps -> accepted NIP-09 requests in the served main database and in the -> tombstone database without age-based expiry. This section records the target -> behavior for approval before implementation planning. +> **Status:** Lifecycle metadata, deterministic admission-use attribution, and +> measured destructive outcomes are implemented. Request cleanup/expiry, +> migration, target-set deduplication removal, and the disrespector +> would-have-deleted classification remain future stages. ### Production motivation @@ -492,6 +492,14 @@ deprecated in favor of the maintenance command. - Delete corresponding archive files ``` +Deletion processing records a structured outcome: successful main-database +removals and successful purgatory removals are counted separately from skipped +fail-safe preservation and query/delete failures. A kind-5 request receives +`last_used_at` only after at least one actual main-DB or purgatory event removal; +archiving to Holding, git/filesystem cleanup, candidate matching, and failed +operations do not constitute use. NIP-62 uses the same rule, including its +author-wide purgatory eviction. + Deletion gate checks tombstones before kind-specific admission and rejects: - events from vanished pubkeys, - re-submission of deleted event IDs, diff --git a/src/nostr/lifecycle/deletion/archival.rs b/src/nostr/lifecycle/deletion/archival.rs index 349ced1..fa8dbd7 100644 --- a/src/nostr/lifecycle/deletion/archival.rs +++ b/src/nostr/lifecycle/deletion/archival.rs @@ -9,7 +9,10 @@ use nostr_relay_builder::prelude::{ }; use tar::Builder as TarBuilder; -use super::policy::{identifier_from_event, owner_directory_component, DeletionPolicy}; +use super::{ + policy::{identifier_from_event, owner_directory_component, DeletionPolicy}, + DeletionOutcome, +}; use crate::nostr::lifecycle::{DeletionSource, GitArchiveMetadata, HoldingMetadata}; #[derive(Debug, Clone, PartialEq, Eq, Hash)] @@ -29,7 +32,11 @@ impl DeletionPolicy { /// repository archive linkage. Any remaining author events are then moved to /// holding before main-DB deletion. The caller is responsible for recording /// the vanish tombstone before invoking this method. - pub(super) async fn apply_nip62_vanish(&self, event: &Event) -> anyhow::Result<()> { + pub(super) async fn apply_nip62_vanish( + &self, + event: &Event, + ) -> anyhow::Result { + let mut outcome = DeletionOutcome::default(); let author = event.pubkey; let deleted_at = event.created_at; @@ -64,13 +71,15 @@ impl DeletionPolicy { continue; }; let coordinate = format!("30617:{}:{}", author.to_hex(), identifier); - self.cascade_delete_announcement( - &author, - &coordinate, - deleted_at, - DeletionSource::Nip62, - ) - .await; + outcome.merge( + self.cascade_delete_announcement( + &author, + &coordinate, + deleted_at, + DeletionSource::Nip62, + ) + .await, + ); } let mut moved_ids = HashSet::new(); @@ -83,15 +92,17 @@ impl DeletionPolicy { git_archive: None, }; - self.archive_and_delete_filter( - Filter::new().author(author), - &metadata, - &mut moved_ids, - "NIP-62 vanish remaining author-event deletion", - ) - .await; + outcome.merge( + self.archive_and_delete_filter( + Filter::new().author(author), + &metadata, + &mut moved_ids, + "NIP-62 vanish remaining author-event deletion", + ) + .await, + ); - Ok(()) + Ok(outcome) } /// Hard-delete the targeted events from the main database. @@ -103,7 +114,8 @@ impl DeletionPolicy { &self, event: &Event, moved_ids: &mut HashSet, - ) { + ) -> DeletionOutcome { + let mut outcome = DeletionOutcome::default(); // `e` tags: delete by event id. let ids = Self::e_tag_ids(event); if !ids.is_empty() { @@ -116,8 +128,15 @@ impl DeletionPolicy { owner_pubkey: None, git_archive: None, }; - self.archive_and_delete_filter(filter, &metadata, moved_ids, "NIP-09 e-tag deletion") - .await; + outcome.merge( + self.archive_and_delete_filter( + filter, + &metadata, + moved_ids, + "NIP-09 e-tag deletion", + ) + .await, + ); } // `a` tags: delete matching addressable/replaceable events up to the @@ -134,24 +153,29 @@ impl DeletionPolicy { continue; } if Self::is_announcement_coordinate_for_author(&v[1], &event.pubkey) { - self.cascade_delete_announcement( - &event.pubkey, - &v[1], - event.created_at, - DeletionSource::Nip09, - ) - .await; + outcome.merge( + self.cascade_delete_announcement( + &event.pubkey, + &v[1], + event.created_at, + DeletionSource::Nip09, + ) + .await, + ); } else { - self.delete_coordinate_from_main_db( - &event.pubkey, - &v[1], - event.created_at, - moved_ids, - DeletionSource::Nip09, - ) - .await; + outcome.merge( + self.delete_coordinate_from_main_db( + &event.pubkey, + &v[1], + event.created_at, + moved_ids, + DeletionSource::Nip09, + ) + .await, + ); } } + outcome } /// Whether `coordinate` is a kind-30617 announcement coordinate whose @@ -173,21 +197,21 @@ impl DeletionPolicy { deletion_created_at: nostr_relay_builder::prelude::Timestamp, moved_ids: &mut HashSet, source: DeletionSource, - ) { + ) -> DeletionOutcome { // coordinate: `::` let parts: Vec<&str> = coordinate.splitn(3, ':').collect(); if parts.len() != 3 { - return; + return DeletionOutcome::default(); } let Ok(kind_num) = parts[0].parse::() else { - return; + return DeletionOutcome::default(); }; let coord_pubkey_hex = parts[1]; let identifier = parts[2]; // The coordinate pubkey must match the deletion author. if coord_pubkey_hex != author.to_hex() { - return; + return DeletionOutcome::default(); } let kind = Kind::from(kind_num); @@ -234,11 +258,14 @@ impl DeletionPolicy { op = "coordinate deletion", "Skipping announcement deletion because git archival failed (fail-safe preservation)" ); - return; + return DeletionOutcome { + skipped: 1, + ..Default::default() + }; } self.archive_and_delete_filter(filter, &metadata, moved_ids, "coordinate deletion") - .await; + .await } pub(super) async fn archive_and_delete_filter( @@ -247,20 +274,24 @@ impl DeletionPolicy { metadata: &HoldingMetadata, moved_ids: &mut HashSet, context: &str, - ) { + ) -> DeletionOutcome { let matches = match self.ctx.database.query(filter).await { Ok(events) => events, Err(e) => { tracing::warn!(error = %e, op = %context, "Failed to query deletion targets"); - return; + return DeletionOutcome { + failures: 1, + ..Default::default() + }; } }; if matches.is_empty() { - return; + return DeletionOutcome::default(); } - let mut deletable = Vec::with_capacity(matches.len()); + let mut deletable = HashSet::with_capacity(matches.len()); + let mut outcome = DeletionOutcome::default(); let mut deleted_announcement_repo_scopes = HashSet::new(); for event in matches { let mut event_metadata = metadata.clone(); @@ -309,6 +340,7 @@ impl DeletionPolicy { op = %context, "Skipping announcement deletion because git archival failed (fail-safe preservation)" ); + outcome.skipped = outcome.skipped.saturating_add(1); continue; } @@ -322,7 +354,7 @@ impl DeletionPolicy { } } - if moved_ids.insert(event.id) { + if !moved_ids.contains(&event.id) { if let Err(e) = self .ctx .holding @@ -335,8 +367,10 @@ impl DeletionPolicy { op = %context, "Skipping main-DB deletion because holding archival failed" ); + outcome.skipped = outcome.skipped.saturating_add(1); continue; } + moved_ids.insert(event.id); if matches!( event.kind, @@ -345,23 +379,26 @@ impl DeletionPolicy { self.delete_pr_event_git_refs(&event).await; } } - deletable.push(event.id); + deletable.insert(event.id); } if deletable.is_empty() { - return; + return outcome; } + let deleted_count = deletable.len(); let delete_filter = Filter::new().ids(deletable); if let Err(e) = self.ctx.database.delete(delete_filter).await { tracing::warn!(error = %e, op = %context, "Failed to delete events from main DB"); - return; + outcome.failures = outcome.failures.saturating_add(1); + return outcome; } - for scope in deleted_announcement_repo_scopes { self.remove_live_repo_if_announcement_scope_unserved(scope, context) .await; } + outcome.main_db_deleted = outcome.main_db_deleted.saturating_add(deleted_count); + outcome } async fn remove_live_repo_if_announcement_scope_unserved( diff --git a/src/nostr/lifecycle/deletion/cascade.rs b/src/nostr/lifecycle/deletion/cascade.rs index 01bc2a8..5990251 100644 --- a/src/nostr/lifecycle/deletion/cascade.rs +++ b/src/nostr/lifecycle/deletion/cascade.rs @@ -10,6 +10,7 @@ use super::policy::{ use crate::nostr::lifecycle::{DeletionSource, HoldingMetadata, HOLDING_METADATA_KIND}; use crate::nostr::SharedDatabase; +use super::DeletionOutcome; use crate::nostr::lifecycle::history::HISTORY_METADATA_KIND; const MAX_CASCADE_CANDIDATE_EVENTS: usize = 50_000; @@ -121,14 +122,15 @@ impl DeletionPolicy { announcement_addr: &str, deletion_created_at: Timestamp, source: DeletionSource, - ) { + ) -> DeletionOutcome { + let mut outcome = DeletionOutcome::default(); let mut moved_ids = HashSet::new(); // Parse `30617::` (already validated as a 30617 // coordinate for this author, but re-parse defensively). let parts: Vec<&str> = announcement_addr.splitn(3, ':').collect(); if parts.len() != 3 { - return; + return outcome; } let identifier = parts[2]; @@ -142,7 +144,10 @@ impl DeletionPolicy { .collect::>(), Err(e) => { tracing::warn!(error = %e, announcement = %announcement_addr, "Cascade deletion: failed to verify announcement versions under deletion cutoff"); - return; + return DeletionOutcome { + failures: 1, + ..Default::default() + }; } }; @@ -152,7 +157,7 @@ impl DeletionPolicy { deletion_created_at = deletion_created_at.as_secs(), "Cascade deletion skipped: no announcement version is deletable under NIP-09 cutoff" ); - return; + return outcome; } let plan = match plan_deleted_announcement_cascade( @@ -165,16 +170,16 @@ impl DeletionPolicy { Ok(plan) => plan, Err(e) => { tracing::warn!(error = %e, "Cascade deletion: graph expansion failed; falling back to simple coordinate deletion"); - self.delete_announcement_coordinate_and_maybe_state( - author, - announcement_addr, - identifier, - deletion_created_at, - &mut moved_ids, - source, - ) - .await; - return; + return self + .delete_announcement_coordinate_and_maybe_state( + author, + announcement_addr, + identifier, + deletion_created_at, + &mut moved_ids, + source, + ) + .await; } }; @@ -196,16 +201,16 @@ impl DeletionPolicy { max = MAX_CASCADE_ORPHAN_DELETES, "Cascade deletion orphan set exceeds limit; deleting announcement coordinate only" ); - self.delete_announcement_coordinate_and_maybe_state( - author, - announcement_addr, - identifier, - deletion_created_at, - &mut moved_ids, - source, - ) - .await; - return; + return self + .delete_announcement_coordinate_and_maybe_state( + author, + announcement_addr, + identifier, + deletion_created_at, + &mut moved_ids, + source, + ) + .await; } if !orphan_ids.is_empty() { tracing::debug!( @@ -227,27 +232,32 @@ impl DeletionPolicy { ) .await, }; - self.archive_and_delete_filter( - filter, - &metadata, - &mut moved_ids, - "cascade orphan deletion", - ) - .await; + outcome.merge( + self.archive_and_delete_filter( + filter, + &metadata, + &mut moved_ids, + "cascade orphan deletion", + ) + .await, + ); } // Step 5b: hard-delete the announcement coordinate itself (up to the // deletion's created_at, per NIP-09), exactly as the simple path does, // then clean state if this identifier is now unanchored. - self.delete_announcement_coordinate_and_maybe_state( - author, - announcement_addr, - identifier, - deletion_created_at, - &mut moved_ids, - source, - ) - .await; + outcome.merge( + self.delete_announcement_coordinate_and_maybe_state( + author, + announcement_addr, + identifier, + deletion_created_at, + &mut moved_ids, + source, + ) + .await, + ); + outcome } /// Apply operator-driven blacklist deletion for a stored kind-30617 @@ -275,13 +285,14 @@ impl DeletionPolicy { }; let coordinate = format!("30617:{}:{}", announcement.pubkey.to_hex(), identifier); - self.cascade_delete_announcement( - &announcement.pubkey, - &coordinate, - Timestamp::now(), - DeletionSource::Blacklist, - ) - .await; + let _ = self + .cascade_delete_announcement( + &announcement.pubkey, + &coordinate, + Timestamp::now(), + DeletionSource::Blacklist, + ) + .await; Ok(()) } @@ -332,7 +343,7 @@ impl DeletionPolicy { deletion_created_at: Timestamp, moved_ids: &mut HashSet, source: DeletionSource, - ) { + ) -> DeletionOutcome { let announcement_filter = Filter::new().kind(Kind::GitRepoAnnouncement).custom_tag( SingleLetterTag::lowercase(Alphabet::D), identifier.to_string(), @@ -346,12 +357,15 @@ impl DeletionPolicy { identifier = %identifier, "Cascade deletion: failed to check remaining announcements for identifier" ); - return; + return DeletionOutcome { + failures: 1, + ..Default::default() + }; } }; if !remaining_announcements.is_empty() { - return; + return DeletionOutcome::default(); } let state_filter = Filter::new().kind(Kind::RepoState).custom_tag( @@ -373,7 +387,7 @@ impl DeletionPolicy { moved_ids, "unanchored state deletion", ) - .await; + .await } async fn delete_announcement_coordinate_and_maybe_state( @@ -384,27 +398,31 @@ impl DeletionPolicy { deletion_created_at: Timestamp, moved_ids: &mut HashSet, source: DeletionSource, - ) { - self.delete_coordinate_from_main_db( - author, - announcement_addr, - deletion_created_at, - moved_ids, - source, - ) - .await; + ) -> DeletionOutcome { + let mut outcome = self + .delete_coordinate_from_main_db( + author, + announcement_addr, + deletion_created_at, + moved_ids, + source, + ) + .await; // Repository state (30618) is keyed by identifier, not by graph edges. // After deleting this announcement coordinate, delete state events for // the identifier only when no announcement for that identifier remains // in the main DB. This must also run on coordinate-only fallbacks. - self.delete_repo_state_if_identifier_is_unanchored( - identifier, - deletion_created_at, - moved_ids, - source, - ) - .await; + outcome.merge( + self.delete_repo_state_if_identifier_is_unanchored( + identifier, + deletion_created_at, + moved_ids, + source, + ) + .await, + ); + outcome } async fn query_address_events_until( diff --git a/src/nostr/lifecycle/deletion/mod.rs b/src/nostr/lifecycle/deletion/mod.rs index 7242de4..84b4a79 100644 --- a/src/nostr/lifecycle/deletion/mod.rs +++ b/src/nostr/lifecycle/deletion/mod.rs @@ -10,6 +10,38 @@ mod runtime; mod service; mod startup; +/// Measured result of destructive deletion work. +/// +/// Only the two successful removal counters make a deletion request "used". +/// `skipped` and `failures` are diagnostic counters for fail-safe preservation +/// and unsuccessful database/filesystem operations respectively. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub(crate) struct DeletionOutcome { + pub main_db_deleted: usize, + pub purgatory_removed: usize, + pub skipped: usize, + pub failures: usize, +} + +impl DeletionOutcome { + pub(crate) fn removed_events(self) -> usize { + self.main_db_deleted.saturating_add(self.purgatory_removed) + } + + pub(crate) fn used(self) -> bool { + self.removed_events() > 0 + } + + pub(crate) fn merge(&mut self, other: Self) { + self.main_db_deleted = self.main_db_deleted.saturating_add(other.main_db_deleted); + self.purgatory_removed = self + .purgatory_removed + .saturating_add(other.purgatory_removed); + self.skipped = self.skipped.saturating_add(other.skipped); + self.failures = self.failures.saturating_add(other.failures); + } +} + pub use context::DeletionContext; pub use policy::DeletionPolicy; pub use runtime::{run_holding_eject, DeletionCleanupTask, DeletionRuntime, HoldingEjectArgs}; @@ -18,3 +50,33 @@ pub use startup::{ BlacklistParityStats, BlacklistRestoreStats, StartupReconciliationStats, WhitelistParityStats, WhitelistRestoreStats, }; + +#[cfg(test)] +mod tests { + use super::DeletionOutcome; + + #[test] + fn outcome_merge_is_saturating_and_only_removals_are_use() { + let mut outcome = DeletionOutcome { + skipped: usize::MAX, + failures: 1, + ..Default::default() + }; + outcome.merge(DeletionOutcome { + main_db_deleted: 1, + purgatory_removed: 2, + skipped: 1, + failures: 2, + }); + assert_eq!(outcome.removed_events(), 3); + assert!(outcome.used()); + assert_eq!(outcome.skipped, usize::MAX); + assert_eq!(outcome.failures, 3); + assert!(!DeletionOutcome { + skipped: 1, + failures: 1, + ..Default::default() + } + .used()); + } +} diff --git a/src/nostr/lifecycle/deletion/policy.rs b/src/nostr/lifecycle/deletion/policy.rs index 7b75aa7..3f22b2f 100644 --- a/src/nostr/lifecycle/deletion/policy.rs +++ b/src/nostr/lifecycle/deletion/policy.rs @@ -180,11 +180,11 @@ impl DeletionPolicy { let mut identifiers_to_realign = Self::state_a_tag_identifiers_for_author(event); // Process purgatory removals (synchronous, in-memory). - self.remove_purgatory_targets(event); + let mut outcome = self.remove_purgatory_targets(event); // Move targeted events into holding DB, then delete from main DB. let mut moved_ids = HashSet::new(); - self.delete_main_db_targets(event, &mut moved_ids).await; + outcome.merge(self.delete_main_db_targets(event, &mut moved_ids).await); for plan in &rollback_plans { self.rollback_deleted_active_replaceable(plan).await; @@ -197,6 +197,29 @@ impl DeletionPolicy { self.realign_identifier_state(&identifier).await; } + if outcome.used() { + match self + .ctx + .tombstones + .mark_request_used(&event.id, nostr_relay_builder::prelude::Timestamp::now()) + .await + { + Ok(Some(_)) => {} + Ok(None) => { + return reject_error( + "internal error updating deletion lifecycle: missing metadata", + ) + } + Err(e) => { + tracing::error!(event_id = %event.id.to_hex(), error = %e, "Failed to mark used NIP-09 deletion request"); + return reject_error(format!( + "internal error updating deletion lifecycle: {e}" + )); + } + } + } + tracing::info!(event_id = %event.id.to_hex(), main_db_deleted = outcome.main_db_deleted, purgatory_removed = outcome.purgatory_removed, skipped = outcome.skipped, failures = outcome.failures, "Processed NIP-09 deletion outcome"); + // Accept the deletion event itself so it is stored. WritePolicyResult::Accept } @@ -453,6 +476,56 @@ mod tests { ); } + #[tokio::test] + async fn nip09_marks_request_used_only_after_successful_removal() { + let ctx = make_context(); + let keys = Keys::generate(); + let target = EventBuilder::new(Kind::TextNote, "target") + .finalize(&keys) + .unwrap(); + ctx.database.save_event(&target).await.unwrap(); + let deletion = EventBuilder::new(Kind::EventDeletion, "") + .tags(vec![Tag::event(target.id)]) + .finalize(&keys) + .unwrap(); + + assert!(matches!( + DeletionPolicy::new(ctx.clone()).handle(&deletion).await, + WritePolicyResult::Accept + )); + let record = ctx + .tombstones + .lifecycle_records() + .await + .into_iter() + .find(|record| record.request.id == deletion.id) + .unwrap(); + assert!(record.last_used_at.is_some()); + } + + #[tokio::test] + async fn nip09_noop_leaves_request_lifecycle_unused() { + let ctx = make_context(); + let keys = Keys::generate(); + let deletion = EventBuilder::new(Kind::EventDeletion, "") + .tags(vec![Tag::event(EventId::all_zeros())]) + .finalize(&keys) + .unwrap(); + + assert!(matches!( + DeletionPolicy::new(ctx.clone()).handle(&deletion).await, + WritePolicyResult::Accept + )); + let record = ctx + .tombstones + .lifecycle_records() + .await + .into_iter() + .find(|record| record.request.id == deletion.id) + .unwrap(); + assert_eq!(record.last_used_at, None); + } + #[tokio::test] async fn newer_coordinate_deletion_compacts_superseded_request_from_main_db() { let ctx = make_context(); diff --git a/src/nostr/lifecycle/deletion/purgatory.rs b/src/nostr/lifecycle/deletion/purgatory.rs index d5f964c..6046d0b 100644 --- a/src/nostr/lifecycle/deletion/purgatory.rs +++ b/src/nostr/lifecycle/deletion/purgatory.rs @@ -3,7 +3,7 @@ use std::process::Command; use nostr_relay_builder::prelude::{Event, Kind, Timestamp}; -use super::policy::DeletionPolicy; +use super::{policy::DeletionPolicy, DeletionOutcome}; use crate::nostr::events::RepositoryAnnouncement; use crate::nostr::lifecycle::DeletionSource; @@ -137,8 +137,10 @@ impl DeletionPolicy { /// /// Only removes entries where the purgatory entry's author matches the deletion /// event's pubkey (enforces author-only deletion). - pub(super) fn remove_purgatory_targets(&self, event: &Event) { + pub(super) fn remove_purgatory_targets(&self, event: &Event) -> DeletionOutcome { let author = &event.pubkey; + let mut outcome = DeletionOutcome::default(); + let mut removed_ids = HashSet::new(); for tag in event.tags.iter() { let tag_vec = tag.as_slice(); @@ -150,16 +152,27 @@ impl DeletionPolicy { "e" => { // Event ID reference: find purgatory announcement with this event ID let target_id = &tag_vec[1]; - self.remove_by_event_id(author, target_id, event.created_at.as_secs()); + outcome.merge(self.remove_by_event_id( + author, + target_id, + event.created_at.as_secs(), + &mut removed_ids, + )); } "a" => { // Addressable coordinate reference: `::` let coord = &tag_vec[1]; - self.remove_by_coordinate(author, coord, event.created_at.as_secs()); + outcome.merge(self.remove_by_coordinate( + author, + coord, + event.created_at.as_secs(), + &mut removed_ids, + )); } _ => {} } } + outcome } /// Remove a purgatory entry (announcement, state event, or PR event) matched by event ID. @@ -171,7 +184,8 @@ impl DeletionPolicy { author: &nostr_relay_builder::prelude::PublicKey, target_id_hex: &str, _deletion_created_at: u64, - ) { + removed_ids: &mut HashSet, + ) -> DeletionOutcome { // --- Check PR events (kind 1617/1618) first — O(1) direct lookup --- // PR purgatory is keyed by event ID hex, so this is the cheapest check. // Only remove if the entry has an actual event (not a placeholder) and the @@ -185,11 +199,14 @@ impl DeletionPolicy { "Deletion request: removing purgatory PR event by event ID" ); self.ctx.purgatory.remove_pr(target_id_hex); - return; + return DeletionOutcome { + purgatory_removed: usize::from(removed_ids.insert(event.id)), + ..Default::default() + }; } } // Entry exists but is a placeholder or wrong author — don't remove - return; + return DeletionOutcome::default(); } // --- Check announcements (kind 30617) --- @@ -217,8 +234,7 @@ impl DeletionPolicy { author = %author.to_hex(), "Deletion request: removing purgatory announcement by event ID" ); - self.evict_purgatory_entry(author, identifier); - return; // event IDs are unique + return self.evict_purgatory_entry(author, identifier, removed_ids); } } } @@ -239,10 +255,14 @@ impl DeletionPolicy { self.ctx .purgatory .remove_state_event(&identifier, &entry.event.id); - return; // event IDs are unique + return DeletionOutcome { + purgatory_removed: usize::from(removed_ids.insert(entry.event.id)), + ..Default::default() + }; } } } + DeletionOutcome::default() } /// Remove a purgatory entry matched by addressable coordinate. @@ -256,11 +276,13 @@ impl DeletionPolicy { author: &nostr_relay_builder::prelude::PublicKey, coordinate: &str, deletion_created_at: u64, - ) { + removed_ids: &mut HashSet, + ) -> DeletionOutcome { + let mut outcome = DeletionOutcome::default(); // Parse coordinate: `::` let parts: Vec<&str> = coordinate.splitn(3, ':').collect(); if parts.len() != 3 { - return; + return DeletionOutcome::default(); } let kind_str = parts[0]; @@ -274,7 +296,7 @@ impl DeletionPolicy { deletion_author = %author.to_hex(), "Ignoring deletion: coordinate pubkey does not match deletion author" ); - return; + return DeletionOutcome::default(); } match kind_str { @@ -287,7 +309,7 @@ impl DeletionPolicy { author = %author.to_hex(), "Deletion request: removing purgatory announcement by coordinate" ); - self.evict_purgatory_entry(author, identifier); + return self.evict_purgatory_entry(author, identifier, removed_ids); } else { tracing::debug!( identifier = %identifier, @@ -309,7 +331,10 @@ impl DeletionPolicy { self.ctx .purgatory .remove_state_event(identifier, &entry.event.id); - removed += 1; + let counted = usize::from(removed_ids.insert(entry.event.id)); + removed += counted; + outcome.purgatory_removed = + outcome.purgatory_removed.saturating_add(counted); } } if removed > 0 { @@ -325,6 +350,7 @@ impl DeletionPolicy { // Other kinds not handled } } + outcome } /// Remove a purgatory announcement and delete its bare repository from disk. @@ -332,7 +358,9 @@ impl DeletionPolicy { &self, author: &nostr_relay_builder::prelude::PublicKey, identifier: &str, - ) { + removed_ids: &mut HashSet, + ) -> DeletionOutcome { + let mut outcome = DeletionOutcome::default(); // Get repo path before removing if let Some(entry) = self.ctx.purgatory.find_announcement(author, identifier) { if entry.repo_path.exists() { @@ -342,6 +370,7 @@ impl DeletionPolicy { error = %e, "Failed to delete bare repository during deletion request processing" ); + outcome.failures = outcome.failures.saturating_add(1); } else { tracing::info!( path = %entry.repo_path.display(), @@ -351,6 +380,11 @@ impl DeletionPolicy { } } + if let Some(entry) = self.ctx.purgatory.find_announcement(author, identifier) { + outcome.purgatory_removed = outcome + .purgatory_removed + .saturating_add(usize::from(removed_ids.insert(entry.event.id))); + } self.ctx.purgatory.remove_announcement(author, identifier); // Remove state events for this identifier only if no other owner's @@ -362,7 +396,13 @@ impl DeletionPolicy { .is_empty(); if !other_owners_remain { + for entry in self.ctx.purgatory.find_state(identifier) { + outcome.purgatory_removed = outcome + .purgatory_removed + .saturating_add(usize::from(removed_ids.insert(entry.event.id))); + } self.ctx.purgatory.remove_state(identifier); } + outcome } } diff --git a/src/nostr/lifecycle/deletion/service.rs b/src/nostr/lifecycle/deletion/service.rs index 307234b..58342aa 100644 --- a/src/nostr/lifecycle/deletion/service.rs +++ b/src/nostr/lifecycle/deletion/service.rs @@ -8,7 +8,7 @@ use crate::nostr::events::RepositoryAnnouncement; use crate::nostr::lifecycle::RequestLifecycleRecord; use crate::nostr::policy::{reject_error, reject_invalid, AnnouncementResult}; -use super::{DeletionContext, DeletionPolicy}; +use super::{DeletionContext, DeletionOutcome, DeletionPolicy}; #[derive(Clone)] pub struct DeletionService { @@ -320,18 +320,45 @@ impl DeletionService { return reject_error(format!("internal error recording vanish: {e}")); } - if let Err(e) = self.policy.apply_nip62_vanish(event).await { - tracing::error!(error = %e, author = %author.to_hex(), "Failed to process vanished author's lifecycle deletion"); - return reject_error(format!("internal error processing vanish: {e}")); - } + let mut outcome = match self.policy.apply_nip62_vanish(event).await { + Ok(outcome) => outcome, + Err(e) => { + tracing::error!(error = %e, author = %author.to_hex(), "Failed to process vanished author's lifecycle deletion"); + return reject_error(format!("internal error processing vanish: {e}")); + } + }; // Evict the author's purgatory entries (and their bare repos). Served // data has already gone through the holding/archive lifecycle above; // purgatory entries are not live served data. - self.evict_author_from_purgatory(&author); + outcome.merge(self.evict_author_from_purgatory(&author)); + + if outcome.used() { + match self + .ctx + .tombstones() + .mark_request_used(&event.id, Timestamp::now()) + .await + { + Ok(Some(_)) => {} + Ok(None) => { + return reject_error( + "internal error updating vanish lifecycle: missing metadata", + ) + } + Err(e) => { + tracing::error!(event_id = %event.id.to_hex(), error = %e, "Failed to mark used NIP-62 vanish request"); + return reject_error(format!("internal error updating vanish lifecycle: {e}")); + } + } + } tracing::info!( author = %author.to_hex(), + main_db_deleted = outcome.main_db_deleted, + purgatory_removed = outcome.purgatory_removed, + skipped = outcome.skipped, + failures = outcome.failures, "Processed NIP-62 request to vanish through deletion lifecycle" ); @@ -608,7 +635,9 @@ impl DeletionService { kind == Kind::GitRepoAnnouncement || kind == Kind::RepoState } - fn evict_author_from_purgatory(&self, author: &PublicKey) { + fn evict_author_from_purgatory(&self, author: &PublicKey) -> DeletionOutcome { + let mut outcome = DeletionOutcome::default(); + let mut removed_ids = std::collections::HashSet::new(); // Announcements owned by this author. for (repo_id, _) in self.ctx.purgatory().announcements_for_sync() { // repo_id format: "30617:{pubkey_hex}:{identifier}" @@ -625,9 +654,15 @@ impl DeletionService { error = %e, "Failed to delete bare repository during vanish processing" ); + outcome.failures = outcome.failures.saturating_add(1); } } } + if let Some(entry) = self.ctx.purgatory().find_announcement(author, identifier) { + outcome.purgatory_removed = outcome + .purgatory_removed + .saturating_add(usize::from(removed_ids.insert(entry.event.id))); + } self.ctx.purgatory().remove_announcement(author, identifier); } @@ -635,12 +670,16 @@ impl DeletionService { for identifier in self.ctx.purgatory().get_all_identifiers() { for entry in self.ctx.purgatory().find_state(&identifier) { if entry.author == *author { + outcome.purgatory_removed = outcome + .purgatory_removed + .saturating_add(usize::from(removed_ids.insert(entry.event.id))); self.ctx .purgatory() .remove_state_event(&identifier, &entry.event.id); } } } + outcome } }