Refactoring field serialization #179
Conversation
This commit introduces 2 changes: - Simplification of SerdeObject methods by using the aux function: u64s_from_bytes - Removal of [0, p-1] check in all versions, including _checked methods. (This should only be re-added if there is a method of performing such check in Montgomery-form directly).
|
Marking this as ready, since I haven't found a solution for the |
| } | ||
|
|
||
| fn from_raw_bytes(bytes: &[u8]) -> Option<Self> { | ||
| if bytes.len() != #size { | ||
| return None; | ||
| } | ||
| let elt = Self::from_raw_bytes_unchecked(bytes); | ||
| Self::is_less_than_modulus(&elt.0).then(|| elt) | ||
| Some(Self::from_raw_bytes_unchecked(bytes)) |
There was a problem hiding this comment.
@davidnevadoc, so now from_raw_bytes_unchecked and from_raw_bytes is the same?
Then from_raw_bytes(&[0u8;#size]) and from_raw_bytes(&[255u8;#size]) will become a valid point.
There was a problem hiding this comment.
Oh, I made a mistake while reading how the arithmetic in Montgomery form works. I believed that the Montgomery form of an element didn't necessarily have to be in the [0, p-1] range. But it does in order to have an efficient reduction after the multiplication.
The issue I opened is wrong then. I will re-introduce the check (for the checked version of course).
Without that change, this PR doesn't make much sense, since it just has some nits. Let's keep it on hold until we have all the planned changes to serialization and put them all together in a single PR.
There was a problem hiding this comment.
Oh, I made a mistake while reading how the arithmetic in Montgomery form works. I believed that the Montgomery form of an element didn't necessarily have to be in the [0, p-1] range. But it does in order to have an efficient reduction after the multiplication.
The issue I opened is wrong then. I will re-introduce the check (for the checked version of course).
Without that change, this PR doesn't make much sense, since it just has some nits. Let's keep it on hold until we have all the planned changes to serialization and put them all together in a single PR.
For example, the docs added in #177
This PR aims to:
is_less_than_modulus(Misuse ofis_less_than_modulus#178)SerdeObjectandfrom_raw. [Still WIP] (Serialization #176)is_less_than_modulusbug.This issue was solved by simply removing the check from the
SerdeObjectfunctions:from_raw_bytesandread_raw.This change makes the functions virtually identical to their
_uncheckedversions. I believe this is fine, as the design still makes sense for curve points, where there are other checks involved.I haven't found a way to keep the check while maintaining the purpose of this interface, which is speed. I'm not sure if there is a way of checking if a value in Montgomery-form lies in the appropriate range w/o performing a Montgomery reduction or any other operation of similar cost.
Simplify
SerdeObjectandFromUniformBytes.I've introduced the utility
u64s_from_bytes, to facilitate transforming bytes into limbs. It is a minor change but I think it reduces boilerplate and improves readability.from_raw[WIP]Still working on a satisfactory solution for this function. Some possibilities are:
_rawhere is canonical and not internal representation like inSerdeObject.Other options have been tried ( exposing functionality through new trait, renaming... ) without success. The roots of the problems are:
const fnin traits.pasta_curveswithout modifying them.