From ee90c228b43c97a1f1a79b8c3b41a1a3302857f4 Mon Sep 17 00:00:00 2001 From: Philipp Hoenisch Date: Sat, 1 May 2021 10:08:54 +1000 Subject: [PATCH] Dynamically calculate fees using electrum's estimate_fee. Electrum has an estimate-fee feature which takes as input the block you want a tx to be included. The result is a recommendation of BTC/vbyte. Using this recommendation and the knowledge about the size of our transactions we compute an appropriate fee. The size of the transactions were taken from real transactions as published on bitcoin testnet. Note: in reality these sizes might fluctuate a bit but not for much. --- swap/src/bin/asb.rs | 1 + swap/src/bin/swap.rs | 1 + swap/src/bitcoin/cancel.rs | 5 +- swap/src/bitcoin/lock.rs | 30 +++++++--- swap/src/bitcoin/punish.rs | 5 +- swap/src/bitcoin/redeem.rs | 5 +- swap/src/bitcoin/refund.rs | 5 +- swap/src/bitcoin/wallet.rs | 99 ++++++++++++++++++++++++-------- swap/src/protocol/alice/state.rs | 10 +++- swap/src/protocol/bob/swap.rs | 8 ++- swap/tests/harness/mod.rs | 60 ++++++++++++------- 11 files changed, 163 insertions(+), 66 deletions(-) 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 {