Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Upgrade bitcoin requirement from 0.28.1 to 0.29.1 #20

Merged
merged 1 commit into from
Oct 24, 2022
Merged

Upgrade bitcoin requirement from 0.28.1 to 0.29.1 #20

merged 1 commit into from
Oct 24, 2022

Conversation

danielgranhao
Copy link
Contributor

Upgrade bitcoin dependency to 0.29.1 to solve compatibility issues with LDK 0.0.111

@danielgranhao danielgranhao mentioned this pull request Oct 18, 2022
Merged
@coveralls
Copy link

coveralls commented Oct 18, 2022 •
edited
Loading

Pull Request Test Coverage Report for Build 3312108090

  • 1 of 3 (33.33%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 78.894%
Changes Missing Coverage Covered Lines Changed/Added Lines %
src/api.rs 0 2 0.0%
Totals Coverage Status
Change from base Build 3304381605: 0.0%
Covered Lines: 699
Relevant Lines: 886
💛 - Coveralls

@notmandatory notmandatory mentioned this pull request Oct 19, 2022
Closed
15 tasks
rajarshimaitra
rajarshimaitra approved these changes Oct 22, 2022
View reviewed changes
Copy link
Contributor

@rajarshimaitra rajarshimaitra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry fat finger over the close button..

tACK b047922

We also need this to use this lib in bdk_core..

@notmandatory notmandatory mentioned this pull request Oct 22, 2022
Merged
8 tasks
notmandatory
notmandatory approved these changes Oct 22, 2022
View reviewed changes
Copy link
Member

@notmandatory notmandatory left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK b047922

@notmandatory
Copy link
Member

@danielgranhao Thanks for the update. It looks like it's ready to merge but can you sign your commits first?

@notmandatory notmandatory added this to the Release 0.2.0 milestone Oct 22, 2022
@danielgranhao
Copy link
Contributor Author

@notmandatory Sorry about that, done!

@benthecarman
Copy link
Contributor

Looks like there is a compile error

@afilini
Copy link
Member

afilini commented Oct 24, 2022

Yes, I think BlockHash isn't Default anymore. You can try replacing that line with BlockHash::all_zeros(), it should work the same.

@danielgranhao
Copy link
Contributor Author

Hum.. I'm not sure why this is failing. The unsigned commit seemed fine and I can't even find the line that is erroring in the repo 🤔

@afilini
Copy link
Member

afilini commented Oct 24, 2022

The line is here:

rust-esplora-client/src/lib.rs

Line 494 in 8c93e0d

let block_hash = BlockHash::default();

@danielgranhao
Copy link
Contributor Author

Thanks! Forgot to rebase 🤦 should be fixed now

@afilini
Copy link
Member

afilini commented Oct 24, 2022

You need to run cargo fmt to format the code

@notmandatory notmandatory merged commit 56b0352 into bitcoindevkit:master Oct 24, 2022
notmandatory added a commit to bitcoindevkit/bdk that referenced this pull request Oct 27, 2022
@notmandatory
Merge #770: Upgrade to rust-bitcoin 0.29
5720e38 
c7a43d9 Remove unused code (Alekos Filini)
1ffd59d Upgrade to rust-bitcoin 0.29 (Alekos Filini)
ae4f4e5 Upgrade `rand` to `0.8` (Alekos Filini)
9854fd3 Remove deprecated address validators (Alekos Filini)

Pull request description:

  ### Description

  Upgrade BDK to rust-bitcoin 0.29

  Missing pieces:

  - [x] rust-miniscript `update_output_with_descriptor` - rust-bitcoin/rust-miniscript#465
  - [x] rust-miniscript 8.0.0 release - rust-bitcoin/rust-miniscript#462
  - [x] Upgrade rust-hwi to bitcoin 0.29 bitcoindevkit/rust-hwi#50
  - [x] Upgrade esplora-client to bitcoin 0.29 bitcoindevkit/rust-esplora-client#20
  - [x] Upgrade rand to 0.8 like secp256k1 did

  ### Notes to the reviewers

  The commits still need to be reordered and cleaned up

  ### Changelog notice

  - Upgrade rust-bitcoin to 0.29
  - Remove deprecated "address validators"

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

Tree-SHA512: 718a1baf3613b31ec1de39fe63467ebee38617963a4ce0670a617e20fe4f46a57c5786933cdde6cfad9fc76ce0af08843f58844fb4a89f5948cb42c697f802ef
@coveralls
Copy link

Pull Request Test Coverage Report for Build 3272847458

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 1 of 3 (33.33%) changed or added relevant lines in 2 files are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage remained the same at 77.938%
Changes Missing Coverage Covered Lines Changed/Added Lines %
src/api.rs 0 2 0.0%
Files with Coverage Reduction New Missed Lines %
src/lib.rs 2 95.36%
Totals Coverage Status
Change from base Build 3241094232: 0.0%
Covered Lines: 650
Relevant Lines: 834
💛 - Coveralls

1 similar comment
@coveralls
Copy link

Pull Request Test Coverage Report for Build 3272847458

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 1 of 3 (33.33%) changed or added relevant lines in 2 files are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage remained the same at 77.938%
Changes Missing Coverage Covered Lines Changed/Added Lines %
src/api.rs 0 2 0.0%
Files with Coverage Reduction New Missed Lines %
src/lib.rs 2 95.36%
Totals Coverage Status
Change from base Build 3241094232: 0.0%
Covered Lines: 650
Relevant Lines: 834
💛 - Coveralls

@tnull
Copy link
Contributor

tnull commented Aug 18, 2024

Wow, coveralls was really late to this party.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@notmandatory notmandatory notmandatory approved these changes

@rajarshimaitra rajarshimaitra rajarshimaitra approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Release 0.2.0
Development

Successfully merging this pull request may close these issues.
7 participants
@danielgranhao @coveralls @notmandatory @benthecarman @afilini @tnull @rajarshimaitra