node: announce node only when accessible

Avoid redundantly emitting Node Announcements about it's publicly
available addresses when it has none.

Signed-off-by: Slack Coder <slackcoder@server.ky>

radicle-node/src/service.rs

@@ -314,6 +314,16 @@ where
         todo!()
     }
 
+    /// Get the address book instance.
+    pub fn addresses(&self) -> &A {
+        &self.addresses
+    }
+
+    /// Get the mutable address book instance.
+    pub fn addresses_mut(&mut self) -> &mut A {
+        &mut self.addresses
+    }
+
     /// Get the storage instance.
     pub fn storage(&self) -> &S {
         &self.storage
@@ -1228,7 +1238,7 @@ mod gossip {
         storage: &S,
         signer: &G,
         config: &Config,
-    ) -> [Message; 4] {
+    ) -> Vec<Message> {
         let inventory = match storage.inventory() {
             Ok(i) => i,
             Err(e) => {
@@ -1239,27 +1249,37 @@ mod gossip {
             }
         };
 
-        [
+        let mut msgs = vec![
             Message::init(*signer.public_key(), config.listen.clone()),
-            Message::node(gossip::node(timestamp, config), signer),
             Message::inventory(gossip::inventory(timestamp, inventory), signer),
             Message::subscribe(config.filter(), timestamp, Timestamp::MAX),
-        ]
+        ];
+        if let Some(m) = gossip::node(timestamp, config) {
+            msgs.push(Message::node(m, signer));
+        };
+
+        msgs
     }
 
-    pub fn node(timestamp: Timestamp, config: &Config) -> NodeAnnouncement {
+    pub fn node(timestamp: Timestamp, config: &Config) -> Option<NodeAnnouncement> {
         let features = node::Features::SEED;
         let alias = config.alias();
         let addresses = config.external_addresses.clone();
 
-        NodeAnnouncement {
-            features,
-            timestamp,
-            alias,
-            addresses,
-            nonce: 0,
+        if addresses.is_empty() {
+            return None;
         }
-        .solve()
+
+        Some(
+            NodeAnnouncement {
+                features,
+                timestamp,
+                alias,
+                addresses,
+                nonce: 0,
+            }
+            .solve(),
+        )
     }
 
     pub fn inventory(timestamp: Timestamp, inventory: Vec<Id>) -> InventoryAnnouncement {

radicle-node/src/test/peer.rs

@@ -7,6 +7,7 @@ use std::ops::{Deref, DerefMut};
 use log::*;
 
 use crate::address;
+use crate::address::Store;
 use crate::clock::{RefClock, Timestamp};
 use crate::crypto::test::signer::MockSigner;
 use crate::crypto::Signer;
@@ -125,6 +126,26 @@ where
         simulator::Peer::addr(self).into()
     }
 
+    pub fn import_addresses<P>(&mut self, peers: P)
+    where
+        P: AsRef<[Self]>,
+    {
+        let timestamp = self.timestamp();
+        for peer in peers.as_ref() {
+            let known_address = address::KnownAddress::new(peer.address(), address::Source::Peer);
+            self.service
+                .addresses_mut()
+                .insert(
+                    &peer.node_id(),
+                    radicle::node::Features::default(),
+                    peer.name,
+                    timestamp,
+                    Some(known_address),
+                )
+                .unwrap();
+        }
+    }
+
     pub fn timestamp(&self) -> Timestamp {
         self.service.clock().timestamp()
     }

radicle-node/src/tests.rs

@@ -661,49 +661,24 @@ fn test_persistent_peer_reconnect() {
     assert_matches!(alice.outbox().next(), None);
 }
 
-fn add_peer<A: address::Store>(address_book: &mut A, peer: &Peer<MockStorage, MockSigner>) {
-    let known_address = address::KnownAddress::new(peer.address(), address::Source::Peer);
-    address_book
-        .insert(
-            &peer.node_id(),
-            radicle::node::Features::NONE,
-            peer.name,
-            LocalTime::now().as_secs(),
-            Some(known_address),
-        )
-        .unwrap();
-}
-
 #[test]
 fn test_maintain_connections() {
+    // Peers alice starts out connected to.
     let connected = vec![
         Peer::new("connected", [8, 8, 8, 1], MockStorage::empty()),
         Peer::new("connected", [8, 8, 8, 2], MockStorage::empty()),
         Peer::new("connected", [8, 8, 8, 3], MockStorage::empty()),
     ];
+    // Peers alice will connect to once the others disconnect.
     let mut unconnected = vec![
         Peer::new("unconnected", [9, 9, 9, 1], MockStorage::empty()),
         Peer::new("unconnected", [9, 9, 9, 2], MockStorage::empty()),
         Peer::new("unconnected", [9, 9, 9, 3], MockStorage::empty()),
     ];
 
-    let mut address_book = address::Book::memory().unwrap();
-    for peer in &unconnected {
-        add_peer(&mut address_book, peer);
-    }
+    let mut alice = Peer::new("alice", [7, 7, 7, 7], MockStorage::empty());
+    alice.import_addresses(&unconnected);
 
-    let mut alice = Peer::config(
-        "alice",
-        Config {
-            project_tracking: ProjectTracking::Allowed(HashSet::default()),
-            ..Config::default()
-        },
-        [7, 7, 7, 7],
-        MockStorage::empty(),
-        address_book,
-        MockSigner::default(),
-        fastrand::Rng::new(),
-    );
     for peer in connected.iter() {
         alice.connect_to(peer);
     }
@@ -713,10 +688,12 @@ fn test_maintain_connections() {
         "alice should be connected to all peers"
     );
 
+    // A transient error such as this will cause Alice to attempt a reconnection.
+    let error = Arc::new(io::Error::from(io::ErrorKind::ConnectionReset));
     for peer in connected.iter() {
         alice.disconnected(
             &peer.addr(),
-            &nakamoto::DisconnectReason::Protocol(DisconnectReason::Error(session::Error::Timeout)),
+            &nakamoto::DisconnectReason::ConnectionError(error.clone()),
         );
 
         let addr = alice