diff --git a/CHANGELOG.md b/CHANGELOG.md index 5d0d8e4d..589ac877 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - An issue where long-running connections are dead without a connection closure being reported back to the swarm. Adding a periodic ping ensures that the connection is kept alive, and a broken connection is reported back resulting in a close event on the swarm. This fixes the error of the ASB being unable to send a transfer proof to the CLI. +- An issue where ASB Bitcoin withdrawal can be done to an address on the wrong network. + A network check was added that compares the wallet's network against the network of the given address when building the transaction. ## [0.6.0] - 2021-05-24 diff --git a/Cargo.lock b/Cargo.lock index 506650d6..2d6d8a43 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4059,18 +4059,18 @@ dependencies = [ [[package]] name = "thiserror" -version = "1.0.24" +version = "1.0.25" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e0f4a65597094d4483ddaed134f409b2cb7c1beccf25201a9f73c719254fa98e" +checksum = "fa6f76457f59514c7eeb4e59d891395fab0b2fd1d40723ae737d64153392e9c6" dependencies = [ "thiserror-impl", ] [[package]] name = "thiserror-impl" -version = "1.0.24" +version = "1.0.25" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7765189610d8241a44529806d6fd1f2e0a08734313a35d5b3a556f92b381f3c0" +checksum = "8a36768c0fbf1bb15eca10defa29526bda730a2376c2ab4393ccfa16fb1a318d" dependencies = [ "proc-macro2 1.0.24", "quote 1.0.9", diff --git a/swap/src/asb/command.rs b/swap/src/asb/command.rs index e19b893f..602312a5 100644 --- a/swap/src/asb/command.rs +++ b/swap/src/asb/command.rs @@ -1,15 +1,210 @@ +use crate::asb::config::GetDefaults; use crate::bitcoin::Amount; +use crate::env; +use crate::env::GetConfig; +use anyhow::{bail, Result}; use bitcoin::Address; +use serde::Serialize; +use std::ffi::OsString; use std::path::PathBuf; +use structopt::StructOpt; use uuid::Uuid; +pub fn parse_args(raw_args: I) -> Result +where + I: IntoIterator, + T: Into + Clone, +{ + let matches = RawArguments::clap().get_matches_from_safe(raw_args)?; + let args = RawArguments::from_clap(&matches); + + let is_json = args.json; + let is_testnet = args.testnet; + let config = args.config; + let command: RawCommand = args.cmd; + + let arguments = match command { + RawCommand::Start { resume_only } => Arguments { + testnet: is_testnet, + json: is_json, + config_path: config_path(config, is_testnet)?, + env_config: env_config(is_testnet), + cmd: Command::Start { resume_only }, + }, + RawCommand::History => Arguments { + testnet: is_testnet, + json: is_json, + config_path: config_path(config, is_testnet)?, + env_config: env_config(is_testnet), + cmd: Command::History, + }, + RawCommand::WithdrawBtc { amount, address } => Arguments { + testnet: is_testnet, + json: is_json, + config_path: config_path(config, is_testnet)?, + env_config: env_config(is_testnet), + cmd: Command::WithdrawBtc { + amount, + address: bitcoin_address(address, is_testnet)?, + }, + }, + RawCommand::Balance => Arguments { + testnet: is_testnet, + json: is_json, + config_path: config_path(config, is_testnet)?, + env_config: env_config(is_testnet), + cmd: Command::Balance, + }, + RawCommand::ManualRecovery(manual_recovery) => match manual_recovery { + ManualRecovery::Redeem { + redeem_params: RecoverCommandParams { swap_id, force }, + do_not_await_finality, + } => Arguments { + testnet: is_testnet, + json: is_json, + config_path: config_path(config, is_testnet)?, + env_config: env_config(is_testnet), + cmd: Command::Redeem { + swap_id, + force, + do_not_await_finality, + }, + }, + ManualRecovery::Cancel { + cancel_params: RecoverCommandParams { swap_id, force }, + } => Arguments { + testnet: is_testnet, + json: is_json, + config_path: config_path(config, is_testnet)?, + env_config: env_config(is_testnet), + cmd: Command::Cancel { swap_id, force }, + }, + ManualRecovery::Refund { + refund_params: RecoverCommandParams { swap_id, force }, + } => Arguments { + testnet: is_testnet, + json: is_json, + config_path: config_path(config, is_testnet)?, + env_config: env_config(is_testnet), + cmd: Command::Refund { swap_id, force }, + }, + ManualRecovery::Punish { + punish_params: RecoverCommandParams { swap_id, force }, + } => Arguments { + testnet: is_testnet, + json: is_json, + config_path: config_path(config, is_testnet)?, + env_config: env_config(is_testnet), + cmd: Command::Punish { swap_id, force }, + }, + ManualRecovery::SafelyAbort { swap_id } => Arguments { + testnet: is_testnet, + json: is_json, + config_path: config_path(config, is_testnet)?, + env_config: env_config(is_testnet), + cmd: Command::SafelyAbort { swap_id }, + }, + }, + }; + + Ok(arguments) +} + +fn bitcoin_address(address: Address, is_testnet: bool) -> Result
{ + let network = if is_testnet { + bitcoin::Network::Testnet + } else { + bitcoin::Network::Bitcoin + }; + + if address.network != network { + bail!(BitcoinAddressNetworkMismatch { + expected: network, + actual: address.network + }); + } + + Ok(address) +} + +fn config_path(config: Option, is_testnet: bool) -> Result { + let config_path = if let Some(config_path) = config { + config_path + } else if is_testnet { + env::Testnet::getConfigFileDefaults()?.config_path + } else { + env::Mainnet::getConfigFileDefaults()?.config_path + }; + + Ok(config_path) +} + +fn env_config(is_testnet: bool) -> env::Config { + if is_testnet { + env::Testnet::get_config() + } else { + env::Mainnet::get_config() + } +} + +#[derive(thiserror::Error, Debug, Clone, Copy, PartialEq, Serialize)] +#[error("Invalid Bitcoin address provided, expected address on network {expected:?} but address provided is on {actual:?}")] +pub struct BitcoinAddressNetworkMismatch { + #[serde(with = "crate::bitcoin::network")] + expected: bitcoin::Network, + #[serde(with = "crate::bitcoin::network")] + actual: bitcoin::Network, +} + +#[derive(Debug, PartialEq)] +pub struct Arguments { + pub testnet: bool, + pub json: bool, + pub config_path: PathBuf, + pub env_config: env::Config, + pub cmd: Command, +} + +#[derive(Debug, PartialEq)] +pub enum Command { + Start { + resume_only: bool, + }, + History, + WithdrawBtc { + amount: Option, + address: Address, + }, + Balance, + Redeem { + swap_id: Uuid, + force: bool, + do_not_await_finality: bool, + }, + Cancel { + swap_id: Uuid, + force: bool, + }, + Refund { + swap_id: Uuid, + force: bool, + }, + Punish { + swap_id: Uuid, + force: bool, + }, + SafelyAbort { + swap_id: Uuid, + }, +} + #[derive(structopt::StructOpt, Debug)] #[structopt( name = "asb", about = "Automated Swap Backend for swapping XMR for BTC", author )] -pub struct Arguments { +pub struct RawArguments { #[structopt(long, help = "Swap on testnet")] pub testnet: bool, @@ -28,12 +223,12 @@ pub struct Arguments { pub config: Option, #[structopt(subcommand)] - pub cmd: Command, + pub cmd: RawCommand, } #[derive(structopt::StructOpt, Debug)] #[structopt(name = "xmr_btc-swap", about = "XMR BTC atomic swap")] -pub enum Command { +pub enum RawCommand { #[structopt(about = "Main command to run the ASB.")] Start { #[structopt( @@ -123,3 +318,333 @@ pub struct RecoverCommandParams { )] pub force: bool, } + +#[cfg(test)] +mod tests { + use super::*; + use std::str::FromStr; + + const BINARY_NAME: &str = "asb"; + const BITCOIN_MAINNET_ADDRESS: &str = "1KFHE7w8BhaENAswwryaoccDb6qcT6DbYY"; + const BITCOIN_TESTNET_ADDRESS: &str = "tb1qyccwk4yun26708qg5h6g6we8kxln232wclxf5a"; + const SWAP_ID: &str = "ea030832-3be9-454f-bb98-5ea9a788406b"; + + #[test] + fn ensure_command_mapping_for_mainnet() { + let default_mainnet_conf_path = env::Mainnet::getConfigFileDefaults().unwrap().config_path; + let mainnet_env_config = env::Mainnet::get_config(); + + let raw_ars = vec![BINARY_NAME, "start"]; + let expected_args = Arguments { + testnet: false, + json: false, + config_path: default_mainnet_conf_path.clone(), + env_config: mainnet_env_config, + cmd: Command::Start { resume_only: false }, + }; + let args = parse_args(raw_ars).unwrap(); + assert_eq!(expected_args, args); + + let raw_ars = vec![BINARY_NAME, "history"]; + let expected_args = Arguments { + testnet: false, + json: false, + config_path: default_mainnet_conf_path.clone(), + env_config: mainnet_env_config, + cmd: Command::History, + }; + let args = parse_args(raw_ars).unwrap(); + assert_eq!(expected_args, args); + + let raw_ars = vec![BINARY_NAME, "balance"]; + let expected_args = Arguments { + testnet: false, + json: false, + config_path: default_mainnet_conf_path.clone(), + env_config: mainnet_env_config, + cmd: Command::Balance, + }; + let args = parse_args(raw_ars).unwrap(); + assert_eq!(expected_args, args); + + let raw_ars = vec![ + BINARY_NAME, + "withdraw-btc", + "--address", + BITCOIN_MAINNET_ADDRESS, + ]; + let expected_args = Arguments { + testnet: false, + json: false, + config_path: default_mainnet_conf_path.clone(), + env_config: mainnet_env_config, + cmd: Command::WithdrawBtc { + amount: None, + address: Address::from_str(BITCOIN_MAINNET_ADDRESS).unwrap(), + }, + }; + let args = parse_args(raw_ars).unwrap(); + assert_eq!(expected_args, args); + + let raw_ars = vec![ + BINARY_NAME, + "manual-recovery", + "cancel", + "--swap-id", + SWAP_ID, + ]; + let expected_args = Arguments { + testnet: false, + json: false, + config_path: default_mainnet_conf_path.clone(), + env_config: mainnet_env_config, + cmd: Command::Cancel { + swap_id: Uuid::parse_str(SWAP_ID).unwrap(), + force: false, + }, + }; + let args = parse_args(raw_ars).unwrap(); + assert_eq!(expected_args, args); + + let raw_ars = vec![ + BINARY_NAME, + "manual-recovery", + "refund", + "--swap-id", + SWAP_ID, + ]; + let expected_args = Arguments { + testnet: false, + json: false, + config_path: default_mainnet_conf_path.clone(), + env_config: mainnet_env_config, + cmd: Command::Refund { + swap_id: Uuid::parse_str(SWAP_ID).unwrap(), + force: false, + }, + }; + let args = parse_args(raw_ars).unwrap(); + assert_eq!(expected_args, args); + + let raw_ars = vec![ + BINARY_NAME, + "manual-recovery", + "punish", + "--swap-id", + SWAP_ID, + ]; + let expected_args = Arguments { + testnet: false, + json: false, + config_path: default_mainnet_conf_path.clone(), + env_config: mainnet_env_config, + cmd: Command::Punish { + swap_id: Uuid::parse_str(SWAP_ID).unwrap(), + force: false, + }, + }; + let args = parse_args(raw_ars).unwrap(); + assert_eq!(expected_args, args); + + let raw_ars = vec![ + BINARY_NAME, + "manual-recovery", + "safely-abort", + "--swap-id", + SWAP_ID, + ]; + let expected_args = Arguments { + testnet: false, + json: false, + config_path: default_mainnet_conf_path, + env_config: mainnet_env_config, + cmd: Command::SafelyAbort { + swap_id: Uuid::parse_str(SWAP_ID).unwrap(), + }, + }; + let args = parse_args(raw_ars).unwrap(); + assert_eq!(expected_args, args); + } + + #[test] + fn ensure_command_mapping_for_testnet() { + let default_testnet_conf_path = env::Testnet::getConfigFileDefaults().unwrap().config_path; + let testnet_env_config = env::Testnet::get_config(); + + let raw_ars = vec![BINARY_NAME, "--testnet", "start"]; + let expected_args = Arguments { + testnet: true, + json: false, + config_path: default_testnet_conf_path.clone(), + env_config: testnet_env_config, + cmd: Command::Start { resume_only: false }, + }; + let args = parse_args(raw_ars).unwrap(); + assert_eq!(expected_args, args); + + let raw_ars = vec![BINARY_NAME, "--testnet", "history"]; + let expected_args = Arguments { + testnet: true, + json: false, + config_path: default_testnet_conf_path.clone(), + env_config: testnet_env_config, + cmd: Command::History, + }; + let args = parse_args(raw_ars).unwrap(); + assert_eq!(expected_args, args); + + let raw_ars = vec![BINARY_NAME, "--testnet", "balance"]; + let expected_args = Arguments { + testnet: true, + json: false, + config_path: default_testnet_conf_path.clone(), + env_config: testnet_env_config, + cmd: Command::Balance, + }; + let args = parse_args(raw_ars).unwrap(); + assert_eq!(expected_args, args); + + let raw_ars = vec![ + BINARY_NAME, + "--testnet", + "withdraw-btc", + "--address", + BITCOIN_TESTNET_ADDRESS, + ]; + let expected_args = Arguments { + testnet: true, + json: false, + config_path: default_testnet_conf_path.clone(), + env_config: testnet_env_config, + cmd: Command::WithdrawBtc { + amount: None, + address: Address::from_str(BITCOIN_TESTNET_ADDRESS).unwrap(), + }, + }; + let args = parse_args(raw_ars).unwrap(); + assert_eq!(expected_args, args); + + let raw_ars = vec![ + BINARY_NAME, + "--testnet", + "manual-recovery", + "cancel", + "--swap-id", + SWAP_ID, + ]; + let expected_args = Arguments { + testnet: true, + json: false, + config_path: default_testnet_conf_path.clone(), + env_config: testnet_env_config, + cmd: Command::Cancel { + swap_id: Uuid::parse_str(SWAP_ID).unwrap(), + force: false, + }, + }; + let args = parse_args(raw_ars).unwrap(); + assert_eq!(expected_args, args); + + let raw_ars = vec![ + BINARY_NAME, + "--testnet", + "manual-recovery", + "refund", + "--swap-id", + SWAP_ID, + ]; + let expected_args = Arguments { + testnet: true, + json: false, + config_path: default_testnet_conf_path.clone(), + env_config: testnet_env_config, + cmd: Command::Refund { + swap_id: Uuid::parse_str(SWAP_ID).unwrap(), + force: false, + }, + }; + let args = parse_args(raw_ars).unwrap(); + assert_eq!(expected_args, args); + + let raw_ars = vec![ + BINARY_NAME, + "--testnet", + "manual-recovery", + "punish", + "--swap-id", + SWAP_ID, + ]; + let expected_args = Arguments { + testnet: true, + json: false, + config_path: default_testnet_conf_path.clone(), + env_config: testnet_env_config, + cmd: Command::Punish { + swap_id: Uuid::parse_str(SWAP_ID).unwrap(), + force: false, + }, + }; + let args = parse_args(raw_ars).unwrap(); + assert_eq!(expected_args, args); + + let raw_ars = vec![ + BINARY_NAME, + "--testnet", + "manual-recovery", + "safely-abort", + "--swap-id", + SWAP_ID, + ]; + let expected_args = Arguments { + testnet: true, + json: false, + config_path: default_testnet_conf_path, + env_config: testnet_env_config, + cmd: Command::SafelyAbort { + swap_id: Uuid::parse_str(SWAP_ID).unwrap(), + }, + }; + let args = parse_args(raw_ars).unwrap(); + assert_eq!(expected_args, args); + } + + #[test] + fn given_user_provides_config_path_then_no_default_config_path_returned() { + let cp = PathBuf::from_str("/some/config/path").unwrap(); + + let expected = config_path(Some(cp.clone()), true).unwrap(); + assert_eq!(expected, cp); + + let expected = config_path(Some(cp.clone()), false).unwrap(); + assert_eq!(expected, cp) + } + + #[test] + fn given_bitcoin_address_network_mismatch_then_error() { + let error = + bitcoin_address(Address::from_str(BITCOIN_MAINNET_ADDRESS).unwrap(), true).unwrap_err(); + + assert_eq!( + error + .downcast_ref::() + .unwrap(), + &BitcoinAddressNetworkMismatch { + expected: bitcoin::Network::Testnet, + actual: bitcoin::Network::Bitcoin + } + ); + + let error = bitcoin_address(Address::from_str(BITCOIN_TESTNET_ADDRESS).unwrap(), false) + .unwrap_err(); + + assert_eq!( + error + .downcast_ref::() + .unwrap(), + &BitcoinAddressNetworkMismatch { + expected: bitcoin::Network::Bitcoin, + actual: bitcoin::Network::Testnet + } + ); + } +} diff --git a/swap/src/bin/asb.rs b/swap/src/bin/asb.rs index 0dde5f46..eea3bcce 100644 --- a/swap/src/bin/asb.rs +++ b/swap/src/bin/asb.rs @@ -17,13 +17,14 @@ use libp2p::core::multiaddr::Protocol; use libp2p::core::Multiaddr; use libp2p::Swarm; use prettytable::{row, Table}; +use std::env; use std::net::{IpAddr, Ipv4Addr, SocketAddr}; use std::sync::Arc; -use structopt::StructOpt; -use swap::asb::command::{Arguments, Command, ManualRecovery, RecoverCommandParams}; +use structopt::clap; +use structopt::clap::ErrorKind; +use swap::asb::command::{parse_args, Arguments, Command}; use swap::asb::config::{ initial_setup, query_user_for_initial_config, read_config, Config, ConfigNotInitialized, - GetDefaults, }; use swap::database::Database; use swap::monero::Amount; @@ -33,7 +34,7 @@ use swap::protocol::alice::event_loop::KrakenRate; use swap::protocol::alice::{redeem, run, EventLoop}; use swap::seed::Seed; use swap::tor::AuthenticatedClient; -use swap::{asb, bitcoin, env, kraken, monero, tor}; +use swap::{asb, bitcoin, kraken, monero, tor}; use tracing::{debug, info, warn}; use tracing_subscriber::filter::LevelFilter; @@ -47,19 +48,29 @@ async fn main() -> Result<()> { let Arguments { testnet, json, - config, + config_path, + env_config, cmd, - } = Arguments::from_args(); - asb::tracing::init(LevelFilter::DEBUG, json).expect("initialize tracing"); - - let config_path = if let Some(config_path) = config { - config_path - } else if testnet { - env::Testnet::getConfigFileDefaults()?.config_path - } else { - env::Mainnet::getConfigFileDefaults()?.config_path + } = match parse_args(env::args_os()) { + Ok(args) => args, + Err(e) => { + if let Some(clap_err) = e.downcast_ref::() { + match clap_err.kind { + ErrorKind::HelpDisplayed | ErrorKind::VersionDisplayed => { + println!("{}", clap_err.message); + std::process::exit(0); + } + _ => { + bail!(e); + } + } + } + bail!(e); + } }; + asb::tracing::init(LevelFilter::DEBUG, json).expect("initialize tracing"); + let config = match read_config(config_path.clone())? { Ok(config) => config, Err(ConfigNotInitialized {}) => { @@ -68,8 +79,6 @@ async fn main() -> Result<()> { } }; - let env_config = env::new(testnet, &config); - if config.monero.network != env_config.monero_network { bail!(format!( "Expected monero network in config file to be {:?} but was {:?}", @@ -228,9 +237,7 @@ async fn main() -> Result<()> { %monero_balance, "Current balance"); } - Command::ManualRecovery(ManualRecovery::Cancel { - cancel_params: RecoverCommandParams { swap_id, force }, - }) => { + Command::Cancel { swap_id, force } => { let bitcoin_wallet = init_bitcoin_wallet(&config, &seed, env_config).await?; let (txid, _) = @@ -238,9 +245,7 @@ async fn main() -> Result<()> { tracing::info!("Cancel transaction successfully published with id {}", txid); } - Command::ManualRecovery(ManualRecovery::Refund { - refund_params: RecoverCommandParams { swap_id, force }, - }) => { + Command::Refund { swap_id, force } => { let bitcoin_wallet = init_bitcoin_wallet(&config, &seed, env_config).await?; let monero_wallet = init_monero_wallet(&config, env_config).await?; @@ -255,9 +260,7 @@ async fn main() -> Result<()> { tracing::info!("Monero successfully refunded"); } - Command::ManualRecovery(ManualRecovery::Punish { - punish_params: RecoverCommandParams { swap_id, force }, - }) => { + Command::Punish { swap_id, force } => { let bitcoin_wallet = init_bitcoin_wallet(&config, &seed, env_config).await?; let (txid, _) = @@ -265,15 +268,16 @@ async fn main() -> Result<()> { tracing::info!("Punish transaction successfully published with id {}", txid); } - Command::ManualRecovery(ManualRecovery::SafelyAbort { swap_id }) => { + Command::SafelyAbort { swap_id } => { alice::safely_abort(swap_id, Arc::new(db)).await?; tracing::info!("Swap safely aborted"); } - Command::ManualRecovery(ManualRecovery::Redeem { - redeem_params: RecoverCommandParams { swap_id, force }, + Command::Redeem { + swap_id, + force, do_not_await_finality, - }) => { + } => { let bitcoin_wallet = init_bitcoin_wallet(&config, &seed, env_config).await?; let (txid, _) = alice::redeem( @@ -287,7 +291,7 @@ async fn main() -> Result<()> { tracing::info!("Redeem transaction successfully published with id {}", txid); } - }; + } Ok(()) } diff --git a/swap/src/bitcoin/wallet.rs b/swap/src/bitcoin/wallet.rs index 6268d64e..c39305f1 100644 --- a/swap/src/bitcoin/wallet.rs +++ b/swap/src/bitcoin/wallet.rs @@ -303,6 +303,10 @@ where address: Address, amount: Amount, ) -> Result { + if self.network != address.network { + bail!("Cannot build PSBT because network of given address is {} but wallet is on network {}", address.network, self.network); + } + let wallet = self.wallet.lock().await; let client = self.client.lock().await; let fee_rate = client.estimate_feerate(self.target_block)?;