Allow 4-character abbreviations of mnemonic words ...#168
Allow 4-character abbreviations of mnemonic words ...#168yorickdowne wants to merge 0 commit intoethereum:masterfrom yorickdowne:master
Conversation
|
Please give this another look for inclusion. |
|
This is now failing one specific test mnemonic. Checksum calculation fails. Let me see why that is. |
|
I know why this fails, the logic to create a word language map is wrong. I'm glad this got caught before it made it in! Let me think on how to improve |
|
This now passes all tests. Logic has been changed. Previously I normalized and sliced to 4 characters while constructing Instead, I am now using a nested for list comprehension that compares the full word language map against abbreviations, then flatten the resulting 2D list again. If there's a more elegant way of doing this, please share! Because abbreviations can match several languages, and then merely fail the checksum, I am now continuing the search through the languages when a checksum fails, rather than returning |
|
Please give this another look for inclusion. Also, please give feedback on whether changing the two |
|
Please give this another look for inclusion. If there are concerns, please raise them. |
|
Ping to get this considered for inclusion, please. |
3 participants
... when using the
existing-mnemonicworkflow.This addresses #167
Choices made:
Because of the way the dict lookups are written, I decided to normalize to 'NFKC' and slice to 4 characters or less. The normalization is there so that combining Unicode characters work and the string doesn't get sliced to 3 visible characters.
I have no particular attachment to this way of handling it. If you'd rather not normalize and do a partial match on dict keys instead, I am game. If there's a third method you favor, ditto.
new-mnemonicworkflow has not been touched and still does a straight compare. One might discuss whether that workflow should allow abbreviations as well, but that might be a different PR.This code passes the test suite.