From c11042ff0da15203581a95a8c6953737365b3a79 Mon Sep 17 00:00:00 2001 From: Franck Royer Date: Tue, 19 Jan 2021 15:21:40 +1100 Subject: [PATCH] Use `builder` terminology instead of `factory` This is not really a factory as a factory design pattern is about producing several instances. In the current usage, we are only interested in one swap instance. Once the swap instance is created, the factory becomes useless. Hence, it is more of a builder pattern. --- swap/src/main.rs | 22 +++--- swap/src/protocol/alice.rs | 142 ++++++++++++++++++++---------------- swap/src/protocol/bob.rs | 125 +++++++++++++++++-------------- swap/tests/testutils/mod.rs | 26 +++---- 4 files changed, 175 insertions(+), 140 deletions(-) diff --git a/swap/src/main.rs b/swap/src/main.rs index b139e95b..676be61d 100644 --- a/swap/src/main.rs +++ b/swap/src/main.rs @@ -23,7 +23,7 @@ use swap::{ config::Config, database::Database, monero, - protocol::{alice, bob, bob::SwapFactory, StartingBalances}, + protocol::{alice, bob, bob::Builder}, trace::init_tracing, SwapAmounts, }; @@ -82,7 +82,7 @@ async fn main() -> Result<()> { send_monero, receive_bitcoin, swap_id ); - let alice_factory = alice::SwapFactory::new( + let alice_factory = alice::Builder::new( seed, config, swap_id, @@ -92,7 +92,8 @@ async fn main() -> Result<()> { listen_addr, ) .await; - let (swap, mut event_loop) = alice_factory.new_swap(swap_amounts).await?; + let (swap, mut event_loop) = + alice_factory.with_init_params(swap_amounts).build().await?; tokio::spawn(async move { event_loop.run().await }); alice::run(swap).await?; @@ -126,7 +127,7 @@ async fn main() -> Result<()> { send_bitcoin, receive_monero, swap_id ); - let bob_factory = SwapFactory::new( + let bob_factory = Builder::new( seed, db_path, swap_id, @@ -135,7 +136,10 @@ async fn main() -> Result<()> { alice_addr, alice_peer_id, ); - let (swap, event_loop) = bob_factory.new_swap(swap_amounts, config).await?; + let (swap, event_loop) = bob_factory + .with_init_params(swap_amounts, config) + .build() + .await?; tokio::spawn(async move { event_loop.run().await }); bob::run(swap).await?; @@ -169,7 +173,7 @@ async fn main() -> Result<()> { ) .await?; - let alice_factory = alice::SwapFactory::new( + let alice_factory = alice::Builder::new( seed, config, swap_id, @@ -179,7 +183,7 @@ async fn main() -> Result<()> { listen_addr, ) .await; - let (swap, mut event_loop) = alice_factory.resume().await?; + let (swap, mut event_loop) = alice_factory.build().await?; tokio::spawn(async move { event_loop.run().await }); alice::run(swap).await?; @@ -200,7 +204,7 @@ async fn main() -> Result<()> { ) .await?; - let bob_factory = SwapFactory::new( + let bob_factory = Builder::new( seed, db_path, swap_id, @@ -209,7 +213,7 @@ async fn main() -> Result<()> { alice_addr, alice_peer_id, ); - let (swap, event_loop) = bob_factory.resume().await?; + let (swap, event_loop) = bob_factory.build().await?; tokio::spawn(async move { event_loop.run().await }); bob::run(swap).await?; diff --git a/swap/src/protocol/alice.rs b/swap/src/protocol/alice.rs index 66950d59..2e27fdb9 100644 --- a/swap/src/protocol/alice.rs +++ b/swap/src/protocol/alice.rs @@ -24,10 +24,7 @@ pub use self::{ state::*, swap::{run, run_until}, }; -use crate::{ - config::Config, database::Database, network::transport::build, protocol::StartingBalances, - seed::Seed, -}; +use crate::{config::Config, database::Database, network::transport::build, seed::Seed}; use libp2p::{core::Multiaddr, identity::Keypair}; use rand::rngs::OsRng; use std::{path::PathBuf, sync::Arc}; @@ -53,7 +50,7 @@ pub struct Swap { pub db: Database, } -pub struct SwapFactory { +pub struct Builder { swap_id: Uuid, identity: Keypair, peer_id: PeerId, @@ -64,9 +61,16 @@ pub struct SwapFactory { bitcoin_wallet: Arc, monero_wallet: Arc, + + init_params: InitParams, } -impl SwapFactory { +enum InitParams { + None, + New { swap_amounts: SwapAmounts }, +} + +impl Builder { pub async fn new( seed: Seed, config: Config, @@ -89,71 +93,83 @@ impl SwapFactory { listen_address, bitcoin_wallet, monero_wallet, + init_params: InitParams::None, } } - pub async fn new_swap(self, swap_amounts: SwapAmounts) -> Result<(Swap, EventLoop)> { - let initial_state = init_alice_state( - swap_amounts.btc, - swap_amounts.xmr, - self.bitcoin_wallet.clone(), - self.config, - ) - .await?; - - let (event_loop, event_loop_handle) = init_alice_event_loop( - self.listen_address.clone(), - self.identity.clone(), - self.peer_id.clone(), - )?; - - let db = Database::open(self.db_path.as_path())?; - - Ok(( - Swap { - event_loop_handle, - bitcoin_wallet: self.bitcoin_wallet.clone(), - monero_wallet: self.monero_wallet.clone(), - config: self.config, - db, - state: initial_state, - swap_id: self.swap_id, - }, - event_loop, - )) + pub fn with_init_params(self, swap_amounts: SwapAmounts) -> Self { + Self { + init_params: InitParams::New { swap_amounts }, + ..self + } } - pub async fn resume(self) -> Result<(Swap, EventLoop)> { - // reopen the existing database - let db = Database::open(self.db_path.as_path())?; + pub async fn build(self) -> Result<(Swap, EventLoop)> { + match self.init_params { + InitParams::New { swap_amounts } => { + let initial_state = init_alice_state( + swap_amounts.btc, + swap_amounts.xmr, + self.bitcoin_wallet.clone(), + self.config, + ) + .await?; - let resume_state = if let database::Swap::Alice(state) = 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) = init_alice_event_loop( + self.listen_address.clone(), + self.identity.clone(), + self.peer_id.clone(), + )?; - let (event_loop, event_loop_handle) = init_alice_event_loop( - self.listen_address.clone(), - self.identity.clone(), - self.peer_id.clone(), - )?; + let db = Database::open(self.db_path.as_path())?; - Ok(( - Swap { - state: resume_state, - event_loop_handle, - bitcoin_wallet: self.bitcoin_wallet.clone(), - monero_wallet: self.monero_wallet.clone(), - config: self.config, - swap_id: self.swap_id, - db, - }, - event_loop, - )) + Ok(( + Swap { + event_loop_handle, + bitcoin_wallet: self.bitcoin_wallet.clone(), + monero_wallet: self.monero_wallet.clone(), + config: self.config, + db, + state: initial_state, + swap_id: self.swap_id, + }, + event_loop, + )) + } + InitParams::None => { + // reopen the existing database + let db = Database::open(self.db_path.as_path())?; + + let resume_state = + if let database::Swap::Alice(state) = 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) = init_alice_event_loop( + self.listen_address.clone(), + self.identity.clone(), + self.peer_id.clone(), + )?; + + Ok(( + Swap { + state: resume_state, + event_loop_handle, + bitcoin_wallet: self.bitcoin_wallet.clone(), + monero_wallet: self.monero_wallet.clone(), + config: self.config, + swap_id: self.swap_id, + db, + }, + event_loop, + )) + } + } } pub fn peer_id(&self) -> PeerId { diff --git a/swap/src/protocol/bob.rs b/swap/src/protocol/bob.rs index 2e12cf4e..d9cfc3fd 100644 --- a/swap/src/protocol/bob.rs +++ b/swap/src/protocol/bob.rs @@ -23,10 +23,7 @@ pub use self::{ state::*, swap::{run, run_until}, }; -use crate::{ - config::Config, database::Database, network::transport::build, protocol::StartingBalances, - seed::Seed, -}; +use crate::{config::Config, database::Database, network::transport::build, seed::Seed}; use libp2p::identity::Keypair; use rand::rngs::OsRng; use std::{path::PathBuf, sync::Arc}; @@ -50,7 +47,7 @@ pub struct Swap { pub swap_id: Uuid, } -pub struct SwapFactory { +pub struct Builder { swap_id: Uuid, identity: Keypair, peer_id: PeerId, @@ -61,9 +58,19 @@ pub struct SwapFactory { bitcoin_wallet: Arc, monero_wallet: Arc, + + init_params: InitParams, } -impl SwapFactory { +enum InitParams { + None, + New { + swap_amounts: SwapAmounts, + config: Config, + }, +} + +impl Builder { pub fn new( seed: Seed, db_path: PathBuf, @@ -85,63 +92,75 @@ impl SwapFactory { alice_peer_id, bitcoin_wallet, monero_wallet, + init_params: InitParams::None, } } - pub async fn new_swap( - self, - swap_amounts: SwapAmounts, - config: Config, - ) -> Result<(bob::Swap, bob::EventLoop)> { - let initial_state = self - .make_initial_state(swap_amounts.btc, swap_amounts.xmr, config) - .await?; - - let (event_loop, event_loop_handle) = self.init_event_loop()?; - - let db = Database::open(self.db_path.as_path())?; - - Ok(( - Swap { - state: initial_state, - event_loop_handle, - db, - bitcoin_wallet: self.bitcoin_wallet.clone(), - monero_wallet: self.monero_wallet.clone(), - swap_id: self.swap_id, + pub fn with_init_params(self, swap_amounts: SwapAmounts, config: Config) -> Self { + Self { + init_params: InitParams::New { + swap_amounts, + config, }, - event_loop, - )) + ..self + } } - pub async fn resume(self) -> Result<(bob::Swap, bob::EventLoop)> { - // reopen the existing database - let db = Database::open(self.db_path.as_path())?; + pub async fn build(self) -> Result<(bob::Swap, bob::EventLoop)> { + match self.init_params { + InitParams::New { + swap_amounts, + config, + } => { + let initial_state = self + .make_initial_state(swap_amounts.btc, swap_amounts.xmr, config) + .await?; - let resume_state = if let database::Swap::Bob(state) = 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()?; - let (event_loop, event_loop_handle) = self.init_event_loop()?; + let db = Database::open(self.db_path.as_path())?; - Ok(( - Swap { - state: resume_state, - event_loop_handle, - db, - bitcoin_wallet: self.bitcoin_wallet.clone(), - monero_wallet: self.monero_wallet.clone(), - swap_id: self.swap_id, - }, - event_loop, - )) + Ok(( + Swap { + state: initial_state, + event_loop_handle, + db, + bitcoin_wallet: self.bitcoin_wallet.clone(), + monero_wallet: self.monero_wallet.clone(), + swap_id: self.swap_id, + }, + event_loop, + )) + } + InitParams::None => { + // reopen the existing database + let db = Database::open(self.db_path.as_path())?; + + let resume_state = if let database::Swap::Bob(state) = 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()?; + + Ok(( + Swap { + state: resume_state, + event_loop_handle, + db, + bitcoin_wallet: self.bitcoin_wallet.clone(), + monero_wallet: self.monero_wallet.clone(), + swap_id: self.swap_id, + }, + event_loop, + )) + } + } } - fn init_event_loop( &self, ) -> Result<(bob::event_loop::EventLoop, bob::event_loop::EventLoopHandle)> { diff --git a/swap/tests/testutils/mod.rs b/swap/tests/testutils/mod.rs index 01a4fa80..9301f6bc 100644 --- a/swap/tests/testutils/mod.rs +++ b/swap/tests/testutils/mod.rs @@ -9,7 +9,7 @@ use swap::{ bitcoin, config::Config, monero, - protocol::{alice, alice::AliceState, bob, bob::BobState, StartingBalances}, + protocol::{alice, alice::AliceState, bob, bob::BobState}, seed::Seed, SwapAmounts, }; @@ -28,12 +28,12 @@ pub struct StartingBalances { pub struct TestContext { swap_amounts: SwapAmounts, - alice_swap_factory: Option, + alice_swap_factory: Option, alice_starting_balances: StartingBalances, alice_bitcoin_wallet: Arc, alice_monero_wallet: Arc, - bob_swap_factory: Option, + bob_swap_factory: Option, bob_starting_balances: StartingBalances, bob_bitcoin_wallet: Arc, bob_monero_wallet: Arc, @@ -45,7 +45,8 @@ impl TestContext { .alice_swap_factory .take() .unwrap() - .new_swap(self.swap_amounts) + .with_init_params(self.swap_amounts) + .build() .await .unwrap(); @@ -59,7 +60,8 @@ impl TestContext { .bob_swap_factory .take() .unwrap() - .new_swap(self.swap_amounts, Config::regtest()) + .with_init_params(self.swap_amounts, Config::regtest()) + .build() .await .unwrap(); @@ -73,7 +75,7 @@ impl TestContext { .alice_swap_factory .take() .unwrap() - .resume() + .build() .await .unwrap(); @@ -83,13 +85,7 @@ impl TestContext { } pub async fn recover_bob_from_db(&mut self) -> bob::Swap { - let (swap, event_loop) = self - .bob_swap_factory - .take() - .unwrap() - .resume() - .await - .unwrap(); + let (swap, event_loop) = self.bob_swap_factory.take().unwrap().build().await.unwrap(); tokio::spawn(async move { event_loop.run().await }); @@ -283,7 +279,7 @@ where ) .await; - let alice_swap_factory = alice::SwapFactory::new( + let alice_swap_factory = alice::Builder::new( Seed::random().unwrap(), config, Uuid::new_v4(), @@ -308,7 +304,7 @@ where ) .await; - let bob_swap_factory = bob::SwapFactory::new( + let bob_swap_factory = bob::Builder::new( Seed::random().unwrap(), tempdir().unwrap().path().to_path_buf(), Uuid::new_v4(),