search

zkcrypto / pairing

Pairing-friendly elliptic curve library.
Other
341 stars 120 forks source link

fix RngCore Sized #108

Closed kigawas closed 4 years ago

kigawas commented 5 years ago

https://github.com/zkcrypto/ff/pull/14

burdges commented 5 years ago

R: RngCore should work fine because impl<R> RngCore for &'a mut R where R: RngCore+?Sized https://rust-random.github.io/rand/rand_core/trait.RngCore.html#foreign-impls And similarly https://github.com/zkcrypto/ff/pull/14

kigawas commented 5 years ago

There's no reason using Sized here, I'm not a fan of implementing outside

burdges commented 5 years ago

An idiomatic usage would actually be this, although it might make error messages less clear and once I saw some other interaction, not sure all the consequences here. https://github.com/rust-random/rand/issues/908

fn random<R: RngCore + ?Sized>(rng: R) -> Self {
kigawas commented 5 years ago

Yes. That's why we need this pr.

Using ?Sized here enables us to make sure we are passing only one Rng instance from the top, especially it's easier to implement rand apis requiring ?Sized (like sample)

ebfull commented 4 years ago

Sorry about the breakage.

ebfull commented 4 years ago

New versions of pairing / ff / ff_derive have been pushed to fix this.

kigawas commented 4 years ago

New versions of pairing / ff / ff_derive have been pushed to fix this.

Can you take a look at this https://github.com/zkcrypto/group/pull/4 ?