feat(manager): don't fetch peers from network-contact file

sn_auditor/src/main.rs

@@ -18,7 +18,6 @@ use color_eyre::eyre::{eyre, Result};
 use dag_db::SpendDagDb;
 use sn_client::Client;
 use sn_logging::{Level, LogBuilder, LogFormat, LogOutputDest};
-use sn_peers_acquisition::get_peers_from_args;
 use sn_peers_acquisition::PeersArgs;
 use std::collections::BTreeSet;
 use std::path::PathBuf;
@@ -138,8 +137,8 @@ fn logging_init(
     Ok(log_builder)
 }
 
-async fn connect_to_network(peers: PeersArgs) -> Result<Client> {
-    let bootstrap_peers = get_peers_from_args(peers).await?;
+async fn connect_to_network(peers_args: PeersArgs) -> Result<Client> {
+    let bootstrap_peers = peers_args.get_peers().await?;
     println!(
         "Connecting to the network with {} bootstrap peers",
         bootstrap_peers.len(),

sn_cli/src/bin/main.rs

@@ -30,7 +30,6 @@ use sn_client::transfers::bls_secret_from_hex;
 use sn_client::{Client, ClientEvent, ClientEventsBroadcaster, ClientEventsReceiver};
 #[cfg(feature = "metrics")]
 use sn_logging::{metrics::init_metrics, Level, LogBuilder, LogFormat};
-use sn_peers_acquisition::get_peers_from_args;
 use std::{io, path::PathBuf, time::Duration};
 use tokio::{sync::broadcast::error::RecvError, task::JoinHandle};
 
@@ -101,7 +100,7 @@ async fn main() -> Result<()> {
     println!("Instantiating a SAFE client...");
     let secret_key = get_client_secret_key(&client_data_dir_path)?;
 
-    let bootstrap_peers = get_peers_from_args(opt.peers).await?;
+    let bootstrap_peers = opt.peers.get_peers().await?;
 
     println!(
         "Connecting to the network with {} peers",

sn_faucet/src/main.rs

@@ -21,7 +21,7 @@ use sn_client::{
     Client, ClientEvent, ClientEventsBroadcaster, ClientEventsReceiver,
 };
 use sn_logging::{Level, LogBuilder, LogOutputDest};
-use sn_peers_acquisition::{get_peers_from_args, PeersArgs};
+use sn_peers_acquisition::PeersArgs;
 use sn_transfers::{get_faucet_data_dir, HotWallet, MainPubkey, NanoTokens, Transfer};
 use std::{path::PathBuf, time::Duration};
 use tokio::{sync::broadcast::error::RecvError, task::JoinHandle};
@@ -31,7 +31,7 @@ use tracing::{debug, error, info};
 async fn main() -> Result<()> {
     let opt = Opt::parse();
 
-    let bootstrap_peers = get_peers_from_args(opt.peers).await?;
+    let bootstrap_peers = opt.peers.get_peers().await?;
     let bootstrap_peers = if bootstrap_peers.is_empty() {
         // empty vec is returned if `local-discovery` flag is provided
         None

sn_node/src/bin/safenode/main.rs

@@ -18,7 +18,7 @@ use libp2p::{identity::Keypair, PeerId};
 use sn_logging::metrics::init_metrics;
 use sn_logging::{Level, LogFormat, LogOutputDest, ReloadHandle};
 use sn_node::{Marker, NodeBuilder, NodeEvent, NodeEventsReceiver};
-use sn_peers_acquisition::{get_peers_from_args, PeersArgs};
+use sn_peers_acquisition::PeersArgs;
 use sn_protocol::{node::get_safenode_root_dir, node_rpc::NodeCtrl};
 use std::{
     env,
@@ -190,7 +190,7 @@ fn main() -> Result<()> {
         init_logging(&opt, keypair.public().to_peer_id())?;
 
     let rt = Runtime::new()?;
-    let bootstrap_peers = rt.block_on(get_peers_from_args(opt.peers))?;
+    let bootstrap_peers = rt.block_on(opt.peers.get_peers())?;
     let msg = format!(
         "Running {} v{}",
         env!("CARGO_BIN_NAME"),

sn_node_manager/src/cmd/auditor.rs

@@ -16,7 +16,7 @@ use crate::{
 use color_eyre::{eyre::eyre, Result};
 use colored::Colorize;
 use semver::Version;
-use sn_peers_acquisition::{get_peers_from_args, PeersArgs};
+use sn_peers_acquisition::PeersArgs;
 use sn_releases::{ReleaseType, SafeReleaseRepoActions};
 use sn_service_management::{
     auditor::AuditorService,
@@ -30,7 +30,7 @@ pub async fn add(
     beta_encryption_key: Option<String>,
     env_variables: Option<Vec<(String, String)>>,
     log_dir_path: Option<PathBuf>,
-    peers: PeersArgs,
+    peers_args: PeersArgs,
     src_path: Option<PathBuf>,
     url: Option<String>,
     version: Option<String>,
@@ -79,7 +79,7 @@ pub async fn add(
             auditor_src_bin_path,
             auditor_install_bin_path: PathBuf::from("/usr/local/bin/auditor"),
             beta_encryption_key,
-            bootstrap_peers: get_peers_from_args(peers).await?,
+            bootstrap_peers: peers_args.get_peers().await?,
             env_variables,
             service_log_dir_path,
             user: service_user.to_string(),

sn_node_manager/src/cmd/faucet.rs

@@ -16,7 +16,7 @@ use crate::{
 use color_eyre::{eyre::eyre, Result};
 use colored::Colorize;
 use semver::Version;
-use sn_peers_acquisition::{get_peers_from_args, PeersArgs};
+use sn_peers_acquisition::PeersArgs;
 use sn_releases::{ReleaseType, SafeReleaseRepoActions};
 use sn_service_management::{
     control::{ServiceControl, ServiceController},
@@ -28,7 +28,7 @@ use std::path::PathBuf;
 pub async fn add(
     env_variables: Option<Vec<(String, String)>>,
     log_dir_path: Option<PathBuf>,
-    peers: PeersArgs,
+    peers_args: PeersArgs,
     src_path: Option<PathBuf>,
     url: Option<String>,
     version: Option<String>,
@@ -74,7 +74,7 @@ pub async fn add(
     info!("Adding faucet service");
     add_faucet(
         AddFaucetServiceOptions {
-            bootstrap_peers: get_peers_from_args(peers).await?,
+            bootstrap_peers: peers_args.get_peers().await?,
             env_variables,
             faucet_src_bin_path,
             faucet_install_bin_path: PathBuf::from("/usr/local/bin/faucet"),

sn_node_manager/src/cmd/local.rs

@@ -15,7 +15,7 @@ use crate::{
 };
 use color_eyre::{eyre::eyre, Help, Report, Result};
 use sn_logging::LogFormat;
-use sn_peers_acquisition::{get_peers_from_args, PeersArgs};
+use sn_peers_acquisition::PeersArgs;
 use sn_releases::{ReleaseType, SafeReleaseRepoActions};
 use sn_service_management::{
     control::ServiceController, get_local_node_registry_path, NodeRegistry,
@@ -33,7 +33,7 @@ pub async fn join(
     log_format: Option<LogFormat>,
     owner: Option<String>,
     owner_prefix: Option<String>,
-    peers: PeersArgs,
+    peers_args: PeersArgs,
     skip_validation: bool,
     verbosity: VerbosityLevel,
 ) -> Result<(), Report> {
@@ -67,7 +67,7 @@ pub async fn join(
 
     // If no peers are obtained we will attempt to join the existing local network, if one
     // is running.
-    let peers = match get_peers_from_args(peers).await {
+    let peers = match peers_args.get_peers().await {
         Ok(peers) => Some(peers),
         Err(err) => match err {
             sn_peers_acquisition::error::Error::PeersNotObtained => {

sn_node_manager/src/cmd/node.rs

@@ -23,7 +23,7 @@ use colored::Colorize;
 use libp2p_identity::PeerId;
 use semver::Version;
 use sn_logging::LogFormat;
-use sn_peers_acquisition::{get_peers_from_args, PeersArgs};
+use sn_peers_acquisition::PeersArgs;
 use sn_releases::{ReleaseType, SafeReleaseRepoActions};
 use sn_service_management::{
     control::{ServiceControl, ServiceController},
@@ -49,7 +49,7 @@ pub async fn add(
     metrics_port: Option<PortRange>,
     node_port: Option<PortRange>,
     owner: Option<String>,
-    peers: PeersArgs,
+    peers_args: PeersArgs,
     rpc_address: Option<Ipv4Addr>,
     rpc_port: Option<PortRange>,
     src_path: Option<PathBuf>,
@@ -106,18 +106,19 @@ pub async fn add(
     // Handle the `PeersNotObtained` error to make the `--peer` argument optional for the node
     // manager.
     //
-    // Since we don't use the `network-contacts` feature in the node manager, the `PeersNotObtained`
-    // error should occur when no `--peer` arguments were used. If the `safenode` binary we're using
-    // has `network-contacts` enabled (which is the case for released binaries), it's fine if the
-    // service definition doesn't call `safenode` with a `--peer` argument.
-    //
-    // It's simpler to not use the `network-contacts` feature in the node manager, because it can
-    // return a huge peer list, and that's been problematic for service definition files.
-    let is_first = peers.first;
-    let bootstrap_peers = match get_peers_from_args(peers).await {
-        Ok(p) => {
-            info!("Obtained peers of length {}", p.len());
-            p
+    // Since any application making use of the node manager can enable the `network-contacts` feature on
+    // sn_peers_acquisition, we might end up getting having a huge peer list, and that's problematic for
+    // service definition files.
+    // Thus make use of get_peers_exclude_network_contacts() instead of get_peers() to make sure we only
+    // parse the --peers and SAFE_PEERS env var.
+
+    // If the `safenode` binary we're using has `network-contacts` enabled (which is the case for released binaries),
+    // it's fine if the service definition doesn't call `safenode` with a `--peer` argument.
+    let is_first = peers_args.first;
+    let bootstrap_peers = match peers_args.get_peers_exclude_network_contacts().await {
+        Ok(peers) => {
+            info!("Obtained peers of length {}", peers.len());
+            peers
         }
         Err(err) => match err {
             sn_peers_acquisition::error::Error::PeersNotObtained => {

sn_peers_acquisition/src/lib.rs

@@ -67,67 +67,95 @@ pub struct PeersArgs {
     pub network_contacts_url: Option<Url>,
 }
 
-/// Gets the peers based on the arguments provided.
-///
-/// If the `--first` flag is used, no peers will be provided.
-///
-/// Otherwise, peers are obtained in the following order of precedence:
-/// * The `--peer` argument.
-/// * The `SAFE_PEERS` environment variable.
-/// * Using the `local-discovery` feature, which will return an empty peer list.
-/// * Using the `network-contacts` feature, which will download the peer list from a file on S3.
-///
-/// Note: the current behaviour is that `--peer` and `SAFE_PEERS` will be combined. Some tests
-/// currently rely on this. We will change it soon.
-pub async fn get_peers_from_args(args: PeersArgs) -> Result<Vec<Multiaddr>> {
-    if args.first {
-        return Ok(vec![]);
+impl PeersArgs {
+    /// Gets the peers based on the arguments provided.
+    ///
+    /// If the `--first` flag is used, no peers will be provided.
+    ///
+    /// Otherwise, peers are obtained in the following order of precedence:
+    /// * The `--peer` argument.
+    /// * The `SAFE_PEERS` environment variable.
+    /// * Using the `local-discovery` feature, which will return an empty peer list.
+    /// * Using the `network-contacts` feature, which will download the peer list from a file on S3.
+    ///
+    /// Note: the current behaviour is that `--peer` and `SAFE_PEERS` will be combined. Some tests
+    /// currently rely on this. We will change it soon.
+    pub async fn get_peers(self) -> Result<Vec<Multiaddr>> {
+        self.get_peers_inner(false).await
     }
 
-    let mut peers = if !args.peers.is_empty() {
-        info!("Using peers supplied with the --peer argument(s) or SAFE_PEERS");
-        args.peers
-    } else if cfg!(feature = "local-discovery") {
-        info!("No peers given");
-        info!(
-            "The `local-discovery` feature is enabled, so peers will be discovered through mDNS."
-        );
-        return Ok(vec![]);
-    } else if cfg!(feature = "network-contacts") {
-        get_network_contacts(&args).await?
-    } else {
-        vec![]
-    };
-
-    if peers.is_empty() {
-        error!("Peers not obtained through any available options");
-        return Err(Error::PeersNotObtained);
-    };
+    /// Gets the peers based on the arguments provided.
+    ///
+    /// If the `--first` flag is used, no peers will be provided.
+    ///
+    /// Otherwise, peers are obtained in the following order of precedence:
+    /// * The `--peer` argument.
+    /// * The `SAFE_PEERS` environment variable.
+    /// * Using the `local-discovery` feature, which will return an empty peer list.
+    ///
+    /// This will not fetch the peers from network-contacts even if the `network-contacts` feature is enabled. Use
+    /// get_peers() instead.
+    ///
+    /// Note: the current behaviour is that `--peer` and `SAFE_PEERS` will be combined. Some tests
+    /// currently rely on this. We will change it soon.
+    pub async fn get_peers_exclude_network_contacts(self) -> Result<Vec<Multiaddr>> {
+        self.get_peers_inner(true).await
+    }
 
-    // Randomly sort peers before we return them to avoid overly hitting any one peer
-    let mut rng = thread_rng();
-    peers.shuffle(&mut rng);
+    async fn get_peers_inner(self, skip_network_contacts: bool) -> Result<Vec<Multiaddr>> {
+        if self.first {
+            info!("First node in a new network");
+            return Ok(vec![]);
+        }
 
-    Ok(peers)
-}
+        let mut peers = if !self.peers.is_empty() {
+            info!("Using peers supplied with the --peer argument(s) or SAFE_PEERS");
+            self.peers
+        } else if cfg!(feature = "local-discovery") {
+            info!("No peers given");
+            info!(
+            "The `local-discovery` feature is enabled, so peers will be discovered through mDNS."
+        );
+            return Ok(vec![]);
+        } else if skip_network_contacts {
+            info!("Skipping network contacts");
+            return Ok(vec![]);
+        } else if cfg!(feature = "network-contacts") {
+            self.get_network_contacts().await?
+        } else {
+            vec![]
+        };
+
+        if peers.is_empty() {
+            error!("Peers not obtained through any available options");
+            return Err(Error::PeersNotObtained);
+        };
+
+        // Randomly sort peers before we return them to avoid overly hitting any one peer
+        let mut rng = thread_rng();
+        peers.shuffle(&mut rng);
+
+        Ok(peers)
+    }
 
-// should not be reachable, but needed for the compiler to be happy.
-#[allow(clippy::unused_async)]
-#[cfg(not(feature = "network-contacts"))]
-async fn get_network_contacts(_args: &PeersArgs) -> Result<Vec<Multiaddr>> {
-    Ok(vec![])
-}
+    // should not be reachable, but needed for the compiler to be happy.
+    #[allow(clippy::unused_async)]
+    #[cfg(not(feature = "network-contacts"))]
+    async fn get_network_contacts(&self) -> Result<Vec<Multiaddr>> {
+        Ok(vec![])
+    }
 
-#[cfg(feature = "network-contacts")]
-async fn get_network_contacts(args: &PeersArgs) -> Result<Vec<Multiaddr>> {
-    let url = args
-        .network_contacts_url
-        .clone()
-        .unwrap_or(Url::parse(NETWORK_CONTACTS_URL.as_str())?);
+    #[cfg(feature = "network-contacts")]
+    async fn get_network_contacts(&self) -> Result<Vec<Multiaddr>> {
+        let url = self
+            .network_contacts_url
+            .clone()
+            .unwrap_or(Url::parse(NETWORK_CONTACTS_URL.as_str())?);
 
-    info!("Trying to fetch the bootstrap peers from {url}");
+        info!("Trying to fetch the bootstrap peers from {url}");
 
-    get_bootstrap_peers_from_url(url).await
+        get_bootstrap_peers_from_url(url).await
+    }
 }
 
 /// Parse strings like `1.2.3.4:1234` and `/ip4/1.2.3.4/tcp/1234` into a multiaddr.