mirror of
https://relay.ngit.dev/npub15qydau2hjma6ngxkl2cyar74wzyjshvl65za5k5rl69264ar2exs5cyejr/ngit-grasp.git
synced 2026-10-05 23:18:24 +00:00
fix: deduplicate covered deletion requests
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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/<npub>/<identifier>-<timestamp>.tar.gz
|
||||
6. Archive git repository to .archive/<npub>/<identifier>-<timestamp>.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
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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<bool> {
|
||||
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
|
||||
|
||||
Reference in New Issue
Block a user