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

Release tracking PR: v0.10.0 #168

Merged
merged 1 commit into from
Feb 22, 2024

Conversation

tcharding
Copy link
Member

@tcharding tcharding commented Jan 14, 2024

In preparation for doing the first "real" (non-beta) release of the new primitives module add a changelog entry and bump the version.

@tcharding tcharding marked this pull request as ready for review January 14, 2024 20:48
@tcharding tcharding changed the title Bump version to 0.10.0 Release tracking PR: 0.10.0 Jan 14, 2024
@tcharding
Copy link
Member Author

tcharding commented Jan 14, 2024

Before this merges can you confirm please that the new accessor methods fully cover your use case in liquid @apoelstra. Using the new API over there was the last step before we were confident to release this, right?

@tcharding tcharding changed the title Release tracking PR: 0.10.0 Release tracking PR: v0.10.0 Jan 14, 2024
@apoelstra
Copy link
Member

Yeah, lemme PR to rust-elements to use the new accessors first please.

@apoelstra
Copy link
Member

@tcharding can we do a beta2 release first which includes the new stuff?

@tcharding
Copy link
Member Author

Sure thing, I couldn't think of a nice next beta version name yesterday - I'll use beta2 as suggested.

@tcharding tcharding marked this pull request as draft January 15, 2024 20:32
@tcharding
Copy link
Member Author

Next beta release done in #169, keeping this here for the v0.10.0 when it comes - can't be far away.

@tcharding
Copy link
Member Author

Note the diff includes change to CHANGELOG removing mention of v0.10.0-beta.2, since we yanked it I believe this is correct.

@tcharding tcharding marked this pull request as ready for review January 24, 2024 04:32
@apoelstra
Copy link
Member

apoelstra commented Jan 28, 2024

Ok, sorry, we have to do one more iteration on the API.

The issue I'm having is that there is a specific sequence to parsing a Segwit string:

  • Parse as an UncheckedHrpstring to check that the hrpstring is well-formed (can be done)
  • First, check (but don't remove) the witness version (can be done with new UncheckedHrpstring::witness_version) (though in retrospect UncheckedHrpstring::remove_witness_version I think is useless). Determine the checksum based on the witness version.
  • Validate and remove the checksum to get a CheckedHrpstring (can be done)
  • Remove the witness version from the CheckedHrpstring (cannot be done)
  • Check segwit padding and length (can be done)

So I think what we want to do is move remove_witness_version from UncheckedHrpstring to CheckedHrpstring and we should be good to go. I suppose it ought to yield the witness version but honestly it doesn't have to, it just be a &mut method that changes self.data to self.data[1..] and doesn't return anything.

But we've made a ton of progress :) on my local rust-elements branch we have

 5 files changed, 40 insertions(+), 546 deletions(-)

which is basically all deleted code that I'd copied and tweaked from this library since the API didn't expose enough for me to use it directly.

@tcharding
Copy link
Member Author

tcharding commented Jan 28, 2024

5 files changed, 40 insertions(+), 546 deletions(-)

That's a win, for sure. Will hack up your suggestions.

@tcharding
Copy link
Member Author

I left the functions on UncheckedHrpstring - from an API perspective seems to be no reason to have them on one and not the other.

In preparation for doing the first "real" (non-beta) release of the new
`primitives` module add a changelog entry and bump the version.
@tcharding tcharding marked this pull request as ready for review February 22, 2024 02:37
Copy link
Member

@clarkmoody clarkmoody left a comment

Choose a reason for hiding this comment

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

ACK 1b64ed4

@tcharding
Copy link
Member Author

ooo yeah, I love waking up in the morning to a fresh release - make my day @apoelstra!

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 1b64ed4 let's do it

@apoelstra apoelstra merged commit 0541ee4 into rust-bitcoin:master Feb 22, 2024
13 checks passed
@apoelstra
Copy link
Member

Tagged and published.

@apoelstra
Copy link
Member

Sigh, cargo seems to think that 0.10.0 is the same major rev as 0.10.0-beta and is breaking downstream crates.

Yanked 0.10.0. Will need to re-release as 0.11.0.

@tcharding
Copy link
Member Author

Bother, I wish I came to the same conclusion before I got in a rage about every PR in rust-bitcoin being broken.

@tcharding
Copy link
Member Author

Like, literally the reason we did the -beta releases ....

@tcharding tcharding deleted the 01-15-release-0.10.0 branch March 25, 2024 02:49
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.

None yet

3 participants