Skip to content

Use bitcoin-units 1.0 #229

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

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

apoelstra
Copy link
Member

Draft until 1.0 is out; for now we are using rc0.

This introduces several changes that move this crate toward its final 1.0 form:

  1. Remove some deprecated methods
  2. Return the bitcoin-units Weight type from weight methods on Transaction and Block.
  3. Replace ad-hoc serde impls on locktimes and PSETs with standard ones that use the consensus encoding.
  4. Replace the Height and Time types in the locktime module with ones from bitcoin-units.

This will improve our compatibility with rust-bitcoin and also improve compatibility with different versions of rust-elements.

We don't need to rename packages anymore to have an optional dependency
that also triggers other depenedncies when enabled.
This completely changes the serde format for `LockTime`, which in turn
changes the format for `Transaction`. This will break e.g. hal-elements.
However, the existing format is kinda ridiculous and I don't believe it
is used anywhere.

Alarmingly this change does not break a single test. We should add some
regression tests.
There is a standard serialization of PSETs (and PSBTs, and PSBT2s): the
base64 of the byte encoding. Rather than making up our own serialization
based on the details of our rust-elements types, which makes basically
every structural property of our types a property of our API, just use
the standard encoding.

This is a major breaking change; we may need a compatibility layer based
on rust-elements 0.25.
In bitcoin-units 1.0 we have types that represent locktime times and
heights. The locktimes themselves are not in units (they will be exposed
in bitcoin-primitives once that comes out) but the essential hard logic
is implemented on these supporting types.

Importantly, the new types don't implement serde, which is why we needed
to do breaking serde changes in the last couple of commits. (But we
should have done these changes a long time ago, anyway.)
The API change is actually only a couple LOC. The real change is that we
start using u64 arithmetic to compute weights rather than usize
arithmetic, because Weight::from_wu wants a u64.

To facilitate this we introduce the private Len64 trait which provides
lengths in u64, and use it throughout the crate. This replaces 23 casts
to usize or u64 with four.
@tcharding
Copy link

4-1 red lines to green, got to be happy with that.

@apoelstra
Copy link
Member Author

Heh, well, much of it is replacing the serde stuff which I think I will PR separately because I want to get those changes in ASAP.

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.

2 participants