From 5d4cf40831d6d08baebea3d7d98ff92fc907739c Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Fri, 5 Mar 2021 16:31:10 +1100 Subject: [PATCH 1/7] Fix comparison of Monero confirmations --- swap/src/monero/wallet.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/swap/src/monero/wallet.rs b/swap/src/monero/wallet.rs index 4f77d4d2..c667d2bb 100644 --- a/swap/src/monero/wallet.rs +++ b/swap/src/monero/wallet.rs @@ -193,7 +193,7 @@ impl Wallet { })); } - if proof.confirmations > confirmations.load(Ordering::SeqCst) { + if proof.confirmations >= confirmations.load(Ordering::SeqCst) { confirmations.store(proof.confirmations, Ordering::SeqCst); let txid = &transfer_proof.tx_hash.0; From 4883e23dd8f46536b51488a51535ecd589abcf7b Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Fri, 5 Mar 2021 16:07:13 +1100 Subject: [PATCH 2/7] Tell the user for how many confirmations we are waiting Without this, the user has no idea for how long the program is waiting. --- swap/src/bitcoin/wallet.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/swap/src/bitcoin/wallet.rs b/swap/src/bitcoin/wallet.rs index 7bc612c4..ff125f0f 100644 --- a/swap/src/bitcoin/wallet.rs +++ b/swap/src/bitcoin/wallet.rs @@ -179,7 +179,7 @@ impl Wallet { format!("Failed to broadcast Bitcoin {} transaction {}", kind, txid) })?; - tracing::info!("Published Bitcoin {} transaction as {}", txid, kind); + tracing::info!(%txid, "Published Bitcoin {} transaction", kind); Ok(txid) } @@ -281,7 +281,10 @@ impl Wallet { txid: Txid, execution_params: ExecutionParams, ) -> Result<()> { - tracing::debug!("waiting for tx finality: {}", txid); + let conf_target = execution_params.bitcoin_finality_confirmations; + + tracing::info!(%txid, "Waiting for {} confirmation{} of Bitcoin transaction", conf_target, if conf_target > 1 { "s" } else { "" }); + // Divide by 4 to not check too often yet still be aware of the new block early // on. let mut interval = interval(execution_params.bitcoin_avg_block_time / 4); @@ -289,15 +292,17 @@ impl Wallet { loop { 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?; tracing::debug!("latest_block_height: {:?}", block_height); + if let Some(confirmations) = block_height.checked_sub( tx_block_height .checked_sub(BlockHeight::new(1)) .expect("transaction must be included in block with height >= 1"), ) { - tracing::debug!("confirmations: {:?}", confirmations); - if u32::from(confirmations) >= execution_params.bitcoin_finality_confirmations { + tracing::debug!(%txid, "confirmations: {:?}", confirmations); + if u32::from(confirmations) >= conf_target { break; } } From 13c4d29d40a4e9a3db4966983f32d6a7623db8a2 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Fri, 5 Mar 2021 16:11:12 +1100 Subject: [PATCH 3/7] Tell the user immediately how many confirmations we expect Without this, the user doesn't see a message before the first confirmation. --- swap/src/monero/wallet.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/swap/src/monero/wallet.rs b/swap/src/monero/wallet.rs index c667d2bb..11514ff9 100644 --- a/swap/src/monero/wallet.rs +++ b/swap/src/monero/wallet.rs @@ -158,8 +158,12 @@ impl Wallet { public_view_key: PublicViewKey, transfer_proof: TransferProof, expected_amount: Amount, - expected_confirmations: u32, + conf_target: u32, ) -> Result<(), InsufficientFunds> { + let txid = &transfer_proof.tx_hash.0; + + tracing::info!(%txid, "Waiting for {} confirmation{} of Monero transaction", conf_target, if conf_target > 1 { "s" } else { "" }); + enum Error { TxNotFound, InsufficientConfirmations, @@ -196,11 +200,10 @@ impl Wallet { if proof.confirmations >= confirmations.load(Ordering::SeqCst) { confirmations.store(proof.confirmations, Ordering::SeqCst); - let txid = &transfer_proof.tx_hash.0; - info!(%txid, "Monero lock tx has {} out of {} confirmations", proof.confirmations, expected_confirmations); + info!(%txid, "Monero lock tx has {} out of {} confirmations", proof.confirmations, conf_target); } - if proof.confirmations < expected_confirmations { + if proof.confirmations < conf_target { return Err(backoff::Error::Transient(Error::InsufficientConfirmations)); } From c2329b19a2597936881dea67f7508ba4417708ed Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Fri, 5 Mar 2021 16:24:02 +1100 Subject: [PATCH 4/7] Tell the user more about the monero lock transaction First, we tell the user that we are now waiting for Alice to lock the monero. Additionally, we tell them once we received the transfer proof which will lead directly into the "waiting for confirmations" function. --- swap/src/monero.rs | 6 ++++++ swap/src/monero/wallet.rs | 2 +- swap/src/protocol/bob/swap.rs | 11 +++++++++-- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/swap/src/monero.rs b/swap/src/monero.rs index d086cfa7..77b7dae9 100644 --- a/swap/src/monero.rs +++ b/swap/src/monero.rs @@ -175,6 +175,12 @@ impl From for String { } } +impl fmt::Display for TxHash { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.0) + } +} + #[derive(Debug, Clone, Copy, thiserror::Error)] #[error("transaction does not pay enough: expected {expected}, got {actual}")] pub struct InsufficientFunds { diff --git a/swap/src/monero/wallet.rs b/swap/src/monero/wallet.rs index 11514ff9..44f1b8f8 100644 --- a/swap/src/monero/wallet.rs +++ b/swap/src/monero/wallet.rs @@ -160,7 +160,7 @@ impl Wallet { expected_amount: Amount, conf_target: u32, ) -> Result<(), InsufficientFunds> { - let txid = &transfer_proof.tx_hash.0; + let txid = &transfer_proof.tx_hash(); tracing::info!(%txid, "Waiting for {} confirmation{} of Monero transaction", conf_target, if conf_target > 1 { "s" } else { "" }); diff --git a/swap/src/protocol/bob/swap.rs b/swap/src/protocol/bob/swap.rs index c6eefb7a..30f84ec6 100644 --- a/swap/src/protocol/bob/swap.rs +++ b/swap/src/protocol/bob/swap.rs @@ -133,16 +133,23 @@ async fn run_until_internal( // block 0 once we create the redeem wallet. let monero_wallet_restore_blockheight = monero_wallet.block_height().await?; + tracing::info!("Waiting for Alice to lock Monero"); + select! { transfer_proof = transfer_proof_watcher => { - let transfer_proof = transfer_proof?; + let transfer_proof = transfer_proof?.tx_lock_proof; + + tracing::info!(txid = %transfer_proof.tx_hash(), "Alice locked Monero"); + BobState::XmrLockProofReceived { state: state3, - lock_transfer_proof: transfer_proof.tx_lock_proof, + lock_transfer_proof: transfer_proof, monero_wallet_restore_blockheight } }, _ = cancel_timelock_expires => { + tracing::info!("Alice took too long to lock Monero, cancelling the swap"); + let state4 = state3.cancel(); BobState::CancelTimelockExpired(state4) } From 418ad7089d64f1bc4fa3b1ed71307f5e3b63b0df Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Fri, 5 Mar 2021 16:45:50 +1100 Subject: [PATCH 5/7] Make tests more readable by following arrange-act-assert --- swap/src/bitcoin/wallet.rs | 37 +++++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/swap/src/bitcoin/wallet.rs b/swap/src/bitcoin/wallet.rs index ff125f0f..cc305253 100644 --- a/swap/src/bitcoin/wallet.rs +++ b/swap/src/bitcoin/wallet.rs @@ -225,7 +225,7 @@ impl Wallet { } pub async fn get_block_height(&self) -> Result { - let url = blocks_tip_height_url(&self.http_url)?; + let url = make_blocks_tip_height_url(&self.http_url)?; let height = retry(ConstantBackoff::new(Duration::from_secs(1)), || async { let height = reqwest::Client::new() .request(Method::GET, url.clone()) @@ -246,7 +246,7 @@ impl Wallet { } pub async fn transaction_block_height(&self, txid: Txid) -> Result { - let url = tx_status_url(txid, &self.http_url)?; + let url = make_tx_status_url(&self.http_url, txid)?; #[derive(Serialize, Deserialize, Debug, Clone)] struct TransactionStatus { block_height: Option, @@ -320,37 +320,42 @@ impl Wallet { } } -fn tx_status_url(txid: Txid, base_url: &Url) -> Result { +fn make_tx_status_url(base_url: &Url, txid: Txid) -> Result { let url = base_url.join(&format!("tx/{}/status", txid))?; + Ok(url) } -fn blocks_tip_height_url(base_url: &Url) -> Result { +fn make_blocks_tip_height_url(base_url: &Url) -> Result { let url = base_url.join("blocks/tip/height")?; + Ok(url) } #[cfg(test)] mod tests { - use crate::bitcoin::wallet::{blocks_tip_height_url, tx_status_url}; - use crate::bitcoin::Txid; + use super::*; use crate::cli::config::DEFAULT_ELECTRUM_HTTP_URL; - use reqwest::Url; #[test] fn create_tx_status_url_from_default_base_url_success() { - let txid: Txid = Txid::default(); - let base_url = Url::parse(DEFAULT_ELECTRUM_HTTP_URL).expect("Could not parse url"); - let url = tx_status_url(txid, &base_url).expect("Could not create url"); - let expected = format!("https://blockstream.info/testnet/api/tx/{}/status", txid); - assert_eq!(url.as_str(), expected); + let base_url = DEFAULT_ELECTRUM_HTTP_URL.parse().unwrap(); + let txid = Txid::default; + + let url = make_tx_status_url(&base_url, txid()).unwrap(); + + assert_eq!(url.as_str(), "https://blockstream.info/testnet/api/tx/0000000000000000000000000000000000000000000000000000000000000000/status"); } #[test] fn create_block_tip_height_url_from_default_base_url_success() { - let base_url = Url::parse(DEFAULT_ELECTRUM_HTTP_URL).expect("Could not parse url"); - let url = blocks_tip_height_url(&base_url).expect("Could not create url"); - let expected = "https://blockstream.info/testnet/api/blocks/tip/height"; - assert_eq!(url.as_str(), expected); + let base_url = DEFAULT_ELECTRUM_HTTP_URL.parse().unwrap(); + + let url = make_blocks_tip_height_url(&base_url).unwrap(); + + assert_eq!( + url.as_str(), + "https://blockstream.info/testnet/api/blocks/tip/height" + ); } } From e9d7d9299c38ce1b17a72f96ea694025c56de3bd Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Fri, 5 Mar 2021 16:49:49 +1100 Subject: [PATCH 6/7] Simplify the GET request to the tx status URL --- swap/src/bitcoin/wallet.rs | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/swap/src/bitcoin/wallet.rs b/swap/src/bitcoin/wallet.rs index cc305253..d019d578 100644 --- a/swap/src/bitcoin/wallet.rs +++ b/swap/src/bitcoin/wallet.rs @@ -246,25 +246,20 @@ impl Wallet { } pub async fn transaction_block_height(&self, txid: Txid) -> Result { - let url = make_tx_status_url(&self.http_url, txid)?; + let status_url = make_tx_status_url(&self.http_url, txid)?; + #[derive(Serialize, Deserialize, Debug, Clone)] struct TransactionStatus { block_height: Option, confirmed: bool, } let height = retry(ConstantBackoff::new(Duration::from_secs(1)), || async { - let resp = reqwest::Client::new() - .request(Method::GET, url.clone()) - .send() + let block_height = reqwest::get(status_url.clone()) .await - .map_err(|err| backoff::Error::Transient(Error::Io(err)))?; - - let tx_status: TransactionStatus = resp - .json() + .map_err(|err| backoff::Error::Transient(Error::Io(err)))? + .json::() .await - .map_err(|err| backoff::Error::Permanent(Error::JsonDeserialization(err)))?; - - let block_height = tx_status + .map_err(|err| backoff::Error::Permanent(Error::JsonDeserialization(err)))? .block_height .ok_or(backoff::Error::Transient(Error::NotYetMined))?; From 08923a14f3d56cedff90d46f97ecfbdf91c9241a Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Fri, 5 Mar 2021 17:06:17 +1100 Subject: [PATCH 7/7] Simplify GET request for block tip height --- swap/src/bitcoin/wallet.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/swap/src/bitcoin/wallet.rs b/swap/src/bitcoin/wallet.rs index d019d578..b73dd666 100644 --- a/swap/src/bitcoin/wallet.rs +++ b/swap/src/bitcoin/wallet.rs @@ -12,7 +12,7 @@ use bdk::electrum_client::{self, Client, ElectrumApi}; use bdk::keys::DerivableKey; use bdk::{FeeRate, KeychainKind}; use bitcoin::Script; -use reqwest::{Method, Url}; +use reqwest::Url; use serde::{Deserialize, Serialize}; use std::path::Path; use std::sync::Arc; @@ -226,10 +226,9 @@ impl Wallet { pub async fn get_block_height(&self) -> Result { let url = make_blocks_tip_height_url(&self.http_url)?; + let height = retry(ConstantBackoff::new(Duration::from_secs(1)), || async { - let height = reqwest::Client::new() - .request(Method::GET, url.clone()) - .send() + let height = reqwest::get(url.clone()) .await .map_err(Error::Io)? .text()