Simplify code within BobState::XmrLockProofReceived

To achieve this, we decompose `watch_for_locked_xmr` into two parts:

1. A non-self-consuming function to construct a `WatchRequest`
2. A state transition that can now consume `self` again because
it is only called once within the whole select! expression.

Ideally, we would move more logic onto this state transition (like
comparing the actual amounts and fail the transition if it is not
valid). Doing so would have an unfortunate side-effect: We would
always wait for the full confirmations before checking whether or
not we actually receive enough XMR.

This allows us to have state transitions that consume self.

swap/src/monero.rs

@@ -182,7 +182,7 @@ impl fmt::Display for TxHash {
 }
 
 #[derive(Debug, Clone, Copy, thiserror::Error)]
-#[error("transaction does not pay enough: expected {expected}, got {actual}")]
+#[error("expected {expected}, got {actual}")]
 pub struct InsufficientFunds {
     pub expected: Amount,
     pub actual: Amount,

swap/src/monero/wallet.rs

@@ -150,14 +150,15 @@ impl Wallet {
         ))
     }
 
-    pub async fn watch_for_transfer(
+    pub async fn watch_for_transfer(&self, request: WatchRequest) -> Result<()> {
-        &self,
+        let WatchRequest {
-        public_spend_key: PublicKey,
+            conf_target,
-        public_view_key: PublicViewKey,
+            public_view_key,
-        transfer_proof: TransferProof,
+            public_spend_key,
-        expected: Amount,
+            transfer_proof,
-        conf_target: u32,
+            expected,
-    ) -> Result<(), InsufficientFunds> {
+        } = request;
+
         let txid = transfer_proof.tx_hash();
 
         tracing::info!(%txid, "Waiting for {} confirmation{} of Monero transaction", conf_target, if conf_target > 1 { "s" } else { "" });
@@ -230,6 +231,15 @@ pub struct TransferRequest {
     pub amount: Amount,
 }
 
+#[derive(Debug)]
+pub struct WatchRequest {
+    pub public_spend_key: PublicKey,
+    pub public_view_key: PublicViewKey,
+    pub transfer_proof: TransferProof,
+    pub conf_target: u32,
+    pub expected: Amount,
+}
+
 async fn wait_for_confirmations<Fut>(
     txid: String,
     fetch_tx: impl Fn(String) -> Fut,

swap/src/protocol/bob/state.rs

@@ -3,7 +3,8 @@ use crate::bitcoin::{
     TxLock, Txid,
 };
 use crate::monero;
-use crate::monero::{monero_private_key, InsufficientFunds, TransferProof};
+use crate::monero::wallet::WatchRequest;
+use crate::monero::{monero_private_key, TransferProof};
 use crate::monero_ext::ScalarExt;
 use crate::protocol::alice::{Message1, Message3};
 use crate::protocol::bob::{EncryptedSignature, Message0, Message2, Message4};
@@ -305,30 +306,22 @@ pub struct State3 {
 }
 
 impl State3 {
-    pub async fn watch_for_lock_xmr(
+    pub fn lock_xmr_watch_request(&self, transfer_proof: TransferProof) -> WatchRequest {
-        self,
-        xmr_wallet: &monero::Wallet,
-        transfer_proof: TransferProof,
-        monero_wallet_restore_blockheight: BlockHeight,
-    ) -> Result<Result<State4, InsufficientFunds>> {
         let S_b_monero =
             monero::PublicKey::from_private_key(&monero::PrivateKey::from_scalar(self.s_b));
         let S = self.S_a_monero + S_b_monero;
 
-        if let Err(e) = xmr_wallet
+        WatchRequest {
-            .watch_for_transfer(
+            public_spend_key: S,
-                S,
+            public_view_key: self.v.public(),
-                self.v.public(),
             transfer_proof,
-                self.xmr,
+            conf_target: self.min_monero_confirmations,
-                self.min_monero_confirmations,
+            expected: self.xmr,
-            )
+        }
-            .await
-        {
-            return Ok(Err(e));
     }
 
-        Ok(Ok(State4 {
+    pub fn xmr_locked(self, monero_wallet_restore_blockheight: BlockHeight) -> State4 {
+        State4 {
             A: self.A,
             b: self.b,
             s_b: self.s_b,
@@ -342,7 +335,7 @@ impl State3 {
             tx_cancel_sig_a: self.tx_cancel_sig_a,
             tx_refund_encsig: self.tx_refund_encsig,
             monero_wallet_restore_blockheight,
-        }))
+        }
     }
 
     pub async fn wait_for_cancel_timelock_to_expire(
@@ -595,6 +588,18 @@ impl State6 {
         Ok(())
     }
 
+    pub async fn wait_for_cancel_timelock_to_expire(
+        &self,
+        bitcoin_wallet: &bitcoin::Wallet,
+    ) -> Result<()> {
+        bitcoin_wallet
+            .watch_until_status(&self.tx_lock, |status| {
+                status.is_confirmed_with(self.cancel_timelock)
+            })
+            .await?;
+        Ok(())
+    }
+
     pub fn tx_lock_id(&self) -> bitcoin::Txid {
         self.tx_lock.txid()
     }

swap/src/protocol/bob/swap.rs

@@ -1,7 +1,6 @@
 use crate::bitcoin::ExpiredTimelocks;
 use crate::database::{Database, Swap};
 use crate::env::Config;
-use crate::monero::InsufficientFunds;
 use crate::protocol::bob;
 use crate::protocol::bob::event_loop::EventLoopHandle;
 use crate::protocol::bob::state::*;
@@ -11,7 +10,7 @@ use async_recursion::async_recursion;
 use rand::rngs::OsRng;
 use std::sync::Arc;
 use tokio::select;
-use tracing::{trace, warn};
+use tracing::trace;
 use uuid::Uuid;
 
 pub fn is_complete(state: &BobState) -> bool {
@@ -143,34 +142,26 @@ async fn run_until_internal(
             if let ExpiredTimelocks::None = state.current_epoch(bitcoin_wallet.as_ref()).await? {
                 event_loop_handle.dial().await?;
 
-                let xmr_lock_watcher = state.clone().watch_for_lock_xmr(
+                let watch_request = state.lock_xmr_watch_request(lock_transfer_proof);
-                    monero_wallet.as_ref(),
-                    lock_transfer_proof,
-                    monero_wallet_restore_blockheight,
-                );
-                let cancel_timelock_expires =
-                    state.wait_for_cancel_timelock_to_expire(bitcoin_wallet.as_ref());
 
                 select! {
-                    state4 = xmr_lock_watcher => {
+                    received_xmr = monero_wallet.watch_for_transfer(watch_request) => {
-                        match state4? {
+                        match received_xmr {
-                            Ok(state4) => BobState::XmrLocked(state4),
+                            Ok(()) => BobState::XmrLocked(state.xmr_locked(monero_wallet_restore_blockheight)),
-                            Err(InsufficientFunds {..}) => {
+                            Err(e) => {
-                                 warn!("The other party has locked insufficient Monero funds! Waiting for refund...");
+                                 tracing::warn!("Waiting for refund because insufficient Monero have been locked! {}", e);
                                  state.wait_for_cancel_timelock_to_expire(bitcoin_wallet.as_ref()).await?;
-                                 let state4 = state.cancel();
+
-                                 BobState::CancelTimelockExpired(state4)
+                                 BobState::CancelTimelockExpired(state.cancel())
                             },
                         }
-                    },
+                    }
-                    _ = cancel_timelock_expires => {
+                    _ = state.wait_for_cancel_timelock_to_expire(bitcoin_wallet.as_ref()) => {
-                        let state4 = state.cancel();
+                        BobState::CancelTimelockExpired(state.cancel())
-                        BobState::CancelTimelockExpired(state4)
                     }
                 }
             } else {
-                let state4 = state.cancel();
+                BobState::CancelTimelockExpired(state.cancel())
-                BobState::CancelTimelockExpired(state4)
             }
         }
         BobState::XmrLocked(state) => {