Automated BIP39 Recovery (scan derivation paths, automatic, restore)

[lukechilds]

Resolves #6155

Implements automated scanning of all known BIP32 chains that can be imported into Electrum.

I'm opening this PR for review, it's not ready to be merged yet. Account discovery is functional but still needs to be integrated in to the GUI.

Currently it's implemented as a script for easy testing, you can quickly test it out by running the electrum/scripts/bip39_recovery.py script with a mnemonic parameter and also an optional second passphrase parameter. It will return a list of any previously used accounts it can find. You can use this mnemonic that I've created a few test accounts in:

much bottom such hurt hunt welcome cushion erosion pulse admit name deer

e.g:

$ ./bip39_recovery.py "much bottom such hurt hunt welcome cushion erosion pulse admit name deer"
[
    {
        "derivation_path": "m/84'/0'/0'",
        "description": "Standard BIP84 native segwit (Account 0)",
        "script_type": "p2wpkh"
    },
    {
        "derivation_path": "m/84'/0'/1'",
        "description": "Standard BIP84 native segwit (Account 1)",
        "script_type": "p2wpkh"
    },
    {
        "derivation_path": "m/0'",
        "description": "Non-standard legacy (Account 0)",
        "script_type": "p2pkh"
    },
    {
        "derivation_path": "m/84'/0'/2147483646'",
        "description": "Samourai Whirlpool post-mix",
        "script_type": "p2wpkh"
    }
]

Diversions from BIP44

Due to the fact that at discovery we only need to check which accounts has been used, not the final balance of each account, we take a few shortcuts:

• Don't scan the change chain, if a BIP44 compliant account is used it will have received a TX in one of the first 20 (BIP44 gap limit) addresses of the external chain.
• Move on to the next account as soon as we find a single address with funds. No need to keep scanning until we exhaust the gap limit.

If one of the discovered accounts is selected by the user for recovery, then Electrum will proceed to properly scan both internal external chains and exhaust the gap limit when it creates the wallet.

Limitations

Electrum takes an account level import path like m/84'/0'/0' and derives the wallet with the non hardened is_change/address_index sub trees. Due to this we cannot import Bitcoin Core accounts as they use the format m/0'/0'/address_index', notice all sub trees use hardened derivation.

All other wallets listed on walletsrecovery.org can be successfully recovered.

---

Please let me know if this code looks ok so far. Next steps will be to migrate the account discovery code from the script to electrum.bip39_recovery and then integrate it in to the recovery wizard GUI.

CC @ecdsa @SomberNight @aantonop

[SomberNight]

Looks fine

[lukechilds]

Thanks for reviewing @SomberNight, I have two questions re GUI integration.

1. Whats the best way to hook into the derivation path view?

I want to add a button in this view that says "Detect Existing Accounts" which when clicked, opens a popup with a loading icon, calls the account discovery function, displays a list of results to the user, allows them to select one, and populate the path/script type inputs with the result.

For example the previous view to enter the seed has it's own file in electrum/gui/qt/seed_dialog.py, so I could just edit that. However the derivation path is implemented in a GUI agnostic way in electrum/base_wizard.py which sets up some params and then inits the UI with self.choice_and_line_dialog() where choice_and_line_dialog() is a generic view provided by the GUI implementation.

electrum/electrum/base_wizard.py

