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

Inconsistent/incorrect derivation policy in wallet.backup for multisig #6688

Closed
Fonta1n3 opened this issue Oct 26, 2020 · 11 comments
Closed

Comments

@Fonta1n3
Copy link

Why is it when you create a multisig wallet with the option of "create new seed" that Electrum saves the derivation as m/0 in the wallet backup and also when you add co signer keys?

Yet when you select "I already have a seed" it allows you to select m/48'/0'/0'/2' as the default derivation (and saves it correctly)? However again the cosigner keys automatically and incorrectly get set to m/0?

For example I import one seed and Electrum derives the correct Zpub from that seed, yet when I add my cosigner keys it incorrectly assigns those Zpubs a m/0.

@SomberNight
Copy link
Member

Why is it when you create a multisig wallet with the option of "create new seed" that Electrum saves the derivation as m/0 in the wallet backup and also when you add co signer keys?

When using "create new seed", you are using Electrum seeds. The derivation paths in that case follow from the seed version; there is no room for error/customisation.

Yet when you select "I already have a seed" it allows you to select m/48'/0'/0'/2' as the default derivation (and saves it correctly)?

You mean when using BIP39 seeds. Indeed they give the user free reign to choose whatever path they want as there is no path information in the seed. If you restore from an Electrum seed, you can't choose a path - just like in the "create new seed" case.

However again the cosigner keys automatically and incorrectly get set to m/0?
For example I import one seed and Electrum derives the correct Zpub from that seed, yet when I add my cosigner keys it incorrectly assigns those Zpubs a m/0.

Presumably these Zpubs are for Electrum seeds?
Anyway, note that master keys don't contain much path information. (only the depth, and the value of the last level)

@Fonta1n3
Copy link
Author

Appreciate the response and info. I will do some more testing to make sure my mental model is correct.

@SomberNight
Copy link
Member

This is how you go electrum_seed_words -> bip32_seed -> xprv:

electrum/electrum/keystore.py

Lines 989 to 1000 in 2232955

elif t in ['standard', 'segwit']:
keystore = BIP32_KeyStore({})
keystore.add_seed(seed)
keystore.passphrase = passphrase
bip32_seed = Mnemonic.mnemonic_to_seed(seed, passphrase)
if t == 'standard':
der = "m/"
xtype = 'standard'
else:
der = "m/1'/" if is_p2sh else "m/0'/"
xtype = 'p2wsh' if is_p2sh else 'p2wpkh'
keystore.add_xprv_from_seed(bip32_seed, xtype, der)

@Fonta1n3
Copy link
Author

Fonta1n3 commented Nov 2, 2020

What I am trying to achieve is the ability for a user to import an Electrum wallet.backup file into Fully Noded. It makes the most sense to do this for multisig wallets so that is what I am focusing on.

Is it correct to assume that when you create a multisig wallet in Electrum selecting the create new seed option that the derivation encoded in the seed is by default m/48'/0'/0'/2' (for segwit wallets)? And that what is happening is Electrum refers to it as m/0 in the backup as the derivation is implied in the seed?

@SomberNight
Copy link
Member

SomberNight commented Nov 2, 2020

Is it correct to assume that when you create a multisig wallet in Electrum selecting the create new seed option that the derivation encoded in the seed is by default m/48'/0'/0'/2' (for segwit wallets)?

No. Electrum seeds do not use the bip44/49/84/45/84/etc derivation paths, as there is no point. They use paths as in linked code above.

Electrum refers to it as m/0 in the backup

Where do you see m/0 precisely? E.g. a segwit seed when used for multisig, would use m/1h.

See e.g. this 2of2 wallet comprised of an electrum segwit seed and a Zpub:

{
    [...]
    "x1/": {
        "derivation": "m/1'",
        "pw_hash_version": 1,
        "root_fingerprint": "43067d63",
        "seed": "snow nest raise royal more walk demise rotate smooth spirit canyon gun",
        "type": "bip32",
        "xprv": "ZprvAjxLRqPiDfPDxXrm8JvcoCGRAW6xUtktucG6AMtdzaEbTEJN8qcECvujfhtDU3jLJ9g3Dr3Gz5m1ypfMs8iSUh62gWyHZ73bYLRWyeHf6y4",
        "xpub": "Zpub6xwgqLvc42wXB1wEELTdALD9iXwStMUkGqBgxkJFYumaL2dWgNvUkjEDWyDFZD3fZuDWDzd1KQJ4NwVHS7hs6H6QkpNYSShfNiUZsgMdtNg"
    },
    "x2/": {
        "derivation": "m/1'",
        "pw_hash_version": 1,
        "root_fingerprint": "53b77ddb",
        "type": "bip32",
        "xprv": null,
        "xpub": "Zpub6y4oYeETXAbzLNg45wcFDGwEG3vpgsyMJybiAfi2pJtNF3i3fJVxK2BeZJaw7VeKZm192QHvXP3uHDNpNmNDbQft9FiMzkKUhNXQafUMYUY"
    }
}

@Fonta1n3
Copy link
Author

Fonta1n3 commented Nov 2, 2020

Why do you allow users to by default select m/48'/0'/0'/2' when supplying their own seed but use m/1' when its a seed generated by Electrum?

@SomberNight
Copy link
Member

It seems you are still missing the point.
There are two different types of seeds we are talking about here: Electrum seeds and BIP39 seeds.
One of the main features of Electrum seeds is that the user never has to interact with derivation paths.

"segwit" type Electrum seeds use m/1' for multisig.
BIP39 seeds use all kinds of arbitrary paths, so when using those, the user is prompted for a derivation path.

@cschneider1984
Copy link

I would like to add to this issue as it creates a problem with my electrum multisig-setup with coldcard: I have a 3 of 5 multisig wallet. There are 4 hardware signers (coldcard and trezors) as well as one signer in FullyNoded. The hardware signer's XPUBs were imported into the electrum multisig wallet diectly via USB connection. It was possible to set the derivation path to m/48'/0'/0'/2' (default) for each one. The FullyNoded XPUB was imported manually (copy & paste) and shows with derivation path undefined in the electrum wallet information.

When trying to sign a transaction with coldcard it shows the error message: FAILURE Input #0: pk#5 wrong, too shallow. I assume this is caused by the missing derivation path of the FullyNoded signer (which is pk#5).

How can I set the derivation path for the FullyNoded XPUB in electrum? Otherwise Coldcard will not accept/sign the multisig transaction.

@Fonta1n3
Copy link
Author

Fonta1n3 commented Jan 18, 2021

The only way I have used FN in an Electrum msig quorum is by using Electrum as a signer via a bip39 seed.

First step being to import a bip39 mnemonic into Electrum and then import each xpub (m/48'/0'/0'/2').

This works without issue for me with Electrum, Coldcard and FN.

Have you created the msig wallet on FN too?

@cschneider1984
Copy link

cschneider1984 commented Jan 18, 2021

My goal is to have 4 hardware wallets and one secure signer on my mobile device (FN).

I set up the electrum wallet as follows:

  • Create new FN signer and export XPUB, convert XPUB to ZPUB
  • Create new Electrum 3 of 5 multisig wallet and connect each hardware wallet via USB for XPUB import (m/48'/0'/0'/2')
  • As last step (no. 5), paste FN-ZPUB into Electrum wallet creator (it is not possible to specify a derivation path)
  • For Coldcard multisig: Export Coldcard multisig wallet file from Electrum and import into Coldcard. The multisig wallet incl. all 5 XPUBs is now visible on Coldcard.
  • For FN multisig: Create new multisig wallet in FN, scan all 4 QR codes (XPUBs of the hardware wallets shown in Electrum) and add 5th XPUB via existing FN signer. (This should be irrelevant to the Coldcard issue)

When I try to sign a transaction with Coldcard, the above mentioned error occurs on Coldcard. I'm able to sign with all other signers without any problem.

EDIT
I've used the "Decode PSBT" function in FN to view the contents. It displays the PSBT as JSON. In section "bip32_derivs" I see for all hardware wallets path = "m/48'/0'/0'/2'/0/2". For the FN-pubkey I see path = "m/0/2". That's obviously a problem for Coldcard.

@SomberNight
Copy link
Member

@cschneider1984 that's essentially #5715

see #5715 (comment)

I'll close this thread as I believe the original issue was mainly a misunderstanding, and the new issue is a direct consequence of #5715

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

No branches or pull requests

3 participants