Skip to content

Rebase on top of master#4

Merged
icota merged 24 commits into
foundation-devfrom
bump-bdk
Jul 16, 2025
Merged

Rebase on top of master#4
icota merged 24 commits into
foundation-devfrom
bump-bdk

Conversation

@icota
Copy link
Copy Markdown
Collaborator

@icota icota commented Jul 15, 2025

Description

Notes to the reviewers

Changelog notice

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing

New Features:

  • I've added tests for the new feature
  • I've added docs for the new feature

Bugfixes:

  • 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

ValuedMammal and others added 24 commits June 4, 2025 17:33
9dff4e8 chore: bump dev version to 2.1.0-alpha.0 (valued mammal)

Pull request description:

  PR to update the master dev version to 2.1.0-alpha.0.

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

ACKs for top commit:
  notmandatory:
    ACK 9dff4e8
  thunderbiscuit:
    ACK 9dff4e8.
  oleonardolima:
    ACK 9dff4e8

Tree-SHA512: e21f18dc436b78801af080bc57dbb6d77402a1092fda108ca2713614a56eaa868878f846210239161012aad2bcb4b400b5c9dfc15a25aa28ed5fcba1654653be
Set to current largest size enum and error sizes.
1c1b1cd ci(clippy): allow larger enum variant size and errors for rustc 1.87 (Steve Myers)
b8f331d ci: automated update to rustc 1.87.0 (Github Action)

