Skip to content
This repository has been archived by the owner on Nov 21, 2019. It is now read-only.

Plans for Ledger Derivation Path? (Multiple Ledger / Path Issues inside) #407

Closed
MicahZoltu opened this issue Apr 7, 2017 · 27 comments
Closed

Comments

@MicahZoltu
Copy link
Contributor

I'm working on a dApp and when attempting to do what I was hoping would be a fairly straightforward ledger integration I found out that MEW and Ledger Chrome App are both using BIP32 derivation paths that claim to be BIP 44 compliant per BIP 43, but aren't actually BIP 44 compliant. After reading #332 it sounds like Ledger team and MEW are interesting in "fixing" this mistake but I haven't seen any forward movement on that. I have a bit of a rant over in EIP 84 on this topic for those who want a bit more depth.

The question I have for the MEW team is whether or not there are any plans to bring MEW into compliance with BIP 43 and 44?

If so, what time frame are those plans (weeks, months, quarters, years)?

I recognize that the UX for backwards compatibility is going to be a pain, but as a dApp developer I would really like to see this resolved. I'm trying to add ledger integration into our dApp but if I follow the standard and the user used MEW+Ledger to fund their account, they won't see their ETH in my dApp. If I ignore the standard and follow MEW, then the same problem will occur for anyone who used a wallet that did follow the standard.

At the moment there is no Ethereum specific standard (waiting on EIP 84 resolution) so I think following the BIP 43 and/or BIP 44 standard has value for end-users while we wait for something Ethereum specific.

@tayvano
Copy link
Contributor

tayvano commented Apr 7, 2017

Sorry for the hot mess of thoughts but wanted to shed some light even though I'm in the middle of like 10 things.

  • We will be moving to some sort of HD mnemonic system for V4. Not sure on timing. If its not done by end of summer, I'll kill something though so.....there's a deadline for you. 😉 We'll likely follow Metamask / Jaxx unless a standard is approved beforehand.

  • But at the moment we don't generate mnemonics. I would recommend including the Ledger team in on this conversation as we don't choose which addresses are on the Ledger. The Ledger has a key. If we were to display different addresses for any standard, Ledger users woulnd't be able to sign valid transactions....

  • I'm curious as to what why your dapp cares about what address is on a Ledger? As far as I can tell, unless you are having users enter their Ledger backup on your dapp, it shouldn't matter....Ledger's have to sign all transactions anyways.

  • There are numerous issues right now with the paths. Trezor opted for a new path for ETC because...well....duh. But Ledger was supporting ETH before the fork so those users have single accounts with both ETH and ETC. And now with EIP-155, it's getting even more complicated. Luckily I don't think you'll have to worry about that. But something to keep in mind. It's not so much about not following standards as it is about the ecosystem changing. Newer projects have huge advantages in that regard. 😉

@MicahZoltu
Copy link
Contributor Author

why your dapp cares about what address is on a Ledger?

Our dApp wants to be able to use hosted nodes (e.g., Infura) for accessing the chain but allow the user to sign transactions with the ledger. The ledger only stores a master seed (per BIP 32) so in order to ask the ledger to sign things on behalf of our users, using an account/address the user is familiar with (and has ETH), we have to supply the Ledger with a path. If we were to follow the BIP 44 standard, we would supply the Ledger with the path m/44'/60'/0'/0/x where x is some index (0 if we decide not not support multi-address wallets, or user selection if we do).

Unfortunately, if the user had previously used MEW to load ETH onto their Ledger, that path would lead us to an account/address that didn't have any ETH, because MEW used the path m/44'/60'/0'/x. As far as we could tell using a BIP 44 path, the Ledger has never been used with Ethereum before.

we don't choose which addresses are on the Ledger

I suspect we are saying the same thing, but I want to make sure we are clear. The Ledger only has a master seed. The seed by itself is not an Ethereum account/address, but when combined with a BIP 32 path the Ledger can deterministicly generate an Ethereum account (public address/private key). If you enter the same path into a ledger with a particular master seed, you will get the same account/address every time. If you use a different path, you'll get a different account/address.

