Skip to content

Commit

Permalink
Merge #1387: fix(wallet): remove the generic from wallet
Browse files Browse the repository at this point in the history
e51af49 fix(wallet): remove generic from wallet (Rob N)

Pull request description:

  ### Description

  The `PersistenceBackend` uses generics to describe errors returned while applying the change set to the persistence layer. This change removes generics wherever possible and introduces a new public error enum. Removing the generics from `PersistenceBackend` errors is the first step towards #1363

  *Update*: I proceeded with removing the generics from `Wallet` by introducing a `Box<dyn PersistenceBackend>` .

  ### Notes to the reviewers

  This one sort of blew up in the number of changes due to the use of generics for most of the `Wallet` error variants. The generics were only used for the persistence errors, so I removed the generics from higher level errors whenever possible. The error variants of `PersistenceBackend` may also be more expressive, but I will level that up for discussion and make any changes required.

  ### Changelog notice

  - Changed `PersistenceBackend` errors to depend on the `anyhow` crate.
  - Remove the generic `T` from `Wallet`

  ### Checklists

  #### All Submissions:

  * [x] I've signed all my commits
  * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
  * [x] I ran `cargo fmt` and `cargo clippy` before committing

  #### New Features:

  * [ ] I've added tests for the new feature
  * [x] I've added docs for the new feature

  #### Bugfixes:

  * [x] This pull request breaks the existing API
  * [ ] I've added tests to reproduce the issue which are now passing
  * [x] I'm linking the issue being fixed by this PR

ACKs for top commit:
  evanlinjin:
    ACK e51af49

Tree-SHA512: 8ce4f1c495310e16145555f4a6a29a0f42cf8944eda68004595c3532580767f64f779185022147a00d75001c40d69fdf8f8de2d348eb68484b170d2a181117ff
  • Loading branch information
evanlinjin committed Apr 20, 2024
2 parents 52f3955 + e51af49 commit f00de9e
Show file tree
Hide file tree
Showing 10 changed files with 157 additions and 234 deletions.
1 change: 1 addition & 0 deletions crates/bdk/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ edition = "2021"
rust-version = "1.63"

[dependencies]
anyhow = { version = "1", default-features = false }
rand = "^0.8"
miniscript = { version = "11.0.0", features = ["serde"], default-features = false }
bitcoin = { version = "0.31.0", features = ["serde", "base64", "rand-std"], default-features = false }
Expand Down
25 changes: 11 additions & 14 deletions crates/bdk/src/wallet/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,11 @@ impl std::error::Error for MiniscriptPsbtError {}
/// Error returned from [`TxBuilder::finish`]
///
/// [`TxBuilder::finish`]: crate::wallet::tx_builder::TxBuilder::finish
pub enum CreateTxError<P> {
pub enum CreateTxError {
/// There was a problem with the descriptors passed in
Descriptor(DescriptorError),
/// We were unable to write wallet data to the persistence backend
Persist(P),
/// We were unable to load wallet data from or write wallet data to the persistence backend
Persist(anyhow::Error),
/// There was a problem while extracting and manipulating policies
Policy(PolicyError),
/// Spending policy is not compatible with this [`KeychainKind`]
Expand Down Expand Up @@ -119,17 +119,14 @@ pub enum CreateTxError<P> {
MiniscriptPsbt(MiniscriptPsbtError),
}

impl<P> fmt::Display for CreateTxError<P>
where
P: fmt::Display,
{
impl fmt::Display for CreateTxError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Self::Descriptor(e) => e.fmt(f),
Self::Persist(e) => {
write!(
f,
"failed to write wallet data to persistence backend: {}",
"failed to load wallet data from or write wallet data to persistence backend: {}",
e
)
}
Expand Down Expand Up @@ -214,38 +211,38 @@ where
}
}

impl<P> From<descriptor::error::Error> for CreateTxError<P> {
impl From<descriptor::error::Error> for CreateTxError {
fn from(err: descriptor::error::Error) -> Self {
CreateTxError::Descriptor(err)
}
}

impl<P> From<PolicyError> for CreateTxError<P> {
impl From<PolicyError> for CreateTxError {
fn from(err: PolicyError) -> Self {
CreateTxError::Policy(err)
}
}

impl<P> From<MiniscriptPsbtError> for CreateTxError<P> {
impl From<MiniscriptPsbtError> for CreateTxError {
fn from(err: MiniscriptPsbtError) -> Self {
CreateTxError::MiniscriptPsbt(err)
}
}

impl<P> From<psbt::Error> for CreateTxError<P> {
impl From<psbt::Error> for CreateTxError {
fn from(err: psbt::Error) -> Self {
CreateTxError::Psbt(err)
}
}

impl<P> From<coin_selection::Error> for CreateTxError<P> {
impl From<coin_selection::Error> for CreateTxError {
fn from(err: coin_selection::Error) -> Self {
CreateTxError::CoinSelection(err)
}
}

#[cfg(feature = "std")]
impl<P: core::fmt::Display + core::fmt::Debug> std::error::Error for CreateTxError<P> {}
impl std::error::Error for CreateTxError {}

#[derive(Debug)]
/// Error returned from [`Wallet::build_fee_bump`]
Expand Down
9 changes: 4 additions & 5 deletions crates/bdk/src/wallet/export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,8 @@
//! # Ok::<_, Box<dyn std::error::Error>>(())
//! ```

use core::str::FromStr;

use alloc::string::{String, ToString};
use core::str::FromStr;
use serde::{Deserialize, Serialize};

use miniscript::descriptor::{ShInner, WshInner};
Expand Down Expand Up @@ -110,8 +109,8 @@ impl FullyNodedExport {
///
/// If the database is empty or `include_blockheight` is false, the `blockheight` field
/// returned will be `0`.
pub fn export_wallet<D>(
wallet: &Wallet<D>,
pub fn export_wallet(
wallet: &Wallet,
label: &str,
include_blockheight: bool,
) -> Result<Self, &'static str> {
Expand Down Expand Up @@ -225,7 +224,7 @@ mod test {
descriptor: &str,
change_descriptor: Option<&str>,
network: Network,
) -> Wallet<()> {
) -> Wallet {
let mut wallet = Wallet::new_no_persist(descriptor, change_descriptor, network).unwrap();
let transaction = Transaction {
input: vec![],
Expand Down
Loading

0 comments on commit f00de9e

Please sign in to comment.