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

Add randomised APIs to ml-kem #546

Merged
merged 8 commits into from
Sep 3, 2024
Merged

Add randomised APIs to ml-kem #546

merged 8 commits into from
Sep 3, 2024

Conversation

franziskuskiefer
Copy link
Member

No description provided.

@franziskuskiefer franziskuskiefer self-assigned this Aug 30, 2024
jschneider-bensch
jschneider-bensch previously requested changes Sep 2, 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, with tiny doc nits 👍

libcrux-ml-kem/src/kem/kyber/kyber1024.rs Outdated Show resolved Hide resolved
libcrux-ml-kem/src/kem/kyber/kyber512.rs Outdated Show resolved Hide resolved
libcrux-ml-kem/src/kem/kyber/kyber768.rs Outdated Show resolved Hide resolved
@franziskuskiefer @jschneider-bensch
Apply suggestions from code review
3fc1eab 
Co-authored-by: Jonas Schneider-Bensch <124457079+jschneider-bensch@users.noreply.github.com>
franziskuskiefer
franziskuskiefer commented Sep 2, 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.

Ah thanks. Copy pasta

@jschneider-bensch
Copy link
Collaborator

Huh, somehow the extraction diff fails now because .gitignore is missing in the extracted version. In the extract jobs, actions/upload-artifact@v4 has option include-hidden-files set to false so it makes sense that the .gitignore should be ignored. It must have been included before though, since that job would have had to fail every time previously, otherwise. Indeed, on an earlier run of that job the action did not have this option: https://github.com/cryspen/libcrux/actions/runs/10636686836/job/29488994163

@jschneider-bensch
Copy link
Collaborator

Ah, I see, it was announced as a breaking change. Still a bit weird to have that without a version bump

@franziskuskiefer
Copy link
Member Author

Ah, I see, it was announced as a breaking change. Still a bit weird to have that without a version bump

🙄 looks like everyone is annoyed by this. @jschneider-bensch would you mind pushing a fix to this branch?

@jschneider-bensch
Copy link
Collaborator

Ah sorry, I would have, but I've already pushed a fix to it's own PR #555

@franziskuskiefer franziskuskiefer added this pull request to the merge queue Sep 3, 2024
Merged via the queue into main with commit 482620d Sep 3, 2024
48 checks passed
@franziskuskiefer franziskuskiefer deleted the franziskus/ml-kem-rand branch September 3, 2024 12:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

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

Assignees

@franziskuskiefer franziskuskiefer

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

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