Merge pull request #48 from comit-network/revert-35-no-transfer-proof

Revert "No need to send Monero transfer proof from Alice to Bob"

monero-harness/Cargo.toml

@@ -18,7 +18,3 @@ testcontainers = "0.11"
 tokio = { version = "0.2", default-features = false, features = ["blocking", "macros", "rt-core", "time"] }
 tracing = "0.1"
 url = "2"
-
-[dev-dependencies]
-curve25519-dalek = "2"
-monero = "0.9"

monero-harness/src/lib.rs

@@ -136,7 +136,11 @@ impl<'c> Monero {
         monerod.start_miner(&miner_address).await?;
 
         tracing::info!("Waiting for miner wallet to catch up...");
-        miner_wallet.refresh().await?;
+        let block_height = monerod.client().get_block_count().await?;
+        miner_wallet
+            .wait_for_wallet_height(block_height)
+            .await
+            .unwrap();
 
         Ok(())
     }

monero-harness/src/rpc/wallet.rs

@@ -236,14 +236,14 @@ impl Client {
     pub async fn generate_from_keys(
         &self,
         address: &str,
-        spend_key: Option<&str>,
+        spend_key: &str,
         view_key: &str,
     ) -> Result<GenerateFromKeys> {
         let params = GenerateFromKeysParams {
             restore_height: 0,
             filename: view_key.into(),
             address: address.into(),
-            spendkey: spend_key.map(|sk| sk.into()),
+            spendkey: spend_key.into(),
             viewkey: view_key.into(),
             password: "".into(),
             autosave_current: true,
@@ -400,8 +400,7 @@ pub struct GenerateFromKeysParams {
     pub restore_height: u32,
     pub filename: String,
     pub address: String,
-    #[serde(skip_serializing_if = "Option::is_none")]
+    pub spendkey: String,
-    pub spendkey: Option<String>,
     pub viewkey: String,
     pub password: String,
     pub autosave_current: bool,

monero-harness/tests/wallet.rs

@@ -1,7 +1,4 @@
-use curve25519_dalek::scalar::Scalar;
-use monero::{Address, Network, PrivateKey, PublicKey};
 use monero_harness::Monero;
-use rand::rngs::OsRng;
 use spectral::prelude::*;
 use testcontainers::clients::Cli;
 
@@ -64,51 +61,3 @@ async fn fund_transfer_and_check_tx_key() {
 
     assert_that!(res.received).is_equal_to(send_to_bob);
 }
-
-#[tokio::test]
-async fn load_watch_only_wallet() {
-    let tc = Cli::default();
-    let (monero, _containers) = Monero::new(&tc, None, vec!["watch-only".into()])
-        .await
-        .unwrap();
-    let miner_wallet = monero.wallet("miner").unwrap();
-
-    let (_spend_sk, spend_pk) = {
-        let scalar = Scalar::random(&mut OsRng);
-        let sk = PrivateKey::from_scalar(scalar);
-        let pk = PublicKey::from_private_key(&sk);
-
-        (sk, pk)
-    };
-    let (view_sk, view_pk) = {
-        let scalar = Scalar::random(&mut OsRng);
-        let sk = PrivateKey::from_scalar(scalar);
-        let pk = PublicKey::from_private_key(&sk);
-
-        (sk, pk)
-    };
-    let address = Address::standard(Network::Mainnet, spend_pk, view_pk);
-
-    let one_xmr = 1_000_000_000_000;
-
-    monero.init(Vec::new()).await.unwrap();
-
-    miner_wallet
-        .client()
-        .transfer(0, one_xmr, &address.to_string())
-        .await
-        .unwrap();
-
-    let watch_only_wallet = monero.wallet("watch-only").unwrap();
-
-    watch_only_wallet
-        .client()
-        .generate_from_keys(&address.to_string(), None, &view_sk.to_string())
-        .await
-        .unwrap();
-    watch_only_wallet.client().refresh().await.unwrap();
-
-    let balance = watch_only_wallet.client().get_balance(0).await.unwrap();
-
-    assert_eq!(balance, one_xmr);
-}

swap/src/alice.rs

@@ -45,13 +45,31 @@ use xmr_btc::{
 
 pub async fn swap(
     bitcoin_wallet: Arc<bitcoin::Wallet>,
-    monero_wallet: Arc<monero::Facade>,
+    monero_wallet: Arc<monero::Wallet>,
     db: Database,
     listen: Multiaddr,
     transport: SwapTransport,
     behaviour: Alice,
 ) -> Result<()> {
-    struct Network(Arc<Mutex<Swarm>>);
+    struct Network {
+        swarm: Arc<Mutex<Swarm>>,
+        channel: Option<ResponseChannel<AliceToBob>>,
+    }
+
+    impl Network {
+        pub async fn send_message2(&mut self, proof: monero::TransferProof) {
+            match self.channel.take() {
+                None => warn!("Channel not found, did you call this twice?"),
+                Some(channel) => {
+                    let mut guard = self.swarm.lock().await;
+                    guard.send_message2(channel, alice::Message2 {
+                        tx_lock_proof: proof,
+                    });
+                    info!("Sent transfer proof");
+                }
+            }
+        }
+    }
 
     // TODO: For retry, use `backoff::ExponentialBackoff` in production as opposed
     // to `ConstantBackoff`.
@@ -62,7 +80,7 @@ pub async fn swap(
             struct UnexpectedMessage;
 
             let encsig = (|| async {
-                let mut guard = self.0.lock().await;
+                let mut guard = self.swarm.lock().await;
                 let encsig = match guard.next().await {
                     OutEvent::Message3(msg) => msg.tx_redeem_encsig,
                     other => {
@@ -151,8 +169,11 @@ pub async fn swap(
     let msg = state2.next_message();
     swarm.send_message1(channel, msg);
 
-    let state3 = match swarm.next().await {
+    let (state3, channel) = match swarm.next().await {
-        OutEvent::Message2(msg) => state2.receive(msg)?,
+        OutEvent::Message2 { msg, channel } => {
+            let state3 = state2.receive(msg)?;
+            (state3, channel)
+        }
         other => panic!("Unexpected event: {:?}", other),
     };
 
@@ -162,12 +183,14 @@ pub async fn swap(
 
     info!("Handshake complete, we now have State3 for Alice.");
 
-    let network = Network(Arc::new(Mutex::new(swarm)));
+    let network = Arc::new(Mutex::new(Network {
+        swarm: Arc::new(Mutex::new(swarm)),
+        channel: Some(channel),
+    }));
 
     let mut action_generator = action_generator(
-        network,
+        network.clone(),
         bitcoin_wallet.clone(),
-        monero_wallet.clone(),
         state3.clone(),
         TX_LOCK_MINE_TIMEOUT,
     );
@@ -186,12 +209,16 @@ pub async fn swap(
                 db.insert_latest_state(swap_id, state::Alice::BtcLocked(state3.clone()).into())
                     .await?;
 
-                let _ = monero_wallet
+                let (transfer_proof, _) = monero_wallet
                     .transfer(public_spend_key, public_view_key, amount)
                     .await?;
 
                 db.insert_latest_state(swap_id, state::Alice::XmrLocked(state3.clone()).into())
                     .await?;
+
+                let mut guard = network.as_ref().lock().await;
+                guard.send_message2(transfer_proof).await;
+                info!("Sent transfer proof");
             }
 
             GeneratorState::Yielded(Action::RedeemBtc(tx)) => {
@@ -276,7 +303,10 @@ pub enum OutEvent {
         msg: bob::Message1,
         channel: ResponseChannel<AliceToBob>,
     },
-    Message2(bob::Message2),
+    Message2 {
+        msg: bob::Message2,
+        channel: ResponseChannel<AliceToBob>,
+    },
     Message3(bob::Message3),
 }
 
@@ -315,7 +345,7 @@ impl From<message1::OutEvent> for OutEvent {
 impl From<message2::OutEvent> for OutEvent {
     fn from(event: message2::OutEvent) -> Self {
         match event {
-            message2::OutEvent::Msg(msg) => OutEvent::Message2(msg),
+            message2::OutEvent::Msg { msg, channel } => OutEvent::Message2 { msg, channel },
         }
     }
 }
@@ -374,6 +404,16 @@ impl Alice {
         self.message1.send(channel, msg);
         debug!("Sent Message1");
     }
+
+    /// Send Message2 to Bob in response to receiving his Message2.
+    pub fn send_message2(
+        &mut self,
+        channel: ResponseChannel<AliceToBob>,
+        msg: xmr_btc::alice::Message2,
+    ) {
+        self.message2.send(channel, msg);
+        debug!("Sent Message2");
+    }
 }
 
 impl Default for Alice {

swap/src/alice/message2.rs

@@ -1,7 +1,7 @@
 use libp2p::{
     request_response::{
         handler::RequestProtocol, ProtocolSupport, RequestResponse, RequestResponseConfig,
-        RequestResponseEvent, RequestResponseMessage,
+        RequestResponseEvent, RequestResponseMessage, ResponseChannel,
     },
     swarm::{NetworkBehaviourAction, NetworkBehaviourEventProcess, PollParameters},
     NetworkBehaviour,
@@ -18,7 +18,12 @@ use xmr_btc::bob;
 
 #[derive(Debug)]
 pub enum OutEvent {
-    Msg(bob::Message2),
+    Msg {
+        /// Received message from Bob.
+        msg: bob::Message2,
+        /// Channel to send back Alice's message 2.
+        channel: ResponseChannel<AliceToBob>,
+    },
 }
 
 /// A `NetworkBehaviour` that represents receiving of message 2 from Bob.
@@ -32,6 +37,11 @@ pub struct Message2 {
 }
 
 impl Message2 {
+    pub fn send(&mut self, channel: ResponseChannel<AliceToBob>, msg: xmr_btc::alice::Message2) {
+        let msg = AliceToBob::Message2(msg);
+        self.rr.send_response(channel, msg);
+    }
+
     fn poll(
         &mut self,
         _: &mut Context<'_>,
@@ -74,10 +84,8 @@ impl NetworkBehaviourEventProcess<RequestResponseEvent<BobToAlice, AliceToBob>>
             } => {
                 if let BobToAlice::Message2(msg) = request {
                     debug!("Received Message2");
-                    self.events.push_back(OutEvent::Msg(msg));
+                    self.events.push_back(OutEvent::Msg { msg, channel });
                 }
-                // Send back empty response so that the request/response protocol completes.
-                self.rr.send_response(channel, AliceToBob::Message2);
             }
             RequestResponseEvent::Message {
                 message: RequestResponseMessage::Response { .. },

swap/src/bob.rs

@@ -1,15 +1,18 @@
 //! Run an XMR/BTC swap in the role of Bob.
 //! Bob holds BTC and wishes receive XMR.
 use anyhow::Result;
+use async_trait::async_trait;
+use backoff::{backoff::Constant as ConstantBackoff, future::FutureOperation as _};
 use futures::{
     channel::mpsc::{Receiver, Sender},
-    StreamExt,
+    FutureExt, StreamExt,
 };
 use genawaiter::GeneratorState;
 use libp2p::{core::identity::Keypair, Multiaddr, NetworkBehaviour, PeerId};
 use rand::rngs::OsRng;
-use std::{process, sync::Arc};
+use std::{process, sync::Arc, time::Duration};
-use tracing::{debug, info};
+use tokio::sync::Mutex;
+use tracing::{debug, info, warn};
 use uuid::Uuid;
 
 mod amounts;
@@ -34,14 +37,14 @@ use crate::{
 use xmr_btc::{
     alice,
     bitcoin::{BroadcastSignedTransaction, EncryptedSignature, SignTxLock},
-    bob::{self, action_generator, State0},
+    bob::{self, action_generator, ReceiveTransferProof, State0},
     monero::CreateWalletForOutput,
 };
 
 #[allow(clippy::too_many_arguments)]
 pub async fn swap(
     bitcoin_wallet: Arc<bitcoin::Wallet>,
-    monero_wallet: Arc<monero::Facade>,
+    monero_wallet: Arc<monero::Wallet>,
     db: Database,
     btc: u64,
     addr: Multiaddr,
@@ -50,6 +53,40 @@ pub async fn swap(
     transport: SwapTransport,
     behaviour: Bob,
 ) -> Result<()> {
+    struct Network(Swarm);
+
+    // TODO: For retry, use `backoff::ExponentialBackoff` in production as opposed
+    // to `ConstantBackoff`.
+
+    #[async_trait]
+    impl ReceiveTransferProof for Network {
+        async fn receive_transfer_proof(&mut self) -> monero::TransferProof {
+            #[derive(Debug)]
+            struct UnexpectedMessage;
+
+            let future = self.0.next().shared();
+
+            let proof = (|| async {
+                let proof = match future.clone().await {
+                    OutEvent::Message2(msg) => msg.tx_lock_proof,
+                    other => {
+                        warn!("Expected transfer proof, got: {:?}", other);
+                        return Err(backoff::Error::Transient(UnexpectedMessage));
+                    }
+                };
+
+                Result::<_, backoff::Error<UnexpectedMessage>>::Ok(proof)
+            })
+            .retry(ConstantBackoff::new(Duration::from_secs(1)))
+            .await
+            .expect("transient errors to be retried");
+
+            info!("Received transfer proof");
+
+            proof
+        }
+    }
+
     let mut swarm = new_swarm(transport, behaviour)?;
 
     libp2p::Swarm::dial_addr(&mut swarm, addr)?;
@@ -112,19 +149,13 @@ pub async fn swap(
         .await?;
 
     swarm.send_message2(alice.clone(), state2.next_message());
-    // NOTE: We must poll the swarm after `send_messageX` to actually trigger
-    // sending the message. This is really weird to me and has been a constant
-    // source of bugs. Is this the only way?
-    match swarm.next().await {
-        OutEvent::Message2 => {
-            debug!("Got message 3 response from Alice");
-        }
-        other => panic!("unexpected event: {:?}", other),
-    };
 
     info!("Handshake complete");
 
+    let network = Arc::new(Mutex::new(Network(swarm)));
+
     let mut action_generator = action_generator(
+        network.clone(),
         monero_wallet.clone(),
         bitcoin_wallet.clone(),
         state2.clone(),
@@ -151,14 +182,20 @@ pub async fn swap(
             GeneratorState::Yielded(bob::Action::SendBtcRedeemEncsig(tx_redeem_encsig)) => {
                 db.insert_latest_state(swap_id, state::Bob::XmrLocked(state2.clone()).into())
                     .await?;
-                swarm.send_message3(alice.clone(), tx_redeem_encsig);
+
-                match swarm.next().await {
+                let mut guard = network.as_ref().lock().await;
+                guard.0.send_message3(alice.clone(), tx_redeem_encsig);
+                info!("Sent Bitcoin redeem encsig");
+
+                // FIXME: Having to wait for Alice's response here is a big problem, because
+                // we're stuck if she doesn't send her response back. I believe this is
+                // currently necessary, so we may have to rework this and/or how we use libp2p
+                match guard.0.next().shared().await {
                     OutEvent::Message3 => {
-                        debug!("Got message 3 response from Alice");
+                        debug!("Got Message3 empty response");
                     }
                     other => panic!("unexpected event: {:?}", other),
                 };
-                info!("Sent Bitcoin redeem encsig");
             }
             GeneratorState::Yielded(bob::Action::CreateXmrWalletForOutput {
                 spend_key,
@@ -220,7 +257,7 @@ pub enum OutEvent {
     Amounts(SwapAmounts),
     Message0(alice::Message0),
     Message1(alice::Message1),
-    Message2,
+    Message2(alice::Message2),
     Message3,
 }
 
@@ -261,7 +298,7 @@ impl From<message1::OutEvent> for OutEvent {
 impl From<message2::OutEvent> for OutEvent {
     fn from(event: message2::OutEvent) -> Self {
         match event {
-            message2::OutEvent::Msg => OutEvent::Message2,
+            message2::OutEvent::Msg(msg) => OutEvent::Message2(msg),
         }
     }
 }

swap/src/bob/message2.rs

@@ -11,14 +11,14 @@ use std::{
     task::{Context, Poll},
     time::Duration,
 };
-use tracing::error;
+use tracing::{debug, error};
 
 use crate::network::request_response::{AliceToBob, BobToAlice, Codec, Message2Protocol, TIMEOUT};
-use xmr_btc::bob;
+use xmr_btc::{alice, bob};
 
 #[derive(Debug)]
 pub enum OutEvent {
-    Msg,
+    Msg(alice::Message2),
 }
 
 /// A `NetworkBehaviour` that represents sending message 2 to Alice.
@@ -78,8 +78,9 @@ impl NetworkBehaviourEventProcess<RequestResponseEvent<BobToAlice, AliceToBob>>
                 message: RequestResponseMessage::Response { response, .. },
                 ..
             } => {
-                if let AliceToBob::Message2 = response {
+                if let AliceToBob::Message2(msg) = response {
-                    self.events.push_back(OutEvent::Msg);
+                    debug!("Received Message2");
+                    self.events.push_back(OutEvent::Msg(msg));
                 }
             }
             RequestResponseEvent::InboundFailure { error, .. } => {

swap/src/cli.rs

@@ -2,15 +2,6 @@ use libp2p::core::Multiaddr;
 use url::Url;
 use uuid::Uuid;
 
-// TODO: Remove monero_watch_only_wallet_rpc_url options.
-//
-// We need an extra `monero-wallet-rpc` to monitor the shared output without
-// unloading the user's Monero wallet. A better approach than passing in an
-// extra argument (and requiring the user to start up 2 `monero-wallet-rpc`
-// instances), may be to start up another `monero-wallet-rpc` instance as
-// part of executing this binary (i.e. requiring `monero-wallet-rpc` to be in
-// the path).
-
 #[derive(structopt::StructOpt, Debug)]
 #[structopt(name = "xmr-btc-swap", about = "Trustless XMR BTC swaps")]
 pub enum Options {
@@ -18,17 +9,8 @@ pub enum Options {
         #[structopt(default_value = "http://127.0.0.1:8332", long = "bitcoind")]
         bitcoind_url: Url,
 
-        #[structopt(
+        #[structopt(default_value = "http://127.0.0.1:18083/json_rpc", long = "monerod")]
-            default_value = "http://127.0.0.1:18083/json_rpc",
+        monerod_url: Url,
-            long = "monero_wallet_rpc"
-        )]
-        monero_wallet_rpc_url: Url,
-
-        #[structopt(
-            default_value = "http://127.0.0.1:18084",
-            long = "monero_watch_only_wallet_rpc"
-        )]
-        monero_watch_only_wallet_rpc_url: Url,
 
         #[structopt(default_value = "/ip4/127.0.0.1/tcp/9876", long = "listen-addr")]
         listen_addr: Multiaddr,
@@ -46,17 +28,8 @@ pub enum Options {
         #[structopt(default_value = "http://127.0.0.1:8332", long = "bitcoind")]
         bitcoind_url: Url,
 
-        #[structopt(
+        #[structopt(default_value = "http://127.0.0.1:18083/json_rpc", long = "monerod")]
-            default_value = "http://127.0.0.1:18083/json_rpc",
+        monerod_url: Url,
-            long = "monero_wallet_rpc"
-        )]
-        monero_wallet_rpc_url: Url,
-
-        #[structopt(
-            default_value = "http://127.0.0.1:18084",
-            long = "monero_watch_only_wallet_rpc"
-        )]
-        monero_watch_only_wallet_rpc_url: Url,
 
         #[structopt(long = "tor")]
         tor: bool,
@@ -69,16 +42,7 @@ pub enum Options {
         #[structopt(default_value = "http://127.0.0.1:8332", long = "bitcoind")]
         bitcoind_url: Url,
 
-        #[structopt(
+        #[structopt(default_value = "http://127.0.0.1:18083/json_rpc", long = "monerod")]
-            default_value = "http://127.0.0.1:18083/json_rpc",
+        monerod_url: Url,
-            long = "monero_wallet_rpc"
-        )]
-        monero_wallet_rpc_url: Url,
-
-        #[structopt(
-            default_value = "http://127.0.0.1:18084",
-            long = "monero_watch_only_wallet_rpc"
-        )]
-        monero_watch_only_wallet_rpc_url: Url,
     },
 }

swap/src/main.rs

@@ -53,8 +53,7 @@ async fn main() -> Result<()> {
     match opt {
         Options::Alice {
             bitcoind_url,
-            monero_wallet_rpc_url,
+            monerod_url,
-            monero_watch_only_wallet_rpc_url,
             listen_addr,
             tor_port,
         } => {
@@ -87,10 +86,7 @@ async fn main() -> Result<()> {
                 .expect("failed to create bitcoin wallet");
             let bitcoin_wallet = Arc::new(bitcoin_wallet);
 
-            let monero_wallet = Arc::new(monero::Facade::new(
+            let monero_wallet = Arc::new(monero::Wallet::new(monerod_url));
-                monero_wallet_rpc_url,
-                monero_watch_only_wallet_rpc_url,
-            ));
 
             swap_as_alice(
                 bitcoin_wallet,
@@ -106,8 +102,7 @@ async fn main() -> Result<()> {
             alice_addr,
             satoshis,
             bitcoind_url,
-            monero_wallet_rpc_url,
+            monerod_url,
-            monero_watch_only_wallet_rpc_url,
             tor,
         } => {
             info!("running swap node as Bob ...");
@@ -125,10 +120,7 @@ async fn main() -> Result<()> {
                 .expect("failed to create bitcoin wallet");
             let bitcoin_wallet = Arc::new(bitcoin_wallet);
 
-            let monero_wallet = Arc::new(monero::Facade::new(
+            let monero_wallet = Arc::new(monero::Wallet::new(monerod_url));
-                monero_wallet_rpc_url,
-                monero_watch_only_wallet_rpc_url,
-            ));
 
             swap_as_bob(
                 bitcoin_wallet,
@@ -156,15 +148,13 @@ async fn main() -> Result<()> {
         Options::Recover {
             swap_id,
             bitcoind_url,
-            monero_wallet_rpc_url,
+            monerod_url,
-            monero_watch_only_wallet_rpc_url,
         } => {
             let state = db.get_state(swap_id)?;
             let bitcoin_wallet = bitcoin::Wallet::new("bob", bitcoind_url)
                 .await
                 .expect("failed to create bitcoin wallet");
-            let monero_wallet =
+            let monero_wallet = monero::Wallet::new(monerod_url);
-                monero::Facade::new(monero_wallet_rpc_url, monero_watch_only_wallet_rpc_url);
 
             recover(bitcoin_wallet, monero_wallet, state).await?;
         }
@@ -193,7 +183,7 @@ async fn create_tor_service(
 
 async fn swap_as_alice(
     bitcoin_wallet: Arc<swap::bitcoin::Wallet>,
-    monero_wallet: Arc<swap::monero::Facade>,
+    monero_wallet: Arc<swap::monero::Wallet>,
     db: Database,
     addr: Multiaddr,
     transport: SwapTransport,
@@ -212,7 +202,7 @@ async fn swap_as_alice(
 
 async fn swap_as_bob(
     bitcoin_wallet: Arc<swap::bitcoin::Wallet>,
-    monero_wallet: Arc<swap::monero::Facade>,
+    monero_wallet: Arc<swap::monero::Wallet>,
     db: Database,
     sats: u64,
     alice: Multiaddr,

swap/src/monero.rs

@@ -1,59 +1,54 @@
 use anyhow::Result;
 use async_trait::async_trait;
 use backoff::{backoff::Constant as ConstantBackoff, future::FutureOperation as _};
-use futures::TryFutureExt;
-use monero::{Address, Network};
 use monero_harness::rpc::wallet;
-use std::time::Duration;
+use std::{str::FromStr, time::Duration};
 use url::Url;
-pub use xmr_btc::monero::{
-    Amount, CreateWalletForOutput, InsufficientFunds, PrivateViewKey, PublicKey, PublicViewKey,
-    Transfer, *,
-};
 
-pub struct Facade {
+pub use xmr_btc::monero::*;
-    pub user_wallet: wallet::Client,
-    pub watch_only_wallet: wallet::Client,
-}
 
-impl Facade {
+pub struct Wallet(pub wallet::Client);
-    pub fn new(user_url: Url, watch_only_url: Url) -> Self {
+
-        Self {
+impl Wallet {
-            user_wallet: wallet::Client::new(user_url),
+    pub fn new(url: Url) -> Self {
-            watch_only_wallet: wallet::Client::new(watch_only_url),
+        Self(wallet::Client::new(url))
-        }
     }
 
     /// Get the balance of the primary account.
     pub async fn get_balance(&self) -> Result<Amount> {
-        let amount = self.user_wallet.get_balance(0).await?;
+        let amount = self.0.get_balance(0).await?;
 
         Ok(Amount::from_piconero(amount))
     }
 }
 
 #[async_trait]
-impl Transfer for Facade {
+impl Transfer for Wallet {
     async fn transfer(
         &self,
         public_spend_key: PublicKey,
         public_view_key: PublicViewKey,
         amount: Amount,
-    ) -> Result<Amount> {
+    ) -> Result<(TransferProof, Amount)> {
         let destination_address =
             Address::standard(Network::Mainnet, public_spend_key, public_view_key.into());
 
         let res = self
-            .user_wallet
+            .0
             .transfer(0, amount.as_piconero(), &destination_address.to_string())
             .await?;
 
-        Ok(Amount::from_piconero(res.fee))
+        let tx_hash = TxHash(res.tx_hash);
+        let tx_key = PrivateKey::from_str(&res.tx_key)?;
+
+        let fee = Amount::from_piconero(res.fee);
+
+        Ok((TransferProof::new(tx_hash, tx_key), fee))
     }
 }
 
 #[async_trait]
-impl CreateWalletForOutput for Facade {
+impl CreateWalletForOutput for Wallet {
     async fn create_and_load_wallet_for_output(
         &self,
         private_spend_key: PrivateKey,
@@ -65,10 +60,10 @@ impl CreateWalletForOutput for Facade {
         let address = Address::standard(Network::Mainnet, public_spend_key, public_view_key);
 
         let _ = self
-            .user_wallet
+            .0
             .generate_from_keys(
                 &address.to_string(),
-                Some(&private_spend_key.to_string()),
+                &private_spend_key.to_string(),
                 &PrivateKey::from(private_view_key).to_string(),
             )
             .await?;
@@ -81,57 +76,57 @@ impl CreateWalletForOutput for Facade {
 // to `ConstantBackoff`.
 
 #[async_trait]
-impl WatchForTransfer for Facade {
+impl WatchForTransfer for Wallet {
     async fn watch_for_transfer(
         &self,
-        address: Address,
+        public_spend_key: PublicKey,
+        public_view_key: PublicViewKey,
+        transfer_proof: TransferProof,
         expected_amount: Amount,
-        private_view_key: PrivateViewKey,
+        expected_confirmations: u32,
-    ) {
+    ) -> Result<(), InsufficientFunds> {
-        let address = address.to_string();
+        enum Error {
-        let private_view_key = PrivateKey::from(private_view_key).to_string();
+            TxNotFound,
-        let load_address = || {
+            InsufficientConfirmations,
-            self.watch_only_wallet
+            InsufficientFunds { expected: Amount, actual: Amount },
-                .generate_from_keys(&address, None, &private_view_key)
+        }
-                .map_err(backoff::Error::Transient)
-        };
 
-        // QUESTION: Should we really retry every error?
+        let address = Address::standard(Network::Mainnet, public_spend_key, public_view_key.into());
-        load_address
-            .retry(ConstantBackoff::new(Duration::from_secs(1)))
-            .await
-            .expect("transient error is never returned");
 
-        // QUESTION: Should we retry this error at all?
+        let res = (|| async {
-        let refresh = || {
+            // NOTE: Currently, this is conflating IO errors with the transaction not being
-            self.watch_only_wallet
+            // in the blockchain yet, or not having enough confirmations on it. All these
-                .refresh()
+            // errors warrant a retry, but the strategy should probably differ per case
-                .map_err(backoff::Error::Transient)
+            let proof = self
-        };
+                .0
-
+                .check_tx_key(
-        refresh
+                    &String::from(transfer_proof.tx_hash()),
-            .retry(ConstantBackoff::new(Duration::from_secs(1)))
+                    &transfer_proof.tx_key().to_string(),
-            .await
+                    &address.to_string(),
-            .expect("transient error is never returned");
+                )
-
-        let check_balance = || async {
-            let balance = self
-                .watch_only_wallet
-                .get_balance(0)
                 .await
-                .map_err(|_| backoff::Error::Transient("io"))?;
+                .map_err(|_| backoff::Error::Transient(Error::TxNotFound))?;
-            let balance = Amount::from_piconero(balance);
 
-            if balance != expected_amount {
+            if proof.received != expected_amount.as_piconero() {
-                return Err(backoff::Error::Transient("insufficient funds"));
+                return Err(backoff::Error::Permanent(Error::InsufficientFunds {
+                    expected: expected_amount,
+                    actual: Amount::from_piconero(proof.received),
+                }));
             }
 
-            Ok(())
+            if proof.confirmations < expected_confirmations {
+                return Err(backoff::Error::Transient(Error::InsufficientConfirmations));
+            }
+
+            Ok(proof)
+        })
+        .retry(ConstantBackoff::new(Duration::from_secs(1)))
+        .await;
+
+        if let Err(Error::InsufficientFunds { expected, actual }) = res {
+            return Err(InsufficientFunds { expected, actual });
         };
 
-        check_balance
+        Ok(())
-            .retry(ConstantBackoff::new(Duration::from_secs(1)))
-            .await
-            .expect("transient error is never returned");
     }
 }

swap/src/network/request_response.rs

@@ -40,7 +40,7 @@ pub enum AliceToBob {
     Amounts(SwapAmounts),
     Message0(alice::Message0),
     Message1(alice::Message1),
-    Message2, // empty response
+    Message2(alice::Message2),
     Message3, // empty response
 }
 

swap/src/recover.rs

@@ -29,8 +29,8 @@ use xmr_btc::bitcoin::{
 };
 
 pub async fn recover(
-    bitcoin_wallet: crate::bitcoin::Wallet,
+    bitcoin_wallet: bitcoin::Wallet,
-    monero_wallet: crate::monero::Facade,
+    monero_wallet: monero::Wallet,
     state: Swap,
 ) -> Result<()> {
     match state {
@@ -40,8 +40,8 @@ pub async fn recover(
 }
 
 pub async fn alice_recover(
-    bitcoin_wallet: crate::bitcoin::Wallet,
+    bitcoin_wallet: bitcoin::Wallet,
-    monero_wallet: crate::monero::Facade,
+    monero_wallet: monero::Wallet,
     state: Alice,
 ) -> Result<()> {
     match state {
@@ -368,7 +368,7 @@ pub async fn alice_recover(
 
 pub async fn bob_recover(
     bitcoin_wallet: crate::bitcoin::Wallet,
-    monero_wallet: crate::monero::Facade,
+    monero_wallet: crate::monero::Wallet,
     state: Bob,
 ) -> Result<()> {
     match state {

swap/tests/e2e.rs

@@ -8,6 +8,11 @@ use tempfile::tempdir;
 use testcontainers::clients::Cli;
 use xmr_btc::bitcoin;
 
+// NOTE: For some reason running these tests overflows the stack. In order to
+// mitigate this run them with:
+//
+//     RUST_MIN_STACK=100000000 cargo test
+
 #[tokio::test]
 async fn swap() {
     use tracing_subscriber::util::SubscriberInitExt as _;
@@ -50,30 +55,23 @@ async fn swap() {
         .await
         .unwrap();
 
-    let (monero, _container) = Monero::new(&cli, None, vec![
+    let (monero, _container) =
-        "alice".to_string(),
+        Monero::new(&cli, None, vec!["alice".to_string(), "bob".to_string()])
-        "alice-watch-only".to_string(),
+            .await
-        "bob".to_string(),
+            .unwrap();
-        "bob-watch-only".to_string(),
-    ])
-    .await
-    .unwrap();
     monero
         .init(vec![("alice", xmr_alice), ("bob", xmr_bob)])
         .await
         .unwrap();
 
-    let alice_xmr_wallet = Arc::new(swap::monero::Facade {
+    let alice_xmr_wallet = Arc::new(swap::monero::Wallet(
-        user_wallet: monero.wallet("alice").unwrap().client(),
+        monero.wallet("alice").unwrap().client(),
-        watch_only_wallet: monero.wallet("alice-watch-only").unwrap().client(),
+    ));
-    });
+    let bob_xmr_wallet = Arc::new(swap::monero::Wallet(monero.wallet("bob").unwrap().client()));
-    let bob_xmr_wallet = Arc::new(swap::monero::Facade {
-        user_wallet: monero.wallet("bob").unwrap().client(),
-        watch_only_wallet: monero.wallet("bob-watch-only").unwrap().client(),
-    });
 
     let alice_behaviour = alice::Alice::default();
     let alice_transport = build(alice_behaviour.identity()).unwrap();
+
     let db = Database::open(std::path::Path::new("../.swap-db/")).unwrap();
     let alice_swap = alice::swap(
         alice_btc_wallet.clone(),
@@ -112,7 +110,7 @@ async fn swap() {
 
     let xmr_alice_final = alice_xmr_wallet.as_ref().get_balance().await.unwrap();
 
-    bob_xmr_wallet.as_ref().user_wallet.refresh().await.unwrap();
+    bob_xmr_wallet.as_ref().0.refresh().await.unwrap();
     let xmr_bob_final = bob_xmr_wallet.as_ref().get_balance().await.unwrap();
 
     assert_eq!(

xmr-btc/src/alice.rs

@@ -24,14 +24,16 @@ use std::{
     sync::Arc,
     time::Duration,
 };
-use tokio::time::timeout;
+use tokio::{sync::Mutex, time::timeout};
 use tracing::{error, info};
 
 pub mod message;
-pub use message::{Message, Message0, Message1};
+pub use message::{Message, Message0, Message1, Message2};
 
 #[derive(Debug)]
 pub enum Action {
+    // This action also includes proving to Bob that this has happened, given that our current
+    // protocol requires a transfer proof to verify that the coins have been locked on Monero
     LockXmr {
         amount: monero::Amount,
         public_spend_key: monero::PublicKey,
@@ -59,10 +61,9 @@ pub trait ReceiveBitcoinRedeemEncsig {
 ///
 /// The argument `bitcoin_tx_lock_timeout` is used to determine how long we will
 /// wait for Bob, the counterparty, to lock up the bitcoin.
-pub fn action_generator<N, B, M>(
+pub fn action_generator<N, B>(
-    mut network: N,
+    network: Arc<Mutex<N>>,
     bitcoin_client: Arc<B>,
-    monero_client: Arc<M>,
     // TODO: Replace this with a new, slimmer struct?
     State3 {
         a,
@@ -92,13 +93,10 @@ where
         + Send
         + Sync
         + 'static,
-    M: monero::WatchForTransfer + Send + Sync + 'static,
 {
-    #[allow(clippy::enum_variant_names)]
     #[derive(Debug)]
     enum SwapFailed {
         BeforeBtcLock(Reason),
-        AfterBtcLock(Reason),
         AfterXmrLock(Reason),
     }
 
@@ -147,30 +145,21 @@ where
             let S_a = monero::PublicKey::from_private_key(&monero::PrivateKey {
                 scalar: s_a.into_ed25519(),
             });
-            let S = S_a + S_b_monero;
 
             co.yield_(Action::LockXmr {
                 amount: xmr,
-                public_spend_key: S,
+                public_spend_key: S_a + S_b_monero,
                 public_view_key: v.public(),
             })
             .await;
 
-            let monero_joint_address =
+            // TODO: Watch for LockXmr using watch-only wallet. Doing so will prevent Alice
-                monero::Address::standard(monero::Network::Mainnet, S, v.public().into());
+            // from cancelling/refunding unnecessarily.
-
-            if let Either::Right(_) = select(
-                monero_client.watch_for_transfer(monero_joint_address, xmr, v),
-                poll_until_btc_has_expired.clone(),
-            )
-            .await
-            {
-                return Err(SwapFailed::AfterBtcLock(Reason::BtcExpired));
-            };
 
             let tx_redeem_encsig = {
+                let mut guard = network.as_ref().lock().await;
                 let tx_redeem_encsig = match select(
-                    network.receive_bitcoin_redeem_encsig(),
+                    guard.receive_bitcoin_redeem_encsig(),
                     poll_until_btc_has_expired.clone(),
                 )
                 .await
@@ -375,6 +364,7 @@ pub async fn next_state<
             Ok(state5.into())
         }
         State::State5(state5) => {
+            transport.send_message(state5.next_message().into()).await?;
             // todo: pass in state4b as a parameter somewhere in this call to prevent the
             // user from waiting for a message that wont be sent
             let message3 = transport.receive_message().await?.try_into()?;
@@ -726,7 +716,7 @@ impl State4 {
         });
         let S_b = self.S_b_monero;
 
-        let fee = monero_wallet
+        let (tx_lock_proof, fee) = monero_wallet
             .transfer(S_a + S_b, self.v.public(), self.xmr)
             .await?;
 
@@ -745,6 +735,7 @@ impl State4 {
             redeem_address: self.redeem_address,
             punish_address: self.punish_address,
             tx_lock: self.tx_lock,
+            tx_lock_proof,
             tx_punish_sig_bob: self.tx_punish_sig_bob,
             tx_cancel_sig_bob: self.tx_cancel_sig_bob,
             lock_xmr_fee: fee,
@@ -815,6 +806,7 @@ pub struct State5 {
     redeem_address: bitcoin::Address,
     punish_address: bitcoin::Address,
     tx_lock: bitcoin::TxLock,
+    tx_lock_proof: monero::TransferProof,
 
     tx_punish_sig_bob: bitcoin::Signature,
 
@@ -823,6 +815,12 @@ pub struct State5 {
 }
 
 impl State5 {
+    pub fn next_message(&self) -> Message2 {
+        Message2 {
+            tx_lock_proof: self.tx_lock_proof.clone(),
+        }
+    }
+
     pub fn receive(self, msg: bob::Message3) -> State6 {
         State6 {
             a: self.a,

xmr-btc/src/alice/message.rs

@@ -9,6 +9,7 @@ use crate::{bitcoin, monero};
 pub enum Message {
     Message0(Message0),
     Message1(Message1),
+    Message2(Message2),
 }
 
 #[derive(Clone, Debug, Serialize, Deserialize)]
@@ -28,8 +29,15 @@ pub struct Message1 {
     pub(crate) tx_refund_encsig: EncryptedSignature,
 }
 
+#[derive(Clone, Debug, Serialize, Deserialize)]
+pub struct Message2 {
+    pub tx_lock_proof: monero::TransferProof,
+}
+
 impl_try_from_parent_enum!(Message0, Message);
 impl_try_from_parent_enum!(Message1, Message);
+impl_try_from_parent_enum!(Message2, Message);
 
 impl_from_child_enum!(Message0, Message);
 impl_from_child_enum!(Message1, Message);
+impl_from_child_enum!(Message2, Message);

xmr-btc/src/bob.rs

@@ -5,11 +5,11 @@ use crate::{
         SignTxLock, TxCancel, WatchForRawTransaction,
     },
     monero,
-    monero::WatchForTransfer,
     serde::monero_private_key,
     transport::{ReceiveMessage, SendMessage},
 };
 use anyhow::{anyhow, Result};
+use async_trait::async_trait;
 use ecdsa_fun::{
     adaptor::{Adaptor, EncryptedSignature},
     nonce::Deterministic,
@@ -28,11 +28,11 @@ use std::{
     sync::Arc,
     time::Duration,
 };
-use tokio::time::timeout;
+use tokio::{sync::Mutex, time::timeout};
 use tracing::error;
 
 pub mod message;
-use crate::monero::CreateWalletForOutput;
+use crate::monero::{CreateWalletForOutput, WatchForTransfer};
 pub use message::{Message, Message0, Message1, Message2, Message3};
 
 #[allow(clippy::large_enum_variant)]
@@ -48,6 +48,12 @@ pub enum Action {
     RefundBtc(bitcoin::Transaction),
 }
 
+// TODO: This could be moved to the monero module
+#[async_trait]
+pub trait ReceiveTransferProof {
+    async fn receive_transfer_proof(&mut self) -> monero::TransferProof;
+}
+
 /// Perform the on-chain protocol to swap monero and bitcoin as Bob.
 ///
 /// This is called post handshake, after all the keys, addresses and most of the
@@ -55,7 +61,8 @@ pub enum Action {
 ///
 /// The argument `bitcoin_tx_lock_timeout` is used to determine how long we will
 /// wait for Bob, the caller of this function, to lock up the bitcoin.
-pub fn action_generator<M, B>(
+pub fn action_generator<N, M, B>(
+    network: Arc<Mutex<N>>,
     monero_client: Arc<M>,
     bitcoin_client: Arc<B>,
     // TODO: Replace this with a new, slimmer struct?
@@ -78,6 +85,7 @@ pub fn action_generator<M, B>(
     bitcoin_tx_lock_timeout: u64,
 ) -> GenBoxed<Action, (), ()>
 where
+    N: ReceiveTransferProof + Send + 'static,
     M: monero::WatchForTransfer + Send + Sync + 'static,
     B: bitcoin::BlockHeight
         + bitcoin::TransactionBlockHeight
@@ -100,6 +108,8 @@ where
         InactiveBob,
         /// The refund timelock has been reached.
         BtcExpired,
+        /// Alice did not lock up enough monero in the shared output.
+        InsufficientXmr(monero::InsufficientFunds),
         /// Could not find Bob's signature on the redeem transaction witness
         /// stack.
         BtcRedeemSignature,
@@ -130,21 +140,39 @@ where
             .shared();
             pin_mut!(poll_until_btc_has_expired);
 
+            let transfer_proof = {
+                let mut guard = network.as_ref().lock().await;
+                let transfer_proof = match select(
+                    guard.receive_transfer_proof(),
+                    poll_until_btc_has_expired.clone(),
+                )
+                .await
+                {
+                    Either::Left((proof, _)) => proof,
+                    Either::Right(_) => return Err(SwapFailed::AfterBtcLock(Reason::BtcExpired)),
+                };
+
+                tracing::debug!("select returned transfer proof from message");
+
+                transfer_proof
+            };
+
             let S_b_monero = monero::PublicKey::from_private_key(&monero::PrivateKey::from_scalar(
                 s_b.into_ed25519(),
             ));
             let S = S_a_monero + S_b_monero;
 
-            let monero_joint_address =
+            match select(
-                monero::Address::standard(monero::Network::Mainnet, S, v.public().into());
+                monero_client.watch_for_transfer(S, v.public(), transfer_proof, xmr, 0),
-
-            if let Either::Right(_) = select(
-                monero_client.watch_for_transfer(monero_joint_address, xmr, v),
                 poll_until_btc_has_expired.clone(),
             )
             .await
             {
-                return Err(SwapFailed::AfterBtcLock(Reason::BtcExpired));
+                Either::Left((Err(e), _)) => {
+                    return Err(SwapFailed::AfterBtcLock(Reason::InsufficientXmr(e)))
+                }
+                Either::Right(_) => return Err(SwapFailed::AfterBtcLock(Reason::BtcExpired)),
+                _ => {}
             }
 
             let tx_redeem = bitcoin::TxRedeem::new(&tx_lock, &redeem_address);
@@ -272,7 +300,8 @@ pub async fn next_state<
             Ok(state3.into())
         }
         State::State3(state3) => {
-            let state4 = state3.watch_for_lock_xmr(monero_wallet).await?;
+            let message2 = transport.receive_message().await?.try_into()?;
+            let state4 = state3.watch_for_lock_xmr(monero_wallet, message2).await?;
             tracing::info!("bob has seen that alice has locked xmr");
             Ok(state4.into())
         }
@@ -560,7 +589,7 @@ pub struct State3 {
 }
 
 impl State3 {
-    pub async fn watch_for_lock_xmr<W>(self, xmr_wallet: &W) -> Result<State4>
+    pub async fn watch_for_lock_xmr<W>(self, xmr_wallet: &W, msg: alice::Message2) -> Result<State4>
     where
         W: monero::WatchForTransfer,
     {
@@ -569,12 +598,15 @@ impl State3 {
         ));
         let S = self.S_a_monero + S_b_monero;
 
-        let monero_joint_address =
-            monero::Address::standard(monero::Network::Mainnet, S, self.v.public().into());
-
         xmr_wallet
-            .watch_for_transfer(monero_joint_address, self.xmr, self.v)
+            .watch_for_transfer(
-            .await;
+                S,
+                self.v.public(),
+                msg.tx_lock_proof,
+                self.xmr,
+                monero::MIN_CONFIRMATIONS,
+            )
+            .await?;
 
         Ok(State4 {
             A: self.A,

xmr-btc/src/monero.rs

@@ -1,11 +1,14 @@
 use crate::serde::monero_private_key;
+use anyhow::Result;
 use async_trait::async_trait;
 use rand::{CryptoRng, RngCore};
 use serde::{Deserialize, Serialize};
 use std::ops::{Add, Sub};
 
 pub use curve25519_dalek::scalar::Scalar;
-pub use monero::{Address, Network, PrivateKey, PublicKey};
+pub use monero::*;
+
+pub const MIN_CONFIRMATIONS: u32 = 10;
 
 pub fn random_private_key<R: RngCore + CryptoRng>(rng: &mut R) -> PrivateKey {
     let scalar = Scalar::random(rng);
@@ -100,6 +103,35 @@ impl From<Amount> for u64 {
     }
 }
 
+#[derive(Clone, Debug, Serialize, Deserialize)]
+pub struct TransferProof {
+    tx_hash: TxHash,
+    #[serde(with = "monero_private_key")]
+    tx_key: PrivateKey,
+}
+
+impl TransferProof {
+    pub fn new(tx_hash: TxHash, tx_key: PrivateKey) -> Self {
+        Self { tx_hash, tx_key }
+    }
+    pub fn tx_hash(&self) -> TxHash {
+        self.tx_hash.clone()
+    }
+    pub fn tx_key(&self) -> PrivateKey {
+        self.tx_key
+    }
+}
+
+// TODO: add constructor/ change String to fixed length byte array
+#[derive(Clone, Debug, Serialize, Deserialize)]
+pub struct TxHash(pub String);
+
+impl From<TxHash> for String {
+    fn from(from: TxHash) -> Self {
+        from.0
+    }
+}
+
 #[async_trait]
 pub trait Transfer {
     async fn transfer(
@@ -107,17 +139,19 @@ pub trait Transfer {
         public_spend_key: PublicKey,
         public_view_key: PublicViewKey,
         amount: Amount,
-    ) -> anyhow::Result<Amount>;
+    ) -> anyhow::Result<(TransferProof, Amount)>;
 }
 
 #[async_trait]
 pub trait WatchForTransfer {
     async fn watch_for_transfer(
         &self,
-        address: Address,
+        public_spend_key: PublicKey,
+        public_view_key: PublicViewKey,
+        transfer_proof: TransferProof,
         amount: Amount,
-        private_view_key: PrivateViewKey,
+        expected_confirmations: u32,
-    );
+    ) -> Result<(), InsufficientFunds>;
 }
 
 #[derive(Debug, Clone, Copy, thiserror::Error)]

xmr-btc/tests/e2e.rs

@@ -22,11 +22,9 @@ mod tests {
     #[tokio::test]
     async fn happy_path() {
         let cli = Cli::default();
-        let (monero, _container) = Monero::new(&cli, None, vec![
+        let (monero, _container) = Monero::new(&cli, Some("hp".to_string()), vec![
             "alice".to_string(),
-            "alice-watch-only".to_string(),
             "bob".to_string(),
-            "bob-watch-only".to_string(),
         ])
         .await
         .unwrap();
@@ -99,11 +97,9 @@ mod tests {
     #[tokio::test]
     async fn both_refund() {
         let cli = Cli::default();
-        let (monero, _container) = Monero::new(&cli, None, vec![
+        let (monero, _container) = Monero::new(&cli, Some("br".to_string()), vec![
             "alice".to_string(),
-            "alice-watch-only".to_string(),
             "bob".to_string(),
-            "bob-watch-only".to_string(),
         ])
         .await
         .unwrap();
@@ -178,11 +174,9 @@ mod tests {
     #[tokio::test]
     async fn alice_punishes() {
         let cli = Cli::default();
-        let (monero, _container) = Monero::new(&cli, None, vec![
+        let (monero, _containers) = Monero::new(&cli, Some("ap".to_string()), vec![
             "alice".to_string(),
-            "alice-watch-only".to_string(),
             "bob".to_string(),
-            "bob-watch-only".to_string(),
         ])
         .await
         .unwrap();

xmr-btc/tests/harness/mod.rs

@@ -135,14 +135,8 @@ pub async fn init_test(
         .await
         .unwrap();
 
-    let alice_monero_wallet = wallet::monero::Wallet {
+    let alice_monero_wallet = wallet::monero::Wallet(monero.wallet("alice").unwrap().client());
-        inner: monero.wallet("alice").unwrap().client(),
+    let bob_monero_wallet = wallet::monero::Wallet(monero.wallet("bob").unwrap().client());
-        watch_only: monero.wallet("alice-watch-only").unwrap().client(),
-    };
-    let bob_monero_wallet = wallet::monero::Wallet {
-        inner: monero.wallet("bob").unwrap().client(),
-        watch_only: monero.wallet("bob-watch-only").unwrap().client(),
-    };
 
     let alice_btc_wallet = wallet::bitcoin::Wallet::new("alice", &bitcoind.node_url)
         .await

xmr-btc/tests/harness/wallet/monero.rs

@@ -1,26 +1,19 @@
 use anyhow::Result;
 use async_trait::async_trait;
 use backoff::{backoff::Constant as ConstantBackoff, future::FutureOperation as _};
-use futures::TryFutureExt;
-use monero::{Address, Network, PrivateKey};
 use monero_harness::rpc::wallet;
-use std::time::Duration;
+use std::{str::FromStr, time::Duration};
 use xmr_btc::monero::{
-    Amount, CreateWalletForOutput, PrivateViewKey, PublicKey, PublicViewKey, Transfer,
+    Address, Amount, CreateWalletForOutput, InsufficientFunds, Network, PrivateKey, PrivateViewKey,
-    WatchForTransfer,
+    PublicKey, PublicViewKey, Transfer, TransferProof, TxHash, WatchForTransfer,
 };
 
-pub struct Wallet {
+pub struct Wallet(pub wallet::Client);
-    pub inner: wallet::Client,
-    /// Secondary wallet which is only used to watch for the Monero lock
-    /// transaction without needing a transfer proof.
-    pub watch_only: wallet::Client,
-}
 
 impl Wallet {
     /// Get the balance of the primary account.
     pub async fn get_balance(&self) -> Result<Amount> {
-        let amount = self.inner.get_balance(0).await?;
+        let amount = self.0.get_balance(0).await?;
 
         Ok(Amount::from_piconero(amount))
     }
@@ -33,16 +26,21 @@ impl Transfer for Wallet {
         public_spend_key: PublicKey,
         public_view_key: PublicViewKey,
         amount: Amount,
-    ) -> Result<Amount> {
+    ) -> Result<(TransferProof, Amount)> {
         let destination_address =
             Address::standard(Network::Mainnet, public_spend_key, public_view_key.into());
 
         let res = self
-            .inner
+            .0
             .transfer(0, amount.as_piconero(), &destination_address.to_string())
             .await?;
 
-        Ok(Amount::from_piconero(res.fee))
+        let tx_hash = TxHash(res.tx_hash);
+        let tx_key = PrivateKey::from_str(&res.tx_key)?;
+
+        let fee = Amount::from_piconero(res.fee);
+
+        Ok((TransferProof::new(tx_hash, tx_key), fee))
     }
 }
 
@@ -59,10 +57,10 @@ impl CreateWalletForOutput for Wallet {
         let address = Address::standard(Network::Mainnet, public_spend_key, public_view_key);
 
         let _ = self
-            .inner
+            .0
             .generate_from_keys(
                 &address.to_string(),
-                Some(&private_spend_key.to_string()),
+                &private_spend_key.to_string(),
                 &PrivateKey::from(private_view_key).to_string(),
             )
             .await?;
@@ -75,50 +73,54 @@ impl CreateWalletForOutput for Wallet {
 impl WatchForTransfer for Wallet {
     async fn watch_for_transfer(
         &self,
-        address: Address,
+        public_spend_key: PublicKey,
+        public_view_key: PublicViewKey,
+        transfer_proof: TransferProof,
         expected_amount: Amount,
-        private_view_key: PrivateViewKey,
+        expected_confirmations: u32,
-    ) {
+    ) -> Result<(), InsufficientFunds> {
-        let address = address.to_string();
+        enum Error {
-        let private_view_key = PrivateKey::from(private_view_key).to_string();
+            TxNotFound,
-        let load_address = || {
+            InsufficientConfirmations,
-            self.watch_only
+            InsufficientFunds { expected: Amount, actual: Amount },
-                .generate_from_keys(&address, None, &private_view_key)
+        }
-                .map_err(backoff::Error::Transient)
-        };
 
-        // QUESTION: Should we really retry every error?
+        let address = Address::standard(Network::Mainnet, public_spend_key, public_view_key.into());
-        load_address
-            .retry(ConstantBackoff::new(Duration::from_secs(1)))
-            .await
-            .expect("transient error is never returned");
 
-        // QUESTION: Should we retry this error at all?
+        let res = (|| async {
-        let refresh = || self.watch_only.refresh().map_err(backoff::Error::Transient);
+            // NOTE: Currently, this is conflating IO errors with the transaction not being
-
+            // in the blockchain yet, or not having enough confirmations on it. All these
-        refresh
+            // errors warrant a retry, but the strategy should probably differ per case
-            .retry(ConstantBackoff::new(Duration::from_secs(1)))
+            let proof = self
-            .await
+                .0
-            .expect("transient error is never returned");
+                .check_tx_key(
-
+                    &String::from(transfer_proof.tx_hash()),
-        let check_balance = || async {
+                    &transfer_proof.tx_key().to_string(),
-            let balance = self
+                    &address.to_string(),
-                .watch_only
+                )
-                .get_balance(0)
                 .await
-                .map_err(|_| backoff::Error::Transient("io"))?;
+                .map_err(|_| backoff::Error::Transient(Error::TxNotFound))?;
-            let balance = Amount::from_piconero(balance);
 
-            if balance != expected_amount {
+            if proof.received != expected_amount.as_piconero() {
-                return Err(backoff::Error::Transient("insufficient funds"));
+                return Err(backoff::Error::Permanent(Error::InsufficientFunds {
+                    expected: expected_amount,
+                    actual: Amount::from_piconero(proof.received),
+                }));
             }
 
-            Ok(())
+            if proof.confirmations < expected_confirmations {
+                return Err(backoff::Error::Transient(Error::InsufficientConfirmations));
+            }
+
+            Ok(proof)
+        })
+        .retry(ConstantBackoff::new(Duration::from_secs(1)))
+        .await;
+
+        if let Err(Error::InsufficientFunds { expected, actual }) = res {
+            return Err(InsufficientFunds { expected, actual });
         };
 
-        check_balance
+        Ok(())
-            .retry(ConstantBackoff::new(Duration::from_secs(1)))
-            .await
-            .expect("transient error is never returned");
     }
 }

xmr-btc/tests/on_chain.rs

@@ -16,18 +16,20 @@ use monero_harness::Monero;
 use rand::rngs::OsRng;
 use std::{convert::TryInto, sync::Arc};
 use testcontainers::clients::Cli;
+use tokio::sync::Mutex;
 use tracing::info;
 use xmr_btc::{
     alice::{self, ReceiveBitcoinRedeemEncsig},
     bitcoin::{self, BroadcastSignedTransaction, EncryptedSignature, SignTxLock},
-    bob,
+    bob::{self, ReceiveTransferProof},
-    monero::{CreateWalletForOutput, Transfer},
+    monero::{CreateWalletForOutput, Transfer, TransferProof},
 };
 
 /// Time given to Bob to get the Bitcoin lock transaction included in a block.
 const BITCOIN_TX_LOCK_TIMEOUT: u64 = 5;
 
 type AliceNetwork = Network<EncryptedSignature>;
+type BobNetwork = Network<TransferProof>;
 
 #[derive(Debug)]
 struct Network<M> {
@@ -44,6 +46,13 @@ impl<M> Network<M> {
     }
 }
 
+#[async_trait]
+impl ReceiveTransferProof for BobNetwork {
+    async fn receive_transfer_proof(&mut self) -> TransferProof {
+        self.receiver.next().await.unwrap()
+    }
+}
+
 #[async_trait]
 impl ReceiveBitcoinRedeemEncsig for AliceNetwork {
     async fn receive_bitcoin_redeem_encsig(&mut self) -> EncryptedSignature {
@@ -92,7 +101,10 @@ impl Default for BobBehaviour {
 }
 
 async fn swap_as_alice(
-    network: AliceNetwork,
+    network: Arc<Mutex<AliceNetwork>>,
+    // FIXME: It would be more intuitive to have a single network/transport struct instead of
+    // splitting into two, but Rust ownership rules make this tedious
+    mut sender: Sender<TransferProof>,
     monero_wallet: Arc<harness::wallet::monero::Wallet>,
     bitcoin_wallet: Arc<harness::wallet::bitcoin::Wallet>,
     behaviour: AliceBehaviour,
@@ -101,7 +113,6 @@ async fn swap_as_alice(
     let mut action_generator = alice::action_generator(
         network,
         bitcoin_wallet.clone(),
-        monero_wallet.clone(),
         state,
         BITCOIN_TX_LOCK_TIMEOUT,
     );
@@ -118,9 +129,11 @@ async fn swap_as_alice(
                 public_view_key,
             }) => {
                 if behaviour.lock_xmr {
-                    let _ = monero_wallet
+                    let (transfer_proof, _) = monero_wallet
                         .transfer(public_spend_key, public_view_key, amount)
                         .await?;
+
+                    sender.send(transfer_proof).await?;
                 }
             }
             GeneratorState::Yielded(alice::Action::RedeemBtc(tx)) => {
@@ -154,6 +167,7 @@ async fn swap_as_alice(
 }
 
 async fn swap_as_bob(
+    network: Arc<Mutex<BobNetwork>>,
     mut sender: Sender<EncryptedSignature>,
     monero_wallet: Arc<harness::wallet::monero::Wallet>,
     bitcoin_wallet: Arc<harness::wallet::bitcoin::Wallet>,
@@ -161,6 +175,7 @@ async fn swap_as_bob(
     state: bob::State2,
 ) -> Result<()> {
     let mut action_generator = bob::action_generator(
+        network,
         monero_wallet.clone(),
         bitcoin_wallet.clone(),
         state,
@@ -218,11 +233,9 @@ async fn swap_as_bob(
 #[tokio::test]
 async fn on_chain_happy_path() {
     let cli = Cli::default();
-    let (monero, _container) = Monero::new(&cli, None, vec![
+    let (monero, _container) = Monero::new(&cli, Some("ochp".to_string()), vec![
         "alice".to_string(),
-        "alice-watch-only".to_string(),
         "bob".to_string(),
-        "bob-watch-only".to_string(),
     ])
     .await
     .unwrap();
@@ -258,16 +271,19 @@ async fn on_chain_happy_path() {
     let bob_monero_wallet = Arc::new(bob_node.monero_wallet);
 
     let (alice_network, bob_sender) = Network::<EncryptedSignature>::new();
+    let (bob_network, alice_sender) = Network::<TransferProof>::new();
 
     try_join(
         swap_as_alice(
-            alice_network,
+            Arc::new(Mutex::new(alice_network)),
+            alice_sender,
             alice_monero_wallet.clone(),
             alice_bitcoin_wallet.clone(),
             AliceBehaviour::default(),
             alice,
         ),
         swap_as_bob(
+            Arc::new(Mutex::new(bob_network)),
             bob_sender,
             bob_monero_wallet.clone(),
             bob_bitcoin_wallet.clone(),
@@ -312,11 +328,9 @@ async fn on_chain_happy_path() {
 #[tokio::test]
 async fn on_chain_both_refund_if_alice_never_redeems() {
     let cli = Cli::default();
-    let (monero, _container) = Monero::new(&cli, None, vec![
+    let (monero, _container) = Monero::new(&cli, Some("ocbr".to_string()), vec![
         "alice".to_string(),
-        "alice-watch-only".to_string(),
         "bob".to_string(),
-        "bob-watch-only".to_string(),
     ])
     .await
     .unwrap();
@@ -352,10 +366,12 @@ async fn on_chain_both_refund_if_alice_never_redeems() {
     let bob_monero_wallet = Arc::new(bob_node.monero_wallet);
 
     let (alice_network, bob_sender) = Network::<EncryptedSignature>::new();
+    let (bob_network, alice_sender) = Network::<TransferProof>::new();
 
     try_join(
         swap_as_alice(
-            alice_network,
+            Arc::new(Mutex::new(alice_network)),
+            alice_sender,
             alice_monero_wallet.clone(),
             alice_bitcoin_wallet.clone(),
             AliceBehaviour {
@@ -365,6 +381,7 @@ async fn on_chain_both_refund_if_alice_never_redeems() {
             alice,
         ),
         swap_as_bob(
+            Arc::new(Mutex::new(bob_network)),
             bob_sender,
             bob_monero_wallet.clone(),
             bob_bitcoin_wallet.clone(),
@@ -406,11 +423,9 @@ async fn on_chain_both_refund_if_alice_never_redeems() {
 #[tokio::test]
 async fn on_chain_alice_punishes_if_bob_never_acts_after_fund() {
     let cli = Cli::default();
-    let (monero, _container) = Monero::new(&cli, None, vec![
+    let (monero, _container) = Monero::new(&cli, Some("ocap".to_string()), vec![
         "alice".to_string(),
-        "alice-watch-only".to_string(),
         "bob".to_string(),
-        "bob-watch-only".to_string(),
     ])
     .await
     .unwrap();
@@ -446,15 +461,18 @@ async fn on_chain_alice_punishes_if_bob_never_acts_after_fund() {
     let bob_monero_wallet = Arc::new(bob_node.monero_wallet);
 
     let (alice_network, bob_sender) = Network::<EncryptedSignature>::new();
+    let (bob_network, alice_sender) = Network::<TransferProof>::new();
 
     let alice_swap = swap_as_alice(
-        alice_network,
+        Arc::new(Mutex::new(alice_network)),
+        alice_sender,
         alice_monero_wallet.clone(),
         alice_bitcoin_wallet.clone(),
         AliceBehaviour::default(),
         alice,
     );
     let bob_swap = swap_as_bob(
+        Arc::new(Mutex::new(bob_network)),
         bob_sender,
         bob_monero_wallet.clone(),
         bob_bitcoin_wallet.clone(),