From 5b798217bcc00a404c1af0f5e3e66b7333f17c81 Mon Sep 17 00:00:00 2001 From: Daniel Karzel Date: Tue, 2 Mar 2021 19:35:55 +1100 Subject: [PATCH 01/14] Open means opening the current wallet --- swap/src/bin/asb.rs | 2 +- swap/src/bin/swap_cli.rs | 4 +--- swap/src/monero.rs | 2 +- swap/src/monero/wallet.rs | 8 ++++++-- 4 files changed, 9 insertions(+), 7 deletions(-) diff --git a/swap/src/bin/asb.rs b/swap/src/bin/asb.rs index 488aade3..26920365 100644 --- a/swap/src/bin/asb.rs +++ b/swap/src/bin/asb.rs @@ -165,7 +165,7 @@ async fn init_wallets( ); // Setup the Monero wallet - let open_wallet_response = monero_wallet.open_wallet(DEFAULT_WALLET_NAME).await; + let open_wallet_response = monero_wallet.open().await; if open_wallet_response.is_err() { monero_wallet .create_wallet(DEFAULT_WALLET_NAME) diff --git a/swap/src/bin/swap_cli.rs b/swap/src/bin/swap_cli.rs index ad6c74dd..0834151c 100644 --- a/swap/src/bin/swap_cli.rs +++ b/swap/src/bin/swap_cli.rs @@ -293,9 +293,7 @@ async fn init_monero_wallet( ); // Setup the temporary Monero wallet necessary for monitoring the blockchain - let open_monitoring_wallet_response = monero_wallet - .open_wallet(MONERO_BLOCKCHAIN_MONITORING_WALLET_NAME) - .await; + let open_monitoring_wallet_response = monero_wallet.open().await; if open_monitoring_wallet_response.is_err() { monero_wallet .create_wallet(MONERO_BLOCKCHAIN_MONITORING_WALLET_NAME) diff --git a/swap/src/monero.rs b/swap/src/monero.rs index 90224948..00056c2e 100644 --- a/swap/src/monero.rs +++ b/swap/src/monero.rs @@ -240,7 +240,7 @@ pub trait CreateWalletForOutputThenLoadDefaultWallet { #[async_trait] pub trait OpenWallet { - async fn open_wallet(&self, file_name: &str) -> Result<()>; + async fn open(&self) -> Result<()>; } #[async_trait] diff --git a/swap/src/monero/wallet.rs b/swap/src/monero/wallet.rs index b79ced60..202b7185 100644 --- a/swap/src/monero/wallet.rs +++ b/swap/src/monero/wallet.rs @@ -166,8 +166,12 @@ impl CreateWalletForOutputThenLoadDefaultWallet for Wallet { #[async_trait] impl OpenWallet for Wallet { - async fn open_wallet(&self, file_name: &str) -> Result<()> { - self.inner.lock().await.open_wallet(file_name).await?; + async fn open(&self) -> Result<()> { + self.inner + .lock() + .await + .open_wallet(self.default_wallet_name.as_str()) + .await?; Ok(()) } } From 70494fcb4f9ca5b9f38b755c88cd569da801a0a3 Mon Sep 17 00:00:00 2001 From: Daniel Karzel Date: Tue, 2 Mar 2021 19:38:07 +1100 Subject: [PATCH 02/14] Create means creating the current wallet --- swap/src/bin/asb.rs | 11 ++++------- swap/src/bin/swap_cli.rs | 11 ++++------- swap/src/monero.rs | 2 +- swap/src/monero/wallet.rs | 8 ++++++-- 4 files changed, 15 insertions(+), 17 deletions(-) diff --git a/swap/src/bin/asb.rs b/swap/src/bin/asb.rs index 26920365..26721e77 100644 --- a/swap/src/bin/asb.rs +++ b/swap/src/bin/asb.rs @@ -167,14 +167,11 @@ async fn init_wallets( // Setup the Monero wallet let open_wallet_response = monero_wallet.open().await; if open_wallet_response.is_err() { - monero_wallet - .create_wallet(DEFAULT_WALLET_NAME) - .await - .context(format!( - "Unable to create Monero wallet.\ + monero_wallet.create().await.context(format!( + "Unable to create Monero wallet.\ Please ensure that the monero-wallet-rpc is available at {}", - config.monero.wallet_rpc_url - ))?; + config.monero.wallet_rpc_url + ))?; info!("Created Monero wallet {}", DEFAULT_WALLET_NAME); } else { diff --git a/swap/src/bin/swap_cli.rs b/swap/src/bin/swap_cli.rs index 0834151c..a68ce755 100644 --- a/swap/src/bin/swap_cli.rs +++ b/swap/src/bin/swap_cli.rs @@ -295,14 +295,11 @@ async fn init_monero_wallet( // Setup the temporary Monero wallet necessary for monitoring the blockchain let open_monitoring_wallet_response = monero_wallet.open().await; if open_monitoring_wallet_response.is_err() { - monero_wallet - .create_wallet(MONERO_BLOCKCHAIN_MONITORING_WALLET_NAME) - .await - .context(format!( - "Unable to create Monero wallet for blockchain monitoring.\ + monero_wallet.create().await.context(format!( + "Unable to create Monero wallet for blockchain monitoring.\ Please ensure that the monero-wallet-rpc is available at {}", - monero_wallet_rpc_url - ))?; + monero_wallet_rpc_url + ))?; debug!( "Created Monero wallet for blockchain monitoring with name {}", diff --git a/swap/src/monero.rs b/swap/src/monero.rs index 00056c2e..b3c9c51a 100644 --- a/swap/src/monero.rs +++ b/swap/src/monero.rs @@ -245,7 +245,7 @@ pub trait OpenWallet { #[async_trait] pub trait CreateWallet { - async fn create_wallet(&self, file_name: &str) -> Result<()>; + async fn create(&self) -> Result<()>; } #[async_trait] diff --git a/swap/src/monero/wallet.rs b/swap/src/monero/wallet.rs index 202b7185..cc79a250 100644 --- a/swap/src/monero/wallet.rs +++ b/swap/src/monero/wallet.rs @@ -178,8 +178,12 @@ impl OpenWallet for Wallet { #[async_trait] impl CreateWallet for Wallet { - async fn create_wallet(&self, file_name: &str) -> Result<()> { - self.inner.lock().await.create_wallet(file_name).await?; + async fn create(&self) -> Result<()> { + self.inner + .lock() + .await + .create_wallet(self.default_wallet_name.as_str()) + .await?; Ok(()) } } From 9f53dab3c640bafa544a832e381919167d8dd548 Mon Sep 17 00:00:00 2001 From: Daniel Karzel Date: Tue, 2 Mar 2021 19:42:23 +1100 Subject: [PATCH 03/14] Harmonize names to make more sense The wallet is an instance of a wallet that has a name. When we use `CreateWalletForOutputThenReloadWallet` we actually unload the wallet. It would be cleaner to create a new instance that does that swap, but I did not go that far. --- swap/src/monero.rs | 4 ++-- swap/src/monero/wallet.rs | 28 +++++++++++----------------- swap/src/protocol/alice/swap.rs | 4 ++-- 3 files changed, 15 insertions(+), 21 deletions(-) diff --git a/swap/src/monero.rs b/swap/src/monero.rs index b3c9c51a..e400be22 100644 --- a/swap/src/monero.rs +++ b/swap/src/monero.rs @@ -229,8 +229,8 @@ pub trait CreateWalletForOutput { } #[async_trait] -pub trait CreateWalletForOutputThenLoadDefaultWallet { - async fn create_and_load_wallet_for_output_then_load_default_wallet( +pub trait CreateWalletForOutputThenReloadWallet { + async fn create_and_load_wallet_for_output_then_reload_wallet( &self, private_spend_key: PrivateKey, private_view_key: PrivateViewKey, diff --git a/swap/src/monero/wallet.rs b/swap/src/monero/wallet.rs index cc79a250..f8e5c2eb 100644 --- a/swap/src/monero/wallet.rs +++ b/swap/src/monero/wallet.rs @@ -1,5 +1,5 @@ use crate::monero::{ - Amount, CreateWallet, CreateWalletForOutput, CreateWalletForOutputThenLoadDefaultWallet, + Amount, CreateWallet, CreateWalletForOutput, CreateWalletForOutputThenReloadWallet, InsufficientFunds, OpenWallet, PrivateViewKey, PublicViewKey, Transfer, TransferProof, TxHash, WatchForTransfer, }; @@ -21,27 +21,23 @@ use url::Url; pub struct Wallet { inner: Mutex, network: Network, - default_wallet_name: String, + name: String, } impl Wallet { - pub fn new(url: Url, network: Network, default_wallet_name: String) -> Self { + pub fn new(url: Url, network: Network, name: String) -> Self { Self { inner: Mutex::new(wallet::Client::new(url)), network, - default_wallet_name, + name, } } - pub fn new_with_client( - client: wallet::Client, - network: Network, - default_wallet_name: String, - ) -> Self { + pub fn new_with_client(client: wallet::Client, network: Network, name: String) -> Self { Self { inner: Mutex::new(client), network, - default_wallet_name, + name, } } @@ -133,8 +129,8 @@ impl CreateWalletForOutput for Wallet { } #[async_trait] -impl CreateWalletForOutputThenLoadDefaultWallet for Wallet { - async fn create_and_load_wallet_for_output_then_load_default_wallet( +impl CreateWalletForOutputThenReloadWallet for Wallet { + async fn create_and_load_wallet_for_output_then_reload_wallet( &self, private_spend_key: PrivateKey, private_view_key: PrivateViewKey, @@ -156,9 +152,7 @@ impl CreateWalletForOutputThenLoadDefaultWallet for Wallet { ) .await?; - let _ = wallet - .open_wallet(self.default_wallet_name.as_str()) - .await?; + let _ = wallet.open_wallet(self.name.as_str()).await?; Ok(()) } @@ -170,7 +164,7 @@ impl OpenWallet for Wallet { self.inner .lock() .await - .open_wallet(self.default_wallet_name.as_str()) + .open_wallet(self.name.as_str()) .await?; Ok(()) } @@ -182,7 +176,7 @@ impl CreateWallet for Wallet { self.inner .lock() .await - .create_wallet(self.default_wallet_name.as_str()) + .create_wallet(self.name.as_str()) .await?; Ok(()) } diff --git a/swap/src/protocol/alice/swap.rs b/swap/src/protocol/alice/swap.rs index addde0a4..4b13dff1 100644 --- a/swap/src/protocol/alice/swap.rs +++ b/swap/src/protocol/alice/swap.rs @@ -7,7 +7,7 @@ use crate::{ database::Database, execution_params::ExecutionParams, monero, - monero::CreateWalletForOutputThenLoadDefaultWallet, + monero::CreateWalletForOutputThenReloadWallet, monero_ext::ScalarExt, protocol::{ alice, @@ -392,7 +392,7 @@ async fn run_until_internal( let view_key = state3.v; monero_wallet - .create_and_load_wallet_for_output_then_load_default_wallet( + .create_and_load_wallet_for_output_then_reload_wallet( spend_key, view_key, monero_wallet_restore_blockheight, From 2bb1c1e177a7bf50be917fdbd58f49e866ee580c Mon Sep 17 00:00:00 2001 From: Daniel Karzel Date: Wed, 3 Mar 2021 10:56:49 +1100 Subject: [PATCH 04/14] No prefix for wallets in monero harness Prefixing docker-containers and -networks is a necessity to be able to spin up multiple containers and networks. However, there is no reason to prefix the wallet names that live inside a container. One cannot add a wallet with the same name twice, so the prefixing of wallets does not bring any advantage. When re-opening a wallet by name the wallet name prefix is cumbersome and was thus removed. --- monero-harness/src/lib.rs | 35 +++++++++++---------------------- monero-harness/tests/monerod.rs | 2 +- monero-harness/tests/wallet.rs | 9 +++------ swap/tests/testutils/mod.rs | 2 +- 4 files changed, 17 insertions(+), 31 deletions(-) diff --git a/monero-harness/src/lib.rs b/monero-harness/src/lib.rs index ab528564..f635da63 100644 --- a/monero-harness/src/lib.rs +++ b/monero-harness/src/lib.rs @@ -44,52 +44,42 @@ const WAIT_WALLET_SYNC_MILLIS: u64 = 1000; pub struct Monero { monerod: Monerod, wallets: Vec, - prefix: String, } impl<'c> Monero { /// Starts a new regtest monero container setup consisting out of 1 monerod - /// node and n wallets. The containers and network will be prefixed, either - /// randomly generated or as defined in `prefix` if provided. There will - /// be 1 miner wallet started automatically. Default monerod container - /// name will be: `prefix`_`monerod` Default miner wallet container name - /// will be: `prefix`_`miner` Default network will be: `prefix`_`monero` + /// node and n wallets. The docker container and network will be prefixed + /// with a randomly generated `prefix`. One miner wallet is started + /// automatically. + /// monerod container name is: `prefix`_`monerod` + /// network is: `prefix`_`monero` + /// miner wallet container name is: `miner` pub async fn new( cli: &'c Cli, - prefix: Option, additional_wallets: Vec, ) -> Result<(Self, Vec>)> { - let prefix = format!("{}_", prefix.unwrap_or_else(random_prefix)); - + let prefix = format!("{}_", random_prefix()); let monerod_name = format!("{}{}", prefix, MONEROD_DAEMON_CONTAINER_NAME); let network = format!("{}{}", prefix, MONEROD_DEFAULT_NETWORK); - tracing::info!("Starting monerod... {}", monerod_name); + tracing::info!("Starting monerod: {}", monerod_name); let (monerod, monerod_container) = Monerod::new(cli, monerod_name, network)?; let mut containers = vec![monerod_container]; let mut wallets = vec![]; - let miner = format!("{}{}", prefix, "miner"); - tracing::info!("Starting miner wallet... {}", miner); + let miner = "miner"; + tracing::info!("Starting miner wallet: {}", miner); let (miner_wallet, miner_container) = MoneroWalletRpc::new(cli, &miner, &monerod).await?; wallets.push(miner_wallet); containers.push(miner_container); for wallet in additional_wallets.iter() { - tracing::info!("Starting wallet: {}...", wallet); - let wallet = format!("{}{}", prefix, wallet); + tracing::info!("Starting wallet: {}", wallet); let (wallet, container) = MoneroWalletRpc::new(cli, &wallet, &monerod).await?; wallets.push(wallet); containers.push(container); } - Ok(( - Self { - monerod, - wallets, - prefix, - }, - containers, - )) + Ok((Self { monerod, wallets }, containers)) } pub fn monerod(&self) -> &Monerod { @@ -97,7 +87,6 @@ impl<'c> Monero { } pub fn wallet(&self, name: &str) -> Result<&MoneroWalletRpc> { - let name = format!("{}{}", self.prefix, name); let wallet = self .wallets .iter() diff --git a/monero-harness/tests/monerod.rs b/monero-harness/tests/monerod.rs index e48e3b87..7e816264 100644 --- a/monero-harness/tests/monerod.rs +++ b/monero-harness/tests/monerod.rs @@ -12,7 +12,7 @@ async fn init_miner_and_mine_to_miner_address() { let _guard = init_tracing(); let tc = Cli::default(); - let (monero, _monerod_container) = Monero::new(&tc, None, vec![]).await.unwrap(); + let (monero, _monerod_container) = Monero::new(&tc, vec![]).await.unwrap(); monero.init(vec![]).await.unwrap(); diff --git a/monero-harness/tests/wallet.rs b/monero-harness/tests/wallet.rs index a571a35e..3314341a 100644 --- a/monero-harness/tests/wallet.rs +++ b/monero-harness/tests/wallet.rs @@ -16,12 +16,9 @@ async fn fund_transfer_and_check_tx_key() { let send_to_bob = 5_000_000_000; let tc = Cli::default(); - let (monero, _containers) = Monero::new(&tc, Some("test_".to_string()), vec![ - "alice".to_string(), - "bob".to_string(), - ]) - .await - .unwrap(); + let (monero, _containers) = Monero::new(&tc, vec!["alice".to_string(), "bob".to_string()]) + .await + .unwrap(); let alice_wallet = monero.wallet("alice").unwrap(); let bob_wallet = monero.wallet("bob").unwrap(); diff --git a/swap/tests/testutils/mod.rs b/swap/tests/testutils/mod.rs index 61cd01db..a3a27ace 100644 --- a/swap/tests/testutils/mod.rs +++ b/swap/tests/testutils/mod.rs @@ -597,7 +597,7 @@ async fn init_test_wallets( let xmr_wallet = swap::monero::Wallet::new_with_client( monero.wallet(name).unwrap().client(), monero::Network::default(), - "irrelevant_for_tests".to_string(), + name.to_string(), ); let electrum_rpc_url = { From 5111a12706d95541fa3b5f3012362ab5b5988611 Mon Sep 17 00:00:00 2001 From: Daniel Karzel Date: Wed, 3 Mar 2021 10:58:47 +1100 Subject: [PATCH 05/14] Wallet name constants for the e2e test setup Container initialization and wallet initialization have to ensure to use the same wallet name. In order to avoid problems constants are introduced to ensure we use the same wallet name. --- swap/tests/testutils/mod.rs | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/swap/tests/testutils/mod.rs b/swap/tests/testutils/mod.rs index a3a27ace..a3981850 100644 --- a/swap/tests/testutils/mod.rs +++ b/swap/tests/testutils/mod.rs @@ -33,7 +33,9 @@ use tracing_log::LogTracer; use url::Url; use uuid::Uuid; -const TEST_WALLET_NAME: &str = "testwallet"; +const MONERO_WALLET_NAME_BOB: &str = "bob"; +const MONERO_WALLET_NAME_ALICE: &str = "alice"; +const BITCOIN_TEST_WALLET_NAME: &str = "testwallet"; #[derive(Debug, Clone)] pub struct StartingBalances { @@ -353,7 +355,7 @@ where let bob_seed = Seed::random().unwrap(); let (alice_bitcoin_wallet, alice_monero_wallet) = init_test_wallets( - "alice", + MONERO_WALLET_NAME_ALICE, containers.bitcoind_url.clone(), &monero, alice_starting_balances.clone(), @@ -375,7 +377,7 @@ where }; let (bob_bitcoin_wallet, bob_monero_wallet) = init_test_wallets( - "bob", + MONERO_WALLET_NAME_BOB, containers.bitcoind_url, &monero, bob_starting_balances.clone(), @@ -529,11 +531,11 @@ async fn init_bitcoind(node_url: Url, spendable_quantity: u32) -> Result let bitcoind_client = Client::new(node_url.clone()); bitcoind_client - .createwallet(TEST_WALLET_NAME, None, None, None, None) + .createwallet(BITCOIN_TEST_WALLET_NAME, None, None, None, None) .await?; let reward_address = bitcoind_client - .with_wallet(TEST_WALLET_NAME)? + .with_wallet(BITCOIN_TEST_WALLET_NAME)? .getnewaddress(None, None) .await?; @@ -550,12 +552,12 @@ pub async fn mint(node_url: Url, address: bitcoin::Address, amount: bitcoin::Amo let bitcoind_client = Client::new(node_url.clone()); bitcoind_client - .send_to_address(TEST_WALLET_NAME, address.clone(), amount) + .send_to_address(BITCOIN_TEST_WALLET_NAME, address.clone(), amount) .await?; // Confirm the transaction let reward_address = bitcoind_client - .with_wallet(TEST_WALLET_NAME)? + .with_wallet(BITCOIN_TEST_WALLET_NAME)? .getnewaddress(None, None) .await?; bitcoind_client @@ -571,9 +573,12 @@ async fn init_monero_container( Monero, Vec>, ) { - let (monero, monerods) = Monero::new(&cli, None, vec!["alice".to_string(), "bob".to_string()]) - .await - .unwrap(); + let (monero, monerods) = Monero::new(&cli, vec![ + MONERO_WALLET_NAME_ALICE.to_string(), + MONERO_WALLET_NAME_BOB.to_string(), + ]) + .await + .unwrap(); (monero, monerods) } From 66c8401c95a98277647ef075d3f7fe61669e90f3 Mon Sep 17 00:00:00 2001 From: Daniel Karzel Date: Tue, 2 Mar 2021 22:07:48 +1100 Subject: [PATCH 06/14] Sweep all from generated wallet to user wallet The default implementation for the command was removed because it does not add additional value if we have a mandatory parameter anyway. --- monero-rpc/src/rpc/wallet.rs | 82 +++++++++++++++++++++++++++++++++++ swap/src/bin/swap_cli.rs | 6 ++- swap/src/cli/command.rs | 28 ++++-------- swap/src/execution_params.rs | 5 +-- swap/src/monero.rs | 3 +- swap/src/monero/wallet.rs | 24 ++++++++-- swap/src/protocol/bob.rs | 6 +++ swap/src/protocol/bob/swap.rs | 16 +++++++ swap/tests/testutils/mod.rs | 36 ++++++++++----- 9 files changed, 166 insertions(+), 40 deletions(-) diff --git a/monero-rpc/src/rpc/wallet.rs b/monero-rpc/src/rpc/wallet.rs index 67298b61..46804221 100644 --- a/monero-rpc/src/rpc/wallet.rs +++ b/monero-rpc/src/rpc/wallet.rs @@ -147,6 +147,28 @@ impl Client { Ok(()) } + /// Opens a wallet using `filename`. + pub async fn close_wallet(&self) -> Result<()> { + let request = Request::new("close_wallet", ""); + + let response = self + .inner + .post(self.url.clone()) + .json(&request) + .send() + .await? + .text() + .await?; + + debug!("close wallet RPC response: {}", response); + + if response.contains("error") { + bail!("Failed to close wallet") + } + + Ok(()) + } + /// Creates a wallet using `filename`. pub async fn create_wallet(&self, filename: &str) -> Result<()> { let params = CreateWalletParams { @@ -313,6 +335,28 @@ impl Client { let r: Response = serde_json::from_str(&response)?; Ok(r.result) } + + /// Transfers the complete balance of the account to `address`. + pub async fn sweep_all(&self, address: &str) -> Result { + let params = SweepAllParams { + address: address.into(), + }; + let request = Request::new("sweep_all", params); + + let response = self + .inner + .post(self.url.clone()) + .json(&request) + .send() + .await? + .text() + .await?; + + debug!("sweep_all RPC response: {}", response); + + let r: Response = serde_json::from_str(&response)?; + Ok(r.result) + } } #[derive(Serialize, Debug, Clone)] @@ -453,3 +497,41 @@ pub struct Refreshed { pub blocks_fetched: u32, pub received_money: bool, } + +#[derive(Debug, Clone, Serialize)] +pub struct SweepAllParams { + pub address: String, +} + +#[derive(Debug, Clone, Deserialize)] +pub struct SweepAll { + amount_list: Vec, + fee_list: Vec, + multisig_txset: String, + pub tx_hash_list: Vec, + unsigned_txset: String, + weight_list: Vec, +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn can_deserialize_sweep_all_response() { + let response = r#"{ + "id": "0", + "jsonrpc": "2.0", + "result": { + "amount_list": [29921410000], + "fee_list": [78590000], + "multisig_txset": "", + "tx_hash_list": ["c1d8cfa87d445c1915a59d67be3e93ba8a29018640cf69b465f07b1840a8f8c8"], + "unsigned_txset": "", + "weight_list": [1448] + } + }"#; + + let _: Response = serde_json::from_str(&response).unwrap(); + } +} diff --git a/swap/src/bin/swap_cli.rs b/swap/src/bin/swap_cli.rs index a68ce755..ec932482 100644 --- a/swap/src/bin/swap_cli.rs +++ b/swap/src/bin/swap_cli.rs @@ -102,8 +102,9 @@ async fn main() -> Result<()> { .run(monero_network, "stagenet.community.xmr.to") .await?; - match args.cmd.unwrap_or_default() { + match args.cmd { Command::BuyXmr { + receive_monero_address, alice_peer_id, alice_addr, } => { @@ -153,6 +154,7 @@ async fn main() -> Result<()> { Arc::new(monero_wallet), execution_params, event_loop_handle, + receive_monero_address, ) .with_init_params(send_bitcoin) .build()?; @@ -180,6 +182,7 @@ async fn main() -> Result<()> { table.printstd(); } Command::Resume { + receive_monero_address, swap_id, alice_peer_id, alice_addr, @@ -205,6 +208,7 @@ async fn main() -> Result<()> { Arc::new(monero_wallet), execution_params, event_loop_handle, + receive_monero_address, ) .build()?; diff --git a/swap/src/cli/command.rs b/swap/src/cli/command.rs index 1dc97fb9..0231f033 100644 --- a/swap/src/cli/command.rs +++ b/swap/src/cli/command.rs @@ -18,13 +18,16 @@ pub struct Arguments { pub debug: bool, #[structopt(subcommand)] - pub cmd: Option, + pub cmd: Command, } #[derive(structopt::StructOpt, Debug)] #[structopt(name = "xmr_btc-swap", about = "XMR BTC atomic swap")] pub enum Command { BuyXmr { + #[structopt(long = "receive-address")] + receive_monero_address: monero::Address, + #[structopt(long = "connect-peer-id", default_value = DEFAULT_ALICE_PEER_ID)] alice_peer_id: PeerId, @@ -36,6 +39,9 @@ pub enum Command { }, History, Resume { + #[structopt(long = "receive-address")] + receive_monero_address: monero::Address, + #[structopt(long = "swap-id")] swap_id: Uuid, @@ -66,22 +72,9 @@ pub enum Command { }, } -impl Default for Command { - fn default() -> Self { - Self::BuyXmr { - alice_peer_id: DEFAULT_ALICE_PEER_ID - .parse() - .expect("default alice peer id str is a valid Multiaddr>"), - alice_addr: DEFAULT_ALICE_MULTIADDR - .parse() - .expect("default alice multiaddr str is a valid PeerId"), - } - } -} - #[cfg(test)] mod tests { - use crate::cli::command::{Command, DEFAULT_ALICE_MULTIADDR, DEFAULT_ALICE_PEER_ID}; + use crate::cli::command::{DEFAULT_ALICE_MULTIADDR, DEFAULT_ALICE_PEER_ID}; use libp2p::{core::Multiaddr, PeerId}; #[test] @@ -97,9 +90,4 @@ mod tests { .parse::() .expect("default alice multiaddr str is a valid Multiaddr>"); } - - #[test] - fn default_command_success() { - Command::default(); - } } diff --git a/swap/src/execution_params.rs b/swap/src/execution_params.rs index ba3ae21e..c9e25c9e 100644 --- a/swap/src/execution_params.rs +++ b/swap/src/execution_params.rs @@ -91,8 +91,7 @@ mod testnet { pub static BITCOIN_AVG_BLOCK_TIME: Lazy = Lazy::new(|| Duration::from_secs(5 * 60)); - // This does not reflect recommended values for mainnet! - pub static MONERO_FINALITY_CONFIRMATIONS: u32 = 5; + pub static MONERO_FINALITY_CONFIRMATIONS: u32 = 10; // This does not reflect recommended values for mainnet! pub static BITCOIN_CANCEL_TIMELOCK: CancelTimelock = CancelTimelock::new(12); @@ -109,7 +108,7 @@ mod regtest { pub static BITCOIN_AVG_BLOCK_TIME: Lazy = Lazy::new(|| Duration::from_secs(5)); - pub static MONERO_FINALITY_CONFIRMATIONS: u32 = 1; + pub static MONERO_FINALITY_CONFIRMATIONS: u32 = 10; pub static BITCOIN_CANCEL_TIMELOCK: CancelTimelock = CancelTimelock::new(100); diff --git a/swap/src/monero.rs b/swap/src/monero.rs index e400be22..c68a84a4 100644 --- a/swap/src/monero.rs +++ b/swap/src/monero.rs @@ -1,7 +1,7 @@ pub mod wallet; mod wallet_rpc; -pub use ::monero::{Network, PrivateKey, PublicKey}; +pub use ::monero::{Address, Network, PrivateKey, PublicKey}; pub use curve25519_dalek::scalar::Scalar; pub use wallet::Wallet; pub use wallet_rpc::{WalletRpc, WalletRpcProcess}; @@ -10,7 +10,6 @@ use crate::bitcoin; use ::bitcoin::hashes::core::fmt::Formatter; use anyhow::Result; use async_trait::async_trait; -use monero::Address; use monero_rpc::wallet::{BlockHeight, Refreshed}; use rand::{CryptoRng, RngCore}; use rust_decimal::{ diff --git a/swap/src/monero/wallet.rs b/swap/src/monero/wallet.rs index f8e5c2eb..8fa0d98b 100644 --- a/swap/src/monero/wallet.rs +++ b/swap/src/monero/wallet.rs @@ -61,6 +61,15 @@ impl Wallet { self.inner.lock().await.refresh().await } + pub async fn sweep_all(&self, address: Address) -> Result> { + self.inner + .lock() + .await + .sweep_all(address.to_string().as_str()) + .await + .map(|sweep_all| sweep_all.tx_hash_list.into_iter().map(TxHash).collect()) + } + pub fn static_tx_fee_estimate(&self) -> Amount { // Median tx fees on Monero as found here: https://www.monero.how/monero-transaction-fees, 0.000_015 * 2 (to be on the safe side) Amount::from_monero(0.000_03f64).expect("static fee to be convertible without problems") @@ -112,10 +121,13 @@ impl CreateWalletForOutput for Wallet { let address = Address::standard(self.network, public_spend_key, public_view_key); - let _ = self - .inner - .lock() - .await + let wallet = self.inner.lock().await; + + // Properly close the 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(), &private_spend_key.to_string(), @@ -143,6 +155,10 @@ impl CreateWalletForOutputThenReloadWallet for Wallet { let wallet = self.inner.lock().await; + // Properly close the 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(), diff --git a/swap/src/protocol/bob.rs b/swap/src/protocol/bob.rs index 70ecc7d6..d69fbb83 100644 --- a/swap/src/protocol/bob.rs +++ b/swap/src/protocol/bob.rs @@ -44,6 +44,7 @@ pub struct Swap { pub monero_wallet: Arc, pub execution_params: ExecutionParams, pub swap_id: Uuid, + pub receive_monero_address: ::monero::Address, } pub struct Builder { @@ -57,6 +58,8 @@ pub struct Builder { execution_params: ExecutionParams, event_loop_handle: bob::EventLoopHandle, + + receive_monero_address: ::monero::Address, } enum InitParams { @@ -73,6 +76,7 @@ impl Builder { monero_wallet: Arc, execution_params: ExecutionParams, event_loop_handle: bob::EventLoopHandle, + receive_monero_address: ::monero::Address, ) -> Self { Self { swap_id, @@ -82,6 +86,7 @@ impl Builder { init_params: InitParams::None, execution_params, event_loop_handle, + receive_monero_address, } } @@ -106,6 +111,7 @@ impl Builder { monero_wallet: self.monero_wallet.clone(), swap_id: self.swap_id, execution_params: self.execution_params, + receive_monero_address: self.receive_monero_address, }) } } diff --git a/swap/src/protocol/bob/swap.rs b/swap/src/protocol/bob/swap.rs index cb412c2a..35b495de 100644 --- a/swap/src/protocol/bob/swap.rs +++ b/swap/src/protocol/bob/swap.rs @@ -43,6 +43,7 @@ pub async fn run_until( swap.monero_wallet, swap.swap_id, swap.execution_params, + swap.receive_monero_address, ) .await } @@ -59,6 +60,7 @@ async fn run_until_internal( monero_wallet: Arc, swap_id: Uuid, execution_params: ExecutionParams, + receive_monero_address: monero::Address, ) -> Result { trace!("Current state: {}", state); if is_target_state(&state) { @@ -90,6 +92,7 @@ async fn run_until_internal( monero_wallet, swap_id, execution_params, + receive_monero_address, ) .await } @@ -111,6 +114,7 @@ async fn run_until_internal( monero_wallet, swap_id, execution_params, + receive_monero_address, ) .await } @@ -159,6 +163,7 @@ async fn run_until_internal( monero_wallet, swap_id, execution_params, + receive_monero_address, ) .await } @@ -213,6 +218,7 @@ async fn run_until_internal( monero_wallet, swap_id, execution_params, + receive_monero_address, ) .await } @@ -255,6 +261,7 @@ async fn run_until_internal( monero_wallet, swap_id, execution_params, + receive_monero_address, ) .await } @@ -290,6 +297,7 @@ async fn run_until_internal( monero_wallet, swap_id, execution_params, + receive_monero_address, ) .await } @@ -297,6 +305,11 @@ async fn run_until_internal( // Bob redeems XMR using revealed s_a state.claim_xmr(monero_wallet.as_ref()).await?; + // Ensure that the generated wallet is synced so we have a proper balance + monero_wallet.refresh().await?; + // Sweep (transfer all funds) to the given address + monero_wallet.sweep_all(receive_monero_address).await?; + let state = BobState::XmrRedeemed { tx_lock_id: state.tx_lock_id(), }; @@ -311,6 +324,7 @@ async fn run_until_internal( monero_wallet, swap_id, execution_params, + receive_monero_address, ) .await } @@ -336,6 +350,7 @@ async fn run_until_internal( monero_wallet, swap_id, execution_params, + receive_monero_address, ) .await } @@ -367,6 +382,7 @@ async fn run_until_internal( monero_wallet, swap_id, execution_params, + receive_monero_address, ) .await } diff --git a/swap/tests/testutils/mod.rs b/swap/tests/testutils/mod.rs index a3981850..e41e124b 100644 --- a/swap/tests/testutils/mod.rs +++ b/swap/tests/testutils/mod.rs @@ -22,6 +22,7 @@ use swap::{ execution_params, execution_params::{ExecutionParams, GetExecutionParams}, monero, + monero::OpenWallet, protocol::{alice, alice::AliceState, bob, bob::BobState}, seed::Seed, }; @@ -56,15 +57,18 @@ struct BobParams { } impl BobParams { - pub fn builder(&self, event_loop_handle: bob::EventLoopHandle) -> bob::Builder { - bob::Builder::new( + pub async fn builder(&self, event_loop_handle: bob::EventLoopHandle) -> Result { + let receive_address = self.monero_wallet.get_main_address().await?; + + Ok(bob::Builder::new( Database::open(&self.db_path.clone().as_path()).unwrap(), self.swap_id, self.bitcoin_wallet.clone(), self.monero_wallet.clone(), self.execution_params, event_loop_handle, - ) + receive_address, + )) } pub fn new_eventloop(&self) -> Result<(bob::EventLoop, bob::EventLoopHandle)> { @@ -109,6 +113,8 @@ impl TestContext { let swap = self .bob_params .builder(event_loop_handle) + .await + .unwrap() .with_init_params(self.btc_amount) .build() .unwrap(); @@ -126,7 +132,13 @@ impl TestContext { let (event_loop, event_loop_handle) = self.bob_params.new_eventloop().unwrap(); - let swap = self.bob_params.builder(event_loop_handle).build().unwrap(); + let swap = self + .bob_params + .builder(event_loop_handle) + .await + .unwrap() + .build() + .unwrap(); let join_handle = tokio::spawn(event_loop.run()); @@ -239,13 +251,15 @@ impl TestContext { self.bob_starting_balances.btc - self.btc_amount - lock_tx_bitcoin_fee ); + // unload the generated wallet by opening the original wallet + self.bob_monero_wallet.open().await.unwrap(); + // refresh the original wallet to make sure the balance is caught up + self.bob_monero_wallet.refresh().await.unwrap(); + // Ensure that Bob's balance is refreshed as we use a newly created wallet self.bob_monero_wallet.as_ref().refresh().await.unwrap(); let xmr_balance_after_swap = self.bob_monero_wallet.as_ref().get_balance().await.unwrap(); - assert_eq!( - xmr_balance_after_swap, - self.bob_starting_balances.xmr + self.xmr_amount - ); + assert!(xmr_balance_after_swap > self.bob_starting_balances.xmr); } pub async fn assert_bob_refunded(&self, state: BobState) { @@ -685,18 +699,20 @@ pub fn init_tracing() -> DefaultGuard { let global_filter = tracing::Level::WARN; let swap_filter = tracing::Level::DEBUG; let xmr_btc_filter = tracing::Level::DEBUG; - let monero_harness_filter = tracing::Level::INFO; + let monero_rpc_filter = tracing::Level::DEBUG; + let monero_harness_filter = tracing::Level::DEBUG; let bitcoin_harness_filter = tracing::Level::INFO; let testcontainers_filter = tracing::Level::DEBUG; use tracing_subscriber::util::SubscriberInitExt as _; tracing_subscriber::fmt() .with_env_filter(format!( - "{},swap={},xmr_btc={},monero_harness={},bitcoin_harness={},testcontainers={}", + "{},swap={},xmr_btc={},monero_harness={},monero_rpc={},bitcoin_harness={},testcontainers={}", global_filter, swap_filter, xmr_btc_filter, monero_harness_filter, + monero_rpc_filter, bitcoin_harness_filter, testcontainers_filter )) From 1041212a603f631492527b86674ac5a1cf1959a3 Mon Sep 17 00:00:00 2001 From: Daniel Karzel Date: Wed, 3 Mar 2021 15:30:58 +1100 Subject: [PATCH 07/14] Work in review comments --- swap/src/bin/asb.rs | 15 ++--------- swap/src/bin/swap_cli.rs | 17 ++---------- swap/src/monero.rs | 12 ++++----- swap/src/monero/wallet.rs | 46 +++++++++++++++++++-------------- swap/src/protocol/alice/swap.rs | 8 ++---- swap/src/protocol/bob/state.rs | 4 +-- 6 files changed, 41 insertions(+), 61 deletions(-) diff --git a/swap/src/bin/asb.rs b/swap/src/bin/asb.rs index 26721e77..5faf0029 100644 --- a/swap/src/bin/asb.rs +++ b/swap/src/bin/asb.rs @@ -32,7 +32,7 @@ use swap::{ execution_params::GetExecutionParams, fs::default_config_path, monero, - monero::{Amount, CreateWallet, OpenWallet}, + monero::{Amount, OpenOrCreate}, protocol::alice::EventLoop, seed::Seed, trace::init_tracing, @@ -165,18 +165,7 @@ async fn init_wallets( ); // Setup the Monero wallet - let open_wallet_response = monero_wallet.open().await; - if open_wallet_response.is_err() { - monero_wallet.create().await.context(format!( - "Unable to create Monero wallet.\ - Please ensure that the monero-wallet-rpc is available at {}", - config.monero.wallet_rpc_url - ))?; - - info!("Created Monero wallet {}", DEFAULT_WALLET_NAME); - } else { - info!("Opened Monero wallet {}", DEFAULT_WALLET_NAME); - } + monero_wallet.open_or_create().await?; let balance = monero_wallet.get_balance().await?; if balance == Amount::ZERO { diff --git a/swap/src/bin/swap_cli.rs b/swap/src/bin/swap_cli.rs index ec932482..9e4ba0bc 100644 --- a/swap/src/bin/swap_cli.rs +++ b/swap/src/bin/swap_cli.rs @@ -28,7 +28,7 @@ use swap::{ execution_params, execution_params::GetExecutionParams, monero, - monero::{CreateWallet, OpenWallet}, + monero::OpenOrCreate, protocol::{ bob, bob::{cancel::CancelError, Builder, EventLoop}, @@ -296,20 +296,7 @@ async fn init_monero_wallet( MONERO_BLOCKCHAIN_MONITORING_WALLET_NAME.to_string(), ); - // Setup the temporary Monero wallet necessary for monitoring the blockchain - let open_monitoring_wallet_response = monero_wallet.open().await; - if open_monitoring_wallet_response.is_err() { - monero_wallet.create().await.context(format!( - "Unable to create Monero wallet for blockchain monitoring.\ - Please ensure that the monero-wallet-rpc is available at {}", - monero_wallet_rpc_url - ))?; - - debug!( - "Created Monero wallet for blockchain monitoring with name {}", - MONERO_BLOCKCHAIN_MONITORING_WALLET_NAME - ); - } + monero_wallet.open_or_create().await?; let _test_wallet_connection = monero_wallet .block_height() diff --git a/swap/src/monero.rs b/swap/src/monero.rs index c68a84a4..e1da64e2 100644 --- a/swap/src/monero.rs +++ b/swap/src/monero.rs @@ -218,8 +218,8 @@ pub struct BalanceTooLow { } #[async_trait] -pub trait CreateWalletForOutput { - async fn create_and_load_wallet_for_output( +pub trait CreateFromAndLoad { + async fn create_from_and_load( &self, private_spend_key: PrivateKey, private_view_key: PrivateViewKey, @@ -228,8 +228,8 @@ pub trait CreateWalletForOutput { } #[async_trait] -pub trait CreateWalletForOutputThenReloadWallet { - async fn create_and_load_wallet_for_output_then_reload_wallet( +pub trait CreateFrom { + async fn create_from( &self, private_spend_key: PrivateKey, private_view_key: PrivateViewKey, @@ -243,8 +243,8 @@ pub trait OpenWallet { } #[async_trait] -pub trait CreateWallet { - async fn create(&self) -> Result<()>; +pub trait OpenOrCreate { + async fn open_or_create(&self) -> Result<()>; } #[async_trait] diff --git a/swap/src/monero/wallet.rs b/swap/src/monero/wallet.rs index 8fa0d98b..38b88c1f 100644 --- a/swap/src/monero/wallet.rs +++ b/swap/src/monero/wallet.rs @@ -1,10 +1,9 @@ use crate::monero::{ - Amount, CreateWallet, CreateWalletForOutput, CreateWalletForOutputThenReloadWallet, - InsufficientFunds, OpenWallet, PrivateViewKey, PublicViewKey, Transfer, TransferProof, TxHash, - WatchForTransfer, + Amount, CreateFrom, CreateFromAndLoad, InsufficientFunds, OpenOrCreate, OpenWallet, + PrivateViewKey, PublicViewKey, Transfer, TransferProof, TxHash, WatchForTransfer, }; use ::monero::{Address, Network, PrivateKey, PublicKey}; -use anyhow::Result; +use anyhow::{Context, Result}; use async_trait::async_trait; use backoff::{backoff::Constant as ConstantBackoff, future::retry}; use bitcoin::hashes::core::sync::atomic::AtomicU32; @@ -14,7 +13,7 @@ use monero_rpc::{ }; use std::{str::FromStr, sync::atomic::Ordering, time::Duration}; use tokio::sync::Mutex; -use tracing::info; +use tracing::{debug, info}; use url::Url; #[derive(Debug)] @@ -62,12 +61,15 @@ impl Wallet { } pub async fn sweep_all(&self, address: Address) -> Result> { - self.inner + let sweep_all = self + .inner .lock() .await .sweep_all(address.to_string().as_str()) - .await - .map(|sweep_all| sweep_all.tx_hash_list.into_iter().map(TxHash).collect()) + .await?; + + let tx_hashes = sweep_all.tx_hash_list.into_iter().map(TxHash).collect(); + Ok(tx_hashes) } pub fn static_tx_fee_estimate(&self) -> Amount { @@ -109,8 +111,8 @@ impl Transfer for Wallet { } #[async_trait] -impl CreateWalletForOutput for Wallet { - async fn create_and_load_wallet_for_output( +impl CreateFromAndLoad for Wallet { + async fn create_from_and_load( &self, private_spend_key: PrivateKey, private_view_key: PrivateViewKey, @@ -141,8 +143,8 @@ impl CreateWalletForOutput for Wallet { } #[async_trait] -impl CreateWalletForOutputThenReloadWallet for Wallet { - async fn create_and_load_wallet_for_output_then_reload_wallet( +impl CreateFrom for Wallet { + async fn create_from( &self, private_spend_key: PrivateKey, private_view_key: PrivateViewKey, @@ -187,13 +189,19 @@ impl OpenWallet for Wallet { } #[async_trait] -impl CreateWallet for Wallet { - async fn create(&self) -> Result<()> { - self.inner - .lock() - .await - .create_wallet(self.name.as_str()) - .await?; +impl OpenOrCreate for Wallet { + 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(()) } } diff --git a/swap/src/protocol/alice/swap.rs b/swap/src/protocol/alice/swap.rs index 4b13dff1..3cd2f45b 100644 --- a/swap/src/protocol/alice/swap.rs +++ b/swap/src/protocol/alice/swap.rs @@ -7,7 +7,7 @@ use crate::{ database::Database, execution_params::ExecutionParams, monero, - monero::CreateWalletForOutputThenReloadWallet, + monero::CreateFrom, monero_ext::ScalarExt, protocol::{ alice, @@ -392,11 +392,7 @@ async fn run_until_internal( let view_key = state3.v; monero_wallet - .create_and_load_wallet_for_output_then_reload_wallet( - spend_key, - view_key, - monero_wallet_restore_blockheight, - ) + .create_from(spend_key, view_key, monero_wallet_restore_blockheight) .await?; let state = AliceState::XmrRefunded; diff --git a/swap/src/protocol/bob/state.rs b/swap/src/protocol/bob/state.rs index f7e562ac..766c1c78 100644 --- a/swap/src/protocol/bob/state.rs +++ b/swap/src/protocol/bob/state.rs @@ -574,7 +574,7 @@ pub struct State5 { impl State5 { pub async fn claim_xmr(&self, monero_wallet: &W) -> Result<()> where - W: monero::CreateWalletForOutput, + W: monero::CreateFromAndLoad, { let s_b = monero::PrivateKey { scalar: self.s_b }; @@ -583,7 +583,7 @@ impl State5 { // NOTE: This actually generates and opens a new wallet, closing the currently // open one. monero_wallet - .create_and_load_wallet_for_output(s, self.v, self.monero_wallet_restore_blockheight) + .create_from_and_load(s, self.v, self.monero_wallet_restore_blockheight) .await?; Ok(()) From d63790c2a6fe3fe203b3a24f44cfa62794ea608d Mon Sep 17 00:00:00 2001 From: Daniel Karzel Date: Wed, 3 Mar 2021 15:42:26 +1100 Subject: [PATCH 08/14] Remove unnecessary monero wallet trait abstractions --- swap/src/bin/asb.rs | 2 +- swap/src/bin/swap_cli.rs | 1 - swap/src/monero.rs | 69 -------------------------------- swap/src/monero/wallet.rs | 36 ++++------------- swap/src/protocol/alice/steps.rs | 10 ++--- swap/src/protocol/alice/swap.rs | 1 - swap/src/protocol/bob/state.rs | 14 ++----- swap/tests/testutils/mod.rs | 1 - 8 files changed, 15 insertions(+), 119 deletions(-) diff --git a/swap/src/bin/asb.rs b/swap/src/bin/asb.rs index 5faf0029..4512db99 100644 --- a/swap/src/bin/asb.rs +++ b/swap/src/bin/asb.rs @@ -32,7 +32,7 @@ use swap::{ execution_params::GetExecutionParams, fs::default_config_path, monero, - monero::{Amount, OpenOrCreate}, + monero::Amount, protocol::alice::EventLoop, seed::Seed, trace::init_tracing, diff --git a/swap/src/bin/swap_cli.rs b/swap/src/bin/swap_cli.rs index 9e4ba0bc..36f1326a 100644 --- a/swap/src/bin/swap_cli.rs +++ b/swap/src/bin/swap_cli.rs @@ -28,7 +28,6 @@ use swap::{ execution_params, execution_params::GetExecutionParams, monero, - monero::OpenOrCreate, protocol::{ bob, bob::{cancel::CancelError, Builder, EventLoop}, diff --git a/swap/src/monero.rs b/swap/src/monero.rs index e1da64e2..613e2095 100644 --- a/swap/src/monero.rs +++ b/swap/src/monero.rs @@ -9,8 +9,6 @@ pub use wallet_rpc::{WalletRpc, WalletRpcProcess}; use crate::bitcoin; use ::bitcoin::hashes::core::fmt::Formatter; use anyhow::Result; -use async_trait::async_trait; -use monero_rpc::wallet::{BlockHeight, Refreshed}; use rand::{CryptoRng, RngCore}; use rust_decimal::{ prelude::{FromPrimitive, ToPrimitive}, @@ -182,28 +180,6 @@ impl From for String { } } -#[async_trait] -pub trait Transfer { - async fn transfer( - &self, - public_spend_key: PublicKey, - public_view_key: PublicViewKey, - amount: Amount, - ) -> 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>; -} - #[derive(Debug, Clone, Copy, thiserror::Error)] #[error("transaction does not pay enough: expected {expected}, got {actual}")] pub struct InsufficientFunds { @@ -217,51 +193,6 @@ pub struct BalanceTooLow { pub balance: Amount, } -#[async_trait] -pub trait CreateFromAndLoad { - async fn create_from_and_load( - &self, - private_spend_key: PrivateKey, - private_view_key: PrivateViewKey, - restore_height: BlockHeight, - ) -> Result<()>; -} - -#[async_trait] -pub trait CreateFrom { - async fn create_from( - &self, - private_spend_key: PrivateKey, - private_view_key: PrivateViewKey, - restore_height: BlockHeight, - ) -> Result<()>; -} - -#[async_trait] -pub trait OpenWallet { - async fn open(&self) -> Result<()>; -} - -#[async_trait] -pub trait OpenOrCreate { - async fn open_or_create(&self) -> Result<()>; -} - -#[async_trait] -pub trait WalletBlockHeight { - async fn block_height(&self) -> Result; -} - -#[async_trait] -pub trait GetAddress { - async fn get_main_address(&self) -> Result
; -} - -#[async_trait] -pub trait Refresh { - async fn refresh(&self) -> Result; -} - #[derive(thiserror::Error, Debug, Clone, PartialEq)] #[error("Overflow, cannot convert {0} to u64")] pub struct OverflowError(pub String); diff --git a/swap/src/monero/wallet.rs b/swap/src/monero/wallet.rs index 38b88c1f..09743f6c 100644 --- a/swap/src/monero/wallet.rs +++ b/swap/src/monero/wallet.rs @@ -1,10 +1,8 @@ use crate::monero::{ - Amount, CreateFrom, CreateFromAndLoad, InsufficientFunds, OpenOrCreate, OpenWallet, - PrivateViewKey, PublicViewKey, Transfer, TransferProof, TxHash, WatchForTransfer, + Amount, InsufficientFunds, PrivateViewKey, PublicViewKey, TransferProof, TxHash, }; use ::monero::{Address, Network, PrivateKey, PublicKey}; use anyhow::{Context, Result}; -use async_trait::async_trait; use backoff::{backoff::Constant as ConstantBackoff, future::retry}; use bitcoin::hashes::core::sync::atomic::AtomicU32; use monero_rpc::{ @@ -76,11 +74,8 @@ impl Wallet { // Median tx fees on Monero as found here: https://www.monero.how/monero-transaction-fees, 0.000_015 * 2 (to be on the safe side) Amount::from_monero(0.000_03f64).expect("static fee to be convertible without problems") } -} -#[async_trait] -impl Transfer for Wallet { - async fn transfer( + pub async fn transfer( &self, public_spend_key: PublicKey, public_view_key: PublicViewKey, @@ -108,11 +103,8 @@ impl Transfer for Wallet { PrivateKey::from_str(&res.tx_key)?, )) } -} -#[async_trait] -impl CreateFromAndLoad for Wallet { - async fn create_from_and_load( + pub async fn create_from_and_load( &self, private_spend_key: PrivateKey, private_view_key: PrivateViewKey, @@ -140,11 +132,8 @@ impl CreateFromAndLoad for Wallet { Ok(()) } -} -#[async_trait] -impl CreateFrom for Wallet { - async fn create_from( + pub async fn create_from( &self, private_spend_key: PrivateKey, private_view_key: PrivateViewKey, @@ -174,11 +163,8 @@ impl CreateFrom for Wallet { Ok(()) } -} -#[async_trait] -impl OpenWallet for Wallet { - async fn open(&self) -> Result<()> { + pub async fn open(&self) -> Result<()> { self.inner .lock() .await @@ -186,11 +172,8 @@ impl OpenWallet for Wallet { .await?; Ok(()) } -} -#[async_trait] -impl OpenOrCreate for Wallet { - async fn open_or_create(&self) -> Result<()> { + 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( @@ -204,13 +187,8 @@ impl OpenOrCreate for Wallet { Ok(()) } -} -// TODO: For retry, use `backoff::ExponentialBackoff` in production as opposed -// to `ConstantBackoff`. -#[async_trait] -impl WatchForTransfer for Wallet { - async fn watch_for_transfer( + pub async fn watch_for_transfer( &self, public_spend_key: PublicKey, public_view_key: PublicViewKey, diff --git a/swap/src/protocol/alice/steps.rs b/swap/src/protocol/alice/steps.rs index e65567d1..7fb0e57d 100644 --- a/swap/src/protocol/alice/steps.rs +++ b/swap/src/protocol/alice/steps.rs @@ -6,7 +6,6 @@ use crate::{ }, execution_params::ExecutionParams, monero, - monero::Transfer, protocol::{ alice, alice::{event_loop::EventLoopHandle, TransferProof}, @@ -49,15 +48,12 @@ pub async fn wait_for_locked_bitcoin( Ok(()) } -pub async fn lock_xmr( +pub async fn lock_xmr( bob_peer_id: PeerId, state3: alice::State3, event_loop_handle: &mut EventLoopHandle, - monero_wallet: Arc, -) -> Result<()> -where - W: Transfer, -{ + monero_wallet: Arc, +) -> Result<()> { let S_a = monero::PublicKey::from_private_key(&monero::PrivateKey { scalar: state3.s_a }); let public_spend_key = S_a + state3.S_b_monero; diff --git a/swap/src/protocol/alice/swap.rs b/swap/src/protocol/alice/swap.rs index 3cd2f45b..f5182227 100644 --- a/swap/src/protocol/alice/swap.rs +++ b/swap/src/protocol/alice/swap.rs @@ -7,7 +7,6 @@ use crate::{ database::Database, execution_params::ExecutionParams, monero, - monero::CreateFrom, monero_ext::ScalarExt, protocol::{ alice, diff --git a/swap/src/protocol/bob/state.rs b/swap/src/protocol/bob/state.rs index 766c1c78..3f407f0f 100644 --- a/swap/src/protocol/bob/state.rs +++ b/swap/src/protocol/bob/state.rs @@ -316,15 +316,12 @@ pub struct State3 { } impl State3 { - pub async fn watch_for_lock_xmr( + pub async fn watch_for_lock_xmr( self, - xmr_wallet: &W, + xmr_wallet: &monero::Wallet, transfer_proof: TransferProof, monero_wallet_restore_blockheight: BlockHeight, - ) -> Result> - where - W: monero::WatchForTransfer, - { + ) -> Result> { let S_b_monero = monero::PublicKey::from_private_key(&monero::PrivateKey::from_scalar(self.s_b)); let S = self.S_a_monero + S_b_monero; @@ -572,10 +569,7 @@ pub struct State5 { } impl State5 { - pub async fn claim_xmr(&self, monero_wallet: &W) -> Result<()> - where - W: monero::CreateFromAndLoad, - { + pub async fn claim_xmr(&self, monero_wallet: &monero::Wallet) -> Result<()> { let s_b = monero::PrivateKey { scalar: self.s_b }; let s = self.s_a + s_b; diff --git a/swap/tests/testutils/mod.rs b/swap/tests/testutils/mod.rs index e41e124b..509123f7 100644 --- a/swap/tests/testutils/mod.rs +++ b/swap/tests/testutils/mod.rs @@ -22,7 +22,6 @@ use swap::{ execution_params, execution_params::{ExecutionParams, GetExecutionParams}, monero, - monero::OpenWallet, protocol::{alice, alice::AliceState, bob, bob::BobState}, seed::Seed, }; From b17e6cbd945e4836c75111bb5f2c09387b3b005f Mon Sep 17 00:00:00 2001 From: Daniel Karzel Date: Wed, 3 Mar 2021 16:32:36 +1100 Subject: [PATCH 09/14] Reorder: Move open to top --- swap/src/monero/wallet.rs | 48 +++++++++++++++++++-------------------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/swap/src/monero/wallet.rs b/swap/src/monero/wallet.rs index 09743f6c..b390657a 100644 --- a/swap/src/monero/wallet.rs +++ b/swap/src/monero/wallet.rs @@ -38,6 +38,30 @@ impl Wallet { } } + pub async fn open(&self) -> Result<()> { + self.inner + .lock() + .await + .open_wallet(self.name.as_str()) + .await?; + 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(()) + } + /// Get the balance of the primary account. pub async fn get_balance(&self) -> Result { let amount = self.inner.lock().await.get_balance(0).await?; @@ -164,30 +188,6 @@ impl Wallet { Ok(()) } - pub async fn open(&self) -> Result<()> { - self.inner - .lock() - .await - .open_wallet(self.name.as_str()) - .await?; - 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(()) - } - pub async fn watch_for_transfer( &self, public_spend_key: PublicKey, From 5a43b3453e389945d72409078ddd409a3362f92a Mon Sep 17 00:00:00 2001 From: Daniel Karzel Date: Wed, 3 Mar 2021 16:33:39 +1100 Subject: [PATCH 10/14] Reorder: Move create after open --- swap/src/monero/wallet.rs | 120 +++++++++++++++++++------------------- 1 file changed, 60 insertions(+), 60 deletions(-) diff --git a/swap/src/monero/wallet.rs b/swap/src/monero/wallet.rs index b390657a..243e81fd 100644 --- a/swap/src/monero/wallet.rs +++ b/swap/src/monero/wallet.rs @@ -62,6 +62,66 @@ impl Wallet { Ok(()) } + pub async fn create_from_and_load( + &self, + private_spend_key: PrivateKey, + private_view_key: PrivateViewKey, + restore_height: BlockHeight, + ) -> Result<()> { + 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 wallet = self.inner.lock().await; + + // Properly close the 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(), + &private_spend_key.to_string(), + &PrivateKey::from(private_view_key).to_string(), + restore_height.height, + ) + .await?; + + Ok(()) + } + + pub async fn create_from( + &self, + private_spend_key: PrivateKey, + private_view_key: PrivateViewKey, + restore_height: BlockHeight, + ) -> Result<()> { + 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 wallet = self.inner.lock().await; + + // Properly close the 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(), + &private_spend_key.to_string(), + &PrivateKey::from(private_view_key).to_string(), + restore_height.height, + ) + .await?; + + let _ = wallet.open_wallet(self.name.as_str()).await?; + + Ok(()) + } + /// Get the balance of the primary account. pub async fn get_balance(&self) -> Result { let amount = self.inner.lock().await.get_balance(0).await?; @@ -128,66 +188,6 @@ impl Wallet { )) } - pub async fn create_from_and_load( - &self, - private_spend_key: PrivateKey, - private_view_key: PrivateViewKey, - restore_height: BlockHeight, - ) -> Result<()> { - 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 wallet = self.inner.lock().await; - - // Properly close the 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(), - &private_spend_key.to_string(), - &PrivateKey::from(private_view_key).to_string(), - restore_height.height, - ) - .await?; - - Ok(()) - } - - pub async fn create_from( - &self, - private_spend_key: PrivateKey, - private_view_key: PrivateViewKey, - restore_height: BlockHeight, - ) -> Result<()> { - 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 wallet = self.inner.lock().await; - - // Properly close the 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(), - &private_spend_key.to_string(), - &PrivateKey::from(private_view_key).to_string(), - restore_height.height, - ) - .await?; - - let _ = wallet.open_wallet(self.name.as_str()).await?; - - Ok(()) - } - pub async fn watch_for_transfer( &self, public_spend_key: PublicKey, From 862c29f1a8108f0f1419dc50157958086b0d8506 Mon Sep 17 00:00:00 2001 From: Daniel Karzel Date: Wed, 3 Mar 2021 16:34:23 +1100 Subject: [PATCH 11/14] Reorder: Move sweep_all after transfer --- swap/src/monero/wallet.rs | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/swap/src/monero/wallet.rs b/swap/src/monero/wallet.rs index 243e81fd..4383b599 100644 --- a/swap/src/monero/wallet.rs +++ b/swap/src/monero/wallet.rs @@ -142,18 +142,6 @@ impl Wallet { self.inner.lock().await.refresh().await } - pub async fn sweep_all(&self, address: Address) -> Result> { - let sweep_all = self - .inner - .lock() - .await - .sweep_all(address.to_string().as_str()) - .await?; - - let tx_hashes = sweep_all.tx_hash_list.into_iter().map(TxHash).collect(); - Ok(tx_hashes) - } - pub fn static_tx_fee_estimate(&self) -> Amount { // Median tx fees on Monero as found here: https://www.monero.how/monero-transaction-fees, 0.000_015 * 2 (to be on the safe side) Amount::from_monero(0.000_03f64).expect("static fee to be convertible without problems") @@ -251,4 +239,16 @@ impl Wallet { Ok(()) } + + pub async fn sweep_all(&self, address: Address) -> Result> { + let sweep_all = self + .inner + .lock() + .await + .sweep_all(address.to_string().as_str()) + .await?; + + let tx_hashes = sweep_all.tx_hash_list.into_iter().map(TxHash).collect(); + Ok(tx_hashes) + } } From 5d807e9647e482f2ab43d0c1cc83fa0ac4e388e1 Mon Sep 17 00:00:00 2001 From: Daniel Karzel Date: Wed, 3 Mar 2021 16:35:13 +1100 Subject: [PATCH 12/14] Reorder: Move utility functionality to bottom --- swap/src/monero/wallet.rs | 50 +++++++++++++++++++-------------------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/swap/src/monero/wallet.rs b/swap/src/monero/wallet.rs index 4383b599..04fe14dd 100644 --- a/swap/src/monero/wallet.rs +++ b/swap/src/monero/wallet.rs @@ -122,31 +122,6 @@ impl Wallet { Ok(()) } - /// Get the balance of the primary account. - pub async fn get_balance(&self) -> Result { - let amount = self.inner.lock().await.get_balance(0).await?; - - Ok(Amount::from_piconero(amount)) - } - - pub async fn block_height(&self) -> Result { - 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 async fn refresh(&self) -> Result { - self.inner.lock().await.refresh().await - } - - pub fn static_tx_fee_estimate(&self) -> Amount { - // Median tx fees on Monero as found here: https://www.monero.how/monero-transaction-fees, 0.000_015 * 2 (to be on the safe side) - Amount::from_monero(0.000_03f64).expect("static fee to be convertible without problems") - } - pub async fn transfer( &self, public_spend_key: PublicKey, @@ -251,4 +226,29 @@ impl Wallet { let tx_hashes = sweep_all.tx_hash_list.into_iter().map(TxHash).collect(); Ok(tx_hashes) } + + /// Get the balance of the primary account. + pub async fn get_balance(&self) -> Result { + let amount = self.inner.lock().await.get_balance(0).await?; + + Ok(Amount::from_piconero(amount)) + } + + pub async fn block_height(&self) -> Result { + 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 async fn refresh(&self) -> Result { + self.inner.lock().await.refresh().await + } + + pub fn static_tx_fee_estimate(&self) -> Amount { + // Median tx fees on Monero as found here: https://www.monero.how/monero-transaction-fees, 0.000_015 * 2 (to be on the safe side) + Amount::from_monero(0.000_03f64).expect("static fee to be convertible without problems") + } } From 0b4c0758bb571c851e92af26513b5dcd6d75e52a Mon Sep 17 00:00:00 2001 From: Daniel Karzel Date: Wed, 3 Mar 2021 15:56:58 +1100 Subject: [PATCH 13/14] Turbofish syntax This way, we are co-locating the type with the expression that requires it. This makes it easier to take this expression and compose it with other ones without having to touch the type hint on the variable. Co-authored-by: Thomas Eizinger --- monero-rpc/src/rpc/wallet.rs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/monero-rpc/src/rpc/wallet.rs b/monero-rpc/src/rpc/wallet.rs index 46804221..bd44cede 100644 --- a/monero-rpc/src/rpc/wallet.rs +++ b/monero-rpc/src/rpc/wallet.rs @@ -44,7 +44,7 @@ impl Client { debug!("get address RPC response: {}", response); - let r: Response = serde_json::from_str(&response)?; + let r = serde_json::from_str::>(&response)?; Ok(r.result) } @@ -69,9 +69,9 @@ impl Client { index, response ); - let res: Response = serde_json::from_str(&response)?; + let r = serde_json::from_str::>(&response)?; - let balance = res.result.balance; + let balance = r.result.balance; Ok(balance) } @@ -93,7 +93,7 @@ impl Client { debug!("create account RPC response: {}", response); - let r: Response = serde_json::from_str(&response)?; + let r = serde_json::from_str::>(&response)?; Ok(r.result) } @@ -115,7 +115,7 @@ impl Client { debug!("get accounts RPC response: {}", response); - let r: Response = serde_json::from_str(&response)?; + let r = serde_json::from_str::>(&response)?; Ok(r.result) } @@ -233,7 +233,7 @@ impl Client { debug!("transfer RPC response: {}", response); - let r: Response = serde_json::from_str(&response)?; + let r = serde_json::from_str::>(&response)?; Ok(r.result) } @@ -252,7 +252,7 @@ impl Client { debug!("wallet height RPC response: {}", response); - let r: Response = serde_json::from_str(&response)?; + let r = serde_json::from_str::>(&response)?; Ok(r.result) } @@ -281,7 +281,7 @@ impl Client { debug!("transfer RPC response: {}", response); - let r: Response = serde_json::from_str(&response)?; + let r = serde_json::from_str::>(&response)?; Ok(r.result) } @@ -314,7 +314,7 @@ impl Client { debug!("generate_from_keys RPC response: {}", response); - let r: Response = serde_json::from_str(&response)?; + let r = serde_json::from_str::>(&response)?; Ok(r.result) } @@ -332,7 +332,7 @@ impl Client { debug!("refresh RPC response: {}", response); - let r: Response = serde_json::from_str(&response)?; + let r = serde_json::from_str::>(&response)?; Ok(r.result) } @@ -354,7 +354,7 @@ impl Client { debug!("sweep_all RPC response: {}", response); - let r: Response = serde_json::from_str(&response)?; + let r = serde_json::from_str::>(&response)?; Ok(r.result) } } From 2e3c2d8edfd0704cc9083319fe03145d5f273112 Mon Sep 17 00:00:00 2001 From: Daniel Karzel Date: Wed, 3 Mar 2021 16:42:05 +1100 Subject: [PATCH 14/14] Remove Arcs in favour of references --- monero-rpc/src/rpc/wallet.rs | 2 +- swap/src/protocol/alice/steps.rs | 8 +++----- swap/src/protocol/alice/swap.rs | 4 ++-- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/monero-rpc/src/rpc/wallet.rs b/monero-rpc/src/rpc/wallet.rs index bd44cede..db106cbc 100644 --- a/monero-rpc/src/rpc/wallet.rs +++ b/monero-rpc/src/rpc/wallet.rs @@ -147,7 +147,7 @@ impl Client { Ok(()) } - /// Opens a wallet using `filename`. + /// Close the currently opened wallet, after trying to save it. pub async fn close_wallet(&self) -> Result<()> { let request = Request::new("close_wallet", ""); diff --git a/swap/src/protocol/alice/steps.rs b/swap/src/protocol/alice/steps.rs index 7fb0e57d..aab80944 100644 --- a/swap/src/protocol/alice/steps.rs +++ b/swap/src/protocol/alice/steps.rs @@ -22,7 +22,6 @@ use futures::{ }; use libp2p::PeerId; use sha2::Sha256; -use std::sync::Arc; use tokio::time::timeout; // TODO(Franck): Use helper functions from xmr-btc instead of re-writing them @@ -52,7 +51,7 @@ pub async fn lock_xmr( bob_peer_id: PeerId, state3: alice::State3, event_loop_handle: &mut EventLoopHandle, - monero_wallet: Arc, + monero_wallet: &monero::Wallet, ) -> Result<()> { let S_a = monero::PublicKey::from_private_key(&monero::PrivateKey { scalar: state3.s_a }); @@ -126,14 +125,13 @@ pub async fn publish_cancel_transaction( B: bitcoin::PublicKey, cancel_timelock: CancelTimelock, tx_cancel_sig_bob: bitcoin::Signature, - bitcoin_wallet: Arc, + bitcoin_wallet: &bitcoin::Wallet, ) -> Result { // First wait for cancel timelock to expire let tx_lock_height = bitcoin_wallet .transaction_block_height(tx_lock.txid()) .await?; - poll_until_block_height_is_gte(bitcoin_wallet.as_ref(), tx_lock_height + cancel_timelock) - .await?; + poll_until_block_height_is_gte(&bitcoin_wallet, tx_lock_height + cancel_timelock).await?; let tx_cancel = bitcoin::TxCancel::new(&tx_lock, cancel_timelock, a.public(), B); diff --git a/swap/src/protocol/alice/swap.rs b/swap/src/protocol/alice/swap.rs index f5182227..c9bd7341 100644 --- a/swap/src/protocol/alice/swap.rs +++ b/swap/src/protocol/alice/swap.rs @@ -130,7 +130,7 @@ async fn run_until_internal( bob_peer_id, *state3.clone(), &mut event_loop_handle, - monero_wallet.clone(), + &monero_wallet, ) .await?; @@ -286,7 +286,7 @@ async fn run_until_internal( state3.B, state3.cancel_timelock, state3.tx_cancel_sig_bob.clone(), - bitcoin_wallet.clone(), + &bitcoin_wallet, ) .await?;