Draft
Conversation
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.
50bd321 to
e7c1565
Compare
|
4-1 red lines to green, got to be happy with that. |
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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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:
Weighttype fromweightmethods onTransactionandBlock.HeightandTimetypes 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.