We'll likely follow Metamask / Jaxx unless a standard is approved beforehand.

Unfortunately, at the moment it seems that there is basically zero consistency across wallets. I drafted up what I could find (may not be 100% correct yet) and you can see that there is really no consistency (even within MEW there are a few different paths used). This, IMO, argues very strongly with just following the existing standards (BIP 43/44) until such time as the community can develop a new Ethereum specific standard.

at the moment we don't generate mnemonics

This isn't really about mnemonics but rather it is about defining how to ask a hardware wallet for an address or addresses. At the moment, everyone is asking for them differently which means everyone is getting a different set of addresses for the same hardware wallet. From the user's point of view, this incredibly confusing because if they open their Ledger in MEW they see one set of addresses/balances but if they open their Ledger in another wallet they see a different set of addresses/balances.

There are numerous issues right now with the paths.

I agree so much with this! 😄

It's not so much about not following standards as it is about the ecosystem changing. Newer projects have huge advantages in that regard.

Also agree with this pretty strongly. However, I do think there is significant value in coming to some community consensus around a temporary solution to the HD Wallet paths problem until there is an official Ethereum supported solution. If everyone had rallied around a non BIP 44 compliant solution I would be mildly annoyed, but would go along with it until an EIP gets an official answer. Unfortunately, since the various wallets have all gone different directions we are in a situation where we need to just pick something (anything) and run with it. Given the only real compelling argument for any one of them is that one is a standard, I'm proposing that be what we rally around until an EIP can get pushed through to make things better.

@tayvano
Copy link
Contributor

tayvano commented Apr 7, 2017

I suspect we are saying the same thing, but I want to make sure we are clear.

yes sorry. braindead. really really really braindead. you are 100% correct.

Unfortunately, at the moment it seems that there is basically zero consistency across wallets.

https://github.com/kvhnuke/etherwallet/blob/mercury/app/scripts/controllers/decryptWalletCtrl.js

$scope.HDWallet = {
    numWallets: 0,
    walletsPerDialog: 5,
    wallets: [],
    id: 0,
    hdk: null,
    dPath: '',
    defaultDPath: "m/44'/60'/0'/0", // first address: m/44'/60'/0'/0/0
    alternativeDPath: "m/44'/60'/0'", // first address: m/44'/60'/0'/0
    customDPath: "m/44'/60'/1'/0",
    ledgerPath: "m/44'/60'/0'",
    trezorTestnetPath: "m/44'/1'/0'/0", // first address: m/44'/1'/0'/0/0
    trezorClassicPath: "m/44'/61'/0'/0", // first address: m/44'/61'/0'/0/0
    trezorPath: "m/44'/60'/0'/0", // first address: m/44'/60'/0'/0/0
};

Supposedly (Jaxx, Metamask, Exodus, imToken, TREZOR) are the same. Well, Trezor is the same sometimes but not for classic / testnet? I dont even know

From the user's point of view, this incredibly confusing because if they open their Ledger in MEW...

As the person who answers alllllll the support requests the answer is yes. There are in fact people who have 1 key between a Ledger or Trezor....have sent ETC to their ETH address on Trezor, have send ETC to their ETC Trezor address on Ledger, and generally making my brain fuck itself sideways trying to keep it all straight.

Anyways sorry I've had an incredibly rough day. How about our formal answer is "We will follow whatever standard or temporary-standard, we will not create new standards, and we would love to help implement whatever to help make any transitions easier. We really don't care what the standard is so you handle that. "

@pyskell
Copy link
Contributor

pyskell commented Apr 19, 2017

Would like to add to this that the Ledger ETC derivation path is: 44’/60’/160720'/0'/0 while Ledger ETH derivation path is as you noted: 44’/60’/0'/0

As it stands right now MyEtherWallet will fail on Ledger ETC transactions with an "Invalid Network ID" error.

Details are here: http://support.ledgerwallet.com/knowledge_base/topics/general-ledger-nano-s-faq

@MicahZoltu
Copy link
Contributor Author

MicahZoltu commented Apr 19, 2017

The BIP 44 derivation path for ETC SHOULD be m/44'/61'/0'/0/<index>.

@MicahZoltu
Copy link
Contributor Author

@pyskell Where do you see that error? In MEW? What are the steps to reproduce it?

@pyskell
Copy link
Contributor

pyskell commented Apr 19, 2017

@MicahZoltu Yes the error is in MEW

I just realized that this may be two separate issues:

  1. The different derivation path so it doesn't align with the ETC address seen in the official Ledger Ethereum Wallet Chrome App.
    a. The ETH address however will match the offical Ledger Ethereum Wallet Chrome App as MEW is using the same derivation path as the official application.
  2. Getting the "Invalid Network ID" error when trying to send the transaction to the network.

Anyway steps to reproduce the error I'm seeing when accessing https://myetherwallet.com:

  1. Make sure "Browser Support" is set to "Yes" on the Ledger Nano S device
  2. Select "ETC (Epool.io)" in the top right
  3. Select "Send Ether & Tokens"
  4. Check "Ledger Nano S"
  5. Choose "Connect to Ledger Nano S"
  6. Addresses shown are not the same ones that the Ledger Ethereum Wallet uses as the derivation path used here is not the same.
  7. Choose an address anyway
  8. Send it a small bit of ETC to test
  9. After transaction is confirmed attempt to send that same ETC back to the source address
  10. Click "Generate Transaction"
  11. Approve transaction on the Ledger Nano S
  12. Try to send transaction resulting in "Invalid Network ID"

@MicahZoltu
Copy link
Contributor Author

I don't believe the drop-down in the top right changes the derivation path used when communicating with a Ledger. I think right now if you talk to a Ledger with MEW it always uses one derivation path which is m/44'/60'/0'/0/0.

I believe the Invalid Network ID issue you are running into is a separate issue. You probably will have more success opening a separate GitHub issue for it.

@pyskell
Copy link
Contributor

pyskell commented Apr 19, 2017

Yeah I think my attempt was to get MEW to use the same derivation path as the official application when it comes to ETC but none of the derivation paths we've mentioned thus far seem to be it (including the ones noted on Ledger's own website). I'll need to open a separate issue with them I suppose to figure out the actual derivation path.

Any idea where comes from for the ETC derivation path? I tried a couple indexes (0, 1) and they don't match what the Ledger application gives either.

I'll open a second issue for Invalid network ID. Thanks!

@MicahZoltu
Copy link
Contributor Author

MicahZoltu commented Apr 19, 2017

If you have your mnemonic you can experiment with derivation paths that way via MEW. I would expect the path to be either the one they documented or m/44'/61'/0'/0/0 (BIP 44) or m/44'/51'/0'/0 (Ledger not-quite-BIP44).

@tayvano
Copy link
Contributor

tayvano commented Apr 19, 2017

Gah. Okay there are like 10 separate issues here. 💀 🔫 @kvhnuke

Actionable List

  1. Default Ledger path needs to be fixed for ETC (offer same as Trezor ETC (61) AND m/44'/60'/160720'/0'/0 as defaults, with custom input as third option) https://www.reddit.com/r/ethereum/comments/628lgf/mew_ethetc_question_with_nano_legder/dfkud56/)

  2. If you choose Ledger, it should allow you to enter custom path because all the paths are fucked regardless so just let the user figure it out (😉)

  3. Look into & fix: "Seems that I need to input the derivation path without the address_index to get the address derived correctly. When I include the address_index then MEW does not correctly derive the address (it gives a wrong set of addresses)."

  4. Update the chainId (of raw tx) AND path based on the node. Update this whenever the user switches the node.

/ rant

@tayvano tayvano changed the title Plans for Ledger Derivation Path? Plans for Ledger Derivation Path? (Multiple Ledger / Path Issues inside) Apr 19, 2017
@pyskell
Copy link
Contributor

pyskell commented Apr 19, 2017

Ok, so I've figured out the "correct" derivation path for Ledger ETC. Using MEW and entering my mnemonic phrase with a Custom derivation path of m/44'/60'/160720'/0' gives me the same first address as the official Ledger application when handling ETC. This seems to be because MEW will automatically tack on an extra /0 (for the index) after I've entered my derivation path.

A more detailed description:

In MEW in etherwallet-master.js (line 997) it has the path as:

ledgerPath: "44’/60’/160720'/0'/0"

I don't know JavaScript well but the comments around this area seem to indicate that MEW tacks on an extra /0 to addresses:

        defaultDPath: "m/44'/60'/0'/0", // first address: m/44'/60'/0'/0/0
        alternativeDPath: "m/44'/60'/0'", // first address: m/44'/60'/0'/0
        customDPath: "m/44'/60'/1'/0",
        ledgerPath: "44’/60’/160720'/0'/0",
        trezorTestnetPath: "m/44'/1'/0'/0", // first address: m/44'/1'/0'/0/0
        trezorClassicPath: "m/44'/61'/0'/0", // first address: m/44'/61'/0'/0/0

So my educated guess here is that when specifying the Custom derivation path for Ledger ETC it needs to be input as: m/44'/60'/160720'/0' which MEW will translate to m/44'/60'/160720'/0'/0 before deriving thus arriving at the correct path.

