From 38540b4de5196ff43cfa09aa84d3bc434ac7d488 Mon Sep 17 00:00:00 2001 From: Philipp Hoenisch Date: Thu, 29 Apr 2021 10:59:40 +1000 Subject: [PATCH] Dynamically chose fee for TxCancel. Bob chooses the fee for TxCancel because he is the one that cares. --- swap/src/bitcoin.rs | 10 +--- swap/src/bitcoin/cancel.rs | 7 ++- swap/src/protocol.rs | 2 + swap/src/protocol/alice/state.rs | 33 ++++++++++-- swap/src/protocol/bob/state.rs | 90 +++++++++++++++++++++++++++----- swap/src/protocol/bob/swap.rs | 4 ++ swap/tests/harness/bitcoind.rs | 4 -- swap/tests/harness/mod.rs | 7 +-- 8 files changed, 121 insertions(+), 36 deletions(-) diff --git a/swap/src/bitcoin.rs b/swap/src/bitcoin.rs index c2f5e053..8df65e05 100644 --- a/swap/src/bitcoin.rs +++ b/swap/src/bitcoin.rs @@ -37,19 +37,11 @@ use serde::{Deserialize, Serialize}; use sha2::Sha256; use std::str::FromStr; +pub use crate::bitcoin::cancel::ESTIMATED_WEIGHT as ESTIMATED_WEIGHT_TX_CANCEL; pub use crate::bitcoin::punish::ESTIMATED_WEIGHT as ESTIMATED_WEIGHT_TX_PUNISH; pub use crate::bitcoin::redeem::ESTIMATED_WEIGHT as ESTIMATED_WEIGHT_TX_REDEEM; pub use crate::bitcoin::refund::ESTIMATED_WEIGHT as ESTIMATED_WEIGHT_TX_REFUND; -// 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 -// test) = 609 assuming segwit and 60 sat/vB: -// (609 / 4) * 60 (sat/vB) = 9135 sats -// Recommended: Overpay a bit to ensure we don't have to wait too long for test -// runs. -pub const TX_FEE: u64 = 15_000; - #[derive(Debug, Clone, Deserialize, Serialize, PartialEq)] pub struct SecretKey { inner: Scalar, diff --git a/swap/src/bitcoin/cancel.rs b/swap/src/bitcoin/cancel.rs index 31549408..3f05cbd9 100644 --- a/swap/src/bitcoin/cancel.rs +++ b/swap/src/bitcoin/cancel.rs @@ -2,7 +2,6 @@ use crate::bitcoin; use crate::bitcoin::wallet::Watchable; use crate::bitcoin::{ build_shared_output_descriptor, Address, Amount, BlockHeight, PublicKey, Transaction, TxLock, - TX_FEE, }; use ::bitcoin::util::bip143::SigHashCache; use ::bitcoin::{OutPoint, Script, SigHash, SigHashType, TxIn, TxOut, Txid}; @@ -14,6 +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; + /// Represent a timelock, expressed in relative block height as defined in /// [BIP68](https://github.com/bitcoin/bips/blob/master/bip-0068.mediawiki). /// E.g. The timelock expires 10 blocks after the reference transaction is @@ -96,6 +98,7 @@ impl TxCancel { cancel_timelock: CancelTimelock, A: PublicKey, B: PublicKey, + spending_fee: Amount, ) -> Self { let cancel_output_descriptor = build_shared_output_descriptor(A.0, B.0); @@ -107,7 +110,7 @@ impl TxCancel { }; let tx_out = TxOut { - value: tx_lock.lock_amount().as_sat() - TX_FEE, + value: tx_lock.lock_amount().as_sat() - spending_fee.as_sat(), script_pubkey: cancel_output_descriptor.script_pubkey(), }; diff --git a/swap/src/protocol.rs b/swap/src/protocol.rs index e37b4075..59b100c0 100644 --- a/swap/src/protocol.rs +++ b/swap/src/protocol.rs @@ -30,6 +30,8 @@ pub struct Message0 { refund_address: bitcoin::Address, #[serde(with = "::bitcoin::util::amount::serde::as_sat")] tx_refund_fee: bitcoin::Amount, + #[serde(with = "::bitcoin::util::amount::serde::as_sat")] + tx_cancel_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 4ee68202..5252c69a 100644 --- a/swap/src/protocol/alice/state.rs +++ b/swap/src/protocol/alice/state.rs @@ -191,6 +191,7 @@ impl State0 { tx_redeem_fee: self.tx_redeem_fee, tx_punish_fee: self.tx_punish_fee, tx_refund_fee: msg.tx_refund_fee, + tx_cancel_fee: msg.tx_cancel_fee, })) } } @@ -217,6 +218,7 @@ pub struct State1 { tx_redeem_fee: bitcoin::Amount, tx_punish_fee: bitcoin::Amount, tx_refund_fee: bitcoin::Amount, + tx_cancel_fee: bitcoin::Amount, } impl State1 { @@ -256,6 +258,7 @@ impl State1 { tx_redeem_fee: self.tx_redeem_fee, tx_punish_fee: self.tx_punish_fee, tx_refund_fee: self.tx_refund_fee, + tx_cancel_fee: self.tx_cancel_fee, }) } } @@ -279,12 +282,18 @@ pub struct State2 { tx_redeem_fee: bitcoin::Amount, tx_punish_fee: bitcoin::Amount, tx_refund_fee: bitcoin::Amount, + tx_cancel_fee: bitcoin::Amount, } impl State2 { pub fn next_message(&self) -> Message3 { - let tx_cancel = - bitcoin::TxCancel::new(&self.tx_lock, self.cancel_timelock, self.a.public(), self.B); + let tx_cancel = bitcoin::TxCancel::new( + &self.tx_lock, + self.cancel_timelock, + self.a.public(), + self.B, + self.tx_cancel_fee, + ); let tx_refund = bitcoin::TxRefund::new(&tx_cancel, &self.refund_address, self.tx_refund_fee); @@ -303,8 +312,13 @@ impl State2 { } pub fn receive(self, msg: Message4) -> Result { - let tx_cancel = - bitcoin::TxCancel::new(&self.tx_lock, self.cancel_timelock, self.a.public(), self.B); + let tx_cancel = bitcoin::TxCancel::new( + &self.tx_lock, + self.cancel_timelock, + self.a.public(), + self.B, + self.tx_cancel_fee, + ); bitcoin::verify_sig(&self.B, &tx_cancel.digest(), &msg.tx_cancel_sig) .context("Failed to verify cancel transaction")?; let tx_punish = bitcoin::TxPunish::new( @@ -336,6 +350,7 @@ impl State2 { tx_redeem_fee: self.tx_redeem_fee, tx_punish_fee: self.tx_punish_fee, tx_refund_fee: self.tx_refund_fee, + tx_cancel_fee: self.tx_cancel_fee, }) } } @@ -365,6 +380,8 @@ pub struct State3 { tx_punish_fee: bitcoin::Amount, #[serde(with = "::bitcoin::util::amount::serde::as_sat")] tx_refund_fee: bitcoin::Amount, + #[serde(with = "::bitcoin::util::amount::serde::as_sat")] + tx_cancel_fee: bitcoin::Amount, } impl State3 { @@ -417,7 +434,13 @@ impl State3 { } pub fn tx_cancel(&self) -> TxCancel { - TxCancel::new(&self.tx_lock, self.cancel_timelock, self.a.public(), self.B) + TxCancel::new( + &self.tx_lock, + self.cancel_timelock, + self.a.public(), + self.B, + self.tx_cancel_fee, + ) } pub fn tx_refund(&self) -> TxRefund { diff --git a/swap/src/protocol/bob/state.rs b/swap/src/protocol/bob/state.rs index 27387003..28d268cc 100644 --- a/swap/src/protocol/bob/state.rs +++ b/swap/src/protocol/bob/state.rs @@ -84,6 +84,7 @@ pub struct State0 { refund_address: bitcoin::Address, min_monero_confirmations: u64, tx_refund_fee: bitcoin::Amount, + tx_cancel_fee: bitcoin::Amount, } impl State0 { @@ -98,6 +99,7 @@ impl State0 { refund_address: bitcoin::Address, min_monero_confirmations: u64, tx_refund_fee: bitcoin::Amount, + tx_cancel_fee: bitcoin::Amount, ) -> Self { let b = bitcoin::SecretKey::new_random(rng); @@ -123,6 +125,7 @@ impl State0 { refund_address, min_monero_confirmations, tx_refund_fee, + tx_cancel_fee, } } @@ -136,6 +139,7 @@ impl State0 { v_b: self.v_b, refund_address: self.refund_address.clone(), tx_refund_fee: self.tx_refund_fee, + tx_cancel_fee: self.tx_cancel_fee, } } @@ -176,6 +180,7 @@ impl State0 { tx_redeem_fee: msg.tx_redeem_fee, tx_refund_fee: self.tx_refund_fee, tx_punish_fee: msg.tx_punish_fee, + tx_cancel_fee: self.tx_cancel_fee, }) } } @@ -199,6 +204,7 @@ pub struct State1 { tx_redeem_fee: bitcoin::Amount, tx_refund_fee: bitcoin::Amount, tx_punish_fee: bitcoin::Amount, + tx_cancel_fee: bitcoin::Amount, } impl State1 { @@ -209,7 +215,13 @@ impl State1 { } pub fn receive(self, msg: Message3) -> Result { - let tx_cancel = TxCancel::new(&self.tx_lock, self.cancel_timelock, self.A, self.b.public()); + let tx_cancel = TxCancel::new( + &self.tx_lock, + self.cancel_timelock, + self.A, + self.b.public(), + self.tx_cancel_fee, + ); let tx_refund = bitcoin::TxRefund::new(&tx_cancel, &self.refund_address, self.tx_refund_fee); @@ -241,6 +253,7 @@ impl State1 { tx_redeem_fee: self.tx_redeem_fee, tx_refund_fee: self.tx_refund_fee, tx_punish_fee: self.tx_punish_fee, + tx_cancel_fee: self.tx_cancel_fee, }) } } @@ -269,11 +282,19 @@ pub struct State2 { tx_punish_fee: bitcoin::Amount, #[serde(with = "::bitcoin::util::amount::serde::as_sat")] tx_refund_fee: bitcoin::Amount, + #[serde(with = "::bitcoin::util::amount::serde::as_sat")] + tx_cancel_fee: bitcoin::Amount, } impl State2 { pub fn next_message(&self) -> Message4 { - let tx_cancel = TxCancel::new(&self.tx_lock, self.cancel_timelock, self.A, self.b.public()); + let tx_cancel = TxCancel::new( + &self.tx_lock, + self.cancel_timelock, + self.A, + self.b.public(), + self.tx_cancel_fee, + ); let tx_cancel_sig = self.b.sign(tx_cancel.digest()); let tx_punish = bitcoin::TxPunish::new( &tx_cancel, @@ -309,6 +330,7 @@ impl State2 { min_monero_confirmations: self.min_monero_confirmations, tx_redeem_fee: self.tx_redeem_fee, tx_refund_fee: self.tx_refund_fee, + tx_cancel_fee: self.tx_cancel_fee, }, self.tx_lock, )) @@ -336,6 +358,8 @@ pub struct State3 { tx_redeem_fee: bitcoin::Amount, #[serde(with = "::bitcoin::util::amount::serde::as_sat")] tx_refund_fee: bitcoin::Amount, + #[serde(with = "::bitcoin::util::amount::serde::as_sat")] + tx_cancel_fee: bitcoin::Amount, } impl State3 { @@ -370,6 +394,7 @@ impl State3 { monero_wallet_restore_blockheight, tx_redeem_fee: self.tx_redeem_fee, tx_refund_fee: self.tx_refund_fee, + tx_cancel_fee: self.tx_cancel_fee, } } @@ -385,6 +410,7 @@ impl State3 { tx_cancel_sig_a: self.tx_cancel_sig_a.clone(), tx_refund_encsig: self.tx_refund_encsig.clone(), tx_refund_fee: self.tx_refund_fee, + tx_cancel_fee: self.tx_cancel_fee, } } @@ -396,7 +422,13 @@ impl State3 { &self, bitcoin_wallet: &bitcoin::Wallet, ) -> Result { - let tx_cancel = TxCancel::new(&self.tx_lock, self.cancel_timelock, self.A, self.b.public()); + let tx_cancel = TxCancel::new( + &self.tx_lock, + self.cancel_timelock, + self.A, + self.b.public(), + self.tx_cancel_fee, + ); let tx_lock_status = bitcoin_wallet.status_of_script(&self.tx_lock).await?; let tx_cancel_status = bitcoin_wallet.status_of_script(&tx_cancel).await?; @@ -429,6 +461,8 @@ pub struct State4 { tx_redeem_fee: bitcoin::Amount, #[serde(with = "::bitcoin::util::amount::serde::as_sat")] tx_refund_fee: bitcoin::Amount, + #[serde(with = "::bitcoin::util::amount::serde::as_sat")] + tx_cancel_fee: bitcoin::Amount, } impl State4 { @@ -469,7 +503,13 @@ impl State4 { &self, bitcoin_wallet: &bitcoin::Wallet, ) -> Result { - let tx_cancel = TxCancel::new(&self.tx_lock, self.cancel_timelock, self.A, self.b.public()); + let tx_cancel = TxCancel::new( + &self.tx_lock, + self.cancel_timelock, + self.A, + self.b.public(), + self.tx_cancel_fee, + ); let tx_lock_status = bitcoin_wallet.status_of_script(&self.tx_lock).await?; let tx_cancel_status = bitcoin_wallet.status_of_script(&tx_cancel).await?; @@ -494,6 +534,7 @@ impl State4 { tx_cancel_sig_a: self.tx_cancel_sig_a, tx_refund_encsig: self.tx_refund_encsig, tx_refund_fee: self.tx_refund_fee, + tx_cancel_fee: self.tx_cancel_fee, } } } @@ -534,6 +575,8 @@ pub struct State6 { tx_refund_encsig: bitcoin::EncryptedSignature, #[serde(with = "::bitcoin::util::amount::serde::as_sat")] pub tx_refund_fee: bitcoin::Amount, + #[serde(with = "::bitcoin::util::amount::serde::as_sat")] + pub tx_cancel_fee: bitcoin::Amount, } impl State6 { @@ -541,7 +584,13 @@ impl State6 { &self, bitcoin_wallet: &bitcoin::Wallet, ) -> Result { - let tx_cancel = TxCancel::new(&self.tx_lock, self.cancel_timelock, self.A, self.b.public()); + let tx_cancel = TxCancel::new( + &self.tx_lock, + self.cancel_timelock, + self.A, + self.b.public(), + self.tx_cancel_fee, + ); let tx_lock_status = bitcoin_wallet.status_of_script(&self.tx_lock).await?; let tx_cancel_status = bitcoin_wallet.status_of_script(&tx_cancel).await?; @@ -558,8 +607,13 @@ impl State6 { &self, bitcoin_wallet: &bitcoin::Wallet, ) -> Result { - let tx_cancel = - bitcoin::TxCancel::new(&self.tx_lock, self.cancel_timelock, self.A, self.b.public()); + let tx_cancel = bitcoin::TxCancel::new( + &self.tx_lock, + self.cancel_timelock, + self.A, + self.b.public(), + self.tx_cancel_fee, + ); let tx = bitcoin_wallet.get_raw_transaction(tx_cancel.txid()).await?; @@ -567,10 +621,15 @@ impl State6 { } pub async fn submit_tx_cancel(&self, bitcoin_wallet: &bitcoin::Wallet) -> Result { - let transaction = - bitcoin::TxCancel::new(&self.tx_lock, self.cancel_timelock, self.A, self.b.public()) - .complete_as_bob(self.A, self.b.clone(), self.tx_cancel_sig_a.clone()) - .context("Failed to complete Bitcoin cancel transaction")?; + let transaction = bitcoin::TxCancel::new( + &self.tx_lock, + self.cancel_timelock, + self.A, + self.b.public(), + self.tx_cancel_fee, + ) + .complete_as_bob(self.A, self.b.clone(), self.tx_cancel_sig_a.clone()) + .context("Failed to complete Bitcoin cancel transaction")?; let (tx_id, _) = bitcoin_wallet.broadcast(transaction, "cancel").await?; @@ -578,8 +637,13 @@ impl State6 { } pub async fn refund_btc(&self, bitcoin_wallet: &bitcoin::Wallet) -> Result<()> { - let tx_cancel = - bitcoin::TxCancel::new(&self.tx_lock, self.cancel_timelock, self.A, self.b.public()); + let tx_cancel = bitcoin::TxCancel::new( + &self.tx_lock, + self.cancel_timelock, + self.A, + self.b.public(), + self.tx_cancel_fee, + ); let tx_refund = bitcoin::TxRefund::new(&tx_cancel, &self.refund_address, self.tx_refund_fee); diff --git a/swap/src/protocol/bob/swap.rs b/swap/src/protocol/bob/swap.rs index 88a5a82f..747b03d7 100644 --- a/swap/src/protocol/bob/swap.rs +++ b/swap/src/protocol/bob/swap.rs @@ -67,6 +67,7 @@ async fn next_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 state2 = request_price_and_setup( swap_id, @@ -75,6 +76,7 @@ async fn next_state( env_config, bitcoin_refund_address, tx_refund_fee, + tx_cancel_fee, ) .await?; @@ -271,6 +273,7 @@ pub async fn request_price_and_setup( env_config: &Config, bitcoin_refund_address: bitcoin::Address, tx_refund_fee: bitcoin::Amount, + tx_cancel_fee: bitcoin::Amount, ) -> Result { let xmr = event_loop_handle.request_spot_price(btc).await?; @@ -286,6 +289,7 @@ pub async fn request_price_and_setup( bitcoin_refund_address, env_config.monero_finality_confirmations, tx_refund_fee, + tx_cancel_fee, ); let state2 = event_loop_handle.execution_setup(state0).await?; diff --git a/swap/tests/harness/bitcoind.rs b/swap/tests/harness/bitcoind.rs index dbe5787f..9f1fcd03 100644 --- a/swap/tests/harness/bitcoind.rs +++ b/swap/tests/harness/bitcoind.rs @@ -8,10 +8,6 @@ pub const RPC_PORT: u16 = 18443; pub const PORT: u16 = 18886; pub const DATADIR: &str = "/home/bdk"; -// Bitcoin regtest TX fee. The fee itself does not matter for our tests. It just -// has to be static so that we can assert on the amounts. -pub const TX_FEE: u64 = 15_000; - #[derive(Debug)] pub struct Bitcoind { args: BitcoindArgs, diff --git a/swap/tests/harness/mod.rs b/swap/tests/harness/mod.rs index c8a5d1d0..d4f9d96f 100644 --- a/swap/tests/harness/mod.rs +++ b/swap/tests/harness/mod.rs @@ -1,7 +1,6 @@ mod bitcoind; mod electrs; -use crate::harness::bitcoind::TX_FEE; use anyhow::{bail, Context, Result}; use async_trait::async_trait; use bitcoin_harness::{BitcoindRpcApi, Client}; @@ -41,7 +40,8 @@ use uuid::Uuid; // 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 = TX_FEE; +const TX_CANCEL_FEE: u64 = 12500; +const TX_PUNISH_FEE: u64 = 12500; pub async fn setup_test(_config: C, testfn: T) where @@ -719,7 +719,8 @@ impl TestContext { fn alice_punished_btc_balance(&self) -> bitcoin::Amount { self.alice_starting_balances.btc + self.btc_amount - - bitcoin::Amount::from_sat(2 * bitcoind::TX_FEE) + - bitcoin::Amount::from_sat(TX_CANCEL_FEE) + - bitcoin::Amount::from_sat(TX_PUNISH_FEE) } fn bob_punished_xmr_balance(&self) -> monero::Amount {