feat: provide --first argument for safenode

In the previous setup, if the `network-contacts` feature was enabled, running `safenode` with no
`--peer` args caused the peers to be retrieved from the default network contacts file. In the
situation where we're launching a genesis node, we don't want this behaviour, and hence we introduce
a `--first` flag to make a distinction.

The `safenode` binary was changed to remove the use of `unwrap_or` when calling
`get_peers_from_args` so that an error will actually occur when peers are not obtainable, rather
than swallowing it by returning an empty peer list.

The `testnet` binary was also updated to use the `--first` argument when a new network was being
created. This necessitated a change to the memcheck workflow, which starts a testnet in a slightly
unusual fashion. The `--first` argument had to be applied to the initial node that is launched, and
then the subsequent local testnet launch had to be changed to use a join rather than starting a new
network. This is because the first peer launched by `testnet`, using `--first`, did not have the
initial node launched outside of `testnet` in its peer list. For the test to work correctly, an
environment variable was introduced to control the starting port deployment inventory, because using
a join network means the port range starts one digit higher. We will be able to remove this when we
switch over to using the node manager.

BREAKING CHANGE: the `parse_peer_args` function was renamed `get_peers_from_args` and the error
handling was changed. The new function name is semantically more accurate, and because
`sn_peers_acquisition` is a library crate, we should prefer an `Error` type rather than using
`eyre`.

Tried to add some unit tests for the `get_peers_from_args` function, but Tokio being an optional
dependency proved to be problematic.

.github/workflows/memcheck.yml

@@ -43,7 +43,7 @@ jobs:
       - name: Start a node instance that does not undergo churn
         run: |
           mkdir -p $BOOTSTRAP_NODE_DATA_PATH
-          ./target/release/safenode \
+          ./target/release/safenode --first \
             --root-dir $BOOTSTRAP_NODE_DATA_PATH --log-output-dest $BOOTSTRAP_NODE_DATA_PATH --local &
           sleep 10
         env:
@@ -55,6 +55,10 @@ jobs:
             rg '/ip4.*$' -m1 -o | rg '"' -r '')
           echo "SAFE_PEERS=$safe_peers" >> $GITHUB_ENV
 
+      - name: Set CHURN_TEST_START_PORT
+        run: |
+          echo "CHURN_TEST_START_PORT=12001" >> $GITHUB_ENV
+
       - name: Check SAFE_PEERS was set
         shell: bash
         run: echo "The SAFE_PEERS variable has been set to $SAFE_PEERS"
@@ -71,6 +75,7 @@ jobs:
           platform: ubuntu-latest
           set-safe-peers: false
           build: true
+          join: true
 
       # In this case we did *not* want SAFE_PEERS to be set to another value by starting the testnet
       - name: Check SAFE_PEERS was not changed
@@ -79,7 +84,13 @@ jobs:
 
       - name: Create and fund a wallet to pay for files storage
         run: |
-          cargo run --bin faucet --release -- --log-output-dest=data-dir send 5000000 $(cargo run --bin safe --release -- --log-output-dest=data-dir wallet address | tail -n 1) > initial_balance_from_faucet.txt
+          echo "Obtaining address for use with the faucet..."
+          address=$(cargo run \
+            --bin safe --release -- --log-output-dest=data-dir wallet address | tail -n 1)
+          echo "Sending tokens to the faucet at $address"
+          cargo run \
+            --bin faucet --release -- \
+            --log-output-dest=data-dir send 5000000 $address > initial_balance_from_faucet.txt
           cat initial_balance_from_faucet.txt
           cat initial_balance_from_faucet.txt | tail -n 1 > transfer_hex
           cat transfer_hex

sn_cli/src/main.rs

@@ -28,7 +28,7 @@ use color_eyre::Result;
 use sn_client::Client;
 #[cfg(feature = "metrics")]
 use sn_logging::{metrics::init_metrics, LogBuilder, LogFormat};
-use sn_peers_acquisition::parse_peers_args;
+use sn_peers_acquisition::get_peers_from_args;
 use sn_transfers::bls_secret_from_hex;
 use std::path::PathBuf;
 use tracing::Level;
@@ -85,11 +85,11 @@ async fn main() -> Result<()> {
     println!("Instantiating a SAFE client...");
     let secret_key = get_client_secret_key(&client_data_dir_path)?;
 
-    let bootstrap_peers = parse_peers_args(opt.peers).await?;
+    let bootstrap_peers = get_peers_from_args(opt.peers).await?;
 
     println!(
-        "Connecting to the network with {} peers:",
-        bootstrap_peers.len()
+        "Connecting to the network with {} peers",
+        bootstrap_peers.len(),
     );
 
     let bootstrap_peers = if bootstrap_peers.is_empty() {

sn_faucet/src/main.rs

@@ -13,7 +13,7 @@ use color_eyre::eyre::{bail, eyre, Result};
 use faucet_server::{restart_faucet_server, run_faucet_server};
 use sn_client::{get_tokens_from_faucet, load_faucet_wallet_from_genesis_wallet, Client};
 use sn_logging::{LogBuilder, LogOutputDest};
-use sn_peers_acquisition::{parse_peers_args, PeersArgs};
+use sn_peers_acquisition::{get_peers_from_args, PeersArgs};
 use sn_transfers::{MainPubkey, NanoTokens, Transfer};
 use std::path::PathBuf;
 use tracing::{error, info};
@@ -23,7 +23,7 @@ use tracing_core::Level;
 async fn main() -> Result<()> {
     let opt = Opt::parse();
 
-    let bootstrap_peers = parse_peers_args(opt.peers).await?;
+    let bootstrap_peers = get_peers_from_args(opt.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::{LogFormat, LogOutputDest};
 use sn_node::{Marker, NodeBuilder, NodeEvent, NodeEventsReceiver};
-use sn_peers_acquisition::{parse_peers_args, PeersArgs};
+use sn_peers_acquisition::{get_peers_from_args, PeersArgs};
 use sn_protocol::node_rpc::NodeCtrl;
 use std::{
     env,
@@ -162,8 +162,7 @@ fn main() -> Result<()> {
     let (log_output_dest, _log_appender_guard) = init_logging(&opt, keypair.public().to_peer_id())?;
 
     let rt = Runtime::new()?;
-    // bootstrap peers can be empty for the genesis node.
-    let bootstrap_peers = rt.block_on(parse_peers_args(opt.peers)).unwrap_or(vec![]);
+    let bootstrap_peers = rt.block_on(get_peers_from_args(opt.peers))?;
     let msg = format!(
         "Running {} v{}",
         env!("CARGO_BIN_NAME"),

sn_node/tests/common/client.rs

@@ -49,17 +49,23 @@ pub fn get_node_count() -> usize {
 /// Get the list of all RPC addresses
 /// If SN_INVENTORY flag is passed, the RPC addresses of all the droplet nodes are returned
 /// else generate local addresses for NODE_COUNT nodes
-pub fn get_all_rpc_addresses() -> Vec<SocketAddr> {
+pub fn get_all_rpc_addresses() -> Result<Vec<SocketAddr>> {
     match DeploymentInventory::load() {
-        Ok(inventory) => inventory.rpc_endpoints,
+        Ok(inventory) => Ok(inventory.rpc_endpoints),
         Err(_) => {
+            let starting_port = match std::env::var("CHURN_TEST_START_PORT") {
+                Ok(val) => val.parse()?,
+                Err(_) => 12000,
+            };
             let mut addresses = Vec::new();
             for i in 1..LOCAL_NODE_COUNT + 1 {
-                let addr =
-                    SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 12000 + i as u16);
+                let addr = SocketAddr::new(
+                    IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)),
+                    starting_port + i as u16,
+                );
                 addresses.push(addr);
             }
-            addresses
+            Ok(addresses)
         }
     }
 }

sn_node/tests/data_with_churn.rs

@@ -494,7 +494,7 @@ fn churn_nodes_task(
 ) {
     let start = Instant::now();
     let _handle = tokio::spawn(async move {
-        let node_rpc_addresses = get_all_rpc_addresses();
+        let node_rpc_addresses = get_all_rpc_addresses().expect("Failed to obtain rpc addresses");
         'main: loop {
             for rpc_address in node_rpc_addresses.iter() {
                 sleep(churn_period).await;

sn_node/tests/msgs_over_gossipsub.rs

@@ -33,7 +33,7 @@ async fn msgs_over_gossipsub() -> Result<()> {
     let node_count = get_node_count();
     let nodes_subscribed = node_count / 2; // 12 out of 25 nodes will be subscribers
 
-    let node_rpc_addresses = get_all_rpc_addresses()
+    let node_rpc_addresses = get_all_rpc_addresses()?
         .into_iter()
         .enumerate()
         .collect::<Vec<_>>();

sn_node/tests/nodes_rewards.rs

@@ -187,7 +187,7 @@ async fn nodes_rewards_transfer_notifs_filter() -> Result<()> {
 
     let (files_api, _content_bytes, _content_addr, chunks) =
         random_content(&client, paying_wallet_dir.to_path_buf(), chunks_dir.path())?;
-    let node_rpc_addresses = get_all_rpc_addresses();
+    let node_rpc_addresses = get_all_rpc_addresses()?;
 
     // this node shall receive the notifications since we set the correct royalties pk as filter
     let royalties_pk = NETWORK_ROYALTIES_PK.public_key();

sn_node/tests/verify_data_location.rs

@@ -91,7 +91,7 @@ async fn verify_data_location() -> Result<()> {
         "Performing data location verification with a churn count of {churn_count} and n_chunks {chunk_count}\nIt will take approx {:?}",
         VERIFICATION_DELAY*churn_count as u32
     );
-    let node_rpc_address = get_all_rpc_addresses();
+    let node_rpc_address = get_all_rpc_addresses()?;
     let mut all_peers = get_all_peer_ids(&node_rpc_address).await?;
 
     // Store chunks

sn_node/tests/verify_routing_table.rs

@@ -42,7 +42,7 @@ async fn verify_routing_table() -> Result<()> {
     println!("Sleeping for {sleep_duration:?} before verification");
     tokio::time::sleep(sleep_duration).await;
 
-    let node_rpc_address = get_all_rpc_addresses();
+    let node_rpc_address = get_all_rpc_addresses()?;
 
     let all_peers = get_all_peer_ids(&node_rpc_address).await?;
     let mut all_failed_list = BTreeMap::new();

sn_node_rpc_client/src/main.rs

@@ -17,7 +17,7 @@ use libp2p::Multiaddr;
 use sn_client::Client;
 use sn_logging::LogBuilder;
 use sn_node::{NodeEvent, ROYALTY_TRANSFER_NOTIF_TOPIC};
-use sn_peers_acquisition::{parse_peers_args, PeersArgs};
+use sn_peers_acquisition::{get_peers_from_args, PeersArgs};
 use sn_protocol::safenode_proto::{
     safe_node_client::SafeNodeClient, GossipsubSubscribeRequest, NodeEventsRequest,
     TransferNotifsFilterRequest,
@@ -134,7 +134,7 @@ async fn main() -> Result<()> {
             log_cash_notes,
             peers,
         } => {
-            let bootstrap_peers = parse_peers_args(peers).await?;
+            let bootstrap_peers = get_peers_from_args(peers).await?;
             let bootstrap_peers = if bootstrap_peers.is_empty() {
                 // empty vec is returned if `local-discovery` flag is provided
                 None

sn_peers_acquisition/Cargo.toml

@@ -17,10 +17,10 @@ network-contacts = ["reqwest", "tokio", "url"]
 
 [dependencies]
 clap = { version = "4.2.1", features = ["derive", "env"] }
-color-eyre = "~0.6"
 libp2p = { version="0.53", features = [] }
 rand = "0.8.5"
 reqwest = { version="0.11.18", default-features=false, features = ["rustls-tls"], optional = true }
+thiserror = "1.0.23"
 tokio = { version = "1.32.0", optional = true}
 tracing = { version = "~0.1.26" }
 url = { version = "2.4.0", optional = true }

sn_peers_acquisition/src/error.rs

@@ -0,0 +1,22 @@
+use thiserror::Error;
+
+pub type Result<T, E = Error> = std::result::Result<T, E>;
+
+#[derive(Debug, Error)]
+#[allow(missing_docs)]
+pub enum Error {
+    #[error("Could not parse the supplied multiaddr or socket address")]
+    InvalidPeerAddr,
+    #[error("Could not obtain network contacts from {0} after {1} retries")]
+    NetworkContactsUnretrievable(String, usize),
+    #[error("No valid multaddr was present in the contacts file at {0}")]
+    NoMultiAddrObtainedFromNetworkContacts(String),
+    #[error("Could not obtain peers through any available options")]
+    PeersNotObtained,
+    #[cfg(feature = "network-contacts")]
+    #[error(transparent)]
+    ReqwestError(#[from] reqwest::Error),
+    #[cfg(feature = "network-contacts")]
+    #[error(transparent)]
+    UrlParseError(#[from] url::ParseError),
+}