search

rust-bitcoin / rust-bech32

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

Checksum length #156

Open tcharding opened 10 months ago

tcharding commented 10 months ago

we should change the checksum length check so that we enforce it only on the data part, not the HRP or separator. (It appears that for segwit, the BIP says the 90 limit applies to the whole string.)

ref: https://github.com/rust-bitcoin/rust-bech32/pull/142#issuecomment-1781113334

tcharding commented 10 months ago

So for segwit I believe we have it correct (assuming #142 merges) - 90 characters for the whole string as per BIP-173.

For lib.rs we use 1023, I thought this was based on what can be error detected so should it not include everything that goes into the checksum algorithm? If so this actually means 1023 is not correct because each character of the HRP feeds in two field elements to the checksum algo (high then low bits), right?

apoelstra commented 10 months ago

If so this actually means 1023 is not correct because each character of the HRP feeds in two field elements to the checksum algo (high then low bits), right?

Yep, exactly.

My view is that the HRP should count for nothing, because if the HRP is wrong then you don't even know what checksum you're supposed to be using. But if we do count it, then we should be double-counting it.

tcharding commented 10 months ago

Yep, exactly.

Do I get my white belt yellow tip in cryptography now?