refactor(rng): move seed and helper crates to rand 0.10#244
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (22)
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request initiates a strategic upgrade of the project's random number generation infrastructure. It introduces a dual-version approach for the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request is a well-executed refactoring to upgrade several crates to use rand version 0.10, while maintaining compatibility with legacy crates that depend on rand 0.8. The use of workspace aliases to manage the two different rand versions is a clean solution. The code changes correctly adapt to the new rand 0.10 API, including updating test utilities to use TryRng. I've found one minor opportunity for improvement by removing a duplicated test helper, but overall the changes are solid.
| struct ConstantRng; | ||
|
|
||
| impl RngCore for ConstantRng { | ||
| fn next_u32(&mut self) -> u32 { | ||
| u32::from_le_bytes([255, 255, 255, 255]) | ||
| } | ||
| impl TryRng for ConstantRng { | ||
| type Error = Infallible; | ||
|
|
||
| fn next_u64(&mut self) -> u64 { | ||
| u64::from_le_bytes([255, 255, 255, 255, 255, 255, 255, 255]) | ||
| fn try_next_u32(&mut self) -> Result<u32, Self::Error> { | ||
| Ok(u32::from_le_bytes([255, 255, 255, 255])) | ||
| } | ||
|
|
||
| fn fill_bytes(&mut self, dest: &mut [u8]) { | ||
| dest.fill(255); | ||
| fn try_next_u64(&mut self) -> Result<u64, Self::Error> { | ||
| Ok(u64::from_le_bytes([255, 255, 255, 255, 255, 255, 255, 255])) | ||
| } | ||
|
|
||
| fn try_fill_bytes(&mut self, dest: &mut [u8]) -> Result<(), rand_core::Error> { | ||
| fn try_fill_bytes(&mut self, dest: &mut [u8]) -> Result<(), Self::Error> { | ||
| self.fill_bytes(dest); | ||
| Ok(()) | ||
| } | ||
| } | ||
|
|
||
| impl ConstantRng { | ||
| fn fill_bytes(&mut self, dest: &mut [u8]) { | ||
| dest.fill(255); | ||
| } | ||
| } |
There was a problem hiding this comment.
This ConstantRng test utility is functionally identical to AllMaxRng defined below. Both produce a stream of 0xFF bytes and return u32::MAX and u64::MAX for try_next_u32 and try_next_u64 respectively.
To reduce code duplication, you can remove ConstantRng and its implementations, and use AllMaxRng in the fallback_path_terminates_for_constant_rng test instead.
c99b664 to
bc58f69
Compare
9f87ad0 to
88cae74
Compare
Summary
This is PR 2 in the RNG upgrade lane, stacked on top of #243.
It moves the seed/core/helper layer to the current RNG line while leaving the stable crypto-edge crates on the legacy line that upstream still requires.
What changed
rand 0.8/rand_core 0.6/rand_chacha 0.3rand10 0.10/rand_core10 0.10/rand_chacha10 0.100.10line:uselesskey-core-factoryuselesskey-core-seeduselesskey-core-base62uselesskey-core-token-shapeuselesskey-core-x509-deriveuselesskey-hmacrand 0.10API shape:OsRng->rand10::rngs::SysRngRngCore->rand_core10::RngTryRnguselesskey-core,uselesskey-core-token, anduselesskey-core-x509Why this split
Stable upstream crypto crates still pin the legacy RNG ABI:
rsa 0.9.10ed25519-dalek 2.2.0p256/p384/elliptic-curve 0.13linepgp 0.19.0So this PR does not try to force full convergence. It moves the parts of the workspace where RNG is an implementation detail, and leaves the crypto-edge island explicit for a follow-up.
Remaining legacy island after this PR
uselesskey-rsauselesskey-ed25519(transitively constrained)uselesskey-ecdsauselesskey-pgpuselesskey-x509proptestdev-deps still bring a separaterand 0.9test-only lineTesting
cargo test -p uselesskey-core-seed -p uselesskey-core-factory -p uselesskey-core-base62 -p uselesskey-core-token-shape -p uselesskey-core-x509-derive -p uselesskey-hmac --no-runcargo test -p uselesskey-core-seed -p uselesskey-core-factory -p uselesskey-core-base62 -p uselesskey-core-token-shape -p uselesskey-core-x509-derive -p uselesskey-hmaccargo test -p uselesskey-core -p uselesskey-core-token -p uselesskey-core-x509cargo xtask gateFollow-up
A separate follow-up PR can migrate local/test-only legacy users such as
uselesskey-rustcrypto,uselesskey-bdd-steps, and the fuzz crate without waiting on upstream crypto releases.