From 40dcf0355a9620224a58d9d791cb94cfe70b2001 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Fri, 26 Feb 2021 15:45:23 +1100 Subject: [PATCH 01/19] Simplify `Transfer::transfer` return type We never use the fee returned from this function, remove it. --- swap/src/monero.rs | 2 +- swap/src/monero/wallet.rs | 6 ++---- swap/src/protocol/alice/steps.rs | 2 +- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/swap/src/monero.rs b/swap/src/monero.rs index 3e7f69ed..90224948 100644 --- a/swap/src/monero.rs +++ b/swap/src/monero.rs @@ -190,7 +190,7 @@ pub trait Transfer { public_spend_key: PublicKey, public_view_key: PublicViewKey, amount: Amount, - ) -> Result<(TransferProof, Amount)>; + ) -> Result; } #[async_trait] diff --git a/swap/src/monero/wallet.rs b/swap/src/monero/wallet.rs index a21ebe6b..b37eba55 100644 --- a/swap/src/monero/wallet.rs +++ b/swap/src/monero/wallet.rs @@ -82,7 +82,7 @@ impl Transfer for Wallet { public_spend_key: PublicKey, public_view_key: PublicViewKey, amount: Amount, - ) -> Result<(TransferProof, Amount)> { + ) -> Result { let destination_address = Address::standard(self.network, public_spend_key, public_view_key.into()); @@ -97,12 +97,10 @@ impl Transfer for Wallet { tracing::info!("Monero tx broadcasted!, tx hash: {:?}", tx_hash); let tx_key = PrivateKey::from_str(&res.tx_key)?; - let fee = Amount::from_piconero(res.fee); - let transfer_proof = TransferProof::new(tx_hash, tx_key); tracing::debug!(" Transfer proof: {:?}", transfer_proof); - Ok((transfer_proof, fee)) + Ok(transfer_proof) } } diff --git a/swap/src/protocol/alice/steps.rs b/swap/src/protocol/alice/steps.rs index b77479c2..9544c3f0 100644 --- a/swap/src/protocol/alice/steps.rs +++ b/swap/src/protocol/alice/steps.rs @@ -69,7 +69,7 @@ where let public_spend_key = S_a + state3.S_b_monero; let public_view_key = state3.v.public(); - let (transfer_proof, _) = monero_wallet + let transfer_proof = monero_wallet .transfer(public_spend_key, public_view_key, state3.xmr) .await?; From a0e7c6ecf7c1afcf4d4f618e275226b63e220bd3 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Fri, 26 Feb 2021 15:53:25 +1100 Subject: [PATCH 02/19] Don't Arc the AtomicU32 We never clone this type, there is no need to wrap it in an `Arc`. --- swap/src/monero/wallet.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/swap/src/monero/wallet.rs b/swap/src/monero/wallet.rs index b37eba55..89e9be5d 100644 --- a/swap/src/monero/wallet.rs +++ b/swap/src/monero/wallet.rs @@ -12,11 +12,7 @@ use monero_rpc::{ wallet, wallet::{BlockHeight, Refreshed}, }; -use std::{ - str::FromStr, - sync::{atomic::Ordering, Arc}, - time::Duration, -}; +use std::{str::FromStr, sync::atomic::Ordering, time::Duration}; use tokio::sync::Mutex; use tracing::info; use url::Url; @@ -201,7 +197,7 @@ impl WatchForTransfer for Wallet { let address = Address::standard(self.network, public_spend_key, public_view_key.into()); - let confirmations = Arc::new(AtomicU32::new(0u32)); + let confirmations = AtomicU32::new(0u32); let res = retry(ConstantBackoff::new(Duration::from_secs(1)), || async { // NOTE: Currently, this is conflicting IO errors with the transaction not being From b8df4a3145b5fb257b731cca09fde15aa33a1c7d Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Fri, 26 Feb 2021 16:22:24 +1100 Subject: [PATCH 03/19] Inline tracing configuration for swap_cli This allows us to configure the presentation separately from the ASB. --- swap/src/bin/swap_cli.rs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/swap/src/bin/swap_cli.rs b/swap/src/bin/swap_cli.rs index c2a9ed4c..f266460a 100644 --- a/swap/src/bin/swap_cli.rs +++ b/swap/src/bin/swap_cli.rs @@ -34,10 +34,9 @@ use swap::{ bob::{cancel::CancelError, Builder}, }, seed::Seed, - trace::init_tracing, }; -use tracing::{debug, error, info, warn}; -use tracing_subscriber::filter::LevelFilter; +use tracing::{debug, error, info, warn, Level}; +use tracing_subscriber::FmtSubscriber; use uuid::Uuid; #[macro_use] @@ -47,7 +46,15 @@ const MONERO_BLOCKCHAIN_MONITORING_WALLET_NAME: &str = "swap-tool-blockchain-mon #[tokio::main] async fn main() -> Result<()> { - init_tracing(LevelFilter::DEBUG).expect("initialize tracing"); + let is_terminal = atty::is(atty::Stream::Stderr); + let subscriber = FmtSubscriber::builder() + .with_env_filter(format!("swap={}", Level::DEBUG)) + .with_writer(std::io::stderr) + .with_ansi(is_terminal) + .with_target(false) + .finish(); + + tracing::subscriber::set_global_default(subscriber)?; let opt = Arguments::from_args(); From 7387884e6d24498d4a88e9bc106f8a9787711abc Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Fri, 26 Feb 2021 17:01:36 +1100 Subject: [PATCH 04/19] Move log messages to the appropriate abstraction layer Log messages are ideally as close to the functionality they are talking about, otherwise we might end up repeating ourselves on several callsites or the log messages gets outdated if the behaviour changes. --- swap/src/bitcoin/wallet.rs | 13 +++++++++---- swap/src/protocol/alice/behaviour.rs | 3 +-- swap/src/protocol/alice/quote_response.rs | 5 ++++- swap/src/protocol/bob.rs | 7 ++----- swap/src/protocol/bob/event_loop.rs | 1 - swap/src/protocol/bob/execution_setup.rs | 2 ++ swap/src/protocol/bob/quote_request.rs | 11 ++++++----- swap/src/protocol/bob/swap.rs | 6 ++++-- 8 files changed, 28 insertions(+), 20 deletions(-) diff --git a/swap/src/bitcoin/wallet.rs b/swap/src/bitcoin/wallet.rs index 1e018b49..5fcf33cf 100644 --- a/swap/src/bitcoin/wallet.rs +++ b/swap/src/bitcoin/wallet.rs @@ -179,10 +179,15 @@ impl SignTxLock for Wallet { #[async_trait] impl BroadcastSignedTransaction for Wallet { async fn broadcast_signed_transaction(&self, transaction: Transaction) -> Result { - tracing::debug!("attempting to broadcast tx: {}", transaction.txid()); - self.inner.lock().await.broadcast(transaction.clone())?; - tracing::info!("Bitcoin tx broadcasted! TXID = {}", transaction.txid()); - Ok(transaction.txid()) + let txid = transaction.txid(); + + self.inner + .lock() + .await + .broadcast(transaction) + .with_context(|| format!("failed to broadcast transaction {}", txid))?; + + Ok(txid) } } diff --git a/swap/src/protocol/alice/behaviour.rs b/swap/src/protocol/alice/behaviour.rs index 8aa59904..ae168122 100644 --- a/swap/src/protocol/alice/behaviour.rs +++ b/swap/src/protocol/alice/behaviour.rs @@ -10,7 +10,7 @@ use crate::{ }; use anyhow::{Error, Result}; use libp2p::{request_response::ResponseChannel, NetworkBehaviour, PeerId}; -use tracing::{debug, info}; +use tracing::debug; #[derive(Debug)] pub enum OutEvent { @@ -121,7 +121,6 @@ impl Behaviour { quote_response: QuoteResponse, ) -> Result<()> { self.quote_response.send(channel, quote_response)?; - info!("Sent quote response"); Ok(()) } diff --git a/swap/src/protocol/alice/quote_response.rs b/swap/src/protocol/alice/quote_response.rs index 878dd530..b9476554 100644 --- a/swap/src/protocol/alice/quote_response.rs +++ b/swap/src/protocol/alice/quote_response.rs @@ -59,7 +59,10 @@ impl From> for OutEvent { RequestResponseEvent::OutboundFailure { error, .. } => { OutEvent::Failure(anyhow!("Outbound failure: {:?}", error)) } - RequestResponseEvent::ResponseSent { .. } => OutEvent::ResponseSent, + RequestResponseEvent::ResponseSent { peer, .. } => { + tracing::debug!("successfully sent quote response to {}", peer); + OutEvent::ResponseSent + } } } } diff --git a/swap/src/protocol/bob.rs b/swap/src/protocol/bob.rs index 4a3e8d38..69d155af 100644 --- a/swap/src/protocol/bob.rs +++ b/swap/src/protocol/bob.rs @@ -15,7 +15,7 @@ use crate::{ use anyhow::{bail, Error, Result}; use libp2p::{core::Multiaddr, identity::Keypair, NetworkBehaviour, PeerId}; use std::sync::Arc; -use tracing::{debug, info}; +use tracing::debug; use uuid::Uuid; pub use self::{ @@ -259,8 +259,7 @@ pub struct Behaviour { impl Behaviour { /// Sends a quote request to Alice to retrieve the rate. pub fn send_quote_request(&mut self, alice: PeerId, quote_request: QuoteRequest) { - let _id = self.quote_request.send(alice, quote_request); - info!("Requesting quote from: {}", alice); + let _ = self.quote_request.send(alice, quote_request); } pub fn start_execution_setup( @@ -271,10 +270,8 @@ impl Behaviour { ) { self.execution_setup .run(alice_peer_id, state0, bitcoin_wallet); - info!("Start execution setup with {}", alice_peer_id); } - /// Sends Bob's fourth message to Alice. pub fn send_encrypted_signature( &mut self, alice: PeerId, diff --git a/swap/src/protocol/bob/event_loop.rs b/swap/src/protocol/bob/event_loop.rs index 57a99675..a5126fc4 100644 --- a/swap/src/protocol/bob/event_loop.rs +++ b/swap/src/protocol/bob/event_loop.rs @@ -72,7 +72,6 @@ impl EventLoopHandle { /// Dials other party and wait for the connection to be established. /// Do nothing if we are already connected pub async fn dial(&mut self) -> Result<()> { - debug!("Attempt to dial Alice"); let _ = self.dial_alice.send(()).await?; self.conn_established diff --git a/swap/src/protocol/bob/execution_setup.rs b/swap/src/protocol/bob/execution_setup.rs index aa301b47..39f265a7 100644 --- a/swap/src/protocol/bob/execution_setup.rs +++ b/swap/src/protocol/bob/execution_setup.rs @@ -72,6 +72,8 @@ impl Behaviour { ) { self.inner .do_protocol_dialer(alice, move |mut substream| async move { + tracing::debug!("Starting execution setup with {}", alice); + substream .write_message( &serde_cbor::to_vec(&state0.next_message()) diff --git a/swap/src/protocol/bob/quote_request.rs b/swap/src/protocol/bob/quote_request.rs index 4262a6bf..d3fabb74 100644 --- a/swap/src/protocol/bob/quote_request.rs +++ b/swap/src/protocol/bob/quote_request.rs @@ -36,6 +36,11 @@ pub struct Behaviour { impl Behaviour { pub fn send(&mut self, alice: PeerId, quote_request: QuoteRequest) -> Result { + debug!( + "Requesting quote for {} from {}", + quote_request.btc_amount, alice + ); + let id = self.rr.send_request(&alice, quote_request); Ok(id) @@ -67,13 +72,9 @@ impl From> for OutEvent { .. } => OutEvent::Failure(anyhow!("Bob should never get a request from Alice")), RequestResponseEvent::Message { - peer, message: RequestResponseMessage::Response { response, .. }, .. - } => { - debug!("Received quote response from {}", peer); - OutEvent::MsgReceived(response) - } + } => OutEvent::MsgReceived(response), RequestResponseEvent::InboundFailure { error, .. } => { OutEvent::Failure(anyhow!("Inbound failure: {:?}", error)) } diff --git a/swap/src/protocol/bob/swap.rs b/swap/src/protocol/bob/swap.rs index 29cac4e8..76bf0890 100644 --- a/swap/src/protocol/bob/swap.rs +++ b/swap/src/protocol/bob/swap.rs @@ -389,12 +389,14 @@ pub async fn request_quote_and_setup( .send_quote_request(QuoteRequest { btc_amount }) .await?; - let quote_response = event_loop_handle.recv_quote_response().await?; + let xmr_amount = event_loop_handle.recv_quote_response().await?.xmr_amount; + + tracing::info!("Quote for {} is {}", btc_amount, xmr_amount); let state0 = State0::new( &mut OsRng, btc_amount, - quote_response.xmr_amount, + xmr_amount, execution_params.bitcoin_cancel_timelock, execution_params.bitcoin_punish_timelock, bitcoin_refund_address, From bbbe5f7ae8c6ccc8b9f0e6ae6a828051a66d8ad8 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Fri, 26 Feb 2021 17:03:07 +1100 Subject: [PATCH 05/19] Demote / promote log messages to their appropriate level --- swap/src/bin/swap_cli.rs | 10 +++++----- swap/src/cli/config.rs | 4 ++-- swap/src/monero/wallet.rs | 19 +++++++++++-------- swap/src/protocol/bob/event_loop.rs | 4 ++-- swap/src/protocol/bob/state.rs | 2 +- swap/src/protocol/bob/swap.rs | 6 +++--- swap/src/seed.rs | 4 ++-- 7 files changed, 26 insertions(+), 23 deletions(-) diff --git a/swap/src/bin/swap_cli.rs b/swap/src/bin/swap_cli.rs index f266460a..1dcd6959 100644 --- a/swap/src/bin/swap_cli.rs +++ b/swap/src/bin/swap_cli.rs @@ -35,7 +35,7 @@ use swap::{ }, seed::Seed, }; -use tracing::{debug, error, info, warn, Level}; +use tracing::{debug, error, warn, Level}; use tracing_subscriber::FmtSubscriber; use uuid::Uuid; @@ -63,8 +63,8 @@ async fn main() -> Result<()> { None => Config::testnet(), }; - info!( - "Database and Seed will be stored in directory: {}", + debug!( + "Database and seed will be stored in {}", config.data.dir.display() ); @@ -240,7 +240,7 @@ async fn main() -> Result<()> { cancel_result = cancel => { match cancel_result? { Ok((txid, _)) => { - info!("Cancel transaction successfully published with id {}", txid) + debug!("Cancel transaction successfully published with id {}", txid) } Err(CancelError::CancelTimelockNotExpiredYet) => error!( "The Cancel Transaction cannot be published yet, \ @@ -353,7 +353,7 @@ async fn init_wallets( monero_wallet_rpc_url ))?; - info!( + debug!( "Created Monero wallet for blockchain monitoring with name {}", MONERO_BLOCKCHAIN_MONITORING_WALLET_NAME ); diff --git a/swap/src/cli/config.rs b/swap/src/cli/config.rs index 2efa1b35..ce963a25 100644 --- a/swap/src/cli/config.rs +++ b/swap/src/cli/config.rs @@ -6,7 +6,7 @@ use std::{ ffi::OsStr, path::{Path, PathBuf}, }; -use tracing::info; +use tracing::debug; use url::Url; pub const DEFAULT_ELECTRUM_HTTP_URL: &str = "https://blockstream.info/testnet/api/"; @@ -66,7 +66,7 @@ pub struct ConfigNotInitialized {} pub fn read_config(config_path: PathBuf) -> Result> { if config_path.exists() { - info!( + debug!( "Using config file at default path: {}", config_path.display() ); diff --git a/swap/src/monero/wallet.rs b/swap/src/monero/wallet.rs index 89e9be5d..b79ced60 100644 --- a/swap/src/monero/wallet.rs +++ b/swap/src/monero/wallet.rs @@ -89,14 +89,17 @@ impl Transfer for Wallet { .transfer(0, amount.as_piconero(), &destination_address.to_string()) .await?; - let tx_hash = TxHash(res.tx_hash); - tracing::info!("Monero tx broadcasted!, tx hash: {:?}", tx_hash); - let tx_key = PrivateKey::from_str(&res.tx_key)?; - - let transfer_proof = TransferProof::new(tx_hash, tx_key); - tracing::debug!(" Transfer proof: {:?}", transfer_proof); - - Ok(transfer_proof) + tracing::debug!( + "sent transfer of {} to {} in {}", + amount, + public_spend_key, + res.tx_hash + ); + + Ok(TransferProof::new( + TxHash(res.tx_hash), + PrivateKey::from_str(&res.tx_key)?, + )) } } diff --git a/swap/src/protocol/bob/event_loop.rs b/swap/src/protocol/bob/event_loop.rs index a5126fc4..8dcf3e89 100644 --- a/swap/src/protocol/bob/event_loop.rs +++ b/swap/src/protocol/bob/event_loop.rs @@ -12,7 +12,7 @@ use futures::FutureExt; use libp2p::{core::Multiaddr, PeerId}; use std::{convert::Infallible, sync::Arc}; use tokio::sync::mpsc::{Receiver, Sender}; -use tracing::{debug, error, info}; +use tracing::{debug, error, info, trace}; #[derive(Debug)] pub struct Channels { @@ -200,7 +200,7 @@ impl EventLoop { if option.is_some() { let peer_id = self.alice_peer_id; if self.swarm.pt.is_connected(&peer_id) { - debug!("Already connected to Alice: {}", peer_id); + trace!("Already connected to Alice at {}", peer_id); let _ = self.conn_established.send(peer_id).await; } else { info!("dialing alice: {}", peer_id); diff --git a/swap/src/protocol/bob/state.rs b/swap/src/protocol/bob/state.rs index 10cdc378..83e75eca 100644 --- a/swap/src/protocol/bob/state.rs +++ b/swap/src/protocol/bob/state.rs @@ -275,7 +275,7 @@ impl State2 { { let signed_tx_lock = bitcoin_wallet.sign_tx_lock(self.tx_lock.clone()).await?; - tracing::info!("{}", self.tx_lock.txid()); + tracing::debug!("locking BTC in transaction {}", self.tx_lock.txid()); let _ = bitcoin_wallet .broadcast_signed_transaction(signed_tx_lock) .await?; diff --git a/swap/src/protocol/bob/swap.rs b/swap/src/protocol/bob/swap.rs index 76bf0890..5f499ea8 100644 --- a/swap/src/protocol/bob/swap.rs +++ b/swap/src/protocol/bob/swap.rs @@ -12,7 +12,7 @@ use async_recursion::async_recursion; use rand::rngs::OsRng; use std::sync::Arc; use tokio::select; -use tracing::info; +use tracing::{trace, warn}; use uuid::Uuid; pub fn is_complete(state: &BobState) -> bool { @@ -61,7 +61,7 @@ async fn run_until_internal( swap_id: Uuid, execution_params: ExecutionParams, ) -> Result { - info!("Current state: {}", state); + trace!("Current state: {}", state); if is_target_state(&state) { Ok(state) } else { @@ -186,7 +186,7 @@ async fn run_until_internal( match state4? { Ok(state4) => BobState::XmrLocked(state4), Err(InsufficientFunds {..}) => { - info!("The other party has locked insufficient Monero funds! Waiting for refund..."); + warn!("The other party has locked insufficient Monero funds! Waiting for refund..."); state.wait_for_cancel_timelock_to_expire(bitcoin_wallet.as_ref()).await?; let state4 = state.cancel(); BobState::CancelTimelockExpired(state4) diff --git a/swap/src/seed.rs b/swap/src/seed.rs index 48c9d039..ebcf6135 100644 --- a/swap/src/seed.rs +++ b/swap/src/seed.rs @@ -45,7 +45,7 @@ impl Seed { return Self::from_file(&file_path); } - tracing::info!("No seed file found, creating at: {}", file_path.display()); + tracing::debug!("No seed file found, creating at: {}", file_path.display()); let random_seed = Seed::random()?; random_seed.write_to(file_path.to_path_buf())?; @@ -61,7 +61,7 @@ impl Seed { let contents = fs::read_to_string(file)?; let pem = pem::parse(contents)?; - tracing::info!("Read in seed from file: {}", file.display()); + tracing::trace!("Read in seed from {}", file.display()); Self::from_pem(pem) } From 3d2d447fba9b304c4cad9790eb5f6ff683a7949b Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Fri, 26 Feb 2021 17:03:38 +1100 Subject: [PATCH 06/19] Improve error message YMMV but I think this sounds better. --- swap/src/protocol/alice/quote_response.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/swap/src/protocol/alice/quote_response.rs b/swap/src/protocol/alice/quote_response.rs index b9476554..474982a1 100644 --- a/swap/src/protocol/alice/quote_response.rs +++ b/swap/src/protocol/alice/quote_response.rs @@ -85,7 +85,9 @@ impl Behaviour { ) -> Result<()> { self.rr .send_response(channel, msg) - .map_err(|_| anyhow!("Sending quote response failed")) + .map_err(|_| anyhow!("failed to send quote response"))?; + + Ok(()) } } From 6b74761e34f00d6049e6848dd514301e5dfa8e2a Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Fri, 26 Feb 2021 17:03:58 +1100 Subject: [PATCH 07/19] Remove tracing context The swap_cli can only do one swap at a time, no need for the swap ID span. --- swap/src/protocol/bob/swap.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/swap/src/protocol/bob/swap.rs b/swap/src/protocol/bob/swap.rs index 5f499ea8..cb412c2a 100644 --- a/swap/src/protocol/bob/swap.rs +++ b/swap/src/protocol/bob/swap.rs @@ -30,7 +30,6 @@ pub async fn run(swap: bob::Swap) -> Result { run_until(swap, is_complete).await } -#[tracing::instrument(name = "swap", skip(swap,is_target_state), fields(id = %swap.swap_id))] pub async fn run_until( swap: bob::Swap, is_target_state: fn(&BobState) -> bool, From 4e9e18646209d5ec4a8f745787271a31cebe8f3c Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Fri, 26 Feb 2021 17:04:51 +1100 Subject: [PATCH 08/19] Don't log things the user doesn't care about The user configured neither a Bitcoin wallet backend nor the monero-wallet-rpc so let's not tell them about it. Fixes #244. --- swap/src/bin/swap_cli.rs | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/swap/src/bin/swap_cli.rs b/swap/src/bin/swap_cli.rs index 1dcd6959..75bf94fd 100644 --- a/swap/src/bin/swap_cli.rs +++ b/swap/src/bin/swap_cli.rs @@ -325,13 +325,7 @@ async fn init_wallets( bitcoin_wallet .sync_wallet() .await - .expect("Could not sync btc wallet"); - - let bitcoin_balance = bitcoin_wallet.balance().await?; - info!( - "Connection to Bitcoin wallet succeeded, balance: {}", - bitcoin_balance - ); + .context("failed to sync balance of bitcoin wallet")?; let monero_wallet = monero::Wallet::new( monero_wallet_rpc_url.clone(), @@ -357,15 +351,12 @@ async fn init_wallets( "Created Monero wallet for blockchain monitoring with name {}", MONERO_BLOCKCHAIN_MONITORING_WALLET_NAME ); - } else { - info!( - "Opened Monero wallet for blockchain monitoring with name {}", - MONERO_BLOCKCHAIN_MONITORING_WALLET_NAME - ); } - let _test_wallet_connection = monero_wallet.block_height().await?; - info!("The Monero wallet RPC is set up correctly!"); + let _test_wallet_connection = monero_wallet + .block_height() + .await + .context("failed to validate connection to monero-wallet-rpc")?; Ok((bitcoin_wallet, monero_wallet)) } From b7c3524b4f53edbb8b31dee5915c9868cb722826 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Fri, 26 Feb 2021 17:05:19 +1100 Subject: [PATCH 09/19] Abort the eventloop if the dialling fails --- swap/src/protocol/bob/event_loop.rs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/swap/src/protocol/bob/event_loop.rs b/swap/src/protocol/bob/event_loop.rs index 8dcf3e89..b7e222a7 100644 --- a/swap/src/protocol/bob/event_loop.rs +++ b/swap/src/protocol/bob/event_loop.rs @@ -7,12 +7,12 @@ use crate::{ bob::{Behaviour, OutEvent, QuoteRequest, State0, State2}, }, }; -use anyhow::{anyhow, bail, Result}; +use anyhow::{anyhow, bail, Context, Result}; use futures::FutureExt; use libp2p::{core::Multiaddr, PeerId}; use std::{convert::Infallible, sync::Arc}; use tokio::sync::mpsc::{Receiver, Sender}; -use tracing::{debug, error, info, trace}; +use tracing::{debug, error, trace}; #[derive(Debug)] pub struct Channels { @@ -203,12 +203,8 @@ impl EventLoop { trace!("Already connected to Alice at {}", peer_id); let _ = self.conn_established.send(peer_id).await; } else { - info!("dialing alice: {}", peer_id); - if let Err(err) = libp2p::Swarm::dial(&mut self.swarm, &peer_id) { - error!("Could not dial alice: {}", err); - // TODO(Franck): If Dial fails then we should report it. - } - + debug!("Dialing alice at {}", peer_id); + libp2p::Swarm::dial(&mut self.swarm, &peer_id).context("failed to dial alice")?; } } }, From cbef577e2d8e51238352864e7d5fac73ecde0824 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Mon, 1 Mar 2021 11:24:27 +1100 Subject: [PATCH 10/19] Inform user that we are going to swap the remainder of the balance --- swap/src/bin/swap_cli.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/swap/src/bin/swap_cli.rs b/swap/src/bin/swap_cli.rs index 75bf94fd..4304de94 100644 --- a/swap/src/bin/swap_cli.rs +++ b/swap/src/bin/swap_cli.rs @@ -117,12 +117,15 @@ async fn main() -> Result<()> { } debug!("Received {}", bitcoin_wallet.balance().await?); + } else { + debug!( + "Still got {} left in wallet, swapping ...", + bitcoin_wallet.balance().await? + ); } let send_bitcoin = bitcoin_wallet.max_giveable(TxLock::script_size()).await?; - info!("Swapping {} ...", send_bitcoin); - let bob_factory = Builder::new( seed, db, From 06e3bccaa6eb2356542fe2b3224678e60af7df8a Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Mon, 1 Mar 2021 11:26:06 +1100 Subject: [PATCH 11/19] Don't print PeerId when requesting quote Bob always just talks to one party, the PeerId is just noise. --- swap/src/protocol/bob/quote_request.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/swap/src/protocol/bob/quote_request.rs b/swap/src/protocol/bob/quote_request.rs index d3fabb74..23d8dd3c 100644 --- a/swap/src/protocol/bob/quote_request.rs +++ b/swap/src/protocol/bob/quote_request.rs @@ -36,10 +36,7 @@ pub struct Behaviour { impl Behaviour { pub fn send(&mut self, alice: PeerId, quote_request: QuoteRequest) -> Result { - debug!( - "Requesting quote for {} from {}", - quote_request.btc_amount, alice - ); + debug!("Requesting quote for {}", quote_request.btc_amount); let id = self.rr.send_request(&alice, quote_request); From a82e82edd58e1b112ed39b5fddc604ef59156a01 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Mon, 1 Mar 2021 11:46:53 +1100 Subject: [PATCH 12/19] Tell the user about the monero-wallet-rpc download Fixes #242. --- Cargo.lock | 7 +++++++ swap/Cargo.toml | 1 + swap/src/monero/wallet_rpc.rs | 21 ++++++++++++++++++--- 3 files changed, 26 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7c7da167..bc6a00c4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -302,6 +302,12 @@ version = "0.7.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cdcf67bb7ba7797a081cd19009948ab533af7c355d5caf1d08c777582d351e9c" +[[package]] +name = "big-bytes" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4d73a8ae8ce52d09395e4cafc83b5b81c3deb70a97740e907669c8683c4dd50a" + [[package]] name = "bincode" version = "1.3.1" @@ -3525,6 +3531,7 @@ dependencies = [ "backoff", "base64 0.12.3", "bdk", + "big-bytes", "bitcoin", "bitcoin-harness", "config", diff --git a/swap/Cargo.toml b/swap/Cargo.toml index 73f926d3..8a615b3a 100644 --- a/swap/Cargo.toml +++ b/swap/Cargo.toml @@ -20,6 +20,7 @@ atty = "0.2" backoff = { version = "0.3", features = ["tokio"] } base64 = "0.12" bdk = { version = "0.4" } +big-bytes = "1" bitcoin = { version = "0.26", features = ["rand", "use-serde"] } config = { version = "0.10", default-features = false, features = ["toml"] } conquer-once = "0.3" diff --git a/swap/src/monero/wallet_rpc.rs b/swap/src/monero/wallet_rpc.rs index 82328210..2cfe1324 100644 --- a/swap/src/monero/wallet_rpc.rs +++ b/swap/src/monero/wallet_rpc.rs @@ -1,8 +1,9 @@ use ::monero::Network; use anyhow::{Context, Result}; use async_compression::tokio::bufread::BzDecoder; +use big_bytes::BigByte; use futures::{StreamExt, TryStreamExt}; -use reqwest::Url; +use reqwest::{header::CONTENT_LENGTH, Url}; use std::{ io::ErrorKind, path::{Path, PathBuf}, @@ -71,8 +72,19 @@ impl WalletRpc { .open(monero_wallet_rpc.tar_path()) .await?; - let byte_stream = reqwest::get(DOWNLOAD_URL) - .await? + let response = reqwest::get(DOWNLOAD_URL).await?; + + let content_length = response.headers()[CONTENT_LENGTH] + .to_str() + .context("failed to convert content-length to string")? + .parse::()?; + + tracing::info!( + "Downloading monero-wallet-rpc ({})", + content_length.big_byte(2) + ); + + let byte_stream = response .bytes_stream() .map_err(|err| std::io::Error::new(ErrorKind::Other, err)); @@ -119,6 +131,8 @@ impl WalletRpc { .local_addr()? .port(); + tracing::debug!("Starting monero-wallet-rpc on port {}", port); + let mut child = Command::new(self.exec_path()) .stdout(Stdio::piped()) .kill_on_drop(true) @@ -148,6 +162,7 @@ impl WalletRpc { break; } } + Ok(WalletRpcProcess { _child: child, port, From f4827e3fa48fafd0335bc71fcdf517ee2db07c3d Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Mon, 1 Mar 2021 12:04:39 +1100 Subject: [PATCH 13/19] Improve time formatting of log output Previously, the time was formatted as ISO8601 timestamps which is barely readable by humans. Activating the `chrono` feature allows us to format with a different format string. The output now looks like this: 2021-03-01 11:59:52 DEBUG Database and seed will be stored in /home/thomas/.local/share/xmr-btc-swap 2021-03-01 11:59:52 DEBUG Starting monero-wallet-rpc on port 40673 2021-03-01 11:59:59 DEBUG Still got 0.00009235 BTC left in wallet, swapping ... 2021-03-01 11:59:59 DEBUG Dialing alice at 12D3KooWCdMKjesXMJz1SiZ7HgotrxuqhQJbP5sgBm2BwP1cqThi 2021-03-01 11:59:59 DEBUG Requesting quote for 0.00008795 BTC There is a double space after the time which is already fixed in tracing-subscriber but not yet released. See https://github.com/tokio-rs/tracing/issues/1271. --- Cargo.lock | 17 +++++++++++++++-- swap/Cargo.toml | 2 +- swap/src/bin/swap_cli.rs | 3 +++ 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bc6a00c4..5453e121 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -566,6 +566,18 @@ dependencies = [ "zeroize 1.2.0", ] +[[package]] +name = "chrono" +version = "0.4.19" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "670ad68c9088c2a963aaa298cb369688cf3f9465ce5e2d4ca10e6e0098a1ce73" +dependencies = [ + "libc", + "num-integer", + "num-traits", + "winapi 0.3.9", +] + [[package]] name = "clap" version = "2.33.3" @@ -3950,11 +3962,12 @@ dependencies = [ [[package]] name = "tracing-subscriber" -version = "0.2.15" +version = "0.2.16" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a1fa8f0c8f4c594e4fc9debc1990deab13238077271ba84dd853d54902ee3401" +checksum = "8ab8966ac3ca27126141f7999361cc97dd6fb4b71da04c02044fa9045d98bb96" dependencies = [ "ansi_term 0.12.1", + "chrono", "lazy_static", "matchers", "regex", diff --git a/swap/Cargo.toml b/swap/Cargo.toml index 8a615b3a..165e5bae 100644 --- a/swap/Cargo.toml +++ b/swap/Cargo.toml @@ -58,7 +58,7 @@ toml = "0.5" tracing = { version = "0.1", features = ["attributes"] } tracing-futures = { version = "0.2", features = ["std-future", "futures-03"] } tracing-log = "0.1" -tracing-subscriber = { version = "0.2", default-features = false, features = ["fmt", "ansi", "env-filter"] } +tracing-subscriber = { version = "0.2", default-features = false, features = ["fmt", "ansi", "env-filter", "chrono"] } url = { version = "2.1", features = ["serde"] } uuid = { version = "0.8", features = ["serde", "v4"] } void = "1" diff --git a/swap/src/bin/swap_cli.rs b/swap/src/bin/swap_cli.rs index 4304de94..853b8432 100644 --- a/swap/src/bin/swap_cli.rs +++ b/swap/src/bin/swap_cli.rs @@ -52,6 +52,9 @@ async fn main() -> Result<()> { .with_writer(std::io::stderr) .with_ansi(is_terminal) .with_target(false) + .with_timer(tracing_subscriber::fmt::time::ChronoLocal::with_format( + "%F %T".to_owned(), + )) .finish(); tracing::subscriber::set_global_default(subscriber)?; From cb4e2c041bb41ec6a75d4d6667b0298cdd5fe58c Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Mon, 1 Mar 2021 12:09:59 +1100 Subject: [PATCH 14/19] Rename `opt` to `args` --- swap/src/bin/swap_cli.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/swap/src/bin/swap_cli.rs b/swap/src/bin/swap_cli.rs index 853b8432..83b56e3a 100644 --- a/swap/src/bin/swap_cli.rs +++ b/swap/src/bin/swap_cli.rs @@ -59,9 +59,9 @@ async fn main() -> Result<()> { tracing::subscriber::set_global_default(subscriber)?; - let opt = Arguments::from_args(); + let args = Arguments::from_args(); - let config = match opt.config { + let config = match args.config { Some(config_path) => read_config(config_path)??, None => Config::testnet(), }; @@ -89,7 +89,7 @@ async fn main() -> Result<()> { .run(monero_network, "stagenet.community.xmr.to") .await?; - match opt.cmd.unwrap_or_default() { + match args.cmd.unwrap_or_default() { Command::BuyXmr { alice_peer_id, alice_addr, From 8c0df23647e536c8ae651d3e25994d28ca097d3a Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Mon, 1 Mar 2021 12:19:05 +1100 Subject: [PATCH 15/19] Only show _log_ output if the user passes `--debug` If the user doesn't pass `--debug`, we only show `INFO` logs but without time and level to make it clearer that it is meant to be read by the user. Without `--debug`, the user sees: Still got 0.00009235 BTC left in wallet, swapping ... With `--debug`, they see: 2021-03-01 12:21:07 DEBUG Database and seed will be stored in /home/thomas/.local/share/xmr-btc-swap 2021-03-01 12:21:07 DEBUG Starting monero-wallet-rpc on port 40779 2021-03-01 12:21:11 INFO Still got 0.00009235 BTC left in wallet, swapping ... 2021-03-01 12:21:11 DEBUG Dialing alice at 12D3KooWCdMKjesXMJz1SiZ7HgotrxuqhQJbP5sgBm2BwP1cqThi 2021-03-01 12:21:12 DEBUG Requesting quote for 0.00008795 BTC --- swap/src/bin/swap_cli.rs | 45 ++++++++++++++++++++++++++-------------- swap/src/cli/command.rs | 3 +++ 2 files changed, 32 insertions(+), 16 deletions(-) diff --git a/swap/src/bin/swap_cli.rs b/swap/src/bin/swap_cli.rs index 83b56e3a..2c23503a 100644 --- a/swap/src/bin/swap_cli.rs +++ b/swap/src/bin/swap_cli.rs @@ -35,7 +35,7 @@ use swap::{ }, seed::Seed, }; -use tracing::{debug, error, warn, Level}; +use tracing::{debug, error, info, warn, Level}; use tracing_subscriber::FmtSubscriber; use uuid::Uuid; @@ -46,20 +46,33 @@ const MONERO_BLOCKCHAIN_MONITORING_WALLET_NAME: &str = "swap-tool-blockchain-mon #[tokio::main] async fn main() -> Result<()> { + let args = Arguments::from_args(); + let is_terminal = atty::is(atty::Stream::Stderr); - let subscriber = FmtSubscriber::builder() - .with_env_filter(format!("swap={}", Level::DEBUG)) - .with_writer(std::io::stderr) - .with_ansi(is_terminal) - .with_target(false) - .with_timer(tracing_subscriber::fmt::time::ChronoLocal::with_format( - "%F %T".to_owned(), - )) - .finish(); - - tracing::subscriber::set_global_default(subscriber)?; + let base_subscriber = |level| { + FmtSubscriber::builder() + .with_writer(std::io::stderr) + .with_ansi(is_terminal) + .with_target(false) + .with_env_filter(format!("swap={}", level)) + }; - let args = Arguments::from_args(); + if args.debug { + let subscriber = base_subscriber(Level::DEBUG) + .with_timer(tracing_subscriber::fmt::time::ChronoLocal::with_format( + "%F %T".to_owned(), + )) + .finish(); + + tracing::subscriber::set_global_default(subscriber)?; + } else { + let subscriber = base_subscriber(Level::INFO) + .without_time() + .with_level(false) + .finish(); + + tracing::subscriber::set_global_default(subscriber)?; + } let config = match args.config { Some(config_path) => read_config(config_path)??, @@ -108,8 +121,8 @@ async fn main() -> Result<()> { // TODO: Also wait for more funds if balance < dust if bitcoin_wallet.balance().await? == Amount::ZERO { - debug!( - "Waiting for BTC at address {}", + info!( + "Please deposit BTC to {}", bitcoin_wallet.new_address().await? ); @@ -121,7 +134,7 @@ async fn main() -> Result<()> { debug!("Received {}", bitcoin_wallet.balance().await?); } else { - debug!( + info!( "Still got {} left in wallet, swapping ...", bitcoin_wallet.balance().await? ); diff --git a/swap/src/cli/command.rs b/swap/src/cli/command.rs index ae859cf1..ff61d692 100644 --- a/swap/src/cli/command.rs +++ b/swap/src/cli/command.rs @@ -14,6 +14,9 @@ pub struct Arguments { )] pub config: Option, + #[structopt(long, help = "Activate debug logging.")] + pub debug: bool, + #[structopt(subcommand)] pub cmd: Option, } From 45cff81ea519da5f2fb17dc29d7ebec3d041e7cd Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Tue, 2 Mar 2021 12:22:23 +1100 Subject: [PATCH 16/19] 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 | 67 ++++---------------------- swap/src/bitcoin/wallet.rs | 80 +++++++++++--------------------- swap/src/protocol/alice/state.rs | 19 ++++---- swap/src/protocol/alice/steps.rs | 53 +++++++-------------- swap/src/protocol/alice/swap.rs | 9 ++-- swap/src/protocol/bob/state.rs | 69 +++++++++++---------------- 6 files changed, 95 insertions(+), 202 deletions(-) diff --git a/swap/src/bitcoin.rs b/swap/src/bitcoin.rs index 05d4c7cb..1e6c5494 100644 --- a/swap/src/bitcoin.rs +++ b/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 Result; -} - -#[async_trait] -pub trait BroadcastSignedTransaction { - async fn broadcast_signed_transaction(&self, transaction: Transaction) -> Result; -} - -#[async_trait] -pub trait WatchForRawTransaction { - async fn watch_for_raw_transaction(&self, txid: Txid) -> Result; -} - -#[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; -} - -#[async_trait] -pub trait TransactionBlockHeight { - async fn transaction_block_height(&self, txid: Txid) -> Result; -} - -#[async_trait] -pub trait GetRawTransaction { - async fn get_raw_transaction(&self, txid: Txid) -> Result; -} - pub fn recover(S: PublicKey, sig: Signature, encsig: EncryptedSignature) -> Result { let adaptor = Adaptor::, Deterministic>::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(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( - 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 -where - W: WatchForRawTransaction + TransactionBlockHeight + GetBlockHeight, -{ +) -> Result { 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( - 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?; diff --git a/swap/src/bitcoin/wallet.rs b/swap/src/bitcoin/wallet.rs index 5fcf33cf..96382321 100644 --- a/swap/src/bitcoin/wallet.rs +++ b/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 { + let txid = transaction.txid(); + + self.inner + .lock() + .await + .broadcast(transaction) + .with_context(|| format!("failed to broadcast transaction {}", txid))?; + + Ok(txid) } -} -#[async_trait] -impl SignTxLock for Wallet { - async fn sign_tx_lock(&self, tx_lock: TxLock) -> Result { + pub async fn sign_tx_lock(&self, tx_lock: TxLock) -> Result { 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 { - 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 { + 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 { + pub async fn watch_for_raw_transaction(&self, txid: Txid) -> Result { 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 { - 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 { + pub async fn get_block_height(&self) -> Result { 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 { + pub async fn transaction_block_height(&self, txid: Txid) -> Result { 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 { diff --git a/swap/src/protocol/alice/state.rs b/swap/src/protocol/alice/state.rs index bc578a4d..1fda6a29 100644 --- a/swap/src/protocol/alice/state.rs +++ b/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(&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(&self, bitcoin_wallet: &W) -> Result - where - W: WatchForRawTransaction + TransactionBlockHeight + GetBlockHeight, - { + pub async fn expired_timelocks( + &self, + bitcoin_wallet: &bitcoin::Wallet, + ) -> Result { current_epoch( bitcoin_wallet, self.cancel_timelock, diff --git a/swap/src/protocol/alice/steps.rs b/swap/src/protocol/alice/steps.rs index 9544c3f0..6adfbac4 100644 --- a/swap/src/protocol/alice/steps.rs +++ b/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( +pub async fn wait_for_locked_bitcoin( lock_bitcoin_txid: bitcoin::Txid, - bitcoin_wallet: Arc, + 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( +pub async fn publish_bitcoin_redeem_transaction( redeem_tx: bitcoin::Transaction, - bitcoin_wallet: Arc, -) -> Result<::bitcoin::Txid> -where - W: BroadcastSignedTransaction + WaitForTransactionFinality, -{ + bitcoin_wallet: Arc, +) -> 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( +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, -) -> Result -where - W: GetRawTransaction + TransactionBlockHeight + GetBlockHeight + BroadcastSignedTransaction, -{ + bitcoin_wallet: Arc, +) -> Result { // 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( +pub async fn wait_for_bitcoin_refund( tx_cancel: &TxCancel, cancel_tx_height: BlockHeight, punish_timelock: PunishTimelock, refund_address: &bitcoin::Address, - bitcoin_wallet: Arc, -) -> Result<(bitcoin::TxRefund, Option)> -where - W: GetBlockHeight + WatchForRawTransaction, -{ + bitcoin_wallet: &bitcoin::Wallet, +) -> Result<(bitcoin::TxRefund, Option)> { 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( +pub async fn publish_bitcoin_punish_transaction( punish_tx: bitcoin::Transaction, - bitcoin_wallet: Arc, + bitcoin_wallet: Arc, execution_params: ExecutionParams, -) -> Result -where - W: BroadcastSignedTransaction + WaitForTransactionFinality, -{ +) -> Result { let txid = bitcoin_wallet .broadcast_signed_transaction(punish_tx) .await?; diff --git a/swap/src/protocol/alice/swap.rs b/swap/src/protocol/alice/swap.rs index ad83b12e..59185617 100644 --- a/swap/src/protocol/alice/swap.rs +++ b/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?; diff --git a/swap/src/protocol/bob/state.rs b/swap/src/protocol/bob/state.rs index 83e75eca..77cac8a8 100644 --- a/swap/src/protocol/bob/state.rs +++ b/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(self, bitcoin_wallet: &W) -> Result - where - W: bitcoin::SignTxLock + bitcoin::BroadcastSignedTransaction, - { + pub async fn lock_btc(self, bitcoin_wallet: &bitcoin::Wallet) -> Result { 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(&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(&self, bitcoin_wallet: &W) -> Result - where - W: WatchForRawTransaction + TransactionBlockHeight + GetBlockHeight, - { + pub async fn current_epoch( + &self, + bitcoin_wallet: &bitcoin::Wallet, + ) -> Result { 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(&self, bitcoin_wallet: &W) -> Result - where - W: GetRawTransaction, - { + pub async fn check_for_tx_cancel( + &self, + bitcoin_wallet: &bitcoin::Wallet, + ) -> Result { 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(&self, bitcoin_wallet: &W) -> Result - where - W: BroadcastSignedTransaction, - { + pub async fn submit_tx_cancel(&self, bitcoin_wallet: &bitcoin::Wallet) -> Result { 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(&self, bitcoin_wallet: &W) -> Result - where - W: WatchForRawTransaction, - { + pub async fn watch_for_redeem_btc(&self, bitcoin_wallet: &bitcoin::Wallet) -> Result { 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(&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(&self, bitcoin_wallet: &W) -> Result - where - W: WatchForRawTransaction + TransactionBlockHeight + GetBlockHeight, - { + pub async fn expired_timelock( + &self, + bitcoin_wallet: &bitcoin::Wallet, + ) -> Result { current_epoch( bitcoin_wallet, self.cancel_timelock, @@ -540,14 +530,11 @@ impl State4 { .await } - pub async fn refund_btc( + 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); From 3a503bf95fb84f32ade27aff48eed82f6b925343 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Tue, 2 Mar 2021 12:25:47 +1100 Subject: [PATCH 17/19] Shorten function name This struct is a wallet. The only thing it can meaningfully broadcast are transactions. The fact that they have to be signed for that is implied. You cannot broadcast unsigned transactions. --- swap/src/bitcoin/wallet.rs | 2 +- swap/src/protocol/alice/steps.rs | 12 +++--------- swap/src/protocol/bob/state.rs | 12 +++--------- 3 files changed, 7 insertions(+), 19 deletions(-) diff --git a/swap/src/bitcoin/wallet.rs b/swap/src/bitcoin/wallet.rs index 96382321..17d7ace5 100644 --- a/swap/src/bitcoin/wallet.rs +++ b/swap/src/bitcoin/wallet.rs @@ -147,7 +147,7 @@ impl Wallet { self.inner.lock().await.network() } - pub async fn broadcast_signed_transaction(&self, transaction: Transaction) -> Result { + pub async fn broadcast(&self, transaction: Transaction) -> Result { let txid = transaction.txid(); self.inner diff --git a/swap/src/protocol/alice/steps.rs b/swap/src/protocol/alice/steps.rs index 6adfbac4..cdb447ac 100644 --- a/swap/src/protocol/alice/steps.rs +++ b/swap/src/protocol/alice/steps.rs @@ -130,9 +130,7 @@ pub async fn publish_bitcoin_redeem_transaction( bitcoin_wallet: Arc, ) -> Result<::bitcoin::Txid> { info!("Attempting to publish bitcoin redeem txn"); - let txid = bitcoin_wallet - .broadcast_signed_transaction(redeem_tx) - .await?; + let txid = bitcoin_wallet.broadcast(redeem_tx).await?; Ok(txid) } @@ -172,9 +170,7 @@ pub async fn publish_cancel_transaction( .expect("sig_{a,b} to be valid signatures for tx_cancel"); // TODO(Franck): Error handling is delicate, why can't we broadcast? - bitcoin_wallet - .broadcast_signed_transaction(tx_cancel) - .await?; + bitcoin_wallet.broadcast(tx_cancel).await?; // TODO(Franck): Wait until transaction is mined and returned mined // block height @@ -258,9 +254,7 @@ pub async fn publish_bitcoin_punish_transaction( bitcoin_wallet: Arc, execution_params: ExecutionParams, ) -> Result { - let txid = bitcoin_wallet - .broadcast_signed_transaction(punish_tx) - .await?; + let txid = bitcoin_wallet.broadcast(punish_tx).await?; bitcoin_wallet .wait_for_transaction_finality(txid, execution_params) diff --git a/swap/src/protocol/bob/state.rs b/swap/src/protocol/bob/state.rs index 77cac8a8..584fcee0 100644 --- a/swap/src/protocol/bob/state.rs +++ b/swap/src/protocol/bob/state.rs @@ -272,9 +272,7 @@ impl State2 { let signed_tx_lock = bitcoin_wallet.sign_tx_lock(self.tx_lock.clone()).await?; tracing::debug!("locking BTC in transaction {}", self.tx_lock.txid()); - let _ = bitcoin_wallet - .broadcast_signed_transaction(signed_tx_lock) - .await?; + let _ = bitcoin_wallet.broadcast(signed_tx_lock).await?; Ok(State3 { A: self.A, @@ -477,9 +475,7 @@ impl State4 { tx_cancel", ); - let tx_id = bitcoin_wallet - .broadcast_signed_transaction(tx_cancel) - .await?; + let tx_id = bitcoin_wallet.broadcast(tx_cancel).await?; Ok(tx_id) } @@ -548,9 +544,7 @@ impl State4 { let signed_tx_refund = tx_refund.add_signatures((self.A, sig_a), (self.b.public(), sig_b))?; - let txid = bitcoin_wallet - .broadcast_signed_transaction(signed_tx_refund) - .await?; + let txid = bitcoin_wallet.broadcast(signed_tx_refund).await?; bitcoin_wallet .wait_for_transaction_finality(txid, execution_params) From 8c9b087e39d0b6016462667a43530d8fde434617 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Tue, 2 Mar 2021 12:29:11 +1100 Subject: [PATCH 18/19] Unify logging of broadcasted transactions We eliminate unnecessary layers of indirection for broadcasting logic and force our callers to provide us with the `kind` of transaction that we are publishing. Eventually, we can replace this string with some type-system magic we can derive the name from the actual transaction. For now, we just require the caller to duplicate this information because it is faster and good enough TM. --- swap/src/bitcoin/wallet.rs | 10 ++++-- swap/src/protocol/alice/steps.rs | 27 +------------- swap/src/protocol/alice/swap.rs | 61 +++++++++++++++----------------- swap/src/protocol/bob/state.rs | 8 ++--- 4 files changed, 42 insertions(+), 64 deletions(-) diff --git a/swap/src/bitcoin/wallet.rs b/swap/src/bitcoin/wallet.rs index 17d7ace5..db1df5c2 100644 --- a/swap/src/bitcoin/wallet.rs +++ b/swap/src/bitcoin/wallet.rs @@ -147,14 +147,20 @@ impl Wallet { self.inner.lock().await.network() } - pub async fn broadcast(&self, transaction: Transaction) -> Result { + /// Broadcast the given transaction to the network and emit a log statement + /// if done so successfully. + pub async fn broadcast(&self, transaction: Transaction, kind: &str) -> Result { let txid = transaction.txid(); self.inner .lock() .await .broadcast(transaction) - .with_context(|| format!("failed to broadcast transaction {}", txid))?; + .with_context(|| { + format!("failed to broadcast Bitcoin {} transaction {}", kind, txid) + })?; + + tracing::info!("Published Bitcoin {} transaction as {}", txid, kind); Ok(txid) } diff --git a/swap/src/protocol/alice/steps.rs b/swap/src/protocol/alice/steps.rs index cdb447ac..e65567d1 100644 --- a/swap/src/protocol/alice/steps.rs +++ b/swap/src/protocol/alice/steps.rs @@ -25,7 +25,6 @@ use libp2p::PeerId; use sha2::Sha256; use std::sync::Arc; use tokio::time::timeout; -use tracing::info; // TODO(Franck): Use helper functions from xmr-btc instead of re-writing them // here @@ -125,16 +124,6 @@ pub fn build_bitcoin_redeem_transaction( Ok(tx) } -pub async fn publish_bitcoin_redeem_transaction( - redeem_tx: bitcoin::Transaction, - bitcoin_wallet: Arc, -) -> Result<::bitcoin::Txid> { - info!("Attempting to publish bitcoin redeem txn"); - let txid = bitcoin_wallet.broadcast(redeem_tx).await?; - - Ok(txid) -} - pub async fn publish_cancel_transaction( tx_lock: TxLock, a: bitcoin::SecretKey, @@ -170,7 +159,7 @@ pub async fn publish_cancel_transaction( .expect("sig_{a,b} to be valid signatures for tx_cancel"); // TODO(Franck): Error handling is delicate, why can't we broadcast? - bitcoin_wallet.broadcast(tx_cancel).await?; + bitcoin_wallet.broadcast(tx_cancel, "cancel").await?; // TODO(Franck): Wait until transaction is mined and returned mined // block height @@ -248,17 +237,3 @@ pub fn build_bitcoin_punish_transaction( Ok(signed_tx_punish) } - -pub async fn publish_bitcoin_punish_transaction( - punish_tx: bitcoin::Transaction, - bitcoin_wallet: Arc, - execution_params: ExecutionParams, -) -> Result { - let txid = bitcoin_wallet.broadcast(punish_tx).await?; - - bitcoin_wallet - .wait_for_transaction_finality(txid, execution_params) - .await?; - - Ok(txid) -} diff --git a/swap/src/protocol/alice/swap.rs b/swap/src/protocol/alice/swap.rs index 59185617..addde0a4 100644 --- a/swap/src/protocol/alice/swap.rs +++ b/swap/src/protocol/alice/swap.rs @@ -15,8 +15,7 @@ use crate::{ event_loop::EventLoopHandle, steps::{ build_bitcoin_punish_transaction, build_bitcoin_redeem_transaction, - extract_monero_private_key, lock_xmr, publish_bitcoin_punish_transaction, - publish_bitcoin_redeem_transaction, publish_cancel_transaction, + extract_monero_private_key, lock_xmr, publish_cancel_transaction, wait_for_bitcoin_encrypted_signature, wait_for_bitcoin_refund, wait_for_locked_bitcoin, }, @@ -219,37 +218,31 @@ async fn run_until_internal( state3.B, &state3.redeem_address, ) { - Ok(tx) => { - match publish_bitcoin_redeem_transaction(tx, bitcoin_wallet.clone()) - .await - { - Ok(txid) => { - let publishded_redeem_tx = bitcoin_wallet - .wait_for_transaction_finality(txid, execution_params) - .await; + Ok(tx) => match bitcoin_wallet.broadcast(tx, "redeem").await { + Ok(txid) => { + let publishded_redeem_tx = bitcoin_wallet + .wait_for_transaction_finality(txid, execution_params) + .await; - match publishded_redeem_tx { - Ok(_) => AliceState::BtcRedeemed, - Err(e) => { - bail!("Waiting for Bitcoin transaction finality failed with {}! The redeem transaction was published, but it is not ensured that the transaction was included! You're screwed.", e) - } + match publishded_redeem_tx { + Ok(_) => AliceState::BtcRedeemed, + Err(e) => { + bail!("Waiting for Bitcoin transaction finality failed with {}! The redeem transaction was published, but it is not ensured that the transaction was included! You're screwed.", e) } } - Err(e) => { - error!("Publishing the redeem transaction failed with {}, attempting to wait for cancellation now. If you restart the application before the timelock is expired publishing the redeem transaction will be retried.", e); - state3 - .wait_for_cancel_timelock_to_expire( - bitcoin_wallet.as_ref(), - ) - .await?; + } + Err(e) => { + error!("Publishing the redeem transaction failed with {}, attempting to wait for cancellation now. If you restart the application before the timelock is expired publishing the redeem transaction will be retried.", e); + state3 + .wait_for_cancel_timelock_to_expire(bitcoin_wallet.as_ref()) + .await?; - AliceState::CancelTimelockExpired { - state3, - monero_wallet_restore_blockheight, - } + AliceState::CancelTimelockExpired { + state3, + monero_wallet_restore_blockheight, } } - } + }, Err(e) => { error!("Constructing the redeem transaction failed with {}, attempting to wait for cancellation now.", e); state3 @@ -427,11 +420,15 @@ async fn run_until_internal( state3.B, )?; - let punish_tx_finalised = publish_bitcoin_punish_transaction( - signed_tx_punish, - bitcoin_wallet.clone(), - execution_params, - ); + let punish_tx_finalised = async { + let txid = bitcoin_wallet.broadcast(signed_tx_punish, "punish").await?; + + bitcoin_wallet + .wait_for_transaction_finality(txid, execution_params) + .await?; + + Result::<_, anyhow::Error>::Ok(txid) + }; let refund_tx_seen = bitcoin_wallet.watch_for_raw_transaction(tx_refund.txid()); diff --git a/swap/src/protocol/bob/state.rs b/swap/src/protocol/bob/state.rs index 584fcee0..f057d41c 100644 --- a/swap/src/protocol/bob/state.rs +++ b/swap/src/protocol/bob/state.rs @@ -271,8 +271,7 @@ impl State2 { pub async fn lock_btc(self, bitcoin_wallet: &bitcoin::Wallet) -> Result { let signed_tx_lock = bitcoin_wallet.sign_tx_lock(self.tx_lock.clone()).await?; - tracing::debug!("locking BTC in transaction {}", self.tx_lock.txid()); - let _ = bitcoin_wallet.broadcast(signed_tx_lock).await?; + let _ = bitcoin_wallet.broadcast(signed_tx_lock, "lock").await?; Ok(State3 { A: self.A, @@ -475,7 +474,8 @@ impl State4 { tx_cancel", ); - let tx_id = bitcoin_wallet.broadcast(tx_cancel).await?; + let tx_id = bitcoin_wallet.broadcast(tx_cancel, "cancel").await?; + Ok(tx_id) } @@ -544,7 +544,7 @@ impl State4 { let signed_tx_refund = tx_refund.add_signatures((self.A, sig_a), (self.b.public(), sig_b))?; - let txid = bitcoin_wallet.broadcast(signed_tx_refund).await?; + let txid = bitcoin_wallet.broadcast(signed_tx_refund, "refund").await?; bitcoin_wallet .wait_for_transaction_finality(txid, execution_params) From 3ad9516188132c1789898cb854dc3819e6f90c48 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Tue, 2 Mar 2021 12:53:40 +1100 Subject: [PATCH 19/19] Reduce logging when signing transactions 1. We can generalize the signing interface by passing a PSBT in instead of the `TxLock` transaction. 2. Knowing the transaction ID of a transaction that we are about to sign is not very useful. Instead, it is much more useful to know what failed. Hence we add a `.context` to the call of `sign_and_finalize`. 3. In case the signing succeeds, we will immediately broadcast it afterwards. The new broadcasting interface will tell us that we broadcasted the "lock" transaction. --- swap/src/bitcoin/wallet.rs | 13 ++++++------- swap/src/protocol/bob/state.rs | 9 ++++++--- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/swap/src/bitcoin/wallet.rs b/swap/src/bitcoin/wallet.rs index db1df5c2..76e17c98 100644 --- a/swap/src/bitcoin/wallet.rs +++ b/swap/src/bitcoin/wallet.rs @@ -1,5 +1,5 @@ use crate::{ - bitcoin::{timelocks::BlockHeight, Address, Amount, Transaction, TxLock}, + bitcoin::{timelocks::BlockHeight, Address, Amount, Transaction}, execution_params::ExecutionParams, }; use ::bitcoin::{util::psbt::PartiallySignedTransaction, Txid}; @@ -165,16 +165,15 @@ impl Wallet { Ok(txid) } - pub async fn sign_tx_lock(&self, tx_lock: TxLock) -> Result { - let txid = tx_lock.txid(); - tracing::debug!("signing tx lock: {}", txid); - let psbt = PartiallySignedTransaction::from(tx_lock); + pub async fn sign_and_finalize(&self, psbt: PartiallySignedTransaction) -> Result { let (signed_psbt, finalized) = self.inner.lock().await.sign(psbt, None)?; + if !finalized { - bail!("Could not finalize TxLock psbt") + bail!("PSBT is not finalized") } + let tx = signed_psbt.extract_tx(); - tracing::debug!("signed tx lock: {}", txid); + Ok(tx) } diff --git a/swap/src/protocol/bob/state.rs b/swap/src/protocol/bob/state.rs index f057d41c..f7e562ac 100644 --- a/swap/src/protocol/bob/state.rs +++ b/swap/src/protocol/bob/state.rs @@ -13,7 +13,7 @@ use crate::{ CROSS_CURVE_PROOF_SYSTEM, }, }; -use anyhow::{anyhow, bail, Result}; +use anyhow::{anyhow, bail, Context, Result}; use ecdsa_fun::{ adaptor::{Adaptor, HashTranscript}, nonce::Deterministic, @@ -269,9 +269,12 @@ impl State2 { } pub async fn lock_btc(self, bitcoin_wallet: &bitcoin::Wallet) -> Result { - let signed_tx_lock = bitcoin_wallet.sign_tx_lock(self.tx_lock.clone()).await?; + let signed_tx = bitcoin_wallet + .sign_and_finalize(self.tx_lock.clone().into()) + .await + .context("failed to sign Bitcoin lock transaction")?; - let _ = bitcoin_wallet.broadcast(signed_tx_lock, "lock").await?; + let _ = bitcoin_wallet.broadcast(signed_tx, "lock").await?; Ok(State3 { A: self.A,