Skip to content

Conversation

@daxpedda
Copy link
Contributor

@daxpedda daxpedda commented Jan 23, 2022

Builds on #55.

  • Implement all Rust traits for Ristretto255.
  • Remove NonVerifiableServerEvaluateResult as it was holding only one type.
  • Apply warn(missing_debug_implementations) and implement Debug on all public types.
  • Remove leftover copy methods now that we are compiling without alloc.
  • Implement all Rust traits for PreparedEvaluationElement and PreparedTscalar.
  • Introduce more concrete types for complex returned Iterators in the public API.
  • Check for zero scalars during Evaluate according to the spec.
  • Move de/serialization of elements and scalars from GenericArrays to slices.
  • Fix new_with_key would panic if bytes don't have the right length.
  • Fix serde de/serialization to use the checked variants in Group.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jan 23, 2022
Comment on lines +30 to +31
/// The protocol has failed and can't be completed.
Protocol,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I couldn't come up with a better name or description that actually explains the error to a user. I think InverseError doesn't make any sense to a user.

@daxpedda daxpedda marked this pull request as ready for review January 25, 2022 08:06
@daxpedda daxpedda mentioned this pull request Jan 27, 2022
Copy link
Contributor

@kevinlewi kevinlewi left a comment

Choose a reason for hiding this comment

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

Looks good overall, will merge!

@kevinlewi kevinlewi merged commit b59b359 into facebook:main Jan 28, 2022
@kevinlewi
Copy link
Contributor

Oh oops, I probably should have re-run the CI before merging this, seems like the main branch now is failing CI. @daxpedda any ideas here?

@daxpedda
Copy link
Contributor Author

Yes ... this is interesting and something I wasn't aware of, depending on pre-releases can automatically update. I thought that pre-releases can not update as they are considered unstable. derive-where is the offender here as it released a breaking change recently, which is fine because it's a pre-release, but downstream has to pin then.

This is addressed by #54, I'm gonna pin pre-release dependencies there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants