From 82d75588baddcb515ca989930aa023ed1fda5a2e Mon Sep 17 00:00:00 2001 From: rishflab Date: Wed, 10 Feb 2021 15:18:45 +1100 Subject: [PATCH] Remove automatic bdk wallet sync calls The bdk wallet was being synced before executing other calls such as getting the balance. This is poor usage of the sync functionality as it potentially long running blocking call. Instead sync is called once after the wallet is initialised. --- swap/src/bitcoin/wallet.rs | 6 +--- swap/src/main.rs | 6 ++++ swap/tests/testutils/mod.rs | 63 ++++++++++++++++++++++++++++--------- 3 files changed, 56 insertions(+), 19 deletions(-) diff --git a/swap/src/bitcoin/wallet.rs b/swap/src/bitcoin/wallet.rs index 7873b7ac..b7eaa779 100644 --- a/swap/src/bitcoin/wallet.rs +++ b/swap/src/bitcoin/wallet.rs @@ -62,7 +62,6 @@ impl Wallet { } pub async fn balance(&self) -> Result { - self.sync_wallet().await?; let balance = self.inner.lock().await.get_balance()?; Ok(Amount::from_sat(balance)) } @@ -81,7 +80,6 @@ impl Wallet { } pub async fn transaction_fee(&self, txid: Txid) -> Result { - self.sync_wallet().await?; let fees = self .inner .lock() @@ -111,8 +109,8 @@ impl BuildTxLockPsbt for Wallet { output_address: Address, output_amount: Amount, ) -> Result { - self.sync_wallet().await?; tracing::debug!("building tx lock"); + self.sync_wallet().await?; let (psbt, _details) = self.inner.lock().await.create_tx( bdk::TxBuilder::with_recipients(vec![( output_address.script_pubkey(), @@ -129,7 +127,6 @@ impl BuildTxLockPsbt for Wallet { #[async_trait] impl SignTxLock for Wallet { async fn sign_tx_lock(&self, tx_lock: TxLock) -> Result { - self.sync_wallet().await?; tracing::debug!("signing tx lock"); let psbt = PartiallySignedTransaction::from(tx_lock); let (signed_psbt, finalized) = self.inner.lock().await.sign(psbt, None)?; @@ -264,7 +261,6 @@ impl WaitForTransactionFinality for Wallet { let mut interval = interval(execution_params.bitcoin_avg_block_time / 4); loop { - tracing::debug!("syncing wallet"); let tx_block_height = self.transaction_block_height(txid).await; let block_height = self.get_block_height().await; let confirmations = block_height - tx_block_height; diff --git a/swap/src/main.rs b/swap/src/main.rs index e81790a8..39fc3e42 100644 --- a/swap/src/main.rs +++ b/swap/src/main.rs @@ -356,6 +356,12 @@ async fn init_wallets( bitcoin_wallet_data_dir, ) .await?; + + bitcoin_wallet + .sync_wallet() + .await + .expect("Could not sync btc wallet"); + let bitcoin_balance = bitcoin_wallet.balance().await?; info!( "Connection to Bitcoin wallet succeeded, balance: {}", diff --git a/swap/tests/testutils/mod.rs b/swap/tests/testutils/mod.rs index 245b25b6..d61d0f2d 100644 --- a/swap/tests/testutils/mod.rs +++ b/swap/tests/testutils/mod.rs @@ -24,7 +24,7 @@ use swap::{ }; use tempfile::tempdir; use testcontainers::{clients::Cli, Container, Docker, RunArgs}; -use tokio::task::JoinHandle; +use tokio::{task::JoinHandle, time::sleep}; use tracing_core::dispatcher::DefaultGuard; use tracing_log::LogTracer; use url::Url; @@ -169,6 +169,11 @@ impl TestContext { pub async fn assert_alice_redeemed(&self, state: AliceState) { assert!(matches!(state, AliceState::BtcRedeemed)); + self.alice_bitcoin_wallet + .sync_wallet() + .await + .expect("Could not sync wallet"); + let btc_balance_after_swap = self.alice_bitcoin_wallet.as_ref().balance().await.unwrap(); assert_eq!( btc_balance_after_swap, @@ -188,6 +193,11 @@ impl TestContext { pub async fn assert_alice_refunded(&self, state: AliceState) { assert!(matches!(state, AliceState::XmrRefunded)); + self.alice_bitcoin_wallet + .sync_wallet() + .await + .expect("Could not sync wallet"); + 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); @@ -210,6 +220,11 @@ 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"); + let btc_balance_after_swap = self.alice_bitcoin_wallet.as_ref().balance().await.unwrap(); assert_eq!( btc_balance_after_swap, @@ -227,6 +242,11 @@ impl TestContext { } pub async fn assert_bob_redeemed(&self, state: BobState) { + self.bob_bitcoin_wallet + .sync_wallet() + .await + .expect("Could not sync wallet"); + let lock_tx_id = if let BobState::XmrRedeemed { tx_lock_id } = state { tx_lock_id } else { @@ -260,6 +280,11 @@ impl TestContext { } pub async fn assert_bob_refunded(&self, state: BobState) { + self.bob_bitcoin_wallet + .sync_wallet() + .await + .expect("Could not sync wallet"); + let lock_tx_id = if let BobState::BtcRefunded(state4) = state { state4.tx_lock_id() } else { @@ -292,6 +317,11 @@ impl TestContext { } pub async fn assert_bob_punished(&self, state: BobState) { + self.bob_bitcoin_wallet + .sync_wallet() + .await + .expect("Could not sync wallet"); + let lock_tx_id = if let BobState::BtcPunished { tx_lock_id } = state { tx_lock_id } else { @@ -580,10 +610,10 @@ async fn init_test_wallets( .await .unwrap(); - let xmr_wallet = Arc::new(swap::monero::Wallet { + let xmr_wallet = swap::monero::Wallet { inner: monero.wallet(name).unwrap().client(), network: monero::Network::default(), - }); + }; let electrum_rpc_url = { let input = format!("tcp://@localhost:{}", electrum_rpc_port); @@ -594,16 +624,14 @@ async fn init_test_wallets( Url::parse(&input).unwrap() }; - let btc_wallet = Arc::new( - swap::bitcoin::Wallet::new( - electrum_rpc_url, - electrum_http_url, - bitcoin::Network::Regtest, - datadir, - ) - .await - .expect("could not init btc wallet"), - ); + let btc_wallet = swap::bitcoin::Wallet::new( + electrum_rpc_url, + electrum_http_url, + bitcoin::Network::Regtest, + datadir, + ) + .await + .expect("could not init btc wallet"); if starting_balances.btc != bitcoin::Amount::ZERO { mint( @@ -615,7 +643,14 @@ async fn init_test_wallets( .expect("could not mint btc starting balance"); } - (btc_wallet, xmr_wallet) + sleep(Duration::from_secs(5)).await; + + btc_wallet + .sync_wallet() + .await + .expect("Could not sync btc wallet"); + + (Arc::new(btc_wallet), Arc::new(xmr_wallet)) } // This is just to keep the containers alive