Merge #102

102: Separate Buy/Sell for resume command r=D4nte a=D4nte

And other clean ups from #100.

Co-authored-by: Franck Royer <franck@coblox.tech>

.github/workflows/ci.yml

@@ -48,6 +48,8 @@ jobs:
         run: cargo clippy --workspace --all-targets --all-features -- -D warnings
 
   build_test:
+    env:
+      RUST_TEST_TASKS: 2
     strategy:
       matrix:
         target: [ x86_64-unknown-linux-gnu, x86_64-apple-darwin ]

swap/src/bin/swap.rs

@@ -23,7 +23,7 @@ use swap::{
     alice::swap::AliceState,
     bitcoin, bob,
     bob::swap::BobState,
-    cli::{Command, Options},
+    cli::{Command, Options, Resume},
     monero,
     network::transport::build,
     storage::Database,
@@ -173,18 +173,16 @@ async fn main() -> Result<()> {
             // Print the table to stdout
             table.printstd();
         }
-        Command::Resume {
+        Command::Resume(Resume::SellXmr {
             swap_id,
             bitcoind_url,
             bitcoin_wallet_name,
             monero_wallet_rpc_url,
             listen_addr,
-            alice_peer_id,
-            alice_addr,
-        } => {
+        }) => {
             let db_swap = db.get_state(swap_id)?;
 
-            if let Ok(alice_state) = AliceState::try_from(db_swap.clone()) {
+            let alice_state = AliceState::try_from(db_swap.clone())?;
             let (bitcoin_wallet, monero_wallet) = setup_wallets(
                 bitcoind_url,
                 bitcoin_wallet_name.as_str(),
@@ -202,7 +200,19 @@ async fn main() -> Result<()> {
                 db,
             )
             .await?;
-            } else if let Ok(bob_state) = BobState::try_from(db_swap) {
+        }
+        Command::Resume(Resume::BuyXmr {
+            swap_id,
+            bitcoind_url,
+            bitcoin_wallet_name,
+            monero_wallet_rpc_url,
+            alice_peer_id,
+            alice_addr,
+        }) => {
+            let db_swap = db.get_state(swap_id)?;
+
+            let bob_state = BobState::try_from(db_swap)?;
+
             let (bitcoin_wallet, monero_wallet) = setup_wallets(
                 bitcoind_url,
                 bitcoin_wallet_name.as_str(),
@@ -220,11 +230,8 @@ async fn main() -> Result<()> {
                 alice_addr,
             )
             .await?;
-            } else {
-                anyhow::bail!("Unable to construct swap state for swap with id {}")
-            }
-        }
         }
+    };
 
     Ok(())
 }
@@ -313,6 +320,6 @@ async fn bob_swap(
         swap_id,
     );
 
-    tokio::spawn(async move { event_loop.run().await });
+    tokio::spawn(event_loop.run());
     swap.await
 }

swap/src/cli.rs

@@ -60,16 +60,15 @@ pub enum Command {
         receive_monero: xmr_btc::monero::Amount,
     },
     History,
-    Resume {
+    Resume(Resume),
+}
+
+#[derive(structopt::StructOpt, Debug)]
+pub enum Resume {
+    SellXmr {
         #[structopt(long = "swap-id")]
         swap_id: Uuid,
 
-        #[structopt(long = "connect-peer-id")]
-        alice_peer_id: PeerId,
-
-        #[structopt(long = "connect-addr")]
-        alice_addr: Multiaddr,
-
         #[structopt(long = "bitcoind-rpc", default_value = "http://127.0.0.1:8332")]
         bitcoind_url: Url,
 
@@ -82,11 +81,31 @@ pub enum Command {
         )]
         monero_wallet_rpc_url: Url,
 
-        // TODO: The listen address is only relevant for Alice, but should be role independent
-        //  see: https://github.com/comit-network/xmr-btc-swap/issues/77
-        #[structopt(long = "p2p-address", default_value = "/ip4/127.0.0.1/tcp/9876")]
+        #[structopt(long = "listen-address", default_value = "/ip4/127.0.0.1/tcp/9876")]
         listen_addr: Multiaddr,
     },
