Skip to content

Commit e35839a

Browse files
notmandatorynotmandatory
committed
fix(wallet): remove TxBuilder allow_shrinking function and unneeded context param
1 parent 80e190b commit e35839a

File tree

3 files changed

+105
-176
lines changed

3 files changed

+105
-176
lines changed

‎crates/bdk/src/wallet/mod.rs

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ pub use utils::IsDust;
5757
#[allow(deprecated)]
5858
use coin_selection::DefaultCoinSelectionAlgorithm;
5959
use signer::{SignOptions, SignerOrdering, SignersContainer, TransactionSigner};
60-
use tx_builder::{BumpFee, CreateTx, FeePolicy, TxBuilder, TxParams};
60+
use tx_builder::{FeePolicy, TxBuilder, TxParams};
6161
use utils::{check_nsequence_rbf, After, Older, SecpCtx};
6262

6363
use crate::descriptor::policy::BuildSatisfaction;
@@ -1252,12 +1252,11 @@ impl<D> Wallet<D> {
12521252
/// ```
12531253
///
12541254
/// [`TxBuilder`]: crate::TxBuilder
1255-
pub fn build_tx(&mut self) -> TxBuilder<'_, D, DefaultCoinSelectionAlgorithm, CreateTx> {
1255+
pub fn build_tx(&mut self) -> TxBuilder<'_, D, DefaultCoinSelectionAlgorithm> {
12561256
TxBuilder {
12571257
wallet: alloc::rc::Rc::new(core::cell::RefCell::new(self)),
12581258
params: TxParams::default(),
12591259
coin_selection: DefaultCoinSelectionAlgorithm::default(),
1260-
phantom: core::marker::PhantomData,
12611260
}
12621261
}
12631262

@@ -1431,7 +1430,6 @@ impl<D> Wallet<D> {
14311430
};
14321431

14331432
let (fee_rate, mut fee_amount) = match params.fee_policy.unwrap_or_default() {
1434-
//FIXME: see https://github.com/bitcoindevkit/bdk/issues/256
14351433
FeePolicy::FeeAmount(fee) => {
14361434
if let Some(previous_fee) = params.bumping_fee {
14371435
if fee < previous_fee.absolute {
@@ -1662,7 +1660,7 @@ impl<D> Wallet<D> {
16621660
pub fn build_fee_bump(
16631661
&mut self,
16641662
txid: Txid,
1665-
) -> Result<TxBuilder<'_, D, DefaultCoinSelectionAlgorithm, BumpFee>, BuildFeeBumpError> {
1663+
) -> Result<TxBuilder<'_, D, DefaultCoinSelectionAlgorithm>, BuildFeeBumpError> {
16661664
let graph = self.indexed_graph.graph();
16671665
let txout_index = &self.indexed_graph.index;
16681666
let chain_tip = self.chain.tip().block_id();
@@ -1786,7 +1784,6 @@ impl<D> Wallet<D> {
17861784
wallet: alloc::rc::Rc::new(core::cell::RefCell::new(self)),
17871785
params,
17881786
coin_selection: DefaultCoinSelectionAlgorithm::default(),
1789-
phantom: core::marker::PhantomData,
17901787
})
17911788
}
17921789

‎crates/bdk/src/wallet/tx_builder.rs

Lines changed: 80 additions & 158 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
//! # use bdk::*;
2020
//! # use bdk::wallet::ChangeSet;
2121
//! # use bdk::wallet::error::CreateTxError;
22-
//! # use bdk::wallet::tx_builder::CreateTx;
2322
//! # use bdk_chain::PersistBackend;
2423
//! # use anyhow::Error;
2524
//! # let to_address = Address::from_str("2N4eQYCbKUHCCTUjBJeHcJp9ok6J2GZsTDt").unwrap().assume_checked();
@@ -43,32 +42,17 @@
4342
use alloc::{boxed::Box, rc::Rc, string::String, vec::Vec};
4443
use core::cell::RefCell;
4544
use core::fmt;
46-
use core::marker::PhantomData;
4745

4846
use bdk_chain::PersistBackend;
4947
use bitcoin::psbt::{self, PartiallySignedTransaction as Psbt};
5048
use bitcoin::script::PushBytes;
5149
use bitcoin::{absolute, FeeRate, OutPoint, ScriptBuf, Sequence, Transaction, Txid};
5250

53-
use super::coin_selection::{CoinSelectionAlgorithm, DefaultCoinSelectionAlgorithm};
51+
use super::coin_selection::CoinSelectionAlgorithm;
5452
use super::{ChangeSet, CreateTxError, Wallet};
5553
use crate::collections::{BTreeMap, HashSet};
5654
use crate::{KeychainKind, LocalOutput, Utxo, WeightedUtxo};
5755

58-
/// Context in which the [`TxBuilder`] is valid
59-
pub trait TxBuilderContext: core::fmt::Debug + Default + Clone {}
60-
61-
/// Marker type to indicate the [`TxBuilder`] is being used to create a new transaction (as opposed
62-
/// to bumping the fee of an existing one).
63-
#[derive(Debug, Default, Clone)]
64-
pub struct CreateTx;
65-
impl TxBuilderContext for CreateTx {}
66-
67-
/// Marker type to indicate the [`TxBuilder`] is being used to bump the fee of an existing transaction.
68-
#[derive(Debug, Default, Clone)]
69-
pub struct BumpFee;
70-
impl TxBuilderContext for BumpFee {}
71-
7256
/// A transaction builder
7357
///
7458
/// A `TxBuilder` is created by calling [`build_tx`] or [`build_fee_bump`] on a wallet. After
@@ -124,11 +108,10 @@ impl TxBuilderContext for BumpFee {}
124108
/// [`finish`]: Self::finish
125109
/// [`coin_selection`]: Self::coin_selection
126110
#[derive(Debug)]
127-
pub struct TxBuilder<'a, D, Cs, Ctx> {
111+
pub struct TxBuilder<'a, D, Cs> {
128112
pub(crate) wallet: Rc<RefCell<&'a mut Wallet<D>>>,
129113
pub(crate) params: TxParams,
130114
pub(crate) coin_selection: Cs,
131-
pub(crate) phantom: PhantomData<Ctx>,
132115
}
133116

134117
/// The parameters for transaction creation sans coin selection algorithm.
@@ -176,19 +159,18 @@ impl Default for FeePolicy {
176159
}
177160
}
178161

