From 8335651e3f01cd9312bf1afa4d8dfc0cbf5d16d4 Mon Sep 17 00:00:00 2001 From: Paul Logan Date: Mon, 15 Jun 2026 23:02:12 -0700 Subject: [PATCH] feat(nostr): persist self nostr relays so the pull-loop is correct when asymmetric MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to #332. The pull-loop pulled only from `peers[*].nostr_transport.relay` (the relays we reach *peers* on), which is correct only when both sides paired over the same relay. A peer sends to us by publishing to a relay *we're* reachable on — not necessarily one we reach them on. So record that: - `endpoints::pin_self_nostr_relay` / `self_nostr_relays` — a deduped `self.nostr_relays[]` set (additive on the self block; composes with the existing slot fields). - `wire nostr pair`/`accept`/`fetch --relay X` now record X as a relay we're reachable on (accept folds it into its existing relay-state RMW). - `relay::nostr_relays_from_peers` now unions `self.nostr_relays[]` (the authoritative "where peers publish our inbound" set) with the peer-transport relays (still covers the symmetric case before a self-relay is recorded). Net: a `transport: nostr` peer round-trips regardless of pairing symmetry. Still additive — no nostr relay recorded → empty pull set → HTTP path byte-identical. Unit tests: `self_nostr_relay_roundtrips_and_dedups` (roundtrip, dedup, empty, doesn't clobber other self keys) + the relay-helper test now asserts the self∪peer union + dedup. 600 lib tests green; clippy clean. Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 2 +- src/cli/nostr.rs | 12 ++++++++- src/cli/relay.rs | 42 +++++++++++++++++++++++------- src/endpoints.rs | 68 ++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 112 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6bb3c2c..c264e3c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,7 @@ the PR description linked in each section. ### Added -- **Nostr transport is now bidirectional through the daemon** (#227 D3, RFC-007): `run_sync_pull` now also pulls Nostr-delivered events — for each relay a peer is reachable on (`peers[*].nostr_transport.relay`) it pulls events `#p`-tagged to our npub (`kind:1`), transport-verifies them (NIP-01 id + schnorr), and feeds them through the **same `process_events` path** as HTTP-pulled events (inner Ed25519 signature + trust pin + inbox dedup all reused). Together with the send-path fallback (#322), a `transport: nostr` peer now fully round-trips through the daemon. Strictly additive — a no-op when this session isn't `wire enroll nostr`'d or no peer carries a nostr transport (the HTTP-slot pull is byte-identical). The last RFC-007 D3 piece; live public-relay e2e (AC-3) remains a manual dogfood via `wire nostr`. +- **Nostr transport is now bidirectional through the daemon** (#227 D3, RFC-007): `run_sync_pull` now also pulls Nostr-delivered events — it pulls events `#p`-tagged to our npub (`kind:1`) from the relays we're reachable on, transport-verifies them (NIP-01 id + schnorr), and feeds them through the **same `process_events` path** as HTTP-pulled events (inner Ed25519 signature + trust pin + inbox dedup all reused). The pull set is `self.nostr_relays[]` (relays we paired/fetched over — where peers publish *our* inbound, so it's correct even when pairing is asymmetric) unioned with the peer-transport relays; `wire nostr pair/accept/fetch --relay X` records X. Together with the send-path fallback (#322), a `transport: nostr` peer now fully round-trips through the daemon. Strictly additive — a no-op when this session isn't `wire enroll nostr`'d or has no nostr relay recorded (the HTTP-slot pull is byte-identical). The last RFC-007 D3 piece; live public-relay e2e (AC-3) remains a manual dogfood via `wire nostr`. - **1.0 surface freeze — deprecation policy + a golden MCP-catalog lock** (ROAD_TO_1.0 §6): published [`docs/DEPRECATION_POLICY.md`](docs/DEPRECATION_POLICY.md) — from 1.0, frozen surfaces (CLI verbs, `--json` shapes, the MCP tool catalog, on-disk state, protocol) change only through a deprecation window (announce → runtime warn → ≥1 MINOR & ≥90 days → remove in the next MAJOR), never a silent break. Added a `mcp_catalog_schema_is_frozen` test that golden-locks all 27 MCP tools' name + input-schema props + `required` list, so the agent-facing API can't drift unnoticed. Also promoted the hello-world first-connection round-trip to a required CI gate (`hello-world` job) now that daemon-survival is fixed (#263). diff --git a/src/cli/nostr.rs b/src/cli/nostr.rs index deca7dd..6994ee0 100644 --- a/src/cli/nostr.rs +++ b/src/cli/nostr.rs @@ -133,8 +133,11 @@ fn cmd_accept(npub: &str, relay: &str, as_json: bool) -> Result<()> { .and_then(Value::as_str) .map(str::to_string) .unwrap_or_else(|| crate::agent_card::display_handle_from_did(&peer_did).to_string()); + // Also record this relay as one WE are reachable on (we received + ack here) + // so the daemon pull-loop pulls our inbound from it — see pin_self_nostr_relay. crate::config::update_relay_state(|rs| { - crate::endpoints::pin_peer_nostr_transport(rs, &peer_handle, npub, relay) + crate::endpoints::pin_peer_nostr_transport(rs, &peer_handle, npub, relay)?; + crate::endpoints::pin_self_nostr_relay(rs, relay) })?; // Send the pair-ack (our card) back over the relay. @@ -186,6 +189,10 @@ fn cmd_pair(npub: &str, relay: &str, as_json: bool) -> Result<()> { ws.publish(&ev).await.context("publish pair-request") })??; + // Record this relay as one we're reachable on (the peer will ack + later + // send to us here) so the daemon pull-loop services it. + crate::config::update_relay_state(|rs| crate::endpoints::pin_self_nostr_relay(rs, relay))?; + if as_json { println!( "{}", @@ -207,6 +214,9 @@ fn cmd_fetch(relay: &str, limit: usize, as_json: bool) -> Result<()> { let nsk = require_transport_key()?; let my_xonly = crate::nostr_key::xonly_from_secret(&nsk) .map_err(|e| anyhow!("transport key unusable: {e}"))?; + // Fetching here means we treat this relay as one we're reachable on — record + // it so the daemon pull-loop keeps servicing it without a manual fetch. + crate::config::update_relay_state(|rs| crate::endpoints::pin_self_nostr_relay(rs, relay))?; let filter = Filter { p_tags: vec![hex::encode(my_xonly)], kinds: vec![ diff --git a/src/cli/relay.rs b/src/cli/relay.rs index 0ea8b43..e6fef26 100644 --- a/src/cli/relay.rs +++ b/src/cli/relay.rs @@ -1408,23 +1408,34 @@ fn inbox_contains_probe_ack(path: &std::path::Path, nonce: &str) -> bool { .any(|e| crate::probe::is_probe_ack_for(&e, nonce)) } -/// RFC-007 D3 pull-loop helper: the distinct relays any peer is reachable on -/// over Nostr (`peers[*].nostr_transport.relay`). In the common symmetric -/// pairing (both sides `wire nostr pair/accept --relay X`) this is exactly the -/// relay a peer publishes our inbound messages to, so it's where we pull from. +/// RFC-007 D3 pull-loop helper: the distinct Nostr relays to pull our inbound +/// from. Two sources, deduped: +/// 1. `self.nostr_relays[]` — relays WE are reachable on (recorded when we +/// `wire nostr pair/accept/fetch --relay X`). A peer sends to us by +/// publishing to a relay *we're* reachable on, so this is the authoritative +/// set — correct even when pairing is asymmetric. +/// 2. `peers[*].nostr_transport.relay` — relays we reach *peers* on. Covers the +/// symmetric same-relay case even before a self-relay was recorded. +/// /// Pure — unit-tested. fn nostr_relays_from_peers(state: &Value) -> Vec { let mut relays: Vec = Vec::new(); + let mut push_distinct = |r: &str| { + if !r.is_empty() && !relays.iter().any(|x| x == r) { + relays.push(r.to_string()); + } + }; + for r in crate::endpoints::self_nostr_relays(state) { + push_distinct(&r); + } if let Some(peers) = state.get("peers").and_then(Value::as_object) { for p in peers.values() { if let Some(r) = p .get("nostr_transport") .and_then(|n| n.get("relay")) .and_then(Value::as_str) - && !r.is_empty() - && !relays.iter().any(|x| x == r) { - relays.push(r.to_string()); + push_distinct(r); } } } @@ -1858,8 +1869,11 @@ mod slot_reresolve_tests { use super::*; #[test] - fn nostr_relays_from_peers_distinct_and_skips_transportless() { + fn nostr_relays_from_peers_unions_self_and_peers_distinct() { let state = serde_json::json!({ + // Relays we're reachable on (the authoritative set) — incl. one no + // peer-transport mentions (the asymmetric case). + "self": { "nostr_relays": ["wss://self", "wss://r1"] }, "peers": { "alice": { "nostr_transport": { "npub": "aa", "relay": "wss://r1" } }, "bob": { "nostr_transport": { "npub": "bb", "relay": "wss://r2" } }, @@ -1873,8 +1887,16 @@ mod slot_reresolve_tests { }); let mut relays = nostr_relays_from_peers(&state); relays.sort(); - assert_eq!(relays, vec!["wss://r1".to_string(), "wss://r2".to_string()]); - // No peers / no transports → empty (the additive no-op case). + // self (wss://self, wss://r1) ∪ peers (r1 dup, r2) → 3 distinct. + assert_eq!( + relays, + vec![ + "wss://r1".to_string(), + "wss://r2".to_string(), + "wss://self".to_string() + ] + ); + // No peers / no transports / no self → empty (the additive no-op case). assert!(nostr_relays_from_peers(&serde_json::json!({})).is_empty()); assert!(nostr_relays_from_peers(&serde_json::json!({"peers": {}})).is_empty()); } diff --git a/src/endpoints.rs b/src/endpoints.rs index 12baf45..9e3f265 100644 --- a/src/endpoints.rs +++ b/src/endpoints.rs @@ -418,6 +418,54 @@ pub fn peer_nostr_transport(relay_state: &Value, peer_handle: &str) -> Option<(S Some((npub, relay)) } +/// RFC-007 D3: record a Nostr relay this session is *reachable on* — one we've +/// paired/fetched over (`wire nostr pair/accept/fetch --relay X`). Persisted as +/// the distinct set `self.nostr_relays[]`. The daemon pull-loop reads this as +/// the authoritative "where do peers publish my inbound" set: a peer sends to me +/// by publishing to a relay *I'm* reachable on, which isn't necessarily a relay +/// *I* reach *them* on (the asymmetric case the peer-transport set misses). +/// Read-modify-write, idempotent (dedups). +pub fn pin_self_nostr_relay(relay_state: &mut Value, relay_url: &str) -> Result<()> { + if relay_url.is_empty() { + return Ok(()); + } + let self_obj = relay_state + .as_object_mut() + .map(|m| { + m.entry("self") + .or_insert_with(|| Value::Object(Default::default())) + }) + .ok_or_else(|| anyhow::anyhow!("relay_state.json root is not an object"))? + .as_object_mut() + .ok_or_else(|| anyhow::anyhow!("relay_state.self is not an object"))?; + let arr = self_obj + .entry("nostr_relays") + .or_insert_with(|| Value::Array(Vec::new())) + .as_array_mut() + .ok_or_else(|| anyhow::anyhow!("relay_state.self.nostr_relays is not an array"))?; + if !arr.iter().any(|v| v.as_str() == Some(relay_url)) { + arr.push(Value::String(relay_url.to_string())); + } + Ok(()) +} + +/// The distinct Nostr relays this session is reachable on (`self.nostr_relays[]`). +/// Empty when never paired over Nostr. +pub fn self_nostr_relays(relay_state: &Value) -> Vec { + relay_state + .get("self") + .and_then(|s| s.get("nostr_relays")) + .and_then(Value::as_array) + .map(|a| { + a.iter() + .filter_map(|v| v.as_str()) + .filter(|s| !s.is_empty()) + .map(str::to_string) + .collect() + }) + .unwrap_or_default() +} + /// Infer an endpoint scope from a relay URL: `unix://` -> Uds, a loopback /// host -> Local, otherwise Federation. LAN is never inferred (a private- /// range IP is indistinguishable from a federation host by URL alone) and @@ -564,6 +612,26 @@ mod tests { assert!(peer_endpoints_in_priority_order(&state, "alice").is_empty()); } + #[test] + fn self_nostr_relay_roundtrips_and_dedups() { + let mut state = json!({}); + pin_self_nostr_relay(&mut state, "wss://r1").unwrap(); + pin_self_nostr_relay(&mut state, "wss://r2").unwrap(); + pin_self_nostr_relay(&mut state, "wss://r1").unwrap(); // dup → no-op + pin_self_nostr_relay(&mut state, "").unwrap(); // empty → no-op + assert_eq!( + self_nostr_relays(&state), + vec!["wss://r1".to_string(), "wss://r2".to_string()] + ); + // Composes with an existing self block (doesn't clobber other self keys). + let mut state2 = json!({"self": {"relay_url": "https://wireup.net", "slot_id": "s"}}); + pin_self_nostr_relay(&mut state2, "wss://x").unwrap(); + assert_eq!(state2["self"]["relay_url"], "https://wireup.net"); + assert_eq!(self_nostr_relays(&state2), vec!["wss://x".to_string()]); + // Absent → empty. + assert!(self_nostr_relays(&json!({})).is_empty()); + } + #[test] fn endpoints_are_local_only_catches_loopback_pin_incl_mislabeled_scope() { // Empty → false (nothing to judge).