docknetwork / crypto

Rust crypto library for data privacy tools
Apache License 2.0
78 stars 21 forks source link

Fixing FieldBytes and AffineGroupBytes serialization to also work with RMP #7

Closed gitmalet closed 1 year ago

gitmalet commented 1 year ago

Hi,

Types that contain fields serialized as FieldBytes or AffineGroupBytes did not properly serialize with serialization crates other than serde_json. The first commit here adds a few lines to the serialization tests that check serialization with rmp_serde (i.e., a crate where it does not work currently). The errors can be seen there. The second commit changes the implementations of FieldBytes and AffineGroupBytes by implementing them as a generic version of the expansion of serde_conv!. Serializing types with this new implementation now works under both serde_json and rmp_serde.

Please consider merging this in order to allow serialization with RMP (and possibly other serialization crates).

lovesh commented 1 year ago

Hi. The no_std and wasm builds fail due to the use of std here. You can import io from ark_std. Some tests fail but if i comment out the rmp_serde serialization here, they work.

gitmalet commented 1 year ago

Thanks, I did both changes (and a cargo format run) and squashed the commits so that they (hopefully) make sense again.

lovesh commented 1 year ago

Hi. The message pack serialization should be checked in the test_serialization macro in test_utils crate like you initial change. And wasm and no_std builds fail due to reason mentioned before.

gitmalet commented 1 year ago

Sorry for the delay. Unit tests run through on my local machine now at least. Also the no-std builds should work as well.

A notable downside of this PR is that the error messages are now generic as well. I am not sure how important this is.

lovesh commented 1 year ago

Thanks @gitmalet . LGTM.