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

Adding support for Electrum Standard (P2PKH) seed phrases #581

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from

Conversation

BamaHodl
Copy link
Contributor

@BamaHodl BamaHodl commented Jul 20, 2024

Description

Now that SeedSigner supports legacy P2PKH, we can add support for the Electrum "Standard" seed phrase type. Because these were 13 words prior to Electrum wallet software version 2.7, it will require changing the settings entry to allow selection of 12 or 13 word electrum seeds:
newscreen

This pull request is categorized as a:

  • New feature

Checklist

  • I’ve run pytest and made sure all unit tests pass before sumbitting the PR

If you modified or added functionality/workflow, did you add new unit tests?

  • No, I’m a fool
  • Yes
  • N/A

I have tested this PR on the following platforms/os:

@newtonick
Copy link
Collaborator

@BamaHodl is there anything special I need to do to test this feature? Does the current Electrum desktop version support creating a new wallet of this type?

@BamaHodl
Copy link
Contributor Author

@BamaHodl is there anything special I need to do to test this feature? Does the current Electrum desktop version support creating a new wallet of this type?

The latest version of Electrum supports recovering a wallet from the old seeds but not creating them (by simply choosing "I have my own seed" when creating the wallet).

What I've been doing to test the old seeds is downloading the portable Windows version for prior releases, placing it in its own folder so that the electrum_data subfolder it creates is not shared across versions, and creating seed phrases that way. The old versions won't be able to connect to any public electrum server anymore, but at least they will generate the seed phrase and you can see the xpub and addresses to verify everything matches. And of course you can then load that seed phrase into the current Electrum release to do anything further that would require connectivity to an electrum server.

I may also be able to write a simple script to generate the various types of seed phrases instead of needing to go through this cumbersome process each time. Will get back if I have any success.

@BamaHodl
Copy link
Contributor Author

Here is a script I threw together for generating electrum seeds that you can use to test in SS and in the current version of electrum. Should be a lot easier than my old portable version method described above.

@kdmukai
Copy link
Contributor

kdmukai commented Jul 25, 2024

Would it make sense to make the 13-word support its own option in Settings? I just hate it when legacy support (which will be used by very few people) adds to the user burden of ALL users going through a particular flow.

Turn it on if you need that option, but the vast majority of users never need be bothered by the prompt.

Only complication is that we don't have any interdependent Settings options (i.e. if Electrum seeds are disabled, the 13-word option should be grayed out, n/a, etc).

@BamaHodl
Copy link
Contributor Author

I agree it's certainly not ideal, especially since the vast majority of users with electrum seeds are probably 12 word seeds (they were 13 for about 18 months, between version 2.0 in March 2015 until version version 2.7 in around August-October 2016.

One way to do it would be to make the ELECTRUM_SEEDS setting a SELECT_1 setting type, instead of enabled/disabled, where the options are NONE, 12WORD, and 13WORD. Then we could eliminate the new seed length selection screen altogether. Thoughts?
Screenshot from 2024-07-25 07-15-04

@newtonick newtonick added this to the 0.9.0 milestone Aug 30, 2024
@newtonick
Copy link
Collaborator

One way to do it would be to make the ELECTRUM_SEEDS setting a SELECT_1 setting type, instead of enabled/disabled, where the options are NONE, 12WORD, and 13WORD

I support this approach. This Electrum seeds will be so rare and those who do use this feature are going to be much more technical in nature.

@BamaHodl
Copy link
Contributor Author

I support this approach.

Thanks! I've updated it to match the new SELECT_1 setting approach and updated the doc and tests accordingly

@newtonick newtonick added the enhancement New feature or request label Oct 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants