search

arkworks-rs / snark

Interfaces for Relations and SNARKs for these relations
https://www.arkworks.rs
Apache License 2.0
770 stars 207 forks source link

ToConstraintFieldGadget for SW ProjectiveVar #284

Closed weikengchen closed 3 years ago

weikengchen commented 3 years ago

The previous PR https://github.com/scipr-lab/zexe/pull/278 does not include the case of SW ProjectiveVar (but only AffineVar), but it is used in the main implementations of PairingVar for SW curves.

So, when we are refactoring the Marlin gadgets, we discovered that ToConstraintFieldGadget for SW ProjectiveVar is needed. This PR provides so.

weikengchen commented 3 years ago

The CI passes!

Pratyush commented 3 years ago

Should this serialize also the zero bit? I.e., by converting to SWAffine and serializing that?

weikengchen commented 3 years ago

You mean the infinity bit?

The current upstream implementation only to_constraint_fields x and y for Affine for SW. Should we also include the infinity bool? I intentionally removed this one since some PR (I forgot) starts to omit the infinity bool.

weikengchen commented 3 years ago

Ah, I misunderstood. So I double-read your comment and the code, now there are two action items:

  1. Handle the case when z=0. Now it would simply fail, but ideally we should do to_affine like https://github.com/scipr-lab/zexe/blob/master/r1cs-std/src/groups/curves/short_weierstrass/mod.rs#L154 first, and convert it to filed gadgets from an affine.

  2. Question: should we also to_constraint_field the infinity bit in SWAffine? Currently, it doesn't, but it seems safer to do so?

Pratyush commented 3 years ago

yes to both!

weikengchen commented 3 years ago

Done and CI passes. Please review it again. (The code is significantly shorter for ProjectiveVar!)

weikengchen commented 3 years ago

Closed temporarily. (The branch has been wrongly used for some other changes).

Will later push one to the arkworks's.