From 8a7d746e96c4e108bbfb69f0c33037d325c4cc91 Mon Sep 17 00:00:00 2001 From: Daniel Karzel Date: Tue, 19 Jan 2021 12:43:20 +1100 Subject: [PATCH] Wait for Bob's refund finality For Alice we ensure to wait for redeem/punish finality, so it should be the same for Bob. --- swap/src/main.rs | 7 ++-- swap/src/protocol/bob.rs | 27 ++++++++-------- swap/src/protocol/bob/state.rs | 51 ++++++++++++------------------ swap/src/protocol/bob/swap.rs | 13 +++++++- swap/tests/refund_restart_alice.rs | 8 ++--- swap/tests/testutils/mod.rs | 5 ++- 6 files changed, 54 insertions(+), 57 deletions(-) diff --git a/swap/src/main.rs b/swap/src/main.rs index 770b95f5..fdad3b55 100644 --- a/swap/src/main.rs +++ b/swap/src/main.rs @@ -135,11 +135,9 @@ async fn main() -> Result<()> { Arc::new(monero_wallet), alice_addr, alice_peer_id, + config, ); - let (swap, event_loop) = bob_factory - .with_init_params(swap_amounts, config) - .build() - .await?; + let (swap, event_loop) = bob_factory.with_init_params(swap_amounts).build().await?; tokio::spawn(async move { event_loop.run().await }); bob::run(swap).await?; @@ -212,6 +210,7 @@ async fn main() -> Result<()> { Arc::new(monero_wallet), alice_addr, alice_peer_id, + config, ); let (swap, event_loop) = bob_factory.build().await?; diff --git a/swap/src/protocol/bob.rs b/swap/src/protocol/bob.rs index d9cfc3fd..173f49eb 100644 --- a/swap/src/protocol/bob.rs +++ b/swap/src/protocol/bob.rs @@ -44,6 +44,7 @@ pub struct Swap { pub db: Database, pub bitcoin_wallet: Arc, pub monero_wallet: Arc, + pub config: Config, pub swap_id: Uuid, } @@ -60,17 +61,16 @@ pub struct Builder { monero_wallet: Arc, init_params: InitParams, + config: Config, } enum InitParams { None, - New { - swap_amounts: SwapAmounts, - config: Config, - }, + New { swap_amounts: SwapAmounts }, } impl Builder { + #[allow(clippy::too_many_arguments)] pub fn new( seed: Seed, db_path: PathBuf, @@ -79,6 +79,7 @@ impl Builder { monero_wallet: Arc, alice_address: Multiaddr, alice_peer_id: PeerId, + config: Config, ) -> Self { let identity = network::Seed::new(seed).derive_libp2p_identity(); let peer_id = identity.public().into_peer_id(); @@ -93,27 +94,22 @@ impl Builder { bitcoin_wallet, monero_wallet, init_params: InitParams::None, + config, } } - pub fn with_init_params(self, swap_amounts: SwapAmounts, config: Config) -> Self { + pub fn with_init_params(self, swap_amounts: SwapAmounts) -> Self { Self { - init_params: InitParams::New { - swap_amounts, - config, - }, + init_params: InitParams::New { swap_amounts }, ..self } } pub async fn build(self) -> Result<(bob::Swap, bob::EventLoop)> { match self.init_params { - InitParams::New { - swap_amounts, - config, - } => { + InitParams::New { swap_amounts } => { let initial_state = self - .make_initial_state(swap_amounts.btc, swap_amounts.xmr, config) + .make_initial_state(swap_amounts.btc, swap_amounts.xmr, self.config) .await?; let (event_loop, event_loop_handle) = self.init_event_loop()?; @@ -128,10 +124,12 @@ impl Builder { bitcoin_wallet: self.bitcoin_wallet.clone(), monero_wallet: self.monero_wallet.clone(), swap_id: self.swap_id, + config: self.config, }, event_loop, )) } + InitParams::None => { // reopen the existing database let db = Database::open(self.db_path.as_path())?; @@ -155,6 +153,7 @@ impl Builder { bitcoin_wallet: self.bitcoin_wallet.clone(), monero_wallet: self.monero_wallet.clone(), swap_id: self.swap_id, + config: self.config, }, event_loop, )) diff --git a/swap/src/protocol/bob/state.rs b/swap/src/protocol/bob/state.rs index 5ab1caea..5c8d348a 100644 --- a/swap/src/protocol/bob/state.rs +++ b/swap/src/protocol/bob/state.rs @@ -15,6 +15,7 @@ use crate::{ BroadcastSignedTransaction, BuildTxLockPsbt, GetBlockHeight, GetNetwork, GetRawTransaction, Transaction, TransactionBlockHeight, TxCancel, Txid, WatchForRawTransaction, }, + config::Config, monero, monero::monero_private_key, protocol::{alice, bob}, @@ -550,46 +551,34 @@ impl State4 { .await } - pub async fn refund_btc( - &self, - bitcoin_wallet: &W, - ) -> Result<()> { + pub async fn refund_btc(&self, bitcoin_wallet: &W, config: Config) -> Result<()> + where + W: bitcoin::BroadcastSignedTransaction + bitcoin::WaitForTransactionFinality, + { let tx_cancel = bitcoin::TxCancel::new(&self.tx_lock, self.cancel_timelock, self.A, self.b.public()); 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 adaptor = Adaptor::>::default(); - let signed_tx_cancel = tx_cancel.clone().add_signatures( - &self.tx_lock, - (self.A, sig_a), - (self.b.public(), sig_b), - )?; + let sig_b = self.b.sign(tx_refund.digest()); + let sig_a = + adaptor.decrypt_signature(&self.s_b.into_secp256k1(), self.tx_refund_encsig.clone()); - let _ = bitcoin_wallet - .broadcast_signed_transaction(signed_tx_cancel) - .await?; - } + let signed_tx_refund = tx_refund.add_signatures( + &tx_cancel.clone(), + (self.A, sig_a), + (self.b.public(), sig_b), + )?; - { - let adaptor = Adaptor::>::default(); + let txid = bitcoin_wallet + .broadcast_signed_transaction(signed_tx_refund) + .await?; - let sig_b = self.b.sign(tx_refund.digest()); - let sig_a = adaptor - .decrypt_signature(&self.s_b.into_secp256k1(), self.tx_refund_encsig.clone()); + bitcoin_wallet + .wait_for_transaction_finality(txid, config) + .await?; - let signed_tx_refund = tx_refund.add_signatures( - &tx_cancel.clone(), - (self.A, sig_a), - (self.b.public(), sig_b), - )?; - - let _ = bitcoin_wallet - .broadcast_signed_transaction(signed_tx_refund) - .await?; - } Ok(()) } diff --git a/swap/src/protocol/bob/swap.rs b/swap/src/protocol/bob/swap.rs index 1eafc2b7..07e0be2e 100644 --- a/swap/src/protocol/bob/swap.rs +++ b/swap/src/protocol/bob/swap.rs @@ -8,6 +8,7 @@ use uuid::Uuid; use crate::{ bitcoin, + config::Config, database::{Database, Swap}, monero, protocol::bob::{self, event_loop::EventLoopHandle, state::*}, @@ -54,6 +55,7 @@ pub async fn run_until( swap.monero_wallet, OsRng, swap.swap_id, + swap.config, ) .await } @@ -70,6 +72,7 @@ async fn run_until_internal( monero_wallet: Arc, mut rng: R, swap_id: Uuid, + config: Config, ) -> Result where R: RngCore + CryptoRng + Send, @@ -103,6 +106,7 @@ where monero_wallet, rng, swap_id, + config, ) .await } @@ -124,6 +128,7 @@ where monero_wallet, rng, swap_id, + config, ) .await } @@ -189,6 +194,7 @@ where monero_wallet, rng, swap_id, + config, ) .await } @@ -230,6 +236,7 @@ where monero_wallet, rng, swap_id, + config, ) .await } @@ -265,6 +272,7 @@ where monero_wallet, rng, swap_id, + config, ) .await } @@ -286,6 +294,7 @@ where monero_wallet, rng, swap_id, + config, ) .await } @@ -311,6 +320,7 @@ where monero_wallet, rng, swap_id, + config, ) .await } @@ -321,7 +331,7 @@ where bail!("Internal error: canceled state reached before cancel timelock was expired"); } ExpiredTimelocks::Cancel => { - state.refund_btc(bitcoin_wallet.as_ref()).await?; + state.refund_btc(bitcoin_wallet.as_ref(), config).await?; BobState::BtcRefunded(state) } ExpiredTimelocks::Punish => BobState::BtcPunished { @@ -340,6 +350,7 @@ where monero_wallet, rng, swap_id, + config, ) .await } diff --git a/swap/tests/refund_restart_alice.rs b/swap/tests/refund_restart_alice.rs index 55fc2abe..a4908c95 100644 --- a/swap/tests/refund_restart_alice.rs +++ b/swap/tests/refund_restart_alice.rs @@ -5,7 +5,7 @@ pub mod testutils; /// Bob locks btc and Alice locks xmr. Alice fails to act so Bob refunds. Alice /// then also refunds. #[tokio::test] -async fn given_alice_restarts_after_xmr_is_locked_abort_swap() { +async fn given_alice_restarts_after_xmr_is_locked_refund_swap() { testutils::setup_test(|mut ctx| async move { let alice_swap = ctx.new_swap_as_alice().await; let bob_swap = ctx.new_swap_as_bob().await; @@ -20,6 +20,7 @@ async fn given_alice_restarts_after_xmr_is_locked_abort_swap() { // Alice does not act, Bob refunds let bob_state = bob_handle.await.unwrap(); + ctx.assert_bob_refunded(bob_state.unwrap()).await; // Once bob has finished Alice is restarted and refunds as well let alice_swap = ctx.recover_alice_from_db().await; @@ -27,11 +28,6 @@ async fn given_alice_restarts_after_xmr_is_locked_abort_swap() { let alice_state = alice::run(alice_swap).await.unwrap(); - // TODO: The test passes like this, but the assertion should be done after Bob - // refunded, not at the end because this can cause side-effects! - // We have to properly wait for the refund tx's finality inside the assertion, - // which requires storing the refund_tx_id in the the state! - ctx.assert_bob_refunded(bob_state.unwrap()).await; ctx.assert_alice_refunded(alice_state).await; }) .await; diff --git a/swap/tests/testutils/mod.rs b/swap/tests/testutils/mod.rs index 7a452d4c..f26a1fa7 100644 --- a/swap/tests/testutils/mod.rs +++ b/swap/tests/testutils/mod.rs @@ -62,6 +62,7 @@ struct BobParams { monero_wallet: Arc, alice_address: Multiaddr, alice_peer_id: PeerId, + config: Config, } impl BobParams { @@ -74,6 +75,7 @@ impl BobParams { self.monero_wallet.clone(), self.alice_address.clone(), self.alice_peer_id.clone(), + self.config, ) } } @@ -112,7 +114,7 @@ impl TestContext { let (swap, event_loop) = self .bob_params .builder() - .with_init_params(self.swap_amounts, Config::regtest()) + .with_init_params(self.swap_amounts) .build() .await .unwrap(); @@ -357,6 +359,7 @@ where monero_wallet: bob_monero_wallet.clone(), alice_address: alice_params.listen_address.clone(), alice_peer_id: alice_params.peer_id().await, + config, }; let test = TestContext {