feat (Cli): Display reason for failed cancel-refund operation to the user (#1668)

We now display the reason for a failed cancel-refund operation to the user.

Fixes #683

.github/workflows/ci.yml

@@ -157,6 +157,7 @@ jobs:
             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,
+            alice_manually_punishes_after_bob_dead_and_bob_cancels,
             punish,
             alice_punishes_after_restart_bob_dead,
             alice_manually_punishes_after_bob_dead,

CHANGELOG.md

@@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ## [Unreleased]
 
 - CLI: Buffer received transfer proofs for later processing if we're currently running a different swap
+- CLI: We now display the reason for a failed cancel-refund operation to the user (#683)
 
 ## [0.13.1] - 2024-06-10
 

swap/src/api/request.rs

@@ -821,6 +821,7 @@ impl Request {
             .await
             .map_err(|err| {
                 method_span.in_scope(|| {
+                    // The {:?} formatter is used to print the entire error chain
                     tracing::debug!(err = format!("{:?}", err), "API call resulted in an error");
                 });
                 err

swap/src/cli/cancel_and_refund.rs

@@ -1,5 +1,4 @@
-use crate::bitcoin::wallet::Subscription;
-use crate::bitcoin::{parse_rpc_error_code, RpcErrorCode, Wallet};
+use crate::bitcoin::{ExpiredTimelocks, Wallet};
 use crate::protocol::bob::BobState;
 use crate::protocol::Database;
 use anyhow::{bail, Result};
@@ -13,7 +12,7 @@ pub async fn cancel_and_refund(
     db: Arc<dyn Database + Send + Sync>,
 ) -> Result<BobState> {
     if let Err(err) = cancel(swap_id, bitcoin_wallet.clone(), db.clone()).await {
-        tracing::info!(%err, "Could not submit cancel transaction");
+        tracing::warn!(%err, "Could not cancel swap. Attempting to refund anyway");
     };
 
     let state = match refund(swap_id, bitcoin_wallet, db).await {
@@ -21,7 +20,6 @@ pub async fn cancel_and_refund(
         Err(e) => bail!(e),
     };
 
-    tracing::info!("Refund transaction submitted");
     Ok(state)
 }
 
@@ -29,7 +27,7 @@ pub async fn cancel(
     swap_id: Uuid,
     bitcoin_wallet: Arc<Wallet>,
     db: Arc<dyn Database + Send + Sync>,
-) -> Result<(Txid, Subscription, BobState)> {
+) -> Result<(Txid, BobState)> {
     let state = db.get_state(swap_id).await?.try_into()?;
 
     let state6 = match state {
@@ -47,34 +45,69 @@ pub async fn cancel(
         | BobState::XmrRedeemed { .. }
         | BobState::BtcPunished { .. }
         | BobState::SafelyAborted => bail!(
-            "Cannot cancel swap {} because it is in state {} which is not refundable.",
+            "Cannot cancel swap {} because it is in state {} which is not cancellable.",
             swap_id,
             state
         ),
     };
 
-    tracing::info!(%swap_id, "Manually cancelling swap");
+    tracing::info!(%swap_id, "Attempting to manually cancel 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);
+    // Attempt to just publish the cancel transaction
+    match state6.submit_tx_cancel(bitcoin_wallet.as_ref()).await {
+        Ok((txid, _)) => {
+            let state = BobState::BtcCancelled(state6);
+            db.insert_latest_state(swap_id, state.clone().into())
+                .await?;
+            Ok((txid, state))
         }
-    };
 
+        // If we fail to submit the cancel transaction it can have one of two reasons:
+        // 1. The cancel timelock hasn't expired yet
+        // 2. The cancel transaction has already been published by Alice
+        Err(err) => {
+            // Check if Alice has already published the cancel transaction while we were absent
+            if let Ok(tx) = state6.check_for_tx_cancel(bitcoin_wallet.as_ref()).await {
                 let state = BobState::BtcCancelled(state6);
                 db.insert_latest_state(swap_id, state.clone().into())
                     .await?;
+                tracing::info!("Alice has already cancelled the swap");
+                return Ok((tx.txid(), state));
+            }
 
-    Ok((txid, subscription, state))
+            // The cancel transaction has not been published yet and we failed to publish it ourselves
+            // Here we try to figure out why
+            match state6.expired_timelock(bitcoin_wallet.as_ref()).await {
+                // We cannot cancel because Alice has already cancelled and punished afterwards
+                Ok(ExpiredTimelocks::Punish { .. }) => {
+                    let state = BobState::BtcPunished {
+                        tx_lock_id: state6.tx_lock_id(),
+                    };
+                    db.insert_latest_state(swap_id, state.clone().into())
+                        .await?;
+                    tracing::info!("You have been punished for not refunding in time");
+                    bail!(err.context("Cannot cancel swap because we have already been punished"));
+                }
+                // We cannot cancel because the cancel timelock has not expired yet
+                Ok(ExpiredTimelocks::None { blocks_left }) => {
+                    bail!(err.context(
+                        format!(
+                            "Cannot cancel swap because the cancel timelock has not expired yet. Blocks left: {}",
+                            blocks_left
+                        )
+                    ));
+                }
+                Ok(ExpiredTimelocks::Cancel { .. }) => {
+                    bail!(err.context("Failed to cancel swap even though cancel timelock has expired. This is unexpected."));
+                }
+                Err(timelock_err) => {
+                    bail!(err
+                        .context(timelock_err)
+                        .context("Failed to cancel swap and could not check timelock status"));
+                }
+            }
+        }
+    }
 }
 
 pub async fn refund(
@@ -104,12 +137,51 @@ pub async fn refund(
         ),
     };
 
-    tracing::info!(%swap_id, "Manually refunding swap");
-    state6.publish_refund_btc(bitcoin_wallet.as_ref()).await?;
+    tracing::info!(%swap_id, "Attempting to manually refund swap");
 
+    // Attempt to just publish the refund transaction
+    match state6.publish_refund_btc(bitcoin_wallet.as_ref()).await {
+        Ok(_) => {
             let state = BobState::BtcRefunded(state6);
             db.insert_latest_state(swap_id, state.clone().into())
                 .await?;
 
             Ok(state)
+        }
+
+        // If we fail to submit the refund transaction it can have one of two reasons:
+        // 1. The cancel transaction has not been published yet
+        // 2. The refund timelock has already expired and we have been punished
+        Err(bitcoin_publication_err) => {
+            match state6.expired_timelock(bitcoin_wallet.as_ref()).await {
+                // We have been punished
+                Ok(ExpiredTimelocks::Punish { .. }) => {
+                    let state = BobState::BtcPunished {
+                        tx_lock_id: state6.tx_lock_id(),
+                    };
+                    db.insert_latest_state(swap_id, state.clone().into())
+                        .await?;
+                    tracing::info!("You have been punished for not refunding in time");
+                    bail!(bitcoin_publication_err
+                        .context("Cannot refund swap because we have already been punished"));
+                }
+                Ok(ExpiredTimelocks::None { blocks_left }) => {
+                    bail!(
+                        bitcoin_publication_err.context(format!(
+                            "Cannot refund swap because the cancel timelock has not expired yet. Blocks left: {}",
+                            blocks_left
+                        ))
+                    );
+                }
+                Ok(ExpiredTimelocks::Cancel { .. }) => {
+                    bail!(bitcoin_publication_err.context("Failed to refund swap even though cancel timelock has expired. This should is unexpected."));
+                }
+                Err(e) => {
+                    bail!(bitcoin_publication_err
+                        .context(e)
+                        .context("Failed to refund swap and could not check timelock status"));
+                }
+            }
+        }
+    }
 }

swap/src/protocol/bob/state.rs

@@ -646,18 +646,20 @@ impl State6 {
             tx_cancel_status,
         ))
     }
-
-    pub async fn check_for_tx_cancel(
-        &self,
-        bitcoin_wallet: &bitcoin::Wallet,
-    ) -> Result<Transaction> {
-        let tx_cancel = bitcoin::TxCancel::new(
+    pub fn construct_tx_cancel(&self) -> Result<bitcoin::TxCancel> {
+        bitcoin::TxCancel::new(
             &self.tx_lock,
             self.cancel_timelock,
             self.A,
             self.b.public(),
             self.tx_cancel_fee,
-        )?;
+        )
+    }
+    pub async fn check_for_tx_cancel(
+        &self,
+        bitcoin_wallet: &bitcoin::Wallet,
+    ) -> Result<Transaction> {
+        let tx_cancel = self.construct_tx_cancel()?;
 
         let tx = bitcoin_wallet.get_raw_transaction(tx_cancel.txid()).await?;
 
@@ -668,13 +670,8 @@ impl State6 {
         &self,
         bitcoin_wallet: &bitcoin::Wallet,
     ) -> Result<(Txid, Subscription)> {
-        let transaction = bitcoin::TxCancel::new(
-            &self.tx_lock,
-            self.cancel_timelock,
-            self.A,
-            self.b.public(),
-            self.tx_cancel_fee,
-        )?
+        let transaction = self
+            .construct_tx_cancel()?
             .complete_as_bob(self.A, self.b.clone(), self.tx_cancel_sig_a.clone())
             .context("Failed to complete Bitcoin cancel transaction")?;
 
@@ -691,13 +688,7 @@ impl State6 {
     }
 
     pub fn signed_refund_transaction(&self) -> Result<Transaction> {
-        let tx_cancel = bitcoin::TxCancel::new(
-            &self.tx_lock,
-            self.cancel_timelock,
-            self.A,
-            self.b.public(),
-            self.tx_cancel_fee,
-        )?;
+        let tx_cancel = self.construct_tx_cancel()?;
         let tx_refund =
             bitcoin::TxRefund::new(&tx_cancel, &self.refund_address, self.tx_refund_fee);
 

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

swap/tests/alice_and_bob_refund_using_cancel_and_refund_command_timelock_not_expired.rs

@@ -42,10 +42,10 @@ async fn given_alice_and_bob_manually_cancel_when_timelock_not_expired_errors()
         let error = cli::cancel(bob_swap.id, bob_swap.bitcoin_wallet, bob_swap.db)
             .await
             .unwrap_err();
-        assert_eq!(
-            parse_rpc_error_code(&error).unwrap(),
-            i64::from(RpcErrorCode::RpcVerifyRejected)
-        );
+
+        assert!(error
+            .to_string()
+            .contains("Cannot cancel swap because the cancel timelock has not expired yet"));
 
         ctx.restart_alice().await;
         let alice_swap = ctx.alice_next_swap().await;
@@ -72,10 +72,9 @@ async fn given_alice_and_bob_manually_cancel_when_timelock_not_expired_errors()
         let error = cli::refund(bob_swap.id, bob_swap.bitcoin_wallet, bob_swap.db)
             .await
             .unwrap_err();
-        assert_eq!(
-            parse_rpc_error_code(&error).unwrap(),
-            i64::from(RpcErrorCode::RpcVerifyError)
-        );
+        assert!(error
+            .to_string()
+            .contains("Cannot refund swap because the cancel timelock has not expired yet"));
 
         let (bob_swap, _) = ctx
             .stop_and_resume_bob_from_db(bob_join_handle, swap_id)

swap/tests/alice_manually_punishes_after_bob_dead_and_bob_cancels.rs

@@ -0,0 +1,98 @@
+pub mod harness;
+
+use harness::alice_run_until::is_xmr_lock_transaction_sent;
+use harness::bob_run_until::is_btc_locked;
+use harness::FastPunishConfig;
+use swap::asb;
+use swap::asb::FixedRate;
+use swap::cli;
+use swap::protocol::alice::AliceState;
+use swap::protocol::bob::BobState;
+use swap::protocol::{alice, bob};
+/// Bob locks Btc and Alice locks Xmr. Bob does not act; he fails to send Alice
+/// the encsig and fail to refund or redeem. Alice punishes using the cancel and
+/// punish command. Then Bob tries to refund.
+#[tokio::test]
+async fn alice_manually_punishes_after_bob_dead_and_bob_cancels() {
+    harness::setup_test(FastPunishConfig, |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_bitcoin_wallet = alice_swap.bitcoin_wallet.clone();
+
+        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??;
+
+        // Ensure cancel timelock is expired
+        if let AliceState::XmrLockTransactionSent { state3, .. } = alice_state {
+            alice_bitcoin_wallet
+                .subscribe_to(state3.tx_lock)
+                .await
+                .wait_until_confirmed_with(state3.cancel_timelock)
+                .await?;
+        } else {
+            panic!("Alice in unexpected state {}", alice_state);
+        }
+
+        // manual cancel (required to be able to punish)
+
+        ctx.restart_alice().await;
+        let alice_swap = ctx.alice_next_swap().await;
+        let (_, alice_state) =
+            asb::cancel(alice_swap.swap_id, alice_swap.bitcoin_wallet, alice_swap.db).await?;
+
+        // Ensure punish timelock is expired
+        if let AliceState::BtcCancelled { state3, .. } = alice_state {
+            alice_bitcoin_wallet
+                .subscribe_to(state3.tx_cancel())
+                .await
+                .wait_until_confirmed_with(state3.punish_timelock)
+                .await?;
+        } else {
+            panic!("Alice in unexpected state {}", alice_state);
+        }
+
+        // manual punish
+
+        ctx.restart_alice().await;
+        let alice_swap = ctx.alice_next_swap().await;
+        let (_, alice_state) =
+            asb::punish(alice_swap.swap_id, alice_swap.bitcoin_wallet, alice_swap.db).await?;
+        ctx.assert_alice_punished(alice_state).await;
+        // Bob is in wrong state.
+        let (bob_swap, bob_join_handle) = ctx
+            .stop_and_resume_bob_from_db(bob_join_handle, bob_swap_id)
+            .await;
+        assert!(matches!(bob_swap.state, BobState::BtcLocked { .. }));
+        bob_join_handle.abort();
+
+        let (_, state) = cli::cancel(bob_swap_id, bob_swap.bitcoin_wallet, bob_swap.db).await?;
+        // Bob should be in BtcCancelled state now.
+        assert!(matches!(state, BobState::BtcCancelled { .. }));
+
+        let (bob_swap, _) = ctx
+            .stop_and_resume_bob_from_db(bob_join_handle, bob_swap_id)
+            .await;
+        assert!(matches!(bob_swap.state, BobState::BtcCancelled { .. }));
+        // Alice punished Bob, so he should be in the BtcPunished state.
+        let error = cli::refund(bob_swap_id, bob_swap.bitcoin_wallet, bob_swap.db)
+            .await
+            .unwrap_err();
+        assert_eq!(
+            error.to_string(),
+            "Cannot refund swap because we have already been punished"
+        );
+        Ok(())
+    })
+    .await;
+}