Skip to content

bip 8: simplify MUST_SIGNAL check #1063

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

Merged

Conversation

ajtowns
Copy link
Contributor

@ajtowns ajtowns commented Feb 4, 2021

Simplify the validity check that at least threshold blocks are signalling during the MUST_SIGNAL phase for BIP 8.

@ajtowns
Copy link
Contributor Author

ajtowns commented Feb 4, 2021

cc @jonasnick @dr-orlovsky @luke-jr -- see also #1021 (comment)

@ajtowns ajtowns force-pushed the 202102-bip8-simplify-mustsignal-check branch from 3a9f7d9 to 63d2800 Compare February 4, 2021 02:51
Copy link
Contributor

@dr-orlovsky dr-orlovsky left a comment

Choose a reason for hiding this comment

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

LGTM. Propose to include it into the next taproot activation meeting for the review

Copy link
Contributor

@benthecarman benthecarman left a comment

Choose a reason for hiding this comment

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

ACK 63d2800

Copy link
Contributor

@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 63d2800

Sanity checked that there's no nasty off-by-one by adjusting my previous implementation according to this PR and running the same tests against it. The nonsignal == 0 was a neat optimization but fine to leave that up to the implementor.

@luke-jr luke-jr merged commit ec213e1 into bitcoin:master Feb 8, 2021
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.

5 participants