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

Add witness_version functions to CheckedHrpstring #175

Merged
merged 2 commits into from
Jan 30, 2024

Conversation

tcharding
Copy link
Member

As we did for UncheckedHrpstring add functions to the CheckedHrpstring struct to allow checking the initial byte of the data part and removing it if required.

Add an initial patch that improves the docs on the UncheckedHrpstring::remove_witness_version function (this is then copied in patch 2).

The `witness_version` function does not remove the witness version;
improve the documentation on `UncheckedHrpstring::witness_version` to
mention this and point devs towards
`UncheckedHrpstring::remove_witness_version`.
As we did for `UncheckedHrpstring` add functions to the
`CheckedHrpstring` struct to allow checking the initial byte of the data
part and removing it if required.
@clarkmoody
Copy link
Member

Do we need standalone tests for this, or is that captured in the docs examples?

@apoelstra
Copy link
Member

Let me confirm that breaking the doc examples breaks CI on my end -- but I'm pretty sure they are run in CI, yes.

@apoelstra
Copy link
Member

Ok, yes, they're run in the real CI but not in my local one :). Glad I checked this. I'll fix the local thing, then remove my failing commit.

@apoelstra apoelstra force-pushed the 01-29-witness-version branch from 017f60b to 2d7832a Compare January 30, 2024 15:06
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 2d7832a tested with rust-elements and it works!

@apoelstra apoelstra merged commit 92146d7 into rust-bitcoin:master Jan 30, 2024
24 checks passed
@tcharding tcharding deleted the 01-29-witness-version branch January 31, 2024 03: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