Add support for encoding/decoding bech32 addresses using iterators

[tcharding]

(Friendly note: clarkmoody unless you are just loving all this iterator stuff I suggest you don't bother reviewing this one (or #113) until apoelstra acks :)

This is a slightly different approach to #113. Done on top of #116, so just the last patch.

• Conversion to/from bytes/fes by way of the extension traits as in Add iter and hrpstring #113 (still in iter.rs)
• Checksum iter adapter as in Add iter and hrpstring #113
• Decoding by way of three structs depending on usage.
• Encoding by way of a new struct Encoder, uses a builder-like pattern to construct the encoder and then one can call chars() to get a character iterator or fes() to get a field-element iterator (over all the fes that go into the checksum).

This should mean:

• Cleaner API i.e., easier to use but also easier to not use incorrectly
• Incorrect use of iterator adaptors should now be harder

IMNSHO the bech32 spec is convoluted as f**k, hiding all the details is not totally possible but this new API attempts to make it possible to correctly use the API without having to spend two weeks reading and re-reading bips 173 and 350.

PR currently does not use the new API

For example usage see:

• the unit tests in decode and encode
• the module level rustdocs in encode.rs and decode.rs
• BIP-173 and BIP-350 test vectors in tests/

[apoelstra]

Done reviewing 5c9b549. This is definitely shaping up.

[tcharding]

Thanks for the review @apoelstra, I read your comments but didn't get to work on this today. I'm good to implement all suggestions except unsure about the thing I commented on.

[tcharding]

I pushed up what I've done. Currently there is no invariant on the checksum being in data (decode::Hrpstring). Perhaps we should pull out a Parsed type again from decode::Hrpstring that is the pre-parsed concept we had before. I had a play but didn't get anything I really liked.

[tcharding]

ooo, I've come up with something mad. Converting to draft while I polish it, will push shortly.

[tcharding]

Hey @apoelstra check this re-write of the decode module out. I'm not happy with the three type names but I'm super happy with the abstractions - let me know what you think. (Its in a separate patch, will squash once you've had a look.)

EDIT: I originally called them Adt0, Adt1, and Adt2, while writing docs I came up with the current names.

[apoelstra]

@tcharding looks great! I'm unsure whether KnownHrp is worth the reduction in flexibility, since we don't really use it for anything.

Agreed that the names are no good. I suggest

• Parsed becomes UncheckedHrpString or maybe Unchecked
• Base32 becomes HrpString
• Segwit can stay as Segwit, or maybe it should be SegwitHrpString.

[tcharding]

Used UnvalidatedHrpString instead so as not to overload "check" and SegwitHrpString, I think we should err on the side of explicitness with this module.

[tcharding]

Removed KnownHrp and inlined the check.

[tcharding]

Just the padding thing left to do.

[tcharding]

Added non-zero padding checks.

I changed two of the validate functions that convert from one type to another HrpString to SegwitHrpString to consume self, can you think of any reason why this might be too restrictive?

I found some bugs in the len methods I wrote, now fixed.

[tcharding]

Changes in force push:

• Added a bunch more rustdocs incl. examples
• Removed the with_checksum function from BytesIterExt (I added it recently and it is no good, found this out while adding doc tests).

[apoelstra]

ACK 3ca7a19

[tcharding]

Changes in force push (all to decode:

• Fixed stale local variable names (after all the type naming churn)
• Fixed an error variant (also stale)
• Fixed incorrect docs on error types (cut'n'pasta fails)

[tcharding]

@clarkmoody this one is ready for review if you have the cycles to spare please. Take your time.

[tcharding]

While depending on this branch in rust-bitcoin I noticed one error variant was misnamed, renamed SegwitError -> SegwitHrpstringError

[tcharding]

Ah sorry @clarkmoody, might want to wait some more ...

@apoelstra this HRP ("bc", "tb", "bcrt") thing is pernicious. I attempted to use this branch today in rust-bitcoin and its basically unusable because of the HRP checks, one cannot get any segwit checks done for a regtest address. Pushed a quick Friday afternoon fix to get the discussion started.

See rust-bitcoin/rust-bitcoin#1951 for usage of this branch, oh and BTW its pretty nice ;)

[apoelstra]

Lol that Bitcoin Core doesn't even respect the "rule" that the HRP has to be bc or tb. It's a stupid rule and ACK dropping it.

[apoelstra]

ACK e3d62d4 except that the last commit should be cleaned up (at least, fix the CI error and remove 'WIP' from the commit message).

[tcharding]

Cleaned up and squashed! Note also the additional functions on Hrp commented on above.

[apoelstra]

ACK de7da50

[tcharding]

Ok @clarkmoody, now its ready for you please if you have time.

[tcharding]

If/when this merges should we consider releasing a new version at this stage? The benefit is that the primitives module is in place so if we release we can use it in rust-bitcoin and the play around with it and improve it as needed. Also I do not think that we can get much more done in time for the next release of rust-bitcoin and it would be super nice to be using this stuff already so we can start work on other address improvements in rust-bitcoin. Is there any downsides of doing so that I'm overlooking?

[apoelstra]

@tcharding we should look at the diff and see how many APIs we've deleted already, and consider trying to make best-effort replacements of those as a stop-gap.

[apoelstra]

Actually, I think we should release this with a -alpha0 suffix or something rather than trying to emulate the old API. Then people can use it but nobody will feel forced into it.

[tcharding]

I think we should release this with a -alpha0 suffix

Good idea.

[tcharding]

Pushed code comment change to bip-173 test to mirror the bip-350 one.

[apoelstra]

ACK 5478e2b

[tcharding]

If you are interested @clarkmoody there is a PR in rust-bitcoin showcasing usage of this branch.

rust-bitcoin/rust-bitcoin#1951

[clarkmoody]

Massive changes! Thanks for the hard work. Agreed with the comment in rust-bitcoin about a shortcut method to collapse that large iterator chain.

[tcharding]

Thanks man, we worked hard on this one.