diff --git a/.env.example b/.env.example index fa892f6..721b793 100644 --- a/.env.example +++ b/.env.example @@ -170,7 +170,7 @@ # NGIT_NAUGHTY_LIST_EXPIRATION_HOURS=12 # ============================================================================ -# HOLDING DB CLEANUP +# HOLDING DB AND DELETION-REQUEST CLEANUP # ============================================================================ # Retention window in seconds for deleted events kept in holding DB @@ -180,7 +180,8 @@ # Default: 7776000 (90 days) # NGIT_HOLDING_RETENTION_SECS=7776000 -# Interval in seconds between holding DB background cleanup passes +# Interval in seconds between holding DB and deletion-request retention background +# cleanup passes. This cadence does not alter timestamp-derived retention deadlines. # Must be greater than 0 # CLI: --holding-cleanup-interval-secs # Default: 86400 (24 hours) @@ -256,21 +257,21 @@ # NGIT_GRASP06_ENABLE=false # ============================================================================ -# DELETION REQUESTS (NIP-09) +# DELETION REQUESTS (NIP-09 AND NIP-62) # ============================================================================ -# Deletion request disrespector: ignore NIP-09 deletion requests (archival mode) +# Deletion request disrespector: ignore NIP-09 and NIP-62 requests (archival mode) # -# When enabled, incoming NIP-09 (kind 5) deletion requests are STORED but NOT -# acted upon: targeted events remain fully accessible. This makes the relay an -# archival server, preserving content and preventing "left-pad" scenarios. -# NIP-11 supported_nips will NOT advertise NIP-09 (deletion) or NIP-62 -# (request to vanish) in this mode. +# When enabled, incoming NIP-09 (kind 5) deletion requests and NIP-62 +# request-to-vanish events are STORED but NOT acted upon: their targets remain +# fully accessible. This makes the relay an archival server, preserving content +# and preventing "left-pad" scenarios. NIP-11 supported_nips will NOT advertise +# NIP-09 (deletion) or NIP-62 (request to vanish) in this mode. # -# This ONLY affects NIP-09 user-initiated deletions. It does NOT prevent -# blacklist-triggered deletions (operator moderation: spam/malware/abuse). +# This ONLY affects NIP-09 and NIP-62 user-initiated requests. It does NOT +# prevent blacklist-triggered deletions (operator moderation: spam/malware/abuse). # -# When disabled (default), deletion requests are honoured: targeted events are +# When disabled (default), both request types are honoured: their targets are # deleted and re-submission stays rejected. NIP-09 and NIP-62 are advertised in # NIP-11. # @@ -280,6 +281,34 @@ # Default: false # NGIT_DELETION_REQUEST_DISRESPECTOR=false +# Deletion-request retention applies to accepted NIP-09 deletion requests and +# NIP-62 request-to-vanish events. All values are integer seconds. Unused clocks +# start at relay-observed first_seen_at; used clocks start at last_used_at. +# "Additional" periods begin after their corresponding served period ends. +# Used requests remain served indefinitely when deletion-request-disrespector is true. +# Seconds permit short tests; production periods should be at least one day and +# comfortably exceed worst-case deletion/archive processing time. + +# How long an unused request remains served from first_seen_at +# CLI: --deletion-request-retention-unused-served-secs +# Default: 2592000 (30 days) +# NGIT_DELETION_REQUEST_RETENTION_UNUSED_SERVED_SECS=2592000 + +# Additional time an unused request remains unserved but eligible to gate +# CLI: --deletion-request-retention-unused-unserved-gating-additional-secs +# Default: 15552000 (180 days) +# NGIT_DELETION_REQUEST_RETENTION_UNUSED_UNSERVED_GATING_ADDITIONAL_SECS=15552000 + +# Normal-mode time a used request remains served after last_used_at +# CLI: --deletion-request-retention-used-served-after-last-used-secs +# Default: 23328000 (270 days; 9 fixed 30-day months) +# NGIT_DELETION_REQUEST_RETENTION_USED_SERVED_AFTER_LAST_USED_SECS=23328000 + +# Additional normal-mode time a used request remains unserved but continues gating +# CLI: --deletion-request-retention-used-unserved-gating-additional-secs +# Default: 7776000 (90 days; 3 fixed 30-day months) +# NGIT_DELETION_REQUEST_RETENTION_USED_UNSERVED_GATING_ADDITIONAL_SECS=7776000 + # ============================================================================ # REPOSITORY WHITELIST # ============================================================================ diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e22bc1..f61bb26 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Added four configuration options for bounded retention of NIP-09 deletion requests and NIP-62 request-to-vanish events, together with cleanup telemetry for operators. + +### Changed + +- Addressed a production storage imbalance where roughly 50k of 60k stored events were deletion requests. Deletion requests now have a bounded lifecycle, so requests that are no longer relevant are reconciled and retired while requests that may still affect valid event handling are preserved. +- Deletion-disrespector mode now explicitly applies to both NIP-09 deletion requests and NIP-62 request-to-vanish events. +- Retired the hidden `repair-deletion-requests` maintenance command. + ## [1.2.0] - 2026-07-06 ### Fixed diff --git a/Cargo.toml b/Cargo.toml index a012e5f..05bac9a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -129,6 +129,10 @@ path = "tests/lifecycle/nip09_state_multi_maintainer.rs" name = "nip09_validation" path = "tests/lifecycle/nip09_validation.rs" +[[test]] +name = "deletion_request_retention" +path = "tests/lifecycle/deletion_request_retention.rs" + [[test]] name = "nip62_lifecycle" path = "tests/lifecycle/nip62_lifecycle.rs" diff --git a/docs/explanation/monitoring.md b/docs/explanation/monitoring.md index 3304f85..82dd9ca 100644 --- a/docs/explanation/monitoring.md +++ b/docs/explanation/monitoring.md @@ -78,6 +78,9 @@ The deletion/recovery operational paths expose these additional metrics: | `ngit_holding_cleanup_runs_total` | Counter | - | Number of holding cleanup passes run | | `ngit_holding_cleanup_deleted_total` | Counter | `type` | Total deleted objects by cleanup (`metadata`, `payload`, `archive_file`) | | `ngit_holding_cleanup_last_run_deleted` | Gauge | `type` | Deleted object counts for most recent cleanup pass | +| `ngit_deletion_request_cleanup_runs_total` | Counter | - | Number of deletion-request cleanup passes run | +| `ngit_deletion_request_cleanup_removed_total` | Counter | `type` | Deletion-request payloads and lifecycle metadata removed (`main`, `tombstone`, `metadata`) | +| `ngit_deletion_request_cleanup_outcomes_total` | Counter | `outcome` | Deletion-request cleanup failures and stale/concurrent skips (`failure`, `stale_or_concurrent_skip`) | | `ngit_recovery_total` | Counter | `result` | Recovery attempts and outcomes (`attempted`, `succeeded`, `failed`, `partial`) | | `ngit_manual_ejections_total` | Counter | - | Number of operator manual ejection operations | | `ngit_manual_ejection_deleted_total` | Counter | `type` | Objects removed by manual ejection (`metadata`, `payload`, `archive_file`) | diff --git a/docs/explanation/repository-lifecycle.md b/docs/explanation/repository-lifecycle.md index 349fdb2..0a310cc 100644 --- a/docs/explanation/repository-lifecycle.md +++ b/docs/explanation/repository-lifecycle.md @@ -8,6 +8,326 @@ NIP-09 deletion requests, NIP-62 request-to-vanish events, operator blacklist an whitelist reconciliation, service de-listing, holding/archive retention, recovery, and purgatory transitions. +## Bounded request retention + +> **Status:** Lifecycle metadata, NIP-09 and NIP-62 lifecycle admission +> (including disrespector read-only would-have-deleted classification), +> deterministic admission-use attribution, measured destructive outcomes, +> startup reconciliation, periodic request cleanup/permanent expiry, removal of +> target-set deduplication, and repair-command retirement are implemented. + +### Production motivation + +Deletion and vanish requests are necessary admission controls, but they can also +create substantial retention pressure without hostile traffic. Valid requests +published by reputable users may be unrelated to this relay's Git data, and a +request can arrive before any event it names. The relay therefore cannot require +the target to exist without breaking legitimate out-of-order delivery. + +Production measurements exposed the cost of retaining every such request +indefinitely: approximately 50,000 of 60,000 stored events were deletion +requests. In other words, the relay was receiving and storing roughly five times +more deletion requests than all other event kinds combined. Target-coverage +deduplication accounted for only about 1,500 requests, leaving approximately +48,000 requests whose retention was not addressed by target-set deduplication. +Retaining every accepted request permanently is therefore the wrong storage +policy; the relay should retain requests according to demonstrated utility +instead. + +Bounded lifecycle retention is a storage-hygiene mechanism, not comprehensive +adversarial admission control. Trust-based admission, rate controls, and +operator moderation are separate concerns and can be layered over this policy. + +### Intent + +The relay should: + +1. accept valid out-of-order requests without requiring an existing target; +2. serve a new request for an intentionally generous probation window that + covers delayed rebroadcasting and intermittently connected clients; +3. retain an unserved request for a further window in which it can still block a + late event; +4. retain and serve requests that demonstrably deleted or blocked data; +5. eventually expire both the request and its admission-gate effect; and +6. permanently remove expired requests and their lifecycle metadata from live + storage. + +This lifecycle applies to NIP-09 deletion requests and NIP-62 vanish requests. +For NIP-62, a request that does not target this relay is never an active local +vanish gate, but it still follows the bounded served/unserved retention schedule. +In deletion-disrespector mode, NIP-09 and NIP-62 requests likewise follow the +storage schedule. A request is classified as used when it would have removed an +existing main-database or purgatory event under normal policy, but the relay does +not perform that removal or enforce an admission gate. Used requests in this +mode remain served indefinitely; unused requests age out normally. + +### Request lifecycle + +Retention is based on a durable relay-observed `first_seen_at` timestamp, not the +client-controlled Nostr `created_at` timestamp. + +#### New and unused + +1. **Served probation — 30 days from first receipt:** store the original signed + request in the main database and serve it normally. Thirty days is + intentionally much longer than ordinary Nostr propagation, which is usually + measured in hours: the additional time accommodates delayed client + rebroadcasting and intermittently connected clients. If processing the + request successfully removes at least one event from the main database or + purgatory, the request is immediately considered used. Attempted or failed + deletions do not count. +2. **Unserved pending gate — a further 180 days:** if still unused after 30 days, + remove it from the main database but retain it in the tombstone database. + Continue consulting locally actionable requests during admission; retained + disrespector and non-targeting NIP-62 records do not enforce a gate. +3. **Expiry:** if it remains unused after the additional 180 days, permanently + remove the request and its lifecycle metadata from the tombstone database. + +A pending request becomes used only when it actually causes a later event to be +rejected under valid ownership, coordinate-cutoff, relay-targeting, and other +NIP-09/NIP-62 rules. Merely matching a tag, inspecting the request, or discovering +that a different author owns the target does not count as use. + +The unserved gate is the deliberate observation period that establishes whether +an apparently unused request still has practical value. While the request was +served, clients that saw it could avoid sending the deleted target to this relay, +even if copies of that target continued circulating elsewhere. Removing the +request from relay queries gives those late copies an opportunity to reach the +relay again. The retained Tombstone gate still rejects a covered target; that +rejection is evidence of current utility, so the winning request becomes used, +is promoted back to Main, and begins the longer used lifecycle. If no covered +target arrives during the unserved period, the request has supplied no evidence +that its gate is still needed and can expire. Replaying the deletion or vanish +request itself is not such evidence and does not update `last_used_at`. + +#### Used + +When a request is used: + +1. promote it back to the served main database if necessary; +2. serve it and enforce its admission gate for **9 months after its last use**; +3. after 9 months, stop serving it but continue enforcing its gate for a further + **3 months**; and +4. after 12 months without use, expire its gate and permanently remove the + request and its lifecycle metadata. + +The normal-mode schedule above does not expire used requests in disrespector +mode: because archival relays exist to preserve deletion history, a request that +would have deleted stored data remains served there indefinitely. + +Every successful use resets `last_used_at` and therefore restarts the 9-month +served plus 3-month unserved lifecycle. Unused retention is based on +`first_seen_at`; used retention is based on `last_used_at`. Expiry is intentional: +once both periods end, a previously deleted event, coordinate version, or +vanished-author event is eligible for admission again unless another live +request covers it. + +The four lifecycle durations are operator-configurable, with defaults of +30 days for unused serving, a further 180 days for unused gating, 9 months for +used serving, and a further 3 months for used gating. Configuration validation +must preserve the ordering of each served period followed by its unserved gate +period. The existing holding cleanup cadence also schedules deletion-request +retention cleanup. Timestamp-derived deadlines determine lifecycle eligibility; +physical removal from Main and Tombstones is asynchronous cleanup and may occur +on the next scheduled pass. This bounded cleanup delay is intentional and does +not extend admission-gate eligibility past the deadline. + +Durations use seconds for configuration consistency and to permit short automated +tests. Production values are expected to be at least one day and comfortably +longer than the worst-case processing time for a deletion or vanish request, +including repository archival and cascade work. Sub-day values are a testing +facility, not a supported production operating point; cleanup is therefore not +coordinated with an initial request handler across an artificially short total +lifetime. + +### Multiple matching requests + +If several pending requests would independently reject the same arriving event, +promoting all of them would turn one target arrival into retention amplification: +an attacker could publish many equivalent requests and make all of them long +lived with one later event. Exactly one deterministic sufficient request should +receive credit for the use and be promoted. The remaining matching requests keep +their existing lifecycle and expire normally. + +The winner-selection rule must be deterministic across restarts and independent +of database iteration order. It must also preserve deletion correctness: for an +`a`-tag target, the selected request must have a cutoff that actually covers the +arriving event. A stable event-ID tie-break should be used after semantic +eligibility and lifecycle priority are considered. + +### Multi-target requests + +Nostr events are signed and cannot be rewritten into a smaller authentic event. +If any target makes a request used, retain and promote the whole signed request, +including all of its valid targets. This is why the existing maximum target-tag +limit remains an important admission bound. + +### Integration with lifecycle storage + +Request retention should extend the existing lifecycle stores rather than add an +independent archive hierarchy: + +1. **Main database — served state:** contains a copy of each request during its + initial 30-day probation and each normally honored used request within 9 + months of `last_used_at`. A used request on a disrespector relay remains here + indefinitely. Presence in this database determines whether normal relay + queries serve the original signed request. +2. **Tombstone database — unserved gate and request lifecycle:** contains the + original signed NIP-09/NIP-62 request plus relay-generated lifecycle metadata. + In normal mode it remains the authoritative admission-gate source. It also + retains unused disrespector and non-targeting NIP-62 requests until their + lifecycle expires, although those records do not enforce a local gate. + Disrespector NIP-09 requests are read-only evaluated after persistence and + receive `last_used_at` only when an existing main-database or purgatory + target would be removed by normal policy. +3. **Holding database — deleted payload retention:** continues to contain events + actually removed by NIP-09/NIP-62, with its existing deletion metadata and + independent holding-retention clock. Expiry of a request does not shorten or + extend holding retention, and expiry of holding data does not change a live + request gate. +4. **Replaceable-history database — rollback state:** remains independent and + continues to supply valid prior replaceable/addressable versions during a + deletion rollback. Request retention must not duplicate this payload history. + +No cold request database is added. This matches holding cleanup's existing model: +when retention ends, payload and internal metadata are permanently removed. + +The Tombstone store should adopt the same payload-plus-internal-metadata-event +pattern already used by Holding and Replaceable History. The original request is +stored verbatim. A relay-generated metadata event, linked to the request with an +`e` tag and never exposed to clients, records at least `first_seen_at`, optional +`last_used_at`, and whether the request is locally actionable, non-targeting, or +handled in disrespector mode. + +State transitions must be ordered so failures cannot silently lose a live gate: + +- On receipt, persist the request and `first_seen_at` metadata in Tombstones + before destructive work or acceptance for main-database storage. +- Mark it used only after at least one main-database or purgatory removal has + succeeded. A partial deletion counts as use if at least one removal succeeded. +- In disrespector mode, classify it as used when a read-only normal-policy + target lookup finds at least one existing main-database or purgatory event + that would be removed, without deleting that event, installing a gate, or + performing holding, archive, cascade, rollback, or repository work. +- When a later admission is blocked, durably update the deterministic winner's + `last_used_at` and restore its main-database copy before completing the + rejection path. If that update fails, report an internal policy error rather + than claiming that retention was extended. +- On expiry, remove the main-database copy first, then its Tombstone payload and + metadata. If Tombstone cleanup fails, the unserved gate remains conservative + and the next cleanup pass retries it. + +The Tombstone store should expose one canonical current metadata record per +request. A `last_used_at` update should save its replacement metadata before +deleting the older metadata record; readers choose the newest valid record during +an interrupted update. Cleanup then compacts stale metadata so frequent reuse +cannot create another unbounded stream. + +### Deterministic use attribution + +Admission lookup may find several sufficient requests. It should first discard +requests that do not actually authorize rejection, including wrong-author +NIP-09 requests, coordinate deletions with an insufficient cutoff, expired +requests, and NIP-62 requests not targeting this relay. From the remaining +candidates it should choose exactly one winner using this stable order: + +1. prefer an already-used request over an unused request, avoiding unnecessary + promotion of another payload; +2. prefer the earliest relay-observed `first_seen_at`; and +3. use the lowest event ID as the final tie-break. + +Only the winner receives a `last_used_at` update and possible promotion to the +main database. Other sufficient requests still participate in rejection +correctness, but their retention clocks do not change. This rule is independent +of database iteration order and prevents one event arrival from extending an +arbitrary number of duplicate requests. + +This deterministic single-winner rule applies when one later target admission is +matched against already-pending requests. Initial processing of distinct deletion +requests is intentionally less strict: two requests processed concurrently may +both observe the same stored target before either removal completes and may both +receive use credit. The database deletion API does not report an authoritative +per-event removed count, so eliminating that narrow race would require broader +target-level serialization. The occasional extra used request is accepted: it is +bounded by actual concurrency, does not change deletion correctness, and still +expires through the normal used lifecycle. + +### Simplification of target-set deduplication + +The lifecycle replaces semantic target-set deduplication and supersession as the +primary storage-control mechanism. Distinct, valid signed requests receive +independent probation windows even when their target sets overlap. Exact replay +of the same event ID remains an ordinary database duplicate and does not create a +new record or reset `first_seen_at`. + +The live write path therefore does not reject a distinct request merely because +another request already covers its targets, and should not delete older requests +by trying to prove one signed request semantically subsumes another. Deterministic +single-winner attribution provides the necessary anti-amplification bound when a +target later arrives, while age-based cleanup handles requests that never become +useful. The temporary target-deduplication repair command has been retired; +startup migration and lifecycle cleanup own historical request handling. + +Once an expired request has been permanently removed, replaying that identical +signed event starts a new lifecycle because no live event-ID marker remains. + +### Migration and policy-mode changes + +Existing request rows predate relay-observed lifecycle metadata, so their true +`first_seen_at` and `last_used_at` cannot be reconstructed reliably. Migration +assigns the migration timestamp as `first_seen_at` and gives every historical +request a fresh 30-day probation window. This deliberately favors preservation +over immediate production cleanup; normal cleanup moves requests that remain +unused out of the served main database after that window. + +Startup performs this migration before blacklist/whitelist reconciliation and +before the relay starts serving traffic. It discovers signed kind-5 and kind-62 +payloads from both the served and tombstone databases, including tombstone +payloads whose metadata is missing or malformed, and deduplicates by signed +event ID. Valid existing metadata is preserved; missing metadata is written +with one startup timestamp and the original payload is promoted to the served +database for its fresh probation. The pass is restart-safe: replacement +metadata preserves the earliest `first_seen_at` and greatest `last_used_at`. + +The same startup pass reclassifies retained requests using current relay +configuration and reconciles served copies against the lifecycle deadline. +In disrespector mode it read-only evaluates unused targeting requests against +main and purgatory data, marking only requests that would currently have an +effect as used; it never changes target data during that evaluation. This +evaluation includes requests whose unused lifecycle elapsed while the relay was +offline: if a matching target is present, startup gives the request use credit +before cleanup runs. Used targeting requests remain served in that mode +indefinitely, while non-targeting NIP-62 and unused no-op requests retain their +ordinary bounded served schedule. Critical query, metadata, promotion, or +removal failures fail startup rather than allowing traffic to begin with an +incomplete lifecycle reconciliation. + +The relay's current deletion-disrespector configuration governs retained +requests; receipt-time mode is not permanent metadata. Switching into +disrespector mode removes their local gates and keeps requests that are used or +would delete currently stored data served indefinitely. Switching back to normal +mode reapplies normal gating and the 9-month served plus 3-month unserved expiry +schedule using retained lifecycle timestamps. Startup reconciliation must apply +these transitions before the relay begins serving traffic. + +Startup catch-up and periodic cleanup use the holding-cleanup cadence. They +evaluate the same timestamp-derived half-open lifecycle boundaries, remove +expired served copies from Main before Tombstones, and permanently remove +payload plus lifecycle metadata only after the additional gating period. Main +and Tombstone removal can therefore lag a deadline until the next cleanup pass; +that is expected cleanup latency, not an extension of the request's gate +eligibility. Admission first probes for matching live requests without the +lifecycle transition lock, so ordinary unrelated event writes are not serialized +behind deletion-store reads. When that probe finds a possible gate, admission +acquires the shared transition lock and repeats candidate discovery and winner +selection before promotion. This confines serialization to events that actually +match a live request while still preventing cleanup from expiring one candidate +and causing admission to overlook another live candidate during promotion. + +Target-set deduplication has been removed and the temporary +`repair-deletion-requests` command has been retired. Startup migration and +periodic cleanup now handle historical requests under the same lifecycle rules. + ## Core consistency invariant ngit-grasp enforces the following invariant across admission, serving, deletion, @@ -187,35 +507,20 @@ deprecated in favor of the maintenance command. ``` 1. Kind 5 deletion request arrives ↓ -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: +2. 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 - target later arrives from a different author, the stale deletion request is - removed from both tombstones and the served deletion-request stream + - pre-emptive `e` deletes for unknown targets may be accepted; candidate + lookup later binds the request author to the arriving event author, so a + foreign target does not block admission or receive use credit - `a` coordinates are acted on only when coordinate pubkey matches deleter ↓ -4. Record deletion tombstone so re-submission is gated +3. Record the signed request and independent lifecycle metadata in Tombstones. + Exact event-ID replay reuses its canonical record without resetting + `first_seen_at`; a distinct signed request is accepted even when targets + overlap. ↓ -5. Compact superseded deletion requests: - - after the new tombstone is recorded, remove older same-author kind-5 - requests from the tombstone DB and served main DB when every actionable - target in the older request is covered by the new request - - a newer `a`-tag cutoff supersedes an older cutoff for the same coordinate; - `e` targets must still be present in the new request - - removal uses the database delete path so event-id/kind/tag indexes stay in - sync with event storage - ↓ -6. Process targets: +4. Process targets: - `a` tag targeting a kind-30617 announcement coordinate: recursively discover the accepted-reference component affected by the announcement deletion @@ -230,29 +535,37 @@ deprecated in favor of the maintenance command. - other valid `e`/`a` targets: delete the targeted event/coordinate without announcement graph cascade ↓ -7. Archive git repository to .archive//-.tar.gz +5. Archive git repository to .archive//-.tar.gz when deleting a repository announcement with live git data ↓ -8. Move deleted events to holding database: +6. 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 ↓ -9. Delete events from main database +7. Delete events from main database ↓ -10. Remove the live git repository when the owner+identifier announcement scope +8. Remove the live git repository when the owner+identifier announcement scope is no longer served ↓ -11. Deleted/tombstoned targets no longer serve in queries +9. Deleted/tombstoned targets no longer serve in queries ↓ -12. Background task (daily): +10. Background task (daily): - Check holding database for expired entries - Delete events older than retention period - Delete corresponding archive files ``` +Deletion processing records a structured outcome: successful main-database +removals and successful purgatory removals are counted separately from skipped +fail-safe preservation and query/delete failures. A kind-5 request receives +`last_used_at` only after at least one actual main-DB or purgatory event removal; +archiving to Holding, git/filesystem cleanup, candidate matching, and failed +operations do not constitute use. NIP-62 uses the same rule, including its +author-wide purgatory eviction. + Deletion gate checks tombstones before kind-specific admission and rejects: - events from vanished pubkeys, - re-submission of deleted event IDs, @@ -328,16 +641,13 @@ When `deletion_request_disrespector = true`: ``` 1. Kind 5 deletion request arrives ↓ -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 +2. Record independent lifecycle metadata and store the signed deletion request + event in the main database. Exact event-ID replay reuses its existing + lifecycle record; distinct overlapping requests remain distinct. ↓ -3. Store deletion request event in main database +3. Do NOT process deletion ↓ -4. Do NOT process deletion - ↓ -5. Repository and events remain fully accessible +4. Repository and events remain fully accessible ↓ Result: Archival relay preserves all content ``` @@ -353,9 +663,8 @@ NIP-62 vanish requests. It does NOT prevent blacklist-triggered deletions. **Implementation Note:** Implemented. The LMDB backend's automatic NIP-09 and NIP-62 processing is disabled (`process_nip09(false)`, `process_nip62(false)`); -ngit-grasp owns deletion handling in relay policy code, which short-circuits -already-covered NIP-09 deletions before the `deletion_request_disrespector` -archival-mode branch. When `deletion_request_disrespector` is set, new kind-5 +ngit-grasp owns deletion handling in relay policy code. When +`deletion_request_disrespector` is set, new kind-5 requests are stored but not acted on, and kind-62 requests are stored but not acted on. NIP-09 and NIP-62 are omitted from the NIP-11 `supported_nips` list in this mode. @@ -822,9 +1131,11 @@ This allows clients to discover whether a relay respects deletion requests. ### Attack Vectors -**DoS via Deletion Spam:** -- Mitigation: ownership checks + normal admission validation -- Mitigation: idempotent delete paths for already-deleted targets +**Deletion-Request Storage Pressure:** +- Mitigation: bounded probation and unserved gating periods +- Mitigation: longer retention only after demonstrated local utility +- Mitigation: permanent expiry of unused request payloads and lifecycle metadata +- Mitigation: ownership checks, normal admission validation, and operator moderation **Archive Disk Exhaustion:** - Mitigation: Background cleanup enforces retention limits @@ -851,6 +1162,9 @@ This allows clients to discover whether a relay respects deletion requests. - `ngit_holding_cleanup_runs_total` - `ngit_holding_cleanup_deleted_total{type}` - `ngit_holding_cleanup_last_run_deleted{type}` +- `ngit_deletion_request_cleanup_runs_total` +- `ngit_deletion_request_cleanup_removed_total{type}` +- `ngit_deletion_request_cleanup_outcomes_total{outcome}` - `ngit_recovery_total{result}` - `ngit_manual_ejections_total` - `ngit_manual_ejection_deleted_total{type}` diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index d544a0c..777d02f 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -492,9 +492,9 @@ NGIT_REJECTED_COLD_INDEX_EXPIRY_SECS=1209600 --- -### Holding DB Cleanup Configuration +### Holding DB and Deletion-Request Cleanup Configuration -These options control retention and cleanup cadence for deleted events archived in the holding database. +These options control retention for deleted events archived in the holding database and the shared cleanup cadence for holding and deletion-request retention records. #### `NGIT_HOLDING_RETENTION_SECS` @@ -522,7 +522,7 @@ NGIT_HOLDING_RETENTION_SECS=2592000 #### `NGIT_HOLDING_CLEANUP_INTERVAL_SECS` -**Description:** Interval between periodic holding DB expiration cleanup passes +**Description:** Interval between periodic holding DB expiration cleanup passes and deletion-request retention cleanup passes **Type:** Integer (seconds) **Default:** `86400` (24 hours) **Required:** No @@ -541,6 +541,7 @@ NGIT_HOLDING_CLEANUP_INTERVAL_SECS=21600 - Must be greater than 0 - Smaller values clean up expired records sooner at the cost of more background work +- This interval determines how soon expired records are observed and physically removed. A deletion request may remain queryable from Main until the next cleanup pass, but admission-gate eligibility still ends at its timestamp-derived deadline; this cleanup latency does not reset or extend the lifecycle. --- @@ -1130,11 +1131,11 @@ Event blacklist does **not** affect NIP-11 metadata: --- -### Deletion Requests (NIP-09) +### Deletion Requests (NIP-09 and NIP-62) #### `NGIT_DELETION_REQUEST_DISRESPECTOR` -**Description:** Ignore NIP-09 deletion requests and act as an archival server +**Description:** Ignore NIP-09 deletion requests and NIP-62 request-to-vanish events and act as an archival server **Type:** Boolean **Default:** `false` (deletion requests are honoured) **Required:** No @@ -1143,22 +1144,24 @@ Event blacklist does **not** affect NIP-11 metadata: **Behavior:** - When `false` (default): - - NIP-09 (kind 5) deletion requests are honoured: targeted events are + - NIP-09 (kind 5) deletion requests and NIP-62 request-to-vanish events are + honoured: their targets are hard-deleted from the relay, matching purgatory entries are evicted, and a persistent tombstone is recorded so re-submission of the deleted event stays rejected across restarts. - NIP-11 `supported_nips` includes `9` (deletion) and `62` (request to vanish). - When `true`: - - Incoming NIP-09 deletion requests are still **stored** (the client receives - an OK), but they are **not acted upon**. Targeted events remain fully - accessible. This makes the relay an archival server, preserving content and - preventing "left-pad" scenarios. + - Incoming NIP-09 deletion requests and NIP-62 request-to-vanish events are + still **stored** (the client receives an OK), but they are **not acted + upon**. Their targets remain fully accessible. This makes the relay an + archival server, preserving content and preventing "left-pad" scenarios. - NIP-11 `supported_nips` does **not** include `9` or `62`, so clients can discover that this relay does not honour deletions. -**IMPORTANT:** This setting ONLY affects NIP-09 user-initiated deletions. It does -**NOT** prevent blacklist-triggered deletions, which are an operator moderation -mechanism (spam/malware/abuse) that archival relays still need. +**IMPORTANT:** This setting ONLY affects NIP-09 and NIP-62 user-initiated +requests. It does **NOT** prevent blacklist-triggered deletions, which are an +operator moderation mechanism (spam/malware/abuse) that archival relays still +need. **Use Cases:** @@ -1169,7 +1172,7 @@ mechanism (spam/malware/abuse) that archival relays still need. **Examples:** ```bash -# Archival relay: preserve deleted content (disrespect NIP-09) +# Archival relay: preserve targets of NIP-09 and NIP-62 requests NGIT_DELETION_REQUEST_DISRESPECTOR=true # Standard relay: honour deletion requests (default) @@ -1181,6 +1184,86 @@ for the full lifecycle design rationale. --- +### Deletion-Request Retention (NIP-09 and NIP-62) + +These options govern the bounded lifecycle of accepted NIP-09 deletion requests and NIP-62 request-to-vanish events. All values are integer seconds. The used-request defaults express months as fixed 30-day periods, not calendar months. + +The relay derives unused deadlines from relay-observed `first_seen_at`, never from the client-controlled event `created_at`. It derives used deadlines from `last_used_at`. Each `...ADDITIONAL...` option begins only after its corresponding served period ends; it is not a total retention duration. + +Seconds are used for configuration consistency and short automated tests. In production, configure every period to at least one day and keep each total lifecycle comfortably longer than the worst-case deletion processing time, including repository archival and cascade work. Sub-day values are intended only for tests. + +#### `NGIT_DELETION_REQUEST_RETENTION_UNUSED_SERVED_SECS` + +- **Description:** How long an unused deletion or vanish request remains served from relay-observed `first_seen_at` +- **Type:** Positive integer (seconds) +- **Default:** `2592000` (30 days) +- **Required:** No +- **CLI:** `--deletion-request-retention-unused-served-secs` + +```bash +# Default: serve an unused request for 30 days from first_seen_at +NGIT_DELETION_REQUEST_RETENTION_UNUSED_SERVED_SECS=2592000 +``` + +After this served period, an unused request enters its additional unserved/gating period. + +--- + +#### `NGIT_DELETION_REQUEST_RETENTION_UNUSED_UNSERVED_GATING_ADDITIONAL_SECS` + +- **Description:** Additional time after unused serving ends that an unused deletion or vanish request remains unserved but eligible to gate admission +- **Type:** Positive integer (seconds) +- **Default:** `15552000` (180 days) +- **Required:** No +- **CLI:** `--deletion-request-retention-unused-unserved-gating-additional-secs` + +```bash +# Default: retain an unused request as an unserved gate for a further 180 days +NGIT_DELETION_REQUEST_RETENTION_UNUSED_UNSERVED_GATING_ADDITIONAL_SECS=15552000 +``` + +The unused request expires after `unused served + this additional period` from `first_seen_at`. Retained disrespector and non-targeting NIP-62 records do not enforce a local admission gate. When an archival relay starts, it reclassifies an expired unused targeting request as used if its target is present before cleanup runs; this preserves the used request indefinitely. + +--- + +#### `NGIT_DELETION_REQUEST_RETENTION_USED_SERVED_AFTER_LAST_USED_SECS` + +- **Description:** In normal mode, how long a used deletion or vanish request remains served after `last_used_at` +- **Type:** Positive integer (seconds) +- **Default:** `23328000` (270 days; 9 fixed 30-day months) +- **Required:** No +- **CLI:** `--deletion-request-retention-used-served-after-last-used-secs` + +```bash +# Default: serve a used request for 270 days after last_used_at +NGIT_DELETION_REQUEST_RETENTION_USED_SERVED_AFTER_LAST_USED_SECS=23328000 +``` + +Every successful use resets `last_used_at` and restarts this served period. When `NGIT_DELETION_REQUEST_DISRESPECTOR=true`, used requests remain served indefinitely, so this normal-mode duration does not expire them. + +--- + +#### `NGIT_DELETION_REQUEST_RETENTION_USED_UNSERVED_GATING_ADDITIONAL_SECS` + +- **Description:** Additional normal-mode time after used serving ends that a used deletion or vanish request remains unserved but continues gating admission +- **Type:** Positive integer (seconds) +- **Default:** `7776000` (90 days; 3 fixed 30-day months) +- **Required:** No +- **CLI:** `--deletion-request-retention-used-unserved-gating-additional-secs` + +```bash +# Default: continue gating for a further 90 days after used serving ends +NGIT_DELETION_REQUEST_RETENTION_USED_UNSERVED_GATING_ADDITIONAL_SECS=7776000 +``` + +The used request expires after `used served + this additional period` from `last_used_at`. When `NGIT_DELETION_REQUEST_DISRESPECTOR=true`, used requests remain served indefinitely, so this normal-mode additional period does not expire their local record. + +**Validation:** Each deletion-request retention duration must be greater than zero. Each served and additional-gating pair must also fit in an unsigned 64-bit second duration when combined. + +**Cleanup cadence:** `NGIT_HOLDING_CLEANUP_INTERVAL_SECS` schedules both holding DB and deletion-request retention cleanup passes. Cleanup evaluates deadlines derived from `first_seen_at` and `last_used_at`; changing its cadence never changes those deadlines. + +--- + ### Rate Limiting & DoS Protection #### `NGIT_MAX_CONNECTIONS` diff --git a/nix/module.nix b/nix/module.nix index 58044e7..ee3d385 100644 --- a/nix/module.nix +++ b/nix/module.nix @@ -189,7 +189,55 @@ let type = types.int; default = 86400; description = - "Interval in seconds between holding DB cleanup passes (default: 24 hours)"; + "Interval in seconds between holding DB and deletion-request retention cleanup passes (default: 24 hours)"; + }; + + deletionRequestRetention = { + unusedServedSecs = mkOption { + type = types.ints.positive; + default = 2592000; + description = '' + Time in seconds an unused deletion or vanish request remains served + from relay-observed first_seen_at (default: 30 days). Production + periods should be at least one day; sub-day values are for tests. + ''; + }; + + unusedUnservedGatingAdditionalSecs = mkOption { + type = types.ints.positive; + default = 15552000; + description = '' + Additional time in seconds after unused serving ends that a deletion + or vanish request remains unserved but eligible to gate admission + (default: 180 days). Production periods should be at least one day; + sub-day values are for tests. + ''; + }; + + usedServedAfterLastUsedSecs = mkOption { + type = types.ints.positive; + default = 23328000; + description = '' + Normal-mode time in seconds a used deletion or vanish request + remains served after last_used_at (default: 270 days, 9 fixed + 30-day months). Used requests remain served indefinitely when + deletionRequestDisrespector is enabled. Production periods should + be at least one day; sub-day values are for tests. + ''; + }; + + usedUnservedGatingAdditionalSecs = mkOption { + type = types.ints.positive; + default = 7776000; + description = '' + Additional normal-mode time in seconds after used serving ends that + a deletion or vanish request remains unserved but continues gating + admission (default: 90 days, 3 fixed 30-day months). This duration + does not expire used requests when deletionRequestDisrespector is + enabled. Production periods should be at least one day; sub-day + values are for tests. + ''; + }; }; archiveAll = mkOption { @@ -275,19 +323,20 @@ let type = types.bool; default = false; description = '' - Ignore NIP-09 deletion requests and act as an archival server. + Ignore NIP-09 deletion requests and NIP-62 request-to-vanish events + and act as an archival server. - When enabled, incoming NIP-09 (kind 5) deletion requests are stored - but NOT acted upon: targeted events remain fully accessible. This - preserves content and prevents "left-pad" scenarios. NIP-11 - supported_nips will NOT advertise NIP-09 (deletion) or NIP-62 - (request to vanish). + When enabled, incoming NIP-09 (kind 5) deletion requests and NIP-62 + request-to-vanish events are stored but NOT acted upon: their targets + remain fully accessible. This preserves content and prevents + "left-pad" scenarios. NIP-11 supported_nips will NOT advertise NIP-09 + (deletion) or NIP-62 (request to vanish). - This ONLY affects NIP-09 user-initiated deletions. It does NOT prevent - blacklist-triggered deletions (operator moderation). + This ONLY affects NIP-09 and NIP-62 user-initiated requests. It does + NOT prevent blacklist-triggered deletions (operator moderation). - When disabled (default), deletion requests are honoured and NIP-09 and - NIP-62 are advertised in NIP-11. + When disabled (default), both request types are honoured and NIP-09 + and NIP-62 are advertised in NIP-11. See: docs/explanation/repository-lifecycle.md ''; @@ -396,6 +445,14 @@ let NGIT_HOLDING_RETENTION_SECS = toString cfg.holdingRetentionSecs; NGIT_HOLDING_CLEANUP_INTERVAL_SECS = toString cfg.holdingCleanupIntervalSecs; + NGIT_DELETION_REQUEST_RETENTION_UNUSED_SERVED_SECS = + toString cfg.deletionRequestRetention.unusedServedSecs; + NGIT_DELETION_REQUEST_RETENTION_UNUSED_UNSERVED_GATING_ADDITIONAL_SECS = + toString cfg.deletionRequestRetention.unusedUnservedGatingAdditionalSecs; + NGIT_DELETION_REQUEST_RETENTION_USED_SERVED_AFTER_LAST_USED_SECS = + toString cfg.deletionRequestRetention.usedServedAfterLastUsedSecs; + NGIT_DELETION_REQUEST_RETENTION_USED_UNSERVED_GATING_ADDITIONAL_SECS = + toString cfg.deletionRequestRetention.usedUnservedGatingAdditionalSecs; NGIT_ARCHIVE_ALL = if cfg.archiveAll then "true" else "false"; NGIT_ARCHIVE_WHITELIST = concatStringsSep "," cfg.archiveWhitelist; NGIT_ARCHIVE_GRASP_SERVICES = diff --git a/src/config.rs b/src/config.rs index e9d622b..270ca2a 100644 --- a/src/config.rs +++ b/src/config.rs @@ -6,6 +6,14 @@ use std::fs; use std::path::PathBuf; use std::time::Duration; +const DEFAULT_DELETION_REQUEST_RETENTION_UNUSED_SERVED_SECS: u64 = 30 * 24 * 60 * 60; +const DEFAULT_DELETION_REQUEST_RETENTION_UNUSED_UNSERVED_GATING_ADDITIONAL_SECS: u64 = + 180 * 24 * 60 * 60; +// Retention months are fixed 30-day periods, avoiding calendar-month ambiguity. +const DEFAULT_DELETION_REQUEST_RETENTION_USED_SERVED_AFTER_LAST_USED_SECS: u64 = 270 * 24 * 60 * 60; +const DEFAULT_DELETION_REQUEST_RETENTION_USED_UNSERVED_GATING_ADDITIONAL_SECS: u64 = + 90 * 24 * 60 * 60; + /// Whitelist entry for repository/archive filtering #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] #[serde(rename_all = "lowercase")] @@ -445,6 +453,41 @@ pub struct Config { )] pub holding_cleanup_interval_secs: u64, + // Retention is expressed in seconds for configuration consistency and short + // tests. Production deployments are expected to use periods of at least one + // day, comfortably exceeding worst-case deletion/archive processing time. + /// How long an unused deletion or vanish request remains served after relay-observed first_seen_at. + #[arg( + long = "deletion-request-retention-unused-served-secs", + env = "NGIT_DELETION_REQUEST_RETENTION_UNUSED_SERVED_SECS", + default_value_t = DEFAULT_DELETION_REQUEST_RETENTION_UNUSED_SERVED_SECS + )] + pub deletion_request_retention_unused_served_secs: u64, + + /// Additional time an unused deletion or vanish request remains unserved but eligible to gate. + #[arg( + long = "deletion-request-retention-unused-unserved-gating-additional-secs", + env = "NGIT_DELETION_REQUEST_RETENTION_UNUSED_UNSERVED_GATING_ADDITIONAL_SECS", + default_value_t = DEFAULT_DELETION_REQUEST_RETENTION_UNUSED_UNSERVED_GATING_ADDITIONAL_SECS + )] + pub deletion_request_retention_unused_unserved_gating_additional_secs: u64, + + /// Normal-mode time a used deletion or vanish request remains served after last_used_at. + #[arg( + long = "deletion-request-retention-used-served-after-last-used-secs", + env = "NGIT_DELETION_REQUEST_RETENTION_USED_SERVED_AFTER_LAST_USED_SECS", + default_value_t = DEFAULT_DELETION_REQUEST_RETENTION_USED_SERVED_AFTER_LAST_USED_SECS + )] + pub deletion_request_retention_used_served_after_last_used_secs: u64, + + /// Additional normal-mode time a used request remains unserved but continues gating after serving ends. + #[arg( + long = "deletion-request-retention-used-unserved-gating-additional-secs", + env = "NGIT_DELETION_REQUEST_RETENTION_USED_UNSERVED_GATING_ADDITIONAL_SECS", + default_value_t = DEFAULT_DELETION_REQUEST_RETENTION_USED_UNSERVED_GATING_ADDITIONAL_SECS + )] + pub deletion_request_retention_used_unserved_gating_additional_secs: u64, + /// Enable GRASP-05 archive mode: accept all announcements regardless of listing (WARNING: storage risk) #[arg(long, env = "NGIT_ARCHIVE_ALL", default_value_t = false)] pub archive_all: bool, @@ -502,19 +545,20 @@ pub struct Config { #[arg(long, env = "NGIT_EVENT_BLACKLIST", default_value = "")] pub event_blacklist: String, - /// Deletion request disrespector: ignore NIP-09 deletion requests (archival mode) + /// Deletion request disrespector: ignore NIP-09 and NIP-62 requests (archival mode) /// - /// When `true`, the relay stores incoming NIP-09 (kind 5) deletion requests but - /// does NOT act on them: targeted events remain fully accessible. This makes the - /// relay an archival server, preventing "left-pad" scenarios by ensuring at least - /// some relays preserve deleted content. + /// When `true`, the relay stores incoming NIP-09 (kind 5) deletion requests and + /// NIP-62 request-to-vanish events but does NOT act on them: their targets remain + /// fully accessible. This makes the relay an archival server, preventing + /// "left-pad" scenarios by ensuring at least some relays preserve deleted content. /// - /// This setting ONLY affects NIP-09 user-initiated deletions. Policy-driven - /// blacklist/whitelist deletion flows still run because they enforce local - /// relay serving policy rather than client deletion requests. + /// This setting ONLY affects NIP-09 and NIP-62 user-initiated requests. + /// Policy-driven blacklist/whitelist deletion flows still run because they + /// enforce local relay serving policy rather than client requests. /// - /// When `true`, NIP-09 (`"deletion"`) is NOT advertised in the NIP-11 supported - /// NIPs list so clients can discover that the relay does not honour deletions. + /// When `true`, NIP-09 (`"deletion"`) and NIP-62 (`"request to vanish"`) are NOT + /// advertised in the NIP-11 supported NIPs list so clients can discover that the + /// relay does not honour either request type. #[arg( long, env = "NGIT_DELETION_REQUEST_DISRESPECTOR", @@ -695,6 +739,34 @@ impl Config { )); } + Self::validate_deletion_request_retention_duration( + self.deletion_request_retention_unused_served_secs, + "NGIT_DELETION_REQUEST_RETENTION_UNUSED_SERVED_SECS", + )?; + Self::validate_deletion_request_retention_duration( + self.deletion_request_retention_unused_unserved_gating_additional_secs, + "NGIT_DELETION_REQUEST_RETENTION_UNUSED_UNSERVED_GATING_ADDITIONAL_SECS", + )?; + Self::validate_deletion_request_retention_duration( + self.deletion_request_retention_used_served_after_last_used_secs, + "NGIT_DELETION_REQUEST_RETENTION_USED_SERVED_AFTER_LAST_USED_SECS", + )?; + Self::validate_deletion_request_retention_duration( + self.deletion_request_retention_used_unserved_gating_additional_secs, + "NGIT_DELETION_REQUEST_RETENTION_USED_UNSERVED_GATING_ADDITIONAL_SECS", + )?; + + Self::validate_deletion_request_retention_deadline( + self.deletion_request_retention_unused_served_secs, + self.deletion_request_retention_unused_unserved_gating_additional_secs, + "unused", + )?; + Self::validate_deletion_request_retention_deadline( + self.deletion_request_retention_used_served_after_last_used_secs, + self.deletion_request_retention_used_unserved_gating_additional_secs, + "used", + )?; + // Fatal error: repository_whitelist with archive_read_only=true (incompatible) if !repository_whitelist.is_empty() { let read_only = self.archive_read_only.unwrap_or(archive_enabled); @@ -795,6 +867,46 @@ impl Config { Duration::from_secs(self.holding_cleanup_interval_secs) } + /// How long an unused request remains served from relay-observed first_seen_at. + pub fn deletion_request_retention_unused_served(&self) -> Duration { + Duration::from_secs(self.deletion_request_retention_unused_served_secs) + } + + /// Additional time an unused request remains unserved but eligible to gate. + pub fn deletion_request_retention_unused_unserved_gating_additional(&self) -> Duration { + Duration::from_secs(self.deletion_request_retention_unused_unserved_gating_additional_secs) + } + + /// Normal-mode time a used request remains served after last_used_at. + pub fn deletion_request_retention_used_served_after_last_used(&self) -> Duration { + Duration::from_secs(self.deletion_request_retention_used_served_after_last_used_secs) + } + + /// Additional normal-mode time a used request remains unserved but continues gating. + pub fn deletion_request_retention_used_unserved_gating_additional(&self) -> Duration { + Duration::from_secs(self.deletion_request_retention_used_unserved_gating_additional_secs) + } + + fn validate_deletion_request_retention_duration(value: u64, name: &str) -> Result<()> { + if value == 0 { + return Err(anyhow!("{name} must be greater than 0")); + } + Ok(()) + } + + fn validate_deletion_request_retention_deadline( + served_secs: u64, + additional_gating_secs: u64, + lifecycle: &str, + ) -> Result<()> { + if served_secs.checked_add(additional_gating_secs).is_none() { + return Err(anyhow!( + "{lifecycle} deletion-request retention served and additional gating durations must not overflow when combined" + )); + } + Ok(()) + } + /// Create config for testing #[cfg(test)] pub fn for_testing() -> Self { @@ -828,6 +940,14 @@ impl Config { holding_retention_secs: crate::nostr::lifecycle::DEFAULT_RETENTION.as_secs(), holding_cleanup_interval_secs: crate::nostr::lifecycle::DEFAULT_CLEANUP_INTERVAL .as_secs(), + deletion_request_retention_unused_served_secs: + DEFAULT_DELETION_REQUEST_RETENTION_UNUSED_SERVED_SECS, + deletion_request_retention_unused_unserved_gating_additional_secs: + DEFAULT_DELETION_REQUEST_RETENTION_UNUSED_UNSERVED_GATING_ADDITIONAL_SECS, + deletion_request_retention_used_served_after_last_used_secs: + DEFAULT_DELETION_REQUEST_RETENTION_USED_SERVED_AFTER_LAST_USED_SECS, + deletion_request_retention_used_unserved_gating_additional_secs: + DEFAULT_DELETION_REQUEST_RETENTION_USED_UNSERVED_GATING_ADDITIONAL_SECS, archive_all: false, archive_whitelist: String::new(), archive_grasp_services: String::new(), @@ -847,6 +967,9 @@ impl Config { #[cfg(test)] mod tests { use super::*; + use std::sync::Mutex; + + static CONFIG_ENV_LOCK: Mutex<()> = Mutex::new(()); #[test] fn test_default_values() { @@ -857,6 +980,135 @@ mod tests { assert_eq!(config.database_backend, DatabaseBackend::Memory); } + #[test] + fn test_deletion_request_retention_cli_defaults() { + let _environment_guard = CONFIG_ENV_LOCK.lock().expect("lock must not be poisoned"); + let config = Config::try_parse_from(["ngit-grasp", "--domain", "example.com"]) + .expect("default deletion-request retention configuration should parse"); + + assert_eq!( + config.deletion_request_retention_unused_served(), + Duration::from_secs(2_592_000) + ); + assert_eq!( + config.deletion_request_retention_unused_unserved_gating_additional(), + Duration::from_secs(15_552_000) + ); + assert_eq!( + config.deletion_request_retention_used_served_after_last_used(), + Duration::from_secs(23_328_000) + ); + assert_eq!( + config.deletion_request_retention_used_unserved_gating_additional(), + Duration::from_secs(7_776_000) + ); + } + + #[test] + fn test_deletion_request_retention_cli_overrides() { + let config = Config::try_parse_from([ + "ngit-grasp", + "--domain", + "example.com", + "--deletion-request-retention-unused-served-secs", + "1", + "--deletion-request-retention-unused-unserved-gating-additional-secs", + "2", + "--deletion-request-retention-used-served-after-last-used-secs", + "3", + "--deletion-request-retention-used-unserved-gating-additional-secs", + "4", + ]) + .expect("custom deletion-request retention configuration should parse"); + + assert_eq!(config.deletion_request_retention_unused_served_secs, 1); + assert_eq!( + config.deletion_request_retention_unused_unserved_gating_additional_secs, + 2 + ); + assert_eq!( + config.deletion_request_retention_used_served_after_last_used_secs, + 3 + ); + assert_eq!( + config.deletion_request_retention_used_unserved_gating_additional_secs, + 4 + ); + } + + #[test] + fn test_deletion_request_retention_environment_override() { + let _environment_guard = CONFIG_ENV_LOCK.lock().expect("lock must not be poisoned"); + const VARIABLE: &str = "NGIT_DELETION_REQUEST_RETENTION_UNUSED_SERVED_SECS"; + let original = std::env::var_os(VARIABLE); + std::env::set_var(VARIABLE, "42"); + + let config = Config::try_parse_from(["ngit-grasp", "--domain", "example.com"]) + .expect("environment deletion-request retention configuration should parse"); + + match original { + Some(value) => std::env::set_var(VARIABLE, value), + None => std::env::remove_var(VARIABLE), + } + + assert_eq!(config.deletion_request_retention_unused_served_secs, 42); + } + + #[test] + fn test_deletion_request_retention_rejects_zero_durations() { + let cases = [ + ( + "NGIT_DELETION_REQUEST_RETENTION_UNUSED_SERVED_SECS", + Config { + deletion_request_retention_unused_served_secs: 0, + ..Config::for_testing() + }, + ), + ( + "NGIT_DELETION_REQUEST_RETENTION_UNUSED_UNSERVED_GATING_ADDITIONAL_SECS", + Config { + deletion_request_retention_unused_unserved_gating_additional_secs: 0, + ..Config::for_testing() + }, + ), + ( + "NGIT_DELETION_REQUEST_RETENTION_USED_SERVED_AFTER_LAST_USED_SECS", + Config { + deletion_request_retention_used_served_after_last_used_secs: 0, + ..Config::for_testing() + }, + ), + ( + "NGIT_DELETION_REQUEST_RETENTION_USED_UNSERVED_GATING_ADDITIONAL_SECS", + Config { + deletion_request_retention_used_unserved_gating_additional_secs: 0, + ..Config::for_testing() + }, + ), + ]; + + for (variable, config) in cases { + let error = config.validate().expect_err("zero duration must fail"); + assert!(error.to_string().contains(variable)); + } + } + + #[test] + fn test_deletion_request_retention_rejects_deadline_overflow() { + let config = Config { + deletion_request_retention_unused_served_secs: u64::MAX, + deletion_request_retention_unused_unserved_gating_additional_secs: 1, + ..Config::for_testing() + }; + + let error = config + .validate() + .expect_err("combined duration overflow must fail"); + assert!(error + .to_string() + .contains("unused deletion-request retention")); + } + #[test] fn test_lmdb_is_default() { // Verify the actual default via the enum's Default trait diff --git a/src/lib.rs b/src/lib.rs index 4797d2f..99aae3c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -7,6 +7,5 @@ pub mod http; pub mod metrics; pub mod nostr; pub mod purgatory; -pub mod repair_deletion_requests; pub mod server; pub mod sync; diff --git a/src/main.rs b/src/main.rs index 8b26f8f..ba8134c 100644 --- a/src/main.rs +++ b/src/main.rs @@ -4,9 +4,7 @@ use tokio::signal; use tracing::info; use tracing_subscriber::{EnvFilter, FmtSubscriber}; -use ngit_grasp::{ - cleanup_empty_repos, config::Config, nostr, repair_deletion_requests, server::RelayServer, -}; +use ngit_grasp::{cleanup_empty_repos, config::Config, nostr, server::RelayServer}; /// Top-level CLI dispatcher. /// @@ -30,10 +28,6 @@ enum Cli { /// /// This is an operator/admin maintenance command and is idempotent. HoldingEject(nostr::lifecycle::HoldingEjectArgs), - - /// Temporarily repair historical redundant kind-5 deletion request rows. - #[command(hide = true)] - RepairDeletionRequests(repair_deletion_requests::RepairDeletionRequestsArgs), } #[tokio::main] @@ -45,13 +39,7 @@ async fn main() -> Result<()> { // If not, prepend the implicit "serve" subcommand so that clap routes to Cli::Serve // and all relay flags are parsed normally (preserving backward compatibility). let mut args: Vec = std::env::args().collect(); - let known_subcommands = [ - "serve", - "cleanup-empty-repos", - "holding-eject", - "repair-deletion-requests", - "help", - ]; + let known_subcommands = ["serve", "cleanup-empty-repos", "holding-eject", "help"]; let has_subcommand = args.get(1).is_some_and(|a| { known_subcommands.contains(&a.as_str()) || matches!(a.as_str(), "-h" | "--help" | "-V" | "--version") @@ -63,9 +51,6 @@ async fn main() -> Result<()> { match Cli::parse_from(args) { Cli::CleanupEmptyRepos(cleanup_args) => cleanup_empty_repos::run(&cleanup_args).await, Cli::HoldingEject(eject_args) => nostr::lifecycle::run_holding_eject(eject_args).await, - Cli::RepairDeletionRequests(repair_args) => { - repair_deletion_requests::run(&repair_args).await - } Cli::Serve(config) => { let mut config = *config; // Finish initialising the Config (load relay owner key if not provided). diff --git a/src/metrics/mod.rs b/src/metrics/mod.rs index 16df4bc..578b041 100644 --- a/src/metrics/mod.rs +++ b/src/metrics/mod.rs @@ -113,6 +113,30 @@ lazy_static! { .expect("register holding cleanup last-run metric"); metric }; + static ref DELETION_REQUEST_CLEANUP_RUNS_TOTAL: Counter = { + let metric = Counter::with_opts(Opts::new( + "ngit_deletion_request_cleanup_runs_total", + "Number of deletion-request cleanup passes run", + )).expect("build deletion request cleanup runs metric"); + REGISTRY.register(Box::new(metric.clone())).expect("register deletion request cleanup runs metric"); + metric + }; + static ref DELETION_REQUEST_CLEANUP_REMOVED_TOTAL: CounterVec = { + let metric = CounterVec::new(Opts::new( + "ngit_deletion_request_cleanup_removed_total", + "Deletion-request cleanup removals by storage type", + ), &["type"]).expect("build deletion request cleanup removed metric"); + REGISTRY.register(Box::new(metric.clone())).expect("register deletion request cleanup removed metric"); + metric + }; + static ref DELETION_REQUEST_CLEANUP_OUTCOMES_TOTAL: CounterVec = { + let metric = CounterVec::new(Opts::new( + "ngit_deletion_request_cleanup_outcomes_total", + "Deletion-request cleanup failures and concurrent skips", + ), &["outcome"]).expect("build deletion request cleanup outcomes metric"); + REGISTRY.register(Box::new(metric.clone())).expect("register deletion request cleanup outcomes metric"); + metric + }; static ref RECOVERY_TOTAL: CounterVec = { let metric = CounterVec::new( Opts::new( @@ -247,6 +271,30 @@ pub fn record_holding_cleanup_run( .set(archive_files_deleted as f64); } +pub fn record_deletion_request_cleanup_run( + main_removed: usize, + tombstone_removed: usize, + metadata_removed: usize, + failures: usize, + skipped: usize, +) { + DELETION_REQUEST_CLEANUP_RUNS_TOTAL.inc(); + for (kind, value) in [ + ("main", main_removed), + ("tombstone", tombstone_removed), + ("metadata", metadata_removed), + ] { + DELETION_REQUEST_CLEANUP_REMOVED_TOTAL + .with_label_values(&[kind]) + .inc_by(value as f64); + } + for (outcome, value) in [("failure", failures), ("stale_or_concurrent_skip", skipped)] { + DELETION_REQUEST_CLEANUP_OUTCOMES_TOTAL + .with_label_values(&[outcome]) + .inc_by(value as f64); + } +} + pub fn record_recovery_attempt() { RECOVERY_TOTAL.with_label_values(&["attempted"]).inc(); } diff --git a/src/nostr/builder.rs b/src/nostr/builder.rs index 5b0bc40..04f6ff0 100644 --- a/src/nostr/builder.rs +++ b/src/nostr/builder.rs @@ -768,12 +768,6 @@ impl WritePolicy for Nip34WritePolicy { _ => self.handle_related_event(event, "Event").await, }; - if !matches!(&result, WritePolicyResult::Reject { .. }) { - self.deletion - .remove_stale_cross_author_event_deletions(event) - .await; - } - result }) } diff --git a/src/nostr/lifecycle/deletion/archival.rs b/src/nostr/lifecycle/deletion/archival.rs index 349ced1..6952791 100644 --- a/src/nostr/lifecycle/deletion/archival.rs +++ b/src/nostr/lifecycle/deletion/archival.rs @@ -9,7 +9,10 @@ use nostr_relay_builder::prelude::{ }; use tar::Builder as TarBuilder; -use super::policy::{identifier_from_event, owner_directory_component, DeletionPolicy}; +use super::{ + policy::{identifier_from_event, owner_directory_component, DeletionPolicy}, + DeletionOutcome, +}; use crate::nostr::lifecycle::{DeletionSource, GitArchiveMetadata, HoldingMetadata}; #[derive(Debug, Clone, PartialEq, Eq, Hash)] @@ -20,6 +23,50 @@ struct DeletedAnnouncementRepoScope { } impl DeletionPolicy { + /// Read-only normal-policy evaluation for a targeting NIP-62 request in + /// disrespector mode. This deliberately does not invoke archive, cascade, + /// holding, rollback, repository, or purgatory mutation helpers. + pub(super) async fn would_nip62_vanish_stored_data( + &self, + event: &Event, + ) -> anyhow::Result { + // The request has already been recorded and may be persisted before an + // exact replay reaches this handler. It is not evidence that the + // request had an effect; only a distinct event by this author is. + if self + .ctx + .database + .query(Filter::new().author(event.pubkey)) + .await? + .into_iter() + .any(|stored| stored.id != event.id) + { + return Ok(true); + } + + let author = event.pubkey; + if self + .ctx + .purgatory + .announcements_for_sync() + .into_iter() + .any(|(repository_id, _)| { + let parts: Vec<_> = repository_id.splitn(3, ':').collect(); + parts.len() == 3 && parts[1] == author.to_hex() + }) + { + return Ok(true); + } + + Ok(self + .ctx + .purgatory + .get_all_identifiers() + .into_iter() + .flat_map(|identifier| self.ctx.purgatory.find_state(&identifier)) + .any(|entry| entry.author == author)) + } + /// Move all currently served data authored by a targeted NIP-62 vanish /// request through the same holding/archive deletion lifecycle used for /// NIP-09 announcement deletion. @@ -29,7 +76,11 @@ impl DeletionPolicy { /// repository archive linkage. Any remaining author events are then moved to /// holding before main-DB deletion. The caller is responsible for recording /// the vanish tombstone before invoking this method. - pub(super) async fn apply_nip62_vanish(&self, event: &Event) -> anyhow::Result<()> { + pub(super) async fn apply_nip62_vanish( + &self, + event: &Event, + ) -> anyhow::Result { + let mut outcome = DeletionOutcome::default(); let author = event.pubkey; let deleted_at = event.created_at; @@ -64,13 +115,15 @@ impl DeletionPolicy { continue; }; let coordinate = format!("30617:{}:{}", author.to_hex(), identifier); - self.cascade_delete_announcement( - &author, - &coordinate, - deleted_at, - DeletionSource::Nip62, - ) - .await; + outcome.merge( + self.cascade_delete_announcement( + &author, + &coordinate, + deleted_at, + DeletionSource::Nip62, + ) + .await, + ); } let mut moved_ids = HashSet::new(); @@ -83,15 +136,34 @@ impl DeletionPolicy { git_archive: None, }; - self.archive_and_delete_filter( - Filter::new().author(author), - &metadata, - &mut moved_ids, - "NIP-62 vanish remaining author-event deletion", - ) - .await; + // Exclude the incoming request itself. On an exact replay it may + // already be stored in Main; deleting it would falsely turn a no-op + // request into a used request and extend its retention lifecycle. + let remaining_ids = self + .ctx + .database + .query(Filter::new().author(author)) + .await + .map_err(|e| { + anyhow::anyhow!("Failed to query vanished author's remaining events: {e}") + })? + .into_iter() + .filter(|stored| stored.id != event.id) + .map(|stored| stored.id) + .collect::>(); + if !remaining_ids.is_empty() { + outcome.merge( + self.archive_and_delete_filter( + Filter::new().ids(remaining_ids), + &metadata, + &mut moved_ids, + "NIP-62 vanish remaining author-event deletion", + ) + .await, + ); + } - Ok(()) + Ok(outcome) } /// Hard-delete the targeted events from the main database. @@ -103,7 +175,8 @@ impl DeletionPolicy { &self, event: &Event, moved_ids: &mut HashSet, - ) { + ) -> DeletionOutcome { + let mut outcome = DeletionOutcome::default(); // `e` tags: delete by event id. let ids = Self::e_tag_ids(event); if !ids.is_empty() { @@ -116,8 +189,15 @@ impl DeletionPolicy { owner_pubkey: None, git_archive: None, }; - self.archive_and_delete_filter(filter, &metadata, moved_ids, "NIP-09 e-tag deletion") - .await; + outcome.merge( + self.archive_and_delete_filter( + filter, + &metadata, + moved_ids, + "NIP-09 e-tag deletion", + ) + .await, + ); } // `a` tags: delete matching addressable/replaceable events up to the @@ -134,24 +214,29 @@ impl DeletionPolicy { continue; } if Self::is_announcement_coordinate_for_author(&v[1], &event.pubkey) { - self.cascade_delete_announcement( - &event.pubkey, - &v[1], - event.created_at, - DeletionSource::Nip09, - ) - .await; + outcome.merge( + self.cascade_delete_announcement( + &event.pubkey, + &v[1], + event.created_at, + DeletionSource::Nip09, + ) + .await, + ); } else { - self.delete_coordinate_from_main_db( - &event.pubkey, - &v[1], - event.created_at, - moved_ids, - DeletionSource::Nip09, - ) - .await; + outcome.merge( + self.delete_coordinate_from_main_db( + &event.pubkey, + &v[1], + event.created_at, + moved_ids, + DeletionSource::Nip09, + ) + .await, + ); } } + outcome } /// Whether `coordinate` is a kind-30617 announcement coordinate whose @@ -173,21 +258,21 @@ impl DeletionPolicy { deletion_created_at: nostr_relay_builder::prelude::Timestamp, moved_ids: &mut HashSet, source: DeletionSource, - ) { + ) -> DeletionOutcome { // coordinate: `::` let parts: Vec<&str> = coordinate.splitn(3, ':').collect(); if parts.len() != 3 { - return; + return DeletionOutcome::default(); } let Ok(kind_num) = parts[0].parse::() else { - return; + return DeletionOutcome::default(); }; let coord_pubkey_hex = parts[1]; let identifier = parts[2]; // The coordinate pubkey must match the deletion author. if coord_pubkey_hex != author.to_hex() { - return; + return DeletionOutcome::default(); } let kind = Kind::from(kind_num); @@ -234,11 +319,14 @@ impl DeletionPolicy { op = "coordinate deletion", "Skipping announcement deletion because git archival failed (fail-safe preservation)" ); - return; + return DeletionOutcome { + skipped: 1, + ..Default::default() + }; } self.archive_and_delete_filter(filter, &metadata, moved_ids, "coordinate deletion") - .await; + .await } pub(super) async fn archive_and_delete_filter( @@ -247,20 +335,24 @@ impl DeletionPolicy { metadata: &HoldingMetadata, moved_ids: &mut HashSet, context: &str, - ) { + ) -> DeletionOutcome { let matches = match self.ctx.database.query(filter).await { Ok(events) => events, Err(e) => { tracing::warn!(error = %e, op = %context, "Failed to query deletion targets"); - return; + return DeletionOutcome { + failures: 1, + ..Default::default() + }; } }; if matches.is_empty() { - return; + return DeletionOutcome::default(); } - let mut deletable = Vec::with_capacity(matches.len()); + let mut deletable = HashSet::with_capacity(matches.len()); + let mut outcome = DeletionOutcome::default(); let mut deleted_announcement_repo_scopes = HashSet::new(); for event in matches { let mut event_metadata = metadata.clone(); @@ -309,6 +401,7 @@ impl DeletionPolicy { op = %context, "Skipping announcement deletion because git archival failed (fail-safe preservation)" ); + outcome.skipped = outcome.skipped.saturating_add(1); continue; } @@ -322,7 +415,7 @@ impl DeletionPolicy { } } - if moved_ids.insert(event.id) { + if !moved_ids.contains(&event.id) { if let Err(e) = self .ctx .holding @@ -335,8 +428,10 @@ impl DeletionPolicy { op = %context, "Skipping main-DB deletion because holding archival failed" ); + outcome.skipped = outcome.skipped.saturating_add(1); continue; } + moved_ids.insert(event.id); if matches!( event.kind, @@ -345,23 +440,34 @@ impl DeletionPolicy { self.delete_pr_event_git_refs(&event).await; } } - deletable.push(event.id); + deletable.insert(event.id); } if deletable.is_empty() { - return; + return outcome; } + // NostrDatabase::delete does not return a per-event removed count. Two + // concurrently processed requests can therefore both have observed one + // target and both receive use credit after successful delete calls. This + // narrow over-credit race is accepted: avoiding it would require broad + // target-level serialization, it cannot affect deletion correctness, + // and any extra credited request still follows bounded used retention. + // Deterministic single-winner attribution remains strict for the more + // important case of one later admission matching many pending requests. + let deleted_count = deletable.len(); let delete_filter = Filter::new().ids(deletable); if let Err(e) = self.ctx.database.delete(delete_filter).await { tracing::warn!(error = %e, op = %context, "Failed to delete events from main DB"); - return; + outcome.failures = outcome.failures.saturating_add(1); + return outcome; } - for scope in deleted_announcement_repo_scopes { self.remove_live_repo_if_announcement_scope_unserved(scope, context) .await; } + outcome.main_db_deleted = outcome.main_db_deleted.saturating_add(deleted_count); + outcome } async fn remove_live_repo_if_announcement_scope_unserved( diff --git a/src/nostr/lifecycle/deletion/cascade.rs b/src/nostr/lifecycle/deletion/cascade.rs index 01bc2a8..5990251 100644 --- a/src/nostr/lifecycle/deletion/cascade.rs +++ b/src/nostr/lifecycle/deletion/cascade.rs @@ -10,6 +10,7 @@ use super::policy::{ use crate::nostr::lifecycle::{DeletionSource, HoldingMetadata, HOLDING_METADATA_KIND}; use crate::nostr::SharedDatabase; +use super::DeletionOutcome; use crate::nostr::lifecycle::history::HISTORY_METADATA_KIND; const MAX_CASCADE_CANDIDATE_EVENTS: usize = 50_000; @@ -121,14 +122,15 @@ impl DeletionPolicy { announcement_addr: &str, deletion_created_at: Timestamp, source: DeletionSource, - ) { + ) -> DeletionOutcome { + let mut outcome = DeletionOutcome::default(); let mut moved_ids = HashSet::new(); // Parse `30617::` (already validated as a 30617 // coordinate for this author, but re-parse defensively). let parts: Vec<&str> = announcement_addr.splitn(3, ':').collect(); if parts.len() != 3 { - return; + return outcome; } let identifier = parts[2]; @@ -142,7 +144,10 @@ impl DeletionPolicy { .collect::>(), Err(e) => { tracing::warn!(error = %e, announcement = %announcement_addr, "Cascade deletion: failed to verify announcement versions under deletion cutoff"); - return; + return DeletionOutcome { + failures: 1, + ..Default::default() + }; } }; @@ -152,7 +157,7 @@ impl DeletionPolicy { deletion_created_at = deletion_created_at.as_secs(), "Cascade deletion skipped: no announcement version is deletable under NIP-09 cutoff" ); - return; + return outcome; } let plan = match plan_deleted_announcement_cascade( @@ -165,16 +170,16 @@ impl DeletionPolicy { Ok(plan) => plan, Err(e) => { tracing::warn!(error = %e, "Cascade deletion: graph expansion failed; falling back to simple coordinate deletion"); - self.delete_announcement_coordinate_and_maybe_state( - author, - announcement_addr, - identifier, - deletion_created_at, - &mut moved_ids, - source, - ) - .await; - return; + return self + .delete_announcement_coordinate_and_maybe_state( + author, + announcement_addr, + identifier, + deletion_created_at, + &mut moved_ids, + source, + ) + .await; } }; @@ -196,16 +201,16 @@ impl DeletionPolicy { max = MAX_CASCADE_ORPHAN_DELETES, "Cascade deletion orphan set exceeds limit; deleting announcement coordinate only" ); - self.delete_announcement_coordinate_and_maybe_state( - author, - announcement_addr, - identifier, - deletion_created_at, - &mut moved_ids, - source, - ) - .await; - return; + return self + .delete_announcement_coordinate_and_maybe_state( + author, + announcement_addr, + identifier, + deletion_created_at, + &mut moved_ids, + source, + ) + .await; } if !orphan_ids.is_empty() { tracing::debug!( @@ -227,27 +232,32 @@ impl DeletionPolicy { ) .await, }; - self.archive_and_delete_filter( - filter, - &metadata, - &mut moved_ids, - "cascade orphan deletion", - ) - .await; + outcome.merge( + self.archive_and_delete_filter( + filter, + &metadata, + &mut moved_ids, + "cascade orphan deletion", + ) + .await, + ); } // Step 5b: hard-delete the announcement coordinate itself (up to the // deletion's created_at, per NIP-09), exactly as the simple path does, // then clean state if this identifier is now unanchored. - self.delete_announcement_coordinate_and_maybe_state( - author, - announcement_addr, - identifier, - deletion_created_at, - &mut moved_ids, - source, - ) - .await; + outcome.merge( + self.delete_announcement_coordinate_and_maybe_state( + author, + announcement_addr, + identifier, + deletion_created_at, + &mut moved_ids, + source, + ) + .await, + ); + outcome } /// Apply operator-driven blacklist deletion for a stored kind-30617 @@ -275,13 +285,14 @@ impl DeletionPolicy { }; let coordinate = format!("30617:{}:{}", announcement.pubkey.to_hex(), identifier); - self.cascade_delete_announcement( - &announcement.pubkey, - &coordinate, - Timestamp::now(), - DeletionSource::Blacklist, - ) - .await; + let _ = self + .cascade_delete_announcement( + &announcement.pubkey, + &coordinate, + Timestamp::now(), + DeletionSource::Blacklist, + ) + .await; Ok(()) } @@ -332,7 +343,7 @@ impl DeletionPolicy { deletion_created_at: Timestamp, moved_ids: &mut HashSet, source: DeletionSource, - ) { + ) -> DeletionOutcome { let announcement_filter = Filter::new().kind(Kind::GitRepoAnnouncement).custom_tag( SingleLetterTag::lowercase(Alphabet::D), identifier.to_string(), @@ -346,12 +357,15 @@ impl DeletionPolicy { identifier = %identifier, "Cascade deletion: failed to check remaining announcements for identifier" ); - return; + return DeletionOutcome { + failures: 1, + ..Default::default() + }; } }; if !remaining_announcements.is_empty() { - return; + return DeletionOutcome::default(); } let state_filter = Filter::new().kind(Kind::RepoState).custom_tag( @@ -373,7 +387,7 @@ impl DeletionPolicy { moved_ids, "unanchored state deletion", ) - .await; + .await } async fn delete_announcement_coordinate_and_maybe_state( @@ -384,27 +398,31 @@ impl DeletionPolicy { deletion_created_at: Timestamp, moved_ids: &mut HashSet, source: DeletionSource, - ) { - self.delete_coordinate_from_main_db( - author, - announcement_addr, - deletion_created_at, - moved_ids, - source, - ) - .await; + ) -> DeletionOutcome { + let mut outcome = self + .delete_coordinate_from_main_db( + author, + announcement_addr, + deletion_created_at, + moved_ids, + source, + ) + .await; // 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. This must also run on coordinate-only fallbacks. - self.delete_repo_state_if_identifier_is_unanchored( - identifier, - deletion_created_at, - moved_ids, - source, - ) - .await; + outcome.merge( + self.delete_repo_state_if_identifier_is_unanchored( + identifier, + deletion_created_at, + moved_ids, + source, + ) + .await, + ); + outcome } async fn query_address_events_until( diff --git a/src/nostr/lifecycle/deletion/cleanup.rs b/src/nostr/lifecycle/deletion/cleanup.rs new file mode 100644 index 0000000..73942f9 --- /dev/null +++ b/src/nostr/lifecycle/deletion/cleanup.rs @@ -0,0 +1,478 @@ +use anyhow::Result; +use nostr_relay_builder::prelude::{Filter, Kind, Timestamp}; + +use crate::nostr::lifecycle::RequestLifecycleRecord; + +use super::{service::request_is_served_with_config, DeletionService}; + +/// Result counters for one bounded deletion-request retention pass. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub struct RequestCleanupStats { + pub canonical_records_examined: usize, + pub main_payloads_removed: usize, + pub tombstone_payloads_removed: usize, + pub metadata_rows_removed: usize, + pub indefinitely_retained_disrespector_requests: usize, + pub stale_or_concurrent_records_skipped: usize, + pub failures: usize, +} + +impl DeletionService { + /// Reconcile served copies and permanently expire elapsed request lifecycles. + /// + /// Deadlines determine lifecycle eligibility, while physical Main and + /// Tombstone removal occurs when this cleanup runs. A request can therefore + /// remain queryable until the next pass without remaining gate-eligible. + /// The supplied time makes every retention boundary deterministic in tests. + pub(crate) async fn cleanup_expired_requests( + &self, + now: Timestamp, + ) -> Result { + let records = self.ctx.tombstones().lifecycle_records_result().await?; + let mut stats = RequestCleanupStats { + canonical_records_examined: records.len(), + ..Default::default() + }; + + for selected in records { + if let Err(error) = self.cleanup_one_request(&selected, now, &mut stats).await { + stats.failures += 1; + tracing::warn!(request_id = %selected.request.id, error = %error, "Deletion-request cleanup failed for record"); + } + } + + match self.ctx.tombstones().remove_orphan_metadata().await { + Ok(removed) => stats.metadata_rows_removed += removed, + Err(error) => { + stats.failures += 1; + tracing::warn!(error = %error, "Deletion-request cleanup failed to remove orphan metadata"); + } + } + Ok(stats) + } + + async fn cleanup_one_request( + &self, + selected: &RequestLifecycleRecord, + now: Timestamp, + stats: &mut RequestCleanupStats, + ) -> Result<()> { + let tombstones = self.ctx.tombstones(); + let _guard = tombstones.lock_lifecycle().await; + let Some(current) = tombstones + .lifecycle_for_request_result(&selected.request.id) + .await? + else { + stats.stale_or_concurrent_records_skipped += 1; + return Ok(()); + }; + // A mark-used/reclassification replacement between enumeration and this + // lock acquisition means this pass must not act on its stale decision. + if current != *selected { + stats.stale_or_concurrent_records_skipped += 1; + return Ok(()); + } + + let targeting = current.request.kind == Kind::EventDeletion + || self.vanish_targets_this_relay(¤t.request); + if self.ctx.config.deletion_request_disrespector + && targeting + && current.last_used_at.is_some() + { + stats.indefinitely_retained_disrespector_requests += 1; + return Ok(()); + } + + if self.request_is_served(¤t, now)? { + return Ok(()); + } + + // Main must be removed before Tombstones. Holding the same lifecycle + // lock as gate attribution prevents stale cleanup from unserving a + // request that an admission just promoted and marked used. + if self + .ctx + .database() + .event_by_id(¤t.request.id) + .await? + .is_some() + { + self.ctx + .database() + .delete(Filter::new().ids(vec![current.request.id])) + .await?; + stats.main_payloads_removed += 1; + } + + if !self.request_is_expired(¤t, now)? { + return Ok(()); + } + let removed = tombstones + .permanently_delete_request_locked(¤t.request.id) + .await?; + stats.tombstone_payloads_removed += removed.payloads_deleted; + stats.metadata_rows_removed += removed.metadata_deleted; + Ok(()) + } + + pub(crate) fn request_is_served( + &self, + record: &RequestLifecycleRecord, + now: Timestamp, + ) -> Result { + request_is_served_with_config(&self.ctx, record, now) + } +} + +#[cfg(test)] +mod tests { + use std::path::PathBuf; + use std::sync::Arc; + + use nostr_relay_builder::prelude::{Event, EventBuilder, EventId, FinalizeEvent, Keys, Tag}; + + use super::*; + use crate::grasp06::receive::new_repo_init_locks; + use crate::nostr::lifecycle::{ + HoldingStore, ReplaceableHistoryStore, RepositoryLifecycle, RequestClassification, + Tombstones, + }; + use crate::purgatory::Purgatory; + + fn service(disrespector: bool) -> DeletionService { + let db = Arc::new(nostr_memory::MemoryDatabase::unbounded()); + let config = crate::config::Config { + deletion_request_disrespector: disrespector, + deletion_request_retention_unused_served_secs: 10, + deletion_request_retention_unused_unserved_gating_additional_secs: 5, + deletion_request_retention_used_served_after_last_used_secs: 20, + deletion_request_retention_used_unserved_gating_additional_secs: 5, + ..crate::config::Config::for_testing() + }; + DeletionService::new(super::super::DeletionContext::new( + "test.example.com", + db, + Tombstones::in_memory(), + HoldingStore::in_memory(), + RepositoryLifecycle::in_memory(), + ReplaceableHistoryStore::in_memory(), + PathBuf::new(), + Arc::new(Purgatory::new(PathBuf::new())), + config, + new_repo_init_locks(), + )) + } + + fn deletion() -> Event { + EventBuilder::new(Kind::EventDeletion, "") + .tags(vec![Tag::event(EventId::all_zeros())]) + .finalize(&Keys::generate()) + .unwrap() + } + + fn non_targeting_vanish() -> Event { + EventBuilder::new(Kind::RequestToVanish, "") + .tags(vec![Tag::custom( + "relay", + vec!["wss://elsewhere.example".to_owned()], + )]) + .finalize(&Keys::generate()) + .unwrap() + } + + async fn retained(service: &DeletionService, request: &Event, first_seen: u64) { + service + .ctx + .tombstones() + .record_request( + request, + Timestamp::from_secs(first_seen), + RequestClassification::LocallyActionable, + ) + .await + .unwrap(); + service.ctx.database().save_event(request).await.unwrap(); + } + + async fn main_has(service: &DeletionService, request: &Event) -> bool { + service + .ctx + .database() + .event_by_id(&request.id) + .await + .unwrap() + .is_some() + } + + #[tokio::test] + async fn unused_boundaries_keep_gate_then_permanently_expire() { + let service = service(false); + let request = deletion(); + retained(&service, &request, 0).await; + assert_eq!( + service + .cleanup_expired_requests(Timestamp::from_secs(9)) + .await + .unwrap() + .main_payloads_removed, + 0 + ); + assert!(main_has(&service, &request).await); + + let stats = service + .cleanup_expired_requests(Timestamp::from_secs(10)) + .await + .unwrap(); + assert_eq!(stats.main_payloads_removed, 1); + assert!(!main_has(&service, &request).await); + assert!(service + .ctx + .tombstones() + .lifecycle_for_request_result(&request.id) + .await + .unwrap() + .is_some()); + assert_eq!( + service + .ctx + .tombstones() + .event_deletion_candidates(&EventId::all_zeros(), &request.pubkey) + .await + .unwrap() + .len(), + 1 + ); + assert_eq!( + service + .ctx + .tombstones() + .lifecycle_records_result() + .await + .unwrap() + .len(), + 1 + ); + + let stats = service + .cleanup_expired_requests(Timestamp::from_secs(15)) + .await + .unwrap(); + assert_eq!(stats.tombstone_payloads_removed, 1); + assert!(service + .ctx + .tombstones() + .lifecycle_for_request_result(&request.id) + .await + .unwrap() + .is_none()); + } + + #[tokio::test] + async fn used_boundaries_and_reuse_reset_the_lifecycle() { + let service = service(false); + let request = deletion(); + retained(&service, &request, 0).await; + service + .ctx + .tombstones() + .mark_request_used(&request.id, Timestamp::from_secs(20)) + .await + .unwrap(); + assert!( + service + .cleanup_expired_requests(Timestamp::from_secs(39)) + .await + .unwrap() + .main_payloads_removed + == 0 + ); + service + .ctx + .tombstones() + .mark_request_used(&request.id, Timestamp::from_secs(39)) + .await + .unwrap(); + assert!( + service + .cleanup_expired_requests(Timestamp::from_secs(40)) + .await + .unwrap() + .main_payloads_removed + == 0 + ); + let stats = service + .cleanup_expired_requests(Timestamp::from_secs(59)) + .await + .unwrap(); + assert_eq!(stats.main_payloads_removed, 1); + assert!(service + .ctx + .tombstones() + .lifecycle_for_request_result(&request.id) + .await + .unwrap() + .is_some()); + let stats = service + .cleanup_expired_requests(Timestamp::from_secs(64)) + .await + .unwrap(); + assert_eq!(stats.tombstone_payloads_removed, 1); + } + + #[tokio::test] + async fn disrespector_only_retains_used_targeting_requests_indefinitely() { + let service = service(true); + let used = deletion(); + retained(&service, &used, 0).await; + service + .ctx + .tombstones() + .mark_request_used(&used.id, Timestamp::from_secs(1)) + .await + .unwrap(); + let stats = service + .cleanup_expired_requests(Timestamp::from_secs(10_000)) + .await + .unwrap(); + assert_eq!(stats.indefinitely_retained_disrespector_requests, 1); + assert!(main_has(&service, &used).await); + + let unused = deletion(); + retained(&service, &unused, 0).await; + service + .cleanup_expired_requests(Timestamp::from_secs(15)) + .await + .unwrap(); + assert!(service + .ctx + .tombstones() + .lifecycle_for_request_result(&unused.id) + .await + .unwrap() + .is_none()); + } + + #[tokio::test] + async fn non_targeting_nip62_expires_and_cleanup_is_idempotent() { + let service = service(false); + let request = non_targeting_vanish(); + retained(&service, &request, 0).await; + assert!( + service + .cleanup_expired_requests(Timestamp::from_secs(15)) + .await + .unwrap() + .tombstone_payloads_removed + == 1 + ); + let rerun = service + .cleanup_expired_requests(Timestamp::from_secs(15)) + .await + .unwrap(); + assert_eq!( + rerun.main_payloads_removed + + rerun.tombstone_payloads_removed + + rerun.metadata_rows_removed, + 0 + ); + } + + #[tokio::test] + async fn stale_cleanup_snapshot_cannot_unserve_a_marked_used_request() { + let service = service(false); + let request = deletion(); + retained(&service, &request, 0).await; + let selected = service + .ctx + .tombstones() + .lifecycle_for_request_result(&request.id) + .await + .unwrap() + .unwrap(); + // This models admission's metadata update and Main promotion happening + // after enumeration but before cleanup acquires the shared lock. + service + .ctx + .tombstones() + .mark_request_used(&request.id, Timestamp::from_secs(14)) + .await + .unwrap(); + let mut stats = RequestCleanupStats::default(); + service + .cleanup_one_request(&selected, Timestamp::from_secs(15), &mut stats) + .await + .unwrap(); + assert_eq!(stats.stale_or_concurrent_records_skipped, 1); + assert!(main_has(&service, &request).await); + } + + #[tokio::test] + async fn cleanup_repairs_orphan_metadata_idempotently_after_partial_removal() { + let service = service(false); + let request = deletion(); + service + .ctx + .tombstones() + .record_request( + &request, + Timestamp::from_secs(0), + RequestClassification::LocallyActionable, + ) + .await + .unwrap(); + service + .ctx + .tombstones() + .remove_request_payload_without_metadata(request.id) + .await + .unwrap(); + + let first = service + .cleanup_expired_requests(Timestamp::from_secs(15)) + .await + .unwrap(); + assert_eq!(first.canonical_records_examined, 0); + assert_eq!(first.metadata_rows_removed, 1); + let rerun = service + .cleanup_expired_requests(Timestamp::from_secs(15)) + .await + .unwrap(); + assert_eq!(rerun.metadata_rows_removed, 0); + assert_eq!(rerun.failures, 0); + } + + #[tokio::test] + async fn startup_reconciliation_and_periodic_cleanup_share_serving_boundary() { + let startup = service(false); + let periodic = service(false); + let startup_request = deletion(); + let periodic_request = deletion(); + retained(&startup, &startup_request, 0).await; + retained(&periodic, &periodic_request, 0).await; + + startup + .run_request_lifecycle_startup_reconciliation(Timestamp::from_secs(10)) + .await + .unwrap(); + periodic + .cleanup_expired_requests(Timestamp::from_secs(10)) + .await + .unwrap(); + assert!(!main_has(&startup, &startup_request).await); + assert!(!main_has(&periodic, &periodic_request).await); + assert!(startup + .ctx + .tombstones() + .lifecycle_for_request_result(&startup_request.id) + .await + .unwrap() + .is_some()); + assert!(periodic + .ctx + .tombstones() + .lifecycle_for_request_result(&periodic_request.id) + .await + .unwrap() + .is_some()); + } +} diff --git a/src/nostr/lifecycle/deletion/mod.rs b/src/nostr/lifecycle/deletion/mod.rs index 7242de4..7d6076d 100644 --- a/src/nostr/lifecycle/deletion/mod.rs +++ b/src/nostr/lifecycle/deletion/mod.rs @@ -1,5 +1,6 @@ mod archival; mod cascade; +mod cleanup; mod context; mod policy; mod pr_refs; @@ -10,11 +11,74 @@ mod runtime; mod service; mod startup; +/// Measured result of destructive deletion work. +/// +/// Only the two successful removal counters make a deletion request "used". +/// `skipped` and `failures` are diagnostic counters for fail-safe preservation +/// and unsuccessful database/filesystem operations respectively. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub(crate) struct DeletionOutcome { + pub main_db_deleted: usize, + pub purgatory_removed: usize, + pub skipped: usize, + pub failures: usize, +} + +impl DeletionOutcome { + pub(crate) fn removed_events(self) -> usize { + self.main_db_deleted.saturating_add(self.purgatory_removed) + } + + pub(crate) fn used(self) -> bool { + self.removed_events() > 0 + } + + pub(crate) fn merge(&mut self, other: Self) { + self.main_db_deleted = self.main_db_deleted.saturating_add(other.main_db_deleted); + self.purgatory_removed = self + .purgatory_removed + .saturating_add(other.purgatory_removed); + self.skipped = self.skipped.saturating_add(other.skipped); + self.failures = self.failures.saturating_add(other.failures); + } +} + +pub use cleanup::RequestCleanupStats; pub use context::DeletionContext; pub use policy::DeletionPolicy; pub use runtime::{run_holding_eject, DeletionCleanupTask, DeletionRuntime, HoldingEjectArgs}; pub use service::DeletionService; pub use startup::{ - BlacklistParityStats, BlacklistRestoreStats, StartupReconciliationStats, WhitelistParityStats, - WhitelistRestoreStats, + BlacklistParityStats, BlacklistRestoreStats, RequestLifecycleStartupStats, + StartupReconciliationStats, WhitelistParityStats, WhitelistRestoreStats, }; + +#[cfg(test)] +mod tests { + use super::DeletionOutcome; + + #[test] + fn outcome_merge_is_saturating_and_only_removals_are_use() { + let mut outcome = DeletionOutcome { + skipped: usize::MAX, + failures: 1, + ..Default::default() + }; + outcome.merge(DeletionOutcome { + main_db_deleted: 1, + purgatory_removed: 2, + skipped: 1, + failures: 2, + }); + assert_eq!(outcome.removed_events(), 3); + assert!(outcome.used()); + assert_eq!(outcome.skipped, usize::MAX); + assert_eq!(outcome.failures, 3); + assert!(!DeletionOutcome { + skipped: 1, + failures: 1, + ..Default::default() + } + .used()); + } +} diff --git a/src/nostr/lifecycle/deletion/policy.rs b/src/nostr/lifecycle/deletion/policy.rs index 7b75aa7..1237ebd 100644 --- a/src/nostr/lifecycle/deletion/policy.rs +++ b/src/nostr/lifecycle/deletion/policy.rs @@ -33,9 +33,14 @@ use std::collections::HashSet; use nostr::nips::nip19::ToBech32; -use nostr_relay_builder::prelude::{Event, EventId, Filter, Kind, PublicKey, WritePolicyResult}; +use nostr_relay_builder::prelude::{ + Alphabet, Event, EventId, Filter, Kind, PublicKey, SingleLetterTag, Timestamp, + WritePolicyResult, +}; +use super::service::{request_is_served_with_config, UNSERVED_REPLAY_MESSAGE}; use super::DeletionContext; +use crate::nostr::lifecycle::RequestClassification; use crate::nostr::policy::{duplicate, reject_error, reject_invalid}; const MAX_DELETION_TARGET_TAGS: usize = 2048; @@ -70,88 +75,85 @@ impl DeletionPolicy { /// /// When `deletion_request_disrespector` is enabled the relay acts as an /// archival server: the kind-5 event is still accepted (and stored in the - /// main database) so clients see an OK and the request is preserved, but it - /// is NOT acted upon — no purgatory eviction, no main-DB deletion, and no - /// tombstone recording. The targeted events therefore remain fully - /// accessible. Already-covered kind-5 requests are still treated as - /// duplicates before archival-mode storage to avoid polluting the served - /// deletion-request stream. The same archival-mode contract is applied to - /// NIP-62 vanish requests in [`DeletionService::handle_vanish`](super::service::DeletionService::handle_vanish). + /// main database) so clients see an OK and the request is preserved. It + /// receives disrespector lifecycle metadata and is evaluated read-only to + /// determine whether it would delete stored data under normal policy, but + /// it never installs a local gate or mutates targets. Distinct signed + /// kind-5 requests receive independent lifecycle records even when their + /// targets overlap. The same archival-mode contract is applied to NIP-62 + /// vanish requests in + /// [`DeletionService::handle_vanish`](super::service::DeletionService::handle_vanish). pub async fn handle(&self, event: &Event) -> WritePolicyResult { - 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}")); - } + if let Err(result) = self.validate_request(event).await { + return result; } - // Archival mode: store the deletion request but do not process it. + let classification = if self.ctx.config.deletion_request_disrespector { + RequestClassification::Disrespector + } else { + RequestClassification::LocallyActionable + }; + // Hold the lifecycle lock through the admission decision so cleanup + // cannot race an exact replay back into Main after its served deadline. + let tombstones = &self.ctx.tombstones; + let record_result = { + let _lifecycle_guard = tombstones.lock_lifecycle().await; + let existing = match tombstones.lifecycle_for_request_result(&event.id).await { + Ok(record) => record, + Err(error) => { + tracing::error!(event_id = %event.id.to_hex(), error = %error, "Failed to read NIP-09 deletion lifecycle"); + return reject_error(format!( + "internal error reading deletion lifecycle: {error}" + )); + } + }; + if let Some(record) = existing { + match request_is_served_with_config(&self.ctx, &record, Timestamp::now()) { + Ok(false) => return duplicate(UNSERVED_REPLAY_MESSAGE), + Ok(true) => {} + Err(error) => { + tracing::error!(event_id = %event.id.to_hex(), error = %error, "Failed to evaluate NIP-09 deletion lifecycle retention"); + return reject_error(format!( + "internal error evaluating deletion lifecycle retention: {error}" + )); + } + } + } + tombstones + .record_request_locked(event, Timestamp::now(), classification) + .await + }; + if let Err(e) = record_result { + tracing::error!(event_id = %event.id.to_hex(), error = %e, "Failed to record NIP-09 deletion lifecycle"); + return reject_error(format!("internal error recording deletion lifecycle: {e}")); + } + + // Archival mode: retain and classify the deletion request without + // invoking any destructive lifecycle paths. if self.ctx.config.deletion_request_disrespector { + let would_delete = match self.would_delete_stored_target(event).await { + Ok(would_delete) => would_delete, + Err(e) => { + tracing::error!(event_id = %event.id.to_hex(), error = %e, "Failed to evaluate disrespector NIP-09 request"); + return reject_error(format!( + "internal error evaluating deletion request: {e}" + )); + } + }; + if would_delete { + if let Err(result) = self.mark_request_used(event).await { + return result; + } + } tracing::info!( event_id = %event.id.to_hex(), author = %event.pubkey.to_hex(), - "Disrespector mode: storing NIP-09 deletion request without acting on it" + would_delete, + "Disrespector mode: stored and read-only evaluated NIP-09 deletion request" ); return WritePolicyResult::Accept; } - let target_tag_count = event - .tags - .iter() - .filter(|tag| { - let v = tag.as_slice(); - v.len() >= 2 && (v[0] == "e" || v[0] == "a") - }) - .count(); - if target_tag_count > MAX_DELETION_TARGET_TAGS { - tracing::warn!( - event_id = %event.id.to_hex(), - target_tag_count, - max = MAX_DELETION_TARGET_TAGS, - "Rejected NIP-09 deletion with too many targets" - ); - return reject_invalid("too many deletion targets"); - } - - // 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 - // `invalid`). `a`-tag author mismatches are simply ignored (the - // coordinate pubkey is part of the tag, so a mismatch is a no-op rather - // than an attack signal). - for id in Self::e_tag_ids(event) { - match self.ctx.database.event_by_id(&id).await { - Ok(Some(target)) if target.pubkey != event.pubkey => { - tracing::warn!( - deleter = %event.pubkey.to_hex(), - target_author = %target.pubkey.to_hex(), - target_id = %id.to_hex(), - "Rejected invalid NIP-09 deletion: target authored by another pubkey" - ); - return reject_invalid("cannot delete event authored by another pubkey"); - } - Ok(_) => {} - Err(e) => { - tracing::warn!(error = %e, "Database lookup failed during deletion validation"); - return reject_error(format!("internal error: {e}")); - } - } - } - // Lifecycle lock discipline: acquire per `(owner, identifier)` locks // before mutating tombstones, main DB, holding DB, archives, repository // directories, or purgatory. Git Smart HTTP fetch/info-refs and push @@ -164,27 +166,15 @@ impl DeletionPolicy { .write_repositories(self.deletion_recovery_scopes(event).await) .await; - // Record the deletion before destructive work so a tombstone write - // failure rejects the deletion request without already having removed - // targets from purgatory/main DB/holding/archive state. Subsequent - // destructive steps are best-effort and log their own failures; the - // tombstone is the durable operation marker and resubmission gate source. - if let Err(e) = self.ctx.tombstones.record_deletion(event).await { - tracing::error!(error = %e, "Failed to record deletion tombstone"); - return reject_error(format!("internal error recording deletion: {e}")); - } - - self.compact_superseded_deletion_requests(event).await; - let rollback_plans = self.collect_replaceable_rollback_plans(event).await; let mut identifiers_to_realign = Self::state_a_tag_identifiers_for_author(event); // Process purgatory removals (synchronous, in-memory). - self.remove_purgatory_targets(event); + let mut outcome = self.remove_purgatory_targets(event); // Move targeted events into holding DB, then delete from main DB. let mut moved_ids = HashSet::new(); - self.delete_main_db_targets(event, &mut moved_ids).await; + outcome.merge(self.delete_main_db_targets(event, &mut moved_ids).await); for plan in &rollback_plans { self.rollback_deleted_active_replaceable(plan).await; @@ -197,67 +187,170 @@ impl DeletionPolicy { self.realign_identifier_state(&identifier).await; } + if outcome.used() { + if let Err(result) = self.mark_request_used(event).await { + return result; + } + } + tracing::info!(event_id = %event.id.to_hex(), main_db_deleted = outcome.main_db_deleted, purgatory_removed = outcome.purgatory_removed, skipped = outcome.skipped, failures = outcome.failures, "Processed NIP-09 deletion outcome"); + // Accept the deletion event itself so it is stored. WritePolicyResult::Accept } - async fn compact_superseded_deletion_requests(&self, event: &Event) { - let superseded_ids = match self.ctx.tombstones.superseded_deletion_ids(event).await { - Ok(ids) => ids, - Err(e) => { - tracing::warn!( - event_id = %event.id.to_hex(), - error = %e, - "Failed to discover superseded NIP-09 deletion requests" - ); - return; + /// Validate all admission inputs before lifecycle persistence or target work. + async fn validate_request(&self, event: &Event) -> Result<(), WritePolicyResult> { + let target_tag_count = event + .tags + .iter() + .filter(|tag| { + let v = tag.as_slice(); + v.len() >= 2 && (v[0] == "e" || v[0] == "a") + }) + .count(); + if target_tag_count > MAX_DELETION_TARGET_TAGS { + tracing::warn!(event_id = %event.id.to_hex(), target_tag_count, max = MAX_DELETION_TARGET_TAGS, "Rejected NIP-09 deletion with too many targets"); + return Err(reject_invalid("too many deletion targets")); + } + + // An existing cross-author `e` target invalidates the entire request. + // Foreign and malformed `a` coordinates remain NIP-09 no-ops. + for id in Self::e_tag_ids(event) { + match self.ctx.database.event_by_id(&id).await { + Ok(Some(target)) if target.pubkey != event.pubkey => { + tracing::warn!(deleter = %event.pubkey.to_hex(), target_author = %target.pubkey.to_hex(), target_id = %id.to_hex(), "Rejected invalid NIP-09 deletion: target authored by another pubkey"); + return Err(reject_invalid( + "cannot delete event authored by another pubkey", + )); + } + Ok(_) => {} + Err(e) => { + tracing::warn!(error = %e, "Database lookup failed during deletion validation"); + return Err(reject_error(format!("internal error: {e}"))); + } } - }; - - if superseded_ids.is_empty() { - return; } + Ok(()) + } - let superseded_count = superseded_ids.len(); - - // Remove superseded kind-5 events from the served main DB. The database - // delete API owns secondary index maintenance; this is the same path - // used for stale cross-author deletion request cleanup. - if let Err(e) = self - .ctx - .database - .delete(Filter::new().ids(superseded_ids.clone())) - .await - { - tracing::warn!( - event_id = %event.id.to_hex(), - superseded_count, - error = %e, - "Failed to remove superseded NIP-09 deletion requests from main DB" - ); - } - - if let Err(e) = self + async fn mark_request_used(&self, event: &Event) -> Result<(), WritePolicyResult> { + match self .ctx .tombstones - .remove_deletions_by_ids(superseded_ids) + .mark_request_used(&event.id, Timestamp::now()) .await { - tracing::warn!( - event_id = %event.id.to_hex(), - superseded_count, - error = %e, - "Failed to remove superseded NIP-09 deletion requests from tombstone DB" - ); - return; + Ok(Some(_)) => Ok(()), + Ok(None) => Err(reject_error( + "internal error updating deletion lifecycle: missing metadata", + )), + Err(e) => { + tracing::error!(event_id = %event.id.to_hex(), error = %e, "Failed to mark used NIP-09 deletion request after target processing"); + Err(reject_error(format!( + "internal error updating deletion lifecycle: {e}" + ))) + } + } + } + + /// Read-only normal-policy target evaluation for disrespector mode. + /// This intentionally only queries the main database and purgatory snapshots; + /// it must not call deletion, holding, archive, rollback, or cascade helpers. + pub(super) async fn would_delete_stored_target(&self, event: &Event) -> anyhow::Result { + for id in Self::e_tag_ids(event) { + if self.ctx.database.event_by_id(&id).await?.is_some() { + return Ok(true); + } } - tracing::info!( - event_id = %event.id.to_hex(), - author = %event.pubkey.to_hex(), - superseded_count, - "Compacted superseded NIP-09 deletion request(s)" - ); + for coordinate in Self::a_tag_coordinates(event) { + let Some((kind, owner, identifier)) = Self::parse_coordinate(&coordinate) else { + continue; + }; + if owner != event.pubkey { + continue; + } + let mut filter = Filter::new() + .kind(kind) + .author(owner) + .until(event.created_at); + if kind.is_addressable() { + filter = + filter.custom_tag(SingleLetterTag::lowercase(Alphabet::D), identifier.clone()); + } + if !self.ctx.database.query(filter).await?.is_empty() { + return Ok(true); + } + } + + Ok(self.purgatory_target_would_be_removed(event)) + } + + fn purgatory_target_would_be_removed(&self, event: &Event) -> bool { + for id in Self::e_tag_ids(event) { + for (repository_id, _) in self.ctx.purgatory.announcements_for_sync() { + let parts: Vec<_> = repository_id.splitn(3, ':').collect(); + if parts.len() == 3 + && parts[1] == event.pubkey.to_hex() + && self + .ctx + .purgatory + .find_announcement(&event.pubkey, parts[2]) + .is_some_and(|entry| entry.event.id == id) + { + return true; + } + } + if self + .ctx + .purgatory + .get_all_identifiers() + .into_iter() + .any(|identifier| { + self.ctx + .purgatory + .find_state(&identifier) + .into_iter() + .any(|entry| entry.author == event.pubkey && entry.event.id == id) + }) + { + return true; + } + } + + for coordinate in Self::a_tag_coordinates(event) { + let Some((kind, owner, identifier)) = Self::parse_coordinate(&coordinate) else { + continue; + }; + if owner != event.pubkey { + continue; + } + let covered = |created_at: Timestamp| created_at <= event.created_at; + match kind { + Kind::GitRepoAnnouncement => { + if self + .ctx + .purgatory + .find_announcement(&owner, &identifier) + .is_some_and(|entry| covered(entry.event.created_at)) + { + return true; + } + } + Kind::RepoState + if self + .ctx + .purgatory + .find_state(&identifier) + .into_iter() + .any(|entry| entry.author == owner && covered(entry.event.created_at)) => + { + return true; + } + _ => {} + } + } + false } /// Extract all event-id (`e` tag) targets from a deletion event. @@ -318,6 +411,7 @@ mod tests { use crate::nostr::lifecycle::HoldingStore; use crate::nostr::lifecycle::ReplaceableHistoryStore; use crate::nostr::lifecycle::RepositoryLifecycle; + use crate::nostr::lifecycle::RequestLifecycleRecord; use crate::nostr::lifecycle::Tombstones; use crate::purgatory::Purgatory; use nostr_relay_builder::prelude::*; @@ -325,6 +419,31 @@ mod tests { use std::path::PathBuf; use std::sync::Arc; + trait TombstoneTestExt { + async fn lifecycle_for_request( + &self, + request_id: &EventId, + ) -> Option; + async fn is_event_deleted(&self, id: &EventId, author: &PublicKey) -> bool; + } + + impl TombstoneTestExt for Tombstones { + async fn lifecycle_for_request( + &self, + request_id: &EventId, + ) -> Option { + self.lifecycle_for_request_result(request_id).await.unwrap() + } + + async fn is_event_deleted(&self, id: &EventId, author: &PublicKey) -> bool { + !self + .event_deletion_candidates(id, author) + .await + .unwrap() + .is_empty() + } + } + fn make_context() -> DeletionContext { let db = Arc::new(nostr_memory::MemoryDatabase::unbounded()); let purgatory = Arc::new(Purgatory::new(PathBuf::new())); @@ -417,16 +536,19 @@ mod tests { .has_purgatory_announcement(&keys.public_key(), identifier), "Purgatory entry should have been removed" ); - assert!( + assert_eq!( ctx.tombstones - .is_event_deleted(&announcement.id, &keys.public_key()) - .await, - "accepted deletion must persist a tombstone as the durable operation marker" + .event_deletion_candidates(&announcement.id, &keys.public_key()) + .await + .unwrap() + .len(), + 1, + "accepted deletion must persist a lifecycle-backed tombstone" ); } #[tokio::test] - async fn duplicate_deletion_request_is_not_accepted_for_storage() { + async fn distinct_overlapping_deletion_requests_are_independently_accepted() { let ctx = make_context(); let keys = Keys::generate(); let target = EventId::all_zeros(); @@ -436,7 +558,7 @@ mod tests { .custom_created_at(Timestamp::from_secs(1000)) .finalize(&keys) .unwrap(); - let duplicate = EventBuilder::new(Kind::EventDeletion, "") + let overlapping = EventBuilder::new(Kind::EventDeletion, "") .tags(vec![Tag::event(target)]) .custom_created_at(Timestamp::from_secs(1001)) .finalize(&keys) @@ -446,15 +568,119 @@ mod tests { 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" + assert!(matches!( + policy.handle(&overlapping).await, + WritePolicyResult::Accept + )); + let records = policy + .ctx + .tombstones + .lifecycle_records_result() + .await + .unwrap(); + assert_eq!(records.len(), 2); + assert!(records.iter().any(|record| record.request.id == first.id)); + assert!(records + .iter() + .any(|record| record.request.id == overlapping.id)); + } + + #[tokio::test] + async fn nip09_marks_request_used_only_after_successful_removal() { + let ctx = make_context(); + let keys = Keys::generate(); + let target = EventBuilder::new(Kind::TextNote, "target") + .finalize(&keys) + .unwrap(); + ctx.database.save_event(&target).await.unwrap(); + let deletion = EventBuilder::new(Kind::EventDeletion, "") + .tags(vec![Tag::event(target.id)]) + .finalize(&keys) + .unwrap(); + + assert!(matches!( + DeletionPolicy::new(ctx.clone()).handle(&deletion).await, + WritePolicyResult::Accept + )); + let record = ctx + .tombstones + .lifecycle_records_result() + .await + .unwrap() + .into_iter() + .find(|record| record.request.id == deletion.id) + .unwrap(); + assert_eq!( + record.classification, + RequestClassification::LocallyActionable + ); + assert!(record.last_used_at.is_some()); + } + + #[tokio::test] + async fn nip09_noop_leaves_request_lifecycle_unused() { + let ctx = make_context(); + let keys = Keys::generate(); + let deletion = EventBuilder::new(Kind::EventDeletion, "") + .tags(vec![Tag::event(EventId::all_zeros())]) + .finalize(&keys) + .unwrap(); + + assert!(matches!( + DeletionPolicy::new(ctx.clone()).handle(&deletion).await, + WritePolicyResult::Accept + )); + let record = ctx + .tombstones + .lifecycle_records_result() + .await + .unwrap() + .into_iter() + .find(|record| record.request.id == deletion.id) + .unwrap(); + assert_eq!(record.last_used_at, None); + } + + #[tokio::test] + async fn nip09_failed_archive_preserves_target_and_leaves_request_unused() { + let mut ctx = make_context(); + let directory = tempfile::tempdir().unwrap(); + ctx.git_data_path = directory.path().to_path_buf(); + let keys = Keys::generate(); + let target = make_announcement_event(&keys, "archive-failure"); + let owner = owner_directory_component(&keys.public_key()); + std::fs::create_dir_all(directory.path().join(&owner).join("archive-failure.git")).unwrap(); + // A file where the archive directory belongs makes the archival attempt + // fail, so the fail-safe path must not award deletion-use credit. + std::fs::write(directory.path().join(".archive"), "not a directory").unwrap(); + ctx.database.save_event(&target).await.unwrap(); + let request = EventBuilder::new(Kind::EventDeletion, "") + .tags(vec![Tag::event(target.id)]) + .finalize(&keys) + .unwrap(); + + assert!(matches!( + DeletionPolicy::new(ctx.clone()).handle(&request).await, + WritePolicyResult::Accept + )); + assert!(ctx + .database + .event_by_id(&target.id) + .await + .unwrap() + .is_some()); + assert_eq!( + ctx.tombstones + .lifecycle_for_request(&request.id) + .await + .unwrap() + .last_used_at, + None ); } #[tokio::test] - async fn newer_coordinate_deletion_compacts_superseded_request_from_main_db() { + async fn coordinate_deletion_requests_with_different_cutoffs_coexist() { let ctx = make_context(); let keys = Keys::generate(); let coord = format!("30618:{}:my-repo", keys.public_key().to_hex()); @@ -485,18 +711,21 @@ mod tests { WritePolicyResult::Accept )); - assert!( - ctx.database.event_by_id(&older.id).await.unwrap().is_none(), - "superseded older kind-5 request should be removed from the served main DB" - ); - assert!( - ctx.tombstones - .superseded_deletion_ids(&newer) + ctx.database.save_event(&newer).await.unwrap(); + for request in [&older, &newer] { + assert!(ctx + .database + .event_by_id(&request.id) .await .unwrap() - .is_empty(), - "superseded older kind-5 tombstone should be compacted after the newer request is recorded" - ); + .is_some()); + assert!(ctx + .tombstones + .lifecycle_for_request_result(&request.id) + .await + .unwrap() + .is_some()); + } } #[tokio::test] @@ -561,56 +790,67 @@ mod tests { .has_purgatory_announcement(&owner_keys.public_key(), identifier), "Purgatory entry should NOT have been removed by wrong author" ); + assert!(ctx + .tombstones + .lifecycle_for_request_result(&deletion.id) + .await + .unwrap() + .is_some()); } #[tokio::test] - async fn test_target_arrival_removes_stale_cross_author_deletion_request() { + async fn preemptive_multi_target_foreign_event_keeps_request_and_valid_target_effective() { let ctx = make_context(); let attacker_keys = Keys::generate(); let victim_keys = Keys::generate(); - let target = EventBuilder::text_note("real event from victim") + let foreign_target = EventBuilder::text_note("real event from victim") .finalize(&victim_keys) .unwrap(); + let valid_target = EventBuilder::text_note("real event from deleter") + .finalize(&attacker_keys) + .unwrap(); let deletion = EventBuilder::new(Kind::EventDeletion, "") - .tags(vec![Tag::event(target.id)]) + .tags(vec![ + Tag::event(foreign_target.id), + Tag::event(valid_target.id), + ]) .finalize(&attacker_keys) .unwrap(); - ctx.tombstones.record_deletion(&deletion).await.unwrap(); + assert!(matches!( + DeletionPolicy::new(ctx.clone()).handle(&deletion).await, + WritePolicyResult::Accept + )); ctx.database.save_event(&deletion).await.unwrap(); - assert!( - ctx.tombstones - .is_event_deleted(&target.id, &attacker_keys.public_key()) - .await - ); - assert!(ctx - .database - .event_by_id(&deletion.id) - .await - .unwrap() - .is_some()); - let service = DeletionService::new(ctx.clone()); - service - .remove_stale_cross_author_event_deletions(&target) - .await; - - assert!( - !ctx.tombstones - .is_event_deleted(&target.id, &attacker_keys.public_key()) - .await, - "stale cross-author deletion tombstone should be removed" - ); + assert!(service.gate(&foreign_target).await.is_none()); assert!( ctx.database .event_by_id(&deletion.id) .await .unwrap() - .is_none(), - "stale cross-author deletion request should be removed from main DB" + .is_some(), + "foreign target discovery must not remove the entire signed request" ); + assert!(ctx + .tombstones + .lifecycle_for_request_result(&deletion.id) + .await + .unwrap() + .unwrap() + .last_used_at + .is_none()); + assert!(service.gate(&valid_target).await.is_some()); + assert!(ctx + .tombstones + .lifecycle_for_request_result(&deletion.id) + .await + .unwrap() + .unwrap() + .last_used_at + .is_some()); } #[tokio::test] @@ -667,7 +907,6 @@ mod tests { #[tokio::test] async fn deletion_with_excessive_target_tags_is_rejected() { - let ctx = make_context(); let keys = Keys::generate(); let tags = (0..=MAX_DELETION_TARGET_TAGS) .map(|i| { @@ -683,10 +922,15 @@ mod tests { .finalize(&keys) .unwrap(); - let policy = DeletionPolicy::new(ctx); - let result = policy.handle(&deletion).await; - - assert!(matches!(result, WritePolicyResult::Reject { .. })); + for ctx in [make_context(), make_disrespector_context()] { + let result = DeletionPolicy::new(ctx.clone()).handle(&deletion).await; + assert!(matches!(result, WritePolicyResult::Reject { .. })); + assert!(ctx + .tombstones + .lifecycle_for_request(&deletion.id) + .await + .is_none()); + } } #[tokio::test] @@ -758,7 +1002,7 @@ mod tests { } #[tokio::test] - async fn test_disrespector_does_not_record_tombstone() { + async fn disrespector_records_unused_lifecycle_without_enforcing_a_gate() { let ctx = make_disrespector_context(); let keys = Keys::generate(); let target = EventId::all_zeros(); @@ -772,7 +1016,14 @@ mod tests { let result = policy.handle(&deletion).await; assert!(matches!(result, WritePolicyResult::Accept)); - // No tombstone recorded -> re-submission of the target is NOT blocked. + let record = ctx + .tombstones + .lifecycle_for_request(&deletion.id) + .await + .expect("disrespector request lifecycle should be recorded"); + assert_eq!(record.classification, RequestClassification::Disrespector); + assert_eq!(record.last_used_at, None); + // Disrespector metadata does not install a gate. assert!( !ctx.tombstones .is_event_deleted(&target, &keys.public_key()) @@ -806,6 +1057,133 @@ mod tests { still_there.is_some(), "Disrespector mode must NOT delete the target from the main DB" ); + assert!(ctx + .tombstones + .lifecycle_for_request(&deletion.id) + .await + .is_some_and(|record| record.last_used_at.is_some())); + } + + #[tokio::test] + async fn disrespector_marks_covered_coordinate_used_without_mutating_target() { + let ctx = make_disrespector_context(); + let keys = Keys::generate(); + let target = make_announcement_event(&keys, "keep-coordinate"); + ctx.database.save_event(&target).await.unwrap(); + let coordinate = format!("30617:{}:keep-coordinate", keys.public_key().to_hex()); + let deletion = EventBuilder::new(Kind::EventDeletion, "") + .tags(vec![Tag::custom("a", vec![coordinate])]) + .finalize(&keys) + .unwrap(); + + assert!(matches!( + DeletionPolicy::new(ctx.clone()).handle(&deletion).await, + WritePolicyResult::Accept + )); + assert!(ctx + .database + .event_by_id(&target.id) + .await + .unwrap() + .is_some()); + assert!(ctx + .tombstones + .lifecycle_for_request(&deletion.id) + .await + .unwrap() + .last_used_at + .is_some()); + } + + #[tokio::test] + async fn disrespector_marks_matching_purgatory_target_used_without_removing_it() { + let ctx = make_disrespector_context(); + let keys = Keys::generate(); + let announcement = make_announcement_event(&keys, "keep-purgatory"); + add_to_purgatory(&ctx, &announcement, "keep-purgatory"); + let deletion = EventBuilder::new(Kind::EventDeletion, "") + .tags(vec![Tag::event(announcement.id)]) + .finalize(&keys) + .unwrap(); + + assert!(matches!( + DeletionPolicy::new(ctx.clone()).handle(&deletion).await, + WritePolicyResult::Accept + )); + assert!(ctx + .purgatory + .has_purgatory_announcement(&keys.public_key(), "keep-purgatory")); + assert!(ctx + .tombstones + .lifecycle_for_request(&deletion.id) + .await + .unwrap() + .last_used_at + .is_some()); + } + + #[tokio::test] + async fn cross_author_existing_target_is_rejected_without_lifecycle_record() { + let ctx = make_disrespector_context(); + let owner = Keys::generate(); + let attacker = Keys::generate(); + let target = EventBuilder::new(Kind::TextNote, "owned") + .finalize(&owner) + .unwrap(); + ctx.database.save_event(&target).await.unwrap(); + let deletion = EventBuilder::new(Kind::EventDeletion, "") + .tags(vec![Tag::event(target.id)]) + .finalize(&attacker) + .unwrap(); + + assert!(matches!( + DeletionPolicy::new(ctx.clone()).handle(&deletion).await, + WritePolicyResult::Reject { .. } + )); + assert!(ctx + .tombstones + .lifecycle_for_request(&deletion.id) + .await + .is_none()); + assert!(ctx + .database + .event_by_id(&target.id) + .await + .unwrap() + .is_some()); + } + + #[tokio::test] + async fn disrespector_foreign_coordinate_is_unused() { + let ctx = make_disrespector_context(); + let owner = Keys::generate(); + let attacker = Keys::generate(); + let target = make_announcement_event(&owner, "foreign-coordinate"); + ctx.database.save_event(&target).await.unwrap(); + let coordinate = format!("30617:{}:foreign-coordinate", owner.public_key().to_hex()); + let deletion = EventBuilder::new(Kind::EventDeletion, "") + .tags(vec![Tag::custom("a", vec![coordinate])]) + .finalize(&attacker) + .unwrap(); + + assert!(matches!( + DeletionPolicy::new(ctx.clone()).handle(&deletion).await, + WritePolicyResult::Accept + )); + assert!(ctx + .database + .event_by_id(&target.id) + .await + .unwrap() + .is_some()); + assert_eq!( + ctx.tombstones + .lifecycle_for_request(&deletion.id) + .await + .unwrap() + .last_used_at, + None + ); } #[test] diff --git a/src/nostr/lifecycle/deletion/purgatory.rs b/src/nostr/lifecycle/deletion/purgatory.rs index d5f964c..6046d0b 100644 --- a/src/nostr/lifecycle/deletion/purgatory.rs +++ b/src/nostr/lifecycle/deletion/purgatory.rs @@ -3,7 +3,7 @@ use std::process::Command; use nostr_relay_builder::prelude::{Event, Kind, Timestamp}; -use super::policy::DeletionPolicy; +use super::{policy::DeletionPolicy, DeletionOutcome}; use crate::nostr::events::RepositoryAnnouncement; use crate::nostr::lifecycle::DeletionSource; @@ -137,8 +137,10 @@ impl DeletionPolicy { /// /// Only removes entries where the purgatory entry's author matches the deletion /// event's pubkey (enforces author-only deletion). - pub(super) fn remove_purgatory_targets(&self, event: &Event) { + pub(super) fn remove_purgatory_targets(&self, event: &Event) -> DeletionOutcome { let author = &event.pubkey; + let mut outcome = DeletionOutcome::default(); + let mut removed_ids = HashSet::new(); for tag in event.tags.iter() { let tag_vec = tag.as_slice(); @@ -150,16 +152,27 @@ impl DeletionPolicy { "e" => { // Event ID reference: find purgatory announcement with this event ID let target_id = &tag_vec[1]; - self.remove_by_event_id(author, target_id, event.created_at.as_secs()); + outcome.merge(self.remove_by_event_id( + author, + target_id, + event.created_at.as_secs(), + &mut removed_ids, + )); } "a" => { // Addressable coordinate reference: `::` let coord = &tag_vec[1]; - self.remove_by_coordinate(author, coord, event.created_at.as_secs()); + outcome.merge(self.remove_by_coordinate( + author, + coord, + event.created_at.as_secs(), + &mut removed_ids, + )); } _ => {} } } + outcome } /// Remove a purgatory entry (announcement, state event, or PR event) matched by event ID. @@ -171,7 +184,8 @@ impl DeletionPolicy { author: &nostr_relay_builder::prelude::PublicKey, target_id_hex: &str, _deletion_created_at: u64, - ) { + removed_ids: &mut HashSet, + ) -> DeletionOutcome { // --- Check PR events (kind 1617/1618) first — O(1) direct lookup --- // PR purgatory is keyed by event ID hex, so this is the cheapest check. // Only remove if the entry has an actual event (not a placeholder) and the @@ -185,11 +199,14 @@ impl DeletionPolicy { "Deletion request: removing purgatory PR event by event ID" ); self.ctx.purgatory.remove_pr(target_id_hex); - return; + return DeletionOutcome { + purgatory_removed: usize::from(removed_ids.insert(event.id)), + ..Default::default() + }; } } // Entry exists but is a placeholder or wrong author — don't remove - return; + return DeletionOutcome::default(); } // --- Check announcements (kind 30617) --- @@ -217,8 +234,7 @@ impl DeletionPolicy { author = %author.to_hex(), "Deletion request: removing purgatory announcement by event ID" ); - self.evict_purgatory_entry(author, identifier); - return; // event IDs are unique + return self.evict_purgatory_entry(author, identifier, removed_ids); } } } @@ -239,10 +255,14 @@ impl DeletionPolicy { self.ctx .purgatory .remove_state_event(&identifier, &entry.event.id); - return; // event IDs are unique + return DeletionOutcome { + purgatory_removed: usize::from(removed_ids.insert(entry.event.id)), + ..Default::default() + }; } } } + DeletionOutcome::default() } /// Remove a purgatory entry matched by addressable coordinate. @@ -256,11 +276,13 @@ impl DeletionPolicy { author: &nostr_relay_builder::prelude::PublicKey, coordinate: &str, deletion_created_at: u64, - ) { + removed_ids: &mut HashSet, + ) -> DeletionOutcome { + let mut outcome = DeletionOutcome::default(); // Parse coordinate: `::` let parts: Vec<&str> = coordinate.splitn(3, ':').collect(); if parts.len() != 3 { - return; + return DeletionOutcome::default(); } let kind_str = parts[0]; @@ -274,7 +296,7 @@ impl DeletionPolicy { deletion_author = %author.to_hex(), "Ignoring deletion: coordinate pubkey does not match deletion author" ); - return; + return DeletionOutcome::default(); } match kind_str { @@ -287,7 +309,7 @@ impl DeletionPolicy { author = %author.to_hex(), "Deletion request: removing purgatory announcement by coordinate" ); - self.evict_purgatory_entry(author, identifier); + return self.evict_purgatory_entry(author, identifier, removed_ids); } else { tracing::debug!( identifier = %identifier, @@ -309,7 +331,10 @@ impl DeletionPolicy { self.ctx .purgatory .remove_state_event(identifier, &entry.event.id); - removed += 1; + let counted = usize::from(removed_ids.insert(entry.event.id)); + removed += counted; + outcome.purgatory_removed = + outcome.purgatory_removed.saturating_add(counted); } } if removed > 0 { @@ -325,6 +350,7 @@ impl DeletionPolicy { // Other kinds not handled } } + outcome } /// Remove a purgatory announcement and delete its bare repository from disk. @@ -332,7 +358,9 @@ impl DeletionPolicy { &self, author: &nostr_relay_builder::prelude::PublicKey, identifier: &str, - ) { + removed_ids: &mut HashSet, + ) -> DeletionOutcome { + let mut outcome = DeletionOutcome::default(); // Get repo path before removing if let Some(entry) = self.ctx.purgatory.find_announcement(author, identifier) { if entry.repo_path.exists() { @@ -342,6 +370,7 @@ impl DeletionPolicy { error = %e, "Failed to delete bare repository during deletion request processing" ); + outcome.failures = outcome.failures.saturating_add(1); } else { tracing::info!( path = %entry.repo_path.display(), @@ -351,6 +380,11 @@ impl DeletionPolicy { } } + if let Some(entry) = self.ctx.purgatory.find_announcement(author, identifier) { + outcome.purgatory_removed = outcome + .purgatory_removed + .saturating_add(usize::from(removed_ids.insert(entry.event.id))); + } self.ctx.purgatory.remove_announcement(author, identifier); // Remove state events for this identifier only if no other owner's @@ -362,7 +396,13 @@ impl DeletionPolicy { .is_empty(); if !other_owners_remain { + for entry in self.ctx.purgatory.find_state(identifier) { + outcome.purgatory_removed = outcome + .purgatory_removed + .saturating_add(usize::from(removed_ids.insert(entry.event.id))); + } self.ctx.purgatory.remove_state(identifier); } + outcome } } diff --git a/src/nostr/lifecycle/deletion/runtime.rs b/src/nostr/lifecycle/deletion/runtime.rs index 9701bbb..6d162b5 100644 --- a/src/nostr/lifecycle/deletion/runtime.rs +++ b/src/nostr/lifecycle/deletion/runtime.rs @@ -14,7 +14,7 @@ use super::startup::{ BlacklistParityStats, BlacklistRestoreStats, StartupReconciliationStats, WhitelistParityStats, WhitelistRestoreStats, }; -use super::DeletionService; +use super::{DeletionService, RequestCleanupStats}; #[derive(Debug, Args)] pub struct HoldingEjectArgs { @@ -61,15 +61,24 @@ impl DeletionRuntime { } /// Run deletion-owned startup tasks before the relay begins serving traffic. - pub async fn run_startup_tasks(&self) { - log_startup_reconciliation(self.service.run_startup_reconciliation().await); + pub async fn run_startup_tasks(&self) -> Result<()> { + log_startup_reconciliation(self.service.run_startup_reconciliation().await?); + // Enumeration failure is fatal before serving traffic; individual record + // failures are conservatively retained and retried by the timer. + let request_stats = self + .service + .cleanup_expired_requests(Timestamp::now()) + .await?; + log_request_cleanup("startup catch-up", request_stats); self.run_holding_startup_cleanup().await; + Ok(()) } /// Spawn deletion-owned background maintenance tasks. pub fn spawn_cleanup_task(&self) -> DeletionCleanupTask { let holding = self.holding.clone(); let lifecycle = self.lifecycle.clone(); + let service = self.service.clone(); let retention = self.holding_retention; let interval_duration = self.holding_cleanup_interval; let (shutdown_tx, mut shutdown_rx) = watch::channel(false); @@ -83,6 +92,10 @@ impl DeletionRuntime { loop { tokio::select! { _ = interval.tick() => { + match service.cleanup_expired_requests(Timestamp::now()).await { + Ok(stats) => log_request_cleanup("periodic pass", stats), + Err(error) => tracing::warn!(error = %error, "Deletion-request cleanup periodic pass failed"), + } match holding.cleanup_expired_with_lifecycle(&lifecycle, Timestamp::now(), retention).await { Ok(stats) => { if stats.expired_records > 0 { @@ -102,7 +115,7 @@ impl DeletionRuntime { } changed = shutdown_rx.changed() => { if changed.is_ok() && *shutdown_rx.borrow() { - tracing::info!("Holding cleanup task received shutdown signal"); + tracing::info!("Deletion lifecycle maintenance task received shutdown signal"); break; } } @@ -113,7 +126,7 @@ impl DeletionRuntime { tracing::info!( retention_secs = retention.as_secs(), interval_secs = interval_duration.as_secs(), - "Holding cleanup task started" + "Deletion lifecycle maintenance task started" ); DeletionCleanupTask { @@ -159,7 +172,7 @@ impl DeletionCleanupTask { pub async fn shutdown(self) { let _ = self.shutdown_tx.send(true); if let Err(e) = self.handle.await { - tracing::warn!(error = %e, "Holding cleanup task join failed during shutdown"); + tracing::warn!(error = %e, "Deletion lifecycle maintenance task join failed during shutdown"); } } } @@ -199,6 +212,27 @@ fn log_startup_reconciliation(stats: StartupReconciliationStats) { log_whitelist_restore(stats.whitelist_restore); } +fn log_request_cleanup(phase: &str, stats: RequestCleanupStats) { + crate::metrics::record_deletion_request_cleanup_run( + stats.main_payloads_removed, + stats.tombstone_payloads_removed, + stats.metadata_rows_removed, + stats.failures, + stats.stale_or_concurrent_records_skipped, + ); + tracing::info!( + phase, + examined = stats.canonical_records_examined, + main_removed = stats.main_payloads_removed, + tombstone_removed = stats.tombstone_payloads_removed, + metadata_removed = stats.metadata_rows_removed, + indefinitely_retained = stats.indefinitely_retained_disrespector_requests, + skipped = stats.stale_or_concurrent_records_skipped, + failures = stats.failures, + "Deletion-request cleanup completed" + ); +} + fn log_blacklist_parity(stats: BlacklistParityStats) { if stats.scanned_announcements > 0 || stats.matched_announcements > 0 { tracing::info!( diff --git a/src/nostr/lifecycle/deletion/service.rs b/src/nostr/lifecycle/deletion/service.rs index 79c8c5d..10f14f4 100644 --- a/src/nostr/lifecycle/deletion/service.rs +++ b/src/nostr/lifecycle/deletion/service.rs @@ -1,12 +1,40 @@ use anyhow::Result; use nostr_relay_builder::prelude::{ - nip62, Alphabet, Event, Filter, Kind, PublicKey, RelayUrl, SingleLetterTag, WritePolicyResult, + nip62, Alphabet, Event, Filter, Kind, PublicKey, RelayUrl, SingleLetterTag, Timestamp, + WritePolicyResult, }; use crate::nostr::events::RepositoryAnnouncement; -use crate::nostr::policy::{reject_error, reject_invalid, AnnouncementResult}; +use crate::nostr::lifecycle::{RequestClassification, RequestLifecycleRecord}; +use crate::nostr::policy::{duplicate, reject_error, reject_invalid, AnnouncementResult}; -use super::{DeletionContext, DeletionPolicy}; +use super::{DeletionContext, DeletionOutcome, DeletionPolicy}; + +pub(super) const UNSERVED_REPLAY_MESSAGE: &str = + "deletion request is no longer served; it will be re-served when used to reject an in-scope event"; + +pub(super) fn request_is_served_with_config( + ctx: &DeletionContext, + record: &RequestLifecycleRecord, + now: Timestamp, +) -> Result { + let (anchor, duration) = match record.last_used_at { + Some(used) => ( + used, + ctx.config + .deletion_request_retention_used_served_after_last_used(), + ), + None => ( + record.first_seen_at, + ctx.config.deletion_request_retention_unused_served(), + ), + }; + let deadline = anchor + .as_secs() + .checked_add(duration.as_secs()) + .ok_or_else(|| anyhow::anyhow!("deletion-request served retention deadline overflow"))?; + Ok(now.as_secs() < deadline) +} #[derive(Clone)] pub struct DeletionService { @@ -21,139 +49,251 @@ impl DeletionService { } pub async fn gate(&self, event: &Event) -> Option { - // 1. Vanished pubkey - if self - .ctx - .tombstones() - .is_pubkey_vanished(&event.pubkey) - .await - { - tracing::debug!( - event_id = %event.id.to_hex(), - author = %event.pubkey.to_hex(), - "Rejected event from vanished pubkey" - ); - return Some(reject_invalid("this pubkey has requested to vanish")); + // The operator's current archival-mode policy governs every retained + // request, regardless of how it was classified when received. + if self.ctx.config.deletion_request_disrespector { + return None; } - // 2. Deleted event id (author-bound: only the event's own author may - // have deleted it) - if self - .ctx - .tombstones() - .is_event_deleted(&event.id, &event.pubkey) - .await - { - tracing::debug!( - event_id = %event.id.to_hex(), - "Rejected re-submission of deleted event" - ); - return Some(reject_invalid("this event is deleted")); - } - - // 3. Deleted coordinate (replaceable / addressable events only) - if event.kind.is_replaceable() || event.kind.is_addressable() { - if let Some(coord) = Self::event_coordinate(event) { - if self - .ctx - .tombstones() - .is_coordinate_deleted(&coord, event.created_at) - .await - { - tracing::debug!( - event_id = %event.id.to_hex(), - coordinate = %coord, - "Rejected event whose coordinate was deleted" - ); - return Some(reject_invalid("this event is deleted")); - } + // Probe without the lifecycle transition lock. Most events have no + // matching deletion request, so serializing their database reads would + // turn this gate into a global relay-write mutex. + let candidates = match self.gate_candidates(event).await { + Ok(candidates) => candidates, + Err(error) => return Some(Self::gate_error("querying deletion candidates", error)), + }; + match self.select_gate_winner(event, candidates, Timestamp::now()) { + Ok(Some(_)) => {} + Ok(None) => return None, + Err(error) => { + return Some(Self::gate_error( + "evaluating deletion-request retention", + error, + )) } } - None - } + let tombstones = self.ctx.tombstones(); + let _lifecycle_guard = tombstones.lock_lifecycle().await; - /// Remove stale out-of-order kind-5 requests that targeted this event ID but - /// were authored by a different pubkey. - /// - /// Main-DB validation rejects cross-author `e` deletes when the target is - /// already known. For pre-emptive deletes, ownership is unknowable until the - /// target arrives. Once an accepted event establishes the real author, any - /// stored deletion requests from other authors are retroactively invalid and - /// should be removed from both the internal tombstone store and the served - /// deletion-request event stream. - pub async fn remove_stale_cross_author_event_deletions(&self, event: &Event) { - let target_id = event.id; - let author = event.pubkey; + // Rediscover under the transition lock instead of trusting the probe. + // Cleanup may have expired its winner while this task waited; a later + // matching request can still be live and must then become the winner. + // Only admissions that actually matched a live request pay this + // serialization cost. + let candidates = match self.gate_candidates(event).await { + Ok(candidates) => candidates, + Err(error) => return Some(Self::gate_error("querying deletion candidates", error)), + }; + let (winner, category, candidate_count) = + match self.select_gate_winner(event, candidates, Timestamp::now()) { + Ok(Some(winner)) => winner, + Ok(None) => return None, + Err(error) => { + return Some(Self::gate_error( + "evaluating deletion-request retention", + error, + )) + } + }; - match self - .ctx - .tombstones() - .remove_event_deletions_by_other_authors(&target_id, &author) + // Cleanup uses this same lock, so it cannot remove the Main copy + // selected by this admission while it is being promoted. + let Some(winner) = (match tombstones + .lifecycle_for_request_result(&winner.request.id) .await { - Ok(removed) if removed > 0 => tracing::info!( - target_id = %target_id.to_hex(), - author = %author.to_hex(), - removed, - "Removed stale cross-author deletion tombstone(s) after target arrival" - ), - Ok(_) => {} - Err(e) => tracing::warn!( - target_id = %target_id.to_hex(), - author = %author.to_hex(), - error = %e, - "Failed to remove stale cross-author deletion tombstones" - ), + Ok(record) => record, + Err(error) => { + return Some(Self::gate_error( + "re-reading deletion-request lifecycle", + error, + )) + } + }) else { + return Some(Self::gate_error( + "re-reading deletion-request lifecycle", + anyhow::anyhow!("request {} disappeared", winner.request.id), + )); + }; + match self.request_is_expired(&winner, Timestamp::now()) { + Ok(true) => return None, + Ok(false) => {} + Err(error) => { + return Some(Self::gate_error( + "evaluating deletion-request retention", + error, + )) + } } - - let filter = Filter::new() - .kind(Kind::EventDeletion) - .custom_tag(SingleLetterTag::lowercase(Alphabet::E), target_id.to_hex()); - - let stale_ids = match self.ctx.database().query(filter).await { - Ok(events) => events - .into_iter() - .filter(|deletion| deletion.pubkey != author) - .map(|deletion| deletion.id) - .collect::>(), - Err(e) => { - tracing::warn!( - target_id = %target_id.to_hex(), - author = %author.to_hex(), - error = %e, - "Failed to query stale cross-author deletion requests in main DB" - ); - return; + let previous_last_used_at = winner.last_used_at; + if let Err(error) = self.ctx.database().save_event(&winner.request).await { + return Some(Self::gate_error("promoting deletion request", error)); + } + let used_at = Timestamp::now(); + let updated = match tombstones + .mark_request_used_locked(&winner.request.id, used_at) + .await + { + Ok(Some(record)) if record.last_used_at >= Some(used_at) => record, + Ok(Some(record)) => { + return Some(Self::gate_error( + "verifying deletion-request lifecycle update", + anyhow::anyhow!( + "request {} retained an older last_used_at {:?}", + record.request.id, + record.last_used_at + ), + )); + } + Ok(None) => { + return Some(Self::gate_error( + "updating deletion-request lifecycle", + anyhow::anyhow!( + "request {} no longer has lifecycle metadata", + winner.request.id + ), + )); + } + Err(error) => { + return Some(Self::gate_error( + "updating deletion-request lifecycle", + error, + )) } }; - if stale_ids.is_empty() { - return; - } - - let removed = stale_ids.len(); - if let Err(e) = self - .ctx - .database() - .delete(Filter::new().ids(stale_ids)) - .await - { - tracing::warn!( - target_id = %target_id.to_hex(), - author = %author.to_hex(), - error = %e, - "Failed to remove stale cross-author deletion requests from main DB" - ); - return; - } - - tracing::info!( - target_id = %target_id.to_hex(), - author = %author.to_hex(), - removed, - "Removed stale cross-author deletion request(s) from main DB after target arrival" + tracing::debug!( + incoming_event_id = %event.id.to_hex(), + winning_request_id = %winner.request.id.to_hex(), + request_kind = winner.request.kind.as_u16(), + first_seen_at = winner.first_seen_at.as_secs(), + previous_last_used_at = ?previous_last_used_at.map(|timestamp| timestamp.as_secs()), + new_last_used_at = ?updated.last_used_at.map(|timestamp| timestamp.as_secs()), + candidate_count, + match_category = category, + "Rejected event through attributed deletion-request gate" ); + + Some(if winner.request.kind == Kind::RequestToVanish { + reject_invalid("this pubkey has requested to vanish") + } else { + reject_invalid("this event is deleted") + }) + } + + pub(crate) fn request_is_expired( + &self, + record: &RequestLifecycleRecord, + now: Timestamp, + ) -> Result { + let (anchor, served, additional) = match record.last_used_at { + Some(last_used_at) => ( + last_used_at, + self.ctx + .config + .deletion_request_retention_used_served_after_last_used(), + self.ctx + .config + .deletion_request_retention_used_unserved_gating_additional(), + ), + None => ( + record.first_seen_at, + self.ctx.config.deletion_request_retention_unused_served(), + self.ctx + .config + .deletion_request_retention_unused_unserved_gating_additional(), + ), + }; + let lifetime = served + .as_secs() + .checked_add(additional.as_secs()) + .ok_or_else(|| anyhow::anyhow!("deletion-request retention duration overflow"))?; + let deadline = anchor + .as_secs() + .checked_add(lifetime) + .ok_or_else(|| anyhow::anyhow!("deletion-request retention deadline overflow"))?; + if now.as_secs() >= deadline { + return Ok(true); + } + Ok(false) + } + + fn winner_order( + left: &RequestLifecycleRecord, + right: &RequestLifecycleRecord, + ) -> std::cmp::Ordering { + right + .last_used_at + .is_some() + .cmp(&left.last_used_at.is_some()) + .then_with(|| left.first_seen_at.cmp(&right.first_seen_at)) + .then_with(|| left.request.id.to_hex().cmp(&right.request.id.to_hex())) + } + + async fn gate_candidates( + &self, + event: &Event, + ) -> Result> { + let tombstones = self.ctx.tombstones(); + let mut candidates = tombstones + .vanish_candidates(&event.pubkey) + .await? + .into_iter() + .map(|record| (record, "vanish")) + .collect::>(); + candidates.extend( + tombstones + .event_deletion_candidates(&event.id, &event.pubkey) + .await? + .into_iter() + .map(|record| (record, "event-id")), + ); + + if event.kind.is_replaceable() || event.kind.is_addressable() { + if let Some(coord) = Self::event_coordinate(event) { + candidates.extend( + tombstones + .coordinate_deletion_candidates(&coord, event.created_at) + .await? + .into_iter() + .map(|record| (record, "coordinate")), + ); + } + } + Ok(candidates) + } + + fn select_gate_winner( + &self, + event: &Event, + candidates: Vec<(RequestLifecycleRecord, &'static str)>, + now: Timestamp, + ) -> Result> { + let mut eligible = std::collections::HashMap::new(); + for (record, category) in candidates { + // A deletion or vanish request must never prove the utility of its + // own gate. In particular, after cleanup moves an unused request + // to Tombstones, replaying its signed payload reaches this gate + // before duplicate handling and would otherwise select itself. + if record.request.id == event.id || self.request_is_expired(&record, now)? { + continue; + } + eligible + .entry(record.request.id) + .or_insert((record, category)); + } + + let candidate_count = eligible.len(); + Ok(eligible + .into_values() + .min_by(|(left, _), (right, _)| Self::winner_order(left, right)) + .map(|(record, category)| (record, category, candidate_count))) + } + + fn gate_error(context: &str, error: impl std::fmt::Display) -> WritePolicyResult { + tracing::error!(error = %error, "Deletion-request admission gate failed while {context}"); + reject_error(format!("internal error {context}: {error}")) } pub async fn handle_nip09(&self, event: &Event) -> WritePolicyResult { @@ -161,22 +301,58 @@ impl DeletionService { } pub async fn handle_vanish(&self, event: &Event) -> WritePolicyResult { - // Archival mode: store the vanish request but do not process it. - if self.ctx.config.deletion_request_disrespector { - tracing::info!( - event_id = %event.id.to_hex(), - author = %event.pubkey.to_hex(), - "Disrespector mode: storing NIP-62 vanish request without acting on it" - ); - return WritePolicyResult::Accept; - } - // Only honour requests that target this relay (or all relays). `domain` // is configured as an authority for GRASP matching, so derive both // `wss://` and `ws://` relay URL candidates when no scheme is present. // We still accept (store) well-formed kind-62 requests that target other // relays so clients get an OK for their event. let targets_relay = self.vanish_targets_this_relay(event); + let classification = if !targets_relay { + RequestClassification::NonTargetingNip62 + } else if self.ctx.config.deletion_request_disrespector { + RequestClassification::Disrespector + } else { + RequestClassification::LocallyActionable + }; + + // Keep the retention decision and lifecycle write atomic with cleanup. + // An exact replay during the unserved-but-gating interval must receive + // an OK duplicate response, not Accept: relay-builder persists Accept + // results back into Main and would make the request queryable again. + let tombstones = self.ctx.tombstones(); + let record_result = { + let _lifecycle_guard = tombstones.lock_lifecycle().await; + let existing = match tombstones.lifecycle_for_request_result(&event.id).await { + Ok(record) => record, + Err(error) => { + tracing::error!(event_id = %event.id.to_hex(), error = %error, "Failed to read NIP-62 vanish lifecycle"); + return reject_error(format!( + "internal error reading vanish lifecycle: {error}" + )); + } + }; + if let Some(record) = existing { + match self.request_is_served(&record, Timestamp::now()) { + Ok(false) => return duplicate(UNSERVED_REPLAY_MESSAGE), + Ok(true) => {} + Err(error) => { + tracing::error!(event_id = %event.id.to_hex(), error = %error, "Failed to evaluate NIP-62 vanish lifecycle retention"); + return reject_error(format!( + "internal error evaluating vanish lifecycle retention: {error}" + )); + } + } + } + tombstones + .record_request_locked(event, Timestamp::now(), classification) + .await + }; + if let Err(error) = record_result { + tracing::error!(event_id = %event.id.to_hex(), error = %error, "Failed to record NIP-62 vanish lifecycle"); + return reject_error(format!( + "internal error recording vanish lifecycle: {error}" + )); + } if !targets_relay { tracing::debug!( @@ -188,35 +364,89 @@ impl DeletionService { let author = event.pubkey; - // 1. Record the vanish so re-submission of the author's events is blocked. - if let Err(e) = self.ctx.tombstones().record_vanish(event).await { - tracing::error!(error = %e, author = %author.to_hex(), "Failed to record vanish tombstone"); - return reject_error(format!("internal error recording vanish: {e}")); + if self.ctx.config.deletion_request_disrespector { + let would_vanish = match self.policy.would_nip62_vanish_stored_data(event).await { + Ok(would_vanish) => would_vanish, + Err(error) => { + tracing::error!(event_id = %event.id.to_hex(), error = %error, "Failed to read-only evaluate NIP-62 vanish request"); + return reject_error(format!( + "internal error evaluating vanish request: {error}" + )); + } + }; + if would_vanish { + if let Err(result) = self.mark_vanish_request_used(event).await { + return result; + } + } + tracing::info!( + event_id = %event.id.to_hex(), + author = %author.to_hex(), + would_vanish, + "Disrespector mode: stored and read-only evaluated NIP-62 vanish request" + ); + return WritePolicyResult::Accept; } - if let Err(e) = self.policy.apply_nip62_vanish(event).await { - tracing::error!(error = %e, author = %author.to_hex(), "Failed to process vanished author's lifecycle deletion"); - return reject_error(format!("internal error processing vanish: {e}")); - } + let mut outcome = match self.policy.apply_nip62_vanish(event).await { + Ok(outcome) => outcome, + Err(e) => { + tracing::error!(error = %e, author = %author.to_hex(), "Failed to process vanished author's lifecycle deletion"); + return reject_error(format!("internal error processing vanish: {e}")); + } + }; // Evict the author's purgatory entries (and their bare repos). Served // data has already gone through the holding/archive lifecycle above; // purgatory entries are not live served data. - self.evict_author_from_purgatory(&author); + outcome.merge(self.evict_author_from_purgatory(&author)); + + if outcome.used() { + if let Err(result) = self.mark_vanish_request_used(event).await { + return result; + } + } tracing::info!( author = %author.to_hex(), + main_db_deleted = outcome.main_db_deleted, + purgatory_removed = outcome.purgatory_removed, + skipped = outcome.skipped, + failures = outcome.failures, "Processed NIP-62 request to vanish through deletion lifecycle" ); WritePolicyResult::Accept } + async fn mark_vanish_request_used( + &self, + event: &Event, + ) -> std::result::Result<(), WritePolicyResult> { + match self + .ctx + .tombstones() + .mark_request_used(&event.id, Timestamp::now()) + .await + { + Ok(Some(_)) => Ok(()), + Ok(None) => Err(reject_error( + "internal error updating vanish lifecycle: missing metadata", + )), + Err(error) => { + tracing::error!(event_id = %event.id.to_hex(), error = %error, "Failed to mark used NIP-62 vanish request"); + Err(reject_error(format!( + "internal error updating vanish lifecycle: {error}" + ))) + } + } + } + pub(crate) fn admission_hooks(&self) -> DeletionAdmissionHooks<'_> { DeletionAdmissionHooks { deletion: self } } - fn vanish_targets_this_relay(&self, event: &Event) -> bool { + pub(crate) fn vanish_targets_this_relay(&self, event: &Event) -> bool { let relay_urls = Self::relay_url_candidates(self.ctx.domain()); if relay_urls.is_empty() { @@ -230,6 +460,17 @@ impl DeletionService { }) } + pub(crate) async fn would_delete_stored_target(&self, event: &Event) -> anyhow::Result { + self.policy.would_delete_stored_target(event).await + } + + pub(crate) async fn would_nip62_vanish_stored_data( + &self, + event: &Event, + ) -> anyhow::Result { + self.policy.would_nip62_vanish_stored_data(event).await + } + fn relay_url_candidates(domain: &str) -> Vec { let domain = domain.trim().trim_end_matches('/'); if domain.is_empty() { @@ -482,7 +723,9 @@ impl DeletionService { kind == Kind::GitRepoAnnouncement || kind == Kind::RepoState } - fn evict_author_from_purgatory(&self, author: &PublicKey) { + fn evict_author_from_purgatory(&self, author: &PublicKey) -> DeletionOutcome { + let mut outcome = DeletionOutcome::default(); + let mut removed_ids = std::collections::HashSet::new(); // Announcements owned by this author. for (repo_id, _) in self.ctx.purgatory().announcements_for_sync() { // repo_id format: "30617:{pubkey_hex}:{identifier}" @@ -499,9 +742,15 @@ impl DeletionService { error = %e, "Failed to delete bare repository during vanish processing" ); + outcome.failures = outcome.failures.saturating_add(1); } } } + if let Some(entry) = self.ctx.purgatory().find_announcement(author, identifier) { + outcome.purgatory_removed = outcome + .purgatory_removed + .saturating_add(usize::from(removed_ids.insert(entry.event.id))); + } self.ctx.purgatory().remove_announcement(author, identifier); } @@ -509,12 +758,16 @@ impl DeletionService { for identifier in self.ctx.purgatory().get_all_identifiers() { for entry in self.ctx.purgatory().find_state(&identifier) { if entry.author == *author { + outcome.purgatory_removed = outcome + .purgatory_removed + .saturating_add(usize::from(removed_ids.insert(entry.event.id))); self.ctx .purgatory() .remove_state_event(&identifier, &entry.event.id); } } } + outcome } } @@ -598,3 +851,688 @@ impl DeletionAdmissionHooks<'_> { } } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::grasp06::receive::new_repo_init_locks; + use crate::nostr::lifecycle::{ + HoldingStore, ReplaceableHistoryStore, RepositoryLifecycle, Tombstones, + }; + use crate::purgatory::Purgatory; + + trait TombstoneTestExt { + async fn lifecycle_for_request( + &self, + request_id: &EventId, + ) -> Option; + } + + impl TombstoneTestExt for Tombstones { + async fn lifecycle_for_request( + &self, + request_id: &EventId, + ) -> Option { + self.lifecycle_for_request_result(request_id).await.unwrap() + } + } + use nostr_relay_builder::prelude::{EventBuilder, EventId, FinalizeEvent, Keys, Tag}; + use std::path::PathBuf; + use std::sync::Arc; + + fn context(config: crate::config::Config) -> DeletionContext { + DeletionContext::new( + "test.example.com", + Arc::new(nostr_memory::MemoryDatabase::unbounded()), + Tombstones::in_memory(), + HoldingStore::in_memory(), + RepositoryLifecycle::in_memory(), + ReplaceableHistoryStore::in_memory(), + PathBuf::new(), + Arc::new(Purgatory::new(PathBuf::new())), + config, + new_repo_init_locks(), + ) + } + + fn deletion(keys: &Keys, target: EventId) -> Event { + EventBuilder::new(Kind::EventDeletion, "") + .tags(vec![Tag::event(target)]) + .finalize(keys) + .unwrap() + } + + fn vanish(keys: &Keys, content: &str) -> Event { + EventBuilder::new(Kind::RequestToVanish, content) + .tags(vec![nostr_relay_builder::prelude::Tag::custom( + "relay", + vec!["ALL_RELAYS".to_string()], + )]) + .finalize(keys) + .unwrap() + } + + fn non_targeting_vanish(keys: &Keys) -> Event { + EventBuilder::new(Kind::RequestToVanish, "non-targeting") + .tags(vec![nostr_relay_builder::prelude::Tag::custom( + "relay", + vec!["wss://other.example".to_string()], + )]) + .finalize(keys) + .unwrap() + } + + #[test] + fn all_relays_vanish_target_takes_precedence_over_relay_specific_target() { + let service = DeletionService::new(context(crate::config::Config::for_testing())); + let request = EventBuilder::new(Kind::RequestToVanish, "multiple targets") + .tags(vec![ + Tag::custom("relay", vec!["wss://other.example".to_string()]), + Tag::custom("relay", vec!["ALL_RELAYS".to_string()]), + ]) + .finalize(&Keys::generate()) + .unwrap(); + + assert!( + service.vanish_targets_this_relay(&request), + "ALL_RELAYS must target this relay even when a relay-specific tag does not" + ); + } + + #[test] + fn retention_expiry_uses_exact_deadline_boundary() { + let service = DeletionService::new(context(crate::config::Config { + deletion_request_retention_unused_served_secs: 2, + deletion_request_retention_unused_unserved_gating_additional_secs: 3, + deletion_request_retention_used_served_after_last_used_secs: 2, + deletion_request_retention_used_unserved_gating_additional_secs: 3, + ..crate::config::Config::for_testing() + })); + let request = deletion(&Keys::generate(), EventId::all_zeros()); + let unused = RequestLifecycleRecord { + metadata_event_id: EventId::all_zeros(), + request: request.clone(), + first_seen_at: Timestamp::from_secs(10), + last_used_at: None, + classification: crate::nostr::lifecycle::RequestClassification::LocallyActionable, + }; + assert!(!service + .request_is_expired(&unused, Timestamp::from_secs(14)) + .unwrap()); + assert!(service + .request_is_expired(&unused, Timestamp::from_secs(15)) + .unwrap()); + let used = RequestLifecycleRecord { + last_used_at: Some(Timestamp::from_secs(20)), + ..unused + }; + assert!(!service + .request_is_expired(&used, Timestamp::from_secs(24)) + .unwrap()); + assert!(service + .request_is_expired(&used, Timestamp::from_secs(25)) + .unwrap()); + } + + #[test] + fn winner_order_prefers_used_then_first_seen_then_event_id() { + let keys = Keys::generate(); + let first_request = EventBuilder::new(Kind::EventDeletion, "first") + .tags(vec![Tag::event(EventId::all_zeros())]) + .finalize(&keys) + .unwrap(); + let second_request = EventBuilder::new(Kind::EventDeletion, "second") + .tags(vec![Tag::event(EventId::all_zeros())]) + .finalize(&keys) + .unwrap(); + let record = |request: Event, first_seen_at: u64, last_used_at: Option| { + RequestLifecycleRecord { + metadata_event_id: EventId::all_zeros(), + request, + first_seen_at: Timestamp::from_secs(first_seen_at), + last_used_at: last_used_at.map(Timestamp::from_secs), + classification: crate::nostr::lifecycle::RequestClassification::LocallyActionable, + } + }; + let unused_early = record(first_request.clone(), 10, None); + let unused_late = record(second_request.clone(), 20, None); + let used_late = record(second_request.clone(), 20, Some(21)); + assert_eq!( + DeletionService::winner_order(&used_late, &unused_early), + std::cmp::Ordering::Less + ); + assert_eq!( + DeletionService::winner_order(&unused_early, &unused_late), + std::cmp::Ordering::Less + ); + let same_time_left = record(first_request, 10, None); + let same_time_right = record(second_request, 10, None); + assert_eq!( + DeletionService::winner_order(&same_time_left, &same_time_right), + same_time_left + .request + .id + .to_hex() + .cmp(&same_time_right.request.id.to_hex()) + ); + } + + #[tokio::test] + async fn gate_deduplicates_matches_promotes_winner_and_updates_only_it() { + let ctx = context(crate::config::Config::for_testing()); + let service = DeletionService::new(ctx.clone()); + let keys = Keys::generate(); + let incoming = EventBuilder::new(Kind::TextNote, "target") + .finalize(&keys) + .unwrap(); + let older = deletion(&keys, incoming.id); + let newer = EventBuilder::new(Kind::EventDeletion, "newer") + .tags(vec![Tag::event(incoming.id)]) + .finalize(&keys) + .unwrap(); + let now = Timestamp::now().as_secs(); + ctx.tombstones + .record_request( + &older, + Timestamp::from_secs(now - 2), + crate::nostr::lifecycle::RequestClassification::LocallyActionable, + ) + .await + .unwrap(); + ctx.tombstones + .record_request( + &newer, + Timestamp::from_secs(now - 1), + crate::nostr::lifecycle::RequestClassification::LocallyActionable, + ) + .await + .unwrap(); + + assert!(service.gate(&incoming).await.is_some()); + assert!(ctx + .database + .query(Filter::new().id(older.id)) + .await + .unwrap() + .iter() + .any(|event| event.id == older.id)); + assert!(ctx + .tombstones + .lifecycle_for_request(&older.id) + .await + .unwrap() + .last_used_at + .is_some()); + assert_eq!( + ctx.tombstones + .lifecycle_for_request(&newer.id) + .await + .unwrap() + .last_used_at, + None + ); + } + + #[tokio::test] + async fn unmatched_gate_does_not_wait_for_lifecycle_transition_lock() { + let ctx = context(crate::config::Config::for_testing()); + let service = DeletionService::new(ctx.clone()); + let incoming = EventBuilder::new(Kind::TextNote, "unmatched") + .finalize(&Keys::generate()) + .unwrap(); + let _guard = ctx.tombstones.lock_lifecycle().await; + + let result = tokio::time::timeout( + std::time::Duration::from_millis(100), + service.gate(&incoming), + ) + .await + .expect("an unmatched event must not wait for lifecycle serialization"); + assert!(result.is_none()); + } + + #[tokio::test] + async fn gate_uses_a_live_later_request_when_an_earlier_request_has_expired() { + let ctx = context(crate::config::Config { + deletion_request_retention_unused_served_secs: 1, + deletion_request_retention_unused_unserved_gating_additional_secs: 1, + ..crate::config::Config::for_testing() + }); + let service = DeletionService::new(ctx.clone()); + let keys = Keys::generate(); + let incoming = EventBuilder::new(Kind::TextNote, "target") + .finalize(&keys) + .unwrap(); + let expired = deletion(&keys, incoming.id); + let live = EventBuilder::new(Kind::EventDeletion, "live fallback") + .tags(vec![Tag::event(incoming.id)]) + .finalize(&keys) + .unwrap(); + let now = Timestamp::now().as_secs(); + for (request, first_seen_at) in [(&expired, now - 3), (&live, now)] { + ctx.tombstones + .record_request( + request, + Timestamp::from_secs(first_seen_at), + RequestClassification::LocallyActionable, + ) + .await + .unwrap(); + } + + assert!(service.gate(&incoming).await.is_some()); + assert_eq!( + ctx.tombstones + .lifecycle_for_request(&expired.id) + .await + .unwrap() + .last_used_at, + None, + "expired requests must not receive gate-use credit" + ); + assert!( + ctx.tombstones + .lifecycle_for_request(&live.id) + .await + .unwrap() + .last_used_at + .is_some(), + "a later live request must still reject the incoming event" + ); + } + + #[tokio::test] + async fn disrespector_mode_bypasses_all_deletion_gates() { + let ctx = context(crate::config::Config { + deletion_request_disrespector: true, + ..crate::config::Config::for_testing() + }); + let service = DeletionService::new(ctx.clone()); + let keys = Keys::generate(); + let event = EventBuilder::new(Kind::TextNote, "target") + .finalize(&keys) + .unwrap(); + let request = deletion(&keys, event.id); + ctx.tombstones + .record_request( + &request, + Timestamp::now(), + crate::nostr::lifecycle::RequestClassification::LocallyActionable, + ) + .await + .unwrap(); + assert!(service.gate(&event).await.is_none()); + assert_eq!( + ctx.tombstones + .lifecycle_for_request(&request.id) + .await + .unwrap() + .last_used_at, + None + ); + } + + #[tokio::test] + async fn targeted_vanish_without_stored_data_is_actionable_and_unused() { + let ctx = context(crate::config::Config::for_testing()); + let service = DeletionService::new(ctx.clone()); + let request = vanish(&Keys::generate(), "empty"); + + assert!(matches!( + service.handle_vanish(&request).await, + WritePolicyResult::Accept + )); + let record = ctx + .tombstones + .lifecycle_for_request(&request.id) + .await + .unwrap(); + assert_eq!( + record.classification, + RequestClassification::LocallyActionable + ); + assert_eq!(record.last_used_at, None); + } + + #[tokio::test] + async fn targeted_vanish_marks_used_after_main_database_removal() { + let ctx = context(crate::config::Config::for_testing()); + let service = DeletionService::new(ctx.clone()); + let keys = Keys::generate(); + let target = EventBuilder::new(Kind::TextNote, "vanish me") + .finalize(&keys) + .unwrap(); + ctx.database.save_event(&target).await.unwrap(); + let request = vanish(&keys, "main-db"); + + assert!(matches!( + service.handle_vanish(&request).await, + WritePolicyResult::Accept + )); + assert!(ctx + .database + .event_by_id(&target.id) + .await + .unwrap() + .is_none()); + assert!(ctx + .tombstones + .lifecycle_for_request(&request.id) + .await + .unwrap() + .last_used_at + .is_some()); + } + + #[tokio::test] + async fn targeted_vanish_marks_used_after_purgatory_only_removal() { + let ctx = context(crate::config::Config::for_testing()); + let service = DeletionService::new(ctx.clone()); + let keys = Keys::generate(); + let state = EventBuilder::new(Kind::RepoState, "") + .tags(vec![nostr_relay_builder::prelude::Tag::identifier( + "purgatory-only", + )]) + .finalize(&keys) + .unwrap(); + ctx.purgatory.add_state( + state.clone(), + "purgatory-only".to_string(), + keys.public_key(), + false, + ); + let request = vanish(&keys, "purgatory-only"); + + assert!(matches!( + service.handle_vanish(&request).await, + WritePolicyResult::Accept + )); + assert!(ctx.purgatory.find_state("purgatory-only").is_empty()); + assert!(ctx + .tombstones + .lifecycle_for_request(&request.id) + .await + .unwrap() + .last_used_at + .is_some()); + } + + #[tokio::test] + async fn non_targeting_vanish_is_unused_and_never_gates() { + let ctx = context(crate::config::Config::for_testing()); + let service = DeletionService::new(ctx.clone()); + let keys = Keys::generate(); + let target = EventBuilder::new(Kind::TextNote, "keep me") + .finalize(&keys) + .unwrap(); + ctx.database.save_event(&target).await.unwrap(); + let request = non_targeting_vanish(&keys); + + assert!(matches!( + service.handle_vanish(&request).await, + WritePolicyResult::Accept + )); + assert!(ctx + .database + .event_by_id(&target.id) + .await + .unwrap() + .is_some()); + let record = ctx + .tombstones + .lifecycle_for_request(&request.id) + .await + .unwrap(); + assert_eq!( + record.classification, + RequestClassification::NonTargetingNip62 + ); + assert_eq!(record.last_used_at, None); + let later = EventBuilder::new(Kind::TextNote, "allowed") + .finalize(&keys) + .unwrap(); + assert!(service.gate(&later).await.is_none()); + } + + #[tokio::test] + async fn non_targeting_vanish_takes_precedence_over_disrespector_classification() { + let ctx = context(crate::config::Config { + deletion_request_disrespector: true, + ..crate::config::Config::for_testing() + }); + let service = DeletionService::new(ctx.clone()); + let request = non_targeting_vanish(&Keys::generate()); + + assert!(matches!( + service.handle_vanish(&request).await, + WritePolicyResult::Accept + )); + assert_eq!( + ctx.tombstones + .lifecycle_for_request(&request.id) + .await + .unwrap() + .classification, + RequestClassification::NonTargetingNip62 + ); + } + + #[tokio::test] + async fn disrespector_vanish_marks_main_database_and_purgatory_matches_used_without_mutating() { + let ctx = context(crate::config::Config { + deletion_request_disrespector: true, + ..crate::config::Config::for_testing() + }); + let service = DeletionService::new(ctx.clone()); + let keys = Keys::generate(); + let target = EventBuilder::new(Kind::TextNote, "preserve me") + .finalize(&keys) + .unwrap(); + ctx.database.save_event(&target).await.unwrap(); + let state = EventBuilder::new(Kind::RepoState, "") + .tags(vec![nostr_relay_builder::prelude::Tag::identifier( + "preserve-purgatory", + )]) + .finalize(&keys) + .unwrap(); + ctx.purgatory.add_state( + state.clone(), + "preserve-purgatory".to_string(), + keys.public_key(), + false, + ); + let request = vanish(&keys, "disrespector-data"); + + assert!(matches!( + service.handle_vanish(&request).await, + WritePolicyResult::Accept + )); + assert!(ctx + .database + .event_by_id(&target.id) + .await + .unwrap() + .is_some()); + assert!(ctx + .purgatory + .find_state("preserve-purgatory") + .iter() + .any(|entry| entry.event.id == state.id)); + let record = ctx + .tombstones + .lifecycle_for_request(&request.id) + .await + .unwrap(); + assert_eq!(record.classification, RequestClassification::Disrespector); + assert!(record.last_used_at.is_some()); + } + + #[tokio::test] + async fn disrespector_vanish_marks_purgatory_only_match_used_without_mutating() { + let ctx = context(crate::config::Config { + deletion_request_disrespector: true, + ..crate::config::Config::for_testing() + }); + let service = DeletionService::new(ctx.clone()); + let keys = Keys::generate(); + let state = EventBuilder::new(Kind::RepoState, "") + .tags(vec![nostr_relay_builder::prelude::Tag::identifier( + "disrespector-purgatory", + )]) + .finalize(&keys) + .unwrap(); + ctx.purgatory.add_state( + state.clone(), + "disrespector-purgatory".to_string(), + keys.public_key(), + false, + ); + let request = vanish(&keys, "disrespector-purgatory-only"); + + assert!(matches!( + service.handle_vanish(&request).await, + WritePolicyResult::Accept + )); + assert!(ctx + .purgatory + .find_state("disrespector-purgatory") + .iter() + .any(|entry| entry.event.id == state.id)); + assert!(ctx + .tombstones + .lifecycle_for_request(&request.id) + .await + .unwrap() + .last_used_at + .is_some()); + } + + #[tokio::test] + async fn disrespector_vanish_noop_remains_unused() { + let ctx = context(crate::config::Config { + deletion_request_disrespector: true, + ..crate::config::Config::for_testing() + }); + let service = DeletionService::new(ctx.clone()); + let request = vanish(&Keys::generate(), "disrespector-empty"); + + assert!(matches!( + service.handle_vanish(&request).await, + WritePolicyResult::Accept + )); + assert_eq!( + ctx.tombstones + .lifecycle_for_request(&request.id) + .await + .unwrap() + .last_used_at, + None + ); + } + + #[tokio::test] + async fn exact_vanish_replay_keeps_original_first_seen_at() { + let ctx = context(crate::config::Config::for_testing()); + let service = DeletionService::new(ctx.clone()); + let request = vanish(&Keys::generate(), "replay"); + let first_seen_at = Timestamp::now(); + ctx.tombstones + .record_request( + &request, + first_seen_at, + RequestClassification::LocallyActionable, + ) + .await + .unwrap(); + + assert!(matches!( + service.handle_vanish(&request).await, + WritePolicyResult::Accept + )); + assert_eq!( + ctx.tombstones + .lifecycle_for_request(&request.id) + .await + .unwrap() + .first_seen_at, + first_seen_at + ); + } + + #[tokio::test] + async fn vanish_gate_promotes_and_updates_only_one_deterministic_request() { + let ctx = context(crate::config::Config::for_testing()); + let service = DeletionService::new(ctx.clone()); + let keys = Keys::generate(); + let older = vanish(&keys, "older"); + let newer = vanish(&keys, "newer"); + let now = Timestamp::now().as_secs(); + for (request, first_seen_at) in [(&older, now - 2), (&newer, now - 1)] { + ctx.tombstones + .record_request( + request, + Timestamp::from_secs(first_seen_at), + RequestClassification::LocallyActionable, + ) + .await + .unwrap(); + } + let incoming = EventBuilder::new(Kind::TextNote, "blocked") + .finalize(&keys) + .unwrap(); + + assert!(service.gate(&incoming).await.is_some()); + assert!(ctx.database.event_by_id(&older.id).await.unwrap().is_some()); + assert!(ctx + .tombstones + .lifecycle_for_request(&older.id) + .await + .unwrap() + .last_used_at + .is_some()); + assert_eq!( + ctx.tombstones + .lifecycle_for_request(&newer.id) + .await + .unwrap() + .last_used_at, + None + ); + } + + #[tokio::test] + async fn coordinate_request_with_insufficient_cutoff_never_receives_use_credit() { + let ctx = context(crate::config::Config::for_testing()); + let service = DeletionService::new(ctx.clone()); + let keys = Keys::generate(); + let coordinate = format!("30617:{}:repo", keys.public_key().to_hex()); + let request = EventBuilder::new(Kind::EventDeletion, "") + .tags(vec![Tag::custom("a", vec![coordinate])]) + .custom_created_at(Timestamp::from_secs(100)) + .finalize(&keys) + .unwrap(); + ctx.tombstones + .record_request( + &request, + Timestamp::from_secs(10), + RequestClassification::LocallyActionable, + ) + .await + .unwrap(); + let incoming = EventBuilder::new(Kind::GitRepoAnnouncement, "") + .tags(vec![Tag::identifier("repo")]) + .custom_created_at(Timestamp::from_secs(101)) + .finalize(&keys) + .unwrap(); + + assert!(service.gate(&incoming).await.is_none()); + assert_eq!( + ctx.tombstones + .lifecycle_for_request(&request.id) + .await + .unwrap() + .last_used_at, + None + ); + } +} diff --git a/src/nostr/lifecycle/deletion/startup.rs b/src/nostr/lifecycle/deletion/startup.rs index 2c99caa..44cdad8 100644 --- a/src/nostr/lifecycle/deletion/startup.rs +++ b/src/nostr/lifecycle/deletion/startup.rs @@ -1,8 +1,11 @@ use nostr::nips::nip19::ToBech32; -use nostr_relay_builder::prelude::{Filter, Kind, PublicKey, Timestamp}; +use std::collections::HashMap; + +use anyhow::Result; +use nostr_relay_builder::prelude::{Event, Filter, Kind, PublicKey, Timestamp}; use crate::nostr::events::RepositoryAnnouncement; -use crate::nostr::lifecycle::DeletionSource; +use crate::nostr::lifecycle::{DeletionSource, RequestClassification}; use super::DeletionService; @@ -48,12 +51,30 @@ pub struct WhitelistRestoreStats { #[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] pub struct StartupReconciliationStats { + pub request_lifecycle: RequestLifecycleStartupStats, pub blacklist_parity: BlacklistParityStats, pub blacklist_restore: BlacklistRestoreStats, pub whitelist_parity: WhitelistParityStats, pub whitelist_restore: WhitelistRestoreStats, } +/// Startup migration and policy reconciliation counters for retained deletion +/// requests. A failure is returned to the caller rather than represented as a +/// successful zero-count pass. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub struct RequestLifecycleStartupStats { + pub main_payloads_scanned: usize, + pub tombstone_payloads_scanned: usize, + pub unique_requests_discovered: usize, + pub requests_migrated: usize, + pub valid_metadata_existing: usize, + pub requests_reclassified: usize, + pub disrespector_requests_newly_used: usize, + pub main_requests_promoted: usize, + pub main_requests_removed: usize, + pub failures: usize, +} + // --------------------------------------------------------------------------- // DeletionService startup pass implementations // --------------------------------------------------------------------------- @@ -63,22 +84,216 @@ impl DeletionService { /// /// Ordering is intentionally owned here so relay startup only depends on a /// single facade while deletion internals decide sequencing: - /// 1) blacklist parity delete - /// 2) blacklist auto-restore - /// 3) whitelist parity delete - /// 4) whitelist restore - pub async fn run_startup_reconciliation(&self) -> StartupReconciliationStats { + /// 1) deletion request lifecycle migration and reconciliation + /// 2) blacklist parity delete + /// 3) blacklist auto-restore + /// 4) whitelist parity delete + /// 5) whitelist restore + pub async fn run_startup_reconciliation(&self) -> Result { + let request_lifecycle = self + .run_request_lifecycle_startup_reconciliation(Timestamp::now()) + .await?; let blacklist_parity = self.run_startup_blacklist_parity_pass().await; let blacklist_restore = self.run_startup_blacklist_restore_pass().await; let whitelist_parity = self.run_startup_whitelist_parity_pass().await; let whitelist_restore = self.run_startup_whitelist_restore_pass().await; - StartupReconciliationStats { + Ok(StartupReconciliationStats { + request_lifecycle, blacklist_parity, blacklist_restore, whitelist_parity, whitelist_restore, + }) + } + + /// Migrate historical request payloads and reconcile their current-policy + /// serving state. The timestamp is supplied for deterministic tests and is + /// captured once by the production startup pass. + pub(crate) async fn run_request_lifecycle_startup_reconciliation( + &self, + now: Timestamp, + ) -> Result { + let main = self.request_payloads_from_main().await?; + let tombstones = self.ctx.tombstones(); + // This bulk query resolves all payload/metadata pairs in one pass. Do + // not call lifecycle_for_request_result once per request here: startup + // runs before serving traffic and must remain linear in retained data. + // Keep payloads lacking valid metadata in the migration set too. + let tombstone_payloads = tombstones.request_payloads().await?; + let tombstone_payloads_scanned = tombstone_payloads.len(); + let existing_lifecycles = tombstones + .lifecycle_records_for_payloads_result(tombstone_payloads.clone()) + .await?; + let mut existing_by_request: HashMap<_, _> = existing_lifecycles + .into_iter() + .map(|record| (record.request.id, record)) + .collect(); + let mut stats = RequestLifecycleStartupStats { + main_payloads_scanned: main.len(), + tombstone_payloads_scanned, + ..Default::default() + }; + let mut requests: HashMap<_, Event> = HashMap::new(); + for request in main.into_iter().chain(tombstone_payloads) { + requests.entry(request.id).or_insert(request); } + stats.unique_requests_discovered = requests.len(); + + for request in requests.into_values() { + let existing = existing_by_request.remove(&request.id); + let mut record = match existing { + Some(record) => { + stats.valid_metadata_existing += 1; + record + } + None => { + let classification = self.current_classification(&request); + let record = self + .ctx + .tombstones() + .record_request(&request, now, classification) + .await?; + self.promote_request(&request, &mut stats).await?; + stats.requests_migrated += 1; + record + } + }; + + let desired = self.current_classification(&record.request); + if record.classification != desired { + record = self + .ctx + .tombstones() + .reclassify_request(&record.request.id, desired) + .await? + .ok_or_else(|| { + anyhow::anyhow!( + "lifecycle disappeared during reclassification: {}", + record.request.id + ) + })?; + stats.requests_reclassified += 1; + } + + let targeting = record.request.kind == Kind::EventDeletion + || self.vanish_targets_this_relay(&record.request); + if self.ctx.config.deletion_request_disrespector + && targeting + && record.last_used_at.is_none() + { + let would_use = if record.request.kind == Kind::EventDeletion { + self.would_delete_stored_target(&record.request).await? + } else { + self.would_nip62_vanish_stored_data(&record.request).await? + }; + if would_use { + record = self + .ctx + .tombstones() + .mark_request_used(&record.request.id, now) + .await? + .ok_or_else(|| { + anyhow::anyhow!( + "lifecycle disappeared while marking used: {}", + record.request.id + ) + })?; + stats.disrespector_requests_newly_used += 1; + } + } + + let serve_indefinitely = self.ctx.config.deletion_request_disrespector + && targeting + && record.last_used_at.is_some(); + if serve_indefinitely || self.request_is_served(&record, now)? { + self.promote_request(&record.request, &mut stats).await?; + } else { + self.remove_request_from_main(&record.request, &mut stats) + .await?; + } + } + tracing::info!( + main_payloads_scanned = stats.main_payloads_scanned, + tombstone_payloads_scanned = stats.tombstone_payloads_scanned, + unique_requests = stats.unique_requests_discovered, + migrated = stats.requests_migrated, + existing_metadata = stats.valid_metadata_existing, + reclassified = stats.requests_reclassified, + disrespector_newly_used = stats.disrespector_requests_newly_used, + promoted = stats.main_requests_promoted, + removed = stats.main_requests_removed, + failures = stats.failures, + "Deletion-request startup lifecycle reconciliation completed" + ); + Ok(stats) + } + + async fn request_payloads_from_main(&self) -> Result> { + let mut requests = Vec::new(); + for kind in [Kind::EventDeletion, Kind::RequestToVanish] { + requests.extend( + self.ctx + .database() + .query(Filter::new().kind(kind)) + .await + .map_err(|e| { + anyhow::anyhow!( + "Failed to list main database deletion-request payloads: {e}" + ) + })?, + ); + } + Ok(requests) + } + + fn current_classification(&self, request: &Event) -> RequestClassification { + if request.kind == Kind::RequestToVanish && !self.vanish_targets_this_relay(request) { + RequestClassification::NonTargetingNip62 + } else if self.ctx.config.deletion_request_disrespector { + RequestClassification::Disrespector + } else { + RequestClassification::LocallyActionable + } + } + + async fn promote_request( + &self, + request: &Event, + stats: &mut RequestLifecycleStartupStats, + ) -> Result<()> { + let exists = self + .ctx + .database() + .event_by_id(&request.id) + .await? + .is_some(); + if !exists { + self.ctx.database().save_event(request).await?; + stats.main_requests_promoted += 1; + } + Ok(()) + } + + async fn remove_request_from_main( + &self, + request: &Event, + stats: &mut RequestLifecycleStartupStats, + ) -> Result<()> { + let exists = self + .ctx + .database() + .event_by_id(&request.id) + .await? + .is_some(); + if exists { + self.ctx + .database() + .delete(Filter::new().ids(vec![request.id])) + .await?; + stats.main_requests_removed += 1; + } + Ok(()) } /// Startup-only blacklist parity pass. @@ -475,3 +690,318 @@ impl DeletionService { stats } } + +#[cfg(test)] +mod tests { + use std::path::PathBuf; + use std::sync::Arc; + + use nostr_relay_builder::prelude::{EventBuilder, FinalizeEvent, Keys, Tag}; + + use super::super::DeletionContext; + use super::*; + use crate::grasp06::receive::new_repo_init_locks; + use crate::nostr::lifecycle::{ + HoldingStore, ReplaceableHistoryStore, RepositoryLifecycle, Tombstones, + }; + use crate::purgatory::Purgatory; + + fn service(disrespector: bool) -> DeletionService { + let db = Arc::new(nostr_memory::MemoryDatabase::unbounded()); + let config = crate::config::Config { + deletion_request_disrespector: disrespector, + deletion_request_retention_unused_served_secs: 10, + deletion_request_retention_unused_unserved_gating_additional_secs: 5, + deletion_request_retention_used_served_after_last_used_secs: 20, + ..crate::config::Config::for_testing() + }; + DeletionService::new(DeletionContext::new( + "test.example.com", + db, + Tombstones::in_memory(), + HoldingStore::in_memory(), + RepositoryLifecycle::in_memory(), + ReplaceableHistoryStore::in_memory(), + PathBuf::new(), + Arc::new(Purgatory::new(PathBuf::new())), + config, + new_repo_init_locks(), + )) + } + + fn deletion(keys: &Keys) -> Event { + EventBuilder::new(Kind::EventDeletion, "") + .finalize(keys) + .unwrap() + } + + #[tokio::test] + async fn migrates_main_only_request_with_fresh_probation_idempotently() { + let service = service(false); + let request = deletion(&Keys::generate()); + service.ctx.database.save_event(&request).await.unwrap(); + + let stats = service + .run_request_lifecycle_startup_reconciliation(Timestamp::from_secs(100)) + .await + .unwrap(); + assert_eq!(stats.requests_migrated, 1); + assert_eq!(stats.main_requests_promoted, 0); + let record = service + .ctx + .tombstones() + .lifecycle_for_request_result(&request.id) + .await + .unwrap() + .unwrap(); + assert_eq!(record.first_seen_at, Timestamp::from_secs(100)); + assert_eq!( + record.classification, + RequestClassification::LocallyActionable + ); + + let rerun = service + .run_request_lifecycle_startup_reconciliation(Timestamp::from_secs(200)) + .await + .unwrap(); + assert_eq!(rerun.requests_migrated, 0); + assert_eq!( + service + .ctx + .tombstones() + .lifecycle_for_request_result(&request.id) + .await + .unwrap() + .unwrap() + .first_seen_at, + Timestamp::from_secs(100) + ); + } + + #[tokio::test] + async fn tombstone_only_request_is_migrated_and_promoted() { + let service = service(false); + let request = deletion(&Keys::generate()); + service + .ctx + .tombstones() + .save_request_payload_without_metadata(&request) + .await + .unwrap(); + + let stats = service + .run_request_lifecycle_startup_reconciliation(Timestamp::from_secs(100)) + .await + .unwrap(); + assert_eq!(stats.requests_migrated, 1); + assert_eq!(stats.main_requests_promoted, 1); + assert!(service + .ctx + .database + .event_by_id(&request.id) + .await + .unwrap() + .is_some()); + } + + #[tokio::test] + async fn startup_reconciles_thousands_of_retained_lifecycles() { + // Keep this large enough that a per-request full payload scan is a + // visible regression (the former implementation inspected millions of + // events), while remaining small enough for the unit-test suite. + const REQUEST_COUNT: usize = 2_000; + + let service = service(false); + let keys = Keys::generate(); + for sequence in 0..REQUEST_COUNT { + let request = EventBuilder::new(Kind::EventDeletion, sequence.to_string()) + .finalize(&keys) + .unwrap(); + service + .ctx + .tombstones() + .record_request( + &request, + Timestamp::from_secs(100), + RequestClassification::LocallyActionable, + ) + .await + .unwrap(); + } + + let stats = service + .run_request_lifecycle_startup_reconciliation(Timestamp::from_secs(101)) + .await + .unwrap(); + + assert_eq!(stats.tombstone_payloads_scanned, REQUEST_COUNT); + assert_eq!(stats.unique_requests_discovered, REQUEST_COUNT); + assert_eq!(stats.valid_metadata_existing, REQUEST_COUNT); + assert_eq!(stats.requests_migrated, 0); + } + + #[tokio::test] + async fn reclassifies_used_request_without_erasing_timestamps() { + let service = service(true); + let request = deletion(&Keys::generate()); + service + .ctx + .tombstones() + .record_request( + &request, + Timestamp::from_secs(10), + RequestClassification::LocallyActionable, + ) + .await + .unwrap(); + service + .ctx + .tombstones() + .mark_request_used(&request.id, Timestamp::from_secs(20)) + .await + .unwrap(); + + service + .run_request_lifecycle_startup_reconciliation(Timestamp::from_secs(30)) + .await + .unwrap(); + let record = service + .ctx + .tombstones() + .lifecycle_for_request_result(&request.id) + .await + .unwrap() + .unwrap(); + assert_eq!(record.first_seen_at, Timestamp::from_secs(10)); + assert_eq!(record.last_used_at, Some(Timestamp::from_secs(20))); + assert_eq!(record.classification, RequestClassification::Disrespector); + assert!(service + .ctx + .database + .event_by_id(&request.id) + .await + .unwrap() + .is_some()); + } + + #[tokio::test] + async fn expired_unused_disrespector_request_is_credited_when_target_is_present() { + let service = service(true); + let keys = Keys::generate(); + let target = EventBuilder::new(Kind::TextNote, "target") + .finalize(&keys) + .unwrap(); + let request = EventBuilder::new(Kind::EventDeletion, "") + .tags(vec![Tag::event(target.id)]) + .finalize(&keys) + .unwrap(); + service.ctx.database().save_event(&target).await.unwrap(); + service + .ctx + .tombstones() + .record_request( + &request, + Timestamp::from_secs(0), + RequestClassification::Disrespector, + ) + .await + .unwrap(); + + let stats = service + .run_request_lifecycle_startup_reconciliation(Timestamp::from_secs(15)) + .await + .unwrap(); + assert_eq!(stats.disrespector_requests_newly_used, 1); + let record = service + .ctx + .tombstones() + .lifecycle_for_request_result(&request.id) + .await + .unwrap() + .expect("request is retained after receiving use credit"); + assert_eq!(record.last_used_at, Some(Timestamp::from_secs(15))); + assert!(service + .ctx + .database() + .event_by_id(&request.id) + .await + .unwrap() + .is_some()); + + let cleanup = service + .cleanup_expired_requests(Timestamp::from_secs(15)) + .await + .unwrap(); + assert_eq!(cleanup.indefinitely_retained_disrespector_requests, 1); + assert!(service + .ctx + .tombstones() + .lifecycle_for_request_result(&request.id) + .await + .unwrap() + .is_some()); + } + + #[tokio::test] + async fn mode_round_trip_reclassifies_without_changing_lifecycle_timestamps() { + let normal = service(false); + let request = deletion(&Keys::generate()); + normal + .ctx + .tombstones() + .record_request( + &request, + Timestamp::from_secs(10), + RequestClassification::LocallyActionable, + ) + .await + .unwrap(); + normal + .ctx + .tombstones() + .mark_request_used(&request.id, Timestamp::from_secs(20)) + .await + .unwrap(); + + let mut disrespector_context = normal.ctx.clone(); + disrespector_context.config.deletion_request_disrespector = true; + let disrespector = DeletionService::new(disrespector_context); + let entering = disrespector + .run_request_lifecycle_startup_reconciliation(Timestamp::from_secs(30)) + .await + .unwrap(); + assert_eq!(entering.requests_reclassified, 1); + let entering_record = disrespector + .ctx + .tombstones() + .lifecycle_for_request_result(&request.id) + .await + .unwrap() + .unwrap(); + assert_eq!(entering_record.first_seen_at, Timestamp::from_secs(10)); + assert_eq!(entering_record.last_used_at, Some(Timestamp::from_secs(20))); + assert_eq!( + entering_record.classification, + RequestClassification::Disrespector + ); + + let leaving = normal + .run_request_lifecycle_startup_reconciliation(Timestamp::from_secs(40)) + .await + .unwrap(); + assert_eq!(leaving.requests_reclassified, 1); + let leaving_record = normal + .ctx + .tombstones() + .lifecycle_for_request_result(&request.id) + .await + .unwrap() + .unwrap(); + assert_eq!(leaving_record.first_seen_at, Timestamp::from_secs(10)); + assert_eq!(leaving_record.last_used_at, Some(Timestamp::from_secs(20))); + assert_eq!( + leaving_record.classification, + RequestClassification::LocallyActionable + ); + } +} diff --git a/src/nostr/lifecycle/tombstones.rs b/src/nostr/lifecycle/tombstones.rs index 3e9b8d9..fa9f2a1 100644 --- a/src/nostr/lifecycle/tombstones.rs +++ b/src/nostr/lifecycle/tombstones.rs @@ -24,8 +24,9 @@ //! ## How it works //! //! Rather than inventing a bespoke key-value schema, we persist the **deletion -//! and vanish request events themselves** (kind 5 and kind 62) in a dedicated -//! nostr database and derive the deleted/vanished state by querying it: +//! and vanish request events themselves** (kind 5 and kind 62), plus private +//! relay-generated lifecycle metadata, in a dedicated nostr database. Deleted / +//! vanished state is derived only from the original request payloads: //! //! - a kind-5 with an `e` tag → that event id is deleted //! - a kind-5 with an `a` tag → that coordinate is deleted up to the kind-5's @@ -40,42 +41,88 @@ //! request its own vanish) is enforced by the caller *before* recording, so any //! request stored here is already authoritative. -use std::collections::HashSet; +use std::collections::HashMap; use std::path::Path; use std::sync::Arc; +use tokio::sync::{Mutex, MutexGuard}; + use nostr_lmdb::NostrLmdb; use nostr_memory::MemoryDatabase; use nostr_relay_builder::prelude::{ - Alphabet, Event, EventId, Filter, Kind, NostrDatabase, PublicKey, SingleLetterTag, Timestamp, + Alphabet, Event, EventBuilder, EventId, Filter, FinalizeEvent, Keys, Kind, NostrDatabase, + PublicKey, SingleLetterTag, Tag, Timestamp, }; /// Directory name (under `relay_data_path`) for the LMDB tombstone database. const TOMBSTONE_DIR: &str = "tombstones"; -#[derive(Debug, Default)] -struct DeletionTargets { - e_ids: HashSet, - a_coordinates: HashSet, +/// Internal metadata event kind stored alongside deletion-request payloads. +pub const TOMBSTONE_REQUEST_METADATA_KIND: u16 = 9907; +/// Tag containing the relay-observed first receipt time for a request. +pub const TOMBSTONE_FIRST_SEEN_AT_TAG: &str = "tombstone-first-seen-at"; +/// Tag containing the most recent successful use time for a request. +pub const TOMBSTONE_LAST_USED_AT_TAG: &str = "tombstone-last-used-at"; +/// Tag containing the relay's deletion-request classification. +pub const TOMBSTONE_REQUEST_CLASSIFICATION_TAG: &str = "tombstone-request-classification"; + +/// Bound the number of request IDs in a metadata `#e` query so a target with +/// many deletion requests cannot create an unbounded database filter. +const METADATA_REQUEST_ID_QUERY_CHUNK_SIZE: usize = 256; + +/// Relay policy classification retained with a deletion request's lifecycle. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum RequestClassification { + LocallyActionable, + NonTargetingNip62, + Disrespector, } -impl DeletionTargets { - fn is_empty(&self) -> bool { - self.e_ids.is_empty() && self.a_coordinates.is_empty() +impl RequestClassification { + fn as_str(self) -> &'static str { + match self { + Self::LocallyActionable => "locally-actionable", + Self::NonTargetingNip62 => "non-targeting-nip62", + Self::Disrespector => "disrespector", + } } - fn covers( - &self, - newer_created_at: Timestamp, - older: &Self, - older_created_at: Timestamp, - ) -> bool { - older.e_ids.is_subset(&self.e_ids) - && older.a_coordinates.is_subset(&self.a_coordinates) - && (older.a_coordinates.is_empty() || newer_created_at >= older_created_at) + fn parse(value: &str) -> Option { + match value { + "locally-actionable" => Some(Self::LocallyActionable), + "non-targeting-nip62" => Some(Self::NonTargetingNip62), + "disrespector" => Some(Self::Disrespector), + _ => None, + } } } +/// The canonical lifecycle state for one original kind-5 or kind-62 request. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct RequestLifecycleRecord { + pub metadata_event_id: EventId, + pub request: Event, + pub first_seen_at: Timestamp, + pub last_used_at: Option, + pub classification: RequestClassification, +} + +/// Counts returned after permanently removing one request lifecycle. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub struct RequestLifecycleDeletionStats { + pub payloads_deleted: usize, + pub metadata_deleted: usize, +} + +#[derive(Debug, Clone, Copy)] +struct ParsedMetadata { + id: EventId, + created_at: Timestamp, + first_seen_at: Timestamp, + last_used_at: Option, + classification: RequestClassification, +} + /// Persistent record of NIP-09 deletions and NIP-62 vanish requests. /// /// Backed by its own nostr database so the deleted/vanished state survives @@ -83,6 +130,12 @@ impl DeletionTargets { #[derive(Clone)] pub struct Tombstones { db: Arc, + metadata_signer: Keys, + /// Serializes lifecycle metadata changes with Main-database transitions + /// performed by `DeletionService`. This is intentionally one simple lock: + /// request lifecycles are a small control plane and correctness matters more + /// than parallel metadata writes. + lifecycle_lock: Arc>, } impl std::fmt::Debug for Tombstones { @@ -95,8 +148,8 @@ impl Tombstones { /// Open a persistent (LMDB) tombstone store under `relay_data_path`. /// /// The database lives at `/tombstones`. NIP-09 / NIP-62 - /// auto-processing is explicitly disabled on this database: it is used purely - /// as a raw, append-only record of deletion/vanish request events. + /// auto-processing is explicitly disabled on this database: requests and + /// relay-private lifecycle metadata are interpreted explicitly by this store. pub async fn open_lmdb(relay_data_path: &Path) -> anyhow::Result { let path = relay_data_path.join(TOMBSTONE_DIR); std::fs::create_dir_all(&path).map_err(|e| { @@ -118,293 +171,661 @@ impl Tombstones { anyhow::anyhow!("Failed to open tombstone LMDB at {}: {}", path.display(), e) })?; - Ok(Self { db: Arc::new(db) }) + Ok(Self { + db: Arc::new(db), + metadata_signer: Keys::generate(), + lifecycle_lock: Arc::new(Mutex::new(())), + }) } /// Create an in-memory tombstone store (used with the memory backend and in /// tests). Not persistent. pub fn in_memory() -> Self { Self { - db: Arc::new(MemoryDatabase::unbounded()), + db: Arc::new( + MemoryDatabase::builder() + .process_nip09(false) + .process_nip62(false) + .build(), + ), + metadata_signer: Keys::generate(), + lifecycle_lock: Arc::new(Mutex::new(())), } } - /// Record a NIP-09 deletion request (kind 5). - /// - /// The caller MUST have already validated author ownership of the targets. - /// Storing the event makes the deletions durable and queryable. - pub async fn record_deletion(&self, event: &Event) -> anyhow::Result<()> { - debug_assert_eq!(event.kind, Kind::EventDeletion); + /// Acquire the lifecycle transition lock. Callers that also transition the + /// Main database must hold this across the Main operation and the matching + /// metadata update/removal. + pub(crate) async fn lock_lifecycle(&self) -> MutexGuard<'_, ()> { + self.lifecycle_lock.lock().await + } + + #[cfg(test)] + pub(crate) async fn save_request_payload_without_metadata( + &self, + event: &Event, + ) -> anyhow::Result<()> { self.db .save_event(event) .await - .map_err(|e| anyhow::anyhow!("Failed to record deletion tombstone: {e}"))?; - Ok(()) + .map(|_| ()) + .map_err(|error| anyhow::anyhow!("Failed to save test request payload: {error}")) } - /// 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) - } - - /// Return older same-author kind-5 request IDs whose actionable target set - /// is fully subsumed by `event`. - /// - /// This supports deletion-request compaction. For example, if a client first - /// publishes `kind:5` with `a=30617::repo` at t=1000 and later - /// publishes the same coordinate at t=2000, the t=2000 request preserves all - /// deletion semantics of the older request and extends the coordinate cutoff. - /// The older request can therefore be removed from both the tombstone store - /// and the served deletion-request stream. - pub async fn superseded_deletion_ids(&self, event: &Event) -> anyhow::Result> { - debug_assert_eq!(event.kind, Kind::EventDeletion); - - let newer_targets = Self::actionable_targets(event); - if newer_targets.is_empty() { - return Ok(Vec::new()); - } - - let candidates = self - .db - .query(Filter::new().kind(Kind::EventDeletion).author(event.pubkey)) + #[cfg(test)] + pub(crate) async fn remove_request_payload_without_metadata( + &self, + request_id: EventId, + ) -> anyhow::Result<()> { + self.db + .delete(Filter::new().ids(vec![request_id])) .await - .map_err(|e| anyhow::anyhow!("Failed to query deletion tombstones: {e}"))?; - - Ok(candidates - .into_iter() - .filter(|candidate| candidate.id != event.id) - .filter(|candidate| { - let older_targets = Self::actionable_targets(candidate); - !older_targets.is_empty() - && newer_targets.covers(event.created_at, &older_targets, candidate.created_at) - }) - .map(|candidate| candidate.id) - .collect()) + .map_err(|error| anyhow::anyhow!("Failed to remove test request payload: {error}")) } - /// Remove deletion request records by event id from this tombstone store. - pub async fn remove_deletions_by_ids(&self, ids: Vec) -> anyhow::Result { - let removed = ids.len(); + /// Persist a signed deletion/vanish request and relay-generated lifecycle + /// metadata. Exact replays retain their original `first_seen_at`. + pub async fn record_request( + &self, + event: &Event, + first_seen_at: Timestamp, + classification: RequestClassification, + ) -> anyhow::Result { + let _guard = self.lock_lifecycle().await; + self.record_request_locked(event, first_seen_at, classification) + .await + } + + pub(crate) async fn record_request_locked( + &self, + event: &Event, + first_seen_at: Timestamp, + classification: RequestClassification, + ) -> anyhow::Result { + if !Self::is_request_kind(event.kind) { + anyhow::bail!("Tombstone lifecycle accepts only kind-5 or kind-62 requests"); + } + + if let Some(record) = self.lifecycle_for_request_result(&event.id).await? { + return Ok(record); + } + + let payload_existed = self.request_payload(&event.id).await?.is_some(); + self.db.save_event(event).await.map_err(|e| { + anyhow::anyhow!( + "Failed to record deletion-request payload {}: {e}", + event.id + ) + })?; + + let metadata = self.build_metadata(event.id, first_seen_at, None, classification)?; + if let Err(error) = self.db.save_event(&metadata).await { + if !payload_existed { + if let Err(rollback_error) = self.db.delete(Filter::new().ids(vec![event.id])).await + { + tracing::error!(error = %rollback_error, request_id = %event.id, "Failed to roll back tombstone payload after metadata write failure"); + } + } + return Err(anyhow::anyhow!( + "Failed to record lifecycle metadata for {}: {error}", + event.id + )); + } + + Ok(RequestLifecycleRecord { + metadata_event_id: metadata.id, + request: event.clone(), + first_seen_at, + last_used_at: None, + classification, + }) + } + + /// Return the canonical lifecycle record, if both a valid payload and valid + /// metadata exist. Multiple metadata rows are compacted lazily by updates. + /// + /// Canonical selection is deterministic: valid rows are required; the record + /// preserves the earliest `first_seen_at` and greatest `last_used_at` across + /// them, while its metadata ID is selected by greatest `last_used_at`, then + /// earliest `first_seen_at`, newest metadata creation time, then lowest ID. + /// Database failures are returned rather than treated as absent lifecycle + /// metadata because callers use this result for admission and cleanup. + pub async fn lifecycle_for_request_result( + &self, + request_id: &EventId, + ) -> anyhow::Result> { + let request = match self.request_payload(request_id).await? { + Some(event) if Self::is_request_kind(event.kind) => event, + Some(event) => { + tracing::warn!(request_id = %request_id, kind = event.kind.as_u16(), "Tombstone lifecycle payload has invalid kind"); + return Ok(None); + } + None => { + let orphaned_metadata = self.metadata_events_for_request_result(request_id).await?; + if !orphaned_metadata.is_empty() { + tracing::warn!( + request_id = %request_id, + metadata_count = orphaned_metadata.len(), + "Ignoring orphan tombstone lifecycle metadata without a request payload" + ); + } + return Ok(None); + } + }; + + let metadata = self.metadata_events_for_request_result(request_id).await?; + Ok(self.lifecycle_record_from_metadata(request, metadata)) + } + + /// Set `last_used_at`, never moving it backwards, then compact superseded + /// metadata only after the replacement is durable. + pub async fn mark_request_used( + &self, + request_id: &EventId, + used_at: Timestamp, + ) -> anyhow::Result> { + let _guard = self.lock_lifecycle().await; + self.mark_request_used_locked(request_id, used_at).await + } + + pub(crate) async fn mark_request_used_locked( + &self, + request_id: &EventId, + used_at: Timestamp, + ) -> anyhow::Result> { + let Some(existing) = self.lifecycle_for_request_result(request_id).await? else { + return Ok(None); + }; + let last_used_at = existing.last_used_at.max(Some(used_at)); + if last_used_at == existing.last_used_at { + return Ok(Some(existing)); + } + + self.replace_lifecycle_metadata( + request_id, + existing.first_seen_at, + last_used_at, + existing.classification, + ) + .await + } + + /// Update a request's policy classification without changing its lifecycle + /// timestamps. The replacement is durable before stale rows are compacted, + /// so an interrupted update always leaves a readable lifecycle record. + pub async fn reclassify_request( + &self, + request_id: &EventId, + classification: RequestClassification, + ) -> anyhow::Result> { + let _guard = self.lock_lifecycle().await; + self.reclassify_request_locked(request_id, classification) + .await + } + + pub(crate) async fn reclassify_request_locked( + &self, + request_id: &EventId, + classification: RequestClassification, + ) -> anyhow::Result> { + let Some(existing) = self.lifecycle_for_request_result(request_id).await? else { + return Ok(None); + }; + if existing.classification == classification { + return Ok(Some(existing)); + } + self.replace_lifecycle_metadata( + request_id, + existing.first_seen_at, + existing.last_used_at, + classification, + ) + .await + } + + /// List canonical request lifecycles for migration and retention work. + /// Private metadata is not returned, and database failures are propagated. + pub async fn lifecycle_records_result(&self) -> anyhow::Result> { + let requests = self.request_payloads().await?; + self.lifecycle_records_for_payloads_result(requests).await + } + + /// Resolve lifecycle records for an already-enumerated payload set. This + /// avoids reading payloads twice during startup reconciliation. + pub(crate) async fn lifecycle_records_for_payloads_result( + &self, + requests: Vec, + ) -> anyhow::Result> { + // Do not resolve metadata one request at a time. In particular, startup + // calls this over the full retained request set, where per-request + // lookups would turn an otherwise linear migration into thousands of + // database reads. The metadata kind is small and indexed by LMDB, so + // load it once and group its standard `e` references in memory. + let metadata = self + .db + .query(Filter::new().kind(Kind::from(TOMBSTONE_REQUEST_METADATA_KIND))) + .await + .map_err(|error| anyhow::anyhow!("Failed to enumerate tombstone metadata: {error}"))?; + let mut metadata_by_request: HashMap> = HashMap::new(); + for event in metadata { + let Some(request_id) = + Self::tag_value(&event, "e").and_then(|value| EventId::from_hex(value).ok()) + else { + continue; + }; + metadata_by_request + .entry(request_id) + .or_default() + .push(event); + } + let mut records = Vec::new(); + for request in requests { + let request_id = request.id; + if let Some(record) = self.lifecycle_record_from_metadata( + request, + metadata_by_request.remove(&request_id).unwrap_or_default(), + ) { + records.push(record); + } + } + records.sort_by_key(|record| record.request.id.to_hex()); + Ok(records) + } + + /// Resolve lifecycle records for already-matched live-admission payloads. + /// + /// Unlike startup enumeration, this must never scan every metadata row. + /// Query relay-private metadata through its indexed `#e` request reference, + /// batching IDs to keep a heavily-targeted event's query bounded. + async fn lifecycle_records_for_matched_payloads_result( + &self, + requests: Vec, + ) -> anyhow::Result> { + let mut records = Vec::new(); + for request_batch in requests.chunks(METADATA_REQUEST_ID_QUERY_CHUNK_SIZE) { + let request_ids: Vec<_> = request_batch.iter().map(|request| request.id).collect(); + let metadata = self + .db + .query( + Filter::new() + .kind(Kind::from(TOMBSTONE_REQUEST_METADATA_KIND)) + .events(request_ids), + ) + .await + .map_err(|error| { + anyhow::anyhow!("Failed to query matched tombstone lifecycle metadata: {error}") + })?; + let mut metadata_by_request: HashMap> = HashMap::new(); + for event in metadata { + let Some(request_id) = + Self::tag_value(&event, "e").and_then(|value| EventId::from_hex(value).ok()) + else { + continue; + }; + metadata_by_request + .entry(request_id) + .or_default() + .push(event); + } + for request in request_batch { + let request_id = request.id; + if let Some(record) = self.lifecycle_record_from_metadata( + request.clone(), + metadata_by_request.remove(&request_id).unwrap_or_default(), + ) { + records.push(record); + } + } + } + records.sort_by_key(|record| record.request.id.to_hex()); + Ok(records) + } + + /// Remove one payload and every metadata row linked to it. This is + /// idempotent when either side was already removed. The caller must hold + /// `lock_lifecycle` and has already completed any required Main removal. + pub(crate) async fn permanently_delete_request_locked( + &self, + request_id: &EventId, + ) -> anyhow::Result { + let payloads_deleted = usize::from(self.request_payload(request_id).await?.is_some()); + let metadata = self.metadata_events_for_request_result(request_id).await?; + if payloads_deleted > 0 { + self.db + .delete(Filter::new().ids(vec![*request_id])) + .await + .map_err(|error| { + anyhow::anyhow!("Failed to remove tombstone request {request_id}: {error}") + })?; + } + let metadata_deleted = metadata.len(); + if metadata_deleted > 0 { + self.db + .delete(Filter::new().ids(metadata.into_iter().map(|event| event.id))) + .await + .map_err(|error| { + anyhow::anyhow!("Failed to remove lifecycle metadata for {request_id}: {error}") + })?; + } + Ok(RequestLifecycleDeletionStats { + payloads_deleted, + metadata_deleted, + }) + } + + /// Remove metadata whose unambiguous `e` link has no retained request. + /// This is deliberately separate from canonical enumeration: malformed + /// metadata remains invisible to queries but is not guessed at. + pub async fn remove_orphan_metadata(&self) -> anyhow::Result { + let _guard = self.lock_lifecycle().await; + let metadata = self + .db + .query(Filter::new().kind(Kind::from(TOMBSTONE_REQUEST_METADATA_KIND))) + .await + .map_err(|error| anyhow::anyhow!("Failed to enumerate tombstone metadata: {error}"))?; + let mut stale = Vec::new(); + for event in metadata { + let Some(request_id) = + Self::tag_value(&event, "e").and_then(|value| EventId::from_hex(value).ok()) + else { + continue; + }; + if self.request_payload(&request_id).await?.is_none() { + stale.push(event.id); + } + } + let removed = stale.len(); if removed > 0 { self.db - .delete(Filter::new().ids(ids)) + .delete(Filter::new().ids(stale)) .await - .map_err(|e| anyhow::anyhow!("Failed to remove deletion tombstones: {e}"))?; + .map_err(|error| { + anyhow::anyhow!("Failed to remove orphan lifecycle metadata: {error}") + })?; } - Ok(removed) } - fn actionable_targets(event: &Event) -> DeletionTargets { - let author_hex = event.pubkey.to_hex(); - let mut targets = DeletionTargets::default(); - - for tag in event.tags.iter() { - let v = tag.as_slice(); - if v.len() < 2 { - continue; - } - - match v[0].as_str() { - "e" => { - if let Ok(target_id) = EventId::from_hex(&v[1]) { - targets.e_ids.insert(target_id); - } - } - "a" => { - let coordinate = &v[1]; - if coordinate - .split(':') - .nth(1) - .map(|owner| owner == author_hex) - .unwrap_or(false) - { - targets.a_coordinates.insert(coordinate.clone()); - } - } - _ => {} - } + /// List the original kind-5 and kind-62 payloads retained by this store. + /// This deliberately does not require lifecycle metadata, and never exposes + /// relay-private metadata events to callers. + pub async fn request_payloads(&self) -> anyhow::Result> { + let mut requests = Vec::new(); + for kind in [Kind::EventDeletion, Kind::RequestToVanish] { + let events = self + .db + .query(Filter::new().kind(kind)) + .await + .map_err(|error| { + anyhow::anyhow!("Failed to list tombstone request payloads: {error}") + })?; + requests.extend(events); } - - targets + requests.sort_by_key(|event| event.id.to_hex()); + requests.dedup_by_key(|event| event.id); + Ok(requests) } - /// Remove recorded kind-5 requests from other authors that targeted `id`. - /// - /// This handles out-of-order cross-author deletes. A relay may accept a - /// pre-emptive `e` deletion before the target event exists, because it cannot - /// yet validate ownership. When the real event later arrives from a different - /// pubkey, the earlier deletion request is known to be invalid for that - /// target and should no longer remain in the tombstone store. - pub async fn remove_event_deletions_by_other_authors( + fn is_request_kind(kind: Kind) -> bool { + kind == Kind::EventDeletion || kind == Kind::RequestToVanish + } + + async fn replace_lifecycle_metadata( &self, - id: &EventId, - author: &PublicKey, - ) -> anyhow::Result { - let filter = Filter::new() - .kind(Kind::EventDeletion) - .custom_tag(SingleLetterTag::lowercase(Alphabet::E), id.to_hex()); - - let stale_ids: Vec = self - .db - .query(filter) - .await - .map_err(|e| anyhow::anyhow!("Failed to query deletion tombstones: {e}"))? + request_id: &EventId, + first_seen_at: Timestamp, + last_used_at: Option, + classification: RequestClassification, + ) -> anyhow::Result> { + // Metadata timestamps only order relay-private replacement rows. Ensure + // this row is newer even when multiple updates happen in one wall-clock + // second, otherwise canonical selection could retain the old class. + let newest_existing = self + .metadata_events_for_request_result(request_id) + .await? + .iter() + .map(|event| event.created_at) + .max(); + let replacement_created_at = match newest_existing { + Some(existing) if existing >= Timestamp::now() => Timestamp::from_secs( + existing + .as_secs() + .checked_add(1) + .ok_or_else(|| anyhow::anyhow!("lifecycle metadata timestamp overflow"))?, + ), + _ => Timestamp::now(), + }; + let replacement = self.build_metadata_at( + *request_id, + first_seen_at, + last_used_at, + classification, + replacement_created_at, + )?; + self.db.save_event(&replacement).await.map_err(|e| { + anyhow::anyhow!("Failed to update lifecycle metadata for {request_id}: {e}") + })?; + let updated = self + .lifecycle_for_request_result(request_id) + .await? + .ok_or_else(|| { + anyhow::anyhow!("Updated lifecycle metadata for {request_id} was not readable") + })?; + let stale_ids: Vec<_> = self + .metadata_events_for_request_result(request_id) + .await? .into_iter() - .filter(|deletion| deletion.pubkey != *author) - .map(|deletion| deletion.id) + .map(|event| event.id) + .filter(|id| *id != updated.metadata_event_id) .collect(); - - let removed = stale_ids.len(); - if removed > 0 { + if !stale_ids.is_empty() { self.db .delete(Filter::new().ids(stale_ids)) .await - .map_err(|e| anyhow::anyhow!("Failed to remove deletion tombstones: {e}"))?; + .map_err(|e| { + anyhow::anyhow!("Failed to compact lifecycle metadata for {request_id}: {e}") + })?; + } + self.lifecycle_for_request_result(request_id).await + } + + async fn request_payload(&self, request_id: &EventId) -> anyhow::Result> { + // `event_by_id` uses the database's primary event-ID index. Never scan + // every kind-5 and kind-62 payload to find one request: this is called + // in admission, cleanup, and startup reconciliation. + self.db.event_by_id(request_id).await.map_err(|error| { + anyhow::anyhow!("Failed to read tombstone request {request_id}: {error}") + }) + } + + fn build_metadata( + &self, + request_id: EventId, + first_seen_at: Timestamp, + last_used_at: Option, + classification: RequestClassification, + ) -> anyhow::Result { + self.build_metadata_at( + request_id, + first_seen_at, + last_used_at, + classification, + Timestamp::now(), + ) + } + + fn build_metadata_at( + &self, + request_id: EventId, + first_seen_at: Timestamp, + last_used_at: Option, + classification: RequestClassification, + created_at: Timestamp, + ) -> anyhow::Result { + let mut tags = vec![ + Tag::event(request_id), + Tag::custom( + TOMBSTONE_FIRST_SEEN_AT_TAG, + vec![first_seen_at.as_secs().to_string()], + ), + Tag::custom( + TOMBSTONE_REQUEST_CLASSIFICATION_TAG, + vec![classification.as_str().to_string()], + ), + ]; + if let Some(last_used_at) = last_used_at { + tags.push(Tag::custom( + TOMBSTONE_LAST_USED_AT_TAG, + vec![last_used_at.as_secs().to_string()], + )); + } + EventBuilder::new(Kind::from(TOMBSTONE_REQUEST_METADATA_KIND), "") + .tags(tags) + .custom_created_at(created_at) + .finalize(&self.metadata_signer) + .map_err(|e| anyhow::anyhow!("Failed to build tombstone lifecycle metadata: {e}")) + } + + #[cfg(test)] + async fn metadata_events_for_request(&self, request_id: &EventId) -> Vec { + match self.metadata_events_for_request_result(request_id).await { + Ok(events) => events, + Err(error) => { + tracing::error!(error = %error, request_id = %request_id, "Failed to query tombstone lifecycle metadata"); + Vec::new() + } + } + } + + async fn metadata_events_for_request_result( + &self, + request_id: &EventId, + ) -> anyhow::Result> { + let filter = Filter::new() + .kind(Kind::from(TOMBSTONE_REQUEST_METADATA_KIND)) + .custom_tag(SingleLetterTag::lowercase(Alphabet::E), request_id.to_hex()); + self.db + .query(filter) + .await + .map(|events| events.into_iter().collect()) + .map_err(|error| { + anyhow::anyhow!( + "Failed to query tombstone lifecycle metadata for {request_id}: {error}" + ) + }) + } + + fn parse_metadata(&self, event: &Event, request_id: &EventId) -> Option { + let linked = Self::tag_value(event, "e") + .and_then(|value| EventId::from_hex(value).ok()) + .is_some_and(|id| id == *request_id); + let first_seen_at = Self::tag_value(event, TOMBSTONE_FIRST_SEEN_AT_TAG) + .and_then(|value| value.parse().ok()) + .map(Timestamp::from_secs); + let classification = Self::tag_value(event, TOMBSTONE_REQUEST_CLASSIFICATION_TAG) + .and_then(RequestClassification::parse); + let last_used_at = Self::tag_value(event, TOMBSTONE_LAST_USED_AT_TAG) + .and_then(|value| value.parse().ok()) + .map(Timestamp::from_secs); + match ( + event.kind.as_u16() == TOMBSTONE_REQUEST_METADATA_KIND, + linked, + first_seen_at, + classification, + ) { + (true, true, Some(first_seen_at), Some(classification)) => Some(ParsedMetadata { + id: event.id, + created_at: event.created_at, + first_seen_at, + last_used_at, + classification, + }), + _ => { + tracing::warn!(metadata_id = %event.id, request_id = %request_id, "Ignoring malformed tombstone lifecycle metadata"); + None + } + } + } + + fn lifecycle_record_from_metadata( + &self, + request: Event, + metadata: Vec, + ) -> Option { + if !Self::is_request_kind(request.kind) { + tracing::warn!(request_id = %request.id, kind = request.kind.as_u16(), "Tombstone lifecycle payload has invalid kind"); + return None; + } + let request_id = request.id; + let mut valid: Vec<_> = metadata + .into_iter() + .filter_map(|event| self.parse_metadata(&event, &request_id)) + .collect(); + if valid.is_empty() { + return None; } - Ok(removed) + let first_seen_at = valid + .iter() + .map(|m| m.first_seen_at) + .min() + .expect("nonempty"); + let last_used_at = valid.iter().filter_map(|m| m.last_used_at).max(); + valid.sort_by(|left, right| { + right + .last_used_at + .cmp(&left.last_used_at) + .then_with(|| left.first_seen_at.cmp(&right.first_seen_at)) + .then_with(|| right.created_at.cmp(&left.created_at)) + .then_with(|| left.id.to_hex().cmp(&right.id.to_hex())) + }); + let canonical = valid.remove(0); + Some(RequestLifecycleRecord { + metadata_event_id: canonical.id, + request, + first_seen_at, + last_used_at, + classification: canonical.classification, + }) } - /// Record a NIP-62 vanish request (kind 62). - /// - /// The caller MUST have already validated that the request targets this relay - /// and is signed by the pubkey being vanished. - pub async fn record_vanish(&self, event: &Event) -> anyhow::Result<()> { - debug_assert_eq!(event.kind, Kind::RequestToVanish); - self.db - .save_event(event) - .await - .map_err(|e| anyhow::anyhow!("Failed to record vanish tombstone: {e}"))?; - Ok(()) + fn tag_value<'a>(event: &'a Event, name: &str) -> Option<&'a str> { + event.tags.iter().find_map(|tag| { + let tag = tag.as_slice(); + (tag.len() >= 2 && tag[0] == name).then_some(tag[1].as_str()) + }) } - /// Has this event id been deleted by a recorded kind-5 (`e` tag) authored by - /// `author`? - /// - /// Mirrors the backend's `is_deleted` check that previously fed the - /// relay-builder `check_id` gate, but with explicit author binding: only the - /// event's own author may delete it (NIP-09). A deletion request signed by a - /// different pubkey must NOT block (re-)submission of `author`'s event — - /// otherwise any pubkey could pre-emptively censor another's events by - /// racing a kind-5 ahead of the target (out-of-order delete). - pub async fn is_event_deleted(&self, id: &EventId, author: &PublicKey) -> bool { - // A stored kind-5 authored by `author` carrying this id in an `e` tag - // means `author` deleted their own event. + /// Return locally actionable, lifecycle-backed kind-5 requests that delete + /// `id` on behalf of `author`. + pub async fn event_deletion_candidates( + &self, + id: &EventId, + author: &PublicKey, + ) -> anyhow::Result> { let filter = Filter::new() .kind(Kind::EventDeletion) .author(*author) .custom_tag(SingleLetterTag::lowercase(Alphabet::E), id.to_hex()); - match self.db.query(filter).await { - Ok(events) => !events.is_empty(), - Err(e) => { - // Fail secure: if we cannot determine deletion status, do not - // claim the event is deleted (so we don't reject legitimate - // events), but log loudly. - tracing::error!(error = %e, "Tombstone query failed for is_event_deleted"); - false - } - } + self.lifecycle_candidates(filter, |request| { + request.kind == Kind::EventDeletion + && request.pubkey == *author + && request.tags.iter().any(|tag| { + tag.as_slice() + .get(0..2) + .is_some_and(|values| values[0] == "e" && values[1] == id.to_hex()) + }) + }) + .await } - /// Has this addressable/replaceable coordinate been deleted at or after - /// `event_created_at`? - /// - /// Per NIP-09, an `a`-tag deletion deletes all versions of the coordinate up - /// to the deletion request's `created_at`. So a candidate event is blocked - /// only if a recorded deletion for the same coordinate has - /// `deletion.created_at >= event_created_at`. - /// - /// Author binding: a coordinate is `::`, so only the pubkey - /// embedded in the coordinate may delete it. A kind-5 signed by a different - /// pubkey referencing this coordinate is ignored, otherwise any pubkey could - /// pre-emptively censor another's addressable events. - pub async fn is_coordinate_deleted( + /// Return locally actionable, lifecycle-backed kind-5 requests that delete + /// the coordinate at or after `event_created_at`. + pub async fn coordinate_deletion_candidates( &self, coordinate: &str, event_created_at: Timestamp, - ) -> bool { + ) -> anyhow::Result> { // The coordinate owner (pubkey hex) is the only party allowed to delete // it. Extract it so we can require the kind-5 author to match. let coord_owner_hex = coordinate.split(':').nth(1); @@ -413,39 +834,67 @@ impl Tombstones { SingleLetterTag::lowercase(Alphabet::A), coordinate.to_string(), ); - match self.db.query(filter).await { - Ok(events) => events.iter().any(|deletion| { - deletion.created_at >= event_created_at - && coord_owner_hex - .map(|owner| owner == deletion.pubkey.to_hex()) - .unwrap_or(false) - }), - Err(e) => { - tracing::error!(error = %e, "Tombstone query failed for is_coordinate_deleted"); - false - } - } + self.lifecycle_candidates(filter, |request| { + request.kind == Kind::EventDeletion + && request.created_at >= event_created_at + && coord_owner_hex + .map(|owner| owner == request.pubkey.to_hex()) + .unwrap_or(false) + && request.tags.iter().any(|tag| { + tag.as_slice() + .get(0..2) + .is_some_and(|values| values[0] == "a" && values[1] == coordinate) + }) + }) + .await } - /// Has this pubkey requested to vanish from this relay (recorded kind-62)? - /// - /// Mirrors the backend's `is_pubkey_vanished` check. - pub async fn is_pubkey_vanished(&self, pubkey: &PublicKey) -> bool { + /// Return locally actionable, lifecycle-backed kind-62 requests made by + /// `pubkey`. Non-targeting and disrespector classifications never gate. + pub async fn vanish_candidates( + &self, + pubkey: &PublicKey, + ) -> anyhow::Result> { let filter = Filter::new().kind(Kind::RequestToVanish).author(*pubkey); - match self.db.query(filter).await { - Ok(events) => !events.is_empty(), - Err(e) => { - tracing::error!(error = %e, "Tombstone query failed for is_pubkey_vanished"); - false - } - } + self.lifecycle_candidates(filter, |request| { + request.kind == Kind::RequestToVanish && request.pubkey == *pubkey + }) + .await + } + + async fn lifecycle_candidates( + &self, + filter: Filter, + matches_request: F, + ) -> anyhow::Result> + where + F: Fn(&Event) -> bool, + { + let requests = + self.db.query(filter).await.map_err(|error| { + anyhow::anyhow!("Failed to query tombstone candidates: {error}") + })?; + // Queries are intentionally restricted to original kind-5/kind-62 + // payloads; private metadata can never enter this path. Resolve the + // resulting payloads together through indexed metadata `#e` queries. + // Startup uses full-table grouping, but using that strategy here would + // scan all retained lifecycle metadata while the admission lock is held. + let matching_requests = requests + .into_iter() + .filter(|request| Self::is_request_kind(request.kind) && matches_request(request)) + .collect(); + Ok(self + .lifecycle_records_for_matched_payloads_result(matching_requests) + .await? + .into_iter() + .filter(|record| record.classification == RequestClassification::LocallyActionable) + .collect()) } } #[cfg(test)] mod tests { use super::*; - use nostr_relay_builder::prelude::*; fn deletion_by_event(keys: &Keys, target: EventId) -> Event { EventBuilder::new(Kind::EventDeletion, "") @@ -454,264 +903,443 @@ mod tests { .unwrap() } - #[tokio::test] - async fn records_and_detects_event_deletion() { - let store = Tombstones::in_memory(); - let keys = Keys::generate(); - let target = EventId::all_zeros(); + fn vanish(keys: &Keys) -> Event { + EventBuilder::new(Kind::RequestToVanish, "") + .tags(vec![Tag::custom("relay", vec!["ALL_RELAYS".to_string()])]) + .finalize(keys) + .unwrap() + } - assert!(!store.is_event_deleted(&target, &keys.public_key()).await); - - let deletion = deletion_by_event(&keys, target); - store.record_deletion(&deletion).await.unwrap(); - - assert!(store.is_event_deleted(&target, &keys.public_key()).await); + async fn metadata_count(store: &Tombstones, request_id: EventId) -> usize { + store.metadata_events_for_request(&request_id).await.len() } #[tokio::test] - async fn detects_already_covered_event_deletion_request() { + async fn lifecycle_round_trip_for_kind_five_preserves_relay_first_seen_at() { let store = Tombstones::in_memory(); let keys = Keys::generate(); - let target = EventId::all_zeros(); + let request = deletion_by_event(&keys, EventId::all_zeros()); + let first_seen_at = Timestamp::from_secs(1234); - 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) + store + .record_request( + &request, + first_seen_at, + RequestClassification::LocallyActionable, + ) .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) + let lifecycle = store + .lifecycle_for_request_result(&request.id) .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() .unwrap(); - assert!(!store - .deletion_targets_already_covered(&newer) - .await - .unwrap()); - } - - #[tokio::test] - async fn newer_coordinate_deletion_supersedes_older_request() { - let store = Tombstones::in_memory(); - let keys = Keys::generate(); - let coord = format!("30618:{}:my-repo", keys.public_key().to_hex()); - - let older = 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(&older).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(); - store.record_deletion(&newer).await.unwrap(); + assert_eq!(lifecycle.request, request); + assert_eq!(lifecycle.first_seen_at, first_seen_at); + assert_eq!(lifecycle.last_used_at, None); assert_eq!( - store.superseded_deletion_ids(&newer).await.unwrap(), - vec![older.id] - ); - - let removed = store.remove_deletions_by_ids(vec![older.id]).await.unwrap(); - assert_eq!(removed, 1); - assert_eq!( - store.superseded_deletion_ids(&newer).await.unwrap(), - Vec::new() + lifecycle.classification, + RequestClassification::LocallyActionable ); } #[tokio::test] - async fn superseded_deletion_detection_requires_full_target_coverage() { + async fn lifecycle_round_trip_for_kind_sixty_two() { + let store = Tombstones::in_memory(); + let keys = Keys::generate(); + let request = vanish(&keys); + + store + .record_request( + &request, + Timestamp::from_secs(2345), + RequestClassification::NonTargetingNip62, + ) + .await + .unwrap(); + let lifecycle = store + .lifecycle_for_request_result(&request.id) + .await + .unwrap() + .unwrap(); + + assert_eq!(lifecycle.request, request); + assert_eq!( + lifecycle.classification, + RequestClassification::NonTargetingNip62 + ); + } + + #[tokio::test] + async fn exact_replay_does_not_reset_first_seen_at() { + let store = Tombstones::in_memory(); + let request = deletion_by_event(&Keys::generate(), EventId::all_zeros()); + store + .record_request( + &request, + Timestamp::from_secs(10), + RequestClassification::LocallyActionable, + ) + .await + .unwrap(); + store + .record_request( + &request, + Timestamp::from_secs(99), + RequestClassification::Disrespector, + ) + .await + .unwrap(); + + let lifecycle = store + .lifecycle_for_request_result(&request.id) + .await + .unwrap() + .unwrap(); + assert_eq!(lifecycle.first_seen_at, Timestamp::from_secs(10)); + assert_eq!( + lifecycle.classification, + RequestClassification::LocallyActionable + ); + assert_eq!(metadata_count(&store, request.id).await, 1); + } + + #[tokio::test] + async fn marking_request_used_advances_without_resetting_first_seen_at() { + let store = Tombstones::in_memory(); + let request = deletion_by_event(&Keys::generate(), EventId::all_zeros()); + store + .record_request( + &request, + Timestamp::from_secs(10), + RequestClassification::LocallyActionable, + ) + .await + .unwrap(); + + let first_use = store + .mark_request_used(&request.id, Timestamp::from_secs(20)) + .await + .unwrap() + .unwrap(); + assert_eq!(first_use.first_seen_at, Timestamp::from_secs(10)); + assert_eq!(first_use.last_used_at, Some(Timestamp::from_secs(20))); + let later_use = store + .mark_request_used(&request.id, Timestamp::from_secs(30)) + .await + .unwrap() + .unwrap(); + assert_eq!(later_use.first_seen_at, Timestamp::from_secs(10)); + assert_eq!(later_use.last_used_at, Some(Timestamp::from_secs(30))); + let older_use = store + .mark_request_used(&request.id, Timestamp::from_secs(25)) + .await + .unwrap() + .unwrap(); + assert_eq!(older_use.first_seen_at, Timestamp::from_secs(10)); + assert_eq!(older_use.last_used_at, Some(Timestamp::from_secs(30))); + assert_eq!(metadata_count(&store, request.id).await, 1); + } + + #[tokio::test] + async fn canonical_metadata_selection_is_deterministic_and_compacted_on_update() { + let store = Tombstones::in_memory(); + let request = deletion_by_event(&Keys::generate(), EventId::all_zeros()); + store + .record_request( + &request, + Timestamp::from_secs(20), + RequestClassification::LocallyActionable, + ) + .await + .unwrap(); + let older_first_seen = store + .build_metadata( + request.id, + Timestamp::from_secs(10), + Some(Timestamp::from_secs(40)), + RequestClassification::Disrespector, + ) + .unwrap(); + let newest_use = store + .build_metadata( + request.id, + Timestamp::from_secs(30), + Some(Timestamp::from_secs(50)), + RequestClassification::NonTargetingNip62, + ) + .unwrap(); + store.db.save_event(&older_first_seen).await.unwrap(); + store.db.save_event(&newest_use).await.unwrap(); + + let lifecycle = store + .lifecycle_for_request_result(&request.id) + .await + .unwrap() + .unwrap(); + assert_eq!(lifecycle.first_seen_at, Timestamp::from_secs(10)); + assert_eq!(lifecycle.last_used_at, Some(Timestamp::from_secs(50))); + assert_eq!(lifecycle.metadata_event_id, newest_use.id); + + store + .mark_request_used(&request.id, Timestamp::from_secs(60)) + .await + .unwrap(); + assert_eq!(metadata_count(&store, request.id).await, 1); + } + + #[tokio::test] + async fn malformed_and_orphan_metadata_are_ignored_safely() { + let store = Tombstones::in_memory(); + let request = deletion_by_event(&Keys::generate(), EventId::all_zeros()); + let malformed = EventBuilder::new(Kind::from(TOMBSTONE_REQUEST_METADATA_KIND), "") + .tags(vec![Tag::event(request.id)]) + .finalize(&store.metadata_signer) + .unwrap(); + store.db.save_event(&malformed).await.unwrap(); + assert!(store + .lifecycle_for_request_result(&request.id) + .await + .unwrap() + .is_none()); + + let orphan_id = EventId::all_zeros(); + let orphan = store + .build_metadata( + orphan_id, + Timestamp::from_secs(10), + None, + RequestClassification::LocallyActionable, + ) + .unwrap(); + store.db.save_event(&orphan).await.unwrap(); + assert!(store + .lifecycle_for_request_result(&orphan_id) + .await + .unwrap() + .is_none()); + } + + #[tokio::test] + async fn metadata_events_do_not_affect_deletion_or_vanish_queries() { let store = Tombstones::in_memory(); let keys = Keys::generate(); - let coord = format!("30618:{}:my-repo", keys.public_key().to_hex()); let target = EventId::all_zeros(); - - let older = EventBuilder::new(Kind::EventDeletion, "") - .tags(vec![ - Tag::event(target), - Tag::custom("a", vec![coord.clone()]), - ]) - .custom_created_at(Timestamp::from_secs(1000)) - .finalize(&keys) + let metadata = store + .build_metadata( + target, + Timestamp::from_secs(10), + None, + RequestClassification::LocallyActionable, + ) .unwrap(); - store.record_deletion(&older).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(); - store.record_deletion(&newer).await.unwrap(); + store.db.save_event(&metadata).await.unwrap(); assert!(store - .superseded_deletion_ids(&newer) + .event_deletion_candidates(&target, &keys.public_key()) + .await + .unwrap() + .is_empty()); + assert!(store + .vanish_candidates(&keys.public_key()) .await .unwrap() .is_empty()); } #[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 - // deleted for a different author A: only the event's own author may - // delete it (NIP-09). Guards against pre-emptive cross-author censorship - // when the deletion races ahead of the target (out-of-order delete). + async fn event_deletion_candidates_are_author_bound_and_lifecycle_backed() { let store = Tombstones::in_memory(); - let attacker = Keys::generate(); - let victim = Keys::generate(); + let owner = Keys::generate(); + let other = Keys::generate(); let target = EventId::all_zeros(); - - let deletion = deletion_by_event(&attacker, target); - store.record_deletion(&deletion).await.unwrap(); - - // The attacker "deleted" it for themselves... - assert!( - store - .is_event_deleted(&target, &attacker.public_key()) - .await - ); - // ...but the victim's identical id is NOT considered deleted. - assert!( - !store.is_event_deleted(&target, &victim.public_key()).await, - "a deletion from another pubkey must not block the victim's event" - ); - } - - #[tokio::test] - async fn removes_deletion_requests_from_other_authors_when_target_author_is_known() { - let store = Tombstones::in_memory(); - let attacker = Keys::generate(); - let victim = Keys::generate(); - let target = EventId::all_zeros(); - - let deletion = deletion_by_event(&attacker, target); - store.record_deletion(&deletion).await.unwrap(); - - assert!( - store - .is_event_deleted(&target, &attacker.public_key()) - .await - ); - - let removed = store - .remove_event_deletions_by_other_authors(&target, &victim.public_key()) + let request = deletion_by_event(&owner, target); + store + .record_request( + &request, + Timestamp::from_secs(10), + RequestClassification::LocallyActionable, + ) .await .unwrap(); - assert_eq!(removed, 1); - assert!( - !store - .is_event_deleted(&target, &attacker.public_key()) - .await, - "cross-author deletion request should be removed once the real author is known" - ); - } - - #[tokio::test] - async fn coordinate_deletion_respects_created_at() { - let store = Tombstones::in_memory(); - let keys = Keys::generate(); - let coord = format!("30618:{}:my-repo", keys.public_key().to_hex()); - - // Deletion at t=1000 - let deletion = EventBuilder::new(Kind::EventDeletion, "") - .tags(vec![Tag::custom("a", vec![coord.clone()])]) - .custom_created_at(Timestamp::from_secs(1000)) - .finalize(&keys) + let candidates = store + .event_deletion_candidates(&target, &owner.public_key()) + .await .unwrap(); - store.record_deletion(&deletion).await.unwrap(); - - // An event created at t<=1000 is deleted; t>1000 is not. - assert!( - store - .is_coordinate_deleted(&coord, Timestamp::from_secs(1000)) - .await - ); - assert!( - store - .is_coordinate_deleted(&coord, Timestamp::from_secs(500)) - .await - ); - assert!( - !store - .is_coordinate_deleted(&coord, Timestamp::from_secs(1500)) - .await - ); + assert_eq!(candidates.len(), 1); + assert_eq!(candidates[0].request.id, request.id); + assert!(store + .event_deletion_candidates(&target, &other.public_key()) + .await + .unwrap() + .is_empty()); } #[tokio::test] - async fn coordinate_deletion_by_other_pubkey_is_ignored() { - // A kind-5 from pubkey B carrying an `a` tag for a coordinate owned by - // pubkey A must NOT mark that coordinate as deleted: only the pubkey - // embedded in the coordinate may delete it. + async fn event_deletion_candidates_resolve_duplicate_requests_together() { let store = Tombstones::in_memory(); - let victim = Keys::generate(); - let attacker = Keys::generate(); - let coord = format!("30618:{}:my-repo", victim.public_key().to_hex()); + let owner = Keys::generate(); + let target = EventId::all_zeros(); + let mut actionable_ids = Vec::new(); - let deletion = EventBuilder::new(Kind::EventDeletion, "") - .tags(vec![Tag::custom("a", vec![coord.clone()])]) - .custom_created_at(Timestamp::from_secs(1000)) + // A single event may have many valid deletion requests. Save their + // payloads and metadata directly so this test isolates the candidate + // resolution path from record_request's single-request replay check. + for created_at in 1..=METADATA_REQUEST_ID_QUERY_CHUNK_SIZE as u64 + 1 { + let request = EventBuilder::new(Kind::EventDeletion, "") + .tags(vec![Tag::event(target)]) + .custom_created_at(Timestamp::from_secs(created_at)) + .finalize(&owner) + .unwrap(); + let classification = if created_at % 2 == 0 { + actionable_ids.push(request.id); + RequestClassification::LocallyActionable + } else { + RequestClassification::Disrespector + }; + store.db.save_event(&request).await.unwrap(); + let metadata = store + .build_metadata( + request.id, + Timestamp::from_secs(created_at), + None, + classification, + ) + .unwrap(); + store.db.save_event(&metadata).await.unwrap(); + } + + let candidate_ids = store + .event_deletion_candidates(&target, &owner.public_key()) + .await + .unwrap() + .into_iter() + .map(|record| record.request.id) + .collect::>(); + + assert_eq!(candidate_ids.len(), actionable_ids.len()); + assert!(candidate_ids + .iter() + .all(|candidate_id| actionable_ids.contains(candidate_id))); + } + + #[tokio::test] + async fn coordinate_candidates_enforce_cutoff_and_coordinate_owner() { + let store = Tombstones::in_memory(); + let owner = Keys::generate(); + let attacker = Keys::generate(); + let coordinate = format!("30618:{}:repo", owner.public_key().to_hex()); + let valid = EventBuilder::new(Kind::EventDeletion, "") + .tags(vec![Tag::custom("a", vec![coordinate.clone()])]) + .custom_created_at(Timestamp::from_secs(100)) + .finalize(&owner) + .unwrap(); + let invalid = EventBuilder::new(Kind::EventDeletion, "") + .tags(vec![Tag::custom("a", vec![coordinate.clone()])]) + .custom_created_at(Timestamp::from_secs(100)) .finalize(&attacker) .unwrap(); - store.record_deletion(&deletion).await.unwrap(); + for request in [&valid, &invalid] { + store + .record_request( + request, + Timestamp::from_secs(10), + RequestClassification::LocallyActionable, + ) + .await + .unwrap(); + } - assert!( - !store - .is_coordinate_deleted(&coord, Timestamp::from_secs(1000)) - .await, - "a coordinate deletion signed by a different pubkey must be ignored" + assert_eq!( + store + .coordinate_deletion_candidates(&coordinate, Timestamp::from_secs(100)) + .await + .unwrap() + .iter() + .map(|record| record.request.id) + .collect::>(), + vec![valid.id] ); + assert_eq!( + store + .coordinate_deletion_candidates(&coordinate, Timestamp::from_secs(99)) + .await + .unwrap() + .len(), + 1 + ); + assert!(store + .coordinate_deletion_candidates(&coordinate, Timestamp::from_secs(101)) + .await + .unwrap() + .is_empty()); } #[tokio::test] - async fn detects_pubkey_vanish() { + async fn vanish_candidates_require_locally_actionable_classification() { let store = Tombstones::in_memory(); let keys = Keys::generate(); - let other = Keys::generate(); - - assert!(!store.is_pubkey_vanished(&keys.public_key()).await); - - let vanish = EventBuilder::new(Kind::RequestToVanish, "") - .tags(vec![Tag::custom("relay", vec!["ALL_RELAYS".to_string()])]) - .finalize(&keys) + let actionable = vanish(&keys); + store + .record_request( + &actionable, + Timestamp::from_secs(10), + RequestClassification::LocallyActionable, + ) + .await .unwrap(); - store.record_vanish(&vanish).await.unwrap(); + assert_eq!( + store + .vanish_candidates(&keys.public_key()) + .await + .unwrap() + .len(), + 1 + ); - assert!(store.is_pubkey_vanished(&keys.public_key()).await); - assert!(!store.is_pubkey_vanished(&other.public_key()).await); + for classification in [ + RequestClassification::NonTargetingNip62, + RequestClassification::Disrespector, + ] { + let store = Tombstones::in_memory(); + let request = vanish(&keys); + store + .record_request(&request, Timestamp::from_secs(10), classification) + .await + .unwrap(); + assert!(store + .vanish_candidates(&keys.public_key()) + .await + .unwrap() + .is_empty()); + } + } + + #[tokio::test] + async fn lifecycle_metadata_survives_lmdb_reopen() { + let directory = tempfile::tempdir().unwrap(); + let keys = Keys::generate(); + let request = deletion_by_event(&keys, EventId::all_zeros()); + let store = Tombstones::open_lmdb(directory.path()).await.unwrap(); + store + .record_request( + &request, + Timestamp::from_secs(1234), + RequestClassification::LocallyActionable, + ) + .await + .unwrap(); + drop(store); + + let reopened = Tombstones::open_lmdb(directory.path()).await.unwrap(); + let lifecycle = reopened + .lifecycle_for_request_result(&request.id) + .await + .unwrap() + .unwrap(); + assert_eq!(lifecycle.first_seen_at, Timestamp::from_secs(1234)); + assert_eq!(lifecycle.request, request); } } diff --git a/src/repair_deletion_requests.rs b/src/repair_deletion_requests.rs deleted file mode 100644 index 51f64cc..0000000 --- a/src/repair_deletion_requests.rs +++ /dev/null @@ -1,416 +0,0 @@ -//! Temporary operator repair for redundant NIP-09 deletion request rows. -//! -//! The live write path now rejects covered kind-5 requests and compacts older -//! superseded requests. This command applies the same idea to an existing LMDB -//! database so production relays can remove historical kind-5 pollution. - -use std::cmp::Ordering; -use std::collections::{HashMap, HashSet}; -use std::path::Path; -use std::sync::Arc; - -use anyhow::{Context, Result}; -use clap::Args; -use nostr_lmdb::NostrLmdb; -use nostr_relay_builder::prelude::{Event, EventId, Filter, Kind, NostrDatabase}; - -/// Arguments for the hidden `repair-deletion-requests` subcommand. -#[derive(Debug, Args)] -pub struct RepairDeletionRequestsArgs { - /// Path to the LMDB relay data directory (contains the nostr event database). - #[arg(long, env = "NGIT_RELAY_DATA_PATH", default_value = "./data/relay")] - pub relay_data_path: String, - - /// Actually remove redundant kind-5 deletion request events. - /// - /// Without this flag the command runs in dry-run mode and only reports what - /// would be deleted. Stop the relay service before using this flag. - #[arg(long, default_value_t = false)] - pub execute: bool, -} - -#[derive(Debug, Default)] -struct ActionableTargets { - e_ids: HashSet, - a_coordinates: HashSet, -} - -impl ActionableTargets { - fn len(&self) -> usize { - self.e_ids.len() + self.a_coordinates.len() - } - - fn is_empty(&self) -> bool { - self.e_ids.is_empty() && self.a_coordinates.is_empty() - } -} - -#[derive(Debug, Clone)] -struct DeletionRequestSummary { - id: EventId, - id_hex: String, - author_hex: String, - created_at: u64, - target_count: usize, -} - -impl DeletionRequestSummary { - fn from_event(event: &Event, targets: &ActionableTargets) -> Self { - Self { - id: event.id, - id_hex: event.id.to_hex(), - author_hex: event.pubkey.to_hex(), - created_at: event.created_at.as_secs(), - target_count: targets.len(), - } - } -} - -#[derive(Debug)] -struct DeletionRequestAnalysis { - total: usize, - actionable: usize, - keep_ids: HashSet, - remove_ids: Vec, -} - -/// Run the repair-deletion-requests subcommand. -pub async fn run(args: &RepairDeletionRequestsArgs) -> Result<()> { - let relay_data_path = Path::new(&args.relay_data_path); - - if args.execute { - println!("=== repair-deletion-requests (EXECUTE MODE) ==="); - println!("WARNING: This will permanently delete redundant kind-5 rows."); - println!("Stop the relay service before running in execute mode."); - } else { - println!("=== repair-deletion-requests (DRY-RUN MODE) ==="); - println!("Pass --execute to actually delete. Stop the relay first."); - } - println!(); - println!("Relay data path: {}", relay_data_path.display()); - println!(); - - let main_db = open_lmdb(relay_data_path) - .await - .with_context(|| format!("Failed to open main LMDB at {}", relay_data_path.display()))?; - - repair_database("main relay DB", main_db, args.execute).await?; - - let tombstone_path = relay_data_path.join("tombstones"); - if tombstone_path.exists() { - let tombstone_db = open_lmdb(&tombstone_path).await.with_context(|| { - format!( - "Failed to open tombstone LMDB at {}", - tombstone_path.display() - ) - })?; - repair_database("tombstone DB", tombstone_db, args.execute).await?; - } else { - println!( - "tombstone DB: skipped ({} does not exist)", - tombstone_path.display() - ); - } - - Ok(()) -} - -async fn open_lmdb(path: &Path) -> Result> { - let db = NostrLmdb::builder(path) - .process_nip09(false) - .process_nip62(false) - .build() - .await?; - Ok(Arc::new(db)) -} - -async fn repair_database( - name: &str, - database: Arc, - execute: bool, -) -> Result<()> { - println!("{}: querying kind-5 deletion requests...", name); - let deletion_requests: Vec = database - .query(Filter::new().kind(Kind::EventDeletion)) - .await - .with_context(|| format!("Failed to query kind-5 events from {name}"))? - .into_iter() - .collect(); - - let analysis = analyze_deletion_requests(&deletion_requests); - println!("{}: found {} kind-5 event(s)", name, analysis.total); - println!( - "{}: {} actionable, {} kept, {} redundant", - name, - analysis.actionable, - analysis.keep_ids.len(), - analysis.remove_ids.len() - ); - - if analysis.remove_ids.is_empty() { - println!("{}: nothing to remove", name); - println!(); - return Ok(()); - } - - if !execute { - println!( - "{}: DRY-RUN would delete {} redundant kind-5 event(s)", - name, - analysis.remove_ids.len() - ); - println!(); - return Ok(()); - } - - let deleted = delete_ids_in_chunks(database.as_ref(), &analysis.remove_ids).await?; - println!("{}: deleted {} redundant kind-5 event(s)", name, deleted); - println!(); - - Ok(()) -} - -async fn delete_ids_in_chunks(database: &dyn NostrDatabase, ids: &[EventId]) -> Result { - let mut deleted = 0usize; - for chunk in ids.chunks(1_000) { - database - .delete(Filter::new().ids(chunk.to_vec())) - .await - .context("Failed to delete redundant kind-5 events")?; - deleted += chunk.len(); - } - Ok(deleted) -} - -fn analyze_deletion_requests(events: &[Event]) -> DeletionRequestAnalysis { - let mut summaries: HashMap = HashMap::new(); - let mut e_best: HashMap<(String, EventId), DeletionRequestSummary> = HashMap::new(); - let mut a_best: HashMap<(String, String), DeletionRequestSummary> = HashMap::new(); - let mut no_actionable_ids = HashSet::new(); - let mut actionable = 0usize; - - for event in events { - let targets = actionable_targets(event); - let summary = DeletionRequestSummary::from_event(event, &targets); - - if targets.is_empty() { - no_actionable_ids.insert(event.id); - summaries.insert(event.id, summary); - continue; - } - - actionable += 1; - - for target_id in targets.e_ids { - keep_best( - &mut e_best, - (summary.author_hex.clone(), target_id), - summary.clone(), - compare_event_target_keeper, - ); - } - - for coordinate in targets.a_coordinates { - keep_best( - &mut a_best, - (summary.author_hex.clone(), coordinate), - summary.clone(), - compare_coordinate_keeper, - ); - } - - summaries.insert(event.id, summary); - } - - let mut keep_ids = no_actionable_ids; - keep_ids.extend(e_best.values().map(|summary| summary.id)); - keep_ids.extend(a_best.values().map(|summary| summary.id)); - - let mut remove_ids: Vec = summaries - .keys() - .copied() - .filter(|id| !keep_ids.contains(id)) - .collect(); - remove_ids.sort_by_key(|id| id.to_hex()); - - DeletionRequestAnalysis { - total: events.len(), - actionable, - keep_ids, - remove_ids, - } -} - -fn actionable_targets(event: &Event) -> ActionableTargets { - let author_hex = event.pubkey.to_hex(); - let mut targets = ActionableTargets::default(); - - for tag in event.tags.iter() { - let v = tag.as_slice(); - if v.len() < 2 { - continue; - } - - match v[0].as_str() { - "e" => { - if let Ok(target_id) = EventId::from_hex(&v[1]) { - targets.e_ids.insert(target_id); - } - } - "a" => { - let coordinate = &v[1]; - if coordinate - .split(':') - .nth(1) - .map(|owner| owner == author_hex) - .unwrap_or(false) - { - targets.a_coordinates.insert(coordinate.clone()); - } - } - _ => {} - } - } - - targets -} - -fn keep_best( - map: &mut HashMap, - key: K, - candidate: DeletionRequestSummary, - compare: fn(&DeletionRequestSummary, &DeletionRequestSummary) -> Ordering, -) where - K: Eq + std::hash::Hash, -{ - match map.get(&key) { - Some(existing) if compare(&candidate, existing) != Ordering::Greater => {} - _ => { - map.insert(key, candidate); - } - } -} - -fn compare_event_target_keeper( - candidate: &DeletionRequestSummary, - existing: &DeletionRequestSummary, -) -> Ordering { - candidate - .target_count - .cmp(&existing.target_count) - .then_with(|| candidate.created_at.cmp(&existing.created_at)) - // Prefer the lower event id for stable tie-breaking. - .then_with(|| existing.id_hex.cmp(&candidate.id_hex)) -} - -fn compare_coordinate_keeper( - candidate: &DeletionRequestSummary, - existing: &DeletionRequestSummary, -) -> Ordering { - candidate - .created_at - .cmp(&existing.created_at) - .then_with(|| candidate.target_count.cmp(&existing.target_count)) - // Prefer the lower event id for stable tie-breaking. - .then_with(|| existing.id_hex.cmp(&candidate.id_hex)) -} - -#[cfg(test)] -mod tests { - use super::*; - use nostr::event::FinalizeEvent; - use nostr_relay_builder::prelude::{EventBuilder, Keys, Tag, Timestamp}; - - fn deletion_by_event(keys: &Keys, target: EventId, created_at: u64) -> Event { - EventBuilder::new(Kind::EventDeletion, "") - .tags(vec![Tag::event(target)]) - .custom_created_at(Timestamp::from_secs(created_at)) - .finalize(keys) - .unwrap() - } - - fn deletion_by_coordinate(keys: &Keys, coordinate: &str, created_at: u64) -> Event { - EventBuilder::new(Kind::EventDeletion, "") - .tags(vec![Tag::custom("a", vec![coordinate.to_string()])]) - .custom_created_at(Timestamp::from_secs(created_at)) - .finalize(keys) - .unwrap() - } - - #[test] - fn duplicate_event_id_deletion_keeps_one_request() { - let keys = Keys::generate(); - let target = EventId::all_zeros(); - let older = deletion_by_event(&keys, target, 1_000); - let newer = deletion_by_event(&keys, target, 1_001); - - let analysis = analyze_deletion_requests(&[older.clone(), newer.clone()]); - - assert_eq!(analysis.keep_ids.len(), 1); - assert_eq!(analysis.remove_ids.len(), 1); - assert!(analysis.keep_ids.contains(&newer.id)); - assert_eq!(analysis.remove_ids, vec![older.id]); - } - - #[test] - fn newer_coordinate_deletion_replaces_older_request() { - let keys = Keys::generate(); - let coordinate = format!("30618:{}:my-repo", keys.public_key().to_hex()); - let older = deletion_by_coordinate(&keys, &coordinate, 1_000); - let newer = deletion_by_coordinate(&keys, &coordinate, 1_100); - - let analysis = analyze_deletion_requests(&[older.clone(), newer.clone()]); - - assert_eq!(analysis.keep_ids.len(), 1); - assert!(analysis.keep_ids.contains(&newer.id)); - assert_eq!(analysis.remove_ids, vec![older.id]); - } - - #[test] - fn no_op_deletion_requests_are_left_alone() { - let keys = Keys::generate(); - let event = EventBuilder::new(Kind::EventDeletion, "") - .custom_created_at(Timestamp::from_secs(1_000)) - .finalize(&keys) - .unwrap(); - - let analysis = analyze_deletion_requests(std::slice::from_ref(&event)); - - assert_eq!(analysis.keep_ids.len(), 1); - assert!(analysis.keep_ids.contains(&event.id)); - assert!(analysis.remove_ids.is_empty()); - } - - #[test] - fn coordinate_for_other_author_is_not_actionable() { - let signer = Keys::generate(); - let other = Keys::generate(); - let coordinate = format!("30618:{}:my-repo", other.public_key().to_hex()); - let event = deletion_by_coordinate(&signer, &coordinate, 1_000); - - let analysis = analyze_deletion_requests(std::slice::from_ref(&event)); - - assert_eq!(analysis.keep_ids.len(), 1); - assert!(analysis.keep_ids.contains(&event.id)); - assert!(analysis.remove_ids.is_empty()); - } - - #[test] - fn broader_request_can_replace_single_target_request() { - let keys = Keys::generate(); - let target = EventId::all_zeros(); - let coordinate = format!("30618:{}:my-repo", keys.public_key().to_hex()); - let single = deletion_by_event(&keys, target, 1_000); - let broader = EventBuilder::new(Kind::EventDeletion, "") - .tags(vec![Tag::event(target), Tag::custom("a", vec![coordinate])]) - .custom_created_at(Timestamp::from_secs(1_100)) - .finalize(&keys) - .unwrap(); - - let analysis = analyze_deletion_requests(&[single.clone(), broader.clone()]); - - assert_eq!(analysis.keep_ids.len(), 1); - assert!(analysis.keep_ids.contains(&broader.id)); - assert_eq!(analysis.remove_ids, vec![single.id]); - } -} diff --git a/src/server.rs b/src/server.rs index d47a831..ff6cd24 100644 --- a/src/server.rs +++ b/src/server.rs @@ -188,7 +188,10 @@ impl RelayServer { .set_local_relay(relay_runtime.relay.clone()); let deletion_runtime = relay_runtime.deletion_runtime.clone(); - deletion_runtime.run_startup_tasks().await; + deletion_runtime + .run_startup_tasks() + .await + .context("failed deletion lifecycle startup reconciliation")?; // Wire the GRASP-06 `/prs/` filesystem cleanup context into // purgatory so the standard expiry sweep can delete dangling diff --git a/tests/common/mod.rs b/tests/common/mod.rs index c6a61fe..e097e0c 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -15,5 +15,5 @@ pub use mock_relay::MockRelay; pub use nip09_helpers::*; pub use port::{reserve_port, PortReservation}; pub use purgatory_helpers::*; -pub use relay::TestRelay; +pub use relay::{DeletionLifecycleOptions, TestRelay}; pub use sync_helpers::*; diff --git a/tests/common/relay.rs b/tests/common/relay.rs index a3cde6a..07b4545 100644 --- a/tests/common/relay.rs +++ b/tests/common/relay.rs @@ -79,6 +79,23 @@ struct RelayOptions { repository_blacklist: Option, git_data_path: Option, relay_data_path: Option, + deletion_lifecycle: Option, +} + +/// Focused configuration for short-lived deletion-request lifecycle tests. +/// +/// Explicit paths are owned by the caller, so they can be reused after +/// [`TestRelay::stop`] to exercise LMDB restart reconciliation. +#[derive(Clone, Debug)] +pub struct DeletionLifecycleOptions { + pub unused_served_retention_secs: u64, + pub unused_gating_additional_secs: u64, + pub used_served_after_last_use_secs: u64, + pub used_gating_additional_secs: u64, + pub cleanup_interval_secs: u64, + pub git_data_path: Option, + pub relay_data_path: Option, + pub deletion_request_disrespector: bool, } impl TestRelay { @@ -255,6 +272,25 @@ impl TestRelay { .await } + /// Start an LMDB relay with explicit, short deletion-request lifecycle + /// timings. This intentionally groups the retention knobs used by the + /// subprocess lifecycle tests rather than expanding the public constructor + /// matrix with positional duration arguments. + pub async fn start_with_deletion_lifecycle(options: DeletionLifecycleOptions) -> Self { + Self::start_internal( + port::reserve_port(), + RelayOptions { + deletion_request_disrespector: options.deletion_request_disrespector, + lmdb_backend: true, + git_data_path: options.git_data_path.clone(), + relay_data_path: options.relay_data_path.clone(), + deletion_lifecycle: Some(options), + ..RelayOptions::default() + }, + ) + .await + } + /// Start a relay with LMDB backend + deletion disrespector mode. pub async fn start_with_lmdb_deletion_disrespector() -> Self { Self::start_internal( @@ -484,6 +520,29 @@ impl TestRelay { cmd.env("NGIT_REPOSITORY_BLACKLIST", blacklist); } + if let Some(lifecycle) = &options.deletion_lifecycle { + cmd.env( + "NGIT_DELETION_REQUEST_RETENTION_UNUSED_SERVED_SECS", + lifecycle.unused_served_retention_secs.to_string(), + ) + .env( + "NGIT_DELETION_REQUEST_RETENTION_UNUSED_UNSERVED_GATING_ADDITIONAL_SECS", + lifecycle.unused_gating_additional_secs.to_string(), + ) + .env( + "NGIT_DELETION_REQUEST_RETENTION_USED_SERVED_AFTER_LAST_USED_SECS", + lifecycle.used_served_after_last_use_secs.to_string(), + ) + .env( + "NGIT_DELETION_REQUEST_RETENTION_USED_UNSERVED_GATING_ADDITIONAL_SECS", + lifecycle.used_gating_additional_secs.to_string(), + ) + .env( + "NGIT_HOLDING_CLEANUP_INTERVAL_SECS", + lifecycle.cleanup_interval_secs.to_string(), + ); + } + // Release the port reservation immediately before spawning the // subprocess that will bind it. Holding the reservation through // env-var setup above is what keeps any concurrent diff --git a/tests/lifecycle/deletion_request_retention.rs b/tests/lifecycle/deletion_request_retention.rs new file mode 100644 index 0000000..3e094bc --- /dev/null +++ b/tests/lifecycle/deletion_request_retention.rs @@ -0,0 +1,527 @@ +//! Live-relay contracts for bounded deletion-request retention. +//! +//! These deliberately cover only the subprocess-visible lifecycle boundaries: +//! main-database serving, admission gates, periodic cleanup, LMDB persistence, +//! and policy-mode restart reconciliation. Component edge cases live in Stage +//! 10A's unit tests. + +#[path = "../common/mod.rs"] +mod common; + +use common::{ + build_deletion, build_vanish, publish_served_repo, DeletionLifecycleOptions, TestRelay, +}; +use grasp_audit::{AuditClient, AuditConfig}; +use ngit_grasp::nostr::lifecycle::Tombstones; +use nostr_sdk::prelude::*; +use std::future::Future; +use std::path::PathBuf; +use std::time::Duration; + +const POLL_INTERVAL: Duration = Duration::from_millis(100); +const POLL_TIMEOUT: Duration = Duration::from_secs(10); + +fn lifecycle_options() -> DeletionLifecycleOptions { + DeletionLifecycleOptions { + // Durations are whole seconds because lifecycle timestamps are durable + // Unix seconds. The non-zero two/four-second boundaries leave a full + // scheduler tick of slack without making this subprocess suite costly. + unused_served_retention_secs: 2, + unused_gating_additional_secs: 4, + used_served_after_last_use_secs: 2, + used_gating_additional_secs: 4, + cleanup_interval_secs: 1, + git_data_path: None, + relay_data_path: None, + deletion_request_disrespector: false, + } +} + +async fn wait_until(description: &str, mut condition: F) +where + F: FnMut() -> Fut, + Fut: Future, +{ + let deadline = tokio::time::Instant::now() + POLL_TIMEOUT; + loop { + if condition().await { + return; + } + assert!( + tokio::time::Instant::now() < deadline, + "timed out after {:?}: {description}", + POLL_TIMEOUT + ); + tokio::time::sleep(POLL_INTERVAL).await; + } +} + +async fn is_served(client: &AuditClient, id: EventId) -> bool { + client + .is_event_on_relay(id) + .await + .expect("query relay event") +} + +async fn open_tombstones(relay_data_path: PathBuf) -> Tombstones { + Tombstones::open_lmdb(&relay_data_path) + .await + .expect("open persistent tombstone store") +} + +#[tokio::test] +async fn unused_request_moves_from_served_probation_to_unserved_gate() { + let relay = TestRelay::start_with_deletion_lifecycle(lifecycle_options()).await; + let client = AuditClient::new(relay.url(), AuditConfig::isolated()) + .await + .expect("create client"); + let (announcement, _) = publish_served_repo(&client, "unused-gate").await; + let target = client + .create_issue( + &announcement, + "late target", + "blocked after probation", + vec![], + ) + .expect("build target"); + let request = build_deletion(&client, &[target.id], &[]); + + client + .send_event(request.clone()) + .await + .expect("accept pre-emptive deletion request"); + assert!( + is_served(&client, request.id).await, + "new request is served" + ); + + wait_until( + "unused request removed from normal relay queries", + || async { !is_served(&client, request.id).await }, + ) + .await; + + let rejected = client.send_event(target).await; + assert!( + rejected.is_err(), + "unserved request must still gate its target" + ); + wait_until( + "gating use promotes request back to served state", + || async { is_served(&client, request.id).await }, + ) + .await; + + let store = open_tombstones(relay.relay_data_path().clone()).await; + assert!( + store + .lifecycle_for_request_result(&request.id) + .await + .expect("read lifecycle") + .expect("retained lifecycle") + .last_used_at + .is_some(), + "admission rejection must persist a last-used timestamp" + ); + relay.stop().await; +} + +#[tokio::test] +async fn unused_request_expires_completely_after_its_gate_period() { + let relay = TestRelay::start_with_deletion_lifecycle(lifecycle_options()).await; + let client = AuditClient::new(relay.url(), AuditConfig::isolated()) + .await + .expect("create client"); + let (announcement, _) = publish_served_repo(&client, "unused-expiry").await; + let target = client + .create_issue( + &announcement, + "future target", + "accepted after expiry", + vec![], + ) + .expect("build target"); + let request = build_deletion(&client, &[target.id], &[]); + client + .send_event(request.clone()) + .await + .expect("accept pre-emptive deletion request"); + + let relay_data_path = relay.relay_data_path().clone(); + wait_until( + "unused request payload and lifecycle metadata permanently removed", + || { + let relay_data_path = relay_data_path.clone(); + async move { + let store = open_tombstones(relay_data_path).await; + store + .lifecycle_for_request_result(&request.id) + .await + .expect("read lifecycle") + .is_none() + && store + .request_payloads() + .await + .expect("read request payloads") + .into_iter() + .all(|payload| payload.id != request.id) + } + }, + ) + .await; + assert!( + !is_served(&client, request.id).await, + "expired request is unserved" + ); + + client + .send_event(target.clone()) + .await + .expect("expired request must no longer reject target"); + wait_until( + "target accepted after request expiry is queryable", + || async { is_served(&client, target.id).await }, + ) + .await; + relay.stop().await; +} + +#[tokio::test] +async fn used_request_reset_from_unserved_gate_outlives_original_expiry() { + let relay = TestRelay::start_with_deletion_lifecycle(lifecycle_options()).await; + let client = AuditClient::new(relay.url(), AuditConfig::isolated()) + .await + .expect("create client"); + let (announcement, _) = publish_served_repo(&client, "used-reset").await; + let target = client + .create_issue(&announcement, "delete then retry", "used lifecycle", vec![]) + .expect("build target"); + client + .send_event(target.clone()) + .await + .expect("accept target before deletion"); + let request = build_deletion(&client, &[target.id], &[]); + client + .send_event(request.clone()) + .await + .expect("accept deletion of existing target"); + wait_until("used request initially served", || async { + is_served(&client, request.id).await + }) + .await; + let initial_last_used_at = open_tombstones(relay.relay_data_path().clone()) + .await + .lifecycle_for_request_result(&request.id) + .await + .expect("read initial lifecycle") + .expect("used lifecycle exists") + .last_used_at + .expect("deletion of existing target records last use"); + + wait_until("used request moves to its unserved gate", || async { + !is_served(&client, request.id).await + }) + .await; + assert!( + client.send_event(target).await.is_err(), + "used request must still reject during its unserved gate" + ); + wait_until("gate rejection restores served request", || async { + is_served(&client, request.id).await + }) + .await; + + // The original deadline is six seconds after first use. Poll past it rather + // than sleeping for a scheduler-dependent duration; the reset request is + // still inside its own additional gate window at this point. + wait_until( + "reset lifecycle survives its original expiry deadline", + || { + let relay_data_path = relay.relay_data_path().clone(); + async move { + if Timestamp::now().as_secs() < initial_last_used_at.as_secs() + 7 { + return false; + } + open_tombstones(relay_data_path) + .await + .lifecycle_for_request_result(&request.id) + .await + .expect("read lifecycle") + .and_then(|record| record.last_used_at) + .is_some() + } + }, + ) + .await; + relay.stop().await; +} + +#[tokio::test] +async fn lmdb_restart_reconciles_current_mode_without_resetting_lifecycle() { + let directories = tempfile::tempdir().expect("create persistent parent directory"); + let git_data_path = directories.path().join("git"); + let relay_data_path = directories.path().join("relay"); + let options = DeletionLifecycleOptions { + unused_served_retention_secs: 15, + unused_gating_additional_secs: 15, + used_served_after_last_use_secs: 15, + used_gating_additional_secs: 15, + cleanup_interval_secs: 1, + git_data_path: Some(git_data_path), + relay_data_path: Some(relay_data_path.clone()), + deletion_request_disrespector: false, + }; + let normal = TestRelay::start_with_deletion_lifecycle(options.clone()).await; + let author = AuditClient::new(normal.url(), AuditConfig::isolated()) + .await + .expect("create author"); + let (announcement, _) = publish_served_repo(&author, "restart-reconcile").await; + let accepted_in_archive = author + .create_issue(&announcement, "archive mode target", "must survive", vec![]) + .expect("build archive target"); + let normally_gated = author + .create_issue( + &announcement, + "normal mode target", + "must be blocked", + vec![], + ) + .expect("build normal target"); + let already_used_target = author + .create_issue( + &announcement, + "already used target", + "timestamps survive mode changes", + vec![], + ) + .expect("build already-used target"); + author + .send_event(already_used_target.clone()) + .await + .expect("store target for an immediately used request"); + let archive_request = build_deletion(&author, &[accepted_in_archive.id], &[]); + let gate_request = build_deletion(&author, &[normally_gated.id], &[]); + let used_request = build_deletion(&author, &[already_used_target.id], &[]); + for request in [&archive_request, &gate_request, &used_request] { + author + .send_event(request.clone()) + .await + .expect("persist retained request"); + } + let before = open_tombstones(relay_data_path.clone()) + .await + .lifecycle_for_request_result(&used_request.id) + .await + .expect("read retained lifecycle") + .expect("retained lifecycle exists"); + assert!( + before.last_used_at.is_some(), + "request that deleted an existing target must be marked used" + ); + normal.stop().await; + + let mut archival_options = options.clone(); + archival_options.deletion_request_disrespector = true; + let archival = TestRelay::start_with_deletion_lifecycle(archival_options).await; + let archive_client = AuditClient::new_with_keys( + archival.url(), + AuditConfig::isolated(), + author.keys().clone(), + ) + .await + .expect("reconnect author to archival relay"); + assert!( + is_served(&archive_client, gate_request.id).await, + "request remains served" + ); + archive_client + .send_event(accepted_in_archive.clone()) + .await + .expect("disrespector mode accepts previously targeted event"); + wait_until("disrespector leaves stored target queryable", || async { + is_served(&archive_client, accepted_in_archive.id).await + }) + .await; + archival.stop().await; + + let restored = TestRelay::start_with_deletion_lifecycle(options).await; + let restored_client = AuditClient::new_with_keys( + restored.url(), + AuditConfig::isolated(), + author.keys().clone(), + ) + .await + .expect("reconnect author to normal relay"); + assert!( + is_served(&restored_client, accepted_in_archive.id).await, + "normal-mode reconciliation must not mutate target stored in archival mode" + ); + assert!( + restored_client.send_event(normally_gated).await.is_err(), + "normal-mode restart must restore retained deletion gates" + ); + let after = open_tombstones(relay_data_path).await; + let after = after + .lifecycle_for_request_result(&used_request.id) + .await + .expect("read reconciled lifecycle") + .expect("reconciled lifecycle exists"); + assert_eq!( + before.first_seen_at, after.first_seen_at, + "restart must not reset first seen" + ); + assert_eq!( + before.last_used_at, after.last_used_at, + "restart must not reset last use" + ); + restored.stop().await; +} + +#[tokio::test] +async fn nip62_targeting_gates_while_non_targeting_request_never_does() { + let relay = TestRelay::start_with_deletion_lifecycle(lifecycle_options()).await; + let repo_owner = AuditClient::new(relay.url(), AuditConfig::isolated()) + .await + .expect("create repository owner"); + let (announcement, _) = publish_served_repo(&repo_owner, "nip62-retention").await; + let targeted_author = AuditClient::new(relay.url(), AuditConfig::isolated()) + .await + .expect("create targeted author"); + let non_targeted_author = AuditClient::new(relay.url(), AuditConfig::isolated()) + .await + .expect("create non-targeted author"); + + let targeting = build_vanish(&targeted_author, None); + targeted_author + .send_event(targeting.clone()) + .await + .expect("accept targeting vanish request"); + let blocked = targeted_author + .create_issue(&announcement, "blocked by vanish", "must reject", vec![]) + .expect("build later targeted event"); + assert!( + targeted_author.send_event(blocked).await.is_err(), + "targeting vanish must gate later author events" + ); + + let non_targeting = build_vanish(&non_targeted_author, Some("wss://other.example")); + non_targeted_author + .send_event(non_targeting.clone()) + .await + .expect("accept non-targeting vanish request"); + assert!(is_served(&non_targeted_author, non_targeting.id).await); + wait_until("non-targeting vanish follows served retention", || async { + !is_served(&non_targeted_author, non_targeting.id).await + }) + .await; + let allowed = non_targeted_author + .create_issue(&announcement, "not locally vanished", "must accept", vec![]) + .expect("build later non-targeted event"); + non_targeted_author + .send_event(allowed.clone()) + .await + .expect("non-targeting vanish must never gate its author"); + wait_until("non-targeted author event is queryable", || async { + is_served(&non_targeted_author, allowed.id).await + }) + .await; + relay.stop().await; +} + +#[tokio::test] +async fn nip09_replay_from_unserved_gate_does_not_restore_its_served_copy() { + let relay = TestRelay::start_with_deletion_lifecycle(lifecycle_options()).await; + let client = AuditClient::new(relay.url(), AuditConfig::isolated()) + .await + .expect("create client"); + let request = build_deletion(&client, &[], &[]); + + client + .send_event(request.clone()) + .await + .expect("accept initial deletion request"); + wait_until("deletion request moves to its unserved gate", || async { + !is_served(&client, request.id).await + }) + .await; + client + .send_event(request.clone()) + .await + .expect("exact replay receives the duplicate acknowledgement"); + assert!( + !is_served(&client, request.id).await, + "duplicate replay must not restore an unserved deletion request to Main" + ); + relay.stop().await; +} + +#[tokio::test] +async fn non_targeting_nip62_replay_from_unserved_gate_does_not_restore_its_served_copy() { + let relay = TestRelay::start_with_deletion_lifecycle(lifecycle_options()).await; + let client = AuditClient::new(relay.url(), AuditConfig::isolated()) + .await + .expect("create client"); + let request = build_vanish(&client, Some("wss://other.example")); + + client + .send_event(request.clone()) + .await + .expect("accept initial non-targeting vanish request"); + wait_until( + "non-targeting vanish request moves to its unserved gate", + || async { !is_served(&client, request.id).await }, + ) + .await; + client + .send_event(request.clone()) + .await + .expect("exact replay receives the duplicate acknowledgement"); + assert!( + !is_served(&client, request.id).await, + "duplicate replay must not restore an unserved non-targeting vanish request to Main" + ); + + let record = open_tombstones(relay.relay_data_path().clone()) + .await + .lifecycle_for_request_result(&request.id) + .await + .expect("read request lifecycle") + .expect("retain request lifecycle"); + assert_eq!( + record.last_used_at, None, + "replaying a non-targeting vanish must not count as use" + ); + relay.stop().await; +} + +#[tokio::test] +async fn disrespector_nip62_replay_does_not_make_a_noop_request_used() { + let mut options = lifecycle_options(); + options.deletion_request_disrespector = true; + let relay = TestRelay::start_with_deletion_lifecycle(options).await; + let client = AuditClient::new(relay.url(), AuditConfig::isolated()) + .await + .expect("create client"); + let request = build_vanish(&client, None); + + client + .send_event(request.clone()) + .await + .expect("accept initial no-op vanish request"); + client + .send_event(request.clone()) + .await + .expect("accept exact no-op vanish replay"); + + let record = open_tombstones(relay.relay_data_path().clone()) + .await + .lifecycle_for_request_result(&request.id) + .await + .expect("read request lifecycle") + .expect("retain request lifecycle"); + assert_eq!( + record.last_used_at, None, + "replaying a no-op vanish must not make it indefinitely retained in disrespector mode" + ); + relay.stop().await; +} diff --git a/tests/lifecycle/nip09_disrespector.rs b/tests/lifecycle/nip09_disrespector.rs index a64a47e..556baf1 100644 --- a/tests/lifecycle/nip09_disrespector.rs +++ b/tests/lifecycle/nip09_disrespector.rs @@ -5,9 +5,9 @@ //! ngit-grasp instance driven by the [`TestRelay`] fixture. //! //! These tests deliberately assert only on observable contract, not on any -//! internal "holding database" — the current implementation is tombstone / -//! purgatory based, so the disrespector simply accepts the kind-5 request and -//! does nothing with it. The two properties under test are: +//! internal lifecycle metadata or "holding database" — the disrespector accepts +//! and read-only evaluates kind-5 requests, but never mutates their targets. +//! The two observable properties under test are: //! //! 1. **Disrespector accepts but ignores deletion/vanish** — a kind-5 or //! kind-62 request gets an `OK`, yet the targeted/author events remain