-
Notifications
You must be signed in to change notification settings - Fork 198
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
k256: normalize before calling is_odd() in sng0() #533
Conversation
Are you asking why CI runs the tests with We've had bugs in the past that only affected the release profile. An easy way to screw up is to perform some sort of effectful operation inside of a Release builds are what people actually use, so we test against them as they're more important. We can potentially test both debug and release builds if you think that'd be helpful. |
I'm actually not sure now how it happened, because |
I hope it's not too much to ask to publish k256 0.10.4 with this? My library is using hash2curve, that's how I noticed this - in my own tests. |
This release backports #533. Since `master` is already v0.11 prereleases, this commit just contains the release notes. The released code can be found via the `k256/v0.10.4` tag: https://github.com/RustCrypto/elliptic-curves/tree/k256/v0.10.4
v0.10.4 released: #534 |
This release backports #533. Since `master` is already v0.11 prereleases, this commit just contains the release notes. The released code can be found via the `k256/v0.10.4` tag: https://github.com/RustCrypto/elliptic-curves/tree/k256/v0.10.4
An
is_odd()
call I forgot to address in #530While this fixes the problem at hand, there are a couple of considerations:
FieldElement::is_odd()
(but not inFieldElementImpl
!). Originally I didn't do that because I wanted to leave a possibility for a slightly more efficient code (where the caller would know whether the value has been previously normalized or not), but perhaps it is not worth it, and it's better to prevent errors instead.FieldElementImpl::is_odd()
#530 because in CI we run tests with--release
, and apparently this bug does not affecthash2curve
test, which is the only one touchingsgn0()
. @tarcieri , why was that decision made originally?