Chacha: integrate c2-chacha dependency#931
Conversation
| } | ||
|
|
||
| #[inline(always)] | ||
| pub fn set_stream_param(&mut self, param: u32, value: u64) { |
There was a problem hiding this comment.
You might want to cut all the *_stream_param stuff--it's unused and not verified against any official test vectors.
There was a problem hiding this comment.
We do use this in set_word_pos and set_stream methods on the RNGs.
We have a couple of our own tests using these methods, although only with three constants in total.
|
This makes sense. I'll be officially discontinuing and freezing my crate since rand turned out to be its only direct user (besides what I originally wrote it for, which is defunct). There's some simplification I plan to do to the ppv-lite86 API eventually, mostly consolidating traits. I'll probably do that when I add the next backend (avx512, NEON, WASM, and packed_simd are planned eventually, mostly waiting on You might want to annex some of my test cases too; they're closer to the implementation and able to cover some edge cases the rand_chacha tests probably don't. I've also just noticed that I SIMD-accelerated a few top-level functions that aren't performance-sensitive, so I've written a patch to just do those bits with portable code. It's a minor change but to keep things simple I'll PR it separately after this lands. |
|
I see no reason not to merge this now. All happy? (Whether we replace this crate is another issue that doesn't need to influence this PR — I'll make a new issue.) |
|
In the end you merged c2-chacha into rand_chacha, so the chacha used here no longer exists stand alone? I suppose doing this sounds unavoidable because rand supplies its own BlockRng which differs from that supplied by stream ciphers. I guess the chacha crate https://github.com/RustCrypto/stream-ciphers/tree/master/chacha20 is a dependency zoo until const generics land, and still significantly slower. |
|
Less 'unavoidable' than 'an unnecessary extra step'. This crate ( |
It only has two mandatory dependencies: Regarding performance: RustCrypto/stream-ciphers#267 (comment)
It does now! |
|
Looks like the performance gap is closed, that's very nice! Migrating to chacha20 would require bumping the MSRV to 1.51. |
|
It looks like chacha20 always tries to uses SSE2 on x86 and x86_64 even if it isn't available. It only checks if AVX2 exists. This is will cause issues on certain CPUs. While the doc comments says "the |
|
@bjorn3 I opened RustCrypto/stream-ciphers#270 for that |
|
Thanks! |
This adds approx 250 lines of code to
rand_chachafrom thec2-chachacrate and reduces the dependency count by 1. It also means thec2-chachacrate no longer needs to feature-gate the "primary" API it implements. Closes #872.Since there is no breaking change to
rand_chacha, the version bump is merely a patch version.Ideally I'd like to see @kazcw's approval. Licences are the same so no problem there.