From 35d774dcf0b2e118610f443211642946652debd5 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Thu, 13 Apr 2023 19:24:55 +0200 Subject: [PATCH] fix: add MaybePlatformPath (#2231) --- bin/reth/src/args/network_args.rs | 6 +-- bin/reth/src/args/secret_key.rs | 6 +-- bin/reth/src/chain/import.rs | 12 ++--- bin/reth/src/chain/init.rs | 6 +-- bin/reth/src/db/mod.rs | 8 +-- bin/reth/src/dirs.rs | 84 ++++++++++++++++++++++++++++++- bin/reth/src/drop_stage.rs | 6 +-- bin/reth/src/dump_stage/mod.rs | 6 +-- bin/reth/src/node/mod.rs | 55 ++++++++++++++++---- bin/reth/src/stage/mod.rs | 12 +++-- bin/reth/src/utils.rs | 9 ++-- 11 files changed, 165 insertions(+), 45 deletions(-) diff --git a/bin/reth/src/args/network_args.rs b/bin/reth/src/args/network_args.rs index c4b6108e29..d4a9d868b6 100644 --- a/bin/reth/src/args/network_args.rs +++ b/bin/reth/src/args/network_args.rs @@ -1,6 +1,6 @@ //! clap [Args](clap::Args) for network related arguments. -use crate::dirs::{KnownPeersPath, PlatformPath}; +use crate::dirs::{KnownPeersPath, MaybePlatformPath}; use clap::Args; use reth_net_nat::NatResolver; use reth_network::NetworkConfigBuilder; @@ -36,7 +36,7 @@ pub struct NetworkArgs { /// dumped to this file on node shutdown, and read on startup. /// Cannot be used with --no-persist-peers #[arg(long, value_name = "FILE", verbatim_doc_comment, default_value_t)] - pub peers_file: PlatformPath, + pub peers_file: MaybePlatformPath, /// Do not persist peers. Cannot be used with --peers-file #[arg(long, verbatim_doc_comment, conflicts_with = "peers_file")] @@ -82,7 +82,7 @@ impl NetworkArgs { return None } - let peers_file = self.peers_file.clone().with_chain(chain); + let peers_file = self.peers_file.clone().unwrap_or_chain_default(chain); Some(peers_file.into()) } } diff --git a/bin/reth/src/args/secret_key.rs b/bin/reth/src/args/secret_key.rs index 6904213f8c..a2648ab65d 100644 --- a/bin/reth/src/args/secret_key.rs +++ b/bin/reth/src/args/secret_key.rs @@ -2,7 +2,7 @@ use crate::dirs::{PlatformPath, SecretKeyPath}; 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; +use std::{fs::read_to_string, path::Path}; use thiserror::Error; /// Errors returned by loading a [`SecretKey`][secp256k1::SecretKey], including IO errors. @@ -19,9 +19,7 @@ pub enum SecretKeyError { /// there, then it generates a secret key and stores it in the default path. I/O /// errors might occur during write operations in the form of a /// [`SecretKeyError`] -pub fn get_secret_key( - secret_key_path: &PlatformPath, -) -> Result { +pub fn get_secret_key(secret_key_path: impl AsRef) -> Result { let fpath = secret_key_path.as_ref(); let exists = fpath.try_exists(); diff --git a/bin/reth/src/chain/import.rs b/bin/reth/src/chain/import.rs index 6a9b6cbf61..ad19335aa5 100644 --- a/bin/reth/src/chain/import.rs +++ b/bin/reth/src/chain/import.rs @@ -1,5 +1,5 @@ use crate::{ - dirs::{ConfigPath, DbPath, PlatformPath}, + dirs::{ConfigPath, DbPath, MaybePlatformPath, PlatformPath}, node::events::{handle_events, NodeEvent}, }; use clap::{crate_version, Parser}; @@ -35,7 +35,7 @@ use tracing::{debug, info}; pub struct ImportCommand { /// The path to the configuration file to use. #[arg(long, value_name = "FILE", verbatim_doc_comment, default_value_t)] - config: PlatformPath, + config: MaybePlatformPath, /// The path to the database folder. /// @@ -45,7 +45,7 @@ pub struct ImportCommand { /// - Windows: `{FOLDERID_RoamingAppData}/reth/db` /// - macOS: `$HOME/Library/Application Support/reth/db` #[arg(long, value_name = "PATH", verbatim_doc_comment, default_value_t)] - db: PlatformPath, + db: MaybePlatformPath, /// The chain this node is running. /// @@ -78,10 +78,10 @@ impl ImportCommand { info!(target: "reth::cli", "reth {} starting", crate_version!()); let config: Config = self.load_config_with_chain(self.chain.chain)?; - info!(target: "reth::cli", path = %self.config.with_chain(self.chain.chain), "Configuration loaded"); + info!(target: "reth::cli", path = %self.config.unwrap_or_chain_default(self.chain.chain), "Configuration loaded"); // add network name to db directory - let db_path = self.db.with_chain(self.chain.chain); + let db_path = self.db.unwrap_or_chain_default(self.chain.chain); info!(target: "reth::cli", path = ?db_path, "Opening database"); let db = Arc::new(init_db(db_path)?); @@ -175,7 +175,7 @@ impl ImportCommand { /// Loads the reth config based on the intended chain fn load_config_with_chain(&self, chain: Chain) -> eyre::Result { // add network name to config directory - let config_path = self.config.with_chain(chain); + let config_path = self.config.unwrap_or_chain_default(chain); confy::load_path::(config_path.clone()) .wrap_err_with(|| format!("Could not load config file {}", config_path)) } diff --git a/bin/reth/src/chain/init.rs b/bin/reth/src/chain/init.rs index bf53467484..cefd173f6c 100644 --- a/bin/reth/src/chain/init.rs +++ b/bin/reth/src/chain/init.rs @@ -1,4 +1,4 @@ -use crate::dirs::{DbPath, PlatformPath}; +use crate::dirs::{DbPath, MaybePlatformPath}; use clap::Parser; use reth_primitives::ChainSpec; use reth_staged_sync::utils::{ @@ -19,7 +19,7 @@ pub struct InitCommand { /// - Windows: `{FOLDERID_RoamingAppData}/reth/db` /// - macOS: `$HOME/Library/Application Support/reth/db` #[arg(long, value_name = "PATH", verbatim_doc_comment, default_value_t)] - db: PlatformPath, + db: MaybePlatformPath, /// The chain this node is running. /// @@ -45,7 +45,7 @@ impl InitCommand { info!(target: "reth::cli", "reth init starting"); // add network name to db directory - let db_path = self.db.with_chain(self.chain.chain); + let db_path = self.db.unwrap_or_chain_default(self.chain.chain); info!(target: "reth::cli", path = %db_path, "Opening database"); let db = Arc::new(init_db(&db_path)?); diff --git a/bin/reth/src/db/mod.rs b/bin/reth/src/db/mod.rs index 845be6bc2a..9eafc92e68 100644 --- a/bin/reth/src/db/mod.rs +++ b/bin/reth/src/db/mod.rs @@ -2,7 +2,7 @@ use std::sync::Arc; use crate::{ - dirs::{DbPath, PlatformPath}, + dirs::{DbPath, MaybePlatformPath}, utils::DbTool, }; use clap::{Parser, Subcommand}; @@ -28,7 +28,7 @@ pub struct Command { /// - Windows: `{FOLDERID_RoamingAppData}/reth/db` /// - macOS: `$HOME/Library/Application Support/reth/db` #[arg(global = true, long, value_name = "PATH", verbatim_doc_comment, default_value_t)] - db: PlatformPath, + db: MaybePlatformPath, /// The chain this node is running. /// @@ -87,7 +87,7 @@ impl Command { /// Execute `db` command pub async fn execute(&self) -> eyre::Result<()> { // add network name to db directory - let db_path = self.db.with_chain(self.chain.chain); + let db_path = self.db.unwrap_or_chain_default(self.chain.chain); std::fs::create_dir_all(&db_path)?; @@ -212,7 +212,7 @@ impl Command { ]); } Subcommands::Drop => { - tool.drop(&self.db)?; + tool.drop(self.db.unwrap_or_chain_default(self.chain.chain))?; } } diff --git a/bin/reth/src/dirs.rs b/bin/reth/src/dirs.rs index 1d467d88f8..e36bc74eda 100644 --- a/bin/reth/src/dirs.rs +++ b/bin/reth/src/dirs.rs @@ -181,7 +181,7 @@ pub trait XdgPath { /// /// assert_ne!(default.as_ref(), custom.as_ref()); /// ``` -#[derive(Clone, Debug, PartialEq)] +#[derive(Debug, PartialEq)] pub struct PlatformPath(PathBuf, std::marker::PhantomData); impl Display for PlatformPath { @@ -190,6 +190,12 @@ impl Display for PlatformPath { } } +impl Clone for PlatformPath { + fn clone(&self) -> Self { + Self(self.0.clone(), std::marker::PhantomData) + } +} + impl Default for PlatformPath { fn default() -> Self { Self( @@ -246,6 +252,55 @@ impl PlatformPath { } } +/// An Optional wrapper type around [PlatformPath]. +/// +/// This is useful for when a path is optional, such as the `--db-path` flag. +#[derive(Clone, Debug, PartialEq)] +pub struct MaybePlatformPath(Option>); + +// === impl MaybePlatformPath === + +impl MaybePlatformPath { + /// Returns the path if it is set, otherwise returns the default path for the given chain. + pub fn unwrap_or_chain_default(&self, chain: Chain) -> PlatformPath { + self.0.clone().unwrap_or_else(|| PlatformPath::default().with_chain(chain).0) + } +} + +impl Display for MaybePlatformPath { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + if let Some(path) = &self.0 { + path.fmt(f) + } else { + // NOTE: this is a workaround for making it work with clap's `default_value_t` which + // computes the default value via `Default -> Display -> FromStr` + write!(f, "default") + } + } +} + +impl Default for MaybePlatformPath { + fn default() -> Self { + Self(None) + } +} + +impl FromStr for MaybePlatformPath { + type Err = shellexpand::LookupError; + + fn from_str(s: &str) -> Result { + let p = match s { + "default" => { + // NOTE: this is a workaround for making it work with clap's `default_value_t` which + // computes the default value via `Default -> Display -> FromStr` + None + } + _ => Some(PlatformPath::from_str(s)?), + }; + Ok(Self(p)) + } +} + /// Wrapper type around PlatformPath that includes a `Chain`, used for separating reth data for /// different networks. /// @@ -282,3 +337,30 @@ impl From> for PathBuf { value.0.into() } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_maybe_db_platform_path() { + let path = MaybePlatformPath::::default(); + let path = path.unwrap_or_chain_default(Chain::mainnet()); + assert!(path.as_ref().ends_with("reth/mainnet/db"), "{:?}", path); + + let path = MaybePlatformPath::::from_str("my/path/to/db").unwrap(); + let path = path.unwrap_or_chain_default(Chain::mainnet()); + assert!(path.as_ref().ends_with("my/path/to/db"), "{:?}", path); + } + + #[test] + fn test_maybe_config_platform_path() { + let path = MaybePlatformPath::::default(); + let path = path.unwrap_or_chain_default(Chain::mainnet()); + assert!(path.as_ref().ends_with("reth/mainnet/reth.toml"), "{:?}", path); + + let path = MaybePlatformPath::::from_str("my/path/to/reth.toml").unwrap(); + let path = path.unwrap_or_chain_default(Chain::mainnet()); + assert!(path.as_ref().ends_with("my/path/to/reth.toml"), "{:?}", path); + } +} diff --git a/bin/reth/src/drop_stage.rs b/bin/reth/src/drop_stage.rs index a6ac04059c..aded46d947 100644 --- a/bin/reth/src/drop_stage.rs +++ b/bin/reth/src/drop_stage.rs @@ -1,6 +1,6 @@ //! Database debugging tool use crate::{ - dirs::{DbPath, PlatformPath}, + dirs::{DbPath, MaybePlatformPath}, utils::DbTool, StageEnum, }; @@ -28,7 +28,7 @@ pub struct Command { /// - Windows: `{FOLDERID_RoamingAppData}/reth/db` /// - macOS: `$HOME/Library/Application Support/reth/db` #[arg(global = true, long, value_name = "PATH", verbatim_doc_comment, default_value_t)] - db: PlatformPath, + db: MaybePlatformPath, /// The chain this node is running. /// @@ -54,7 +54,7 @@ impl Command { /// Execute `db` command pub async fn execute(&self) -> eyre::Result<()> { // add network name to db directory - let db_path = self.db.with_chain(self.chain.chain); + let db_path = self.db.unwrap_or_chain_default(self.chain.chain); std::fs::create_dir_all(&db_path)?; diff --git a/bin/reth/src/dump_stage/mod.rs b/bin/reth/src/dump_stage/mod.rs index 338f7dfc4b..cbe32b5a88 100644 --- a/bin/reth/src/dump_stage/mod.rs +++ b/bin/reth/src/dump_stage/mod.rs @@ -15,7 +15,7 @@ use merkle::dump_merkle_stage; use reth_primitives::ChainSpec; use crate::{ - dirs::{DbPath, PlatformPath}, + dirs::{DbPath, MaybePlatformPath, PlatformPath}, utils::DbTool, }; use clap::Parser; @@ -36,7 +36,7 @@ pub struct Command { /// - Windows: `{FOLDERID_RoamingAppData}/reth/db` /// - macOS: `$HOME/Library/Application Support/reth/db` #[arg(long, value_name = "PATH", verbatim_doc_comment, default_value_t)] - db: PlatformPath, + db: MaybePlatformPath, /// The chain this node is running. /// @@ -100,7 +100,7 @@ impl Command { /// Execute `dump-stage` command pub async fn execute(&self) -> eyre::Result<()> { // add network name to db directory - let db_path = self.db.with_chain(self.chain.chain); + let db_path = self.db.unwrap_or_chain_default(self.chain.chain); std::fs::create_dir_all(&db_path)?; diff --git a/bin/reth/src/node/mod.rs b/bin/reth/src/node/mod.rs index 3c67166ddf..197ac5a8b9 100644 --- a/bin/reth/src/node/mod.rs +++ b/bin/reth/src/node/mod.rs @@ -3,7 +3,7 @@ //! Starts the client use crate::{ args::{get_secret_key, DebugArgs, NetworkArgs, RpcServerArgs}, - dirs::{ConfigPath, DbPath, PlatformPath, SecretKeyPath}, + dirs::{ConfigPath, DbPath, SecretKeyPath}, prometheus_exporter, runner::CliContext, utils::get_single_header, @@ -70,6 +70,7 @@ use tokio::sync::{ }; use tracing::*; +use crate::dirs::MaybePlatformPath; use reth_interfaces::p2p::headers::client::HeadersClient; use reth_stages::stages::{MERKLE_EXECUTION, MERKLE_UNWIND}; @@ -80,7 +81,7 @@ pub mod events; pub struct Command { /// The path to the configuration file to use. #[arg(long, value_name = "FILE", verbatim_doc_comment, default_value_t)] - config: PlatformPath, + config: MaybePlatformPath, /// The path to the database folder. /// @@ -90,7 +91,7 @@ pub struct Command { /// - Windows: `{FOLDERID_RoamingAppData}/reth/db` /// - macOS: `$HOME/Library/Application Support/reth/db` #[arg(long, value_name = "PATH", verbatim_doc_comment, default_value_t)] - db: PlatformPath, + db: MaybePlatformPath, /// The chain this node is running. /// @@ -113,7 +114,7 @@ pub struct Command { /// /// This also will deterministically set the peer ID. #[arg(long, value_name = "PATH", global = true, required = false, default_value_t)] - p2p_secret_key: PlatformPath, + p2p_secret_key: MaybePlatformPath, /// Enable Prometheus metrics. /// @@ -145,10 +146,10 @@ impl Command { raise_fd_limit(); let mut config: Config = self.load_config_with_chain(self.chain.chain)?; - info!(target: "reth::cli", path = %self.config.with_chain(self.chain.chain), "Configuration loaded"); + info!(target: "reth::cli", path = %self.config.unwrap_or_chain_default(self.chain.chain), "Configuration loaded"); // add network name to db directory - let db_path = self.db.with_chain(self.chain.chain); + let db_path = self.db.unwrap_or_chain_default(self.chain.chain); info!(target: "reth::cli", path = %db_path, "Opening database"); let db = Arc::new(init_db(&db_path)?); @@ -177,7 +178,8 @@ impl Command { info!(target: "reth::cli", "Test transaction pool initialized"); info!(target: "reth::cli", "Connecting to P2P network"); - let secret_key = get_secret_key(&self.p2p_secret_key)?; + let secret_key = + get_secret_key(self.p2p_secret_key.unwrap_or_chain_default(self.chain.chain))?; let network_config = self.load_network_config( &config, Arc::clone(&db), @@ -407,7 +409,7 @@ impl Command { /// Loads the reth config based on the intended chain fn load_config_with_chain(&self, chain: Chain) -> eyre::Result { // add network name to config directory - let config_path = self.config.with_chain(chain); + let config_path = self.config.unwrap_or_chain_default(chain); confy::load_path::(config_path.clone()) .wrap_err_with(|| format!("Could not load config file {}", config_path)) } @@ -694,7 +696,7 @@ async fn run_network_until_shutdown( #[cfg(test)] mod tests { use super::*; - use std::net::IpAddr; + use std::{net::IpAddr, path::Path}; #[test] fn parse_help_node_command() { @@ -735,4 +737,39 @@ mod tests { let cmd = Command::try_parse_from(["reth", "--metrics", "localhost:9000"]).unwrap(); assert_eq!(cmd.metrics, Some(SocketAddr::new(IpAddr::V4(Ipv4Addr::LOCALHOST), 9000))); } + + #[test] + fn parse_config_path() { + let cmd = Command::try_parse_from(["reth", "--config", "my/path/to/reth.toml"]).unwrap(); + assert_eq!( + cmd.config.unwrap_or_chain_default(cmd.chain.chain).as_ref(), + Path::new("my/path/to/reth.toml") + ); + + let cmd = Command::try_parse_from(["reth"]).unwrap(); + assert!( + cmd.config + .unwrap_or_chain_default(cmd.chain.chain) + .as_ref() + .ends_with("reth/mainnet/reth.toml"), + "{:?}", + cmd.config + ); + } + + #[test] + fn parse_db_path() { + let cmd = Command::try_parse_from(["reth", "--db", "my/path/to/db"]).unwrap(); + assert_eq!( + cmd.db.unwrap_or_chain_default(cmd.chain.chain).as_ref(), + Path::new("my/path/to/db") + ); + + let cmd = Command::try_parse_from(["reth"]).unwrap(); + assert!( + cmd.db.unwrap_or_chain_default(cmd.chain.chain).as_ref().ends_with("reth/mainnet/db"), + "{:?}", + cmd.config + ); + } } diff --git a/bin/reth/src/stage/mod.rs b/bin/reth/src/stage/mod.rs index 7a88be2c08..84ead7d299 100644 --- a/bin/reth/src/stage/mod.rs +++ b/bin/reth/src/stage/mod.rs @@ -3,7 +3,7 @@ //! Stage debugging tool use crate::{ args::{get_secret_key, NetworkArgs}, - dirs::{ConfigPath, DbPath, PlatformPath, SecretKeyPath}, + dirs::{ConfigPath, DbPath, MaybePlatformPath, PlatformPath, SecretKeyPath}, prometheus_exporter, StageEnum, }; use clap::Parser; @@ -33,11 +33,11 @@ pub struct Command { /// - Windows: `{FOLDERID_RoamingAppData}/reth/db` /// - macOS: `$HOME/Library/Application Support/reth/db` #[arg(long, value_name = "PATH", verbatim_doc_comment, default_value_t)] - db: PlatformPath, + db: MaybePlatformPath, /// The path to the configuration file to use. #[arg(long, value_name = "FILE", verbatim_doc_comment, default_value_t)] - config: PlatformPath, + config: MaybePlatformPath, /// The chain this node is running. /// @@ -99,7 +99,9 @@ impl Command { // Does not do anything on windows. fdlimit::raise_fd_limit(); - let config: Config = confy::load_path(&self.config).unwrap_or_default(); + let config: Config = + confy::load_path(self.config.unwrap_or_chain_default(self.chain.chain)) + .unwrap_or_default(); info!(target: "reth::cli", "reth {} starting stage {:?}", clap::crate_version!(), self.stage); let input = ExecInput { @@ -110,7 +112,7 @@ impl Command { let unwind = UnwindInput { stage_progress: self.to, unwind_to: self.from, bad_block: None }; // add network name to db directory - let db_path = self.db.with_chain(self.chain.chain); + let db_path = self.db.unwrap_or_chain_default(self.chain.chain); let db = Arc::new(init_db(db_path)?); let mut tx = Transaction::new(db.as_ref())?; diff --git a/bin/reth/src/utils.rs b/bin/reth/src/utils.rs index 111c21c915..9bbb3d5fb6 100644 --- a/bin/reth/src/utils.rs +++ b/bin/reth/src/utils.rs @@ -1,5 +1,5 @@ //! Common CLI utility functions. -use crate::dirs::{DbPath, PlatformPath}; + use eyre::{Result, WrapErr}; use reth_db::{ cursor::{DbCursorRO, Walker}, @@ -16,7 +16,7 @@ use reth_interfaces::{ }; use reth_primitives::{BlockHashOrNumber, HeadersDirection, SealedHeader}; use reth_provider::insert_canonical_block; -use std::collections::BTreeMap; +use std::{collections::BTreeMap, path::Path}; use tracing::info; /// Get a single header from network @@ -106,8 +106,9 @@ impl<'a, DB: Database> DbTool<'a, DB> { } /// Drops the database at the given path. - pub fn drop(&mut self, path: &PlatformPath) -> Result<()> { - info!(target: "reth::cli", "Dropping db at {}", path); + pub fn drop(&mut self, path: impl AsRef) -> Result<()> { + let path = path.as_ref(); + info!(target: "reth::cli", "Dropping db at {:?}", path); std::fs::remove_dir_all(path).wrap_err("Dropping the database failed")?; Ok(()) }