From 8a3133e137c7f5012e0dd5da63645c3dffdac690 Mon Sep 17 00:00:00 2001 From: Alexis Sellier Date: Mon, 24 Jul 2023 12:56:53 +0200 Subject: [PATCH] node: Include addresses in `Command::Seeds` result This will allow the CLI to connect to seeds on the go. We also do some refactoring of some of the code involved in fulfilling the `Seeds` command. --- radicle-cli/src/commands/sync.rs | 14 +++---- radicle-cli/tests/commands.rs | 3 +- radicle-node/src/service.rs | 47 ++++++++++++----------- radicle-remote-helper/src/push.rs | 2 +- radicle/src/node.rs | 62 ++++++++++++++++--------------- radicle/src/node/address/types.rs | 6 ++- 6 files changed, 68 insertions(+), 66 deletions(-) diff --git a/radicle-cli/src/commands/sync.rs b/radicle-cli/src/commands/sync.rs index 3600e1d8..2ffdc601 100644 --- a/radicle-cli/src/commands/sync.rs +++ b/radicle-cli/src/commands/sync.rs @@ -149,12 +149,12 @@ pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> { fn announce(rid: Id, timeout: time::Duration, mut node: Node) -> anyhow::Result<()> { let seeds = node.seeds(rid)?; - if !seeds.has_connections() { + let connected = seeds.connected().map(|s| s.nid).collect::>(); + if connected.is_empty() { term::info!("Not connected to any seeds."); return Ok(()); } - let connected = seeds.connected().cloned().collect::>(); let mut spinner = term::spinner(format!("Syncing with {} node(s)..", connected.len())); let result = node.announce(rid, connected, timeout, |event| match event { node::AnnounceEvent::Announced => {} @@ -210,12 +210,10 @@ pub fn fetch_all(rid: Id, node: &mut Node) -> Result let seeds = node.seeds(rid)?; let mut results = FetchResults::default(); - if seeds.has_connections() { - // Fetch from all seeds. - for seed in seeds.connected() { - let result = fetch_from(rid, seed, node)?; - results.push(*seed, result); - } + // Fetch from connected seeds. + for seed in seeds.connected() { + let result = fetch_from(rid, &seed.nid, node)?; + results.push(seed.nid, result); } Ok(results) } diff --git a/radicle-cli/tests/commands.rs b/radicle-cli/tests/commands.rs index 51b08002..1d701b03 100644 --- a/radicle-cli/tests/commands.rs +++ b/radicle-cli/tests/commands.rs @@ -636,8 +636,9 @@ fn test_clone_without_seeds() { let rid = alice.project("heartwood", "Radicle Heartwood Protocol & Stack"); let mut alice = alice.spawn(); let seeds = alice.handle.seeds(rid).unwrap(); + let connected = seeds.connected().collect::>(); - assert!(!seeds.has_connections()); + assert!(connected.is_empty()); alice .rad("clone", &[rid.to_string().as_str()], working.as_path()) diff --git a/radicle-node/src/service.rs b/radicle-node/src/service.rs index da36c01b..22925291 100644 --- a/radicle-node/src/service.rs +++ b/radicle-node/src/service.rs @@ -17,6 +17,7 @@ use crossbeam_channel as chan; use fastrand::Rng; use localtime::{LocalDuration, LocalTime}; use log::*; +use nonempty::NonEmpty; use radicle::node::address; use radicle::node::address::{AddressBook, KnownAddress}; @@ -502,10 +503,11 @@ where } Command::Seeds(rid, resp) => match self.seeds(&rid) { Ok(seeds) => { + let (connected, disconnected) = seeds.partition(); debug!( target: "service", "Found {} connected seed(s) and {} disconnected seed(s) for {}", - seeds.connected().count(), seeds.disconnected().count(), rid + connected.len(), disconnected.len(), rid ); resp.send(seeds).ok(); } @@ -1322,28 +1324,25 @@ where } fn seeds(&self, rid: &Id) -> Result { - #[derive(Default)] - pub struct Stats { - connected: usize, - disconnected: usize, - } + let seeds = match self.routing.get(rid) { + Ok(seeds) => seeds.into_iter().fold(Seeds::default(), |mut seeds, node| { + if node != self.node_id() { + let addrs: Vec = self + .addresses + .get(&node) + .ok() + .flatten() + .map(|n| n.addrs) + .unwrap_or(vec![]); - let (_, seeds) = match self.routing.get(rid) { - Ok(seeds) => seeds.into_iter().fold( - (Stats::default(), Seeds::default()), - |(mut stats, mut seeds), node| { - if node != self.node_id() { - if self.sessions.is_connected(&node) { - seeds.insert(Seed::Connected(node)); - stats.connected += 1; - } else if self.sessions.is_disconnected(&node) { - seeds.insert(Seed::Disconnected(node)); - stats.disconnected += 1; - } + if let Some(s) = self.sessions.get(&node) { + seeds.insert(Seed::new(node, addrs, Some(s.state.clone()))); + } else { + seeds.insert(Seed::new(node, addrs, None)); } - (stats, seeds) - }, - ), + } + seeds + }), Err(err) => { return Err(Error::Routing(err)); } @@ -1464,9 +1463,9 @@ where for rid in missing { match self.seeds(&rid) { Ok(seeds) => { - if seeds.has_connections() { - for seed in seeds.connected() { - self.fetch(rid, seed); + if let Some(connected) = NonEmpty::from_vec(seeds.connected().collect()) { + for seed in connected { + self.fetch(rid, &seed.nid); } } else { // TODO: We should make sure that this fetch is retried later, either diff --git a/radicle-remote-helper/src/push.rs b/radicle-remote-helper/src/push.rs index fe028b98..d195b216 100644 --- a/radicle-remote-helper/src/push.rs +++ b/radicle-remote-helper/src/push.rs @@ -509,7 +509,7 @@ fn push_ref( /// Sync with the network. fn sync(rid: Id, mut node: radicle::Node) -> Result<(), radicle::node::Error> { let seeds = node.seeds(rid)?; - let connected = seeds.connected().cloned().collect::>(); + let connected = seeds.connected().map(|s| s.nid).collect::>(); if connected.is_empty() { eprintln!("Not connected to any seeds."); diff --git a/radicle/src/node.rs b/radicle/src/node.rs index 57ae6dad..74f2c211 100644 --- a/radicle/src/node.rs +++ b/radicle/src/node.rs @@ -6,7 +6,7 @@ pub mod events; pub mod routing; pub mod tracking; -use std::collections::{BTreeSet, HashMap, HashSet}; +use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet}; use std::io::{BufRead, BufReader}; use std::ops::Deref; use std::os::unix::net::UnixStream; @@ -25,6 +25,7 @@ use crate::crypto::PublicKey; use crate::identity::Id; use crate::storage::RefUpdate; +pub use address::KnownAddress; pub use config::Config; pub use cyphernet::addr::PeerAddr; pub use events::{Event, Events}; @@ -359,49 +360,50 @@ pub struct Session { pub state: State, } -#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, serde::Serialize, serde::Deserialize)] +#[derive(Clone, Debug, serde::Serialize, serde::Deserialize)] #[serde(rename_all = "kebab-case")] -#[serde(tag = "state", content = "id")] -pub enum Seed { - Disconnected(NodeId), - Connected(NodeId), +pub struct Seed { + pub nid: NodeId, + pub addrs: Vec, + pub state: Option, } -#[derive(Clone, Debug, Default, PartialEq, Eq, serde::Serialize, serde::Deserialize)] -pub struct Seeds(BTreeSet); +impl Seed { + /// Check if this is a "connected" seed. + pub fn is_connected(&self) -> bool { + matches!(self.state, Some(State::Connected { .. })) + } + + pub fn new(nid: NodeId, addrs: Vec, state: Option) -> Self { + Self { nid, addrs, state } + } +} + +#[derive(Clone, Debug, Default, serde::Serialize, serde::Deserialize)] +pub struct Seeds(BTreeMap); impl Seeds { pub fn insert(&mut self, seed: Seed) { - self.0.insert(seed); + self.0.insert(seed.nid, seed); } - pub fn connected(&self) -> impl Iterator { - self.0.iter().filter_map(|s| match s { - Seed::Connected(node) => Some(node), - Seed::Disconnected(_) => None, - }) + /// Partitions the list of seeds into connected and disconnected seeds. + /// Note that the disconnected seeds may be in a "connecting" state. + pub fn partition(&self) -> (Vec, Vec) { + self.0.values().cloned().partition(|s| s.is_connected()) } - pub fn disconnected(&self) -> impl Iterator { - self.0.iter().filter_map(|s| match s { - Seed::Disconnected(node) => Some(node), - Seed::Connected(_) => None, - }) + /// Return connected seeds. + pub fn connected(&self) -> impl Iterator { + self.0.values().filter(|s| s.is_connected()) } - pub fn has_connections(&self) -> bool { - self.0.iter().any(|s| match s { - Seed::Connected(_) => true, - Seed::Disconnected(_) => false, - }) + pub fn iter(&self) -> impl Iterator { + self.0.values() } - pub fn is_connected(&self, node: &NodeId) -> bool { - self.0.contains(&Seed::Connected(*node)) - } - - pub fn is_disconnected(&self, node: &NodeId) -> bool { - self.0.contains(&Seed::Disconnected(*node)) + pub fn is_connected(&self, nid: &NodeId) -> bool { + self.0.get(nid).map_or(false, |s| s.is_connected()) } } diff --git a/radicle/src/node/address/types.rs b/radicle/src/node/address/types.rs index 53b1b700..0e3ee44a 100644 --- a/radicle/src/node/address/types.rs +++ b/radicle/src/node/address/types.rs @@ -90,7 +90,8 @@ pub struct Node { } /// A known address. -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)] +#[serde(rename_all = "kebab-case")] pub struct KnownAddress { /// Network address. pub addr: Address, @@ -115,7 +116,8 @@ impl KnownAddress { } /// Address source. Specifies where an address originated from. -#[derive(Debug, Copy, Clone, PartialEq, Eq)] +#[derive(Debug, Copy, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)] +#[serde(rename_all = "kebab-case")] pub enum Source { /// An address that was shared by another peer. Peer,