Skip to content

Commit 1ab9204

Browse files
notmandatorynotmandatory
committed
fix(wallet): remove TxBuilder allow_shrinking function and unneeded context param
1 parent 62619d3 commit 1ab9204

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
@@ -56,7 +56,7 @@ pub use utils::IsDust;
5656

5757
use coin_selection::DefaultCoinSelectionAlgorithm;
5858
use signer::{SignOptions, SignerOrdering, SignersContainer, TransactionSigner};
59-
use tx_builder::{BumpFee, CreateTx, FeePolicy, TxBuilder, TxParams};
59+
use tx_builder::{FeePolicy, TxBuilder, TxParams};
6060
use utils::{check_nsequence_rbf, After, Older, SecpCtx};
6161

6262
use crate::descriptor::policy::BuildSatisfaction;
@@ -1248,12 +1248,11 @@ impl<D> Wallet<D> {
12481248
/// ```
12491249
///
12501250
/// [`TxBuilder`]: crate::TxBuilder
1251-
pub fn build_tx(&mut self) -> TxBuilder<'_, D, DefaultCoinSelectionAlgorithm, CreateTx> {
1251+
pub fn build_tx(&mut self) -> TxBuilder<'_, D, DefaultCoinSelectionAlgorithm> {
12521252
TxBuilder {
12531253
wallet: alloc::rc::Rc::new(core::cell::RefCell::new(self)),
12541254
params: TxParams::default(),
12551255
coin_selection: DefaultCoinSelectionAlgorithm::default(),
1256-
phantom: core::marker::PhantomData,
12571256
}
12581257
}
12591258

@@ -1427,7 +1426,6 @@ impl<D> Wallet<D> {
14271426
};
14281427

14291428
let (fee_rate, mut fee_amount) = match params.fee_policy.unwrap_or_default() {
1430-
//FIXME: see https://github.com/bitcoindevkit/bdk/issues/256
14311429
FeePolicy::FeeAmount(fee) => {
14321430
if let Some(previous_fee) = params.bumping_fee {
14331431
if fee < previous_fee.absolute {
@@ -1658,7 +1656,7 @@ impl<D> Wallet<D> {
16581656
pub fn build_fee_bump(
16591657
&mut self,
16601658
txid: Txid,
1661-
) -> Result<TxBuilder<'_, D, DefaultCoinSelectionAlgorithm, BumpFee>, BuildFeeBumpError> {
1659+
) -> Result<TxBuilder<'_, D, DefaultCoinSelectionAlgorithm>, BuildFeeBumpError> {
16621660
let graph = self.indexed_graph.graph();
16631661
let txout_index = &self.indexed_graph.index;
16641662
let chain_tip = self.chain.tip().block_id();
@@ -1782,7 +1780,6 @@ impl<D> Wallet<D> {
17821780
wallet: alloc::rc::Rc::new(core::cell::RefCell::new(self)),
17831781
params,
17841782
coin_selection: DefaultCoinSelectionAlgorithm::default(),
1785-
phantom: core::marker::PhantomData,
17861783
})
17871784
}
17881785

‎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.
@@ -556,16 +538,17 @@ impl<'a, D, Cs, Ctx> TxBuilder<'a, D, Cs, Ctx> {
556538
///
557539
/// Overrides the [`DefaultCoinSelectionAlgorithm`].
558540
///
559-
/// 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.
541+
/// 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+
///
543+
/// [`DefaultCoinSelectionAlgorithm`]: super::coin_selection::DefaultCoinSelectionAlgorithm
560544
pub fn coin_selection<P: CoinSelectionAlgorithm>(
561545
self,
562546
coin_selection: P,
563-
) -> TxBuilder<'a, D, P, Ctx> {
547+
) -> TxBuilder<'a, D, P> {
564548
TxBuilder {
565549
wallet: self.wallet,
566550
params: self.params,
567551
coin_selection,
568-
phantom: PhantomData,
569552
}
570553
}
571554

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

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

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

0 commit comments

Comments
 (0)