diff --git a/src/node/mod.rs b/src/node/mod.rs index c3b2a45..aebfa73 100644 --- a/src/node/mod.rs +++ b/src/node/mod.rs @@ -1223,7 +1223,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(); diff --git a/src/node/tests/unit.rs b/src/node/tests/unit.rs index f3c78a6..1a82a96 100644 --- a/src/node/tests/unit.rs +++ b/src/node/tests/unit.rs @@ -2119,3 +2119,49 @@ fn test_transport_drop_state_steady_counter_fires_once() { assert!(!s.observe_drops(7)); assert!(!s.observe_drops(7)); } + +#[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() + ); +} diff --git a/src/peer/active.rs b/src/peer/active.rs index 1ae58fd..1b437c5 100644 --- a/src/peer/active.rs +++ b/src/peer/active.rs @@ -81,6 +81,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 === /// Link used to reach this peer. @@ -224,6 +234,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, link_id, connectivity: ConnectivityState::Connected, @@ -310,6 +322,8 @@ impl ActivePeer { ) -> Self { let now = Instant::now(); Self { + npub: identity.npub(), + short_npub: identity.short_npub(), identity, link_id, connectivity: ConnectivityState::Connected, @@ -423,8 +437,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 === @@ -1219,6 +1246,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();