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

Feature/Plugin request: Mnemonic recovery UI with derivation path "scanning" #6155

Closed
aantonop opened this issue May 10, 2020 · 45 comments · Fixed by #6219
Closed

Feature/Plugin request: Mnemonic recovery UI with derivation path "scanning" #6155

aantonop opened this issue May 10, 2020 · 45 comments · Fixed by #6219
Labels
topic-wallet 👛 related to wallet.py, or maybe address_synchronizer.py/coinchooser.py topic-wizard 🧙‍♂️ related to wallet creation/restore wizard

Comments

@aantonop
Copy link

@aantonop aantonop commented May 10, 2020

I see many newbies having trouble with recovery of BIP39 mnemonics when they don't know the mnemonic path, especially if created by a wallet that used a non-standard path. I am motivated to help them, while steering them away from dangerous alternative choices such as web-based seed derivation tools or giving their seeds to strangers in return for promises of help (both of which are often proposed on various forums). While this can be solved with a full node and a rescan, such a solution is not useful for newbies as it is too complex. A GUI-based self-hosted multi-platform solution like Electrum, that already has access to indexed UTXO scripts seems to fit perfectly with the use case and be more secure than alternatives.

I would like to fund a plugin that allows Electrum to import a BIP39 mnemonic such that it "scans" several possible derivation paths to find ones that have UTXO history on at least one address. The plugin would then offer a drop down of "discovered" active derivation paths and allow the user to select the one to import into a wallet file, using that derivation path to follow the normal BIP39 derivation procedure. See https://walletsrecovery.org/ for a good list of how varied the derivation paths in different wallet implementations are.

I am considering funding this with a gitcoin grant, to encourage someone with experience to implement this, as well as attract more funding from others and more developers to Electrum.

At this stage, I'd like to gauge interest in the idea and see if there are any principled objections to doing this. I don't want to start any drama accidentally if this is an undesirable use-case or violates some kind of principle held by Electrum developers. I would like to fund this only if it is likely to succeed and be incorporated into a future release.

@SomberNight SomberNight added topic-wallet 👛 related to wallet.py, or maybe address_synchronizer.py/coinchooser.py topic-wizard 🧙‍♂️ related to wallet creation/restore wizard labels May 11, 2020
@SomberNight
Copy link
Member

@SomberNight SomberNight commented May 11, 2020

I agree that BIP39-based recovery is becoming more and more fragile/tedious.
I guess what you describe is a valid usecase, and yes if someone makes a PR it can get merged.

Just as you described, this scan should be a one-time thing at restore time -- once the wallet is created it should be tied to a specific derivation path and script type (as is current behaviour).

Further, the scan should be opt-in; e.g. there should be a button that the user needs to press.

In terms of UI, it should be part of the Wizard, maybe part of (or before) the current "Script type and derivation" dialog -- but in any case it should also work for hardware wallets and not just bip39 software seeds.


See https://walletsrecovery.org/ for a good list of how varied the derivation paths in different wallet implementations are.

Note that it is sometimes not just the derivation path but also the script type that needs to be bruteforced. For example, breadwallet derives p2wpkh at m/0h, while multibit HD derives p2pkh at m/0h.


