From d0db6cba103839e0fdafd031a8333f6124e3e5b0 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Thu, 4 Mar 2021 16:52:29 +1100 Subject: [PATCH 01/11] Favour individual logs over one in main --- swap/src/bin/swap_cli.rs | 5 ----- swap/src/database.rs | 2 ++ swap/src/seed.rs | 2 +- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/swap/src/bin/swap_cli.rs b/swap/src/bin/swap_cli.rs index 02275a54..dd9d88b7 100644 --- a/swap/src/bin/swap_cli.rs +++ b/swap/src/bin/swap_cli.rs @@ -76,11 +76,6 @@ async fn main() -> Result<()> { None => Config::testnet(), }; - debug!( - "Database and seed will be stored in {}", - config.data.dir.display() - ); - let db = Database::open(config.data.dir.join("database").as_path()) .context("Could not open database")?; diff --git a/swap/src/database.rs b/swap/src/database.rs index b591d2c1..e9389e47 100644 --- a/swap/src/database.rs +++ b/swap/src/database.rs @@ -51,6 +51,8 @@ pub struct Database(sled::Db); impl Database { pub fn open(path: &Path) -> Result { + tracing::debug!("Opening database at {}", path.display()); + let db = sled::open(path).with_context(|| format!("Could not open the DB at {:?}", path))?; diff --git a/swap/src/seed.rs b/swap/src/seed.rs index 03413475..397482eb 100644 --- a/swap/src/seed.rs +++ b/swap/src/seed.rs @@ -90,7 +90,7 @@ impl Seed { let contents = fs::read_to_string(file)?; let pem = pem::parse(contents)?; - tracing::trace!("Read in seed from {}", file.display()); + tracing::debug!("Reading in seed from {}", file.display()); Self::from_pem(pem) } From 4642e6c0e3f3c376df9baec277e122c5813c9dee Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Thu, 4 Mar 2021 16:54:26 +1100 Subject: [PATCH 02/11] Simplify arguments to `init_XYZ_wallet` functions This makes the function calls fit onto one line. --- swap/src/bin/swap_cli.rs | 64 ++++++++++++++++++---------------------- 1 file changed, 28 insertions(+), 36 deletions(-) diff --git a/swap/src/bin/swap_cli.rs b/swap/src/bin/swap_cli.rs index dd9d88b7..404c82a6 100644 --- a/swap/src/bin/swap_cli.rs +++ b/swap/src/bin/swap_cli.rs @@ -14,10 +14,8 @@ use anyhow::{bail, Context, Result}; use prettytable::{row, Table}; -use reqwest::Url; use std::cmp::min; use std::future::Future; -use std::path::Path; use std::sync::Arc; use std::time::Duration; use structopt::StructOpt; @@ -79,7 +77,6 @@ async fn main() -> Result<()> { let db = Database::open(config.data.dir.join("database").as_path()) .context("Could not open database")?; - let wallet_data_dir = config.data.dir.join("wallet"); let seed = Seed::from_file_or_generate(&config.data.dir).expect("Could not retrieve/initialize seed"); @@ -88,12 +85,6 @@ async fn main() -> Result<()> { let monero_network = monero::Network::Stagenet; let execution_params = execution_params::Testnet::get_execution_params(); - let monero_wallet_rpc = monero::WalletRpc::new(config.data.dir.join("monero")).await?; - - let monero_wallet_rpc_process = monero_wallet_rpc - .run(monero_network, "monero-stagenet.exan.tech") - .await?; - match args.cmd { Command::BuyXmr { receive_monero_address, @@ -108,10 +99,8 @@ async fn main() -> Result<()> { ) } - let bitcoin_wallet = - init_bitcoin_wallet(config, bitcoin_network, &wallet_data_dir, seed).await?; - let monero_wallet = - init_monero_wallet(monero_network, monero_wallet_rpc_process.endpoint()).await?; + let bitcoin_wallet = init_bitcoin_wallet(bitcoin_network, &config, seed).await?; + let (monero_wallet, _process) = init_monero_wallet(monero_network, &config).await?; let bitcoin_wallet = Arc::new(bitcoin_wallet); let (event_loop, mut event_loop_handle) = EventLoop::new( &seed.derive_libp2p_identity(), @@ -182,10 +171,8 @@ async fn main() -> Result<()> { bail!("The given monero address is on network {:?}, expected address of network {:?}.", receive_monero_address.network, monero_network) } - let bitcoin_wallet = - init_bitcoin_wallet(config, bitcoin_network, &wallet_data_dir, seed).await?; - let monero_wallet = - init_monero_wallet(monero_network, monero_wallet_rpc_process.endpoint()).await?; + let bitcoin_wallet = init_bitcoin_wallet(bitcoin_network, &config, seed).await?; + let (monero_wallet, _process) = init_monero_wallet(monero_network, &config).await?; let bitcoin_wallet = Arc::new(bitcoin_wallet); let (event_loop, event_loop_handle) = EventLoop::new( @@ -218,8 +205,7 @@ async fn main() -> Result<()> { } } Command::Cancel { swap_id, force } => { - let bitcoin_wallet = - init_bitcoin_wallet(config, bitcoin_network, &wallet_data_dir, seed).await?; + let bitcoin_wallet = init_bitcoin_wallet(bitcoin_network, &config, seed).await?; let resume_state = db.get_state(swap_id)?.try_into_bob()?.into(); let cancel = @@ -239,8 +225,7 @@ async fn main() -> Result<()> { } } Command::Refund { swap_id, force } => { - let bitcoin_wallet = - init_bitcoin_wallet(config, bitcoin_network, &wallet_data_dir, seed).await?; + let bitcoin_wallet = init_bitcoin_wallet(bitcoin_network, &config, seed).await?; let resume_state = db.get_state(swap_id)?.try_into_bob()?.into(); @@ -259,34 +244,41 @@ async fn main() -> Result<()> { } async fn init_bitcoin_wallet( - config: Config, - bitcoin_network: bitcoin::Network, - bitcoin_wallet_data_dir: &Path, + network: bitcoin::Network, + config: &Config, seed: Seed, ) -> Result { - let bitcoin_wallet = bitcoin::Wallet::new( - config.bitcoin.electrum_rpc_url, - config.bitcoin.electrum_http_url, - bitcoin_network, - bitcoin_wallet_data_dir, - seed.derive_extended_private_key(bitcoin_network)?, + let wallet_dir = config.data.dir.join("wallet"); + + let wallet = bitcoin::Wallet::new( + config.bitcoin.electrum_rpc_url.clone(), + config.bitcoin.electrum_http_url.clone(), + network, + &wallet_dir, + seed.derive_extended_private_key(network)?, ) .await?; - bitcoin_wallet + wallet .sync_wallet() .await .context("failed to sync balance of bitcoin wallet")?; - Ok(bitcoin_wallet) + Ok(wallet) } async fn init_monero_wallet( monero_network: monero::Network, - monero_wallet_rpc_url: Url, -) -> Result { + config: &Config, +) -> Result<(monero::Wallet, monero::WalletRpcProcess)> { + let monero_wallet_rpc = monero::WalletRpc::new(config.data.dir.join("monero")).await?; + + let monero_wallet_rpc_process = monero_wallet_rpc + .run(monero_network, "monero-stagenet.exan.tech") + .await?; + let monero_wallet = monero::Wallet::new( - monero_wallet_rpc_url.clone(), + monero_wallet_rpc_process.endpoint(), monero_network, MONERO_BLOCKCHAIN_MONITORING_WALLET_NAME.to_string(), ); @@ -298,7 +290,7 @@ async fn init_monero_wallet( .await .context("failed to validate connection to monero-wallet-rpc")?; - Ok(monero_wallet) + Ok((monero_wallet, monero_wallet_rpc_process)) } async fn determine_btc_to_swap( From 87f928f56cb456124576f6e3c2c2b2773d12db1d Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Thu, 4 Mar 2021 16:55:40 +1100 Subject: [PATCH 03/11] Move const to function where it is used --- swap/src/bin/swap_cli.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/swap/src/bin/swap_cli.rs b/swap/src/bin/swap_cli.rs index 404c82a6..9d95e41e 100644 --- a/swap/src/bin/swap_cli.rs +++ b/swap/src/bin/swap_cli.rs @@ -37,8 +37,6 @@ use uuid::Uuid; #[macro_use] extern crate prettytable; -const MONERO_BLOCKCHAIN_MONITORING_WALLET_NAME: &str = "swap-tool-blockchain-monitoring-wallet"; - #[tokio::main] async fn main() -> Result<()> { let args = Arguments::from_args(); @@ -271,6 +269,8 @@ async fn init_monero_wallet( monero_network: monero::Network, config: &Config, ) -> 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_process = monero_wallet_rpc From 5953037b8184fc3de1c849c3abc80cfe4c8e37c4 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Thu, 4 Mar 2021 17:02:03 +1100 Subject: [PATCH 04/11] Don't repeat the module name within the type --- swap/src/bin/swap_cli.rs | 5 ++--- swap/src/protocol/bob/cancel.rs | 8 ++++---- ...sing_cancel_and_refund_command_timelock_not_expired.rs | 4 ++-- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/swap/src/bin/swap_cli.rs b/swap/src/bin/swap_cli.rs index 9d95e41e..ffe8cf51 100644 --- a/swap/src/bin/swap_cli.rs +++ b/swap/src/bin/swap_cli.rs @@ -26,7 +26,6 @@ use swap::database::Database; use swap::execution_params::GetExecutionParams; use swap::network::quote::BidQuote; use swap::protocol::bob; -use swap::protocol::bob::cancel::CancelError; use swap::protocol::bob::{Builder, EventLoop}; use swap::seed::Seed; use swap::{bitcoin, execution_params, monero}; @@ -213,11 +212,11 @@ async fn main() -> Result<()> { Ok((txid, _)) => { debug!("Cancel transaction successfully published with id {}", txid) } - Err(CancelError::CancelTimelockNotExpiredYet) => error!( + Err(bob::cancel::Error::CancelTimelockNotExpiredYet) => error!( "The Cancel Transaction cannot be published yet, \ because the timelock has not expired. Please try again later." ), - Err(CancelError::CancelTxAlreadyPublished) => { + Err(bob::cancel::Error::CancelTxAlreadyPublished) => { warn!("The Cancel Transaction has already been published.") } } diff --git a/swap/src/protocol/bob/cancel.rs b/swap/src/protocol/bob/cancel.rs index c3c70fd1..90209383 100644 --- a/swap/src/protocol/bob/cancel.rs +++ b/swap/src/protocol/bob/cancel.rs @@ -6,7 +6,7 @@ use std::sync::Arc; use uuid::Uuid; #[derive(Debug, thiserror::Error, Clone, Copy)] -pub enum CancelError { +pub enum Error { #[error("The cancel timelock has not expired yet.")] CancelTimelockNotExpiredYet, #[error("The cancel transaction has already been published.")] @@ -19,7 +19,7 @@ pub async fn cancel( bitcoin_wallet: Arc, db: Database, force: bool, -) -> Result> { +) -> Result> { let state4 = match state { BobState::BtcLocked(state3) => state3.cancel(), BobState::XmrLockProofReceived { state, .. } => state.cancel(), @@ -35,7 +35,7 @@ pub async fn cancel( if !force { if let ExpiredTimelocks::None = state4.expired_timelock(bitcoin_wallet.as_ref()).await? { - return Ok(Err(CancelError::CancelTimelockNotExpiredYet)); + return Ok(Err(Error::CancelTimelockNotExpiredYet)); } if state4 @@ -47,7 +47,7 @@ pub async fn cancel( let db_state = state.into(); db.insert_latest_state(swap_id, Swap::Bob(db_state)).await?; - return Ok(Err(CancelError::CancelTxAlreadyPublished)); + return Ok(Err(Error::CancelTxAlreadyPublished)); } } diff --git a/swap/tests/bob_refunds_using_cancel_and_refund_command_timelock_not_expired.rs b/swap/tests/bob_refunds_using_cancel_and_refund_command_timelock_not_expired.rs index f2d03608..d5f23aad 100644 --- a/swap/tests/bob_refunds_using_cancel_and_refund_command_timelock_not_expired.rs +++ b/swap/tests/bob_refunds_using_cancel_and_refund_command_timelock_not_expired.rs @@ -1,6 +1,6 @@ pub mod testutils; -use bob::cancel::CancelError; +use bob::cancel::Error; use swap::protocol::bob; use swap::protocol::bob::BobState; use testutils::bob_run_until::is_btc_locked; @@ -30,7 +30,7 @@ async fn given_bob_manually_cancels_when_timelock_not_expired_errors() { .err() .unwrap(); - assert!(matches!(result, CancelError::CancelTimelockNotExpiredYet)); + assert!(matches!(result, Error::CancelTimelockNotExpiredYet)); let (bob_swap, bob_join_handle) = ctx.stop_and_resume_bob_from_db(bob_join_handle).await; assert!(matches!(bob_swap.state, BobState::BtcLocked { .. })); From 4f6626988763c0c7c75122c8473b5541962e0c18 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Thu, 4 Mar 2021 17:05:00 +1100 Subject: [PATCH 05/11] Move error message on sync _into_ the function The bitcoin::Wallet::sync_wallet function doesn't do anything else other than delegating. As such, we have just as much information about what went wrong inside this function as we have outside. By moving the .context call into the function, we can avoid repeating us on every call-site. --- swap/src/bin/asb.rs | 5 +---- swap/src/bin/swap_cli.rs | 5 +---- swap/src/bitcoin/wallet.rs | 7 ++++++- swap/tests/testutils/mod.rs | 35 +++++++---------------------------- 4 files changed, 15 insertions(+), 37 deletions(-) diff --git a/swap/src/bin/asb.rs b/swap/src/bin/asb.rs index 94f144eb..01e72c69 100644 --- a/swap/src/bin/asb.rs +++ b/swap/src/bin/asb.rs @@ -141,10 +141,7 @@ async fn init_wallets( ) .await?; - bitcoin_wallet - .sync_wallet() - .await - .expect("Could not sync btc wallet"); + bitcoin_wallet.sync_wallet().await?; let bitcoin_balance = bitcoin_wallet.balance().await?; info!( diff --git a/swap/src/bin/swap_cli.rs b/swap/src/bin/swap_cli.rs index ffe8cf51..d80a9a7d 100644 --- a/swap/src/bin/swap_cli.rs +++ b/swap/src/bin/swap_cli.rs @@ -256,10 +256,7 @@ async fn init_bitcoin_wallet( ) .await?; - wallet - .sync_wallet() - .await - .context("failed to sync balance of bitcoin wallet")?; + wallet.sync_wallet().await?; Ok(wallet) } diff --git a/swap/src/bitcoin/wallet.rs b/swap/src/bitcoin/wallet.rs index 8fecd22f..1f470e63 100644 --- a/swap/src/bitcoin/wallet.rs +++ b/swap/src/bitcoin/wallet.rs @@ -106,7 +106,12 @@ impl Wallet { } pub async fn sync_wallet(&self) -> Result<()> { - self.inner.lock().await.sync(noop_progress(), None)?; + self.inner + .lock() + .await + .sync(noop_progress(), None) + .context("failed to sync balance of Bitcoin wallet")?; + Ok(()) } diff --git a/swap/tests/testutils/mod.rs b/swap/tests/testutils/mod.rs index c1b3d3bc..1dcaeabb 100644 --- a/swap/tests/testutils/mod.rs +++ b/swap/tests/testutils/mod.rs @@ -151,10 +151,7 @@ impl TestContext { assert!(matches!(state, AliceState::BtcRedeemed)); - self.alice_bitcoin_wallet - .sync_wallet() - .await - .expect("Could not sync wallet"); + self.alice_bitcoin_wallet.sync_wallet().await.unwrap(); let btc_balance_after_swap = self.alice_bitcoin_wallet.as_ref().balance().await.unwrap(); assert_eq!( @@ -184,10 +181,7 @@ impl TestContext { assert!(matches!(state, AliceState::XmrRefunded)); - self.alice_bitcoin_wallet - .sync_wallet() - .await - .expect("Could not sync wallet"); + self.alice_bitcoin_wallet.sync_wallet().await.unwrap(); let btc_balance_after_swap = self.alice_bitcoin_wallet.as_ref().balance().await.unwrap(); assert_eq!(btc_balance_after_swap, self.alice_starting_balances.btc); @@ -206,10 +200,7 @@ impl TestContext { pub async fn assert_alice_punished(&self, state: AliceState) { assert!(matches!(state, AliceState::BtcPunished)); - self.alice_bitcoin_wallet - .sync_wallet() - .await - .expect("Could not sync wallet"); + self.alice_bitcoin_wallet.sync_wallet().await.unwrap(); let btc_balance_after_swap = self.alice_bitcoin_wallet.as_ref().balance().await.unwrap(); assert_eq!( @@ -228,10 +219,7 @@ impl TestContext { } pub async fn assert_bob_redeemed(&self, state: BobState) { - self.bob_bitcoin_wallet - .sync_wallet() - .await - .expect("Could not sync wallet"); + self.bob_bitcoin_wallet.sync_wallet().await.unwrap(); let lock_tx_id = if let BobState::XmrRedeemed { tx_lock_id } = state { tx_lock_id @@ -263,10 +251,7 @@ impl TestContext { } pub async fn assert_bob_refunded(&self, state: BobState) { - self.bob_bitcoin_wallet - .sync_wallet() - .await - .expect("Could not sync wallet"); + self.bob_bitcoin_wallet.sync_wallet().await.unwrap(); let lock_tx_id = if let BobState::BtcRefunded(state4) = state { state4.tx_lock_id() @@ -300,10 +285,7 @@ impl TestContext { } pub async fn assert_bob_punished(&self, state: BobState) { - self.bob_bitcoin_wallet - .sync_wallet() - .await - .expect("Could not sync wallet"); + self.bob_bitcoin_wallet.sync_wallet().await.unwrap(); let lock_tx_id = if let BobState::BtcPunished { tx_lock_id } = state { tx_lock_id @@ -654,10 +636,7 @@ async fn init_test_wallets( let max_retries = 30u8; loop { retries += 1; - btc_wallet - .sync_wallet() - .await - .expect("Could not sync btc wallet"); + btc_wallet.sync_wallet().await.unwrap(); let btc_balance = btc_wallet.balance().await.unwrap(); From 37f97ac4715713d657a7944240c02ea955643c81 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Thu, 4 Mar 2021 17:07:02 +1100 Subject: [PATCH 06/11] Shorten function name The variable will always be at least called `wallet`, hence we can omit the `_wallet` postfix from the function name. --- swap/src/bin/asb.rs | 2 +- swap/src/bin/swap_cli.rs | 4 ++-- swap/src/bitcoin/wallet.rs | 2 +- swap/tests/testutils/mod.rs | 14 +++++++------- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/swap/src/bin/asb.rs b/swap/src/bin/asb.rs index 01e72c69..df0b7676 100644 --- a/swap/src/bin/asb.rs +++ b/swap/src/bin/asb.rs @@ -141,7 +141,7 @@ async fn init_wallets( ) .await?; - bitcoin_wallet.sync_wallet().await?; + bitcoin_wallet.sync().await?; let bitcoin_balance = bitcoin_wallet.balance().await?; info!( diff --git a/swap/src/bin/swap_cli.rs b/swap/src/bin/swap_cli.rs index d80a9a7d..e02f69a3 100644 --- a/swap/src/bin/swap_cli.rs +++ b/swap/src/bin/swap_cli.rs @@ -113,7 +113,7 @@ async fn main() -> Result<()> { bitcoin_wallet.new_address(), async { while bitcoin_wallet.balance().await? == Amount::ZERO { - bitcoin_wallet.sync_wallet().await?; + bitcoin_wallet.sync().await?; tokio::time::sleep(Duration::from_secs(1)).await; } @@ -256,7 +256,7 @@ async fn init_bitcoin_wallet( ) .await?; - wallet.sync_wallet().await?; + wallet.sync().await?; Ok(wallet) } diff --git a/swap/src/bitcoin/wallet.rs b/swap/src/bitcoin/wallet.rs index 1f470e63..25f2e5d7 100644 --- a/swap/src/bitcoin/wallet.rs +++ b/swap/src/bitcoin/wallet.rs @@ -105,7 +105,7 @@ impl Wallet { Ok(Amount::from_sat(fees)) } - pub async fn sync_wallet(&self) -> Result<()> { + pub async fn sync(&self) -> Result<()> { self.inner .lock() .await diff --git a/swap/tests/testutils/mod.rs b/swap/tests/testutils/mod.rs index 1dcaeabb..7b15d44f 100644 --- a/swap/tests/testutils/mod.rs +++ b/swap/tests/testutils/mod.rs @@ -151,7 +151,7 @@ impl TestContext { assert!(matches!(state, AliceState::BtcRedeemed)); - self.alice_bitcoin_wallet.sync_wallet().await.unwrap(); + self.alice_bitcoin_wallet.sync().await.unwrap(); let btc_balance_after_swap = self.alice_bitcoin_wallet.as_ref().balance().await.unwrap(); assert_eq!( @@ -181,7 +181,7 @@ impl TestContext { assert!(matches!(state, AliceState::XmrRefunded)); - self.alice_bitcoin_wallet.sync_wallet().await.unwrap(); + self.alice_bitcoin_wallet.sync().await.unwrap(); let btc_balance_after_swap = self.alice_bitcoin_wallet.as_ref().balance().await.unwrap(); assert_eq!(btc_balance_after_swap, self.alice_starting_balances.btc); @@ -200,7 +200,7 @@ impl TestContext { pub async fn assert_alice_punished(&self, state: AliceState) { assert!(matches!(state, AliceState::BtcPunished)); - self.alice_bitcoin_wallet.sync_wallet().await.unwrap(); + self.alice_bitcoin_wallet.sync().await.unwrap(); let btc_balance_after_swap = self.alice_bitcoin_wallet.as_ref().balance().await.unwrap(); assert_eq!( @@ -219,7 +219,7 @@ impl TestContext { } pub async fn assert_bob_redeemed(&self, state: BobState) { - self.bob_bitcoin_wallet.sync_wallet().await.unwrap(); + self.bob_bitcoin_wallet.sync().await.unwrap(); let lock_tx_id = if let BobState::XmrRedeemed { tx_lock_id } = state { tx_lock_id @@ -251,7 +251,7 @@ impl TestContext { } pub async fn assert_bob_refunded(&self, state: BobState) { - self.bob_bitcoin_wallet.sync_wallet().await.unwrap(); + self.bob_bitcoin_wallet.sync().await.unwrap(); let lock_tx_id = if let BobState::BtcRefunded(state4) = state { state4.tx_lock_id() @@ -285,7 +285,7 @@ impl TestContext { } pub async fn assert_bob_punished(&self, state: BobState) { - self.bob_bitcoin_wallet.sync_wallet().await.unwrap(); + self.bob_bitcoin_wallet.sync().await.unwrap(); let lock_tx_id = if let BobState::BtcPunished { tx_lock_id } = state { tx_lock_id @@ -636,7 +636,7 @@ async fn init_test_wallets( let max_retries = 30u8; loop { retries += 1; - btc_wallet.sync_wallet().await.unwrap(); + btc_wallet.sync().await.unwrap(); let btc_balance = btc_wallet.balance().await.unwrap(); From 5c24a4629880a26234435d7c77666d59f4fc6f08 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Thu, 4 Mar 2021 17:16:18 +1100 Subject: [PATCH 07/11] Improve error message if stuff fails directly in main --- swap/src/bin/swap_cli.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/swap/src/bin/swap_cli.rs b/swap/src/bin/swap_cli.rs index e02f69a3..cf7da039 100644 --- a/swap/src/bin/swap_cli.rs +++ b/swap/src/bin/swap_cli.rs @@ -72,10 +72,10 @@ async fn main() -> Result<()> { }; let db = Database::open(config.data.dir.join("database").as_path()) - .context("Could not open database")?; + .context("Failed to open database")?; let seed = - Seed::from_file_or_generate(&config.data.dir).expect("Could not retrieve/initialize seed"); + Seed::from_file_or_generate(&config.data.dir).context("Failed to read in seed file")?; // hardcode to testnet/stagenet let bitcoin_network = bitcoin::Network::Testnet; @@ -254,7 +254,8 @@ async fn init_bitcoin_wallet( &wallet_dir, seed.derive_extended_private_key(network)?, ) - .await?; + .await + .context("Failed to initialize Bitcoin wallet")?; wallet.sync().await?; From 816e8b9b9665b0c8b86924ae3ce5a056ec6a7c2f Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Thu, 4 Mar 2021 17:22:59 +1100 Subject: [PATCH 08/11] Add more context to fallible functions inside bitcoin::Wallet --- swap/src/bitcoin/wallet.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/swap/src/bitcoin/wallet.rs b/swap/src/bitcoin/wallet.rs index 25f2e5d7..44996702 100644 --- a/swap/src/bitcoin/wallet.rs +++ b/swap/src/bitcoin/wallet.rs @@ -74,12 +74,13 @@ impl Wallet { } pub async fn balance(&self) -> Result { - let balance = self.inner.lock().await.get_balance()?; + let balance = self.inner.lock().await.get_balance().context("Failed to calculate Bitcoin balance")?; + Ok(Amount::from_sat(balance)) } pub async fn new_address(&self) -> Result
{ - let address = self.inner.lock().await.get_new_address()?; + let address = self.inner.lock().await.get_new_address().context("Failed to get new Bitcoin address")?; Ok(address) } From 4138039ea086c9b8de233e764ba787f7738e74b4 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Thu, 4 Mar 2021 17:25:05 +1100 Subject: [PATCH 09/11] Make sure all error messages start with an uppercase letter These might potentially be shown to a user, let's make them all consistent. --- swap/src/asb/config.rs | 2 +- swap/src/asb/rate.rs | 10 ++++----- swap/src/bin/swap_cli.rs | 4 ++-- swap/src/bitcoin.rs | 4 ++-- swap/src/bitcoin/redeem.rs | 2 +- swap/src/bitcoin/refund.rs | 2 +- swap/src/bitcoin/wallet.rs | 24 +++++++++++++++------- swap/src/cli/config.rs | 2 +- swap/src/database.rs | 6 +++--- swap/src/monero/wallet_rpc.rs | 2 +- swap/src/protocol/alice/behaviour.rs | 4 ++-- swap/src/protocol/alice/execution_setup.rs | 10 ++++----- swap/src/protocol/alice/steps.rs | 2 +- swap/src/protocol/bob/event_loop.rs | 2 +- swap/src/protocol/bob/execution_setup.rs | 10 ++++----- swap/src/protocol/bob/state.rs | 2 +- 16 files changed, 49 insertions(+), 39 deletions(-) diff --git a/swap/src/asb/config.rs b/swap/src/asb/config.rs index 2b411c21..9f764041 100644 --- a/swap/src/asb/config.rs +++ b/swap/src/asb/config.rs @@ -77,7 +77,7 @@ pub fn read_config(config_path: PathBuf) -> Result Result { debug!("Requesting quote"); - let bid_quote = request_quote.await.context("failed to request quote")?; + let bid_quote = request_quote.await.context("Failed to request quote")?; info!("Received quote: 1 XMR ~ {}", bid_quote.price); diff --git a/swap/src/bitcoin.rs b/swap/src/bitcoin.rs index 386c0e57..03966199 100644 --- a/swap/src/bitcoin.rs +++ b/swap/src/bitcoin.rs @@ -23,7 +23,7 @@ pub use wallet::Wallet; use ::bitcoin::hashes::hex::ToHex; use ::bitcoin::hashes::Hash; use ::bitcoin::{secp256k1, SigHash}; -use anyhow::{anyhow, bail, Result}; +use anyhow::{bail, Context, Result}; use ecdsa_fun::adaptor::{Adaptor, HashTranscript}; use ecdsa_fun::fun::Point; use ecdsa_fun::nonce::Deterministic; @@ -204,7 +204,7 @@ pub fn recover(S: PublicKey, sig: Signature, encsig: EncryptedSignature) -> Resu let s = adaptor .recover_decryption_key(&S.0, &sig, &encsig) .map(SecretKey::from) - .ok_or_else(|| anyhow!("secret recovery failure"))?; + .context("Failed to recover secret from adaptor signature")?; Ok(s) } diff --git a/swap/src/bitcoin/redeem.rs b/swap/src/bitcoin/redeem.rs index 5171db16..bc670da4 100644 --- a/swap/src/bitcoin/redeem.rs +++ b/swap/src/bitcoin/redeem.rs @@ -107,7 +107,7 @@ impl TxRedeem { let sig = sigs .into_iter() .find(|sig| verify_sig(&B, &self.digest(), &sig).is_ok()) - .context("neither signature on witness stack verifies against B")?; + .context("Neither signature on witness stack verifies against B")?; Ok(sig) } diff --git a/swap/src/bitcoin/refund.rs b/swap/src/bitcoin/refund.rs index 10643614..5257d859 100644 --- a/swap/src/bitcoin/refund.rs +++ b/swap/src/bitcoin/refund.rs @@ -105,7 +105,7 @@ impl TxRefund { let sig = sigs .into_iter() .find(|sig| verify_sig(&B, &self.digest(), &sig).is_ok()) - .context("neither signature on witness stack verifies against B")?; + .context("Neither signature on witness stack verifies against B")?; Ok(sig) } diff --git a/swap/src/bitcoin/wallet.rs b/swap/src/bitcoin/wallet.rs index 44996702..622c871a 100644 --- a/swap/src/bitcoin/wallet.rs +++ b/swap/src/bitcoin/wallet.rs @@ -74,13 +74,23 @@ impl Wallet { } pub async fn balance(&self) -> Result { - let balance = self.inner.lock().await.get_balance().context("Failed to calculate Bitcoin balance")?; + let balance = self + .inner + .lock() + .await + .get_balance() + .context("Failed to calculate Bitcoin balance")?; Ok(Amount::from_sat(balance)) } pub async fn new_address(&self) -> Result
{ - let address = self.inner.lock().await.get_new_address().context("Failed to get new Bitcoin address")?; + let address = self + .inner + .lock() + .await + .get_new_address() + .context("Failed to get new Bitcoin address")?; Ok(address) } @@ -111,7 +121,7 @@ impl Wallet { .lock() .await .sync(noop_progress(), None) - .context("failed to sync balance of Bitcoin wallet")?; + .context("Failed to sync balance of Bitcoin wallet")?; Ok(()) } @@ -166,7 +176,7 @@ impl Wallet { .await .broadcast(transaction) .with_context(|| { - format!("failed to broadcast Bitcoin {} transaction {}", kind, txid) + format!("Failed to broadcast Bitcoin {} transaction {}", kind, txid) })?; tracing::info!("Published Bitcoin {} transaction as {}", txid, kind); @@ -209,7 +219,7 @@ impl Wallet { Result::<_, backoff::Error>::Ok(tx) }) .await - .context("transient errors to be retried")?; + .context("Transient errors should be retried")?; Ok(tx) } @@ -230,7 +240,7 @@ impl Wallet { Result::<_, backoff::Error>::Ok(height) }) .await - .context("transient errors to be retried")?; + .context("Transient errors should be retried")?; Ok(BlockHeight::new(height)) } @@ -261,7 +271,7 @@ impl Wallet { Result::<_, backoff::Error>::Ok(block_height) }) .await - .context("transient errors to be retried")?; + .context("Transient errors should be retried")?; Ok(BlockHeight::new(height)) } diff --git a/swap/src/cli/config.rs b/swap/src/cli/config.rs index 4872ecf5..0c49b1a3 100644 --- a/swap/src/cli/config.rs +++ b/swap/src/cli/config.rs @@ -73,7 +73,7 @@ pub fn read_config(config_path: PathBuf) -> Result { let swap_id = deserialize::(&key); - let swap = deserialize::(&value).context("failed to deserialize swap"); + let swap = deserialize::(&value).context("Failed to deserialize swap"); match (swap_id, swap) { (Ok(swap_id), Ok(swap)) => Ok((swap_id, swap)), (Ok(_), Err(err)) => Err(err), - _ => bail!("failed to deserialize swap"), + _ => bail!("Failed to deserialize swap"), } } - Err(err) => Err(err).context("failed to retrieve swap from DB"), + Err(err) => Err(err).context("Failed to retrieve swap from DB"), }) .collect() } diff --git a/swap/src/monero/wallet_rpc.rs b/swap/src/monero/wallet_rpc.rs index 925b9d95..5abb8f90 100644 --- a/swap/src/monero/wallet_rpc.rs +++ b/swap/src/monero/wallet_rpc.rs @@ -80,7 +80,7 @@ impl WalletRpc { let content_length = response.headers()[CONTENT_LENGTH] .to_str() - .context("failed to convert content-length to string")? + .context("Failed to convert content-length to string")? .parse::()?; tracing::info!( diff --git a/swap/src/protocol/alice/behaviour.rs b/swap/src/protocol/alice/behaviour.rs index bb6adf87..c3790392 100644 --- a/swap/src/protocol/alice/behaviour.rs +++ b/swap/src/protocol/alice/behaviour.rs @@ -184,7 +184,7 @@ impl Behaviour { ) -> Result<()> { self.quote .send_response(channel, response) - .map_err(|_| anyhow!("failed to respond with quote"))?; + .map_err(|_| anyhow!("Failed to respond with quote"))?; Ok(()) } @@ -196,7 +196,7 @@ impl Behaviour { ) -> Result<()> { self.spot_price .send_response(channel, response) - .map_err(|_| anyhow!("failed to respond with spot price"))?; + .map_err(|_| anyhow!("Failed to respond with spot price"))?; Ok(()) } diff --git a/swap/src/protocol/alice/execution_setup.rs b/swap/src/protocol/alice/execution_setup.rs index 1cc7fa76..d5c50e38 100644 --- a/swap/src/protocol/alice/execution_setup.rs +++ b/swap/src/protocol/alice/execution_setup.rs @@ -65,31 +65,31 @@ impl Behaviour { .do_protocol_listener(bob, move |mut substream| async move { let message0 = serde_cbor::from_slice::(&substream.read_message(BUF_SIZE).await?) - .context("failed to deserialize message0")?; + .context("Failed to deserialize message0")?; let state1 = state0.receive(message0)?; substream .write_message( &serde_cbor::to_vec(&state1.next_message()) - .context("failed to serialize message1")?, + .context("Failed to serialize message1")?, ) .await?; let message2 = serde_cbor::from_slice::(&substream.read_message(BUF_SIZE).await?) - .context("failed to deserialize message2")?; + .context("Failed to deserialize message2")?; let state2 = state1.receive(message2); substream .write_message( &serde_cbor::to_vec(&state2.next_message()) - .context("failed to serialize message3")?, + .context("Failed to serialize message3")?, ) .await?; let message4 = serde_cbor::from_slice::(&substream.read_message(BUF_SIZE).await?) - .context("failed to deserialize message4")?; + .context("Failed to deserialize message4")?; let state3 = state2.receive(message4)?; Ok((bob, state3)) diff --git a/swap/src/protocol/alice/steps.rs b/swap/src/protocol/alice/steps.rs index 3e8558b8..a0e619c8 100644 --- a/swap/src/protocol/alice/steps.rs +++ b/swap/src/protocol/alice/steps.rs @@ -106,7 +106,7 @@ pub fn build_bitcoin_redeem_transaction( let tx = tx_redeem .add_signatures((a.public(), sig_a), (B, sig_b)) - .context("sig_{a,b} are invalid for tx_redeem")?; + .context("Failed to sign Bitcoin redeem transaction")?; Ok(tx) } diff --git a/swap/src/protocol/bob/event_loop.rs b/swap/src/protocol/bob/event_loop.rs index a8609936..1cc1b3b9 100644 --- a/swap/src/protocol/bob/event_loop.rs +++ b/swap/src/protocol/bob/event_loop.rs @@ -236,7 +236,7 @@ impl EventLoop { let _ = self.conn_established.send(peer_id).await; } else { debug!("Dialing alice at {}", peer_id); - libp2p::Swarm::dial(&mut self.swarm, &peer_id).context("failed to dial alice")?; + libp2p::Swarm::dial(&mut self.swarm, &peer_id).context("Failed to dial alice")?; } } }, diff --git a/swap/src/protocol/bob/execution_setup.rs b/swap/src/protocol/bob/execution_setup.rs index 1443a040..f2106737 100644 --- a/swap/src/protocol/bob/execution_setup.rs +++ b/swap/src/protocol/bob/execution_setup.rs @@ -73,31 +73,31 @@ impl Behaviour { substream .write_message( &serde_cbor::to_vec(&state0.next_message()) - .context("failed to serialize message0")?, + .context("Failed to serialize message0")?, ) .await?; let message1 = serde_cbor::from_slice::(&substream.read_message(BUF_SIZE).await?) - .context("failed to deserialize message1")?; + .context("Failed to deserialize message1")?; let state1 = state0.receive(bitcoin_wallet.as_ref(), message1).await?; substream .write_message( &serde_cbor::to_vec(&state1.next_message()) - .context("failed to serialize message2")?, + .context("Failed to serialize message2")?, ) .await?; let message3 = serde_cbor::from_slice::(&substream.read_message(BUF_SIZE).await?) - .context("failed to deserialize message3")?; + .context("Failed to deserialize message3")?; let state2 = state1.receive(message3)?; substream .write_message( &serde_cbor::to_vec(&state2.next_message()) - .context("failed to serialize message4")?, + .context("Failed to serialize message4")?, ) .await?; diff --git a/swap/src/protocol/bob/state.rs b/swap/src/protocol/bob/state.rs index 67cf0bc6..1588df50 100644 --- a/swap/src/protocol/bob/state.rs +++ b/swap/src/protocol/bob/state.rs @@ -266,7 +266,7 @@ impl State2 { let signed_tx = bitcoin_wallet .sign_and_finalize(self.tx_lock.clone().into()) .await - .context("failed to sign Bitcoin lock transaction")?; + .context("Failed to sign Bitcoin lock transaction")?; let _ = bitcoin_wallet.broadcast(signed_tx, "lock").await?; From 1aa6d177bf1649e9e4353709259a04ee6796c007 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Thu, 4 Mar 2021 17:39:17 +1100 Subject: [PATCH 10/11] Improve error messages when determining BTC amount to be swapped --- swap/src/bin/swap_cli.rs | 8 ++++++-- swap/src/bitcoin/wallet.rs | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/swap/src/bin/swap_cli.rs b/swap/src/bin/swap_cli.rs index 8152a089..fffcee53 100644 --- a/swap/src/bin/swap_cli.rs +++ b/swap/src/bin/swap_cli.rs @@ -313,14 +313,18 @@ async fn determine_btc_to_swap( bid_quote.max_quantity ); - let new_balance = wait_for_deposit.await?; + let new_balance = wait_for_deposit + .await + .context("Failed to wait for Bitcoin deposit")?; info!("Received {}", new_balance); } else { info!("Found {} in wallet", initial_balance); } - let max_giveable = max_giveable.await?; + let max_giveable = max_giveable + .await + .context("Failed to compute max 'giveable' Bitcoin amount")?; let max_accepted = bid_quote.max_quantity; if max_giveable > max_accepted { diff --git a/swap/src/bitcoin/wallet.rs b/swap/src/bitcoin/wallet.rs index 622c871a..7bc612c4 100644 --- a/swap/src/bitcoin/wallet.rs +++ b/swap/src/bitcoin/wallet.rs @@ -155,7 +155,7 @@ impl Wallet { tx_builder.set_single_recipient(dummy_script); tx_builder.drain_wallet(); tx_builder.fee_rate(self.select_feerate()); - let (_, details) = tx_builder.finish()?; + let (_, details) = tx_builder.finish().context("Failed to build transaction")?; let max_giveable = details.sent - details.fees; From c826a28911b8d1e15a5b376a1a5229703e7339be Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Thu, 4 Mar 2021 17:40:51 +1100 Subject: [PATCH 11/11] Add context if we fail to compute extended private key --- swap/src/seed.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/swap/src/seed.rs b/swap/src/seed.rs index 397482eb..7432adc5 100644 --- a/swap/src/seed.rs +++ b/swap/src/seed.rs @@ -1,7 +1,7 @@ use crate::fs::ensure_directory_exists; use ::bitcoin::secp256k1::constants::SECRET_KEY_SIZE; use ::bitcoin::secp256k1::{self, SecretKey}; -use anyhow::Result; +use anyhow::{Context, Result}; use bdk::bitcoin::util::bip32::ExtendedPrivKey; use bitcoin::hashes::{sha256, Hash, HashEngine}; use libp2p::identity; @@ -34,7 +34,8 @@ impl Seed { network: bitcoin::Network, ) -> Result { let seed = self.derive(b"BITCOIN_EXTENDED_PRIVATE_KEY").bytes(); - let private_key = ExtendedPrivKey::new_master(network, &seed)?; + let private_key = ExtendedPrivKey::new_master(network, &seed) + .context("Failed to create new master extended private key")?; Ok(private_key) }