From 396c4177a60dd999424ea1527558c027b6566a90 Mon Sep 17 00:00:00 2001 From: Daniel Karzel Date: Tue, 16 Mar 2021 19:24:41 +1100 Subject: [PATCH] Alice sweeps refunded funds into default wallet Since Alice's refund scenario starts with generating the temporary wallet from keys to claim the XMR which results in Alice' unloading the wallet. Alice then loads her original wallet to be able to handle more swaps. Since Alice is in the role of the long running daemon handling concurrent swaps, the operation to close, claim and re-open her default wallet must be atomic. This PR adds an additional step, that sweeps all the refunded XMR back into the default wallet. In order to ensure that this is possible, Alice has to ensure that the locked XMR got enough confirmations. These changes allow us to assert Alice's balance after refunding. --- swap/src/bin/asb.rs | 10 +- swap/src/bin/swap.rs | 12 +-- swap/src/monero/wallet.rs | 91 +++++++++++++------ swap/src/protocol/alice/state.rs | 21 ++++- swap/src/protocol/alice/swap.rs | 13 ++- ...refunds_using_cancel_and_refund_command.rs | 5 +- swap/tests/testutils/mod.rs | 20 +++- 7 files changed, 119 insertions(+), 53 deletions(-) diff --git a/swap/src/bin/asb.rs b/swap/src/bin/asb.rs index 76a4d5b5..b148ced7 100644 --- a/swap/src/bin/asb.rs +++ b/swap/src/bin/asb.rs @@ -164,18 +164,16 @@ async fn init_wallets( bitcoin_balance ); - let monero_wallet = monero::Wallet::new( + let monero_wallet = monero::Wallet::open_or_create( config.monero.wallet_rpc_url.clone(), DEFAULT_WALLET_NAME.to_string(), env_config, - ); - - // Setup the Monero wallet - monero_wallet.open_or_create().await?; + ) + .await?; let balance = monero_wallet.get_balance().await?; if balance == Amount::ZERO { - let deposit_address = monero_wallet.get_main_address().await?; + let deposit_address = monero_wallet.get_main_address(); warn!( "The Monero balance is 0, make sure to deposit funds at: {}", deposit_address diff --git a/swap/src/bin/swap.rs b/swap/src/bin/swap.rs index e4c9f102..5647a746 100644 --- a/swap/src/bin/swap.rs +++ b/swap/src/bin/swap.rs @@ -295,18 +295,12 @@ async fn init_monero_wallet( .run(network, monero_daemon_host.as_str()) .await?; - let monero_wallet = monero::Wallet::new( + let monero_wallet = monero::Wallet::open_or_create( monero_wallet_rpc_process.endpoint(), MONERO_BLOCKCHAIN_MONITORING_WALLET_NAME.to_string(), env_config, - ); - - monero_wallet.open_or_create().await?; - - let _test_wallet_connection = monero_wallet - .block_height() - .await - .context("Failed to validate connection to monero-wallet-rpc")?; + ) + .await?; Ok((monero_wallet, monero_wallet_rpc_process)) } diff --git a/swap/src/monero/wallet.rs b/swap/src/monero/wallet.rs index db9c7c57..4c643356 100644 --- a/swap/src/monero/wallet.rs +++ b/swap/src/monero/wallet.rs @@ -5,7 +5,7 @@ use crate::monero::{ use ::monero::{Address, Network, PrivateKey, PublicKey}; use anyhow::{Context, Result}; use monero_rpc::wallet; -use monero_rpc::wallet::{BlockHeight, CheckTxKey, Client, Refreshed}; +use monero_rpc::wallet::{BlockHeight, CheckTxKey, Refreshed}; use std::future::Future; use std::str::FromStr; use std::time::Duration; @@ -19,24 +19,44 @@ pub struct Wallet { inner: Mutex, network: Network, name: String, + main_address: monero::Address, sync_interval: Duration, } impl Wallet { - pub fn new(url: Url, name: String, env_config: Config) -> Self { - Self::new_with_client(Client::new(url), name, env_config) + /// Connect to a wallet RPC and load the given wallet by name. + pub async fn open_or_create(url: Url, name: String, env_config: Config) -> Result { + let client = wallet::Client::new(url); + + let open_wallet_response = client.open_wallet(name.as_str()).await; + if open_wallet_response.is_err() { + client.create_wallet(name.as_str()).await.context( + "Unable to create Monero wallet, please ensure that the monero-wallet-rpc is available", + )?; + + debug!("Created Monero wallet {}", name); + } else { + debug!("Opened Monero wallet {}", name); + } + + Self::connect(client, name, env_config).await } - pub fn new_with_client(client: wallet::Client, name: String, env_config: Config) -> Self { - Self { + /// Connects to a wallet RPC where a wallet is already loaded. + pub async fn connect(client: wallet::Client, name: String, env_config: Config) -> Result { + let main_address = + monero::Address::from_str(client.get_address(0).await?.address.as_str())?; + Ok(Self { inner: Mutex::new(client), network: env_config.monero_network, name, + main_address, sync_interval: env_config.monero_sync_interval(), - } + }) } - pub async fn open(&self) -> Result<()> { + /// Re-open the wallet using the internally stored name. + pub async fn re_open(&self) -> Result<()> { self.inner .lock() .await @@ -45,21 +65,8 @@ impl Wallet { Ok(()) } - pub async fn open_or_create(&self) -> Result<()> { - let open_wallet_response = self.open().await; - if open_wallet_response.is_err() { - self.inner.lock().await.create_wallet(self.name.as_str()).await.context( - "Unable to create Monero wallet, please ensure that the monero-wallet-rpc is available", - )?; - - debug!("Created Monero wallet {}", self.name); - } else { - debug!("Opened Monero wallet {}", self.name); - } - - Ok(()) - } - + /// Close the wallet and open (load) another wallet by generating it from + /// keys. The generated wallet will remain loaded. pub async fn create_from_and_load( &self, private_spend_key: PrivateKey, @@ -89,6 +96,10 @@ impl Wallet { Ok(()) } + /// Close the wallet and open (load) another wallet by generating it from + /// keys. The generated wallet will be opened, all funds sweeped to the + /// main_address and then the wallet will be re-loaded using the internally + /// stored name. pub async fn create_from( &self, private_spend_key: PrivateKey, @@ -98,23 +109,48 @@ impl Wallet { let public_spend_key = PublicKey::from_private_key(&private_spend_key); let public_view_key = PublicKey::from_private_key(&private_view_key.into()); - let address = Address::standard(self.network, public_spend_key, public_view_key); + let temp_wallet_address = + Address::standard(self.network, public_spend_key, public_view_key); let wallet = self.inner.lock().await; - // Properly close the wallet before generating the other wallet to ensure that + // Close the default wallet before generating the other wallet to ensure that // it saves its state correctly let _ = wallet.close_wallet().await?; let _ = wallet .generate_from_keys( - &address.to_string(), + &temp_wallet_address.to_string(), &private_spend_key.to_string(), &PrivateKey::from(private_view_key).to_string(), restore_height.height, ) .await?; + // Try to send all the funds from the generated wallet to the default wallet + match wallet.refresh().await { + Ok(_) => match wallet + .sweep_all(self.main_address.to_string().as_str()) + .await + { + Ok(sweep_all) => { + for tx in sweep_all.tx_hash_list { + tracing::info!(%tx, "Monero transferred back to default wallet {}", self.main_address); + } + } + Err(e) => { + tracing::warn!( + "Transferring Monero back to default wallet {} failed with {:#}", + self.main_address, + e + ); + } + }, + Err(e) => { + tracing::warn!("Refreshing the generated wallet failed with {:#}", e); + } + } + let _ = wallet.open_wallet(self.name.as_str()).await?; Ok(()) @@ -209,9 +245,8 @@ impl Wallet { self.inner.lock().await.block_height().await } - pub async fn get_main_address(&self) -> Result
{ - let address = self.inner.lock().await.get_address(0).await?; - Ok(Address::from_str(address.address.as_str())?) + pub fn get_main_address(&self) -> Address { + self.main_address } pub async fn refresh(&self) -> Result { diff --git a/swap/src/protocol/alice/state.rs b/swap/src/protocol/alice/state.rs index bc443f03..3b035a41 100644 --- a/swap/src/protocol/alice/state.rs +++ b/swap/src/protocol/alice/state.rs @@ -2,7 +2,8 @@ use crate::bitcoin::{ current_epoch, CancelTimelock, ExpiredTimelocks, PunishTimelock, TxCancel, TxPunish, TxRefund, }; use crate::env::Config; -use crate::monero::wallet::TransferRequest; +use crate::monero::wallet::{TransferRequest, WatchRequest}; +use crate::monero::TransferProof; use crate::protocol::alice::{Message1, Message3}; use crate::protocol::bob::{Message0, Message2, Message4}; use crate::protocol::CROSS_CURVE_PROOF_SYSTEM; @@ -357,6 +358,24 @@ impl State3 { } } + pub fn lock_xmr_watch_request( + &self, + transfer_proof: TransferProof, + conf_target: u32, + ) -> WatchRequest { + let S_a = monero::PublicKey::from_private_key(&monero::PrivateKey { scalar: self.s_a }); + + let public_spend_key = S_a + self.S_b_monero; + let public_view_key = self.v.public(); + WatchRequest { + public_spend_key, + public_view_key, + transfer_proof, + conf_target, + expected: self.xmr, + } + } + pub fn tx_cancel(&self) -> TxCancel { TxCancel::new(&self.tx_lock, self.cancel_timelock, self.a.public(), self.B) } diff --git a/swap/src/protocol/alice/swap.rs b/swap/src/protocol/alice/swap.rs index baa734d7..55c305a2 100644 --- a/swap/src/protocol/alice/swap.rs +++ b/swap/src/protocol/alice/swap.rs @@ -97,13 +97,20 @@ async fn run_until_internal( .transfer(state3.lock_xmr_transfer_request()) .await?; - // TODO(Franck): Wait for Monero to be confirmed once - // Waiting for XMR confirmations should not be done in here, but in a separate + monero_wallet + .watch_for_transfer(state3.lock_xmr_watch_request(transfer_proof.clone(), 1)) + .await?; + + // TODO: Waiting for XMR confirmations should be done in a separate // state! We have to record that Alice has already sent the transaction. // Otherwise Alice might publish the lock tx twice! event_loop_handle - .send_transfer_proof(transfer_proof) + .send_transfer_proof(transfer_proof.clone()) + .await?; + + monero_wallet + .watch_for_transfer(state3.lock_xmr_watch_request(transfer_proof, 10)) .await?; AliceState::XmrLocked { diff --git a/swap/tests/bob_refunds_using_cancel_and_refund_command.rs b/swap/tests/bob_refunds_using_cancel_and_refund_command.rs index df0783c0..dc2ea482 100644 --- a/swap/tests/bob_refunds_using_cancel_and_refund_command.rs +++ b/swap/tests/bob_refunds_using_cancel_and_refund_command.rs @@ -12,7 +12,7 @@ async fn given_bob_manually_refunds_after_btc_locked_bob_refunds() { let bob_swap = tokio::spawn(bob::run_until(bob_swap, is_btc_locked)); let alice_swap = ctx.alice_next_swap().await; - let _ = tokio::spawn(alice::run(alice_swap)); + let alice_swap = tokio::spawn(alice::run(alice_swap)); let bob_state = bob_swap.await??; assert!(matches!(bob_state, BobState::BtcLocked { .. })); @@ -56,6 +56,9 @@ async fn given_bob_manually_refunds_after_btc_locked_bob_refunds() { ctx.assert_bob_refunded(bob_state).await; + let alice_state = alice_swap.await??; + ctx.assert_alice_refunded(alice_state).await; + Ok(()) }) .await diff --git a/swap/tests/testutils/mod.rs b/swap/tests/testutils/mod.rs index 7f4c21f6..d9070d8e 100644 --- a/swap/tests/testutils/mod.rs +++ b/swap/tests/testutils/mod.rs @@ -57,7 +57,7 @@ struct BobParams { impl BobParams { pub async fn builder(&self, event_loop_handle: bob::EventLoopHandle) -> Result { - let receive_address = self.monero_wallet.get_main_address().await?; + let receive_address = self.monero_wallet.get_main_address(); Ok(bob::Builder::new( Database::open(&self.db_path.clone().as_path()).unwrap(), @@ -191,7 +191,15 @@ impl TestContext { .get_balance() .await .unwrap(); - assert_eq!(xmr_balance_after_swap, self.xmr_amount); + + // Alice pays fees - comparison does not take exact lock fee into account + assert!( + xmr_balance_after_swap > self.alice_starting_balances.xmr - self.xmr_amount, + "{} > {} - {}", + xmr_balance_after_swap, + self.alice_starting_balances.xmr, + self.xmr_amount + ); } pub async fn assert_alice_punished(&self, state: AliceState) { @@ -237,7 +245,7 @@ impl TestContext { ); // unload the generated wallet by opening the original wallet - self.bob_monero_wallet.open().await.unwrap(); + self.bob_monero_wallet.re_open().await.unwrap(); // refresh the original wallet to make sure the balance is caught up self.bob_monero_wallet.refresh().await.unwrap(); @@ -587,11 +595,13 @@ async fn init_test_wallets( .await .unwrap(); - let xmr_wallet = swap::monero::Wallet::new_with_client( + let xmr_wallet = swap::monero::Wallet::connect( monero.wallet(name).unwrap().client(), name.to_string(), env_config, - ); + ) + .await + .unwrap(); let electrum_rpc_url = { let input = format!("tcp://@localhost:{}", electrum_rpc_port);