From dc8dd5af28d8e64734dde3106ca0bac4e94eb6f1 Mon Sep 17 00:00:00 2001 From: Philipp Hoenisch Date: Tue, 4 May 2021 11:34:00 +1000 Subject: [PATCH] Add relative and absolute max transaction fee. --- swap/src/bitcoin.rs | 10 +- swap/src/bitcoin/wallet.rs | 182 +++++++++++++++++++++----- swap/src/protocol/alice/event_loop.rs | 4 +- swap/src/protocol/bob/swap.rs | 8 +- swap/tests/harness/mod.rs | 10 +- 5 files changed, 168 insertions(+), 46 deletions(-) diff --git a/swap/src/bitcoin.rs b/swap/src/bitcoin.rs index e7236a67..17ddfd3d 100644 --- a/swap/src/bitcoin.rs +++ b/swap/src/bitcoin.rs @@ -325,8 +325,14 @@ mod tests { let btc_amount = Amount::from_sat(500_000); let xmr_amount = crate::monero::Amount::from_piconero(10000); - let tx_redeem_fee = alice_wallet.estimate_fee(TxRedeem::weight()).await.unwrap(); - let tx_punish_fee = alice_wallet.estimate_fee(TxPunish::weight()).await.unwrap(); + let tx_redeem_fee = alice_wallet + .estimate_fee(TxRedeem::weight(), btc_amount) + .await + .unwrap(); + let tx_punish_fee = alice_wallet + .estimate_fee(TxPunish::weight(), btc_amount) + .await + .unwrap(); let redeem_address = alice_wallet.new_address().await.unwrap(); let punish_address = alice_wallet.new_address().await.unwrap(); diff --git a/swap/src/bitcoin/wallet.rs b/swap/src/bitcoin/wallet.rs index 64b48939..28cb9375 100644 --- a/swap/src/bitcoin/wallet.rs +++ b/swap/src/bitcoin/wallet.rs @@ -22,7 +22,12 @@ use std::time::{Duration, Instant}; use tokio::sync::{watch, Mutex}; const SLED_TREE_NAME: &str = "default_tree"; -const MAX_TX_FEE: u64 = 100_000; + +// TODO make these values configurable +/// Assuming we add a spread of 3% we don't want to pay more than 3% of the +/// amount for tx fees. +const MAX_RELATIVE_TX_FEE: f64 = 0.03; +const MAX_ABSOLUTE_TX_FEE: u64 = 100_000; pub struct Wallet { client: Arc>, @@ -322,46 +327,81 @@ where } /// Estimate total tx fee based for a pre-defined target block based on the - /// transaction weight - pub async fn estimate_fee(&self, weight: usize) -> Result { + /// transaction weight. The max fee cannot be more than MAX_PERCENTAGE_FEE + /// of amount + pub async fn estimate_fee( + &self, + weight: usize, + transfer_amount: bitcoin::Amount, + ) -> Result { let client = self.client.lock().await; let fee_rate = client.estimate_feerate(self.target_block)?; let min_relay_fee = client.min_relay_fee()?; tracing::debug!("Min relay fee: {}", min_relay_fee); - // Doing some heavy math here :) - // `usize` is 32 or 64 bits wide, but `f32`'s mantissa is only 23 bits wide. - // This fine because such a big transaction cannot exist and there are also no - // negative fees. - #[allow( - clippy::cast_precision_loss, - clippy::cast_possible_truncation, - clippy::cast_sign_loss - )] - let sats_per_vbyte = ((weight as f32) / 4.0 * fee_rate.as_sat_vb()) as u64; - tracing::debug!( - "Estimated fee for weight: {} for fee_rate: {:?} is in total: {}", - weight, - fee_rate, - sats_per_vbyte - ); - if sats_per_vbyte < min_relay_fee.as_sat() { - tracing::warn!( - "Estimated fee of {} is smaller than the min relay fee, defaulting to min relay fee {}", - sats_per_vbyte, - min_relay_fee.as_sat() - ); - Ok(min_relay_fee) - } else if sats_per_vbyte > MAX_TX_FEE { - tracing::warn!( - "Hard bound of max transaction fees reached. Falling back to: {} sats", - MAX_TX_FEE - ); - Ok(bitcoin::Amount::from_sat(MAX_TX_FEE)) - } else { - Ok(bitcoin::Amount::from_sat(sats_per_vbyte)) - } + Ok(estimate_fee( + weight, + transfer_amount, + fee_rate, + min_relay_fee, + )) + } +} + +fn estimate_fee( + weight: usize, + transfer_amount: Amount, + fee_rate: FeeRate, + min_relay_fee: Amount, +) -> Amount { + // Doing some heavy math here :) + // `usize` is 32 or 64 bits wide, but `f32`'s mantissa is only 23 bits wide. + // This fine because such a big transaction cannot exist and there are also no + // negative fees. + #[allow( + clippy::cast_precision_loss, + clippy::cast_possible_truncation, + clippy::cast_sign_loss + )] + let sats_per_vbyte = ((weight as f32) / 4.0 * fee_rate.as_sat_vb()) as u64; + tracing::debug!( + "Estimated fee for weight: {} for fee_rate: {:?} is in total: {}", + weight, + fee_rate, + sats_per_vbyte + ); + + // Similar as above: we do not care about fractional fees and have to cast a + // couple of times. + #[allow( + clippy::cast_precision_loss, + clippy::cast_possible_truncation, + clippy::cast_sign_loss + )] + let max_allowed_fee = (transfer_amount.as_sat() as f64 * MAX_RELATIVE_TX_FEE).ceil() as u64; + + if sats_per_vbyte < min_relay_fee.as_sat() { + tracing::warn!( + "Estimated fee of {} is smaller than the min relay fee, defaulting to min relay fee {}", + sats_per_vbyte, + min_relay_fee.as_sat() + ); + min_relay_fee + } else if sats_per_vbyte > max_allowed_fee && sats_per_vbyte > MAX_ABSOLUTE_TX_FEE { + tracing::warn!( + "Hard bound of transaction fees reached. Falling back to: {} sats", + MAX_ABSOLUTE_TX_FEE + ); + bitcoin::Amount::from_sat(MAX_ABSOLUTE_TX_FEE) + } else if sats_per_vbyte > max_allowed_fee { + tracing::warn!( + "Relative bound of transaction fees reached. Falling back to: {} sats", + max_allowed_fee + ); + bitcoin::Amount::from_sat(max_allowed_fee) + } else { + bitcoin::Amount::from_sat(sats_per_vbyte) } } @@ -734,4 +774,76 @@ mod tests { assert_eq!(confirmed.depth, 0) } + + #[test] + fn given_one_BTC_and_100k_sats_per_vb_fees_should_not_hit_max() { + // 400 weight = 100 vbyte + let weight = 400; + let amount = bitcoin::Amount::from_sat(100_000_000); + + let sat_per_vb = 100.0; + let fee_rate = FeeRate::from_sat_per_vb(sat_per_vb); + + let relay_fee = bitcoin::Amount::ONE_SAT; + let is_fee = estimate_fee(weight, amount, fee_rate, relay_fee); + + // weight / 4.0 * sat_per_vb + let should_fee = bitcoin::Amount::from_sat(10_000); + assert_eq!(is_fee, should_fee); + } + + #[test] + fn given_1BTC_and_1_sat_per_vb_fees_and_100ksat_min_relay_fee_should_hit_min() { + // 400 weight = 100 vbyte + let weight = 400; + let amount = bitcoin::Amount::from_sat(100_000_000); + + let sat_per_vb = 1.0; + let fee_rate = FeeRate::from_sat_per_vb(sat_per_vb); + + let relay_fee = bitcoin::Amount::from_sat(100_000); + let is_fee = estimate_fee(weight, amount, fee_rate, relay_fee); + + // weight / 4.0 * sat_per_vb would be smaller than relay fee hence we take min + // relay fee + let should_fee = bitcoin::Amount::from_sat(100_000); + assert_eq!(is_fee, should_fee); + } + + #[test] + fn given_1mio_sat_and_1k_sats_per_vb_fees_should_hit_relative_max() { + // 400 weight = 100 vbyte + let weight = 400; + let amount = bitcoin::Amount::from_sat(1_000_000); + + let sat_per_vb = 1_000.0; + let fee_rate = FeeRate::from_sat_per_vb(sat_per_vb); + + let relay_fee = bitcoin::Amount::ONE_SAT; + let is_fee = estimate_fee(weight, amount, fee_rate, relay_fee); + + // weight / 4.0 * sat_per_vb would be greater than 0.03% hence we take max + // relative fee. + let should_fee = bitcoin::Amount::from_sat(30_000); + assert_eq!(is_fee, should_fee); + } + + #[test] + fn given_1BTC_and_4mio_sats_per_vb_fees_should_hit_total_max() { + // even if we send 1BTC we don't want to pay 0.3BTC in fees. This would be + // $1,650 at the moment. + let weight = 400; + let amount = bitcoin::Amount::from_sat(100_000_000); + + let sat_per_vb = 4_000_000.0; + let fee_rate = FeeRate::from_sat_per_vb(sat_per_vb); + + let relay_fee = bitcoin::Amount::ONE_SAT; + let is_fee = estimate_fee(weight, amount, fee_rate, relay_fee); + + // weight / 4.0 * sat_per_vb would be greater than 0.03% hence we take total + // max allowed fee. + let should_fee = bitcoin::Amount::from_sat(MAX_ABSOLUTE_TX_FEE); + assert_eq!(is_fee, should_fee); + } } diff --git a/swap/src/protocol/alice/event_loop.rs b/swap/src/protocol/alice/event_loop.rs index fc38bb42..c20fb9d9 100644 --- a/swap/src/protocol/alice/event_loop.rs +++ b/swap/src/protocol/alice/event_loop.rs @@ -163,10 +163,10 @@ where } // TODO: This should be cleaned up. let tx_redeem_fee = self.bitcoin_wallet - .estimate_fee(bitcoin::TxRedeem::weight()) + .estimate_fee(bitcoin::TxRedeem::weight(), btc) .await; let tx_punish_fee = self.bitcoin_wallet - .estimate_fee(bitcoin::TxPunish::weight()) + .estimate_fee(bitcoin::TxPunish::weight(), btc) .await; let redeem_address = self.bitcoin_wallet.new_address().await; let punish_address = self.bitcoin_wallet.new_address().await; diff --git a/swap/src/protocol/bob/swap.rs b/swap/src/protocol/bob/swap.rs index dffe3a8d..07425f53 100644 --- a/swap/src/protocol/bob/swap.rs +++ b/swap/src/protocol/bob/swap.rs @@ -66,8 +66,12 @@ async fn next_state( Ok(match state { BobState::Started { btc_amount } => { let bitcoin_refund_address = bitcoin_wallet.new_address().await?; - let tx_refund_fee = bitcoin_wallet.estimate_fee(TxRefund::weight()).await?; - let tx_cancel_fee = bitcoin_wallet.estimate_fee(TxCancel::weight()).await?; + let tx_refund_fee = bitcoin_wallet + .estimate_fee(TxRefund::weight(), btc_amount) + .await?; + let tx_cancel_fee = bitcoin_wallet + .estimate_fee(TxCancel::weight(), btc_amount) + .await?; let state2 = request_price_and_setup( swap_id, diff --git a/swap/tests/harness/mod.rs b/swap/tests/harness/mod.rs index 9dd5cbb8..cadce4c4 100644 --- a/swap/tests/harness/mod.rs +++ b/swap/tests/harness/mod.rs @@ -634,12 +634,12 @@ impl TestContext { let cancel_fee = self .alice_bitcoin_wallet - .estimate_fee(TxCancel::weight()) + .estimate_fee(TxCancel::weight(), self.btc_amount) .await .expect("To estimate fee correctly"); let refund_fee = self .alice_bitcoin_wallet - .estimate_fee(TxRefund::weight()) + .estimate_fee(TxRefund::weight(), self.btc_amount) .await .expect("To estimate fee correctly"); @@ -682,7 +682,7 @@ impl TestContext { async fn alice_redeemed_btc_balance(&self) -> bitcoin::Amount { let fee = self .alice_bitcoin_wallet - .estimate_fee(TxRedeem::weight()) + .estimate_fee(TxRedeem::weight(), self.btc_amount) .await .expect("To estimate fee correctly"); self.alice_starting_balances.btc + self.btc_amount - fee @@ -725,12 +725,12 @@ impl TestContext { async fn alice_punished_btc_balance(&self) -> bitcoin::Amount { let cancel_fee = self .alice_bitcoin_wallet - .estimate_fee(TxCancel::weight()) + .estimate_fee(TxCancel::weight(), self.btc_amount) .await .expect("To estimate fee correctly"); let punish_fee = self .alice_bitcoin_wallet - .estimate_fee(TxPunish::weight()) + .estimate_fee(TxPunish::weight(), self.btc_amount) .await .expect("To estimate fee correctly"); self.alice_starting_balances.btc + self.btc_amount - cancel_fee - punish_fee