Skip to content

Refactor identity methods into Identity trait#42

Open
daxpedda wants to merge 3 commits intozkcrypto:mainfrom
khonsulabs:identity
Open

Refactor identity methods into Identity trait#42
daxpedda wants to merge 3 commits intozkcrypto:mainfrom
khonsulabs:identity

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.

Co-authored-by: Tony Arcieri <bascule@gmail.com>
src/lib.rs Outdated
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