Repository navigation
Conversation
ounsworth
force-pushed
the
refactor/algparams_traits
branch
3 times, most recently
from
September 7, 2026 01:16
93b16cd to
3ab3bb0
Compare
ounsworth
force-pushed
the
refactor/algparams_traits
branch
from
September 7, 2026 01:56
3ab3bb0 to
79cc258
Compare
… This commit changes constant names back to their origin case to match FIPS 203 / 204.
1 of 4 tasks
dghgit
self-requested a review
September 7, 2026 10:34
dghgit
approved these changes
Sep 7, 2026
dghgit
left a comment
Contributor
There was a problem hiding this comment.
Code passes sanity check, nice reduction, some reduction in public API as well.
Fable review was okay, I've sent you the report, there was one possible regression and a couple minor things picked up. I think they all predate this PR though.
…be in the public API surface because they are hidden behind the type alias MLKEM512
…nd therefore would not get zeroized-on-drop.
…ust into refactor/algparams_traits
…cessary extra memory.
…ams hygeine and integration tests.
dghgit
reviewed
Sep 7, 2026
Contributor
There was a problem hiding this comment.
Good to see. I wonder if there's some one we can tell our electronic friends to keep a watch out for patterns like this as well, it's the kind of thing that's very easy to do, even in a post-LLM world.
dghgit
pushed a commit
that referenced
this pull request
Sep 7, 2026
…rams/HashMLDSAParams/MLKEMParams traits, one impl per parameter set (#117)
Contributor
|
Merged on release/0.1.3alpha |
hubot
pushed a commit
that referenced
this pull request
Sep 9, 2026
…rams/HashMLDSAParams/MLKEMParams traits, one impl per parameter set (#117)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Solution: #105 introduces a really nice pattern for encapsulating AES params, and I want to also apply this to the ML-DSA and ML-KEM crates.
The pattern is to hide all the params in a pub trait, with a private trait that seals it.
The entrypoint to start the review is: the new
trait MLDSAParamsin params.rs.This allows for greatly simplifying the previous turbofish construction, which was getting quite out of hand, brittle, and easy to make a mistake.
It used to be:
And it's now:
Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com