frost/bdk
3
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge bitcoindevkit/bdk#666: Various fixes to the fee_amount calculation in create_tx

Browse Source 
419dc248b667db05295cd4c68347c4ef51f51023 test: Document `test_bump_fee_add_input_change_dust` (Daniela Brozzoni)
632dabaa07ef9c58926facf0af5190f62bb65d12 test: Check tx feerate with longer signatures (Daniela Brozzoni)
2756411ef7cf0415baf2f2401e2d5a78481d0aa1 test: Reproduce #660 conditions (Daniela Brozzoni)
50af51da5a5c906d8bf660d35a4f922ceb996068 test: Fix P2WPKH_FAKE_WITNESS_SIZE (Daniela Brozzoni)
ae919061e2b341ae74c90f0133ba392e835cb4e1 Take into account the segwit tx header when... ...selecting coins (Daniela Brozzoni)
7ac87b8f99fc0897753ce57d48ea24adf495a633 TXIN_BASE_WEIGHT shouldn't include the script len (Daniela Brozzoni)
ac051d7ae9512883e11a89ab002ad2d2c3c55c19 Calculate fee amount after output addition (Daniela Brozzoni)
00d426b88546a346820c102386cd1bfff82ca8f6 test: Check that the feerate is never below... ...the requested one in assert_fee_rate (Daniela Brozzoni)
42fde6d4575b4aea121286f884f712b1c1cf64be test: Check fee_amount in assert_fee_rate (Daniela Brozzoni)

