From ec019550ddc9d28948eccb2148700cf2edb7bfd1 Mon Sep 17 00:00:00 2001 From: Daniel Karzel Date: Thu, 8 Jul 2021 11:49:03 +1000 Subject: [PATCH] Revert "Add a mandatory `--change-address` parameter to `buy-xmr`" This reverts commit 5463bde4 --- CHANGELOG.md | 3 --- swap/Cargo.toml | 2 +- swap/src/bin/asb.rs | 4 +-- swap/src/bin/swap.rs | 2 -- swap/src/bitcoin/lock.rs | 11 ++------ swap/src/bitcoin/wallet.rs | 47 +--------------------------------- swap/src/cli/command.rs | 35 +------------------------ swap/src/database/bob.rs | 20 ++------------- swap/src/protocol/bob.rs | 7 +---- swap/src/protocol/bob/state.rs | 10 +------- swap/src/protocol/bob/swap.rs | 8 +++--- swap/tests/harness/mod.rs | 1 - 12 files changed, 13 insertions(+), 137 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2c16a52a..a9c7034c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,9 +26,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 The rendezvous node address (`rendezvous_point`), as well as the ASB's external addresses (`external_addresses`) to be registered, is configured in the `network` section of the ASB config file. A rendezvous node is provided at `/dnsaddr/rendezvous.coblox.tech/p2p/12D3KooWQUt9DkNZxEn2R5ymJzWj15MpG6mTW84kyd8vDaRZi46o` which is used as default for discovery in the CLI. Upon discovery using `list-sellers` CLI users are provided with quote and connection information for each ASB discovered through the rendezvous node. -- A mandatory `--change-address` parameter to the CLI's `buy-xmr` command. - The provided address is used to transfer Bitcoin in case of a refund and in case the user transfers more than the specified amount into the swap. - For more information see [#513](https://github.com/comit-network/xmr-btc-swap/issues/513). ### Fixed diff --git a/swap/Cargo.toml b/swap/Cargo.toml index 1374096e..3b5a0968 100644 --- a/swap/Cargo.toml +++ b/swap/Cargo.toml @@ -45,7 +45,7 @@ rust_decimal_macros = "1" serde = { version = "1", features = [ "derive" ] } serde_cbor = "0.11" serde_json = "1" -serde_with = { version = "1", features = [ "macros" ] } +serde_with = { version = "1.9.4", features = [ "macros" ] } sha2 = "0.9" sigma_fun = { git = "https://github.com/LLFourn/secp256kfun", default-features = false, features = [ "ed25519", "serde" ] } sled = "0.34" diff --git a/swap/src/bin/asb.rs b/swap/src/bin/asb.rs index 08c81310..827d297d 100644 --- a/swap/src/bin/asb.rs +++ b/swap/src/bin/asb.rs @@ -225,9 +225,7 @@ async fn main() -> Result<()> { } }; - let psbt = bitcoin_wallet - .send_to_address(address, amount, None) - .await?; + let psbt = bitcoin_wallet.send_to_address(address, amount).await?; let signed_tx = bitcoin_wallet.sign_and_finalize(psbt).await?; bitcoin_wallet.broadcast(signed_tx, "withdraw").await?; diff --git a/swap/src/bin/swap.rs b/swap/src/bin/swap.rs index 5c39af1d..bae71728 100644 --- a/swap/src/bin/swap.rs +++ b/swap/src/bin/swap.rs @@ -58,7 +58,6 @@ async fn main() -> Result<()> { seller, bitcoin_electrum_rpc_url, bitcoin_target_block, - bitcoin_change_address, monero_receive_address, monero_daemon_address, tor_socks5_port, @@ -124,7 +123,6 @@ async fn main() -> Result<()> { env_config, event_loop_handle, monero_receive_address, - bitcoin_change_address, amount, ); diff --git a/swap/src/bitcoin/lock.rs b/swap/src/bitcoin/lock.rs index b1b2be0e..433468ac 100644 --- a/swap/src/bitcoin/lock.rs +++ b/swap/src/bitcoin/lock.rs @@ -24,7 +24,6 @@ impl TxLock { amount: Amount, A: PublicKey, B: PublicKey, - change: bitcoin::Address, ) -> Result where C: EstimateFeeRate, @@ -35,9 +34,7 @@ impl TxLock { .address(wallet.get_network()) .expect("can derive address from descriptor"); - let psbt = wallet - .send_to_address(address, amount, Some(change)) - .await?; + let psbt = wallet.send_to_address(address, amount).await?; Ok(Self { inner: psbt, @@ -254,11 +251,7 @@ mod tests { wallet: &Wallet<(), bdk::database::MemoryDatabase, StaticFeeRate>, amount: Amount, ) -> PartiallySignedTransaction { - let change = wallet.new_address().await.unwrap(); - TxLock::new(&wallet, amount, A, B, change) - .await - .unwrap() - .into() + TxLock::new(&wallet, amount, A, B).await.unwrap().into() } fn alice_and_bob() -> (PublicKey, PublicKey) { diff --git a/swap/src/bitcoin/wallet.rs b/swap/src/bitcoin/wallet.rs index 7eb63794..1a38303c 100644 --- a/swap/src/bitcoin/wallet.rs +++ b/swap/src/bitcoin/wallet.rs @@ -314,18 +314,11 @@ where &self, address: Address, amount: Amount, - change_override: Option
, ) -> Result { if self.network != address.network { bail!("Cannot build PSBT because network of given address is {} but wallet is on network {}", address.network, self.network); } - if let Some(change) = change_override.as_ref() { - if self.network != change.network { - bail!("Cannot build PSBT because network of given address is {} but wallet is on network {}", change.network, self.network); - } - } - let wallet = self.wallet.lock().await; let client = self.client.lock().await; let fee_rate = client.estimate_feerate(self.target_block)?; @@ -352,17 +345,6 @@ where _ => bail!("Unexpected transaction layout"), } - if let ([_, change], [_, psbt_output], Some(change_override)) = ( - &mut psbt.global.unsigned_tx.output.as_mut_slice(), - &mut psbt.outputs.as_mut_slice(), - change_override, - ) { - change.script_pubkey = change_override.script_pubkey(); - // Might be populated based on the previously set change address, but for the - // overwrite we don't know unless we ask the user for more information. - psbt_output.bip32_derivation.clear(); - } - Ok(psbt) } @@ -1082,8 +1064,7 @@ mod tests { // if the change output is below dust it will be dropped by the BDK for amount in above_dust..(balance - (above_dust - 1)) { let (A, B) = (PublicKey::random(), PublicKey::random()); - let change = wallet.new_address().await.unwrap(); - let txlock = TxLock::new(&wallet, bitcoin::Amount::from_sat(amount), A, B, change) + let txlock = TxLock::new(&wallet, bitcoin::Amount::from_sat(amount), A, B) .await .unwrap(); let txlock_output = txlock.script_pubkey(); @@ -1098,30 +1079,4 @@ mod tests { ); } } - - #[tokio::test] - async fn can_override_change_address() { - let wallet = Wallet::new_funded_default_fees(50_000); - let custom_change = "bcrt1q08pfqpsyrt7acllzyjm8q5qsz5capvyahm49rw" - .parse::
() - .unwrap(); - - let psbt = wallet - .send_to_address( - wallet.new_address().await.unwrap(), - Amount::from_sat(10_000), - Some(custom_change.clone()), - ) - .await - .unwrap(); - let transaction = wallet.sign_and_finalize(psbt).await.unwrap(); - - match transaction.output.as_slice() { - [first, change] => { - assert_eq!(first.value, 10_000); - assert_eq!(change.script_pubkey, custom_change.script_pubkey()); - } - _ => panic!("expected exactly two outputs"), - } - } } diff --git a/swap/src/cli/command.rs b/swap/src/cli/command.rs index 6c3542f0..920d6b84 100644 --- a/swap/src/cli/command.rs +++ b/swap/src/cli/command.rs @@ -74,7 +74,6 @@ where bitcoin_electrum_rpc_url, bitcoin_target_block, }, - bitcoin_change_address, monero: Monero { monero_daemon_address, }, @@ -92,7 +91,6 @@ where is_testnet, )?, bitcoin_target_block: bitcoin_target_block_from(bitcoin_target_block, is_testnet), - bitcoin_change_address, monero_receive_address: validate_monero_address( monero_receive_address, is_testnet, @@ -212,7 +210,6 @@ pub enum Command { seller: Multiaddr, bitcoin_electrum_rpc_url: Url, bitcoin_target_block: usize, - bitcoin_change_address: bitcoin::Address, monero_receive_address: monero::Address, monero_daemon_address: String, tor_socks5_port: u16, @@ -290,12 +287,6 @@ enum RawCommand { #[structopt(flatten)] bitcoin: Bitcoin, - #[structopt( - long = "change-address", - help = "The bitcoin address where any form of change or excess funds should be sent to" - )] - bitcoin_change_address: bitcoin::Address, - #[structopt(flatten)] monero: Monero, @@ -361,7 +352,7 @@ enum RawCommand { } #[derive(structopt::StructOpt, Debug)] -struct Monero { +pub struct Monero { #[structopt( long = "monero-daemon-address", help = "Specify to connect to a monero daemon of your choice: :" @@ -521,9 +512,7 @@ mod tests { const MAINNET: &str = "mainnet"; const MONERO_STAGENET_ADDRESS: &str = "53gEuGZUhP9JMEBZoGaFNzhwEgiG7hwQdMCqFxiyiTeFPmkbt1mAoNybEUvYBKHcnrSgxnVWgZsTvRBaHBNXPa8tHiCU51a"; - const BITCOIN_TESTNET_ADDRESS: &str = "tb1qr3em6k3gfnyl8r7q0v7t4tlnyxzgxma3lressv"; const MONERO_MAINNET_ADDRESS: &str = "44Ato7HveWidJYUAVw5QffEcEtSH1DwzSP3FPPkHxNAS4LX9CqgucphTisH978FLHE34YNEx7FcbBfQLQUU8m3NUC4VqsRa"; - const BITCOIN_MAINNET_ADDRESS: &str = "bc1qe4epnfklcaa0mun26yz5g8k24em5u9f92hy325"; const MULTI_ADDRESS: &str = "/ip4/127.0.0.1/tcp/9939/p2p/12D3KooWCdMKjesXMJz1SiZ7HgotrxuqhQJbP5sgBm2BwP1cqThi"; const SWAP_ID: &str = "ea030832-3be9-454f-bb98-5ea9a788406b"; @@ -535,8 +524,6 @@ mod tests { "buy-xmr", "--receive-address", MONERO_MAINNET_ADDRESS, - "--change-address", - BITCOIN_MAINNET_ADDRESS, "--seller", MULTI_ADDRESS, ]; @@ -555,8 +542,6 @@ mod tests { "buy-xmr", "--receive-address", MONERO_STAGENET_ADDRESS, - "--change-address", - BITCOIN_TESTNET_ADDRESS, "--seller", MULTI_ADDRESS, ]; @@ -576,8 +561,6 @@ mod tests { "buy-xmr", "--receive-address", MONERO_STAGENET_ADDRESS, - "--change-address", - BITCOIN_TESTNET_ADDRESS, "--seller", MULTI_ADDRESS, ]; @@ -601,8 +584,6 @@ mod tests { "buy-xmr", "--receive-address", MONERO_MAINNET_ADDRESS, - "--change-address", - BITCOIN_MAINNET_ADDRESS, "--seller", MULTI_ADDRESS, ]; @@ -699,8 +680,6 @@ mod tests { "--data-base-dir", data_dir, "buy-xmr", - "--change-address", - BITCOIN_MAINNET_ADDRESS, "--receive-address", MONERO_MAINNET_ADDRESS, "--seller", @@ -723,8 +702,6 @@ mod tests { "--data-base-dir", data_dir, "buy-xmr", - "--change-address", - BITCOIN_TESTNET_ADDRESS, "--receive-address", MONERO_STAGENET_ADDRESS, "--seller", @@ -787,8 +764,6 @@ mod tests { BINARY_NAME, "--debug", "buy-xmr", - "--change-address", - BITCOIN_MAINNET_ADDRESS, "--receive-address", MONERO_MAINNET_ADDRESS, "--seller", @@ -806,8 +781,6 @@ mod tests { "--testnet", "--debug", "buy-xmr", - "--change-address", - BITCOIN_TESTNET_ADDRESS, "--receive-address", MONERO_STAGENET_ADDRESS, "--seller", @@ -850,8 +823,6 @@ mod tests { BINARY_NAME, "--json", "buy-xmr", - "--change-address", - BITCOIN_MAINNET_ADDRESS, "--receive-address", MONERO_MAINNET_ADDRESS, "--seller", @@ -869,8 +840,6 @@ mod tests { "--testnet", "--json", "buy-xmr", - "--change-address", - BITCOIN_TESTNET_ADDRESS, "--receive-address", MONERO_STAGENET_ADDRESS, "--seller", @@ -919,7 +888,6 @@ mod tests { bitcoin_electrum_rpc_url: Url::from_str(DEFAULT_ELECTRUM_RPC_URL_TESTNET) .unwrap(), bitcoin_target_block: DEFAULT_BITCOIN_CONFIRMATION_TARGET_TESTNET, - bitcoin_change_address: BITCOIN_TESTNET_ADDRESS.parse().unwrap(), monero_receive_address: monero::Address::from_str(MONERO_STAGENET_ADDRESS) .unwrap(), monero_daemon_address: DEFAULT_MONERO_DAEMON_ADDRESS_STAGENET.to_string(), @@ -938,7 +906,6 @@ mod tests { seller: Multiaddr::from_str(MULTI_ADDRESS).unwrap(), bitcoin_electrum_rpc_url: Url::from_str(DEFAULT_ELECTRUM_RPC_URL).unwrap(), bitcoin_target_block: DEFAULT_BITCOIN_CONFIRMATION_TARGET, - bitcoin_change_address: BITCOIN_MAINNET_ADDRESS.parse().unwrap(), monero_receive_address: monero::Address::from_str(MONERO_MAINNET_ADDRESS) .unwrap(), monero_daemon_address: DEFAULT_MONERO_DAEMON_ADDRESS.to_string(), diff --git a/swap/src/database/bob.rs b/swap/src/database/bob.rs index 82c6b848..b0258b00 100644 --- a/swap/src/database/bob.rs +++ b/swap/src/database/bob.rs @@ -3,17 +3,13 @@ use crate::protocol::bob; use crate::protocol::bob::BobState; use monero_rpc::wallet::BlockHeight; use serde::{Deserialize, Serialize}; -use serde_with::{serde_as, DisplayFromStr}; use std::fmt; -#[serde_as] #[derive(Clone, Debug, Deserialize, Serialize, PartialEq)] pub enum Bob { Started { #[serde(with = "::bitcoin::util::amount::serde::as_sat")] btc_amount: bitcoin::Amount, - #[serde_as(as = "DisplayFromStr")] - change_address: bitcoin::Address, }, ExecutionSetupDone { state2: bob::State2, @@ -49,13 +45,7 @@ pub enum BobEndState { impl From for Bob { fn from(bob_state: BobState) -> Self { match bob_state { - BobState::Started { - btc_amount, - change_address, - } => Bob::Started { - btc_amount, - change_address, - }, + BobState::Started { btc_amount } => Bob::Started { btc_amount }, BobState::SwapSetupCompleted(state2) => Bob::ExecutionSetupDone { state2 }, BobState::BtcLocked(state3) => Bob::BtcLocked { state3 }, BobState::XmrLockProofReceived { @@ -87,13 +77,7 @@ impl From for Bob { impl From for BobState { fn from(db_state: Bob) -> Self { match db_state { - Bob::Started { - btc_amount, - change_address, - } => BobState::Started { - btc_amount, - change_address, - }, + Bob::Started { btc_amount } => BobState::Started { btc_amount }, Bob::ExecutionSetupDone { state2 } => BobState::SwapSetupCompleted(state2), Bob::BtcLocked { state3 } => BobState::BtcLocked(state3), Bob::XmrLockProofReceived { diff --git a/swap/src/protocol/bob.rs b/swap/src/protocol/bob.rs index dc8f7aec..37a70da9 100644 --- a/swap/src/protocol/bob.rs +++ b/swap/src/protocol/bob.rs @@ -33,14 +33,10 @@ impl Swap { env_config: env::Config, event_loop_handle: cli::EventLoopHandle, monero_receive_address: monero::Address, - bitcoin_change_address: bitcoin::Address, btc_amount: bitcoin::Amount, ) -> Self { Self { - state: BobState::Started { - btc_amount, - change_address: bitcoin_change_address, - }, + state: BobState::Started { btc_amount }, event_loop_handle, db, bitcoin_wallet, @@ -51,7 +47,6 @@ impl Swap { } } - #[allow(clippy::too_many_arguments)] pub fn from_db( db: Database, id: Uuid, diff --git a/swap/src/protocol/bob/state.rs b/swap/src/protocol/bob/state.rs index 8e0bac9a..e80634b6 100644 --- a/swap/src/protocol/bob/state.rs +++ b/swap/src/protocol/bob/state.rs @@ -25,7 +25,6 @@ use uuid::Uuid; pub enum BobState { Started { btc_amount: bitcoin::Amount, - change_address: bitcoin::Address, }, SwapSetupCompleted(State2), BtcLocked(State3), @@ -170,14 +169,7 @@ impl State0 { bail!("Alice's dleq proof doesn't verify") } - let tx_lock = bitcoin::TxLock::new( - wallet, - self.btc, - msg.A, - self.b.public(), - self.refund_address.clone(), - ) - .await?; + let tx_lock = bitcoin::TxLock::new(wallet, self.btc, msg.A, self.b.public()).await?; let v = msg.v_a + self.v_b; Ok(State1 { diff --git a/swap/src/protocol/bob/swap.rs b/swap/src/protocol/bob/swap.rs index 0ebab92b..2d9d04a2 100644 --- a/swap/src/protocol/bob/swap.rs +++ b/swap/src/protocol/bob/swap.rs @@ -61,10 +61,8 @@ async fn next_state( tracing::trace!(%state, "Advancing state"); Ok(match state { - BobState::Started { - btc_amount, - change_address, - } => { + BobState::Started { btc_amount } => { + let bitcoin_refund_address = bitcoin_wallet.new_address().await?; let tx_refund_fee = bitcoin_wallet .estimate_fee(TxRefund::weight(), btc_amount) .await?; @@ -78,7 +76,7 @@ async fn next_state( btc: btc_amount, tx_refund_fee, tx_cancel_fee, - bitcoin_refund_address: change_address, + bitcoin_refund_address, }) .await?; diff --git a/swap/tests/harness/mod.rs b/swap/tests/harness/mod.rs index ad43bd9e..7a03db2d 100644 --- a/swap/tests/harness/mod.rs +++ b/swap/tests/harness/mod.rs @@ -434,7 +434,6 @@ impl BobParams { self.env_config, handle, self.monero_wallet.get_main_address(), - self.bitcoin_wallet.new_address().await?, btc_amount, );