Skip to content

Conversation

@alvroble
Copy link

@alvroble alvroble commented Jul 9, 2025

Summary

This PR addresses the cryptographically incorrect and misleading SLIP39 to BIP39 recovery functionality identified in issue #90. The changes add clear warnings about the problematic nature of these methods and clarify their proper usage to prevent user confusion and potential fund loss.

Problem

The current SLIP39 implementation includes methods that can mislead users into thinking they can seamlessly convert between SLIP39 and BIP39 formats while maintaining the same wallet addresses. This is has several problematics:

  • recover_mnemonic is misleading: this method recovers the original BIP32 seed formatted as a BIP39 mnemonic, but using this mnemonic with standard BIP39 seed derivation (mnemonic_to_seed) generates a completely different seed, resulting in a new wallet with different addresses.

  • generate_shares usage confusion: while technically correct for creating new wallets, the method's documentation didn't clearly explain that it's not suitable for converting existing BIP39-backed wallets to SLIP39 while preserving addresses.

Solution

This PR implements the following changes:

  1. Added deprecation warning to recover_mnemonic that the function will be removed in a future version. Clarifies that the returned BIP39 mnemonic will generate different addresses than the original wallet.
  2. Enhanced documentation for generate_shares. This function is designed for creating NEW wallets from scratch. Explained that it does NOT convert existing BIP39-backed wallets while preserving addresses. Additional refactor is done in feat: add slip39 extendable flag #91.
  3. Updated SLIP39 test cases with proper BIP32 recovery validation, by adding extended private key (xprv) tests as per slip39 test vectors.

References

Future Work

  • Complete removal of recover_mnemonic method in a future major version

This PR prioritizes user safety by making the cryptographic limitations explicit, preventing accidental fund loss while maintaining backward compatibility.

alvroble added 2 commits July 9, 2025 17:41
* Add xprvs as per shamir spec
* recover_mnemonic: future deletion as it's misleading
* generate_shares: note on the implications of this method
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.

1 participant