From d5c1b6693e6128f6538e9c89de025e8623a944c4 Mon Sep 17 00:00:00 2001 From: Philipp Hoenisch Date: Wed, 28 Apr 2021 17:08:00 +1000 Subject: [PATCH] Dynamically chose fee for TxRedeem. Alice chooses the fee for TxRedeem because she is the one that cares. Note must be taken here because if the fee is too low (e.g. < min tx fee) then she might not be able to publish TxRedeem at all. --- swap/src/bitcoin.rs | 2 ++ swap/src/bitcoin/lock.rs | 2 +- swap/src/bitcoin/redeem.rs | 9 ++++++--- swap/src/bitcoin/wallet.rs | 13 +++++++++++++ swap/src/protocol.rs | 2 ++ swap/src/protocol/alice/state.rs | 13 ++++++++++++- swap/src/protocol/bob/state.rs | 17 +++++++++++++++-- swap/tests/harness/mod.rs | 7 ++++++- 8 files changed, 57 insertions(+), 8 deletions(-) diff --git a/swap/src/bitcoin.rs b/swap/src/bitcoin.rs index 84434ffe..d2451e7d 100644 --- a/swap/src/bitcoin.rs +++ b/swap/src/bitcoin.rs @@ -37,6 +37,8 @@ use serde::{Deserialize, Serialize}; use sha2::Sha256; use std::str::FromStr; +pub use crate::bitcoin::redeem::ESTIMATED_WEIGHT as ESTIMATED_WEIGHT_TX_REDEEM; + // TODO: Configurable tx-fee (note: parties have to agree prior to swapping) // Current reasoning: // tx with largest weight (as determined by get_weight() upon broadcast in e2e diff --git a/swap/src/bitcoin/lock.rs b/swap/src/bitcoin/lock.rs index 63f2fb53..8df6c263 100644 --- a/swap/src/bitcoin/lock.rs +++ b/swap/src/bitcoin/lock.rs @@ -1,6 +1,6 @@ use crate::bitcoin::wallet::Watchable; use crate::bitcoin::{ - build_shared_output_descriptor, Address, Amount, PublicKey, Transaction, Wallet, TX_FEE, + build_shared_output_descriptor, Address, Amount, PublicKey, Transaction, Wallet, }; use ::bitcoin::util::psbt::PartiallySignedTransaction; use ::bitcoin::{OutPoint, TxIn, TxOut, Txid}; diff --git a/swap/src/bitcoin/redeem.rs b/swap/src/bitcoin/redeem.rs index da073693..87dc59a8 100644 --- a/swap/src/bitcoin/redeem.rs +++ b/swap/src/bitcoin/redeem.rs @@ -1,6 +1,6 @@ use crate::bitcoin::wallet::Watchable; use crate::bitcoin::{ - verify_encsig, verify_sig, Address, EmptyWitnessStack, EncryptedSignature, NoInputs, + verify_encsig, verify_sig, Address, Amount, EmptyWitnessStack, EncryptedSignature, NoInputs, NotThreeWitnesses, PublicKey, SecretKey, TooManyInputs, Transaction, TxLock, }; use ::bitcoin::util::bip143::SigHashCache; @@ -15,6 +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; + #[derive(Clone, Debug)] pub struct TxRedeem { inner: Transaction, @@ -24,10 +27,10 @@ pub struct TxRedeem { } impl TxRedeem { - pub fn new(tx_lock: &TxLock, redeem_address: &Address) -> Self { + pub fn new(tx_lock: &TxLock, redeem_address: &Address, spending_fee: Amount) -> Self { // lock_input is the shared output that is now being used as an input for the // redeem transaction - let tx_redeem = tx_lock.build_spend_transaction(redeem_address, None); + let tx_redeem = tx_lock.build_spend_transaction(redeem_address, None, spending_fee); let digest = SigHashCache::new(&tx_redeem).signature_hash( 0, // Only one input: lock_input (lock transaction) diff --git a/swap/src/bitcoin/wallet.rs b/swap/src/bitcoin/wallet.rs index a5efa20f..872f0c37 100644 --- a/swap/src/bitcoin/wallet.rs +++ b/swap/src/bitcoin/wallet.rs @@ -347,6 +347,19 @@ impl Wallet { // 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) + } } #[cfg(test)] diff --git a/swap/src/protocol.rs b/swap/src/protocol.rs index 58b616b3..b4dd148f 100644 --- a/swap/src/protocol.rs +++ b/swap/src/protocol.rs @@ -39,6 +39,8 @@ pub struct Message1 { v_a: monero::PrivateViewKey, redeem_address: bitcoin::Address, punish_address: bitcoin::Address, + #[serde(with = "::bitcoin::util::amount::serde::as_sat")] + tx_redeem_fee: bitcoin::Amount, } #[derive(Clone, Debug, Serialize, Deserialize)] diff --git a/swap/src/protocol/alice/state.rs b/swap/src/protocol/alice/state.rs index 86810e5b..f4cbe979 100644 --- a/swap/src/protocol/alice/state.rs +++ b/swap/src/protocol/alice/state.rs @@ -1,5 +1,6 @@ use crate::bitcoin::{ current_epoch, CancelTimelock, ExpiredTimelocks, PunishTimelock, TxCancel, TxPunish, TxRefund, + ESTIMATED_WEIGHT_TX_REDEEM, }; use crate::env::Config; use crate::monero::wallet::{TransferRequest, WatchRequest}; @@ -108,6 +109,7 @@ pub struct State0 { punish_timelock: PunishTimelock, redeem_address: bitcoin::Address, punish_address: bitcoin::Address, + tx_redeem_fee: bitcoin::Amount, } impl State0 { @@ -144,6 +146,7 @@ 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), }) } @@ -183,6 +186,7 @@ impl State0 { refund_address: msg.refund_address, redeem_address: self.redeem_address, punish_address: self.punish_address, + tx_redeem_fee: self.tx_redeem_fee, })) } } @@ -206,6 +210,7 @@ pub struct State1 { refund_address: bitcoin::Address, redeem_address: bitcoin::Address, punish_address: bitcoin::Address, + tx_redeem_fee: bitcoin::Amount, } impl State1 { @@ -218,6 +223,7 @@ impl State1 { v_a: self.v_a, redeem_address: self.redeem_address.clone(), punish_address: self.punish_address.clone(), + tx_redeem_fee: self.tx_redeem_fee, } } @@ -240,6 +246,7 @@ impl State1 { redeem_address: self.redeem_address, punish_address: self.punish_address, tx_lock, + tx_redeem_fee: self.tx_redeem_fee, }) } } @@ -260,6 +267,7 @@ pub struct State2 { redeem_address: bitcoin::Address, punish_address: bitcoin::Address, tx_lock: bitcoin::TxLock, + tx_redeem_fee: bitcoin::Amount, } impl State2 { @@ -309,6 +317,7 @@ impl State2 { tx_lock: self.tx_lock, tx_punish_sig_bob: msg.tx_punish_sig, tx_cancel_sig_bob: msg.tx_cancel_sig, + tx_redeem_fee: self.tx_redeem_fee, }) } } @@ -332,6 +341,8 @@ pub struct State3 { pub tx_lock: bitcoin::TxLock, tx_punish_sig_bob: bitcoin::Signature, tx_cancel_sig_bob: bitcoin::Signature, + #[serde(with = "::bitcoin::util::amount::serde::as_sat")] + tx_redeem_fee: bitcoin::Amount, } impl State3 { @@ -407,7 +418,7 @@ impl State3 { &self, sig: bitcoin::EncryptedSignature, ) -> Result { - bitcoin::TxRedeem::new(&self.tx_lock, &self.redeem_address) + bitcoin::TxRedeem::new(&self.tx_lock, &self.redeem_address, self.tx_redeem_fee) .complete(sig, self.a.clone(), self.s_a.to_secpfun_scalar(), self.B) .context("Failed to complete Bitcoin redeem transaction") } diff --git a/swap/src/protocol/bob/state.rs b/swap/src/protocol/bob/state.rs index df184f6e..453159d4 100644 --- a/swap/src/protocol/bob/state.rs +++ b/swap/src/protocol/bob/state.rs @@ -169,6 +169,7 @@ impl State0 { punish_address: msg.punish_address, tx_lock, min_monero_confirmations: self.min_monero_confirmations, + tx_redeem_fee: msg.tx_redeem_fee, }) } } @@ -189,6 +190,7 @@ pub struct State1 { punish_address: bitcoin::Address, tx_lock: bitcoin::TxLock, min_monero_confirmations: u64, + tx_redeem_fee: bitcoin::Amount, } impl State1 { @@ -227,6 +229,7 @@ impl State1 { tx_cancel_sig_a: msg.tx_cancel_sig, tx_refund_encsig: msg.tx_refund_encsig, min_monero_confirmations: self.min_monero_confirmations, + tx_redeem_fee: self.tx_redeem_fee, }) } } @@ -249,6 +252,8 @@ pub struct State2 { tx_cancel_sig_a: Signature, tx_refund_encsig: bitcoin::EncryptedSignature, min_monero_confirmations: u64, + #[serde(with = "::bitcoin::util::amount::serde::as_sat")] + tx_redeem_fee: bitcoin::Amount, } impl State2 { @@ -283,6 +288,7 @@ impl State2 { tx_cancel_sig_a: self.tx_cancel_sig_a, tx_refund_encsig: self.tx_refund_encsig, min_monero_confirmations: self.min_monero_confirmations, + tx_redeem_fee: self.tx_redeem_fee, }, self.tx_lock, )) @@ -306,6 +312,8 @@ pub struct State3 { tx_cancel_sig_a: Signature, tx_refund_encsig: bitcoin::EncryptedSignature, min_monero_confirmations: u64, + #[serde(with = "::bitcoin::util::amount::serde::as_sat")] + tx_redeem_fee: bitcoin::Amount, } impl State3 { @@ -338,6 +346,7 @@ impl State3 { tx_cancel_sig_a: self.tx_cancel_sig_a, tx_refund_encsig: self.tx_refund_encsig, monero_wallet_restore_blockheight, + tx_redeem_fee: self.tx_redeem_fee, } } @@ -392,16 +401,20 @@ pub struct State4 { tx_cancel_sig_a: Signature, tx_refund_encsig: bitcoin::EncryptedSignature, monero_wallet_restore_blockheight: BlockHeight, + #[serde(with = "::bitcoin::util::amount::serde::as_sat")] + tx_redeem_fee: bitcoin::Amount, } impl State4 { pub fn tx_redeem_encsig(&self) -> bitcoin::EncryptedSignature { - let tx_redeem = bitcoin::TxRedeem::new(&self.tx_lock, &self.redeem_address); + let tx_redeem = + bitcoin::TxRedeem::new(&self.tx_lock, &self.redeem_address, self.tx_redeem_fee); self.b.encsign(self.S_a_bitcoin, tx_redeem.digest()) } pub async fn watch_for_redeem_btc(&self, bitcoin_wallet: &bitcoin::Wallet) -> Result { - let tx_redeem = bitcoin::TxRedeem::new(&self.tx_lock, &self.redeem_address); + let tx_redeem = + bitcoin::TxRedeem::new(&self.tx_lock, &self.redeem_address, self.tx_redeem_fee); let tx_redeem_encsig = self.b.encsign(self.S_a_bitcoin, tx_redeem.digest()); bitcoin_wallet diff --git a/swap/tests/harness/mod.rs b/swap/tests/harness/mod.rs index eda8c504..3c9ae60d 100644 --- a/swap/tests/harness/mod.rs +++ b/swap/tests/harness/mod.rs @@ -35,6 +35,11 @@ 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; + pub async fn setup_test(_config: C, testfn: T) where T: Fn(TestContext) -> F, @@ -678,7 +683,7 @@ impl TestContext { fn alice_redeemed_btc_balance(&self) -> bitcoin::Amount { self.alice_starting_balances.btc + self.btc_amount - - bitcoin::Amount::from_sat(bitcoind::TX_FEE) + - bitcoin::Amount::from_sat(TX_REDEEM_FEE) } fn bob_redeemed_xmr_balance(&self) -> monero::Amount {