Skip to content

Commit 10e6d21

Browse files
notmandatorynotmandatory
committed
refactor(wallet)!: Add CreateTxError and use as error type for TxBuilder::finish()
1 parent 2f2f138 commit 10e6d21

File tree

4 files changed

+113
-33
lines changed

4 files changed

+113
-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: 90 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;
@@ -265,6 +266,61 @@ pub enum InsertTxError {
265266
#[cfg(feature = "std")]
266267
impl<P: core::fmt::Display + core::fmt::Debug> std::error::Error for NewError<P> {}
267268

269+
#[derive(Debug)]
270+
/// Error returned from [`TxBuilder::finish`]
271+
pub enum CreateTxError<P> {
272+
/// There was a problem with the descriptors passed in
273+
Descriptor(DescriptorError),
274+
/// We were unable to write wallet data to the persistence backend
275+
Persist(P),
276+
/// There was a problem while extracting and manipulating policies
277+
Policy(PolicyError),
278+
/// TODO: replace this with specific error types
279+
Bdk(Error),
280+
}
281+
282+
#[cfg(feature = "std")]
283+
impl<P> fmt::Display for CreateTxError<P>
284+
where
285+
P: fmt::Display,
286+
{
287+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
288+
match self {
289+
Self::Descriptor(e) => e.fmt(f),
290+
Self::Persist(e) => {
291+
write!(
292+
f,
293+
"failed to write wallet data to persistence backend: {}",
294+
e
295+
)
296+
}
297+
Self::Bdk(e) => e.fmt(f),
298+
Self::Policy(e) => e.fmt(f),
299+
}
300+
}
301+
}
302+
303+
impl<P> From<descriptor::error::Error> for CreateTxError<P> {
304+
fn from(err: descriptor::error::Error) -> Self {
305+
CreateTxError::Descriptor(err)
306+
}
307+
}
308+
309+
impl<P> From<PolicyError> for CreateTxError<P> {
310+
fn from(err: PolicyError) -> Self {
311+
CreateTxError::Policy(err)
312+
}
313+
}
314+
315+
impl<P> From<Error> for CreateTxError<P> {
316+
fn from(err: Error) -> Self {
317+
CreateTxError::Bdk(err)
318+
}
319+
}
320+
321+
#[cfg(feature = "std")]
322+
impl<P: core::fmt::Display + core::fmt::Debug> std::error::Error for CreateTxError<P> {}
323+
268324
impl<D> Wallet<D> {
269325
/// Create a wallet from a `descriptor` (and an optional `change_descriptor`) and load related
270326
/// transaction data from `db`.
@@ -823,6 +879,8 @@ impl<D> Wallet<D> {
823879
/// # use std::str::FromStr;
824880
/// # use bitcoin::*;
825881
/// # use bdk::*;
882+
/// # use bdk::wallet::{ChangeSet,CreateTxError};
883+
/// # use bdk_chain::PersistBackend;
826884
/// # let descriptor = "wpkh(tpubD6NzVbkrYhZ4Xferm7Pz4VnjdcDPFyjVu5K4iZXQ4pVN8Cks4pHVowTBXBKRhX64pkRyJZJN5xAKj4UDNnLPb5p2sSKXhewoYx5GbTdUFWq/*)";
827885
/// # let mut wallet = doctest_wallet!();
828886
/// # let to_address = Address::from_str("2N4eQYCbKUHCCTUjBJeHcJp9ok6J2GZsTDt").unwrap().assume_checked();
@@ -834,7 +892,7 @@ impl<D> Wallet<D> {
834892
/// };
835893
///
836894
/// // sign and broadcast ...
837-
/// # Ok::<(), bdk::Error>(())
895+
/// # Ok::<(), CreateTxError<<() as PersistBackend<ChangeSet>>::WriteError>>(())
838896
/// ```
839897
///
840898
/// [`TxBuilder`]: crate::TxBuilder
@@ -886,15 +944,15 @@ impl<D> Wallet<D> {
886944
&& external_policy.requires_path()
887945
&& params.external_policy_path.is_none()
888946
{
889-
return Err(Error::SpendingPolicyRequired(KeychainKind::External));
947+
return Err(Error::SpendingPolicyRequired(KeychainKind::External).into());
890948
};
891949
// Same for the internal_policy path, if present
892950
if let Some(internal_policy) = &internal_policy {
893951
if params.change_policy != tx_builder::ChangeSpendPolicy::ChangeForbidden
894952
&& internal_policy.requires_path()
895953
&& params.internal_policy_path.is_none()
896954
{
897-
return Err(Error::SpendingPolicyRequired(KeychainKind::Internal));
955+
return Err(Error::SpendingPolicyRequired(KeychainKind::Internal).into());
898956
};
899957
}
900958

@@ -923,13 +981,14 @@ impl<D> Wallet<D> {
923981

924982
let version = match params.version {
925983
Some(tx_builder::Version(0)) => {
926-
return Err(Error::Generic("Invalid version `0`".into()))
984+
return Err(Error::Generic("Invalid version `0`".into()).into())
927985
}
928986
Some(tx_builder::Version(1)) if requirements.csv.is_some() => {
929987
return Err(Error::Generic(
930988
"TxBuilder requested version `1`, but at least `2` is needed to use OP_CSV"
931989
.into(),
932-
))
990+
)
991+
.into())
933992
}
934993
Some(tx_builder::Version(x)) => x,
935994
None if requirements.csv.is_some() => 2,
@@ -971,7 +1030,7 @@ impl<D> Wallet<D> {
9711030
// Specific nLockTime required and it's compatible with the constraints
9721031
Some(x) if requirements.timelock.unwrap().is_same_unit(x) && x >= requirements.timelock.unwrap() => x,
9731032
// Invalid nLockTime required
974-
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())))
1033+
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())
9751034
};
9761035

