Merge branch 'maint'

# Conflicts:
#	src/node/retry.rs

src/node/bloom.rs

@@ -29,27 +29,19 @@ impl Node {
         filters
     }
 
-    /// Build a FilterAnnounce for a specific peer.
-    ///
-    /// The outgoing filter excludes the destination peer's own filter
-    /// to prevent routing loops (don't tell a peer about destinations
-    /// reachable only through them).
-    fn build_filter_announce(&mut self, exclude_peer: &NodeAddr) -> FilterAnnounce {
-        let peer_filters = self.peer_inbound_filters();
-        let filter = self
-            .bloom_state
-            .compute_outgoing_filter(exclude_peer, &peer_filters);
-        let sequence = self.bloom_state.next_sequence();
-        FilterAnnounce::new(filter, sequence)
-    }
-
     /// Send a FilterAnnounce to a specific peer, respecting debounce.
     ///
+    /// `filter` is the outgoing filter for this peer, already computed
+    /// with the destination peer's own contribution excluded to prevent
+    /// routing loops (don't tell a peer about destinations reachable
+    /// only through them).
+    ///
     /// If the peer is rate-limited, the update stays pending for
     /// delivery on the next tick cycle.
     pub(super) async fn send_filter_announce_to_peer(
         &mut self,
         peer_addr: &NodeAddr,
+        filter: BloomFilter,
     ) -> Result<(), NodeError> {
         let now_ms = std::time::SystemTime::now()
             .duration_since(std::time::UNIX_EPOCH)
@@ -64,7 +56,7 @@ impl Node {
         }
 
         // Build and encode
-        let announce = self.build_filter_announce(peer_addr);
+        let announce = FilterAnnounce::new(filter, self.bloom_state.next_sequence());
         let sent_filter = announce.filter.clone();
         let encoded = announce.encode().map_err(|e| NodeError::SendFailed {
             node_addr: *peer_addr,
@@ -142,8 +134,24 @@ impl Node {
             .copied()
             .collect();
 
+        if ready.is_empty() {
+            return;
+        }
+
+        // One snapshot and one union pass for the whole ready set. The
+        // send path never mutates peer inbound filters or the tree state,
+        // and the rx loop holds `&mut self` across the awaits, so the
+        // snapshot cannot go stale mid-loop.
+        let peer_filters = self.peer_inbound_filters();
+        let mut outgoing = self
+            .bloom_state
+            .compute_outgoing_filters(&ready, &peer_filters);
+
         for peer_addr in ready {
-            if let Err(e) = self.send_filter_announce_to_peer(&peer_addr).await {
+            let Some(filter) = outgoing.remove(&peer_addr) else {
+                continue;
+            };
+            if let Err(e) = self.send_filter_announce_to_peer(&peer_addr, filter).await {
                 debug!(
                     peer = %self.peer_display_name(&peer_addr),
                     error = %e,

src/node/mod.rs

@@ -1142,7 +1142,7 @@ impl Node {
             return name.clone();
         }
         if let Some(peer) = self.peers.get(addr) {
-            return peer.identity().short_npub();
+            return peer.short_npub().to_string();
         }
         if let Some(entry) = self.sessions.get(addr) {
             let (xonly, _) = entry.remote_pubkey().x_only_public_key();

src/node/peering/driver.rs

@@ -104,16 +104,24 @@ impl Node {
                 continue;
             };
 
-            // Refresh the peer's overlay advert before retrying. The cache is
+            // Kick off a refresh of the peer's overlay advert. The cache is
             // read-only on hit, so a retry without a refetch dials the same
             // cached endpoint — and the most common reason a peer landed in the
             // retry schedule is that endpoint just stopped working (NAT rebind,
-            // port change, peer restart). Cheap (one Filter fetch, bounded by
-            // the retry backoff cadence).
+            // port change, peer restart).
+            //
+            // Fire-and-forget, NOT awaited: this runs inline on the 1s rx-loop
+            // tick, and the fetch carries a 2s relay timeout that would stall
+            // the tick — and every other rx-loop arm with it — by up to 2s per
+            // due peer. So the dial below uses whatever advert is cached now
+            // and the refreshed one lands for the *next* retry of this peer.
+            // Retries are backoff-paced, so that defers the benefit by one
+            // backoff interval rather than losing it.
             if let Some(bootstrap) = self.supervisor.nostr_rendezvous.engine_arc() {
-                let _ = bootstrap
-                    .refetch_advert_for_stale_check(&peer_config.npub)
-                    .await;
+                let npub = peer_config.npub.clone();
+                tokio::spawn(async move {
+                    let _ = bootstrap.refetch_advert_for_stale_check(&npub).await;
+                });
             }
 
             match self.initiate_peer_connection(&peer_config).await {

src/node/tests/unit.rs

@@ -1993,6 +1993,101 @@ async fn process_pending_retries_gated_at_capacity() {
     );
 }
 
+/// A TCP listener that accepts connections and then never speaks. A relay
+/// URL pointed at it makes the nostr client's websocket handshake hang, so
+/// `refetch_advert_for_stale_check` burns its full 2s fetch timeout without
+/// any network egress.
+fn spawn_blackhole_relay() -> String {
+    use std::net::TcpListener;
+    let listener = TcpListener::bind("127.0.0.1:0").expect("bind blackhole listener");
+    let port = listener.local_addr().expect("blackhole local addr").port();
+    std::thread::spawn(move || {
+        let mut held = Vec::new();
+        while let Ok((stream, _)) = listener.accept() {
+            held.push(stream);
+        }
+    });
+    format!("ws://127.0.0.1:{port}")
+}
+
+/// The per-tick retry loop must not await the pre-dial advert refetch.
+///
+/// `process_pending_retries` runs inline on the node's 1s rx-loop tick. Each
+/// due peer's refetch carries a 2s relay-fetch timeout, so awaiting it stalls
+/// the whole tick by 2s per peer — up to `MAX_RETRY_CONNECTIONS_PER_TICK`
+/// times in one tick body. The refresh is fire-and-forget: it exists to make
+/// the *next* retry dial a fresh endpoint, and retries are backoff-paced.
+///
+/// Discriminator: wall-clock duration of one `process_pending_retries` call
+/// with several due peers whose refetches all hang. Awaited, the call takes
+/// `2s * peers`; spawned, it returns without waiting on any of them.
+#[tokio::test]
+async fn process_pending_retries_does_not_await_advert_refetch() {
+    use std::time::Instant;
+
+    const DUE_PEERS: usize = 4;
+    // Awaited: >= 8s (4 x 2s). Spawned: milliseconds. A 3s bound sits far
+    // from both, so neither machine load nor the 2s timeout's own slack can
+    // flip the verdict.
+    const MAX_TICK_MS: u128 = 3_000;
+
+    let mut node = make_node_with_max_peers(64);
+
+    let mut bootstrap = crate::nostr::NostrRendezvous::new_for_test();
+    bootstrap
+        .set_advert_relays_for_test(vec![spawn_blackhole_relay()])
+        .await;
+    node.supervisor
+        .nostr_rendezvous
+        .set_engine(Arc::new(bootstrap));
+    // Running gate, so the admission short-circuit is not what suppresses the
+    // dial — same fingerprint the capacity-gate test relies on.
+    node.supervisor.state = crate::node::NodeState::Running;
+
+    let mut queued = Vec::new();
+    for _ in 0..DUE_PEERS {
+        let peer_npub = Identity::generate().npub();
+        let peer_node_addr = *PeerIdentity::from_npub(&peer_npub).unwrap().node_addr();
+        let mut state = super::super::peering::retry::RetryState::new(
+            crate::config::PeerConfig::new(peer_npub, "udp", "127.0.0.1:9"),
+        );
+        state.retry_after_ms = 0;
+        state.reconnect = true;
+        node.peering
+            .reconciler
+            .retry_pending
+            .insert(peer_node_addr, state);
+        queued.push(peer_node_addr);
+    }
+
+    let started = Instant::now();
+    node.process_pending_retries(1_000).await;
+    let elapsed = started.elapsed();
+
+    assert!(
+        elapsed.as_millis() < MAX_TICK_MS,
+        "retry tick must not block on the advert refetch: took {}ms for {} due peers \
+         (a per-peer 2s relay-fetch timeout awaited inline is the fingerprint)",
+        elapsed.as_millis(),
+        DUE_PEERS
+    );
+
+    // The rest of the loop body is unchanged: every due peer was still
+    // attempted, failed for want of a transport, and was rescheduled.
+    for addr in &queued {
+        let state = node
+            .peering
+            .reconciler
+            .retry_pending
+            .get(addr)
+            .expect("due peer must remain queued after a failed attempt");
+        assert_eq!(
+            state.retry_count, 1,
+            "each due peer must still have been attempted and rescheduled"
+        );
+    }
+}
+
 #[tokio::test]
 async fn poll_nostr_rendezvous_established_gated_at_capacity() {
     use crate::nostr::EstablishedTraversal;
@@ -2726,3 +2821,49 @@ async fn test_machine_carries_link_direction_and_address_on_dial_and_inbound() {
     );
     assert_eq!(machine.link_id(), *link);
 }
+
+#[test]
+fn test_peer_display_name_uses_cached_short_npub() {
+    // Path 3 of `peer_display_name` (no host entry, no alias) reads the
+    // per-peer cached short npub; it must still equal the value derived
+    // from the peer's identity.
+    let mut node = make_node();
+    let peer_identity_full = Identity::generate();
+    let peer_addr = *peer_identity_full.node_addr();
+    let peer_identity = PeerIdentity::from_pubkey(peer_identity_full.pubkey());
+    node.peers
+        .insert(peer_addr, ActivePeer::new(peer_identity, LinkId::new(1), 0));
+
+    assert_eq!(
+        node.peer_display_name(&peer_addr),
+        peer_identity.short_npub()
+    );
+}
+
+#[test]
+fn test_peer_display_name_tracks_alias_change() {
+    // The display name is NOT cached on the peer: `peer_aliases` is a
+    // runtime-mutable map (`update_peers` inserts and removes entries), so
+    // a cached name would go stale. Caching only the immutable short npub
+    // must leave that tracking intact.
+    let mut node = make_node();
+    let peer_identity_full = Identity::generate();
+    let peer_addr = *peer_identity_full.node_addr();
+    let peer_identity = PeerIdentity::from_pubkey(peer_identity_full.pubkey());
+    node.peers
+        .insert(peer_addr, ActivePeer::new(peer_identity, LinkId::new(1), 0));
+
+    assert_eq!(
+        node.peer_display_name(&peer_addr),
+        peer_identity.short_npub()
+    );
+
+    node.peer_aliases.insert(peer_addr, "gateway".to_string());
+    assert_eq!(node.peer_display_name(&peer_addr), "gateway");
+
+    node.peer_aliases.remove(&peer_addr);
+    assert_eq!(
+        node.peer_display_name(&peer_addr),
+        peer_identity.short_npub()
+    );
+}

src/nostr/runtime.rs

@@ -1783,6 +1783,18 @@ impl NostrRendezvous {
         }
     }
 
+    /// Point the test instance's advert relays at explicit URLs. Unit tests
+    /// that exercise `refetch_advert_for_stale_check` use this to replace the
+    /// default public relay list with a local blackhole, so the refetch runs
+    /// its full 2s timeout without touching the network.
+    pub(crate) async fn set_advert_relays_for_test(&mut self, relays: Vec<String>) {
+        for url in &relays {
+            let _ = self.client.add_relay(url.as_str()).await;
+        }
+        self.client.connect().await;
+        self.config.advert_relays = relays;
+    }
+
     /// Insert a cached advert directly into the in-memory cache. Used by
     /// unit tests to set up consumer-side state without needing live relays.
     pub(crate) async fn insert_advert_for_test(&self, npub: String, advert: CachedOverlayAdvert) {

src/peer/active.rs

@@ -201,6 +201,16 @@ pub struct ActivePeer {
     // === Identity (Verified) ===
     /// Cryptographic identity (verified via handshake).
     identity: PeerIdentity,
+    /// Bech32 npub, derived once at construction.
+    ///
+    /// The npub is a pure function of `identity`'s public key, and
+    /// `identity` is never mutated after construction, so this can never
+    /// go stale. Deriving it costs a bech32 encode, which the per-tick
+    /// stats snapshot was paying once per peer per tick.
+    npub: String,
+    /// Shortened npub for log/UI display, derived once at construction.
+    /// Immutable for the same reason as [`ActivePeer::npub`].
+    short_npub: String,
 
     // === Connection ===
     /// Current connectivity state.
@@ -285,6 +295,8 @@ impl ActivePeer {
     pub fn new(identity: PeerIdentity, link_id: LinkId, authenticated_at: u64) -> Self {
         let now = Instant::now();
         Self {
+            npub: identity.npub(),
+            short_npub: identity.short_npub(),
             identity,
             connectivity: ConnectivityState::Connected,
             declaration: None,
@@ -362,6 +374,8 @@ impl ActivePeer {
             is_initiator,
         ));
         Self {
+            npub: identity.npub(),
+            short_npub: identity.short_npub(),
             identity,
             connectivity: ConnectivityState::Connected,
             declaration: None,
@@ -453,8 +467,21 @@ impl ActivePeer {
     }
 
     /// Get the peer's npub string.
+    ///
+    /// Returns a clone of the value cached at construction; the bech32
+    /// encode is not repeated.
     pub fn npub(&self) -> String {
-        self.identity.npub()
+        self.npub.clone()
+    }
+
+    /// Borrow the peer's cached npub without allocating.
+    pub fn npub_str(&self) -> &str {
+        &self.npub
+    }
+
+    /// Borrow the peer's cached shortened npub (e.g. `npub1abcd...wxyz`).
+    pub fn short_npub(&self) -> &str {
+        &self.short_npub
     }
 
     // === Connection Accessors ===
@@ -1294,6 +1321,60 @@ mod tests {
         assert!(peer.needs_filter_update()); // New peers need filter
     }
 
+    #[test]
+    fn test_npub_cache_matches_identity() {
+        let identity = make_peer_identity();
+        let peer = ActivePeer::new(identity, LinkId::new(1), 1000);
+
+        assert_eq!(peer.npub(), identity.npub());
+        assert_eq!(peer.npub_str(), identity.npub());
+        assert_eq!(peer.short_npub(), identity.short_npub());
+    }
+
+    #[test]
+    fn test_npub_cache_matches_identity_with_session() {
+        // `with_session` builds its own struct literal, so it needs its
+        // own check that the cache is populated from the same identity.
+        let identity = make_peer_identity();
+        let (session, _peer_session) = ik_session_pair();
+
+        let peer = ActivePeer::with_session(
+            identity,
+            LinkId::new(1),
+            1000,
+            session,
+            SessionIndex::new(1),
+            SessionIndex::new(2),
+            TransportId::new(1),
+            TransportAddr::from_string("127.0.0.1:9000"),
+            LinkStats::new(),
+            true,
+            &MmpConfig::default(),
+            None,
+        );
+
+        assert_eq!(peer.npub(), identity.npub());
+        assert_eq!(peer.short_npub(), identity.short_npub());
+    }
+
+    #[test]
+    fn test_npub_is_memoized_not_rederived() {
+        // The whole point of the fix: the strings are stored on the peer,
+        // not recomputed per call. A stored string keeps one heap buffer,
+        // so repeated borrows have a stable address. A per-call bech32
+        // encode would hand back a fresh allocation each time.
+        let identity = make_peer_identity();
+        let peer = ActivePeer::new(identity, LinkId::new(1), 1000);
+
+        let first = peer.npub_str().as_ptr();
+        let second = peer.npub_str().as_ptr();
+        assert_eq!(first, second);
+
+        let short_first = peer.short_npub().as_ptr();
+        let short_second = peer.short_npub().as_ptr();
+        assert_eq!(short_first, short_second);
+    }
+
     #[test]
     fn test_connectivity_transitions() {
         let identity = make_peer_identity();

src/proto/bloom/state.rs

@@ -172,21 +172,79 @@ impl BloomState {
         peer_addrs: &[NodeAddr],
         peer_filters: &BTreeMap<NodeAddr, BloomFilter>,
     ) {
-        for peer_addr in peer_addrs {
-            if peer_addr == exclude_from {
-                continue;
-            }
-            let new_filter = self.compute_outgoing_filter(peer_addr, peer_filters);
-            let changed = match self.last_sent_filters.get(peer_addr) {
+        let targets: Vec<NodeAddr> = peer_addrs
+            .iter()
+            .filter(|addr| *addr != exclude_from)
+            .copied()
+            .collect();
+
+        for (peer_addr, new_filter) in self.compute_outgoing_filters(&targets, peer_filters) {
+            let changed = match self.last_sent_filters.get(&peer_addr) {
                 Some(last) => *last != new_filter,
                 None => true, // never sent → must send
             };
             if changed {
-                self.pending_updates.insert(*peer_addr);
+                self.pending_updates.insert(peer_addr);
             }
         }
     }
 
+    /// Compute the outgoing filter for many peers in one pass.
+    ///
+    /// Equivalent to calling [`compute_outgoing_filter`](Self::compute_outgoing_filter)
+    /// once per target, but linear in the number of contributing peer
+    /// filters instead of quadratic. The per-peer call rebuilds the whole
+    /// union from scratch, so computing it for every peer costs
+    /// O(targets × filters) 1 KB merges; announce fan-out on a
+    /// large node does exactly that, once per tick and again on every
+    /// inbound announce.
+    ///
+    /// The split-horizon exclusion is the only thing that differs between
+    /// targets, so the union of "everything except peer i" is assembled
+    /// from a running prefix union and a precomputed suffix union. Merging
+    /// is a bytewise OR, which is commutative and associative, so the
+    /// result is bit-identical to the per-peer computation.
+    pub fn compute_outgoing_filters(
+        &self,
+        targets: &[NodeAddr],
+        peer_filters: &BTreeMap<NodeAddr, BloomFilter>,
+    ) -> BTreeMap<NodeAddr, BloomFilter> {
+        let base = self.base_filter();
+        let keys: Vec<NodeAddr> = peer_filters.keys().copied().collect();
+        let n = keys.len();
+
+        // suffix[i] = union of peer_filters[keys[i..]]; suffix[n] is empty.
+        let mut suffix = vec![BloomFilter::new(); n + 1];
+        for i in (0..n).rev() {
+            let mut acc = suffix[i + 1].clone();
+            // Size mismatches are skipped, exactly as in the per-peer path.
+            let _ = acc.merge(&peer_filters[&keys[i]]);
+            suffix[i] = acc;
+        }
+
+        // Filter for a target that contributes nothing: everything merged.
+        let mut all = base.clone();
+        let _ = all.merge(&suffix[0]);
+
+        let mut per_key: BTreeMap<NodeAddr, BloomFilter> = BTreeMap::new();
+        let mut prefix = BloomFilter::new();
+        for i in 0..n {
+            let mut outgoing = base.clone();
+            let _ = outgoing.merge(&prefix);
+            let _ = outgoing.merge(&suffix[i + 1]);
+            per_key.insert(keys[i], outgoing);
+            let _ = prefix.merge(&peer_filters[&keys[i]]);
+        }
+
+        targets
+            .iter()
+            .map(|target| {
+                let filter = per_key.get(target).cloned().unwrap_or_else(|| all.clone());
+                (*target, filter)
+            })
+            .collect()
+    }
+
     /// Compute the outgoing filter for a specific peer.
     ///
     /// The filter includes:

src/proto/bloom/tests/core.rs

@@ -1,5 +1,9 @@
 //! Tests for the `BloomFilter` data structure.
 
+use alloc::collections::BTreeMap;
+
+use crate::NodeAddr;
+use crate::proto::bloom::state::BloomState;
 use crate::proto::bloom::{BloomError, BloomFilter, DEFAULT_FILTER_SIZE_BITS, DEFAULT_HASH_COUNT};
 use crate::testutil::make_node_addr;
 
@@ -365,3 +369,70 @@ fn test_bloom_filter_bit_indices_match_double_hashing_formula() {
     let absent = make_node_addr(200);
     assert!(!filter.contains(&absent));
 }
+
+#[test]
+fn test_compute_outgoing_filters_matches_per_peer() {
+    let node = make_node_addr(0);
+    let mut state = BloomState::new(node);
+    state.add_leaf_dependent(make_node_addr(200));
+    state.add_leaf_dependent(make_node_addr(201));
+
+    // Six contributing peers with overlapping content, plus one whose
+    // filter is a different size and must be skipped by both paths.
+    let mut peer_filters = BTreeMap::new();
+    for i in 1u8..=6 {
+        let mut filter = BloomFilter::new();
+        for j in 0..5u8 {
+            filter.insert(&make_node_addr(i.wrapping_mul(7).wrapping_add(j)));
+        }
+        peer_filters.insert(make_node_addr(i), filter);
+    }
+    let odd_peer = make_node_addr(7);
+    let mut odd = BloomFilter::with_params(4096, DEFAULT_HASH_COUNT).unwrap();
+    odd.insert(&make_node_addr(99));
+    peer_filters.insert(odd_peer, odd);
+
+    // Targets: every contributing peer, the odd-sized one, and two peers
+    // that contribute nothing (non-tree peers get announces too).
+    let mut targets: Vec<NodeAddr> = (1u8..=7).map(make_node_addr).collect();
+    targets.push(make_node_addr(120));
+    targets.push(make_node_addr(121));
+
+    let batch = state.compute_outgoing_filters(&targets, &peer_filters);
+
+    assert_eq!(batch.len(), targets.len());
+    for target in &targets {
+        let expected = state.compute_outgoing_filter(target, &peer_filters);
+        assert_eq!(
+            batch.get(target),
+            Some(&expected),
+            "batch filter for {:?} differs from per-peer computation",
+            target
+        );
+    }
+
+    // Split horizon is real, not vacuous: a contributing peer's own
+    // entries must be absent from its own outgoing filter, and present
+    // in another peer's.
+    let peer3 = make_node_addr(3);
+    let own_entry = make_node_addr(3u8.wrapping_mul(7));
+    assert!(!batch[&peer3].contains(&own_entry));
+    assert!(batch[&make_node_addr(1)].contains(&own_entry));
+}
+
+#[test]
+fn test_compute_outgoing_filters_empty_inputs() {
+    let node = make_node_addr(0);
+    let state = BloomState::new(node);
+    let peer_filters = BTreeMap::new();
+
+    assert!(
+        state
+            .compute_outgoing_filters(&[], &peer_filters)
+            .is_empty()
+    );
+
+    let target = make_node_addr(1);
+    let batch = state.compute_outgoing_filters(&[target], &peer_filters);
+    assert_eq!(batch[&target], state.base_filter());
+}