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

Update crates/bdk/src/wallet/coin_selection.rs

Co-authored-by: ValuedMammal <valuedmammal@protonmail.com>
  • Loading branch information
storopoli and ValuedMammal committed Mar 23, 2024
1 parent 80e190b commit 1bf5bcf
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 42 deletions.
28 changes: 13 additions & 15 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 bitcoin::FeeRate;
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 @@ -347,10 +344,10 @@ fn select_sorted_utxos(
if must_use || **selected_amount < target_amount + **fee_amount {
**fee_amount += (fee_rate
* Weight::from_wu(
(TXIN_BASE_WEIGHT + weighted_utxo.satisfaction_weight) as u64,
(TxIn::default().segwit_weight() + weighted_utxo.satisfaction_weight)
as u64,
))
.to_sat();

**selected_amount += weighted_utxo.utxo.txout().value;
Some(weighted_utxo.utxo)
} else {
Expand Down Expand Up @@ -392,9 +389,10 @@ struct OutputGroup {
impl OutputGroup {
fn new(weighted_utxo: WeightedUtxo, fee_rate: FeeRate) -> Self {
let fee = (fee_rate
* Weight::from_wu((TXIN_BASE_WEIGHT + weighted_utxo.satisfaction_weight) as u64))
* Weight::from_wu(
(TxIn::default().segwit_weight() + weighted_utxo.satisfaction_weight) as u64,
))
.to_sat();

let effective_value = weighted_utxo.utxo.txout().value as i64 - fee as i64;
OutputGroup {
weighted_utxo,
Expand Down Expand Up @@ -744,7 +742,7 @@ mod test {
use core::str::FromStr;

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

use super::*;
use crate::types::*;
Expand All @@ -754,9 +752,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;
// signature len (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_weight = (TXIN_BASE_WEIGHT + P2WPKH_SATISFACTION_SIZE) as u64;
let input_weight = (TxIn::default().segwit_weight() + P2WPKH_SATISFACTION_SIZE) as u64;
// the final fee rate should be exactly the same as the fee rate given
let result_feerate = Amount::from_sat(result.fee_amount) / Weight::from_wu(input_weight);
assert_eq!(result_feerate, feerate);
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 @@ -1712,10 +1711,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 @@ -1733,7 +1731,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 @@ -1743,6 +1740,7 @@ impl<D> Wallet<D> {
..Default::default()
}),
},
satisfaction_weight,
}
}
};
Expand Down Expand Up @@ -2057,13 +2055,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
9 changes: 4 additions & 5 deletions crates/bdk/src/wallet/tx_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -314,8 +314,7 @@ 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 = descriptor.max_weight_to_satisfy().unwrap();
self.params.utxos.push(WeightedUtxo {
satisfaction_weight,
utxo: Utxo::Local(utxo),
Expand Down Expand Up @@ -356,9 +355,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 @@ -379,7 +378,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 @@ -1171,10 +1171,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 @@ -1247,10 +1246,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 @@ -1273,10 +1271,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 @@ -1295,10 +1292,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 @@ -1340,10 +1336,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 @@ -3074,10 +3069,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 1bf5bcf

Please sign in to comment.