If someone volunteers to implement this, they can ask questions here or on IRC (#electrum on freenode).

@ecdsa
Copy link
Member

@ecdsa ecdsa commented May 11, 2020

This is exactly the kind of situation I wanted to avoid.
I think we should really take steps to discourage further use of BIP39 in Electrum.

@ecdsa
Copy link
Member

@ecdsa ecdsa commented May 11, 2020

If the scan means forever syncing multiple HD accounts, it would introduce quite some complexity in the Electrum code, and it would increase server load in a very wasteful manner. We got rid of multiple account wallets a while ago, and I do not want to see that back.

If OTOH the scan is a one-time thing, then it would be more acceptable. In that case once could sweep the wallet into addresses derived from an Electrum seed, and avoid dealing with BIP39 all over the place.

@aantonop
Copy link
Author

@aantonop aantonop commented May 11, 2020

@ecdsa I remember that this is the primary objection you had to BIP39 as a standard, and this is why I asked before proceeding. Whether we like it or not, BIP39 is a de-facto standard and not just on Bitcoin. Users will use it whether we make it easy for them or not, as they don't have a choice when all the wallets use it. The question that remains is whether we will make it easy for them. I think making it hard for them just discourages self-custody and pushes people to exchange wallets. But that's just my opinion.

If you do not want this operating as a GUI option in Electrum, I can look to build it as a stand-alone Qt app that leverages Electrum servers. Would that make it more or less agreeable? I'm happy to hardcode my own Electrum server into it, so that it doesn't burden others.

In case it isn't obvious, I'm a huge fan of Electrum and extremely grateful for all the work everyone has done on this wallet. The last thing I want to do is seem ungrateful or annoy the devs!

@aantonop
Copy link
Author

@aantonop aantonop commented May 11, 2020

If the scan means forever syncing multiple HD accounts, it would introduce quite some complexity in the Electrum code, and it would increase server load in a very wasteful manner. We got rid of multiple account wallets a while ago, and I do not want to see that back.

If OTOH the scan is a one-time thing, then it would be more acceptable. In that case once could sweep the wallet into addresses derived from an Electrum seed, and avoid dealing with BIP39 all over the place.

Absolutely a one-time thing. The point is to "find" the paths, not to support multi-path wallets.

@aantonop
Copy link
Author

@aantonop aantonop commented May 11, 2020

Also, we could combine this with a BIG warning explaining why an Electrum seed is superior, and suggest a sweep operation. That might be a great way to "heal" some of this problem.

@lukechilds
Copy link
Contributor

@lukechilds lukechilds commented May 11, 2020

FWIW I would like to see this feature in Electrum and would be interested in implementing it if there are no objections to merging.

In that case once could sweep the wallet into addresses derived from an Electrum seed, and avoid dealing with BIP39 all over the place.

Electrum already supports importing a BIP39 seed. If we discover funds in a BIP39 seed, why not just allow them to import it? They can already do that anyway if they know their derivation path. As I understand it the point of this plugin would just be to help discover an unknown derivation path.

I appreciate there are objections against BIP39 and you want to encourage Electrum seed usage to Electrum users, but if the funds are already in a BIP39 seed, sweeping the entire wallet over seems a bit invasive. It's also subjecting the user to privacy risks.

@SomberNight
Copy link
Member

@SomberNight SomberNight commented May 11, 2020

I think if this code will be well written it should not impose a noticeable maintenance burden on the project, and then it can be merged.

Whether we like it or not, BIP39 is a de-facto standard and not just on Bitcoin. Users will use it whether we make it easy for them or not, as they don't have a choice when all the wallets use it. The question that remains is whether we will make it easy for them. I think making it hard for them just discourages self-custody and pushes people to exchange wallets. But that's just my opinion.

Also, if we don't provide this auto-scanning tool you propose, as BIP39 gets even more fragmented in the future, I foresee websites appearing that do this type of scanning. (maybe there are already some? I recall seeing something similar)
It would be better to provide safe tools to users.

If you do not want this operating as a GUI option in Electrum, I can look to build it as a stand-alone Qt app that leverages Electrum servers. Would that make it more or less agreeable? I'm happy to hardcode my own Electrum server into it, so that it doesn't burden others.

Of course you can build any tool you want :)
Or do you mean there could be a separate binary that we build/distribute for this purpose? In that case, no, we will most definitely not do that as that would have significant maintenance burden. It would be much easier for us to integrate it into the main application.


Also, we could combine this with a BIG warning explaining why an Electrum seed is superior, and suggest a sweep operation. That might be a great way to "heal" some of this problem.

I believe this is orthogonal to/independent from the main proposal here.
Although yes it might be worth considering that e.g. if the user restores a bip39 seed we offer an automated creation of a new electrum seed + sweeping. (but note complications with hw wallets; privacy implications, tx fee...)

@tiagotrs
Copy link
Contributor

@tiagotrs tiagotrs commented May 11, 2020

I see many newbies having trouble with recovery of BIP39 mnemonics when they don't know the mnemonic path, especially if created by a wallet that used a non-standard path. I am motivated to help them

@aantonop Independently of this path scanning tool, please help them by helping the industry move away from BIP39.

Whether we like it or not, BIP39 is a de-facto standard and not just on Bitcoin.

Being de-facto standard is not a reason to perpetuate a flawed system ;)

Intuitively, the seedphrase is all one needs to recover funds - BIP39 is deceptive in this regard. The word-list dependency is another major limitation, with direct impact in portability and localization/usability.

besides electrum, @Roasbeef and @jonasschnelli have also proposed different seed formats.

Maybe a standardization effort is fruitless as wallets are free to derive their keys in arbitrary ways. In the other hand, with a superior format, there might be a chance for a new standard to emerge.

@gitcoinbot
Copy link

@gitcoinbot gitcoinbot commented May 11, 2020

Issue Status: 1. Open 2. Started 3. Submitted 4. Done


This issue now has a funding of 1000.0 DAI (1000.0 USD @ $1.0/DAI) attached to it as part of the aantonop fund.

@ecdsa
Copy link
Member

@ecdsa ecdsa commented May 14, 2020

@aantonop sorry to answer late. as I wrote above, I will not accept a pull request that re-introduces multi-chain support in the wallet, because that's way too complex and it would waste our time. I just wanted to restate that in case someone decides to go for that bounty.

I see two options:

  • sweep all detected chains into an Electrum seed. (bad for short-term privacy, but this would be a one-time thing). Note that utxos could be mapped to a range of new addresses in order to improve privacy.
  • create one bip39 wallet per detected chain (short-term privacy is preserved, but bad for the blockchain, because wallets will keep using deprecated address formats). more challenging in terms of interacting with the existing wizard flow.

I wrote "short-term privacy" because in the long term it is also bad for your privacy to keep using old address formats, when the rest of the ecosystem has moved on.

Multi-chain wallets may implement slow transitions to new address formats (eg send the change of each transaction to a new address format), but this is the kind of complexity that I want to avoid in Electrum.

@ecdsa
Copy link
Member

@ecdsa ecdsa commented May 14, 2020

PS: thank you for that bounty, that's a good thing if it can attract developers to Electrum

@lukechilds
Copy link
Contributor

