Skip to content

Commit

Permalink
Merge bitcoindevkit#1763: Change coin_selection and DescriptorExt::du…
Browse files Browse the repository at this point in the history
…st_value to use Amount type

2a9eeed ci: pin msrv dep version for rustls (Steve Myers)
20789ec refactor(chain)!: use Amount for DescriptorExt::dust_value() (Steve Myers)
58a8435 refactor(coin_selection)!: use Amount and SignedAmount for API and internally (Steve Myers)

Pull request description:

  ### Description

  refactor(coin_selection)!: use Amount and SignedAmount for API and internally
  refactor(chain)!: use Amount for DescriptorExt::dust_value()

  Using named types make the API and internal code easier to read and reason about since it makes it clear that the values are bitcoin amounts. Also to create these types the units (ie .from_sat()) must be specified.

  ### Notes to the reviewers

  For coin_selection using Amount and SignedAmount makes internal code safer against overflow errors. In particular because these types will panic if an amount overflow occurs. Using u64/i64 on the other hand can silently rollover. See: https://doc.rust-lang.org/book/ch03-02-data-types.html#integer-overflow

  This is a continuation of the work done in bitcoindevkit#1595.  Since this is an API breaking change I would like to include it in the 1.0.0-beta milestone if possible.

  ### Changelog notice

  - Change coin_selection to use Amount instead of u64 for all bitcoin amounts.
  - Change DescriptorExt::dust_value() to return an Amount instead of u64.

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

ACKs for top commit:
  ValuedMammal:
    ACK 2a9eeed

Tree-SHA512: 9dd1b31d0f8d3d8c383c7aae7ec0fffb55bfcfe49c804e273faa740d30efde7efb7c9504e87cceb56798ea14a3e34fc8a7b65a8ad5e52ea38b8523150c9b6bc2
  • Loading branch information
notmandatory committed Dec 11, 2024
2 parents 955593c + 2a9eeed commit 49d6477
Show file tree
Hide file tree
Showing 7 changed files with 227 additions and 192 deletions.
1 change: 1 addition & 0 deletions .github/workflows/cont_integration.yml
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ jobs:
cargo update -p security-framework-sys --precise "2.11.1"
cargo update -p csv --precise "1.3.0"
cargo update -p unicode-width --precise "0.1.13"
cargo update -p [email protected] --precise "0.23.19"
- name: Build
run: cargo build --workspace --exclude 'example_*' ${{ matrix.features }}
- name: Test
Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ cargo update -p indexmap --precise "2.5.0"
cargo update -p security-framework-sys --precise "2.11.1"
cargo update -p csv --precise "1.3.0"
cargo update -p unicode-width --precise "0.1.13"
cargo update -p [email protected] --precise "0.23.19"
```

## License
Expand Down
8 changes: 4 additions & 4 deletions crates/chain/src/descriptor_ext.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::miniscript::{Descriptor, DescriptorPublicKey};
use bitcoin::hashes::{hash_newtype, sha256, Hash};
use bitcoin::Amount;

hash_newtype! {
/// Represents the unique ID of a descriptor.
Expand All @@ -13,22 +14,21 @@ hash_newtype! {

/// A trait to extend the functionality of a miniscript descriptor.
pub trait DescriptorExt {
/// Returns the minimum value (in satoshis) at which an output is broadcastable.
/// Returns the minimum [`Amount`] at which an output is broadcast-able.
/// Panics if the descriptor wildcard is hardened.
fn dust_value(&self) -> u64;
fn dust_value(&self) -> Amount;

/// Returns the descriptor ID, calculated as the sha256 hash of the spk derived from the
/// descriptor at index 0.
fn descriptor_id(&self) -> DescriptorId;
}

impl DescriptorExt for Descriptor<DescriptorPublicKey> {
fn dust_value(&self) -> u64 {
fn dust_value(&self) -> Amount {
self.at_derivation_index(0)
.expect("descriptor can't have hardened derivation")
.script_pubkey()
.minimal_non_dust()
.to_sat()
}

fn descriptor_id(&self) -> DescriptorId {
Expand Down
Loading

0 comments on commit 49d6477

Please sign in to comment.