Allow blockchain calls to fail

Prior to this change, functions could not fail early on permanent errors eg. parsing a url. Merged error enums.

swap/src/bitcoin.rs

@@ -211,7 +211,7 @@ pub trait BroadcastSignedTransaction {
 
 #[async_trait]
 pub trait WatchForRawTransaction {
-    async fn watch_for_raw_transaction(&self, txid: Txid) -> Transaction;
+    async fn watch_for_raw_transaction(&self, txid: Txid) -> Result<Transaction>;
 }
 
 #[async_trait]
@@ -225,12 +225,12 @@ pub trait WaitForTransactionFinality {
 
 #[async_trait]
 pub trait GetBlockHeight {
-    async fn get_block_height(&self) -> BlockHeight;
+    async fn get_block_height(&self) -> Result<BlockHeight>;
 }
 
 #[async_trait]
 pub trait TransactionBlockHeight {
-    async fn transaction_block_height(&self, txid: Txid) -> BlockHeight;
+    async fn transaction_block_height(&self, txid: Txid) -> Result<BlockHeight>;
 }
 
 #[async_trait]
@@ -259,13 +259,14 @@ 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)
+pub async fn poll_until_block_height_is_gte<B>(client: &B, target: BlockHeight) -> Result<()>
 where
     B: GetBlockHeight,
 {
-    while client.get_block_height().await < target {
+    while client.get_block_height().await? < target {
         tokio::time::sleep(std::time::Duration::from_secs(1)).await;
     }
+    Ok(())
 }
 
 pub async fn current_epoch<W>(
@@ -277,8 +278,8 @@ pub async fn current_epoch<W>(
 where
     W: WatchForRawTransaction + TransactionBlockHeight + GetBlockHeight,
 {
-    let current_block_height = bitcoin_wallet.get_block_height().await;
-    let lock_tx_height = bitcoin_wallet.transaction_block_height(lock_tx_id).await;
+    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;
     let punish_timelock_height = cancel_timelock_height + punish_timelock;
 
@@ -300,9 +301,9 @@ pub async fn wait_for_cancel_timelock_to_expire<W>(
 where
     W: WatchForRawTransaction + TransactionBlockHeight + GetBlockHeight,
 {
-    let tx_lock_height = bitcoin_wallet.transaction_block_height(lock_tx_id).await;
+    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;
+    poll_until_block_height_is_gte(bitcoin_wallet, tx_lock_height + cancel_timelock).await?;
     Ok(())
 }
 

swap/src/bitcoin/wallet.rs

@@ -23,6 +23,14 @@ use tokio::{sync::Mutex, time::interval};
 
 const SLED_TREE_NAME: &str = "default_tree";
 
+#[derive(Debug)]
+enum Error {
+    Io(reqwest::Error),
+    Parse(std::num::ParseIntError),
+    NotYetMined,
+    JsonDeserialisation(reqwest::Error),
+}
+
 pub struct Wallet {
     pub inner: Arc<Mutex<bdk::Wallet<ElectrumBlockchain, bdk::sled::Tree>>>,
     pub network: bitcoin::Network,
@@ -150,7 +158,7 @@ impl BroadcastSignedTransaction for Wallet {
 
 #[async_trait]
 impl WatchForRawTransaction for Wallet {
-    async fn watch_for_raw_transaction(&self, txid: Txid) -> Transaction {
+    async fn watch_for_raw_transaction(&self, txid: Txid) -> Result<Transaction> {
         tracing::debug!("watching for tx: {}", txid);
         retry(ConstantBackoff::new(Duration::from_secs(1)), || async {
             let client = Client::new(self.rpc_url.as_ref())?;
@@ -159,7 +167,7 @@ impl WatchForRawTransaction for Wallet {
             Ok(tx)
         })
         .await
-        .expect("transient errors to be retried")
+        .map_err(|err| anyhow!("transient errors to be retried: {:?}", err))
     }
 }
 
@@ -174,19 +182,11 @@ impl GetRawTransaction for Wallet {
 
 #[async_trait]
 impl GetBlockHeight for Wallet {
-    async fn get_block_height(&self) -> BlockHeight {
-        // todo: create this url using the join() api in the Url type
-        let url = format!("{}{}", self.http_url.as_str(), "blocks/tip/height");
-        #[derive(Debug)]
-        enum Error {
-            Io(reqwest::Error),
-            Parse(std::num::ParseIntError),
-        }
+    async fn get_block_height(&self) -> Result<BlockHeight> {
+        let url = self.http_url.join("blocks/tip/height")?;
         let height = retry(ConstantBackoff::new(Duration::from_secs(1)), || async {
-            // todo: We may want to return early if we cannot connect to the electrum node
-            // rather than retrying
             let height = reqwest::Client::new()
-                .request(Method::GET, &url)
+                .request(Method::GET, url.clone())
                 .send()
                 .await
                 .map_err(Error::Io)?
@@ -194,37 +194,29 @@ impl GetBlockHeight for Wallet {
                 .await
                 .map_err(Error::Io)?
                 .parse::<u32>()
-                .map_err(Error::Parse)?;
+                .map_err(|err| backoff::Error::Permanent(Error::Parse(err)))?;
             Result::<_, backoff::Error<Error>>::Ok(height)
         })
         .await
-        .expect("transient errors to be retried");
+        .map_err(|err| anyhow!("transient errors to be retried: {:?}", err))?;
 
-        BlockHeight::new(height)
+        Ok(BlockHeight::new(height))
     }
 }
 
 #[async_trait]
 impl TransactionBlockHeight for Wallet {
-    async fn transaction_block_height(&self, txid: Txid) -> BlockHeight {
-        // todo: create this url using the join() api in the Url type
-        let url = format!("{}tx/{}/status", self.http_url, txid);
+    async fn transaction_block_height(&self, txid: Txid) -> Result<BlockHeight> {
+        let url = self.http_url.join(&format!("tx/{}/status", txid))?;
+
         #[derive(Serialize, Deserialize, Debug, Clone)]
         struct TransactionStatus {
             block_height: Option<u32>,
             confirmed: bool,
         }
-        // todo: See if we can make this error handling more elegant
-        // errors
-        #[derive(Debug)]
-        enum Error {
-            Io(reqwest::Error),
-            NotYetMined,
-            JsonDeserialisation(reqwest::Error),
-        }
         let height = retry(ConstantBackoff::new(Duration::from_secs(1)), || async {
             let resp = reqwest::Client::new()
-                .request(Method::GET, &url)
+                .request(Method::GET, url.clone())
                 .send()
                 .await
                 .map_err(|err| backoff::Error::Transient(Error::Io(err)))?;
@@ -241,9 +233,9 @@ impl TransactionBlockHeight for Wallet {
             Result::<_, backoff::Error<Error>>::Ok(block_height)
         })
         .await
-        .expect("transient errors to be retried");
+        .map_err(|err| anyhow!("transient errors to be retried: {:?}", err))?;
 
-        BlockHeight::new(height)
+        Ok(BlockHeight::new(height))
     }
 }
 
@@ -260,9 +252,9 @@ impl WaitForTransactionFinality for Wallet {
         let mut interval = interval(execution_params.bitcoin_avg_block_time / 4);
 
         loop {
-            let tx_block_height = self.transaction_block_height(txid).await;
+            let tx_block_height = self.transaction_block_height(txid).await?;
             tracing::debug!("tx_block_height: {:?}", tx_block_height);
-            let block_height = self.get_block_height().await;
+            let block_height = self.get_block_height().await?;
             tracing::debug!("latest_block_height: {:?}", block_height);
             if let Some(confirmations) = block_height.checked_sub(tx_block_height) {
                 tracing::debug!("confirmations: {:?}", confirmations);

swap/src/protocol/alice/steps.rs

@@ -42,7 +42,7 @@ where
         bitcoin_wallet.watch_for_raw_transaction(lock_bitcoin_txid),
     )
     .await
-    .context("Failed to find lock Bitcoin tx")?;
+    .context("Failed to find lock Bitcoin tx")??;
 
     // // We saw the transaction in the mempool, waiting for it to be confirmed.
     bitcoin_wallet
@@ -158,8 +158,9 @@ where
     // First wait for cancel timelock to expire
     let tx_lock_height = bitcoin_wallet
         .transaction_block_height(tx_lock.txid())
-        .await;
-    poll_until_block_height_is_gte(bitcoin_wallet.as_ref(), tx_lock_height + cancel_timelock).await;
+        .await?;
+    poll_until_block_height_is_gte(bitcoin_wallet.as_ref(), tx_lock_height + cancel_timelock)
+        .await?;
 
     let tx_cancel = bitcoin::TxCancel::new(&tx_lock, cancel_timelock, a.public(), B);
 
@@ -216,7 +217,7 @@ where
 
     match select(punish_timelock_expired, seen_refund_tx).await {
         Either::Left(_) => Ok((tx_refund, None)),
-        Either::Right((published_refund_tx, _)) => Ok((tx_refund, Some(published_refund_tx))),
+        Either::Right((published_refund_tx, _)) => Ok((tx_refund, Some(published_refund_tx?))),
     }
 }
 

swap/src/protocol/alice/swap.rs

@@ -292,7 +292,7 @@ async fn run_until_internal(
             AliceState::BtcCancelled { state3, tx_cancel } => {
                 let tx_cancel_height = bitcoin_wallet
                     .transaction_block_height(tx_cancel.txid())
-                    .await;
+                    .await?;
 
                 let (tx_refund, published_refund_tx) = wait_for_bitcoin_refund(
                     &tx_cancel,
@@ -388,7 +388,7 @@ async fn run_until_internal(
                 match select(refund_tx_seen, punish_tx_finalised).await {
                     Either::Left((published_refund_tx, _)) => {
                         let spend_key = extract_monero_private_key(
-                            published_refund_tx,
+                            published_refund_tx?,
                             tx_refund,
                             state3.s_a,
                             state3.a.clone(),

swap/src/protocol/bob/state.rs

@@ -480,7 +480,7 @@ impl State4 {
 
         let tx_redeem_candidate = bitcoin_wallet
             .watch_for_raw_transaction(tx_redeem.txid())
-            .await;
+            .await?;
 
         let tx_redeem_sig =
             tx_redeem.extract_signature_by_key(tx_redeem_candidate, self.b.public())?;

swap/tests/testutils/mod.rs

@@ -155,11 +155,7 @@ impl TestContext {
         let swap_handle = self.alice_swap_handle.recv().await.unwrap();
         let state = swap_handle.await.unwrap();
 
-        assert!(
-            matches!(state, AliceState::XmrRefunded),
-            "Alice state is not XmrRefunded: {}",
-            state
-        );
+        assert!(matches!(state, AliceState::XmrRefunded));
 
         self.alice_bitcoin_wallet
             .sync_wallet()