Skip to content

Commit

Permalink
fix: remove deprecated `max_satisfaction_weight
Browse files Browse the repository at this point in the history
- Change deprecated `max_satisfaction_weight` to `max_weight_to_satisfy`
- Remove `#[allow(deprecated)]` flags
- updates the calculations in TXIN_BASE_WEIGHT and P2WPKH_SATISFACTION_SIZE
  • Loading branch information
storopoli committed Mar 2, 2024
1 parent d77a7f2 commit 6d0f282
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 40 deletions.
24 changes: 11 additions & 13 deletions crates/bdk/src/wallet/coin_selection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
//! # use bdk::*;
//! # use bdk::wallet::coin_selection::decide_change;
//! # use anyhow::Error;
//! # const TXIN_BASE_WEIGHT: usize = (32 + 4 + 4) * 4;
//! #[derive(Debug)]
//! struct AlwaysSpendEverything;
//!
Expand All @@ -55,7 +54,8 @@
//! |(selected_amount, additional_weight), weighted_utxo| {
//! **selected_amount += weighted_utxo.utxo.txout().value;
//! **additional_weight += Weight::from_wu(
//! (TXIN_BASE_WEIGHT + weighted_utxo.satisfaction_weight) as u64,
//! (TxIn::default().segwit_weight() + weighted_utxo.satisfaction_weight)
//! as u64,
//! );
//! Some(weighted_utxo.utxo)
//! },
Expand Down Expand Up @@ -109,6 +109,7 @@ use crate::WeightedUtxo;
use alloc::vec::Vec;
use bitcoin::consensus::encode::serialize;
use bitcoin::OutPoint;
use bitcoin::TxIn;
use bitcoin::{Script, Weight};

use core::convert::TryInto;
Expand All @@ -119,10 +120,6 @@ use rand::seq::SliceRandom;
/// overridden
pub type DefaultCoinSelectionAlgorithm = BranchAndBoundCoinSelection;

// Base weight of a Txin, not counting the weight needed for satisfying it.
// prev_txid (32 bytes) + prev_vout (4 bytes) + sequence (4 bytes)
pub(crate) const TXIN_BASE_WEIGHT: usize = (32 + 4 + 4) * 4;

