Skip to content
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

Add rand_core::InfallibleRng marker trait #1412

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion rand_chacha/src/chacha.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
use self::core::fmt;
use crate::guts::ChaCha;
use rand_core::block::{BlockRng, BlockRngCore, CryptoBlockRng};
use rand_core::{CryptoRng, Error, RngCore, SeedableRng};
use rand_core::{CryptoRng, Error, InfallibleRng, RngCore, SeedableRng};

#[cfg(feature = "serde1")] use serde::{Serialize, Deserialize, Serializer, Deserializer};

Expand Down Expand Up @@ -101,6 +101,8 @@ macro_rules! chacha_impl {

impl CryptoBlockRng for $ChaChaXCore {}

impl InfallibleRng for $ChaChaXRng {}

/// A cryptographically secure random number generator that uses the ChaCha algorithm.
///
/// ChaCha is a stream cipher designed by Daniel J. Bernstein[^1], that we use as an RNG. It is
Expand Down
8 changes: 8 additions & 0 deletions rand_core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,14 @@ pub trait RngCore {
/// [`BlockRngCore`]: block::BlockRngCore
pub trait CryptoRng: RngCore {}

/// A marker trait used to indicate that an [`RngCore`] implementation is
/// supposed to never return errors from the [`RngCore::try_fill_bytes`] method
/// and never panic on unwrapping [`Error`] while calling other methods.
///
/// This trait is usually implemented by PRNGs, as opposed to OS, hardware,
/// and periodically-reseeded RNGs, which may fail with IO errors.
pub trait InfallibleRng: RngCore {}
Comment on lines +215 to +221
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commenting just for visibility of the significant addition.

It falls into the same trap as std::iter::TrustedLen in that it promises something about another trait.

I think I'm okay with this, but worth thinking about / getting more input on.

Copy link
Member Author

@newpavlov newpavlov Mar 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about different alternatives and it looks like it's the simplest option. Plus, we already have the CryptoRng trait.

One potential option is to make error an associated type (and CryptoRng may become associated bool constant). This way we could use Infallible by default. But without trait aliases it would be significantly less ergonomic to work with. Here is a draft of how it could look in future: playground.

Copy link
Member

@dhardy dhardy Mar 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding associated types + consts to RngCore changes the meaning on every existing usage as a generic bound.

For "crypto grade", I prefer the trait-inheritance model. After all, every CryptoRng is also a valid RNG. Also, can't have CRYPTO_STRONG default to true on all impls.

For infallibility, we'd at least need a bound on that error type: Error: std::error::Error or Error: Into<Something> where Something is a summarized RNG error (NotInitialized, EndOfSequence, IOError, MissingHardwareDevice, ???).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

every CryptoRng is also a valid RNG.

Yes, and? I don't see why the associated constant would be at odds with it. "Cryptographically strong" is a binary property of RNG, which can be modeled perfectly by an associated bool constant.

Also, can't have CRYPTO_STRONG default to true on all impls.

Yes, it should've been false in my example.

For infallibility, we'd at least need a bound on that error type

Sure. But I think it's an unimportant detail. Without stabilization of at least trait aliases, this approach is, arguably, not practical anyway.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, and?

Only that this case does work well with the trait inheritance model; not every boolean property does.


/// A random number generator that can be explicitly seeded.
///
/// This trait encapsulates the low-level functionality common to all
Expand Down
5 changes: 4 additions & 1 deletion rand_pcg/src/pcg128.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
const MULTIPLIER: u128 = 0x2360_ED05_1FC6_5DA4_4385_DF64_9FCC_F645;

use core::fmt;
use rand_core::{impls, le, Error, RngCore, SeedableRng};
use rand_core::{impls, le, Error, InfallibleRng, RngCore, SeedableRng};
#[cfg(feature = "serde1")] use serde::{Deserialize, Serialize};

/// A PCG random number generator (XSL RR 128/64 (LCG) variant).
Expand Down Expand Up @@ -159,6 +159,7 @@ impl RngCore for Lcg128Xsl64 {
}
}

impl InfallibleRng for Lcg128Xsl64 {}

/// A PCG random number generator (XSL 128/64 (MCG) variant).
///
Expand Down Expand Up @@ -269,6 +270,8 @@ impl RngCore for Mcg128Xsl64 {
}
}

impl InfallibleRng for Mcg128Xsl64 {}

#[inline(always)]
fn output_xsl_rr(state: u128) -> u64 {
// Output function XSL RR ("xorshift low (bits), random rotation")
Expand Down
4 changes: 3 additions & 1 deletion rand_pcg/src/pcg128cm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
const MULTIPLIER: u64 = 15750249268501108917;

use core::fmt;
use rand_core::{impls, le, Error, RngCore, SeedableRng};
use rand_core::{impls, le, Error, InfallibleRng, RngCore, SeedableRng};
#[cfg(feature = "serde1")] use serde::{Deserialize, Serialize};

/// A PCG random number generator (CM DXSM 128/64 (LCG) variant).
Expand Down Expand Up @@ -165,6 +165,8 @@ impl RngCore for Lcg128CmDxsm64 {
}
}

