`List` deserialization (serde) broken (tested with bincode)

[DragonDev1906]

The List type does not correctly serialize its size (serde), thus breaking any serialization implementation (tested with bincode) that requires a correct length. The test below serializes and then immediately deserializes a List of length 5 with a maximum size of 32, but the serialized data contains the length 2000000000000000 (32), thus breaking the deserialization.

---- test::bincode_sszlist stdout ----
[ethereum-runner/src/bin/eth-runner-inprocess.rs:64] list.encode_hex::<String>() = "48656c6c6f"
[ethereum-runner/src/bin/eth-runner-inprocess.rs:66] data.encode_hex::<String>() = "200000000000000048656c6c6f"
thread 'test::bincode_sszlist' panicked at ethereum-runner/src/bin/eth-runner-inprocess.rs:67:65:
called `Result::unwrap()` on an `Err` value: Io(Kind(UnexpectedEof))
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

#[cfg(test)]
mod test {
    use hex::ToHex;
    use ssz_rs::List;

    #[test]
    fn bincode_sszlist() {
        type MyList = List<u8, 32>;

        let mut list: MyList = MyList::default();
        list.push(b'H');
        list.push(b'e');
        list.push(b'l');
        list.push(b'l');
        list.push(b'o');
        dbg!(list.encode_hex::<String>());

        let data = bincode::serialize(&list).unwrap();
        dbg!(data.encode_hex::<String>());

        let out: MyList = bincode::deserialize(data.as_slice()).unwrap();
    }
}

[dependencies]
ssz-rs = { package = "ssz_rs", version = "0.9.0" }
bincode = "1.3.3"
hex = { version = "0.4.3", features = ["serde"] }

[ralexstokes]

it seems that this is only an issue w/ bincode and this repo doesn't aim to support any possible serialization scheme, just ssz.

if you try your example w/ just the relevant SimpleSerialize and serde traits, there is no problem with encoding/decoding

it looks like there are specific bincode traits to derive Encode and Decode -- do you still have this problem if you derive those on your type?

[DragonDev1906]

if you try your example w/ just the relevant SimpleSerialize and serde traits, there is no problem with encoding/decoding

bincode just uses serde and its derived types. I assume there is no issue when using other serde formats (e.g. serde_json) because they don't rely on the size for deserialization.

it looks like there are specific bincode traits to derive Encode and Decode

Not that I know of and I couldn't find specific traits in bincode. As mentioned above: As far as I can tell bincode just uses the serde types.

In fact, when I tell the serde derive macro to set the correct length everything works as expected:

pub mod sszlist_hotfix {
    use serde::ser::SerializeSeq;
    use ssz_rs::List;

    pub fn serialize<S, T, const N: usize>(
        value: &List<T, N>,
        serializer: S,
    ) -> Result<S::Ok, S::Error>
    where
        S: serde::Serializer,
        T: ssz_rs::SimpleSerialize + serde::Serialize,
    {
        let mut seq = serializer.serialize_seq(Some(value.len()))?;
        for element in value.iter() {
            seq.serialize_element(element)?;
        }
        seq.end()
    }

The following is the serde::Serialize implementation for List<T, N> in ssz-rs. According to the documentation of serde, this argument is not the max size but the actual amount of elements.

#[cfg(feature = "serde")]
impl<T: SimpleSerialize + serde::Serialize, const N: usize> serde::Serialize for List<T, N> {
    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
    where
        S: serde::Serializer,
    {
        let mut seq = serializer.serialize_seq(Some(N))?;      // <---- Bug?
        for element in &self.data {
            seq.serialize_element(element)?;
        }
        seq.end()
    }
}

The argument is the number of elements in the sequence, which may or may not be computable before the sequence is iterated.

[ralexstokes]

Not that I know of and I couldn't find specific traits

ah sorry, looks like this is in version 2 of bincode and im not sure what the difference is

his argument is not the max size but the actual amount of elements.

yeah, I see this and I think you are correct this is a bug! thanks for raising this issue

I'll triple-check and go fix today if its an issue

[ralexstokes]

@DragonDev1906 this should work now after #111

let me know if you have any trouble