Skip to content

Conversation

mrecachinas
Copy link

Addresses #412.

I looked through the others and it seemed like this is just a trait addition, but no methods are required. Let me know if this does require something else.

Copy link
Contributor

@pitdicker pitdicker left a comment

Choose a reason for hiding this comment

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

@mrecachinas Thank you for the PR!

I also believe this is it 😄.

But let's wait and see if @dhardy agrees.

@dhardy
Copy link
Member

dhardy commented Apr 24, 2018

Yes it is; thank you @mrecachinas.

My only concern is that the "crypto quality" of the source is up to each implementation to assure, therefore it seems like perhaps we should do impl CryptoRng for OsRng where imp::OsRng: CryptoRng or something, though probably that's more counter-productive than useful (i.e. it could make tests fail on other platforms unexpectedly and for no good reason).

Are all implementations crypto-grade? I think they are all supposed to be; system-specific caveats are beyond the scope of this code so long as we document which sources we use (which @pitdicker already did). So yes, lets merge this.

@pitdicker
Copy link
Contributor

Are all implementations crypto-grade?

We assume so in other places, like EntropyRng. Currently they are, and I would make it a requirement for any new platforms when they get added.

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.

4 participants