Allow Serde-serialisation of Streamable #127
Conversation
|
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 |
|
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. |
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 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:
I checked equality by dumping the |
| impl<const N: usize> Serialize for BytesImpl<N> { | ||
| fn serialize<S: Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> { | ||
| let s = format!("0x{}", hex::encode(&self.0)); | ||
| serializer.serialize_str(&s) |
There was a problem hiding this comment.
this is a bit of a hack. it will make the JSON serializer do the right thing, but any other serde serializer will do the wrong thing. which may not be the end of the world, since we don't need any other one, but it might also be surprising.
There was a problem hiding this comment.
Yes, this is a bit of a hack. I'll have a look if I can limit the specialisation to just serde_json's Serializer and let every other serde serializer just serialize as bytes.
There was a problem hiding this comment.
Nope, still no way, afaict.
|
I suspect the |
|
technically they are 1-tuples of |
|
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 |
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.