9771036
let n_sequence = match (params.rbf, requirements.csv) {
@@ -991,7 +1050,8 @@ impl<D> Wallet<D> {
9911050
(Some(tx_builder::RbfValue::Value(rbf)), _) if !rbf.is_rbf() => {
9921051
return Err(Error::Generic(
9931052
"Cannot enable RBF with a nSequence >= 0xFFFFFFFE".into(),
994-
))
1053+
)
1054+
.into())
9951055
}
9961056
// RBF with a specific value requested, but the value is incompatible with CSV
9971057
(Some(tx_builder::RbfValue::Value(rbf)), Some(csv))
@@ -1000,7 +1060,8 @@ impl<D> Wallet<D> {
10001060
return Err(Error::Generic(format!(
10011061
"Cannot enable RBF with nSequence `{:?}` given a required OP_CSV of `{:?}`",
10021062
rbf, csv
1003-
)))
1063+
))
1064+
.into())
10041065
}
10051066

10061067
// RBF enabled with the default value with CSV also enabled. CSV takes precedence
@@ -1021,7 +1082,8 @@ impl<D> Wallet<D> {
10211082
if *fee < previous_fee.absolute {
10221083
return Err(Error::FeeTooLow {
10231084
required: previous_fee.absolute,
1024-
});
1085+
}
1086+
.into());
10251087
}
10261088
}
10271089
(FeeRate::from_sat_per_vb(0.0), *fee)
@@ -1032,7 +1094,8 @@ impl<D> Wallet<D> {
10321094
if *rate < required_feerate {
10331095
return Err(Error::FeeRateTooLow {
10341096
required: required_feerate,
1035-
});
1097+
}
1098+
.into());
10361099
}
10371100
}
10381101
(*rate, 0)
@@ -1047,7 +1110,7 @@ impl<D> Wallet<D> {
10471110
};
10481111

10491112
if params.manually_selected_only && params.utxos.is_empty() {
1050-
return Err(Error::NoUtxosSelected);
1113+
return Err(Error::NoUtxosSelected.into());
10511114
}
10521115

