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

arkworks independent projective arithmetic ops #77

Merged
merged 2 commits into from
Jan 29, 2024
Merged

Conversation

redshiftzero
Copy link
Member

@redshiftzero redshiftzero commented Jan 26, 2024

Adding projective operations for #63

@cronokirby
Copy link
Contributor

What's the difference between ops and projective/ops?

@redshiftzero redshiftzero marked this pull request as ready for review January 26, 2024 19:18
@redshiftzero
Copy link
Member Author

Currently nothing, my thinking was to mirror the structure from the previous code (now under the arkworks feature) where we have a projective.rs and affine.rs for implementing arithmetic operations on Element and AffineElement respectively

@redshiftzero redshiftzero changed the title wip: projective arithmetic ops arkworks independent projective arithmetic ops Jan 26, 2024
@cronokirby
Copy link
Contributor

It surprises me that the code even compiles; shouldn't Rust complain about there being two implementations of the same trait for Element?

Also, I think we should just not expose arithmetic on Affine. Like, the reason Affine exists is only for compatability with arkworks, and as a "I know what I'm doing" button that people can press to convert from the Affine representation to the Element.

@redshiftzero
Copy link
Member Author

ah, so it compiles because the projective arithmetic implementations in the arkworks feature are on the Element that holds an inner arkworks EdwardsProjective point, but the projective arithmetic implementations added in this PR are on the arkworks independent Element in the smol_curve module

Also, I think we should just not expose arithmetic on Affine. Like, the reason Affine exists is only for compatability with arkworks, and as a "I know what I'm doing" button that people can press to convert from the Affine representation to the Element.

agreed 100%!

@cronokirby
Copy link
Contributor

I think that second Element type will not exist in the near future, because we won't have the arkworks EdwardsProjective appear anywhere; instead I imagine we'll just implement the Arkworks traits on Element and AffineElement.

This makes sense to me because we don't plan on implementing operations
for affine points, so removing the level of nesting is a good change.
@cronokirby cronokirby merged commit 1d85804 into main Jan 29, 2024
6 checks passed
@cronokirby cronokirby deleted the smol-ecc-2 branch January 29, 2024 17:53
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