Skip to content

chore: add CryptoRng trait bound - #427

Merged
ebfull merged 2 commits into
tachyon-zcash:mainfrom
alxiong:crypto-rng
Feb 6, 2026
Merged

chore: add CryptoRng trait bound#427
ebfull merged 2 commits into
tachyon-zcash:mainfrom
alxiong:crypto-rng

Conversation

@alxiong

@alxiong alxiong commented Feb 5, 2026

Copy link
Copy Markdown
Contributor

References #116

Right now, we mostly use StdRng which already implements CryptoRng (even though not deterministic) or not as fast as ChaCha20Rng.

But this should be a first step towards CryptoRng trait bounds to relevant APIs.

@alxiong
alxiong requested review from TalDerei and ebfull February 5, 2026 03:55

@TalDerei TalDerei left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

looks good to me 👍

@ebfull ebfull left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

If we're stuck using an old version of rand right now for some reason, then I don't want to bother with a refactoring like this. Otherwise, concept ACK from me.

Comment thread crates/ragu_pcd/src/verify.rs Outdated
@alxiong

alxiong commented Feb 6, 2026

Copy link
Copy Markdown
Contributor Author

If we're stuck using an old version of rand right now for some reason,

pasta_curves shackle us to rand ^0.8 (where the latest stable rand is 0.9.3, and 0.10.0-rc.9 is also available)

@ebfull

ebfull commented Feb 6, 2026

Copy link
Copy Markdown
Collaborator

I've opened #437 to help us move in the right direction on rand, at the cost of some temporary dependency patches. I'll work with str4d to try to get these merged and released in their crates over the next month or two hopefully.

@ebfull

ebfull commented Feb 6, 2026

Copy link
Copy Markdown
Collaborator

This can be rebased on main to use rand 0.9 APIs instead.

@alxiong

alxiong commented Feb 6, 2026

Copy link
Copy Markdown
Contributor Author

rebased. but we should still have Rng + CryptoRng (instead of either or) imo. Do you disagree?

@ebfull

ebfull commented Feb 6, 2026

Copy link
Copy Markdown
Collaborator

In rand 0.9, CryptoRng: RngCore and there is a blanket impl<R: RngCore + ?Sized> Rng for R. This means any CryptoRng automatically implements Rng, making Rng + CryptoRng redundant. The idiomatic bound is just CryptoRng.

Currently, I don't think we even call Rng extension methods. We just pass the rng into things like F::random (which take RngCore).

@ebfull
ebfull merged commit 39b7e54 into tachyon-zcash:main Feb 6, 2026
11 checks passed
@alxiong
alxiong deleted the crypto-rng branch February 17, 2026 01:11
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