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

Switch to a different BIP39 implementation #399

Closed
notmandatory opened this issue Jul 19, 2021 · 8 comments · Fixed by #400 or #462
Closed

Switch to a different BIP39 implementation #399

notmandatory opened this issue Jul 19, 2021 · 8 comments · Fixed by #400 or #462
Labels
module-keys new feature New feature or request

Comments

@notmandatory
Copy link
Member

The tiny-bip39 dependency used in the all-keys feature depends on zeroize which appears to only support MSRV 1.51+

https://github.com/iqlusioninc/crates/

This is causing the following error in our CI workflow:

Downloaded zeroize v1.4.0
error: failed to parse manifest at `/home/runner/.cargo/registry/src/github.jparrowsec.cn-1ecc6299db9ec823/zeroize-1.4.0/Cargo.toml`

Caused by:
  feature `resolver` is required

  this Cargo does not support nightly features, but if you
  switch to nightly channel you can add
  `cargo-features = ["resolver"]` to enable this feature
Error: Process completed with exit code 101.

https://github.com/bitcoindevkit/bdk/runs/3101954688?check_suite_focus=true

@notmandatory notmandatory added the bug Something isn't working label Jul 19, 2021
@notmandatory
Copy link
Member Author

Can we switch to https://github.com/rust-bitcoin/rust-bip39? it's MSRV is 1.29.

@afilini
Copy link
Member

afilini commented Jul 20, 2021

Can we switch to https://github.com/rust-bitcoin/rust-bip39? it's MSRV is 1.29.

Yeah I guess it makes sense, I'm gonna reopen this issue to keep track of that

@afilini afilini reopened this Jul 20, 2021
@afilini afilini changed the title Build with +1.46.0 toolchain fails for all-keys features Switch to a different BIP39 implementation Jul 20, 2021
@afilini afilini added new feature New feature or request module-keys and removed bug Something isn't working labels Jul 20, 2021
@notmandatory
Copy link
Member Author

I did a preliminary look at swapping tiny-bip39 with rust-bip39 and the one missing part right now is rust-bip39 doesn't currently implement seed generation from the phrase.

@notmandatory
Copy link
Member Author

As commented in bip39.rs, another option is to write our own bip-39 implementation, ideally with no new dependencies.

@notmandatory
Copy link
Member Author

notmandatory commented Sep 21, 2021

I confirmed that using @LLFourn 's PR maciejhirsz/tiny-bip39#29 patched version of tiny-bip39 in bdk I'm able to remove the fix from #400 and build and test the all-keys feature with rust 1.46.0.
Oops, nevermind. still broken after removing Cargo.lock and doing a cargo clean.

@rajarshimaitra
Copy link
Contributor

rajarshimaitra commented Oct 25, 2021

I am willing to work on this.. Does it make sense to implement our own bip39? Should not be too difficult. If yes I can start sketching one.

@rajarshimaitra
Copy link
Contributor

I looked into rust-bip39 and it seems it should be possible to use that with slight refactoring of our keys.rs module. rust-bip39 does support seed generation from mnemonic phrase, https://github.com/rust-bitcoin/rust-bip39/blob/520e15a6e5b86940dae2d386d90b4b3616491d13/src/lib.rs#L477-L486.

I am working on the transition. Should not take long.

@notmandatory
Copy link
Member Author

Sounds great if we can use rust-bip39 that will be ideal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module-keys new feature New feature or request
Projects
None yet
3 participants