Merge #314

314: Remove CLI config file in favour of parameters r=thomaseizinger a=da-kami

Fixes #282 

The CLI has sensible default values for all parameters,
thus a config file is not really an advantage but just
keeps getting in our way, so re remove it.

Trait impls on `Data` needed for structopt, see https://docs.rs/structopt/0.3.21/structopt/#default-values

Co-authored-by: Daniel Karzel <daniel@comit.network>

swap/src/bin/swap.rs

@@ -16,12 +16,14 @@ use anyhow::{bail, Context, Result};
 use prettytable::{row, Table};
 use std::cmp::min;
 use std::future::Future;
+use std::path::PathBuf;
 use std::sync::Arc;
 use std::time::Duration;
 use structopt::StructOpt;
 use swap::bitcoin::{Amount, TxLock};
-use swap::cli::command::{AliceConnectParams, Arguments, Command, MoneroParams};
-use swap::cli::config::{read_config, Config};
+use swap::cli::command::{
+    AliceConnectParams, Arguments, BitcoinParams, Command, Data, MoneroParams,
+};
 use swap::database::Database;
 use swap::execution_params::{ExecutionParams, GetExecutionParams};
 use swap::network::quote::BidQuote;
@@ -31,6 +33,7 @@ use swap::seed::Seed;
 use swap::{bitcoin, execution_params, monero};
 use tracing::{debug, error, info, warn, Level};
 use tracing_subscriber::FmtSubscriber;
+use url::Url;
 use uuid::Uuid;
 
 #[macro_use]
@@ -66,16 +69,14 @@ async fn main() -> Result<()> {
         tracing::subscriber::set_global_default(subscriber)?;
     }
 
-    let config = match args.file_path {
-        Some(config_path) => read_config(config_path)??,
-        None => Config::testnet(),
-    };
+    let data: Data = args.data;
+    let data_dir = data.0;
 
-    let db = Database::open(config.data.dir.join("database").as_path())
-        .context("Failed to open database")?;
+    let db =
+        Database::open(data_dir.join("database").as_path()).context("Failed to open database")?;
 
     let seed =
-        Seed::from_file_or_generate(&config.data.dir).context("Failed to read in seed file")?;
+        Seed::from_file_or_generate(data_dir.as_path()).context("Failed to read in seed file")?;
 
     // hardcode to testnet/stagenet
     let bitcoin_network = bitcoin::Network::Testnet;
@@ -94,6 +95,11 @@ async fn main() -> Result<()> {
                     receive_monero_address,
                     monero_daemon_host,
                 },
+            bitcoin_params:
+                BitcoinParams {
+                    electrum_http_url,
+                    electrum_rpc_url,
+                },
         } => {
             if receive_monero_address.network != monero_network {
                 bail!(
@@ -103,10 +109,17 @@ async fn main() -> Result<()> {
                 )
             }
 
-            let bitcoin_wallet = init_bitcoin_wallet(bitcoin_network, &config, seed).await?;
+            let bitcoin_wallet = init_bitcoin_wallet(
+                bitcoin_network,
+                electrum_rpc_url,
+                electrum_http_url,
+                seed,
+                data_dir.clone(),
+            )
+            .await?;
             let (monero_wallet, _process) = init_monero_wallet(
                 monero_network,
-                &config,
+                data_dir,
                 monero_daemon_host,
                 execution_params,
             )
@@ -183,15 +196,27 @@ async fn main() -> Result<()> {
                     receive_monero_address,
                     monero_daemon_host,
                 },
+            bitcoin_params:
+                BitcoinParams {
+                    electrum_http_url,
+                    electrum_rpc_url,
+                },
         } => {
             if receive_monero_address.network != monero_network {
                 bail!("The given monero address is on network {:?}, expected address of network {:?}.", receive_monero_address.network, monero_network)
             }
 
-            let bitcoin_wallet = init_bitcoin_wallet(bitcoin_network, &config, seed).await?;
+            let bitcoin_wallet = init_bitcoin_wallet(
+                bitcoin_network,
+                electrum_rpc_url,
+                electrum_http_url,
+                seed,
+                data_dir.clone(),
+            )
+            .await?;
             let (monero_wallet, _process) = init_monero_wallet(
                 monero_network,
-                &config,
+                data_dir,
                 monero_daemon_host,
                 execution_params,
             )
@@ -227,8 +252,23 @@ async fn main() -> Result<()> {
                 }
             }
         }
