Skip to content

Add BIP39 Elixir implementation (mnemo)#883

Closed
aerosol wants to merge 1 commit intobitcoin:masterfrom
aerosol:add-bip39-implementation
Closed

Add BIP39 Elixir implementation (mnemo)#883
aerosol wants to merge 1 commit intobitcoin:masterfrom
aerosol:add-bip39-implementation

Conversation

@aerosol
Copy link
Copy Markdown
Contributor

@aerosol aerosol commented Jan 26, 2020

This PR adds another Elixir implementation of BIP39.
Thanks

@luke-jr
Copy link
Copy Markdown
Member

luke-jr commented Feb 19, 2020

@luke-jr luke-jr added the Proposed BIP modification PR by non-owner to update BIP content label Feb 19, 2020
@prusnak
Copy link
Copy Markdown
Contributor

prusnak commented Feb 20, 2020

The BIP is not supposed to be the list of all alternatives, but rather provide one really good implementation.

@luke-jr luke-jr closed this Feb 20, 2020
@aerosol
Copy link
Copy Markdown
Contributor Author

aerosol commented Feb 20, 2020

@prusnak fair, yet unfair - JavaScript and Swift sections link to two implementations on that page.

@izelnakri as the author of the currently linked implementation, would you mind looking at https://github.com/aerosol/mnemo and sharing your thoughts wrt code/tests quality? Cheers!

@prusnak
Copy link
Copy Markdown
Contributor

prusnak commented Feb 20, 2020

@aerosol send a PR that removes the worse of Javascript/Swift implementations

The same applies to Elixir. If you think your implementation is better than the other one, just send a PR that replaces the old one.

@izelnakri
Copy link
Copy Markdown
Contributor

@aerosol thanks for sharing your work. Could you create a PR with your improvements on https://github.com/izelnakri/mnemonic ? I can add you as a collaborator after we merge your improvements.

@aerosol
Copy link
Copy Markdown
Contributor Author

aerosol commented Feb 21, 2020

@izelnakri Thanks for looking. Sorry, it's not really feasible to do it chunks and I doubt you meant "submit a PR that replaces the whole repo"

@Fatima-yo
Copy link
Copy Markdown

Fatima-yo commented Feb 21, 2020 via email

@iancoleman
Copy link
Copy Markdown
Contributor

@Fatima-yo

Have a look at the instructions at

https://github.com/bitcoin/bips/blob/master/bip-0039/bip-0039-wordlists.md

and also #863 for further info / context on new wordlists.

@Fatima-yo
Copy link
Copy Markdown

Fatima-yo commented Feb 24, 2020 via email

@Fatima-yo
Copy link
Copy Markdown

Fatima-yo commented Feb 24, 2020 via email

@izelnakri
Copy link
Copy Markdown
Contributor

izelnakri commented Feb 24, 2020

@izelnakri Thanks for looking. Sorry, it's not really feasible to do it chunks and I doubt you meant "submit a PR that replaces the whole repo"

@aerosol Im not convinced why it cant be done in chunks and requires a whole rewrite to my own repo. Just briefly checked your repo it isnt that big in terms of lines of code or complex to require a complete rewrite.

@aerosol
Copy link
Copy Markdown
Contributor Author

aerosol commented Feb 25, 2020

@izelnakri Thank you for your assessment. The primary motivation behind writing mnemo was to get a better understanding of BIP39. I found the pre-existing code difficult to reason about (one could say complex), hence I decided to start from scratch. If I had started by forking your repo, I'd happily submit a PR. Otherwise all I can see is two fundamentally different implementations - if you're not convinced, have a look at the tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Proposed BIP modification PR by non-owner to update BIP content

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants