From b1267e861842c794e26d084c337af090ddd9ecef Mon Sep 17 00:00:00 2001 From: TheCharlatan Date: Tue, 5 Jul 2022 00:14:32 +0200 Subject: [PATCH 1/8] Swap: Handle monero under- and overfunding --- src/swapd/runtime.rs | 35 ++++++++++++++++++++++++++++- src/swapd/swap_state.rs | 50 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 1 deletion(-) diff --git a/src/swapd/runtime.rs b/src/swapd/runtime.rs index e00b4a5e3..60072d5fa 100644 --- a/src/swapd/runtime.rs +++ b/src/swapd/runtime.rs @@ -748,7 +748,9 @@ impl Runtime { self.send_wallet(msg_bus, endpoints, request)?; } // alice receives, bob sends - Msg::BuyProcedureSignature(buy_proc_sig) if self.state.a_refundsig() => { + Msg::BuyProcedureSignature(buy_proc_sig) + if self.state.a_refundsig() && !self.state.a_overfunded() => + { // Alice verifies that she has sent refund procedure signatures before // processing the buy signatures from Bob let tx_label = TxLabel::Buy; @@ -1098,6 +1100,28 @@ impl Runtime { id, hash, amount, block, tx ); self.state.a_sup_refundsig_xmrlocked(); + + // if the funding amount does not match the expected, abort and wait for the swap to refund + let required_funding_amount = + self.state.a_required_funding_amount().unwrap(); + if amount.clone() < required_funding_amount { + // Alice still views underfunding as valid in the hope that Bob still passes her BuyProcSig + let msg = format!( + "Too small amount funded. Required: {}, Funded: {}. Do not fund this swap anymore, will attempt to refund.", + monero::Amount::from_pico(required_funding_amount), + monero::Amount::from_pico(amount.clone()) + ); + error!("{}", msg); + } else if amount.clone() > required_funding_amount { + self.state.a_sup_overfunded(); + let msg = format!( + "Too big amount funded. Required: {}, Funded: {}. Do not fund this swap anymore, will attempt to refund.", + monero::Amount::from_pico(required_funding_amount), + monero::Amount::from_pico(amount.clone()) + ); + error!("{}", msg); + } + let txlabel = TxLabel::AccLock; if !self.syncer_state.is_watched_tx(&txlabel) { if self.syncer_state.awaiting_funding { @@ -1519,6 +1543,7 @@ impl Runtime { amount.bright_green_bold(), address.addr(), ); + self.state.a_sup_required_funding_amount(amount); let funding_request = Request::FundingInfo( FundingInfo::Monero(MoneroFundingInfo { swap_id, @@ -1569,12 +1594,16 @@ impl Runtime { && self.state.a_refundsig() && !self.state.a_buy_published() && !self.state.cancel_seen() + && !self.state.a_overfunded() // don't publish buy in case we overfunded && self.txs.contains_key(&TxLabel::Buy) && self.state.remote_params().is_some() && self.state.local_params().is_some() => { let xmr_locked = self.state.a_xmr_locked(); let btc_locked = self.state.a_btc_locked(); + let overfunded = self.state.a_overfunded(); + let required_funding_amount = + self.state.a_required_funding_amount(); if let Some((txlabel, buy_tx)) = self.txs.remove_entry(&TxLabel::Buy) { @@ -1591,6 +1620,8 @@ impl Runtime { .state .last_checkpoint_type() .unwrap(), + required_funding_amount, + overfunded, }); } else { warn!( @@ -2094,6 +2125,8 @@ impl Runtime { cancel_seen: false, refund_seen: false, remote_params: self.state.remote_params().unwrap(), + required_funding_amount: None, + overfunded: false, }); self.state_update(endpoints, next_state)?; } diff --git a/src/swapd/swap_state.rs b/src/swapd/swap_state.rs index 40548ad2c..700462a0f 100644 --- a/src/swapd/swap_state.rs +++ b/src/swapd/swap_state.rs @@ -40,6 +40,8 @@ pub enum AliceState { remote_params: Params, /* #[display("local_view_share({0})")] */ local_params: Params, + required_funding_amount: Option, // TODO: Should be monero::Amount + overfunded: bool, }, #[display("Finish({0})")] FinishA(Outcome), @@ -320,6 +322,24 @@ impl State { pub fn a_refundsig(&self) -> bool { matches!(self, State::Alice(AliceState::RefundSigA { .. })) } + pub fn a_required_funding_amount(&self) -> Option { + match self { + State::Alice(AliceState::RefundSigA { + required_funding_amount, + .. + }) => required_funding_amount.clone(), + _ => None, + } + } + pub fn a_overfunded(&self) -> bool { + matches!( + self, + State::Alice(AliceState::RefundSigA { + overfunded: true, + .. + }) + ) + } pub fn b_buy_tx_seen(&self) -> bool { if !self.b_buy_sig() { return false; @@ -510,6 +530,19 @@ impl State { false } } + /// Update Alice RefundSig state from overfunded=false to overfunded=true + pub fn a_sup_overfunded(&mut self) { + if let State::Alice(AliceState::RefundSigA { overfunded, .. }) = self { + if !*overfunded { + trace!("setting overfunded"); + *overfunded = true; + } else { + trace!("overfunded was already set to true"); + } + } else { + error!("Not on RefundSig state"); + } + } /// Update Alice RefundSig state from XMR unlocked to locked state pub fn a_sup_refundsig_xmrlocked(&mut self) -> bool { if let State::Alice(AliceState::RefundSigA { xmr_locked, .. }) = self { @@ -526,6 +559,23 @@ impl State { false } } + /// Update Alice RefundSig state with the required Monero funding amount + pub fn a_sup_required_funding_amount(&mut self, amount: monero::Amount) { + if let State::Alice(AliceState::RefundSigA { + required_funding_amount, + .. + }) = self + { + if required_funding_amount.is_none() { + *required_funding_amount = Some(amount.as_pico()); + } else { + trace!("required funding amount was already set"); + } + } else { + error!("Not on RefundSig state"); + } + } + pub fn a_sup_refundsig_refund_seen(&mut self) -> bool { if let State::Alice(AliceState::RefundSigA { refund_seen, .. }) = self { if !*refund_seen { From c8765907d4cffad6e84823fdbeb2eb323c71a14c Mon Sep 17 00:00:00 2001 From: TheCharlatan Date: Tue, 5 Jul 2022 10:21:28 +0200 Subject: [PATCH 2/8] Functional tests: Refund alice if she overfunds --- tests/functional-swap.rs | 208 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 208 insertions(+) diff --git a/tests/functional-swap.rs b/tests/functional-swap.rs index 524fa8eb4..1daf55676 100644 --- a/tests/functional-swap.rs +++ b/tests/functional-swap.rs @@ -220,6 +220,44 @@ async fn swap_revoke_offer_bob_maker_normal() { cleanup_processes(vec![farcasterd_maker, farcasterd_taker]); } +#[tokio::test] +#[timeout(600000)] +#[ignore] +async fn swap_bob_maker_refund_alice_overfunds() { + let execution_mutex = Arc::new(Mutex::new(0)); + let bitcoin_rpc = Arc::new(bitcoin_setup()); + let (monero_regtest, monero_wallet) = monero_setup().await; + + let (farcasterd_maker, data_dir_maker, farcasterd_taker, data_dir_taker) = + setup_farcaster_clients().await; + + let (xmr_dest_wallet_name, bitcoin_address, swap_id) = make_and_take_offer( + data_dir_maker.clone(), + data_dir_taker.clone(), + "Bob".to_string(), + Arc::clone(&bitcoin_rpc), + Arc::clone(&monero_wallet), + bitcoin::Amount::from_str("1 BTC").unwrap(), + monero::Amount::from_str_with_denomination("1 XMR").unwrap(), + ) + .await; + + run_refund_swap_alice_overfunds( + swap_id, + data_dir_taker, + data_dir_maker, + Arc::clone(&bitcoin_rpc), + bitcoin_address, + monero_regtest, + Arc::clone(&monero_wallet), + xmr_dest_wallet_name, + execution_mutex, + ) + .await; + + cleanup_processes(vec![farcasterd_maker, farcasterd_taker]); +} + #[tokio::test] #[timeout(600000)] #[ignore] @@ -965,6 +1003,176 @@ async fn run_restore_checkpoint_bob_pre_buy_alice_pre_lock( cleanup_processes(vec![farcasterd_maker, farcasterd_taker]); } +#[allow(clippy::too_many_arguments)] +async fn run_refund_swap_alice_overfunds( + swap_id: SwapId, + data_dir_alice: Vec, + data_dir_bob: Vec, + bitcoin_rpc: Arc, + funding_btc_address: bitcoin::Address, + monero_regtest: monero_rpc::RegtestDaemonClient, + monero_wallet: Arc>, + monero_dest_wallet_name: String, + execution_mutex: Arc>, +) { + let cli_bob_progress_args: Vec = progress_args(data_dir_bob.clone(), swap_id.clone()); + let cli_alice_progress_args: Vec = + progress_args(data_dir_alice.clone(), swap_id.clone()); + let cli_bob_needs_funding_args: Vec = + needs_funding_args(data_dir_bob, "bitcoin".to_string()); + let cli_alice_needs_funding_args: Vec = + needs_funding_args(data_dir_alice, "monero".to_string()); + + bitcoin_rpc + .generate_to_address(1, &reusable_btc_address()) + .unwrap(); + + // run until bob has the btc funding address + let (address, amount) = + retry_until_bitcoin_funding_address(swap_id.clone(), cli_bob_needs_funding_args.clone()) + .await; + + // fund the bitcoin address + let lock = execution_mutex.lock().await; + bitcoin_rpc + .send_to_address(&address, amount, None, None, None, None, None, None) + .unwrap(); + + println!("waiting for AliceState(RefundSigs"); + retry_until_finish_state_transition( + cli_alice_progress_args.clone(), + "AliceState(RefundSigs".to_string(), + ) + .await; + + // run until BobState(CoreArb) is received + println!("waiting for BobState(CoreArb)"); + retry_until_finish_state_transition( + cli_bob_progress_args.clone(), + "BobState(CoreArb)".to_string(), + ) + .await; + + // run until the funding infos are cleared again + println!("waiting for the bitcoin funding info to clear"); + retry_until_funding_info_cleared(swap_id.clone(), cli_bob_needs_funding_args.clone()).await; + + tokio::time::sleep(time::Duration::from_secs(10)).await; + + // generate some bitcoin blocks to finalize the bitcoin arb lock tx + bitcoin_rpc + .generate_to_address(3, &reusable_btc_address()) + .unwrap(); + + // run until the alice has the monero funding address and fund it + let (monero_address, monero_amount) = + retry_until_monero_funding_address(swap_id, cli_alice_needs_funding_args.clone()).await; + send_monero( + Arc::clone(&monero_wallet), + monero_address, + monero::Amount::from_pico(monero_amount.as_pico() + 1), + ) + .await; + + tokio::time::sleep(time::Duration::from_secs(20)).await; + + // finalize alice's lock tx + monero_regtest + .generate_blocks(10, reusable_xmr_address()) + .await + .unwrap(); + + tokio::time::sleep(time::Duration::from_secs(20)).await; + + // generate some bitcoin blocks for confirmations + bitcoin_rpc + .generate_to_address(20, &reusable_btc_address()) + .unwrap(); + + tokio::time::sleep(time::Duration::from_secs(20)).await; + + // generate some bitcoin blocks to finalize the bitcoin cancel tx + bitcoin_rpc + .generate_to_address(3, &reusable_btc_address()) + .unwrap(); + + // generate some bitcoin blocks for confirmations + bitcoin_rpc + .generate_to_address(20, &reusable_btc_address()) + .unwrap(); + tokio::time::sleep(time::Duration::from_secs(20)).await; + + // generate some bitcoin blocks to finalize the bitcoin refund tx + bitcoin_rpc + .generate_to_address(3, &reusable_btc_address()) + .unwrap(); + + // run until the BobState(Finish(Failure(Refunded))) is received + retry_until_finish_state_transition( + cli_bob_progress_args.clone(), + "BobState(Finish(Failure(Refunded)))".to_string(), + ) + .await; + + // generate some blocks on bitcoin's side + bitcoin_rpc + .generate_to_address(1, &reusable_btc_address()) + .unwrap(); + + let (_stdout, _stderr) = run("../swap-cli", cli_bob_progress_args.clone()).unwrap(); + + // check that btc was received in the destination address + let balance = bitcoin_rpc + .get_received_by_address(&funding_btc_address, None) + .unwrap(); + assert!(balance.as_sat() > 90000000); + + // cache the monero balance before sweeping + let monero_wallet_lock = monero_wallet.lock().await; + monero_wallet_lock + .open_wallet(monero_dest_wallet_name.clone(), None) + .await + .unwrap(); + let before_balance = monero_wallet_lock.get_balance(0, None).await.unwrap(); + drop(monero_wallet_lock); + + // Sleep here to work around a race condition between pending + // SweepXmrAddress requests and tx Acc Lock confirmations. If Acc Lock + // confirmations are produced before the pending request is queued, no + // action will take place after this point. + tokio::time::sleep(time::Duration::from_secs(10)).await; + + // generate some blocks on monero's side + monero_regtest + .generate_blocks(10, reusable_xmr_address()) + .await + .unwrap(); + + // run until the AliceState(Finish) is received + retry_until_finish_state_transition( + cli_alice_progress_args.clone(), + "AliceState(Finish(Failure(Refunded)))".to_string(), + ) + .await; + + monero_regtest + .generate_blocks(1, reusable_xmr_address()) + .await + .unwrap(); + + let monero_wallet_lock = monero_wallet.lock().await; + monero_wallet_lock + .open_wallet(monero_dest_wallet_name, None) + .await + .unwrap(); + monero_wallet_lock.refresh(Some(1)).await.unwrap(); + let after_balance = monero_wallet_lock.get_balance(0, None).await.unwrap(); + drop(monero_wallet_lock); + let delta_balance = after_balance.balance - before_balance.balance; + assert!(delta_balance > 999660000000); + drop(lock); +} + #[allow(clippy::too_many_arguments)] async fn run_refund_swap_race_cancel( swap_id: SwapId, From 8a5e68c623c8618ef519c5b5fe37f9007cd99007 Mon Sep 17 00:00:00 2001 From: TheCharlatan Date: Tue, 5 Jul 2022 10:48:19 +0200 Subject: [PATCH 3/8] Functional tests: Tweak alice overfunds to wait for bob buysig state --- tests/functional-swap.rs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/tests/functional-swap.rs b/tests/functional-swap.rs index 1daf55676..46b08cde2 100644 --- a/tests/functional-swap.rs +++ b/tests/functional-swap.rs @@ -1074,17 +1074,26 @@ async fn run_refund_swap_alice_overfunds( ) .await; - tokio::time::sleep(time::Duration::from_secs(20)).await; + // run until the funding infos are cleared again + println!("waiting for the monero funding info to clear"); + retry_until_funding_info_cleared(swap_id.clone(), cli_alice_needs_funding_args.clone()).await; - // finalize alice's lock tx + // generate some monero blocks to finalize the monero acc lock tx monero_regtest .generate_blocks(10, reusable_xmr_address()) .await .unwrap(); + // run until BobState(BuySig) is received + retry_until_finish_state_transition( + cli_bob_progress_args.clone(), + "BobState(BuySig)".to_string(), + ) + .await; + tokio::time::sleep(time::Duration::from_secs(20)).await; - // generate some bitcoin blocks for confirmations + // generate some bitcoin blocks for confirmations and triggering cancel bitcoin_rpc .generate_to_address(20, &reusable_btc_address()) .unwrap(); From 9966c04301cd94df397e444c75700acdcbd324f5 Mon Sep 17 00:00:00 2001 From: TheCharlatan Date: Tue, 5 Jul 2022 10:48:52 +0200 Subject: [PATCH 4/8] Swap: Improve comments for incorrect Monero funding --- src/swapd/runtime.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/swapd/runtime.rs b/src/swapd/runtime.rs index 60072d5fa..4eabbc9fc 100644 --- a/src/swapd/runtime.rs +++ b/src/swapd/runtime.rs @@ -1101,9 +1101,10 @@ impl Runtime { ); self.state.a_sup_refundsig_xmrlocked(); - // if the funding amount does not match the expected, abort and wait for the swap to refund - let required_funding_amount = - self.state.a_required_funding_amount().unwrap(); + let required_funding_amount = self + .state + .a_required_funding_amount() + .expect("set when monero funding address is displayed"); if amount.clone() < required_funding_amount { // Alice still views underfunding as valid in the hope that Bob still passes her BuyProcSig let msg = format!( @@ -1112,7 +1113,9 @@ impl Runtime { monero::Amount::from_pico(amount.clone()) ); error!("{}", msg); + self.report_progress_message_to(endpoints, ServiceId::Farcasterd, msg)?; } else if amount.clone() > required_funding_amount { + // Alice set overfunded to ensure that she does not publish the buy transaction if Bob gives her the BuySig. self.state.a_sup_overfunded(); let msg = format!( "Too big amount funded. Required: {}, Funded: {}. Do not fund this swap anymore, will attempt to refund.", @@ -1120,6 +1123,7 @@ impl Runtime { monero::Amount::from_pico(amount.clone()) ); error!("{}", msg); + self.report_progress_message_to(endpoints, ServiceId::Farcasterd, msg)?; } let txlabel = TxLabel::AccLock; From 5d0a9d11820c5d10443cb9e1f9b4b5a82e367817 Mon Sep 17 00:00:00 2001 From: TheCharlatan Date: Thu, 7 Jul 2022 10:33:21 +0200 Subject: [PATCH 5/8] fixup! Functional tests: Tweak alice overfunds to wait for bob buysig state --- tests/functional-swap.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/functional-swap.rs b/tests/functional-swap.rs index 46b08cde2..e69907326 100644 --- a/tests/functional-swap.rs +++ b/tests/functional-swap.rs @@ -1078,6 +1078,8 @@ async fn run_refund_swap_alice_overfunds( println!("waiting for the monero funding info to clear"); retry_until_funding_info_cleared(swap_id.clone(), cli_alice_needs_funding_args.clone()).await; + tokio::time::sleep(time::Duration::from_secs(10)).await; + // generate some monero blocks to finalize the monero acc lock tx monero_regtest .generate_blocks(10, reusable_xmr_address()) From dbbf86396fa792e012fed54fbbe1f6965e6aa50d Mon Sep 17 00:00:00 2001 From: zkao Date: Fri, 8 Jul 2022 17:57:27 +0200 Subject: [PATCH 6/8] swap state: state update returns bool --- src/swapd/swap_state.rs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/swapd/swap_state.rs b/src/swapd/swap_state.rs index 700462a0f..74ae15a8f 100644 --- a/src/swapd/swap_state.rs +++ b/src/swapd/swap_state.rs @@ -531,16 +531,19 @@ impl State { } } /// Update Alice RefundSig state from overfunded=false to overfunded=true - pub fn a_sup_overfunded(&mut self) { + pub fn a_sup_overfunded(&mut self) -> bool { if let State::Alice(AliceState::RefundSigA { overfunded, .. }) = self { if !*overfunded { trace!("setting overfunded"); *overfunded = true; + true } else { - trace!("overfunded was already set to true"); + warn!("overfunded was already set to true"); + false } } else { error!("Not on RefundSig state"); + false } } /// Update Alice RefundSig state from XMR unlocked to locked state @@ -560,7 +563,7 @@ impl State { } } /// Update Alice RefundSig state with the required Monero funding amount - pub fn a_sup_required_funding_amount(&mut self, amount: monero::Amount) { + pub fn a_sup_required_funding_amount(&mut self, amount: monero::Amount) -> bool { if let State::Alice(AliceState::RefundSigA { required_funding_amount, .. @@ -568,11 +571,14 @@ impl State { { if required_funding_amount.is_none() { *required_funding_amount = Some(amount.as_pico()); + true } else { - trace!("required funding amount was already set"); + warn!("required funding amount was already set"); + false } } else { error!("Not on RefundSig state"); + false } } From 732fa6a9a31539101703cd4f7c4d0ce0bf04a667 Mon Sep 17 00:00:00 2001 From: zkao Date: Fri, 8 Jul 2022 18:01:38 +0200 Subject: [PATCH 7/8] swapd: report msgs s/ServiceId::Farcasterd/self.enquirer.clone() --- src/swapd/runtime.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/swapd/runtime.rs b/src/swapd/runtime.rs index 4eabbc9fc..151df6757 100644 --- a/src/swapd/runtime.rs +++ b/src/swapd/runtime.rs @@ -1113,7 +1113,7 @@ impl Runtime { monero::Amount::from_pico(amount.clone()) ); error!("{}", msg); - self.report_progress_message_to(endpoints, ServiceId::Farcasterd, msg)?; + self.report_progress_message_to(endpoints, self.enquirer.clone(), msg)?; } else if amount.clone() > required_funding_amount { // Alice set overfunded to ensure that she does not publish the buy transaction if Bob gives her the BuySig. self.state.a_sup_overfunded(); @@ -1123,7 +1123,7 @@ impl Runtime { monero::Amount::from_pico(amount.clone()) ); error!("{}", msg); - self.report_progress_message_to(endpoints, ServiceId::Farcasterd, msg)?; + self.report_progress_message_to(endpoints, self.enquirer.clone(), msg)?; } let txlabel = TxLabel::AccLock; From 6252d135e8542df3bc877ac4043e23c4fe6880a3 Mon Sep 17 00:00:00 2001 From: zkao Date: Fri, 8 Jul 2022 20:35:26 +0200 Subject: [PATCH 8/8] swapd: add guard that asserts tx is the correct one --- src/swapd/runtime.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/swapd/runtime.rs b/src/swapd/runtime.rs index 151df6757..9c7dca5d2 100644 --- a/src/swapd/runtime.rs +++ b/src/swapd/runtime.rs @@ -1163,7 +1163,9 @@ impl Runtime { tx: _, }) if self.state.swap_role() == SwapRole::Bob && self.syncer_state.tasks.watched_addrs.contains_key(id) - && self.syncer_state.is_watched_addr(&TxLabel::AccLock) => + && self.syncer_state.is_watched_addr(&TxLabel::AccLock) + && self.syncer_state.tasks.watched_addrs.get(id).unwrap() + == &TxLabel::AccLock => { let amount = monero::Amount::from_pico(*amount); if amount < self.syncer_state.monero_amount {