From 75cfc6b0d4169d72f136266af122381a05bb9efe Mon Sep 17 00:00:00 2001
From: binarybaron <86064887+binarybaron@users.noreply.github.com>
Date: Thu, 25 Jul 2024 00:13:15 +0200
Subject: [PATCH] fix(cli): allow bitcoin-change-address to be omitted when
 making request to rpc server (#1728)

---
 swap/src/api.rs         |  2 +-
 swap/src/api/request.rs | 21 ++++++++++++++++++++-
 swap/src/cli/command.rs | 24 +++---------------------
 swap/src/rpc/methods.rs | 29 +++++++++++++++++++----------
 4 files changed, 43 insertions(+), 33 deletions(-)

diff --git a/swap/src/api.rs b/swap/src/api.rs
index 36c6972f5..5640c8332 100644
--- a/swap/src/api.rs
+++ b/swap/src/api.rs
@@ -437,7 +437,7 @@ pub mod api_test {
 
             Request::new(Method::BuyXmr {
                 seller,
-                bitcoin_change_address,
+                bitcoin_change_address: Some(bitcoin_change_address),
                 monero_receive_address,
                 swap_id: Uuid::new_v4(),
             })
diff --git a/swap/src/api/request.rs b/swap/src/api/request.rs
index 50274ab1a..b1e9c68cc 100644
--- a/swap/src/api/request.rs
+++ b/swap/src/api/request.rs
@@ -31,7 +31,7 @@ pub struct Request {
 pub enum Method {
     BuyXmr {
         seller: Multiaddr,
-        bitcoin_change_address: bitcoin::Address,
+        bitcoin_change_address: Option<bitcoin::Address>,
         monero_receive_address: monero::Address,
         swap_id: Uuid,
     },
@@ -335,6 +335,25 @@ impl Request {
                 let env_config = context.config.env_config;
                 let seed = context.config.seed.clone().context("Could not get seed")?;
 
+                // When no change address was provided we default to the internal wallet
+                let bitcoin_change_address = match bitcoin_change_address {
+                    Some(addr) => addr,
+                    None => {
+                        let internal_wallet_address = context
+                            .bitcoin_wallet()
+                            .expect("bitcoin wallet should exist")
+                            .new_address()
+                            .await?;
+
+                        tracing::info!(
+                            internal_wallet_address=%internal_wallet_address,
+                            "No --change-address supplied. Any change will be received to the internal wallet."
+                        );
+
+                        internal_wallet_address
+                    }
+                };
+
                 let seller_peer_id = seller
                     .extract_peer_id()
                     .context("Seller address must contain peer ID")?;
diff --git a/swap/src/cli/command.rs b/swap/src/cli/command.rs
index f3d61c925..4881d94ee 100644
--- a/swap/src/cli/command.rs
+++ b/swap/src/cli/command.rs
@@ -81,29 +81,11 @@ where
             )
             .await?;
 
-            // when no refund address was provided we default to the internal wallet
-            let bitcoin_change_address = match bitcoin_change_address {
-                Some(addr) => addr,
-                None => {
-                    let internal_wallet_address = context
-                        .bitcoin_wallet()
-                        .expect("bitcoin wallet should exist")
-                        .new_address()
-                        .await?;
-
-                    tracing::info!(
-                        internal_wallet_address=%internal_wallet_address,
-                        "No --change-address supplied. Any change will be received to the internal wallet."
-                    );
-
-                    internal_wallet_address
-                }
-            };
-
             let monero_receive_address =
                 monero_address::validate_is_testnet(monero_receive_address, is_testnet)?;
-            let bitcoin_change_address =
-                bitcoin_address::validate_is_testnet(bitcoin_change_address, is_testnet)?;
+            let bitcoin_change_address = bitcoin_change_address
+                .map(|address| bitcoin_address::validate_is_testnet(address, is_testnet))
+                .transpose()?;
 
             let request = Request::new(Method::BuyXmr {
                 seller,
diff --git a/swap/src/rpc/methods.rs b/swap/src/rpc/methods.rs
index 83100d4a7..f804e085b 100644
--- a/swap/src/rpc/methods.rs
+++ b/swap/src/rpc/methods.rs
@@ -135,16 +135,25 @@ pub fn register_modules(context: Arc<Context>) -> Result<RpcModule<Arc<Context>>
     module.register_async_method("buy_xmr", |params_raw, context| async move {
         let params: HashMap<String, String> = params_raw.parse()?;
 
-        let bitcoin_change_address =
-            bitcoin::Address::from_str(params.get("bitcoin_change_address").ok_or_else(|| {
-                jsonrpsee_core::Error::Custom("Does not contain bitcoin_change_address".to_string())
-            })?)
-            .map_err(|err| jsonrpsee_core::Error::Custom(err.to_string()))?;
-
-        let bitcoin_change_address = bitcoin_address::validate(
-            bitcoin_change_address,
-            context.config.env_config.bitcoin_network,
-        )?;
+        let bitcoin_change_address = params
+            .get("bitcoin_change_address")
+            .map(|addr_str| {
+                bitcoin::Address::from_str(addr_str)
+                    .map_err(|err| {
+                        jsonrpsee_core::Error::Custom(format!(
+                            "Could not parse bitcoin address: {}",
+                            err
+                        ))
+                    })
+                    .and_then(|address| {
+                        bitcoin_address::validate(
+                            address,
+                            context.config.env_config.bitcoin_network,
+                        )
+                        .map_err(|err| jsonrpsee_core::Error::Custom(err.to_string()))
+                    })
+            })
+            .transpose()?;
 
         let monero_receive_address =
             monero::Address::from_str(params.get("monero_receive_address").ok_or_else(|| {