fix: Improve serialization for prime fields

[huitseeker]

Summary

256-bit field serialization is currently 4x u64, ie. the native format which isn't common
This implements the standard byte-serialization for those fields (corresponding to the PrimeField::{to,from}_repr), and an hex-encoded variant of those bytes for (de)serializers that are human-readable (e.g. json) and hence typically not raw byte-friendly.

Hex is a straightforward, common and very simple choice for string formats that's harder to mess up than Base64.

Details

• Added a new macro serialize_deserialize_32_byte_primefield! for custom serialization and deserialization of 32-byte prime field in different struct (Fq, Fp, Fr) across the secp256r, bn256, and secp256k1 modules.
• Implemented the new macro for serialization and deserialization in various structs, replacing the previous serde::{Deserialize, Serialize} direct derive use.
• Enhanced error checking in the custom serialization methods to ensure valid field elements.
• Updated the test function in the tests/field.rs file to include JSON serialization and deserialization tests

See also

#88

[han0110]

LGTM!

[mratsim]

For fields, we should point to the I2OSP and OSP2I spec as reference: https://www.rfc-editor.org/rfc/rfc8017.html#section-4

This serializing bytes by bytes, most significant byte first. (i.e. bigEndian serialization)

[han0110]

This serializing bytes by bytes, most significant byte first. (i.e. bigEndian serialization)

I think this makes more sense considering it's for human reading, didn't realize that the output will be in little-endian. Tho pasta_curves has it in little-endian.

[huitseeker]

RFC 8017 compliance is spotty at best, but more importantly it really is the domain of the field definition, which in the trait Primefield owns the methods (to_repr, from_repr). This PR deals with the more narrow scope of simply reusing this trait, which defines the binary representation, for serialization.
Perhaps remarks as to RFC conformance should be placed in the Primefield implementation?

[mratsim]

Looking at upstream: https://github.com/zkcrypto/ff/blob/e853770/ff_derive/src/lib.rs#L51-L103 they actually don't look like their purpose is serialization but more having a way to dump the physical representation (and not the logical one). Maybe @str4d can confirm.

Looking at zkcrypto/bls12_381, for serialization they use to_bytes:

• https://github.com/zkcrypto/bls12_381/blob/7de7b9d/src/g1.rs#L219-L224
• and it is big-endian as specified in https://github.com/zkcrypto/bls12_381/blob/7de7b9d/src/notes/serialization.rs

[huitseeker]

Looking at zkcrypto/bls12_381, the to_bytes of the Scalar field is in lower endian:
https://github.com/zkcrypto/bls12_381/blob/7de7b9d9c509b9973b35a3241b74bbbea95e700a/src/scalar.rs#L282-L296
The correct link for Fp's definition of to_bytes is this:
https://github.com/zkcrypto/bls12_381/blob/7de7b9d9c509b9973b35a3241b74bbbea95e700a/src/fp.rs#L211-L227
and you're right, it's in big endian. That to_bytes method is not in any trait, incidentally, and seems specific to serialization for BLS12-381. But note that to_repr and from_repr proxy those.

The point to me seems to be: to_repr, from_repr encode a specific choice of conversion of prime field elements to a byte string.

You mentioned the ff-derive macros, I would note that this includes:

• a modular reduction,
• requiring the user to specify one compile-time choice of endianness, using the macro attribute PrimeFieldReprEndianness. There is no runtime optionality re: endianness.

In that context, I'm not sure what you mean by "dumping the physical representation".

But the footprint of these derive macros is rather sparse, so perhaps taking a tour through some landmark implementations would help ¹:

• pasta : canonical lower-endian,
• rustcrypto canonical big-endian, and the linked PR documents why they removed endianness optionality (their to_repr implementations point to the to_bytes I link to)
• curve25519-dalek canonical lower-endian, and their to_repr implementation points to the to_bytes I link to,
• JubJub canonical lower-endian, and their to_repr implementation points to the to_bytes I link to,

All of this to say: there is a plurality of choices regarding endianness in {to,from}_repr, that's correct. Due to the orphan rule, those choices can only be made by the implementer of the curve, without overrides. Using to_repr in serialization (resp. from_repr in deserialization) offers a way to defer to the curve implementer's choice in that regard, which guarantees compatibility with those implementations, if not, admittedly, RFC 8017 compliance.

If we want to revisit the endianness choices for the curves defined in this repo, to e.g. align them with RFC 8017, I'm all for giving this consideration. But I think that falls beyond the scope of this PR, which is about making sure we don't "dump the physical representation" (as you say), and use a defined byte representation, as specified by the traits we both implement and use.

Footnotes

1. by canonical I mean the reduced form of the field element ↩

[mratsim]

To be clear, I'm OK with the PR as is. I'll open a RFC to discuss the standard.

[CPerezz]

LGTM! Would appreciate if I can get a reply first to my comment from @huitseeker .

Also, two things to add.

• I did not know about is_human_readable and it's super cool!
• Endianness-wise, I'm a big supporter of LE. I kinda agree that the concern with the spec should be probably lifted to ff/group first. And then resolve it here. While I understand that the spec is important, it's also important to consider that as @huitseeker all the traits & libs use LE almost everywhere.

I belive we can leave an issue for that maybe? And discuss it back during the next days?

[CPerezz]

I'm afraid this if will even make performance worse when it comes to serialization.

On another hand, this never has anything to do with raw serialization (which is usually the used for performance).
Do I understand correctly that we're using serde then as "debugging serialization"??

[huitseeker]

Serializers usually just implement is_human_readable as a constant, so this if is as cheap as can be. Examples:

• the default, inlined and often not overridden
• bincode
• cborium

The semantics of is_human_readable, as the doc indicates, is not debugging. It's simply articulating the difference between formats that are meant to be human-readable (JSON, YAML), and those that are not (bincode, CBOR, RLP, ...).

[CPerezz]

Serializers usually just implement is_human_readable as a constant, so this if is as cheap as can be.

Fair point.