rust-bitcoin / rust-bech32

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

I think `Checksum::Engine` should be able to input `u8`s. #197

Closed apoelstra closed 1 month ago

apoelstra commented 1 month ago

This would complicate the implementation a little bit but I think it'd make implementation of the rust-lightning invoice encoding much easier. Basically we would have two new methods:

Right now you can simulate "input a series of bytes encoded as a u5-string, followed by another such series of bytes" by doing bytes1iter.bytes_to_fes().chain(bytes2iter.bytes_to_fes()), but this is a pain to write, results in a type with a huge name, and kinda disguises the weird nature of this encoding.

tcharding commented 1 month ago

Are you planning on getting to this yourself at some stage @apoelstra or hoping someone else will pick it up? I can probably do it but it will take me a fair bit of brain power, not sure how high a priority it is?

apoelstra commented 1 month ago

I'll take it on.

tcharding commented 1 month ago

Legend, thanks.

apoelstra commented 1 month ago

I've implemented this but I'm no longer convinced that it's useful.

When encoding, you almost always want the ability to stream Fes into the checksum engine and access the Fes themselves (so that you can encode them as characters). Having the checksum engine directly take u8s doesn't help with that (and having the checksum engine somehow output the Fes that it's internally constructing would lead to an ugly and confusing API).

When decoding, you have a stream of Fes anyway and don't have any need to put u8s directly into a checksum engine.

Ulimately, I think our existing API, where you build iterators of Fes however you want, then call the .checksummed adaptor which both yields the Fes and inputs them into a checksum engine, is the correct API. While this can lead to large types which you sometimes need to name explicitly, this is ultimately a bug in Rust and not in our API. My hope was that I could construct an "internal iterator" API where you just fed data into a checksumming object and it would somehow yield a bech32 string, but I just don't think this makes sense.

You can sorta simulate it with a pair of Checksum::Engine and a fmt::Display, feeding Fes into both as you produce them. But again, the existing API provides the tools you need to make this work: you take any u8s you have, iterate them with the .bytes_to_fes adaptor, and if you need to intersperse them with other Fes you can go ahead and do that.