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

Revert #219. #233

Merged
merged 1 commit into from
Feb 15, 2019
Merged

Revert #219. #233

merged 1 commit into from
Feb 15, 2019

Conversation

hdevalence
Copy link
Contributor

One fix to #232

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.
Copy link
Contributor Author

@hdevalence hdevalence left a comment

Choose a reason for hiding this comment

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

Tested on slingshot that this fixes the breakage.

@hdevalence hdevalence merged commit d4cc999 into develop Feb 15, 2019
@hdevalence hdevalence deleted the revert-rng-changes-from-219 branch February 15, 2019 21:44
@oleganza
Copy link
Contributor

Confirm.

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.

2 participants