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

Update rust-miniscript (and thereby rust-bitcoin) to latest version #565

Merged
merged 5 commits into from
Jul 13, 2023

Conversation

darosior
Copy link
Member

@darosior darosior commented Jul 12, 2023

It was a big chunk, especially in making sure we don't introduce any silent bug with all the upstream recent code movements.

Also, we no longer depend on my custom rust-miniscript branch! 🎉

@darosior darosior force-pushed the update_miniscript branch 2 times, most recently from 677e995 to cd47e99 Compare July 12, 2023 16:18
@darosior darosior changed the title WIP: update rust-miniscript / rust-bitcoin dependencies Update rust-miniscript (and thereby rust-bitcoin) to latest version Jul 12, 2023
@darosior darosior marked this pull request as ready for review July 12, 2023 16:25
@darosior darosior force-pushed the update_miniscript branch 2 times, most recently from 3c033c9 to d97152e Compare July 13, 2023 07:29
Rust-bitcoin, that we use through rust-miniscript, has seen plenty of
breaking changes in the latest version. I've tried to keep the necessary
changes here minimal, still it had to be a single commit to keep it
hygienic. But i'll try to summarize the main things here. Tobin also
wrote a guide about the release at
https://rust-bitcoin.org/blog/release-0.30.0/.

The most verbose change in this commit is probably due to the `Address`
type overhaul. It's overengineered if you ask me but hey here we are. I
tried to keep network validation in commands, and otherwise passing
around unchecked addresses (to avoid having to pass around a global
state between our various components).

Another non-obvious change was changes in hash types upstream and the
removal of `ToHex`, forcing us to get the hex representation of a txid
through its `Display` implementation. It is however displayed backward
in this case ("little-endian" if you will), and we need a regular hex
encoding for some queries to the database. We needed to make sure we
didn't implement any silent bug here.

The rest (Script type changes, PSBT serialization updates, ..) is
probably self-explanatory.
For some reason it started warning about them after the upgrade of
rust-miniscript.
The latest rust-miniscript version deprecated the helper we were using,
in favour of one that gives the maximum size difference of a transaction
input before and after satisfaction. The new helper differs in that it
does not account for the empty ScriptSig byte (which uncovered we were
actually double-counting it), and assumes the non-satisfied transaction
input is already part of a Segwit transaction (which we rectified).

This uncovered a mistake in the computation of the witness script size
in the unit test. We also get rid of the needless wu_to_vb() standalone
function.
rust-bitcoin now takes care of that.
@darosior
Copy link
Member Author

Self-ACK a19f2c1.

@darosior darosior merged commit 12a3d50 into wizardsardine:master Jul 13, 2023
@darosior darosior deleted the update_miniscript branch July 13, 2023 08:03
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.

1 participant