+    BuyXmr {
+        #[structopt(long = "swap-id")]
+        swap_id: Uuid,
+
+        #[structopt(long = "counterpart-peer-id")]
+        alice_peer_id: PeerId,
+
+        #[structopt(long = "counterpart-addr")]
+        alice_addr: Multiaddr,
+
+        #[structopt(long = "bitcoind-rpc", default_value = "http://127.0.0.1:8332")]
+        bitcoind_url: Url,
+
+        #[structopt(long = "bitcoin-wallet-name")]
+        bitcoin_wallet_name: String,
+
+        #[structopt(
+            long = "monero-wallet-rpc",
+            default_value = "http://127.0.0.1:18083/json_rpc"
+        )]
+        monero_wallet_rpc_url: Url,
+    },
 }
 
 fn parse_btc(str: &str) -> anyhow::Result<bitcoin::Amount> {

swap/tests/happy_path_restart_alice.rs

@@ -90,7 +90,7 @@ async fn given_alice_restarts_after_encsig_is_learned_resume_swap() {
     let alice_db = Database::open(alice_db_datadir.path()).unwrap();
 
     tokio::spawn(async move { alice_event_loop.run().await });
-    tokio::spawn(bob_fut);
+    let bob_swap_handle = tokio::spawn(bob_fut);
     tokio::spawn(bob_event_loop.run());
 
     let alice_swap_id = Uuid::new_v4();
@@ -119,7 +119,7 @@ async fn given_alice_restarts_after_encsig_is_learned_resume_swap() {
 
     let (mut event_loop_after_restart, event_loop_handle_after_restart) =
         testutils::init_alice_event_loop(alice_multiaddr);
-    let _alice_swarm_fut = tokio::spawn(async move { event_loop_after_restart.run().await });
+    tokio::spawn(async move { event_loop_after_restart.run().await });
 
     let db_swap = alice_db.get_state(alice_swap_id).unwrap();
     let resume_state = AliceState::try_from(db_swap).unwrap();
@@ -136,6 +136,9 @@ async fn given_alice_restarts_after_encsig_is_learned_resume_swap() {
     .await
     .unwrap();
 
+    // Wait for Bob to finish
+    bob_swap_handle.await.unwrap().unwrap();
+
     assert!(matches!(alice_state, AliceState::BtcRedeemed {..}));
 
     let btc_alice_final = alice_btc_wallet.as_ref().balance().await.unwrap();

swap/tests/happy_path_restart_bob_after_comm.rs

@@ -80,8 +80,7 @@ async fn given_bob_restarts_after_encsig_is_sent_resume_swap() {
     let bob_btc_wallet_clone = bob_btc_wallet.clone();
     let bob_xmr_wallet_clone = bob_xmr_wallet.clone();
 
-    let _ = tokio::spawn(async move {
-        alice::swap::swap(
+    let alice_swap_handle = tokio::spawn(alice::swap::swap(
         alice_state,
         alice_event_loop_handle,
         alice_btc_wallet.clone(),
@@ -89,13 +88,11 @@ async fn given_bob_restarts_after_encsig_is_sent_resume_swap() {
         config,
         Uuid::new_v4(),
         alice_db,
-        )
-        .await
-    });
+    ));
 
-    let _alice_swarm_fut = tokio::spawn(async move { alice_event_loop.run().await });
+    tokio::spawn(async move { alice_event_loop.run().await });
 
-    let _bob_swarm_fut = tokio::spawn(async move { bob_event_loop.run().await });
+    tokio::spawn(bob_event_loop.run());
 
     let bob_swap_id = Uuid::new_v4();
     let bob_db_datadir = tempdir().unwrap();
@@ -125,7 +122,7 @@ async fn given_bob_restarts_after_encsig_is_sent_resume_swap() {
 
     let (event_loop_after_restart, event_loop_handle_after_restart) =
         testutils::init_bob_event_loop(alice_peer_id, alice_multiaddr);
-    let _bob_swarm_fut = tokio::spawn(async move { event_loop_after_restart.run().await });
+    tokio::spawn(event_loop_after_restart.run());
 
     let db_swap = bob_db.get_state(bob_swap_id).unwrap();
     let resume_state = BobState::try_from(db_swap).unwrap();
@@ -142,6 +139,9 @@ async fn given_bob_restarts_after_encsig_is_sent_resume_swap() {
     .await
     .unwrap();
 
+    // Wait for Alice to finish too
+    alice_swap_handle.await.unwrap().unwrap();
+
     assert!(matches!(bob_state, BobState::XmrRedeemed {..}));
 
     let btc_alice_final = alice_btc_wallet_clone.as_ref().balance().await.unwrap();

swap/tests/refund_restart_alice.rs

@@ -100,7 +100,7 @@ async fn given_alice_restarts_after_xmr_is_locked_abort_swap() {
         )
     };
 
-    tokio::spawn(async move { bob_event_loop.run().await });
+    tokio::spawn(bob_event_loop.run());
 
     // We are selecting with alice_event_loop_1 so that we stop polling on it once
     // the try_join is finished.