Consistency regarding from_bytes APIs

[tarcieri]

There are several APIs for converting various types from bytes which generally take the form:

TypeName::from_bytes(bytes: &[u8; N])

There's one exception to this:

Scalar:::from_bytes_mod_order(bytes: [u8; 32])

...which makes a copy of the byte array. It should probably be changed to borrow the array, especially as from_bytes_mod_order_wide takes a &[u8; 64].

Another question is how From/TryFrom impls should be defined for these types:

1. If a type can be infallibly parsed from a byte array, should we add a From<[u8; N]> impl?
2. If a type can fallibly be parsed from a byte array, should we add a TryFrom<[u8; N]> impl?
3. Should we impl TryFrom<&[u8]>? (always fallible since the slice might be the wrong size)

An important consideration here is if multiple overlapping impls of the same trait are allowed. If we try to add that after a major release, it breaks inference, which I think is generally allowed under Rust SemVer, but can break downstream code and is generally annoying to deal with.

Edit: CompressedEdwardsY and CompressedRistretto both have a from_slice method that panics of the slice is the wrong length. We should perhaps consider returning a Result instead.

[tarcieri]

Note that right now ed25519-dalek also follows the borrowed byte array convention for from_bytes, and impls 3, but not 1 or 2.

[tarcieri]

Related is #490: should we add an EdwardsPoint::from_bytes API that avoids the need to use CompressedEdwardsY?

[SergeStrashko]

Scalar has more than one method from_* ([u8;32])

pub fn from_bytes_mod_order(bytes: [u8; 32]) -> Scalar 

then

    pub fn from_canonical_bytes(bytes: [u8; 32]) -> CtOption<Scalar> 

and

    pub const fn from_bits(bytes: [u8; 32]) -> Scalar

I think it might be confusing using From<&[u8;32]> for Scalar what to expect? Is it same as from_bits or from_bytes_mod_order ?

[tarcieri]

For Scalar specifically I think if we were to add a trait-based conversion, it would make sense to have TryFrom<[u8; 32]> which calls Scalar::from_canonical_bytes.

That would provide a nice convenience wrapper for cases where Scalar is non-secret and users don't want to deal with CtOption.

[tarcieri]

Well, now I see why Scalar:::from_bytes_mod_order(bytes: [u8; 32]) accepts its argument that way: the first thing it does is move it into the Scalar struct.

Sidebar: I'm a bit confused why Scalar is internally a [u8; 32] rather than a newtype for a (backend-specific) UnpackedScalar. It seems like the very first thing Scalar does before any arithmetic is unpack().

[tarcieri]

Whoops, didn't mean to close that quite yet.

I guess one advantage of the Scalar/UnpackedScalar is it makes const fn support a bit easier, since UnpackedScalar has to be done on a backend-by-backend basis, and it's not possible to make the SIMD backends const fn.

Opened #493 regarding const fn support.

[SergeStrashko]

I can make existing Scalar52 and Scalar29 most of the methods const
For reference please have a look SergeStrashko#1
It should allow Scalar(pub(crate) UnpackedScalar) at least for serial backends.
The PR above also changes Scalar to be Scalar(UnpackedScalar)

[nox]

How do we get progress on this, to finally release 4.0.0?

[tarcieri]

As noted above, upon further review the inherent methods seem to have good reasons for being the way they are.

So the main thing would be to survey places where TryFrom could be used where it presently isn't.

[hrxi]

So the main thing would be to survey places where TryFrom could be used where it presently isn't.

Since TryFrom implementations could be added later, maybe this could be made non-blocking for the release?

[tarcieri]

Yeah, only thing to worry about is overlapping impls causing inference failure when you go from 1 -> 2 or more, but that isn't technically a breaking change.

That said, I can close this to unblock a release.