Merge #263

263: Cleanup swap initialization for Alice and Bob r=da-kami a=da-kami



Co-authored-by: Daniel Karzel <daniel@comit.network>

swap/src/bin/swap_cli.rs

@@ -107,15 +107,10 @@ async fn main() -> Result<()> {
             alice_peer_id,
             alice_addr,
         } => {
-            let (bitcoin_wallet, monero_wallet) = init_wallets(
+            let bitcoin_wallet =
-                config,
+                init_bitcoin_wallet(config, bitcoin_network, &wallet_data_dir, seed).await?;
-                bitcoin_network,
+            let monero_wallet =
-                &wallet_data_dir,
+                init_monero_wallet(monero_network, monero_wallet_rpc_process.endpoint()).await?;
-                monero_network,
-                seed,
-                monero_wallet_rpc_process.endpoint(),
-            )
-            .await?;
 
             let swap_id = Uuid::new_v4();
 
@@ -182,15 +177,10 @@ async fn main() -> Result<()> {
             alice_peer_id,
             alice_addr,
         } => {
-            let (bitcoin_wallet, monero_wallet) = init_wallets(
+            let bitcoin_wallet =
-                config,
+                init_bitcoin_wallet(config, bitcoin_network, &wallet_data_dir, seed).await?;
-                bitcoin_network,
+            let monero_wallet =
-                &wallet_data_dir,
+                init_monero_wallet(monero_network, monero_wallet_rpc_process.endpoint()).await?;
-                monero_network,
-                seed,
-                monero_wallet_rpc_process.endpoint(),
-            )
-            .await?;
 
             let bob_factory = Builder::new(
                 seed,
@@ -214,124 +204,53 @@ async fn main() -> Result<()> {
                 }
             }
         }
-        Command::Cancel {
+        Command::Cancel { swap_id, force } => {
-            swap_id,
+            let bitcoin_wallet =
-            alice_peer_id,
+                init_bitcoin_wallet(config, bitcoin_network, &wallet_data_dir, seed).await?;
-            alice_addr,
-            force,
-        } => {
-            // TODO: Optimization: Only init the Bitcoin wallet, Monero wallet unnecessary
-            let (bitcoin_wallet, monero_wallet) = init_wallets(
-                config,
-                bitcoin_network,
-                &wallet_data_dir,
-                monero_network,
-                seed,
-                monero_wallet_rpc_process.endpoint(),
-            )
-            .await?;
 
-            let bob_factory = Builder::new(
+            let resume_state = db.get_state(swap_id)?.try_into_bob()?.into();
-                seed,
+            let cancel =
-                db,
+                bob::cancel(swap_id, resume_state, Arc::new(bitcoin_wallet), db, force).await?;
-                swap_id,
-                Arc::new(bitcoin_wallet),
-                Arc::new(monero_wallet),
-                alice_addr,
-                alice_peer_id,
-                execution_params,
-            );
-            let (swap, event_loop) = bob_factory.build().await?;
-            let handle = tokio::spawn(async move { event_loop.run().await });
-
-            let cancel = bob::cancel(
-                swap.swap_id,
-                swap.state,
-                swap.bitcoin_wallet,
-                swap.db,
-                force,
-            );
 
-            tokio::select! {
+            match cancel {
-                event_loop_result = handle => {
+                Ok((txid, _)) => {
-                    event_loop_result??;
+                    debug!("Cancel transaction successfully published with id {}", txid)
-                },
+                }
-                cancel_result = cancel => {
+                Err(CancelError::CancelTimelockNotExpiredYet) => error!(
-                    match cancel_result? {
+                    "The Cancel Transaction cannot be published yet, \
-                        Ok((txid, _)) => {
+                        because the timelock has not expired. Please try again later."
-                            debug!("Cancel transaction successfully published with id {}", txid)
+                ),
-                        }
+                Err(CancelError::CancelTxAlreadyPublished) => {
-                        Err(CancelError::CancelTimelockNotExpiredYet) => error!(
+                    warn!("The Cancel Transaction has already been published.")
-                            "The Cancel Transaction cannot be published yet, \
-                            because the timelock has not expired. Please try again later."
-                        ),
-                        Err(CancelError::CancelTxAlreadyPublished) => {
-                            warn!("The Cancel Transaction has already been published.")
-                        }
-                    }
                 }
             }
         }
-        Command::Refund {
+        Command::Refund { swap_id, force } => {
-            swap_id,
+            let bitcoin_wallet =
-            alice_peer_id,
+                init_bitcoin_wallet(config, bitcoin_network, &wallet_data_dir, seed).await?;
-            alice_addr,
-            force,
-        } => {
-            let (bitcoin_wallet, monero_wallet) = init_wallets(
-                config,
-                bitcoin_network,
-                &wallet_data_dir,
-                monero_network,
-                seed,
-                monero_wallet_rpc_process.endpoint(),
-            )
-            .await?;
 
-            // TODO: Optimize to only use the Bitcoin wallet, Monero wallet is unnecessary
+            let resume_state = db.get_state(swap_id)?.try_into_bob()?.into();
-            let bob_factory = Builder::new(
+
-                seed,
+            bob::refund(
-                db,
                 swap_id,
-                Arc::new(bitcoin_wallet),
+                resume_state,
-                Arc::new(monero_wallet),
-                alice_addr,
-                alice_peer_id,
                 execution_params,
-            );
+                Arc::new(bitcoin_wallet),
-            let (swap, event_loop) = bob_factory.build().await?;
+                db,
-
-            let handle = tokio::spawn(async move { event_loop.run().await });
-            let refund = bob::refund(
-                swap.swap_id,
-                swap.state,
-                swap.execution_params,
-                swap.bitcoin_wallet,
-                swap.db,
                 force,
-            );
+            )
-
+            .await??;
-            tokio::select! {
-                event_loop_result = handle => {
-                    event_loop_result??;
-                },
-                refund_result = refund => {
-                    refund_result??;
-                }
-            }
         }
     };
     Ok(())
 }
 
-async fn init_wallets(
+async fn init_bitcoin_wallet(
     config: Config,
     bitcoin_network: bitcoin::Network,
     bitcoin_wallet_data_dir: &Path,
-    monero_network: monero::Network,
     seed: Seed,
-    monero_wallet_rpc_url: Url,
+) -> Result<bitcoin::Wallet> {
-) -> Result<(bitcoin::Wallet, monero::Wallet)> {
     let bitcoin_wallet = bitcoin::Wallet::new(
         config.bitcoin.electrum_rpc_url,
         config.bitcoin.electrum_http_url,
@@ -346,6 +265,13 @@ async fn init_wallets(
         .await
         .context("failed to sync balance of bitcoin wallet")?;
 
+    Ok(bitcoin_wallet)
+}
+
+async fn init_monero_wallet(
+    monero_network: monero::Network,
+    monero_wallet_rpc_url: Url,
+) -> Result<monero::Wallet> {
     let monero_wallet = monero::Wallet::new(
         monero_wallet_rpc_url.clone(),
         monero_network,
@@ -377,5 +303,5 @@ async fn init_wallets(
         .await
         .context("failed to validate connection to monero-wallet-rpc")?;
 
-    Ok((bitcoin_wallet, monero_wallet))
+    Ok(monero_wallet)
 }

swap/src/cli/command.rs

@@ -54,17 +54,6 @@ pub enum Command {
         #[structopt(long = "swap-id")]
         swap_id: Uuid,
 
-        // TODO: Remove Alice peer-id/address, it should be saved in the database when running swap
-        // and loaded from the database when running resume/cancel/refund
-        #[structopt(long = "counterpart-peer-id", default_value = DEFAULT_ALICE_PEER_ID)]
-        alice_peer_id: PeerId,
-
-        #[structopt(
-             long = "counterpart-addr",
-             default_value = DEFAULT_ALICE_MULTIADDR
-             )]
-        alice_addr: Multiaddr,
-
         #[structopt(short, long)]
         force: bool,
     },
@@ -72,17 +61,6 @@ pub enum Command {
         #[structopt(long = "swap-id")]
         swap_id: Uuid,
 
-        // TODO: Remove Alice peer-id/address, it should be saved in the database when running swap
-        // and loaded from the database when running resume/cancel/refund
-        #[structopt(long = "counterpart-peer-id", default_value = DEFAULT_ALICE_PEER_ID)]
-        alice_peer_id: PeerId,
-
-        #[structopt(
-        long = "counterpart-addr",
-        default_value = DEFAULT_ALICE_MULTIADDR
-        )]
-        alice_addr: Multiaddr,
-
         #[structopt(short, long)]
         force: bool,
     },

swap/src/database.rs

@@ -36,6 +36,15 @@ impl Display for Swap {
     }
 }
 
+impl Swap {
+    pub fn try_into_bob(self) -> Result<Bob> {
+        match self {
+            Swap::Bob(bob) => Ok(bob),
+            Swap::Alice(_) => bail!("Swap instance is not Bob"),
+        }
+    }
+}
+
 pub struct Database(sled::Db);
 
 impl Database {

swap/src/protocol/alice.rs

@@ -1,8 +1,6 @@
 //! Run an XMR/BTC swap in the role of Alice.
 //! Alice holds XMR and wishes receive BTC.
-use crate::{bitcoin, database, database::Database, execution_params::ExecutionParams, monero};
+use crate::{bitcoin, database::Database, execution_params::ExecutionParams, monero};
-use anyhow::{bail, Result};
-use libp2p::{core::Multiaddr, PeerId};
 use std::sync::Arc;
 use uuid::Uuid;
 
@@ -36,114 +34,3 @@ pub struct Swap {
     pub swap_id: Uuid,
     pub db: Arc<Database>,
 }
-
-pub struct Builder {
-    swap_id: Uuid,
-    peer_id: PeerId,
-    db: Arc<Database>,
-    execution_params: ExecutionParams,
-    event_loop_handle: EventLoopHandle,
-    listen_address: Multiaddr,
-
-    bitcoin_wallet: Arc<bitcoin::Wallet>,
-    monero_wallet: Arc<monero::Wallet>,
-
-    init_params: InitParams,
-}
-
-enum InitParams {
-    None,
-    New {
-        bob_peer_id: PeerId,
-        state3: Box<State3>,
-    },
-}
-
-impl Builder {
-    #[allow(clippy::too_many_arguments)]
-    pub fn new(
-        self_peer_id: PeerId,
-        execution_params: ExecutionParams,
-        swap_id: Uuid,
-        bitcoin_wallet: Arc<bitcoin::Wallet>,
-        monero_wallet: Arc<monero::Wallet>,
-        db: Arc<Database>,
-        listen_address: Multiaddr,
-        event_loop_handle: EventLoopHandle,
-    ) -> Self {
-        Self {
-            swap_id,
-            peer_id: self_peer_id,
-            db,
-            execution_params,
-            event_loop_handle,
-            listen_address,
-            bitcoin_wallet,
-            monero_wallet,
-            init_params: InitParams::None,
-        }
-    }
-
-    pub fn with_init_params(self, bob_peer_id: PeerId, state3: State3) -> Self {
-        Self {
-            init_params: InitParams::New {
-                bob_peer_id,
-                state3: Box::new(state3),
-            },
-            ..self
-        }
-    }
-
-    pub async fn build(self) -> Result<Swap> {
-        match self.init_params {
-            InitParams::New {
-                bob_peer_id,
-                ref state3,
-            } => {
-                let initial_state = AliceState::Started {
-                    state3: state3.clone(),
-                    bob_peer_id,
-                };
-
-                Ok(Swap {
-                    event_loop_handle: self.event_loop_handle,
-                    bitcoin_wallet: self.bitcoin_wallet,
-                    monero_wallet: self.monero_wallet,
-                    execution_params: self.execution_params,
-                    db: self.db,
-                    state: initial_state,
-                    swap_id: self.swap_id,
-                })
-            }
-            InitParams::None => {
-                let resume_state =
-                    if let database::Swap::Alice(state) = self.db.get_state(self.swap_id)? {
-                        state.into()
-                    } else {
-                        bail!(
-                            "Trying to load swap with id {} for the wrong direction.",
-                            self.swap_id
-                        )
-                    };
-
-                Ok(Swap {
-                    state: resume_state,
-                    event_loop_handle: self.event_loop_handle,
-                    bitcoin_wallet: self.bitcoin_wallet,
-                    monero_wallet: self.monero_wallet,
-                    execution_params: self.execution_params,
-                    swap_id: self.swap_id,
-                    db: self.db,
-                })
-            }
-        }
-    }
-
-    pub fn peer_id(&self) -> PeerId {
-        self.peer_id
-    }
-
-    pub fn listen_address(&self) -> Multiaddr {
-        self.listen_address.clone()
-    }
-}

swap/src/protocol/alice/event_loop.rs

@@ -10,7 +10,7 @@ use crate::{
     protocol::{
         alice,
         alice::{
-            AliceState, Behaviour, Builder, OutEvent, QuoteResponse, State0, State3, TransferProof,
+            AliceState, Behaviour, OutEvent, QuoteResponse, State0, State3, Swap, TransferProof,
         },
         bob::{EncryptedSignature, QuoteRequest},
     },
@@ -86,7 +86,6 @@ pub struct EventLoop<RS> {
     bitcoin_wallet: Arc<bitcoin::Wallet>,
     monero_wallet: Arc<monero::Wallet>,
     db: Arc<Database>,
-    listen_address: Multiaddr,
     rate_service: RS,
     max_sell: Amount,
 
@@ -139,7 +138,6 @@ where
             bitcoin_wallet,
             monero_wallet,
             db,
-            listen_address,
             rate_service,
             recv_encrypted_signature: recv_encrypted_signature.sender,
             send_transfer_proof: send_transfer_proof.receiver,
@@ -271,19 +269,20 @@ where
         let swap_id = Uuid::new_v4();
         let handle = self.new_handle();
 
-        let swap = Builder::new(
+        let initial_state = AliceState::Started {
-            self.peer_id,
+            state3: Box::new(state3),
-            self.execution_params,
+            bob_peer_id,
+        };
+
+        let swap = Swap {
+            event_loop_handle: handle,
+            bitcoin_wallet: self.bitcoin_wallet.clone(),
+            monero_wallet: self.monero_wallet.clone(),
+            execution_params: self.execution_params,
+            db: self.db.clone(),
+            state: initial_state,
             swap_id,
-            self.bitcoin_wallet.clone(),
+        };
-            self.monero_wallet.clone(),
-            self.db.clone(),
-            self.listen_address.clone(),
-            handle,
-        )
-        .with_init_params(bob_peer_id, state3)
-        .build()
-        .await?;
 
         let (swap, swap_handle) = alice::run(swap).remote_handle();
         tokio::spawn(swap);

swap/src/protocol/bob.rs

@@ -1,7 +1,7 @@
 //! Run an XMR/BTC swap in the role of Bob.
 //! Bob holds BTC and wishes receive XMR.
 use crate::{
-    bitcoin, database,
+    bitcoin,
     database::Database,
     execution_params::ExecutionParams,
     monero, network,
@@ -12,7 +12,7 @@ use crate::{
     protocol::{alice, alice::TransferProof, bob},
     seed::Seed,
 };
-use anyhow::{bail, Error, Result};
+use anyhow::{Error, Result};
 use libp2p::{core::Multiaddr, identity::Keypair, NetworkBehaviour, PeerId};
 use std::sync::Arc;
 use tracing::debug;
@@ -129,15 +129,7 @@ impl Builder {
             }
 
             InitParams::None => {
-                let resume_state =
+                let resume_state = self.db.get_state(self.swap_id)?.try_into_bob()?.into();
-                    if let database::Swap::Bob(state) = self.db.get_state(self.swap_id)? {
-                        state.into()
-                    } else {
-                        bail!(
-                            "Trying to load swap with id {} for the wrong direction.",
-                            self.swap_id
-                        )
-                    };
 
                 let (event_loop, event_loop_handle) = self.init_event_loop()?;