Skip to content

Add BlockHeader::clear_witness and ::is_dynafed methods #31

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Oct 4, 2019
Merged

Add BlockHeader::clear_witness and ::is_dynafed methods #31

merged 2 commits into from
Oct 4, 2019

Conversation

@stevenroose
Copy link
Contributor

@stevenroose stevenroose commented Sep 20, 2019 •
edited
Loading

No description provided.

@stevenroose stevenroose changed the title Add BlockHeader::clear_witness method Add BlockHeader::clear_witness and ::is_dynafed methods Sep 25, 2019
@stevenroose
Copy link
Contributor Author

stevenroose commented Sep 25, 2019

Fixed for 1.22.

jonasnick
jonasnick reviewed Sep 25, 2019
View reviewed changes
Copy link
Collaborator

@jonasnick jonasnick left a comment

Choose a reason for hiding this comment

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

Code looks good. What's the motivation?

@stevenroose
Copy link
Contributor Author

stevenroose commented Sep 25, 2019

Those matches over complex enum variants are super painful and inflated line-wise. I think we would have better have had 2 structs and simpler tuple variants taking that one struct instead of the variant taking fields.
It's basically a PITA when you're actually using these variants to get any fields from them once you know which variant is chosen. Same goes for #34.

So just some simple utilities to save lines and improve readability of code using these enums.

@jonasnick
Copy link
Collaborator

jonasnick commented Sep 26, 2019

Sorry, should have been more specific. I get that it's more concise but I was wondering where clear_witness is useful.

@instagibbs
Copy link

instagibbs commented Sep 26, 2019

@jonasnick witness-stripped blockheaders for when you don't need witness data to persist after witness is validated

jonasnick
jonasnick approved these changes Sep 26, 2019
View reviewed changes
Copy link
Collaborator

@jonasnick jonasnick left a comment

Choose a reason for hiding this comment

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

ACK de82b59

@apoelstra
Copy link
Member

apoelstra commented Sep 27, 2019

Needs rebase.

@stevenroose
Copy link
Contributor Author

stevenroose commented Oct 1, 2019 •
edited
Loading

rebased

@stevenroose stevenroose closed this Oct 1, 2019
@apoelstra apoelstra mentioned this pull request Oct 1, 2019
Merged
@stevenroose stevenroose reopened this Oct 2, 2019
jonasnick added a commit that referenced this pull request Oct 4, 2019
@jonasnick
Merge #31: Add BlockHeader::clear_witness and ::is_dynafed methods
effc26e 
3e7c7b1 Add BlockHeader::clear_witness method (Steven Roose)
8b0d786 Add BlockHeader::is_dynafed (Steven Roose)

Pull request description:

Top commit has no ACKs.

Tree-SHA512: df4b899bec11b24da2abcfeed2a81f607fb62e28159ad7c0aa1937101d847cd2dbf6531ba177eae274aed3301a523e24a947a90db1f8ece3d8e2fa23e8962cef
@jonasnick jonasnick merged commit 3e7c7b1 into ElementsProject:master Oct 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jonasnick jonasnick jonasnick approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@stevenroose @jonasnick @instagibbs @apoelstra