/// Errors that can be thrown by the [`coin_selection`](crate::wallet::coin_selection) module
#[derive(Debug)]
pub enum Error {
Expand Down Expand Up @@ -345,7 +342,8 @@ fn select_sorted_utxos(
|(selected_amount, fee_amount), (must_use, weighted_utxo)| {
if must_use || **selected_amount < target_amount + **fee_amount {
**fee_amount += fee_rate.fee_wu(Weight::from_wu(
(TXIN_BASE_WEIGHT + weighted_utxo.satisfaction_weight) as u64,
(TxIn::default().segwit_weight() + weighted_utxo.satisfaction_weight)
as u64,
));
**selected_amount += weighted_utxo.utxo.txout().value;
Some(weighted_utxo.utxo)
Expand Down Expand Up @@ -388,7 +386,7 @@ struct OutputGroup {
impl OutputGroup {
fn new(weighted_utxo: WeightedUtxo, fee_rate: FeeRate) -> Self {
let fee = fee_rate.fee_wu(Weight::from_wu(
(TXIN_BASE_WEIGHT + weighted_utxo.satisfaction_weight) as u64,
(TxIn::default().segwit_weight() + weighted_utxo.satisfaction_weight) as u64,
));
let effective_value = weighted_utxo.utxo.txout().value as i64 - fee as i64;
OutputGroup {
Expand Down Expand Up @@ -738,7 +736,7 @@ mod test {
use core::str::FromStr;

use bdk_chain::ConfirmationTime;
use bitcoin::{OutPoint, ScriptBuf, TxOut};
use bitcoin::{OutPoint, ScriptBuf, TxIn, TxOut};

use super::*;
use crate::types::*;
Expand All @@ -749,9 +747,9 @@ mod test {
use rand::seq::SliceRandom;
use rand::{Rng, RngCore, SeedableRng};

// n. of items on witness (1WU) + signature len (1WU) + signature and sighash (72WU)
// + pubkey len (1WU) + pubkey (33WU) + script sig len (1 byte, 4WU)
const P2WPKH_SATISFACTION_SIZE: usize = 1 + 1 + 72 + 1 + 33 + 4;
// n. of items on witness (1WU) signature and sighash (72WU)
// + pubkey len (1WU) + pubkey (33WU)
const P2WPKH_SATISFACTION_SIZE: usize = 1 + 72 + 1 + 33;

const FEE_AMOUNT: u64 = 50;

Expand Down Expand Up @@ -1240,7 +1238,7 @@ mod test {

assert_eq!(result.selected.len(), 1);
assert_eq!(result.selected_amount(), 100_000);
let input_size = (TXIN_BASE_WEIGHT + P2WPKH_SATISFACTION_SIZE).vbytes();
let input_size = (TxIn::default().segwit_weight() + P2WPKH_SATISFACTION_SIZE).vbytes();
// the final fee rate should be exactly the same as the fee rate given
assert!((1.0 - (result.fee_amount as f32 / input_size as f32)).abs() < f32::EPSILON);
}
Expand Down
16 changes: 6 additions & 10 deletions crates/bdk/src/wallet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ pub(crate) mod utils;
pub mod error;
pub use utils::IsDust;

#[allow(deprecated)]
use coin_selection::DefaultCoinSelectionAlgorithm;
use signer::{SignOptions, SignerOrdering, SignersContainer, TransactionSigner};
use tx_builder::{BumpFee, CreateTx, FeePolicy, TxBuilder, TxParams};
Expand Down Expand Up @@ -1715,10 +1714,9 @@ impl<D> Wallet<D> {

let weighted_utxo = match txout_index.index_of_spk(&txout.script_pubkey) {
Some((keychain, derivation_index)) => {
#[allow(deprecated)]
let satisfaction_weight = self
.get_descriptor_for_keychain(keychain)
.max_satisfaction_weight()
.max_weight_to_satisfy()
.unwrap();
WeightedUtxo {
utxo: Utxo::Local(LocalOutput {
Expand All @@ -1736,7 +1734,6 @@ impl<D> Wallet<D> {
let satisfaction_weight =
serialize(&txin.script_sig).len() * 4 + serialize(&txin.witness).len();
WeightedUtxo {
satisfaction_weight,
utxo: Utxo::Foreign {
outpoint: txin.previous_output,
sequence: Some(txin.sequence),
Expand All @@ -1746,6 +1743,7 @@ impl<D> Wallet<D> {
..Default::default()
}),
},
satisfaction_weight,
}
}
};
Expand Down Expand Up @@ -2045,13 +2043,11 @@ impl<D> Wallet<D> {
self.list_unspent()
.map(|utxo| {
let keychain = utxo.keychain;
#[allow(deprecated)]
(
utxo,
(utxo, {
self.get_descriptor_for_keychain(keychain)
.max_satisfaction_weight()
.unwrap(),
)
.max_weight_to_satisfy()
.unwrap()
})
})
.collect()
}
Expand Down
22 changes: 17 additions & 5 deletions crates/bdk/src/wallet/tx_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ use bitcoin::{absolute, script::PushBytes, OutPoint, ScriptBuf, Sequence, Transa

use super::coin_selection::{CoinSelectionAlgorithm, DefaultCoinSelectionAlgorithm};
use super::ChangeSet;
use crate::descriptor::DescriptorMeta;
use crate::types::{FeeRate, KeychainKind, LocalOutput, WeightedUtxo};
use crate::wallet::CreateTxError;
use crate::{Utxo, Wallet};
Expand Down Expand Up @@ -318,8 +319,19 @@ impl<'a, D, Cs, Ctx> TxBuilder<'a, D, Cs, Ctx> {

for utxo in utxos {
let descriptor = wallet.get_descriptor_for_keychain(utxo.keychain);
#[allow(deprecated)]
let satisfaction_weight = descriptor.max_satisfaction_weight().unwrap();
let satisfaction_weight = {
let is_segwit = wallet
.get_descriptor_for_keychain(utxo.keychain)
.is_witness()
|| wallet
.get_descriptor_for_keychain(utxo.keychain)
.is_taproot();
let segwit_add = match is_segwit {
true => 4,
false => 0,
};
descriptor.max_weight_to_satisfy().unwrap() + segwit_add
};
self.params.utxos.push(WeightedUtxo {
satisfaction_weight,
utxo: Utxo::Local(utxo),
Expand Down Expand Up @@ -360,9 +372,9 @@ impl<'a, D, Cs, Ctx> TxBuilder<'a, D, Cs, Ctx> {
/// causing you to pay a fee that is too high. The party who is broadcasting the transaction can
/// of course check the real input weight matches the expected weight prior to broadcasting.
///
/// To guarantee the `satisfaction_weight` is correct, you can require the party providing the
/// To guarantee the `max_weight_to_satisfy` is correct, you can require the party providing the
/// `psbt_input` provide a miniscript descriptor for the input so you can check it against the
/// `script_pubkey` and then ask it for the [`max_satisfaction_weight`].
/// `script_pubkey` and then ask it for the [`max_weight_to_satisfy`].
///
/// This is an **EXPERIMENTAL** feature, API and other major changes are expected.
///
Expand All @@ -383,7 +395,7 @@ impl<'a, D, Cs, Ctx> TxBuilder<'a, D, Cs, Ctx> {
///
/// [`only_witness_utxo`]: Self::only_witness_utxo
/// [`finish`]: Self::finish
/// [`max_satisfaction_weight`]: miniscript::Descriptor::max_satisfaction_weight
/// [`max_weight_to_satisfy`]: miniscript::Descriptor::max_weight_to_satisfy
pub fn add_foreign_utxo(
&mut self,
outpoint: OutPoint,
Expand Down
18 changes: 6 additions & 12 deletions crates/bdk/tests/wallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1165,10 +1165,9 @@ fn test_add_foreign_utxo() {
.unwrap()
.assume_checked();
let utxo = wallet2.list_unspent().next().expect("must take!");
#[allow(deprecated)]
let foreign_utxo_satisfaction = wallet2
.get_descriptor_for_keychain(KeychainKind::External)
.max_satisfaction_weight()
.max_weight_to_satisfy()
.unwrap();

let psbt_input = psbt::Input {
Expand Down Expand Up @@ -1241,10 +1240,9 @@ fn test_calculate_fee_with_missing_foreign_utxo() {
.unwrap()
.assume_checked();
let utxo = wallet2.list_unspent().next().expect("must take!");
#[allow(deprecated)]
let foreign_utxo_satisfaction = wallet2
.get_descriptor_for_keychain(KeychainKind::External)
.max_satisfaction_weight()
.max_weight_to_satisfy()
.unwrap();

let psbt_input = psbt::Input {
Expand All @@ -1267,10 +1265,9 @@ fn test_calculate_fee_with_missing_foreign_utxo() {
fn test_add_foreign_utxo_invalid_psbt_input() {
let (mut wallet, _) = get_funded_wallet(get_test_wpkh());
let outpoint = wallet.list_unspent().next().expect("must exist").outpoint;
#[allow(deprecated)]
let foreign_utxo_satisfaction = wallet
.get_descriptor_for_keychain(KeychainKind::External)
.max_satisfaction_weight()
.max_weight_to_satisfy()
.unwrap();

let mut builder = wallet.build_tx();
Expand All @@ -1289,10 +1286,9 @@ fn test_add_foreign_utxo_where_outpoint_doesnt_match_psbt_input() {
let tx1 = wallet1.get_tx(txid1).unwrap().tx_node.tx.clone();
let tx2 = wallet2.get_tx(txid2).unwrap().tx_node.tx.clone();

#[allow(deprecated)]
let satisfaction_weight = wallet2
.get_descriptor_for_keychain(KeychainKind::External)
.max_satisfaction_weight()
.max_weight_to_satisfy()
.unwrap();

let mut builder = wallet1.build_tx();
Expand Down Expand Up @@ -1334,10 +1330,9 @@ fn test_add_foreign_utxo_only_witness_utxo() {
.assume_checked();
let utxo2 = wallet2.list_unspent().next().unwrap();

#[allow(deprecated)]
let satisfaction_weight = wallet2
.get_descriptor_for_keychain(KeychainKind::External)
.max_satisfaction_weight()
.max_weight_to_satisfy()
.unwrap();

let mut builder = wallet1.build_tx();
Expand Down Expand Up @@ -3030,10 +3025,9 @@ fn test_taproot_foreign_utxo() {
.assume_checked();
let utxo = wallet2.list_unspent().next().unwrap();
let psbt_input = wallet2.get_psbt_input(utxo.clone(), None, false).unwrap();
#[allow(deprecated)]
let foreign_utxo_satisfaction = wallet2
.get_descriptor_for_keychain(KeychainKind::External)
.max_satisfaction_weight()
.max_weight_to_satisfy()
.unwrap();

assert!(
Expand Down

0 comments on commit 6d0f282

Please sign in to comment.