diff options
| author | DanConwayDev <DanConwayDev@protonmail.com> | 2026-02-18 09:24:01 +0000 |
|---|---|---|
| committer | DanConwayDev <DanConwayDev@protonmail.com> | 2026-02-18 09:24:01 +0000 |
| commit | 0c01797812bb77fc81d0efe58f0e7858f2b7af66 (patch) | |
| tree | 9310aca4f3e921ca9950a2b11af23ea82788b98b | |
| parent | 467690f33bbbfd442852e61de221e4e5e161b878 (diff) | |
fix: handle announcement replacement when original is still in purgatory
Previously, has_active_announcement() only queried the database, so when
a newer announcement arrived for the same (pubkey, identifier) while the
original was still in purgatory, it was incorrectly routed as a brand-new
announcement (AcceptPurgatory) rather than replacing the existing entry.
This change splits the logic into two cases:
- If the existing entry is in the database: return Accept (replacement) as before
- If the existing entry is only in purgatory: replace the purgatory entry via
add_announcement() (which overwrites by key) and extend expiries for both the
announcement and any waiting state events, then return Accept
- If the owner sends a Reject-classified announcement (service removed) but has
a purgatory entry: clear the purgatory entry, delete the bare repo, and remove
any waiting state events before rejecting
Also add an explicit comment to find_accepted_repository() in related.rs
clarifying that it intentionally only checks the database. Related events
should only be accepted after the repository announcement has been promoted
(validated via git data) - this is correct behaviour, not a missing check.
| -rw-r--r-- | src/nostr/policy/announcement.rs | 161 | ||||
| -rw-r--r-- | src/nostr/policy/related.rs | 5 |
2 files changed, 141 insertions, 25 deletions
diff --git a/src/nostr/policy/announcement.rs b/src/nostr/policy/announcement.rs index abe9651..a90ec94 100644 --- a/src/nostr/policy/announcement.rs +++ b/src/nostr/policy/announcement.rs | |||
| @@ -4,6 +4,7 @@ | |||
| 4 | /// according to GRASP-01 specification. | 4 | /// according to GRASP-01 specification. |
| 5 | use nostr_relay_builder::prelude::{Alphabet, Event, Filter, Kind, PublicKey, SingleLetterTag}; | 5 | use nostr_relay_builder::prelude::{Alphabet, Event, Filter, Kind, PublicKey, SingleLetterTag}; |
| 6 | use std::collections::HashSet; | 6 | use std::collections::HashSet; |
| 7 | use std::time::Duration; | ||
| 7 | 8 | ||
| 8 | use super::PolicyContext; | 9 | use super::PolicyContext; |
| 9 | use crate::config::Config; | 10 | use crate::config::Config; |
| @@ -39,7 +40,8 @@ impl AnnouncementPolicy { | |||
| 39 | /// Validate a repository announcement event | 40 | /// Validate a repository announcement event |
| 40 | /// | 41 | /// |
| 41 | /// Returns: | 42 | /// Returns: |
| 42 | /// - `Accept` if this is a replacement announcement (active announcement exists) | 43 | /// - `Accept` if this is a replacement announcement (active announcement exists in DB or |
| 44 | /// purgatory) | ||
| 43 | /// - `AcceptPurgatory` if this is a new announcement (no active announcement exists) | 45 | /// - `AcceptPurgatory` if this is a new announcement (no active announcement exists) |
| 44 | /// - `AcceptMaintainer` if accepted via maintainer exception | 46 | /// - `AcceptMaintainer` if accepted via maintainer exception |
| 45 | /// - `AcceptArchive` if accepted via GRASP-05 archive config | 47 | /// - `AcceptArchive` if accepted via GRASP-05 archive config |
| @@ -54,6 +56,17 @@ impl AnnouncementPolicy { | |||
| 54 | // GRASP-01 Exception: Accept announcements from recursive maintainers | 56 | // GRASP-01 Exception: Accept announcements from recursive maintainers |
| 55 | match RepositoryAnnouncement::from_event(event.clone()) { | 57 | match RepositoryAnnouncement::from_event(event.clone()) { |
| 56 | Ok(announcement) => { | 58 | Ok(announcement) => { |
| 59 | // If this pubkey+identifier had a purgatory entry, the owner may be | ||
| 60 | // sending a new announcement that removes our service. Clear the | ||
| 61 | // purgatory entry and its bare repo so we don't hold stale data. | ||
| 62 | if self | ||
| 63 | .ctx | ||
| 64 | .purgatory | ||
| 65 | .has_purgatory_announcement(&event.pubkey, &announcement.identifier) | ||
| 66 | { | ||
| 67 | self.remove_purgatory_announcement(&event.pubkey, &announcement.identifier); | ||
| 68 | } | ||
| 69 | |||
| 57 | match self | 70 | match self |
| 58 | .is_maintainer_in_any_announcement( | 71 | .is_maintainer_in_any_announcement( |
| 59 | &announcement.identifier, | 72 | &announcement.identifier, |
| @@ -76,38 +89,55 @@ impl AnnouncementPolicy { | |||
| 76 | // Parse announcement to check for existing active announcement | 89 | // Parse announcement to check for existing active announcement |
| 77 | match RepositoryAnnouncement::from_event(event.clone()) { | 90 | match RepositoryAnnouncement::from_event(event.clone()) { |
| 78 | Ok(announcement) => { | 91 | Ok(announcement) => { |
| 79 | // Check if there's already an active announcement for this (pubkey, identifier) | 92 | let in_db = match self |
| 80 | match self | 93 | .has_db_announcement(&event.pubkey, &announcement.identifier) |
| 81 | .has_active_announcement(&event.pubkey, &announcement.identifier) | ||
| 82 | .await | 94 | .await |
| 83 | { | 95 | { |
| 84 | Ok(true) => { | 96 | Ok(v) => v, |
| 85 | // Replacement announcement - accept immediately | ||
| 86 | tracing::debug!( | ||
| 87 | identifier = %announcement.identifier, | ||
| 88 | "Replacement announcement - accepting immediately" | ||
| 89 | ); | ||
| 90 | validation_result | ||
| 91 | } | ||
| 92 | Ok(false) => { | ||
| 93 | // New announcement - route to purgatory | ||
| 94 | tracing::debug!( | ||
| 95 | identifier = %announcement.identifier, | ||
| 96 | "New announcement - routing to purgatory" | ||
| 97 | ); | ||
| 98 | AnnouncementResult::AcceptPurgatory | ||
| 99 | } | ||
| 100 | Err(e) => { | 97 | Err(e) => { |
| 101 | tracing::warn!( | 98 | tracing::warn!( |
| 102 | error = %e, | 99 | error = %e, |
| 103 | "Failed to check for existing announcement - rejecting" | 100 | "Failed to check for existing DB announcement - rejecting" |
| 104 | ); | 101 | ); |
| 105 | AnnouncementResult::Reject(format!( | 102 | return AnnouncementResult::Reject(format!( |
| 106 | "Database error checking existing announcement: {}", | 103 | "Database error checking existing announcement: {}", |
| 107 | e | 104 | e |
| 108 | )) | 105 | )); |
| 109 | } | 106 | } |
| 107 | }; | ||
| 108 | |||
| 109 | if in_db { | ||
| 110 | // Replacement announcement with DB entry - accept immediately | ||
| 111 | tracing::debug!( | ||
| 112 | identifier = %announcement.identifier, | ||
| 113 | "Replacement announcement (DB) - accepting immediately" | ||
| 114 | ); | ||
| 115 | return validation_result; | ||
| 110 | } | 116 | } |
| 117 | |||
| 118 | let in_purgatory = self | ||
| 119 | .ctx | ||
| 120 | .purgatory | ||
| 121 | .has_purgatory_announcement(&event.pubkey, &announcement.identifier); | ||
| 122 | |||
| 123 | if in_purgatory { | ||
| 124 | // Replacement announcement with purgatory entry - replace it and | ||
| 125 | // extend expiry so the new announcement gets a fresh 30-minute window. | ||
| 126 | tracing::debug!( | ||
| 127 | identifier = %announcement.identifier, | ||
| 128 | "Replacement announcement (purgatory) - replacing purgatory entry" | ||
| 129 | ); | ||
| 130 | self.replace_purgatory_announcement(event, &announcement); | ||
| 131 | // Return Accept (not AcceptPurgatory) - this is a replacement, not new | ||
| 132 | return validation_result; | ||
| 133 | } | ||
| 134 | |||
| 135 | // No existing announcement - route to purgatory | ||
| 136 | tracing::debug!( | ||
| 137 | identifier = %announcement.identifier, | ||
| 138 | "New announcement - routing to purgatory" | ||
| 139 | ); | ||
| 140 | AnnouncementResult::AcceptPurgatory | ||
| 111 | } | 141 | } |
| 112 | Err(e) => AnnouncementResult::Reject(format!( | 142 | Err(e) => AnnouncementResult::Reject(format!( |
| 113 | "Failed to parse announcement: {}", | 143 | "Failed to parse announcement: {}", |
| @@ -120,8 +150,89 @@ impl AnnouncementPolicy { | |||
| 120 | } | 150 | } |
| 121 | } | 151 | } |
| 122 | 152 | ||
| 123 | /// Check if there's an active announcement in the database for this (pubkey, identifier) | 153 | /// Replace a purgatory announcement entry with a newer event. |
| 124 | async fn has_active_announcement( | 154 | /// |
| 155 | /// Called when a replacement announcement arrives for a (pubkey, identifier) pair | ||
| 156 | /// that is currently in purgatory. Updates the purgatory entry and extends the | ||
| 157 | /// expiry so the new announcement has a fresh waiting window. | ||
| 158 | fn replace_purgatory_announcement( | ||
| 159 | &self, | ||
| 160 | event: &Event, | ||
| 161 | announcement: &RepositoryAnnouncement, | ||
| 162 | ) { | ||
| 163 | let repo_path = self.ctx.git_data_path.join(announcement.repo_path()); | ||
| 164 | let relays: HashSet<String> = announcement.relays.iter().cloned().collect(); | ||
| 165 | |||
| 166 | // add_announcement uses the (owner, identifier) key so it overwrites the old entry | ||
| 167 | self.ctx.purgatory.add_announcement( | ||
| 168 | event.clone(), | ||
| 169 | announcement.identifier.clone(), | ||
| 170 | event.pubkey, | ||
| 171 | repo_path, | ||
| 172 | relays, | ||
| 173 | ); | ||
| 174 | |||
| 175 | // Extend the announcement's expiry (reset to full 30 min window) | ||
| 176 | self.ctx.purgatory.extend_announcement_expiry( | ||
| 177 | &event.pubkey, | ||
| 178 | &announcement.identifier, | ||
| 179 | Duration::from_secs(1800), | ||
| 180 | ); | ||
| 181 | |||
| 182 | // Also extend any state events waiting for this identifier | ||
| 183 | let state_entries = self.ctx.purgatory.find_state(&announcement.identifier); | ||
| 184 | if !state_entries.is_empty() { | ||
| 185 | let state_ids: Vec<_> = state_entries.iter().map(|e| e.event.id).collect(); | ||
| 186 | self.ctx.purgatory.extend_expiry( | ||
| 187 | &announcement.identifier, | ||
| 188 | &state_ids, | ||
| 189 | Duration::from_secs(1800), | ||
| 190 | ); | ||
| 191 | } | ||
| 192 | } | ||
| 193 | |||
| 194 | /// Remove a purgatory announcement and clean up associated resources. | ||
| 195 | /// | ||
| 196 | /// Called when a replacement announcement is rejected (owner removed our service). | ||
| 197 | /// Deletes the bare repository from disk and removes any state events waiting for | ||
| 198 | /// this identifier. | ||
| 199 | fn remove_purgatory_announcement(&self, pubkey: &PublicKey, identifier: &str) { | ||
| 200 | // Get the repo path before removing from purgatory | ||
| 201 | if let Some(entry) = self.ctx.purgatory.find_announcement(pubkey, identifier) { | ||
| 202 | // Delete the bare repository from disk | ||
| 203 | if entry.repo_path.exists() { | ||
| 204 | if let Err(e) = std::fs::remove_dir_all(&entry.repo_path) { | ||
| 205 | tracing::warn!( | ||
| 206 | path = %entry.repo_path.display(), | ||
| 207 | error = %e, | ||
| 208 | "Failed to delete bare repository during purgatory cleanup" | ||
| 209 | ); | ||
| 210 | } else { | ||
| 211 | tracing::info!( | ||
| 212 | path = %entry.repo_path.display(), | ||
| 213 | "Deleted bare repository for rejected purgatory announcement" | ||
| 214 | ); | ||
| 215 | } | ||
| 216 | } | ||
| 217 | } | ||
| 218 | |||
| 219 | // Remove the announcement from purgatory | ||
| 220 | self.ctx.purgatory.remove_announcement(pubkey, identifier); | ||
| 221 | |||
| 222 | // Remove any state events waiting for this identifier | ||
| 223 | self.ctx.purgatory.remove_state(identifier); | ||
| 224 | |||
| 225 | tracing::info!( | ||
| 226 | identifier = %identifier, | ||
| 227 | "Cleared purgatory entry: owner removed our service from announcement" | ||
| 228 | ); | ||
| 229 | } | ||
| 230 | |||
| 231 | /// Check if there's an announcement in the database for this (pubkey, identifier). | ||
| 232 | /// | ||
| 233 | /// Only checks the database (promoted announcements). For purgatory checks use | ||
| 234 | /// `purgatory.has_purgatory_announcement()` directly. | ||
| 235 | async fn has_db_announcement( | ||
| 125 | &self, | 236 | &self, |
| 126 | pubkey: &PublicKey, | 237 | pubkey: &PublicKey, |
| 127 | identifier: &str, | 238 | identifier: &str, |
diff --git a/src/nostr/policy/related.rs b/src/nostr/policy/related.rs index 7ce87db..cfe04a7 100644 --- a/src/nostr/policy/related.rs +++ b/src/nostr/policy/related.rs | |||
| @@ -139,6 +139,11 @@ impl RelatedEventPolicy { | |||
| 139 | .push((addr, pubkey, identifier)); | 139 | .push((addr, pubkey, identifier)); |
| 140 | } | 140 | } |
| 141 | 141 | ||
| 142 | // NOTE: Intentionally only checks the database (promoted announcements), not purgatory. | ||
| 143 | // Related events should only be accepted once the repository announcement has been | ||
| 144 | // validated (promoted via git data). Events referencing purgatory-only repositories | ||
| 145 | // are correctly rejected as orphans and can be re-submitted after promotion. | ||
| 146 | |||
| 142 | // Query each kind group | 147 | // Query each kind group |
| 143 | for (kind, refs) in by_kind { | 148 | for (kind, refs) in by_kind { |
| 144 | let authors: Vec<PublicKey> = refs.iter().map(|(_, pk, _)| *pk).collect(); | 149 | let authors: Vec<PublicKey> = refs.iter().map(|(_, pk, _)| *pk).collect(); |