Skip to content

Commit c482e70

Browse files
notmandatorynotmandatory
committed
refactor(wallet)!: Add CreateTxError and use as error type for TxBuilder::finish()
1 parent cc552c5 commit c482e70

File tree

4 files changed

+116
-33
lines changed

4 files changed

+116
-33
lines changed

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@
2626
//! ```
2727
//! # use std::str::FromStr;
2828
//! # use bitcoin::*;
29-
//! # use bdk::wallet::{self, coin_selection::*};
29+
//! # use bdk::wallet::{self, ChangeSet, coin_selection::*, CreateTxError};
30+
//! # use bdk_chain::PersistBackend;
3031
//! # use bdk::*;
3132
//! # use bdk::wallet::coin_selection::decide_change;
3233
//! # const TXIN_BASE_WEIGHT: usize = (32 + 4 + 4) * 4;
@@ -94,7 +95,7 @@
9495
//!
9596
//! // inspect, sign, broadcast, ...
9697
//!
97-
//! # Ok::<(), bdk::Error>(())
98+
//! # Ok::<(), CreateTxError<<() as PersistBackend<ChangeSet>>::WriteError>>(())
9899
//! ```
99100
100101
use crate::types::FeeRate;

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

Lines changed: 93 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -56,16 +56,17 @@ pub mod hardwaresigner;
5656

5757
pub use utils::IsDust;
5858

59+
use crate::descriptor;
5960
#[allow(deprecated)]
6061
use coin_selection::DefaultCoinSelectionAlgorithm;
6162
use signer::{SignOptions, SignerOrdering, SignersContainer, TransactionSigner};
6263
use tx_builder::{BumpFee, CreateTx, FeePolicy, TxBuilder, TxParams};
6364
use utils::{check_nsequence_rbf, After, Older, SecpCtx};
6465

65-
use crate::descriptor::policy::BuildSatisfaction;
66+
use crate::descriptor::policy::{BuildSatisfaction, PolicyError};
6667
use crate::descriptor::{
67-
calc_checksum, into_wallet_descriptor_checked, DerivedDescriptor, DescriptorMeta,
68-
ExtendedDescriptor, ExtractPolicy, IntoWalletDescriptor, Policy, XKeyUtils,
68+
calc_checksum, into_wallet_descriptor_checked, DerivedDescriptor, DescriptorError,
69+
DescriptorMeta, ExtendedDescriptor, ExtractPolicy, IntoWalletDescriptor, Policy, XKeyUtils,
6970
};
7071
use crate::error::{Error, MiniscriptPsbtError};
7172
use crate::psbt::PsbtUtils;
@@ -401,6 +402,64 @@ pub enum InsertTxError {
401402
},
402403
}
403404

