khonsulabs / bonsaidb

A developer-friendly document database that grows with you, written in Rust
https://bonsaidb.io/
Apache License 2.0
998 stars 37 forks source link

Remove reliance on `hpke`'s serde implementation #311

Closed rozbb closed 7 months ago

rozbb commented 8 months ago

We are removing the serde_impls feature from hpke (https://github.com/rozbb/rust-hpke/pull/53). This pull request cleans up some HPKE code, and moves it off of the HPKE serde impls. The serde code here is identical to hpke's.

The only thing I do not check here is that this change does not break existing databases. Since the code is identical, it would be very surprising if it did. But nonetheless, this should be checked. I would appreciate if someone could help verify this.

Let me know if you have any questions. Sorry for the hassle about removing the feature. Turns out there's lots of things people use serde for, and implementing serialization for everyone all at once is probably not the right choice for us.

ecton commented 8 months ago

No need to apologize for making a breaking change! If you look at my change log, there's a lot in this project's history :laughing:.

I'll make sure to test backwards compatibility before releasing an update.

Thank you so much for the PR!

rozbb commented 8 months ago

Great, thanks for the quick response! The 0.12 release hasn't dropped yet and it might have one more breaking change. I will update this PR once 0.12 drops, or if it's merged already, I'll just make a new one that upgrades HPKE and deals with the new stuff.

ecton commented 8 months ago

I'm not primarily focused on this project at the moment, so I'm happy to wait on the 0.12 release to get this merged. There is actually a backwards compatibility test that broke due to this. I'm happy to debug this and help carry it across the finish line, but if you want to try to investigate it you can try running cargo test -p bonsaidb-local --features encryption -- self_compatibility.

Thank you again, and because I didn't say it before, thank you for the great crate!

ecton commented 7 months ago

I ended up touching this project today to fix another bug I found while using it, so I've went ahead and merged this. Thank you again, and don't worry about a PR for the breaking changes in 0.12 unless you really want to send one in, I'm happy to take care of the updates.

rozbb commented 7 months ago

Awesome! I still haven't ironed out the details for 0.12 but I'll happily send a PR when it's ready. Thanks for the eyes and the kind words!