chore(deps): upgrade rust-bitcoin to 0.32.0#99
Conversation
3ff002f to
f0f9bfd
Compare
da1b5e7 to
4fe5143
Compare
rust-bitcoin to 0.32.0rust-bitcoin to 0.32.0
4fe5143 to
8178fa7
Compare
|
@notmandatory Is the CI known for being flaky ? It seems to have a problem importing the external hwi library 🤔 |
d675ae2 to
c062eda
Compare
|
The good news is I confirmed this project CI was not working even without your changes, the bad news is I don't know how to fix it. I'm going to try getting all the emulators working on my local system and then figure out from there what needs to be updated. |
I've been trying that, I found that:
|
@notmandatory Also, if you manage to get the emulators running on macos easily let me know, I surrendered and I'm using containers/linux machines instead 😂 |
23f68f2 to
e3f622e
Compare
|
BTW, I was not able to get the emulators working locally on my mac, had to use the CI for testing. |
3b246dd to
7b0b04e
Compare
Yep, I did the same, not sure if it's because is macOS, or it's also hard to run on linux |
|
I merged #101, please rebase and then this one should be ready to go. Thanks! |
7b0b04e to
9eba3fd
Compare
9eba3fd to
be7933a
Compare
|
| path: &DerivationPath, | ||
| expert: bool, | ||
| ) -> Result<HWIExtendedPubKey, Error> { | ||
| let prefixed_path = format!("m/{}", path); |
There was a problem hiding this comment.
Drats, that turned out ugly didn't it. Maybe we should have a function that explicitly does this.
There was a problem hiding this comment.
Yes, a specific function for that would help.
There was a problem hiding this comment.
Including m/ is actually incorrect/misunderstanding of the BIP. Why did you keep the incorrect behavior?
There was a problem hiding this comment.
Hey @Kixunil , which BIP in particular do you mean?
Anyway this PR isn't meant to change how this lib works, only to bump the version of rust-bitcoin. If there's something we need to fix we can do it in another PR.
There was a problem hiding this comment.
32, see this issue: rust-bitcoin/rust-bitcoin#2451
However, I've just noticed this is still debated: rust-bitcoin/rust-bitcoin#2671
There was a problem hiding this comment.
Including
m/is actually incorrect/misunderstanding of the BIP. Why did you keep the incorrect behavior?
Hey @Kixunil , which BIP in particular do you mean?
Anyway this PR isn't meant to change how this lib works, only to bump the version of rust-bitcoin. If there's something we need to fix we can do it in another PR.
Yes, I kept it because it would require changes on the upstream hwilib, ACK's, and so on, as Steve mentioned not the scope of this PR.
Thanks, I'll keep an eye on the issue discussion, and update it in another PR if appropriate.
…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
…to `0.32.0` be7933a9f79f9a08cd0c2f0e44727c709efbe263 chore: bump version to `0.9.0` (Leonardo Lima) ede9ea751519703221bab4ea485ef5e205de3da1 fix: format `DerivationPath` w/ prefix `m/` when using as hwilib function argument (Leonardo Lima) 7afc15a1098b4b61feb2be4c0247cd758263c3b3 fix: use `compute_txid()` instead of deprecated `txid()` (Leonardo Lima) c15e1a2ece853a3154a9c73bf1a83de54f8ca43d chore: upgrade `miniscript` dependency to `0.12.0` (Leonardo Lima) be5b382ab3543b72bebc925a54e24811f4c94314 chore: upgrade `bitcoin` dependency to `0.32.0` (Leonardo Lima) Pull request description: <!-- You can erase any parts of this template not applicable to your Pull Request. --> `partially fixes` [#1422](bitcoindevkit/bdk#1422) ### Description It updates the `rust-bitcoin` dependency to `0.32.0`, and the `miniscript` to `0.12.0`. <!-- Describe the purpose of this PR, what's being adding and/or fixed --> ### Notes to the reviewers It's open for any comments. <!-- 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 - Update the `bitcoin` crate dependency to `0.32.0` - Update the `miniscript` crate dependency to `0.12.0` - Expliciltly format the `DerivationPath` with `m/` prefix before passing it as argument to HWI interface, the `fmt::Display` trait removed the `m/` prefix on `rust-bitcoin` `0.32.0`. <!-- 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 ACKs for top commit: notmandatory: ACK be7933a9f79f9a08cd0c2f0e44727c709efbe263 Tree-SHA512: 52bca723ae6e7dd9642c016d7881323be21d03bef05e9f9e21803e7fd5d8924c25d7a9731b4b3c61018648b4666e92ee1ca695ea8df8016f90a9446e8d64cefa
partially fixes#1422Description
It updates the
rust-bitcoindependency to0.32.0, and theminiscriptto0.12.0.Notes to the reviewers
It's open for any comments.
Changelog notice
bitcoincrate dependency to0.32.0miniscriptcrate dependency to0.12.0DerivationPathwithm/prefix before passing it as argument to HWI interface, thefmt::Displaytrait removed them/prefix onrust-bitcoin0.32.0.Checklists
All Submissions:
cargo fmtandcargo clippybefore committing