fix(wallet): remove TxBuilder::allow_shrinking function and TxBuilderContext

Author: notmandatory

crates/wallet/src/wallet/mod.rs

@@ -59,7 +59,7 @@ pub use utils::IsDust;
 
 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;
@@ -1293,12 +1293,11 @@ impl Wallet {
     /// ```
     ///
     /// [`TxBuilder`]: crate::TxBuilder
-    pub fn build_tx(&mut self) -> TxBuilder<'_, DefaultCoinSelectionAlgorithm, CreateTx> {
+    pub fn build_tx(&mut self) -> TxBuilder<'_, DefaultCoinSelectionAlgorithm> {
         TxBuilder {
             wallet: alloc::rc::Rc::new(core::cell::RefCell::new(self)),
             params: TxParams::default(),
             coin_selection: DefaultCoinSelectionAlgorithm::default(),
-            phantom: core::marker::PhantomData,
         }
     }
 
@@ -1684,7 +1683,7 @@ impl Wallet {
     pub fn build_fee_bump(
         &mut self,
         txid: Txid,
-    ) -> Result<TxBuilder<'_, DefaultCoinSelectionAlgorithm, BumpFee>, BuildFeeBumpError> {
+    ) -> Result<TxBuilder<'_, DefaultCoinSelectionAlgorithm>, BuildFeeBumpError> {
         let graph = self.indexed_graph.graph();
         let txout_index = &self.indexed_graph.index;
         let chain_tip = self.chain.tip().block_id();
@@ -1808,7 +1807,6 @@ impl Wallet {
             wallet: alloc::rc::Rc::new(core::cell::RefCell::new(self)),
             params,
             coin_selection: DefaultCoinSelectionAlgorithm::default(),
-            phantom: core::marker::PhantomData,
         })
     }
 

crates/wallet/src/wallet/tx_builder.rs

@@ -19,7 +19,6 @@
 //! # use bdk_wallet::*;
 //! # use bdk_wallet::wallet::ChangeSet;
 //! # use bdk_wallet::wallet::error::CreateTxError;
-//! # use bdk_wallet::wallet::tx_builder::CreateTx;
 //! # use bdk_persist::PersistBackend;
 //! # use anyhow::Error;
 //! # let to_address = Address::from_str("2N4eQYCbKUHCCTUjBJeHcJp9ok6J2GZsTDt").unwrap().assume_checked();
@@ -43,31 +42,16 @@
 use alloc::{boxed::Box, rc::Rc, string::String, vec::Vec};
 use core::cell::RefCell;
 use core::fmt;
-use core::marker::PhantomData;
 
 use bitcoin::psbt::{self, Psbt};
 use bitcoin::script::PushBytes;
 use bitcoin::{absolute, Amount, FeeRate, OutPoint, ScriptBuf, Sequence, Transaction, Txid};
 
-use super::coin_selection::{CoinSelectionAlgorithm, DefaultCoinSelectionAlgorithm};
+use super::coin_selection::CoinSelectionAlgorithm;
 use super::{CreateTxError, Wallet};
 use crate::collections::{BTreeMap, HashSet};
 use crate::{KeychainKind, LocalOutput, Utxo, WeightedUtxo};
 
-/// Context in which the [`TxBuilder`] is valid
-pub trait TxBuilderContext: core::fmt::Debug + Default + Clone {}
-
-/// Marker type to indicate the [`TxBuilder`] is being used to create a new transaction (as opposed
-/// to bumping the fee of an existing one).
-#[derive(Debug, Default, Clone)]
-pub struct CreateTx;
-impl TxBuilderContext for CreateTx {}
-
-/// Marker type to indicate the [`TxBuilder`] is being used to bump the fee of an existing transaction.
-#[derive(Debug, Default, Clone)]
-pub struct BumpFee;
-impl TxBuilderContext for BumpFee {}
-
 /// A transaction builder
 ///
 /// A `TxBuilder` is created by calling [`build_tx`] or [`build_fee_bump`] on a wallet. After
@@ -123,11 +107,10 @@ impl TxBuilderContext for BumpFee {}
 /// [`finish`]: Self::finish
 /// [`coin_selection`]: Self::coin_selection
 #[derive(Debug)]
-pub struct TxBuilder<'a, Cs, Ctx> {
+pub struct TxBuilder<'a, Cs> {
     pub(crate) wallet: Rc<RefCell<&'a mut Wallet>>,
     pub(crate) params: TxParams,
     pub(crate) coin_selection: Cs,
-    pub(crate) phantom: PhantomData<Ctx>,
 }
 
 /// The parameters for transaction creation sans coin selection algorithm.
