Issue on encoding bech32 when version > 31

[fix]

here: https://github.com/bitcoinjs/bitcoinjs-lib/blob/f48abd322f14f6eec8bfc19e7838a1a150eefb56/src/address.js#L39-L40

if version > 31 the bech32 encoder is throwing because the word is encoding over 5 bits. I have not checked in details the bech32 but since version is coded originally on a byte, woud not it be wise to unshift the version to the data before calling bech32.toWords(data)?

[junderw]

Nice catch.

1. This should be a fix to the BIP 173 (You should make a pull request)
2. Many wallets already have this behavior (your fix to this library would break and create different addresses even for version 0 (current segwit)) so it's too late to fix every implementation, must fix the BIP.

I think the cause of the problem is that version is assumed to be only 0-16 and therefore the reference implementation was probably made forgetting which data should be 5-bit words and which data should be 8-bit words.

[junderw]

https://github.com/bitcoin/bips/blob/0042dec548f8c819df7ea48fdeec78af21974384/bip-0173.mediawiki#Segwit_address_format

Here's a suggestion:

diff --git a/bip-0173.mediawiki b/bip-0173.mediawiki
index c3ee060..ea1d3ff 100644
--- a/bip-0173.mediawiki
+++ b/bip-0173.mediawiki
@@ -208,10 +208,11 @@ be of the same length as the mainnet counterpart (to simplify
 implementations' assumptions about lengths), but still be visually
 distinct.</ref> for testnet.
 * The data-part values:
-** 1 byte: the witness version
+** A single 5-bit word: the witness version (between 0 and 16 inclusive)
 ** A conversion of the 2-to-40-byte witness program (as defined by [https://github.com/bitcoin/bips/blob/master/bip-0141.mediawiki BIP141]) to base32:
 *** Start with the bits of the witness program, most significant bit per byte first.
 *** Re-arrange those bits into groups of 5, and pad with zeroes at the end if needed.
+*** Prepend the version (5-bit word) at the beginning of the array of 5 bit groups.
 *** Translate those bits to characters using the table above.

 '''Decoding'''

[junderw]

@sipa might be interested in this.

The reference implementation that was released with this BIP had the behavior described in my suggestion.

[junderw]

Going to close this since it's not an issue with us.