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

Introduce NonIdentityPoint #1170

Closed
daxpedda opened this issue Dec 16, 2022 · 7 comments · Fixed by #1176
Closed

Introduce NonIdentityPoint #1170

daxpedda opened this issue Dec 16, 2022 · 7 comments · Fixed by #1176

Comments

@daxpedda
Copy link
Contributor

This is similar to NonZeroScalar.

I often found myself handrolling custom serde implementations to make sure that a ProjectivePoint is not an identity element.

Additionally it could be useful to allow arithmetic that can guarantee some outputs without having to constantly check for identity elements. For example multiplying a NonIdentityElement with a NonZeroScalar should yield a NonIdentityElement without having to do any checks or returning an (Ct)Option.

Interactions with PublicKey could be similar to whats done between NonZeroScalar and SecretKey, enabling non-fallible conversions.

Happy to make a PR of course.

@tarcieri
Copy link
Member

Seems fine to me.

Wonder if it should be NonIdentity<T> so you can have NonIdentity<AffinePoint<C>> and NonIdentity<ProjectivePoint<C>>

@daxpedda
Copy link
Contributor Author

Personally I don't have a use-case for it, but it's a great idea.
The API might look a bit convoluted though, because there is no common trait.

Alternatively we could also do NonProjectiveIdentity and NonAffineIdentity.

I will start on an implementation with NonIdentity<T> as you suggested and we can go from there, feel free to suggest otherwise anytime.

@daxpedda
Copy link
Contributor Author

daxpedda commented Dec 16, 2022

So somehow I have to implement the same functions over NonIdentity<ProjectivePoint<C>> and NonIdentity<AffinePoint<C>>.

As they don't share traits I thought I could do something like this: impl<C> NonIdentity<ProjectivePoint<C>> { ... }, which I quickly found out isn't allowed.

I could solve this issue by making a NonIdentityExt trait, which would maybe allow something like impl<C> NonIdentityExt for NonIdentity<ProjectivePoint<C>> and impl<C> NonIdentityExt for NonIdentity<AffinePoint<C>> with the same function names.

So currently I'm leaning towards just making them two separate types, as proposed above.
Any different suggestions?

@tarcieri
Copy link
Member

So somehow I have to implement the same functions over NonIdentity<ProjectivePoint> and NonIdentity<AffinePoint>.

As they don't share traits...

It seems like quite a few of them should be able to be generic. They do share traits, namely the core::ops ones.

@daxpedda
Copy link
Contributor Author

Yeah, so the problem was how to make a new function, there is no commonly shared trait between AffinePoint and ProjectivePoint to get the identity point. We could add one?

Alternatively I will just make new_projective() and new_affine().

@daxpedda
Copy link
Contributor Author

Cc zkcrypto/group#41.

@tarcieri
Copy link
Member

there is no commonly shared trait between AffinePoint and ProjectivePoint to get the identity point.

Default ?

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 a pull request may close this issue.

2 participants