@lukechilds lukechilds commented May 14, 2020

@ecdsa I think there may be some misunderstanding here, I don't think @aantonop was suggesting multi-chain support at the wallet level.

At least my understanding is:

  1. User goes to create a new wallet.
  2. User selects an option along the lines of "Recover from BIP39 mnemonic seed".
  3. Electrum scans all common derivation paths from that seed e.g: m/44', m/49', m/84' etc.
  4. Electrum display a list of derivation paths that contain funds to the user.
  5. The user selects one of these to import.
  6. Electrum inits a new wallet with chosen xpriv/derivation path combo.

So Electrum doesn't have to change anything at the wallet level or support multiple derivation paths in a single wallet if that's your concern.

In fact the last step works exactly the same as how BIP39 seed import currently works in Electrum. The only difference being currently the user is required to already know their derivation path. This feature is just an automated way to discover the relevant derivation path(s) for the user.

If they import a seed that has wallets on multiple derivation paths, they would repeat the import process multiple times, and choosing a different derivation path each time, resulting in multiple Electrum wallets.

Does this resolve your concern?

Also @aantonop please correct me if I've misunderstood your proposal.

@SomberNight
Copy link
Member

@SomberNight SomberNight commented May 14, 2020

@ecdsa
I think the easiest thing to implement would be something like this:

  • in wizard, in the "script type and derivation" dialog, there would be a button ~ "scan blockchain using common parameters"
  • if clicked, it would do the scan for some small gap limit for all these chains and then either go to a new dialog or open a popup
  • in the new dialog, it would list for which parameters it found any history
  • the user could choose one of those parameters (path + script type) to create the wallet for, if there are multiple
    (if there are multiple, and the user wants to create a wallet for all, they can just restart the wizard and do the scan again. depending on how the scan is implemented (e.g. blockchain.scripthash.subscribe), the network requests might be cached)

@ecdsa
Copy link
Member

@ecdsa ecdsa commented May 14, 2020

@SomberNight @lukechilds indeed, scanning and creating a wallet with a single chain is probably easier to implement.

@lukechilds
Copy link
Contributor

@lukechilds lukechilds commented May 14, 2020

Also worth noting, we don't need to perform full account discovery as defined in BIP44. That's to discover full transaction history and final balance of the wallet. All we care about is "Has this wallet ever been used?".

This can be implemented much more efficiently by just requesting accountDerivationPath/0-gapLimit and checking if any of those addresses have any TX history. There's no need to check change addresses or continue to scan the chain until the gap limit is empty.

So as an example for m/44'/0'/0', with the default gap limit of 20, all we need to do is check the tx history for the first 20 external addresses, m/44'/0'/0'/0/0-m/44'/0'/0'/0/19, if we find any TX history for any of those addresses, offer that derivation path to the user, if not, don't.

Then let the wallet handle full account discovery on initial open.

@jonasschnelli
Copy link

@jonasschnelli jonasschnelli commented May 14, 2020

here are my 5 cents:

1. BIP39 mnemonic recovery is a broken concept

1.1 Missing derivation path
BIP39 mnemonics have no information about the used BIP32 derivation path, thus resulting in guessing which path was used (maybe BIP44, but maybe BIP32 default)

1.2 Missing script pub type
The mnemonic also have no information about the used script types, leading to deriving tons of possible scripts (addresses) in order to scan the blockchain.

1.3 Missing birthday
There is also no information about the birthday of that seed. This makes it necessary to always scan from the genesis (or say from when BIP39 was proposed).

1.4 BIP44 gap limit is a broken concept
Most wallets use BIP44 in conjunction with BIP39 where a gap limit of 20 is recommended and mostly used. If one hands out 100 receiving addresses to other participants where only the 21th till the 100th make a payment, all those funds would not be reconstructed in case of a BIP39/BIP44 recovery. Eventually lost forever in case of a sweep/seed-destory process.
A gap limit of 20 makes it probably only useful for HODLer like wallets.

1.5 BIP39/44 recoveries almost always require central validation
Right now users expect a quick, within-seconds recovery of mnemonics which is only possible when having access to a fully indexed blockchain, thus often resulting in using centralised validation providers (like electrum servers, etc.). Result: full indexes are bad in scaling, which leads to a compromise in privacy.

In my opinion, BIP39/44 recoveries should be the last option when all other backup methods have failed because there are mild (and increasing) risks for lost funds.

However, I'm in favour to have a tool like @aantonop suggests. Though, Ideally, it works with just a pruned Bitcoin Core instance (only the UTXO set).
What would be possible is to extend Bitcoin Cores scantxoutset command.
An option would be to take a BIP39 mnemonic as parameter (with some options). By default, all ever used derivation paths and script types would be derived with an extra large lookup window (takes a few MB of RAM). With that set of scripts, it would be possible to scan the UTXO set (though not restoring the transaction history of spent coins [only unspent]). This would take a minute or two and could be massively speed up with an UTXO set address-index.

In the best case, it would produce a sweep transaction (by providing a new send-to-address).

The call could also warn if the lookup window was almost reached with used uxtos (example: if address 9200 would be used in a window of 10000).

