-
-
Notifications
You must be signed in to change notification settings - Fork 3
Remove CryptoGenerator and reduce BlockRng to BlockBuffer #68
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
387369b
d3dcbb2
f9ff50b
994f56d
dbe91ab
c46cb62
03a4e38
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,29 +1,25 @@ | ||
| //! The [`Generator`] trait and [`BlockRng`] | ||
| //! The [`BlockRng`] trait and [`BlockBuffer`] | ||
| //! | ||
| //! Trait [`Generator`] and marker trait [`CryptoGenerator`] may be implemented | ||
| //! by block-generators; that is PRNGs whose output is a *block* of words, such | ||
| //! as `[u32; 16]`. | ||
| //! Trait [`BlockRng`] may be implemented by block-generators; that is PRNGs | ||
| //! whose output is a *block* of words, such as `[u32; 16]`. | ||
| //! | ||
| //! The struct [`BlockRng`] wraps such a [`Generator`] together with an output | ||
| //! buffer and implements several methods (e.g. [`BlockRng::next_word`]) to | ||
| //! assist in the implementation of [`TryRng`]. Note that (unlike in earlier | ||
| //! versions of `rand_core`) [`BlockRng`] itself does not implement [`TryRng`] | ||
| //! since in practice we found it was always beneficial to use a wrapper type | ||
| //! over [`BlockRng`]. | ||
| //! The struct [`BlockBuffer`] may be used with a [`BlockRng`] to implement | ||
| //! [`TryRng`]. Note that (unlike in earlier versions of `rand_core`) | ||
| //! [`BlockBuffer`] itself does not implement [`TryRng`]. | ||
| //! | ||
| //! # Example | ||
| //! | ||
| //! ``` | ||
| //! use core::convert::Infallible; | ||
| //! use rand_core::{Rng, SeedableRng, TryRng}; | ||
| //! use rand_core::block::{Generator, BlockRng}; | ||
| //! use rand_core::block::{BlockRng, BlockBuffer}; | ||
| //! | ||
| //! struct MyRngCore { | ||
| //! // Generator state ... | ||
| //! # state: [u32; 8], | ||
| //! } | ||
| //! | ||
| //! impl Generator for MyRngCore { | ||
| //! impl BlockRng for MyRngCore { | ||
| //! type Output = [u32; 8]; | ||
| //! | ||
| //! fn generate(&mut self, output: &mut Self::Output) { | ||
|
|
@@ -32,17 +28,22 @@ | |
| //! } | ||
| //! } | ||
| //! | ||
| //! // Our RNG is a wrapper over BlockRng | ||
| //! pub struct MyRng(BlockRng<MyRngCore>); | ||
| //! // Our RNG is a wrapper over BlockBuffer | ||
| //! pub struct MyRng { | ||
| //! core: MyRngCore, | ||
| //! buffer: BlockBuffer<MyRngCore>, | ||
| //! } | ||
| //! | ||
| //! impl SeedableRng for MyRng { | ||
| //! type Seed = [u8; 32]; | ||
| //! fn from_seed(seed: Self::Seed) -> Self { | ||
| //! let core = MyRngCore { | ||
| //! // ... | ||
| //! # state: rand_core::utils::read_words(&seed), | ||
| //! }; | ||
| //! MyRng(BlockRng::new(core)) | ||
| //! MyRng { | ||
| //! core: MyRngCore { | ||
| //! // ... | ||
| //! # state: rand_core::utils::read_words(&seed), | ||
| //! }, | ||
| //! buffer: BlockBuffer::default(), | ||
| //! } | ||
| //! } | ||
| //! } | ||
| //! | ||
|
|
@@ -51,17 +52,17 @@ | |
| //! | ||
| //! #[inline] | ||
| //! fn try_next_u32(&mut self) -> Result<u32, Infallible> { | ||
| //! Ok(self.0.next_word()) | ||
| //! Ok(self.buffer.next_word(&mut self.core)) | ||
| //! } | ||
| //! | ||
| //! #[inline] | ||
| //! fn try_next_u64(&mut self) -> Result<u64, Infallible> { | ||
| //! Ok(self.0.next_u64_from_u32()) | ||
| //! Ok(self.buffer.next_u64_from_u32(&mut self.core)) | ||
| //! } | ||
| //! | ||
| //! #[inline] | ||
| //! fn try_fill_bytes(&mut self, bytes: &mut [u8]) -> Result<(), Infallible> { | ||
| //! Ok(self.0.fill_bytes(bytes)) | ||
| //! Ok(self.buffer.fill_bytes(&mut self.core, bytes)) | ||
| //! } | ||
| //! } | ||
| //! | ||
|
|
@@ -72,23 +73,13 @@ | |
| //! # assert_eq!(rng.next_u32(), 1171109249); | ||
| //! ``` | ||
| //! | ||
| //! # ReseedingRng | ||
| //! | ||
| //! The [`Generator`] trait supports usage of [`rand::rngs::ReseedingRng`]. | ||
| //! This requires that [`SeedableRng`] be implemented on the "core" generator. | ||
| //! Additionally, it may be useful to implement [`CryptoGenerator`]. | ||
| //! (This is in addition to any implementations on an [`TryRng`] type.) | ||
| //! | ||
| //! [`Generator`]: crate::block::Generator | ||
| //! [`TryRng`]: crate::TryRng | ||
| //! [`SeedableRng`]: crate::SeedableRng | ||
| //! [`rand::rngs::ReseedingRng`]: https://docs.rs/rand/latest/rand/rngs/struct.ReseedingRng.html | ||
|
|
||
| use crate::utils::Word; | ||
| use core::fmt; | ||
|
|
||
| /// A random (block) generator | ||
| pub trait Generator { | ||
| pub trait BlockRng { | ||
| /// The output type. | ||
| /// | ||
| /// For use with [`rand_core::block`](crate::block) code this must be `[u32; _]` or `[u64; _]`. | ||
|
|
@@ -98,32 +89,12 @@ pub trait Generator { | |
| /// | ||
| /// This must fill `output` with random data. | ||
| fn generate(&mut self, output: &mut Self::Output); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe rename the method to |
||
|
|
||
| /// Destruct the output buffer | ||
| /// | ||
| /// This method is called on [`Drop`] of the [`Self::Output`] buffer. | ||
| /// The default implementation does nothing. | ||
| #[inline] | ||
| fn drop(&mut self, output: &mut Self::Output) { | ||
| let _ = output; | ||
| } | ||
| } | ||
|
|
||
| /// A cryptographically secure generator | ||
| /// | ||
| /// This is a marker trait used to indicate that a [`Generator`] implementation | ||
| /// is supposed to be cryptographically secure. | ||
| /// | ||
| /// Mock generators should not implement this trait *except* under a | ||
| /// `#[cfg(test)]` attribute to ensure that mock "crypto" generators cannot be | ||
| /// used in production. | ||
| /// | ||
| /// See [`TryCryptoRng`](crate::TryCryptoRng) docs for more information. | ||
| pub trait CryptoGenerator: Generator {} | ||
|
|
||
| /// RNG functionality for a block [`Generator`] | ||
| /// Buffer providing RNG methods over a [`BlockRng`] | ||
| /// | ||
| /// This type encompasses a [`Generator`] [`core`](Self::core) and a buffer. | ||
| /// This type does not encapuslate a [`BlockRng`], but is designed to be used | ||
| /// alongside one. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be nice to explicitly document that |
||
| /// It provides optimized implementations of methods required by an [`Rng`]. | ||
| /// | ||
| /// All values are consumed in-order of generation. No whole words (e.g. `u32` | ||
|
|
@@ -133,60 +104,41 @@ pub trait CryptoGenerator: Generator {} | |
| /// | ||
| /// [`Rng`]: crate::Rng | ||
| #[derive(Clone)] | ||
| pub struct BlockRng<G: Generator> { | ||
| #[allow(missing_debug_implementations)] | ||
| pub struct BlockBuffer<G: BlockRng> { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use |
||
| results: G::Output, | ||
| /// The *core* part of the RNG, implementing the `generate` function. | ||
| pub core: G, | ||
| } | ||
|
|
||
| // Custom Debug implementation that does not expose the contents of `results`. | ||
| impl<G> fmt::Debug for BlockRng<G> | ||
| where | ||
| G: Generator + fmt::Debug, | ||
| { | ||
| fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { | ||
| fmt.debug_struct("BlockRng") | ||
| .field("core", &self.core) | ||
| .finish_non_exhaustive() | ||
| } | ||
| } | ||
|
|
||
| impl<G: Generator> Drop for BlockRng<G> { | ||
| fn drop(&mut self) { | ||
| self.core.drop(&mut self.results); | ||
| } | ||
| } | ||
|
|
||
| impl<W: Word + Default, const N: usize, G: Generator<Output = [W; N]>> BlockRng<G> { | ||
| /// Create a new `BlockRng` from an existing RNG implementing | ||
| /// `Generator`. Results will be generated on first use. | ||
| impl<W: Word + Default, const N: usize, G: BlockRng<Output = [W; N]>> Default for BlockBuffer<G> { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| #[inline] | ||
| pub fn new(core: G) -> BlockRng<G> { | ||
| fn default() -> BlockBuffer<G> { | ||
| let mut results = [W::default(); N]; | ||
| results[0] = W::from_usize(N); | ||
| BlockRng { core, results } | ||
| BlockBuffer { results } | ||
| } | ||
| } | ||
|
|
||
| impl<W: Word + Default, const N: usize, G: BlockRng<Output = [W; N]>> BlockBuffer<G> { | ||
| /// Reconstruct from a core and a remaining-results buffer. | ||
| /// | ||
| /// This may be used to deserialize using a `core` and the output of | ||
| /// [`Self::remaining_results`]. | ||
| /// | ||
| /// Returns `None` if `remaining_results` is too long. | ||
| pub fn reconstruct(core: G, remaining_results: &[W]) -> Option<Self> { | ||
| pub fn reconstruct(remaining_results: &[W]) -> Option<Self> { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this method used in practice? I think we had a discussion about this and came to conclusion that it's better to use serialization/deserialization over
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, it is used: https://github.com/rust-random/rngs/blob/master/rand_isaac/src/isaac.rs#L171 And no, that is the conclusion that you came to. My conclusion was that where fixed-size serialization is preferred it is still possible with this approach (since
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, whatever... But at least add methods for fixed-sized (de)serialization. It would mean that your struct has two ways of handling serialization, but so be it.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The existing API is already usable for fixed-size serialization though (with slightly more code elsewhere, but storing the length is not exactly hard).
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are you seriously suggesting that users should manually implement the fixed-size serialization based on the variable sized methods???
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. And why not? It's possible. It's not that much extra code, and probably won't be used in that many cases anyway. It's not even particularly inefficient, if that even matters for serialisation.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok... Feel free to do with this module whatever you want. I will not be reviewing/approving PRs which deal with block stuff anymore.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am confused. But as you wish. |
||
| let mut results = [W::default(); N]; | ||
| if remaining_results.len() < N { | ||
| let index = N - remaining_results.len(); | ||
| results[index..].copy_from_slice(remaining_results); | ||
| results[0] = W::from_usize(index); | ||
| Some(BlockRng { results, core }) | ||
| Some(BlockBuffer { results }) | ||
| } else { | ||
| None | ||
| } | ||
| } | ||
| } | ||
|
|
||
| impl<W: Word, const N: usize, G: Generator<Output = [W; N]>> BlockRng<G> { | ||
| impl<W: Word, const N: usize, G: BlockRng<Output = [W; N]>> BlockBuffer<G> { | ||
| /// Get the index into the result buffer. | ||
| /// | ||
| /// If this is equal to or larger than the size of the result buffer then | ||
|
|
@@ -214,14 +166,14 @@ impl<W: Word, const N: usize, G: Generator<Output = [W; N]>> BlockRng<G> { | |
| /// This method will panic if `n >= N` where `N` is the buffer size (in | ||
| /// words). | ||
| #[inline] | ||
| pub fn reset_and_skip(&mut self, n: usize) { | ||
| pub fn reset_and_skip(&mut self, core: &mut G, n: usize) { | ||
| if n == 0 { | ||
| self.set_index(N); | ||
| return; | ||
| } | ||
|
|
||
| assert!(n < N); | ||
| self.core.generate(&mut self.results); | ||
| core.generate(&mut self.results); | ||
| self.set_index(n); | ||
| } | ||
|
|
||
|
|
@@ -238,7 +190,7 @@ impl<W: Word, const N: usize, G: Generator<Output = [W; N]>> BlockRng<G> { | |
| /// Access the unused part of the results buffer | ||
| /// | ||
| /// The length of the returned slice is guaranteed to be less than the | ||
| /// length of `<Self as Generator>::Output` (i.e. less than `N` where | ||
| /// length of `<Self as BlockRng>::Output` (i.e. less than `N` where | ||
| /// `Output = [W; N]`). | ||
| /// | ||
| /// This is a low-level interface intended for serialization. | ||
|
|
@@ -251,10 +203,10 @@ impl<W: Word, const N: usize, G: Generator<Output = [W; N]>> BlockRng<G> { | |
|
|
||
| /// Generate the next word (e.g. `u32`) | ||
| #[inline] | ||
| pub fn next_word(&mut self) -> W { | ||
| pub fn next_word(&mut self, core: &mut G) -> W { | ||
| let mut index = self.index(); | ||
| if index >= N { | ||
| self.core.generate(&mut self.results); | ||
| core.generate(&mut self.results); | ||
| index = 0; | ||
| } | ||
|
|
||
|
|
@@ -264,10 +216,10 @@ impl<W: Word, const N: usize, G: Generator<Output = [W; N]>> BlockRng<G> { | |
| } | ||
| } | ||
|
|
||
| impl<const N: usize, G: Generator<Output = [u32; N]>> BlockRng<G> { | ||
| impl<const N: usize, G: BlockRng<Output = [u32; N]>> BlockBuffer<G> { | ||
| /// Generate a `u64` from two `u32` words | ||
| #[inline] | ||
| pub fn next_u64_from_u32(&mut self) -> u64 { | ||
| pub fn next_u64_from_u32(&mut self, core: &mut G) -> u64 { | ||
| let index = self.index(); | ||
| let mut new_index; | ||
| let (mut lo, mut hi); | ||
|
|
@@ -277,7 +229,7 @@ impl<const N: usize, G: Generator<Output = [u32; N]>> BlockRng<G> { | |
| new_index = index + 2; | ||
| } else { | ||
| lo = self.results[N - 1]; | ||
| self.core.generate(&mut self.results); | ||
| core.generate(&mut self.results); | ||
| hi = self.results[0]; | ||
| new_index = 1; | ||
| if index >= N { | ||
|
|
@@ -291,15 +243,15 @@ impl<const N: usize, G: Generator<Output = [u32; N]>> BlockRng<G> { | |
| } | ||
| } | ||
|
|
||
| impl<W: Word, const N: usize, G: Generator<Output = [W; N]>> BlockRng<G> { | ||
| impl<W: Word, const N: usize, G: BlockRng<Output = [W; N]>> BlockBuffer<G> { | ||
| /// Fill `dest` | ||
| #[inline] | ||
| pub fn fill_bytes(&mut self, dest: &mut [u8]) { | ||
| pub fn fill_bytes(&mut self, core: &mut G, dest: &mut [u8]) { | ||
dhardy marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| let mut read_len = 0; | ||
| let mut index = self.index(); | ||
| while read_len < dest.len() { | ||
| if index >= N { | ||
| self.core.generate(&mut self.results); | ||
| core.generate(&mut self.results); | ||
| index = 0; | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update changelog.