405+
#[cfg(feature = "std")]
406+
impl<P: core::fmt::Display + core::fmt::Debug> std::error::Error for NewError<P> {}
407+
408+
#[derive(Debug)]
409+
/// Error returned from [`TxBuilder::finish`]
410+
pub enum CreateTxError<P> {
411+
/// There was a problem with the descriptors passed in
412+
Descriptor(DescriptorError),
413+
/// We were unable to write wallet data to the persistence backend
414+
Persist(P),
415+
/// There was a problem while extracting and manipulating policies
416+
Policy(PolicyError),
417+
/// TODO: replace this with specific error types
418+
Bdk(Error),
419+
}
420+
421+
#[cfg(feature = "std")]
422+
impl<P> fmt::Display for CreateTxError<P>
423+
where
424+
P: fmt::Display,
425+
{
426+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
427+
match self {
428+
Self::Descriptor(e) => e.fmt(f),
429+
Self::Persist(e) => {
430+
write!(
431+
f,
432+
"failed to write wallet data to persistence backend: {}",
433+
e
434+
)
435+
}
436+
Self::Bdk(e) => e.fmt(f),
437+
Self::Policy(e) => e.fmt(f),
438+
}
439+
}
440+
}
441+
442+
impl<P> From<descriptor::error::Error> for CreateTxError<P> {
443+
fn from(err: descriptor::error::Error) -> Self {
444+
CreateTxError::Descriptor(err)
445+
}
446+
}
447+
448+
impl<P> From<PolicyError> for CreateTxError<P> {
449+
fn from(err: PolicyError) -> Self {
450+
CreateTxError::Policy(err)
451+
}
452+
}
453+
454+
impl<P> From<Error> for CreateTxError<P> {
455+
fn from(err: Error) -> Self {
456+
CreateTxError::Bdk(err)
457+
}
458+
}
459+
460+
#[cfg(feature = "std")]
461+
impl<P: core::fmt::Display + core::fmt::Debug> std::error::Error for CreateTxError<P> {}
462+
404463
impl<D> Wallet<D> {
405464
/// Initialize an empty [`Wallet`].
406465
pub fn new<E: IntoWalletDescriptor>(
@@ -1092,6 +1151,8 @@ impl<D> Wallet<D> {
10921151
/// # use std::str::FromStr;
10931152
/// # use bitcoin::*;
10941153
/// # use bdk::*;
1154+
/// # use bdk::wallet::{ChangeSet,CreateTxError};
1155+
/// # use bdk_chain::PersistBackend;
10951156
/// # let descriptor = "wpkh(tpubD6NzVbkrYhZ4Xferm7Pz4VnjdcDPFyjVu5K4iZXQ4pVN8Cks4pHVowTBXBKRhX64pkRyJZJN5xAKj4UDNnLPb5p2sSKXhewoYx5GbTdUFWq/*)";
10961157
/// # let mut wallet = doctest_wallet!();
10971158
/// # let to_address = Address::from_str("2N4eQYCbKUHCCTUjBJeHcJp9ok6J2GZsTDt").unwrap().assume_checked();
@@ -1103,7 +1164,7 @@ impl<D> Wallet<D> {
11031164
/// };
11041165
///
11051166
/// // sign and broadcast ...
1106-
/// # Ok::<(), bdk::Error>(())
1167+
/// # Ok::<(), CreateTxError<<() as PersistBackend<ChangeSet>>::WriteError>>(())
11071168
/// ```
11081169
///
11091170
/// [`TxBuilder`]: crate::TxBuilder
@@ -1155,15 +1216,15 @@ impl<D> Wallet<D> {
11551216
&& external_policy.requires_path()
11561217
&& params.external_policy_path.is_none()
11571218
{
1158-
return Err(Error::SpendingPolicyRequired(KeychainKind::External));
1219+
return Err(Error::SpendingPolicyRequired(KeychainKind::External).into());
11591220
};
11601221
// Same for the internal_policy path, if present
11611222
if let Some(internal_policy) = &internal_policy {
11621223
if params.change_policy != tx_builder::ChangeSpendPolicy::ChangeForbidden
11631224
&& internal_policy.requires_path()
11641225
&& params.internal_policy_path.is_none()
11651226
{
1166-
return Err(Error::SpendingPolicyRequired(KeychainKind::Internal));
1227+
return Err(Error::SpendingPolicyRequired(KeychainKind::Internal).into());
11671228
};
11681229
}
11691230

@@ -1192,13 +1253,14 @@ impl<D> Wallet<D> {
11921253

11931254
let version = match params.version {
11941255
Some(tx_builder::Version(0)) => {
1195-
return Err(Error::Generic("Invalid version `0`".into()))
1256+
return Err(Error::Generic("Invalid version `0`".into()).into())
11961257
}
11971258
Some(tx_builder::Version(1)) if requirements.csv.is_some() => {
11981259
return Err(Error::Generic(
11991260
"TxBuilder requested version `1`, but at least `2` is needed to use OP_CSV"
12001261
.into(),
1201-
))
1262+
)
1263+
.into())
12021264
}
12031265
Some(tx_builder::Version(x)) => x,
12041266
None if requirements.csv.is_some() => 2,
@@ -1240,7 +1302,7 @@ impl<D> Wallet<D> {
12401302
// Specific nLockTime required and it's compatible with the constraints
12411303
Some(x) if requirements.timelock.unwrap().is_same_unit(x) && x >= requirements.timelock.unwrap() => x,
12421304
// Invalid nLockTime required
1243-
Some(x) => return Err(Error::Generic(format!("TxBuilder requested timelock of `{:?}`, but at least `{:?}` is required to spend from this script", x, requirements.timelock.unwrap())))
1305+
Some(x) => return Err(Error::Generic(format!("TxBuilder requested timelock of `{:?}`, but at least `{:?}` is required to spend from this script", x, requirements.timelock.unwrap())).into())
12441306
};
12451307

12461308
let n_sequence = match (params.rbf, requirements.csv) {
@@ -1260,7 +1322,8 @@ impl<D> Wallet<D> {
12601322
(Some(tx_builder::RbfValue::Value(rbf)), _) if !rbf.is_rbf() => {
12611323
return Err(Error::Generic(
12621324
"Cannot enable RBF with a nSequence >= 0xFFFFFFFE".into(),
1263-
))
1325+
)
1326+
.into())
12641327
}
12651328
// RBF with a specific value requested, but the value is incompatible with CSV
12661329
(Some(tx_builder::RbfValue::Value(rbf)), Some(csv))
@@ -1269,7 +1332,8 @@ impl<D> Wallet<D> {
12691332
return Err(Error::Generic(format!(
12701333
"Cannot enable RBF with nSequence `{:?}` given a required OP_CSV of `{:?}`",
12711334
rbf, csv
1272-
)))
1335+
))
1336+
.into())
12731337
}
12741338

12751339
// RBF enabled with the default value with CSV also enabled. CSV takes precedence
@@ -1290,7 +1354,8 @@ impl<D> Wallet<D> {
12901354
if *fee < previous_fee.absolute {
12911355
return Err(Error::FeeTooLow {
12921356
required: previous_fee.absolute,
1293-
});
1357+
}
1358+
.into());
12941359
}
12951360
}
12961361
(FeeRate::from_sat_per_vb(0.0), *fee)
@@ -1301,7 +1366,8 @@ impl<D> Wallet<D> {
13011366
if *rate < required_feerate {
13021367
return Err(Error::FeeRateTooLow {
13031368
required: required_feerate,
1304-
});
1369+
}
1370+
.into());
13051371
}
13061372
}
13071373
(*rate, 0)
@@ -1316,7 +1382,7 @@ impl<D> Wallet<D> {
13161382
};
13171383

