SeedSigner / seedsigner

Use an air-gapped Raspberry Pi Zero to sign for Bitcoin transactions! (and do other cool stuff)
MIT License
699 stars 161 forks source link

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

Open BamaHodl opened 2 months ago

BamaHodl commented 2 months ago

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:

Checklist

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

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

newtonick commented 2 months ago

@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 commented 2 months ago

@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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 4 weeks ago

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 commented 4 weeks ago

I support this approach.

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