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

Merge bitcoindevkit/bdk#1345: fix: remove deprecated max_satisfaction_weight

Browse Source 
798ed8ced25156049126645435127a22245e916f fix: remove deprecated `max_satisfaction_weight (Jose Storopoli)

Pull request description:

  ### Description
  Continuation of #1115.
  Closes #1036.

  * Change deprecated `max_satisfaction_weight` to `max_weight_to_satisfy`
  * Remove `#[allow(deprecated)]` flags

  ### Notes to the reviewers

  I've changed all `max_satisfaction_weight()` to `max_weight_to_satisfy()` in `Wallet.get_available_utxo()` and `Wallet.build_fee_bump()`. Checking the docs on the `miniscript` crate for `max_weight_to_satisfy` has the following note:

  We are testing if the underlying descriptor `is.segwit()` or `.is_taproot`,
  then adding 4WU if true or leaving as it is otherwise.

  Another thing, we are not testing in BDK tests for legacy (pre-segwit) descriptors.
  Should I also add them to this PR?

  ### Changelog notice
  ### Fixed
  Replace the deprecated `max_satisfaction_weight` from `rust-miniscript` to `max_weight_to_satisfy`.

  ### 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
  * [ ]  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:
  evanlinjin:
    ACK 798ed8ced25156049126645435127a22245e916f

Tree-SHA512: 60babecee13c24915348ddb64894127a76a59d9421d52ea37acc714913685d57cc2be1904f9d0508078dd1db1f7d7dad83a734af5ee981801ca87de2e9984429
This commit is contained in:
志宇 2024-04-02 19:21:18 +08:00
commit 2bb654077d
No known key found for this signature in database
GPG Key ID: F6345C9837C2BDE8
4 changed files with 29 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
crates/bdk/src/wallet/coin_selection.rs
View File

@ -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;
//!
@ -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)
//! },
@ -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;
@ -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 {
@ -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 {
@ -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,
@ -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::*;
@ -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;
@ -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);

16
crates/bdk/src/wallet/mod.rs
View File

@ -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};
@ -1714,10 +1713,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 {
@ -1735,7 +1733,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),
@ -1745,6 +1742,7 @@ impl<D> Wallet<D> {
..Default::default()
}),
},
satisfaction_weight,
}
}
};
@ -2059,13 +2057,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()
}

9
crates/bdk/src/wallet/tx_builder.rs
View File

@ -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),
@ -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.
///
@ -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,

18
crates/bdk/tests/wallet.rs
View File

@ -1173,10 +1173,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 {
@ -1249,10 +1248,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 {
@ -1275,10 +1273,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();
@ -1297,10 +1294,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();
@ -1342,10 +1338,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();
@ -3077,10 +3072,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!(