Implement a checked conversion from bytes method in Fq #81
Conversation
| /// None, enforcing canonical serialization. | ||
| pub fn from_bytes_checked(bytes: &[u8; 32]) -> Option<Self> { | ||
| let reduced = Self::from_raw_bytes(bytes); | ||
| if reduced.to_bytes_le() == *bytes { |
There was a problem hiding this comment.
unrelated to the contents of this pr, but why is this to_bytes_le rather than just to_bytes? are we ever doing big-endian serialization? if not, could we have it just be to_bytes?
There was a problem hiding this comment.
My motivation was that:
- Arkworks has
from_bytes_leandfrom_bytes_bemethods, so this mirrors that. - Furthermore, I think it's good to specify what the endianness is, because it prevents any confusion on the user's part.
There was a problem hiding this comment.
this is a bikeshed, but my counterpoints would be:
- it's inconsistent to have from_bytes but to_bytes_le
- it's better to not convey endianness, because it signals that endianness is not a user-visible concern, the cryptography layer just exposes an "encoding" abstraction which is to be treated as opaque bytes
There was a problem hiding this comment.
That's a good point, although for context, there's a separate type for the Encoding of an Element on the curve, and in practice there's no reason for users to be aware of Fq at all. For Fr, this is a reasonable point though, yeah. One reason having those can be nice is that the direct methods on each field are also what we use to implement the Arkworks traits, and there, because different methods care about endiannness, it's useful to know what endianness we're using when implementing them.
In an ideal world Arkworks would be more opaque; alas.
Can help implement #80.