Test the bitcoin v0.32.0-rc1 release
#1400
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Upgrade: - bitcoin to v0.31.0 - miniscript to v11.0.0 Note: The bitcoin upgrade includes improvements to the `Transaction::weight()` function, it appears those guys did good, we no longer need to add the 2 additional weight units "just in case".
apoelstra
reviewed
Apr 10, 2024
crates/bdk/src/descriptor/policy.rs
Outdated
| sorted: bool, | ||
| secp: &SecpCtx, | ||
| ) -> Result<Option<Policy>, PolicyError> { | ||
| if threshold == 0 { | ||
| if threshold.k() == 0 { |
There was a problem hiding this comment.
This condition is impossible with the new threshold type.
There was a problem hiding this comment.
Removed - two instances.
Test the latest bitcoin release candidate. Includes bumping the version numbers so we can use this branch to test crates further up the stack.
|
@tcharding Are you planning on continuing working on this PR towards merge ? I'm asking just to know if I should start working on another one (on top of this one) on behalf of #1422 :) |
|
I"d love it if you took over. Feel free to use any of my work, change it, or drop it as you see fit. No attribution needed. FTR we need to get an updated version of |
Great, I'll do it then. Thanks, I'll follow the discussions on the PRs (rust-bitcoin/rust-bitcoincore-rpc#345 rust-bitcoin/rust-miniscript#679 repositories closely then. |
|
Hey @oleonardolima you should be good to go after this one goes in: rust-bitcoin/rust-miniscript#688 |
8 tasks
notmandatory
added a commit
that referenced
this pull request
Jun 13, 2024
…ipt` to `0.12.0` and others 1120081 chore(wallet): rm dup code (志宇) 2a45640 deps(bdk): bump `bitcoin` to `0.32.0`, miniscript to `12.0.0` (Leonardo Lima) Pull request description: fixes #1422 <!-- You can erase any parts of this template not applicable to your Pull Request. --> ### Description This PR focuses on upgrading the `rust-bitcoin` and `miniscript` versions, to `0.32.0` and `0.12.0`, respectively. It also bumps the versions of other BDK ecosystem crates that also rely on both `rust-bitcoin` and `miniscript`, being: - electrum-client bitcoindevkit/rust-electrum-client#133 - esplora-client bitcoindevkit/rust-esplora-client#85 - hwi bitcoindevkit/rust-hwi#99 <ins>I structured the PR in multiple commits, with closed scope, being one for each BDK crate being upgraded, and one for each kind of fix and upgrade required, it seems like a lot of commits (**that should be squashed before merging**), but I think it'll make it easier during review phase.</ins> In summary I can already mention some of the main changes: - using `compute_txid()` instead of deprecated `txid()` - using `minimal_non_dust()` instead of `dust_value()` - using the renamed `signature` and `sighash_type` fields - using proper `sighash::P2wpkhError`, `sighash::TaprootError` instead of older `sighash::Error` - conversion from `Network` to new expected `NetworkKind` #1465 - conversion from the new `Weight` type to current expected `usize` #1466 - using `.into()` to convert from AbsLockTime and `RelLockTime` to `absolute::LockTime` and `relative::LockTime` - using Message::from_digest() instead of relying on deprecated `ThirtyTwoByteHash` trait. - updating the miniscript policy and dsl to proper expect and handle new `Threshold` type, instead of the previous two parameters. <!-- Describe the purpose of this PR, what's being adding and/or fixed --> ### Notes to the reviewers <ins>Again, I structured the PR in multiple commits, with closed scope, being one for each BDK crate being upgraded, and one for each kind of fix and upgrade required, it seems like a lot of commits (**that should be squashed before merging**), but I think it'll make it easier during review phase.</ins> It should definitely be carefully reviewed, especially the last commits for the wallet crate scope, the ones with the semantic `fix(wallet)`. I would also appreciate if @tcharding reviewed it, as he gave a try in the past (#1400 ), and I relied on some of it for the policy part of it, other rust-bitcoin maintainers reviews are a definitely welcome 😁 <!-- In this section you can include notes directed to the reviewers, like explaining why some parts of the PR were done in a specific way --> ### Changelog notice > // TODO(@oleonardolima): Do another pass and double check the changes - Use `compute_txid()` instead of deprecated `txid()` - Use `minimal_non_dust()` instead of `dust_value()` - Use `signature` and `sighash_type` fields, instead of previous `sig` and `hash_type` - Use `sighash::P2wpkhError`, and `sighash::TaprootError` instead of older `sighash::Error` - Converts from `Network` to `NetworkKind`, where expected - Converts from `Weight` type to current used `usize` - Use `.into()` to convert from `AbsLockTime` and `RelLockTime` to `absolute::LockTime` and `relative::LockTime` - Remove use of deprecated `ThirtyTwoByteHash` trait, use `Message::from_digest()` - Update the miniscript policy and dsl macros to proper expect and handle new `Threshold` type, instead of the previous two parameters. <!-- Notice the release manager should include in the release tag message changelog --> <!-- See https://keepachangelog.com/en/1.0.0/ for examples --> ### 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 * [ ] 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 ACKs for top commit: notmandatory: ACK 1120081 Tree-SHA512: ba1ab64603b41014d3f0866d846167f77d31959ca6f1d9c3181d5e543964f5d772e05651d63935ba7bbffeba41a66868d27de4c32129739b9ca50f3bbaf9f2a1
|
replaced by #1448 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Please excuse the noise if this is not of interest to you guys right now. No action required from the
bdkteam - this is just informational and so therust-bitcointeam can see that bitcoin rc1 release is ok.Test the latest bitcoin release candidate. Includes an unreleased patched version of
rust-miniscript(masterbranch + depend on bitcoin rc release).Notes:
miniscript::Thresholdusage took me down a dark path for a while :)expectthat require more invasive error handling improvementsweight.to_wu() as usizeinstead of using the newWeighttype more thoroughlynetwork.into()liberally instead of using the newNetworkKindmore thoroughlySequence/RelLockTimestuff is a bit rough tooBut I believe this proves the rc1 release is pretty good as it is.
cargo test --color=always --workspace --all-features --all-targetspasses all tests on my machine.