From 2236371537004fc87c51e13362250a27666a81f2 Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Sat, 19 Sep 2026 01:47:09 +0000 Subject: [PATCH] fix(nostr): stop signing traversal deletion requests with the routing key After a traversal attempt, both sides published a NIP-09 deletion request signed by the node's routing key and naming the attempt's offer and answer wraps. Each request put the node's public identity next to the ids of its traversal signals on every relay it reached, which the one-time signing keys on the wraps are meant to avoid. What the requests deleted differed by side. A relay honouring NIP-59 deletes a gift wrap when the request is signed by the wrap's p-tagged recipient, so the initiator's request, sent after a successful punch, did delete the answer wrap addressed to it; strfry in the NAT lab logged that deletion. The offer it also named is addressed to the responder, and the responder's requests named only the answer it had sent to the initiator, so those deleted nothing. Drop all three traversal calls (the initiator after a punch, the responder on refusing an offer, and the responder after its punch attempt), and accept that an answer wrap now stays on a relay that stores it until its NIP-40 expiration. The wraps are ephemeral kinds, so relays that do not store ephemeral events never held them. The advertisement retraction keeps its deletion request, since it names an event the routing key signed itself. The signal envelope's event id had no other reader and is removed with its import. The NAT lab now asserts that its relay holds no kind 5 event after the cone, symmetric and lan scenarios, scanning the relay's own store and failing if the scan cannot run or be parsed. Before the fix each scenario failed it: cone and lan held two requests (the initiator's naming offer and answer, the responder's naming the answer) and symmetric held the responder's one. After it, all three held none. The responder's refusal path is not reached by any lab scenario and is covered by reading only. --- src/nostr/runtime.rs | 8 ---- src/nostr/signal.rs | 2 - testing/nat/scripts/nat-test.sh | 51 +++++++++++++++++++++++++ testing/nat/scripts/nostr-relay-test.sh | 6 +-- 4 files changed, 54 insertions(+), 13 deletions(-) diff --git a/src/nostr/runtime.rs b/src/nostr/runtime.rs index 7903c94f..557441b0 100644 --- a/src/nostr/runtime.rs +++ b/src/nostr/runtime.rs @@ -850,7 +850,6 @@ impl NostrRendezvous { { let _ = tx.send(SignalEnvelope { payload: answer, - event_id: event.id, sender_npub: sender_npub.clone(), }); } @@ -1301,10 +1300,6 @@ impl NostrRendezvous { "traversal: initiator punch succeeded" ); - let _ = self - .publish_delete(&relays, [offer_event.id, answer.event_id]) - .await; - self.failure_state .record_success(&peer_config.npub, now_ms()); @@ -1471,7 +1466,6 @@ impl NostrRendezvous { "traversal: answer sent" ); if !accepted { - let _ = self.publish_delete(&relays, [answer_event.id]).await; return Ok(()); } @@ -1521,8 +1515,6 @@ impl NostrRendezvous { ); } } - - let _ = self.publish_delete(&relays, [answer_event.id]).await; Ok(()) } diff --git a/src/nostr/signal.rs b/src/nostr/signal.rs index 6b2306e5..3d3eddac 100644 --- a/src/nostr/signal.rs +++ b/src/nostr/signal.rs @@ -1,4 +1,3 @@ -use nostr::EventId; use nostr::nips::{nip44, nip59}; use nostr::prelude::{ Event, EventBuilder, JsonUtil, Kind, NostrSigner, PublicKey, Tag, Timestamp, UnsignedEvent, @@ -19,7 +18,6 @@ pub(crate) const FRESHNESS_SKEW_TOLERANCE_MS: u64 = 60_000; pub(super) struct SignalEnvelope { pub(super) payload: T, - pub(super) event_id: EventId, pub(super) sender_npub: String, } diff --git a/testing/nat/scripts/nat-test.sh b/testing/nat/scripts/nat-test.sh index aec7aa72..5e9c040e 100755 --- a/testing/nat/scripts/nat-test.sh +++ b/testing/nat/scripts/nat-test.sh @@ -448,6 +448,45 @@ ping_peer() { fi } +# Fail if the relay holds any kind 5 deletion request. +# +# A node signs a deletion request with its routing key, so one naming a +# traversal signal's wrap would tie that key to a signal sent under a one-time +# key. Nodes delete only adverts they withdraw, and the lab's nodes advertise +# throughout, so after a scenario the relay should hold none. strfry scan reads +# the relay's own store and prints one event per line. A scan that cannot run +# or print parseable events has observed nothing, so it fails the check too. +assert_no_deletion_requests() { + local relay="$1" events="" report="" + if ! events="$(docker exec "$relay" strfry scan '{"kinds":[5]}' 2>/dev/null)"; then + echo "FAIL: could not scan $relay for deletion requests" >&2 + return 1 + fi + if ! report="$(python3 -c ' +import json, sys +events = [] +for line in sys.stdin: + line = line.strip() + if line: + events.append(json.loads(line)) +for ev in events: + ids = [t[1] for t in ev.get("tags", []) if len(t) > 1 and t[0] == "e"] + print(" kind 5 by %s... naming %s" % (ev["pubkey"][:16], ", ".join(ids))) +print(len(events)) +' <<<"$events")"; then + echo "FAIL: could not parse $relay's scan for deletion requests" >&2 + return 1 + fi + local count="${report##*$'\n'}" + if [ "$count" != "0" ]; then + echo "FAIL: $relay holds $count deletion request(s):" >&2 + sed '$d' <<<"$report" >&2 + return 1 + fi + echo " $relay holds no deletion requests" + return 0 +} + # A relay that faulted while a scenario's assertions still passed is a finding # about the relay, not about the scenario, so it is reported and not made a # failure: the scenario proved what it set out to prove. @@ -497,6 +536,10 @@ run_cone() { dump_cone_diagnostics return 1 } + assert_no_deletion_requests "$RELAY_CONTAINER" || { + dump_cone_diagnostics + return 1 + } note_relay_event cleanup } @@ -543,6 +586,10 @@ run_symmetric() { dump_symmetric_diagnostics return 1 } + assert_no_deletion_requests "$RELAY_CONTAINER" || { + dump_symmetric_diagnostics + return 1 + } note_relay_event cleanup } @@ -586,6 +633,10 @@ run_lan() { dump_lan_diagnostics return 1 } + assert_no_deletion_requests "$RELAY_CONTAINER" || { + dump_lan_diagnostics + return 1 + } note_relay_event # Skip the final teardown when the mesh-lab harness wraps this # script: it needs to docker-logs the containers before teardown, diff --git a/testing/nat/scripts/nostr-relay-test.sh b/testing/nat/scripts/nostr-relay-test.sh index 8cffbb2c..df6bdfff 100755 --- a/testing/nat/scripts/nostr-relay-test.sh +++ b/testing/nat/scripts/nostr-relay-test.sh @@ -115,9 +115,9 @@ dump_diagnostics() { # # Which rejection they take is not what the event's gibberish `content` # suggests. `parse_overlay_advert_event` looks for the `protocol` tag -# first (src/nostr/runtime.rs:1665-1671) and this event carries only `d` +# first (src/nostr/runtime.rs:1657-1663) and this event carries only `d` # and `app`, so it fails with `missing required protocol tag` and never -# reaches the `serde_json::from_str` at :1679. The content is therefore +# reaches the `serde_json::from_str` at :1671. The content is therefore # belt and braces rather than the thing under test. # # Neither branch logs anything: see the coverage-gap note in run_test. @@ -236,7 +236,7 @@ while int.from_bytes(secret, "big") == 0 or int.from_bytes(secret, "big") >= N: pubkey = xonly_pubkey(secret).hex() created_at = int(time.time()) # Both of these must match the consumers' subscription filter, which is -# kind + identifier and no author clause (src/nostr/runtime.rs:1042-1044). +# kind + identifier and no author clause (src/nostr/runtime.rs:1041-1043). # The literals are ADVERT_KIND and ADVERT_IDENTIFIER in src/nostr/types.rs # and are duplicated here rather than derived, so changing either there # silently stops this event reaching the daemons while the relay goes on