From slice take and split

[vlopes11]

A recurrent pattern is when we have multiple serial structs in a single slice of bytes.

One efficient solution is to have a function fn from_slice_split<T>(bytes: &[u8]) -> Result<(T, &[u8]), _>

It will take the first n bytes required to parse the provided type, split the slice and return a tuple with the parsed type and the remainder bytes.

Considering the required traits are implemented for T and we have a tuple (T, T) = (a, b), the following test case should pass:

const B: u8 = 0xfa;

let mut bytes = [B; 2 * T::SIZE + 1];
bytes[..T::SIZE].copy_from_slice(&a.to_bytes());
bytes[T::SIZE..2 * T::SIZE].copy_from_slice(&b.to_bytes());

let (a_p, bytes) = T::from_slice_split(&bytes).unwrap();
let (b_p, bytes) = T::from_slice_split(&bytes).unwrap();

assert_eq!(a, a_p);
assert_eq!(b, b_p);
assert_eq!(bytes, [B]);

[ZER0]

When I was checking the serialisation module of plonk I was thinking something similar, but I'm still unsure it's a good API. First, it can be easily obtain using bytes[T::SIZE..] and seconds it looks like we're overlapping a bit with other functionalities. Maybe we could create a from_slice_mut or similar where we consume the bytes given as parameter? Is there any similar API in Rust core libraries we could take inspiration from?

[vlopes11]

One of the most used bytes crate uses this pattern as well

https://crates.io/crates/nom

[ZER0]

When considering API I usually prefer to the idioms used in the Rust core languages as first, so if you can think of anything similar there that we can take a look let me know; I will check the nom crates in the meantime.

[ZER0]

I pursued the idea of a from_slice_mut and I ended up with the following API, that I think it works quite nice, I have already a PR in draft.

Instead of having something like:

    // This is our common scenario, where we use `from_bytes` 
    // from inside another `from_bytes`
    fn from_bytes(bytes: &[u8; Self::SIZE]) -> Result<Self, Self::Error> {
        let a = BlsScalar::from_slice(&bytes[..32])?;
        let b = BlsScalar::from_slice(&bytes[32..64])?;
        let c = BlsScalar::from_slice(&bytes[64..])?;

        Ok(Self { a, b, c })
    }

We can have:

    fn from_bytes(bytes: &[u8; Self::SIZE]) -> Result<Self, Self::Error> {
        let mut bytes = &bytes[..];
        let a = BlsScalar::from_slice_mut(&mut bytes)?;
        let b = BlsScalar::from_slice_mut(&mut bytes)?;
        let c = BlsScalar::from_slice_mut(&mut bytes)?;

        Ok(Self { a, b, c })
    }

It also take in account if the buffer is not enough or if the bytes are not valid to be used to build the type. In the first case the bytes ARE NOT consumed (since there aren't enough bytes) and the error is returned; in the second case the bytes ARE consumed (since there are enough) but once we try to use them to build the type we got the error back.

The bonus point of introducing a method like this one, is that we do not make assumption of the SIZE of the type we're reading. In fact, instead of using 32, 64, etc we should have used BlsScalar::SIZE making the whole code unreadable (you can imaging with the PoseidonCipher::SIZE as well, with the need to carry the previous offset).

[CPerezz]

I would go go that! It looks really nice!

[ZER0]

As I mentioned in the rocket chat, the name is misleading (I used the one I initially proposed) since a) it does not return a mutable (the naming convention for the suffix _mut in rust indicates that) and b) it technically accept anything that is implement the Read trait. So it's more from_reader than from_slice_mut.

It's just happen that we only have &[u8] that implement this trait and most likely we'll always use just that. But formally this is how it should looks like:

    fn from_bytes(bytes: &[u8; Self::SIZE]) -> Result<Self, Self::Error> {
        let mut bytes = &bytes[..];
        let a = BlsScalar::from_reader(&mut bytes)?;
        let b = BlsScalar::from_reader(&mut bytes)?;
        let c = BlsScalar::from_reader(&mut bytes)?;

        Ok(Self { a, b, c })
    }

Currently it's added to DeserializableSlice and it might be more pertinent under another trait, but if we keep as is we basically get this new method for free: previous approaches will still work unchanged, and we can use from_reader for the new code, and progressively refactor the old at the first occasion.