Merge #1263

1263: feat(swap): merge cancel/refund commands into one command r=binarybaron a=delta1

Merges the `cancel`/`refund` commands in the swap cli into one `cancel-and-refund` command

closes #1020  



Co-authored-by: Byron Hambly <bhambly@blockstream.com>

.github/workflows/ci.yml

@@ -134,6 +134,7 @@ jobs:
             happy_path_restart_bob_before_xmr_locked,
             happy_path_restart_alice_after_xmr_locked,
             alice_and_bob_refund_using_cancel_and_refund_command,
+            alice_and_bob_refund_using_cancel_then_refund_command,
             alice_and_bob_refund_using_cancel_and_refund_command_timelock_not_expired,
             punish,
             alice_punishes_after_restart_bob_dead,

CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Changed
+
+- Swap: merge separate cancel/refund commands into one `cancel-and-refund` command for stuck swaps
+
 ## [0.12.0] - 2022-12-31
 
 ### Changed

bors.toml

@@ -13,6 +13,7 @@ status = [
     "docker_tests (happy_path_restart_alice_after_xmr_locked)",
     "docker_tests (happy_path_restart_bob_before_xmr_locked)",
     "docker_tests (alice_and_bob_refund_using_cancel_and_refund_command)",
+    "docker_tests (alice_and_bob_refund_using_cancel_then_refund_command)",
     "docker_tests (alice_and_bob_refund_using_cancel_and_refund_command_timelock_not_expired)",
     "docker_tests (punish)",
     "docker_tests (alice_punishes_after_restart_bob_dead)",

swap/src/bin/swap.rs

@@ -324,7 +324,7 @@ async fn main() -> Result<()> {
                 }
             }
         }