Lines 407 to 439 in d0ab003

	def derivation_and_script_type_dialog(self, f):
	message1 = _('Choose the type of addresses in your wallet.')
	message2 = ' '.join([
	_('You can override the suggested derivation path.'),
	_('If you are not sure what this is, leave this field unchanged.')
	])
	if self.wallet_type == 'multisig':
	# There is no general standard for HD multisig.
	# For legacy, this is partially compatible with BIP45; assumes index=0
	# For segwit, a custom path is used, as there is no standard at all.
	default_choice_idx = 2
	choices = [
	('standard', 'legacy multisig (p2sh)', normalize_bip32_derivation("m/45'/0")),
	('p2wsh-p2sh', 'p2sh-segwit multisig (p2wsh-p2sh)', purpose48_derivation(0, xtype='p2wsh-p2sh')),
	('p2wsh', 'native segwit multisig (p2wsh)', purpose48_derivation(0, xtype='p2wsh')),
	]
	else:
	default_choice_idx = 2
	choices = [
	('standard', 'legacy (p2pkh)', bip44_derivation(0, bip43_purpose=44)),
	('p2wpkh-p2sh', 'p2sh-segwit (p2wpkh-p2sh)', bip44_derivation(0, bip43_purpose=49)),
	('p2wpkh', 'native segwit (p2wpkh)', bip44_derivation(0, bip43_purpose=84)),
	]
	while True:
	try:
	self.choice_and_line_dialog(
	run_next=f, title=_('Script type and Derivation path'), message1=message1,
	message2=message2, choices=choices, test_text=is_bip32_derivation,
	default_choice_idx=default_choice_idx)
	return
	except ScriptTypeNotSupported as e:
	self.show_error(e)
	# let the user choose again

I can see the QT implementation of choice_and_line_dialog here:

electrum/electrum/gui/qt/installwizard.py

Lines 605 to 633 in d0ab003

	@wizard_dialog
	def choice_and_line_dialog(self, title: str, message1: str, choices: List[Tuple[str, str, str]],
	message2: str, test_text: Callable[[str], int],
	run_next, default_choice_idx: int=0) -> Tuple[str, str]:
	vbox = QVBoxLayout()
	c_values = [x[0] for x in choices]
	c_titles = [x[1] for x in choices]
	c_default_text = [x[2] for x in choices]
	def on_choice_click(clayout):
	idx = clayout.selected_index()
	line.setText(c_default_text[idx])
	clayout = ChoicesLayout(message1, c_titles, on_choice_click,
	checked_index=default_choice_idx)
	vbox.addLayout(clayout.layout())
	vbox.addSpacing(50)
	vbox.addWidget(WWLabel(message2))
	line = QLineEdit()
	def on_text_change(text):
	self.next_button.setEnabled(test_text(text))
	line.textEdited.connect(on_text_change)
	on_choice_click(clayout) # set default text for "line"
	vbox.addWidget(line)
	self.exec_layout(vbox, title)
	choice = c_values[clayout.selected_index()]
	return str(line.text()), choice

I could just edit that method and wrap my code with. if title == "Script type and Derivation path" but that seems like a pretty nasty solution and would also break when i18n is active.

What's the best way to hook into this?

2. How should I provide network access to my method?

Currently in the script I'm creating a network instance with:

from electrum.simple_config import SimpleConfig
from electrum.network import Network
config = SimpleConfig()
network = Network(config)
network.start()

When I add this to the GUI I'll obviously want to use the existing network instance (if it exists during account creation?). I see Network is a singleton, and I can get the instance with Network.get_instance(), however I'd also like to keep the script functional by just updating it to import electrum.bip39_recovery and running it. It's useful to quickly test during dev and seems like a pretty handy script to expose to users too.

What's the best way to go about this? Inside the functoin use the global instance if it exists or create one if it doesn't? Or maybe pass a network instance in to the function, and pass in the one I create for the script or pass in the global instance for the GUI?

Not quite sure the best way to go about this.

[SomberNight]

2. How should I provide network access to my method?
   Or maybe pass a network instance in to the function, and pass in the one I create for the script or pass in the global instance for the GUI?

Yes, that sounds good.

GUI I'll obviously want to use the existing network instance (if it exists during account creation?)

Yes, typically it exists, unless the user used the --offline CLI flag.
When you get a reference to network, you should test if it's None, and if it is, just display an error message that they are offline.

