Skip to content

Commit

Permalink
Merge #1028: Add CreateTxError and use as error type for TxBuilder::f…
Browse files Browse the repository at this point in the history
…inish()

00ec19e ci: fix MSRV pinning for rustls 0.21.9 (Steve Myers)
77f9977 feat(wallet): Add infallible Wallet get_address(), get_internal_address functions (Steve Myers)
9e7d99e refactor(bdk)!: add context specific error types, remove top level error mod (Steve Myers)

Pull request description:

  ### Description

  To remove some places where there were `.expect("TODO")` I added a new `CreateTxError` type which is returned from `TxBuilder::finish()`. I also updated related tests and doc tests.

  Fixes #996 (comment)

  Also added fallible `Wallet::try_get_address()` and `try_get_internal_address()`  to return `Result` with a possible `D:WriteError` when a PersistBackend is used. This should fix #996.

  I removed catch-all bdk::Error and replaced usages with new types and updated related functions, fixes #994.

  ### Notes to the reviewers

  ~~I didn't add all possible bdk::Error types that `Wallet::create_tx()` and `TxBuilder::finish()` functions might throw. It's probably not too much more work but will take a bit more research so I want to make sure this is the right general approach first.~~

  I added `anyhow` to the dev-dependencies so I could remove some `.expect()` lines from the docs tests and make the examples closer to what an end user should do.  I also used the `anyhow!()` macro to replace a few places that were using the `bdk::Error::Generic` in example code.

  I also moved the module level error.rs file to wallet/error.rs so no one would be tempted to make any new catch all errors and to make it clear that all the errors in it are wallet module related.

  ### Changelog notice

  Changed

  - Updated bdk module to use new context specific error types
    - wallet: MiniscriptPsbtError, CreateTxError, BuildFeeBumpError error enums
    - coin_selection: module Error enum
  - Renamed fallible Wallet address functions to try_get_address() and try_get_internal_address()

  Removed

  - Removed catch-all top level bdk::Error enum
  - Removed impl_error macro

  Added

  -  Added infallible Wallet get_address(), get_internal_address functions

  ### 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

  #### Bugfixes:

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

Top commit has no ACKs.

Tree-SHA512: a87c0856d71f9c945d12b6de6d368f49bd62d73886ac46ac83d00ddb81f2c38c5233ba053e40c76dea73ee7bfc19dac510eec5d7c9026ae50a2dc7308ac4786f
  • Loading branch information
notmandatory committed Nov 16, 2023
2 parents cc552c5 + 00ec19e commit 46d39be
Show file tree
Hide file tree
Showing 22 changed files with 720 additions and 369 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/cont_integration.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ jobs:
run: |
cargo update -p log --precise "0.4.18"
cargo update -p tempfile --precise "3.6.0"
cargo update -p rustls:0.21.8 --precise "0.21.1"
cargo update -p rustls:0.21.9 --precise "0.21.1"
cargo update -p rustls:0.20.9 --precise "0.20.8"
cargo update -p tokio --precise "1.29.1"
cargo update -p tokio-util --precise "0.7.8"
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ cargo update -p log --precise "0.4.18"
# tempfile 3.7.0 has MSRV 1.63.0+
cargo update -p tempfile --precise "3.6.0"
# rustls 0.21.7 has MSRV 1.60.0+
cargo update -p rustls:0.21.8 --precise "0.21.1"
cargo update -p rustls:0.21.9 --precise "0.21.1"
# rustls 0.20.9 has MSRV 1.60.0+
cargo update -p rustls:0.20.9 --precise "0.20.8"
# tokio 1.33 has MSRV 1.63.0+
Expand Down
1 change: 1 addition & 0 deletions crates/bdk/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ env_logger = "0.7"
assert_matches = "1.5.0"
tempfile = "3"
bdk_file_store = { path = "../file_store" }
anyhow = "1"

[package.metadata.docs.rs]
all-features = true
Expand Down
7 changes: 3 additions & 4 deletions crates/bdk/examples/mnemonic_to_descriptors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
// You may not use this file except in accordance with one or both of these
// licenses.

use anyhow::anyhow;
use bdk::bitcoin::bip32::DerivationPath;
use bdk::bitcoin::secp256k1::Secp256k1;
use bdk::bitcoin::Network;
Expand All @@ -14,21 +15,19 @@ use bdk::descriptor::IntoWalletDescriptor;
use bdk::keys::bip39::{Language, Mnemonic, WordCount};
use bdk::keys::{GeneratableKey, GeneratedKey};
use bdk::miniscript::Tap;
use bdk::Error as BDK_Error;
use std::error::Error;
use std::str::FromStr;

