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

Adds From<iXXX> implementations to fields #263

Merged
merged 3 commits into from
Apr 22, 2021

Conversation

huitseeker
Copy link
Contributor

Those delegate to the respective, existing From<uXXX> implementations for XXX in [8, 16, 32, 64, 128]

@huitseeker huitseeker changed the title Adds From<iXXX> implementations to quadratic / field extensions Adds From<iXXX> implementations to quadratic / cubic field extensions Apr 16, 2021
@ValarDragon
Copy link
Member

Should we add these From<ixxx> trait bounds to the field trait?

@huitseeker huitseeker changed the title Adds From<iXXX> implementations to quadratic / cubic field extensions Adds From<iXXX> implementations to prime / quadratic / cubic field extensions Apr 16, 2021
@huitseeker
Copy link
Contributor Author

@ValarDragon Good question. It breaks compatibility of the libraries out there which may implement an ark-ff::Field on their own custom structure. At any rate, and so that this trait bound is easier to do in the future, I've added a commit doing the harder part of deriving the implas part of the prime field macros.

Copy link
Member

@ValarDragon ValarDragon left a comment

Choose a reason for hiding this comment

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

This looks good to me! Thanks for adding it to fields.
I've never seen / used paste before, but from looking at the docs this seems right to me.

cref #226, but I think its fine to assume that the ordering -(p-1)/2..0..(p-1)/2 would be implied for signed ints here

@huitseeker
Copy link
Contributor Author

@ValarDragon FYI, you can inspect the output with cargo expand
Here's the output of :
cargo expand 2>/dev/null | grep -A7 -Ee 'impl<P:.*> From<[iu][0-9]+> for' |head -n 50
https://gist.github.com/8f66c293bc5b231932dca3544460feec

@huitseeker huitseeker changed the title Adds From<iXXX> implementations to prime / quadratic / cubic field extensions Adds From<iXXX> implementations to prime / quadratic / cubic field (extensions) Apr 17, 2021
Those delegate to the respective, existing `From<uXXX>` implementations for `XXX` in `[8, 16, 32, 64, 128]`
ff/src/fields/models/cubic_extension.rs Outdated Show resolved Hide resolved
@Pratyush Pratyush changed the title Adds From<iXXX> implementations to prime / quadratic / cubic field (extensions) Adds From<iXXX> implementations to fields Apr 22, 2021
@Pratyush Pratyush merged commit 68d4347 into arkworks-rs:master Apr 22, 2021
huitseeker added a commit to huitseeker/algebra that referenced this pull request Apr 23, 2021
weikengchen added a commit that referenced this pull request Apr 23, 2021
…lize` (#265)

* Implements hashing as a blanket trait for instances of `CanonicalSerialize`

- this saves an alllocation w.r.t the suggested approach by implementing `io::Write` on the input instance of `digest::Digest`,
- note that most instances of `digest::Digest` [already](https://gist.github.com/huitseeker/e827161413063e347ce5a496b66ff287) have an [`io::Write` instance](https://github.com/rustcrypto/hashes#hashing-readable-objects), but `CanonicalSerialize` consuming its `io::Write` argument prevents its usage,
- this hence implements `io::Write` on a [cheap newtype wrapper](https://rust-unofficial.github.io/patterns/patterns/behavioural/newtype.html)

Fixes #86

* Adjust post review

- rename Hash -> CanonicalSerialize Ext according to [extension trait best practices](https://rust-lang.github.io/rfcs/0445-extension-trait-conventions.html#the-convention)
- add the same structure for hashing uncompressed bytes

* Changelog entries for #263, #265

* Update CHANGELOG.md

Co-authored-by: Weikeng Chen <w.k@berkeley.edu>
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