Introduce blech32m format for v1+ witness programs#1025
Introduce blech32m format for v1+ witness programs#1025apoelstra merged 10 commits intoElementsProject:masterfrom
Conversation
apoelstra
force-pushed
the
2021-08--blech32m
branch
from
6f9d616 to
6172914
Compare
|
I'm late to the party but I still think "blech" is a bad name because it means "(metal) sheet" in Germany and is associated with cheap metal and often has a negative connotation (e.g., sometimes people ironically call the worthless fourth place in a competition "Blech", i.e., you missed the gold/silver/bronze medals.) |
|
It was something of a joke, because it also has a negative connotation in English (it's basically a gross sound you might make, e.g. see this picture of Bill the Cat from Bloom County) |
src/blech32.cpp
Outdated
| // resulting checksum to be 1 instead. | ||
| return PolyMod(Cat(ExpandHRP(hrp), values)) == 1; | ||
| // list of values would result in a new valid list. For that reason, Bech32 requires the | ||
| // resulting checksum to be 1 instead. In Bech32m, this constant was amended. |
There was a problem hiding this comment.
Nit: Bech->Blech on this line and above (but it doesn't really matter.)
src/blech32.cpp
Outdated
| @@ -158,7 +182,7 @@ std::string Encode(const std::string& hrp, const data& values) { | |||
| } | |||
|
|
|||
| /** Decode a Blech32 string. */ | |||
There was a problem hiding this comment.
Nit: missed comment change ("Blech32" -> "Blech32 or Blech32m")
src/blech32.h
Outdated
| BLECH32M, //!< Blech32m tweaked a la bech32 | ||
| }; | ||
|
|
||
| /** Encode a Bech32 string. Returns the empty string in case of failure. */ |
There was a problem hiding this comment.
Nit: Bech->Blech.
(Upstream has a longer comment here, but this was already true before this change -- I don't know if it's untrue for us for some reason. Converted for our use it would be:
/** Encode a Blech32 or Blech32m string. If hrp contains uppercase characters, this will cause an
* assertion error. Encoding must be one of BLECH32 or BLECH32M. */
)
This commit addresses #20809. We add an additional 'error' property in the result of 'validateaddress' in case the address is not valid that gives a short description of why the address in invalid. We also change the error message returned by 'getaddressinfo' in case the address is invalid. bitcoin/bitcoin#20832 (1/1) ELEMENTS: Merge conflicts resolved based on d6c85c5 (from 22.0 rebase)
This also includes updates to the Python test framework implementation, test vectors, and release notes. bitcoin/bitcoin#20861 (3/5)
|
FYI, It would hard for me to catch any blech32-specific stuff around blinding keys in DecodeDestination, the bit-fiddling is extremely fiddly and doesn't parallel anything in bech32. So maybe double-check that part. |
|
Ok, I have checked all the bits that weren't backports, comments above. |
|
Checked the cherry-pick of bitcoin/bitcoin#20832 using my usual trick. (Redo it myself, with |
|
Checked the other backport the same way, looks good. I notice that we removed an anonymous namespace in I've now looked at everything, and it's all good other than my comments above. |
apoelstra
force-pushed
the
2021-08--blech32m
branch
from
dcc6cf3 to
f28acf8
Compare
Converted the bech32 valid test vectors. Did not convert the invalid test vectors because it was not obvious how.
apoelstra
force-pushed
the
2021-08--blech32m
branch
from
f28acf8 to
c72c949
Compare
|
Addressed Glenn's comments and changed the constant as per @roconnor-blockstream's recommendation. |
|
utACK c72c949. |
|
LGTM |
6 participants
Includes backports of bitcoin/bitcoin#20832 (1 commit) and bitcoin/bitcoin#20861 (5 commits)