It could reliable reconstruct funds from BIP39 mnemonics with just a UTXO set (pruned Bitcoin Core ~10GB of data). It would work offline as well (take it offline after syncing the chain).

Long term, an alternative to BIP39 could emerge. One proposal I once started (draft) is "cipherseed" which is based on the LN seed concept but with some additional stuff.
https://gist.github.com/jonasschnelli/245f35894f6ff585b3f3d33c6f208991

Also, long term, I'm unsure if just backing up the seed is sufficient for a standard backup. Metadata might become more important (and eventually more valuable than the utxo itself).

However, providing users a tool to easily and extensively recover a mnemonic (where one might no longer know its origin) is certainly worth the effort. I think an electrum plugin could be a useful piece. Just make sure it has by default a crazy large lookup window (min 10000) and/or a super large gap limit... it should also derive a whole bunch of different derivation paths (m/0'/x', BIP44, etc.). The servers won't like it, ... but that's a whole other issue.

@aantonop
Copy link
Author

@aantonop aantonop commented May 14, 2020

@gitcoinbot
Copy link

@gitcoinbot gitcoinbot commented May 15, 2020

Issue Status: 1. Open 2. Started 3. Submitted 4. Done


Work has been started.

These users each claimed they can complete the work by 3 weeks, 6 days from now.
Please review their action plans below:

1) lukechilds has been approved to start work.

I can start work on this after I've completed my work on the payjoin BitcoinJS client and payjoin integration in BlueWallet.

Learn more on the Gitcoin Issue Details page.

@andronoob
Copy link

@andronoob andronoob commented May 17, 2020

Sorry, but I can't bear any more to speak out.

If I didn't misunderstand BIP39, it's the feature, instead of a bug, of BIP39, that it's universal, so that the user won't have to redo the backup job over and over again if (s)he wants to try new things like segwit, or just another cryptocurrency.

BIP39 gets even more fragmented in the future

Couldn't fragmentation happen to incompatible standards of mnemonic phrase as well? IMHO, BIP39 with misterious derivation path is still better than a variety of incompatible mnemonic standards, since the former only requires a simple magical string called "derivation path" for the user to do the job of wallet recovery, meanwhile, the later requires executable, which obviously introduces extra risks.

@andronoob
Copy link

@andronoob andronoob commented May 17, 2020

one-time versus full support all the time

To me, the case of BIP39 is probably not worse than the case that the wallet simply refuses to import an incompatible mnemonic, since the fund is unusable in either case. If a wallet doesn't support a feature, simply let it unsupported.

But I also have to admit that there's an uncomfortable fact that the wallet won't explicitly complain to the user in the case of BIP39, so that the user might feel confused (or even worse, unaware), since the balance won't be correct.

However, I could still insist to nitpick, that the user can feel confused as well if (s)he doesn't recognise what the exact type of a mnemonic phrase (s)he's proceeding is at all!


As for segwit (which has two different address types other than the legacy address) and the "accounts" of BIP39, I wish that Electrum could support them, in the form that the user could switch among them in the runtime, it would be a great convenience.

