Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add 7 languages from the bip39 spec #10

Merged

Conversation

cornfeedhobo
Copy link
Contributor

@cornfeedhobo cornfeedhobo commented Jun 23, 2018

Adds Japanese, Korean, Spanish, Chinese (simplified and traditional), and Italian, with included crc32 checksum protections. A small reorganization to account for more languages.

French was not added because the official wordlist has an artifact as the first byte, making "official" crc32 checksums invalid. I am following up with bitcoin devs on this issue.

Fixes #9

… and Italian, with included crc32 checksum protections. A small reorganization to account for more languages.
@cornfeedhobo
Copy link
Contributor Author

@tyler-smith ping (and thanks)

@tyler-smith
Copy link
Owner

@cornfeedhobo Thanks for the submission. It looks good, but I'm going to play with it some and think about potential issues or changes and then merge it if everything still looks good.

I'm thinking we might want to tweak the API eventually to make language selection easier but we can do that as a second step later on. I'll think about how it should work while experimenting with this PR.

@cornfeedhobo
Copy link
Contributor Author

@tyler-smith Sounds good. I already have some thoughts on this, but didn't want to shake things up too much with this PR. I'll try to draft up a branch for you to look at and see what I'm thinking.

@tyler-smith
Copy link
Owner

All manual and automated tests/test vectors are passing for me. Thanks again for the submission @cornfeedhobo.

@tyler-smith tyler-smith merged commit 320452e into tyler-smith:master Jul 11, 2018
@cornfeedhobo cornfeedhobo deleted the add-7-languages-from-bip39-spec branch September 9, 2018 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants