Merge #515

515: Make it easier to create a bitcoin::Wallet for testing r=thomaseizinger a=thomaseizinger

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.

Co-authored-by: Thomas Eizinger <thomas@eizinger.io>

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<FeeRate> {
-            Ok(FeeRate::default_min_relay_fee())
-        }
-
-        fn min_relay_fee(&self) -> Result<bitcoin::Amount> {
-            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);

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<FeeRate> {
-            Ok(FeeRate::default_min_relay_fee())
-        }
-
-        fn min_relay_fee(&self) -> Result<bitcoin::Amount> {
-            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();

swap/src/bitcoin/wallet.rs

@@ -497,12 +497,42 @@ pub trait EstimateFeeRate {
 }
 
 #[cfg(test)]
-impl<EFR> 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<FeeRate> {
+        Ok(self.fee_rate)
+    }
+
+    fn min_relay_fee(&self) -> Result<bitcoin::Amount> {
+        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;
@@ -523,7 +553,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,
@@ -977,23 +1010,9 @@ mod tests {
         }
     }
 
-    struct StaticFeeRate {
-        min_relay_fee: u64,
-    }
-
-    impl EstimateFeeRate for StaticFeeRate {
-        fn estimate_feerate(&self, _target_block: usize) -> Result<FeeRate> {
-            Ok(FeeRate::default_min_relay_fee())
-        }
-
-        fn min_relay_fee(&self) -> Result<bitcoin::Amount> {
-            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);
@@ -1001,9 +1020,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);
@@ -1011,9 +1028,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);
@@ -1026,25 +1041,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<FeeRate> {
-                Ok(FeeRate::from_sat_per_vb(0.0))
-            }
-
-            fn min_relay_fee(&self) -> Result<bitcoin::Amount> {
-                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