10531116
// we keep it as a float while we accumulate it, and only round it at the end
@@ -1061,7 +1124,7 @@ impl<D> Wallet<D> {
10611124
&& value.is_dust(script_pubkey)
10621125
&& !script_pubkey.is_provably_unspendable()
10631126
{
1064-
return Err(Error::OutputBelowDustLimit(index));
1127+
return Err(Error::OutputBelowDustLimit(index).into());
10651128
}
10661129

10671130
if self.is_mine(script_pubkey) {
@@ -1096,7 +1159,8 @@ impl<D> Wallet<D> {
10961159
{
10971160
return Err(Error::Generic(
10981161
"The `change_policy` can be set only if the wallet has a change_descriptor".into(),
1099-
));
1162+
)
1163+
.into());
11001164
}
11011165

11021166
let (required_utxos, optional_utxos) = self.preselect_utxos(
@@ -1122,7 +1186,7 @@ impl<D> Wallet<D> {
11221186
.stage(ChangeSet::from(indexed_tx_graph::ChangeSet::from(
11231187
index_changeset,
11241188
)));
1125-
self.persist.commit().expect("TODO");
1189+
self.persist.commit().map_err(CreateTxError::Persist)?;
11261190
spk
11271191
}
11281192
};
@@ -1166,10 +1230,11 @@ impl<D> Wallet<D> {
11661230
return Err(Error::InsufficientFunds {
11671231
needed: *dust_threshold,
11681232
available: remaining_amount.saturating_sub(*change_fee),
1169-
});
1233+
}
1234+
.into());
11701235
}
11711236
} else {
1172-
return Err(Error::NoRecipients);
1237+
return Err(Error::NoRecipients.into());
11731238
}
11741239
}
11751240

@@ -1216,6 +1281,8 @@ impl<D> Wallet<D> {
12161281
/// # use std::str::FromStr;
12171282
/// # use bitcoin::*;
12181283
/// # use bdk::*;
1284+
/// # use bdk::wallet::{ChangeSet, CreateTxError};
1285+
/// # use bdk_chain::PersistBackend;
12191286
/// # let descriptor = "wpkh(tpubD6NzVbkrYhZ4Xferm7Pz4VnjdcDPFyjVu5K4iZXQ4pVN8Cks4pHVowTBXBKRhX64pkRyJZJN5xAKj4UDNnLPb5p2sSKXhewoYx5GbTdUFWq/*)";
12201287
/// # let mut wallet = doctest_wallet!();
12211288
/// # let to_address = Address::from_str("2N4eQYCbKUHCCTUjBJeHcJp9ok6J2GZsTDt").unwrap().assume_checked();
@@ -1239,7 +1306,7 @@ impl<D> Wallet<D> {
12391306
/// let _ = wallet.sign(&mut psbt, SignOptions::default())?;
12401307
/// let fee_bumped_tx = psbt.extract_tx();
12411308
/// // broadcast fee_bumped_tx to replace original
1242-
/// # Ok::<(), bdk::Error>(())
1309+
/// # Ok::<(), CreateTxError<<() as PersistBackend<ChangeSet>>::WriteError>>(())
12431310
/// ```
12441311
// TODO: support for merging multiple transactions while bumping the fees
12451312
pub fn build_fee_bump(
@@ -1386,6 +1453,8 @@ impl<D> Wallet<D> {
13861453
/// # use std::str::FromStr;
13871454
/// # use bitcoin::*;
13881455
/// # use bdk::*;
1456+
/// # use bdk::wallet::{ChangeSet, CreateTxError};
1457+
/// # use bdk_chain::PersistBackend;
13891458
/// # let descriptor = "wpkh(tpubD6NzVbkrYhZ4Xferm7Pz4VnjdcDPFyjVu5K4iZXQ4pVN8Cks4pHVowTBXBKRhX64pkRyJZJN5xAKj4UDNnLPb5p2sSKXhewoYx5GbTdUFWq/*)";
13901459
/// # let mut wallet = doctest_wallet!();
13911460
/// # let to_address = Address::from_str("2N4eQYCbKUHCCTUjBJeHcJp9ok6J2GZsTDt").unwrap().assume_checked();
@@ -1396,7 +1465,7 @@ impl<D> Wallet<D> {
13961465
/// };
13971466
/// let finalized = wallet.sign(&mut psbt, SignOptions::default())?;
13981467
/// assert!(finalized, "we should have signed all the inputs");
1399-
/// # Ok::<(), bdk::Error>(())
1468+
/// # Ok::<(), CreateTxError<<() as PersistBackend<ChangeSet>>::WriteError>>(())
14001469
pub fn sign(
14011470
&self,
14021471
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)