impl InfallibleRng for Lcg128CmDxsm64 {}

#[inline(always)]
fn output_dxsm(state: u128) -> u64 {
// See https://github.com/imneme/pcg-cpp/blob/ffd522e7188bef30a00c74dc7eb9de5faff90092/include/pcg_random.hpp#L1016
Expand Down
4 changes: 3 additions & 1 deletion rand_pcg/src/pcg64.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
//! PCG random number generators

use core::fmt;
use rand_core::{impls, le, Error, RngCore, SeedableRng};
use rand_core::{impls, le, Error, InfallibleRng, RngCore, SeedableRng};
#[cfg(feature = "serde1")] use serde::{Deserialize, Serialize};

// This is the default multiplier used by PCG for 64-bit state.
Expand Down Expand Up @@ -167,3 +167,5 @@ impl RngCore for Lcg64Xsh32 {
Ok(())
}
}

impl InfallibleRng for Lcg64Xsh32 {}
2 changes: 1 addition & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ macro_rules! error { ($($x:tt)*) => (
) }

// Re-exports from rand_core
pub use rand_core::{CryptoRng, Error, RngCore, SeedableRng};
pub use rand_core::{CryptoRng, Error, InfallibleRng, RngCore, SeedableRng};

// Public modules
pub mod distributions;
Expand Down
4 changes: 3 additions & 1 deletion src/rngs/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

//! Mock random number generator

use rand_core::{impls, Error, RngCore};
use rand_core::{impls, Error, InfallibleRng, RngCore};

#[cfg(feature = "serde1")]
use serde::{Serialize, Deserialize};
Expand Down Expand Up @@ -81,6 +81,8 @@ impl RngCore for StepRng {
}
}

impl InfallibleRng for StepRng {}

#[cfg(test)]
mod tests {
#[cfg(any(feature = "alloc", feature = "serde1"))]
Expand Down
4 changes: 3 additions & 1 deletion src/rngs/small.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

//! A small fast RNG

use rand_core::{Error, RngCore, SeedableRng};
use rand_core::{Error, InfallibleRng, RngCore, SeedableRng};

#[cfg(target_pointer_width = "64")]
type Rng = super::xoshiro256plusplus::Xoshiro256PlusPlus;
Expand Down Expand Up @@ -64,6 +64,8 @@ impl RngCore for SmallRng {
}
}

impl InfallibleRng for SmallRng {}

impl SmallRng {
/// Construct an instance seeded from another `Rng`
///
Expand Down
3 changes: 2 additions & 1 deletion src/rngs/std.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

//! The standard RNG

use crate::{CryptoRng, Error, RngCore, SeedableRng};
use rand_core::{CryptoRng, Error, InfallibleRng, RngCore, SeedableRng};

#[cfg(any(test, feature = "getrandom"))]
pub(crate) use rand_chacha::ChaCha12Core as Core;
Expand Down Expand Up @@ -73,6 +73,7 @@ impl SeedableRng for StdRng {

impl CryptoRng for StdRng {}

impl InfallibleRng for StdRng {}

#[cfg(test)]
mod test {
Expand Down
4 changes: 3 additions & 1 deletion src/rngs/xoshiro128plusplus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
#[cfg(feature="serde1")] use serde::{Serialize, Deserialize};
use rand_core::impls::{next_u64_via_u32, fill_bytes_via_next};
use rand_core::le::read_u32_into;
use rand_core::{SeedableRng, RngCore, Error};
use rand_core::{Error, InfallibleRng, RngCore, SeedableRng};

/// A xoshiro128++ random number generator.
///
Expand Down Expand Up @@ -97,6 +97,8 @@ impl RngCore for Xoshiro128PlusPlus {
}
}

impl InfallibleRng for Xoshiro128PlusPlus {}

#[cfg(test)]
mod tests {
use super::*;
Expand Down
4 changes: 3 additions & 1 deletion src/rngs/xoshiro256plusplus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
#[cfg(feature="serde1")] use serde::{Serialize, Deserialize};
use rand_core::impls::fill_bytes_via_next;
use rand_core::le::read_u64_into;
use rand_core::{SeedableRng, RngCore, Error};
use rand_core::{Error, InfallibleRng, RngCore, SeedableRng};

/// A xoshiro256++ random number generator.
///
Expand Down Expand Up @@ -99,6 +99,8 @@ impl RngCore for Xoshiro256PlusPlus {
}
}

impl InfallibleRng for Xoshiro256PlusPlus {}

#[cfg(test)]
mod tests {
use super::*;
Expand Down