Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: remove deprecated max_satisfaction_weight #1345

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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)]
evanlinjin marked this conversation as resolved.
Show resolved Hide resolved
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
Loading