Merge #145

145: Make lock-tx id available in redeem/punish state to be able to assert exact fees r=da-kami a=da-kami

We can do exact assertions for Bob's redeem as well, but have to store Bob's tx_lock id in the respective final state. Make tx_lock available in BtcRedeemed and BtcPunished to have better assertions / harmonize test behaviour.

Storing this information is strictly speaking not needed for the production environment. But it is static information that can be seen as additional information that can be handy for a user. We could potentially extract it inside the tests as well (for redeem without restart would be a bit tricky), but I think this solution is more elegant. 

Co-authored-by: Daniel Karzel <daniel@comit.network>
Co-authored-by: Franck Royer <franck@coblox.tech>
Co-authored-by: bors[bot] <26634292+bors[bot]@users.noreply.github.com>

swap/src/database.rs

@@ -130,7 +130,7 @@ mod tests {
             .await
             .expect("Failed to save second state");
 
-        let state_2 = Swap::Bob(Bob::Done(BobEndState::XmrRedeemed));
+        let state_2 = Swap::Bob(Bob::Done(BobEndState::SafelyAborted));
         let swap_id_2 = Uuid::new_v4();
         db.insert_latest_state(swap_id_2, state_2.clone())
             .await
@@ -186,7 +186,7 @@ mod tests {
             .await
             .expect("Failed to save second state");
 
-        let state_2 = Swap::Bob(Bob::Done(BobEndState::BtcPunished));
+        let state_2 = Swap::Bob(Bob::Done(BobEndState::SafelyAborted));
         let swap_id_2 = Uuid::new_v4();
         db.insert_latest_state(swap_id_2, state_2.clone())
             .await

swap/src/database/bob.rs

@@ -33,9 +33,9 @@ pub enum Bob {
 #[derive(Clone, strum::Display, Debug, Deserialize, Serialize, PartialEq)]
 pub enum BobEndState {
     SafelyAborted,
-    XmrRedeemed,
+    XmrRedeemed { tx_lock_id: bitcoin::Txid },
     BtcRefunded(Box<bob::State4>),
-    BtcPunished,
+    BtcPunished { tx_lock_id: bitcoin::Txid },
 }
 
 impl From<BobState> for Bob {
@@ -50,8 +50,12 @@ impl From<BobState> for Bob {
             BobState::CancelTimelockExpired(state4) => Bob::CancelTimelockExpired(state4),
             BobState::BtcCancelled(state4) => Bob::BtcCancelled(state4),
             BobState::BtcRefunded(state4) => Bob::Done(BobEndState::BtcRefunded(Box::new(state4))),
-            BobState::XmrRedeemed => Bob::Done(BobEndState::XmrRedeemed),
+            BobState::XmrRedeemed { tx_lock_id } => {
-            BobState::BtcPunished => Bob::Done(BobEndState::BtcPunished),
+                Bob::Done(BobEndState::XmrRedeemed { tx_lock_id })
+            }
+            BobState::BtcPunished { tx_lock_id } => {
+                Bob::Done(BobEndState::BtcPunished { tx_lock_id })
+            }
             BobState::SafelyAborted => Bob::Done(BobEndState::SafelyAborted),
         }
     }
@@ -70,9 +74,9 @@ impl From<Bob> for BobState {
             Bob::BtcCancelled(state4) => BobState::BtcCancelled(state4),
             Bob::Done(end_state) => match end_state {
                 BobEndState::SafelyAborted => BobState::SafelyAborted,
-                BobEndState::XmrRedeemed => BobState::XmrRedeemed,
+                BobEndState::XmrRedeemed { tx_lock_id } => BobState::XmrRedeemed { tx_lock_id },
                 BobEndState::BtcRefunded(state4) => BobState::BtcRefunded(*state4),
-                BobEndState::BtcPunished => BobState::BtcPunished,
+                BobEndState::BtcPunished { tx_lock_id } => BobState::BtcPunished { tx_lock_id },
             },
         }
     }

swap/src/protocol/bob/state.rs

@@ -35,8 +35,12 @@ pub enum BobState {
     CancelTimelockExpired(State4),
     BtcCancelled(State4),
     BtcRefunded(State4),
-    XmrRedeemed,
+    XmrRedeemed {
-    BtcPunished,
+        tx_lock_id: bitcoin::Txid,
+    },
+    BtcPunished {
+        tx_lock_id: bitcoin::Txid,
+    },
     SafelyAborted,
 }
 
@@ -52,8 +56,8 @@ impl fmt::Display for BobState {
             BobState::CancelTimelockExpired(..) => write!(f, "cancel timelock is expired"),
             BobState::BtcCancelled(..) => write!(f, "btc is cancelled"),
             BobState::BtcRefunded(..) => write!(f, "btc is refunded"),
-            BobState::XmrRedeemed => write!(f, "xmr is redeemed"),
+            BobState::XmrRedeemed { .. } => write!(f, "xmr is redeemed"),
-            BobState::BtcPunished => write!(f, "btc is punished"),
+            BobState::BtcPunished { .. } => write!(f, "btc is punished"),
             BobState::SafelyAborted => write!(f, "safely aborted"),
         }
     }

swap/src/protocol/bob/swap.rs

@@ -45,8 +45,8 @@ pub fn is_complete(state: &BobState) -> bool {
     matches!(
         state,
         BobState::BtcRefunded(..)
-            | BobState::XmrRedeemed
+            | BobState::XmrRedeemed { .. }
-            | BobState::BtcPunished
+            | BobState::BtcPunished { .. }
             | BobState::SafelyAborted
     )
 }
@@ -277,7 +277,9 @@ where
                 // Bob redeems XMR using revealed s_a
                 state.claim_xmr(monero_wallet.as_ref()).await?;
 
-                let state = BobState::XmrRedeemed;
+                let state = BobState::XmrRedeemed {
+                    tx_lock_id: state.tx_lock_id(),
+                };
                 let db_state = state.clone().into();
                 db.insert_latest_state(swap_id, Swap::Bob(db_state)).await?;
                 run_until(
@@ -327,7 +329,9 @@ where
                         state.refund_btc(bitcoin_wallet.as_ref()).await?;
                         BobState::BtcRefunded(state)
                     }
-                    ExpiredTimelocks::Punish => BobState::BtcPunished,
+                    ExpiredTimelocks::Punish => BobState::BtcPunished {
+                        tx_lock_id: state.tx_lock_id(),
+                    },
                 };
 
                 let db_state = state.clone().into();
