From 0c616c74379d572a343dead39cb5dfd5b38d03b7 Mon Sep 17 00:00:00 2001 From: Daniel Karzel Date: Thu, 29 Apr 2021 11:02:26 +1000 Subject: [PATCH] Move loading the state into the function In the production code it is a weird indirection that we load the state and then pass in the state and the database. In the tests we have one additional load by doing it inside the command, but loading from the db is not expensive. --- swap/src/bin/swap.rs | 8 ++---- swap/src/protocol/bob/cancel.rs | 3 +- swap/src/protocol/bob/refund.rs | 3 +- ...refunds_using_cancel_and_refund_command.rs | 20 +++---------- ...and_refund_command_timelock_not_expired.rs | 28 ++++++------------- ...fund_command_timelock_not_expired_force.rs | 24 ++++------------ 6 files changed, 24 insertions(+), 62 deletions(-) diff --git a/swap/src/bin/swap.rs b/swap/src/bin/swap.rs index bb43c1a9..ef337512 100644 --- a/swap/src/bin/swap.rs +++ b/swap/src/bin/swap.rs @@ -242,9 +242,7 @@ async fn main() -> Result<()> { ) .await?; - let resume_state = db.get_state(swap_id)?.try_into_bob()?.into(); - let cancel = - bob::cancel(swap_id, resume_state, Arc::new(bitcoin_wallet), db, force).await?; + let cancel = bob::cancel(swap_id, Arc::new(bitcoin_wallet), db, force).await?; match cancel { Ok((txid, _)) => { @@ -279,9 +277,7 @@ async fn main() -> Result<()> { ) .await?; - let resume_state = db.get_state(swap_id)?.try_into_bob()?.into(); - - bob::refund(swap_id, resume_state, Arc::new(bitcoin_wallet), db, force).await??; + bob::refund(swap_id, Arc::new(bitcoin_wallet), db, force).await??; } }; Ok(()) diff --git a/swap/src/protocol/bob/cancel.rs b/swap/src/protocol/bob/cancel.rs index ee764e79..9e667a5f 100644 --- a/swap/src/protocol/bob/cancel.rs +++ b/swap/src/protocol/bob/cancel.rs @@ -13,11 +13,12 @@ pub enum Error { pub async fn cancel( swap_id: Uuid, - state: BobState, bitcoin_wallet: Arc, db: Database, force: bool, ) -> Result> { + let state = db.get_state(swap_id)?.try_into_bob()?.into(); + let state6 = match state { BobState::BtcLocked(state3) => state3.cancel(), BobState::XmrLockProofReceived { state, .. } => state.cancel(), diff --git a/swap/src/protocol/bob/refund.rs b/swap/src/protocol/bob/refund.rs index 82168eb0..492f8191 100644 --- a/swap/src/protocol/bob/refund.rs +++ b/swap/src/protocol/bob/refund.rs @@ -11,11 +11,12 @@ pub struct SwapNotCancelledYet(Uuid); pub async fn refund( swap_id: Uuid, - state: BobState, bitcoin_wallet: Arc, db: Database, force: bool, ) -> Result> { + let state = db.get_state(swap_id)?.try_into_bob()?.into(); + let state6 = if force { match state { BobState::BtcLocked(state3) => state3.cancel(), diff --git a/swap/tests/bob_refunds_using_cancel_and_refund_command.rs b/swap/tests/bob_refunds_using_cancel_and_refund_command.rs index d218341e..50e99fd9 100644 --- a/swap/tests/bob_refunds_using_cancel_and_refund_command.rs +++ b/swap/tests/bob_refunds_using_cancel_and_refund_command.rs @@ -36,14 +36,8 @@ async fn given_bob_manually_refunds_after_btc_locked_bob_refunds() { // Bob manually cancels bob_join_handle.abort(); - let (_, state) = bob::cancel( - bob_swap.id, - bob_swap.state, - bob_swap.bitcoin_wallet, - bob_swap.db, - false, - ) - .await??; + let (_, state) = + bob::cancel(bob_swap.id, bob_swap.bitcoin_wallet, bob_swap.db, false).await??; assert!(matches!(state, BobState::BtcCancelled { .. })); let (bob_swap, bob_join_handle) = ctx @@ -53,14 +47,8 @@ async fn given_bob_manually_refunds_after_btc_locked_bob_refunds() { // Bob manually refunds bob_join_handle.abort(); - let bob_state = bob::refund( - bob_swap.id, - bob_swap.state, - bob_swap.bitcoin_wallet, - bob_swap.db, - false, - ) - .await??; + let bob_state = + bob::refund(bob_swap.id, bob_swap.bitcoin_wallet, bob_swap.db, false).await??; ctx.assert_bob_refunded(bob_state).await; diff --git a/swap/tests/bob_refunds_using_cancel_and_refund_command_timelock_not_expired.rs b/swap/tests/bob_refunds_using_cancel_and_refund_command_timelock_not_expired.rs index 5c8e0f20..152c74c7 100644 --- a/swap/tests/bob_refunds_using_cancel_and_refund_command_timelock_not_expired.rs +++ b/swap/tests/bob_refunds_using_cancel_and_refund_command_timelock_not_expired.rs @@ -25,16 +25,10 @@ async fn given_bob_manually_cancels_when_timelock_not_expired_errors() { assert!(matches!(bob_swap.state, BobState::BtcLocked { .. })); // Bob tries but fails to manually cancel - let result = bob::cancel( - bob_swap.id, - bob_swap.state, - bob_swap.bitcoin_wallet, - bob_swap.db, - false, - ) - .await? - .err() - .unwrap(); + let result = bob::cancel(bob_swap.id, bob_swap.bitcoin_wallet, bob_swap.db, false) + .await? + .err() + .unwrap(); assert!(matches!(result, Error::CancelTimelockNotExpiredYet)); @@ -44,16 +38,10 @@ async fn given_bob_manually_cancels_when_timelock_not_expired_errors() { assert!(matches!(bob_swap.state, BobState::BtcLocked { .. })); // Bob tries but fails to manually refund - bob::refund( - bob_swap.id, - bob_swap.state, - bob_swap.bitcoin_wallet, - bob_swap.db, - false, - ) - .await? - .err() - .unwrap(); + bob::refund(bob_swap.id, bob_swap.bitcoin_wallet, bob_swap.db, false) + .await? + .err() + .unwrap(); let (bob_swap, _) = ctx .stop_and_resume_bob_from_db(bob_join_handle, bob_swap_id) diff --git a/swap/tests/bob_refunds_using_cancel_and_refund_command_timelock_not_expired_force.rs b/swap/tests/bob_refunds_using_cancel_and_refund_command_timelock_not_expired_force.rs index df06416d..85f4de88 100644 --- a/swap/tests/bob_refunds_using_cancel_and_refund_command_timelock_not_expired_force.rs +++ b/swap/tests/bob_refunds_using_cancel_and_refund_command_timelock_not_expired_force.rs @@ -24,15 +24,9 @@ async fn given_bob_manually_forces_cancel_when_timelock_not_expired_errors() { assert!(matches!(bob_swap.state, BobState::BtcLocked { .. })); // Bob forces a cancel that will fail - let is_error = bob::cancel( - bob_swap.id, - bob_swap.state, - bob_swap.bitcoin_wallet, - bob_swap.db, - true, - ) - .await - .is_err(); + let is_error = bob::cancel(bob_swap.id, bob_swap.bitcoin_wallet, bob_swap.db, true) + .await + .is_err(); assert!(is_error); @@ -42,15 +36,9 @@ async fn given_bob_manually_forces_cancel_when_timelock_not_expired_errors() { assert!(matches!(bob_swap.state, BobState::BtcLocked { .. })); // Bob forces a refund that will fail - let is_error = bob::refund( - bob_swap.id, - bob_swap.state, - bob_swap.bitcoin_wallet, - bob_swap.db, - true, - ) - .await - .is_err(); + let is_error = bob::refund(bob_swap.id, bob_swap.bitcoin_wallet, bob_swap.db, true) + .await + .is_err(); assert!(is_error); let (bob_swap, _) = ctx