Note: If I recalled correctly, Electrum added radio buttons of address type selection for BIP39 mnemonic in the wallet recovery procedure (in other words, it's not avaiable in some older versions of Electrum) some time ago - I think this should be on the correct direction.

@andronoob
Copy link

@andronoob andronoob commented May 17, 2020

@jonasschnelli

  1. BIP39 mnemonic recovery is a broken concept

Anyway, It's the de-facto standard which has already involved countless users, who already have no way to escape.

1.1 Missing derivation path
BIP39 mnemonics have no information about the used BIP32 derivation path, thus resulting in guessing which path was used (maybe BIP44, but maybe BIP32 default)

To my understanding, it could on the contrary be the advantage of BIP39 that it's universal. like ,the backup could be made effective once and for all.

Even in the case that the wallet cannot scan out the correct balance, the fund is still not lost, it's just not recognised by the wallet, so that it's just temporarily unusable.

Missing script pub type
The mnemonic also have no information about the used script types, leading to deriving tons of possible scripts (addresses) in order to scan the blockchain.

AFAIK, BIP44/49/84 has already handled this stuff quite well. If some wallet (oh, like Bitcoin Core, which doesn't adopt BIP39 however) doesn't follow the standard, it might not be elegant to put some "ugly patch" or "compromise", but I think it's still necessary from the usability aspect.

Edit: I think some "discrimination" against non-standard (not compatible with BIP44/49/84) derivation paths is not bad - at least not worse than the current situation that Electrum has been "discriminating" against BIP39 for quite a long time.

1.3 Missing birthday
There is also no information about the birthday of that seed. This makes it necessary to always scan from the genesis (or say from when BIP39 was proposed).

To me it's just a marginal problem. With comprehensive indices (like what online block explorers and Electrum servers have), it seems that this problem could be easily mitigated.

Edit: the "wallet birthday" doesn't come from nowhere, it comes from the system clock - so what if the system clock has its own problem? Couldn't such "feature" on the contrary turn into a bug that the actual wallet birthday is earlier than what the seed had encoded?

1.4 BIP44 gap limit is a broken concept
Most wallets use BIP44 in conjunction with BIP39 where a gap limit of 20 is recommended and mostly used. If one hands out 100 receiving addresses to other participants where only the 21th till the 100th make a payment, all those funds would not be reconstructed in case of a BIP39/BIP44 recovery. Eventually lost forever in case of a sweep/seed-destory process.
A gap limit of 20 makes it probably only useful for HODLer like wallets.

I don't have much knowledge about this problem. However, I think such problem is unavoidable as long as the user can specify the child key index (to "skip" some addresses) on his own.

1.5 BIP39/44 recoveries almost always require central validation
Right now users expect a quick, within-seconds recovery of mnemonics which is only possible when having access to a fully indexed blockchain, thus often resulting in using centralised validation providers (like electrum servers, etc.). Result: full indexes are bad in scaling, which leads to a compromise in privacy.

To my understanding, it's actually impossible to achieve "quick, within-seconds recovery" without comprehensive indices - I don't know how this is related to BIP39/44...

If the indices are too costy to generate, I think BIP157/158 could be a nice trade-off.

In my opinion, BIP39/44 recoveries should be the last option when all other backup methods have failed because there are mild (and increasing) risks for lost funds.

I don't know what you meant. However, I agree that mnemonic-based backup should be the portable, last-resort recovery measure, which is obviously a parallel approach other than something like a whole copy of wallet.dat.

In my opinion, BIP39 should have made the fetishism of single WIF privkey obsolete quite a long time ago - sadly, I still see many users (including experienced ones who have been in the crypto field for quite a long time) who stick to WIF privkeys so tightly.

(related: bitcoin/bitcoin#17748 )

However, I'm in favour to have a tool like @aantonop suggests. Though, Ideally, it works with just a pruned Bitcoin Core instance (only the UTXO set).
What would be possible is to extend Bitcoin Cores scantxoutset command.

As long as the child key index keeps growing, it seems that, a potential problem that the fund could be out of the scanning scope, always exists, if the UTXO set is the only locally available data.

...migrating/sweeping from BIP39...

Long term, an alternative to BIP39 could emerge. One proposal I once started (draft) is "cipherseed" which is based on the LN seed concept but with some additional stuff.
https://gist.github.com/jonasschnelli/245f35894f6ff585b3f3d33c6f208991

I haven't read through your post. However, I think a good alternative to BIP39 must be invertible, or in other words, be an "encoding" like base64 of the BIP32 seed actually, so that the funds won't need to be moved (which introduces a lot of unpleasant things, including but not limited to: miners fees, privacy implications due to UTXO merging, anxious due to transaction queueing in the mempool, etc).

@lukechilds
Copy link
Contributor

@lukechilds lukechilds commented May 17, 2020

It's probably best if we keep this issue focused on the topic of the recovery feature.

It seems like @ecdsa is open to merging this feature as long as it's cleanly implemented and follows the unintrusive steps we've outlined above.

It doesn't seem productive to continue to argue about the pros/cons of the different seed formats.

@andronoob
Copy link

@andronoob andronoob commented May 17, 2020

It's probably best if we keep this issue focused on the topic of the recovery feature.
It doesn't seem productive to continue to argue about the pros/cons of the different seed formats.

I just don't think blaming BIP39 would be productive either.


Edit, to summarize:

I see that @aantonop linked to a website ( https://walletsrecovery.org/ ) which shows how varied the derivation paths in different wallet implementations are - however, this website also shows wallets which don't adopt BIP39 at all.

The live example is Bitcoin Core, which doesn't even provide any mnemonic at all.

How could BIP39 be blamed in the case that some "deviated" wallets have never adopted BIP39 at all?


I don't consider BIP39 ideal or perfect either. However, an objection like "BIP39 lacks derivation path!" cannot convince me. Sure, BIP39 itself doesn't define the derivation path, but it's clearly defined in BIP44/49/84, isn't it?

Even Electrum provides three options corresponding to three different address types - this has already worked, in a such a simple way, isn't it?

In my bold speculation, it's probably not a problem around seed/mnemonic format itself, it's about developers who insist on implementing things in their own way, which is not fixable by proposing better mnemonic schemes.

Edit: if the idea of invertible mnemonic encoding scheme is viable, maybe it's not that "unfixable", since any other seed could be converted into the newly designed invertible mnemonic phrase.


Yeah, there are also wallets using exotic derivation paths with BIP39.

However, the worst case to me is only that the user forgets he actually own some more coins stored on some unscanned derivation paths. This is not good, obviously. But, it's probably not worse than the case that the user cannot determine what the exact type of the mnemonic he's dealing with.

If the user still remember that he has much more coins than what the wallet displayed, I think copy-pasting derivation path (the "derivation path magic") doesn't seem to do harm, meanwhile, looking for "wallet recovery tool/service" (which is almostly inevitable in cases that the "derivation path magic" can't work) seems to be more risky, since the user might encounter malicious executables or scammers.

In other words, I think what @aantonop proposed is just making the "derivation path magic" closer to "stupidly-easy" - I appreciate this idea, but, I shamelessly has more expectation on seeing Electrum following the BIP39/44/49/84 standard, rather than seeing the handy feature of "stupidly-easy derivation path magic" (or "exotic derivation path probing" in other words) appearing on Electrum.

@andronoob
Copy link

@andronoob andronoob commented May 18, 2020

@tiagotrs

Being de-facto standard is not a reason to perpetuate a flawed system

IMHO, de-facto fragmentation of a bunch of various incompatible seed formats is not good, either.

Boldly speaking, I think a dropdown menu listing different derivation paths used by various different wallets can already work, it's just not so stupidly-easy for the user, while an automatic probing could just further improve this a little bit.

Besides, I think discriminating (just like how Electrum is currently treating BIP39) against non-standard (incompatible with BIP44/49/84) derivation paths is probably not bad.

Intuitively, the seedphrase is all one needs to recover funds - BIP39 is deceptive in this regard.

It's not BIP39, but BIP44/49/84 which define the standard derivation paths. Currently Electrum itself already provides three radio buttons for that three paths - it already works. It could work better if the user could switch among three paths (or even better, among different accounts) any time (s)he wants.

It was SegWit which introduced another two different address types, which has become a fait-accompli.

Sure, I have seen many cases myself that the user encountered "zero balance? WTF" problem (because generally the wallet scans only one derivation path, which is possibly not the path user has been using). However, I think this is fixable, by considering/showing all UTXOs from all three standard derivation paths.

I agree that such fix is not elegant. I actually think that BIP44/49/84 put unnecessary isolation here, which introduces extra steps if an offline wallet user wants to adopt SegWit, since the corresponding account-level extended public keys themselves are hardened-derivated.

However, I just think it's not so bad, not the degree that we must abandon the entire BIP39/44/49/84 scheme.

The word-list dependency is another major limitation, with direct impact in portability and localization/usability.

I can't see how the word-list dependency itself could be a problem.

I agree that BIP39 has poor design from localization aspect, because the BIP32 seed is one-way hashed from the mnemonic words, which disallows the conversion among different languages.

In my opinion, a good successor of BIP39 must be invertible, so that any other seed format could be painlessly (without sending on-chain transaction to migrate funds) converted into it.

besides electrum, @Roasbeef and @jonasschnelli have also proposed different seed formats.

Frankly speaking, I think proposing more and more different seed formats would inevitably complicate the recovery process.

Maybe a standardization effort is fruitless as wallets are free to derive their keys in arbitrary ways.

I totally agree with this.

In fact, I think any developer totally has his/her freedom to do what (s)he likes. I don't think it's a bad thing.

So why I'm unhappy seeing you guys blaming BIP39? Stopping you from proposing/implementing new seed formats is not what I meant. I just can't understand how the benefits could surpass the costs.

@jonasschnelli
Copy link

@jonasschnelli jonasschnelli commented May 18, 2020

Even in the case that the wallet cannot scan out the correct balance, the fund is still not lost, it's just not recognised by the wallet, so that it's just temporarily unusable.

I disagree. Users import a BIP39 mnemonic, assume all funds are restored, destroy the backup (digital, paper, steal) and move on. Later they eventually detect missing funds which could be lost forever.

Edit: the "wallet birthday" doesn't come from nowhere, it comes from the system clock - so what if the system clock has its own problem? Couldn't such "feature" on the contrary turn into a bug that the actual wallet birthday is earlier than what the seed had encoded?

Same as "the words come from somewhere". The software should verify the creation-date via the user.

To my understanding, it's actually impossible to achieve "quick, within-seconds recovery" without comprehensive indices - I don't know how this is related to BIP39/44...

Scanning the UTXO set for a mid-size set of scripts is something that takes a couple of seconds (maybe 1-2minutes on slower systems / see scantxoutset RPC call of Bitcoin Core).

The problem is, that the current usage of BIP39 implies that the user has access to comprehensive indices which in almost all cases are done via centralised validation (which could lie/omit).

@andronoob
Copy link

@andronoob andronoob commented May 18, 2020

@jonasschnelli

destroy the backup

Generally, the mnemonic phrase itself is the backup - the user should on the contrary take good care of it.

IMHO, if the devs of a wallet consider interoperability seriously, the wallet must adopt BIP44/49/84, so that any other standard-complaint wallet would be able to scan out the correct balance.

The problem you described seems to be an incomplete sweeping, which is generally a case of proceeding a paper wallet came from some untrusted source. Even in that case, I can't see the necessity to destroy old mnemonic still.

I still have to agree that, there could still be a tendency that the old privkey/mnemonic might be eventually lost carelessly, due to illusions like "there's nothing on that old privkey/mnemonic".

However, similar problem can happen to WIF privkeys as well, in the case of SegWit - currently Bitcoin Core (without the output descriptor wallet support) hasn't adopt BIP44/49/84 yet, so all three different address types share one common key chain, so that the address type of a WIF privkey exported from Bitcoin Core is obviously ambiguous. Besides the case of SegWit, there's also a possibility that someone carelessly sends coins to an old address that it's privkey has been lost.

Edit: there's another possibility that the "plausible deniability" can also lead to similar problem.

Edit2: The multisig scenario can also lead to similar problem, at least Electrum has failed to avoid such problem, because Electrum 2.0 seed version bit doesn't limit the use case to p2(w)pkh only - a seed can be used in either cases of multisig or single-privkey.

I also remember that Satoshi once said:

why delete a wallet instead of moving it aside and keeping the old copy just in case? You should never delete a wallet.


Same as "the words come from somewhere".

Just like how the Glacier Protocol was designed, generating and verifying random number can be really tricky, which involves two laptops according to it. Although the wallet birthday is not so sensitive as the raw entropy, it's still tricky to "verify" it, especially during importing/recovery process.

The software should verify the creation-date via the user.

During the seed creation process, this would obviously introduce extra steps, while the benefits are not so clear to me, because the birthday doesn't seem to be so reliable - even if it is reliable, I still think it's not so necessary.


Scanning the UTXO set for a mid-size set of scripts is something that takes a couple of seconds (maybe 1-2minutes on slower systems / see scantxoutset RPC call of Bitcoin Core).

According to my experience, scantxoutset usually takes several minutes, for a simple address. I'm running it on a laptop which has a 4th gen quad core i7 running at 3.x GHz, 16GB of dual channel DDR3 1600 RAM, and SSD.

Also, my point was that the child key index would eventually grow, so that you can't know the current index number merely with the UTXO set.

The problem is, that the current usage of BIP39 implies that the user has access to comprehensive indices which in almost all cases are done via centralised validation (which could lie/omit).

I can't see how BIP39 implies that.

I'm using Trezor with Electrum and Electrum Personal Server (connected to local Bitcoin Core full node). I agree that EPS has a long initialization process, but generally I feel that such combination works fine. I think it might work better if Bitcoin Core gains the ability of fast (and even better, P2P-based, in case of pruning) rescan with BIP157 block filter index, which seems to be a good trade-off between comprehensive indices and the status quo of Bitcoin Core that only simple txindex is optionally available.

@jonasschnelli
Copy link

@jonasschnelli jonasschnelli commented May 18, 2020

IMHO, if the devs of a wallet consider interoperability seriously, the wallet must adopt BIP44/49/84, so that any other standard-complaint wallet would be able to scan out the correct balance.

BIP44/49 was designed for hardware wallets / cold-storage where the hot keys remain on a different device/layer. This is the reason why it is currently not adopted in Core/BRD and eventually other hot key wallets.

@andronoob
Copy link

@andronoob andronoob commented May 18, 2020

BIP44/49 was designed for hardware wallets / cold-storage where the hot keys remain on a different device/layer. This is the reason why it is currently not adopted in Core/BRD and eventually other hot key wallets.

IIRC, you didn't implement HD support of Bitcoin Core with compliance of BIP44 mainly due to the reason of simplicity, so that it would be easier for the community to review the code. Another reason was probably about the inherent weakness of normal derivation of BIP32, that a single child privkey plus the corresponding parent xpub can derive the corresponding parent xprv, as well as all the corresponding child privkeys.

However, I can't see how BIP44/49/84 could be incompatible with hot key wallets - even Bitcoin Core itself has implemented the native output descriptor support with BIP44/49/84 derivation path.

I agree that countless users have already developed the (bad) habit of exporting privkeys. However, in my opinion, it's probably better to reshape those users' habit with warnings/click-throughs, while turning more and more wallets (including Bitcoin Core) standard-compliant.

Edit: personally I have another criticism against BIP39/44/49/84 scheme, that users generally tend to import the mnemonic, which is the root of the whole wallet, with insufficient care. Therefore, the isolation provided by BIP44/49/84 would be in fact bypassed/nullified, so that a risk of "one compromised, all compromised" could arise. To address this, I think some guidance might work, so that the user would learn to make use of the account extended public/private keys.

@aantonop
Copy link
Author

@aantonop aantonop commented May 18, 2020

@jonasschnelli
Copy link

@jonasschnelli jonasschnelli commented May 18, 2020

@aantonop sorry for misusing this issue. It’s obviously off-topic.

I think a such plugin would be a great additional feature for Electrum.

@andronoob
Copy link

@andronoob andronoob commented May 18, 2020

@aantonop Sorry.

However, I just can't agree with the idea that "any funds stored on BIP39 mnemonic are strongly adviced to be sweeped away (otherwise you have to either give up using Electrum, or create three separate Electrum wallets)" - it seems that, some members of this projects have already expressed the intention to implement the feature you proposed in that way.

It would still be probably fine if Electrum insists on that "you are strongly adviced to sweep the funds" approach, however, I can't hold my impulse to express my personal feelings/opinions/biases around that.

Just like what @lukechilds has outlined:

sweeping the entire wallet over seems a bit invasive.

(By the way, actually I think the sweeping function of WIF privkeys is also "invasive" or "misleading", because generally the user doesn't want to send an on-chain transaction, which costs him/her extra money and attention)


@ecdsa

Multi-chain wallets may implement slow transitions to new address formats (eg send the change of each transaction to a new address format),

It's actually gradual or smooth transition to new address formats to me. It's not slow.

but this is the kind of complexity that I want to avoid in Electrum.

Personally I wish that Electrum could implement multi-keychain support, but only for standard paths like BIP44/49/84. It seems that my wish is not likely to come true...


@lukechilds

Also worth noting, we don't need to perform full account discovery as defined in BIP44. That's to discover full transaction history and final balance of the wallet. All we care about is "Has this wallet ever been used?".

I think the wallet could provide something like a menu option of "troubleshooting -> scan for extra addresses or accounts", which would let the user him/herself to decide the range to scan.


There could also be something like "troubleshooting -> non-standard derivation path", which would only lead the user to create another single-keychain (or only two separated key chains for receiving and change addresses) wallet.

Anyway, I think it's probably more important to make the user recall/aware of the exact model of the previously used wallet, instead of sheerly relying on blind probing.

@aantonop
Copy link
Author

@aantonop aantonop commented May 18, 2020

@aantonop
Copy link
Author

@aantonop aantonop commented May 18, 2020

@andronoob
Copy link

@andronoob andronoob commented May 19, 2020

@aantonop

no value judgments on mnemonic format

Sigh. Even if Electrum would like to stop the discrimination against BIP39, I on the contrary still think that BIP39 itself has a fatal "flaw", that it's just as exclusive as other incompatible seed formats, including but not limited to Electrum 2.0 seed, aezeed, or something other... Maybe I'm just misusing another issue, but at least I don't want to repeat same words posted here: #1300 (comment)


As for the multi-keychain support, I can't help to repeat my wish, that Electrum could support the standard BIP44/49/84 simultaneously, just like a lot of other wallets have done. I'm aware that I'm shamelessly repeating off-topic stuff once again, but in my opinion it might provide much more improvement in the user experience.

@andronoob
Copy link

@andronoob andronoob commented May 19, 2020

@aantonop

Instead of selecting a BIP32 derivation path from a radio-button or text-input form, we offer the (additional) option to "Scan for previously used branches"

To me, this actually implies a value judgement that BIP44/49/84 should no longer be respected. Honestly I don't agree with this. IMHO the current design of three radio buttons, which is standard-compliant, should be the better way to evolve. I think cancelling such design is stepping back.

Edit: I also have to admit that a distinction between standard-compliant (BIP44/49/84) and non-standard derivation paths might also imply value judgement - okay, I don't want to dodge, I just think a standardized way is better, personally.

Edit2: I think a description/caption like this is too technical to be understood by the user:

"Scan for previously used branches"

In my opinion, it could be better described like:

"Not sure about your previous wallet model? Let Electrum probe it for you!"

or:

"Not sure? Let me guess!"


Scan for use of the first G addresses (gap limit perhaps or shorter), across P possible derivation paths (most commonly used paths) and S possible script types

This seems like a blind probing - I have expressed my idea that it's probably more important to make the user aware of the exact model of previously used wallet, so that even without an automatic probing it can still work under some circumstances.

Edit: find the correct derivation path corresponding to the wallet model - it could even be as simple as a text table. It is that simple, and it can work. Ian Coleman's BIP39 tool has already implemented this. It's just not so stupidly-easy for the user.

@gitcoinbot
Copy link

@gitcoinbot gitcoinbot commented May 21, 2020

@lukechilds Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • reminder (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

1 similar comment
@gitcoinbot
Copy link

@gitcoinbot gitcoinbot commented May 30, 2020

@lukechilds Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • reminder (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

@lukechilds
Copy link
Contributor

@lukechilds lukechilds commented Jun 1, 2020

@aantonop any chance we can disable Gitcoin?

It's kind of spammy and annoying and I'm perfectly happy trusting you to hold the funds until you're happy the work has been completed to an adequate standard.

@AbdussamadA
Copy link
Contributor

@AbdussamadA AbdussamadA commented Jun 2, 2020

So as an example for m/44'/0'/0', with the default gap limit of 20, all we need to do is check the tx history for the first 20 external addresses, m/44'/0'/0'/0/0-m/44'/0'/0'/0/19, if we find any TX history for any of those addresses, offer that derivation path to the user, if not, don't.

will you be scanning multiple HD accounts or just the first one? some people may have received money to accounts other than the first.

@lukechilds
Copy link
Contributor

@lukechilds lukechilds commented Jun 2, 2020

@AbdussamadA yes we should keep iterating over the account index until we find an account with no transaction history as per BIP44 account discovery: https://github.com/bitcoin/bips/blob/master/bip-0044.mediawiki#Account_discovery

@lukechilds
Copy link
Contributor

@lukechilds lukechilds commented Jun 8, 2020

Draft PR submitted: #6219

@gitcoinbot
Copy link

@gitcoinbot gitcoinbot commented Jun 18, 2020

Issue Status: 1. Open 2. Cancelled


The funding of 1000.0 DAI (1000.0 USD @ $1.0/DAI) attached to this issue has been cancelled by the bounty submitter

@aantonop
Copy link
Author

@aantonop aantonop commented Jun 18, 2020

Bounty will be paid directly to @lukechilds, because Gitcoin didn't do what I expected it to do and instead of paying out cancelled the bounty. (I'm still learning how to use it and this was probably operator error).

Thanks for doing such great work @lukechilds !

@lukechilds
Copy link
Contributor

@lukechilds lukechilds commented Jun 19, 2020

I can confirm I've received the payment from @aantonop.

SomberNight added a commit that referenced this issue Aug 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic-wallet 👛 related to wallet.py, or maybe address_synchronizer.py/coinchooser.py topic-wizard 🧙‍♂️ related to wallet creation/restore wizard
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants