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

Write own BIP39 implementation #561

Closed
Tracked by #37
jblachly opened this issue Mar 9, 2022 · 19 comments
Closed
Tracked by #37

Write own BIP39 implementation #561

jblachly opened this issue Mar 9, 2022 · 19 comments
Labels
good first issue Good for newcomers

Comments

@jblachly
Copy link

jblachly commented Mar 9, 2022

As suggested in the code base:

bdk/src/keys/bip39.rs

Lines 14 to 17 in adf7d0c

// TODO: maybe write our own implementation of bip39? Seems stupid to have an extra dependency for
// something that should be fairly simple to re-implement.
use bitcoin::util::bip32;

I also recommend writing own BIP39 implementation.

In issue #399, BDK transitioned from tiny-bip39 to rust-bip39, however, the latter package (which BDK now depends on) appears to be unmaintained or poorly maintained, with PRs and issues ignored for about a year.

https://github.com/rust-bitcoin/rust-bip39

At a minimum, BIP39 wordlists, and words -> entropy -> seed generation would be valuable.

@jblachly jblachly added the summer-of-bitcoin Summer of Bitcoin Project Proposal label Mar 9, 2022
@notmandatory
Copy link
Member

@jblachly I think it's too late to add this as a 2022 summer of bitcoin project, but I'll tag it as a good first issue for someone new.

@notmandatory notmandatory added good first issue Good for newcomers and removed summer-of-bitcoin Summer of Bitcoin Project Proposal labels Mar 9, 2022
@jblachly
Copy link
Author

jblachly commented Mar 9, 2022

Thanks; accidentally started with that template (there was no other feature request template) and didn't realize it would add an issue tag.

@notmandatory
Copy link
Member

No problem, it's on my todo list to add an enhancement issue template. 😅

@violet360
Copy link

violet360 commented Mar 14, 2022

Hey, guys !!
I am a bitcoin enthusiast and an SoB participant who recently started with RUST and new to this project as well, I was looking for cool rust issues to work on so that I can improve my understanding and this one really caught my attention, I would love to work on this project.

@Eunoia1729
Copy link
Contributor

Eunoia1729 commented Mar 15, 2022

Hi @violet360!
I've been working on this since past few days. Hope we can review/discuss if I'm going in the right direction.
I can make a draft PR.

@violet360
Copy link

Ok cool, for sure :)

@evanlinjin
Copy link
Member

@violet360 @Eunoia1729 How is progress on this issue so far?

@evanlinjin evanlinjin mentioned this issue Apr 7, 2022
1 task
afilini added a commit that referenced this issue Apr 25, 2022
c752ccb Expose bip39::Error (志宇)

Pull request description:

  <!-- You can erase any parts of this template not applicable to your Pull Request. -->

  ### Description

  Expose `bip39::Error` (fixes #581 )

  ### Notes to the reviewers

  I am aware that the `bip39` module plans to be rewritten (as per #561 ), however this seems like a rather straightforward and quick change that may be useful in the short/mid term.

  ### Checklists

  #### Bugfixes:

  * [x] Expose `bip39::Error`

ACKs for top commit:
  afilini:
    ACK c752ccb

Tree-SHA512: 98b7ac1ba88aed07d9160830ee80496c32d531c15ada0e9b50a97f0883fbfced22fa83a7c7f8366aadb7e7a667d8a63dde869d31cc375206d277e55b2ec3089d
@danielabrozzoni
Copy link
Member

@Eunoia1729 are you stll working on this? :)

@Eunoia1729
Copy link
Contributor

Sorry for the late response.
I didnt progress much after that. Anyone interested can start with it, I'll be happy to join or help in any way.

@vladimirfomene
Copy link
Contributor

Hello @danielabrozzoni, has this been worked on? Can I pick it up?

@danielabrozzoni
Copy link
Member

Nobody that I know of, you can pick it up :)

@atalw
Copy link

atalw commented May 12, 2022

Hey, sorry that I didn't update here but I started working on this earlier today. Should I continue? Perhaps we could work together?

@vladimirfomene
Copy link
Contributor

vladimirfomene commented May 12, 2022

@atalw, yes we can work on it together. Can I reach out to you over discord? What is your discord handle?

@atalw
Copy link

atalw commented May 13, 2022

Awesome. It's atalw#3934.

@atalw atalw mentioned this issue May 16, 2022
9 tasks
@evanlinjin evanlinjin mentioned this issue Jun 29, 2022
6 tasks
@notmandatory notmandatory moved this to Todo in BDK Maintenance Aug 6, 2022
@notmandatory notmandatory moved this from Todo to In Progress in BDK Maintenance Aug 6, 2022
@notmandatory
Copy link
Member

I know we have @evanlinjin 's PR #644 in progress for this. But want to ask the question, since we already moved off of tiny-bip39 and we're using rust-bitcoin/rust-bip39 , does it make sense to create our own implementation? what features are we missing in rust-bip39 ? And if it's just a maintenance issue what needs to be updated and can we do PRs for those?

@jblachly
Copy link
Author

And if it's just a maintenance issue what needs to be updated and can we do PRs for those?

The problem is that the maintainer of rust-bitcoin/rust-bip39 is unresponsive to PRs

@notmandatory
Copy link
Member

If there are no technical reason against using rust-bitcoin/rust-bip39 then should we maintain and use a fork in the bitcoindevkit org? Then we can get PRs we need merged and upstream can still pull them in if/when they want.

@jblachly
Copy link
Author

jblachly commented Jan 1, 2023

should we maintain and use a fork in the bitcoindevkit org?

Sounds reasonable; alternatively if anyone has personal connections to org owners of rust-bitcoin we could lobby for an additional person to get admin permissions to the repository as co-maintainer

@tnull tnull mentioned this issue Feb 28, 2023
3 tasks
@notmandatory
Copy link
Member

@jblachly I'm going to close this issue as the rust-bitcoin/rust-bip39 crate seems to be back on track with new releases. Happy to reopen if you think more discussion is needed.

@github-project-automation github-project-automation bot moved this from In Progress to Done in BDK Maintenance Mar 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

8 participants