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

Add helpers to get 0/1 as integer associated types #819

Merged
merged 2 commits into from
Nov 8, 2023

Conversation

divergentdave
Copy link
Collaborator

This adds new methods zero_integer() and one_integer() to FieldElementWithInteger. These replace an existing pattern of constructing zero or one with a round trip through the Montgomery representation, by constructing it as a field element first, and then converting it back. This preparatory work for addressing #643.

I considered and rejected creating a new trait for integers, and putting these methods there, because adding a new trait bound to the Integer associated type would be a breaking change. We could reconsider this API design the next time we make significant breaking changes.

@divergentdave divergentdave requested a review from a team as a code owner November 3, 2023 19:47
Copy link
Contributor

@tgeoghegan tgeoghegan left a comment

Choose a reason for hiding this comment

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

In the interest of limiting the public API commitment, would it be possible to provide the public helpers in #820 while keeping the new zero_integer and one_integer methods private? i.e. put them on a new trait FieldElementWithIntegerPrivate (a mouthful, I know) that we don't export.

edit: ah, we already have FieldElementWithIntegerExt for just this purpose.

@divergentdave
Copy link
Collaborator Author

Sure, we can always make it public later, maybe via a new Integer trait laid out above

@divergentdave divergentdave merged commit e0e8f54 into main Nov 8, 2023
6 checks passed
@divergentdave divergentdave deleted the david/integer-identities branch November 8, 2023 16:34
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