From aed8358fb7662c765073d3bd2bb6d6f79310fa2e Mon Sep 17 00:00:00 2001 From: Daniel Karzel Date: Wed, 24 Feb 2021 14:00:17 +1100 Subject: [PATCH 1/8] Remove dead code --- Cargo.lock | 137 +++++++++++++++++++++ swap/src/protocol/alice/state.rs | 198 +------------------------------ 2 files changed, 139 insertions(+), 196 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 844cecb7..bd5667d0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -614,6 +614,22 @@ version = "0.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "245097e9a4535ee1e3e3931fcfcd55a796a44c643e8596ff6566d68f09b87bbc" +[[package]] +name = "core-foundation" +version = "0.9.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0a89e2ae426ea83155dccf10c0fa6b1463ef6d5fcb44cee0b224a408fa640a62" +dependencies = [ + "core-foundation-sys", + "libc", +] + +[[package]] +name = "core-foundation-sys" +version = "0.8.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ea221b5284a47e40033bf9b66f35f984ec0ea2931eb03505246cd27a963f981b" + [[package]] name = "cpuid-bool" version = "0.1.2" @@ -994,6 +1010,21 @@ version = "1.0.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3f9eec918d3f24069decb9af1554cad7c880e2da24a9afd88aca000531ab82c1" +[[package]] +name = "foreign-types" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f6f339eb8adc052cd2ca78910fda869aefa38d22d5cb648e6485e4d3fc06f3b1" +dependencies = [ + "foreign-types-shared", +] + +[[package]] +name = "foreign-types-shared" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "00b0228411908ca8685dba7fc2cdd70ec9990a6e753e89b6ac91a84c40fbaf4b" + [[package]] name = "form_urlencoded" version = "1.0.0" @@ -1984,6 +2015,24 @@ dependencies = [ "unsigned-varint 0.6.0", ] +[[package]] +name = "native-tls" +version = "0.2.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b8d96b2e1c8da3957d58100b09f102c6d9cfdfced01b7ec5a8974044bb09dbd4" +dependencies = [ + "lazy_static", + "libc", + "log", + "openssl", + "openssl-probe", + "openssl-sys", + "schannel", + "security-framework", + "security-framework-sys", + "tempfile", +] + [[package]] name = "nb-connect" version = "1.0.2" @@ -2126,6 +2175,39 @@ version = "0.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "624a8340c38c1b80fd549087862da4ba43e08858af025b236e509b6649fc13d5" +[[package]] +name = "openssl" +version = "0.10.32" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "038d43985d1ddca7a9900630d8cd031b56e4794eecc2e9ea39dd17aa04399a70" +dependencies = [ + "bitflags", + "cfg-if 1.0.0", + "foreign-types", + "lazy_static", + "libc", + "openssl-sys", +] + +[[package]] +name = "openssl-probe" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "77af24da69f9d9341038eba93a073b1fdaaa1b788221b00a69bce9e762cb32de" + +[[package]] +name = "openssl-sys" +version = "0.9.60" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "921fc71883267538946025deffb622905ecad223c28efbfdef9bb59a0175f3e6" +dependencies = [ + "autocfg 1.0.1", + "cc", + "libc", + "pkg-config", + "vcpkg", +] + [[package]] name = "parity-multiaddr" version = "0.11.0" @@ -2266,6 +2348,12 @@ version = "0.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8b870d8c151b6f2fb93e84a13146138f05d02ed11c7e7c54f8826aaaf7c9f184" +[[package]] +name = "pkg-config" +version = "0.3.19" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3831453b3449ceb48b6d9c7ad7c96d5ea673e9b470a1dc578c2ce6521230884c" + [[package]] name = "polling" version = "2.0.2" @@ -2870,6 +2958,16 @@ version = "1.0.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "71d301d4193d031abdd79ff7e3dd721168a9572ef3fe51a1517aba235bd8f86e" +[[package]] +name = "schannel" +version = "0.1.19" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8f05ba609c234e60bee0d547fe94a4c7e9da733d1c962cf6e59efa4cd9c8bc75" +dependencies = [ + "lazy_static", + "winapi 0.3.9", +] + [[package]] name = "scopeguard" version = "1.1.0" @@ -2928,6 +3026,29 @@ dependencies = [ "subtle 2.4.0", ] +[[package]] +name = "security-framework" +version = "2.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c1759c2e3c8580017a484a7ac56d3abc5a6c1feadf88db2f3633f12ae4268c69" +dependencies = [ + "bitflags", + "core-foundation", + "core-foundation-sys", + "libc", + "security-framework-sys", +] + +[[package]] +name = "security-framework-sys" +version = "2.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f99b9d5e26d2a71633cc4f2ebae7cc9f874044e0c351a27e17892d76dce5678b" +dependencies = [ + "core-foundation-sys", + "libc", +] + [[package]] name = "semver" version = "0.9.0" @@ -3581,6 +3702,16 @@ dependencies = [ "syn", ] +[[package]] +name = "tokio-native-tls" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f7d995660bd2b7f8c1568414c1126076c13fbb725c40112dc0120b78eb9b717b" +dependencies = [ + "native-tls", + "tokio", +] + [[package]] name = "tokio-stream" version = "0.1.2" @@ -3863,6 +3994,12 @@ dependencies = [ "serde", ] +[[package]] +name = "vcpkg" +version = "0.2.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b00bca6106a5e23f3eee943593759b7fcddb00554332e856d990c893966879fb" + [[package]] name = "vec-arena" version = "1.0.0" diff --git a/swap/src/protocol/alice/state.rs b/swap/src/protocol/alice/state.rs index 9538e84e..35fd8d1f 100644 --- a/swap/src/protocol/alice/state.rs +++ b/swap/src/protocol/alice/state.rs @@ -7,22 +7,16 @@ use crate::{ }, execution_params::ExecutionParams, monero, - monero_ext::ScalarExt, protocol::{ - alice::{Message1, Message3, TransferProof}, - bob::{EncryptedSignature, Message0, Message2, Message4}, + alice::{Message1, Message3}, + bob::{Message0, Message2, Message4}, CROSS_CURVE_PROOF_SYSTEM, }, }; use anyhow::{anyhow, bail, Context, Result}; -use ecdsa_fun::{ - adaptor::{Adaptor, HashTranscript}, - nonce::Deterministic, -}; use libp2p::PeerId; use rand::{CryptoRng, RngCore}; use serde::{Deserialize, Serialize}; -use sha2::Sha256; use sigma_fun::ext::dl_secp256k1_ed25519_eq::CrossCurveDLEQProof; use std::fmt; @@ -349,191 +343,3 @@ impl State3 { .await } } - -#[derive(Clone, Debug, Deserialize, Serialize)] -pub struct State4 { - a: bitcoin::SecretKey, - B: bitcoin::PublicKey, - s_a: monero::Scalar, - S_b_monero: monero::PublicKey, - S_b_bitcoin: bitcoin::PublicKey, - v: monero::PrivateViewKey, - xmr: monero::Amount, - cancel_timelock: CancelTimelock, - punish_timelock: PunishTimelock, - refund_address: bitcoin::Address, - redeem_address: bitcoin::Address, - punish_address: bitcoin::Address, - tx_lock: bitcoin::TxLock, - tx_punish_sig_bob: bitcoin::Signature, - tx_cancel_sig_bob: bitcoin::Signature, -} - -impl State4 { - pub async fn lock_xmr(self, monero_wallet: &W) -> Result - where - W: monero::Transfer, - { - let S_a = monero::PublicKey::from_private_key(&monero::PrivateKey { scalar: self.s_a }); - let S_b = self.S_b_monero; - - let (tx_lock_proof, fee) = monero_wallet - .transfer(S_a + S_b, self.v.public(), self.xmr) - .await?; - - Ok(State5 { - a: self.a, - B: self.B, - s_a: self.s_a, - S_b_monero: self.S_b_monero, - S_b_bitcoin: self.S_b_bitcoin, - v: self.v, - cancel_timelock: self.cancel_timelock, - punish_timelock: self.punish_timelock, - refund_address: self.refund_address, - 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, - }) - } - - pub async fn punish( - &self, - bitcoin_wallet: &W, - ) -> Result<()> { - let tx_cancel = - bitcoin::TxCancel::new(&self.tx_lock, self.cancel_timelock, self.a.public(), self.B); - let tx_punish = - bitcoin::TxPunish::new(&tx_cancel, &self.punish_address, self.punish_timelock); - - { - let sig_a = self.a.sign(tx_cancel.digest()); - let sig_b = self.tx_cancel_sig_bob.clone(); - - let signed_tx_cancel = tx_cancel.clone().add_signatures( - &self.tx_lock, - (self.a.public(), sig_a), - (self.B, sig_b), - )?; - - let _ = bitcoin_wallet - .broadcast_signed_transaction(signed_tx_cancel) - .await?; - } - - { - let sig_a = self.a.sign(tx_punish.digest()); - let sig_b = self.tx_punish_sig_bob.clone(); - - let signed_tx_punish = - tx_punish.add_signatures(&tx_cancel, (self.a.public(), sig_a), (self.B, sig_b))?; - - let _ = bitcoin_wallet - .broadcast_signed_transaction(signed_tx_punish) - .await?; - } - - Ok(()) - } -} - -#[derive(Clone, Debug, Deserialize, Serialize)] -pub struct State5 { - a: bitcoin::SecretKey, - B: bitcoin::PublicKey, - s_a: monero::Scalar, - S_b_monero: monero::PublicKey, - S_b_bitcoin: bitcoin::PublicKey, - v: monero::PrivateViewKey, - cancel_timelock: CancelTimelock, - punish_timelock: PunishTimelock, - refund_address: bitcoin::Address, - redeem_address: bitcoin::Address, - punish_address: bitcoin::Address, - tx_lock: bitcoin::TxLock, - tx_lock_proof: monero::TransferProof, - - tx_punish_sig_bob: bitcoin::Signature, - - tx_cancel_sig_bob: bitcoin::Signature, - lock_xmr_fee: monero::Amount, -} - -impl State5 { - pub fn next_message(&self) -> TransferProof { - TransferProof { - tx_lock_proof: self.tx_lock_proof.clone(), - } - } - - pub fn receive(self, msg: EncryptedSignature) -> State6 { - State6 { - a: self.a, - B: self.B, - s_a: self.s_a, - S_b_monero: self.S_b_monero, - S_b_bitcoin: self.S_b_bitcoin, - v: self.v, - cancel_timelock: self.cancel_timelock, - punish_timelock: self.punish_timelock, - refund_address: self.refund_address, - redeem_address: self.redeem_address, - punish_address: self.punish_address, - tx_lock: self.tx_lock, - tx_punish_sig_bob: self.tx_punish_sig_bob, - tx_redeem_encsig: msg.tx_redeem_encsig, - lock_xmr_fee: self.lock_xmr_fee, - } - } -} - -#[derive(Clone, Debug, Deserialize, Serialize)] -pub struct State6 { - a: bitcoin::SecretKey, - B: bitcoin::PublicKey, - s_a: monero::Scalar, - S_b_monero: monero::PublicKey, - S_b_bitcoin: bitcoin::PublicKey, - v: monero::PrivateViewKey, - cancel_timelock: CancelTimelock, - punish_timelock: PunishTimelock, - refund_address: bitcoin::Address, - redeem_address: bitcoin::Address, - punish_address: bitcoin::Address, - tx_lock: bitcoin::TxLock, - - tx_punish_sig_bob: bitcoin::Signature, - tx_redeem_encsig: bitcoin::EncryptedSignature, - lock_xmr_fee: monero::Amount, -} - -impl State6 { - pub async fn redeem_btc( - &self, - bitcoin_wallet: &W, - ) -> Result<()> { - let adaptor = Adaptor::, Deterministic>::default(); - - let tx_redeem = bitcoin::TxRedeem::new(&self.tx_lock, &self.redeem_address); - - let sig_a = self.a.sign(tx_redeem.digest()); - let sig_b = - adaptor.decrypt_signature(&self.s_a.to_secpfun_scalar(), self.tx_redeem_encsig.clone()); - - let sig_tx_redeem = - tx_redeem.add_signatures(&self.tx_lock, (self.a.public(), sig_a), (self.B, sig_b))?; - bitcoin_wallet - .broadcast_signed_transaction(sig_tx_redeem) - .await?; - - Ok(()) - } - - pub fn lock_xmr_fee(&self) -> monero::Amount { - self.lock_xmr_fee - } -} From 9f1deb9fdc1755b699c4d77da2f4ab492abebd36 Mon Sep 17 00:00:00 2001 From: Daniel Karzel Date: Wed, 24 Feb 2021 16:34:04 +1100 Subject: [PATCH 2/8] Wrap the Monero wallet client in a Mutex In order to ensure that we can atomically generate_from_keys and then reload a wallet, we have to wrap the client of the monero wallet RPC inside a mutex. When introducing the Mutex I noticed that several inner RPC calls were leaking to the swap crate monero wallet. As this is a violation of boundaries I introduced the traits `GetAddress`, `WalletBlockHeight` and `Refresh`. Note that the monero wallet could potentially know its own public view key and public spend key. If we refactor the wallet to include this information upon wallet creation we can also generate addresses using `monero::Address::standard`. --- swap/src/bin/asb.rs | 4 +-- swap/src/bin/swap_cli.rs | 4 +-- swap/src/monero.rs | 17 +++++++++++ swap/src/monero/wallet.rs | 53 ++++++++++++++++++++++++++++------- swap/src/protocol/bob/swap.rs | 5 ++-- swap/tests/testutils/mod.rs | 23 ++++++--------- 6 files changed, 75 insertions(+), 31 deletions(-) diff --git a/swap/src/bin/asb.rs b/swap/src/bin/asb.rs index 1f06003a..38cc88da 100644 --- a/swap/src/bin/asb.rs +++ b/swap/src/bin/asb.rs @@ -31,7 +31,7 @@ use swap::{ execution_params::GetExecutionParams, fs::default_config_path, monero, - monero::{Amount, CreateWallet, OpenWallet}, + monero::{Amount, CreateWallet, GetAddress, OpenWallet}, protocol::alice::EventLoop, seed::Seed, trace::init_tracing, @@ -177,7 +177,7 @@ async fn init_wallets( let balance = monero_wallet.get_balance().await?; if balance == Amount::ZERO { - let deposit_address = monero_wallet.inner.get_address(0).await?.address; + let deposit_address = monero_wallet.get_main_address().await?; warn!( "The Monero balance is 0, make sure to deposit funds at: {}", deposit_address diff --git a/swap/src/bin/swap_cli.rs b/swap/src/bin/swap_cli.rs index affe443d..f17d224e 100644 --- a/swap/src/bin/swap_cli.rs +++ b/swap/src/bin/swap_cli.rs @@ -30,7 +30,7 @@ use swap::{ execution_params::GetExecutionParams, fs::default_config_path, monero, - monero::{CreateWallet, OpenWallet}, + monero::{CreateWallet, OpenWallet, WalletBlockHeight}, protocol::{ bob, bob::{cancel::CancelError, Builder}, @@ -316,7 +316,7 @@ async fn init_wallets( ); } - let _test_wallet_connection = monero_wallet.inner.block_height().await?; + let _test_wallet_connection = monero_wallet.block_height().await?; info!("The Monero wallet RPC is set up correctly!"); Ok((bitcoin_wallet, monero_wallet)) diff --git a/swap/src/monero.rs b/swap/src/monero.rs index a01c68c3..469de5d4 100644 --- a/swap/src/monero.rs +++ b/swap/src/monero.rs @@ -8,6 +8,8 @@ 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::{ prelude::{FromPrimitive, ToPrimitive}, @@ -228,6 +230,21 @@ pub trait CreateWallet { async fn create_wallet(&self, file_name: &str) -> 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 ce1d1a97..f3a061c9 100644 --- a/swap/src/monero/wallet.rs +++ b/swap/src/monero/wallet.rs @@ -1,38 +1,43 @@ use crate::monero::{ - Amount, CreateWallet, CreateWalletForOutput, InsufficientFunds, OpenWallet, PrivateViewKey, - PublicViewKey, Transfer, TransferProof, TxHash, WatchForTransfer, + Amount, CreateWallet, CreateWalletForOutput, GetAddress, InsufficientFunds, OpenWallet, + PrivateViewKey, PublicViewKey, Refresh, Transfer, TransferProof, TxHash, WalletBlockHeight, + WatchForTransfer, }; use ::monero::{Address, Network, PrivateKey, PublicKey}; use anyhow::Result; use async_trait::async_trait; use backoff::{backoff::Constant as ConstantBackoff, future::retry}; use bitcoin::hashes::core::sync::atomic::AtomicU32; -use monero_rpc::wallet; +use monero_rpc::{ + wallet, + wallet::{BlockHeight, Refreshed}, +}; use std::{ str::FromStr, sync::{atomic::Ordering, Arc}, time::Duration, }; +use tokio::sync::Mutex; use tracing::info; use url::Url; #[derive(Debug)] pub struct Wallet { - pub inner: wallet::Client, + pub inner: Mutex, pub network: Network, } impl Wallet { pub fn new(url: Url, network: Network) -> Self { Self { - inner: wallet::Client::new(url), + inner: Mutex::new(wallet::Client::new(url)), network, } } /// Get the balance of the primary account. pub async fn get_balance(&self) -> Result { - let amount = self.inner.get_balance(0).await?; + let amount = self.inner.lock().await.get_balance(0).await?; Ok(Amount::from_piconero(amount)) } @@ -51,6 +56,8 @@ impl Transfer for Wallet { let res = self .inner + .lock() + .await .transfer(0, amount.as_piconero(), &destination_address.to_string()) .await?; @@ -82,6 +89,8 @@ impl CreateWalletForOutput for Wallet { let _ = self .inner + .lock() + .await .generate_from_keys( &address.to_string(), &private_spend_key.to_string(), @@ -97,7 +106,7 @@ impl CreateWalletForOutput for Wallet { #[async_trait] impl OpenWallet for Wallet { async fn open_wallet(&self, file_name: &str) -> Result<()> { - self.inner.open_wallet(file_name).await?; + self.inner.lock().await.open_wallet(file_name).await?; Ok(()) } } @@ -105,7 +114,7 @@ impl OpenWallet for Wallet { #[async_trait] impl CreateWallet for Wallet { async fn create_wallet(&self, file_name: &str) -> Result<()> { - self.inner.create_wallet(file_name).await?; + self.inner.lock().await.create_wallet(file_name).await?; Ok(()) } } @@ -129,7 +138,6 @@ impl WatchForTransfer for Wallet { } let address = Address::standard(self.network, public_spend_key, public_view_key.into()); - let wallet = self.inner.clone(); let confirmations = Arc::new(AtomicU32::new(0u32)); @@ -137,7 +145,10 @@ impl WatchForTransfer for Wallet { // NOTE: Currently, this is conflicting 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 = wallet + let proof = self + .inner + .lock() + .await .check_tx_key( &String::from(transfer_proof.tx_hash()), &transfer_proof.tx_key().to_string(), @@ -176,3 +187,25 @@ impl WatchForTransfer for Wallet { Ok(()) } } + +#[async_trait] +impl WalletBlockHeight for Wallet { + async fn block_height(&self) -> Result { + self.inner.lock().await.block_height().await + } +} + +#[async_trait] +impl GetAddress for Wallet { + 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())?) + } +} + +#[async_trait] +impl Refresh for Wallet { + async fn refresh(&self) -> Result { + self.inner.lock().await.refresh().await + } +} diff --git a/swap/src/protocol/bob/swap.rs b/swap/src/protocol/bob/swap.rs index 72686335..a6b75316 100644 --- a/swap/src/protocol/bob/swap.rs +++ b/swap/src/protocol/bob/swap.rs @@ -4,7 +4,7 @@ use crate::{ database::{Database, Swap}, execution_params::ExecutionParams, monero, - monero::InsufficientFunds, + monero::{InsufficientFunds, WalletBlockHeight}, protocol::bob::{self, event_loop::EventLoopHandle, state::*, QuoteRequest}, }; use anyhow::{bail, Result}; @@ -132,8 +132,7 @@ async fn run_until_internal( // TODO: This can be optimized further by extracting the block height when // tx-lock was included. However, scanning a few more blocks won't do any harm // and is simpler. - let monero_wallet_restore_blockheight = - monero_wallet.inner.block_height().await?; + let monero_wallet_restore_blockheight = monero_wallet.block_height().await?; select! { transfer_proof = transfer_proof_watcher => { diff --git a/swap/tests/testutils/mod.rs b/swap/tests/testutils/mod.rs index 392d1ba8..0d4fc750 100644 --- a/swap/tests/testutils/mod.rs +++ b/swap/tests/testutils/mod.rs @@ -21,12 +21,17 @@ use swap::{ execution_params, execution_params::{ExecutionParams, GetExecutionParams}, monero, + monero::Refresh, protocol::{alice, alice::AliceState, bob, bob::BobState}, seed::Seed, }; use tempfile::tempdir; use testcontainers::{clients::Cli, Container, Docker, RunArgs}; -use tokio::{sync::mpsc, task::JoinHandle, time::interval}; +use tokio::{ + sync::{mpsc, Mutex}, + task::JoinHandle, + time::interval, +}; use tracing::dispatcher::DefaultGuard; use tracing_log::LogTracer; use url::Url; @@ -168,12 +173,7 @@ impl TestContext { assert_eq!(btc_balance_after_swap, self.alice_starting_balances.btc); // Ensure that Alice's balance is refreshed as we use a newly created wallet - self.alice_monero_wallet - .as_ref() - .inner - .refresh() - .await - .unwrap(); + self.alice_monero_wallet.as_ref().refresh().await.unwrap(); let xmr_balance_after_swap = self .alice_monero_wallet .as_ref() @@ -232,12 +232,7 @@ impl TestContext { ); // Ensure that Bob's balance is refreshed as we use a newly created wallet - self.bob_monero_wallet - .as_ref() - .inner - .refresh() - .await - .unwrap(); + 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, @@ -595,7 +590,7 @@ async fn init_test_wallets( .unwrap(); let xmr_wallet = swap::monero::Wallet { - inner: monero.wallet(name).unwrap().client(), + inner: Mutex::new(monero.wallet(name).unwrap().client()), network: monero::Network::default(), }; From 1404057dbe68b60594f1f663d664c659fed6f694 Mon Sep 17 00:00:00 2001 From: Daniel Karzel Date: Wed, 24 Feb 2021 17:01:24 +1100 Subject: [PATCH 3/8] Remove misleading TODO This TDOO is misleading, because - to our current knowledge - it is impossible for Bob to retrieve the exact inclusion block-height of the lock transaction (send by Alice). The wallet RPC is only capable of retrieving the inclusion block height of a transaction through `get_payments` and `get_bulk_payments` which requires the `payment_id`. The `payment_id` can be retrieved through `get_transfer_by_txid` which states "Show information about a transfer to/from this address." - however the address that the transfer goes to is not part of Bob's wallet yet! Thus, it is impossible for Bob to use `get_transfer_by_txid` which in turn means Bob is unable to use `get_payments`. The only possible way for Bob to know the exact inclusion block/height of the lock transaction would be if Alice sends it over to Bob. But for that Alice would have to extract it she would have to wait for confirmation - which she currently does not and might never do. Even if she does await the first confirmation before sending the transfer proof the solution for retrieving the inclusion block-height is not fleshed out on her side yet. --- swap/src/protocol/bob/swap.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/swap/src/protocol/bob/swap.rs b/swap/src/protocol/bob/swap.rs index a6b75316..628462cc 100644 --- a/swap/src/protocol/bob/swap.rs +++ b/swap/src/protocol/bob/swap.rs @@ -129,9 +129,6 @@ async fn run_until_internal( // Record the current monero wallet block height so we don't have to scan from // block 0 once we create the redeem wallet. - // TODO: This can be optimized further by extracting the block height when - // tx-lock was included. However, scanning a few more blocks won't do any harm - // and is simpler. let monero_wallet_restore_blockheight = monero_wallet.block_height().await?; select! { From fa047751888547f0ae43b7efbb9ae8ba37f53a43 Mon Sep 17 00:00:00 2001 From: Daniel Karzel Date: Wed, 24 Feb 2021 17:36:37 +1100 Subject: [PATCH 4/8] Rename function explicit to cancellation to cancel For transitioning to state4 we either go into a redeem or a cancellation scenario. The function name state4 is misleading, because it is only used for cancellation scenarios. --- swap/src/protocol/bob/cancel.rs | 4 ++-- swap/src/protocol/bob/refund.rs | 4 ++-- swap/src/protocol/bob/state.rs | 2 +- swap/src/protocol/bob/swap.rs | 10 +++++----- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/swap/src/protocol/bob/cancel.rs b/swap/src/protocol/bob/cancel.rs index ccce6361..dab0fc79 100644 --- a/swap/src/protocol/bob/cancel.rs +++ b/swap/src/protocol/bob/cancel.rs @@ -23,8 +23,8 @@ pub async fn cancel( force: bool, ) -> Result> { let state4 = match state { - BobState::BtcLocked(state3) => state3.state4(), - BobState::XmrLockProofReceived { state, .. } => state.state4(), + BobState::BtcLocked(state3) => state3.cancel(), + BobState::XmrLockProofReceived { state, .. } => state.cancel(), BobState::XmrLocked(state4) => state4, BobState::EncSigSent(state4) => state4, BobState::CancelTimelockExpired(state4) => state4, diff --git a/swap/src/protocol/bob/refund.rs b/swap/src/protocol/bob/refund.rs index b341dad2..8536eed0 100644 --- a/swap/src/protocol/bob/refund.rs +++ b/swap/src/protocol/bob/refund.rs @@ -22,8 +22,8 @@ pub async fn refund( ) -> Result> { let state4 = if force { match state { - BobState::BtcLocked(state3) => state3.state4(), - BobState::XmrLockProofReceived { state, .. } => state.state4(), + BobState::BtcLocked(state3) => state3.cancel(), + BobState::XmrLockProofReceived { state, .. } => state.cancel(), BobState::XmrLocked(state4) => state4, BobState::EncSigSent(state4) => state4, BobState::CancelTimelockExpired(state4) => state4, diff --git a/swap/src/protocol/bob/state.rs b/swap/src/protocol/bob/state.rs index 0d6349e7..91930cdf 100644 --- a/swap/src/protocol/bob/state.rs +++ b/swap/src/protocol/bob/state.rs @@ -379,7 +379,7 @@ impl State3 { .await } - pub fn state4(&self) -> State4 { + pub fn cancel(&self) -> State4 { State4 { A: self.A, b: self.b.clone(), diff --git a/swap/src/protocol/bob/swap.rs b/swap/src/protocol/bob/swap.rs index 628462cc..ba91dbfa 100644 --- a/swap/src/protocol/bob/swap.rs +++ b/swap/src/protocol/bob/swap.rs @@ -141,12 +141,12 @@ async fn run_until_internal( } }, _ = cancel_timelock_expires => { - let state4 = state3.state4(); + let state4 = state3.cancel(); BobState::CancelTimelockExpired(state4) } } } else { - let state4 = state3.state4(); + let state4 = state3.cancel(); BobState::CancelTimelockExpired(state4) }; let db_state = state.clone().into(); @@ -188,18 +188,18 @@ async fn run_until_internal( Err(InsufficientFunds {..}) => { info!("The other party has locked insufficient Monero funds! Waiting for refund..."); state.wait_for_cancel_timelock_to_expire(bitcoin_wallet.as_ref()).await?; - let state4 = state.state4(); + let state4 = state.cancel(); BobState::CancelTimelockExpired(state4) }, } }, _ = cancel_timelock_expires => { - let state4 = state.state4(); + let state4 = state.cancel(); BobState::CancelTimelockExpired(state4) } } } else { - let state4 = state.state4(); + let state4 = state.cancel(); BobState::CancelTimelockExpired(state4) }; From 684cbe4d0b71193602fa968c5e89c00d3acbe035 Mon Sep 17 00:00:00 2001 From: Daniel Karzel Date: Wed, 24 Feb 2021 17:38:35 +1100 Subject: [PATCH 5/8] Remember monero wallet-height for Alice's refund scenario --- monero-rpc/src/rpc/wallet.rs | 8 +- swap/src/database/alice.rs | 121 +++++++++++++++++++++++-------- swap/src/monero.rs | 2 +- swap/src/monero/wallet.rs | 4 +- swap/src/protocol/alice/state.rs | 7 ++ swap/src/protocol/alice/swap.rs | 91 +++++++++++++++++++---- swap/src/protocol/bob/state.rs | 16 ++-- swap/src/protocol/bob/swap.rs | 2 +- 8 files changed, 186 insertions(+), 65 deletions(-) diff --git a/monero-rpc/src/rpc/wallet.rs b/monero-rpc/src/rpc/wallet.rs index cd92588c..67298b61 100644 --- a/monero-rpc/src/rpc/wallet.rs +++ b/monero-rpc/src/rpc/wallet.rs @@ -268,14 +268,8 @@ impl Client { address: &str, spend_key: &str, view_key: &str, - restore_height: Option, + restore_height: u32, ) -> Result { - let restore_height = if let Some(restore_height) = restore_height { - restore_height - } else { - 0 - }; - let params = GenerateFromKeysParams { restore_height, filename: view_key.into(), diff --git a/swap/src/database/alice.rs b/swap/src/database/alice.rs index 22b90afe..61a785af 100644 --- a/swap/src/database/alice.rs +++ b/swap/src/database/alice.rs @@ -6,6 +6,7 @@ use crate::{ }; use ::bitcoin::hashes::core::fmt::Display; use libp2p::PeerId; +use monero_rpc::wallet::BlockHeight; use serde::{Deserialize, Serialize}; // Large enum variant is fine because this is only used for database @@ -23,15 +24,29 @@ pub enum Alice { #[serde(with = "crate::serde_peer_id")] bob_peer_id: PeerId, }, - XmrLocked(alice::State3), + XmrLocked { + monero_wallet_restore_blockheight: BlockHeight, + state3: alice::State3, + }, EncSigLearned { + monero_wallet_restore_blockheight: BlockHeight, encrypted_signature: EncryptedSignature, state3: alice::State3, }, - CancelTimelockExpired(alice::State3), - BtcCancelled(alice::State3), - BtcPunishable(alice::State3), + CancelTimelockExpired { + monero_wallet_restore_blockheight: BlockHeight, + state3: alice::State3, + }, + BtcCancelled { + monero_wallet_restore_blockheight: BlockHeight, + state3: alice::State3, + }, + BtcPunishable { + monero_wallet_restore_blockheight: BlockHeight, + state3: alice::State3, + }, BtcRefunded { + monero_wallet_restore_blockheight: BlockHeight, state3: alice::State3, #[serde(with = "monero_private_key")] spend_key: monero::PrivateKey, @@ -53,7 +68,6 @@ impl From<&AliceState> for Alice { AliceState::Started { state3, bob_peer_id, - .. } => Alice::Started { state3: state3.as_ref().clone(), bob_peer_id: *bob_peer_id, @@ -61,32 +75,60 @@ impl From<&AliceState> for Alice { AliceState::BtcLocked { state3, bob_peer_id, - .. } => Alice::BtcLocked { state3: state3.as_ref().clone(), bob_peer_id: *bob_peer_id, }, - AliceState::XmrLocked { state3 } => Alice::XmrLocked(state3.as_ref().clone()), + AliceState::XmrLocked { + monero_wallet_restore_blockheight, + state3, + } => Alice::XmrLocked { + monero_wallet_restore_blockheight: *monero_wallet_restore_blockheight, + state3: state3.as_ref().clone(), + }, AliceState::EncSigLearned { + monero_wallet_restore_blockheight, state3, encrypted_signature, } => Alice::EncSigLearned { + monero_wallet_restore_blockheight: *monero_wallet_restore_blockheight, state3: state3.as_ref().clone(), encrypted_signature: *encrypted_signature.clone(), }, AliceState::BtcRedeemed => Alice::Done(AliceEndState::BtcRedeemed), - AliceState::BtcCancelled { state3, .. } => Alice::BtcCancelled(state3.as_ref().clone()), - AliceState::BtcRefunded { spend_key, state3 } => Alice::BtcRefunded { + AliceState::BtcCancelled { + monero_wallet_restore_blockheight, + state3, + .. + } => Alice::BtcCancelled { + monero_wallet_restore_blockheight: *monero_wallet_restore_blockheight, + state3: state3.as_ref().clone(), + }, + AliceState::BtcRefunded { + monero_wallet_restore_blockheight, + spend_key, + state3, + } => Alice::BtcRefunded { + monero_wallet_restore_blockheight: *monero_wallet_restore_blockheight, spend_key: *spend_key, state3: state3.as_ref().clone(), }, - AliceState::BtcPunishable { state3, .. } => { - Alice::BtcPunishable(state3.as_ref().clone()) - } + AliceState::BtcPunishable { + monero_wallet_restore_blockheight, + state3, + .. + } => Alice::BtcPunishable { + monero_wallet_restore_blockheight: *monero_wallet_restore_blockheight, + state3: state3.as_ref().clone(), + }, AliceState::XmrRefunded => Alice::Done(AliceEndState::XmrRefunded), - AliceState::CancelTimelockExpired { state3 } => { - Alice::CancelTimelockExpired(state3.as_ref().clone()) - } + AliceState::CancelTimelockExpired { + monero_wallet_restore_blockheight, + state3, + } => Alice::CancelTimelockExpired { + monero_wallet_restore_blockheight: *monero_wallet_restore_blockheight, + state3: state3.as_ref().clone(), + }, AliceState::BtcPunished => Alice::Done(AliceEndState::BtcPunished), AliceState::SafelyAborted => Alice::Done(AliceEndState::SafelyAborted), } @@ -110,33 +152,50 @@ impl From for AliceState { bob_peer_id, state3: Box::new(state3), }, - Alice::XmrLocked(state3) => AliceState::XmrLocked { + Alice::XmrLocked { + monero_wallet_restore_blockheight, + state3, + } => AliceState::XmrLocked { + monero_wallet_restore_blockheight, state3: Box::new(state3), }, Alice::EncSigLearned { + monero_wallet_restore_blockheight, state3: state, encrypted_signature, } => AliceState::EncSigLearned { + monero_wallet_restore_blockheight, state3: Box::new(state), encrypted_signature: Box::new(encrypted_signature), }, - Alice::CancelTimelockExpired(state3) => AliceState::CancelTimelockExpired { + Alice::CancelTimelockExpired { + monero_wallet_restore_blockheight, + state3, + } => AliceState::CancelTimelockExpired { + monero_wallet_restore_blockheight, state3: Box::new(state3), }, - Alice::BtcCancelled(state) => { + Alice::BtcCancelled { + monero_wallet_restore_blockheight, + state3, + } => { let tx_cancel = TxCancel::new( - &state.tx_lock, - state.cancel_timelock, - state.a.public(), - state.B, + &state3.tx_lock, + state3.cancel_timelock, + state3.a.public(), + state3.B, ); AliceState::BtcCancelled { - state3: Box::new(state), + monero_wallet_restore_blockheight, + state3: Box::new(state3), tx_cancel: Box::new(tx_cancel), } } - Alice::BtcPunishable(state3) => { + Alice::BtcPunishable { + monero_wallet_restore_blockheight, + state3, + } => { let tx_cancel = TxCancel::new( &state3.tx_lock, state3.cancel_timelock, @@ -145,13 +204,17 @@ impl From for AliceState { ); let tx_refund = TxRefund::new(&tx_cancel, &state3.refund_address); AliceState::BtcPunishable { + monero_wallet_restore_blockheight, tx_refund, state3: Box::new(state3), } } Alice::BtcRefunded { - state3, spend_key, .. + monero_wallet_restore_blockheight, + state3, + spend_key, } => AliceState::BtcRefunded { + monero_wallet_restore_blockheight, spend_key, state3: Box::new(state3), }, @@ -170,10 +233,10 @@ impl Display for Alice { match self { Alice::Started { .. } => write!(f, "Started"), Alice::BtcLocked { .. } => f.write_str("Bitcoin locked"), - Alice::XmrLocked(_) => f.write_str("Monero locked"), - Alice::CancelTimelockExpired(_) => f.write_str("Cancel timelock is expired"), - Alice::BtcCancelled(_) => f.write_str("Bitcoin cancel transaction published"), - Alice::BtcPunishable(_) => f.write_str("Bitcoin punishable"), + Alice::XmrLocked { .. } => f.write_str("Monero locked"), + Alice::CancelTimelockExpired { .. } => f.write_str("Cancel timelock is expired"), + Alice::BtcCancelled { .. } => f.write_str("Bitcoin cancel transaction published"), + Alice::BtcPunishable { .. } => f.write_str("Bitcoin punishable"), Alice::BtcRefunded { .. } => f.write_str("Monero refundable"), Alice::Done(end_state) => write!(f, "Done: {}", end_state), Alice::EncSigLearned { .. } => f.write_str("Encrypted signature learned"), diff --git a/swap/src/monero.rs b/swap/src/monero.rs index 469de5d4..ccaffa3b 100644 --- a/swap/src/monero.rs +++ b/swap/src/monero.rs @@ -216,7 +216,7 @@ pub trait CreateWalletForOutput { &self, private_spend_key: PrivateKey, private_view_key: PrivateViewKey, - restore_height: Option, + restore_height: BlockHeight, ) -> Result<()>; } diff --git a/swap/src/monero/wallet.rs b/swap/src/monero/wallet.rs index f3a061c9..1c38de31 100644 --- a/swap/src/monero/wallet.rs +++ b/swap/src/monero/wallet.rs @@ -80,7 +80,7 @@ impl CreateWalletForOutput for Wallet { &self, private_spend_key: PrivateKey, private_view_key: PrivateViewKey, - restore_height: Option, + 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()); @@ -95,7 +95,7 @@ impl CreateWalletForOutput for Wallet { &address.to_string(), &private_spend_key.to_string(), &PrivateKey::from(private_view_key).to_string(), - restore_height, + restore_height.height, ) .await?; diff --git a/swap/src/protocol/alice/state.rs b/swap/src/protocol/alice/state.rs index 35fd8d1f..ade62fe1 100644 --- a/swap/src/protocol/alice/state.rs +++ b/swap/src/protocol/alice/state.rs @@ -15,6 +15,7 @@ use crate::{ }; use anyhow::{anyhow, bail, Context, Result}; use libp2p::PeerId; +use monero_rpc::wallet::BlockHeight; use rand::{CryptoRng, RngCore}; use serde::{Deserialize, Serialize}; use sigma_fun::ext::dl_secp256k1_ed25519_eq::CrossCurveDLEQProof; @@ -31,27 +32,33 @@ pub enum AliceState { state3: Box, }, XmrLocked { + monero_wallet_restore_blockheight: BlockHeight, state3: Box, }, EncSigLearned { + monero_wallet_restore_blockheight: BlockHeight, encrypted_signature: Box, state3: Box, }, BtcRedeemed, BtcCancelled { + monero_wallet_restore_blockheight: BlockHeight, tx_cancel: Box, state3: Box, }, BtcRefunded { + monero_wallet_restore_blockheight: BlockHeight, spend_key: monero::PrivateKey, state3: Box, }, BtcPunishable { + monero_wallet_restore_blockheight: BlockHeight, tx_refund: TxRefund, state3: Box, }, XmrRefunded, CancelTimelockExpired { + monero_wallet_restore_blockheight: BlockHeight, state3: Box, }, BtcPunished, diff --git a/swap/src/protocol/alice/swap.rs b/swap/src/protocol/alice/swap.rs index e24808fe..0f480dfd 100644 --- a/swap/src/protocol/alice/swap.rs +++ b/swap/src/protocol/alice/swap.rs @@ -10,7 +10,7 @@ use crate::{ database::Database, execution_params::ExecutionParams, monero, - monero::CreateWalletForOutput, + monero::{CreateWalletForOutput, WalletBlockHeight}, monero_ext::ScalarExt, protocol::{ alice, @@ -127,6 +127,10 @@ async fn run_until_internal( bob_peer_id, state3, } => { + // Record the current monero wallet block height so we don't have to scan from + // block 0 for scenarios where we create a refund wallet. + let monero_wallet_restore_blockheight = monero_wallet.block_height().await?; + lock_xmr( bob_peer_id, *state3.clone(), @@ -135,7 +139,10 @@ async fn run_until_internal( ) .await?; - let state = AliceState::XmrLocked { state3 }; + let state = AliceState::XmrLocked { + state3, + monero_wallet_restore_blockheight, + }; let db_state = (&state).into(); db.insert_latest_state(swap_id, database::Swap::Alice(db_state)) @@ -152,7 +159,10 @@ async fn run_until_internal( ) .await } - AliceState::XmrLocked { state3 } => { + AliceState::XmrLocked { + state3, + monero_wallet_restore_blockheight, + } => { let state = match state3.expired_timelocks(bitcoin_wallet.as_ref()).await? { ExpiredTimelocks::None => { let wait_for_enc_sig = @@ -165,14 +175,21 @@ async fn run_until_internal( pin_mut!(cancel_timelock_expires); match select(cancel_timelock_expires, wait_for_enc_sig).await { - Either::Left(_) => AliceState::CancelTimelockExpired { state3 }, + Either::Left(_) => AliceState::CancelTimelockExpired { + state3, + monero_wallet_restore_blockheight, + }, Either::Right((enc_sig, _)) => AliceState::EncSigLearned { state3, encrypted_signature: Box::new(enc_sig?), + monero_wallet_restore_blockheight, }, } } - _ => AliceState::CancelTimelockExpired { state3 }, + _ => AliceState::CancelTimelockExpired { + state3, + monero_wallet_restore_blockheight, + }, }; let db_state = (&state).into(); @@ -193,6 +210,7 @@ async fn run_until_internal( AliceState::EncSigLearned { state3, encrypted_signature, + monero_wallet_restore_blockheight, } => { let state = match state3.expired_timelocks(bitcoin_wallet.as_ref()).await? { ExpiredTimelocks::None => { @@ -228,7 +246,10 @@ async fn run_until_internal( ) .await?; - AliceState::CancelTimelockExpired { state3 } + AliceState::CancelTimelockExpired { + state3, + monero_wallet_restore_blockheight, + } } } } @@ -238,11 +259,17 @@ async fn run_until_internal( .wait_for_cancel_timelock_to_expire(bitcoin_wallet.as_ref()) .await?; - AliceState::CancelTimelockExpired { state3 } + AliceState::CancelTimelockExpired { + state3, + monero_wallet_restore_blockheight, + } } } } - _ => AliceState::CancelTimelockExpired { state3 }, + _ => AliceState::CancelTimelockExpired { + state3, + monero_wallet_restore_blockheight, + }, }; let db_state = (&state).into(); @@ -260,7 +287,10 @@ async fn run_until_internal( ) .await } - AliceState::CancelTimelockExpired { state3 } => { + AliceState::CancelTimelockExpired { + state3, + monero_wallet_restore_blockheight, + } => { let tx_cancel = publish_cancel_transaction( state3.tx_lock.clone(), state3.a.clone(), @@ -274,6 +304,7 @@ async fn run_until_internal( let state = AliceState::BtcCancelled { state3, tx_cancel: Box::new(tx_cancel), + monero_wallet_restore_blockheight, }; let db_state = (&state).into(); db.insert_latest_state(swap_id, database::Swap::Alice(db_state)) @@ -290,7 +321,11 @@ async fn run_until_internal( ) .await } - AliceState::BtcCancelled { state3, tx_cancel } => { + AliceState::BtcCancelled { + state3, + tx_cancel, + monero_wallet_restore_blockheight, + } => { let tx_cancel_height = bitcoin_wallet .transaction_block_height(tx_cancel.txid()) .await?; @@ -307,7 +342,11 @@ async fn run_until_internal( // TODO(Franck): Review error handling match published_refund_tx { None => { - let state = AliceState::BtcPunishable { tx_refund, state3 }; + let state = AliceState::BtcPunishable { + tx_refund, + state3, + monero_wallet_restore_blockheight, + }; let db_state = (&state).into(); db.insert_latest_state(swap_id, database::Swap::Alice(db_state)) .await?; @@ -333,7 +372,11 @@ async fn run_until_internal( state3.S_b_bitcoin, )?; - let state = AliceState::BtcRefunded { spend_key, state3 }; + let state = AliceState::BtcRefunded { + spend_key, + state3, + monero_wallet_restore_blockheight, + }; let db_state = (&state).into(); db.insert_latest_state(swap_id, database::Swap::Alice(db_state)) .await?; @@ -351,11 +394,19 @@ async fn run_until_internal( } } } - AliceState::BtcRefunded { spend_key, state3 } => { + AliceState::BtcRefunded { + spend_key, + state3, + monero_wallet_restore_blockheight, + } => { let view_key = state3.v; monero_wallet - .create_and_load_wallet_for_output(spend_key, view_key, None) + .create_and_load_wallet_for_output( + spend_key, + view_key, + monero_wallet_restore_blockheight, + ) .await?; let state = AliceState::XmrRefunded; @@ -364,7 +415,11 @@ async fn run_until_internal( .await?; Ok(state) } - AliceState::BtcPunishable { tx_refund, state3 } => { + AliceState::BtcPunishable { + tx_refund, + state3, + monero_wallet_restore_blockheight, + } => { let signed_tx_punish = build_bitcoin_punish_transaction( &state3.tx_lock, state3.cancel_timelock, @@ -395,7 +450,11 @@ async fn run_until_internal( state3.a.clone(), state3.S_b_bitcoin, )?; - let state = AliceState::BtcRefunded { spend_key, state3 }; + let state = AliceState::BtcRefunded { + spend_key, + state3, + monero_wallet_restore_blockheight, + }; let db_state = (&state).into(); db.insert_latest_state(swap_id, database::Swap::Alice(db_state)) .await?; diff --git a/swap/src/protocol/bob/state.rs b/swap/src/protocol/bob/state.rs index 91930cdf..3387eb35 100644 --- a/swap/src/protocol/bob/state.rs +++ b/swap/src/protocol/bob/state.rs @@ -328,7 +328,7 @@ impl State3 { self, xmr_wallet: &W, transfer_proof: TransferProof, - monero_wallet_restore_blockheight: u32, + monero_wallet_restore_blockheight: BlockHeight, ) -> Result> where W: monero::WatchForTransfer, @@ -393,7 +393,9 @@ impl State3 { tx_lock: self.tx_lock.clone(), tx_cancel_sig_a: self.tx_cancel_sig_a.clone(), tx_refund_encsig: self.tx_refund_encsig.clone(), - monero_wallet_restore_blockheight: 0u32, + // For cancel scenarios the monero wallet rescan blockchain height is irrelevant for + // Bob, because Bob's cancel can only lead to refunding on Bitcoin + monero_wallet_restore_blockheight: BlockHeight { height: 0 }, } } @@ -429,7 +431,7 @@ pub struct State4 { tx_lock: bitcoin::TxLock, tx_cancel_sig_a: Signature, tx_refund_encsig: bitcoin::EncryptedSignature, - monero_wallet_restore_blockheight: u32, + monero_wallet_restore_blockheight: BlockHeight, } impl State4 { @@ -589,7 +591,7 @@ pub struct State5 { s_b: monero::Scalar, v: monero::PrivateViewKey, tx_lock: bitcoin::TxLock, - monero_wallet_restore_blockheight: u32, + monero_wallet_restore_blockheight: BlockHeight, } impl State5 { @@ -604,11 +606,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, - Some(self.monero_wallet_restore_blockheight), - ) + .create_and_load_wallet_for_output(s, self.v, self.monero_wallet_restore_blockheight) .await?; Ok(()) diff --git a/swap/src/protocol/bob/swap.rs b/swap/src/protocol/bob/swap.rs index ba91dbfa..485e8f74 100644 --- a/swap/src/protocol/bob/swap.rs +++ b/swap/src/protocol/bob/swap.rs @@ -176,7 +176,7 @@ async fn run_until_internal( let xmr_lock_watcher = state.clone().watch_for_lock_xmr( monero_wallet.as_ref(), lock_transfer_proof, - monero_wallet_restore_blockheight.height, + monero_wallet_restore_blockheight, ); let cancel_timelock_expires = state.wait_for_cancel_timelock_to_expire(bitcoin_wallet.as_ref()); From 947bcb61927a9342f65eb68d13a642d475dae297 Mon Sep 17 00:00:00 2001 From: Daniel Karzel Date: Wed, 24 Feb 2021 18:00:07 +1100 Subject: [PATCH 6/8] ASB reloads the default wallet after generate_from_keys atomically --- swap/src/bin/asb.rs | 6 ++++- swap/src/bin/swap_cli.rs | 6 ++++- swap/src/monero.rs | 10 ++++++++ swap/src/monero/wallet.rs | 42 +++++++++++++++++++++++++++++---- swap/src/protocol/alice/swap.rs | 4 ++-- swap/tests/testutils/mod.rs | 1 + 6 files changed, 61 insertions(+), 8 deletions(-) diff --git a/swap/src/bin/asb.rs b/swap/src/bin/asb.rs index 38cc88da..d63246fa 100644 --- a/swap/src/bin/asb.rs +++ b/swap/src/bin/asb.rs @@ -156,7 +156,11 @@ async fn init_wallets( bitcoin_balance ); - let monero_wallet = monero::Wallet::new(config.monero.wallet_rpc_url.clone(), MONERO_NETWORK); + let monero_wallet = monero::Wallet::new( + config.monero.wallet_rpc_url.clone(), + MONERO_NETWORK, + DEFAULT_WALLET_NAME.to_string(), + ); // Setup the Monero wallet let open_wallet_response = monero_wallet.open_wallet(DEFAULT_WALLET_NAME).await; diff --git a/swap/src/bin/swap_cli.rs b/swap/src/bin/swap_cli.rs index f17d224e..2efffb3f 100644 --- a/swap/src/bin/swap_cli.rs +++ b/swap/src/bin/swap_cli.rs @@ -289,7 +289,11 @@ async fn init_wallets( bitcoin_balance ); - let monero_wallet = monero::Wallet::new(config.monero.wallet_rpc_url.clone(), monero_network); + let monero_wallet = monero::Wallet::new( + config.monero.wallet_rpc_url.clone(), + monero_network, + MONERO_BLOCKCHAIN_MONITORING_WALLET_NAME.to_string(), + ); // Setup the temporary Monero wallet necessary for monitoring the blockchain let open_monitoring_wallet_response = monero_wallet diff --git a/swap/src/monero.rs b/swap/src/monero.rs index ccaffa3b..e17e137b 100644 --- a/swap/src/monero.rs +++ b/swap/src/monero.rs @@ -220,6 +220,16 @@ pub trait CreateWalletForOutput { ) -> Result<()>; } +#[async_trait] +pub trait CreateWalletForOutputThenLoadDefaultWallet { + async fn create_and_load_wallet_for_output_then_load_default_wallet( + &self, + private_spend_key: PrivateKey, + private_view_key: PrivateViewKey, + restore_height: BlockHeight, + ) -> Result<()>; +} + #[async_trait] pub trait OpenWallet { async fn open_wallet(&self, file_name: &str) -> Result<()>; diff --git a/swap/src/monero/wallet.rs b/swap/src/monero/wallet.rs index 1c38de31..08fbb9c8 100644 --- a/swap/src/monero/wallet.rs +++ b/swap/src/monero/wallet.rs @@ -1,7 +1,7 @@ use crate::monero::{ - Amount, CreateWallet, CreateWalletForOutput, GetAddress, InsufficientFunds, OpenWallet, - PrivateViewKey, PublicViewKey, Refresh, Transfer, TransferProof, TxHash, WalletBlockHeight, - WatchForTransfer, + Amount, CreateWallet, CreateWalletForOutput, CreateWalletForOutputThenLoadDefaultWallet, + GetAddress, InsufficientFunds, OpenWallet, PrivateViewKey, PublicViewKey, Refresh, Transfer, + TransferProof, TxHash, WalletBlockHeight, WatchForTransfer, }; use ::monero::{Address, Network, PrivateKey, PublicKey}; use anyhow::Result; @@ -25,13 +25,15 @@ use url::Url; pub struct Wallet { pub inner: Mutex, pub network: Network, + pub default_wallet_name: String, } impl Wallet { - pub fn new(url: Url, network: Network) -> Self { + pub fn new(url: Url, network: Network, default_wallet_name: String) -> Self { Self { inner: Mutex::new(wallet::Client::new(url)), network, + default_wallet_name, } } @@ -103,6 +105,38 @@ impl CreateWalletForOutput for Wallet { } } +#[async_trait] +impl CreateWalletForOutputThenLoadDefaultWallet for Wallet { + async fn create_and_load_wallet_for_output_then_load_default_wallet( + &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; + + 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.default_wallet_name.as_str()) + .await?; + + Ok(()) + } +} + #[async_trait] impl OpenWallet for Wallet { async fn open_wallet(&self, file_name: &str) -> Result<()> { diff --git a/swap/src/protocol/alice/swap.rs b/swap/src/protocol/alice/swap.rs index 0f480dfd..e233c1e7 100644 --- a/swap/src/protocol/alice/swap.rs +++ b/swap/src/protocol/alice/swap.rs @@ -10,7 +10,7 @@ use crate::{ database::Database, execution_params::ExecutionParams, monero, - monero::{CreateWalletForOutput, WalletBlockHeight}, + monero::{CreateWalletForOutputThenLoadDefaultWallet, WalletBlockHeight}, monero_ext::ScalarExt, protocol::{ alice, @@ -402,7 +402,7 @@ async fn run_until_internal( let view_key = state3.v; monero_wallet - .create_and_load_wallet_for_output( + .create_and_load_wallet_for_output_then_load_default_wallet( spend_key, view_key, monero_wallet_restore_blockheight, diff --git a/swap/tests/testutils/mod.rs b/swap/tests/testutils/mod.rs index 0d4fc750..8688083a 100644 --- a/swap/tests/testutils/mod.rs +++ b/swap/tests/testutils/mod.rs @@ -592,6 +592,7 @@ async fn init_test_wallets( let xmr_wallet = swap::monero::Wallet { inner: Mutex::new(monero.wallet(name).unwrap().client()), network: monero::Network::default(), + default_wallet_name: "irrelevant_for_tests".to_string(), }; let electrum_rpc_url = { From 578d23d7fc9d2f703e96384a2b3e911aaec176a6 Mon Sep 17 00:00:00 2001 From: Daniel Karzel Date: Thu, 25 Feb 2021 10:30:24 +1100 Subject: [PATCH 7/8] Proper encapsulation of wallet boundaries through private fields --- swap/src/monero/wallet.rs | 18 +++++++++++++++--- swap/tests/testutils/mod.rs | 16 ++++++---------- 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/swap/src/monero/wallet.rs b/swap/src/monero/wallet.rs index 08fbb9c8..72226225 100644 --- a/swap/src/monero/wallet.rs +++ b/swap/src/monero/wallet.rs @@ -23,9 +23,9 @@ use url::Url; #[derive(Debug)] pub struct Wallet { - pub inner: Mutex, - pub network: Network, - pub default_wallet_name: String, + inner: Mutex, + network: Network, + default_wallet_name: String, } impl Wallet { @@ -37,6 +37,18 @@ impl Wallet { } } + pub fn new_with_client( + client: wallet::Client, + network: Network, + default_wallet_name: String, + ) -> Self { + Self { + inner: Mutex::new(client), + network, + default_wallet_name, + } + } + /// Get the balance of the primary account. pub async fn get_balance(&self) -> Result { let amount = self.inner.lock().await.get_balance(0).await?; diff --git a/swap/tests/testutils/mod.rs b/swap/tests/testutils/mod.rs index 8688083a..8210f566 100644 --- a/swap/tests/testutils/mod.rs +++ b/swap/tests/testutils/mod.rs @@ -27,11 +27,7 @@ use swap::{ }; use tempfile::tempdir; use testcontainers::{clients::Cli, Container, Docker, RunArgs}; -use tokio::{ - sync::{mpsc, Mutex}, - task::JoinHandle, - time::interval, -}; +use tokio::{sync::mpsc, task::JoinHandle, time::interval}; use tracing::dispatcher::DefaultGuard; use tracing_log::LogTracer; use url::Url; @@ -589,11 +585,11 @@ async fn init_test_wallets( .await .unwrap(); - let xmr_wallet = swap::monero::Wallet { - inner: Mutex::new(monero.wallet(name).unwrap().client()), - network: monero::Network::default(), - default_wallet_name: "irrelevant_for_tests".to_string(), - }; + let xmr_wallet = swap::monero::Wallet::new_with_client( + monero.wallet(name).unwrap().client(), + monero::Network::default(), + "irrelevant_for_tests".to_string(), + ); let electrum_rpc_url = { let input = format!("tcp://@localhost:{}", electrum_rpc_port); From 0945cee459aeae17f3e7234ec3aa49cb26293c5d Mon Sep 17 00:00:00 2001 From: Daniel Karzel Date: Thu, 25 Feb 2021 10:34:22 +1100 Subject: [PATCH 8/8] Remove traits in favour of public functions --- swap/src/bin/asb.rs | 2 +- swap/src/bin/swap_cli.rs | 2 +- swap/src/monero/wallet.rs | 39 +++++++++++++-------------------- swap/src/protocol/alice/swap.rs | 2 +- swap/src/protocol/bob/swap.rs | 2 +- swap/tests/testutils/mod.rs | 1 - 6 files changed, 19 insertions(+), 29 deletions(-) diff --git a/swap/src/bin/asb.rs b/swap/src/bin/asb.rs index d63246fa..ed985d47 100644 --- a/swap/src/bin/asb.rs +++ b/swap/src/bin/asb.rs @@ -31,7 +31,7 @@ use swap::{ execution_params::GetExecutionParams, fs::default_config_path, monero, - monero::{Amount, CreateWallet, GetAddress, OpenWallet}, + monero::{Amount, CreateWallet, OpenWallet}, 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 2efffb3f..4b13e6ba 100644 --- a/swap/src/bin/swap_cli.rs +++ b/swap/src/bin/swap_cli.rs @@ -30,7 +30,7 @@ use swap::{ execution_params::GetExecutionParams, fs::default_config_path, monero, - monero::{CreateWallet, OpenWallet, WalletBlockHeight}, + monero::{CreateWallet, OpenWallet}, protocol::{ bob, bob::{cancel::CancelError, Builder}, diff --git a/swap/src/monero/wallet.rs b/swap/src/monero/wallet.rs index 72226225..c8e3577d 100644 --- a/swap/src/monero/wallet.rs +++ b/swap/src/monero/wallet.rs @@ -1,7 +1,7 @@ use crate::monero::{ Amount, CreateWallet, CreateWalletForOutput, CreateWalletForOutputThenLoadDefaultWallet, - GetAddress, InsufficientFunds, OpenWallet, PrivateViewKey, PublicViewKey, Refresh, Transfer, - TransferProof, TxHash, WalletBlockHeight, WatchForTransfer, + InsufficientFunds, OpenWallet, PrivateViewKey, PublicViewKey, Transfer, TransferProof, TxHash, + WatchForTransfer, }; use ::monero::{Address, Network, PrivateKey, PublicKey}; use anyhow::Result; @@ -55,6 +55,19 @@ impl Wallet { 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 + } } #[async_trait] @@ -233,25 +246,3 @@ impl WatchForTransfer for Wallet { Ok(()) } } - -#[async_trait] -impl WalletBlockHeight for Wallet { - async fn block_height(&self) -> Result { - self.inner.lock().await.block_height().await - } -} - -#[async_trait] -impl GetAddress for Wallet { - 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())?) - } -} - -#[async_trait] -impl Refresh for Wallet { - async fn refresh(&self) -> Result { - self.inner.lock().await.refresh().await - } -} diff --git a/swap/src/protocol/alice/swap.rs b/swap/src/protocol/alice/swap.rs index e233c1e7..55484983 100644 --- a/swap/src/protocol/alice/swap.rs +++ b/swap/src/protocol/alice/swap.rs @@ -10,7 +10,7 @@ use crate::{ database::Database, execution_params::ExecutionParams, monero, - monero::{CreateWalletForOutputThenLoadDefaultWallet, WalletBlockHeight}, + monero::CreateWalletForOutputThenLoadDefaultWallet, monero_ext::ScalarExt, protocol::{ alice, diff --git a/swap/src/protocol/bob/swap.rs b/swap/src/protocol/bob/swap.rs index 485e8f74..29cac4e8 100644 --- a/swap/src/protocol/bob/swap.rs +++ b/swap/src/protocol/bob/swap.rs @@ -4,7 +4,7 @@ use crate::{ database::{Database, Swap}, execution_params::ExecutionParams, monero, - monero::{InsufficientFunds, WalletBlockHeight}, + monero::InsufficientFunds, protocol::bob::{self, event_loop::EventLoopHandle, state::*, QuoteRequest}, }; use anyhow::{bail, Result}; diff --git a/swap/tests/testutils/mod.rs b/swap/tests/testutils/mod.rs index 8210f566..646a3cc4 100644 --- a/swap/tests/testutils/mod.rs +++ b/swap/tests/testutils/mod.rs @@ -21,7 +21,6 @@ use swap::{ execution_params, execution_params::{ExecutionParams, GetExecutionParams}, monero, - monero::Refresh, protocol::{alice, alice::AliceState, bob, bob::BobState}, seed::Seed, };