@@ -345,9 +349,9 @@ where
                 .await
             }
             BobState::BtcRefunded(state4) => Ok(BobState::BtcRefunded(state4)),
-            BobState::BtcPunished => Ok(BobState::BtcPunished),
+            BobState::BtcPunished { tx_lock_id } => Ok(BobState::BtcPunished { tx_lock_id }),
             BobState::SafelyAborted => Ok(BobState::SafelyAborted),
-            BobState::XmrRedeemed => Ok(BobState::XmrRedeemed),
+            BobState::XmrRedeemed { tx_lock_id } => Ok(BobState::XmrRedeemed { tx_lock_id }),
         }
     }
 }

swap/tests/punish.rs

@@ -45,13 +45,6 @@ async fn alice_punishes_if_bob_never_acts_after_fund() {
         let bob = bob_harness.recover_bob_from_db().await;
         assert!(matches!(bob.state, BobState::BtcLocked {..}));
 
-        // TODO: make lock-tx-id available in final states
-        let lock_tx_id = if let BobState::BtcLocked(state3) = bob_state {
-            state3.tx_lock_id()
-        } else {
-            panic!("Bob in unexpected state");
-        };
-
         let bob_state = bob::swap(
             bob.state,
             bob.event_loop_handle,
@@ -64,7 +57,7 @@ async fn alice_punishes_if_bob_never_acts_after_fund() {
         .await
         .unwrap();
 
-        bob_harness.assert_punished(bob_state, lock_tx_id).await;
+        bob_harness.assert_punished(bob_state).await;
     })
     .await;
 }

swap/tests/testutils/mod.rs

@@ -363,10 +363,23 @@ impl BobHarness {
     }
 
     pub async fn assert_redeemed(&self, state: BobState) {
-        assert!(matches!(state, BobState::XmrRedeemed));
+        let lock_tx_id = if let BobState::XmrRedeemed { tx_lock_id } = state {
+            tx_lock_id
+        } else {
+            panic!("Bob in unexpected state");
+        };
+
+        let lock_tx_bitcoin_fee = self
+            .bitcoin_wallet
+            .transaction_fee(lock_tx_id)
+            .await
+            .unwrap();
 
         let btc_balance_after_swap = self.bitcoin_wallet.as_ref().balance().await.unwrap();
-        assert!(btc_balance_after_swap <= self.starting_balances.btc - self.swap_amounts.btc);
+        assert_eq!(
+            btc_balance_after_swap,
+            self.starting_balances.btc - self.swap_amounts.btc - lock_tx_bitcoin_fee
+        );
 
         // Ensure that Bob's balance is refreshed as we use a newly created wallet
         self.monero_wallet.as_ref().inner.refresh().await.unwrap();
@@ -409,8 +422,12 @@ impl BobHarness {
         assert_eq!(xmr_balance_after_swap, self.starting_balances.xmr);
     }
 
-    pub async fn assert_punished(&self, state: BobState, lock_tx_id: ::bitcoin::Txid) {
+    pub async fn assert_punished(&self, state: BobState) {
-        assert!(matches!(state, BobState::BtcPunished));
+        let lock_tx_id = if let BobState::BtcPunished { tx_lock_id } = state {
+            tx_lock_id
+        } else {
+            panic!("Bob in unexpected state");
+        };
 
         let lock_tx_bitcoin_fee = self
             .bitcoin_wallet