From 4492b0cb4e6664af58fa591a7846b40ac9b35a5b Mon Sep 17 00:00:00 2001 From: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com> Date: Thu, 21 May 2026 19:42:16 -0400 Subject: [PATCH] fix(relay): NIP-09 a-tag deletion soft-deletes addressable rows (#714) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `handle_a_tag_deletion` in `crates/sprout-relay/src/handlers/side_effects.rs` previously only had a side-effect branch for `KIND_WORKFLOW_DEF`. Every other addressable kind — including kind:30023 (NIP-23 long-form) — fell through to the `_ =>` arm which only emitted a debug log. The kind:5 deletion event was accepted and stored, but the target row was never soft-deleted, so subsequent REQs still returned it. This was discovered while building `sprout notes` (CLI for NIP-23 long-form content): the semantically correct way to delete a parameterized-replaceable note is by `a` coordinate, but until this fix the relay silently no-op'd. The CLI shipped a dual-tag workaround (`["e", id]` + `["a", coord]`); once this lands, the a-tag becomes load-bearing. Changes: - `sprout_db::event::soft_delete_by_coordinate(pool, kind, pubkey, d_tag)`: new helper mirroring `soft_delete_event`. WHERE clause matches the NIP-33 replacement key used by `replace_parameterized_event` exactly — `channel_id` is intentionally not in the key (NIP-33 replacement is global per the spec; `channel_id` is for query scoping, not identity). - `Db::soft_delete_by_coordinate` wrapper for caller ergonomics. - `handle_a_tag_deletion`: new arm `k if is_parameterized_replaceable(k) =>` between the existing workflow branch and `_ =>`. Workflow keeps its bespoke deletion (`delete_workflow` only touches `workflows`; the underlying kind:30620 row in `events` is left alive by design — out of scope for this fix). For every other NIP-33 kind the new helper runs and the row is soft-deleted. `_ =>` is now correctly scoped to "non-NIP-33 kind" only. - `e2e_long_form.rs::test_long_form_a_tag_deletion`: full round-trip regression test (publish → verify queryable → a-tag kind:5 → verify REQ returns 0). `#[ignore]`'d like the rest of that file (needs a running relay). Read-path filtering in `req.rs` already gates on `deleted_at IS NULL`, so once the side-effect path soft-deletes the row, REQs hide it automatically. No filter changes needed. Verified locally: - `cargo build -p sprout-db -p sprout-relay -p sprout-test-client` - `cargo test -p sprout-db --lib` → 64/64 - `cargo test -p sprout-relay --lib` → 192/192 - `cargo clippy -p sprout-db -p sprout-relay -p sprout-test-client --all-targets -- -D warnings` - `cargo fmt --check` - New e2e test compiles cleanly; runs against a live relay via CI. Signed-off-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com> Co-authored-by: Dawn (sprout agent) --- crates/sprout-db/src/event.rs | 30 +++++++ crates/sprout-db/src/lib.rs | 11 +++ .../sprout-relay/src/handlers/side_effects.rs | 50 ++++++++++- .../sprout-test-client/tests/e2e_long_form.rs | 86 +++++++++++++++++++ 4 files changed, 173 insertions(+), 4 deletions(-) diff --git a/crates/sprout-db/src/event.rs b/crates/sprout-db/src/event.rs index 043a32093..22b01f880 100644 --- a/crates/sprout-db/src/event.rs +++ b/crates/sprout-db/src/event.rs @@ -550,6 +550,36 @@ pub async fn soft_delete_event(pool: &PgPool, event_id: &[u8]) -> Result { Ok(result.rows_affected() > 0) } +/// Soft-delete the live row for an addressable coordinate +/// `(kind, pubkey, d_tag)` — the NIP-33 replacement key. +/// +/// Used by `handle_a_tag_deletion` to honour NIP-09 a-tag deletions for any +/// parameterized-replaceable kind. The WHERE clause mirrors +/// `replace_parameterized_event` so the coordinate semantics stay consistent: +/// `channel_id` is intentionally NOT in the key (NIP-33 replacement is global +/// per the spec — `channel_id` is stored for query scoping, not identity). +/// +/// Returns `Ok(true)` if a row was deleted, `Ok(false)` if no live row matched +/// (already deleted, or never existed). +pub async fn soft_delete_by_coordinate( + pool: &PgPool, + kind: i32, + pubkey: &[u8], + d_tag: &str, +) -> Result { + let result = sqlx::query( + "UPDATE events SET deleted_at = NOW() \ + WHERE kind = $1 AND pubkey = $2 AND d_tag = $3 AND deleted_at IS NULL", + ) + .bind(kind) + .bind(pubkey) + .bind(d_tag) + .execute(pool) + .await?; + + Ok(result.rows_affected() > 0) +} + /// Atomically soft-delete an event and decrement thread reply counters. /// /// Wraps the delete + counter update in a single transaction so a crash between diff --git a/crates/sprout-db/src/lib.rs b/crates/sprout-db/src/lib.rs index 867875112..6a0ffd385 100644 --- a/crates/sprout-db/src/lib.rs +++ b/crates/sprout-db/src/lib.rs @@ -267,6 +267,17 @@ impl Db { event::soft_delete_event(&self.pool, event_id).await } + /// Soft-delete the live row for an addressable coordinate `(kind, pubkey, d_tag)`. + /// Used by NIP-09 a-tag deletion for parameterized-replaceable kinds. + pub async fn soft_delete_by_coordinate( + &self, + kind: i32, + pubkey: &[u8], + d_tag: &str, + ) -> Result { + event::soft_delete_by_coordinate(&self.pool, kind, pubkey, d_tag).await + } + /// Atomically soft-delete an event and decrement thread reply counters. pub async fn soft_delete_event_and_update_thread( &self, diff --git a/crates/sprout-relay/src/handlers/side_effects.rs b/crates/sprout-relay/src/handlers/side_effects.rs index 6876da2ba..a2d72808c 100644 --- a/crates/sprout-relay/src/handlers/side_effects.rs +++ b/crates/sprout-relay/src/handlers/side_effects.rs @@ -7,9 +7,9 @@ use tracing::{info, warn}; use uuid::Uuid; use sprout_core::kind::{ - event_kind_u32, KIND_GIT_REPO_ANNOUNCEMENT, KIND_MEMBER_ADDED_NOTIFICATION, - KIND_MEMBER_REMOVED_NOTIFICATION, KIND_NIP29_GROUP_ADMINS, KIND_NIP29_GROUP_MEMBERS, - KIND_NIP29_GROUP_METADATA, KIND_NIP43_MEMBERSHIP_LIST, KIND_REACTION, + event_kind_u32, is_parameterized_replaceable, KIND_GIT_REPO_ANNOUNCEMENT, + KIND_MEMBER_ADDED_NOTIFICATION, KIND_MEMBER_REMOVED_NOTIFICATION, KIND_NIP29_GROUP_ADMINS, + KIND_NIP29_GROUP_MEMBERS, KIND_NIP29_GROUP_METADATA, KIND_NIP43_MEMBERSHIP_LIST, KIND_REACTION, }; use sprout_db::channel::MemberRole; @@ -1380,11 +1380,53 @@ async fn handle_a_tag_deletion(event: &Event, state: &Arc) -> anyhow:: } } } + // Generic NIP-33 (parameterized-replaceable) soft-delete by coordinate. + // + // Listed after the workflow branch so workflow's bespoke deletion + // (which doesn't soft-delete the `events` row by design — that's a + // separate concern) takes precedence. For every other addressable + // kind, including kind:30023 (NIP-23 long-form), we soft-delete the + // live row matching `(kind, pubkey, d_tag)` so REQs stop returning it. + // See https://github.com/block/sprout/issues/714. + k if is_parameterized_replaceable(k) => { + let pubkey_bytes = match hex::decode(pubkey_hex) { + Ok(b) => b, + Err(e) => { + return Err(anyhow::anyhow!( + "invalid pubkey hex in a-tag {pubkey_hex}: {e}" + )); + } + }; + // Safe cast: NIP-33 kinds are 30000–39999, well within i32. + let kind_i32 = k as i32; + let deleted = state + .db + .soft_delete_by_coordinate(kind_i32, &pubkey_bytes, d_tag) + .await + .map_err(|e| { + anyhow::anyhow!( + "failed to soft-delete by coordinate {kind_i32}:{pubkey_hex}:{d_tag}: {e}" + ) + })?; + if deleted { + tracing::info!( + kind = k, + d_tag = d_tag, + "NIP-09 a-tag deletion: soft-deleted addressable event by coordinate" + ); + } else { + tracing::debug!( + kind = k, + d_tag = d_tag, + "NIP-09 a-tag deletion: no live row matched coordinate" + ); + } + } _ => { tracing::debug!( kind = kind_num, d_tag = d_tag, - "NIP-09 a-tag deletion for unhandled kind — no side effect" + "NIP-09 a-tag deletion for non-NIP-33 kind — no side effect" ); } } diff --git a/crates/sprout-test-client/tests/e2e_long_form.rs b/crates/sprout-test-client/tests/e2e_long_form.rs index ff2c4934b..0aa703895 100644 --- a/crates/sprout-test-client/tests/e2e_long_form.rs +++ b/crates/sprout-test-client/tests/e2e_long_form.rs @@ -317,3 +317,89 @@ async fn test_long_form_stale_write_rejected() { client.disconnect().await.expect("disconnect"); } + +/// NIP-09 a-tag deletion: a kind:5 deletion targeting the addressable +/// coordinate `30023::` causes the live event row for that +/// coordinate to be soft-deleted, so subsequent REQs no longer return it. +/// +/// Regression test for issue #714 — before the fix, +/// `handle_a_tag_deletion` only handled the workflow kind and silently +/// no-op'd for kind:30023. +#[tokio::test] +#[ignore] +async fn test_long_form_a_tag_deletion() { + let url = relay_url(); + let keys = Keys::generate(); + let mut client = SproutTestClient::connect(&url, &keys) + .await + .expect("connect"); + + // Publish a note. + let d_tag = format!("a-del-{}", uuid::Uuid::new_v4().simple()); + let note = build_long_form_event(&keys, &d_tag, "Doomed Article", "Body.", vec![]); + let note_id = note.id; + let ok = client.send_event(note).await.expect("send note"); + assert!(ok.accepted, "note should be accepted: {}", ok.message); + + // Sanity check it's queryable before deletion. + let sid_pre = sub_id("a-del-pre"); + let filter_pre = Filter::new() + .kind(Kind::Custom(KIND_LONG_FORM)) + .author(keys.public_key()) + .custom_tag(SingleLetterTag::lowercase(Alphabet::D), [d_tag.as_str()]); + client + .subscribe(&sid_pre, vec![filter_pre]) + .await + .expect("subscribe pre"); + let pre = client + .collect_until_eose(&sid_pre, Duration::from_secs(5)) + .await + .expect("collect pre"); + assert!( + pre.iter().any(|e| e.id == note_id), + "note should be queryable before deletion" + ); + + // Build the addressable coordinate and emit a kind:5 deletion targeting it. + let a_coord = format!( + "{}:{}:{}", + KIND_LONG_FORM, + keys.public_key().to_hex(), + d_tag + ); + let del = EventBuilder::new( + Kind::EventDeletion, + "", + vec![Tag::parse(&["a", &a_coord]).unwrap()], + ) + .sign_with_keys(&keys) + .unwrap(); + let ok_del = client.send_event(del).await.expect("send deletion"); + assert!( + ok_del.accepted, + "a-tag deletion should be accepted: {}", + ok_del.message + ); + + // Query — should now be empty. + let sid_post = sub_id("a-del-post"); + let filter_post = Filter::new() + .kind(Kind::Custom(KIND_LONG_FORM)) + .author(keys.public_key()) + .custom_tag(SingleLetterTag::lowercase(Alphabet::D), [d_tag.as_str()]); + client + .subscribe(&sid_post, vec![filter_post]) + .await + .expect("subscribe post"); + let post = client + .collect_until_eose(&sid_post, Duration::from_secs(5)) + .await + .expect("collect post"); + assert!( + post.is_empty(), + "a-tag deletion should remove the note from REQ results (got {} events)", + post.len() + ); + + client.disconnect().await.expect("disconnect"); +}