sipa / bech32

Code snippets and analysis of the Bech32 format
190 stars 107 forks source link

Case handling inconsistencies in reference implementations #38

Open theuni opened 6 years ago

theuni commented 6 years ago

From an IRC discussion:

[cfields] sipa: the bip shows a test vector of 'A12UEL5L', which i presume to be a hrp of 'A' and empty data.... [cfields] It says "Encoders MUST always output an all lowercase Bech32 string". [cfields] Confusingly, the c ref returns a failure on an uppercase HRP, but c++ returns an invalid encoding instead. [cfields] I think the c++ ref needs to lc() the hrp input first, but the c ref leads me to believe that encoders should expect the hrp to already be lowercased. [sipa] cfields: i think someone has ponted out that it's not entirely consistent [sipa] either we alwaya assume the hrp is already lowercase [sipa] or lc it [cfields] well the bip specifies what decoders must not accept mixed case, maybe it would be worth amending to specify that either "encoders must not accept an uppercase hrp", or "encoders must first lowecase the hrp" ? [cfields] either way, as-is, the c++ ref returns an incorrect encoding for bech32::encode('A', {})

I'm happy to PR a fix, but it's unclear what needs to be fixed. The c/c++ encoders (I haven't looked at the others) should either fail to encode an uppercase hrp character, or quietly convert internally first.

dcousens commented 6 years ago

In https://github.com/bitcoinjs/bech32, we .toLowerCase() the HRP.