-
Notifications
You must be signed in to change notification settings - Fork 95
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
Convert oqs-kem-info.md code points to hex #188
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a change to an automatically generated file and as such won't persist. Please change at the master file (oqs-template/generate.yml
), re-generate (python3 oqs-template/generate.py
) and re-run all tests.
0d4691c
to
261bc2f
Compare
I'm not able to see the CircleCI results. I'll try to re-run the tests to reproduce the CI failures locally. Also, I've noticed that both before and after this PR's changeset, the |
Thanks for the update: This is now perfect to retain.
This is basically a question/reminder to @bhess to re-evaluate the original ECX logic (via an extra-YML file) he originally introduced. IMO it could be removed as #177 completely changed the build logic to permit more than a single X-hybrid for the same PQ alg. |
It appears that the erroneous warning message motivating this commit is due to some vestigial code special-casing x-curves. The `nid_ecx_hyybrid` kem field was only used to generate the duplicate ecx nid warning message, so we track duplicates in a local `set` instead. Additionally, we expand the warning message to all curve NIDs, not just x-curves and treat duplicates as fatal because extra curve NIDs should be unique for each KEM.
fd8057a
to
4411ed1
Compare
@WillChilds-Klein Thanks again for the contribution! CI has run successfully so we'll merge this tomorrow. As this is more than a trivial change, please consider amending this PR before merge by adding yourself to the Contributors list in the README. |
* Convert Kyber768 code points to hex * Fix generate.py duplicate hybrid NID warning It appears that the erroneous warning message motivating this commit is due to some vestigial code special-casing x-curves. The `nid_ecx_hyybrid` kem field was only used to generate the duplicate ecx nid warning message, so we track duplicates in a local `set` instead. Additionally, we expand the warning message to all curve NIDs, not just x-curves and treat duplicates as fatal because extra curve NIDs should be unique for each KEM. * Update contributors section of README Signed-off-by: Felipe Ventura <felipe.ventura@entrust.com>
Minor documentation update to use hexadecimal notation for KEM code points.
Not that I'm aware of.
Notes
The initial motivation for this PR was a small documentation change. That change uncovered an unexpected warning message. It appears that the erroneous warning message motivating this commit is due to some vestigial code special-casing x-curves. The
nid_ecx_hyybrid
kem field was only used to generate the duplicate ecx nid warning message, so we track duplicates in a localset
instead. Additionally, we expand the warning message to all curve NIDs, not just x-curves and treat duplicates as fatal because extra curve NIDs should be unique for each KEM.Duplicate NID detection is tested below:
I'm not sure what's up with the CI; I don't appear to have permissions to view the build errors in CircleCI. I ran the tests locally on a linux box: