feat: matching merkle db encoding

[xinifinity]

No description provided.

[rkuris]

pub => pub(crate)

Please consider making these methods on Data:

impl Data {
    fn encode_int(val: i64) -> Result<Self, CodecError> {
        // TODO: construct Data{}
    }
}

I'm not sure how these get used, but if you're just appending to this trailing vec every time, then these methods should probably be called append_int or something, and can take &mut self instead.

[rkuris]

https://docs.rs/varint/latest/varint/trait.VarintWrite.html can be used for varint protobuf like encoding. This does mean unpacking nodes will be less efficient. I wonder if this should be a feature flag we can enable or disable -- it would be so much faster to avoid all this encoding

[richardpringle]

Yikes... whirlwind of a review. Let me know if you need clarification anywhere (I'm sure you will)

[richardpringle]

Docs? Seems weird to serde(skip) here...

[richardpringle]

🤔 , have you looked at the serde::Serializer implementation for strings? https://docs.rs/bincode/latest/src/bincode/ser/mod.rs.html#121-124

It looks like the length is already encoded...

[richardpringle]

I see the difference now, my bad.

I still think we should use marker traits here (see my comment on this file).

use serde::Serializer;
use std::io::Write;

trait Encode: Serialize {
    fn encode(&self) -> Result<bincode::Serializer::Ok, bincode::Serializer::Error> {
        DefaultOptions::new().serialize(self)
    }
}

impl Encode for str {
    fn encode(&self) -> Result<S::Ok, S::Error> {
        let mut data = Vec::with_capacity(s.len() + 1);
        DefaultOptions::new().serialize_into(&mut data, s.len() as i64)?;

        data.write(self.as_bytes());

        Ok(data)
    }
}

If we just start with str, it's not really beneficial to have Encode: Serialize, but that would come in super handy for all the other primitives.

[richardpringle]

BUT... They might potentially make some changes in MerkleDB so we could potentially just use the default implementation.

[richardpringle]

After reviewing the whole file, I think everything can be greatly simplified:

use bincode::{Options, Error}; 
use serde::{Serialize, Deserialize};

pub(crate) fn encode<T: Serialize>(t: &T) -> Result<Vec<u8>, Error> {
    DefaultOptions::new().serialize(t)
}

pub(crate) fn decode<'de, T: Deserialize<'de>>(bytes: &'de [u8]) -> Result<T, Error> {
    DefaultOptions::new().deserialize(bytes)
}

If we want to add further constraints on types that are encodable/decodable, we can have the following traits:

trait Encode: Serialize {}
trait Decode<'de>: Deserialize<'de> {}

// can make this easier with a macro
impl Encode for String {}
impl Decode<'_> for String {}

impl Encode for i64 {}
impl Decode<'_> for i64 {}

And bound the encode and decode functions by these traits instead.

[xinifinity]

Closed as it may no longer applicable ATM, will reopen if needed.