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.

Sign up for GitHub

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

Jump to bottom

Why is NonZeroScalar::invert() fallible? #499

Closed
fjarri opened this issue Dec 27, 2021 · 3 comments · Fixed by RustCrypto/traits#894
Closed

Why is NonZeroScalar::invert() fallible? #499

fjarri opened this issue Dec 27, 2021 · 3 comments · Fixed by RustCrypto/traits#894

Comments

@fjarri
Copy link
Contributor

fjarri commented Dec 27, 2021

It feels like, since we know that it is nonzero, there should be some fn invert(&self) -> NonZeroScalar, or at least -> Scalar, instead of -> CtOption<Scalar>, which will never have the None variant. Isn't guaranteed invertibility one of the major uses of a NonZeroScalar type?
@tarcieri
Copy link
Member

tarcieri commented Dec 27, 2021 •
edited
Loading

The issue is Invert is a trait and the current definition uses CtOption<Self::Output> as a return type.

It would probably make sense to get that CtOption out of the trait signature and allow NonZeroScalar's impl to be infallible. Other impls could choose to set type Output = CtOption<Self>.

Unfortunately this would require a breaking change to the elliptic-curve crate so it can't happen until the next release.

@fjarri
Copy link
Contributor Author

fjarri commented Dec 30, 2021

Or perhaps we could have Invert (infallible) and TryInvert (defaulting to Invert if it's defined)?

@fjarri fjarri mentioned this issue Dec 30, 2021
Merged
tarcieri added a commit to RustCrypto/traits that referenced this issue Jan 17, 2022
@tarcieri
elliptic-curve: make NonZeroScalar::invert infallible
44c72a1 
Because `NonZeroScalar` means we'll never divide by 0, it's possible to
make the implementation infallible.

To accomplish this, `CtOption` is removed from the `Invert` trait's
signature, and used as the result type for scalars that are potentially
zero as part of the blanket impl of `Invert`.

Fixes RustCrypto/elliptic-curves#499
@tarcieri tarcieri mentioned this issue Jan 17, 2022
Merged
tarcieri added a commit to RustCrypto/traits that referenced this issue Jan 17, 2022
@tarcieri
elliptic-curve: make NonZeroScalar::invert infallible
712434d 
Because `NonZeroScalar` means we'll never divide by 0, it's possible to
make the implementation infallible.

To accomplish this, `CtOption` is removed from the `Invert` trait's
signature, and used as the result type for scalars that are potentially
zero as part of the blanket impl of `Invert`.

Fixes RustCrypto/elliptic-curves#499
tarcieri added a commit to RustCrypto/traits that referenced this issue Jan 17, 2022
@tarcieri
elliptic-curve: make NonZeroScalar::invert infallible (#894)
bb3d0de 
Because `NonZeroScalar` means we'll never divide by 0, it's possible to
make the implementation infallible.

To accomplish this, `CtOption` is removed from the `Invert` trait's
signature, and used as the result type for scalars that are potentially
zero as part of the blanket impl of `Invert`.

Fixes RustCrypto/elliptic-curves#499
@tarcieri
Copy link
Member

RustCrypto/traits#894 implemented the proposed changes:

  • CtOption is no longer a mandatory part of the return type for Invert
  • The blanket impl of Invert returns a CtOption
  • The impl of Invert for NonZeroScalar now infallibly returns a NonZeroScalar

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

elliptic-curve: make NonZeroScalar::invert infallible RustCrypto/traits
2 participants
@tarcieri @fjarri