diff --git a/swap/src/bin/asb.rs b/swap/src/bin/asb.rs index 663cf266..051cee3f 100644 --- a/swap/src/bin/asb.rs +++ b/swap/src/bin/asb.rs @@ -211,6 +211,7 @@ async fn init_bitcoin_wallet( &wallet_dir, seed.derive_extended_private_key(env_config.bitcoin_network)?, env_config, + 6, // TODO move this into config ) .await .context("Failed to initialize Bitcoin wallet")?; diff --git a/swap/src/bin/swap.rs b/swap/src/bin/swap.rs index eb5e9322..28f79cce 100644 --- a/swap/src/bin/swap.rs +++ b/swap/src/bin/swap.rs @@ -272,6 +272,7 @@ async fn init_bitcoin_wallet( &wallet_dir, seed.derive_extended_private_key(env_config.bitcoin_network)?, env_config, + 6, // TODO move this into config ) .await .context("Failed to initialize Bitcoin wallet")?; diff --git a/swap/src/bitcoin/cancel.rs b/swap/src/bitcoin/cancel.rs index 3f05cbd9..8b182a1a 100644 --- a/swap/src/bitcoin/cancel.rs +++ b/swap/src/bitcoin/cancel.rs @@ -13,8 +13,9 @@ use std::cmp::Ordering; use std::collections::HashMap; use std::ops::Add; -// TODO take real tx weight from past transaction -pub const ESTIMATED_WEIGHT: usize = 10_000; +// Taken from https://mempool.space/testnet/tx/8da32d6cada4903c7043563c50cacce656a8be9e02f233201996739df5368b1f +// The weight might fluctuate slightly in reality. +pub const ESTIMATED_WEIGHT: usize = 485; /// Represent a timelock, expressed in relative block height as defined in /// [BIP68](https://github.com/bitcoin/bips/blob/master/bip-0068.mediawiki). diff --git a/swap/src/bitcoin/lock.rs b/swap/src/bitcoin/lock.rs index 58d3ff9d..c66b3881 100644 --- a/swap/src/bitcoin/lock.rs +++ b/swap/src/bitcoin/lock.rs @@ -1,4 +1,4 @@ -use crate::bitcoin::wallet::Watchable; +use crate::bitcoin::wallet::{EstimateFeeRate, Watchable}; use crate::bitcoin::{ build_shared_output_descriptor, Address, Amount, PublicKey, Transaction, Wallet, }; @@ -26,6 +26,7 @@ impl TxLock { B: PublicKey, ) -> Result where + C: EstimateFeeRate, D: BatchDatabase, { let lock_output_descriptor = build_shared_output_descriptor(A.0, B.0); @@ -147,9 +148,10 @@ impl TxLock { witness: Vec::new(), }; + let i = spending_fee.as_sat(); + tracing::debug!("Redeem tx fee: {}", i); let tx_out = TxOut { - value: self.inner.clone().extract_tx().output[self.lock_output_vout()].value - - spending_fee.as_sat(), + value: self.inner.clone().extract_tx().output[self.lock_output_vout()].value - i, script_pubkey: spend_address.script_pubkey(), }; @@ -181,11 +183,23 @@ impl Watchable for TxLock { #[cfg(test)] mod tests { use super::*; + use bdk::FeeRate; + + struct StaticFeeRate {} + impl EstimateFeeRate for StaticFeeRate { + fn estimate_feerate(&self, _target_block: usize) -> Result { + Ok(FeeRate::default_min_relay_fee()) + } + + fn min_relay_fee(&self) -> Result { + Ok(bitcoin::Amount::from_sat(1_000)) + } + } #[tokio::test] async fn given_bob_sends_good_psbt_when_reconstructing_then_succeeeds() { let (A, B) = alice_and_bob(); - let wallet = Wallet::new_funded(50000); + let wallet = Wallet::new_funded(50000, StaticFeeRate {}); let agreed_amount = Amount::from_sat(10000); let psbt = bob_make_psbt(A, B, &wallet, agreed_amount).await; @@ -199,7 +213,7 @@ mod tests { let (A, B) = alice_and_bob(); let fees = 610; let agreed_amount = Amount::from_sat(10000); - let wallet = Wallet::new_funded(agreed_amount.as_sat() + fees); + let wallet = Wallet::new_funded(agreed_amount.as_sat() + fees, StaticFeeRate {}); let psbt = bob_make_psbt(A, B, &wallet, agreed_amount).await; assert_eq!( @@ -215,7 +229,7 @@ mod tests { #[tokio::test] async fn given_bob_is_sending_less_than_agreed_when_reconstructing_txlock_then_fails() { let (A, B) = alice_and_bob(); - let wallet = Wallet::new_funded(50000); + let wallet = Wallet::new_funded(50000, StaticFeeRate {}); let agreed_amount = Amount::from_sat(10000); let bad_amount = Amount::from_sat(5000); @@ -228,7 +242,7 @@ mod tests { #[tokio::test] async fn given_bob_is_sending_to_a_bad_output_reconstructing_txlock_then_fails() { let (A, B) = alice_and_bob(); - let wallet = Wallet::new_funded(50000); + let wallet = Wallet::new_funded(50000, StaticFeeRate {}); let agreed_amount = Amount::from_sat(10000); let E = eve(); @@ -244,7 +258,7 @@ mod tests { async fn bob_make_psbt( A: PublicKey, B: PublicKey, - wallet: &Wallet<(), bdk::database::MemoryDatabase, ()>, + wallet: &Wallet<(), bdk::database::MemoryDatabase, StaticFeeRate>, amount: Amount, ) -> PartiallySignedTransaction { TxLock::new(&wallet, amount, A, B).await.unwrap().into() diff --git a/swap/src/bitcoin/punish.rs b/swap/src/bitcoin/punish.rs index c698f07c..5dced7c4 100644 --- a/swap/src/bitcoin/punish.rs +++ b/swap/src/bitcoin/punish.rs @@ -7,8 +7,9 @@ use bdk::bitcoin::Script; use miniscript::{Descriptor, DescriptorTrait}; use std::collections::HashMap; -// TODO take real tx weight from past transaction -pub const ESTIMATED_WEIGHT: usize = 10_000; +// Taken from https://mempool.space/testnet/tx/ed4d60bc1fd172feca444ed3d06cccb90346b9098c2d28d2d034dac66f608f68 +// The weight might fluctuate slightly in reality. +pub const ESTIMATED_WEIGHT: usize = 547; #[derive(Debug)] pub struct TxPunish { diff --git a/swap/src/bitcoin/redeem.rs b/swap/src/bitcoin/redeem.rs index 87dc59a8..6a5662c5 100644 --- a/swap/src/bitcoin/redeem.rs +++ b/swap/src/bitcoin/redeem.rs @@ -15,8 +15,9 @@ use miniscript::{Descriptor, DescriptorTrait}; use sha2::Sha256; use std::collections::HashMap; -// TODO take real tx weight from past transaction -pub const ESTIMATED_WEIGHT: usize = 10_000; +// Taken from https://mempool.space/testnet/tx/80c265f5c3862075aab2bd8c94e261b092bc13a34294c6fb4071717fbf46c801 +// The weight might fluctuate slightly in reality. +pub const ESTIMATED_WEIGHT: usize = 547; #[derive(Clone, Debug)] pub struct TxRedeem { diff --git a/swap/src/bitcoin/refund.rs b/swap/src/bitcoin/refund.rs index 63d0818c..5b6af9b1 100644 --- a/swap/src/bitcoin/refund.rs +++ b/swap/src/bitcoin/refund.rs @@ -11,8 +11,9 @@ use ecdsa_fun::Signature; use miniscript::{Descriptor, DescriptorTrait}; use std::collections::HashMap; -// TODO take real tx weight from past transaction -pub const ESTIMATED_WEIGHT: usize = 10_000; +// Taken from https://mempool.space/testnet/tx/10aef570973bcf524b1a6e8d2eaf3bc1522e776381fc7520fd8987fba96e5424 +// The weight might fluctuate slightly in reality. +pub const ESTIMATED_WEIGHT: usize = 547; #[derive(Debug)] pub struct TxRefund { diff --git a/swap/src/bitcoin/wallet.rs b/swap/src/bitcoin/wallet.rs index 872f0c37..48eb1eb5 100644 --- a/swap/src/bitcoin/wallet.rs +++ b/swap/src/bitcoin/wallet.rs @@ -28,6 +28,7 @@ pub struct Wallet { wallet: Arc>>, finality_confirmations: u32, network: Network, + target_block: usize, } impl Wallet { @@ -36,6 +37,7 @@ impl Wallet { wallet_dir: &Path, key: impl DerivableKey + Clone, env_config: env::Config, + target_block: usize, ) -> Result { let client = bdk::electrum_client::Client::new(electrum_rpc_url.as_str()) .context("Failed to initialize Electrum RPC client")?; @@ -63,6 +65,7 @@ impl Wallet { wallet: Arc::new(Mutex::new(wallet)), finality_confirmations: env_config.bitcoin_finality_confirmations, network, + target_block, }) } @@ -238,6 +241,7 @@ impl Subscription { impl Wallet where + C: EstimateFeeRate, D: BatchDatabase, { pub async fn balance(&self) -> Result { @@ -282,10 +286,12 @@ where amount: Amount, ) -> Result { let wallet = self.wallet.lock().await; + let client = self.client.lock().await; + let fee_rate = client.estimate_feerate(self.target_block)?; let mut tx_builder = wallet.build_tx(); tx_builder.add_recipient(address.script_pubkey(), amount.as_sat()); - tx_builder.fee_rate(self.select_feerate()); + tx_builder.fee_rate(fee_rate); let (psbt, _details) = tx_builder.finish()?; Ok(psbt) @@ -298,19 +304,57 @@ where /// transaction confirmed. pub async fn max_giveable(&self, locking_script_size: usize) -> Result { let wallet = self.wallet.lock().await; + let client = self.client.lock().await; + let fee_rate = client.estimate_feerate(self.target_block)?; let mut tx_builder = wallet.build_tx(); let dummy_script = Script::from(vec![0u8; locking_script_size]); tx_builder.set_single_recipient(dummy_script); tx_builder.drain_wallet(); - tx_builder.fee_rate(self.select_feerate()); + tx_builder.fee_rate(fee_rate); let (_, details) = tx_builder.finish().context("Failed to build transaction")?; let max_giveable = details.sent - details.fees; Ok(Amount::from_sat(max_giveable)) } + + /// 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 { + 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 { + Ok(bitcoin::Amount::from_sat(sats_per_vbyte)) + } + } } impl Wallet @@ -340,32 +384,20 @@ impl Wallet { pub fn get_network(&self) -> bitcoin::Network { self.network } +} - /// Selects an appropriate [`FeeRate`] to be used for getting transactions - /// confirmed within a reasonable amount of time. - fn select_feerate(&self) -> FeeRate { - // TODO: This should obviously not be a const :) - FeeRate::from_sat_per_vb(5.0) - } - - pub fn estimate_fee(&self, weight: usize) -> bitcoin::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. - #[allow(clippy::cast_precision_loss)] - let calc_fee_bytes = (weight as f32) * self.select_feerate().as_sat_vb() / 4.0; - // There are no fractional satoshi, hence we just round to the next one and - // truncate. We also do not support negative fees. - #[allow(clippy::cast_possible_truncation, clippy::cast_sign_loss)] - let calc_fee_bytes_rounded = ((calc_fee_bytes * 10.0).round() as u64) / 10; - bitcoin::Amount::from_sat(calc_fee_bytes_rounded) - } +pub trait EstimateFeeRate { + fn estimate_feerate(&self, target_block: usize) -> Result; + fn min_relay_fee(&self) -> Result; } #[cfg(test)] -impl Wallet<(), bdk::database::MemoryDatabase, ()> { +impl Wallet<(), bdk::database::MemoryDatabase, EFR> +where + EFR: EstimateFeeRate, +{ /// Creates a new, funded wallet to be used within tests. - pub fn new_funded(amount: u64) -> Self { + pub fn new_funded(amount: u64, estimate_fee_rate: EFR) -> Self { use bdk::database::MemoryDatabase; use bdk::{LocalUtxo, TransactionDetails}; use bitcoin::OutPoint; @@ -387,10 +419,11 @@ impl Wallet<(), bdk::database::MemoryDatabase, ()> { bdk::Wallet::new_offline(&descriptors.0, None, Network::Regtest, database).unwrap(); Self { - client: Arc::new(Mutex::new(())), + client: Arc::new(Mutex::new(estimate_fee_rate)), wallet: Arc::new(Mutex::new(wallet)), finality_confirmations: 1, network: Network::Regtest, + target_block: 1, } } } @@ -557,6 +590,24 @@ impl Client { } } +impl EstimateFeeRate for Client { + fn estimate_feerate(&self, target_block: usize) -> Result { + // https://github.com/romanz/electrs/blob/f9cf5386d1b5de6769ee271df5eef324aa9491bc/src/rpc.rs#L213 + // Returned estimated fees are per BTC/kb. + let fee_per_byte = self.electrum.estimate_fee(target_block)?; + // we do not expect fees being that high. + #[allow(clippy::cast_possible_truncation)] + Ok(FeeRate::from_btc_per_kvb(fee_per_byte as f32)) + } + + fn min_relay_fee(&self) -> Result { + // https://github.com/romanz/electrs/blob/f9cf5386d1b5de6769ee271df5eef324aa9491bc/src/rpc.rs#L219 + // Returned fee is in BTC/kb + let relay_fee = bitcoin::Amount::from_btc(self.electrum.relay_fee()?)?; + Ok(relay_fee) + } +} + #[derive(Debug, Copy, Clone, PartialEq)] pub enum ScriptStatus { Unseen, diff --git a/swap/src/protocol/alice/state.rs b/swap/src/protocol/alice/state.rs index 5252c69a..d177f10f 100644 --- a/swap/src/protocol/alice/state.rs +++ b/swap/src/protocol/alice/state.rs @@ -132,6 +132,12 @@ impl State0 { let s_a = monero::Scalar::random(rng); let (dleq_proof_s_a, (S_a_bitcoin, S_a_monero)) = CROSS_CURVE_PROOF_SYSTEM.prove(&s_a, rng); + let tx_redeem_fee = bitcoin_wallet + .estimate_fee(ESTIMATED_WEIGHT_TX_REDEEM) + .await?; + let tx_punish_fee = bitcoin_wallet + .estimate_fee(ESTIMATED_WEIGHT_TX_PUNISH) + .await?; Ok(Self { a, s_a, @@ -147,8 +153,8 @@ impl State0 { xmr, cancel_timelock: env_config.bitcoin_cancel_timelock, punish_timelock: env_config.bitcoin_punish_timelock, - tx_redeem_fee: bitcoin_wallet.estimate_fee(ESTIMATED_WEIGHT_TX_REDEEM), - tx_punish_fee: bitcoin_wallet.estimate_fee(ESTIMATED_WEIGHT_TX_PUNISH), + tx_redeem_fee, + tx_punish_fee, }) } diff --git a/swap/src/protocol/bob/swap.rs b/swap/src/protocol/bob/swap.rs index 747b03d7..5bd2a0df 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(bitcoin::ESTIMATED_WEIGHT_TX_REDEEM); - let tx_cancel_fee = bitcoin_wallet.estimate_fee(bitcoin::ESTIMATED_WEIGHT_TX_CANCEL); + let tx_refund_fee = bitcoin_wallet + .estimate_fee(bitcoin::ESTIMATED_WEIGHT_TX_REDEEM) + .await?; + let tx_cancel_fee = bitcoin_wallet + .estimate_fee(bitcoin::ESTIMATED_WEIGHT_TX_CANCEL) + .await?; let state2 = request_price_and_setup( swap_id, diff --git a/swap/tests/harness/mod.rs b/swap/tests/harness/mod.rs index d4f9d96f..66f08821 100644 --- a/swap/tests/harness/mod.rs +++ b/swap/tests/harness/mod.rs @@ -14,7 +14,10 @@ use std::fmt; use std::path::{Path, PathBuf}; use std::sync::Arc; use std::time::Duration; -use swap::bitcoin::{CancelTimelock, PunishTimelock}; +use swap::bitcoin::{ + CancelTimelock, PunishTimelock, ESTIMATED_WEIGHT_TX_CANCEL, ESTIMATED_WEIGHT_TX_REDEEM, + ESTIMATED_WEIGHT_TX_REFUND, +}; use swap::database::Database; use swap::env::{Config, GetConfig}; use swap::network::swarm; @@ -35,14 +38,6 @@ use tracing_subscriber::util::SubscriberInitExt; use url::Url; use uuid::Uuid; -// Note: this needs to be adopted if bitcoin::wallet.select_feerate() or -// bitcoin::ESTIMATED_WEIGHT_TX_REDEEM changes. -// TODO: see if there is a better way. -const TX_REDEEM_FEE: u64 = 12500; -const TX_REFUND_FEE: u64 = 12500; -const TX_CANCEL_FEE: u64 = 12500; -const TX_PUNISH_FEE: u64 = 12500; - pub async fn setup_test(_config: C, testfn: T) where T: Fn(TestContext) -> F, @@ -293,6 +288,7 @@ async fn init_test_wallets( seed.derive_extended_private_key(env_config.bitcoin_network) .expect("Could not create extended private key from seed"), env_config, + 1, ) .await .expect("could not init btc wallet"); @@ -547,7 +543,7 @@ impl TestContext { assert_eventual_balance( self.alice_bitcoin_wallet.as_ref(), Ordering::Equal, - self.alice_redeemed_btc_balance(), + self.alice_redeemed_btc_balance().await, ) .await .unwrap(); @@ -588,7 +584,7 @@ impl TestContext { assert_eventual_balance( self.alice_bitcoin_wallet.as_ref(), Ordering::Equal, - self.alice_punished_btc_balance(), + self.alice_punished_btc_balance().await, ) .await .unwrap(); @@ -639,11 +635,19 @@ impl TestContext { let btc_balance_after_swap = self.bob_bitcoin_wallet.balance().await.unwrap(); + let cancel_fee = self + .alice_bitcoin_wallet + .estimate_fee(ESTIMATED_WEIGHT_TX_CANCEL) + .await + .expect("To estimate fee correctly"); + let refund_fee = self + .alice_bitcoin_wallet + .estimate_fee(ESTIMATED_WEIGHT_TX_REFUND) + .await + .expect("To estimate fee correctly"); + let bob_cancelled_and_refunded = btc_balance_after_swap - == self.bob_starting_balances.btc - - lock_tx_bitcoin_fee - - bitcoin::Amount::from_sat(TX_CANCEL_FEE) - - bitcoin::Amount::from_sat(TX_REFUND_FEE); + == self.bob_starting_balances.btc - lock_tx_bitcoin_fee - cancel_fee - refund_fee; assert!(bob_cancelled_and_refunded); @@ -678,9 +682,13 @@ impl TestContext { self.alice_starting_balances.xmr - self.xmr_amount } - fn alice_redeemed_btc_balance(&self) -> bitcoin::Amount { - self.alice_starting_balances.btc + self.btc_amount - - bitcoin::Amount::from_sat(TX_REDEEM_FEE) + async fn alice_redeemed_btc_balance(&self) -> bitcoin::Amount { + let fee = self + .alice_bitcoin_wallet + .estimate_fee(ESTIMATED_WEIGHT_TX_REDEEM) + .await + .expect("To estimate fee correctly"); + self.alice_starting_balances.btc + self.btc_amount - fee } fn bob_redeemed_xmr_balance(&self) -> monero::Amount { @@ -717,10 +725,18 @@ impl TestContext { self.alice_starting_balances.xmr - self.xmr_amount } - fn alice_punished_btc_balance(&self) -> bitcoin::Amount { - self.alice_starting_balances.btc + self.btc_amount - - bitcoin::Amount::from_sat(TX_CANCEL_FEE) - - bitcoin::Amount::from_sat(TX_PUNISH_FEE) + async fn alice_punished_btc_balance(&self) -> bitcoin::Amount { + let cancel_fee = self + .alice_bitcoin_wallet + .estimate_fee(ESTIMATED_WEIGHT_TX_CANCEL) + .await + .expect("To estimate fee correctly"); + let punish_fee = self + .alice_bitcoin_wallet + .estimate_fee(ESTIMATED_WEIGHT_TX_REFUND) + .await + .expect("To estimate fee correctly"); + self.alice_starting_balances.btc + self.btc_amount - cancel_fee - punish_fee } fn bob_punished_xmr_balance(&self) -> monero::Amount {