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.

Sign up for GitHub

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

Jump to bottom

ML-DSA 44 instantiations and multiplexing #524

Merged
merged 9 commits into from
Sep 9, 2024
Merged

ML-DSA 44 instantiations and multiplexing #524

merged 9 commits into from
Sep 9, 2024

Conversation

franziskuskiefer
Copy link
Member

@franziskuskiefer franziskuskiefer commented Aug 22, 2024 •
edited
Loading

  • add build.rs for feature detection
  • add instantiations for 44
  • add neon sha3

Towards #275

@franziskuskiefer franziskuskiefer requested a review from keks August 22, 2024 12:06
keks
keks reviewed Aug 22, 2024
View reviewed changes
Copy link
Member

@keks keks left a comment

Choose a reason for hiding this comment

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

So far I only have pretty high level comments, as this code is pretty dense with meta programming and I always find it difficult to get into. I do have two style questions. It seems like the tests are still failing, so maybe this isn't quite ready yet anyway?

libcrux-ml-dsa/Cargo.toml Show resolved Hide resolved
libcrux-ml-dsa/src/ml_dsa_44.rs Outdated Show resolved Hide resolved
franziskuskiefer
franziskuskiefer commented Aug 22, 2024
View reviewed changes
Copy link
Member Author

@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.

Yeah, no one else is around right now to review and I thought it could be nice to get into 😉 You don't need to go too deep.

It seems like the tests are still failing

Github seems to have issues.

libcrux-ml-dsa/src/ml_dsa_44.rs Outdated Show resolved Hide resolved
libcrux-ml-dsa/Cargo.toml Show resolved Hide resolved
@franziskuskiefer franziskuskiefer self-assigned this Aug 23, 2024
@franziskuskiefer franziskuskiefer force-pushed the franziskus/ml-dsa-boilerplate branch from f3e2120 to 4dccc28 Compare August 30, 2024 07:32
jschneider-bensch
jschneider-bensch approved these changes Sep 9, 2024
View reviewed changes
Copy link
Collaborator

@jschneider-bensch jschneider-bensch left a comment

Choose a reason for hiding this comment

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

Looks good to me! Just some incidental things below.

libcrux-ml-dsa/build.rs Outdated Show resolved Hide resolved
libcrux-ml-dsa/build.rs Show resolved Hide resolved
libcrux-ml-dsa/src/hash_functions.rs Outdated Show resolved Hide resolved
franziskuskiefer and others added 2 commits September 9, 2024 12:05
@franziskuskiefer @jschneider-bensch
Apply suggestions from code review
99498ee 
Co-authored-by: Jonas Schneider-Bensch <124457079+jschneider-bensch@users.noreply.github.com>
@franziskuskiefer
update extracted C code
717b165
@franziskuskiefer franziskuskiefer added this pull request to the merge queue Sep 9, 2024
Merged via the queue into main with commit 4a129c7 Sep 9, 2024
48 checks passed
@franziskuskiefer franziskuskiefer deleted the franziskus/ml-dsa-boilerplate branch September 9, 2024 11:51
@franziskuskiefer franziskuskiefer mentioned this pull request Sep 10, 2024
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@keks keks keks left review comments

@jschneider-bensch jschneider-bensch jschneider-bensch approved these changes

@karthikbhargavan karthikbhargavan Awaiting requested review from karthikbhargavan

Assignees

@franziskuskiefer franziskuskiefer

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@franziskuskiefer @keks @jschneider-bensch