13181384
if params.manually_selected_only && params.utxos.is_empty() {
1319-
return Err(Error::NoUtxosSelected);
1385+
return Err(Error::NoUtxosSelected.into());
13201386
}
13211387

13221388
// we keep it as a float while we accumulate it, and only round it at the end
@@ -1330,7 +1396,7 @@ impl<D> Wallet<D> {
13301396
&& value.is_dust(script_pubkey)
13311397
&& !script_pubkey.is_provably_unspendable()
13321398
{
1333-
return Err(Error::OutputBelowDustLimit(index));
1399+
return Err(Error::OutputBelowDustLimit(index).into());
13341400
}
13351401

13361402
if self.is_mine(script_pubkey) {
@@ -1365,7 +1431,8 @@ impl<D> Wallet<D> {
13651431
{
13661432
return Err(Error::Generic(
13671433
"The `change_policy` can be set only if the wallet has a change_descriptor".into(),
1368-
));
1434+
)
1435+
.into());
13691436
}
13701437

13711438
let (required_utxos, optional_utxos) = self.preselect_utxos(
@@ -1391,7 +1458,7 @@ impl<D> Wallet<D> {
13911458
.stage(ChangeSet::from(indexed_tx_graph::ChangeSet::from(
13921459
index_changeset,
13931460
)));
1394-
self.persist.commit().expect("TODO");
1461+
self.persist.commit().map_err(CreateTxError::Persist)?;
13951462
spk
13961463
}
13971464
};
@@ -1435,10 +1502,11 @@ impl<D> Wallet<D> {
14351502
return Err(Error::InsufficientFunds {
14361503
needed: *dust_threshold,
14371504
available: remaining_amount.saturating_sub(*change_fee),
1438-
});
1505+
}
1506+
.into());
14391507
}
14401508
} else {
1441-
return Err(Error::NoRecipients);
1509+
return Err(Error::NoRecipients.into());
14421510
}
14431511
}
14441512

