fix(wallet): remove TxBuilder allow_shrinking function and unneeded context param

Author: notmandatory

crates/bdk/src/wallet/mod.rs

@@ -57,7 +57,7 @@ pub use utils::IsDust;
 #[allow(deprecated)]
 use coin_selection::DefaultCoinSelectionAlgorithm;
 use signer::{SignOptions, SignerOrdering, SignersContainer, TransactionSigner};
-use tx_builder::{BumpFee, CreateTx, FeePolicy, TxBuilder, TxParams};
+use tx_builder::{FeePolicy, TxBuilder, TxParams};
 use utils::{check_nsequence_rbf, After, Older, SecpCtx};
 
 use crate::descriptor::policy::BuildSatisfaction;
@@ -1252,12 +1252,11 @@ impl<D> Wallet<D> {
     /// ```
     ///
     /// [`TxBuilder`]: crate::TxBuilder
-    pub fn build_tx(&mut self) -> TxBuilder<'_, D, DefaultCoinSelectionAlgorithm, CreateTx> {
+    pub fn build_tx(&mut self) -> TxBuilder<'_, D, DefaultCoinSelectionAlgorithm> {
         TxBuilder {
             wallet: alloc::rc::Rc::new(core::cell::RefCell::new(self)),
             params: TxParams::default(),
             coin_selection: DefaultCoinSelectionAlgorithm::default(),
-            phantom: core::marker::PhantomData,
         }
     }
 
@@ -1662,7 +1661,7 @@ impl<D> Wallet<D> {
     pub fn build_fee_bump(
         &mut self,
         txid: Txid,
-    ) -> Result<TxBuilder<'_, D, DefaultCoinSelectionAlgorithm, BumpFee>, BuildFeeBumpError> {
+    ) -> Result<TxBuilder<'_, D, DefaultCoinSelectionAlgorithm>, BuildFeeBumpError> {
         let graph = self.indexed_graph.graph();
         let txout_index = &self.indexed_graph.index;
         let chain_tip = self.chain.tip().block_id();
@@ -1786,7 +1785,6 @@ impl<D> Wallet<D> {
             wallet: alloc::rc::Rc::new(core::cell::RefCell::new(self)),
             params,
             coin_selection: DefaultCoinSelectionAlgorithm::default(),
-            phantom: core::marker::PhantomData,
         })
     }
 

crates/bdk/src/wallet/tx_builder.rs

@@ -43,14 +43,13 @@
 use alloc::{boxed::Box, rc::Rc, string::String, vec::Vec};
 use core::cell::RefCell;
 use core::fmt;
-use core::marker::PhantomData;
 
 use bdk_chain::PersistBackend;
 use bitcoin::psbt::{self, PartiallySignedTransaction as Psbt};
 use bitcoin::script::PushBytes;
 use bitcoin::{absolute, FeeRate, OutPoint, ScriptBuf, Sequence, Transaction, Txid};
 
-use super::coin_selection::{CoinSelectionAlgorithm, DefaultCoinSelectionAlgorithm};
+use super::coin_selection::CoinSelectionAlgorithm;
 use super::{ChangeSet, CreateTxError, Wallet};
 use crate::collections::{BTreeMap, HashSet};
 use crate::{KeychainKind, LocalOutput, Utxo, WeightedUtxo};
@@ -124,11 +123,10 @@ impl TxBuilderContext for BumpFee {}
 /// [`finish`]: Self::finish
 /// [`coin_selection`]: Self::coin_selection
 #[derive(Debug)]
-pub struct TxBuilder<'a, D, Cs, Ctx> {
+pub struct TxBuilder<'a, D, Cs> {
     pub(crate) wallet: Rc<RefCell<&'a mut Wallet<D>>>,
     pub(crate) params: TxParams,
     pub(crate) coin_selection: Cs,
-    pub(crate) phantom: PhantomData<Ctx>,
 }
 
 /// The parameters for transaction creation sans coin selection algorithm.
@@ -176,19 +174,18 @@ impl Default for FeePolicy {
     }
 }
 
