search

rust-bitcoin / rust-bech32

Bech32 format encoding and decoding
MIT License
94 stars 49 forks source link

Define u5 type to distinguish between 5bit and 8bit slices #16

Closed sgeisler closed 6 years ago

sgeisler commented 6 years ago

In rust-lightning-invoice I pass around slices of the data part as &[u8], but each byte is guaranteed to be in the range 0..32. I'd like to encode this guarantee in the type by defining pub struct u5(u8).

The traits FromBase32 and IntoBase32 could be introduced and implemented for &[u8] and &[u5] respectively and replace convert_bits (From<&[u8]> for &[u5] and From<&[u5]> for &[u8] can't be implemented since type and trait are foreign and it would be quite ambiguous). Furthermore FromBase32 could be implemented for data structures that get parsed from bech32 data (like lightning invoice fields).

The best case UX would be that you can now implement FromBase32 for your types and don't have to worry about out-of-range errors anymore. The worst case UX would probably be that every use of an u5 (formerly u8) will now require an .into::<u8>().

What do you think about this proposal @clarkmoody and @apoelstra (I know I brought this up already in #5 but it got lost when we released the new version)?

apoelstra commented 6 years ago

This sounds like a good idea to me. I generally don't mind explicit conversions.

clarkmoody commented 6 years ago

I agree. I'd like to see the implementation details and hope we can avoid that worst-case UX.

sgeisler commented 6 years ago

I was just thinking about the semantics of the FromBase32 and IntoBase32 traits. As far as I know padding is always applied when converting from base256 to base32 and removed when converting from base32 to base256. Do you know of any exceptions?

If not, then only FromBase32 has to return a Result because of possibly set padding bits and I don't have to think about encoding the padding strategy as a trait parameter.