@@ -1485,6 +1553,8 @@ impl<D> Wallet<D> {
14851553
/// # use std::str::FromStr;
14861554
/// # use bitcoin::*;
14871555
/// # use bdk::*;
1556+
/// # use bdk::wallet::{ChangeSet, CreateTxError};
1557+
/// # use bdk_chain::PersistBackend;
14881558
/// # let descriptor = "wpkh(tpubD6NzVbkrYhZ4Xferm7Pz4VnjdcDPFyjVu5K4iZXQ4pVN8Cks4pHVowTBXBKRhX64pkRyJZJN5xAKj4UDNnLPb5p2sSKXhewoYx5GbTdUFWq/*)";
14891559
/// # let mut wallet = doctest_wallet!();
14901560
/// # let to_address = Address::from_str("2N4eQYCbKUHCCTUjBJeHcJp9ok6J2GZsTDt").unwrap().assume_checked();
@@ -1508,7 +1578,7 @@ impl<D> Wallet<D> {
15081578
/// let _ = wallet.sign(&mut psbt, SignOptions::default())?;
15091579
/// let fee_bumped_tx = psbt.extract_tx();
15101580
/// // broadcast fee_bumped_tx to replace original
1511-
/// # Ok::<(), bdk::Error>(())
1581+
/// # Ok::<(), CreateTxError<<() as PersistBackend<ChangeSet>>::WriteError>>(())
15121582
/// ```
15131583
// TODO: support for merging multiple transactions while bumping the fees
15141584
pub fn build_fee_bump(
@@ -1655,6 +1725,8 @@ impl<D> Wallet<D> {
16551725
/// # use std::str::FromStr;
16561726
/// # use bitcoin::*;
16571727
/// # use bdk::*;
1728+
/// # use bdk::wallet::{ChangeSet, CreateTxError};
1729+
/// # use bdk_chain::PersistBackend;
16581730
/// # let descriptor = "wpkh(tpubD6NzVbkrYhZ4Xferm7Pz4VnjdcDPFyjVu5K4iZXQ4pVN8Cks4pHVowTBXBKRhX64pkRyJZJN5xAKj4UDNnLPb5p2sSKXhewoYx5GbTdUFWq/*)";
16591731
/// # let mut wallet = doctest_wallet!();
16601732
/// # let to_address = Address::from_str("2N4eQYCbKUHCCTUjBJeHcJp9ok6J2GZsTDt").unwrap().assume_checked();
@@ -1665,7 +1737,7 @@ impl<D> Wallet<D> {
16651737
/// };
16661738
/// let finalized = wallet.sign(&mut psbt, SignOptions::default())?;
16671739
/// assert!(finalized, "we should have signed all the inputs");
1668-
/// # Ok::<(), bdk::Error>(())
1740+
/// # Ok::<(), CreateTxError<<() as PersistBackend<ChangeSet>>::WriteError>>(())
16691741
pub fn sign(
16701742
&self,
16711743
psbt: &mut psbt::PartiallySignedTransaction,

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

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,10 @@
1717
//! # use std::str::FromStr;
1818
//! # use bitcoin::*;
1919
//! # use bdk::*;
20+
//! # use bdk::wallet::{ChangeSet, CreateTxError};
2021
//! # use bdk::wallet::tx_builder::CreateTx;
2122
//! # let to_address = Address::from_str("2N4eQYCbKUHCCTUjBJeHcJp9ok6J2GZsTDt").unwrap().assume_checked();
23+
//! # use bdk_chain::PersistBackend;
2224
//! # let mut wallet = doctest_wallet!();
2325
//! // create a TxBuilder from a wallet
2426
//! let mut tx_builder = wallet.build_tx();
@@ -33,7 +35,7 @@
3335
//! // Turn on RBF signaling
3436
//! .enable_rbf();
3537
//! let psbt = tx_builder.finish()?;
36-
//! # Ok::<(), bdk::Error>(())
38+
//! # Ok::<(), CreateTxError<<() as PersistBackend<ChangeSet>>::WriteError>>(())
3739
//! ```
3840
3941
use crate::collections::BTreeMap;
@@ -48,6 +50,7 @@ use bitcoin::{absolute, script::PushBytes, OutPoint, ScriptBuf, Sequence, Transa
4850

4951
use super::coin_selection::{CoinSelectionAlgorithm, DefaultCoinSelectionAlgorithm};
5052
use super::ChangeSet;
53+
use crate::wallet::CreateTxError;
5154
use crate::types::{FeeRate, KeychainKind, LocalUtxo, WeightedUtxo};
5255
use crate::{Error, Utxo, Wallet};
5356
/// Context in which the [`TxBuilder`] is valid
@@ -78,6 +81,8 @@ impl TxBuilderContext for BumpFee {}
7881
/// # use bdk::wallet::tx_builder::*;
7982
/// # use bitcoin::*;
8083
/// # use core::str::FromStr;
84+
/// # use bdk::wallet::{ChangeSet, CreateTxError};
85+
/// # use bdk_chain::PersistBackend;
8186
/// # let mut wallet = doctest_wallet!();
8287
/// # let addr1 = Address::from_str("2N4eQYCbKUHCCTUjBJeHcJp9ok6J2GZsTDt").unwrap().assume_checked();
8388
/// # let addr2 = addr1.clone();
@@ -102,7 +107,7 @@ impl TxBuilderContext for BumpFee {}
102107
/// };
103108
///
104109
/// assert_eq!(psbt1.unsigned_tx.output[..2], psbt2.unsigned_tx.output[..2]);
105-
/// # Ok::<(), bdk::Error>(())
110+
/// # Ok::<(), CreateTxError<<() as PersistBackend<ChangeSet>>::WriteError>>(())
106111
/// ```
107112
///
108113
/// At the moment [`coin_selection`] is an exception to the rule as it consumes `self`.
@@ -540,7 +545,7 @@ impl<'a, D, Cs: CoinSelectionAlgorithm, Ctx: TxBuilderContext> TxBuilder<'a, D,
540545
/// Returns the [`BIP174`] "PSBT" and summary details about the transaction.
541546
///
542547
/// [`BIP174`]: https://github.com/bitcoin/bips/blob/master/bip-0174.mediawiki
543-
pub fn finish(self) -> Result<Psbt, Error>
548+
pub fn finish(self) -> Result<Psbt, , CreateTxError<D::WriteError>>
544549
where
545550
D: PersistBackend<ChangeSet>,
546551
{
@@ -639,7 +644,9 @@ impl<'a, D, Cs: CoinSelectionAlgorithm> TxBuilder<'a, D, Cs, CreateTx> {
639644
/// # use std::str::FromStr;
640645
/// # use bitcoin::*;
641646
/// # use bdk::*;
647+
/// # use bdk::wallet::{ChangeSet, CreateTxError};
642648
/// # use bdk::wallet::tx_builder::CreateTx;
649+
/// # use bdk_chain::PersistBackend;
643650
/// # let to_address =
644651
/// Address::from_str("2N4eQYCbKUHCCTUjBJeHcJp9ok6J2GZsTDt")
645652
/// .unwrap()
@@ -655,7 +662,7 @@ impl<'a, D, Cs: CoinSelectionAlgorithm> TxBuilder<'a, D, Cs, CreateTx> {
655662
/// .fee_rate(bdk::FeeRate::from_sat_per_vb(5.0))
656663
/// .enable_rbf();
657664
/// let psbt = tx_builder.finish()?;
658-
/// # Ok::<(), bdk::Error>(())
665+
/// # Ok::<(), CreateTxError<<() as PersistBackend<ChangeSet>>::WriteError>>(())
659666
/// ```
660667
///
661668
/// [`allow_shrinking`]: Self::allow_shrinking

0 commit comments

Comments
 (0)