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

Change BigInteger <-> BigUint conversions to use From instead of Into #321

Merged
merged 1 commit into from
Oct 20, 2021

Conversation

hdevalence
Copy link
Contributor

@hdevalence hdevalence commented Oct 4, 2021

Description

This is a strictly more general bound, and means that it's possible to write
the conversion without relying on type inference.


Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

  • Targeted PR against correct branch (master)
  • Linked to Github issue with discussion and accepted design OR have an explanation in the PR that describes this work.
  • [ ] Wrote unit tests
  • [ ] Updated relevant documentation in the code
  • Added a relevant changelog entry to the Pending section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer

Copy link
Member

@Pratyush Pratyush left a comment

Choose a reason for hiding this comment

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

Thanks!

@weikengchen
Copy link
Member

Let me try to fix the patching script. So I can get the "test against curves" passing.

@Pratyush
Copy link
Member

Pratyush commented Oct 4, 2021

@weikengchen a better way would be to temporarily add both git and version specifiers, like here: https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#multiple-locations

@Pratyush
Copy link
Member

Pratyush commented Oct 5, 2021

@hdevalence I pushed a fix for the CI issue in #324, and added the corresponding Fp impl in #325. If you could rebase then we can get this merged (I can't rebase because of the issue with "allow edits from maintainers")

@Pratyush
Copy link
Member

@hdevalence if you could rebase/merge on top of the latest master, then we can get this merged, and can then merge #322 as well. Thanks!

This is a strictly more general bound, and means that it's possible to write
the conversion without relying on type inference.
@hdevalence
Copy link
Contributor Author

@Pratyush thanks for the bump; I rebased this on top of current master.

@Pratyush Pratyush changed the title Change bigint conversions to impl From instead of Into. Change BigInteger <-> BigUint conversions to use From instead of Into Oct 20, 2021
@Pratyush Pratyush merged commit 6e568cd into arkworks-rs:master Oct 20, 2021
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.

3 participants