fix: respect --p2p-secret-key argument (#2552)

bin/reth/src/args/network_args.rs

@@ -36,6 +36,13 @@ pub struct NetworkArgs {
     #[arg(long, value_name = "FILE", verbatim_doc_comment, conflicts_with = "no_persist_peers")]
     pub peers_file: Option<PathBuf>,
 
+    /// Secret key to use for this node.
+    ///
+    /// This will also deterministically set the peer ID. If not specified, it will be set in the
+    /// data dir for the chain being used.
+    #[arg(long, value_name = "PATH")]
+    pub p2p_secret_key: Option<PathBuf>,
+
     /// Do not persist peers.
     #[arg(long, verbatim_doc_comment)]
     pub no_persist_peers: bool,

bin/reth/src/args/secret_key.rs

@@ -1,7 +1,11 @@
 use hex::encode as hex_encode;
 use reth_network::config::rng_secret_key;
 use secp256k1::{Error as SecretKeyBaseError, SecretKey};
-use std::{fs::read_to_string, path::Path};
+use std::{
+    fs::read_to_string,
+    io,
+    path::{Path, PathBuf},
+};
 use thiserror::Error;
 
 /// Errors returned by loading a [`SecretKey`][secp256k1::SecretKey], including IO errors.
@@ -10,8 +14,14 @@ use thiserror::Error;
 pub enum SecretKeyError {
     #[error(transparent)]
     SecretKeyDecodeError(#[from] SecretKeyBaseError),
-    #[error("An I/O error occurred: {0}")]
+    #[error("Failed to create parent directory {dir:?} for secret key: {error}")]
-    IOError(#[from] std::io::Error),
+    FailedToCreateSecretParentDir { error: io::Error, dir: PathBuf },
+    #[error("Failed to write secret key file {secret_file:?}: {error}")]
+    FailedToWriteSecretKeyFile { error: io::Error, secret_file: PathBuf },
+    #[error("Failed to read secret key file {secret_file:?}: {error}")]
+    FailedToReadSecretKeyFile { error: io::Error, secret_file: PathBuf },
+    #[error("Failed to access key file {secret_file:?}: {error}")]
+    FailedToAccessKeyFile { error: io::Error, secret_file: PathBuf },
 }
 
 /// Attempts to load a [`SecretKey`] from a specified path. If no file exists there, then it
@@ -22,20 +32,35 @@ pub fn get_secret_key(secret_key_path: &Path) -> Result<SecretKey, SecretKeyErro
 
     match exists {
         Ok(true) => {
-            let contents = read_to_string(secret_key_path)?;
+            let contents = read_to_string(secret_key_path).map_err(|error| {
+                SecretKeyError::FailedToReadSecretKeyFile {
+                    error,
+                    secret_file: secret_key_path.to_path_buf(),
+                }
+            })?;
             (contents.as_str().parse::<SecretKey>()).map_err(SecretKeyError::SecretKeyDecodeError)
         }
         Ok(false) => {
             if let Some(dir) = secret_key_path.parent() {
                 // Create parent directory
-                std::fs::create_dir_all(dir)?
+                std::fs::create_dir_all(dir).map_err(|error| {
+                    SecretKeyError::FailedToCreateSecretParentDir { error, dir: dir.to_path_buf() }
+                })?;
             }
 
             let secret = rng_secret_key();
             let hex = hex_encode(secret.as_ref());
-            std::fs::write(secret_key_path, hex)?;
+            std::fs::write(secret_key_path, hex).map_err(|error| {
+                SecretKeyError::FailedToWriteSecretKeyFile {
+                    error,
+                    secret_file: secret_key_path.to_path_buf(),
+                }
+            })?;
             Ok(secret)
         }
-        Err(e) => Err(SecretKeyError::IOError(e)),
+        Err(error) => Err(SecretKeyError::FailedToAccessKeyFile {
+            error,
+            secret_file: secret_key_path.to_path_buf(),
+        }),
     }
 }

bin/reth/src/cli.rs

@@ -205,7 +205,8 @@ mod tests {
     fn test_parse_help_all_subcommands() {
         let reth = Cli::command();
         for sub_command in reth.get_subcommands() {
-            let err = Cli::try_parse_from(["reth", sub_command.get_name(), "--help"]).unwrap_err();
+            let err = Cli::try_parse_from(["reth", sub_command.get_name(), "--help"]).err().unwrap_or_else(|| panic!("Failed to parse help message {}", sub_command.get_name()));
+
             // --help is treated as error, but
             // > Not a true "error" as it means --help or similar was used. The help message will be sent to stdout.
             assert_eq!(err.kind(), clap::error::ErrorKind::DisplayHelp);

bin/reth/src/node/mod.rs

@@ -97,13 +97,6 @@ pub struct Command {
     #[arg(long, value_name = "PATH", verbatim_doc_comment)]
     db: Option<PathBuf>,
 
-    /// Secret key to use for this node.
-    ///
-    /// This will also deterministically set the peer ID. If not specified, it will be set in the
-    /// data dir for the chain being used.
-    #[arg(long, value_name = "PATH", global = true, required = false)]
-    p2p_secret_key: Option<PathBuf>,
-
     /// The chain this node is running.
     ///
     /// Possible values are either a built-in chain or the path to a chain specification file.
@@ -229,9 +222,11 @@ impl Command {
         }
 
         info!(target: "reth::cli", "Connecting to P2P network");
-        let default_secret_key_path = data_dir.p2p_secret_path();
+        let network_secret_path =
+            self.network.p2p_secret_key.clone().unwrap_or_else(|| data_dir.p2p_secret_path());
+        debug!(target: "reth::cli", ?network_secret_path, "Loading p2p key file");
+        let secret_key = get_secret_key(&network_secret_path)?;
         let default_peers_path = data_dir.known_peers_path();
-        let secret_key = get_secret_key(&default_secret_key_path)?;
         let network_config = self.load_network_config(
             &config,
             Arc::clone(&db),
@@ -377,7 +372,7 @@ impl Command {
         );
         info!(target: "reth::cli", "Engine API handler initialized");
 
-        // extract the jwt secret from the the args if possible
+        // extract the jwt secret from the args if possible
         let default_jwt_path = data_dir.jwt_path();
         let jwt_secret = self.rpc.jwt_secret(default_jwt_path)?;
 

bin/reth/src/p2p/mod.rs

@@ -54,7 +54,7 @@ pub struct Command {
     /// Secret key to use for this node.
     ///
     /// This also will deterministically set the peer ID.
-    #[arg(long, value_name = "PATH", global = true, required = false)]
+    #[arg(long, value_name = "PATH")]
     p2p_secret_key: Option<PathBuf>,
 
     /// Disable the discovery service.

bin/reth/src/stage/mod.rs

@@ -56,12 +56,6 @@ pub struct Command {
     )]
     chain: Arc<ChainSpec>,
 
-    /// Secret key to use for this node.
-    ///
-    /// This also will deterministically set the peer ID.
-    #[arg(long, value_name = "PATH", global = true, required = false)]
-    p2p_secret_key: Option<PathBuf>,
-
     /// Enable Prometheus metrics.
     ///
     /// The metrics will be served at the given interface and port.
@@ -139,8 +133,12 @@ impl Command {
                     });
                 }
 
-                let default_secret_key_path = data_dir.p2p_secret_path();
+                let network_secret_path = self
-                let p2p_secret_key = get_secret_key(&default_secret_key_path)?;
+                    .network
+                    .p2p_secret_key
+                    .clone()
+                    .unwrap_or_else(|| data_dir.p2p_secret_path());
+                let p2p_secret_key = get_secret_key(&network_secret_path)?;
 
                 let default_peers_path = data_dir.known_peers_path();