-        Command::Cancel {
+        Command::CancelAndRefund {
             swap_id,
             bitcoin_electrum_rpc_url,
             bitcoin_target_block,
@@ -344,30 +344,7 @@ async fn main() -> Result<()> {
             )
             .await?;
 
-            let (txid, _) = cli::cancel(swap_id, Arc::new(bitcoin_wallet), db).await?;
-            tracing::debug!("Cancel transaction successfully published with id {}", txid);
-        }
-        Command::Refund {
-            swap_id,
-            bitcoin_electrum_rpc_url,
-            bitcoin_target_block,
-        } => {
-            cli::tracing::init(debug, json, data_dir.join("logs"), Some(swap_id))?;
-
-            let db = open_db(data_dir.join("sqlite")).await?;
-            let seed = Seed::from_file_or_generate(data_dir.as_path())
-                .context("Failed to read in seed file")?;
-
-            let bitcoin_wallet = init_bitcoin_wallet(
-                bitcoin_electrum_rpc_url,
-                &seed,
-                data_dir,
-                env_config,
-                bitcoin_target_block,
-            )
-            .await?;
-
-            cli::refund(swap_id, Arc::new(bitcoin_wallet), db).await?;
+            cli::cancel_and_refund(swap_id, Arc::new(bitcoin_wallet), db).await?;
         }
         Command::ListSellers {
             rendezvous_point,
@@ -523,6 +500,7 @@ async fn init_bitcoin_wallet(
     env_config: Config,
     bitcoin_target_block: usize,
 ) -> Result<bitcoin::Wallet> {
+    tracing::debug!("Initializing bitcoin wallet");
     let xprivkey = seed.derive_extended_private_key(env_config.bitcoin_network)?;
 
     let wallet = bitcoin::Wallet::new(
@@ -535,6 +513,7 @@ async fn init_bitcoin_wallet(
     .await
     .context("Failed to initialize Bitcoin wallet")?;
 
+    tracing::debug!("Syncing bitcoin wallet");
     wallet.sync().await?;
 
     Ok(wallet)

swap/src/cli.rs

@@ -1,17 +1,15 @@
 mod behaviour;
-pub mod cancel;
+pub mod cancel_and_refund;
 pub mod command;
 mod event_loop;
 mod list_sellers;
-pub mod refund;
 pub mod tracing;
 pub mod transport;
 
 pub use behaviour::{Behaviour, OutEvent};
-pub use cancel::cancel;
+pub use cancel_and_refund::{cancel, cancel_and_refund, refund};
 pub use event_loop::{EventLoop, EventLoopHandle};
 pub use list_sellers::{list_sellers, Seller, Status as SellerStatus};
-pub use refund::refund;
 
 #[cfg(test)]
 mod tests {

swap/src/cli/cancel.rs

@@ -1,56 +0,0 @@
-use crate::bitcoin::{parse_rpc_error_code, RpcErrorCode, Txid, Wallet};
-use crate::protocol::bob::BobState;
-use crate::protocol::Database;
-use anyhow::{bail, Result};
-use std::convert::TryInto;
-use std::sync::Arc;
-use uuid::Uuid;
-
-pub async fn cancel(
-    swap_id: Uuid,
-    bitcoin_wallet: Arc<Wallet>,
-    db: Arc<dyn Database>,
-) -> Result<(Txid, BobState)> {
-    let state = db.get_state(swap_id).await?.try_into()?;
-
-    let state6 = match state {
-        BobState::BtcLocked { state3, .. } => state3.cancel(),
-        BobState::XmrLockProofReceived { state, .. } => state.cancel(),
-        BobState::XmrLocked(state4) => state4.cancel(),
-        BobState::EncSigSent(state4) => state4.cancel(),
-        BobState::CancelTimelockExpired(state6) => state6,
-        BobState::BtcRefunded(state6) => state6,
-        BobState::BtcCancelled(state6) => state6,
-
-        BobState::Started { .. }
-        | BobState::SwapSetupCompleted(_)
-        | BobState::BtcRedeemed(_)
-        | BobState::XmrRedeemed { .. }
-        | BobState::BtcPunished { .. }
-        | BobState::SafelyAborted => bail!(
-            "Cannot cancel swap {} because it is in state {} which is not refundable.",
-            swap_id,
-            state
-        ),
-    };
-
-    tracing::info!(%swap_id, "Manually cancelling swap");
-
-    let txid = match state6.submit_tx_cancel(bitcoin_wallet.as_ref()).await {
-        Ok(txid) => txid,
-        Err(err) => {
-            if let Ok(code) = parse_rpc_error_code(&err) {
-                if code == i64::from(RpcErrorCode::RpcVerifyAlreadyInChain) {
-                    tracing::info!("Cancel transaction has already been confirmed on chain")
-                }
-            }
-            bail!(err);
-        }
-    };
-
-    let state = BobState::BtcCancelled(state6);
-    db.insert_latest_state(swap_id, state.clone().into())
-        .await?;
-
-    Ok((txid, state))
-}

swap/src/cli/cancel_and_refund.rs

@@ -0,0 +1,115 @@
+use crate::bitcoin::wallet::Subscription;
+use crate::bitcoin::{parse_rpc_error_code, RpcErrorCode, Wallet};
+use crate::protocol::bob::BobState;
+use crate::protocol::Database;
+use anyhow::{bail, Result};
+use bitcoin::Txid;
+use std::sync::Arc;
+use uuid::Uuid;
+
+pub async fn cancel_and_refund(
+    swap_id: Uuid,
+    bitcoin_wallet: Arc<Wallet>,
+    db: Arc<dyn Database>,
+) -> Result<BobState> {
+    if let Err(err) = cancel(swap_id, bitcoin_wallet.clone(), db.clone()).await {
+        tracing::info!(%err, "Could not submit cancel transaction");
+    };
+
+    let state = match refund(swap_id, bitcoin_wallet, db).await {
+        Ok(s) => s,
+        Err(e) => bail!(e),
+    };
+
+    tracing::info!("Refund transaction submitted");
+    Ok(state)
+}
+
+pub async fn cancel(
+    swap_id: Uuid,
+    bitcoin_wallet: Arc<Wallet>,
+    db: Arc<dyn Database>,
+) -> Result<(Txid, Subscription, BobState)> {
+    let state = db.get_state(swap_id).await?.try_into()?;
+
+    let state6 = match state {
+        BobState::BtcLocked { state3, .. } => state3.cancel(),
+        BobState::XmrLockProofReceived { state, .. } => state.cancel(),
+        BobState::XmrLocked(state4) => state4.cancel(),
+        BobState::EncSigSent(state4) => state4.cancel(),
+        BobState::CancelTimelockExpired(state6) => state6,
+        BobState::BtcRefunded(state6) => state6,
+        BobState::BtcCancelled(state6) => state6,
+
+        BobState::Started { .. }
+        | BobState::SwapSetupCompleted(_)
+        | BobState::BtcRedeemed(_)
+        | BobState::XmrRedeemed { .. }
+        | BobState::BtcPunished { .. }
+        | BobState::SafelyAborted => bail!(
+            "Cannot cancel swap {} because it is in state {} which is not refundable.",
+            swap_id,
+            state
+        ),
+    };
+
+    tracing::info!(%swap_id, "Manually cancelling swap");
+
+    let (txid, subscription) = match state6.submit_tx_cancel(bitcoin_wallet.as_ref()).await {
+        Ok(txid) => txid,
+        Err(err) => {
+            if let Ok(error_code) = parse_rpc_error_code(&err) {
+                tracing::debug!(%error_code, "parse rpc error");
+                if error_code == i64::from(RpcErrorCode::RpcVerifyAlreadyInChain) {
+                    tracing::info!("Cancel transaction has already been confirmed on chain");
+                } else if error_code == i64::from(RpcErrorCode::RpcVerifyError) {
+                    tracing::info!("General error trying to submit cancel transaction");
+                }
+            }
+            bail!(err);
+        }
+    };
+
+    let state = BobState::BtcCancelled(state6);
+    db.insert_latest_state(swap_id, state.clone().into())
+        .await?;
+
+    Ok((txid, subscription, state))
+}
+
+pub async fn refund(
+    swap_id: Uuid,
+    bitcoin_wallet: Arc<Wallet>,
+    db: Arc<dyn Database>,
+) -> Result<BobState> {
+    let state = db.get_state(swap_id).await?.try_into()?;
+
+    let state6 = match state {
+        BobState::BtcLocked { state3, .. } => state3.cancel(),
+        BobState::XmrLockProofReceived { state, .. } => state.cancel(),
+        BobState::XmrLocked(state4) => state4.cancel(),
+        BobState::EncSigSent(state4) => state4.cancel(),
+        BobState::CancelTimelockExpired(state6) => state6,
+        BobState::BtcCancelled(state6) => state6,
+        BobState::Started { .. }
+        | BobState::SwapSetupCompleted(_)
+        | BobState::BtcRedeemed(_)
+        | BobState::BtcRefunded(_)
+        | BobState::XmrRedeemed { .. }
+        | BobState::BtcPunished { .. }
+        | BobState::SafelyAborted => bail!(
+            "Cannot refund swap {} because it is in state {} which is not refundable.",
+            swap_id,
+            state
+        ),
+    };
+
+    tracing::info!(%swap_id, "Manually refunding swap");
+    state6.publish_refund_btc(bitcoin_wallet.as_ref()).await?;
+
+    let state = BobState::BtcRefunded(state6);
+    db.insert_latest_state(swap_id, state.clone().into())
+        .await?;
+
+    Ok(state)
+}

swap/src/cli/command.rs

@@ -184,7 +184,7 @@ where
                 },
             }
         }
-        RawCommand::Cancel {
+        RawCommand::CancelAndRefund {
             swap_id: SwapId { swap_id },
             bitcoin,
         } => {
@@ -196,26 +196,7 @@ where
                 debug,
                 json,
                 data_dir: data::data_dir_from(data, is_testnet)?,
-                cmd: Command::Cancel {
-                    swap_id,
-                    bitcoin_electrum_rpc_url,
-                    bitcoin_target_block,
-                },
-            }
-        }
-        RawCommand::Refund {
-            swap_id: SwapId { swap_id },
-            bitcoin,
-        } => {
-            let (bitcoin_electrum_rpc_url, bitcoin_target_block) =
-                bitcoin.apply_defaults(is_testnet)?;
-
-            Arguments {
-                env_config: env_config_from(is_testnet),
-                debug,
-                json,
-                data_dir: data::data_dir_from(data, is_testnet)?,
-                cmd: Command::Refund {
+                cmd: Command::CancelAndRefund {
                     swap_id,
                     bitcoin_electrum_rpc_url,
                     bitcoin_target_block,
@@ -297,12 +278,7 @@ pub enum Command {
         tor_socks5_port: u16,
         namespace: XmrBtcNamespace,
     },
-    Cancel {
-        swap_id: Uuid,
-        bitcoin_electrum_rpc_url: Url,
-        bitcoin_target_block: usize,
-    },
-    Refund {
+    CancelAndRefund {
         swap_id: Uuid,
         bitcoin_electrum_rpc_url: Url,
         bitcoin_target_block: usize,
@@ -422,18 +398,9 @@ enum RawCommand {
         #[structopt(flatten)]
         tor: Tor,
     },
-    /// Force submission of the cancel transaction overriding the protocol state
-    /// machine and blockheight checks (expert users only)
-    Cancel {
-        #[structopt(flatten)]
-        swap_id: SwapId,
-
-        #[structopt(flatten)]
-        bitcoin: Bitcoin,
-    },
-    /// Force submission of the refund transaction overriding the protocol state
-    /// machine and blockheight checks (expert users only)
-    Refund {
+    /// Force the submission of the cancel and refund transactions of a swap
+    #[structopt(aliases = &["cancel", "refund"])]
+    CancelAndRefund {
         #[structopt(flatten)]
         swap_id: SwapId,
 
@@ -1275,7 +1242,7 @@ mod tests {
                 debug: false,
                 json: false,
                 data_dir: data_dir_path_cli().join(TESTNET),
-                cmd: Command::Cancel {
+                cmd: Command::CancelAndRefund {
                     swap_id: Uuid::from_str(SWAP_ID).unwrap(),
                     bitcoin_electrum_rpc_url: Url::from_str(DEFAULT_ELECTRUM_RPC_URL_TESTNET)
                         .unwrap(),
@@ -1290,7 +1257,7 @@ mod tests {
                 debug: false,
                 json: false,
                 data_dir: data_dir_path_cli().join(MAINNET),
-                cmd: Command::Cancel {
+                cmd: Command::CancelAndRefund {
                     swap_id: Uuid::from_str(SWAP_ID).unwrap(),
                     bitcoin_electrum_rpc_url: Url::from_str(DEFAULT_ELECTRUM_RPC_URL).unwrap(),
                     bitcoin_target_block: DEFAULT_BITCOIN_CONFIRMATION_TARGET,
@@ -1304,7 +1271,7 @@ mod tests {
                 debug: false,
                 json: false,
                 data_dir: data_dir_path_cli().join(TESTNET),
-                cmd: Command::Refund {
+                cmd: Command::CancelAndRefund {
                     swap_id: Uuid::from_str(SWAP_ID).unwrap(),
                     bitcoin_electrum_rpc_url: Url::from_str(DEFAULT_ELECTRUM_RPC_URL_TESTNET)
                         .unwrap(),
@@ -1319,7 +1286,7 @@ mod tests {
                 debug: false,
                 json: false,
                 data_dir: data_dir_path_cli().join(MAINNET),
-                cmd: Command::Refund {
+                cmd: Command::CancelAndRefund {
                     swap_id: Uuid::from_str(SWAP_ID).unwrap(),
                     bitcoin_electrum_rpc_url: Url::from_str(DEFAULT_ELECTRUM_RPC_URL).unwrap(),
                     bitcoin_target_block: DEFAULT_BITCOIN_CONFIRMATION_TARGET,

swap/src/cli/refund.rs

@@ -1,43 +0,0 @@
-use crate::bitcoin::Wallet;
-use crate::protocol::bob::BobState;
-use crate::protocol::Database;
-use anyhow::{bail, Result};
-use std::convert::TryInto;
-use std::sync::Arc;
-use uuid::Uuid;
-
-pub async fn refund(
-    swap_id: Uuid,
-    bitcoin_wallet: Arc<Wallet>,
-    db: Arc<dyn Database>,
-) -> Result<BobState> {
-    let state = db.get_state(swap_id).await?.try_into()?;
-
-    let state6 = match state {
-        BobState::BtcLocked { state3, .. } => state3.cancel(),
-        BobState::XmrLockProofReceived { state, .. } => state.cancel(),
-        BobState::XmrLocked(state4) => state4.cancel(),
-        BobState::EncSigSent(state4) => state4.cancel(),
-        BobState::CancelTimelockExpired(state6) => state6,
-        BobState::BtcCancelled(state6) => state6,
-        BobState::Started { .. }
-        | BobState::SwapSetupCompleted(_)
-        | BobState::BtcRedeemed(_)
-        | BobState::BtcRefunded(_)
-        | BobState::XmrRedeemed { .. }
-        | BobState::BtcPunished { .. }
-        | BobState::SafelyAborted => bail!(
-            "Cannot refund swap {} because it is in state {} which is not refundable.",
-            swap_id,
-            state
-        ),
-    };
-
-    state6.publish_refund_btc(bitcoin_wallet.as_ref()).await?;
-
-    let state = BobState::BtcRefunded(state6);
-    db.insert_latest_state(swap_id, state.clone().into())
-        .await?;
-
-    Ok(state)
-}

swap/src/protocol/bob/state.rs

@@ -1,4 +1,4 @@
-use crate::bitcoin::wallet::EstimateFeeRate;
+use crate::bitcoin::wallet::{EstimateFeeRate, Subscription};
 use crate::bitcoin::{
     self, current_epoch, CancelTimelock, ExpiredTimelocks, PunishTimelock, Transaction, TxCancel,
     TxLock, Txid,
@@ -641,7 +641,10 @@ impl State6 {
         Ok(tx)
     }
 
-    pub async fn submit_tx_cancel(&self, bitcoin_wallet: &bitcoin::Wallet) -> Result<Txid> {
+    pub async fn submit_tx_cancel(
+        &self,
+        bitcoin_wallet: &bitcoin::Wallet,
+    ) -> Result<(Txid, Subscription)> {
         let transaction = bitcoin::TxCancel::new(
             &self.tx_lock,
             self.cancel_timelock,
@@ -652,9 +655,9 @@ impl State6 {
         .complete_as_bob(self.A, self.b.clone(), self.tx_cancel_sig_a.clone())
         .context("Failed to complete Bitcoin cancel transaction")?;
 
-        let (tx_id, _) = bitcoin_wallet.broadcast(transaction, "cancel").await?;
+        let (tx_id, subscription) = bitcoin_wallet.broadcast(transaction, "cancel").await?;
 
-        Ok(tx_id)
+        Ok((tx_id, subscription))
     }
 
     pub async fn publish_refund_btc(&self, bitcoin_wallet: &bitcoin::Wallet) -> Result<()> {

swap/tests/alice_and_bob_refund_using_cancel_and_refund_command.rs

@@ -50,7 +50,7 @@ async fn given_alice_and_bob_manually_refund_after_funds_locked_both_refund() {
 
         // Bob manually cancels
         bob_join_handle.abort();
-        let (_, state) = cli::cancel(bob_swap.id, bob_swap.bitcoin_wallet, bob_swap.db).await?;
+        let (_, _, state) = cli::cancel(bob_swap.id, bob_swap.bitcoin_wallet, bob_swap.db).await?;
         assert!(matches!(state, BobState::BtcCancelled { .. }));
 
         let (bob_swap, bob_join_handle) = ctx
@@ -64,7 +64,7 @@ async fn given_alice_and_bob_manually_refund_after_funds_locked_both_refund() {
 
         ctx.assert_bob_refunded(bob_state).await;
 
-        // manually refund ALice's swap
+        // manually refund Alice's swap
         ctx.restart_alice().await;
         let alice_swap = ctx.alice_next_swap().await;
         let alice_state = asb::refund(

swap/tests/alice_and_bob_refund_using_cancel_then_refund_command.rs

@@ -0,0 +1,74 @@
+pub mod harness;
+
+use harness::alice_run_until::is_xmr_lock_transaction_sent;
+use harness::bob_run_until::is_btc_locked;
+use harness::FastCancelConfig;
+use swap::asb::FixedRate;
+use swap::protocol::alice::AliceState;
+use swap::protocol::bob::BobState;
+use swap::protocol::{alice, bob};
+use swap::{asb, cli};
+
+#[tokio::test]
+async fn given_alice_and_bob_manually_cancel_and_refund_after_funds_locked_both_refund() {
+    harness::setup_test(FastCancelConfig, |mut ctx| async move {
+        let (bob_swap, bob_join_handle) = ctx.bob_swap().await;
+        let bob_swap_id = bob_swap.id;
+        let bob_swap = tokio::spawn(bob::run_until(bob_swap, is_btc_locked));
+
+        let alice_swap = ctx.alice_next_swap().await;
+        let alice_swap = tokio::spawn(alice::run_until(
+            alice_swap,
+            is_xmr_lock_transaction_sent,
+            FixedRate::default(),
+        ));
+
+        let bob_state = bob_swap.await??;
+        assert!(matches!(bob_state, BobState::BtcLocked { .. }));
+
+        let alice_state = alice_swap.await??;
+        assert!(matches!(
+            alice_state,
+            AliceState::XmrLockTransactionSent { .. }
+        ));
+
+        let (bob_swap, bob_join_handle) = ctx
+            .stop_and_resume_bob_from_db(bob_join_handle, bob_swap_id)
+            .await;
+
+        // Ensure cancel timelock is expired
+        if let BobState::BtcLocked { state3, .. } = bob_swap.state.clone() {
+            bob_swap
+                .bitcoin_wallet
+                .subscribe_to(state3.tx_lock)
+                .await
+                .wait_until_confirmed_with(state3.cancel_timelock)
+                .await?;
+        } else {
+            panic!("Bob in unexpected state {}", bob_swap.state);
+        }
+
+        // Bob manually cancels and refunds
+        bob_join_handle.abort();
+        let bob_state =
+            cli::cancel_and_refund(bob_swap.id, bob_swap.bitcoin_wallet, bob_swap.db).await?;
+
+        ctx.assert_bob_refunded(bob_state).await;
+
+        // manually refund Alice's swap
+        ctx.restart_alice().await;
+        let alice_swap = ctx.alice_next_swap().await;
+        let alice_state = asb::refund(
+            alice_swap.swap_id,
+            alice_swap.bitcoin_wallet,
+            alice_swap.monero_wallet,
+            alice_swap.db,
+        )
+        .await?;
+
+        ctx.assert_alice_refunded(alice_state).await;
+
+        Ok(())
+    })
+    .await
+}