search

danlehmann / arbitrary-int

A modern and lightweight implementation of arbitrary integers for Rust
MIT License
32 stars 13 forks source link

feat: borsh and json schemas support #43

Closed dzmitry-lahoda closed 3 months ago

dzmitry-lahoda commented 4 months ago

ping @danlehmann.

danlehmann commented 3 months ago

Thanks for the contribution!

I'll work on reviewing this (have to learn about borsh first). Will try to get back asap

dzmitry-lahoda commented 3 months ago

So what I saw from impl. It is hard to get le bytes of number like u35. Really I can borsh it into 5 bytes. While doing it for 8 now. So support getting LE bytes minimally needed would be awesome, not underlying.

dzmitry-lahoda commented 3 months ago

@danlehmann about tests. I tested that arbitrary ints work same as native primitive ints, minus some well know diff. hence I tested layout, but in the way that I did not plug exact binary values. this approach allows to extend these tests to property tests and doing meta tests over all serdes/schemas option.

danlehmann commented 3 months ago

I'd like to have full binary tests for these. Right now, there are many incorrect implementations that would pass those tests.

However, we can merge and I'll work on the test in a follow-up. Please give me a day or two

danlehmann commented 3 months ago

This patch didn't actually build (see https://github.com/danlehmann/arbitrary-int/actions/runs/10065181794/job/27823714420)

Working on a fix now

danlehmann commented 3 months ago

Reworked things somewhat: https://github.com/danlehmann/arbitrary-int/pull/45

danlehmann commented 3 months ago

@dzmitry-lahoda Please take a look at https://github.com/danlehmann/arbitrary-int/pull/45 . That adds a lot of test coverage, removes the allocation in deserialize and simplifies serialize.

Review appreciated.