From 7e945a11c1f9c2f3e3f672e71cc5b13a51e9a271 Mon Sep 17 00:00:00 2001 From: patrini32 <171664803+patrini32@users.noreply.github.com> Date: Thu, 6 Jun 2024 16:45:38 +0300 Subject: [PATCH] Check if Alice already published tx_cancel/tx_punish if we get an error when trying to cancel/refund. Test: use cli::cancel and cli::refund to check if cli::cancel sets state to BtcCancelled --- swap/src/bitcoin/punish.rs | 4 +- swap/src/cli/cancel_and_refund.rs | 84 +++++++++---------- swap/src/protocol/bob/state.rs | 38 +++++++++ ...punishes_after_bob_dead_and_bob_cancels.rs | 15 +++- 4 files changed, 95 insertions(+), 46 deletions(-) diff --git a/swap/src/bitcoin/punish.rs b/swap/src/bitcoin/punish.rs index 9d687544..a2280dba 100644 --- a/swap/src/bitcoin/punish.rs +++ b/swap/src/bitcoin/punish.rs @@ -44,7 +44,9 @@ impl TxPunish { watch_script: punish_address.script_pubkey(), } } - + pub fn txid(&self) -> Txid { + self.inner.txid() + } pub fn digest(&self) -> Sighash { self.digest } diff --git a/swap/src/cli/cancel_and_refund.rs b/swap/src/cli/cancel_and_refund.rs index 65ee457b..d5844f78 100644 --- a/swap/src/cli/cancel_and_refund.rs +++ b/swap/src/cli/cancel_and_refund.rs @@ -1,4 +1,4 @@ -use crate::bitcoin::{parse_rpc_error_code, ExpiredTimelocks, RpcErrorCode, Wallet}; +use crate::bitcoin::{parse_rpc_error_code, RpcErrorCode, Wallet}; use crate::protocol::bob::BobState; use crate::protocol::Database; use anyhow::{bail, Result}; @@ -65,26 +65,27 @@ pub async fn cancel( Ok((txid, state)) } Err(err) => { + if state6 + .check_for_tx_cancel(bitcoin_wallet.as_ref()) + .await + .is_ok() + { + // Alice already cancelled, so we are out-of-sync with Alice. + let txid = state6 + // Construct tx_cancel without broadcasting to the network, because swap has already been cancelled by Alice. + .construct_tx_cancel() + .expect("Error when constructing tx_cancel") + .txid(); + let state = BobState::BtcCancelled(state6); // Set state to cancelled to sync with Alice. + db.insert_latest_state(swap_id, state.clone().into()) + .await?; + tracing::info!("Cancel transaction has already been confirmed on chain. The swap has therefore already been cancelled by Alice."); + return Ok((txid, state)); + } if let Ok(error_code) = parse_rpc_error_code(&err) { if error_code == i64::from(RpcErrorCode::RpcVerifyError) { - if let ExpiredTimelocks::Punish { .. } = // Check if timelock is punished. - state6.expired_timelock(bitcoin_wallet.as_ref()).await? - { - // Timelock expired and network rejected tx_cancel, so we are out-of-sync with Alice. (Assuming that there's no other possible states under these conditions) - let txid = state6 - // Construct tx_cancel without broadcasting to the network, because swap has already been cancelled by Alice. - .construct_tx_cancel() - .expect("Error when constructing tx_cancel") - .txid(); - let state = BobState::BtcCancelled(state6); // Set state to cancelled to sync with Alice. - db.insert_latest_state(swap_id, state.clone().into()) - .await?; - tracing::info!("Cancel transaction has already been confirmed on chain. The swap has therefore already been cancelled by Alice."); - return Ok((txid, state)); - } else { - tracing::debug!(%error_code, "parse rpc error"); - tracing::info!("General error trying to submit cancel transaction"); - } + tracing::debug!(%error_code, "parse rpc error"); + tracing::info!("General error trying to submit cancel transaction"); } } bail!(err); @@ -121,32 +122,31 @@ pub async fn refund( tracing::info!(%swap_id, "Manually refunding swap"); match state6.publish_refund_btc(bitcoin_wallet.as_ref()).await { - Ok(_) => (), - Err(error) => { - if let Ok(error_code) = parse_rpc_error_code(&error) { - if error_code == i64::from(RpcErrorCode::RpcVerifyError) { - // Check if timelock expired. - if let ExpiredTimelocks::Punish { .. } = - state6.expired_timelock(bitcoin_wallet.as_ref()).await? - { - // Timelock expired and network rejected refund, so we are out-of-sync with Alice and punished. (Assuming that there's no other possible states under these conditions) - let state = BobState::BtcPunished { - tx_lock_id: state6.tx_lock_id(), - }; - db.insert_latest_state(swap_id, state.clone().into()) - .await?; + Ok(_) => { + let state = BobState::BtcRefunded(state6); + db.insert_latest_state(swap_id, state.clone().into()) + .await?; - tracing::info!("Cannot refund because BTC is punished by Alice."); - return Ok(state); - } - } + Ok(state) + } + Err(error) => { + if state6 + .check_for_tx_punish(bitcoin_wallet.as_ref()) + .await + .is_ok() + { + // Alice already punished us, so we are out-of-sync with Alice and should set our state to the BtcPunished. + tracing::debug!("we are punished"); + let state = BobState::BtcPunished { + tx_lock_id: state6.tx_lock_id(), + }; + db.insert_latest_state(swap_id, state.clone().into()) + .await?; + + tracing::info!("Cannot refund because BTC is punished by Alice."); + return Ok(state); } bail!(error); } } - let state = BobState::BtcRefunded(state6); - db.insert_latest_state(swap_id, state.clone().into()) - .await?; - - Ok(state) } diff --git a/swap/src/protocol/bob/state.rs b/swap/src/protocol/bob/state.rs index 042257f8..cea537e0 100644 --- a/swap/src/protocol/bob/state.rs +++ b/swap/src/protocol/bob/state.rs @@ -347,6 +347,7 @@ impl State2 { punish_timelock: self.punish_timelock, refund_address: self.refund_address, redeem_address: self.redeem_address, + punish_address: self.punish_address, tx_lock: self.tx_lock.clone(), tx_cancel_sig_a: self.tx_cancel_sig_a, tx_refund_encsig: self.tx_refund_encsig, @@ -354,6 +355,7 @@ impl State2 { tx_redeem_fee: self.tx_redeem_fee, tx_refund_fee: self.tx_refund_fee, tx_cancel_fee: self.tx_cancel_fee, + tx_punish_fee: self.tx_punish_fee, }, self.tx_lock, )) @@ -373,6 +375,7 @@ pub struct State3 { punish_timelock: PunishTimelock, refund_address: bitcoin::Address, redeem_address: bitcoin::Address, + punish_address: bitcoin::Address, pub tx_lock: bitcoin::TxLock, tx_cancel_sig_a: Signature, tx_refund_encsig: bitcoin::EncryptedSignature, @@ -383,6 +386,8 @@ pub struct State3 { tx_refund_fee: bitcoin::Amount, #[serde(with = "::bitcoin::util::amount::serde::as_sat")] tx_cancel_fee: bitcoin::Amount, + #[serde(with = "::bitcoin::util::amount::serde::as_sat")] + tx_punish_fee: bitcoin::Amount, } impl State3 { @@ -411,6 +416,7 @@ impl State3 { punish_timelock: self.punish_timelock, refund_address: self.refund_address, redeem_address: self.redeem_address, + punish_address: self.punish_address, tx_lock: self.tx_lock, tx_cancel_sig_a: self.tx_cancel_sig_a, tx_refund_encsig: self.tx_refund_encsig, @@ -418,6 +424,7 @@ impl State3 { tx_redeem_fee: self.tx_redeem_fee, tx_refund_fee: self.tx_refund_fee, tx_cancel_fee: self.tx_cancel_fee, + tx_punish_fee: self.tx_punish_fee, } } @@ -429,11 +436,13 @@ impl State3 { cancel_timelock: self.cancel_timelock, punish_timelock: self.punish_timelock, refund_address: self.refund_address.clone(), + punish_address: self.punish_address.clone(), tx_lock: self.tx_lock.clone(), tx_cancel_sig_a: self.tx_cancel_sig_a.clone(), tx_refund_encsig: self.tx_refund_encsig.clone(), tx_refund_fee: self.tx_refund_fee, tx_cancel_fee: self.tx_cancel_fee, + tx_punish_fee: self.tx_punish_fee, } } @@ -476,6 +485,7 @@ pub struct State4 { punish_timelock: PunishTimelock, refund_address: bitcoin::Address, redeem_address: bitcoin::Address, + punish_address: bitcoin::Address, pub tx_lock: bitcoin::TxLock, tx_cancel_sig_a: Signature, tx_refund_encsig: bitcoin::EncryptedSignature, @@ -486,6 +496,8 @@ pub struct State4 { tx_refund_fee: bitcoin::Amount, #[serde(with = "::bitcoin::util::amount::serde::as_sat")] tx_cancel_fee: bitcoin::Amount, + #[serde(with = "::bitcoin::util::amount::serde::as_sat")] + tx_punish_fee: bitcoin::Amount, } impl State4 { @@ -574,11 +586,13 @@ impl State4 { cancel_timelock: self.cancel_timelock, punish_timelock: self.punish_timelock, refund_address: self.refund_address, + punish_address: self.punish_address, tx_lock: self.tx_lock, tx_cancel_sig_a: self.tx_cancel_sig_a, tx_refund_encsig: self.tx_refund_encsig, tx_refund_fee: self.tx_refund_fee, tx_cancel_fee: self.tx_cancel_fee, + tx_punish_fee: self.tx_punish_fee, } } } @@ -614,6 +628,7 @@ pub struct State6 { cancel_timelock: CancelTimelock, punish_timelock: PunishTimelock, refund_address: bitcoin::Address, + punish_address: bitcoin::Address, tx_lock: bitcoin::TxLock, tx_cancel_sig_a: Signature, tx_refund_encsig: bitcoin::EncryptedSignature, @@ -621,6 +636,8 @@ pub struct State6 { pub tx_refund_fee: bitcoin::Amount, #[serde(with = "::bitcoin::util::amount::serde::as_sat")] pub tx_cancel_fee: bitcoin::Amount, + #[serde(with = "::bitcoin::util::amount::serde::as_sat")] + pub tx_punish_fee: bitcoin::Amount, } impl State6 { @@ -663,6 +680,27 @@ impl State6 { Ok(tx) } + // Check if tx_punish already exists. + pub async fn check_for_tx_punish( + &self, + bitcoin_wallet: &bitcoin::Wallet, + ) -> Result { + let tx_cancel = bitcoin::TxCancel::new( + &self.tx_lock, + self.cancel_timelock, + self.A, + self.b.public(), + self.tx_cancel_fee, + )?; + let tx_punish = bitcoin::TxPunish::new( + &tx_cancel, + &self.punish_address, + self.punish_timelock, + self.tx_punish_fee, + ); + let tx = bitcoin_wallet.get_raw_transaction(tx_punish.txid()).await?; + Ok(tx) + } pub fn construct_tx_cancel(&self) -> Result { // Just construct the tx_cancel without broadcasting. bitcoin::TxCancel::new( diff --git a/swap/tests/alice_manually_punishes_after_bob_dead_and_bob_cancels.rs b/swap/tests/alice_manually_punishes_after_bob_dead_and_bob_cancels.rs index c7344883..d57dd783 100644 --- a/swap/tests/alice_manually_punishes_after_bob_dead_and_bob_cancels.rs +++ b/swap/tests/alice_manually_punishes_after_bob_dead_and_bob_cancels.rs @@ -70,12 +70,21 @@ async fn alice_manually_punishes_after_bob_dead_and_bob_cancels() { asb::punish(alice_swap.swap_id, alice_swap.bitcoin_wallet, alice_swap.db).await?; ctx.assert_alice_punished(alice_state).await; - let (bob_swap, _) = ctx + 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 is out of sync with Alice. - let state = - cli::cancel_and_refund(bob_swap_id, bob_swap.bitcoin_wallet, bob_swap.db).await?; // Bob tries to cancel and refund. + bob_join_handle.abort(); + + let (_, state) = cli::cancel(bob_swap_id, bob_swap.bitcoin_wallet, bob_swap.db).await?; + 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 { .. })); + + let state = cli::refund(bob_swap_id, bob_swap.bitcoin_wallet, bob_swap.db).await?; // Bob tries to refund. ctx.assert_bob_punished(state).await; // Bob should be in punished state now. (PR #1668 adds that behaviour.) Ok(()) })