Decouple ASB/CLI Errors from Error sent over wire

What goes over the wire should not be coupled to the errors being printed.
For the CLI and ASB we introduce a separate error enum that is used for logging.
When sending over the wire the errors are mapped to and from the `network::spot_price::Error`.

As part of Bob-specific spot_price code was moved from the network into bob.
Clearly separation of the network API from bob/alice.

swap/src/network/spot_price.rs

@@ -1,17 +1,12 @@
 use crate::monero;
 use crate::network::cbor_request_response::CborCodec;
-use crate::protocol::bob;
 use libp2p::core::ProtocolName;
-use libp2p::request_response::{
-    ProtocolSupport, RequestResponse, RequestResponseConfig, RequestResponseEvent,
-    RequestResponseMessage,
-};
-use libp2p::PeerId;
+use libp2p::request_response::{RequestResponse, RequestResponseEvent, RequestResponseMessage};
 use serde::{Deserialize, Serialize};
 
-const PROTOCOL: &str = "/comit/xmr/btc/spot-price/1.0.0";
+pub const PROTOCOL: &str = "/comit/xmr/btc/spot-price/1.0.0";
 pub type OutEvent = RequestResponseEvent<Request, Response>;
-type Message = RequestResponseMessage<Request, Response>;
+pub type Message = RequestResponseMessage<Request, Response>;
 
 pub type Behaviour = RequestResponse<CborCodec<SpotPriceProtocol, Request, Response>>;
 
@@ -45,55 +40,57 @@ pub enum Response {
     Error(Error),
 }
 
-#[derive(Clone, Debug, thiserror::Error, Serialize, Deserialize)]
+#[derive(Clone, Debug, Serialize, Deserialize)]
 pub enum Error {
-    #[error(
-        "This seller currently does not accept incoming swap requests, please try again later"
-    )]
     NoSwapsAccepted,
-    #[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,
     },
-
     /// To be used for errors that cannot be explained on the CLI side (e.g.
     /// rate update problems on the seller side)
-    #[error("The seller encountered a problem, please try again later.")]
     Other,
 }
 