-        Command::Cancel { swap_id, force } => {
-            let bitcoin_wallet = init_bitcoin_wallet(bitcoin_network, &config, seed).await?;
+        Command::Cancel {
+            swap_id,
+            force,
+            bitcoin_params:
+                BitcoinParams {
+                    electrum_http_url,
+                    electrum_rpc_url,
+                },
+        } => {
+            let bitcoin_wallet = init_bitcoin_wallet(
+                bitcoin_network,
+                electrum_rpc_url,
+                electrum_http_url,
+                seed,
+                data_dir,
+            )
+            .await?;
 
             let resume_state = db.get_state(swap_id)?.try_into_bob()?.into();
             let cancel =
@@ -247,8 +287,23 @@ async fn main() -> Result<()> {
                 }
             }
         }
-        Command::Refund { swap_id, force } => {
-            let bitcoin_wallet = init_bitcoin_wallet(bitcoin_network, &config, seed).await?;
+        Command::Refund {
+            swap_id,
+            force,
+            bitcoin_params:
+                BitcoinParams {
+                    electrum_http_url,
+                    electrum_rpc_url,
+                },
+        } => {
+            let bitcoin_wallet = init_bitcoin_wallet(
+                bitcoin_network,
+                electrum_rpc_url,
+                electrum_http_url,
+                seed,
+                data_dir,
+            )
+            .await?;
 
             let resume_state = db.get_state(swap_id)?.try_into_bob()?.into();
 
@@ -268,14 +323,16 @@ async fn main() -> Result<()> {
 
 async fn init_bitcoin_wallet(
     network: bitcoin::Network,
-    config: &Config,
+    electrum_rpc_url: Url,
+    electrum_http_url: Url,
     seed: Seed,
+    data_dir: PathBuf,
 ) -> Result<bitcoin::Wallet> {
-    let wallet_dir = config.data.dir.join("wallet");
+    let wallet_dir = data_dir.join("wallet");
 
     let wallet = bitcoin::Wallet::new(
-        config.bitcoin.electrum_rpc_url.clone(),
-        config.bitcoin.electrum_http_url.clone(),
+        electrum_rpc_url.clone(),
+        electrum_http_url.clone(),
         network,
         &wallet_dir,
         seed.derive_extended_private_key(network)?,
@@ -290,13 +347,13 @@ async fn init_bitcoin_wallet(
 
 async fn init_monero_wallet(
     monero_network: monero::Network,
-    config: &Config,
+    data_dir: PathBuf,
     monero_daemon_host: String,
     execution_params: ExecutionParams,
 ) -> Result<(monero::Wallet, monero::WalletRpcProcess)> {
     const MONERO_BLOCKCHAIN_MONITORING_WALLET_NAME: &str = "swap-tool-blockchain-monitoring-wallet";
 
-    let monero_wallet_rpc = monero::WalletRpc::new(config.data.dir.join("monero")).await?;
+    let monero_wallet_rpc = monero::WalletRpc::new(data_dir.join("monero")).await?;
 
     let monero_wallet_rpc_process = monero_wallet_rpc
         .run(monero_network, monero_daemon_host.as_str())

swap/src/bitcoin/wallet.rs

@@ -329,7 +329,7 @@ fn make_blocks_tip_height_url(base_url: &Url) -> Result<Url> {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::cli::config::DEFAULT_ELECTRUM_HTTP_URL;
+    use crate::cli::command::DEFAULT_ELECTRUM_HTTP_URL;
 
     #[test]
     fn create_tx_status_url_from_default_base_url_success() {

swap/src/cli.rs

@@ -1,2 +1 @@
 pub mod command;
-pub mod config;

swap/src/cli/command.rs

@@ -1,8 +1,10 @@
+use crate::fs::default_data_dir;
 use anyhow::{Context, Result};
 use libp2p::core::Multiaddr;
 use libp2p::PeerId;
 use std::path::PathBuf;
 use std::str::FromStr;
+use url::Url;
 use uuid::Uuid;
 
 pub const DEFAULT_ALICE_MULTIADDR: &str = "/dns4/xmr-btc-asb.coblox.tech/tcp/9876";
@@ -11,15 +13,18 @@ pub const DEFAULT_ALICE_PEER_ID: &str = "12D3KooWCdMKjesXMJz1SiZ7HgotrxuqhQJbP5s
 // Port is assumed to be stagenet standard port 38081
 pub const DEFAULT_STAGENET_MONERO_DAEMON_HOST: &str = "monero-stagenet.exan.tech";
 
+pub const DEFAULT_ELECTRUM_HTTP_URL: &str = "https://blockstream.info/testnet/api/";
+const DEFAULT_ELECTRUM_RPC_URL: &str = "ssl://electrum.blockstream.info:60002";
+
 #[derive(structopt::StructOpt, Debug)]
 #[structopt(name = "xmr-btc-swap", about = "Atomically swap BTC for XMR")]
 pub struct Arguments {
     #[structopt(
-        long = "config",
-        help = "Provide a custom path to the configuration file. The configuration file must be a toml file.",
-        parse(from_os_str)
+        long = "--data-dir",
+        help = "Provide the data directory path to be used to store application data",
+        default_value
     )]
-    pub file_path: Option<PathBuf>,
+    pub data: Data,
 
     #[structopt(long, help = "Activate debug logging.")]
     pub debug: bool,
@@ -35,6 +40,9 @@ pub enum Command {
         #[structopt(flatten)]
         connect_params: AliceConnectParams,
 
+        #[structopt(flatten)]
+        bitcoin_params: BitcoinParams,
+
         #[structopt(flatten)]
         monero_params: MoneroParams,
     },
@@ -51,6 +59,9 @@ pub enum Command {
         #[structopt(flatten)]
         connect_params: AliceConnectParams,
 
+        #[structopt(flatten)]
+        bitcoin_params: BitcoinParams,
+
         #[structopt(flatten)]
         monero_params: MoneroParams,
     },
@@ -64,6 +75,9 @@ pub enum Command {
 
         #[structopt(short, long)]
         force: bool,
+
+        #[structopt(flatten)]
+        bitcoin_params: BitcoinParams,
     },
     /// Try to cancel a swap and refund my BTC (expert users only)
     Refund {
@@ -75,6 +89,9 @@ pub enum Command {
 
         #[structopt(short, long)]
         force: bool,
+
+        #[structopt(flatten)]
+        bitcoin_params: BitcoinParams,
     },
 }
 
@@ -111,6 +128,48 @@ pub struct MoneroParams {
     pub monero_daemon_host: String,
 }
 
+#[derive(structopt::StructOpt, Debug)]
+pub struct BitcoinParams {
+    #[structopt(long = "electrum-http",
+    help = "Provide the Bitcoin Electrum HTTP URL",
+    default_value = DEFAULT_ELECTRUM_HTTP_URL
+    )]
+    pub electrum_http_url: Url,
+
+    #[structopt(long = "electrum-rpc",
+    help = "Provide the Bitcoin Electrum RPC URL",
+    default_value = DEFAULT_ELECTRUM_RPC_URL
+    )]
+    pub electrum_rpc_url: Url,
+}
+
+#[derive(Clone, Debug)]
+pub struct Data(pub PathBuf);
+
+impl Default for Data {
+    fn default() -> Self {
+        Data(default_data_dir().expect("computed valid path for data dir"))
+    }
+}
+
+impl FromStr for Data {
+    type Err = core::convert::Infallible;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        Ok(Data(PathBuf::from_str(s)?))
+    }
+}
+
+impl ToString for Data {
+    fn to_string(&self) -> String {
+        self.0
+            .clone()
+            .into_os_string()
+            .into_string()
+            .expect("default datadir to be convertible to string")
+    }
+}
+
 fn parse_monero_address(s: &str) -> Result<monero::Address> {
     monero::Address::from_str(s).with_context(|| {
         format!(

swap/src/cli/config.rs

@@ -1,118 +0,0 @@
-use crate::fs::default_data_dir;
-use anyhow::{Context, Result};
-use config::ConfigError;
-use serde::{Deserialize, Serialize};
-use std::ffi::OsStr;
-use std::path::{Path, PathBuf};
-use tracing::debug;
-use url::Url;
-
-pub const DEFAULT_ELECTRUM_HTTP_URL: &str = "https://blockstream.info/testnet/api/";
-const DEFAULT_ELECTRUM_RPC_URL: &str = "ssl://electrum.blockstream.info:60002";
-
-#[derive(Clone, Debug, serde::Serialize, serde::Deserialize, PartialEq)]
-pub struct Config {
-    pub data: Data,
-    pub bitcoin: Bitcoin,
-}
-
-impl Config {
-    pub fn read<D>(config_file: D) -> Result<Self, ConfigError>
-    where
-        D: AsRef<OsStr>,
-    {
-        let config_file = Path::new(&config_file);
-
-        let mut config = config::Config::new();
-        config.merge(config::File::from(config_file))?;
-        config.try_into()
-    }
-
-    pub fn testnet() -> Self {
-        Self {
-            data: Data {
-                dir: default_data_dir().expect("computed valid path for data dir"),
-            },
-            bitcoin: Bitcoin {
-                electrum_http_url: DEFAULT_ELECTRUM_HTTP_URL
-                    .parse()
-                    .expect("default electrum http str is a valid url"),
-                electrum_rpc_url: DEFAULT_ELECTRUM_RPC_URL
-                    .parse()
-                    .expect("default electrum rpc str is a valid url"),
-            },
-        }
-    }
-}
-
-#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)]
-#[serde(deny_unknown_fields)]
-pub struct Data {
-    pub dir: PathBuf,
-}
-
-#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)]
-#[serde(deny_unknown_fields)]
-pub struct Bitcoin {
-    pub electrum_http_url: Url,
-    pub electrum_rpc_url: Url,
-}
-
-#[derive(thiserror::Error, Debug, Clone, Copy)]
-#[error("config not initialized")]
-pub struct ConfigNotInitialized {}
-
-pub fn read_config(config_path: PathBuf) -> Result<Result<Config, ConfigNotInitialized>> {
-    if config_path.exists() {
-        debug!(
-            "Using config file at default path: {}",
-            config_path.display()
-        );
-    } else {
-        return Ok(Err(ConfigNotInitialized {}));
-    }
-
-    let file = Config::read(&config_path)
-        .with_context(|| format!("Failed to read config file at {}", config_path.display()))?;
-
-    Ok(Ok(file))
-}
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-    use crate::fs::ensure_directory_exists;
-    use std::fs;
-    use std::str::FromStr;
-    use tempfile::tempdir;
-
-    pub fn initial_setup(config_path: PathBuf, config: Config) -> Result<()> {
-        ensure_directory_exists(config_path.as_path())?;
-
-        let toml = toml::to_string(&config)?;
-        fs::write(&config_path, toml)?;
-
-        Ok(())
-    }
-
-    #[test]
-    fn config_roundtrip() {
-        let temp_dir = tempdir().unwrap().path().to_path_buf();
-        let config_path = Path::join(&temp_dir, "config.toml");
-
-        let expected = Config {
-            data: Data {
-                dir: Default::default(),
-            },
-            bitcoin: Bitcoin {
-                electrum_http_url: Url::from_str(DEFAULT_ELECTRUM_HTTP_URL).unwrap(),
-                electrum_rpc_url: Url::from_str(DEFAULT_ELECTRUM_RPC_URL).unwrap(),
-            },
-        };
-
-        initial_setup(config_path.clone(), expected.clone()).unwrap();
-        let actual = read_config(config_path).unwrap().unwrap();
-
-        assert_eq!(expected, actual);
-    }
-}