Using memcmp() should improve the readability of the function GetWitnessCommitmentIndex() by avoiding the overly long if condition and having the commitment header directly visible in one place.
validation: find witness commitment header using memcmp() instead of byte-by-byte comparison #16892
pull theStack wants to merge 1 commits into bitcoin:master from theStack:20190917-validation-find_witness_commitment_with_memcmp changing 1 files +3 −1-
theStack commented at 5:50 AM on September 17, 2019: member
-
validation: find witness commitment header using memcmp() instead of byte-by-byte comparison 057dc78206
- fanquake added the label Validation on Sep 17, 2019
-
DrahtBot commented at 6:01 AM on September 17, 2019: member
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--174a7506f384e20aa4161008e828411d-->
Conflicts
Reviewers, this pull request conflicts with the following ones:
- #16411 (Signet support by kallewoof)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
-
MarcoFalke commented at 11:33 AM on September 17, 2019: member
Thank you for your contribution. While this stylistic change makes sense on its own, it comes at a cost and risk for the project as a whole. The weak motivation for the change does not justify the burden that it places on the project. A burden could be any of the following:
- Time spent on review
- Accidental introduction of bugs
- (Silent) merge conflicts, either in the branch or a backport branch. Those conflicts demand further developer and reviewer time or introduce bugs.
For more information about refactoring changes and stylistic cleanup, see
- https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#refactoring
- https://github.com/bitcoin/bitcoin/blob/master/.github/PULL_REQUEST_TEMPLATE.md
- #15465
Generally, if the style is not mentioned nor enforced by the developer notes, we leave it up to the original author to pick whatever fits them best personally and then leave it that way until the line is touched for other reasons.
Let me know if you have any questions.
- MarcoFalke closed this on Sep 17, 2019
- MarcoFalke locked this on Dec 16, 2021