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

WIP: Bump to rand[_core] 0.9 #729

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

pinkforest
Copy link
Contributor

@pinkforest pinkforest commented Jan 28, 2025

Pending / Clarifications

  • Breaking/Minor (Opt-in): Requires MSRV 1.63 - need to bump from current 1.60
  • Breaking/Minor (Opt-in): rand_core 0.9 via API
  • Breaking/Minor (Opt-in): We need to implement TryCryptoRng which needs API changes from current infallible API.
  • group and ff needs to bump rand + rand_core - Bump rand + rand_core zkcrypto/group#55

Notes

@pinkforest pinkforest changed the title Bump to rand[_core] 0.9 WIP: Bump to rand[_core] 0.9 Jan 28, 2025
@tarcieri
Copy link
Contributor

@pinkforest it needs to be os_rng now

@NimonSour
Copy link

Since rand_core changed the trait for OsRng from CryptoRngCore( which is RngCore + CryptoRng ) to TryRngCore +TryCryptoRngit makes sense to keep all the methods of types across dalek crates that are dependent on CryptoRngCore ( like generate and random ) as R : RngCore + CryptoRng plus adding the additional methods ( try_generate and try_random ) respectively where R : TryRngCore + CryptoRngCore.

Example for ed25519/src/signing.rs

    pub fn generate<R: RngCore + CryptoRng + ?Sized>(csprng: &mut R) -> SigningKey {
        let mut secret = SecretKey::default();
        csprng.fill_bytes(&mut secret);
        Self::from_bytes(&secret)
    }
    
    pub fn try_generate<R: TryRngCore + TryCryptoRng + ?Sized>(csprng: &mut R) -> Result<SigningKey,<R as TryRngCore>::Error> {
        let mut secret = SecretKey::default();
        csprng.try_fill_bytes(&mut secret)?;
        Ok(Self::from_bytes(&secret))
    }

It will offer some backward compatibility while aligning with rand_core's RngCoreand TryRngCore traits, where the user has a choice on RNG implementation.

@tarcieri
Copy link
Contributor

tarcieri commented Feb 4, 2025

it makes sense to keep all the methods of types across dalek crates that are dependent on CryptoRngCore ( like generate and random ) as R : RngCore + CryptoRng

CryptoRng now has a supertrait bound on RngCore, so instead CryptoRngCore should just be replaced with just CryptoRng. Adding RngCore + ... is redundant due to the supertrait bound. See:

https://docs.rs/rand_core/0.9.0/rand_core/trait.CryptoRng.html

Also you can call OsRng.unwrap_err() to get a CryptoRng-friendly version of OsRng.

@pinkforest
Copy link
Contributor Author

I assume we can live with ff/group will get duplicate 0.8 rand until ff/group upgrades - unblocking us to adopt 0.9 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants