Skip to content

Conversation

@tarcieri
Copy link
Member

@tarcieri tarcieri commented Jul 31, 2020

Adds a NonZeroScalar type generic over curves along with a Generator trait which together with the Mul trait from core can replace the MulBase trait.

NonZeroScalar is able to use the bounds on Arithmetic::Scalar and therefore usable anywhere a Scalar otherwise would, but with the guarantee that multiplication by an AffinePoint will not result in the point at infinity, for which no AffinePoint representation is possible given current trait bounds/implementations.

cc @fjarri

@tarcieri tarcieri requested a review from str4d July 31, 2020 18:49
/// Create a [`NonZeroScalar`] from a scalar, performing a constant-time
/// check that it's non-zero.
pub fn new(scalar: S) -> CtOption<Self> {
let is_some = scalar.ct_eq(&S::default());
Copy link
Contributor

Choose a reason for hiding this comment

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

It's a bit weird using default() and not zero()/is_zero(), but I guess it's the best that can be done at the moment. Should probably be refactored when there's a fully-functional Scalar trait (or a collection of traits) is available.

Copy link
Member Author

@tarcieri tarcieri Jul 31, 2020

Choose a reason for hiding this comment

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

Yeah, it's the only extant trait to define this in terms of right now.

However, I wouldn't say it's "weird" per se, at least for usage of Default in Rust: zero is the Default value for all numerical types in the standard library.

Copy link
Contributor

Choose a reason for hiding this comment

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

I suppose it's ok for simple types, but this approach may fail if Scalar ever implements lazy reduction arithmetic similar to that in FieldElement (and it will fail for the current AffinePoint/ProjectivePoint), because in that case you could have something that is zero/identity and yet isn't default().

@tarcieri tarcieri force-pushed the elliptic-curve/non-zero-scalar branch 2 times, most recently from fabba1e to f420e79 Compare August 2, 2020 17:50
@tarcieri tarcieri changed the title elliptic-curve: NonZeroScalar type elliptic-curve: NonZeroScalar + Generator (replaces MulBase) Aug 2, 2020
@tarcieri
Copy link
Member Author

tarcieri commented Aug 2, 2020

Pushed up a slightly modified version of this PR, and also added Generator to it and removed MulBase. I have k256 wired up with all of this locally.

I changed NonZeroScalar to be generic over Curve + Arithmetic, which allows it to reuse the trait bounds for Arithmetic::Scalar. Perhaps a better solution here would be a Scalar trait, but I think that's something to explore in a separate PR.

I think I'm going to move forward with this for now as it also solves the problem of generic variable-base scalar mult in addition to fixed-base, which is useful for a high-level generic ECDHE API. That's not to say I consider this to be the final say here, just incremental progress in exposing more functionality.

Adds a `NonZeroScalar` type generic over curves along with a `Generator`
trait which together with the `Mul` trait from `core` can replace the
`MulBase` trait.

`NonZeroScalar` is able to use the bounds on `Arithmetic::Scalar` and
therefore usable anywhere a `Scalar` otherwise would, but with the
guarantee that multiplication by an `AffinePoint` will not result in the
point at infinity, for which no `AffinePoint` representation is possible
given current trait bounds/implementations.
@tarcieri tarcieri force-pushed the elliptic-curve/non-zero-scalar branch from f420e79 to c1bcc90 Compare August 2, 2020 17:57
@tarcieri tarcieri merged commit efefde4 into master Aug 2, 2020
@tarcieri tarcieri deleted the elliptic-curve/non-zero-scalar branch August 2, 2020 18:03
tarcieri added a commit to RustCrypto/elliptic-curves that referenced this pull request Aug 2, 2020
Impls the changes from RustCrypto/traits#241, replacing the `MulBase`
trait with a `Mul` impl on `AffinePoint`, which is now one of the
required bounds for `Arithmetic`.
tarcieri added a commit to RustCrypto/elliptic-curves that referenced this pull request Aug 2, 2020
Impls the changes from RustCrypto/traits#241, replacing the `MulBase`
trait with a `Mul` impl on `AffinePoint`, which is now one of the
required bounds for `Arithmetic`.
@tarcieri tarcieri mentioned this pull request Aug 10, 2020
dns2utf8 pushed a commit to dns2utf8/traits that referenced this pull request Jan 24, 2023
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.

3 participants