From 503a6187435260f8b73fcacb06bb5f599f50434b Mon Sep 17 00:00:00 2001 From: Slack Coder Date: Mon, 7 Nov 2022 08:54:46 -0500 Subject: [PATCH] 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 --- radicle-node/src/service.rs | 44 +++++++++++++++++++++++++---------- radicle-node/src/test/peer.rs | 21 +++++++++++++++++ radicle-node/src/tests.rs | 37 ++++++----------------------- 3 files changed, 60 insertions(+), 42 deletions(-) diff --git a/radicle-node/src/service.rs b/radicle-node/src/service.rs index 1c28b979..759935dc 100644 --- a/radicle-node/src/service.rs +++ b/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 { 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 { 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) -> InventoryAnnouncement { diff --git a/radicle-node/src/test/peer.rs b/radicle-node/src/test/peer.rs index 1e4fd635..8c1f0753 100644 --- a/radicle-node/src/test/peer.rs +++ b/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

(&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() } diff --git a/radicle-node/src/tests.rs b/radicle-node/src/tests.rs index 804014dc..b3bb74bc 100644 --- a/radicle-node/src/tests.rs +++ b/radicle-node/src/tests.rs @@ -661,49 +661,24 @@ fn test_persistent_peer_reconnect() { assert_matches!(alice.outbox().next(), None); } -fn add_peer(address_book: &mut A, peer: &Peer) { - 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