Move Input and Output traits to a separate crate codec-io #122
Conversation
We will eventually face the issue of dealing with multiple binary encoding formats (such as ssz). In those situations, it is apparent that serde's model (using one `Serialize` and `Deserialize` trait for everything) is not sufficient. For example, U256 needs to be serialized as string in json, while as bytearray in SCALE and ssz. This is not possible to be represented by basic Rust structures. Instead, we need to define Encode/Decode traits for each format we want to encode (have `scale::Encode`, `ssz::Encode`, etc). Those definitions can share common traits `Input` and `Output`. In this PR, I moved those two traits into a separate crate `codec-io` to make it reusable. Note that I also generalized `Error` -- in some other encoding libraries we may not want to represent Error as solely `&str` or unit type and want to actually derive the actual error and pass it to library user. This requires changing `Decode` trait in parity-codec to have an extra bound `From<I::Error>`, which is a breaking change.
There was a problem hiding this comment.
While I understand the intentions behind your changes, I'm not sure we want to integrate them.
They are more like legacy traits and I would more likely switch to BufMut and Buf. Both seem to be widely accepted in the Rust ecosystem.
If you want to use Input/Output in ssz, I would propose to just copy them. We don't really have any special implementations for these traits outside of codec, so I don't see any real advantage for reusing them. With this change we would need to replicate the From bound to everywhere implementation of Decode and this does not sounds like something we want to strive for.
Cargo.toml
Outdated
| @@ -1,7 +1,7 @@ | |||
| [package] | |||
| name = "parity-scale-codec" | |||
| description = "SCALE - Simple Concatenating Aggregated Little Endians" | |||
| version = "1.0.0" | |||
| version = "2.0.0" | |||
There was a problem hiding this comment.
This version is not released, so please revert this.
| NotEnoughData, | ||
| } | ||
|
|
||
| #[cfg(not(feature = "std"))] |
There was a problem hiding this comment.
As far as I understand in std slice implements io::Read, so we cannot have this here.
|
@bkchr |
Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>
Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>
Co-Authored-By: Bastian Köcher <bkchr@users.noreply.github.com>
|
(I'm going to temporarily close this PR and embed this in ssz for now. I realized that there're still a few design that I haven't finalized.) |
|
They are working on it: tokio-rs/bytes#256 |
We will eventually face the issue of dealing with multiple binary encoding formats (such as ssz). In those situations, it is apparent that serde's model (using one
SerializeandDeserializetrait for everything) is not sufficient. For example, U256 needs to be serialized as string in json, while as bytearray in SCALE and ssz. This is not possible to be represented by basic Rust structures.Instead, we need to define Encode/Decode traits for each format we want to encode (have
scale::Encode,ssz::Encode, etc). Those definitions can share common traitsInputandOutput. In this PR, I moved those two traits into a separate cratecodec-ioto make it reusable.Note that I also generalized(This is not needed any more. I tried a different design that aliasError-- in some other encoding libraries we may not want to represent Error as solely&stror unit type and want to actually derive the error and pass it to library user. This requires changingDecodetrait in parity-codec to have an extra boundFrom<I::Error>, which is a breaking change.codec_io::Errortostd::io::Errorinstd.)