From e66e84085b0211bd2c5e43056c93913aedee7fa0 Mon Sep 17 00:00:00 2001 From: Daniel Karzel Date: Fri, 26 Feb 2021 16:02:44 +1100 Subject: [PATCH 1/2] Rename Bob's Behavior Failure to CommunicationError Failure does not express what the error represents. It is only used for communication errors for quote requests, receiving the XMR transfer proof and sending the encryption signature. --- swap/src/protocol/bob.rs | 8 ++++---- swap/src/protocol/bob/event_loop.rs | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/swap/src/protocol/bob.rs b/swap/src/protocol/bob.rs index 7117365b..5ca668bd 100644 --- a/swap/src/protocol/bob.rs +++ b/swap/src/protocol/bob.rs @@ -185,7 +185,7 @@ pub enum OutEvent { }, EncryptedSignatureAcknowledged, ResponseSent, // Same variant is used for all messages as no processing is done - Failure(Error), + CommunicationError(Error), } impl From for OutEvent { @@ -203,7 +203,7 @@ impl From for OutEvent { use quote_request::OutEvent::*; match event { MsgReceived(quote_response) => OutEvent::QuoteResponse(quote_response), - Failure(err) => OutEvent::Failure(err.context("Failure with Quote Request")), + Failure(err) => OutEvent::CommunicationError(err.context("Failure with Quote Request")), } } } @@ -225,7 +225,7 @@ impl From for OutEvent { channel, }, AckSent => OutEvent::ResponseSent, - Failure(err) => OutEvent::Failure(err.context("Failure with Transfer Proof")), + Failure(err) => OutEvent::CommunicationError(err.context("Failure with Transfer Proof")), } } } @@ -235,7 +235,7 @@ impl From for OutEvent { use encrypted_signature::OutEvent::*; match event { Acknowledged => OutEvent::EncryptedSignatureAcknowledged, - Failure(err) => OutEvent::Failure(err.context("Failure with Encrypted Signature")), + Failure(err) => OutEvent::CommunicationError(err.context("Failure with Encrypted Signature")), } } } diff --git a/swap/src/protocol/bob/event_loop.rs b/swap/src/protocol/bob/event_loop.rs index e31b429f..23ca57e1 100644 --- a/swap/src/protocol/bob/event_loop.rs +++ b/swap/src/protocol/bob/event_loop.rs @@ -192,7 +192,7 @@ impl EventLoop { debug!("Alice acknowledged encrypted signature"); } OutEvent::ResponseSent => {} - OutEvent::Failure(err) => { + OutEvent::CommunicationError(err) => { error!("Communication error: {:#}", err) } } From bb1537d6f2634c722f0290b0a1043cb6541dd5e4 Mon Sep 17 00:00:00 2001 From: Daniel Karzel Date: Fri, 26 Feb 2021 16:11:14 +1100 Subject: [PATCH 2/2] Error feedback for the user upon communication errors If communication with the other party fails the program should stop and the user should see the respective error. Communication errors are handled in the event-loop. Upon a communication error the event loop is stopped. Since the event loop is only stopped upon error the Result returned from the event loop is Infallible. If one of the two futures, event loop and swap, finishes (success/failure) the other future should be stopped as well. We use tokio::selec! to stop either future if the other stops. --- swap/src/bin/swap_cli.rs | 78 ++++++++++++++++++++--------- swap/src/protocol/bob.rs | 8 ++- swap/src/protocol/bob/event_loop.rs | 8 +-- swap/tests/testutils/mod.rs | 3 +- 4 files changed, 65 insertions(+), 32 deletions(-) diff --git a/swap/src/bin/swap_cli.rs b/swap/src/bin/swap_cli.rs index 13a2725c..653c5800 100644 --- a/swap/src/bin/swap_cli.rs +++ b/swap/src/bin/swap_cli.rs @@ -118,8 +118,16 @@ async fn main() -> Result<()> { ); let (swap, event_loop) = bob_factory.with_init_params(send_bitcoin).build().await?; - tokio::spawn(async move { event_loop.run().await }); - bob::run(swap).await?; + let handle = tokio::spawn(async move { event_loop.run().await }); + let swap = bob::run(swap); + tokio::select! { + event_loop_result = handle => { + event_loop_result??; + }, + swap_result = swap => { + swap_result?; + } + } } Command::History => { let mut table = Table::new(); @@ -159,9 +167,16 @@ async fn main() -> Result<()> { execution_params, ); let (swap, event_loop) = bob_factory.build().await?; - - tokio::spawn(async move { event_loop.run().await }); - bob::run(swap).await?; + let handle = tokio::spawn(async move { event_loop.run().await }); + let swap = bob::run(swap); + tokio::select! { + event_loop_result = handle => { + event_loop_result??; + }, + swap_result = swap => { + swap_result?; + } + } } Command::Cancel(Cancel::BuyXmr { swap_id, @@ -191,27 +206,33 @@ async fn main() -> Result<()> { execution_params, ); let (swap, event_loop) = bob_factory.build().await?; + let handle = tokio::spawn(async move { event_loop.run().await }); - tokio::spawn(async move { event_loop.run().await }); - - match bob::cancel( + let cancel = bob::cancel( swap.swap_id, swap.state, swap.bitcoin_wallet, swap.db, force, - ) - .await? - { - Ok((txid, _)) => { - info!("Cancel transaction successfully published with id {}", txid) - } - Err(CancelError::CancelTimelockNotExpiredYet) => error!( - "The Cancel Transaction cannot be published yet, \ - because the timelock has not expired. Please try again later." - ), - Err(CancelError::CancelTxAlreadyPublished) => { - warn!("The Cancel Transaction has already been published.") + ); + + tokio::select! { + event_loop_result = handle => { + event_loop_result??; + }, + cancel_result = cancel => { + match cancel_result? { + Ok((txid, _)) => { + info!("Cancel transaction successfully published with id {}", txid) + } + Err(CancelError::CancelTimelockNotExpiredYet) => error!( + "The Cancel Transaction cannot be published yet, \ + because the timelock has not expired. Please try again later." + ), + Err(CancelError::CancelTxAlreadyPublished) => { + warn!("The Cancel Transaction has already been published.") + } + } } } } @@ -244,19 +265,26 @@ async fn main() -> Result<()> { ); let (swap, event_loop) = bob_factory.build().await?; - tokio::spawn(async move { event_loop.run().await }); - bob::refund( + let handle = tokio::spawn(async move { event_loop.run().await }); + let refund = bob::refund( swap.swap_id, swap.state, swap.execution_params, swap.bitcoin_wallet, swap.db, force, - ) - .await??; + ); + + tokio::select! { + event_loop_result = handle => { + event_loop_result??; + }, + refund_result = refund => { + refund_result??; + } + } } }; - Ok(()) } diff --git a/swap/src/protocol/bob.rs b/swap/src/protocol/bob.rs index 5ca668bd..4a3e8d38 100644 --- a/swap/src/protocol/bob.rs +++ b/swap/src/protocol/bob.rs @@ -225,7 +225,9 @@ impl From for OutEvent { channel, }, AckSent => OutEvent::ResponseSent, - Failure(err) => OutEvent::CommunicationError(err.context("Failure with Transfer Proof")), + Failure(err) => { + OutEvent::CommunicationError(err.context("Failure with Transfer Proof")) + } } } } @@ -235,7 +237,9 @@ impl From for OutEvent { use encrypted_signature::OutEvent::*; match event { Acknowledged => OutEvent::EncryptedSignatureAcknowledged, - Failure(err) => OutEvent::CommunicationError(err.context("Failure with Encrypted Signature")), + Failure(err) => { + OutEvent::CommunicationError(err.context("Failure with Encrypted Signature")) + } } } } diff --git a/swap/src/protocol/bob/event_loop.rs b/swap/src/protocol/bob/event_loop.rs index 23ca57e1..57a99675 100644 --- a/swap/src/protocol/bob/event_loop.rs +++ b/swap/src/protocol/bob/event_loop.rs @@ -7,10 +7,10 @@ use crate::{ bob::{Behaviour, OutEvent, QuoteRequest, State0, State2}, }, }; -use anyhow::{anyhow, Result}; +use anyhow::{anyhow, bail, Result}; use futures::FutureExt; use libp2p::{core::Multiaddr, PeerId}; -use std::sync::Arc; +use std::{convert::Infallible, sync::Arc}; use tokio::sync::mpsc::{Receiver, Sender}; use tracing::{debug, error, info}; @@ -167,7 +167,7 @@ impl EventLoop { Ok((event_loop, handle)) } - pub async fn run(mut self) { + pub async fn run(mut self) -> Result { loop { tokio::select! { swarm_event = self.swarm.next().fuse() => { @@ -193,7 +193,7 @@ impl EventLoop { } OutEvent::ResponseSent => {} OutEvent::CommunicationError(err) => { - error!("Communication error: {:#}", err) + bail!("Communication error: {:#}", err) } } }, diff --git a/swap/tests/testutils/mod.rs b/swap/tests/testutils/mod.rs index 646a3cc4..7619e18b 100644 --- a/swap/tests/testutils/mod.rs +++ b/swap/tests/testutils/mod.rs @@ -9,6 +9,7 @@ use get_port::get_port; use libp2p::{core::Multiaddr, PeerId}; use monero_harness::{image, Monero}; use std::{ + convert::Infallible, path::{Path, PathBuf}, sync::Arc, time::Duration, @@ -67,7 +68,7 @@ impl BobParams { } } -pub struct BobEventLoopJoinHandle(JoinHandle<()>); +pub struct BobEventLoopJoinHandle(JoinHandle>); impl BobEventLoopJoinHandle { pub fn abort(&self) {