179-
impl<'a, D, Cs: Clone, Ctx> Clone for TxBuilder<'a, D, Cs, Ctx> {
162+
impl<'a, D, Cs: Clone> Clone for TxBuilder<'a, D, Cs> {
180163
fn clone(&self) -> Self {
181164
TxBuilder {
182165
wallet: self.wallet.clone(),
183166
params: self.params.clone(),
184167
coin_selection: self.coin_selection.clone(),
185-
phantom: PhantomData,
186168
}
187169
}
188170
}
189171

190-
// methods supported by both contexts, for any CoinSelectionAlgorithm
191-
impl<'a, D, Cs, Ctx> TxBuilder<'a, D, Cs, Ctx> {
172+
// methods supported for any CoinSelectionAlgorithm
173+
impl<'a, D, Cs> TxBuilder<'a, D, Cs> {
192174
/// Set a custom fee rate.
193175
///
194176
/// This method sets the mining fee paid by the transaction as a rate on its size.
@@ -557,16 +539,17 @@ impl<'a, D, Cs, Ctx> TxBuilder<'a, D, Cs, Ctx> {
557539
///
558540
/// Overrides the [`DefaultCoinSelectionAlgorithm`].
559541
///
560-
/// 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.
542+
/// 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.
543+
///
544+
/// [`DefaultCoinSelectionAlgorithm`]: super::coin_selection::DefaultCoinSelectionAlgorithm
561545
pub fn coin_selection<P: CoinSelectionAlgorithm>(
562546
self,
563547
coin_selection: P,
564-
) -> TxBuilder<'a, D, P, Ctx> {
548+
) -> TxBuilder<'a, D, P> {
565549
TxBuilder {
566550
wallet: self.wallet,
567551
params: self.params,
568552
coin_selection,
569-
phantom: PhantomData,
570553
}
571554
}
572555

@@ -614,9 +597,79 @@ impl<'a, D, Cs, Ctx> TxBuilder<'a, D, Cs, Ctx> {
614597
self.params.allow_dust = allow_dust;
615598
self
616599
}
600+
601+
/// Replace the recipients already added with a new list
602+
pub fn set_recipients(&mut self, recipients: Vec<(ScriptBuf, u64)>) -> &mut Self {
603+
self.params.recipients = recipients;
604+
self
605+
}
606+
607+
/// Add a recipient to the internal list
608+
pub fn add_recipient(&mut self, script_pubkey: ScriptBuf, amount: u64) -> &mut Self {
609+
self.params.recipients.push((script_pubkey, amount));
610+
self
611+
}
612+
613+
/// Add data as an output, using OP_RETURN
614+
pub fn add_data<T: AsRef<PushBytes>>(&mut self, data: &T) -> &mut Self {
615+
let script = ScriptBuf::new_op_return(data);
616+
self.add_recipient(script, 0u64);
617+
self
618+
}
619+
620+
/// Sets the address to *drain* excess coins to.
621+
///
622+
/// Usually, when there are excess coins they are sent to a change address generated by the
623+
/// wallet. This option replaces the usual change address with an arbitrary `script_pubkey` of
624+
/// your choosing. Just as with a change output, if the drain output is not needed (the excess
625+
/// coins are too small) it will not be included in the resulting transaction. The only
626+
/// difference is that it is valid to use `drain_to` without setting any ordinary recipients
627+
/// with [`add_recipient`] (but it is perfectly fine to add recipients as well).
628+
///
629+
/// If you choose not to set any recipients, you should provide the utxos that the
630+
/// transaction should spend via [`add_utxos`].
631+
///
632+
/// # Example
633+
///
634+
/// `drain_to` is very useful for draining all the coins in a wallet with [`drain_wallet`] to a
635+
/// single address.
636+
///
637+
/// ```
638+
/// # use std::str::FromStr;
639+
/// # use bitcoin::*;
640+
/// # use bdk::*;
641+
/// # use bdk::wallet::ChangeSet;
642+
/// # use bdk::wallet::error::CreateTxError;
643+
/// # use bdk_chain::PersistBackend;
644+
/// # use anyhow::Error;
645+
/// # let to_address =
646+
/// Address::from_str("2N4eQYCbKUHCCTUjBJeHcJp9ok6J2GZsTDt")
647+
/// .unwrap()
648+
/// .assume_checked();
649+
/// # let mut wallet = doctest_wallet!();
650+
/// let mut tx_builder = wallet.build_tx();
651+
///
652+
/// tx_builder
653+
/// // Spend all outputs in this wallet.
654+
/// .drain_wallet()
655+
/// // Send the excess (which is all the coins minus the fee) to this address.
656+
/// .drain_to(to_address.script_pubkey())
657+
/// .fee_rate(FeeRate::from_sat_per_vb(5).expect("valid feerate"))
658+
/// .enable_rbf();
659+
/// let psbt = tx_builder.finish()?;
660+
/// # Ok::<(), anyhow::Error>(())
661+
/// ```
662+
///
663+
/// [`add_recipient`]: Self::add_recipient
664+
/// [`add_utxos`]: Self::add_utxos
665+
/// [`drain_wallet`]: Self::drain_wallet
666+
pub fn drain_to(&mut self, script_pubkey: ScriptBuf) -> &mut Self {
667+
self.params.drain_to = Some(script_pubkey);
668+
self
669+
}
617670
}
618671

619-
impl<'a, D, Cs: CoinSelectionAlgorithm, Ctx> TxBuilder<'a, D, Cs, Ctx> {
672+
impl<'a, D, Cs: CoinSelectionAlgorithm> TxBuilder<'a, D, Cs> {
620673
/// Finish building the transaction.
621674
///
622675
/// Returns a new [`Psbt`] per [`BIP174`].
@@ -694,137 +747,6 @@ impl fmt::Display for AddForeignUtxoError {
694747
#[cfg(feature = "std")]
695748
impl std::error::Error for AddForeignUtxoError {}
696749

697-
#[derive(Debug)]
698-
/// Error returned from [`TxBuilder::allow_shrinking`]
699-
pub enum AllowShrinkingError {
700-
/// Script/PubKey was not in the original transaction
701-
MissingScriptPubKey(ScriptBuf),
702-
}
703-
704-
impl fmt::Display for AllowShrinkingError {
705-
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
706-
match self {
707-
Self::MissingScriptPubKey(script_buf) => write!(
708-
f,
709-
"Script/PubKey was not in the original transaction: {}",
710-
script_buf,
711-
),
712-
}
713-
}
714-
}
715-
716-
#[cfg(feature = "std")]
717-
impl std::error::Error for AllowShrinkingError {}
718-
719-
impl<'a, D, Cs: CoinSelectionAlgorithm> TxBuilder<'a, D, Cs, CreateTx> {
720-
/// Replace the recipients already added with a new list
721-
pub fn set_recipients(&mut self, recipients: Vec<(ScriptBuf, u64)>) -> &mut Self {
722-
self.params.recipients = recipients;
723-
self
724-
}
725-
726-
/// Add a recipient to the internal list
727-
pub fn add_recipient(&mut self, script_pubkey: ScriptBuf, amount: u64) -> &mut Self {
728-
self.params.recipients.push((script_pubkey, amount));
729-
self
730-
}
731-
732-
/// Add data as an output, using OP_RETURN
733-
pub fn add_data<T: AsRef<PushBytes>>(&mut self, data: &T) -> &mut Self {
734-
let script = ScriptBuf::new_op_return(data);
735-
self.add_recipient(script, 0u64);
736-
self
737-
}
738-
739-
/// Sets the address to *drain* excess coins to.
740-
///
741-
/// Usually, when there are excess coins they are sent to a change address generated by the
742-
/// wallet. This option replaces the usual change address with an arbitrary `script_pubkey` of
743-
/// your choosing. Just as with a change output, if the drain output is not needed (the excess
744-
/// coins are too small) it will not be included in the resulting transaction. The only
745-
/// difference is that it is valid to use `drain_to` without setting any ordinary recipients
746-
/// with [`add_recipient`] (but it is perfectly fine to add recipients as well).
747-
///
748-
/// If you choose not to set any recipients, you should either provide the utxos that the
749-
/// transaction should spend via [`add_utxos`], or set [`drain_wallet`] to spend all of them.
750-
///
751-
/// When bumping the fees of a transaction made with this option, you probably want to
752-
/// use [`allow_shrinking`] to allow this output to be reduced to pay for the extra fees.
753-
///
754-
/// # Example
755-
///
756-
/// `drain_to` is very useful for draining all the coins in a wallet with [`drain_wallet`] to a
757-
/// single address.
758-
///
759-
/// ```
760-
/// # use std::str::FromStr;
761-
/// # use bitcoin::*;
762-
/// # use bdk::*;
763-
/// # use bdk::wallet::ChangeSet;
764-
/// # use bdk::wallet::error::CreateTxError;
765-
/// # use bdk::wallet::tx_builder::CreateTx;
766-
/// # use bdk_chain::PersistBackend;
767-
/// # use anyhow::Error;
768-
/// # let to_address =
769-
/// Address::from_str("2N4eQYCbKUHCCTUjBJeHcJp9ok6J2GZsTDt")
770-
/// .unwrap()
771-
/// .assume_checked();
772-
/// # let mut wallet = doctest_wallet!();
773-
/// let mut tx_builder = wallet.build_tx();
774-
///
775-
/// tx_builder
776-
/// // Spend all outputs in this wallet.
777-
/// .drain_wallet()
778-
/// // Send the excess (which is all the coins minus the fee) to this address.
779-
/// .drain_to(to_address.script_pubkey())
780-
/// .fee_rate(FeeRate::from_sat_per_vb(5).expect("valid feerate"))
781-
/// .enable_rbf();
782-
/// let psbt = tx_builder.finish()?;
783-
/// # Ok::<(), anyhow::Error>(())
784-
/// ```
785-
///
786-
/// [`allow_shrinking`]: Self::allow_shrinking
787-
/// [`add_recipient`]: Self::add_recipient
788-
/// [`add_utxos`]: Self::add_utxos
789-
/// [`drain_wallet`]: Self::drain_wallet
790-
pub fn drain_to(&mut self, script_pubkey: ScriptBuf) -> &mut Self {
791-
self.params.drain_to = Some(script_pubkey);
792-
self
793-
}
794-
}
795-
796-
// methods supported only by bump_fee
797-
impl<'a, D> TxBuilder<'a, D, DefaultCoinSelectionAlgorithm, BumpFee> {
798-
/// Explicitly tells the wallet that it is allowed to reduce the amount of the output matching this
799-
/// `script_pubkey` in order to bump the transaction fee. Without specifying this the wallet
800-
/// will attempt to find a change output to shrink instead.
801-
///
802-
/// **Note** that the output may shrink to below the dust limit and therefore be removed. If it is
803-
/// preserved then it is currently not guaranteed to be in the same position as it was
804-
/// originally.
805-
///
806-
/// Returns an `Err` if `script_pubkey` can't be found among the recipients of the
807-
/// transaction we are bumping.
808-
pub fn allow_shrinking(
809-
&mut self,
810-
script_pubkey: ScriptBuf,
811-
) -> Result<&mut Self, AllowShrinkingError> {
812-
match self
813-
.params
814-
.recipients
815-
.iter()
816-
.position(|(recipient_script, _)| *recipient_script == script_pubkey)
817-
{
818-
Some(position) => {
819-
self.params.recipients.remove(position);
820-
self.params.drain_to = Some(script_pubkey);
821-
Ok(self)
822-
}
823-
None => Err(AllowShrinkingError::MissingScriptPubKey(script_pubkey)),
824-
}
825-
}
826-
}
827-
828750
/// Ordering of the transaction's inputs and outputs
829751
#[derive(Default, Debug, Ord, PartialOrd, Eq, PartialEq, Hash, Clone, Copy)]
830752
pub enum TxOrdering {

0 commit comments

Comments
 (0)