Allow Serde-serialisation of Streamable
This pull-request adds a "serde" build feature to chia-protocol. When this build feature is enabled, all chia-protocol types derive Serde's Serialize. This allows chia-protocol types to be serialised via Serde to different formats, most notably probably JSON, which can be very helpful for testing or glueing into other systems.
This PR only gets basic Serde serialisation working, without support for deserialisation. Serialisation follows Serde's defaults wherever possible. Fixed-length byte types (bytes48, bytes96, etc) are serialised to an array of bytes (numbers) for a start.
This PR is primarily to start discussion if there's interest in merging this into chia_rs at all. We are currently using this change to use chia_rs to convert from fullblock data to JSON for ad-hoc scripting/reporting. If there's interest in merging this, a next step would probably ensure that the JSON generated by chia_rs matches the JSON generated by chia-blockchain's Python.
one thing that worries me a bit about this is that it may create an expectation of stability of names of struct members. serializing classes into json based on reflection has bitten us a few times in python, and we have a few (pretty ugly) work-arounds to preserve some level of backwards compatibility.
Are the FullBlock JSON you produce compatible with what chia-blockchain produces?
that said, we already serialize to JSON on the python side, so we will most likely want to do the same on the rust side, in a compatible manner.
Are the FullBlock JSON you produce compatible with what chia-blockchain produces?
Nope, not yet, at least not intentionally or verified. If there's interest in merging this, I'll look into getting chia_rs's Rust JSON to structurally match chia-blockchain's Python JSON.
Meanwhile, you could either merge this as is right away and I'll send a separate PR for JSON-compatibility with Python in the coming days/weeks. Or you leave this PR pending and I'll update it with necessary changes for Python compatibility.
the reason I hesitate is because we'll need caompatibility with the JSON protocol established by python already. In order to make it compatible, I imagine you'd need to "fork" the built-in json serializer that comes with serde to modify it, or possibly modify some other aspect to make it compatible.
I'm pretty confident that it would be simpler to add json support in the macros we use for Streamable support right now. Our first attempt to serialize and deserialize classes in Streamable format was via serde, and we needed a lot of code to get that to work. Partly because we were required to handle types that aren't supported by Streamable, but we also had to write more code just to deal with the serde API. using the macro greatly simplified the code.
Ok, I'll have a quick look into establishing structural compatibility with the Python JSON to get an idea how much work that'll be. I don't think a serde fork is necessary for that, currently I believe it would boil down to adding a few more Serialize specializations. Just like the one already in place for BytesImpl that serialises [u8, N] into a hex-encoded string instead of an array of numbers.
JSON serialisation via the streamable macros would certainly also be a good option. I'm not sure, though, it would be much simpler. You'd basically need the same type-specific overrides and at least a basic JSON serialiser on top of that.
This latest version now produces JSON identical in structure and content to chia-blockchain's Python JSON serialisation.
Two things were missing compared to my original version:
- bytes serialised as hexstrings needed a
0xprefix. - Variable-length bytes also needed serde specialisation to be serialised as hexstrings.
I checked equality by dumping the FullBlocks for mainnet height 1519806 (~48KiB raw streamable-encoded data, after zstd decompression) and height 1592629 (~417KiB) as JSON from Python (json.dumps(block.to_json_dict())) and from Rust. I then ran both JSONs through jq --compact . to unify formatting and then diffed the results.
I suspect the G1Element and G2Element may need some fixup too
I suspect the
G1ElementandG2Elementmay need some fixup too
G1Element is just a Bytes48 and G2Element a Bytes96, at the moment. The specialisation for fixed-length BytesImpl takes care of those. Similarly, with variable-length Bytes and Program.
G1Element is just a Bytes48 and G2Element a Bytes96, at the moment. The specialisation for fixed-length BytesImpl takes care of those. Similarly, with variable-length Bytes and Program.
technically they are 1-tuples of Bytes48 and Bytes96. Perhaps the serde JSON serializer flattens those. I don't know.
Yes, the JSON serialiser flattens those (https://serde.rs/json.html#structs-and-enums-in-json).
I believe @Rigidity might be interested in picking this up
Pull Request Test Coverage Report for Build 9875600010
Details
- 1 of 16 (6.25%) changed or added relevant lines in 4 files are covered.
- 1157 unchanged lines in 32 files lost coverage.
- Overall coverage decreased (-8.3%) to 74.364%
| Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
|---|---|---|---|
| crates/chia-protocol/src/bytes.rs | 0 | 3 | 0.0% |
| crates/chia-bls/src/public_key.rs | 0 | 6 | 0.0% |
| crates/chia-bls/src/signature.rs | 0 | 6 | 0.0% |
| <!-- | Total: | 1 | 16 |
| Files with Coverage Reduction | New Missed Lines | % |
|---|---|---|
| crates/chia-consensus/src/gen/run_puzzle.rs | 1 | 98.31% |
| crates/chia-protocol/src/classgroup.rs | 1 | 0.0% |
| crates/chia-protocol/src/chia_protocol.rs | 2 | 0.0% |
| crates/chia-consensus/src/gen/run_block_generator.rs | 2 | 94.83% |
| crates/chia-consensus/src/consensus_constants.rs | 2 | 0.0% |
| crates/chia-bls/src/parse_hex.rs | 3 | 60.0% |
| crates/chia-consensus/src/gen/validation_error.rs | 4 | 24.19% |
| wheel/src/adapt_response.rs | 4 | 0.0% |
| crates/chia-bls/src/error.rs | 4 | 0.0% |
| crates/chia-traits/src/chia_error.rs | 4 | 0.0% |
| <!-- | Total: | 1157 |
| Totals | |
|---|---|
| Change from base Build 9866893150: | -8.3% |
| Covered Lines: | 10405 |
| Relevant Lines: | 13992 |
💛 - Coveralls
Hey, sorry for the huge delay on this, it's gone through several iterations that we weren't happy with but I think this is finally resolved by https://github.com/Chia-Network/chia_rs/pull/901