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.

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

Upgrade bitcoin dependency #618

Merged
merged 1 commit into from
Nov 22, 2023

Conversation

tcharding
Copy link
Member

@tcharding tcharding commented Oct 16, 2023

Upgrade rust-bitcoin to version 0.31.0 and also other required dependencies from our stack:

  • secp256k1: to v0.28.0
  • internals: to v0.2.0
  • bitcoind: to v0.34.0

@tcharding
Copy link
Member Author

tcharding commented Oct 16, 2023

Includes removal of some "lol, sorta" comments, these did not make me laugh out loud and the lines marked are 100% technically correct for the function name :)

EDIT: I was salty when I wrote that, my bad. I put the comments back in.

Upgrade to the latest versions the dependencies required to use `bitcoin
v0.31.0-rc1`:

- bitcoin: to v0.31.0
- secp256k1: to v0.28.0
- internals: to v0.2.0
- bitcoind: to v0.34.0
@tcharding tcharding marked this pull request as ready for review November 15, 2023 23:30
@@ -614,7 +614,7 @@ impl ScriptContext for Tap {
// When the transaction sizes get close to block limits,
// some guarantees are not easy to satisfy because of knapsack
// constraints
if ms.ext.pk_cost > MAX_BLOCK_WEIGHT as usize {
if ms.ext.pk_cost as u64 > Weight::MAX_BLOCK.to_wu() {
Copy link
Member

Choose a reason for hiding this comment

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

In e7b5db2:

In a followup we should change the units of pk_cost to be Weight so we don't need to cast.

Copy link
Member Author

Choose a reason for hiding this comment

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

I added it to my todo list.

@apoelstra
Copy link
Member

Lots of renames. ACK from me. I hope that it was possible to do this in more than 1 commit (by fixing all the actual errors, then going through one class of deprecation message at a time). But I won't ask you to redo the PR :).

@tcharding
Copy link
Member Author

I hope that it was possible to do this in more than 1 commit (by fixing all the actual errors, then going through one class of deprecation message at a time). But I won't ask you to redo the PR :).

I can definitely do better in future, I have been doing upgrades in a very inefficient, embarrassingly manual manner. Evertime I start I think to myself "its only a few changes ..."

Copy link
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

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

ACK e7b5db2

@apoelstra
Copy link
Member

cc @sanket1729 can I go ahead and merge this?

@apoelstra
Copy link
Member

I'm gonna do it and cut a release, since I think you're busy/unavailable for a bit.

@sanket1729
Copy link
Member

sanket1729 commented Nov 22, 2023

@apoelstra, I am busy with some personal events for the next two weeks. Please go ahead and cut a release.

@apoelstra
Copy link
Member

Thanks!

@apoelstra apoelstra merged commit f9cfa37 into rust-bitcoin:master Nov 22, 2023
16 checks passed
@tcharding tcharding deleted the 10-16-upgrade-bitcoin branch November 23, 2023 02:09
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.

3 participants