Storage interface upgrades

[nplasterer]

Migrate from json to tls serialization

More performance?

[franziskuskiefer]

The easiest way to do this switch would be to implement serde for the tls_codec. I'd be happy to take a PR for this upstream in https://github.com/RustCrypto/formats/tree/master/tls_codec 🙂

[tuddman]

The easiest way to do this switch would be to implement serde for the tls_codec. I'd be happy to take a PR for this upstream in https://github.com/RustCrypto/formats/tree/master/tls_codec 🙂

👋 @franziskuskiefer Can you kindly share some more context and frankly concrete guidance on what specifically would need to be done to achieve what's being asked for here? We had discussed possibly replacing this function and/or the read and read_list fns with ones that do not rely on serde_json and write impl Serialize for X {...} as we need to, as the most expedient approach to getting something working. I fully recognize this is in contrast to your suggestion quoted above, but I am wondering if you can share your thoughts on the matter?

I have begun writing additional changes in this open PR: https://github.com/xmtp/libxmtp/pull/831

[franziskuskiefer]

Doing a serde backend for tls_codec would allow a drop in replacement. But this is of course a little more work than just updating some functions in here.

First, we have only the serde bounds on the types. So by default we can not do much else than using serde.

There is the option to add tls_codec as another bound on the types. But I'm not sure if we want to do that upstream and if we even have this implemented for all types. With that in mind, you can't just replace a function like the build_key, because we only have K: Serialize.

Right now it looks like the only function that you can change is the one that takes a label, because that's a byte slice.

I tried a couple of things but I really only see two options

1. Add the tls_codec as a trait bound
2. Implement serde for tls_codec

I looked at 2. Taking the example from the docs, I started adding some of the functions. This generally seems doable. Finishing the crate here, would allow a drop in replacement.

For option 1 we need to add some additional implementations like bool (which is not part of the TLS syntax, but could be encoded as a u8 for example). In OpenMLS itself there are also a lot of structs that don't implement the TLS serialization directly. They would have to be added.

[neekolas]

There is another option, rather than keeping the current behaviour of read and read_list. Just make them return byte arrays and leave it to the methods that take concrete types to serialize/deserialize. We already do this for write operations.

[franziskuskiefer]

Indeed, you could also implement some functions only for specific types, which allows you then to interact with the actual types rather than an unknown type with trait bounds.