Remove CryptoGenerator and reduce BlockRng to BlockBuffer by dhardy · Pull Request #68 · rust-random/rand_core

dhardy · 2026-01-28T15:56:12Z

Added a CHANGELOG.md entry

Motivation

rust-random/rand#1722 makes this useless.

Other

Any suggestions for a new name for trait Generator? As-is it matches its most important generate method, so the name doesn't seem too bad.

newpavlov · 2026-01-28T16:01:48Z

Any suggestions for a new name for trait Generator?

BlockRng would be the most consistent name with BlockRng::generate_block (or next_block) method. Instead of the wrapper I would prefer to have a separate BlockBuffer struct.

dhardy · 2026-01-28T16:07:11Z

The names are consistent.

What is the motivation for making BlockBuffer separate? It makes little sense to me (without removing the trait as in the more recent #24 design).

newpavlov · 2026-01-28T16:16:52Z

Separation of concerns. IMO struct Rng { core: RngCore, buffer: BlockBuffer<..> } is more transparent than struct Rng(BlockRng(RngCore));. And zeroization support would look less confusing. It's easy to understand what happens in:

self.core.zeroize();self.buffer = Default::default();
zeroize::optimization_barrier(&self.buffer)

While the wrapper would result in a less straightforward code.

The same applies to serialization as well.

dhardy · 2026-01-28T16:53:28Z

Separation of concerns

If they were truly separate components I'd agree, but the only pub methods not needing both parameters are word_offset and remaining_results.

I see you succeeded in convincing @tarcieri to make optimization_barrier public in RustCrypto/utils#1261, though it's not in a released version yet.

dhardy · 2026-01-28T17:12:34Z

@newpavlov I made significant additions to this PR; please re-review.

newpavlov · 2026-01-28T18:28:13Z

-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> {


Word is already bounded by Default via the Sealed trait.
~~And I think we can simplify the bound to just G::Output: Default, no?~~ UPD: No, we use Word functionality in the implementation.

newpavlov · 2026-01-28T18:30:30Z

 ///
 /// 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> {


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 [W; N] instead of &[W].

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 N is known), while the reverse is not with your approach (and variable length serialization is preferable if the output format happens to be something like JSON).

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.

The existing API is already usable for fixed-size serialization though (with slightly more code elsewhere, but storing the length is not exactly hard).

Are you seriously suggesting that users should manually implement the fixed-size serialization based on the variable sized methods???

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.

Ok... Feel free to do with this module whatever you want. I will not be reviewing/approving PRs which deal with block stuff anymore.

I am confused. But as you wish.

newpavlov · 2026-01-28T18:32:45Z

 - `BlockRng::reset` method ([#44])
 - `BlockRng::index` method (replaced with `BlockRng::word_offset`) ([#44])
 - `Generator::Item` associated type ([#26])
+- `CryptoBlockRng` ([#68])


Update changelog.

newpavlov · 2026-01-28T18:33:35Z

 #[derive(Clone)]
-pub struct BlockRng<G: Generator> {
+#[allow(missing_debug_implementations)]
+pub struct BlockBuffer<G: BlockRng> {


Use R instead of G like in all other bounds?

newpavlov · 2026-01-28T18:38:14Z

@@ -98,32 +89,12 @@ pub trait Generator {
 ///
 /// This must fill `output` with random data.
 fn generate(&mut self, output: &mut Self::Output);


Maybe rename the method to next_block for consistency with the Rng methods?

newpavlov · 2026-01-28T18:46:00Z

 ///
-/// 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.


It would be nice to explicitly document that BlockBuffer::default() initializes all bits of the state to make it friendlier to the optimization_barrier-based zeroization.

dhardy · 2026-01-29T08:24:24Z

Regarding the renaming, the largest problem is actually that we use 'core' to refer to the BlockRng (e.g. in the doc example and as a parameter to methods like next_word). Why 'core'? This is the core (random) generator.

So, BlockRng? The old struct BlockRng referred to a block-based random-number generator. So the name was fine.

BlockBuffer is okay but ultimately a poor name; it buffers results not blocks. ResultsBuffer or just Buffer might work.

The new trait BlockRng is actually a bad name: implementations are (random) (data) block generators, not number generators (no one needs 512-bit numbers).

The old name block::Generator is about as close to "random block generator" as one can get without an unwieldy name I think (unless we want to call it Rbg).

Since the main argument for separation of the core and the buffer seems to be zeroization and alternatives need specific documentation, why don't we just add a method like fn clear?

To avoid complicating history of a single PR too much I will close this one and replace it.

dhardy · 2026-01-28T15:56:12Z

Added a CHANGELOG.md entry

Motivation

rust-random/rand#1722 makes this useless.

Other

Any suggestions for a new name for trait Generator? As-is it matches its most important generate method, so the name doesn't seem too bad.

newpavlov · 2026-01-28T16:01:48Z

Any suggestions for a new name for trait Generator?

BlockRng would be the most consistent name with BlockRng::generate_block (or next_block) method. Instead of the wrapper I would prefer to have a separate BlockBuffer struct.

dhardy · 2026-01-28T16:07:11Z

The names are consistent.

What is the motivation for making BlockBuffer separate? It makes little sense to me (without removing the trait as in the more recent #24 design).

newpavlov · 2026-01-28T16:16:52Z

Separation of concerns. IMO struct Rng { core: RngCore, buffer: BlockBuffer<..> } is more transparent than struct Rng(BlockRng(RngCore));. And zeroization support would look less confusing. It's easy to understand what happens in:

self.core.zeroize();self.buffer = Default::default();
zeroize::optimization_barrier(&self.buffer)

While the wrapper would result in a less straightforward code.

The same applies to serialization as well.

dhardy · 2026-01-28T16:53:28Z

Separation of concerns

If they were truly separate components I'd agree, but the only pub methods not needing both parameters are word_offset and remaining_results.

I see you succeeded in convincing @tarcieri to make optimization_barrier public in RustCrypto/utils#1261, though it's not in a released version yet.

dhardy · 2026-01-28T17:12:34Z

@newpavlov I made significant additions to this PR; please re-review.

newpavlov · 2026-01-28T18:28:13Z

-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> {


Word is already bounded by Default via the Sealed trait.
~~And I think we can simplify the bound to just G::Output: Default, no?~~ UPD: No, we use Word functionality in the implementation.

newpavlov · 2026-01-28T18:30:30Z

 ///
 /// 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> {


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 [W; N] instead of &[W].

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 N is known), while the reverse is not with your approach (and variable length serialization is preferable if the output format happens to be something like JSON).

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.

The existing API is already usable for fixed-size serialization though (with slightly more code elsewhere, but storing the length is not exactly hard).

Are you seriously suggesting that users should manually implement the fixed-size serialization based on the variable sized methods???

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.

Ok... Feel free to do with this module whatever you want. I will not be reviewing/approving PRs which deal with block stuff anymore.

I am confused. But as you wish.

newpavlov · 2026-01-28T18:32:45Z

 - `BlockRng::reset` method ([#44])
 - `BlockRng::index` method (replaced with `BlockRng::word_offset`) ([#44])
 - `Generator::Item` associated type ([#26])
+- `CryptoBlockRng` ([#68])


Update changelog.

newpavlov · 2026-01-28T18:33:35Z

 #[derive(Clone)]
-pub struct BlockRng<G: Generator> {
+#[allow(missing_debug_implementations)]
+pub struct BlockBuffer<G: BlockRng> {


Use R instead of G like in all other bounds?

newpavlov · 2026-01-28T18:38:14Z

@@ -98,32 +89,12 @@ pub trait Generator {
 ///
 /// This must fill `output` with random data.
 fn generate(&mut self, output: &mut Self::Output);


Maybe rename the method to next_block for consistency with the Rng methods?

newpavlov · 2026-01-28T18:46:00Z

 ///
-/// 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.


It would be nice to explicitly document that BlockBuffer::default() initializes all bits of the state to make it friendlier to the optimization_barrier-based zeroization.

dhardy · 2026-01-29T08:24:24Z

Regarding the renaming, the largest problem is actually that we use 'core' to refer to the BlockRng (e.g. in the doc example and as a parameter to methods like next_word). Why 'core'? This is the core (random) generator.

So, BlockRng? The old struct BlockRng referred to a block-based random-number generator. So the name was fine.

BlockBuffer is okay but ultimately a poor name; it buffers results not blocks. ResultsBuffer or just Buffer might work.

The new trait BlockRng is actually a bad name: implementations are (random) (data) block generators, not number generators (no one needs 512-bit numbers).

The old name block::Generator is about as close to "random block generator" as one can get without an unwieldy name I think (unless we want to call it Rbg).

Since the main argument for separation of the core and the buffer seems to be zeroization and alternatives need specific documentation, why don't we just add a method like fn clear?

To avoid complicating history of a single PR too much I will close this one and replace it.

dhardy · 2026-01-28T15:56:12Z

Added a CHANGELOG.md entry

Motivation

rust-random/rand#1722 makes this useless.

Other

Any suggestions for a new name for trait Generator? As-is it matches its most important generate method, so the name doesn't seem too bad.

newpavlov · 2026-01-28T16:01:48Z

Any suggestions for a new name for trait Generator?

BlockRng would be the most consistent name with BlockRng::generate_block (or next_block) method. Instead of the wrapper I would prefer to have a separate BlockBuffer struct.

dhardy · 2026-01-28T16:07:11Z

The names are consistent.

What is the motivation for making BlockBuffer separate? It makes little sense to me (without removing the trait as in the more recent #24 design).

newpavlov · 2026-01-28T16:16:52Z

Separation of concerns. IMO struct Rng { core: RngCore, buffer: BlockBuffer<..> } is more transparent than struct Rng(BlockRng(RngCore));. And zeroization support would look less confusing. It's easy to understand what happens in:

self.core.zeroize();self.buffer = Default::default();
zeroize::optimization_barrier(&self.buffer)

While the wrapper would result in a less straightforward code.

The same applies to serialization as well.

dhardy · 2026-01-28T16:53:28Z

Separation of concerns

If they were truly separate components I'd agree, but the only pub methods not needing both parameters are word_offset and remaining_results.

I see you succeeded in convincing @tarcieri to make optimization_barrier public in RustCrypto/utils#1261, though it's not in a released version yet.

dhardy · 2026-01-28T17:12:34Z

@newpavlov I made significant additions to this PR; please re-review.

newpavlov · 2026-01-28T18:28:13Z

-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> {


Word is already bounded by Default via the Sealed trait.
~~And I think we can simplify the bound to just G::Output: Default, no?~~ UPD: No, we use Word functionality in the implementation.

newpavlov · 2026-01-28T18:30:30Z

 ///
 /// 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> {


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 [W; N] instead of &[W].

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 N is known), while the reverse is not with your approach (and variable length serialization is preferable if the output format happens to be something like JSON).

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.

The existing API is already usable for fixed-size serialization though (with slightly more code elsewhere, but storing the length is not exactly hard).

Are you seriously suggesting that users should manually implement the fixed-size serialization based on the variable sized methods???

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.

Ok... Feel free to do with this module whatever you want. I will not be reviewing/approving PRs which deal with block stuff anymore.

I am confused. But as you wish.

newpavlov · 2026-01-28T18:32:45Z

 - `BlockRng::reset` method ([#44])
 - `BlockRng::index` method (replaced with `BlockRng::word_offset`) ([#44])
 - `Generator::Item` associated type ([#26])
+- `CryptoBlockRng` ([#68])


Update changelog.

newpavlov · 2026-01-28T18:33:35Z

 #[derive(Clone)]
-pub struct BlockRng<G: Generator> {
+#[allow(missing_debug_implementations)]
+pub struct BlockBuffer<G: BlockRng> {


Use R instead of G like in all other bounds?

newpavlov · 2026-01-28T18:38:14Z

@@ -98,32 +89,12 @@ pub trait Generator {
 ///
 /// This must fill `output` with random data.
 fn generate(&mut self, output: &mut Self::Output);


Maybe rename the method to next_block for consistency with the Rng methods?

newpavlov · 2026-01-28T18:46:00Z

 ///
-/// 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.


It would be nice to explicitly document that BlockBuffer::default() initializes all bits of the state to make it friendlier to the optimization_barrier-based zeroization.

dhardy · 2026-01-29T08:24:24Z

Regarding the renaming, the largest problem is actually that we use 'core' to refer to the BlockRng (e.g. in the doc example and as a parameter to methods like next_word). Why 'core'? This is the core (random) generator.

So, BlockRng? The old struct BlockRng referred to a block-based random-number generator. So the name was fine.

BlockBuffer is okay but ultimately a poor name; it buffers results not blocks. ResultsBuffer or just Buffer might work.

The new trait BlockRng is actually a bad name: implementations are (random) (data) block generators, not number generators (no one needs 512-bit numbers).

The old name block::Generator is about as close to "random block generator" as one can get without an unwieldy name I think (unless we want to call it Rbg).

Since the main argument for separation of the core and the buffer seems to be zeroization and alternatives need specific documentation, why don't we just add a method like fn clear?

To avoid complicating history of a single PR too much I will close this one and replace it.

dhardy · 2026-01-28T15:56:12Z

Added a CHANGELOG.md entry

Motivation

rust-random/rand#1722 makes this useless.

Other

Any suggestions for a new name for trait Generator? As-is it matches its most important generate method, so the name doesn't seem too bad.

newpavlov · 2026-01-28T16:01:48Z

Any suggestions for a new name for trait Generator?

BlockRng would be the most consistent name with BlockRng::generate_block (or next_block) method. Instead of the wrapper I would prefer to have a separate BlockBuffer struct.

dhardy · 2026-01-28T16:07:11Z

The names are consistent.

What is the motivation for making BlockBuffer separate? It makes little sense to me (without removing the trait as in the more recent #24 design).

newpavlov · 2026-01-28T16:16:52Z

Separation of concerns. IMO struct Rng { core: RngCore, buffer: BlockBuffer<..> } is more transparent than struct Rng(BlockRng(RngCore));. And zeroization support would look less confusing. It's easy to understand what happens in:

self.core.zeroize();self.buffer = Default::default();
zeroize::optimization_barrier(&self.buffer)

While the wrapper would result in a less straightforward code.

The same applies to serialization as well.

dhardy · 2026-01-28T16:53:28Z

Separation of concerns

If they were truly separate components I'd agree, but the only pub methods not needing both parameters are word_offset and remaining_results.

I see you succeeded in convincing @tarcieri to make optimization_barrier public in RustCrypto/utils#1261, though it's not in a released version yet.

dhardy · 2026-01-28T17:12:34Z

@newpavlov I made significant additions to this PR; please re-review.

newpavlov · 2026-01-28T18:28:13Z

-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> {


Word is already bounded by Default via the Sealed trait.
~~And I think we can simplify the bound to just G::Output: Default, no?~~ UPD: No, we use Word functionality in the implementation.

newpavlov · 2026-01-28T18:30:30Z

 ///
 /// 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> {


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 [W; N] instead of &[W].

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 N is known), while the reverse is not with your approach (and variable length serialization is preferable if the output format happens to be something like JSON).

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.

The existing API is already usable for fixed-size serialization though (with slightly more code elsewhere, but storing the length is not exactly hard).

Are you seriously suggesting that users should manually implement the fixed-size serialization based on the variable sized methods???

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.

Ok... Feel free to do with this module whatever you want. I will not be reviewing/approving PRs which deal with block stuff anymore.

I am confused. But as you wish.

newpavlov · 2026-01-28T18:32:45Z

 - `BlockRng::reset` method ([#44])
 - `BlockRng::index` method (replaced with `BlockRng::word_offset`) ([#44])
 - `Generator::Item` associated type ([#26])
+- `CryptoBlockRng` ([#68])


Update changelog.

newpavlov · 2026-01-28T18:33:35Z

 #[derive(Clone)]
-pub struct BlockRng<G: Generator> {
+#[allow(missing_debug_implementations)]
+pub struct BlockBuffer<G: BlockRng> {


Use R instead of G like in all other bounds?

newpavlov · 2026-01-28T18:38:14Z

@@ -98,32 +89,12 @@ pub trait Generator {
 ///
 /// This must fill `output` with random data.
 fn generate(&mut self, output: &mut Self::Output);


Maybe rename the method to next_block for consistency with the Rng methods?

newpavlov · 2026-01-28T18:46:00Z

 ///
-/// 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.


It would be nice to explicitly document that BlockBuffer::default() initializes all bits of the state to make it friendlier to the optimization_barrier-based zeroization.

dhardy · 2026-01-29T08:24:24Z

Regarding the renaming, the largest problem is actually that we use 'core' to refer to the BlockRng (e.g. in the doc example and as a parameter to methods like next_word). Why 'core'? This is the core (random) generator.

So, BlockRng? The old struct BlockRng referred to a block-based random-number generator. So the name was fine.

BlockBuffer is okay but ultimately a poor name; it buffers results not blocks. ResultsBuffer or just Buffer might work.

The new trait BlockRng is actually a bad name: implementations are (random) (data) block generators, not number generators (no one needs 512-bit numbers).

The old name block::Generator is about as close to "random block generator" as one can get without an unwieldy name I think (unless we want to call it Rbg).

Since the main argument for separation of the core and the buffer seems to be zeroization and alternatives need specific documentation, why don't we just add a method like fn clear?

To avoid complicating history of a single PR too much I will close this one and replace it.

dhardy · 2026-01-28T15:56:12Z

Added a CHANGELOG.md entry

Motivation

rust-random/rand#1722 makes this useless.

Other

Any suggestions for a new name for trait Generator? As-is it matches its most important generate method, so the name doesn't seem too bad.

newpavlov · 2026-01-28T16:01:48Z

Any suggestions for a new name for trait Generator?

BlockRng would be the most consistent name with BlockRng::generate_block (or next_block) method. Instead of the wrapper I would prefer to have a separate BlockBuffer struct.

dhardy · 2026-01-28T16:07:11Z

The names are consistent.

What is the motivation for making BlockBuffer separate? It makes little sense to me (without removing the trait as in the more recent #24 design).

newpavlov · 2026-01-28T16:16:52Z

Separation of concerns. IMO struct Rng { core: RngCore, buffer: BlockBuffer<..> } is more transparent than struct Rng(BlockRng(RngCore));. And zeroization support would look less confusing. It's easy to understand what happens in:

self.core.zeroize();self.buffer = Default::default();
zeroize::optimization_barrier(&self.buffer)

While the wrapper would result in a less straightforward code.

The same applies to serialization as well.

dhardy · 2026-01-28T16:53:28Z

Separation of concerns

If they were truly separate components I'd agree, but the only pub methods not needing both parameters are word_offset and remaining_results.

I see you succeeded in convincing @tarcieri to make optimization_barrier public in RustCrypto/utils#1261, though it's not in a released version yet.

dhardy · 2026-01-28T17:12:34Z

@newpavlov I made significant additions to this PR; please re-review.

newpavlov · 2026-01-28T18:28:13Z

-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> {


Word is already bounded by Default via the Sealed trait.
~~And I think we can simplify the bound to just G::Output: Default, no?~~ UPD: No, we use Word functionality in the implementation.

newpavlov · 2026-01-28T18:30:30Z

 ///
 /// 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> {


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 [W; N] instead of &[W].

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 N is known), while the reverse is not with your approach (and variable length serialization is preferable if the output format happens to be something like JSON).

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.

The existing API is already usable for fixed-size serialization though (with slightly more code elsewhere, but storing the length is not exactly hard).

Are you seriously suggesting that users should manually implement the fixed-size serialization based on the variable sized methods???

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.

Ok... Feel free to do with this module whatever you want. I will not be reviewing/approving PRs which deal with block stuff anymore.

I am confused. But as you wish.

newpavlov · 2026-01-28T18:32:45Z

 - `BlockRng::reset` method ([#44])
 - `BlockRng::index` method (replaced with `BlockRng::word_offset`) ([#44])
 - `Generator::Item` associated type ([#26])
+- `CryptoBlockRng` ([#68])


Update changelog.

newpavlov · 2026-01-28T18:33:35Z

 #[derive(Clone)]
-pub struct BlockRng<G: Generator> {
+#[allow(missing_debug_implementations)]
+pub struct BlockBuffer<G: BlockRng> {


Use R instead of G like in all other bounds?

newpavlov · 2026-01-28T18:38:14Z

@@ -98,32 +89,12 @@ pub trait Generator {
 ///
 /// This must fill `output` with random data.
 fn generate(&mut self, output: &mut Self::Output);


Maybe rename the method to next_block for consistency with the Rng methods?

newpavlov · 2026-01-28T18:46:00Z

 ///
-/// 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.


It would be nice to explicitly document that BlockBuffer::default() initializes all bits of the state to make it friendlier to the optimization_barrier-based zeroization.

dhardy · 2026-01-29T08:24:24Z

Regarding the renaming, the largest problem is actually that we use 'core' to refer to the BlockRng (e.g. in the doc example and as a parameter to methods like next_word). Why 'core'? This is the core (random) generator.

So, BlockRng? The old struct BlockRng referred to a block-based random-number generator. So the name was fine.

BlockBuffer is okay but ultimately a poor name; it buffers results not blocks. ResultsBuffer or just Buffer might work.

The new trait BlockRng is actually a bad name: implementations are (random) (data) block generators, not number generators (no one needs 512-bit numbers).

The old name block::Generator is about as close to "random block generator" as one can get without an unwieldy name I think (unless we want to call it Rbg).

Since the main argument for separation of the core and the buffer seems to be zeroization and alternatives need specific documentation, why don't we just add a method like fn clear?

To avoid complicating history of a single PR too much I will close this one and replace it.

dhardy · 2026-01-28T15:56:12Z

Added a CHANGELOG.md entry

Motivation

rust-random/rand#1722 makes this useless.

Other

Any suggestions for a new name for trait Generator? As-is it matches its most important generate method, so the name doesn't seem too bad.

newpavlov · 2026-01-28T16:01:48Z

Any suggestions for a new name for trait Generator?

BlockRng would be the most consistent name with BlockRng::generate_block (or next_block) method. Instead of the wrapper I would prefer to have a separate BlockBuffer struct.

dhardy · 2026-01-28T16:07:11Z

The names are consistent.

What is the motivation for making BlockBuffer separate? It makes little sense to me (without removing the trait as in the more recent #24 design).

newpavlov · 2026-01-28T16:16:52Z

Separation of concerns. IMO struct Rng { core: RngCore, buffer: BlockBuffer<..> } is more transparent than struct Rng(BlockRng(RngCore));. And zeroization support would look less confusing. It's easy to understand what happens in:

self.core.zeroize();self.buffer = Default::default();
zeroize::optimization_barrier(&self.buffer)

While the wrapper would result in a less straightforward code.

The same applies to serialization as well.

dhardy · 2026-01-28T16:53:28Z

Separation of concerns

If they were truly separate components I'd agree, but the only pub methods not needing both parameters are word_offset and remaining_results.

I see you succeeded in convincing @tarcieri to make optimization_barrier public in RustCrypto/utils#1261, though it's not in a released version yet.

dhardy · 2026-01-28T17:12:34Z

@newpavlov I made significant additions to this PR; please re-review.

newpavlov · 2026-01-28T18:28:13Z

-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> {


Word is already bounded by Default via the Sealed trait.
~~And I think we can simplify the bound to just G::Output: Default, no?~~ UPD: No, we use Word functionality in the implementation.

newpavlov · 2026-01-28T18:30:30Z

 ///
 /// 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> {


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 [W; N] instead of &[W].

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 N is known), while the reverse is not with your approach (and variable length serialization is preferable if the output format happens to be something like JSON).

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.

The existing API is already usable for fixed-size serialization though (with slightly more code elsewhere, but storing the length is not exactly hard).

Are you seriously suggesting that users should manually implement the fixed-size serialization based on the variable sized methods???

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.

Ok... Feel free to do with this module whatever you want. I will not be reviewing/approving PRs which deal with block stuff anymore.

I am confused. But as you wish.

newpavlov · 2026-01-28T18:32:45Z

 - `BlockRng::reset` method ([#44])
 - `BlockRng::index` method (replaced with `BlockRng::word_offset`) ([#44])
 - `Generator::Item` associated type ([#26])
+- `CryptoBlockRng` ([#68])


Update changelog.

newpavlov · 2026-01-28T18:33:35Z

 #[derive(Clone)]
-pub struct BlockRng<G: Generator> {
+#[allow(missing_debug_implementations)]
+pub struct BlockBuffer<G: BlockRng> {


Use R instead of G like in all other bounds?

newpavlov · 2026-01-28T18:38:14Z

@@ -98,32 +89,12 @@ pub trait Generator {
 ///
 /// This must fill `output` with random data.
 fn generate(&mut self, output: &mut Self::Output);


Maybe rename the method to next_block for consistency with the Rng methods?

newpavlov · 2026-01-28T18:46:00Z

 ///
-/// 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.


It would be nice to explicitly document that BlockBuffer::default() initializes all bits of the state to make it friendlier to the optimization_barrier-based zeroization.

dhardy · 2026-01-29T08:24:24Z

Regarding the renaming, the largest problem is actually that we use 'core' to refer to the BlockRng (e.g. in the doc example and as a parameter to methods like next_word). Why 'core'? This is the core (random) generator.

So, BlockRng? The old struct BlockRng referred to a block-based random-number generator. So the name was fine.

BlockBuffer is okay but ultimately a poor name; it buffers results not blocks. ResultsBuffer or just Buffer might work.

The new trait BlockRng is actually a bad name: implementations are (random) (data) block generators, not number generators (no one needs 512-bit numbers).

The old name block::Generator is about as close to "random block generator" as one can get without an unwieldy name I think (unless we want to call it Rbg).

Since the main argument for separation of the core and the buffer seems to be zeroization and alternatives need specific documentation, why don't we just add a method like fn clear?

To avoid complicating history of a single PR too much I will close this one and replace it.

dhardy · 2026-01-28T15:56:12Z

Added a CHANGELOG.md entry

Motivation

rust-random/rand#1722 makes this useless.

Other

Any suggestions for a new name for trait Generator? As-is it matches its most important generate method, so the name doesn't seem too bad.

newpavlov · 2026-01-28T16:01:48Z

Any suggestions for a new name for trait Generator?

BlockRng would be the most consistent name with BlockRng::generate_block (or next_block) method. Instead of the wrapper I would prefer to have a separate BlockBuffer struct.

dhardy · 2026-01-28T16:07:11Z

The names are consistent.

What is the motivation for making BlockBuffer separate? It makes little sense to me (without removing the trait as in the more recent #24 design).

newpavlov · 2026-01-28T16:16:52Z

Separation of concerns. IMO struct Rng { core: RngCore, buffer: BlockBuffer<..> } is more transparent than struct Rng(BlockRng(RngCore));. And zeroization support would look less confusing. It's easy to understand what happens in:

self.core.zeroize();self.buffer = Default::default();
zeroize::optimization_barrier(&self.buffer)

While the wrapper would result in a less straightforward code.

The same applies to serialization as well.

dhardy · 2026-01-28T16:53:28Z

Separation of concerns

If they were truly separate components I'd agree, but the only pub methods not needing both parameters are word_offset and remaining_results.

I see you succeeded in convincing @tarcieri to make optimization_barrier public in RustCrypto/utils#1261, though it's not in a released version yet.

dhardy · 2026-01-28T17:12:34Z

@newpavlov I made significant additions to this PR; please re-review.

newpavlov · 2026-01-28T18:28:13Z

-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> {


Word is already bounded by Default via the Sealed trait.
~~And I think we can simplify the bound to just G::Output: Default, no?~~ UPD: No, we use Word functionality in the implementation.

newpavlov · 2026-01-28T18:30:30Z

 ///
 /// 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> {


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 [W; N] instead of &[W].

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 N is known), while the reverse is not with your approach (and variable length serialization is preferable if the output format happens to be something like JSON).

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.

The existing API is already usable for fixed-size serialization though (with slightly more code elsewhere, but storing the length is not exactly hard).

Are you seriously suggesting that users should manually implement the fixed-size serialization based on the variable sized methods???

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.

Ok... Feel free to do with this module whatever you want. I will not be reviewing/approving PRs which deal with block stuff anymore.

I am confused. But as you wish.

newpavlov · 2026-01-28T18:32:45Z

 - `BlockRng::reset` method ([#44])
 - `BlockRng::index` method (replaced with `BlockRng::word_offset`) ([#44])
 - `Generator::Item` associated type ([#26])
+- `CryptoBlockRng` ([#68])


Update changelog.

newpavlov · 2026-01-28T18:33:35Z

 #[derive(Clone)]
-pub struct BlockRng<G: Generator> {
+#[allow(missing_debug_implementations)]
+pub struct BlockBuffer<G: BlockRng> {


Use R instead of G like in all other bounds?

newpavlov · 2026-01-28T18:38:14Z

@@ -98,32 +89,12 @@ pub trait Generator {
 ///
 /// This must fill `output` with random data.
 fn generate(&mut self, output: &mut Self::Output);


Maybe rename the method to next_block for consistency with the Rng methods?

newpavlov · 2026-01-28T18:46:00Z

 ///
-/// 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.


It would be nice to explicitly document that BlockBuffer::default() initializes all bits of the state to make it friendlier to the optimization_barrier-based zeroization.

dhardy · 2026-01-29T08:24:24Z

Regarding the renaming, the largest problem is actually that we use 'core' to refer to the BlockRng (e.g. in the doc example and as a parameter to methods like next_word). Why 'core'? This is the core (random) generator.

So, BlockRng? The old struct BlockRng referred to a block-based random-number generator. So the name was fine.

BlockBuffer is okay but ultimately a poor name; it buffers results not blocks. ResultsBuffer or just Buffer might work.

The new trait BlockRng is actually a bad name: implementations are (random) (data) block generators, not number generators (no one needs 512-bit numbers).

The old name block::Generator is about as close to "random block generator" as one can get without an unwieldy name I think (unless we want to call it Rbg).

Since the main argument for separation of the core and the buffer seems to be zeroization and alternatives need specific documentation, why don't we just add a method like fn clear?

To avoid complicating history of a single PR too much I will close this one and replace it.

Remove CryptoGenerator
387369b

dhardy requested a review from newpavlov January 28, 2026 15:56

newpavlov approved these changes Jan 28, 2026
View reviewed changes

Rename BlockRng -> BlockBuffer
d3dcbb2

dhardy added 5 commits January 28, 2026 16:53

Remove fn Generator::drop
f9ff50b

Remove Debug impl for BlockBuffer
994f56d

Remove core from BlockBuffer
dbe91ab

Rename Generator -> BlockRng
c46cb62

impl Default for BlockRng
03a4e38

dhardy changed the title ~~Remove CryptoGenerator~~Remove CryptoGenerator and reduce BlockRng to BlockBufferJan 28, 2026

dhardy mentioned this pull request Jan 28, 2026
Remove ChaChaXCore types RustCrypto/stream-ciphers#512
Merged

newpavlov reviewed Jan 28, 2026
View reviewed changes

dhardy closed this Jan 29, 2026

newpavlov deleted the push-ytsyzvnpyymq branch January 29, 2026 18:17

Remove CryptoGenerator
387369b

dhardy requested a review from newpavlov January 28, 2026 15:56

newpavlov approved these changes Jan 28, 2026
View reviewed changes

Rename BlockRng -> BlockBuffer
d3dcbb2

dhardy added 5 commits January 28, 2026 16:53

Remove fn Generator::drop
f9ff50b

Remove Debug impl for BlockBuffer
994f56d

Remove core from BlockBuffer
dbe91ab

Rename Generator -> BlockRng
c46cb62

impl Default for BlockRng
03a4e38

dhardy changed the title ~~Remove CryptoGenerator~~Remove CryptoGenerator and reduce BlockRng to BlockBufferJan 28, 2026

dhardy mentioned this pull request Jan 28, 2026
Remove ChaChaXCore types RustCrypto/stream-ciphers#512
Merged

newpavlov reviewed Jan 28, 2026
View reviewed changes

dhardy closed this Jan 29, 2026

newpavlov deleted the push-ytsyzvnpyymq branch January 29, 2026 18:17

Remove CryptoGenerator
387369b

dhardy requested a review from newpavlov January 28, 2026 15:56

newpavlov approved these changes Jan 28, 2026
View reviewed changes

Rename BlockRng -> BlockBuffer
d3dcbb2

dhardy added 5 commits January 28, 2026 16:53

Remove fn Generator::drop
f9ff50b

Remove Debug impl for BlockBuffer
994f56d

Remove core from BlockBuffer
dbe91ab

Rename Generator -> BlockRng
c46cb62

impl Default for BlockRng
03a4e38

dhardy changed the title ~~Remove CryptoGenerator~~Remove CryptoGenerator and reduce BlockRng to BlockBufferJan 28, 2026

dhardy mentioned this pull request Jan 28, 2026
Remove ChaChaXCore types RustCrypto/stream-ciphers#512
Merged

newpavlov reviewed Jan 28, 2026
View reviewed changes

dhardy closed this Jan 29, 2026

newpavlov deleted the push-ytsyzvnpyymq branch January 29, 2026 18:17

Remove CryptoGenerator
387369b

dhardy requested a review from newpavlov January 28, 2026 15:56

newpavlov approved these changes Jan 28, 2026
View reviewed changes

Rename BlockRng -> BlockBuffer
d3dcbb2

dhardy added 5 commits January 28, 2026 16:53

Remove fn Generator::drop
f9ff50b

Remove Debug impl for BlockBuffer
994f56d

Remove core from BlockBuffer
dbe91ab

Rename Generator -> BlockRng
c46cb62

impl Default for BlockRng
03a4e38

dhardy changed the title ~~Remove CryptoGenerator~~Remove CryptoGenerator and reduce BlockRng to BlockBufferJan 28, 2026

dhardy mentioned this pull request Jan 28, 2026
Remove ChaChaXCore types RustCrypto/stream-ciphers#512
Merged

newpavlov reviewed Jan 28, 2026
View reviewed changes

dhardy closed this Jan 29, 2026

newpavlov deleted the push-ytsyzvnpyymq branch January 29, 2026 18:17

Remove CryptoGenerator
387369b

dhardy requested a review from newpavlov January 28, 2026 15:56

newpavlov approved these changes Jan 28, 2026
View reviewed changes

Rename BlockRng -> BlockBuffer
d3dcbb2

dhardy added 5 commits January 28, 2026 16:53

Remove fn Generator::drop
f9ff50b

Remove Debug impl for BlockBuffer
994f56d

Remove core from BlockBuffer
dbe91ab

Rename Generator -> BlockRng
c46cb62

impl Default for BlockRng
03a4e38

dhardy changed the title ~~Remove CryptoGenerator~~Remove CryptoGenerator and reduce BlockRng to BlockBufferJan 28, 2026

dhardy mentioned this pull request Jan 28, 2026
Remove ChaChaXCore types RustCrypto/stream-ciphers#512
Merged

newpavlov reviewed Jan 28, 2026
View reviewed changes

dhardy closed this Jan 29, 2026

newpavlov deleted the push-ytsyzvnpyymq branch January 29, 2026 18:17

Remove CryptoGenerator
387369b

dhardy requested a review from newpavlov January 28, 2026 15:56

newpavlov approved these changes Jan 28, 2026
View reviewed changes

Rename BlockRng -> BlockBuffer
d3dcbb2

dhardy added 5 commits January 28, 2026 16:53

Remove fn Generator::drop
f9ff50b

Remove Debug impl for BlockBuffer
994f56d

Remove core from BlockBuffer
dbe91ab

Rename Generator -> BlockRng
c46cb62

impl Default for BlockRng
03a4e38

dhardy changed the title ~~Remove CryptoGenerator~~Remove CryptoGenerator and reduce BlockRng to BlockBufferJan 28, 2026

dhardy mentioned this pull request Jan 28, 2026
Remove ChaChaXCore types RustCrypto/stream-ciphers#512
Merged

newpavlov reviewed Jan 28, 2026
View reviewed changes

dhardy closed this Jan 29, 2026

newpavlov deleted the push-ytsyzvnpyymq branch January 29, 2026 18:17

Remove CryptoGenerator
387369b

dhardy requested a review from newpavlov January 28, 2026 15:56

newpavlov approved these changes Jan 28, 2026
View reviewed changes

Rename BlockRng -> BlockBuffer
d3dcbb2

dhardy added 5 commits January 28, 2026 16:53

Remove fn Generator::drop
f9ff50b

Remove Debug impl for BlockBuffer
994f56d

Remove core from BlockBuffer
dbe91ab

Rename Generator -> BlockRng
c46cb62

impl Default for BlockRng
03a4e38

dhardy changed the title ~~Remove CryptoGenerator~~Remove CryptoGenerator and reduce BlockRng to BlockBufferJan 28, 2026

dhardy mentioned this pull request Jan 28, 2026
Remove ChaChaXCore types RustCrypto/stream-ciphers#512
Merged

newpavlov reviewed Jan 28, 2026
View reviewed changes

dhardy closed this Jan 29, 2026

newpavlov deleted the push-ytsyzvnpyymq branch January 29, 2026 18:17

Uh oh!

Uh oh!

Conversation

dhardy commented Jan 28, 2026

Motivation

Other

Uh oh!

newpavlov commented Jan 28, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dhardy commented Jan 28, 2026

Uh oh!

newpavlov commented Jan 28, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dhardy commented Jan 28, 2026

Uh oh!

dhardy commented Jan 28, 2026

Uh oh!

newpavlovJan 28, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

newpavlovJan 29, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

newpavlovJan 30, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dhardy commented Jan 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

dhardy commented Jan 28, 2026

Motivation

Other

Uh oh!

newpavlov commented Jan 28, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dhardy commented Jan 28, 2026

Uh oh!

newpavlov commented Jan 28, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dhardy commented Jan 28, 2026

Uh oh!

newpavlov commented Jan 28, 2026•
edited
Loading

newpavlov commented Jan 28, 2026•
edited
Loading

newpavlovJan 28, 2026•
edited
Loading

newpavlovJan 29, 2026•
edited
Loading

newpavlovJan 30, 2026•
edited
Loading

newpavlov commented Jan 28, 2026•
edited
Loading

newpavlov commented Jan 28, 2026•
edited
Loading

newpavlovJan 28, 2026•
edited
Loading

newpavlovJan 29, 2026•
edited
Loading

newpavlovJan 30, 2026•
edited
Loading