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

Reduce duplicate code across different curve cycle providers #255

Merged
merged 3 commits into from
Nov 20, 2023

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.

src/traits/mod.rs Outdated Show resolved Hide resolved
src/provider/mod.rs Outdated Show resolved Hide resolved
@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
6 checks passed
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 <[email protected]>
Co-authored-by: ashWhiteHat <[email protected]>
Co-authored-by: Srinath Setty <[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.

3 participants