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

Add the SNARK gadget traits #2

Merged
merged 14 commits into from
Oct 29, 2020
Merged

Add the SNARK gadget traits #2

merged 14 commits into from
Oct 29, 2020

Conversation

weikengchen
Copy link
Member

This PR adds the SNARK trait and the shared component Input Gadgets (will be used by Groth16 and GM17). And it fixes a few compilation errors.

The PR is not finished---there is an input gadget for nonnative fields, which is required for Marlin. We will wait for the dependency.

src/snark/constraints.rs Outdated Show resolved Hide resolved
src/snark/constraints.rs Outdated Show resolved Hide resolved
src/snark/constraints.rs Outdated Show resolved Hide resolved
src/snark/constraints.rs Outdated Show resolved Hide resolved
src/snark/constraints.rs Outdated Show resolved Hide resolved
src/snark/constraints.rs Outdated Show resolved Hide resolved
src/snark/constraints.rs Outdated Show resolved Hide resolved
src/snark/constraints.rs Outdated Show resolved Hide resolved
src/snark/constraints.rs Outdated Show resolved Hide resolved
src/snark/constraints.rs Outdated Show resolved Hide resolved
src/snark/constraints.rs Outdated Show resolved Hide resolved
@weikengchen
Copy link
Member Author

(still adding comments)

@weikengchen
Copy link
Member Author

ready for more review

src/snark/constraints.rs Outdated Show resolved Hide resolved
src/snark/constraints.rs Outdated Show resolved Hide resolved
Comment on lines 216 to 217
// the same routine as the new_input above

Copy link
Member

Choose a reason for hiding this comment

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

If it's the same routine, then can we reuse the implementation to avoid duplication and things going out of sync?

Comment on lines 470 to 471
// the same routine as the new_input above.

Copy link
Member

Choose a reason for hiding this comment

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

Same comment: can we share the impl between the two methods?

fn from_field_elements(src: &Vec<FpVar<CF>>) -> Result<Self, SynthesisError> {
// the same routine as the new_input above.

let cs = src.cs();
Copy link
Member

Choose a reason for hiding this comment

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

You probably want to handle the case of constants separately from variables (in the former I guess you don't do any constraint generation?)

weikengchen and others added 2 commits October 27, 2020 13:58
Co-authored-by: Pratyush Mishra <[email protected]>
Co-authored-by: Pratyush Mishra <[email protected]>
@weikengchen
Copy link
Member Author

Will get back to this in one hour.

@weikengchen
Copy link
Member Author

Not an easy one.

For those "the same routine as the new_input above", I read the code and agree there are some overlapping, but it seems hard to extract some common elements that make a lot of sense. So, I just expand the comments.

I also changed the from_field_elements for NonNativeFieldGadgets to specifically handle the constant case.

@Pratyush
Copy link
Member

I think this is failing Check no_std because ark-nonnative-fields is not no_std. Let me make a PR there, and then I'll rerun CI here.

@weikengchen
Copy link
Member Author

Thanks!

@weikengchen
Copy link
Member Author

All checks have passed now.

@Pratyush Pratyush merged commit e79680a into arkworks-rs:main Oct 29, 2020
lazovicff referenced this pull request in tangle-network/crypto-primitives Mar 18, 2021
Co-authored-by: Pratyush Mishra <[email protected]>
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