-
Notifications
You must be signed in to change notification settings - Fork 167
Description
from @davidnevadoc
In halo2curves, the serialization of fields could be improved. There are methods that support different:
Endianness
Limbs, Bytes, bits
Motgomery form, non-montgomery form
Some are implemented through traits like SerdeObject others are implemented in PrimeField, others are implemented directly on the field. It would be great to remove redundancies, estabilish a clear name convention and document the end result.
Related issue: #109
First round, initial idea
Halo2curves serializations comes with two flavours:
-
Standard serialization
- API
- Implement trait
Field.{from,to}_repr
fn - Implement trait
group::GroupEncoding (for compressed)
, andgroup::UncompresseEncoding
(for uncompressed), allowing "unchecked" reads.
- Implement trait
- To match the serialization "standards", needs predefined format, configurable via macro at field/curve definition, not configurable when serialize/deserialize:
- Fields: field
endian
infield_impl!
macro - Curves: impl
EndianRepr
for endianess, implCompressed
for flags and to uncompress.
- Fields: field
- Since repr does not have to match with encoding endianess, fields implements {from,to}_bytes(). Let's say that since we are using PrimeFields, we do not need checked reads here because this is only checking that fits inside modulos.
- Implements serde wrappers over group:xxxxEncodings
- Note that allows hex encodings (::hex::serde::deserialize(deserializer)) and I'm not very sure if this could break constant-time without notice
- API
-
Vainilla "raw" serialization
- API
- fn
{from, to}_raw_bytes_[unchecked]
- fn
{read,write}_raw_[unchecked]
- fn
- Quick, just little endian, no special flags.
- Implememented via SerdeObject trait (which is not serde) at Field, Curve and tower
- API
-
Others
Notes:
- I really would like to get rid of
EndianRepr
but it seems like some kind of type-hack to allow to knowing the endiannes of CurveAffine::BASE when encoding compressing curves. Since our halo2curves takes traits from pasta, we are not able to modify it. - There are other ways to build fields:
- Field.from_raw() is not designed for serialization, and it seems using this name. A more rational name will be from_integer IMO, but we cannot change this because from_raw is used in a derive calling pasta_curves at this moment.
- from_uniform_bytes for random ones
- from_mont for montgomery
Unserialize performance:

legend:
- cur / afi : Curve / Affine
- chk / unchk : Checked / Unchecked
- comp / uncomp : Compressed / Uncompressed
code: https://gist.github.com/adria0/c440185d765a368aaf21ca5741a63ab7
Initial proposal:
- Remove SerdeObject. Why we need this? If we want to quick process data between trusted parties we have UncompresseEncoding::from_uncompressed_unchecked (that is in the order of magnitude of SerdeObject decodings), if not we can use GroupEncoding::from_bytes that fits into standards. Also having a mix of endianess if we use one or another API just creates confusion IMO. The memory footprint that would save by using {read,write}_raw will be really low, so even if we decide to enter into zerocopy stuff, r/w traits here are not a significant improvment.
- IMO Arkworks is the standard of interoperability at this moment, we must encode everything as this guys do.
- Implement serde for Field, is not implemented
EndianRepr::from_bytes
,EndianRepr::to_bytes
is implemented in Field (that aready have these fields). This is extra confusing, these are just intended to be used internally as helpers. So rename this fields for something like endianrepr_{from,to}_bytes
Comments: @davidnevadoc @duguorong009 @kilic ?