brainwallet / brainwallet.github.io

Brainwallet site
https://brainwallet.org
291 stars 409 forks source link

Converter does not work for arbitrary base58 #36

Closed janderholm closed 10 years ago

janderholm commented 10 years ago

Steps to reproduce:

I'm not at all sure why this is but if there's some kind of restriction on the kind of base58 that is allowed maybe this should be made more clear.

janderholm commented 10 years ago

I've looked into this a bit more and found out that Brainwallet does not handle different versions of base58check adresses. It also does strange things when converting to poetry.

It seems as though when using RFC1751 or poetry and possibly other options, the checksum and version byte is stripped before conversion! This is really bad for several reasons. In my opinion conversions should include both checksum and version byte. Version byte to make everything forward and backwards compatible and checksum to prevent mistypings and tampered keys. It is very easy to accidentally double type a word or for someone to change one word. The checksum is there to protect against such things.

These things will likely be a real problem in the future when several versions of base58check encoded addresses are used. There should be options for keeping version and checksum intact while converting.

brainwallet commented 10 years ago

fixed in d6d895cc880a2665b93ab9e0e9e0e70b219fd9c0