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

Metamask QR cannot be generated when address is using customized derivation path (My Crypto, MEW) #128

Open
sbacelic opened this issue Mar 25, 2022 · 9 comments

Comments

@sbacelic
Copy link

sbacelic commented Mar 25, 2022

I have one account with custom derivation path using legacy format used by My Crypto/MEW: m/44'/60'/0'/1. When I try to sync the account and choose MetaMask QR I get the message: "Message is not compatible with the selected QR code type".

@sbacelic
Copy link
Author

sbacelic commented Mar 25, 2022

The problem is that address publicKey in not in extended public key format and MetamaskGenerator expects it to be, otherwise it will fail when decoding from base58:

    generateCryptoAccountMessage(data) {
        return __awaiter(this, void 0, void 0, function* () {
            const account = data.payload;
            const extendedPublicKey = bip32.fromBase58(account.publicKey);

Would you be interested in PR that could support different derivation paths (Ledger Live, BIP44, MEW/MyCrypto), similar like Keystone does so migration from legacy Ledger setup could be supported?

@sbacelic sbacelic changed the title QR cannot be generated for customized derivation path Metamask QR cannot be generated when account is restored using mnemonic phrase Mar 25, 2022
@sbacelic sbacelic changed the title Metamask QR cannot be generated when account is restored using mnemonic phrase Metamask QR cannot be generated when address is using customized derivation path (My Crypto, MEW) Mar 25, 2022
@AndreasGassmann
Copy link
Member

Is your account with the derivation path m/44'/60'/0'/1 a single account or an HD account?

I don't think it's possible to use this account with MetaMask if it's a single account.

MetMask expects an extended public key, it does not work with a regular public key. It will then derive multiple accounts by using the derivation paths /0/0, /0/1, /0/2, ... So the derivation path you have, m/44'/60'/0'/1, can not be constructed this way.

If your account is an HD account, meaning your first ETH account is m/44'/60'/0'/1/0/0, then you should be able to simply add an ETH HD account in AirGap Vault and set the derivation path to m/44'/60'/0'/1, which will then result in MetaMask displaying the accounts m/44'/60'/0'/1/0/0, m/44'/60'/0'/1/0/1, etc.

@sbacelic
Copy link
Author

Is your account with the derivation path m/44'/60'/0'/1 a single account or an HD account?
I don't think it's possible to use this account with MetaMask if it's a single account.

No, it is actually a HD account, I use 3 addresses m/44'/60'/0'/0, m/44'/60'/0'/1 and m/44'/60'/0'/2.

MetMask expects an extended public key, it does not work with a regular public key. It will then derive multiple accounts by using the derivation paths /0/0, /0/1, /0/2, ... So the derivation path you have, m/44'/60'/0'/1, can not be constructed this way.

Metamask will ask for HD path, when connecting using ledger:

Screenshot 2022-04-10 at 13 56 15

Based on the selection it will generate ext. pubkey using preselected HD format.

If your account is an HD account, meaning your first ETH account is m/44'/60'/0'/1/0/0, then you should be able to simply add an ETH HD account in AirGap Vault and set the derivation path to m/44'/60'/0'/1, which will then result in MetaMask displaying the accounts m/44'/60'/0'/1/0/0, m/44'/60'/0'/1/0/1, etc.

AirGap Vault assumes that HD path is in BIP44 format, however as my wallet was generated a long time ago my addresses are under an unexpected (legacy/mew) HD path.

I was able to hack support for different HD format in my local dev. env., but I'm not sure if supporting this will be useful for the wide audience. I'm already in the process of migrating to addresses under BIP44 HD path so I won't be needing this in the end.

Maybe its just enough to mention that only BIP44 HD path is supported.

@AndreasGassmann
Copy link
Member

Thanks for the information. Just to confirm, when pairing with the Ledger you can select different paths, but with QR based signers you cannot?

If that's the case I'll pass this information along to MetaMask.

@sbacelic
Copy link
Author

The actual issue in AirGap Vault, not in MetaMask. MM just reads the QR. It is up to AirGap to derive the right path. Check how Keystone does it.

@pvdyck
Copy link

pvdyck commented Dec 20, 2022

Any solution to this issue ?

@anotherBobDot
Copy link

anotherBobDot commented Jun 23, 2023

Folks, this is a big issue. There has to be a way to pass an {account_index} variable for HD wallet.
For example: https://gist.github.com/miguelmota/034c81886bd23b25c64ec5eda9251641
Notice how Ledger Live accounts are made and Legacy ones.
Airgap automatically assumes to append /0/{account_index} at the end. Example: if HD path is:
m/44'/60'/0' then the first address is
m/44'/60'/0'/0/0 and next one is m/44'/60'/0'/0/1 which is not WHAT is desired.
What is needed for ledger legacy support is
m/44'/60'/0'/1 = m/44'/60'/0'/{account_index}
for ledger live
m/44'/60'/1'/0/0 = m/44'/60'/{account_index}'/0/0
Please make it the way as Keystone wallet did in order for people have seamless transition from ledger seed phrases. Thank you.

@AndreasGassmann

@l1quid8
Copy link

l1quid8 commented Aug 24, 2023

Can we please get this feature implemented? Requiring users to move assets over to a new address creates too much friction for new users.

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

5 participants