Pull request description:

  ### Description

  This PR mainly fixes two bugs:
  1. TXIN_BASE_WEIGHT wrongly included the `script_len` (Fixes #160)
  2. We wouldn't take into account the segwit header in the fee calculation, which could have resulted in a transaction with a lower feerate than the requested one
  3. In tests we used to push 108 bytes on the witness as a fake signature, but we should have pushed 106 instead

  I also add a test to reproduce the conditions of #660, to check if it's solved. Turns out it's been solved already in #630, but if you're curious about what the bug was, here it is: https://github.com/bitcoindevkit/bdk/issues/660#issuecomment-1196436776
  ### Checklists

  #### All Submissions:

  * [x] I've signed all my commits
  * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
  * [x] I ran `cargo fmt` and `cargo clippy` before committing

  #### Bugfixes:

  * [ ] This pull request breaks the existing API
  * [x] I've added tests to reproduce the issue which are now passing
  * [x] I'm linking the issue being fixed by this PR

ACKs for top commit:
  afilini:
    ACK 419dc248b667db05295cd4c68347c4ef51f51023

Tree-SHA512: c7b55342eac440a3607a16b94560cb9c08c4805c853432adfda8e21c5177f85d5a8afe0e7e61140e92c8f10934332459c6234fc5f1509ea699d97b1d04f030c6
This commit is contained in:
Alekos Filini 2022-08-03 11:40:17 +02:00
commit 1730e0150f
No known key found for this signature in database
GPG Key ID: 431401E4A4530061
2 changed files with 181 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
src/wallet/coin_selection.rs
View File

@ -30,7 +30,7 @@
//! # use bdk::database::Database; //! # use bdk::database::Database;
//! # use bdk::*; //! # use bdk::*;
//! # use bdk::wallet::coin_selection::decide_change; //! # use bdk::wallet::coin_selection::decide_change;
//! # const TXIN_BASE_WEIGHT: usize = (32 + 4 + 4 + 1) * 4; //! # const TXIN_BASE_WEIGHT: usize = (32 + 4 + 4) * 4;
//! #[derive(Debug)] //! #[derive(Debug)]
//! struct AlwaysSpendEverything; //! struct AlwaysSpendEverything;
//! //!
@ -119,8 +119,8 @@ pub type DefaultCoinSelectionAlgorithm = BranchAndBoundCoinSelection;
pub type DefaultCoinSelectionAlgorithm = LargestFirstCoinSelection; // make the tests more predictable pub type DefaultCoinSelectionAlgorithm = LargestFirstCoinSelection; // make the tests more predictable
// Base weight of a Txin, not counting the weight needed for satisfying it. // Base weight of a Txin, not counting the weight needed for satisfying it.
// prev_txid (32 bytes) + prev_vout (4 bytes) + sequence (4 bytes) + script_len (1 bytes) // prev_txid (32 bytes) + prev_vout (4 bytes) + sequence (4 bytes)
pub(crate) const TXIN_BASE_WEIGHT: usize = (32 + 4 + 4 + 1) * 4; pub(crate) const TXIN_BASE_WEIGHT: usize = (32 + 4 + 4) * 4;
#[derive(Debug)] #[derive(Debug)]
/// Remaining amount after performing coin selection /// Remaining amount after performing coin selection
@ -731,7 +731,7 @@ mod test {
use rand::seq::SliceRandom; use rand::seq::SliceRandom;
use rand::{Rng, SeedableRng}; use rand::{Rng, SeedableRng};
const P2WPKH_WITNESS_SIZE: usize = 73 + 33 + 2; const P2WPKH_SATISFACTION_SIZE: usize = 73 + 33 + 2 + 1;
const FEE_AMOUNT: u64 = 50; const FEE_AMOUNT: u64 = 50;
@ -743,7 +743,7 @@ mod test {
)) ))
.unwrap(); .unwrap();
WeightedUtxo { WeightedUtxo {
satisfaction_weight: P2WPKH_WITNESS_SIZE, satisfaction_weight: P2WPKH_SATISFACTION_SIZE,
utxo: Utxo::Local(LocalUtxo { utxo: Utxo::Local(LocalUtxo {
outpoint, outpoint,
txout: TxOut { txout: TxOut {
@ -823,7 +823,7 @@ mod test {
let mut res = Vec::new(); let mut res = Vec::new();
for _ in 0..utxos_number { for _ in 0..utxos_number {
res.push(WeightedUtxo { res.push(WeightedUtxo {
satisfaction_weight: P2WPKH_WITNESS_SIZE, satisfaction_weight: P2WPKH_SATISFACTION_SIZE,
utxo: Utxo::Local(LocalUtxo { utxo: Utxo::Local(LocalUtxo {
outpoint: OutPoint::from_str( outpoint: OutPoint::from_str(
"ebd9813ecebc57ff8f30797de7c205e3c7498ca950ea4341ee51a685ff2fa30a:0", "ebd9813ecebc57ff8f30797de7c205e3c7498ca950ea4341ee51a685ff2fa30a:0",
@ -843,7 +843,7 @@ mod test {
fn generate_same_value_utxos(utxos_value: u64, utxos_number: usize) -> Vec<WeightedUtxo> { fn generate_same_value_utxos(utxos_value: u64, utxos_number: usize) -> Vec<WeightedUtxo> {
let utxo = WeightedUtxo { let utxo = WeightedUtxo {
satisfaction_weight: P2WPKH_WITNESS_SIZE, satisfaction_weight: P2WPKH_SATISFACTION_SIZE,
utxo: Utxo::Local(LocalUtxo { utxo: Utxo::Local(LocalUtxo {
outpoint: OutPoint::from_str( outpoint: OutPoint::from_str(
"ebd9813ecebc57ff8f30797de7c205e3c7498ca950ea4341ee51a685ff2fa30a:0", "ebd9813ecebc57ff8f30797de7c205e3c7498ca950ea4341ee51a685ff2fa30a:0",
@ -1313,7 +1313,7 @@ mod test {
assert_eq!(result.selected.len(), 1); assert_eq!(result.selected.len(), 1);
assert_eq!(result.selected_amount(), 100_000); assert_eq!(result.selected_amount(), 100_000);
let input_size = (TXIN_BASE_WEIGHT + P2WPKH_WITNESS_SIZE).vbytes(); let input_size = (TXIN_BASE_WEIGHT + P2WPKH_SATISFACTION_SIZE).vbytes();
let epsilon = 0.5; let epsilon = 0.5;
assert!((1.0 - (result.fee_amount as f32 / input_size as f32)).abs() < epsilon); assert!((1.0 - (result.fee_amount as f32 / input_size as f32)).abs() < epsilon);
} }

203
src/wallet/mod.rs
View File

@ -736,8 +736,6 @@ where
let mut outgoing: u64 = 0; let mut outgoing: u64 = 0;
let mut received: u64 = 0; let mut received: u64 = 0;
fee_amount += fee_rate.fee_wu(tx.weight());
let recipients = params.recipients.iter().map(|(r, v)| (r, *v)); let recipients = params.recipients.iter().map(|(r, v)| (r, *v));
for (index, (script_pubkey, value)) in recipients.enumerate() { for (index, (script_pubkey, value)) in recipients.enumerate() {
@ -753,13 +751,25 @@ where
script_pubkey: script_pubkey.clone(), script_pubkey: script_pubkey.clone(),
value, value,
}; };
fee_amount += fee_rate.fee_vb(serialize(&new_out).len());
tx.output.push(new_out); tx.output.push(new_out);
outgoing += value; outgoing += value;
} }
fee_amount += fee_rate.fee_wu(tx.weight());
// Segwit transactions' header is 2WU larger than legacy txs' header,
// as they contain a witness marker (1WU) and a witness flag (1WU) (see BIP144).
// At this point we really don't know if the resulting transaction will be segwit
// or legacy, so we just add this 2WU to the fee_amount - overshooting the fee amount
// is better than undershooting it.
// If we pass a fee_amount that is slightly higher than the final fee_amount, we
// end up with a transaction with a slightly higher fee rate than the requested one.
// If, instead, we undershoot, we may end up with a feerate lower than the requested one
// - we might come up with non broadcastable txs!
fee_amount += fee_rate.fee_wu(2);
if params.change_policy != tx_builder::ChangeSpendPolicy::ChangeAllowed if params.change_policy != tx_builder::ChangeSpendPolicy::ChangeAllowed
&& self.change_descriptor.is_none() && self.change_descriptor.is_none()
{ {
@ -851,6 +861,9 @@ where
script_pubkey: drain_script, script_pubkey: drain_script,
}; };
// TODO: We should pay attention when adding a new output: this might increase
// the lenght of the "number of vouts" parameter by 2 bytes, potentially making
// our feerate too low
tx.output.push(drain_output); tx.output.push(drain_output);
} }
}; };
@ -1858,6 +1871,14 @@ pub(crate) mod test {
use crate::signer::{SignOptions, SignerError}; use crate::signer::{SignOptions, SignerError};
use crate::wallet::AddressIndex::{LastUnused, New, Peek, Reset}; use crate::wallet::AddressIndex::{LastUnused, New, Peek, Reset};
// The satisfaction size of a P2WPKH is 112 WU =
// 1 (elements in witness) + 1 (OP_PUSH) + 33 (pk) + 1 (OP_PUSH) + 72 (signature + sighash) + 1*4 (script len)
// On the witness itself, we have to push once for the pk (33WU) and once for signature + sighash (72WU), for
// a total of 105 WU.
// Here, we push just once for simplicity, so we have to add an extra byte for the missing
// OP_PUSH.
const P2WPKH_FAKE_WITNESS_SIZE: usize = 106;
#[test] #[test]
fn test_cache_addresses_fixed() { fn test_cache_addresses_fixed() {
let db = MemoryDatabase::new(); let db = MemoryDatabase::new();
@ -1992,12 +2013,14 @@ pub(crate) mod test {
} }
macro_rules! assert_fee_rate { macro_rules! assert_fee_rate {
($tx:expr, $fees:expr, $fee_rate:expr $( ,@dust_change $( $dust_change:expr )* )* $( ,@add_signature $( $add_signature:expr )* )* ) => ({ ($psbt:expr, $fees:expr, $fee_rate:expr $( ,@dust_change $( $dust_change:expr )* )* $( ,@add_signature $( $add_signature:expr )* )* ) => ({
let mut tx = $tx.clone(); let psbt = $psbt.clone();
#[allow(unused_mut)]
let mut tx = $psbt.clone().extract_tx();
$( $(
$( $add_signature )* $( $add_signature )*
for txin in &mut tx.input { for txin in &mut tx.input {
txin.witness.push([0x00; 108]); // fake signature txin.witness.push([0x00; P2WPKH_FAKE_WITNESS_SIZE]); // fake signature
} }
)* )*
@ -2009,11 +2032,23 @@ pub(crate) mod test {
dust_change = true; dust_change = true;
)* )*
let fee_amount = psbt
.inputs
.iter()
.fold(0, |acc, i| acc + i.witness_utxo.as_ref().unwrap().value)
- psbt
.unsigned_tx
.output
.iter()
.fold(0, |acc, o| acc + o.value);
assert_eq!(fee_amount, $fees);
let tx_fee_rate = FeeRate::from_wu($fees, tx.weight()); let tx_fee_rate = FeeRate::from_wu($fees, tx.weight());
let fee_rate = $fee_rate; let fee_rate = $fee_rate;
if !dust_change { if !dust_change {
assert!((tx_fee_rate - fee_rate).as_sat_vb().abs() < 0.5, "Expected fee rate of {:?}, the tx has {:?}", fee_rate, tx_fee_rate); assert!(tx_fee_rate >= fee_rate && (tx_fee_rate - fee_rate).as_sat_vb().abs() < 0.5, "Expected fee rate of {:?}, the tx has {:?}", fee_rate, tx_fee_rate);
} else { } else {
assert!(tx_fee_rate >= fee_rate, "Expected fee rate of at least {:?}, the tx has {:?}", fee_rate, tx_fee_rate); assert!(tx_fee_rate >= fee_rate, "Expected fee rate of at least {:?}, the tx has {:?}", fee_rate, tx_fee_rate);
} }
@ -2388,7 +2423,7 @@ pub(crate) mod test {
builder.add_recipient(addr.script_pubkey(), 25_000); builder.add_recipient(addr.script_pubkey(), 25_000);
let (psbt, details) = builder.finish().unwrap(); let (psbt, details) = builder.finish().unwrap();
assert_fee_rate!(psbt.extract_tx(), details.fee.unwrap_or(0), FeeRate::default(), @add_signature); assert_fee_rate!(psbt, details.fee.unwrap_or(0), FeeRate::default(), @add_signature);
} }
#[test] #[test]
@ -2401,7 +2436,7 @@ pub(crate) mod test {
.fee_rate(FeeRate::from_sat_per_vb(5.0)); .fee_rate(FeeRate::from_sat_per_vb(5.0));
let (psbt, details) = builder.finish().unwrap(); let (psbt, details) = builder.finish().unwrap();
assert_fee_rate!(psbt.extract_tx(), details.fee.unwrap_or(0), FeeRate::from_sat_per_vb(5.0), @add_signature); assert_fee_rate!(psbt, details.fee.unwrap_or(0), FeeRate::from_sat_per_vb(5.0), @add_signature);
} }
#[test] #[test]
@ -3214,7 +3249,7 @@ pub(crate) mod test {
let txid = tx.txid(); let txid = tx.txid();
// skip saving the new utxos, we know they can't be used anyways // skip saving the new utxos, we know they can't be used anyways
for txin in &mut tx.input { for txin in &mut tx.input {
txin.witness.push([0x00; 108]); // fake signature txin.witness.push([0x00; P2WPKH_FAKE_WITNESS_SIZE]); // fake signature
wallet wallet
.database .database
.borrow_mut() .borrow_mut()
@ -3258,7 +3293,7 @@ pub(crate) mod test {
details.received details.received
); );
assert_fee_rate!(psbt.extract_tx(), details.fee.unwrap_or(0), FeeRate::from_sat_per_vb(2.5), @add_signature); assert_fee_rate!(psbt, details.fee.unwrap_or(0), FeeRate::from_sat_per_vb(2.5), @add_signature);
} }
#[test] #[test]
@ -3274,7 +3309,7 @@ pub(crate) mod test {
let txid = tx.txid(); let txid = tx.txid();
// skip saving the new utxos, we know they can't be used anyways // skip saving the new utxos, we know they can't be used anyways
for txin in &mut tx.input { for txin in &mut tx.input {
txin.witness.push([0x00; 108]); // fake signature txin.witness.push([0x00; P2WPKH_FAKE_WITNESS_SIZE]); // fake signature
wallet wallet
.database .database
.borrow_mut() .borrow_mut()
@ -3340,7 +3375,7 @@ pub(crate) mod test {
let mut tx = psbt.extract_tx(); let mut tx = psbt.extract_tx();
let txid = tx.txid(); let txid = tx.txid();
for txin in &mut tx.input { for txin in &mut tx.input {
txin.witness.push([0x00; 108]); // fake signature txin.witness.push([0x00; P2WPKH_FAKE_WITNESS_SIZE]); // fake signature
wallet wallet
.database .database
.borrow_mut() .borrow_mut()
@ -3368,7 +3403,7 @@ pub(crate) mod test {
assert_eq!(tx.output.len(), 1); assert_eq!(tx.output.len(), 1);
assert_eq!(tx.output[0].value + details.fee.unwrap_or(0), details.sent); assert_eq!(tx.output[0].value + details.fee.unwrap_or(0), details.sent);
assert_fee_rate!(psbt.extract_tx(), details.fee.unwrap_or(0), FeeRate::from_sat_per_vb(2.5), @add_signature); assert_fee_rate!(psbt, details.fee.unwrap_or(0), FeeRate::from_sat_per_vb(2.5), @add_signature);
} }
#[test] #[test]
@ -3384,7 +3419,7 @@ pub(crate) mod test {
let mut tx = psbt.extract_tx(); let mut tx = psbt.extract_tx();
let txid = tx.txid(); let txid = tx.txid();
for txin in &mut tx.input { for txin in &mut tx.input {
txin.witness.push([0x00; 108]); // fake signature txin.witness.push([0x00; P2WPKH_FAKE_WITNESS_SIZE]); // fake signature
wallet wallet
.database .database
.borrow_mut() .borrow_mut()
@ -3440,7 +3475,7 @@ pub(crate) mod test {
let mut tx = psbt.extract_tx(); let mut tx = psbt.extract_tx();
let txid = tx.txid(); let txid = tx.txid();
for txin in &mut tx.input { for txin in &mut tx.input {
txin.witness.push([0x00; 108]); // fake signature txin.witness.push([0x00; P2WPKH_FAKE_WITNESS_SIZE]); // fake signature
wallet wallet
.database .database
.borrow_mut() .borrow_mut()
@ -3497,7 +3532,7 @@ pub(crate) mod test {
let mut tx = psbt.extract_tx(); let mut tx = psbt.extract_tx();
let txid = tx.txid(); let txid = tx.txid();
for txin in &mut tx.input { for txin in &mut tx.input {
txin.witness.push([0x00; 108]); // fake signature txin.witness.push([0x00; P2WPKH_FAKE_WITNESS_SIZE]); // fake signature
wallet wallet
.database .database
.borrow_mut() .borrow_mut()
@ -3538,7 +3573,7 @@ pub(crate) mod test {
let txid = tx.txid(); let txid = tx.txid();
// skip saving the new utxos, we know they can't be used anyways // skip saving the new utxos, we know they can't be used anyways
for txin in &mut tx.input { for txin in &mut tx.input {
txin.witness.push([0x00; 108]); // fake signature txin.witness.push([0x00; P2WPKH_FAKE_WITNESS_SIZE]); // fake signature
wallet wallet
.database .database
.borrow_mut() .borrow_mut()
@ -3579,7 +3614,7 @@ pub(crate) mod test {
details.received details.received
); );
assert_fee_rate!(psbt.extract_tx(), details.fee.unwrap_or(0), FeeRate::from_sat_per_vb(50.0), @add_signature); assert_fee_rate!(psbt, details.fee.unwrap_or(0), FeeRate::from_sat_per_vb(50.0), @add_signature);
} }
#[test] #[test]
@ -3601,7 +3636,7 @@ pub(crate) mod test {
let txid = tx.txid(); let txid = tx.txid();
// skip saving the new utxos, we know they can't be used anyways // skip saving the new utxos, we know they can't be used anyways
for txin in &mut tx.input { for txin in &mut tx.input {
txin.witness.push([0x00; 108]); // fake signature txin.witness.push([0x00; P2WPKH_FAKE_WITNESS_SIZE]); // fake signature
wallet wallet
.database .database
.borrow_mut() .borrow_mut()
@ -3672,7 +3707,7 @@ pub(crate) mod test {
let txid = tx.txid(); let txid = tx.txid();
// skip saving the new utxos, we know they can't be used anyways // skip saving the new utxos, we know they can't be used anyways
for txin in &mut tx.input { for txin in &mut tx.input {
txin.witness.push([0x00; 108]); // fake signature txin.witness.push([0x00; P2WPKH_FAKE_WITNESS_SIZE]); // fake signature
wallet wallet
.database .database
.borrow_mut() .borrow_mut()
@ -3719,7 +3754,7 @@ pub(crate) mod test {
75_000 - original_send_all_amount - details.fee.unwrap_or(0) 75_000 - original_send_all_amount - details.fee.unwrap_or(0)
); );
assert_fee_rate!(psbt.extract_tx(), details.fee.unwrap_or(0), FeeRate::from_sat_per_vb(50.0), @add_signature); assert_fee_rate!(psbt, details.fee.unwrap_or(0), FeeRate::from_sat_per_vb(50.0), @add_signature);
} }
#[test] #[test]
@ -3743,13 +3778,14 @@ pub(crate) mod test {
let txid = tx.txid(); let txid = tx.txid();
// skip saving the new utxos, we know they can't be used anyways // skip saving the new utxos, we know they can't be used anyways
for txin in &mut tx.input { for txin in &mut tx.input {
txin.witness.push([0x00; 108]); // fake signature txin.witness.push([0x00; P2WPKH_FAKE_WITNESS_SIZE]); // fake signature
wallet wallet
.database .database
.borrow_mut() .borrow_mut()
.del_utxo(&txin.previous_output) .del_utxo(&txin.previous_output)
.unwrap(); .unwrap();
} }
let original_tx_weight = tx.weight();
original_details.transaction = Some(tx); original_details.transaction = Some(tx);
wallet wallet
.database .database
@ -3758,7 +3794,20 @@ pub(crate) mod test {
.unwrap(); .unwrap();
let mut builder = wallet.build_fee_bump(txid).unwrap(); let mut builder = wallet.build_fee_bump(txid).unwrap();
builder.fee_rate(FeeRate::from_sat_per_vb(141.0)); // We set a fee high enough that during rbf we are forced to add
// a new input and also that we have to remove the change
// that we had previously
// We calculate the new weight as:
// original weight
// + extra input weight: 160 WU = (32 (prevout) + 4 (vout) + 4 (nsequence)) * 4
// + input satisfaction weight: 112 WU = 106 (witness) + 2 (witness len) + (1 (script len)) * 4
// - change output weight: 124 WU = (8 (value) + 1 (script len) + 22 (script)) * 4
let new_tx_weight = original_tx_weight + 160 + 112 - 124;
// two inputs (50k, 25k) and one output (45k) - epsilon
// We use epsilon here to avoid asking for a slightly too high feerate
let fee_abs = 50_000 + 25_000 - 45_000 - 10;
builder.fee_rate(FeeRate::from_wu(fee_abs, new_tx_weight));
let (psbt, details) = builder.finish().unwrap(); let (psbt, details) = builder.finish().unwrap();
assert_eq!( assert_eq!(
@ -3782,7 +3831,7 @@ pub(crate) mod test {
45_000 45_000
); );
assert_fee_rate!(psbt.extract_tx(), details.fee.unwrap_or(0), FeeRate::from_sat_per_vb(140.0), @dust_change, @add_signature); assert_fee_rate!(psbt, details.fee.unwrap_or(0), FeeRate::from_sat_per_vb(140.0), @dust_change, @add_signature);
} }
#[test] #[test]
@ -3804,7 +3853,7 @@ pub(crate) mod test {
let txid = tx.txid(); let txid = tx.txid();
// skip saving the new utxos, we know they can't be used anyways // skip saving the new utxos, we know they can't be used anyways
for txin in &mut tx.input { for txin in &mut tx.input {
txin.witness.push([0x00; 108]); // fake signature txin.witness.push([0x00; P2WPKH_FAKE_WITNESS_SIZE]); // fake signature
wallet wallet
.database .database
.borrow_mut() .borrow_mut()
@ -3853,7 +3902,7 @@ pub(crate) mod test {
details.received details.received
); );
assert_fee_rate!(psbt.extract_tx(), details.fee.unwrap_or(0), FeeRate::from_sat_per_vb(5.0), @add_signature); assert_fee_rate!(psbt, details.fee.unwrap_or(0), FeeRate::from_sat_per_vb(5.0), @add_signature);
} }
#[test] #[test]
@ -3875,7 +3924,7 @@ pub(crate) mod test {
let txid = tx.txid(); let txid = tx.txid();
// skip saving the new utxos, we know they can't be used anyways // skip saving the new utxos, we know they can't be used anyways
for txin in &mut tx.input { for txin in &mut tx.input {
txin.witness.push([0x00; 108]); // fake signature txin.witness.push([0x00; P2WPKH_FAKE_WITNESS_SIZE]); // fake signature
wallet wallet
.database .database
.borrow_mut() .borrow_mut()
@ -3954,7 +4003,7 @@ pub(crate) mod test {
let mut tx = psbt.extract_tx(); let mut tx = psbt.extract_tx();
let txid = tx.txid(); let txid = tx.txid();
for txin in &mut tx.input { for txin in &mut tx.input {
txin.witness.push([0x00; 108]); // fake signature txin.witness.push([0x00; P2WPKH_FAKE_WITNESS_SIZE]); // fake signature
wallet wallet
.database .database
.borrow_mut() .borrow_mut()
@ -3998,7 +4047,7 @@ pub(crate) mod test {
let mut tx = psbt.extract_tx(); let mut tx = psbt.extract_tx();
let txid = tx.txid(); let txid = tx.txid();
for txin in &mut tx.input { for txin in &mut tx.input {
txin.witness.push([0x00; 108]); // fake signature txin.witness.push([0x00; P2WPKH_FAKE_WITNESS_SIZE]); // fake signature
wallet wallet
.database .database
.borrow_mut() .borrow_mut()
@ -4020,6 +4069,38 @@ pub(crate) mod test {
builder.finish().unwrap(); builder.finish().unwrap();
} }
#[test]
fn test_fee_amount_negative_drain_val() {
// While building the transaction, bdk would calculate the drain_value
// as
// current_delta - fee_amount - drain_fee
// using saturating_sub, meaning that if the result would end up negative,
// it'll remain to zero instead.
// This caused a bug in master where we would calculate the wrong fee
// for a transaction.
// See https://github.com/bitcoindevkit/bdk/issues/660
let (wallet, descriptors, _) = get_funded_wallet(get_test_wpkh());
let send_to = Address::from_str("tb1ql7w62elx9ucw4pj5lgw4l028hmuw80sndtntxt").unwrap();
let fee_rate = FeeRate::from_sat_per_vb(2.01);
let incoming_txid = crate::populate_test_db!(
wallet.database.borrow_mut(),
testutils! (@tx ( (@external descriptors, 0) => 8859 ) (@confirmations 1)),
Some(100),
);
let mut builder = wallet.build_tx();
builder
.add_recipient(send_to.script_pubkey(), 8630)
.add_utxo(OutPoint::new(incoming_txid, 0))
.unwrap()
.enable_rbf()
.fee_rate(fee_rate);
let (psbt, details) = builder.finish().unwrap();
assert!(psbt.inputs.len() == 1);
assert_fee_rate!(psbt, details.fee.unwrap_or(0), fee_rate, @add_signature);
}
#[test] #[test]
fn test_sign_single_xprv() { fn test_sign_single_xprv() {
let (wallet, _, _) = get_funded_wallet("wpkh(tprv8ZgxMBicQKsPd3EupYiPRhaMooHKUHJxNsTfYuScep13go8QFfHdtkG9nRkFGb7busX4isf6X9dURGCoKgitaApQ6MupRhZMcELAxTBRJgS/*)"); let (wallet, _, _) = get_funded_wallet("wpkh(tprv8ZgxMBicQKsPd3EupYiPRhaMooHKUHJxNsTfYuScep13go8QFfHdtkG9nRkFGb7busX4isf6X9dURGCoKgitaApQ6MupRhZMcELAxTBRJgS/*)");
@ -5187,4 +5268,66 @@ pub(crate) mod test {
.current_height(maturity_time); .current_height(maturity_time);
builder.finish().unwrap(); builder.finish().unwrap();
} }
#[test]
fn test_fee_rate_sign_no_grinding_high_r() {
// Our goal is to obtain a transaction with a signature with high-R (71 bytes
// instead of 70). We then check that our fee rate and fee calculation is
// alright.
let (wallet, _, _) = get_funded_wallet("wpkh(tprv8ZgxMBicQKsPd3EupYiPRhaMooHKUHJxNsTfYuScep13go8QFfHdtkG9nRkFGb7busX4isf6X9dURGCoKgitaApQ6MupRhZMcELAxTBRJgS/*)");
let addr = wallet.get_address(New).unwrap();
let fee_rate = FeeRate::from_sat_per_vb(1.0);
let mut builder = wallet.build_tx();
let mut data = vec![0];
builder
.drain_to(addr.script_pubkey())
.drain_wallet()
.fee_rate(fee_rate)
.add_data(&data);
let (mut psbt, details) = builder.finish().unwrap();
let (op_return_vout, _) = psbt
.unsigned_tx
.output
.iter()
.enumerate()
.find(|(_n, i)| i.script_pubkey.is_op_return())
.unwrap();
let mut sig_len: usize = 0;
// We try to sign many different times until we find a longer signature (71 bytes)
while sig_len < 71 {
// Changing the OP_RETURN data will make the signature change (but not the fee, until
// data[0] is small enough)
data[0] += 1;
psbt.unsigned_tx.output[op_return_vout].script_pubkey = Script::new_op_return(&data);
// Clearing the previous signature
psbt.inputs[0].partial_sigs.clear();
// Signing
wallet
.sign(
&mut psbt,
SignOptions {
remove_partial_sigs: false,
try_finalize: false,
..Default::default()
},
)
.unwrap();
// We only have one key in the partial_sigs map, this is a trick to retrieve it
let key = psbt.inputs[0].partial_sigs.keys().next().unwrap();
sig_len = psbt.inputs[0].partial_sigs[key].sig.serialize_der().len();
}
// Actually finalizing the transaction...
wallet
.sign(
&mut psbt,
SignOptions {
remove_partial_sigs: false,
..Default::default()
},
)
.unwrap();
// ...and checking that everything is fine
assert_fee_rate!(psbt, details.fee.unwrap_or(0), fee_rate);
}
} }