1. Whats the best way to hook into the derivation path view?
   the derivation path is implemented in a GUI agnostic way in electrum/base_wizard.py which sets up some params and then inits the UI with self.choice_and_line_dialog() where choice_and_line_dialog() is a generic view provided by the GUI implementation.

choice_and_line_dialog is already only used for this specific usecase.
I think you can just rename it to something "derivation"-specific, and modify it directly.
I imagine you only want to implement all this for the Qt GUI? (which is fine)
Make sure that the change does not break the kivy GUI.
i.e. rename the method, pass it whatever arguments you need, make sure both GUIs accept all the arguments even if they ignore some, make sure the return types are the same
Then you can just implement extra logic in the Qt-specific implementation of this method -- well you probably want to implement it somewhere else as I imagine it would be many lines of code, and just call it from there.

[lukechilds]

@SomberNight thanks, makes sense!

Hadn't realised choice_and_line_dialog was only used for this one dialog.

And yes, I was only planning on implementing it in the QT GUI. CLI users can use the script, and I don't think the Kivy GUI allows setting the BIP39 option on a seed so not really applicable there.

[lukechilds]

@SomberNight when calling an async function from a QT WindowModalDialog I can do:

accounts = asyncio.run_coroutine_threadsafe(account_discovery(), network.asyncio_loop).result()

Which works but it blocks the QT thread so the UI hangs and there's no way for the user click the cancel button while they're waiting.

I'd like to:

• Show the modal
• Show some text like "Scanning for existing accounts..." or maybe an animated spinner
• Fire off the account discovery to run asynchronously
• Allow the user to interact with the UI, cancel the modal if they want
• Once account discovery has completed, update the modal with the results

Are you able to point me in the right direction for how to achieve this?

Disclaimer: Apologies if this question seems obvious, I'm not really a Python developer, in fact the only other time I've ever written Python was for my SSL fingerprint PR. I'm not familiar with Python's async API works.

[SomberNight]

if this question seems obvious

No worries, it's not obvious at all. Qt does not really support python asyncio.

Take a look at

electrum/electrum/gui/qt/main_window.py

Line 1658 in b0230f6

WaitingDialog(self, msg, task, on_success, on_failure)

It does not behave exactly as you described but similar.

[lukechilds]

I've integrated the functionality into the GUI.

Demo:

Let me know if it looks ok or if you have any suggestions, thanks!

[rdymac]

The resulting file name could use another label (as the [standard] one), so the user knows which one he selected after the scan.

[jlopp]

I'd highly encourage using a gap limit that far exceeds the standard. Consider the fact that there are a variety of applications such as payment processing that can result in huge gaps. If I recall correctly, btcpayserver set their gap limit to either 1,000 or 10,000. In my opinion the additional computation required is well worth the pain it will save for folks who have funds stranded beyond the standard gap limit.

[Enegnei]

If I recall correctly, btcpayserver set their gap limit to either 1,000 or 10,000.

For reference, yes, their gap limit appears to be 10,000.

[SomberNight]

I'd highly encourage using a gap limit that far exceeds the standard.

Do you mean for the discovery, or after the wallet is created for normal operations?

If you mean for the discovery, then ok, perhaps. It seems a bit unlucky to have a wallet where none of the first few addresses are used but later ones are, but I guess it can happen if a merchant was griefed as soon as they started using a wallet.

But to be clear, regardless of what insane gap limits some services are using, we are not able to use insane high gap limits simply due to architecture limitations. IMO what these services do when they use a gap limit of 10000 by default is anti-social behaviour: they are pulling the whole ecosystem in the same direction and slowly forcing everyone to use larger and larger gap limits (what if poor user used insane service no.17, we should use a higher gap limit then...). By using those high gap limits they introduce the risk for the user that they might not be able to restore easily using other services. That's just how it goes, we are not participating in this game.

[lukechilds]

Some back-of-the-envelope calculations re gap limit:

It currently takes around 2 seconds per account for discovery to complete with a gap limit of 20.

