diff --git a/monero-harness/Cargo.toml b/monero-harness/Cargo.toml index 24ad8220..060df781 100644 --- a/monero-harness/Cargo.toml +++ b/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" diff --git a/monero-harness/src/lib.rs b/monero-harness/src/lib.rs index 4cf16da6..7d42a586 100644 --- a/monero-harness/src/lib.rs +++ b/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(()) } diff --git a/monero-harness/src/rpc/wallet.rs b/monero-harness/src/rpc/wallet.rs index fbd24e06..97c2804d 100644 --- a/monero-harness/src/rpc/wallet.rs +++ b/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 { 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: Option, + pub spendkey: String, pub viewkey: String, pub password: String, pub autosave_current: bool, diff --git a/monero-harness/tests/wallet.rs b/monero-harness/tests/wallet.rs index af131acf..44fc6ec5 100644 --- a/monero-harness/tests/wallet.rs +++ b/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); -} diff --git a/swap/src/alice.rs b/swap/src/alice.rs index 5e8efc19..52ae7cc4 100644 --- a/swap/src/alice.rs +++ b/swap/src/alice.rs @@ -45,13 +45,31 @@ use xmr_btc::{ pub async fn swap( bitcoin_wallet: Arc, - monero_wallet: Arc, + monero_wallet: Arc, db: Database, listen: Multiaddr, transport: SwapTransport, behaviour: Alice, ) -> Result<()> { - struct Network(Arc>); + struct Network { + swarm: Arc>, + channel: Option>, + } + + 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 { - OutEvent::Message2(msg) => state2.receive(msg)?, + let (state3, channel) = match swarm.next().await { + 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, }, - Message2(bob::Message2), + Message2 { + msg: bob::Message2, + channel: ResponseChannel, + }, Message3(bob::Message3), } @@ -315,7 +345,7 @@ impl From for OutEvent { impl From 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, + msg: xmr_btc::alice::Message2, + ) { + self.message2.send(channel, msg); + debug!("Sent Message2"); + } } impl Default for Alice { diff --git a/swap/src/alice/message2.rs b/swap/src/alice/message2.rs index ebb4f6f0..bab62d3e 100644 --- a/swap/src/alice/message2.rs +++ b/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, + }, } /// 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, 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> } => { 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 { .. }, diff --git a/swap/src/bob.rs b/swap/src/bob.rs index 9d6cd0ea..3b5c5936 100644 --- a/swap/src/bob.rs +++ b/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 tracing::{debug, info}; +use std::{process, sync::Arc, time::Duration}; +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, - monero_wallet: Arc, + monero_wallet: Arc, 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>::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 for OutEvent { impl From for OutEvent { fn from(event: message2::OutEvent) -> Self { match event { - message2::OutEvent::Msg => OutEvent::Message2, + message2::OutEvent::Msg(msg) => OutEvent::Message2(msg), } } } diff --git a/swap/src/bob/message2.rs b/swap/src/bob/message2.rs index 9daac0c6..17425227 100644 --- a/swap/src/bob/message2.rs +++ b/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> message: RequestResponseMessage::Response { response, .. }, .. } => { - if let AliceToBob::Message2 = response { - self.events.push_back(OutEvent::Msg); + if let AliceToBob::Message2(msg) = response { + debug!("Received Message2"); + self.events.push_back(OutEvent::Msg(msg)); } } RequestResponseEvent::InboundFailure { error, .. } => { diff --git a/swap/src/cli.rs b/swap/src/cli.rs index aa247541..2d13fcb8 100644 --- a/swap/src/cli.rs +++ b/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( - default_value = "http://127.0.0.1:18083/json_rpc", - 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 = "http://127.0.0.1:18083/json_rpc", long = "monerod")] + monerod_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( - default_value = "http://127.0.0.1:18083/json_rpc", - 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 = "http://127.0.0.1:18083/json_rpc", long = "monerod")] + monerod_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( - default_value = "http://127.0.0.1:18083/json_rpc", - 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 = "http://127.0.0.1:18083/json_rpc", long = "monerod")] + monerod_url: Url, }, } diff --git a/swap/src/main.rs b/swap/src/main.rs index 8512afcb..afdf110c 100644 --- a/swap/src/main.rs +++ b/swap/src/main.rs @@ -53,8 +53,7 @@ async fn main() -> Result<()> { match opt { Options::Alice { bitcoind_url, - monero_wallet_rpc_url, - monero_watch_only_wallet_rpc_url, + monerod_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( - monero_wallet_rpc_url, - monero_watch_only_wallet_rpc_url, - )); + let monero_wallet = Arc::new(monero::Wallet::new(monerod_url)); swap_as_alice( bitcoin_wallet, @@ -106,8 +102,7 @@ async fn main() -> Result<()> { alice_addr, satoshis, bitcoind_url, - monero_wallet_rpc_url, - monero_watch_only_wallet_rpc_url, + monerod_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( - monero_wallet_rpc_url, - monero_watch_only_wallet_rpc_url, - )); + let monero_wallet = Arc::new(monero::Wallet::new(monerod_url)); swap_as_bob( bitcoin_wallet, @@ -156,15 +148,13 @@ async fn main() -> Result<()> { Options::Recover { swap_id, bitcoind_url, - monero_wallet_rpc_url, - monero_watch_only_wallet_rpc_url, + monerod_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 = - monero::Facade::new(monero_wallet_rpc_url, monero_watch_only_wallet_rpc_url); + let monero_wallet = monero::Wallet::new(monerod_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, - monero_wallet: Arc, + monero_wallet: Arc, db: Database, addr: Multiaddr, transport: SwapTransport, @@ -212,7 +202,7 @@ async fn swap_as_alice( async fn swap_as_bob( bitcoin_wallet: Arc, - monero_wallet: Arc, + monero_wallet: Arc, db: Database, sats: u64, alice: Multiaddr, diff --git a/swap/src/monero.rs b/swap/src/monero.rs index 7d0129f7..7d252b69 100644 --- a/swap/src/monero.rs +++ b/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 user_wallet: wallet::Client, - pub watch_only_wallet: wallet::Client, -} +pub use xmr_btc::monero::*; -impl Facade { - pub fn new(user_url: Url, watch_only_url: Url) -> Self { - Self { - user_wallet: wallet::Client::new(user_url), - watch_only_wallet: wallet::Client::new(watch_only_url), - } +pub struct Wallet(pub wallet::Client); + +impl Wallet { + pub fn new(url: Url) -> Self { + Self(wallet::Client::new(url)) } /// Get the balance of the primary account. pub async fn get_balance(&self) -> Result { - 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 { + ) -> 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, - ) { - let address = address.to_string(); - let private_view_key = PrivateKey::from(private_view_key).to_string(); - let load_address = || { - self.watch_only_wallet - .generate_from_keys(&address, None, &private_view_key) - .map_err(backoff::Error::Transient) - }; + expected_confirmations: u32, + ) -> Result<(), InsufficientFunds> { + enum Error { + TxNotFound, + InsufficientConfirmations, + InsufficientFunds { expected: Amount, actual: Amount }, + } - // QUESTION: Should we really retry every error? - load_address - .retry(ConstantBackoff::new(Duration::from_secs(1))) - .await - .expect("transient error is never returned"); + let address = Address::standard(Network::Mainnet, public_spend_key, public_view_key.into()); - // QUESTION: Should we retry this error at all? - let refresh = || { - self.watch_only_wallet - .refresh() - .map_err(backoff::Error::Transient) - }; - - refresh - .retry(ConstantBackoff::new(Duration::from_secs(1))) - .await - .expect("transient error is never returned"); - - let check_balance = || async { - let balance = self - .watch_only_wallet - .get_balance(0) + let res = (|| async { + // 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 + // errors warrant a retry, but the strategy should probably differ per case + let proof = self + .0 + .check_tx_key( + &String::from(transfer_proof.tx_hash()), + &transfer_proof.tx_key().to_string(), + &address.to_string(), + ) .await - .map_err(|_| backoff::Error::Transient("io"))?; - let balance = Amount::from_piconero(balance); + .map_err(|_| backoff::Error::Transient(Error::TxNotFound))?; - if balance != expected_amount { - return Err(backoff::Error::Transient("insufficient funds")); + if proof.received != expected_amount.as_piconero() { + 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 - .retry(ConstantBackoff::new(Duration::from_secs(1))) - .await - .expect("transient error is never returned"); + Ok(()) } } diff --git a/swap/src/network/request_response.rs b/swap/src/network/request_response.rs index 8d3a78d0..a042debc 100644 --- a/swap/src/network/request_response.rs +++ b/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 } diff --git a/swap/src/recover.rs b/swap/src/recover.rs index 9934dd51..b71b31fd 100644 --- a/swap/src/recover.rs +++ b/swap/src/recover.rs @@ -29,8 +29,8 @@ use xmr_btc::bitcoin::{ }; pub async fn recover( - bitcoin_wallet: crate::bitcoin::Wallet, - monero_wallet: crate::monero::Facade, + bitcoin_wallet: bitcoin::Wallet, + 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, - monero_wallet: crate::monero::Facade, + bitcoin_wallet: bitcoin::Wallet, + 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 { diff --git a/swap/tests/e2e.rs b/swap/tests/e2e.rs index 34bab971..358657ca 100644 --- a/swap/tests/e2e.rs +++ b/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![ - "alice".to_string(), - "alice-watch-only".to_string(), - "bob".to_string(), - "bob-watch-only".to_string(), - ]) - .await - .unwrap(); + let (monero, _container) = + Monero::new(&cli, None, vec!["alice".to_string(), "bob".to_string()]) + .await + .unwrap(); monero .init(vec![("alice", xmr_alice), ("bob", xmr_bob)]) .await .unwrap(); - let alice_xmr_wallet = Arc::new(swap::monero::Facade { - user_wallet: monero.wallet("alice").unwrap().client(), - watch_only_wallet: monero.wallet("alice-watch-only").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_xmr_wallet = Arc::new(swap::monero::Wallet( + monero.wallet("alice").unwrap().client(), + )); + let bob_xmr_wallet = Arc::new(swap::monero::Wallet(monero.wallet("bob").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!( diff --git a/xmr-btc/src/alice.rs b/xmr-btc/src/alice.rs index 7062d4f7..fa35a140 100644 --- a/xmr-btc/src/alice.rs +++ b/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( - mut network: N, +pub fn action_generator( + network: Arc>, bitcoin_client: Arc, - monero_client: Arc, // 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 = - monero::Address::standard(monero::Network::Mainnet, S, v.public().into()); - - 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)); - }; + // TODO: Watch for LockXmr using watch-only wallet. Doing so will prevent Alice + // from cancelling/refunding unnecessarily. 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, diff --git a/xmr-btc/src/alice/message.rs b/xmr-btc/src/alice/message.rs index 8088bc18..b82f6f9f 100644 --- a/xmr-btc/src/alice/message.rs +++ b/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); diff --git a/xmr-btc/src/bob.rs b/xmr-btc/src/bob.rs index 86dbd33d..e9b98ac8 100644 --- a/xmr-btc/src/bob.rs +++ b/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( +pub fn action_generator( + network: Arc>, monero_client: Arc, bitcoin_client: Arc, // TODO: Replace this with a new, slimmer struct? @@ -78,6 +85,7 @@ pub fn action_generator( bitcoin_tx_lock_timeout: u64, ) -> GenBoxed 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 = - monero::Address::standard(monero::Network::Mainnet, S, v.public().into()); - - if let Either::Right(_) = select( - monero_client.watch_for_transfer(monero_joint_address, xmr, v), + match select( + monero_client.watch_for_transfer(S, v.public(), transfer_proof, xmr, 0), 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(self, xmr_wallet: &W) -> Result + pub async fn watch_for_lock_xmr(self, xmr_wallet: &W, msg: alice::Message2) -> Result 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) - .await; + .watch_for_transfer( + S, + self.v.public(), + msg.tx_lock_proof, + self.xmr, + monero::MIN_CONFIRMATIONS, + ) + .await?; Ok(State4 { A: self.A, diff --git a/xmr-btc/src/monero.rs b/xmr-btc/src/monero.rs index 6ec2f644..643c4d32 100644 --- a/xmr-btc/src/monero.rs +++ b/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(rng: &mut R) -> PrivateKey { let scalar = Scalar::random(rng); @@ -100,6 +103,35 @@ impl From 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 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; + ) -> 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)] diff --git a/xmr-btc/tests/e2e.rs b/xmr-btc/tests/e2e.rs index f29f2088..373c759e 100644 --- a/xmr-btc/tests/e2e.rs +++ b/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(); diff --git a/xmr-btc/tests/harness/mod.rs b/xmr-btc/tests/harness/mod.rs index 754d1efc..3d789b8f 100644 --- a/xmr-btc/tests/harness/mod.rs +++ b/xmr-btc/tests/harness/mod.rs @@ -135,14 +135,8 @@ pub async fn init_test( .await .unwrap(); - let alice_monero_wallet = wallet::monero::Wallet { - inner: monero.wallet("alice").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_monero_wallet = wallet::monero::Wallet(monero.wallet("alice").unwrap().client()); + let bob_monero_wallet = wallet::monero::Wallet(monero.wallet("bob").unwrap().client()); let alice_btc_wallet = wallet::bitcoin::Wallet::new("alice", &bitcoind.node_url) .await diff --git a/xmr-btc/tests/harness/wallet/monero.rs b/xmr-btc/tests/harness/wallet/monero.rs index fef131cd..fbfd4270 100644 --- a/xmr-btc/tests/harness/wallet/monero.rs +++ b/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, - WatchForTransfer, + Address, Amount, CreateWalletForOutput, InsufficientFunds, Network, PrivateKey, PrivateViewKey, + PublicKey, PublicViewKey, Transfer, TransferProof, TxHash, WatchForTransfer, }; -pub struct Wallet { - 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, -} +pub struct Wallet(pub wallet::Client); impl Wallet { /// Get the balance of the primary account. pub async fn get_balance(&self) -> Result { - 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 { + ) -> 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, - ) { - let address = address.to_string(); - let private_view_key = PrivateKey::from(private_view_key).to_string(); - let load_address = || { - self.watch_only - .generate_from_keys(&address, None, &private_view_key) - .map_err(backoff::Error::Transient) - }; + expected_confirmations: u32, + ) -> Result<(), InsufficientFunds> { + enum Error { + TxNotFound, + InsufficientConfirmations, + InsufficientFunds { expected: Amount, actual: Amount }, + } - // QUESTION: Should we really retry every error? - load_address - .retry(ConstantBackoff::new(Duration::from_secs(1))) - .await - .expect("transient error is never returned"); + let address = Address::standard(Network::Mainnet, public_spend_key, public_view_key.into()); - // QUESTION: Should we retry this error at all? - let refresh = || self.watch_only.refresh().map_err(backoff::Error::Transient); - - refresh - .retry(ConstantBackoff::new(Duration::from_secs(1))) - .await - .expect("transient error is never returned"); - - let check_balance = || async { - let balance = self - .watch_only - .get_balance(0) + let res = (|| async { + // 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 + // errors warrant a retry, but the strategy should probably differ per case + let proof = self + .0 + .check_tx_key( + &String::from(transfer_proof.tx_hash()), + &transfer_proof.tx_key().to_string(), + &address.to_string(), + ) .await - .map_err(|_| backoff::Error::Transient("io"))?; - let balance = Amount::from_piconero(balance); + .map_err(|_| backoff::Error::Transient(Error::TxNotFound))?; - if balance != expected_amount { - return Err(backoff::Error::Transient("insufficient funds")); + if proof.received != expected_amount.as_piconero() { + 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 - .retry(ConstantBackoff::new(Duration::from_secs(1))) - .await - .expect("transient error is never returned"); + Ok(()) } } diff --git a/xmr-btc/tests/on_chain.rs b/xmr-btc/tests/on_chain.rs index 763d082a..459b368e 100644 --- a/xmr-btc/tests/on_chain.rs +++ b/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, - monero::{CreateWalletForOutput, Transfer}, + bob::{self, ReceiveTransferProof}, + 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; +type BobNetwork = Network; #[derive(Debug)] struct Network { @@ -44,6 +46,13 @@ impl Network { } } +#[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>, + // 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, monero_wallet: Arc, bitcoin_wallet: Arc, 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>, mut sender: Sender, monero_wallet: Arc, bitcoin_wallet: Arc, @@ -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::::new(); + let (bob_network, alice_sender) = Network::::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::::new(); + let (bob_network, alice_sender) = Network::::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::::new(); + let (bob_network, alice_sender) = Network::::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(),