-/// Constructs a new instance of the `spot-price` behaviour to be used by Bob.
-///
-/// Bob only supports outbound connections, i.e. requesting a spot price for a
-/// given amount of BTC in XMR.
-pub fn bob() -> Behaviour {
-    Behaviour::new(
-        CborCodec::default(),
-        vec![(SpotPriceProtocol, ProtocolSupport::Outbound)],
-        RequestResponseConfig::default(),
-    )
-}
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::monero;
+
+    #[test]
+    fn snapshot_test_serialize() {
+        let amount = monero::Amount::from_piconero(100_000u64);
+        let xmr = r#"{"Xmr":100000}"#.to_string();
+        let serialized = serde_json::to_string(&Response::Xmr(amount)).unwrap();
+        assert_eq!(xmr, serialized);
+
+        let error = r#"{"Error":"NoSwapsAccepted"}"#.to_string();
+        let serialized = serde_json::to_string(&Response::Error(Error::NoSwapsAccepted)).unwrap();
+        assert_eq!(error, serialized);
+
+        let error = r#"{"Error":{"MaxBuyAmountExceeded":{"max":0,"buy":0}}}"#.to_string();
+        let serialized = serde_json::to_string(&Response::Error(Error::MaxBuyAmountExceeded {
+            max: Default::default(),
+            buy: Default::default(),
+        }))
+        .unwrap();
+        assert_eq!(error, serialized);
+
+        let error = r#"{"Error":{"BalanceTooLow":{"buy":0}}}"#.to_string();
+        let serialized = serde_json::to_string(&Response::Error(Error::BalanceTooLow {
+            buy: Default::default(),
+        }))
+        .unwrap();
+        assert_eq!(error, serialized);
 
-impl From<(PeerId, Message)> for bob::OutEvent {
-    fn from((peer, message): (PeerId, Message)) -> Self {
-        match message {
-            Message::Request { .. } => Self::unexpected_request(peer),
-            Message::Response {
-                response,
-                request_id,
-            } => Self::SpotPriceReceived {
-                id: request_id,
-                response,
-            },
-        }
+        let error = r#"{"Error":"Other"}"#.to_string();
+        let serialized = serde_json::to_string(&Response::Error(Error::Other)).unwrap();
+        assert_eq!(error, serialized);
     }
 }
-crate::impl_from_rr_event!(OutEvent, bob::OutEvent, PROTOCOL);

swap/src/network/swarm.rs

@@ -6,6 +6,7 @@ use crate::{monero, tor};
 use anyhow::Result;
 use libp2p::swarm::{NetworkBehaviour, SwarmBuilder};
 use libp2p::{PeerId, Swarm};
+use std::fmt::Debug;
 
 pub fn alice<LR>(
     seed: &Seed,
@@ -16,7 +17,7 @@ pub fn alice<LR>(
     resume_only: bool,
 ) -> Result<Swarm<alice::Behaviour<LR>>>
 where
-    LR: LatestRate + Send + 'static,
+    LR: LatestRate + Send + 'static + Debug,
 {
     with_clear_net(
         seed,

swap/src/protocol/alice/behaviour.rs

@@ -6,6 +6,7 @@ use crate::protocol::alice::{execution_setup, spot_price, State3};
 use anyhow::{anyhow, Error};
 use libp2p::request_response::{RequestId, ResponseChannel};
 use libp2p::{NetworkBehaviour, PeerId};
+use std::fmt::Debug;
 use uuid::Uuid;
 
 #[derive(Debug)]
@@ -62,7 +63,10 @@ impl OutEvent {
 #[derive(NetworkBehaviour)]
 #[behaviour(out_event = "OutEvent", event_process = false)]
 #[allow(missing_debug_implementations)]
-pub struct Behaviour<LR: LatestRate + Send + 'static> {
+pub struct Behaviour<LR>
+where
+    LR: LatestRate + Send + 'static + Debug,
+{
     pub quote: quote::Behaviour,
     pub spot_price: spot_price::Behaviour<LR>,
     pub execution_setup: execution_setup::Behaviour,
@@ -72,7 +76,7 @@ pub struct Behaviour<LR: LatestRate + Send + 'static> {
 
 impl<LR> Behaviour<LR>
 where
-    LR: LatestRate + Send + 'static,
+    LR: LatestRate + Send + 'static + Debug,
 {
     pub fn new(
         balance: monero::Amount,

swap/src/protocol/alice/event_loop.rs

@@ -16,6 +16,7 @@ use rand::rngs::OsRng;
 use rust_decimal::Decimal;
 use std::collections::HashMap;
 use std::convert::Infallible;
+use std::fmt::Debug;
 use std::sync::Arc;
 use tokio::sync::mpsc;
 use uuid::Uuid;
@@ -31,7 +32,10 @@ type OutgoingTransferProof =
     BoxFuture<'static, Result<(PeerId, transfer_proof::Request, bmrng::Responder<()>)>>;
 
 #[allow(missing_debug_implementations)]
-pub struct EventLoop<LR: LatestRate + Send + 'static> {
+pub struct EventLoop<LR>
+where
+    LR: LatestRate + Send + 'static + Debug,
+{
     swarm: libp2p::Swarm<Behaviour<LR>>,
     env_config: Config,
     bitcoin_wallet: Arc<bitcoin::Wallet>,
@@ -59,7 +63,7 @@ pub struct EventLoop<LR: LatestRate + Send + 'static> {
 
 impl<LR> EventLoop<LR>
 where
-    LR: LatestRate + Send + 'static,
+    LR: LatestRate + Send + 'static + Debug,
 {
     #[allow(clippy::too_many_arguments)]
     pub fn new(

swap/src/protocol/alice/spot_price.rs

@@ -11,6 +11,7 @@ use libp2p::request_response::{
 use libp2p::swarm::{NetworkBehaviourAction, NetworkBehaviourEventProcess, PollParameters};
 use libp2p::{NetworkBehaviour, PeerId};
 use std::collections::VecDeque;
+use std::fmt::Debug;
 use std::task::{Context, Poll};
 
 pub struct OutEvent {
@@ -22,7 +23,10 @@ pub struct OutEvent {
 #[derive(NetworkBehaviour)]
 #[behaviour(out_event = "OutEvent", poll_method = "poll", event_process = true)]
 #[allow(missing_debug_implementations)]
-pub struct Behaviour<LR: LatestRate + Send + 'static> {
+pub struct Behaviour<LR>
+where
+    LR: LatestRate + Send + 'static + Debug,
+{
     behaviour: spot_price::Behaviour,
 
     #[behaviour(ignore)]
@@ -46,7 +50,7 @@ pub struct Behaviour<LR: LatestRate + Send + 'static> {
 /// bubbled up to the parent behaviour.
 impl<LR> Behaviour<LR>
 where
-    LR: LatestRate + Send + 'static,
+    LR: LatestRate + Send + 'static + Debug,
 {
     pub fn new(
         balance: monero::Amount,
@@ -78,11 +82,22 @@ where
         &mut self,
         peer: PeerId,
         channel: ResponseChannel<spot_price::Response>,
-        error: spot_price::Error,
+        error: Error<LR>,
     ) {
+        match error {
+            Error::ResumeOnlyMode | Error::MaxBuyAmountExceeded { .. } => {
+                tracing::warn!(%peer, "Ignoring spot price request because: {}", error);
+            }
+            Error::BalanceTooLow { .. }
+            | Error::LatestRateFetchFailed(_)
+            | Error::SellQuoteCalculationFailed(_) => {
+                tracing::error!(%peer, "Ignoring spot price request because: {}", error);
+            }
+        }
+
         if self
             .behaviour
-            .send_response(channel, spot_price::Response::Error(error))
+            .send_response(channel, spot_price::Response::Error(error.into()))
             .is_err()
         {
             tracing::debug!(%peer, "Unable to send error response for spot price request");
@@ -105,7 +120,7 @@ where
 
 impl<LR> NetworkBehaviourEventProcess<spot_price::OutEvent> for Behaviour<LR>
 where
-    LR: LatestRate + Send + 'static,
+    LR: LatestRate + Send + 'static + Debug,
 {
     fn inject_event(&mut self, event: spot_price::OutEvent) {
         let (peer, message) = match event {
@@ -135,15 +150,13 @@ where
         };
 
         if self.resume_only {
-            tracing::warn!(%peer, "Ignoring spot price request from {} because ASB is running in resume-only mode", peer);
-            self.send_error_response(peer, channel, spot_price::Error::NoSwapsAccepted);
+            self.send_error_response(peer, channel, Error::ResumeOnlyMode);
             return;
         }
 
         let btc = request.btc;
         if btc > self.max_buy {
-            tracing::warn!(%peer, "Ignoring spot price request from {} because max muy amount exceeded", peer);
-            self.send_error_response(peer, channel, spot_price::Error::MaxBuyAmountExceeded {
+            self.send_error_response(peer, channel, Error::MaxBuyAmountExceeded {
                 max: self.max_buy,
                 buy: btc,
             });
@@ -153,16 +166,14 @@ where
         let rate = match self.latest_rate.latest_rate() {
             Ok(rate) => rate,
             Err(e) => {
-                tracing::error!(%peer, "Ignoring spot price request from {} because we encountered a problem with fetching the latest rate: {:#}", peer, e);
-                self.send_error_response(peer, channel, spot_price::Error::Other);
+                self.send_error_response(peer, channel, Error::LatestRateFetchFailed(e));
                 return;
             }
         };
         let xmr = match rate.sell_quote(btc) {
             Ok(xmr) => xmr,
             Err(e) => {
-                tracing::error!(%peer, "Ignoring spot price request from {} because we encountered a problem with calculating the amount from rate: {:#}", peer, e);
-                self.send_error_response(peer, channel, spot_price::Error::Other);
+                self.send_error_response(peer, channel, Error::SellQuoteCalculationFailed(e));
                 return;
             }
         };
@@ -171,8 +182,7 @@ where
         let xmr_lock_fees = self.lock_fee;
 
         if xmr_balance < xmr + xmr_lock_fees {
-            tracing::error!(%peer, "Ignoring spot price request from {} because the XMR balance is too low to fulfill the swap: {}", peer, xmr_balance);
-            self.send_error_response(peer, channel, spot_price::Error::BalanceTooLow { buy: btc });
+            self.send_error_response(peer, channel, Error::BalanceTooLow { buy: btc });
             return;
         }
 
@@ -197,3 +207,43 @@ impl From<OutEvent> for alice::OutEvent {
         }
     }
 }
+
+impl<LR> From<Error<LR>> for spot_price::Error
+where
+    LR: LatestRate + Debug,
+{
+    fn from(error: Error<LR>) -> Self {
+        match error {
+            Error::ResumeOnlyMode => spot_price::Error::NoSwapsAccepted,
+            Error::MaxBuyAmountExceeded { max, buy } => {
+                spot_price::Error::MaxBuyAmountExceeded { max, buy }
+            }
+            Error::BalanceTooLow { buy } => spot_price::Error::BalanceTooLow { buy },
+            Error::LatestRateFetchFailed(_) | Error::SellQuoteCalculationFailed(_) => {
+                spot_price::Error::Other
+            }
+        }
+    }
+}
+
+#[derive(Debug, thiserror::Error)]
+pub enum Error<LR>
+where
+    LR: LatestRate + Debug,
+{
+    #[error("ASB is running in resume-only mode")]
+    ResumeOnlyMode,
+    #[error("Maximum buy {max} exceeded {buy}")]
+    MaxBuyAmountExceeded {
+        max: bitcoin::Amount,
+        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 { buy: bitcoin::Amount },
+
+    #[error("Failed to fetch latest rate")]
+    LatestRateFetchFailed(LR::Error),
+
+    #[error("Failed to calculate quote: {0}")]
+    SellQuoteCalculationFailed(anyhow::Error),
+}

swap/src/protocol/bob.rs

@@ -16,6 +16,7 @@ pub mod cancel;
 pub mod event_loop;
 mod execution_setup;
 pub mod refund;
+mod spot_price;
 pub mod state;
 pub mod swap;
 

swap/src/protocol/bob/behaviour.rs

@@ -1,5 +1,6 @@
 use crate::network::quote::BidQuote;
 use crate::network::{encrypted_signature, quote, redial, spot_price, transfer_proof};
+use crate::protocol::bob;
 use crate::protocol::bob::{execution_setup, State2};
 use anyhow::{anyhow, Error, Result};
 use libp2p::core::Multiaddr;
@@ -71,7 +72,7 @@ impl Behaviour {
     pub fn new(alice: PeerId) -> Self {
         Self {
             quote: quote::bob(),
-            spot_price: spot_price::bob(),
+            spot_price: bob::spot_price::bob(),
             execution_setup: Default::default(),
             transfer_proof: transfer_proof::bob(),
             encrypted_signature: encrypted_signature::bob(),

swap/src/protocol/bob/event_loop.rs

@@ -2,6 +2,7 @@ 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;
 use crate::protocol::bob::{Behaviour, OutEvent, State0, State2};
 use crate::{bitcoin, monero};
 use anyhow::{bail, Context, Result};
@@ -270,6 +271,7 @@ impl EventLoopHandle {
         match response {
             Response::Xmr(xmr) => Ok(xmr),
             Response::Error(error) => {
+                let error: bob::spot_price::Error = error.into();
                 bail!(error);
             }
         }

swap/src/protocol/bob/spot_price.rs

@@ -0,0 +1,69 @@
+use crate::network::cbor_request_response::CborCodec;
+use crate::network::spot_price;
+use crate::network::spot_price::SpotPriceProtocol;
+use crate::protocol::bob::OutEvent;
+use libp2p::request_response::{ProtocolSupport, RequestResponseConfig};
+use libp2p::PeerId;
+
+const PROTOCOL: &str = spot_price::PROTOCOL;
+type SpotPriceOutEvent = spot_price::OutEvent;
+
+/// Constructs a new instance of the `spot-price` behaviour to be used by Bob.
+///
+/// Bob only supports outbound connections, i.e. requesting a spot price for a
+/// given amount of BTC in XMR.
+pub fn bob() -> spot_price::Behaviour {
+    spot_price::Behaviour::new(
+        CborCodec::default(),
+        vec![(SpotPriceProtocol, ProtocolSupport::Outbound)],
+        RequestResponseConfig::default(),
+    )
+}
+
+impl From<(PeerId, spot_price::Message)> for OutEvent {
+    fn from((peer, message): (PeerId, spot_price::Message)) -> Self {
+        match message {
+            spot_price::Message::Request { .. } => Self::unexpected_request(peer),
+            spot_price::Message::Response {
+                response,
+                request_id,
+            } => Self::SpotPriceReceived {
+                id: request_id,
+                response,
+            },
+        }
+    }
+}
+
+crate::impl_from_rr_event!(SpotPriceOutEvent, OutEvent, PROTOCOL);
+
+#[derive(Clone, Debug, thiserror::Error)]
+pub enum Error {
+    #[error("Seller currently does not accept incoming swap requests, please try again later")]
+    NoSwapsAccepted,
+    #[error("Seller refused to buy {buy} because the maximum configured buy limit is {max}")]
+    MaxBuyAmountExceeded {
+        max: bitcoin::Amount,
+        buy: bitcoin::Amount,
+    },
+    #[error("Seller's XMR balance is currently too low to fulfill the swap request to buy {buy}, please try again later")]
+    BalanceTooLow { buy: bitcoin::Amount },
+
+    /// To be used for errors that cannot be explained on the CLI side (e.g.
+    /// rate update problems on the seller side)
+    #[error("Seller encountered a problem, please try again later.")]
+    Other,
+}
+
+impl From<spot_price::Error> for Error {
+    fn from(error: spot_price::Error) -> Self {
+        match error {
+            spot_price::Error::NoSwapsAccepted => Error::NoSwapsAccepted,
+            spot_price::Error::MaxBuyAmountExceeded { max, buy } => {
+                Error::MaxBuyAmountExceeded { max, buy }
+            }
+            spot_price::Error::BalanceTooLow { buy } => Error::BalanceTooLow { buy },
+            spot_price::Error::Other => Error::Other,
+        }
+    }
+}