From 180e778df973241e7bb2eb75dfee21a13eaec8d3 Mon Sep 17 00:00:00 2001 From: rishflab Date: Mon, 15 Feb 2021 16:13:29 +1100 Subject: [PATCH] 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 | 19 ++++++----- swap/src/bitcoin/wallet.rs | 56 ++++++++++++++------------------ swap/src/protocol/alice/steps.rs | 9 ++--- swap/src/protocol/alice/swap.rs | 4 +-- swap/src/protocol/bob/state.rs | 2 +- swap/tests/testutils/mod.rs | 6 +--- 6 files changed, 43 insertions(+), 53 deletions(-) diff --git a/swap/src/bitcoin.rs b/swap/src/bitcoin.rs index 602eaae9..a2dbca29 100644 --- a/swap/src/bitcoin.rs +++ b/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; } #[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; } #[async_trait] pub trait TransactionBlockHeight { - async fn transaction_block_height(&self, txid: Txid) -> BlockHeight; + async fn transaction_block_height(&self, txid: Txid) -> Result; } #[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(client: &B, target: BlockHeight) +pub async fn poll_until_block_height_is_gte(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( @@ -277,8 +278,8 @@ pub async fn current_epoch( 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( 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(()) } diff --git a/swap/src/bitcoin/wallet.rs b/swap/src/bitcoin/wallet.rs index f37a4f69..9615d180 100644 --- a/swap/src/bitcoin/wallet.rs +++ b/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>>, 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 { 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 { + 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::() - .map_err(Error::Parse)?; + .map_err(|err| backoff::Error::Permanent(Error::Parse(err)))?; Result::<_, backoff::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 { + let url = self.http_url.join(&format!("tx/{}/status", txid))?; + #[derive(Serialize, Deserialize, Debug, Clone)] struct TransactionStatus { block_height: Option, 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>::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); diff --git a/swap/src/protocol/alice/steps.rs b/swap/src/protocol/alice/steps.rs index bd1f9a2a..680b4f2b 100644 --- a/swap/src/protocol/alice/steps.rs +++ b/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?))), } } diff --git a/swap/src/protocol/alice/swap.rs b/swap/src/protocol/alice/swap.rs index 67875db7..7d8129d0 100644 --- a/swap/src/protocol/alice/swap.rs +++ b/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(), diff --git a/swap/src/protocol/bob/state.rs b/swap/src/protocol/bob/state.rs index 17fafa2f..aa32b5e4 100644 --- a/swap/src/protocol/bob/state.rs +++ b/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())?; diff --git a/swap/tests/testutils/mod.rs b/swap/tests/testutils/mod.rs index 8797be6d..96e0589c 100644 --- a/swap/tests/testutils/mod.rs +++ b/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()