Remove traits in favor of using the wallet struct directly

Abstracting over the individual bits of functionality of the wallet
does have its place, especially if one wants to keep a separation
of an abstract protocol library that other people can use with their
own wallets.

However, at the moment, the traits only cause unnecessary friction.
We can always add such abstraction layers again once we need them.

swap/src/bitcoin.rs

@@ -19,13 +19,11 @@ pub use ::bitcoin::{util::amount::Amount, Address, Network, Transaction, Txid};
 pub use ecdsa_fun::{adaptor::EncryptedSignature, fun::Scalar, Signature};
 pub use wallet::Wallet;
 
-use crate::execution_params::ExecutionParams;
 use ::bitcoin::{
     hashes::{hex::ToHex, Hash},
     secp256k1, SigHash,
 };
 use anyhow::{anyhow, bail, Result};
-use async_trait::async_trait;
 use ecdsa_fun::{
     adaptor::{Adaptor, HashTranscript},
     fun::Point,
@@ -201,45 +199,6 @@ pub fn build_shared_output_descriptor(A: Point, B: Point) -> Descriptor<bitcoin:
     Descriptor::Wsh(Wsh::new(miniscript).expect("a valid descriptor"))
 }
 
-#[async_trait]
-pub trait SignTxLock {
-    async fn sign_tx_lock(&self, tx_lock: TxLock) -> Result<Transaction>;
-}
-
-#[async_trait]
-pub trait BroadcastSignedTransaction {
-    async fn broadcast_signed_transaction(&self, transaction: Transaction) -> Result<Txid>;
-}
-
-#[async_trait]
-pub trait WatchForRawTransaction {
-    async fn watch_for_raw_transaction(&self, txid: Txid) -> Result<Transaction>;
-}
-
-#[async_trait]
-pub trait WaitForTransactionFinality {
-    async fn wait_for_transaction_finality(
-        &self,
-        txid: Txid,
-        execution_params: ExecutionParams,
-    ) -> Result<()>;
-}
-
-#[async_trait]
-pub trait GetBlockHeight {
-    async fn get_block_height(&self) -> Result<BlockHeight>;
-}
-
-#[async_trait]
-pub trait TransactionBlockHeight {
-    async fn transaction_block_height(&self, txid: Txid) -> Result<BlockHeight>;
-}
-
-#[async_trait]
-pub trait GetRawTransaction {
-    async fn get_raw_transaction(&self, txid: Txid) -> Result<Transaction>;
-}
-
 pub fn recover(S: PublicKey, sig: Signature, encsig: EncryptedSignature) -> Result<SecretKey> {
     let adaptor = Adaptor::<HashTranscript<Sha256>, Deterministic<Sha256>>::default();
 
@@ -251,25 +210,22 @@ pub fn recover(S: PublicKey, sig: Signature, encsig: EncryptedSignature) -> Resu
     Ok(s)
 }
 
