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

sn_node_manager/src/cmd/node.rs

@@ -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.
+    // 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().await {
-        Ok(p) => {
-            info!("Obtained peers of length {}", p.len());
-            p
+    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

@@ -81,7 +81,30 @@ impl PeersArgs {
     /// 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
+    }
+
+    /// 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
+    }
+
+    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![]);
         }
 
@@ -94,6 +117,9 @@ impl PeersArgs {
             "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 {