From 620216a596b81ecf8e887e675967350b8b0d077f Mon Sep 17 00:00:00 2001 From: Lucas Soriano del Pino Date: Wed, 4 Nov 2020 20:16:54 +1100 Subject: [PATCH] Remove need to send Monero transfer proof from Alice to Bob --- monero-harness/src/lib.rs | 6 +- swap/src/alice.rs | 62 +++--------- swap/src/alice/message2.rs | 18 +--- swap/src/bob.rs | 77 ++++----------- swap/src/bob/message2.rs | 11 +-- swap/src/cli.rs | 29 +++++- swap/src/main.rs | 20 ++-- swap/src/monero.rs | 129 +++++++++++++------------ swap/src/network/request_response.rs | 2 +- swap/tests/e2e.rs | 44 ++++----- xmr-btc/src/alice.rs | 41 +++----- xmr-btc/src/alice/message.rs | 8 -- xmr-btc/src/bob.rs | 66 ++++--------- xmr-btc/src/monero.rs | 48 +-------- xmr-btc/tests/harness/wallet/monero.rs | 102 ++++--------------- xmr-btc/tests/on_chain.rs | 39 ++------ 16 files changed, 229 insertions(+), 473 deletions(-) diff --git a/monero-harness/src/lib.rs b/monero-harness/src/lib.rs index 7d42a586..4cf16da6 100644 --- a/monero-harness/src/lib.rs +++ b/monero-harness/src/lib.rs @@ -136,11 +136,7 @@ impl<'c> Monero { monerod.start_miner(&miner_address).await?; tracing::info!("Waiting for miner wallet to catch up..."); - let block_height = monerod.client().get_block_count().await?; - miner_wallet - .wait_for_wallet_height(block_height) - .await - .unwrap(); + miner_wallet.refresh().await?; Ok(()) } diff --git a/swap/src/alice.rs b/swap/src/alice.rs index 52ae7cc4..5e8efc19 100644 --- a/swap/src/alice.rs +++ b/swap/src/alice.rs @@ -45,31 +45,13 @@ 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 { - 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"); - } - } - } - } + struct Network(Arc>); // TODO: For retry, use `backoff::ExponentialBackoff` in production as opposed // to `ConstantBackoff`. @@ -80,7 +62,7 @@ pub async fn swap( struct UnexpectedMessage; let encsig = (|| async { - let mut guard = self.swarm.lock().await; + let mut guard = self.0.lock().await; let encsig = match guard.next().await { OutEvent::Message3(msg) => msg.tx_redeem_encsig, other => { @@ -169,11 +151,8 @@ pub async fn swap( let msg = state2.next_message(); swarm.send_message1(channel, msg); - let (state3, channel) = match swarm.next().await { - OutEvent::Message2 { msg, channel } => { - let state3 = state2.receive(msg)?; - (state3, channel) - } + let state3 = match swarm.next().await { + OutEvent::Message2(msg) => state2.receive(msg)?, other => panic!("Unexpected event: {:?}", other), }; @@ -183,14 +162,12 @@ pub async fn swap( info!("Handshake complete, we now have State3 for Alice."); - let network = Arc::new(Mutex::new(Network { - swarm: Arc::new(Mutex::new(swarm)), - channel: Some(channel), - })); + let network = Network(Arc::new(Mutex::new(swarm))); let mut action_generator = action_generator( - network.clone(), + network, bitcoin_wallet.clone(), + monero_wallet.clone(), state3.clone(), TX_LOCK_MINE_TIMEOUT, ); @@ -209,16 +186,12 @@ pub async fn swap( db.insert_latest_state(swap_id, state::Alice::BtcLocked(state3.clone()).into()) .await?; - let (transfer_proof, _) = monero_wallet + let _ = 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)) => { @@ -303,10 +276,7 @@ pub enum OutEvent { msg: bob::Message1, channel: ResponseChannel, }, - Message2 { - msg: bob::Message2, - channel: ResponseChannel, - }, + Message2(bob::Message2), Message3(bob::Message3), } @@ -345,7 +315,7 @@ impl From for OutEvent { impl From for OutEvent { fn from(event: message2::OutEvent) -> Self { match event { - message2::OutEvent::Msg { msg, channel } => OutEvent::Message2 { msg, channel }, + message2::OutEvent::Msg(msg) => OutEvent::Message2(msg), } } } @@ -404,16 +374,6 @@ 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 bab62d3e..ebb4f6f0 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, ResponseChannel, + RequestResponseEvent, RequestResponseMessage, }, swarm::{NetworkBehaviourAction, NetworkBehaviourEventProcess, PollParameters}, NetworkBehaviour, @@ -18,12 +18,7 @@ use xmr_btc::bob; #[derive(Debug)] pub enum OutEvent { - Msg { - /// Received message from Bob. - msg: bob::Message2, - /// Channel to send back Alice's message 2. - channel: ResponseChannel, - }, + Msg(bob::Message2), } /// A `NetworkBehaviour` that represents receiving of message 2 from Bob. @@ -37,11 +32,6 @@ 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<'_>, @@ -84,8 +74,10 @@ impl NetworkBehaviourEventProcess> } => { if let BobToAlice::Message2(msg) = request { debug!("Received Message2"); - self.events.push_back(OutEvent::Msg { msg, channel }); + self.events.push_back(OutEvent::Msg(msg)); } + // 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 3b5c5936..9d6cd0ea 100644 --- a/swap/src/bob.rs +++ b/swap/src/bob.rs @@ -1,18 +1,15 @@ //! 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}, - FutureExt, StreamExt, + StreamExt, }; use genawaiter::GeneratorState; use libp2p::{core::identity::Keypair, Multiaddr, NetworkBehaviour, PeerId}; use rand::rngs::OsRng; -use std::{process, sync::Arc, time::Duration}; -use tokio::sync::Mutex; -use tracing::{debug, info, warn}; +use std::{process, sync::Arc}; +use tracing::{debug, info}; use uuid::Uuid; mod amounts; @@ -37,14 +34,14 @@ use crate::{ use xmr_btc::{ alice, bitcoin::{BroadcastSignedTransaction, EncryptedSignature, SignTxLock}, - bob::{self, action_generator, ReceiveTransferProof, State0}, + bob::{self, action_generator, 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, @@ -53,40 +50,6 @@ 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)?; @@ -149,13 +112,19 @@ 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(), @@ -182,20 +151,14 @@ 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?; - - 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 { + swarm.send_message3(alice.clone(), tx_redeem_encsig); + match swarm.next().await { OutEvent::Message3 => { - debug!("Got Message3 empty response"); + debug!("Got message 3 response from Alice"); } other => panic!("unexpected event: {:?}", other), }; + info!("Sent Bitcoin redeem encsig"); } GeneratorState::Yielded(bob::Action::CreateXmrWalletForOutput { spend_key, @@ -257,7 +220,7 @@ pub enum OutEvent { Amounts(SwapAmounts), Message0(alice::Message0), Message1(alice::Message1), - Message2(alice::Message2), + Message2, Message3, } @@ -298,7 +261,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 => OutEvent::Message2, } } } diff --git a/swap/src/bob/message2.rs b/swap/src/bob/message2.rs index 17425227..9daac0c6 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::{debug, error}; +use tracing::error; use crate::network::request_response::{AliceToBob, BobToAlice, Codec, Message2Protocol, TIMEOUT}; -use xmr_btc::{alice, bob}; +use xmr_btc::bob; #[derive(Debug)] pub enum OutEvent { - Msg(alice::Message2), + Msg, } /// A `NetworkBehaviour` that represents sending message 2 to Alice. @@ -78,9 +78,8 @@ impl NetworkBehaviourEventProcess> message: RequestResponseMessage::Response { response, .. }, .. } => { - if let AliceToBob::Message2(msg) = response { - debug!("Received Message2"); - self.events.push_back(OutEvent::Msg(msg)); + if let AliceToBob::Message2 = response { + self.events.push_back(OutEvent::Msg); } } RequestResponseEvent::InboundFailure { error, .. } => { diff --git a/swap/src/cli.rs b/swap/src/cli.rs index 2d13fcb8..b2b238b5 100644 --- a/swap/src/cli.rs +++ b/swap/src/cli.rs @@ -2,6 +2,15 @@ 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 { @@ -9,8 +18,14 @@ 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 = "monerod")] - monerod_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 = "/ip4/127.0.0.1/tcp/9876", long = "listen-addr")] listen_addr: Multiaddr, @@ -28,8 +43,14 @@ 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 = "monerod")] - monerod_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(long = "tor")] tor: bool, diff --git a/swap/src/main.rs b/swap/src/main.rs index afdf110c..a3ab68ed 100644 --- a/swap/src/main.rs +++ b/swap/src/main.rs @@ -53,7 +53,8 @@ async fn main() -> Result<()> { match opt { Options::Alice { bitcoind_url, - monerod_url, + monero_wallet_rpc_url, + monero_watch_only_wallet_rpc_url, listen_addr, tor_port, } => { @@ -86,7 +87,10 @@ async fn main() -> Result<()> { .expect("failed to create bitcoin wallet"); let bitcoin_wallet = Arc::new(bitcoin_wallet); - let monero_wallet = Arc::new(monero::Wallet::new(monerod_url)); + let monero_wallet = Arc::new(monero::Facade::new( + monero_wallet_rpc_url, + monero_watch_only_wallet_rpc_url, + )); swap_as_alice( bitcoin_wallet, @@ -102,7 +106,8 @@ async fn main() -> Result<()> { alice_addr, satoshis, bitcoind_url, - monerod_url, + monero_wallet_rpc_url, + monero_watch_only_wallet_rpc_url, tor, } => { info!("running swap node as Bob ..."); @@ -120,7 +125,10 @@ async fn main() -> Result<()> { .expect("failed to create bitcoin wallet"); let bitcoin_wallet = Arc::new(bitcoin_wallet); - let monero_wallet = Arc::new(monero::Wallet::new(monerod_url)); + let monero_wallet = Arc::new(monero::Facade::new( + monero_wallet_rpc_url, + monero_watch_only_wallet_rpc_url, + )); swap_as_bob( bitcoin_wallet, @@ -183,7 +191,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, @@ -202,7 +210,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 56dd2d66..f757df0c 100644 --- a/swap/src/monero.rs +++ b/swap/src/monero.rs @@ -1,54 +1,59 @@ 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::{str::FromStr, time::Duration}; +use std::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::*; - -pub struct Wallet(pub wallet::Client); - -impl Wallet { - pub fn new(url: Url) -> Self { - Self(wallet::Client::new(url)) +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), + } } /// Get the balance of the primary account. pub async fn get_balance(&self) -> Result { - let amount = self.0.get_balance(0).await?; + let amount = self.user_wallet.get_balance(0).await?; Ok(Amount::from_piconero(amount)) } } #[async_trait] -impl Transfer for Wallet { +impl Transfer for Facade { async fn transfer( &self, public_spend_key: PublicKey, public_view_key: PublicViewKey, amount: Amount, - ) -> Result<(TransferProof, Amount)> { + ) -> Result { let destination_address = Address::standard(Network::Mainnet, public_spend_key, public_view_key.into()); let res = self - .0 + .user_wallet .transfer(0, amount.as_piconero(), &destination_address.to_string()) .await?; - 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)) + Ok(Amount::from_piconero(res.fee)) } } #[async_trait] -impl CreateWalletForOutput for Wallet { +impl CreateWalletForOutput for Facade { async fn create_and_load_wallet_for_output( &self, private_spend_key: PrivateKey, @@ -60,7 +65,7 @@ impl CreateWalletForOutput for Wallet { let address = Address::standard(Network::Mainnet, public_spend_key, public_view_key); let _ = self - .0 + .user_wallet .generate_from_keys( &address.to_string(), Some(&private_spend_key.to_string()), @@ -76,57 +81,57 @@ impl CreateWalletForOutput for Wallet { // to `ConstantBackoff`. #[async_trait] -impl WatchForTransfer for Wallet { +impl WatchForTransfer for Facade { async fn watch_for_transfer( &self, - public_spend_key: PublicKey, - public_view_key: PublicViewKey, - transfer_proof: TransferProof, + address: Address, expected_amount: Amount, - expected_confirmations: u32, - ) -> Result<(), InsufficientFunds> { - enum Error { - TxNotFound, - InsufficientConfirmations, - InsufficientFunds { expected: Amount, actual: 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) + }; - let address = Address::standard(Network::Mainnet, public_spend_key, public_view_key.into()); - - 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(Error::TxNotFound))?; + // QUESTION: Should we really retry every error? + 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 refresh = || { + self.watch_only_wallet + .refresh() + .map_err(backoff::Error::Transient) + }; - if proof.received != expected_amount.as_piconero() { - return Err(backoff::Error::Permanent(Error::InsufficientFunds { - expected: expected_amount, - actual: Amount::from_piconero(proof.received), - })); - } + refresh + .retry(ConstantBackoff::new(Duration::from_secs(1))) + .await + .expect("transient error is never returned"); - if proof.confirmations < expected_confirmations { - return Err(backoff::Error::Transient(Error::InsufficientConfirmations)); - } + let check_balance = || async { + let balance = self + .watch_only_wallet + .get_balance(0) + .await + .map_err(|_| backoff::Error::Transient("io"))?; + let balance = Amount::from_piconero(balance); - Ok(proof) - }) - .retry(ConstantBackoff::new(Duration::from_secs(1))) - .await; + if balance != expected_amount { + return Err(backoff::Error::Transient("insufficient funds")); + } - if let Err(Error::InsufficientFunds { expected, actual }) = res { - return Err(InsufficientFunds { expected, actual }); + Ok(()) }; - Ok(()) + check_balance + .retry(ConstantBackoff::new(Duration::from_secs(1))) + .await + .expect("transient error is never returned"); } } diff --git a/swap/src/network/request_response.rs b/swap/src/network/request_response.rs index a042debc..8d3a78d0 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(alice::Message2), + Message2, // empty response Message3, // empty response } diff --git a/swap/tests/e2e.rs b/swap/tests/e2e.rs index 358657ca..e8aa507a 100644 --- a/swap/tests/e2e.rs +++ b/swap/tests/e2e.rs @@ -8,11 +8,6 @@ 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 _; @@ -55,43 +50,46 @@ async fn swap() { .await .unwrap(); - let (monero, _container) = - Monero::new(&cli, None, vec!["alice".to_string(), "bob".to_string()]) - .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(); monero .init(vec![("alice", xmr_alice), ("bob", xmr_bob)]) .await .unwrap(); - 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_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_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( +let db = Database::open(std::path::Path::new("../.swap-db/")).unwrap(); let alice_swap = alice::swap( alice_btc_wallet.clone(), - alice_xmr_wallet.clone(), - db, + alice_xmr_wallet.clone(),db, alice_multiaddr.clone(), alice_transport, alice_behaviour, ); let db_dir = tempdir().unwrap(); - let db = Database::open(db_dir.path()).unwrap(); - let (cmd_tx, mut _cmd_rx) = mpsc::channel(1); + let db = Database::open(db_dir.path()).unwrap();let (cmd_tx, mut _cmd_rx) = mpsc::channel(1); let (mut rsp_tx, rsp_rx) = mpsc::channel(1); let bob_behaviour = bob::Bob::default(); let bob_transport = build(bob_behaviour.identity()).unwrap(); let bob_swap = bob::swap( bob_btc_wallet.clone(), - bob_xmr_wallet.clone(), - db, + bob_xmr_wallet.clone(),db, btc.as_sat(), alice_multiaddr, cmd_tx, @@ -110,7 +108,7 @@ async fn swap() { let xmr_alice_final = alice_xmr_wallet.as_ref().get_balance().await.unwrap(); - bob_xmr_wallet.as_ref().0.refresh().await.unwrap(); + bob_xmr_wallet.as_ref().user_wallet.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 a65a4ab1..7062d4f7 100644 --- a/xmr-btc/src/alice.rs +++ b/xmr-btc/src/alice.rs @@ -24,16 +24,14 @@ use std::{ sync::Arc, time::Duration, }; -use tokio::{sync::Mutex, time::timeout}; +use tokio::time::timeout; use tracing::{error, info}; pub mod message; -pub use message::{Message, Message0, Message1, Message2}; +pub use message::{Message, Message0, Message1}; #[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, @@ -62,7 +60,7 @@ 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( - network: Arc>, + mut network: N, bitcoin_client: Arc, monero_client: Arc, // TODO: Replace this with a new, slimmer struct? @@ -94,7 +92,7 @@ where + Send + Sync + 'static, - M: monero::WatchForTransferImproved + Send + Sync + 'static, + M: monero::WatchForTransfer + Send + Sync + 'static, { #[allow(clippy::enum_variant_names)] #[derive(Debug)] @@ -149,25 +147,20 @@ where let S_a = monero::PublicKey::from_private_key(&monero::PrivateKey { scalar: s_a.into_ed25519(), }); - - let public_spend_key = S_a + S_b_monero; - let public_view_key = v.public(); + let S = S_a + S_b_monero; co.yield_(Action::LockXmr { amount: xmr, - public_spend_key, - public_view_key, + public_spend_key: S, + public_view_key: v.public(), }) .await; - let monero_joint_address = monero::Address::standard( - monero::Network::Mainnet, - public_spend_key, - public_view_key.into(), - ); + let monero_joint_address = + monero::Address::standard(monero::Network::Mainnet, S, v.public().into()); if let Either::Right(_) = select( - monero_client.watch_for_transfer_improved(monero_joint_address, xmr, v), + monero_client.watch_for_transfer(monero_joint_address, xmr, v), poll_until_btc_has_expired.clone(), ) .await @@ -176,9 +169,8 @@ where }; let tx_redeem_encsig = { - let mut guard = network.as_ref().lock().await; let tx_redeem_encsig = match select( - guard.receive_bitcoin_redeem_encsig(), + network.receive_bitcoin_redeem_encsig(), poll_until_btc_has_expired.clone(), ) .await @@ -383,7 +375,6 @@ 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()?; @@ -735,7 +726,7 @@ impl State4 { }); let S_b = self.S_b_monero; - let (tx_lock_proof, fee) = monero_wallet + let fee = monero_wallet .transfer(S_a + S_b, self.v.public(), self.xmr) .await?; @@ -754,7 +745,6 @@ 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, @@ -825,7 +815,6 @@ 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, @@ -834,12 +823,6 @@ 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 b82f6f9f..8088bc18 100644 --- a/xmr-btc/src/alice/message.rs +++ b/xmr-btc/src/alice/message.rs @@ -9,7 +9,6 @@ use crate::{bitcoin, monero}; pub enum Message { Message0(Message0), Message1(Message1), - Message2(Message2), } #[derive(Clone, Debug, Serialize, Deserialize)] @@ -29,15 +28,8 @@ 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 e9b98ac8..86dbd33d 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::{sync::Mutex, time::timeout}; +use tokio::time::timeout; use tracing::error; pub mod message; -use crate::monero::{CreateWalletForOutput, WatchForTransfer}; +use crate::monero::CreateWalletForOutput; pub use message::{Message, Message0, Message1, Message2, Message3}; #[allow(clippy::large_enum_variant)] @@ -48,12 +48,6 @@ 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 @@ -61,8 +55,7 @@ pub trait ReceiveTransferProof { /// /// 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( - network: Arc>, +pub fn action_generator( monero_client: Arc, bitcoin_client: Arc, // TODO: Replace this with a new, slimmer struct? @@ -85,7 +78,6 @@ 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 @@ -108,8 +100,6 @@ 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, @@ -140,39 +130,21 @@ 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; - match select( - monero_client.watch_for_transfer(S, v.public(), transfer_proof, xmr, 0), + 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 { - Either::Left((Err(e), _)) => { - return Err(SwapFailed::AfterBtcLock(Reason::InsufficientXmr(e))) - } - Either::Right(_) => return Err(SwapFailed::AfterBtcLock(Reason::BtcExpired)), - _ => {} + return Err(SwapFailed::AfterBtcLock(Reason::BtcExpired)); } let tx_redeem = bitcoin::TxRedeem::new(&tx_lock, &redeem_address); @@ -300,8 +272,7 @@ pub async fn next_state< Ok(state3.into()) } State::State3(state3) => { - let message2 = transport.receive_message().await?.try_into()?; - let state4 = state3.watch_for_lock_xmr(monero_wallet, message2).await?; + let state4 = state3.watch_for_lock_xmr(monero_wallet).await?; tracing::info!("bob has seen that alice has locked xmr"); Ok(state4.into()) } @@ -589,7 +560,7 @@ pub struct State3 { } impl State3 { - pub async fn watch_for_lock_xmr(self, xmr_wallet: &W, msg: alice::Message2) -> Result + pub async fn watch_for_lock_xmr(self, xmr_wallet: &W) -> Result where W: monero::WatchForTransfer, { @@ -598,15 +569,12 @@ 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( - S, - self.v.public(), - msg.tx_lock_proof, - self.xmr, - monero::MIN_CONFIRMATIONS, - ) - .await?; + .watch_for_transfer(monero_joint_address, self.xmr, self.v) + .await; Ok(State4 { A: self.A, diff --git a/xmr-btc/src/monero.rs b/xmr-btc/src/monero.rs index 114a91f5..6ec2f644 100644 --- a/xmr-btc/src/monero.rs +++ b/xmr-btc/src/monero.rs @@ -1,5 +1,4 @@ use crate::serde::monero_private_key; -use anyhow::Result; use async_trait::async_trait; use rand::{CryptoRng, RngCore}; use serde::{Deserialize, Serialize}; @@ -8,8 +7,6 @@ use std::ops::{Add, Sub}; pub use curve25519_dalek::scalar::Scalar; pub use monero::{Address, Network, PrivateKey, PublicKey}; -pub const MIN_CONFIRMATIONS: u32 = 10; - pub fn random_private_key(rng: &mut R) -> PrivateKey { let scalar = Scalar::random(rng); @@ -103,35 +100,6 @@ 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( @@ -139,29 +107,17 @@ pub trait Transfer { public_spend_key: PublicKey, public_view_key: PublicViewKey, amount: Amount, - ) -> anyhow::Result<(TransferProof, Amount)>; + ) -> anyhow::Result; } #[async_trait] pub trait WatchForTransfer { async fn watch_for_transfer( - &self, - public_spend_key: PublicKey, - public_view_key: PublicViewKey, - transfer_proof: TransferProof, - amount: Amount, - expected_confirmations: u32, - ) -> Result<(), InsufficientFunds>; -} - -#[async_trait] -pub trait WatchForTransferImproved { - async fn watch_for_transfer_improved( &self, address: Address, amount: Amount, private_view_key: PrivateViewKey, - ) -> Result<(), InsufficientFunds>; + ); } #[derive(Debug, Clone, Copy, thiserror::Error)] diff --git a/xmr-btc/tests/harness/wallet/monero.rs b/xmr-btc/tests/harness/wallet/monero.rs index 302edc2d..fef131cd 100644 --- a/xmr-btc/tests/harness/wallet/monero.rs +++ b/xmr-btc/tests/harness/wallet/monero.rs @@ -4,10 +4,10 @@ use backoff::{backoff::Constant as ConstantBackoff, future::FutureOperation as _ use futures::TryFutureExt; use monero::{Address, Network, PrivateKey}; use monero_harness::rpc::wallet; -use std::{str::FromStr, time::Duration}; +use std::time::Duration; use xmr_btc::monero::{ - Address, Amount, CreateWalletForOutput, InsufficientFunds, Network, PrivateKey, PrivateViewKey, - PublicKey, PublicViewKey, Transfer, TransferProof, TxHash, WatchForTransfer, WatchForTransferImproved, + Amount, CreateWalletForOutput, PrivateViewKey, PublicKey, PublicViewKey, Transfer, + WatchForTransfer, }; pub struct Wallet { @@ -33,7 +33,7 @@ impl Transfer for Wallet { public_spend_key: PublicKey, public_view_key: PublicViewKey, amount: Amount, - ) -> Result<(TransferProof, Amount)> { + ) -> Result { let destination_address = Address::standard(Network::Mainnet, public_spend_key, public_view_key.into()); @@ -42,12 +42,7 @@ impl Transfer for Wallet { .transfer(0, amount.as_piconero(), &destination_address.to_string()) .await?; - 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)) + Ok(Amount::from_piconero(res.fee)) } } @@ -79,67 +74,11 @@ impl CreateWalletForOutput for Wallet { #[async_trait] impl WatchForTransfer for Wallet { async fn watch_for_transfer( - &self, - public_spend_key: PublicKey, - public_view_key: PublicViewKey, - transfer_proof: TransferProof, - expected_amount: Amount, - expected_confirmations: u32, - ) -> Result<(), InsufficientFunds> { - enum Error { - TxNotFound, - InsufficientConfirmations, - InsufficientFunds { expected: Amount, actual: Amount }, - } - - let address = Address::standard(Network::Mainnet, public_spend_key, public_view_key.into()); - - 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 - .inner - .check_tx_key( - &String::from(transfer_proof.tx_hash()), - &transfer_proof.tx_key().to_string(), - &address.to_string(), - ) - .await - .map_err(|_| backoff::Error::Transient(Error::TxNotFound))?; - - if proof.received != expected_amount.as_piconero() { - return Err(backoff::Error::Permanent(Error::InsufficientFunds { - expected: expected_amount, - actual: Amount::from_piconero(proof.received), - })); - } - - 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 }); - }; - - Ok(()) - } -} - -#[async_trait] -impl WatchForTransferImproved for Wallet { - async fn watch_for_transfer_improved( &self, address: Address, expected_amount: Amount, private_view_key: PrivateViewKey, - ) -> Result<(), InsufficientFunds> { + ) { let address = address.to_string(); let private_view_key = PrivateKey::from(private_view_key).to_string(); let load_address = || { @@ -148,11 +87,13 @@ impl WatchForTransferImproved for Wallet { .map_err(backoff::Error::Transient) }; + // QUESTION: Should we really retry every error? 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 refresh = || self.watch_only.refresh().map_err(backoff::Error::Transient); refresh @@ -160,25 +101,24 @@ impl WatchForTransferImproved for Wallet { .await .expect("transient error is never returned"); - let get_balance = || { - self.watch_only + let check_balance = || async { + let balance = self + .watch_only .get_balance(0) - .map_err(backoff::Error::Transient) + .await + .map_err(|_| backoff::Error::Transient("io"))?; + let balance = Amount::from_piconero(balance); + + if balance != expected_amount { + return Err(backoff::Error::Transient("insufficient funds")); + } + + Ok(()) }; - let balance = get_balance + check_balance .retry(ConstantBackoff::new(Duration::from_secs(1))) .await .expect("transient error is never returned"); - let balance = Amount::from_piconero(balance); - - if balance != expected_amount { - return Err(InsufficientFunds { - expected: expected_amount, - actual: balance, - }); - } - - Ok(()) } } diff --git a/xmr-btc/tests/on_chain.rs b/xmr-btc/tests/on_chain.rs index 615fa323..763d082a 100644 --- a/xmr-btc/tests/on_chain.rs +++ b/xmr-btc/tests/on_chain.rs @@ -16,20 +16,18 @@ 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::{self, ReceiveTransferProof}, - monero::{CreateWalletForOutput, Transfer, TransferProof}, + bob, + monero::{CreateWalletForOutput, Transfer}, }; /// 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 { @@ -46,13 +44,6 @@ 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 { @@ -101,10 +92,7 @@ impl Default for BobBehaviour { } async fn swap_as_alice( - 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, + network: AliceNetwork, monero_wallet: Arc, bitcoin_wallet: Arc, behaviour: AliceBehaviour, @@ -130,11 +118,9 @@ async fn swap_as_alice( public_view_key, }) => { if behaviour.lock_xmr { - let (transfer_proof, _) = monero_wallet + let _ = monero_wallet .transfer(public_spend_key, public_view_key, amount) .await?; - - sender.send(transfer_proof).await?; } } GeneratorState::Yielded(alice::Action::RedeemBtc(tx)) => { @@ -168,7 +154,6 @@ async fn swap_as_alice( } async fn swap_as_bob( - network: Arc>, mut sender: Sender, monero_wallet: Arc, bitcoin_wallet: Arc, @@ -176,7 +161,6 @@ async fn swap_as_bob( state: bob::State2, ) -> Result<()> { let mut action_generator = bob::action_generator( - network, monero_wallet.clone(), bitcoin_wallet.clone(), state, @@ -274,19 +258,16 @@ 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( - Arc::new(Mutex::new(alice_network)), - alice_sender, + alice_network, 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(), @@ -371,12 +352,10 @@ 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( - Arc::new(Mutex::new(alice_network)), - alice_sender, + alice_network, alice_monero_wallet.clone(), alice_bitcoin_wallet.clone(), AliceBehaviour { @@ -386,7 +365,6 @@ 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(), @@ -468,18 +446,15 @@ 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( - Arc::new(Mutex::new(alice_network)), - alice_sender, + alice_network, 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(),