Skip to content
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

Added a dummy Kyber implementation and test scaffolding #438

Merged
merged 10 commits into from
Dec 6, 2023

Conversation

xvzcf
Copy link

@xvzcf xvzcf commented Dec 5, 2023

Fixes #437

Copy link

cla-bot bot commented Dec 5, 2023

We require contributors to sign our Contributor License Agreement https://github.com/cryspen/hacl/blob/main/CLA.md ensuring that the contribution can be licensed under Apache 2.0 and MIT. In order for us to review and merge your code, please mention @cryspen/core in a comment below to get yourself added.

Copy link

cla-bot bot commented Dec 5, 2023

We require contributors to sign our Contributor License Agreement https://github.com/cryspen/hacl/blob/main/CLA.md ensuring that the contribution can be licensed under Apache 2.0 and MIT. In order for us to review and merge your code, please mention @cryspen/core in a comment below to get yourself added.

@coveralls
Copy link

coveralls commented Dec 5, 2023

Pull Request Test Coverage Report for Build 7117191938

Warning: This coverage report may be inaccurate.

We've detected an issue with your CI configuration that might affect the accuracy of this pull request's coverage report.
To ensure accuracy in future PRs, please see these guidelines.
A quick fix for this PR: rebase it; your next report should be accurate.

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 59.089%

Totals Coverage Status
Change from base Build 7085212369: 0.0%
Covered Lines: 37671
Relevant Lines: 63753

💛 - Coveralls

Copy link

cla-bot bot commented Dec 5, 2023

We require contributors to sign our Contributor License Agreement https://github.com/cryspen/hacl/blob/main/CLA.md ensuring that the contribution can be licensed under Apache 2.0 and MIT. In order for us to review and merge your code, please mention @cryspen/core in a comment below to get yourself added.

Copy link

cla-bot bot commented Dec 5, 2023

We require contributors to sign our Contributor License Agreement https://github.com/cryspen/hacl/blob/main/CLA.md ensuring that the contribution can be licensed under Apache 2.0 and MIT. In order for us to review and merge your code, please mention @cryspen/core in a comment below to get yourself added.

@franziskuskiefer franziskuskiefer changed the base branch from main to dev December 6, 2023 06:34
Copy link

cla-bot bot commented Dec 6, 2023

We require contributors to sign our Contributor License Agreement https://github.com/cryspen/hacl/blob/main/CLA.md ensuring that the contribution can be licensed under Apache 2.0 and MIT. In order for us to review and merge your code, please mention @cryspen/core in a comment below to get yourself added.

@franziskuskiefer franziskuskiefer linked an issue Dec 6, 2023 that may be closed by this pull request
@franziskuskiefer franziskuskiefer marked this pull request as ready for review December 6, 2023 13:49
@franziskuskiefer franziskuskiefer requested a review from a team as a code owner December 6, 2023 13:49
Copy link
Member

@franziskuskiefer franziskuskiefer left a comment

Choose a reason for hiding this comment

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

Thanks, generally lgtm with a few nits.

@@ -6,5 +6,5 @@ printf " ! USE ./mach FOR MORE OPTIONS !\n\n"

mkdir build
cp config/default_config.cmake build/config.cmake
cmake -B build -G"Ninja Multi-Config"
cmake -B build -G"Ninja Multi-Config" -DBUILD_LIBCRUX=ON
Copy link
Member

Choose a reason for hiding this comment

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

Is this used anywhere where we want libcrux on?
I don't think we want libcrux here just yet.

Copy link
Author

Choose a reason for hiding this comment

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

I just added it there so that it's tested on some of the CI jobs that run this script, but I can remove it.

Copy link
Member

Choose a reason for hiding this comment

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

If they go through, leave it. But if it makes problems, drop it.

tools/test.py Show resolved Hide resolved
uint8_t publicKey[KYBER768_PUBLICKEYBYTES];
uint8_t secretKey[KYBER768_SECRETKEYBYTES];

for (auto kat : kats) {
Copy link
Member

Choose a reason for hiding this comment

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

Can we use TEST_P instead to make sure we can run individual tests?

@cla-bot cla-bot bot added the cla-signed label Dec 6, 2023
Copy link
Member

@franziskuskiefer franziskuskiefer left a comment

Choose a reason for hiding this comment

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

thanks :shipit:

@franziskuskiefer franziskuskiefer merged commit 6fda3af into dev Dec 6, 2023
38 checks passed
@franziskuskiefer franziskuskiefer deleted the goutam/libcrux-kyber branch December 6, 2023 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

C Testing harness for Kyber
3 participants