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

Refactor identity methods into Identity trait #42

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

daxpedda
Copy link

Introduces a new trait: Identity.
Moving all identity() and is_identity() methods to it. This enables generalizing over different point types when the identity point is needed.

This is a breaking change.
Fixes #41.

src/lib.rs Outdated Show resolved Hide resolved
Co-authored-by: Tony Arcieri <bascule@gmail.com>
src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated
@@ -172,3 +167,13 @@ pub trait UncompressedEncoding: Sized {
/// the point at infinity.
fn to_uncompressed(&self) -> Self::Uncompressed;
}

/// Obtain or determine the identity point.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issue ("point" instead of "element").

I don't know how to describe this trait, which is concerning. It's not an extension trait because it's being added as a required bound. But the constant and method it defines do not make sense in a vacuum; they need to be in the context of a group.

This makes me question the premise of the PR: what is the motivating use case for generalising over both the affine and efficient forms of a curve point, such that a shared trait is needed?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies for the delay.

The original motivation was RustCrypto/traits#1170. We currently use Default to obtain the identity element, which isn't ideal and doesn't support const.

Unfortunately this was a long time ago and I don't have all the details in my head anymore, but I assume these sort of requirements will keep popping up in elliptic-curve because of it's nature: it introduces types that are generic over many different things, so it requires traits that are equally generic to support this idea.

Maybe @tarcieri has more to say on this topic.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't think of a use case for such a generalization

Co-Authored-By: str4d <str4d@users.noreply.github.com>
@daxpedda daxpedda requested a review from str4d August 8, 2023 14:27
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.

Shared identity point trait
3 participants