@@ -175,19 +158,18 @@ impl Default for FeePolicy {
     }
 }
 
-impl<'a, Cs: Clone, Ctx> Clone for TxBuilder<'a, Cs, Ctx> {
+impl<'a, Cs: Clone> Clone for TxBuilder<'a, 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, Cs, Ctx> TxBuilder<'a, Cs, Ctx> {
+// Methods supported for any CoinSelectionAlgorithm.
+impl<'a, Cs> TxBuilder<'a, Cs> {
     /// Set a custom fee rate.
     ///
     /// This method sets the mining fee paid by the transaction as a rate on its size.
@@ -212,8 +194,8 @@ impl<'a, Cs, Ctx> TxBuilder<'a, Cs, Ctx> {
     /// Note that this is really a minimum absolute fee -- it's possible to
     /// overshoot it slightly since adding a change output to drain the remaining
     /// excess might not be viable.
-    pub fn fee_absolute(&mut self, fee_amount: u64) -> &mut Self {
-        self.params.fee_policy = Some(FeePolicy::FeeAmount(fee_amount));
+    pub fn fee_absolute(&mut self, fee_amount: Amount) -> &mut Self {
+        self.params.fee_policy = Some(FeePolicy::FeeAmount(fee_amount.to_sat()));
         self
     }
 
@@ -553,18 +535,14 @@ impl<'a, Cs, Ctx> TxBuilder<'a, Cs, Ctx> {
 
     /// Choose the coin selection algorithm
     ///
-    /// Overrides the [`DefaultCoinSelectionAlgorithm`].
+    /// Overrides the [`CoinSelectionAlgorithm`].
     ///
     /// Note that this function consumes the builder and returns it so it is usually best to put this as the first call on the builder.
-    pub fn coin_selection<P: CoinSelectionAlgorithm>(
-        self,
-        coin_selection: P,
-    ) -> TxBuilder<'a, P, Ctx> {
+    pub fn coin_selection<P: CoinSelectionAlgorithm>(self, coin_selection: P) -> TxBuilder<'a, P> {
         TxBuilder {
             wallet: self.wallet,
             params: self.params,
             coin_selection,
-            phantom: PhantomData,
         }
     }
 
@@ -612,106 +590,7 @@ impl<'a, Cs, Ctx> TxBuilder<'a, Cs, Ctx> {
         self.params.allow_dust = allow_dust;
         self
     }
-}
-
-impl<'a, Cs: CoinSelectionAlgorithm, Ctx> TxBuilder<'a, Cs, Ctx> {
-    /// Finish building the transaction.
-    ///
-    /// Returns a new [`Psbt`] per [`BIP174`].
-    ///
-    /// [`BIP174`]: https://github.com/bitcoin/bips/blob/master/bip-0174.mediawiki
-    pub fn finish(self) -> Result<Psbt, CreateTxError> {
-        self.wallet
-            .borrow_mut()
-            .create_tx(self.coin_selection, self.params)
-    }
-}
-
-#[derive(Debug)]
-/// Error returned from [`TxBuilder::add_utxo`] and [`TxBuilder::add_utxos`]
-pub enum AddUtxoError {
-    /// Happens when trying to spend an UTXO that is not in the internal database
-    UnknownUtxo(OutPoint),
-}
-
-impl fmt::Display for AddUtxoError {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        match self {
-            Self::UnknownUtxo(outpoint) => write!(
-                f,
-                "UTXO not found in the internal database for txid: {} with vout: {}",
-                outpoint.txid, outpoint.vout
-            ),
-        }
-    }
-}
-
-#[cfg(feature = "std")]
-impl std::error::Error for AddUtxoError {}
-
-#[derive(Debug)]
-/// Error returned from [`TxBuilder::add_foreign_utxo`].
-pub enum AddForeignUtxoError {
-    /// Foreign utxo outpoint txid does not match PSBT input txid
-    InvalidTxid {
-        /// PSBT input txid
-        input_txid: Txid,
-        /// Foreign UTXO outpoint
-        foreign_utxo: OutPoint,
-    },
-    /// Requested outpoint doesn't exist in the tx (vout greater than available outputs)
-    InvalidOutpoint(OutPoint),
-    /// Foreign utxo missing witness_utxo or non_witness_utxo
-    MissingUtxo,
-}
-
-impl fmt::Display for AddForeignUtxoError {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        match self {
-            Self::InvalidTxid {
-                input_txid,
-                foreign_utxo,
-            } => write!(
-                f,
-                "Foreign UTXO outpoint txid: {} does not match PSBT input txid: {}",
-                foreign_utxo.txid, input_txid,
-            ),
-            Self::InvalidOutpoint(outpoint) => write!(
-                f,
-                "Requested outpoint doesn't exist for txid: {} with vout: {}",
-                outpoint.txid, outpoint.vout,
-            ),
-            Self::MissingUtxo => write!(f, "Foreign utxo missing witness_utxo or non_witness_utxo"),
-        }
-    }
-}
-
-#[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, Cs: CoinSelectionAlgorithm> TxBuilder<'a, Cs, CreateTx> {
     /// Replace the recipients already added with a new list
     pub fn set_recipients(&mut self, recipients: Vec<(ScriptBuf, Amount)>) -> &mut Self {
         self.params.recipients = recipients
@@ -745,11 +624,8 @@ impl<'a, Cs: CoinSelectionAlgorithm> TxBuilder<'a, 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
     ///
@@ -762,7 +638,6 @@ impl<'a, Cs: CoinSelectionAlgorithm> TxBuilder<'a, Cs, CreateTx> {
     /// # use bdk_wallet::*;
     /// # use bdk_wallet::wallet::ChangeSet;
     /// # use bdk_wallet::wallet::error::CreateTxError;
-    /// # use bdk_wallet::wallet::tx_builder::CreateTx;
     /// # use bdk_persist::PersistBackend;
     /// # use anyhow::Error;
     /// # let to_address =
@@ -783,7 +658,6 @@ impl<'a, Cs: CoinSelectionAlgorithm> TxBuilder<'a, 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 +667,81 @@ impl<'a, Cs: CoinSelectionAlgorithm> TxBuilder<'a, Cs, CreateTx> {
     }
 }
 
-// methods supported only by bump_fee
-impl<'a> TxBuilder<'a, 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.
+impl<'a, Cs: CoinSelectionAlgorithm> TxBuilder<'a, Cs> {
+    /// Finish building the transaction.
     ///
-    /// **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 a new [`Psbt`] per [`BIP174`].
     ///
-    /// 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)),
+    /// [`BIP174`]: https://github.com/bitcoin/bips/blob/master/bip-0174.mediawiki
+    pub fn finish(self) -> Result<Psbt, CreateTxError> {
+        self.wallet
+            .borrow_mut()
+            .create_tx(self.coin_selection, self.params)
+    }
+}
+
+#[derive(Debug)]
+/// Error returned from [`TxBuilder::add_utxo`] and [`TxBuilder::add_utxos`]
+pub enum AddUtxoError {
+    /// Happens when trying to spend an UTXO that is not in the internal database
+    UnknownUtxo(OutPoint),
+}
+
+impl fmt::Display for AddUtxoError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            Self::UnknownUtxo(outpoint) => write!(
+                f,
+                "UTXO not found in the internal database for txid: {} with vout: {}",
+                outpoint.txid, outpoint.vout
+            ),
+        }
+    }
+}
+
+#[cfg(feature = "std")]
+impl std::error::Error for AddUtxoError {}
+
+#[derive(Debug)]
+/// Error returned from [`TxBuilder::add_foreign_utxo`].
+pub enum AddForeignUtxoError {
+    /// Foreign utxo outpoint txid does not match PSBT input txid
+    InvalidTxid {
+        /// PSBT input txid
+        input_txid: Txid,
+        /// Foreign UTXO outpoint
+        foreign_utxo: OutPoint,
+    },
+    /// Requested outpoint doesn't exist in the tx (vout greater than available outputs)
+    InvalidOutpoint(OutPoint),
+    /// Foreign utxo missing witness_utxo or non_witness_utxo
+    MissingUtxo,
+}
+
+impl fmt::Display for AddForeignUtxoError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            Self::InvalidTxid {
+                input_txid,
+                foreign_utxo,
+            } => write!(
+                f,
+                "Foreign UTXO outpoint txid: {} does not match PSBT input txid: {}",
+                foreign_utxo.txid, input_txid,
+            ),
+            Self::InvalidOutpoint(outpoint) => write!(
+                f,
+                "Requested outpoint doesn't exist for txid: {} with vout: {}",
+                outpoint.txid, outpoint.vout,
+            ),
+            Self::MissingUtxo => write!(f, "Foreign utxo missing witness_utxo or non_witness_utxo"),
         }
     }
 }
 
+#[cfg(feature = "std")]
+impl std::error::Error for AddForeignUtxoError {}
+
 /// Ordering of the transaction's inputs and outputs
 #[derive(Default, Debug, Ord, PartialOrd, Eq, PartialEq, Hash, Clone, Copy)]
 pub enum TxOrdering {