diff --git a/CHANGELOG.md b/CHANGELOG.md index b42df96..28529b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- Prevented redundant NIP-09 deletion requests from polluting relay storage and query results. + ## [1.2.0-rc.1] - 2026-06-29 ### Added diff --git a/docs/explanation/repository-lifecycle.md b/docs/explanation/repository-lifecycle.md index af4ae49..aebf82f 100644 --- a/docs/explanation/repository-lifecycle.md +++ b/docs/explanation/repository-lifecycle.md @@ -187,7 +187,16 @@ deprecated in favor of the maintenance command. ``` 1. Kind 5 deletion request arrives ↓ -2. Validate targets: +2. Check idempotency: + - if every actionable `e`/`a` target is already covered by an existing + same-author kind-5 request, return a duplicate success response without + storing the new deletion event + - `e` targets are covered by any existing same-author deletion for that id + - `a` targets are covered only when an existing same-author deletion for the + same coordinate has `created_at >=` the new deletion request, preserving + NIP-09 coordinate cutoff semantics + ↓ +3. Validate targets: - `e` targets found in the main DB must be authored by the deleter; cross-author main-DB targets reject the whole request - pre-emptive `e` deletes for unknown targets may be accepted, but if the @@ -195,9 +204,9 @@ deprecated in favor of the maintenance command. removed from both tombstones and the served deletion-request stream - `a` coordinates are acted on only when coordinate pubkey matches deleter ↓ -3. Record deletion tombstone so re-submission is gated +4. Record deletion tombstone so re-submission is gated ↓ -4. Process targets: +5. Process targets: - `a` tag targeting a kind-30617 announcement coordinate: recursively discover the accepted-reference component affected by the announcement deletion @@ -212,24 +221,24 @@ deprecated in favor of the maintenance command. - other valid `e`/`a` targets: delete the targeted event/coordinate without announcement graph cascade ↓ -5. Archive git repository to .archive//-.tar.gz +6. Archive git repository to .archive//-.tar.gz when deleting a repository announcement with live git data ↓ -6. Move deleted events to holding database: +7. Move deleted events to holding database: - Targeted events - Repository announcements, when announcement deletion is involved - All main-DB events that lose their accepted-reference path after cascade reevaluation, for cascade paths - Deletion metadata for retention/cleanup/recovery ↓ -7. Delete events from main database +8. Delete events from main database ↓ -8. Remove the live git repository when the owner+identifier announcement scope +9. Remove the live git repository when the owner+identifier announcement scope is no longer served ↓ -9. Deleted/tombstoned targets no longer serve in queries +10. Deleted/tombstoned targets no longer serve in queries ↓ -10. Background task (daily): +11. Background task (daily): - Check holding database for expired entries - Delete events older than retention period - Delete corresponding archive files diff --git a/src/nostr/lifecycle/deletion/policy.rs b/src/nostr/lifecycle/deletion/policy.rs index 874cc4a..49920fc 100644 --- a/src/nostr/lifecycle/deletion/policy.rs +++ b/src/nostr/lifecycle/deletion/policy.rs @@ -36,7 +36,7 @@ use nostr::nips::nip19::ToBech32; use nostr_relay_builder::prelude::{Event, EventId, Kind, PublicKey, WritePolicyResult}; use super::DeletionContext; -use crate::nostr::policy::{reject_error, reject_invalid}; +use crate::nostr::policy::{duplicate, reject_error, reject_invalid}; const MAX_DELETION_TARGET_TAGS: usize = 2048; /// Policy for handling NIP-09 event deletion requests @@ -104,6 +104,27 @@ impl DeletionPolicy { return reject_invalid("too many deletion targets"); } + match self + .ctx + .tombstones + .deletion_targets_already_covered(event) + .await + { + Ok(true) => { + tracing::info!( + event_id = %event.id.to_hex(), + author = %event.pubkey.to_hex(), + "Skipping duplicate NIP-09 deletion request; all actionable targets are already covered" + ); + return duplicate("deletion target(s) already covered"); + } + Ok(false) => {} + Err(e) => { + tracing::warn!(error = %e, "Tombstone lookup failed during duplicate deletion check"); + return reject_error(format!("internal error checking deletion coverage: {e}")); + } + } + // Validate authorship of all targets first. If any `e`-tag target exists // in the main DB and is owned by someone else, the whole deletion is // invalid (mirrors the backend's `handle_deletion_event` returning @@ -341,6 +362,34 @@ mod tests { ); } + #[tokio::test] + async fn duplicate_deletion_request_is_not_accepted_for_storage() { + let ctx = make_context(); + let keys = Keys::generate(); + let target = EventId::all_zeros(); + + let first = EventBuilder::new(Kind::EventDeletion, "") + .tags(vec![Tag::event(target)]) + .custom_created_at(Timestamp::from_secs(1000)) + .finalize(&keys) + .unwrap(); + let duplicate = EventBuilder::new(Kind::EventDeletion, "") + .tags(vec![Tag::event(target)]) + .custom_created_at(Timestamp::from_secs(1001)) + .finalize(&keys) + .unwrap(); + + let policy = DeletionPolicy::new(ctx); + let first_result = policy.handle(&first).await; + assert!(matches!(first_result, WritePolicyResult::Accept)); + + let duplicate_result = policy.handle(&duplicate).await; + assert!( + !matches!(duplicate_result, WritePolicyResult::Accept), + "duplicate deletion request should not be returned as Accept, because Accept causes relay storage" + ); + } + #[tokio::test] async fn test_deletion_by_coordinate_removes_purgatory_entry() { let ctx = make_context(); diff --git a/src/nostr/lifecycle/tombstones.rs b/src/nostr/lifecycle/tombstones.rs index cf9b722..7683a5e 100644 --- a/src/nostr/lifecycle/tombstones.rs +++ b/src/nostr/lifecycle/tombstones.rs @@ -118,6 +118,92 @@ impl Tombstones { Ok(()) } + /// Return `true` when every actionable NIP-09 target in `event` is already + /// covered by an existing same-author kind-5 request in this store. + /// + /// This is an idempotency helper for admission: a client may generate a new + /// kind-5 event id by changing only `created_at` while tagging the same + /// targets. Once the relay has already accepted a deletion request that + /// covers those targets, accepting another equivalent request only pollutes + /// the served deletion-request stream. + /// + /// Coverage is target-specific: + /// - `e` targets are covered by any existing same-author kind-5 with that + /// event id. + /// - `a` targets are covered only by an existing same-author kind-5 for the + /// same coordinate whose `created_at` is greater than or equal to the new + /// request's `created_at`, preserving NIP-09's "delete versions up to this + /// timestamp" semantics. + /// + /// Non-actionable targets (malformed tags, or `a` coordinates owned by a + /// different pubkey than the event author) are ignored so this helper does + /// not change existing no-op acceptance semantics for those events. + pub async fn deletion_targets_already_covered(&self, event: &Event) -> anyhow::Result { + debug_assert_eq!(event.kind, Kind::EventDeletion); + + let author = event.pubkey; + let author_hex = author.to_hex(); + let mut actionable_targets = 0usize; + + for tag in event.tags.iter() { + let v = tag.as_slice(); + if v.len() < 2 { + continue; + } + + match v[0].as_str() { + "e" => { + let Ok(target_id) = EventId::from_hex(&v[1]) else { + continue; + }; + actionable_targets += 1; + + let filter = Filter::new() + .kind(Kind::EventDeletion) + .author(author) + .custom_tag(SingleLetterTag::lowercase(Alphabet::E), target_id.to_hex()); + let covered = !self + .db + .query(filter) + .await + .map_err(|e| anyhow::anyhow!("Failed to query deletion tombstones: {e}"))? + .is_empty(); + if !covered { + return Ok(false); + } + } + "a" => { + let coordinate = &v[1]; + let Some(coord_owner_hex) = coordinate.split(':').nth(1) else { + continue; + }; + if coord_owner_hex != author_hex { + continue; + } + actionable_targets += 1; + + let filter = Filter::new() + .kind(Kind::EventDeletion) + .author(author) + .custom_tag(SingleLetterTag::lowercase(Alphabet::A), coordinate.clone()); + let covered = self + .db + .query(filter) + .await + .map_err(|e| anyhow::anyhow!("Failed to query deletion tombstones: {e}"))? + .iter() + .any(|deletion| deletion.created_at >= event.created_at); + if !covered { + return Ok(false); + } + } + _ => {} + } + } + + Ok(actionable_targets > 0) + } + /// Remove recorded kind-5 requests from other authors that targeted `id`. /// /// This handles out-of-order cross-author deletes. A relay may accept a @@ -276,6 +362,57 @@ mod tests { assert!(store.is_event_deleted(&target, &keys.public_key()).await); } + #[tokio::test] + async fn detects_already_covered_event_deletion_request() { + let store = Tombstones::in_memory(); + let keys = Keys::generate(); + let target = EventId::all_zeros(); + + let first = deletion_by_event(&keys, target); + store.record_deletion(&first).await.unwrap(); + + let duplicate = deletion_by_event(&keys, target); + + assert!(store + .deletion_targets_already_covered(&duplicate) + .await + .unwrap()); + } + + #[tokio::test] + async fn coordinate_coverage_respects_deletion_created_at() { + let store = Tombstones::in_memory(); + let keys = Keys::generate(); + let coord = format!("30618:{}:my-repo", keys.public_key().to_hex()); + + let first = EventBuilder::new(Kind::EventDeletion, "") + .tags(vec![Tag::custom("a", vec![coord.clone()])]) + .custom_created_at(Timestamp::from_secs(1000)) + .finalize(&keys) + .unwrap(); + store.record_deletion(&first).await.unwrap(); + + let older_or_equal = EventBuilder::new(Kind::EventDeletion, "") + .tags(vec![Tag::custom("a", vec![coord.clone()])]) + .custom_created_at(Timestamp::from_secs(900)) + .finalize(&keys) + .unwrap(); + assert!(store + .deletion_targets_already_covered(&older_or_equal) + .await + .unwrap()); + + let newer = EventBuilder::new(Kind::EventDeletion, "") + .tags(vec![Tag::custom("a", vec![coord])]) + .custom_created_at(Timestamp::from_secs(1100)) + .finalize(&keys) + .unwrap(); + assert!(!store + .deletion_targets_already_covered(&newer) + .await + .unwrap()); + } + #[tokio::test] async fn deletion_does_not_block_a_different_authors_event() { // A kind-5 from pubkey B targeting an event id must NOT mark that id as