From 7a740e876e3ab250ce257eecdd393f84cd2e4c9c Mon Sep 17 00:00:00 2001
From: Daniel Karzel <daniel@comit.network>
Date: Tue, 22 Dec 2020 17:56:23 +1100
Subject: [PATCH] Remove tx_cancel broadcast from refund, is published in
 T1Expired

Tx_cancel is handled independent of refund in state T1Expired.
Refund should not try to broadcast tx_cancel.
---
 swap/src/bob/swap.rs | 12 ++++++++----
 xmr-btc/src/bob.rs   | 40 +++++++++++++---------------------------
 2 files changed, 21 insertions(+), 31 deletions(-)

diff --git a/swap/src/bob/swap.rs b/swap/src/bob/swap.rs
index f5ea9eeb..eed73f5c 100644
--- a/swap/src/bob/swap.rs
+++ b/swap/src/bob/swap.rs
@@ -10,7 +10,7 @@ use async_recursion::async_recursion;
 use rand::{CryptoRng, RngCore};
 use std::{convert::TryFrom, fmt, sync::Arc};
 use tokio::select;
-use tracing::info;
+use tracing::{info, warn};
 use uuid::Uuid;
 use xmr_btc::{
     bob::{self, State2},
@@ -395,17 +395,21 @@ where
                     Epoch::T0 => panic!("Cancelled before t1??? Something is really wrong"),
                     Epoch::T1 => {
                         // If T1 has expired but not T2 we must be able to refund
-                        state.refund_btc(bitcoin_wallet.as_ref()).await?;
+                        state.submit_tx_refund(bitcoin_wallet.as_ref()).await?;
                         BobState::BtcRefunded(state)
                     }
                     Epoch::T2 => {
                         // If T2 expired we still try to refund in case Alice has not acted.
                         // This is especially important for scenarios where Alice is offline
                         // indefinitely after starting the swap.
-                        let refund_result = state.refund_btc(bitcoin_wallet.as_ref()).await;
+                        info!("Trying to refund even though T2 has passed...");
+                        let refund_result = state.submit_tx_refund(bitcoin_wallet.as_ref()).await;
                         match refund_result {
                             Ok(()) => BobState::BtcRefunded(state),
-                            Err(_) => BobState::Punished,
+                            Err(err) => {
+                                warn!("Bob tried to refund after T2 has passed, but sending the refund tx errored with {}", err);
+                                BobState::Punished
+                            }
                         }
                     }
                 };
diff --git a/xmr-btc/src/bob.rs b/xmr-btc/src/bob.rs
index d0aa4049..2b2bf3ed 100644
--- a/xmr-btc/src/bob.rs
+++ b/xmr-btc/src/bob.rs
@@ -707,8 +707,7 @@ impl State4 {
     where
         W: GetRawTransaction,
     {
-        let tx_cancel =
-            bitcoin::TxCancel::new(&self.tx_lock, self.refund_timelock, self.A, self.b.public());
+        let tx_cancel = self.build_tx_cancel();
 
         let sig_a = self.tx_cancel_sig_a.clone();
         let sig_b = self.b.sign(tx_cancel.digest());
@@ -726,17 +725,16 @@ impl State4 {
         Ok(tx)
     }
 
-    pub async fn submit_tx_cancel<W>(&self, bitcoin_wallet: &W) -> Result<Txid>
+    pub async fn submit_tx_cancel<W>(&self, bitcoin_wallet: &W) -> Result<(TxCancel, Txid)>
     where
         W: BroadcastSignedTransaction,
     {
-        let tx_cancel =
-            bitcoin::TxCancel::new(&self.tx_lock, self.refund_timelock, self.A, self.b.public());
+        let tx_cancel = self.build_tx_cancel();
 
         let sig_a = self.tx_cancel_sig_a.clone();
         let sig_b = self.b.sign(tx_cancel.digest());
 
-        let tx_cancel = tx_cancel
+        let signed_tx_cancel = tx_cancel
             .clone()
             .add_signatures(&self.tx_lock, (self.A, sig_a), (self.b.public(), sig_b))
             .expect(
@@ -745,9 +743,14 @@ impl State4 {
             );
 
         let tx_id = bitcoin_wallet
-            .broadcast_signed_transaction(tx_cancel)
+            .broadcast_signed_transaction(signed_tx_cancel)
             .await?;
-        Ok(tx_id)
+
+        Ok((tx_cancel, tx_id))
+    }
+
+    pub fn build_tx_cancel(&self) -> TxCancel {
+        bitcoin::TxCancel::new(&self.tx_lock, self.refund_timelock, self.A, self.b.public())
     }
 
     pub async fn watch_for_redeem_btc<W>(&self, bitcoin_wallet: &W) -> Result<State5>
@@ -807,29 +810,12 @@ impl State4 {
         .await
     }
 
-    pub async fn refund_btc<W: bitcoin::BroadcastSignedTransaction>(
+    pub async fn submit_tx_refund<W: bitcoin::BroadcastSignedTransaction>(
         &self,
         bitcoin_wallet: &W,
     ) -> Result<()> {
-        let tx_cancel =
-            bitcoin::TxCancel::new(&self.tx_lock, self.refund_timelock, self.A, self.b.public());
+        let tx_cancel = self.build_tx_cancel();
         let tx_refund = bitcoin::TxRefund::new(&tx_cancel, &self.refund_address);
-
-        {
-            let sig_b = self.b.sign(tx_cancel.digest());
-            let sig_a = self.tx_cancel_sig_a.clone();
-
-            let signed_tx_cancel = tx_cancel.clone().add_signatures(
-                &self.tx_lock,
-                (self.A, sig_a),
-                (self.b.public(), sig_b),
-            )?;
-
-            let _ = bitcoin_wallet
-                .broadcast_signed_transaction(signed_tx_cancel)
-                .await?;
-        }
-
         {
             let adaptor = Adaptor::<Sha256, Deterministic<Sha256>>::default();