From 51316d8183bb812631840d26a616c31c16979bab Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Thu, 20 May 2021 10:08:18 +1000 Subject: [PATCH] Make it easier to create a bitcoin::Wallet for testing Forcing the user to create an implementation of `EstimateFeeRate` every time they want to create a wallet for testing is tedious and leads to duplicated code. The implementation for tests is rarely dynamic and thus can be simplified to static arguments. This also allows us to provide convenience constructors to make tests that don't care about fees less distracting by reducing the number of constants that are floating around. --- swap/src/bitcoin.rs | 17 +------- swap/src/bitcoin/lock.rs | 21 +++------- swap/src/bitcoin/wallet.rs | 84 ++++++++++++++++++++------------------ 3 files changed, 51 insertions(+), 71 deletions(-) diff --git a/swap/src/bitcoin.rs b/swap/src/bitcoin.rs index 9534bd04..fc91996b 100644 --- a/swap/src/bitcoin.rs +++ b/swap/src/bitcoin.rs @@ -265,10 +265,8 @@ pub struct NotThreeWitnesses(usize); #[cfg(test)] mod tests { use super::*; - use crate::bitcoin::wallet::EstimateFeeRate; use crate::env::{GetConfig, Regtest}; use crate::protocol::{alice, bob}; - use bdk::FeeRate; use rand::rngs::OsRng; use uuid::Uuid; @@ -317,21 +315,10 @@ mod tests { assert_eq!(expired_timelock, ExpiredTimelocks::Punish) } - struct StaticFeeRate {} - impl EstimateFeeRate for StaticFeeRate { - fn estimate_feerate(&self, _target_block: usize) -> Result { - Ok(FeeRate::default_min_relay_fee()) - } - - fn min_relay_fee(&self) -> Result { - Ok(bitcoin::Amount::from_sat(1_000)) - } - } - #[tokio::test] async fn calculate_transaction_weights() { - let alice_wallet = Wallet::new_funded(Amount::ONE_BTC.as_sat(), StaticFeeRate {}); - let bob_wallet = Wallet::new_funded(Amount::ONE_BTC.as_sat(), StaticFeeRate {}); + let alice_wallet = Wallet::new_funded_default_fees(Amount::ONE_BTC.as_sat()); + let bob_wallet = Wallet::new_funded_default_fees(Amount::ONE_BTC.as_sat()); let spending_fee = Amount::from_sat(1_000); let btc_amount = Amount::from_sat(500_000); let xmr_amount = crate::monero::Amount::from_piconero(10000); diff --git a/swap/src/bitcoin/lock.rs b/swap/src/bitcoin/lock.rs index 5d7933d6..231edff4 100644 --- a/swap/src/bitcoin/lock.rs +++ b/swap/src/bitcoin/lock.rs @@ -184,23 +184,12 @@ impl Watchable for TxLock { #[cfg(test)] mod tests { use super::*; - use bdk::FeeRate; - - struct StaticFeeRate {} - impl EstimateFeeRate for StaticFeeRate { - fn estimate_feerate(&self, _target_block: usize) -> Result { - Ok(FeeRate::default_min_relay_fee()) - } - - fn min_relay_fee(&self) -> Result { - Ok(bitcoin::Amount::from_sat(1_000)) - } - } + use crate::bitcoin::wallet::StaticFeeRate; #[tokio::test] async fn given_bob_sends_good_psbt_when_reconstructing_then_succeeeds() { let (A, B) = alice_and_bob(); - let wallet = Wallet::new_funded(50000, StaticFeeRate {}); + let wallet = Wallet::new_funded_default_fees(50000); let agreed_amount = Amount::from_sat(10000); let psbt = bob_make_psbt(A, B, &wallet, agreed_amount).await; @@ -214,7 +203,7 @@ mod tests { let (A, B) = alice_and_bob(); let fees = 610; let agreed_amount = Amount::from_sat(10000); - let wallet = Wallet::new_funded(agreed_amount.as_sat() + fees, StaticFeeRate {}); + let wallet = Wallet::new_funded_default_fees(agreed_amount.as_sat() + fees); let psbt = bob_make_psbt(A, B, &wallet, agreed_amount).await; assert_eq!( @@ -230,7 +219,7 @@ mod tests { #[tokio::test] async fn given_bob_is_sending_less_than_agreed_when_reconstructing_txlock_then_fails() { let (A, B) = alice_and_bob(); - let wallet = Wallet::new_funded(50000, StaticFeeRate {}); + let wallet = Wallet::new_funded_default_fees(50000); let agreed_amount = Amount::from_sat(10000); let bad_amount = Amount::from_sat(5000); @@ -243,7 +232,7 @@ mod tests { #[tokio::test] async fn given_bob_is_sending_to_a_bad_output_reconstructing_txlock_then_fails() { let (A, B) = alice_and_bob(); - let wallet = Wallet::new_funded(50000, StaticFeeRate {}); + let wallet = Wallet::new_funded_default_fees(50000); let agreed_amount = Amount::from_sat(10000); let E = eve(); diff --git a/swap/src/bitcoin/wallet.rs b/swap/src/bitcoin/wallet.rs index 6268d64e..4aa07825 100644 --- a/swap/src/bitcoin/wallet.rs +++ b/swap/src/bitcoin/wallet.rs @@ -493,12 +493,42 @@ pub trait EstimateFeeRate { } #[cfg(test)] -impl Wallet<(), bdk::database::MemoryDatabase, EFR> -where - EFR: EstimateFeeRate, -{ +pub struct StaticFeeRate { + fee_rate: FeeRate, + min_relay_fee: bitcoin::Amount, +} + +#[cfg(test)] +impl EstimateFeeRate for StaticFeeRate { + fn estimate_feerate(&self, _target_block: usize) -> Result { + Ok(self.fee_rate) + } + + fn min_relay_fee(&self) -> Result { + Ok(self.min_relay_fee) + } +} + +#[cfg(test)] +impl Wallet<(), bdk::database::MemoryDatabase, StaticFeeRate> { + /// Creates a new, funded wallet with sane default fees. + /// + /// Unless you are testing things related to fees, this is likely what you + /// want. + pub fn new_funded_default_fees(amount: u64) -> Self { + Self::new_funded(amount, 1.0, 1000) + } + + /// Creates a new, funded wallet that doesn't pay any fees. + /// + /// This will create invalid transactions but can be useful if you want full + /// control over the output amounts. + pub fn new_funded_zero_fees(amount: u64) -> Self { + Self::new_funded(amount, 0.0, 0) + } + /// Creates a new, funded wallet to be used within tests. - pub fn new_funded(amount: u64, estimate_fee_rate: EFR) -> Self { + pub fn new_funded(amount: u64, sats_per_vb: f32, min_relay_fee_sats: u64) -> Self { use bdk::database::MemoryDatabase; use bdk::{LocalUtxo, TransactionDetails}; use bitcoin::OutPoint; @@ -519,7 +549,10 @@ where bdk::Wallet::new_offline(&descriptors.0, None, Network::Regtest, database).unwrap(); Self { - client: Arc::new(Mutex::new(estimate_fee_rate)), + client: Arc::new(Mutex::new(StaticFeeRate { + fee_rate: FeeRate::from_sat_per_vb(sats_per_vb), + min_relay_fee: bitcoin::Amount::from_sat(min_relay_fee_sats), + })), wallet: Arc::new(Mutex::new(wallet)), finality_confirmations: 1, network: Network::Regtest, @@ -973,23 +1006,9 @@ mod tests { } } - struct StaticFeeRate { - min_relay_fee: u64, - } - - impl EstimateFeeRate for StaticFeeRate { - fn estimate_feerate(&self, _target_block: usize) -> Result { - Ok(FeeRate::default_min_relay_fee()) - } - - fn min_relay_fee(&self) -> Result { - Ok(bitcoin::Amount::from_sat(self.min_relay_fee)) - } - } - #[tokio::test] async fn given_no_balance_returns_amount_0() { - let wallet = Wallet::new_funded(0, StaticFeeRate { min_relay_fee: 1 }); + let wallet = Wallet::new_funded(0, 1.0, 1); let amount = wallet.max_giveable(TxLock::script_size()).await.unwrap(); assert_eq!(amount, Amount::ZERO); @@ -997,9 +1016,7 @@ mod tests { #[tokio::test] async fn given_balance_below_min_relay_fee_returns_amount_0() { - let wallet = Wallet::new_funded(1000, StaticFeeRate { - min_relay_fee: 1001, - }); + let wallet = Wallet::new_funded(1000, 1.0, 1001); let amount = wallet.max_giveable(TxLock::script_size()).await.unwrap(); assert_eq!(amount, Amount::ZERO); @@ -1007,9 +1024,7 @@ mod tests { #[tokio::test] async fn given_balance_above_relay_fee_returns_amount_greater_0() { - let wallet = Wallet::new_funded(10_000, StaticFeeRate { - min_relay_fee: 1000, - }); + let wallet = Wallet::new_funded_default_fees(10_000); let amount = wallet.max_giveable(TxLock::script_size()).await.unwrap(); assert!(amount.as_sat() > 0); @@ -1022,25 +1037,14 @@ mod tests { #[tokio::test] async fn given_amounts_with_change_outputs_when_signing_tx_then_output_index_0_is_ensured_for_script( ) { - // We don't care about fees in this test, thus use a zero fee rate - struct NoFeeRate(); - impl EstimateFeeRate for NoFeeRate { - fn estimate_feerate(&self, _target_block: usize) -> Result { - Ok(FeeRate::from_sat_per_vb(0.0)) - } - - fn min_relay_fee(&self) -> Result { - Ok(bitcoin::Amount::from_sat(0)) - } - } - // This value is somewhat arbitrary but the indexation problem usually occurred // on the first or second value (i.e. 547, 548) We keep the test // iterations relatively low because these tests are expensive. let above_dust = 547; let balance = 2000; - let wallet = Wallet::new_funded(balance, NoFeeRate()); + // We don't care about fees in this test, thus use a zero fee rate + let wallet = Wallet::new_funded_zero_fees(balance); // sorting is only relevant for amounts that have a change output // if the change output is below dust it will be dropped by the BDK