Pull request description:

  Automated update to Github CI workflow `cont_integration.yml` by [create-pull-request](https://github.com/peter-evans/create-pull-request) GitHub action

Top commit has no ACKs.

Tree-SHA512: 1a4a3ba6939958238fde79a5a721a5b760b0730d804afce9e20db8104bf9d82cfffa504323303685deecea60ef7b704c13d65151372953b3c1e89f5d72b8a659
Starting with commit `f6fd9853e`, `bdk_wallet` makes use of
`bitcoin::key::TweakedKeyPair::to_keypair` which was introduced with
v0.32.6. Here, we adjust the minimum API requirement in `Cargo.toml`, as
BDK 2.0.0 builds could otherwise fail with:

```
error[E0599]: no method named `to_keypair` found for struct `TweakedKeypair` in the current scope
   --> /home/tnull/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/bdk_wallet-2.0.0/src/wallet/signer.rs:580:14
    |
578 |           None => keypair
    |  _________________-
579 | |             .tap_tweak(secp, psbt_input.tap_merkle_root)
580 | |             .to_keypair(),
    | |             -^^^^^^^^^^ method not found in `TweakedKeypair`
    | |_____________|
    |

For more information about this error, try `rustc --explain E0599`.
```
…32.6 to reflect used API

af792ea Bump `bitcoin` dependency requirement to v0.32.6 to reflect used API (Elias Rohrer)

Pull request description:

  ### Description

  Starting with commit `f6fd9853e`, `bdk_wallet` makes use of `bitcoin::key::TweakedKeyPair::to_keypair` which was introduced with v0.32.6. Here, we adjust the minimum API requirement in `Cargo.toml`, as BDK 2.0.0 builds could otherwise fail with:

  ```
  error[E0599]: no method named `to_keypair` found for struct `TweakedKeypair` in the current scope
     --> /home/tnull/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/bdk_wallet-2.0.0/src/wallet/signer.rs:580:14
      |
  578 |           None => keypair
      |  _________________-
  579 | |             .tap_tweak(secp, psbt_input.tap_merkle_root)
  580 | |             .to_keypair(),
      | |             -^^^^^^^^^^ method not found in `TweakedKeypair`
      | |_____________|
      |

  For more information about this error, try `rustc --explain E0599`.
  ```

ACKs for top commit:
  ValuedMammal:
    reACK af792ea

Tree-SHA512: 549334a8959dd985045d432003edc6f7ac3525916bcd1dcc8544aa39b25a68f1caa175f9ffbc82c37a4155f231179fb4eadce9cfab284eb38e74fd2fa007d6c6
789c77b feat: add `justfile` (Luis Schwab)

Pull request description:

  ### Description

  Closes bitcoindevkit#240.

  This PR adds a `justfile`, updates the PR template to use it, and adds a section on the `README.md` to show available recipes.

  These are the implemented recipes:
  ```justfile
  alias b := build
  alias c := check
  alias f := fmt
  alias t := test
  alias p := pre-push

  build:
      cargo build

  check:
      cargo +nightly fmt --all -- --check
      cargo check --workspace --exclude 'example_*' --all-features
      cargo clippy --all-features --all-targets -- -D warnings
      @[ "$(git log --pretty='format:%G?' -1 HEAD)" = "N" ] && \
          echo "\n⚠️  Unsigned commit: BDK requires that commits be signed." || \
          true

  fmt:
      cargo +nightly fmt

  test:
      cargo test --workspace --exclude 'example_*' --all-features

  pre-push: fmt check test
  ```

  `check` will verify if `HEAD` was signed and echo that warning if not. It does not check all commits, but I think that checking only the last is a pretty good heuristic (who only signs the last commit?).

  Before pushing, one only needs to run `just p`.

  ### 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 `just p` before pushing

ACKs for top commit:
  notmandatory:
    ACK 789c77b

Tree-SHA512: deead107f961b05cb7d790cbd205d0bc8d81ec7a1be4d43bf69eddedb126789535ecdb893c9aacf03f2a8246942654668e75beb35b445122d7b48fdc99a899a3
These are convenience methods on `TxBuilder` that can be used to filter
out outpoints which do not meet a given confirmation threshold.
When TxBuilder::ordering is TxOrdering::Untouched, the insertion order
of recipients and manually selected UTxOs should be preserved in
transaction's output and input vectors respectively.

Fixes bitcoindevkit#244
…estFirstCoinSelection

345cc6e fix(coin_selection): prefer Utxo::Local over Utxo::Foreign in OldestFirstCoinSelection (nymius)

Pull request description:

  ### Description

  The comments in the `OldestFirstCoinSelection` implementation stated the following:
  > // For utxo that doesn't exist in DB, they will have lowest priority to be selected

  But this was not honored in the code.

  This PR enforces this and ensures the expected behaviour through the two following new tests:
  - `test_oldest_first_coin_selection_uses_all_optional_with_foreign_utxo_locals_sorted_first`
  - `test_oldest_first_coin_selection_uses_only_all_optional_local_utxos_not_a_single_foreign`

  Fixes bitcoindevkit#264

  ### Changelog notice

  No public APIs are changed by these commits.

  ### Checklists

  > [!IMPORTANT]
  > This pull request **DOES NOT** break the existing API
  * [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 +nightly fmt` and `cargo clippy` before committing
  * [x] I've added tests for the new code
  * [x] I've expanded docs addressing new code
  * [x] 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:
  ValuedMammal:
    ACK 345cc6e
  oleonardolima:
    ACK 345cc6e

Tree-SHA512: 8a538e0765ca55f870fcbc33e4c865656e9074765a5bb13c370afe2626dbf3ecbb71e8a372607e1e405a9e35f26418bfdd9c71fec018999858a5fe5680af866b
…d utxos if `TxOrdering::Untouched`

0522114 test(tx_builder): update precedence check of local UTXOs over add_foreign_utxos (valued mammal)
73bef28 doc(tx_builder): add info about manually selected UTxOs priority (nymius)
3316236 fix(tx_builder): preserve insertion order with TxOrdering::Untouched (nymius)

Pull request description:

  ### Description

  On my attempt to fix bitcoindevkit/bdk#1794 in bitcoindevkit/bdk#1798, I broke the assumption that insertion order is preserved when `TxBuilder::ordering` is `TxOrdering::Untouched`. Some users are relying in this assumption, so here I'm trying to restore it back, without adding a new dependency for this single use case like bitcoindevkit#252, or creating a new struct just to track this.

  In this fourth alternative solution I'm going back to use `Vec` to store the manually selected UTxOs.

  I was reluctant to do it in this way because `HashMap` seems a better solution giving its property of avoiding duplicates, but as we also want to keep the secuential nature of the insertion of UTxOs in `TxBuilder`, here is an alternative aligned with that principle.

  May replace bitcoindevkit#252
  May replace bitcoindevkit#261 .
  Fixes bitcoindevkit#244

  ### Notes to the reviewers

  Also, as I was working on this, I came back to the following tests:
  - `test_prexisting_foreign_utxo_have_no_precedence_over_local_utxo_with_same_outpoint`
  - `test_prexisting_local_utxo_have_precedence_over_foreign_utxo_with_same_outpoint`

  Motivated during the implementation and review of bitcoindevkit/bdk#1798.

  Which required the underlying structure to also hold the properties of no duplication for manually selected UTxOs, as the structures were accessed directly for these cases.

  The test tries to cover the case when there are two wallets using the same descriptor, one tracks transactions and the other does not. The first passes UTxOs belonging to the second one and this one creates transactions using the `add_foreign_utxo` interface.
  In this case it was expected for any `LocalUtxo` of the offline wallet to supersede any conflicting foreign UTxO. But, the simulation of this case went against the borrowing constraints of rust.
  By how costly was to reproduce this behavior for me in the tests, I would like to have second opinions in the feasibility of the test case.

  ### Changelog notice

  No public APIs are changed by these commits.

  ### Checklists

  > [!IMPORTANT]
  > This pull request **DOES NOT** break the existing API
  * [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 +nightly fmt` and `cargo clippy` before committing
  * [x] I've added tests for the new code
  * [x] I've expanded docs addressing new code
  * [x] 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:
  ValuedMammal:
    reACK 0522114
  oleonardolima:
    ACK 0522114

Tree-SHA512: f2726d75eab83e28cc748ac5cd6bd0c7f3dddb409ac61baf7d0a7030ddf81c11b10dbd5b18e8ac3d29a6afb4b8f29ee9a88f83094aebec771fdb4da2cd718326
…_below_confirmations`

5a65422 feat: Add `exclude_unconfirmed` and `exclude_below_confirmations` (志宇)

Pull request description:

  Fixes bitcoindevkit#143

  ### Description

  These are convenience methods on `TxBuilder` that can be used to filter out outpoints which do not meet a given confirmation threshold.

  Previously, I advocated a feature freeze on `TxBuilder`, but I believe we can make an exception for these helper methods, as they do not broaden the `TxBuilder` interface’s error surface. Going forward, all other development should be directed to the [`bdk-tx`](https://github.com/bitcoindevkit/bdk-tx) repository and crate.

  cc. @tnull

  ### Changelog notice

  ```md
  Added:
  - `TxBuilder::exclude_unconfirmed`
  - `TxBuilder::exclude_below_confirmations`
  ```

  ### 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 +nightly fmt` and `cargo clippy` before committing

  #### New Features:

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

ACKs for top commit:
  ValuedMammal:
    ACK 5a65422
  oleonardolima:
    utACK 5a65422
  notmandatory:
    ACK 5a65422

Tree-SHA512: f458ce62cf377c847bf19e6ae458d9ff028d6a28b9ee8df14c4a0dfeb7004249ff522c3462dfb41f391ba3cf92cdd7445c28e58ea614676c9fcf2a09dce7ef67
ea39339 test: refactor wallet tests (valued mammal)

Pull request description:

  These files are added to wallet/tests directory

  - `add_foreign_utxo.rs`
  - `persisted_wallet.rs`
  - `build_fee_bump`.rs
  - `common.rs`

  fixes bitcoindevkit#238

  ### Checklists

  #### All Submissions:

  * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)

ACKs for top commit:
  tvpeter:
    tACK ea39339
  oleonardolima:
    ACK ea39339

Tree-SHA512: 50f63a369320fb8df240d78943044e402ab565eb742ccae09b7470150b611d1ab9be89c6838f154f9e90ca577d4f98e3af0a3d344e5b5fdbde674020b3494618
This is a fix to address the clippy lints

- `clippy::large_enum_variant`
- `clippy::result_large_error`

BREAKING: The types `LoadMismatch` and `CreateWithPersistError`
are changed as a result of the now boxed variants.
650c3ee fix!: `Box` large enum variants (valued mammal)

Pull request description:

  ### Description

  This is a fix to address clippy lints

  - `clippy::large_enum_variant`
  - `clippy::result_large_error`

  BREAKING: The types `LoadMismatch`, and `CreateWithPersistError` are changed as a result of the now boxed variants.

  fixes bitcoindevkit#245

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

  #### 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:
  evanlinjin:
    ACK 650c3ee

Tree-SHA512: 3634425499e3ddd401cf36c1c0519d19a2173a8c8414ea95e3195f9f6477f98b490899a57fb9c22195f6beaa2c4c3d9e00131524c195c88e8c25ad34e0033de0
@icota icota merged commit 58647e4 into foundation-dev Jul 16, 2025
12 of 40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants