Skip to content

Conversation

@ashWhiteHat
Copy link
Contributor

@ashWhiteHat ashWhiteHat commented Nov 8, 2023

I reduced test and transcript trait code across curve cycle by macros.

improvement
curve cycle group methods difference is only vartime_multiscalar_mul.
https://github.com/microsoft/Nova/blob/main/src/provider/mod.rs#L159
If we call msm method through such that Self::msm(), we can use same trait between pasta and other cycle pair.

question
Is there any reason not to use complete addition for ecc gadget?
https://github.com/microsoft/Nova/blob/main/src/gadgets/ecc.rs#L135
We can skip condition branch constraint.

typo
the duplication
the
https://eprint.iacr.org/2023/1192.pdf#page=4&zoom=100,100,250

I would appreciate it if you could confirm.
Thank you.

@srinathsetty
Copy link
Collaborator

close #202

I reduced test and transcript trait code across curve cycle by macros.

Does it actually close the issue? There is still impl_traits that is duplicated between mod.rs and pasta.rs, right?

@srinathsetty
Copy link
Collaborator

question
Is there any reason not to use complete addition for ecc gadget?
https://github.com/microsoft/Nova/blob/main/src/gadgets/ecc.rs#L135
We can skip condition branch constraint.

I don't fully understand this question. Can you please elaborate?

@ashWhiteHat
Copy link
Contributor Author

Does it actually close the issue? There is still impl_traits that is duplicated between mod.rs and pasta.rs, right?

Exactly.
I unlinked the issue.

I don't fully understand this question. Can you please elaborate?

Sorry for confusing.
This seems my mistake.
I thought that the curve complete addition law can save the number of constraints because it can skip condition branch (identity and doubling case).
but it can save the number of constraints because it can perform affine coordinate arithmetic by using the feature that the inversion cost is as cheap as multiplication as following article says.
https://bitzecbzc.github.io/technology/jubjub/index.html#introducing-jubjub

It seems we can't save the number of constraints, because the Weierstrass affine coordinate still needs condition branch.

@srinathsetty srinathsetty merged commit 3e05f5d into microsoft:main Nov 20, 2023
huitseeker added a commit to huitseeker/Nova that referenced this pull request Nov 27, 2023
* Small code improvement to the minroot example (microsoft#264)

about 10% improvement for the non-release mode

* Reduce duplicate code across different curve cycle providers (microsoft#255)

* refactor: impl folding macro

* refactor: generalize curve test

* chore: rename impl_folding to impl_engine

* reorganize provider module (microsoft#267)

---------

Co-authored-by: field-worker <151173028+field-worker@users.noreply.github.com>
Co-authored-by: ashWhiteHat <phantomofrotten@gmail.com>
Co-authored-by: Srinath Setty <srinath@microsoft.com>
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