Update random and cryptography crates#8010
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughRustPython upgrades workspace and stdlib crypto/RNG dependencies, adapts hashlib to use SimpleHmac for SHA‑3 and a fixed SHAKE block size, changes random seeding to use rand::rng(), and updates a test import for rand 0.10. ChangesCryptographic and RNG Dependency Modernization
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Cargo.toml`:
- Line 254: The comment on the mt19937 dependency entry ("mt19937 = \"3.3\" #
upgrade it once rand is upgraded") is now stale because rand has already been
bumped to 0.10; update or remove that trailing comment on the mt19937 line so it
no longer instructs future readers to upgrade mt19937 once rand is
upgraded—locate the mt19937 = "3.3" entry in Cargo.toml and either delete the
outdated comment or replace it with a short factual note reflecting the current
state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 732efa0d-2cbf-4d7c-977a-c7420cac61d0
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
Cargo.tomlcrates/literal/src/float.rscrates/stdlib/Cargo.tomlcrates/stdlib/src/hashlib.rscrates/stdlib/src/random.rs
74122c6 to
2e14a1e
Compare
| Self::Shake128(_) => Shake128::block_size(), | ||
| Self::Shake256(_) => Shake256::block_size(), | ||
| Self::Shake128(_) => 168, | ||
| Self::Shake256(_) => 136, |
There was a problem hiding this comment.
aren't those values match ot Shake{128,256}::block_size()? then why does it happen? do those block_size refer different thiing?
There was a problem hiding this comment.
block_size() doesn't exist anymore, so I had to hard code the values. For XOF (SHAKE and other algorithms), the block size is just the rate in bytes.
I'll check if there's a nicer way to get the block size without hard coding it. 😁
There was a problem hiding this comment.
The shake crate doesn't provide a different way to get the rate. 😓 However, the rate wouldn't change so it's probably okay to just hard code it.
|
Could you please try rebasing this branch onto a fresh copy of |
| checksum = "613afe47fcd5fac7ccf1db93babcb082c5994d996f20b8b159f2ad1658eb5724" | ||
|
|
||
| [[package]] | ||
| name = "chacha20" |
There was a problem hiding this comment.
Does this need to be added to the random category in .github/dependabot.yml?
There was a problem hiding this comment.
I'm not sure about this one since it's not a direct dependency. Should I add it anyway?
There was a problem hiding this comment.
I seem to recall that we've had problems in the past where Dependabot would update a dependency only found in Cargo.lock and the build would break. It probably wouldn't hurt to add it.
e4abecf to
25df0c3
Compare
Redo of RustPython#7800. Bumping the random and crypto crates is not trivial. Both sets of crates have had API changes which require manual updates. AI use: I wrote all of the code myself but used AI to figure out the new APIs. AI explained SHAKE's block size and why the block_size() function doesn't exist in the new crate. I confirmed the block size, which is actually represented in the new types for SHAKE. `Shake128` is actually `Shake<168>` - 168 is the block size. I cited Wikipedia as it lists the SHAKE block size in bits. I also used AI to figure out HMAC for SHA3. Assisted-by: Codex:gpt-5.4
25df0c3 to
f904031
Compare
Redo of #7800.
Bumping the random and crypto crates is not trivial. Both sets of crates have had API changes which require manual updates.
AI use:
I wrote all of the code myself but used AI to figure out the new APIs. AI explained SHAKE's block size and why the block_size() function doesn't exist in the new crate. I confirmed the block size, which is actually represented in the new types for SHAKE. Shake128 is actually
Shake<168>- 168 is the block size. I cited Wikipedia as it lists the SHAKE block size in bits.I also used AI to figure out HMAC for SHA3.
Assisted-by: Codex:gpt-5.4
Summary
shakecrate.shakeused to be part of thesha3crate but the RustCrypto maintainers moved it to its own crate.Fun fact: Both the LLM and I were horribly confused at the HMAC issue.
Summary by CodeRabbit
Chores
Refactor
Bug Fixes