-pub async fn poll_until_block_height_is_gte<B>(client: &B, target: BlockHeight) -> Result<()>
-where
-    B: GetBlockHeight,
-{
+pub async fn poll_until_block_height_is_gte(
+    client: &crate::bitcoin::Wallet,
+    target: BlockHeight,
+) -> Result<()> {
     while client.get_block_height().await? < target {
         tokio::time::sleep(std::time::Duration::from_secs(1)).await;
     }
     Ok(())
 }
 
-pub async fn current_epoch<W>(
-    bitcoin_wallet: &W,
+pub async fn current_epoch(
+    bitcoin_wallet: &crate::bitcoin::Wallet,
     cancel_timelock: CancelTimelock,
     punish_timelock: PunishTimelock,
     lock_tx_id: ::bitcoin::Txid,
-) -> Result<ExpiredTimelocks>
-where
-    W: WatchForRawTransaction + TransactionBlockHeight + GetBlockHeight,
-{
+) -> Result<ExpiredTimelocks> {
     let current_block_height = bitcoin_wallet.get_block_height().await?;
     let lock_tx_height = bitcoin_wallet.transaction_block_height(lock_tx_id).await?;
     let cancel_timelock_height = lock_tx_height + cancel_timelock;
@@ -285,14 +241,11 @@ where
     }
 }
 
-pub async fn wait_for_cancel_timelock_to_expire<W>(
-    bitcoin_wallet: &W,
+pub async fn wait_for_cancel_timelock_to_expire(
+    bitcoin_wallet: &crate::bitcoin::Wallet,
     cancel_timelock: CancelTimelock,
     lock_tx_id: ::bitcoin::Txid,
-) -> Result<()>
-where
-    W: WatchForRawTransaction + TransactionBlockHeight + GetBlockHeight,
-{
+) -> Result<()> {
     let tx_lock_height = bitcoin_wallet.transaction_block_height(lock_tx_id).await?;
 
     poll_until_block_height_is_gte(bitcoin_wallet, tx_lock_height + cancel_timelock).await?;

swap/src/bitcoin/wallet.rs

@@ -1,14 +1,9 @@
 use crate::{
-    bitcoin::{
-        timelocks::BlockHeight, Address, Amount, BroadcastSignedTransaction, GetBlockHeight,
-        GetRawTransaction, SignTxLock, Transaction, TransactionBlockHeight, TxLock,
-        WaitForTransactionFinality, WatchForRawTransaction,
-    },
+    bitcoin::{timelocks::BlockHeight, Address, Amount, Transaction, TxLock},
     execution_params::ExecutionParams,
 };
 use ::bitcoin::{util::psbt::PartiallySignedTransaction, Txid};
 use anyhow::{anyhow, bail, Context, Result};
-use async_trait::async_trait;
 use backoff::{backoff::Constant as ConstantBackoff, future::retry};
 use bdk::{
     blockchain::{noop_progress, Blockchain, ElectrumBlockchain},
@@ -152,17 +147,19 @@ impl Wallet {
         self.inner.lock().await.network()
     }
 
-    /// Selects an appropriate [`FeeRate`] to be used for getting transactions
-    /// confirmed within a reasonable amount of time.
-    fn select_feerate(&self) -> FeeRate {
-        // TODO: This should obviously not be a const :)
-        FeeRate::from_sat_per_vb(5.0)
-    }
-}
+    pub async fn broadcast_signed_transaction(&self, transaction: Transaction) -> Result<Txid> {
+        let txid = transaction.txid();
 
-#[async_trait]
-impl SignTxLock for Wallet {
-    async fn sign_tx_lock(&self, tx_lock: TxLock) -> Result<Transaction> {
+        self.inner
+            .lock()
+            .await
+            .broadcast(transaction)
+            .with_context(|| format!("failed to broadcast transaction {}", txid))?;
+
+        Ok(txid)
+    }
+
+    pub async fn sign_tx_lock(&self, tx_lock: TxLock) -> Result<Transaction> {
         let txid = tx_lock.txid();
         tracing::debug!("signing tx lock: {}", txid);
         let psbt = PartiallySignedTransaction::from(tx_lock);
@@ -174,26 +171,14 @@ impl SignTxLock for Wallet {
         tracing::debug!("signed tx lock: {}", txid);
         Ok(tx)
     }
-}
 
-#[async_trait]
-impl BroadcastSignedTransaction for Wallet {
-    async fn broadcast_signed_transaction(&self, transaction: Transaction) -> Result<Txid> {
-        let txid = transaction.txid();
-
-        self.inner
-            .lock()
-            .await
-            .broadcast(transaction)
-            .with_context(|| format!("failed to broadcast transaction {}", txid))?;
-
-        Ok(txid)
+    pub async fn get_raw_transaction(&self, txid: Txid) -> Result<Transaction> {
+        self.get_tx(txid)
+            .await?
+            .ok_or_else(|| anyhow!("Could not get raw tx with id: {}", txid))
     }
-}
 
-#[async_trait]
-impl WatchForRawTransaction for Wallet {
-    async fn watch_for_raw_transaction(&self, txid: Txid) -> Result<Transaction> {
+    pub async fn watch_for_raw_transaction(&self, txid: Txid) -> Result<Transaction> {
         tracing::debug!("watching for tx: {}", txid);
         let tx = retry(ConstantBackoff::new(Duration::from_secs(1)), || async {
             let client = Client::new(self.rpc_url.as_ref())
@@ -214,20 +199,8 @@ impl WatchForRawTransaction for Wallet {
 
         Ok(tx)
     }
-}
 
-#[async_trait]
-impl GetRawTransaction for Wallet {
-    async fn get_raw_transaction(&self, txid: Txid) -> Result<Transaction> {
-        self.get_tx(txid)
-            .await?
-            .ok_or_else(|| anyhow!("Could not get raw tx with id: {}", txid))
-    }
-}
-
-#[async_trait]
-impl GetBlockHeight for Wallet {
-    async fn get_block_height(&self) -> Result<BlockHeight> {
+    pub async fn get_block_height(&self) -> Result<BlockHeight> {
         let url = blocks_tip_height_url(&self.http_url)?;
         let height = retry(ConstantBackoff::new(Duration::from_secs(1)), || async {
             let height = reqwest::Client::new()
@@ -247,11 +220,8 @@ impl GetBlockHeight for Wallet {
 
         Ok(BlockHeight::new(height))
     }
-}
 
-#[async_trait]
-impl TransactionBlockHeight for Wallet {
-    async fn transaction_block_height(&self, txid: Txid) -> Result<BlockHeight> {
+    pub async fn transaction_block_height(&self, txid: Txid) -> Result<BlockHeight> {
         let url = tx_status_url(txid, &self.http_url)?;
         #[derive(Serialize, Deserialize, Debug, Clone)]
         struct TransactionStatus {
@@ -281,11 +251,8 @@ impl TransactionBlockHeight for Wallet {
 
         Ok(BlockHeight::new(height))
     }
-}
 
-#[async_trait]
-impl WaitForTransactionFinality for Wallet {
-    async fn wait_for_transaction_finality(
+    pub async fn wait_for_transaction_finality(
         &self,
         txid: Txid,
         execution_params: ExecutionParams,
@@ -315,6 +282,13 @@ impl WaitForTransactionFinality for Wallet {
 
         Ok(())
     }
+
+    /// Selects an appropriate [`FeeRate`] to be used for getting transactions
+    /// confirmed within a reasonable amount of time.
+    fn select_feerate(&self) -> FeeRate {
+        // TODO: This should obviously not be a const :)
+        FeeRate::from_sat_per_vb(5.0)
+    }
 }
 
 fn tx_status_url(txid: Txid, base_url: &Url) -> Result<Url> {

swap/src/protocol/alice/state.rs

@@ -2,8 +2,7 @@ use crate::{
     bitcoin,
     bitcoin::{
         current_epoch, wait_for_cancel_timelock_to_expire, CancelTimelock, ExpiredTimelocks,
-        GetBlockHeight, PunishTimelock, TransactionBlockHeight, TxCancel, TxRefund,
-        WatchForRawTransaction,
+        PunishTimelock, TxCancel, TxRefund,
     },
     execution_params::ExecutionParams,
     monero,
@@ -325,10 +324,10 @@ pub struct State3 {
 }
 
 impl State3 {
-    pub async fn wait_for_cancel_timelock_to_expire<W>(&self, bitcoin_wallet: &W) -> Result<()>
-    where
-        W: WatchForRawTransaction + TransactionBlockHeight + GetBlockHeight,
-    {
+    pub async fn wait_for_cancel_timelock_to_expire(
+        &self,
+        bitcoin_wallet: &bitcoin::Wallet,
+    ) -> Result<()> {
         wait_for_cancel_timelock_to_expire(
             bitcoin_wallet,
             self.cancel_timelock,
@@ -337,10 +336,10 @@ impl State3 {
         .await
     }
 
-    pub async fn expired_timelocks<W>(&self, bitcoin_wallet: &W) -> Result<ExpiredTimelocks>
-    where
-        W: WatchForRawTransaction + TransactionBlockHeight + GetBlockHeight,
-    {
+    pub async fn expired_timelocks(
+        &self,
+        bitcoin_wallet: &bitcoin::Wallet,
+    ) -> Result<ExpiredTimelocks> {
         current_epoch(
             bitcoin_wallet,
             self.cancel_timelock,

swap/src/protocol/alice/steps.rs

@@ -1,10 +1,8 @@
 use crate::{
     bitcoin,
     bitcoin::{
-        poll_until_block_height_is_gte, BlockHeight, BroadcastSignedTransaction, CancelTimelock,
-        EncryptedSignature, GetBlockHeight, GetRawTransaction, PunishTimelock,
-        TransactionBlockHeight, TxCancel, TxLock, TxRefund, WaitForTransactionFinality,
-        WatchForRawTransaction,
+        poll_until_block_height_is_gte, BlockHeight, CancelTimelock, EncryptedSignature,
+        PunishTimelock, TxCancel, TxLock, TxRefund,
     },
     execution_params::ExecutionParams,
     monero,
@@ -31,14 +29,11 @@ use tracing::info;
 
 // TODO(Franck): Use helper functions from xmr-btc instead of re-writing them
 // here
-pub async fn wait_for_locked_bitcoin<W>(
+pub async fn wait_for_locked_bitcoin(
     lock_bitcoin_txid: bitcoin::Txid,
-    bitcoin_wallet: Arc<W>,
+    bitcoin_wallet: &bitcoin::Wallet,
     execution_params: ExecutionParams,
-) -> Result<()>
-where
-    W: WatchForRawTransaction + WaitForTransactionFinality,
-{
+) -> Result<()> {
     // We assume we will see Bob's transaction in the mempool first.
     timeout(
         execution_params.bob_time_to_act,
@@ -130,13 +125,10 @@ pub fn build_bitcoin_redeem_transaction(
     Ok(tx)
 }
 
-pub async fn publish_bitcoin_redeem_transaction<W>(
+pub async fn publish_bitcoin_redeem_transaction(
     redeem_tx: bitcoin::Transaction,
-    bitcoin_wallet: Arc<W>,
-) -> Result<::bitcoin::Txid>
-where
-    W: BroadcastSignedTransaction + WaitForTransactionFinality,
-{
+    bitcoin_wallet: Arc<bitcoin::Wallet>,
+) -> Result<::bitcoin::Txid> {
     info!("Attempting to publish bitcoin redeem txn");
     let txid = bitcoin_wallet
         .broadcast_signed_transaction(redeem_tx)
@@ -145,17 +137,14 @@ where
     Ok(txid)
 }
 
-pub async fn publish_cancel_transaction<W>(
+pub async fn publish_cancel_transaction(
     tx_lock: TxLock,
     a: bitcoin::SecretKey,
     B: bitcoin::PublicKey,
     cancel_timelock: CancelTimelock,
     tx_cancel_sig_bob: bitcoin::Signature,
-    bitcoin_wallet: Arc<W>,
-) -> Result<bitcoin::TxCancel>
-where
-    W: GetRawTransaction + TransactionBlockHeight + GetBlockHeight + BroadcastSignedTransaction,
-{
+    bitcoin_wallet: Arc<bitcoin::Wallet>,
+) -> Result<bitcoin::TxCancel> {
     // First wait for cancel timelock to expire
     let tx_lock_height = bitcoin_wallet
         .transaction_block_height(tx_lock.txid())
@@ -194,18 +183,15 @@ where
     Ok(tx_cancel)
 }
 
-pub async fn wait_for_bitcoin_refund<W>(
+pub async fn wait_for_bitcoin_refund(
     tx_cancel: &TxCancel,
     cancel_tx_height: BlockHeight,
     punish_timelock: PunishTimelock,
     refund_address: &bitcoin::Address,
-    bitcoin_wallet: Arc<W>,
-) -> Result<(bitcoin::TxRefund, Option<bitcoin::Transaction>)>
-where
-    W: GetBlockHeight + WatchForRawTransaction,
-{
+    bitcoin_wallet: &bitcoin::Wallet,
+) -> Result<(bitcoin::TxRefund, Option<bitcoin::Transaction>)> {
     let punish_timelock_expired =
-        poll_until_block_height_is_gte(bitcoin_wallet.as_ref(), cancel_tx_height + punish_timelock);
+        poll_until_block_height_is_gte(bitcoin_wallet, cancel_tx_height + punish_timelock);
 
     let tx_refund = bitcoin::TxRefund::new(tx_cancel, refund_address);
 
@@ -267,14 +253,11 @@ pub fn build_bitcoin_punish_transaction(
     Ok(signed_tx_punish)
 }
 
-pub async fn publish_bitcoin_punish_transaction<W>(
+pub async fn publish_bitcoin_punish_transaction(
     punish_tx: bitcoin::Transaction,
-    bitcoin_wallet: Arc<W>,
+    bitcoin_wallet: Arc<bitcoin::Wallet>,
     execution_params: ExecutionParams,
-) -> Result<bitcoin::Txid>
-where
-    W: BroadcastSignedTransaction + WaitForTransactionFinality,
-{
+) -> Result<bitcoin::Txid> {
     let txid = bitcoin_wallet
         .broadcast_signed_transaction(punish_tx)
         .await?;

swap/src/protocol/alice/swap.rs

@@ -2,10 +2,7 @@
 //! Alice holds XMR and wishes receive BTC.
 use crate::{
     bitcoin,
-    bitcoin::{
-        ExpiredTimelocks, TransactionBlockHeight, WaitForTransactionFinality,
-        WatchForRawTransaction,
-    },
+    bitcoin::ExpiredTimelocks,
     database,
     database::Database,
     execution_params::ExecutionParams,
@@ -98,7 +95,7 @@ async fn run_until_internal(
             } => {
                 let _ = wait_for_locked_bitcoin(
                     state3.tx_lock.txid(),
-                    bitcoin_wallet.clone(),
+                    &bitcoin_wallet,
                     execution_params,
                 )
                 .await?;
@@ -335,7 +332,7 @@ async fn run_until_internal(
                     tx_cancel_height,
                     state3.punish_timelock,
                     &state3.refund_address,
-                    bitcoin_wallet.clone(),
+                    &bitcoin_wallet,
                 )
                 .await?;
 

swap/src/protocol/bob/state.rs

@@ -1,8 +1,7 @@
 use crate::{
     bitcoin::{
-        self, current_epoch, wait_for_cancel_timelock_to_expire, BroadcastSignedTransaction,
-        CancelTimelock, ExpiredTimelocks, GetBlockHeight, GetRawTransaction, PunishTimelock,
-        Transaction, TransactionBlockHeight, TxCancel, Txid, WatchForRawTransaction,
+        self, current_epoch, wait_for_cancel_timelock_to_expire, CancelTimelock, ExpiredTimelocks,
+        PunishTimelock, Transaction, TxCancel, Txid,
     },
     execution_params::ExecutionParams,
     monero,
@@ -269,10 +268,7 @@ impl State2 {
         }
     }
 
-    pub async fn lock_btc<W>(self, bitcoin_wallet: &W) -> Result<State3>
-    where
-        W: bitcoin::SignTxLock + bitcoin::BroadcastSignedTransaction,
-    {
+    pub async fn lock_btc(self, bitcoin_wallet: &bitcoin::Wallet) -> Result<State3> {
         let signed_tx_lock = bitcoin_wallet.sign_tx_lock(self.tx_lock.clone()).await?;
 
         tracing::debug!("locking BTC in transaction {}", self.tx_lock.txid());
@@ -363,10 +359,10 @@ impl State3 {
         }))
     }
 
-    pub async fn wait_for_cancel_timelock_to_expire<W>(&self, bitcoin_wallet: &W) -> Result<()>
-    where
-        W: WatchForRawTransaction + TransactionBlockHeight + GetBlockHeight,
-    {
+    pub async fn wait_for_cancel_timelock_to_expire(
+        &self,
+        bitcoin_wallet: &bitcoin::Wallet,
+    ) -> Result<()> {
         wait_for_cancel_timelock_to_expire(
             bitcoin_wallet,
             self.cancel_timelock,
@@ -399,10 +395,10 @@ impl State3 {
         self.tx_lock.txid()
     }
 
-    pub async fn current_epoch<W>(&self, bitcoin_wallet: &W) -> Result<ExpiredTimelocks>
-    where
-        W: WatchForRawTransaction + TransactionBlockHeight + GetBlockHeight,
-    {
+    pub async fn current_epoch(
+        &self,
+        bitcoin_wallet: &bitcoin::Wallet,
+    ) -> Result<ExpiredTimelocks> {
         current_epoch(
             bitcoin_wallet,
             self.cancel_timelock,
@@ -443,10 +439,10 @@ impl State4 {
         self.b.encsign(self.S_a_bitcoin, tx_redeem.digest())
     }
 
-    pub async fn check_for_tx_cancel<W>(&self, bitcoin_wallet: &W) -> Result<Transaction>
-    where
-        W: GetRawTransaction,
-    {
+    pub async fn check_for_tx_cancel(
+        &self,
+        bitcoin_wallet: &bitcoin::Wallet,
+    ) -> Result<Transaction> {
         let tx_cancel =
             bitcoin::TxCancel::new(&self.tx_lock, self.cancel_timelock, self.A, self.b.public());
 
@@ -466,10 +462,7 @@ impl State4 {
         Ok(tx)
     }
 
-    pub async fn submit_tx_cancel<W>(&self, bitcoin_wallet: &W) -> Result<Txid>
-    where
-        W: BroadcastSignedTransaction,
-    {
+    pub async fn submit_tx_cancel(&self, bitcoin_wallet: &bitcoin::Wallet) -> Result<Txid> {
         let tx_cancel =
             bitcoin::TxCancel::new(&self.tx_lock, self.cancel_timelock, self.A, self.b.public());
 
@@ -490,10 +483,7 @@ impl State4 {
         Ok(tx_id)
     }
 
-    pub async fn watch_for_redeem_btc<W>(&self, bitcoin_wallet: &W) -> Result<State5>
-    where
-        W: WatchForRawTransaction,
-    {
+    pub async fn watch_for_redeem_btc(&self, bitcoin_wallet: &bitcoin::Wallet) -> Result<State5> {
         let tx_redeem = bitcoin::TxRedeem::new(&self.tx_lock, &self.redeem_address);
         let tx_redeem_encsig = self.b.encsign(self.S_a_bitcoin, tx_redeem.digest());
 
@@ -515,10 +505,10 @@ impl State4 {
         })
     }
 
-    pub async fn wait_for_cancel_timelock_to_expire<W>(&self, bitcoin_wallet: &W) -> Result<()>
-    where
-        W: WatchForRawTransaction + TransactionBlockHeight + GetBlockHeight,
-    {
+    pub async fn wait_for_cancel_timelock_to_expire(
+        &self,
+        bitcoin_wallet: &bitcoin::Wallet,
+    ) -> Result<()> {
         wait_for_cancel_timelock_to_expire(
             bitcoin_wallet,
             self.cancel_timelock,
@@ -527,10 +517,10 @@ impl State4 {
         .await
     }
 
-    pub async fn expired_timelock<W>(&self, bitcoin_wallet: &W) -> Result<ExpiredTimelocks>
-    where
-        W: WatchForRawTransaction + TransactionBlockHeight + GetBlockHeight,
-    {
+    pub async fn expired_timelock(
+        &self,
+        bitcoin_wallet: &bitcoin::Wallet,
+    ) -> Result<ExpiredTimelocks> {
         current_epoch(
             bitcoin_wallet,
             self.cancel_timelock,
@@ -540,14 +530,11 @@ impl State4 {
         .await
     }
 
-    pub async fn refund_btc<W>(
+    pub async fn refund_btc(
         &self,
-        bitcoin_wallet: &W,
+        bitcoin_wallet: &bitcoin::Wallet,
         execution_params: ExecutionParams,
-    ) -> Result<()>
-    where
-        W: bitcoin::BroadcastSignedTransaction + bitcoin::WaitForTransactionFinality,
-    {
+    ) -> Result<()> {
         let tx_cancel =
             bitcoin::TxCancel::new(&self.tx_lock, self.cancel_timelock, self.A, self.b.public());
         let tx_refund = bitcoin::TxRefund::new(&tx_cancel, &self.refund_address);