/// This example demonstrates how to generate a mnemonic phrase
/// using BDK and use that to generate a descriptor string.
fn main() -> Result<(), Box<dyn Error>> {
fn main() -> Result<(), anyhow::Error> {
let secp = Secp256k1::new();

// In this example we are generating a 12 words mnemonic phrase
// but it is also possible generate 15, 18, 21 and 24 words
// using their respective `WordCount` variant.
let mnemonic: GeneratedKey<_, Tap> =
Mnemonic::generate((WordCount::Words12, Language::English))
.map_err(|_| BDK_Error::Generic("Mnemonic generation error".to_string()))?;
.map_err(|_| anyhow!("Mnemonic generation error"))?;

println!("Mnemonic phrase: {}", *mnemonic);
let mnemonic_with_passphrase = (mnemonic, None);
Expand Down
42 changes: 35 additions & 7 deletions crates/bdk/src/descriptor/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
// licenses.

//! Descriptor errors

use core::fmt;

/// Errors related to the parsing and usage of descriptors
Expand Down Expand Up @@ -87,9 +86,38 @@ impl fmt::Display for Error {
#[cfg(feature = "std")]
impl std::error::Error for Error {}

impl_error!(bitcoin::bip32::Error, Bip32);
impl_error!(bitcoin::base58::Error, Base58);
impl_error!(bitcoin::key::Error, Pk);
impl_error!(miniscript::Error, Miniscript);
impl_error!(bitcoin::hashes::hex::Error, Hex);
impl_error!(crate::descriptor::policy::PolicyError, Policy);
impl From<bitcoin::bip32::Error> for Error {
fn from(err: bitcoin::bip32::Error) -> Self {
Error::Bip32(err)
}
}

impl From<bitcoin::base58::Error> for Error {
fn from(err: bitcoin::base58::Error) -> Self {
Error::Base58(err)
}
}

impl From<bitcoin::key::Error> for Error {
fn from(err: bitcoin::key::Error) -> Self {
Error::Pk(err)
}
}

impl From<miniscript::Error> for Error {
fn from(err: miniscript::Error) -> Self {
Error::Miniscript(err)
}
}

impl From<bitcoin::hashes::hex::Error> for Error {
fn from(err: bitcoin::hashes::hex::Error) -> Self {
Error::Hex(err)
}
}

impl From<crate::descriptor::policy::PolicyError> for Error {
fn from(err: crate::descriptor::policy::PolicyError) -> Self {
Error::Policy(err)
}
}
3 changes: 2 additions & 1 deletion crates/bdk/src/descriptor/policy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,14 @@
//! let signers = Arc::new(SignersContainer::build(key_map, &extended_desc, &secp));
//! let policy = extended_desc.extract_policy(&signers, BuildSatisfaction::None, &secp)?;
//! println!("policy: {}", serde_json::to_string(&policy).unwrap());
//! # Ok::<(), bdk::Error>(())
//! # Ok::<(), anyhow::Error>(())
//! ```

use crate::collections::{BTreeMap, HashSet, VecDeque};
use alloc::string::String;
use alloc::vec::Vec;
use core::cmp::max;

use core::fmt;

use serde::ser::SerializeMap;
Expand Down
201 changes: 0 additions & 201 deletions crates/bdk/src/error.rs

This file was deleted.

15 changes: 12 additions & 3 deletions crates/bdk/src/keys/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,7 @@ impl<Ctx: ScriptContext> From<bip32::ExtendedPrivKey> for ExtendedKey<Ctx> {
/// }
/// ```
///
/// Types that don't internally encode the [`Network`](bitcoin::Network) in which they are valid need some extra
/// Types that don't internally encode the [`Network`] in which they are valid need some extra
/// steps to override the set of valid networks, otherwise only the network specified in the
/// [`ExtendedPrivKey`] or [`ExtendedPubKey`] will be considered valid.
///
Expand Down Expand Up @@ -932,8 +932,17 @@ pub enum KeyError {
Miniscript(miniscript::Error),
}

impl_error!(miniscript::Error, Miniscript, KeyError);
impl_error!(bitcoin::bip32::Error, Bip32, KeyError);
impl From<miniscript::Error> for KeyError {
fn from(err: miniscript::Error) -> Self {
KeyError::Miniscript(err)
}
}

impl From<bip32::Error> for KeyError {
fn from(err: bip32::Error) -> Self {
KeyError::Bip32(err)
}
}

impl fmt::Display for KeyError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
Expand Down
4 changes: 0 additions & 4 deletions crates/bdk/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,6 @@ extern crate serde_json;
#[cfg(feature = "keys-bip39")]
extern crate bip39;

#[allow(unused_imports)]
#[macro_use]
pub(crate) mod error;
pub mod descriptor;
pub mod keys;
pub mod psbt;
Expand All @@ -38,7 +35,6 @@ pub mod wallet;

pub use descriptor::template;
pub use descriptor::HdKeyPaths;
pub use error::Error;
pub use types::*;
pub use wallet::signer;
pub use wallet::signer::SignOptions;
Expand Down
Loading

0 comments on commit 46d39be

Please sign in to comment.