-impl<'a, D, Cs: Clone, Ctx> Clone for TxBuilder<'a, D, Cs, Ctx> {
+impl<'a, D, Cs: Clone> Clone for TxBuilder<'a, D, Cs> {
     fn clone(&self) -> Self {
         TxBuilder {
             wallet: self.wallet.clone(),
             params: self.params.clone(),
             coin_selection: self.coin_selection.clone(),
-            phantom: PhantomData,
         }
     }
 }
 
 // methods supported by both contexts, for any CoinSelectionAlgorithm
-impl<'a, D, Cs, Ctx> TxBuilder<'a, D, Cs, Ctx> {
+impl<'a, D, Cs> TxBuilder<'a, D, Cs> {
     /// Set a custom fee rate.
     ///
     /// This method sets the mining fee paid by the transaction as a rate on its size.
@@ -561,12 +558,11 @@ impl<'a, D, Cs, Ctx> TxBuilder<'a, D, Cs, Ctx> {
     pub fn coin_selection<P: CoinSelectionAlgorithm>(
         self,
         coin_selection: P,
-    ) -> TxBuilder<'a, D, P, Ctx> {
+    ) -> TxBuilder<'a, D, P> {
         TxBuilder {
             wallet: self.wallet,
             params: self.params,
             coin_selection,
-            phantom: PhantomData,
         }
     }
 
@@ -616,7 +612,7 @@ impl<'a, D, Cs, Ctx> TxBuilder<'a, D, Cs, Ctx> {
     }
 }
 
-impl<'a, D, Cs: CoinSelectionAlgorithm, Ctx> TxBuilder<'a, D, Cs, Ctx> {
+impl<'a, D, Cs: CoinSelectionAlgorithm> TxBuilder<'a, D, Cs> {
     /// Finish building the transaction.
     ///
     /// Returns a new [`Psbt`] per [`BIP174`].
@@ -694,29 +690,7 @@ impl fmt::Display for AddForeignUtxoError {
 #[cfg(feature = "std")]
 impl std::error::Error for AddForeignUtxoError {}
 
-#[derive(Debug)]
-/// Error returned from [`TxBuilder::allow_shrinking`]
-pub enum AllowShrinkingError {
-    /// Script/PubKey was not in the original transaction
-    MissingScriptPubKey(ScriptBuf),
-}
-
-impl fmt::Display for AllowShrinkingError {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        match self {
-            Self::MissingScriptPubKey(script_buf) => write!(
-                f,
-                "Script/PubKey was not in the original transaction: {}",
-                script_buf,
-            ),
-        }
-    }
-}
-
-#[cfg(feature = "std")]
-impl std::error::Error for AllowShrinkingError {}
-
-impl<'a, D, Cs: CoinSelectionAlgorithm> TxBuilder<'a, D, Cs, CreateTx> {
+impl<'a, D, Cs: CoinSelectionAlgorithm> TxBuilder<'a, D, Cs> {
     /// Replace the recipients already added with a new list
     pub fn set_recipients(&mut self, recipients: Vec<(ScriptBuf, u64)>) -> &mut Self {
         self.params.recipients = recipients;
@@ -745,11 +719,8 @@ impl<'a, D, Cs: CoinSelectionAlgorithm> TxBuilder<'a, D, Cs, CreateTx> {
     /// difference is that it is valid to use `drain_to` without setting any ordinary recipients
     /// with [`add_recipient`] (but it is perfectly fine to add recipients as well).
     ///
-    /// If you choose not to set any recipients, you should either provide the utxos that the
-    /// transaction should spend via [`add_utxos`], or set [`drain_wallet`] to spend all of them.
-    ///
-    /// When bumping the fees of a transaction made with this option, you probably want to
-    /// use [`allow_shrinking`] to allow this output to be reduced to pay for the extra fees.
+    /// If you choose not to set any recipients, you should provide the utxos that the
+    /// transaction should spend via [`add_utxos`].
     ///
     /// # Example
     ///
@@ -783,7 +754,6 @@ impl<'a, D, Cs: CoinSelectionAlgorithm> TxBuilder<'a, D, Cs, CreateTx> {
     /// # Ok::<(), anyhow::Error>(())
     /// ```
     ///
-    /// [`allow_shrinking`]: Self::allow_shrinking
     /// [`add_recipient`]: Self::add_recipient
     /// [`add_utxos`]: Self::add_utxos
     /// [`drain_wallet`]: Self::drain_wallet
@@ -793,38 +763,6 @@ impl<'a, D, Cs: CoinSelectionAlgorithm> TxBuilder<'a, D, Cs, CreateTx> {
     }
 }
 
-// methods supported only by bump_fee
-impl<'a, D> TxBuilder<'a, D, DefaultCoinSelectionAlgorithm, BumpFee> {
-    /// Explicitly tells the wallet that it is allowed to reduce the amount of the output matching this
-    /// `script_pubkey` in order to bump the transaction fee. Without specifying this the wallet
-    /// will attempt to find a change output to shrink instead.
-    ///
-    /// **Note** that the output may shrink to below the dust limit and therefore be removed. If it is
-    /// preserved then it is currently not guaranteed to be in the same position as it was
-    /// originally.
-    ///
-    /// Returns an `Err` if `script_pubkey` can't be found among the recipients of the
-    /// transaction we are bumping.
-    pub fn allow_shrinking(
-        &mut self,
-        script_pubkey: ScriptBuf,
-    ) -> Result<&mut Self, AllowShrinkingError> {
-        match self
-            .params
-            .recipients
-            .iter()
-            .position(|(recipient_script, _)| *recipient_script == script_pubkey)
-        {
-            Some(position) => {
-                self.params.recipients.remove(position);
-                self.params.drain_to = Some(script_pubkey);
-                Ok(self)
-            }
-            None => Err(AllowShrinkingError::MissingScriptPubKey(script_pubkey)),
-        }
-    }
-}
-
 /// Ordering of the transaction's inputs and outputs
 #[derive(Default, Debug, Ord, PartialOrd, Eq, PartialEq, Hash, Clone, Copy)]
 pub enum TxOrdering {

crates/bdk/tests/wallet.rs

@@ -1684,8 +1684,12 @@ fn test_bump_fee_reduce_single_recipient() {
     let mut builder = wallet.build_fee_bump(txid).unwrap();
     builder
         .fee_rate(feerate)
-        .allow_shrinking(addr.script_pubkey())
-        .unwrap();
+        // remove original tx drain_to address and amount
+        .set_recipients(Vec::new())
+        // set back original drain_to address
+        .drain_to(addr.script_pubkey())
+        // drain wallet output amount will be re-calculated with new fee rate
+        .drain_wallet();
     let psbt = builder.finish().unwrap();
     let sent_received = wallet.sent_and_received(&psbt.clone().extract_tx());
     let fee = check_fee!(wallet, psbt);
@@ -1722,9 +1726,13 @@ fn test_bump_fee_absolute_reduce_single_recipient() {
 
     let mut builder = wallet.build_fee_bump(txid).unwrap();
     builder
-        .allow_shrinking(addr.script_pubkey())
-        .unwrap()
-        .fee_absolute(300);
+        .fee_absolute(300)
+        // remove original tx drain_to address and amount
+        .set_recipients(Vec::new())
+        // set back original drain_to address
+        .drain_to(addr.script_pubkey())
+        // drain wallet output amount will be re-calculated with new fee rate
+        .drain_wallet();
     let psbt = builder.finish().unwrap();
     let tx = &psbt.unsigned_tx;
     let sent_received = wallet.sent_and_received(tx);
@@ -1790,8 +1798,6 @@ fn test_bump_fee_drain_wallet() {
     let mut builder = wallet.build_fee_bump(txid).unwrap();
     builder
         .drain_wallet()
-        .allow_shrinking(addr.script_pubkey())
-        .unwrap()
         .fee_rate(FeeRate::from_sat_per_vb_unchecked(5));
     let psbt = builder.finish().unwrap();
     let sent_received = wallet.sent_and_received(&psbt.extract_tx());
@@ -1807,7 +1813,7 @@ fn test_bump_fee_remove_output_manually_selected_only() {
     // them, and make sure that `bump_fee` doesn't try to add more. This fails because we've
     // told the wallet it's not allowed to add more inputs AND it can't reduce the value of the
     // existing output. In other words, bump_fee + manually_selected_only is always an error
-    // unless you've also set "allow_shrinking" OR there is a change output.
+    // unless there is a change output.
     let init_tx = Transaction {
         version: 1,
         lock_time: absolute::LockTime::ZERO,
@@ -2003,8 +2009,8 @@ fn test_bump_fee_no_change_add_input_and_change() {
         .insert_tx(tx, ConfirmationTime::Unconfirmed { last_seen: 0 })
         .unwrap();
 
-    // now bump the fees without using `allow_shrinking`. the wallet should add an
-    // extra input and a change output, and leave the original output untouched
+    // Now bump the fees, the wallet should add an extra input and a change output, and leave
+    // the original output untouched.
     let mut builder = wallet.build_fee_bump(txid).unwrap();
     builder.fee_rate(FeeRate::from_sat_per_vb_unchecked(50));
     let psbt = builder.finish().unwrap();
@@ -2297,8 +2303,12 @@ fn test_bump_fee_unconfirmed_input() {
     let mut builder = wallet.build_fee_bump(txid).unwrap();
     builder
         .fee_rate(FeeRate::from_sat_per_vb_unchecked(15))
-        .allow_shrinking(addr.script_pubkey())
-        .unwrap();
+        // remove original tx drain_to address and amount
+        .set_recipients(Vec::new())
+        // set back original drain_to address
+        .drain_to(addr.script_pubkey())
+        // drain wallet output amount will be re-calculated with new fee rate
+        .drain_wallet();
     builder.finish().unwrap();
 }