Some more fixes and comments after testing Alice's recovery

swap/src/bob.rs

@@ -167,6 +167,9 @@ pub async fn swap(
 
         info!("Resumed execution of generator, got: {:?}", state);
 
+        // TODO: Protect against transient errors
+        // TODO: Ignore transaction-already-in-block-chain errors
+
         match state {
             GeneratorState::Yielded(bob::Action::LockBtc(tx_lock)) => {
                 let signed_tx_lock = bitcoin_wallet.sign_tx_lock(tx_lock).await?;
@@ -184,8 +187,9 @@ pub async fn swap(
                 guard.0.send_message3(alice.clone(), tx_redeem_encsig);
                 info!("Sent Bitcoin redeem encsig");
 
-                // TODO: Does Bob need to wait for Alice to send an empty response, or can we
-                // just continue?
+                // FIXME: Having to wait for Alice's response here is a big problem, because
+                // we're stuck if she doesn't send her response back. I believe this is
+                // currently necessary, so we may have to rework this and/or how we use libp2p
                 match guard.0.next().shared().await {
                     OutEvent::Message3 => {
                         debug!("Got Message3 empty response");

swap/src/recover.rs

@@ -1,3 +1,15 @@
+//! This module is used to attempt to recover an unfinished swap.
+//!
+//! Recovery is only supported for certain states and the strategy followed is
+//! to perform the simplest steps that require no further action from the
+//! counterparty.
+//!
+//! The quality of this module is bad because there is a lot of code
+//! duplication, both within the module and with respect to
+//! `xmr_btc/src/{alice,bob}.rs`. In my opinion, a better approach to support
+//! swap recovery would be through the `action_generator`s themselves, but this
+//! was deemed too complicated for the time being.
+
 use crate::{
     monero::CreateWalletForOutput,
     state::{Alice, Bob, Swap},
@@ -257,7 +269,7 @@ pub async fn alice_recover(
                             TxPunish::new(&tx_cancel, &state.punish_address, state.punish_timelock);
 
                         let sig_a = state.a.sign(tx_punish.digest());
-                        let sig_b = state.tx_cancel_sig_bob.clone();
+                        let sig_b = state.tx_punish_sig_bob.clone();
 
                         let sig_tx_punish = tx_punish.add_signatures(
                             &tx_cancel,
@@ -282,8 +294,45 @@ pub async fn alice_recover(
                 state.a.public(),
                 state.B.clone(),
             );
+            let tx_refund = TxRefund::new(&tx_cancel, &state.refund_address);
 
-            let tx_punish = TxPunish::new(&tx_cancel, &state.punish_address, state.punish_timelock);
+            info!("Checking if Bitcoin has already been refunded");
+
+            // TODO: Protect against transient errors so that we can correctly decide if the
+            // bitcoin has been refunded
+            match bitcoin_wallet.0.get_raw_transaction(tx_refund.txid()).await {
+                Ok(tx_refund_published) => {
+                    info!("Bitcoin already refunded");
+
+                    let s_a = monero::PrivateKey {
+                        scalar: state.s_a.into_ed25519(),
+                    };
+
+                    let tx_refund_sig = tx_refund
+                        .extract_signature_by_key(tx_refund_published, state.a.public())?;
+                    let tx_refund_encsig = state
+                        .a
+                        .encsign(state.S_b_bitcoin.clone(), tx_refund.digest());
+
+                    let s_b = xmr_btc::bitcoin::recover(
+                        state.S_b_bitcoin,
+                        tx_refund_sig,
+                        tx_refund_encsig,
+                    )?;
+                    let s_b = monero::PrivateKey::from_scalar(
+                        xmr_btc::monero::Scalar::from_bytes_mod_order(s_b.to_bytes()),
+                    );
+
+                    monero_wallet
+                        .create_and_load_wallet_for_output(s_a + s_b, state.v)
+                        .await?;
+                    info!("Successfully refunded monero");
+                }
+                Err(_) => {
+                    info!("Bitcoin not yet refunded");
+
+                    let tx_punish =
+                        TxPunish::new(&tx_cancel, &state.punish_address, state.punish_timelock);
 
                     let sig_a = state.a.sign(tx_punish.digest());
                     let sig_b = state.tx_punish_sig_bob.clone();
@@ -299,6 +348,8 @@ pub async fn alice_recover(
                         .await?;
                     info!("Successfully punished Bob's inactivity by taking bitcoin");
                 }
+            }
+        }
         Alice::BtcRefunded {
             view_key,
             spend_key,

swap/tests/e2e.rs

@@ -1,5 +1,3 @@
-#[cfg(not(feature = "tor"))]
-mod e2e_test {
 use bitcoin_harness::Bitcoind;
 use futures::{channel::mpsc, future::try_join};
 use libp2p::Multiaddr;
@@ -16,12 +14,19 @@ mod e2e_test {
 
 #[tokio::test]
 async fn swap() {
+    use tracing_subscriber::util::SubscriberInitExt as _;
+    let _guard = tracing_subscriber::fmt()
+        .with_env_filter("swap=info,xmr_btc=info")
+        .with_ansi(false)
+        .set_default();
+
     let alice_multiaddr: Multiaddr = "/ip4/127.0.0.1/tcp/9876"
         .parse()
         .expect("failed to parse Alice's address");
 
     let cli = Cli::default();
     let bitcoind = Bitcoind::new(&cli, "0.19.1").unwrap();
+    dbg!(&bitcoind.node_url);
     let _ = bitcoind.init(5).await;
 
     let btc = bitcoin::Amount::from_sat(1_000_000);
@@ -66,8 +71,7 @@ mod e2e_test {
     let alice_behaviour = alice::Alice::default();
     let alice_transport = build(alice_behaviour.identity()).unwrap();
 
-        let db_dir = tempdir().unwrap();
-        let db = Database::open(db_dir.path()).unwrap();
+    let db = Database::open(std::path::Path::new("../.swap-db/")).unwrap();
     let alice_swap = alice::swap(
         alice_btc_wallet.clone(),
         alice_xmr_wallet.clone(),
@@ -117,4 +121,3 @@ mod e2e_test {
     assert!(xmr_alice_final.as_piconero() <= xmr_alice - xmr);
     assert_eq!(xmr_bob_final.as_piconero(), xmr_bob + xmr);
 }
-}