Skip to content

dyn RNG#43

Open
ounsworth wants to merge 12 commits into
bcgit:release/0.1.2alphafrom
ounsworth:feature/mikeo/dyn_rng_v2
Open

dyn RNG#43
ounsworth wants to merge 12 commits into
bcgit:release/0.1.2alphafrom
ounsworth:feature/mikeo/dyn_rng_v2

Conversation

@ounsworth

Copy link
Copy Markdown
Contributor

Things that require randomness can now be handed an instance of &mut dyn RNG, including allowing users of the library to impl their own RNG and use it, for example, as the entropy source for an ML-DSA keygen.

@ounsworth ounsworth mentioned this pull request Jun 25, 2026
…hecks the security_strength of the RNG it was handed.
@ounsworth

Copy link
Copy Markdown
Contributor Author

@officialfrancismendoza Does b95fed9 address your first comment here?

@ounsworth

ounsworth commented Jun 27, 2026

Copy link
Copy Markdown
Contributor Author

A note on this change to traits.rs:

OLD

    fn add_seed_keymaterial(
        &mut self,
        additional_seed: impl KeyMaterialTrait,

NEW

    fn add_seed_keymaterial(
        &mut self,
        additional_seed: &dyn KeyMaterialTrait,

It was nice that this was a move and consumed the additional_seed in order to make it difficult for the user to accidentally use it twice, unfortunately, having a method that takes a moved KeyMaterialTrait breaks dyn-compatability of the RNG trait, so this has to be a borrow now. See this comment for more detail.

@officialfrancismendoza

officialfrancismendoza commented Jun 29, 2026

Copy link
Copy Markdown

@officialfrancismendoza Does b95fed9 address your first comment here?

@ounsworth yes, this addresses my first comment functionally. However (nitpick), I don’t see a regression test specifically where encaps_from_rng() rejects an insufficient-strength RNG; the tests added seem focused on keygen. Did we want to add that just in case? If not, everything else looks good.

@ounsworth

Copy link
Copy Markdown
Contributor Author

@officialfrancismendoza Oh that's not a nitpick, that's a great catch! Thanks for double-checking my work carefully!

@officialfrancismendoza

Copy link
Copy Markdown

@ounsworth looks good. Just for the record: I made sure to push upstream to source (David's bare metal server), as changes on this public mirror are overwritten on a regular interval. Since that's now reflected in 0.1.2alpha, approving this feature branch.

@officialfrancismendoza officialfrancismendoza left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Approved and squash-merged to 0.1.2alpha on source (to reflect on the public mirror here). LGTM!

hubot pushed a commit that referenced this pull request Jul 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants