Add encode_to_fmt method for efficient encoding

[stevenroose]

Export a public method that directly writes a Vec<u5> to an fmt::Formatter.

[sgeisler]

LGTM.

[stevenroose]

@sgeisler would you like an additional method directly producing a string based on this? I don't need it, but it makes sense because using fmt::Formatter is kinda Display-only.

 pub fn encode<T: AsRef<[u5]>>(hrp: &str, data: T) -> String;

[sgeisler]

That last comment gave me the impression that you intend this function to be used by "normal" users. I don't know if I'd like that in it's current form (for some odd reason I considered it an internal optimization to be used only by us before).

If what you want to do is avoid allocating/cloning (which is currently necessary to construct a Bech32 struct) it might be better to create a BorrowedBech32<'a> struct or use a Cow<'a, T> in Bech32. The problem with (&str, &[u5]) as basis for bech32 encoding as proposed here is that it doesn't enforce the invariants that are needed to guarantee that standard compliant bech32 is produced when encoding (e.g. no mixed case).

[stevenroose]

So you are saying we should make sure that the hrp part is also written in the same case as the data? That makes sense.

[sgeisler]

Yes, it's part of the specification that a bech32 string has only one case. And if encoding only lowercase is allowed and it's left to the application to transform it to uppercase if QR encoding is intended.

And I think these invariants should be part of a struct's "contract". That's why a (&str, &[u8]) tuple is not sufficient in my opinion.

The IRC discussion lead to the insight that we can just put the encoding validation logic into the encoding function and get rid of the Bech32 struct and the related ownership problems making encoding less efficient than possible. The only drawback is that decoding and subsequent encoding of the same data would require unnecessary checks. Since the Bech32 struct was in the library since before I started contributing I'd like to ask @clarkmoody if it had a certain purpose beyond enforcing invariants?

[stevenroose]

Yes, it's part of the specification that a bech32 string has only one case. And if encoding only lowercase is allowed and it's left to the application to transform it to uppercase if QR encoding is intended.

The check that makes sure the HRP is valid is quite negligible, no? Most prefixes will be < 10 characters long and the check is just a byte comparison IIRC. The only question is then whether to panic or Err on an incorrect one. encode_to_fmt is kinda made to return an fmt::Result. A Result<fmt::Result, bech32::Error> would be a bit awkward, but I guess then we could unwrap it in the `Display impl where we use a static HRP.

[stevenroose]

Superseded by #34. Let's move discussion there.

[dongcarl]

@stevenroose Okay to close?

[sgeisler]

I think #35 included #34 which came out of this discussion, and got merged, so this can be closed too.