That means in the best case scenario where there are 0 used accounts, we will only scan 13 paths so it'll take ~26 seconds.

Setting the gap limit to 10,000 will mean the best case recovery process increases from ~26 seconds to ~3.6 hours.

(assuming each account scan is exactly 500x slower since it's scanning 500x more addresses)

[SomberNight]

Setting the gap limit to 10,000 will mean the best case recovery process increases from ~26 seconds to ~3.6 hours.

Default ElectrumX DOS limits will get hit a lot sooner than that, slowing you down to oblivion.

[lukechilds]

One solution could be to read the gap limit from the Electrum config if it exists instead of hardcoding to 20. Or maybe add a new BIP39_recovery_gap_limit param specifically for this.

Then if you connect to your own local Electrum instance, disable DoS limits, and manually set the config param to 10,000 it would work.

But if you're capable of all this, you're probably also capable of just manually restoring your wallet.

[aantonop]

The audience for this "wizard" is newbies who don't know their derivation path. The standard gap limit on import is enough. This is not a tool for merchants or advanced users. Let's keep it as simple and efficient as possible.

[lukechilds]

The resulting file name could use another label (as the [standard] one), so the user knows which one he selected after the scan.

@rdymac that would indeed be nice but unfortunately that isn't really possible without making quite large changes to the way the Electrum import wizard works.

Currently this PR is just adding a modal to one window and then auto-filling some fields in that window on completion.

Keeping this change simple and not too intrusive into the way Electrum works was a specific goal for this PR: #6155 (comment)

[SomberNight]

The resulting file name could use another label (as the [standard] one), so the user knows which one he selected after the scan.

@rdymac that would indeed be nice but unfortunately that isn't really possible without making quite large changes to the way the Electrum import wizard works.

Yes, let's not change [standard], it is related to the wallet type.
Note that the chosen script type is visible in Wallet>Information.
The derivation path is not visible in the GUI atm, but we could add it to the same dialog. If you want this, please see #4700 (and move that discussion there).

[ecdsa]

IMO what these services do when they use a gap limit of 10000 by default is anti-social behaviour

that's my opinion too. a high limit might work if you are scanning blocks yourself, but we should not engage in such behaviour with public servers.

[lukechilds]

I've addresses the feedback and tested/fixed when setting up with a HWW wallet.

Unless storing seed data on BaseWizard is an issue #6219 (comment) I think this should be ok to be merged.

Let me know if you require any other changes or you think there's any other scenarios I should test.

[SomberNight]

To expand on https://github.com/spesmilo/electrum/pull/6219/files#r444897453 ,
I think what we could pass around instead is a function (derivation_prefix) -> (account_xpub) (or similar), as then this could be (later?) implemented for hardware wallets as well.

and then, in the GUI, instead of guarding with self.seed_type == "bip39", you could check whether this function was supplied (whether it is None).

[andronoob]

@aantonop

This is not a tool for merchants or advanced users.

Why it can't? In my opinion, it can bring convenience for them as well. I think this may be implemented in a progressive style (with a manually specified derivation path, favorable to be selected from a list), with some rate limit to avoid unintentionally DoSing the server.

Sorry, I didn't get your point. It's not about the first funded address only. Maybe it's just not suitable for those wallets with huge gap-limit to be imported into Electrum.

Allowing the user to manually specify child key index also enables mis-uses.

[andronoob]

I also wonder whether the "BIP39-Electrum2.0 duality" issue (#1300) is properly handled here?

Electrum 2.0 seed phrase is not compatible with BIP39, but it still shares exactly the same wordlist with BIP39. Therefore, an Electrum 2.0 seed can be both a valid Electrum 2.0 seed and a valid BIP39 mnemonic, with a probability of one in sixteen. (12-word BIP39 mnemonic phrase has 4-bit checksum. 2^4=16)

It's then possible (although not very possible, because the BIP39 wallet would show empty transaction history and zero balance, so that the user is highly unlikely to continue) for a user to import an Electrum 2.0 seed into another BIP39-compliant wallet.

I think the seed generating step of the wallet wizard should also show some reminds/warnings about this issue.

[andronoob]

I dreamed that the new derivation path step of the wizard may look like this:

Specify your derivation path

---

Standard derivation paths

• Legacy (P2PKH)
  Example: 1CuC3b...

• SegWit, compatible (P2SH-P2WPKH)
  Example: 3Dq4Y9...

• SegWit, native Bech32 (P2WPKH)
  Example: bc1qff6a...

  Account index (don't change unless you know what you are doing): _____

Non-standard derivation paths

• Click here to select from the list...

Not sure?

• Let me guess!

(empty, if no detection is ever executed) / No fund detected / Recoverable funds detected, congratulations! You may click "next" now. Detect funds / (Button hidden after executing the detection)

---

Don't touch things below if you have no idea what they mean.

BIP32 derivation path: _________ Address type: P2WPKH (dropdown menu)

Gap limit: ____ (auto-filled, depends on wallet type)

[ecdsa]

related: #6001

[lukechilds]

@SomberNight @ecdsa are you able to re-review when you have time?

I've resolved the issue of attaching the mnemonic/passphrase to the instance and instead now pass an (account_path) -> (account_xpub) function around.

I haven't implemented this function for when a HWW is being restored but I have tested the wizard during HWW restore and it correctly leaves out the recovery option.

I don't currently have an Android development environment setup to test the Kivy GUI, however I had a quick look at the Kivy view and it just accesses all the keyword args with kwargs.get(key). This change just adds a single get_account_xpub keyword arg so it should be safely ignored on Android.

[lukechilds]

@SomberNight @ecdsa sorry to keep pinging you guys but not heard anything for ~1 month now so just wanted to check you haven't forgotten about this.

AFAIK I've implemented all required changes and there aren't any conflicts, is there anything else you want from me for this or can we get it merged in?

[SomberNight]

Starting with 41827cf (copy here: https://github.com/SomberNight/electrum/tree/pr/6219_bak20200820),
I've squashed all your commits, cc7a6e8,
and rebased on master, 7b122d2.

[SomberNight]

Looks good. Reviewed code and tested.

I have some minor nits (in addition to what I pushed) but that's fine.
Noticed that the Qt dialog has a sizing bug on dark theme (too small by default) - might be platform dependent, and probably related to QDarkStyle, so ignoring for now (my naive attempts to fix it failed).

[SomberNight]

Would be better to return a NamedTuple or similar; e.g. so that fields can be typed and can be enumerated statically (by IDE).

[SomberNight]

Thank you for your work.

[AlistairTB]

I would like to do exactly this with a multisig wallet using Zpub keys (for a read-only wallet). In case it's not obvious, the Zpubs were derived from BIP39 words and extended passphrase. Is this possible?

I've integrated the functionality into the GUI.

Demo:

Let me know if it looks ok or if you have any suggestions, thanks!

[SomberNight]

@AlistairTB No, this is only implemented for singlesig wallets.

Also, it is not clear exactly how you would want to use this.
The functionality here is for bruteforcing the derivation path and script type.
For a multisig wallet given Zpubs, you already know the script type, and you would typically have the Zpubs already at the "account level" of the derivation, so you already know the derivation path.
Further note that it is not possible to do hardened derivation starting from a master public key.

[AlistairTB]

Thank you for the reality check. I was using it the way it was intended, however, I had the wallet type wrong (3/3 vs 2/3) and was looking to solve the wrong problem.

[St333p]

Today I was helping a friend recover funds from a ledger and I'd say this feature would be very well appreciated also when creating a watch-only wallet from a hardware device. Shall I open a dedicated issue?

[SomberNight]

I'd say this feature would be very well appreciated also when creating a watch-only wallet from a hardware device. Shall I open a dedicated issue?

Sure, go ahead.