From 19a88b3db3ef58b656e8b5fe5ef04205a620d99c Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Wed, 17 Jun 2026 10:58:41 +0000 Subject: [PATCH] feat(deletion): cascade-delete repo state when announcement unanchors --- docs/explanation/deletion-requests.md | 23 ++-- src/nostr/policy/deletion.rs | 55 ++++++++- tests/common/nip09_helpers.rs | 79 +++++++++++++ tests/nip09_announcement_cascade.rs | 156 ++++++++++++++++++++++++++ tests/nip09_cascade_event_types.rs | 109 ++++++++++++++++++ tests/nip09_state_cascade.rs | 155 +++++++++++++++++++++++++ 6 files changed, 567 insertions(+), 10 deletions(-) create mode 100644 tests/nip09_cascade_event_types.rs create mode 100644 tests/nip09_state_cascade.rs diff --git a/docs/explanation/deletion-requests.md b/docs/explanation/deletion-requests.md index 807ac2a..c7d30a1 100644 --- a/docs/explanation/deletion-requests.md +++ b/docs/explanation/deletion-requests.md @@ -8,12 +8,12 @@ This document describes the planned architecture for NIP-09 deletion request sup ## Implemented: ngit-grasp owns NIP-09 / NIP-62 handling -> The rest of this document is the **planned** design for further work -> (holding database, cascade, archival, recovery). It is still aspirational. +> The rest of this document is mostly historical **planned** design +> (holding database, archival, recovery). It is still aspirational. > What follows in this section is what is **actually built today** and is the > foundation that work builds on. The `deletion-request-disrespector` archival -> mode is now implemented (see below); the holding DB, cascade, git archival, -> and recovery remain planned. +> mode is now implemented (see below); the holding DB, git archival, and +> recovery remain planned. Up to the rust-nostr 0.45 bump, ngit-grasp relied on the LMDB backend's *automatic* NIP-09 / NIP-62 processing (`NostrLmdb` defaults @@ -26,10 +26,10 @@ We disable that automatic processing ([`src/nostr/builder.rs`](../../src/nostr/builder.rs) sets `process_nip09(false)` and `process_nip62(false)` on both the main database and the tombstone database) and implement NIP-09 / NIP-62 in ngit-grasp itself, -bypassing the backend so we can apply our own customisations in the future. The -observable behaviour is unchanged; only the implementation moved. There is no -cascade, holding DB, archival, recovery, or disrespector yet — those remain -planned (see below). +bypassing the backend so we can apply our own customisations in the future. +ngit-grasp now has custom announcement cascade deletion in the main DB (pure +hard-delete, no holding DB), including dependent NIP-34 graph events and +repository-state (kind 30618) cleanup when an identifier becomes unanchored. **Components added:** @@ -59,6 +59,13 @@ planned (see below). tombstone store. An `e`-tag delete targeting an event owned by a different author is rejected (matching the backend's `InvalidDelete`). + For kind-30617 announcement `a`-coordinate deletes, this includes + graph-based cascade deletion of orphaned dependent events, plus special + handling for kind-30618 repository state events: because state is keyed by + repository identifier (`d` tag) instead of graph edges, state events are + hard-deleted when (and only when) no announcement remains for that + identifier. + - **NIP-62 handler** (`Nip34WritePolicy::handle_vanish` in [`src/nostr/builder.rs`](../../src/nostr/builder.rs)): for a vanish request targeting this relay (or `ALL_RELAYS`), records the vanish tombstone, diff --git a/src/nostr/policy/deletion.rs b/src/nostr/policy/deletion.rs index af1bcf3..115807e 100644 --- a/src/nostr/policy/deletion.rs +++ b/src/nostr/policy/deletion.rs @@ -47,8 +47,10 @@ use crate::nostr::policy::reject_invalid; /// /// Kept deliberately broad (issues, patches, PR/PR-updates, the four status /// kinds, and NIP-22 comments) so the cascade can reach the whole reachable -/// sub-graph. Kind 30618 (repository state) is intentionally excluded because -/// it is keyed by a `d` tag rather than connected via graph edges. +/// sub-graph. Kind 30618 (repository state) is intentionally excluded from +/// graph traversal because it is keyed by repository identifier (`d` tag), not +/// by `a`/`e`/`q` edges. It is handled separately in +/// [`DeletionPolicy::delete_repo_state_if_identifier_is_unanchored`]. const GRAPH_DEPENDENT_KINDS: &[u16] = &[ 1111, // NIP-22 comment 1617, // patch @@ -248,6 +250,7 @@ impl DeletionPolicy { if parts.len() != 3 { return; } + let identifier = parts[2]; // Step 1: candidate event set — all announcements (to know which other // maintainers survive) plus all NIP-34 dependent events. @@ -356,6 +359,54 @@ impl DeletionPolicy { // deletion's created_at, per NIP-09), exactly as the simple path does. self.delete_coordinate_from_main_db(author, announcement_addr, deletion_created_at) .await; + + // Step 5c: 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. + self.delete_repo_state_if_identifier_is_unanchored(identifier) + .await; + } + + /// Delete kind-30618 state events for `identifier` when the repository is + /// now unanchored (no surviving kind-30617 announcement with that `d` tag). + /// + /// This is separate from graph traversal because state events are keyed by + /// identifier rather than `a`/`e`/`q` relationships. + async fn delete_repo_state_if_identifier_is_unanchored(&self, identifier: &str) { + let announcement_filter = Filter::new().kind(Kind::GitRepoAnnouncement).custom_tag( + SingleLetterTag::lowercase(Alphabet::D), + identifier.to_string(), + ); + + let remaining_announcements = match self.ctx.database.query(announcement_filter).await { + Ok(events) => events, + Err(e) => { + tracing::warn!( + error = %e, + identifier = %identifier, + "Cascade deletion: failed to check remaining announcements for identifier" + ); + return; + } + }; + + if !remaining_announcements.is_empty() { + return; + } + + let state_filter = Filter::new().kind(Kind::RepoState).custom_tag( + SingleLetterTag::lowercase(Alphabet::D), + identifier.to_string(), + ); + + if let Err(e) = self.ctx.database.delete(state_filter).await { + tracing::warn!( + error = %e, + identifier = %identifier, + "Cascade deletion: failed to delete unanchored repository state events" + ); + } } /// Query the candidate event set for the cascade graph: every repository diff --git a/tests/common/nip09_helpers.rs b/tests/common/nip09_helpers.rs index bfd424b..85682fa 100644 --- a/tests/common/nip09_helpers.rs +++ b/tests/common/nip09_helpers.rs @@ -14,6 +14,11 @@ use nostr_sdk::prelude::*; use std::path::PathBuf; use std::time::Duration; +use super::purgatory_helpers::{ + create_state_event, create_test_repo_with_commit, push_to_relay, CommitVariant, +}; +use super::sync_helpers::create_repo_announcement; + /// Publish a repo announcement, submit a matching state event, and push the /// deterministic git data so the announcement is promoted out of purgatory and /// becomes queryable. @@ -132,6 +137,80 @@ pub async fn publish_served_repo(client: &AuditClient, test_name: &str) -> (Even (announcement, repo_id) } +/// Publish a served announcement + served state event for one repository. +/// +/// Unlike [`publish_served_repo`], this helper returns the exact kind-30618 +/// state event it published so cascade tests can assert strict pre/post +/// conditions by event id. +pub async fn publish_served_repo_with_state_event( + client: &AuditClient, + test_name: &str, +) -> (Event, String, Event) { + let relay_url = client + .relay_url() + .await + .expect("client should have a relay"); + let relay_domain = relay_url + .trim_start_matches("ws://") + .trim_start_matches("wss://") + .to_string(); + let npub = client.public_key().to_bech32().expect("pubkey to bech32"); + + let repo_id = format!( + "{}-{}", + test_name, + &Keys::generate().public_key().to_hex()[..8] + ); + + let temp_dir = tempfile::tempdir().expect("create temp dir for repo push"); + let commit_hash = create_test_repo_with_commit(temp_dir.path(), CommitVariant::StateTest) + .expect("create deterministic state-test commit"); + + let announcement = create_repo_announcement(client.keys(), &[relay_domain.as_str()], &repo_id); + client + .send_event(announcement.clone()) + .await + .expect("relay should accept announcement"); + + let clone_url = format!("http://{}/{}/{}.git", relay_domain, npub, repo_id); + let state_event = create_state_event( + client.keys(), + &repo_id, + &[("main", commit_hash.as_str())], + &[], + &[clone_url.as_str()], + &[relay_url.as_str()], + ) + .expect("build repo state event"); + + client + .send_event_and_note_purgatory(state_event.clone()) + .await + .expect("relay should accept state event"); + + push_to_relay(temp_dir.path(), &relay_domain, &npub, &repo_id) + .expect("git push should promote announcement + state event out of purgatory"); + + tokio::time::sleep(Duration::from_millis(300)).await; + + assert!( + client + .is_event_on_relay(announcement.id) + .await + .expect("query announcement"), + "announcement should be served after git data arrives" + ); + assert!( + client + .is_event_on_relay(state_event.id) + .await + .expect("query state event"), + "state event should be served after git data arrives" + ); + + (announcement, repo_id, state_event) +} + /// Query the relay for a served repo announcement using a real client's access /// pattern: author + kind + `d` identifier (a NIP-01 addressable-event query), /// rather than a by-id lookup. Returns whether the relay serves it. diff --git a/tests/nip09_announcement_cascade.rs b/tests/nip09_announcement_cascade.rs index 02c2953..2137d13 100644 --- a/tests/nip09_announcement_cascade.rs +++ b/tests/nip09_announcement_cascade.rs @@ -164,3 +164,159 @@ async fn test_announcement_cascade_deletes_dependents() { announcement.id ); } + +/// Deleting a patch by `e` tag must stay on the simple deletion path: +/// only the patch is removed, while the announcement and unrelated siblings +/// survive. +#[tokio::test] +async fn test_deleting_patch_by_event_id_does_not_delete_announcement_or_siblings() { + let relay = TestRelay::start().await; + let client = AuditClient::new(relay.url(), AuditConfig::isolated()) + .await + .expect("create audit client"); + let keys = client.keys().clone(); + + let (announcement, repo_id) = publish_served_repo(&client, "simple-delete-patch").await; + let coordinate = announcement_coordinate(&announcement, &repo_id); + + let issue = client + .create_issue(&announcement, "Sibling issue", "must survive", vec![]) + .expect("build issue"); + + let patch = client + .event_builder(Kind::from(1617), "patch to delete") + .tag(Tag::custom("a", vec![coordinate.clone()])) + .build(&keys) + .expect("build patch"); + + for event in [&issue, &patch] { + client + .send_event(event.clone()) + .await + .expect("relay should accept dependent event"); + } + + tokio::time::sleep(Duration::from_millis(300)).await; + + let precondition = [ + ("announcement", announcement.id), + ("issue", issue.id), + ("patch", patch.id), + ]; + for (label, id) in precondition { + assert!( + client + .is_event_on_relay(id) + .await + .expect("query event before deletion"), + "{label} ({id}) must be served before patch deletion" + ); + } + + let deletion = build_deletion(&client, &[patch.id], &[]); + client + .send_event(deletion) + .await + .expect("relay should accept patch 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 issue_survived = client + .is_event_on_relay(issue.id) + .await + .expect("query issue after deletion"); + let patch_survived = client + .is_event_on_relay(patch.id) + .await + .expect("query patch after deletion"); + relay.stop().await; + + assert!( + announcement_survived, + "announcement must survive patch e-tag deletion" + ); + assert!( + issue_survived, + "unrelated sibling issue must survive patch e-tag deletion" + ); + assert!(!patch_survived, "patch must be deleted by its e-tag"); +} + +/// Deleting a comment by `e` tag must not trigger repository graph cascade: +/// the parent issue and announcement must survive. +#[tokio::test] +async fn test_deleting_comment_by_event_id_does_not_cascade_to_issue_or_announcement() { + let relay = TestRelay::start().await; + let client = AuditClient::new(relay.url(), AuditConfig::isolated()) + .await + .expect("create audit client"); + let keys = client.keys().clone(); + + let (announcement, _repo_id) = publish_served_repo(&client, "simple-delete-comment").await; + + let issue = client + .create_issue(&announcement, "Issue anchor", "must survive", vec![]) + .expect("build issue"); + let comment = client + .event_builder(Kind::from(1111), "comment to delete") + .tag(Tag::custom("e", vec![issue.id.to_hex()])) + .build(&keys) + .expect("build comment"); + + for event in [&issue, &comment] { + client + .send_event(event.clone()) + .await + .expect("relay should accept event"); + } + + tokio::time::sleep(Duration::from_millis(300)).await; + + for (label, id) in [ + ("announcement", announcement.id), + ("issue", issue.id), + ("comment", comment.id), + ] { + assert!( + client + .is_event_on_relay(id) + .await + .expect("query event before deletion"), + "{label} ({id}) must be served before comment deletion" + ); + } + + let deletion = build_deletion(&client, &[comment.id], &[]); + client + .send_event(deletion) + .await + .expect("relay should accept comment 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 issue_survived = client + .is_event_on_relay(issue.id) + .await + .expect("query issue after deletion"); + let comment_survived = client + .is_event_on_relay(comment.id) + .await + .expect("query comment after deletion"); + + relay.stop().await; + + assert!( + announcement_survived, + "announcement must survive comment e-tag deletion" + ); + assert!(issue_survived, "issue must survive comment e-tag deletion"); + assert!(!comment_survived, "comment must be deleted by its e-tag"); +} diff --git a/tests/nip09_cascade_event_types.rs b/tests/nip09_cascade_event_types.rs new file mode 100644 index 0000000..8858b0f --- /dev/null +++ b/tests/nip09_cascade_event_types.rs @@ -0,0 +1,109 @@ +//! NIP-09 announcement cascade coverage across supported dependent kinds. +//! +//! Covered here (single-maintainer): patch (1617), issue (1621), issue-status +//! (1630), comment (1111), repo-status (1633). +//! +//! Not covered here: PR/PR-update/status chain (1618/1619/1631/1632), because +//! those events require additional PR-specific git wiring and acceptance setup. + +mod common; + +use common::{announcement_coordinate, build_deletion, publish_served_repo, TestRelay}; + +use grasp_audit::{AuditClient, AuditConfig}; +use nostr_sdk::prelude::*; +use std::time::Duration; + +#[tokio::test] +async fn test_announcement_cascade_deletes_supported_dependent_kinds() { + let relay = TestRelay::start().await; + let client = AuditClient::new(relay.url(), AuditConfig::isolated()) + .await + .expect("create audit client"); + let keys = client.keys().clone(); + + let (announcement, repo_id) = publish_served_repo(&client, "cascade-event-types").await; + let coordinate = announcement_coordinate(&announcement, &repo_id); + + let issue = client + .create_issue(&announcement, "Cascade issue", "issue body", vec![]) + .expect("build issue"); + + let patch = client + .event_builder(Kind::from(1617), "patch content") + .tag(Tag::custom("a", vec![coordinate.clone()])) + .build(&keys) + .expect("build patch"); + + let issue_status = client + .event_builder(Kind::from(1630), "status: open") + .tag(Tag::custom("e", vec![issue.id.to_hex()])) + .build(&keys) + .expect("build issue status"); + + let comment = client + .event_builder(Kind::from(1111), "issue comment") + .tag(Tag::custom("e", vec![issue.id.to_hex()])) + .build(&keys) + .expect("build comment"); + + let repo_status = client + .event_builder(Kind::from(1633), "repo status") + .tag(Tag::custom("e", vec![announcement.id.to_hex()])) + .build(&keys) + .expect("build repository status"); + + for event in [&issue, &patch, &issue_status, &comment, &repo_status] { + client + .send_event(event.clone()) + .await + .expect("relay should accept dependent event"); + } + + tokio::time::sleep(Duration::from_millis(300)).await; + + let dependents = [ + ("announcement", announcement.id), + ("issue", issue.id), + ("patch", patch.id), + ("issue_status", issue_status.id), + ("comment", comment.id), + ("repo_status", repo_status.id), + ]; + + for (label, id) in dependents { + assert!( + client + .is_event_on_relay(id) + .await + .expect("query event before deletion"), + "{label} ({id}) must be served before deletion" + ); + } + + let deletion = build_deletion(&client, &[], std::slice::from_ref(&coordinate)); + client + .send_event(deletion) + .await + .expect("relay should accept announcement deletion"); + + tokio::time::sleep(Duration::from_millis(600)).await; + + let mut survivors = Vec::new(); + for (label, id) in dependents { + let served = client + .is_event_on_relay(id) + .await + .expect("query event after deletion"); + if served { + survivors.push(format!("{label} ({id})")); + } + } + + relay.stop().await; + + assert!( + survivors.is_empty(), + "announcement deletion should cascade across covered dependent kinds; survivors: {survivors:?}" + ); +} diff --git a/tests/nip09_state_cascade.rs b/tests/nip09_state_cascade.rs new file mode 100644 index 0000000..5b7256a --- /dev/null +++ b/tests/nip09_state_cascade.rs @@ -0,0 +1,155 @@ +//! NIP-09 repository-state cascade-deletion integration tests. +//! +//! These tests extend the announcement-cascade coverage to kind-30618 +//! repository state events in the **single-maintainer** case. + +mod common; + +use common::{ + announcement_coordinate, announcement_served_by_coordinate, build_deletion, + publish_served_repo_with_state_event, TestRelay, +}; + +use grasp_audit::{AuditClient, AuditConfig}; +use std::time::Duration; + +/// Deleting an announcement by coordinate must hard-delete the repository state +/// event (kind 30618) for the same repository from the main DB. +#[tokio::test] +async fn test_state_event_deleted_with_announcement() { + let relay = TestRelay::start().await; + let client = AuditClient::new(relay.url(), AuditConfig::isolated()) + .await + .expect("create audit client"); + + let (announcement, repo_id, state_event) = + publish_served_repo_with_state_event(&client, "state-cascade-single").await; + let coordinate = announcement_coordinate(&announcement, &repo_id); + + // STRICT pre-condition: both announcement and state are genuinely served. + for (label, id) in [("announcement", announcement.id), ("state", state_event.id)] { + assert!( + client + .is_event_on_relay(id) + .await + .expect("query event before deletion"), + "{label} ({id}) must be served before deletion" + ); + } + + let deletion = build_deletion(&client, &[], std::slice::from_ref(&coordinate)); + client + .send_event(deletion) + .await + .expect("relay should accept 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 state_survived = client + .is_event_on_relay(state_event.id) + .await + .expect("query state event after deletion"); + let announcement_by_coordinate = + announcement_served_by_coordinate(&client, &announcement, &repo_id).await; + + relay.stop().await; + + assert!( + !announcement_survived, + "announcement {} must be hard-deleted", + announcement.id + ); + assert!( + !state_survived, + "state event {} must be hard-deleted with its announcement", + state_event.id + ); + assert!( + !announcement_by_coordinate, + "announcement must not be served by author+kind+identifier after deletion" + ); +} + +/// Two repositories by the same maintainer are independent: deleting one +/// announcement must delete only that repository's state event. +#[tokio::test] +async fn test_state_events_are_independent_per_repository() { + let relay = TestRelay::start().await; + let client = AuditClient::new(relay.url(), AuditConfig::isolated()) + .await + .expect("create audit client"); + + let (announcement_a, repo_a, state_a) = + publish_served_repo_with_state_event(&client, "state-cascade-a").await; + let (announcement_b, repo_b, state_b) = + publish_served_repo_with_state_event(&client, "state-cascade-b").await; + let coordinate_a = announcement_coordinate(&announcement_a, &repo_a); + + for (label, id) in [ + ("announcement_a", announcement_a.id), + ("state_a", state_a.id), + ("announcement_b", announcement_b.id), + ("state_b", state_b.id), + ] { + assert!( + client + .is_event_on_relay(id) + .await + .expect("query event before deletion"), + "{label} ({id}) must be served before deletion" + ); + } + + let deletion = build_deletion(&client, &[], std::slice::from_ref(&coordinate_a)); + client + .send_event(deletion) + .await + .expect("relay should accept announcement deletion"); + + tokio::time::sleep(Duration::from_millis(600)).await; + + let announcement_a_survived = client + .is_event_on_relay(announcement_a.id) + .await + .expect("query announcement a after deletion"); + let state_a_survived = client + .is_event_on_relay(state_a.id) + .await + .expect("query state a after deletion"); + let announcement_b_survived = client + .is_event_on_relay(announcement_b.id) + .await + .expect("query announcement b after deletion"); + let state_b_survived = client + .is_event_on_relay(state_b.id) + .await + .expect("query state b after deletion"); + + let announcement_a_by_coordinate = + announcement_served_by_coordinate(&client, &announcement_a, &repo_a).await; + let announcement_b_by_coordinate = + announcement_served_by_coordinate(&client, &announcement_b, &repo_b).await; + + relay.stop().await; + + assert!( + !announcement_a_survived && !state_a_survived, + "deleted repository must lose both announcement and state event" + ); + assert!( + announcement_b_survived && state_b_survived, + "unrelated repository announcement and state must survive" + ); + assert!( + !announcement_a_by_coordinate, + "deleted repository coordinate must not be served" + ); + assert!( + announcement_b_by_coordinate, + "unrelated repository coordinate must remain served" + ); +}