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

Trait bounds for random functions in 1.1 #232

Closed
hdevalence opened this issue Feb 15, 2019 · 3 comments
Closed

Trait bounds for random functions in 1.1 #232

hdevalence opened this issue Feb 15, 2019 · 3 comments

Comments

@hdevalence
Copy link
Contributor

1.1 changed the trait bounds for RistrettoPoint::random and Scalar::random, see #222 and #219.

These changes have two benefits:

  • they unlink us from the rand crate and make us depend only on rand_core;
  • they allow passing both owned and borrowed RNGs.

The change was not supposed to be a breaking change, since the new bounds are strictly more general than the old ones (as every RngCore is an Rng and every &mut RngCore is an RngCore), so the new bound is satisfied in every situation where the old bound applied.

The 1.1.0-pre.0 version didn't cause problems on the crates I tested it on, but there was an unexpected problem: https://github.com/interstellar/slingshot/blob/ce71c93a9a29ac3b4f69ce71feb987bd64d6c4ec/spacesuit/src/value.rs#L160-L161 broke, since it took a borrow as input and used it twice. So there was slight breakage.

One option is to revert the changes (probably just the ones from #219) and release 1.1.3; another would be to fix up slingshot and leave the new bound.

@oleganza
Copy link
Contributor

Here's my fix, for reference: stellar/slingshot#89

@hdevalence
Copy link
Contributor Author

My feeling is that it would be better to revert #219 in 1.1.3, so that there's no potential for future problems. We will have to release a 2.0 eventually once the rand crate is stabilized, so if we wanted to change it then we could do it then.

hdevalence added a commit that referenced this issue Feb 15, 2019
See discussion at #232 , copied below:

`1.1` changed the trait bounds for `RistrettoPoint::random` and `Scalar::random`, see #222 and #219.

These changes have two benefits:
* they unlink us from the `rand` crate and make us depend only on `rand_core`;
* they allow passing both owned and borrowed RNGs.

The change was not supposed to be a breaking change, since the new bounds are strictly more general than the old ones (as every `RngCore` is an `Rng` and every `&mut RngCore` is an `RngCore`), so the new bound is satisfied in every situation where the old bound applied.

The `1.1.0-pre.0` version didn't cause problems on the crates I tested it on, but there was an unexpected problem: https://github.com/interstellar/slingshot/blob/ce71c93a9a29ac3b4f69ce71feb987bd64d6c4ec/spacesuit/src/value.rs#L160-L161 broke, since it took a borrow as input and used it twice. So there was slight breakage.

One option is to revert the changes (probably just the ones from #219) and release 1.1.3; another would be to fix up `slingshot` and leave the new bound.
@hdevalence
Copy link
Contributor Author

Closing this since it was fixed in #233

pinkforest pushed a commit to pinkforest/curve25519-dalek that referenced this issue Jun 27, 2023
GitHub Actions runners are not guaranteed to have the necessary CPU
features in order for these tests to work.

Uses a `--target x86_64-unknown-linux-gnu` directive when compiling so
the `target_feature` flags don't apply to build scripts.
plotskogwq added a commit to plotskogwq/curve25519-dalek that referenced this issue Aug 10, 2024
See discussion at dalek-cryptography/curve25519-dalek#232 , copied below:

`1.1` changed the trait bounds for `RistrettoPoint::random` and `Scalar::random`, see #222 and #219.

These changes have two benefits:
* they unlink us from the `rand` crate and make us depend only on `rand_core`;
* they allow passing both owned and borrowed RNGs.

The change was not supposed to be a breaking change, since the new bounds are strictly more general than the old ones (as every `RngCore` is an `Rng` and every `&mut RngCore` is an `RngCore`), so the new bound is satisfied in every situation where the old bound applied.

The `1.1.0-pre.0` version didn't cause problems on the crates I tested it on, but there was an unexpected problem: https://github.com/interstellar/slingshot/blob/ce71c93a9a29ac3b4f69ce71feb987bd64d6c4ec/spacesuit/src/value.rs#L160-L161 broke, since it took a borrow as input and used it twice. So there was slight breakage.

One option is to revert the changes (probably just the ones from #219) and release 1.1.3; another would be to fix up `slingshot` and leave the new bound.
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

No branches or pull requests

2 participants