From ea76ae582158a248bf6ed9542e6cdb87cb4e543d Mon Sep 17 00:00:00 2001 From: Daniel Karzel Date: Fri, 30 Apr 2021 12:36:26 +1000 Subject: [PATCH] Return proper error to CLI for all expected scenarios When a CLI requests a spot price have some errors that are expected, where we can provide a proper error message for the CLI: - Balance of ASB too low - Buy amount sent by CLI exceeds maximum buy amount accepted by ASB - ASB is running in maintenance mode and does not accept incoming swap requests All of these errors returns a proper error to the CLI and prints a warning in the ASB logs. Any other unexpected error will result in closing the channel with the CLI and printing an error in the ASB logs. --- CHANGELOG.md | 6 +++ swap/src/monero.rs | 6 --- swap/src/network/spot_price.rs | 20 +++++++-- swap/src/protocol/alice/event_loop.rs | 64 ++++++++++++--------------- swap/src/protocol/bob/event_loop.rs | 12 ++--- 5 files changed, 55 insertions(+), 53 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f601fd4f..37152d5a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 If you want to access data created by a previous version you will have to rename the data folder or one of the following: 1. For the CLI you can use `--data-dir` to point to the old directory. 2. For the ASB you can change the data-dir in the config file of the ASB. +- The CLI receives proper Error messages if setting up a swap with the ASB fails. + This is a breaking change because the spot-price protocol response changed. + Expected errors scenarios that are now reported back to the CLI: + 1. Balance of ASB too low + 2. Buy amount sent by CLI exceeds maximum buy amount accepted by ASB + 3. ASB is running in resume-only mode and does not accept incoming swap requests ## [0.5.0] - 2021-04-17 diff --git a/swap/src/monero.rs b/swap/src/monero.rs index c52bd0b5..6886cbf1 100644 --- a/swap/src/monero.rs +++ b/swap/src/monero.rs @@ -192,12 +192,6 @@ pub struct InsufficientFunds { pub actual: Amount, } -#[derive(Debug, Clone, Copy, thiserror::Error)] -#[error("The balance is too low, current balance: {balance}")] -pub struct BalanceTooLow { - pub balance: Amount, -} - #[derive(thiserror::Error, Debug, Clone, PartialEq)] #[error("Overflow, cannot convert {0} to u64")] pub struct OverflowError(pub String); diff --git a/swap/src/network/spot_price.rs b/swap/src/network/spot_price.rs index bd2ca0a7..24671481 100644 --- a/swap/src/network/spot_price.rs +++ b/swap/src/network/spot_price.rs @@ -1,6 +1,6 @@ +use crate::monero; use crate::network::cbor_request_response::CborCodec; use crate::protocol::{alice, bob}; -use crate::{bitcoin, monero}; use libp2p::core::ProtocolName; use libp2p::request_response::{ ProtocolSupport, RequestResponse, RequestResponseConfig, RequestResponseEvent, @@ -40,9 +40,9 @@ pub struct Request { } #[derive(Serialize, Deserialize, Debug, Clone)] -pub struct Response { - pub xmr: Option, - pub error: Option, +pub enum Response { + Xmr(monero::Amount), + Error(Error), } #[derive(Clone, Debug, thiserror::Error, Serialize, Deserialize)] @@ -51,6 +51,18 @@ pub enum Error { "This seller currently does not accept incoming swap requests, please try again later" )] MaintenanceMode, + #[error("Seller refused to buy {buy} because the maximum configured buy limit is {max}")] + MaxBuyAmountExceeded { + #[serde(with = "::bitcoin::util::amount::serde::as_sat")] + max: bitcoin::Amount, + #[serde(with = "::bitcoin::util::amount::serde::as_sat")] + buy: bitcoin::Amount, + }, + #[error("This seller's XMR balance is currently too low to fulfill the swap request to buy {buy}, please try again later")] + BalanceTooLow { + #[serde(with = "::bitcoin::util::amount::serde::as_sat")] + buy: bitcoin::Amount, + }, } /// Constructs a new instance of the `spot-price` behaviour to be used by Alice. diff --git a/swap/src/protocol/alice/event_loop.rs b/swap/src/protocol/alice/event_loop.rs index b81fecfe..bf2b5ba0 100644 --- a/swap/src/protocol/alice/event_loop.rs +++ b/swap/src/protocol/alice/event_loop.rs @@ -1,12 +1,11 @@ use crate::asb::Rate; use crate::database::Database; use crate::env::Config; -use crate::monero::BalanceTooLow; use crate::network::quote::BidQuote; use crate::network::{spot_price, transfer_proof}; use crate::protocol::alice::{AliceState, Behaviour, OutEvent, State0, State3, Swap}; use crate::{bitcoin, kraken, monero}; -use anyhow::{bail, Context, Result}; +use anyhow::{Context, Result}; use futures::future; use futures::future::{BoxFuture, FutureExt}; use futures::stream::{FuturesUnordered, StreamExt}; @@ -148,30 +147,30 @@ where swarm_event = self.swarm.next_event() => { match swarm_event { SwarmEvent::Behaviour(OutEvent::SpotPriceRequested { request, channel, peer }) => { - if self.resume_only { - tracing::warn!(%peer, "Ignoring spot price request from {} because ASB started in resume-only mode", peer); - - match self.swarm.behaviour_mut().spot_price.send_response(channel, spot_price::Response { xmr: None, error: Some(spot_price::Error::MaintenanceMode) }) { - Ok(_) => {}, - Err(_) => { - tracing::debug!(%peer, "Failed to respond with error to spot price request"); - continue; - } - } - - continue; - } - let btc = request.btc; let xmr = match self.handle_spot_price_request(btc, self.monero_wallet.clone()).await { - Ok(xmr) => xmr, + Ok(xmr) => match xmr { + Ok(xmr) => xmr, + Err(e) => { + tracing::warn!(%peer, "Ignoring spot price request from {} because: {:#}", peer, e); + match self.swarm.behaviour_mut().spot_price.send_response(channel, spot_price::Response::Error(e)) { + Ok(_) => { + continue; + }, + Err(_) => { + tracing::debug!(%peer, "Failed to respond with error to spot price request"); + continue; + } + } + } + }, Err(e) => { - tracing::warn!(%peer, "Failed to produce spot price for {}: {:#}", btc, e); + tracing::error!(%peer, "Unrecoverable error while producing spot price for {}: {:#}", btc, e); continue; } }; - match self.swarm.behaviour_mut().spot_price.send_response(channel, spot_price::Response { xmr: Some(xmr), error: None }) { + match self.swarm.behaviour_mut().spot_price.send_response(channel, spot_price::Response::Xmr(xmr)) { Ok(_) => {}, Err(_) => { // if we can't respond, the peer probably just disconnected so it is not a huge deal, only log this on debug @@ -365,17 +364,21 @@ where &mut self, btc: bitcoin::Amount, monero_wallet: Arc, - ) -> Result { + ) -> Result> { + if self.resume_only { + return Ok(Err(spot_price::Error::MaintenanceMode)); + } + let rate = self .latest_rate .latest_rate() .context("Failed to get latest rate")?; if btc > self.max_buy { - bail!(MaximumBuyAmountExceeded { - actual: btc, - max: self.max_buy - }) + return Ok(Err(spot_price::Error::MaxBuyAmountExceeded { + buy: btc, + max: self.max_buy, + })); } let xmr_balance = monero_wallet.get_balance().await?; @@ -383,12 +386,10 @@ where let xmr = rate.sell_quote(btc)?; if xmr_balance < xmr + xmr_lock_fees { - bail!(BalanceTooLow { - balance: xmr_balance - }) + return Ok(Err(spot_price::Error::BalanceTooLow { buy: btc })); } - Ok(xmr) + Ok(Ok(xmr)) } async fn make_quote(&mut self, max_buy: bitcoin::Amount) -> Result { @@ -569,13 +570,6 @@ impl EventLoopHandle { } } -#[derive(Debug, Clone, Copy, thiserror::Error)] -#[error("Refusing to buy {actual} because the maximum configured limit is {max}")] -pub struct MaximumBuyAmountExceeded { - pub max: bitcoin::Amount, - pub actual: bitcoin::Amount, -} - #[allow(missing_debug_implementations)] struct MpscChannels { sender: mpsc::Sender, diff --git a/swap/src/protocol/bob/event_loop.rs b/swap/src/protocol/bob/event_loop.rs index 1b978efe..2045a94f 100644 --- a/swap/src/protocol/bob/event_loop.rs +++ b/swap/src/protocol/bob/event_loop.rs @@ -1,5 +1,6 @@ use crate::bitcoin::EncryptedSignature; use crate::network::quote::BidQuote; +use crate::network::spot_price::Response; use crate::network::{encrypted_signature, spot_price}; use crate::protocol::bob::{Behaviour, OutEvent, State0, State2}; use crate::{bitcoin, monero}; @@ -266,16 +267,11 @@ impl EventLoopHandle { .send_receive(spot_price::Request { btc }) .await?; - match (response.xmr, response.error) { - (Some(xmr), None) => Ok(xmr), - (_, Some(error)) => { + match response { + Response::Xmr(xmr) => Ok(xmr), + Response::Error(error) => { bail!(error); } - (None, None) => { - bail!( - "Unexpected response for spot-price request, neither price nor error received" - ); - } } }