However if I input it as m/44'/60'/160720'/0'/0 (ie. verbatim as shown on Ledger's website) then it seems MEW may translate it to m/44'/60'/160720'/0'/0/0 which will be a different derivation path.

I'm wondering if this is desired behavior or not when using a custom derivation path as I don't know how people normally communicate these paths (ie. whether they include the /0 index or not). But perhaps it could do with at least a notification to users? Or perhaps some parsing to determine whether or not the user has indicated the index to start at?

@pyskell
Copy link
Contributor

pyskell commented Apr 19, 2017

@tayvano Gotcha, well this user has figured this out, but the problem still exists for actually using my Ledger Nano S, as when I do I'm unable to choose my own derivation path.

Entering my mnemonic onto an internet-connected machine is very much the opposite of the purpose of hardware wallets so hopefully you can at least let the user input their own derivation path when using the Ledger Nano S device.

@tayvano
Copy link
Contributor

tayvano commented Apr 19, 2017

Okay so the spec is:

m / purpose' / coin_type' / account' / change / address_index

The "last 0" that you speak of is the address index. The Ledger CX only shows the first address. We pull all the addresses.

I'm not sure what you mean by "adding a zero" though. The first address will be ..../ 0, the second will be ..../ 1 and so forth.

And yes, I think allowing users to enter a cust path when they are interacting with the Ledger should be the top priority.

@pyskell
Copy link
Contributor

pyskell commented Apr 19, 2017

Gotcha, thanks. By "adding a zero" I was referring to the address_index.

Seems that I need to input the derivation path without the address_index to get the address derived correctly. When I include the address_index then MEW does not correctly derive the address (it gives a wrong set of addresses). Wrong in the sense that they do not match the address that Ledger's official Ethereum app reports.

@tayvano
Copy link
Contributor

tayvano commented Apr 19, 2017

Interesting, I'm going to add a note to look into that. We should probably just ignore any specified address_index...or only show that address. Not sure why it would be specifying a separate set of addresses.

Also, finally found the comment regarding Ledger + ETC:

temporary path for ETC (m/44'/60'/160720'/0'/0)

https://www.reddit.com/r/ethereum/comments/628lgf/mew_ethetc_question_with_nano_legder/dfkud56/

@MicahZoltu
Copy link
Contributor Author

I'm going to add a note to look into that. We should probably just ignore any specified address_index...or only show that address. Not sure why it would be specifying a separate set of addresses.

@tayvano Unfortunately you can't do this because everything is terrible. BIP44 says that if the second segment is 44' then the full path is m / purpose' / coin_type' / account' / change / address_index as you mentioned. Unfortunately, some wallets (including the Ledger wallet app) leave out the change segment despite BIP 43/44 explicitly saying not to. This means that when an end-user puts in a derivation path you can't tell whether they put in a full non-compliant BIP 44 derivation path or a partial (missing address_index) BIP 44 compliant derivation path as they look identical.

For a full list of the known bad actors in the community see EIP 84. Also, support EIP 600 and EIP 601 which attempts to make it so that wallets can figure out the correct derivation path from the mnemonic alone. This will resolve this whole fiasco going forward and give Ethereum wallets something to standardize on that doesn't depend on the end-user knowing anything about derivation paths.

@tayvano
Copy link
Contributor

tayvano commented Apr 28, 2017

v3.6.6 should allow for customization of all paths. If anyone wants to pull current mercury version and test it, it would be much appreciated. (Except.....maybe you can't because you'd have to set up SSL first?)

93d9e14

@kevinmonahan Please let me know how it works with the Ledger and if you find anything un-intuitive

@kvhnuke You STILL have our Trezor so um.....if it's broken imma blame you.

👍

@tayvano tayvano added this to the v3.6.6 milestone Apr 28, 2017
@pyskell
Copy link
Contributor

pyskell commented Apr 28, 2017

Yeah can't test without setting up a server w/ SSL first. Will do later if I can get something spun up fast.

Also, just curious, why does MEW require SSL when running a local copy?

@tayvano
Copy link
Contributor

tayvano commented Apr 28, 2017

@pyskell it's a requirement of U2F, not MEW or Ledger. The reason Ledger uses a Chrome App over and extension is bc the app has access to USB directly and doesn't have to use the U2F functionality.

This is also the reason it doesn't work in firefox. U2F is kinda led by Google.

@MicahZoltu
Copy link
Contributor Author

You can tell chrome to allow U2F from pages served by HTTP. I found this to be easier than setting up a local HTTPS server.

@pyskell
Copy link
Contributor

pyskell commented Apr 28, 2017

@MicahZoltu How do you do that? A quick google search doesn't say.

Also I've tested it anyway and it works correctly for ETC with the Ledger Nano S. Providing the derivation path m/44'/60'/160720'/0'/0 as listed on Ledger's website leads to MEW providing the same address as in the official Ledger client.

And thanks. You devs are the best devs!

@tayvano
Copy link
Contributor

tayvano commented Apr 28, 2017

@MicahZoltu If you could share how, it would make a lot of folks lives easier - especially those running locally.

Thank you @pyskell. I will test a couple more times and then push live. Considering the amount of issues people are currently having with this, as long as it isn't hopelessly broken, It'll be an improvement. :/ We can make further updates / warnings / instructions based on how the support requests come in.

Thank you both for your assistance on this.

@MicahZoltu
Copy link
Contributor Author

https://github.com/google/u2f-ref-code#option-1-use-the-extension-from-the-webstore (at the end of the readme).

I went with option 2 because the extension linked in option 1 is from some random dude on the internet and I didn't trust it (though, it is linked from a Google GitHub repo).

@MicahZoltu
Copy link
Contributor Author

Note: HTTP_ORIGINS_ALLOWED resets itself to false after some period of time, so while testing you may find that you have to go back in and set it again. I wasn't able to figure out what triggered the reset, but it seemed to happen after maybe 30+ minutes.

@tayvano
Copy link
Contributor

tayvano commented Apr 28, 2017

Thank you! Reminds me of cloudflares dev mode: it's on for 3 hours. After 3 hours you about kill yourself trying to figure out why your changes are working and even your 5 alerts on document ready arent firing, before you realize it's cached.

@tayvano
Copy link
Contributor

tayvano commented Apr 30, 2017

The issues (besides the issue of having too many damn paths for ETH) is resolved. (I hope) with v3.6.6 which I just pushed live. Feel free to reopen or comment if it ain't.

@pyskell I just realized what you were talking about with the index: We are asking for the path, sans address index, as we then display 5 addresses (.... /0, ..... /1, ...../2,.......). Regardless of which way we chose to do things, it would be confusing so I'm going to opt for not changing this, as there are enough variables floating around.

@tayvano tayvano closed this as completed Apr 30, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants