From 1012e395272da597664c80687012f427f784af03 Mon Sep 17 00:00:00 2001 From: Philipp Hoenisch Date: Thu, 29 Apr 2021 10:40:04 +1000 Subject: [PATCH] Dynamically chose fee for TxRefund and TxPunish. Alice chooses the fee for TxPunish because she is the one that cares. Bob chooses the fee for TxRefund because he 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/cancel.rs | 3 ++- swap/src/bitcoin/lock.rs | 4 +++- swap/src/bitcoin/punish.rs | 9 ++++++-- swap/src/bitcoin/refund.rs | 15 ++++++++----- swap/src/protocol.rs | 4 ++++ swap/src/protocol/alice/state.rs | 33 ++++++++++++++++++++++----- swap/src/protocol/bob/state.rs | 38 ++++++++++++++++++++++++++++---- swap/src/protocol/bob/swap.rs | 4 ++++ swap/tests/harness/bitcoind.rs | 2 +- swap/tests/harness/mod.rs | 17 ++++++-------- 11 files changed, 101 insertions(+), 30 deletions(-) diff --git a/swap/src/bitcoin.rs b/swap/src/bitcoin.rs index d2451e7d..c2f5e053 100644 --- a/swap/src/bitcoin.rs +++ b/swap/src/bitcoin.rs @@ -37,7 +37,9 @@ use serde::{Deserialize, Serialize}; use sha2::Sha256; use std::str::FromStr; +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: diff --git a/swap/src/bitcoin/cancel.rs b/swap/src/bitcoin/cancel.rs index d1d0bc43..31549408 100644 --- a/swap/src/bitcoin/cancel.rs +++ b/swap/src/bitcoin/cancel.rs @@ -216,6 +216,7 @@ impl TxCancel { &self, spend_address: &Address, sequence: Option, + spending_fee: Amount, ) -> Transaction { let previous_output = self.as_outpoint(); @@ -227,7 +228,7 @@ impl TxCancel { }; let tx_out = TxOut { - value: self.amount().as_sat() - TX_FEE, + value: self.amount().as_sat() - spending_fee.as_sat(), script_pubkey: spend_address.script_pubkey(), }; diff --git a/swap/src/bitcoin/lock.rs b/swap/src/bitcoin/lock.rs index 8df6c263..58d3ff9d 100644 --- a/swap/src/bitcoin/lock.rs +++ b/swap/src/bitcoin/lock.rs @@ -136,6 +136,7 @@ impl TxLock { &self, spend_address: &Address, sequence: Option, + spending_fee: Amount, ) -> Transaction { let previous_output = self.as_outpoint(); @@ -147,7 +148,8 @@ impl TxLock { }; let tx_out = TxOut { - value: self.inner.clone().extract_tx().output[self.lock_output_vout()].value - TX_FEE, + value: self.inner.clone().extract_tx().output[self.lock_output_vout()].value + - spending_fee.as_sat(), script_pubkey: spend_address.script_pubkey(), }; diff --git a/swap/src/bitcoin/punish.rs b/swap/src/bitcoin/punish.rs index 4845429d..c698f07c 100644 --- a/swap/src/bitcoin/punish.rs +++ b/swap/src/bitcoin/punish.rs @@ -1,5 +1,5 @@ use crate::bitcoin::wallet::Watchable; -use crate::bitcoin::{self, Address, PunishTimelock, Transaction, TxCancel, Txid}; +use crate::bitcoin::{self, Address, Amount, PunishTimelock, Transaction, TxCancel, Txid}; use ::bitcoin::util::bip143::SigHashCache; use ::bitcoin::{SigHash, SigHashType}; use anyhow::{Context, Result}; @@ -7,6 +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; + #[derive(Debug)] pub struct TxPunish { inner: Transaction, @@ -20,8 +23,10 @@ impl TxPunish { tx_cancel: &TxCancel, punish_address: &Address, punish_timelock: PunishTimelock, + spending_fee: Amount, ) -> Self { - let tx_punish = tx_cancel.build_spend_transaction(punish_address, Some(punish_timelock)); + let tx_punish = + tx_cancel.build_spend_transaction(punish_address, Some(punish_timelock), spending_fee); let digest = SigHashCache::new(&tx_punish).signature_hash( 0, // Only one input: cancel transaction diff --git a/swap/src/bitcoin/refund.rs b/swap/src/bitcoin/refund.rs index 34057c79..63d0818c 100644 --- a/swap/src/bitcoin/refund.rs +++ b/swap/src/bitcoin/refund.rs @@ -1,7 +1,7 @@ use crate::bitcoin::wallet::Watchable; use crate::bitcoin::{ - verify_sig, Address, EmptyWitnessStack, NoInputs, NotThreeWitnesses, PublicKey, TooManyInputs, - Transaction, TxCancel, + verify_sig, Address, Amount, EmptyWitnessStack, NoInputs, NotThreeWitnesses, PublicKey, + TooManyInputs, Transaction, TxCancel, }; use crate::{bitcoin, monero}; use ::bitcoin::util::bip143::SigHashCache; @@ -11,6 +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; + #[derive(Debug)] pub struct TxRefund { inner: Transaction, @@ -20,10 +23,10 @@ pub struct TxRefund { } impl TxRefund { - pub fn new(tx_cancel: &TxCancel, refund_address: &Address) -> Self { - let tx_punish = tx_cancel.build_spend_transaction(refund_address, None); + pub fn new(tx_cancel: &TxCancel, refund_address: &Address, spending_fee: Amount) -> Self { + let tx_refund = tx_cancel.build_spend_transaction(refund_address, None, spending_fee); - let digest = SigHashCache::new(&tx_punish).signature_hash( + let digest = SigHashCache::new(&tx_refund).signature_hash( 0, // Only one input: cancel transaction &tx_cancel.output_descriptor.script_code(), tx_cancel.amount().as_sat(), @@ -31,7 +34,7 @@ impl TxRefund { ); Self { - inner: tx_punish, + inner: tx_refund, digest, cancel_output_descriptor: tx_cancel.output_descriptor.clone(), watch_script: refund_address.script_pubkey(), diff --git a/swap/src/protocol.rs b/swap/src/protocol.rs index b4dd148f..e37b4075 100644 --- a/swap/src/protocol.rs +++ b/swap/src/protocol.rs @@ -28,6 +28,8 @@ pub struct Message0 { dleq_proof_s_b: CrossCurveDLEQProof, v_b: monero::PrivateViewKey, refund_address: bitcoin::Address, + #[serde(with = "::bitcoin::util::amount::serde::as_sat")] + tx_refund_fee: bitcoin::Amount, } #[derive(Clone, Debug, Serialize, Deserialize)] @@ -41,6 +43,8 @@ pub struct Message1 { punish_address: bitcoin::Address, #[serde(with = "::bitcoin::util::amount::serde::as_sat")] tx_redeem_fee: bitcoin::Amount, + #[serde(with = "::bitcoin::util::amount::serde::as_sat")] + tx_punish_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 f4cbe979..4ee68202 100644 --- a/swap/src/protocol/alice/state.rs +++ b/swap/src/protocol/alice/state.rs @@ -1,6 +1,6 @@ use crate::bitcoin::{ current_epoch, CancelTimelock, ExpiredTimelocks, PunishTimelock, TxCancel, TxPunish, TxRefund, - ESTIMATED_WEIGHT_TX_REDEEM, + ESTIMATED_WEIGHT_TX_PUNISH, ESTIMATED_WEIGHT_TX_REDEEM, }; use crate::env::Config; use crate::monero::wallet::{TransferRequest, WatchRequest}; @@ -110,6 +110,7 @@ pub struct State0 { redeem_address: bitcoin::Address, punish_address: bitcoin::Address, tx_redeem_fee: bitcoin::Amount, + tx_punish_fee: bitcoin::Amount, } impl State0 { @@ -147,6 +148,7 @@ impl State0 { 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), }) } @@ -187,6 +189,8 @@ impl State0 { redeem_address: self.redeem_address, punish_address: self.punish_address, tx_redeem_fee: self.tx_redeem_fee, + tx_punish_fee: self.tx_punish_fee, + tx_refund_fee: msg.tx_refund_fee, })) } } @@ -211,6 +215,8 @@ pub struct State1 { redeem_address: bitcoin::Address, punish_address: bitcoin::Address, tx_redeem_fee: bitcoin::Amount, + tx_punish_fee: bitcoin::Amount, + tx_refund_fee: bitcoin::Amount, } impl State1 { @@ -224,6 +230,7 @@ impl State1 { redeem_address: self.redeem_address.clone(), punish_address: self.punish_address.clone(), tx_redeem_fee: self.tx_redeem_fee, + tx_punish_fee: self.tx_punish_fee, } } @@ -247,6 +254,8 @@ impl State1 { punish_address: self.punish_address, tx_lock, tx_redeem_fee: self.tx_redeem_fee, + tx_punish_fee: self.tx_punish_fee, + tx_refund_fee: self.tx_refund_fee, }) } } @@ -268,6 +277,8 @@ pub struct State2 { punish_address: bitcoin::Address, tx_lock: bitcoin::TxLock, tx_redeem_fee: bitcoin::Amount, + tx_punish_fee: bitcoin::Amount, + tx_refund_fee: bitcoin::Amount, } impl State2 { @@ -275,7 +286,8 @@ impl State2 { let tx_cancel = bitcoin::TxCancel::new(&self.tx_lock, self.cancel_timelock, self.a.public(), self.B); - let tx_refund = bitcoin::TxRefund::new(&tx_cancel, &self.refund_address); + let tx_refund = + bitcoin::TxRefund::new(&tx_cancel, &self.refund_address, self.tx_refund_fee); // Alice encsigns the refund transaction(bitcoin) digest with Bob's monero // pubkey(S_b). The refund transaction spends the output of // tx_lock_bitcoin to Bob's refund address. @@ -295,8 +307,12 @@ impl State2 { bitcoin::TxCancel::new(&self.tx_lock, self.cancel_timelock, self.a.public(), self.B); bitcoin::verify_sig(&self.B, &tx_cancel.digest(), &msg.tx_cancel_sig) .context("Failed to verify cancel transaction")?; - let tx_punish = - bitcoin::TxPunish::new(&tx_cancel, &self.punish_address, self.punish_timelock); + let tx_punish = bitcoin::TxPunish::new( + &tx_cancel, + &self.punish_address, + self.punish_timelock, + self.tx_punish_fee, + ); bitcoin::verify_sig(&self.B, &tx_punish.digest(), &msg.tx_punish_sig) .context("Failed to verify punish transaction")?; @@ -318,6 +334,8 @@ impl State2 { tx_punish_sig_bob: msg.tx_punish_sig, tx_cancel_sig_bob: msg.tx_cancel_sig, tx_redeem_fee: self.tx_redeem_fee, + tx_punish_fee: self.tx_punish_fee, + tx_refund_fee: self.tx_refund_fee, }) } } @@ -343,6 +361,10 @@ pub struct State3 { tx_cancel_sig_bob: bitcoin::Signature, #[serde(with = "::bitcoin::util::amount::serde::as_sat")] tx_redeem_fee: bitcoin::Amount, + #[serde(with = "::bitcoin::util::amount::serde::as_sat")] + tx_punish_fee: bitcoin::Amount, + #[serde(with = "::bitcoin::util::amount::serde::as_sat")] + tx_refund_fee: bitcoin::Amount, } impl State3 { @@ -399,7 +421,7 @@ impl State3 { } pub fn tx_refund(&self) -> TxRefund { - bitcoin::TxRefund::new(&self.tx_cancel(), &self.refund_address) + bitcoin::TxRefund::new(&self.tx_cancel(), &self.refund_address, self.tx_refund_fee) } pub fn extract_monero_private_key( @@ -440,6 +462,7 @@ impl State3 { &self.tx_cancel(), &self.punish_address, self.punish_timelock, + self.tx_punish_fee, ) } } diff --git a/swap/src/protocol/bob/state.rs b/swap/src/protocol/bob/state.rs index 453159d4..27387003 100644 --- a/swap/src/protocol/bob/state.rs +++ b/swap/src/protocol/bob/state.rs @@ -83,6 +83,7 @@ pub struct State0 { punish_timelock: PunishTimelock, refund_address: bitcoin::Address, min_monero_confirmations: u64, + tx_refund_fee: bitcoin::Amount, } impl State0 { @@ -96,6 +97,7 @@ impl State0 { punish_timelock: PunishTimelock, refund_address: bitcoin::Address, min_monero_confirmations: u64, + tx_refund_fee: bitcoin::Amount, ) -> Self { let b = bitcoin::SecretKey::new_random(rng); @@ -120,6 +122,7 @@ impl State0 { punish_timelock, refund_address, min_monero_confirmations, + tx_refund_fee, } } @@ -132,6 +135,7 @@ impl State0 { dleq_proof_s_b: self.dleq_proof_s_b.clone(), v_b: self.v_b, refund_address: self.refund_address.clone(), + tx_refund_fee: self.tx_refund_fee, } } @@ -170,6 +174,8 @@ impl State0 { tx_lock, min_monero_confirmations: self.min_monero_confirmations, tx_redeem_fee: msg.tx_redeem_fee, + tx_refund_fee: self.tx_refund_fee, + tx_punish_fee: msg.tx_punish_fee, }) } } @@ -191,6 +197,8 @@ pub struct State1 { tx_lock: bitcoin::TxLock, min_monero_confirmations: u64, tx_redeem_fee: bitcoin::Amount, + tx_refund_fee: bitcoin::Amount, + tx_punish_fee: bitcoin::Amount, } impl State1 { @@ -202,7 +210,8 @@ 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_refund = bitcoin::TxRefund::new(&tx_cancel, &self.refund_address); + let tx_refund = + bitcoin::TxRefund::new(&tx_cancel, &self.refund_address, self.tx_refund_fee); bitcoin::verify_sig(&self.A, &tx_cancel.digest(), &msg.tx_cancel_sig)?; bitcoin::verify_encsig( @@ -230,6 +239,8 @@ impl State1 { tx_refund_encsig: msg.tx_refund_encsig, min_monero_confirmations: self.min_monero_confirmations, tx_redeem_fee: self.tx_redeem_fee, + tx_refund_fee: self.tx_refund_fee, + tx_punish_fee: self.tx_punish_fee, }) } } @@ -254,14 +265,22 @@ pub struct State2 { min_monero_confirmations: u64, #[serde(with = "::bitcoin::util::amount::serde::as_sat")] tx_redeem_fee: bitcoin::Amount, + #[serde(with = "::bitcoin::util::amount::serde::as_sat")] + tx_punish_fee: bitcoin::Amount, + #[serde(with = "::bitcoin::util::amount::serde::as_sat")] + tx_refund_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_sig = self.b.sign(tx_cancel.digest()); - let tx_punish = - bitcoin::TxPunish::new(&tx_cancel, &self.punish_address, self.punish_timelock); + let tx_punish = bitcoin::TxPunish::new( + &tx_cancel, + &self.punish_address, + self.punish_timelock, + self.tx_punish_fee, + ); let tx_punish_sig = self.b.sign(tx_punish.digest()); Message4 { @@ -289,6 +308,7 @@ impl State2 { tx_refund_encsig: self.tx_refund_encsig, min_monero_confirmations: self.min_monero_confirmations, tx_redeem_fee: self.tx_redeem_fee, + tx_refund_fee: self.tx_refund_fee, }, self.tx_lock, )) @@ -314,6 +334,8 @@ pub struct State3 { min_monero_confirmations: u64, #[serde(with = "::bitcoin::util::amount::serde::as_sat")] tx_redeem_fee: bitcoin::Amount, + #[serde(with = "::bitcoin::util::amount::serde::as_sat")] + tx_refund_fee: bitcoin::Amount, } impl State3 { @@ -347,6 +369,7 @@ impl State3 { tx_refund_encsig: self.tx_refund_encsig, monero_wallet_restore_blockheight, tx_redeem_fee: self.tx_redeem_fee, + tx_refund_fee: self.tx_refund_fee, } } @@ -361,6 +384,7 @@ impl State3 { tx_lock: self.tx_lock.clone(), 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, } } @@ -403,6 +427,8 @@ pub struct State4 { monero_wallet_restore_blockheight: BlockHeight, #[serde(with = "::bitcoin::util::amount::serde::as_sat")] tx_redeem_fee: bitcoin::Amount, + #[serde(with = "::bitcoin::util::amount::serde::as_sat")] + tx_refund_fee: bitcoin::Amount, } impl State4 { @@ -467,6 +493,7 @@ impl State4 { tx_lock: self.tx_lock, tx_cancel_sig_a: self.tx_cancel_sig_a, tx_refund_encsig: self.tx_refund_encsig, + tx_refund_fee: self.tx_refund_fee, } } } @@ -505,6 +532,8 @@ pub struct State6 { tx_lock: bitcoin::TxLock, tx_cancel_sig_a: Signature, tx_refund_encsig: bitcoin::EncryptedSignature, + #[serde(with = "::bitcoin::util::amount::serde::as_sat")] + pub tx_refund_fee: bitcoin::Amount, } impl State6 { @@ -551,7 +580,8 @@ 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_refund = bitcoin::TxRefund::new(&tx_cancel, &self.refund_address); + let tx_refund = + bitcoin::TxRefund::new(&tx_cancel, &self.refund_address, self.tx_refund_fee); let adaptor = Adaptor::, Deterministic>::default(); diff --git a/swap/src/protocol/bob/swap.rs b/swap/src/protocol/bob/swap.rs index 88ad040d..88a5a82f 100644 --- a/swap/src/protocol/bob/swap.rs +++ b/swap/src/protocol/bob/swap.rs @@ -66,6 +66,7 @@ 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 state2 = request_price_and_setup( swap_id, @@ -73,6 +74,7 @@ async fn next_state( event_loop_handle, env_config, bitcoin_refund_address, + tx_refund_fee, ) .await?; @@ -268,6 +270,7 @@ pub async fn request_price_and_setup( event_loop_handle: &mut EventLoopHandle, env_config: &Config, bitcoin_refund_address: bitcoin::Address, + tx_refund_fee: bitcoin::Amount, ) -> Result { let xmr = event_loop_handle.request_spot_price(btc).await?; @@ -282,6 +285,7 @@ pub async fn request_price_and_setup( env_config.bitcoin_punish_timelock, bitcoin_refund_address, env_config.monero_finality_confirmations, + tx_refund_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 91490f1a..dbe5787f 100644 --- a/swap/tests/harness/bitcoind.rs +++ b/swap/tests/harness/bitcoind.rs @@ -10,7 +10,7 @@ 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 = 10_000; +pub const TX_FEE: u64 = 15_000; #[derive(Debug)] pub struct Bitcoind { diff --git a/swap/tests/harness/mod.rs b/swap/tests/harness/mod.rs index 3c9ae60d..c8a5d1d0 100644 --- a/swap/tests/harness/mod.rs +++ b/swap/tests/harness/mod.rs @@ -1,6 +1,7 @@ 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}; @@ -39,6 +40,8 @@ use uuid::Uuid; // 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 = TX_FEE; pub async fn setup_test(_config: C, testfn: T) where @@ -636,19 +639,13 @@ impl TestContext { let btc_balance_after_swap = self.bob_bitcoin_wallet.balance().await.unwrap(); - let alice_submitted_cancel = btc_balance_after_swap + let bob_cancelled_and_refunded = btc_balance_after_swap == self.bob_starting_balances.btc - lock_tx_bitcoin_fee - - bitcoin::Amount::from_sat(bitcoind::TX_FEE); + - bitcoin::Amount::from_sat(TX_CANCEL_FEE) + - bitcoin::Amount::from_sat(TX_REFUND_FEE); - let bob_submitted_cancel = btc_balance_after_swap - == self.bob_starting_balances.btc - - lock_tx_bitcoin_fee - - bitcoin::Amount::from_sat(2 * bitcoind::TX_FEE); - - // The cancel tx can be submitted by both Alice and Bob. - // Since we cannot be sure who submitted it we have to assert accordingly - assert!(alice_submitted_cancel || bob_submitted_cancel); + assert!(bob_cancelled_and_refunded); assert_eventual_balance( self.bob_monero_wallet.as_ref(),