Abstract out and merge all the magnitude/normalized logic#1066
Merged
sipa merged 29 commits intobitcoin-core:masterfrom May 11, 2023
Merged
Abstract out and merge all the magnitude/normalized logic#1066sipa merged 29 commits intobitcoin-core:masterfrom
sipa merged 29 commits intobitcoin-core:masterfrom
Conversation
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Right now, all the logic for propagating/computing the magnitude/normalized fields in
secp256k1_fe(whenVERIFYis defined) and the code for checking it, is duplicated across the two field implementations. I believe that is undesirable, as these properties should purely be a function of the performed fe_ functions, and not of the choice of field implementation. This becomes even uglier with #967, which would copy all that, and even needs an additional dimension that would then need to be added to the two other fields. It's also related to #1001, which I think will become easier if it doesn't need to be done/reasoned about separately for every field.This PR moves all logic around these fields (collectively called field verification) to implementations in field_impl.h, which dispatch to renamed functions in field_*_impl.h for the actual implementation.
Fixes #1060.