Conversation
|
I opened #1643. This PR is useful as a draft but won't be merged in its current form (likely we'll want the MSRV/edition bump first as its own PR). I'm not certain on the timeline yet; the main blocker is the |
0fbb567 to
e41e428
Compare
|
@dhardy I've rebased and the new tests failed as expected (the default ChaCha12 in chacha20 is the IETF variant). I tried to change to the "Legacy" variant, but chacha20 only exports ChaCha20Legacy, not the 12 round variant that we will need here. The ChaCha20Legacy type is just an alias, so I tried to use the underlying type directly to make a 12 round variant ( @tarcieri It seems that I might stuck here until someone either exports |
It's an oversight, we should export it. Could you create a PR? It also may be worth to add aliases for the legacy variants as well. |
|
@hpenne I'm confused, why do you need a |
|
@tarcieri When I add the "legacy" feature to the chacha20 crate dependency in the rand cargo.toml, the chacha20 crate fails to build: Seems to work fine when i build run tests in chacha20 itself. Odd. |
|
@hpenne all of the RNG types now have a 64-bit counter as of the latest prerelease. Also looks like you found a bug. Perhaps it was being hidden by feature unification? Strange. |
|
@hpenne oh, you're using an out-of-date version! Please upgrade to v0.10.0-rc.1 |
…sions that are no longer needed.
@tarcieri That was embarrassing. Works much better with the correct version. All tests pass now. The only strangeness that I am left with is that if I do not enable the "legacy" feature (the only enabled feature is "rng"), chacha20 fails to build with: I'm not able to reproduce this when I build chacha20 locally from master, so perhaps I've done something wrong or you have already fixed this on master. I'll look closer tomorrow. |
|
@hpenne aah that was RustCrypto/stream-ciphers#454 which has been fixed I can cut an |
dhardy
left a comment
There was a problem hiding this comment.
Sorry, I did not think to appraise you of the chacha20 changes. I think we have all the pieces to make a rand pre-release now.
examples/rayon-monte-carlo.rs
Outdated
| rng.set_stream(i); | ||
| rng.set_stream(u64::from(i)); |
There was a problem hiding this comment.
Yes. I used to need to convert to u128 with the old chacha20. I had to change u64 when I rebased but failed to notice that I could simply remove the conversion. Will fix.
Cargo.toml
Outdated
| rand_chacha = { path = "rand_chacha", version = "0.9.0", default-features = false, optional = true } | ||
| chacha20 = { version = "=0.10.0-rc.1", default-features = false, features = ["rng", "legacy"], optional = true } |
There was a problem hiding this comment.
We can depend on the "legacy" feature for now, but add a TODO comment to remove it on the next version.
CHANGELOG.md
Outdated
|
|
||
| ## [0.10.0 — Unreleased] | ||
| ### Changes | ||
| - The dependency on `rand_chacha` has been replaced with a dependency on `chacha20`. This changes the implementation behind `StdRng`, but the output remains the same. There may be some API breakage when using the ChaCha-types directly as these are now the ones in `chacha20` instead of `rand_chacha`. |
…:from in rayon-monte-carlo.rs. Added a ToDo in Cargo.toml to remove the "legacy" feature of chacha20.
Cargo.toml
Outdated
| # ToDo: Remove the "legacy" feature from chacha20 when this is not longer necessary | ||
| chacha20 = { version = "=0.10.0-rc.1", default-features = false, features = ["rng", "legacy"], optional = true } |
There was a problem hiding this comment.
I've released an rc.2 with the feature-related bugfix:
| # ToDo: Remove the "legacy" feature from chacha20 when this is not longer necessary | |
| chacha20 = { version = "=0.10.0-rc.1", default-features = false, features = ["rng", "legacy"], optional = true } | |
| # ToDo: Remove the "legacy" feature from chacha20 when this is not longer necessary | |
| chacha20 = { version = "=0.10.0-rc.2", default-features = false, features = ["rng"], optional = true } |
There was a problem hiding this comment.
Done. I removed the ToDo-comment as well, now that the "legacy" feature is no longer necessary.
CHANGELOG.mdentrySummary
Replaced the dependency on
rand_chachawith one onchacha20. Added some tests tostd.rsto ensure that the output ofStdRngdid not change.Fixes #934.
Motivation
Reduces total code size and the total amount of unsafe code.
Details
Changes to config.toml and some replacement of
rand_chacha::withchacha20::.Added three new unit tests to std.rs. These were based on tests of IETF test vectors from
rand_chacha, but the actualexpectedvalues had to be replaced, as the IETF test vectors are for ChaCha20 whileranduses ChaCha12. Theexpectedvalues were generated by usingrand_chacha(beforechacha20was used) to verify that the algorithm change did not affect the output.