From cd6e2a7fdeab350ea82e6007f1cc00a3fc5b2ace Mon Sep 17 00:00:00 2001 From: Roland Sherwin Date: Thu, 6 Jun 2024 14:19:56 +0530 Subject: [PATCH] feat(manager): don't fetch peers from network-contact file --- sn_node_manager/src/cmd/node.rs | 23 ++++++++++++----------- sn_peers_acquisition/src/lib.rs | 26 ++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 11 deletions(-) diff --git a/sn_node_manager/src/cmd/node.rs b/sn_node_manager/src/cmd/node.rs index 394472528a..05f3fa518c 100644 --- a/sn_node_manager/src/cmd/node.rs +++ b/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 => { diff --git a/sn_peers_acquisition/src/lib.rs b/sn_peers_acquisition/src/lib.rs index 25f9624fd0..1d358b9993 100644 --- a/sn_peers_acquisition/src/lib.rs +++ b/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> { + 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> { + self.get_peers_inner(true).await + } + + async fn get_peers_inner(self, skip_network_contacts: bool) -> Result> { 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 {