HDNodeWallet derivePath not working properly

[Jouzep]

Ethers Version

^6.9.2

Search Terms

HDNodeWallet, DerivePath

Describe the Problem

I am trying to derivePath from an HDNodeWallet, but the provided path is not the same path when the wallet is generated
for example i am trying to generate a wallet using Mnemonic and a path
input path="m/44'/60'/0'/0/1"
output path="m/44'/60'/0'/0/1/44'/60'/0'/0/1"

Maybe i have a bad understanding of HDWallet but in my mind they should be the same path

Code Snippet

const ethers = require("ethers");
const path = "m/44'/60'/0'/0/1";

const phrase = "word word word word word word word word word word word word";

const mnemonic = ethers.Mnemonic.fromPhrase(phrase);
const wallet = ethers.HDNodeWallet.fromMnemonic(mnemonic, path);

console.log(wallet.path);
// output: m/44'/60'/0'/0/1
const wallet1 = wallet.derivePath(path);
console.log(wallet1.path);
// output: m/44'/60'/0'/0/1/44'/60'/0'/0/1

Contract ABI

No response

Errors

No response

Environment

node.js (v12 or newer)

Environment (Other)

No response

[niZmosis]

Appears the deviation paths have been different since 6.0.0. We may have to revert to 5.7.2 if you want to derive accounts with the BIP44. As you can see from these tests, the base account from wallet is correct, but if you derive an account they are not correct, and the base account doesn't match derived account 0.

Test with 5.7.2:
`const { ethers } = require("ethers")

function test() {
const seedPhrase = 'escape joke bright reform stem industry cool announce hurt survey blossom wrap'

const wallet = ethers.Wallet.fromMnemonic(seedPhrase)

const path1 = m/44'/60'/0'/0/${0}
const derivedWallet1 = ethers.Wallet.fromMnemonic(seedPhrase, path1)
const path2 = m/44'/60'/0'/0/${1}
const derivedWallet2 = ethers.Wallet.fromMnemonic(seedPhrase, path2)

const metaMaskAccount1 = '0x904c2b17cdf69198eed7004E6b4C99e4C1DdB930'
const metaMaskAccount2 = '0x8Cdd9312b3D5Aa52d9ADccE816F9cfB90363A76b'

console.log('Base Wallet Address:', wallet.address)
console.log('Metamask 1:', metaMaskAccount1)
console.log('Derived 1:', derivedWallet1.address)
console.log('Metamask 2:', metaMaskAccount2)
console.log('Derived 2:', derivedWallet2.address)
}

test()`

Results:
Base Wallet Address: 0x904c2b17cdf69198eed7004E6b4C99e4C1DdB930 Metamask 1: 0x904c2b17cdf69198eed7004E6b4C99e4C1DdB930 Derived 1: 0x904c2b17cdf69198eed7004E6b4C99e4C1DdB930 Metamask 2: 0x8Cdd9312b3D5Aa52d9ADccE816F9cfB90363A76b Derived 2: 0x8Cdd9312b3D5Aa52d9ADccE816F9cfB90363A76b

Test with >=6.0.0
`const seedPhrase =
'escape joke bright reform stem industry cool announce hurt survey blossom wrap'

const wallet = Wallet.fromPhrase(seedPhrase)
const path1 = m/44'/60'/0'/0/${0}
const derivedWallet1 = wallet.derivePath(path1)
const path2 = m/44'/60'/0'/0/${1}
const derivedWallet2 = wallet.derivePath(path2)

const metaMaskAccount1 = '0x904c2b17cdf69198eed7004E6b4C99e4C1DdB930'
const metaMaskAccount2 = '0x8Cdd9312b3D5Aa52d9ADccE816F9cfB90363A76b'

console.log('Base Wallet Address:', wallet.address)
console.log('Metamask 1:', metaMaskAccount1)
console.log('Derived 1:', derivedWallet1.address)
console.log('Metamask 2:', metaMaskAccount2)
console.log('Derived 2:', derivedWallet2.address)`

Results:
Base Wallet Address: 0x904c2b17cdf69198eed7004E6b4C99e4C1DdB930 Metamask 1: 0x904c2b17cdf69198eed7004E6b4C99e4C1DdB930 Derived 1: 0xD3e7f7496373597bd382f8231B5C267952804058 Metamask 2: 0x8Cdd9312b3D5Aa52d9ADccE816F9cfB90363A76b Derived 2: 0x6eB2Da796aee744087F0DF30aFec8501282dfF22

[niZmosis]

Alright I found out what is causing it, and found a temp work around as well. So when you call derivePath, it will call that wallet instances "deriveChild" function. Once a wallet is instantiated, it will hold on to its deviation path with the class prop called "path". So when you call derivePath(), the class will go call deriveChild for each part of the path. What happens is it appends that class prop "path" when making the deviated path which isn't right and which is why it ends up like "m/44'/60'/0'/0/0/44'/60'/0'/0/1". This is why it works fine when first making the wallet as "path" is an empty string. It goes deeper and not sure the full intention of the code so I will leave it at that, as this is more than adding a line of code to fix. But for a work around, do not use "deviatePath()", use the static functions.

Here is where the problem is in hdwallet.ts

` /**

• Return the child for %%index%%.
  */
  deriveChild(_index: Numeric): HDNodeWallet {
  const index = getNumber(_index, "index");
  assertArgument(index <= 0xffffffff, "invalid index", "index", index);

// Base path
let path = this.path; // This will already have 'm/44'/60'/0'/0/0' when you call derivePath()
if (path) {
  path += "/" + (index & ~HardenedBit);
  if (index & HardenedBit) { path += "'"; }
}

const { IR, IL } = ser_I(index, this.chainCode, this.publicKey, this.privateKey);
const ki = new SigningKey(toBeHex((toBigInt(IL) + BigInt(this.privateKey)) % N, 32));

return new HDNodeWallet(_guard, ki, this.fingerprint, hexlify(IR),
  path, index, this.depth + 1, this.mnemonic, this.provider);

}`

Workaround:

`const { ethers } = require("ethers")

function test() {
const seedPhrase =
'escape joke bright reform stem industry cool announce hurt survey blossom wrap'

const wallet = ethers.HDNodeWallet.fromPhrase(seedPhrase)
const mnemonic = wallet.mnemonic

const path1 = m/44'/60'/0'/0/${0}
const derivedWallet1 = ethers.HDNodeWallet.fromMnemonic(mnemonic, path1)
// const derivedWallet1 = ethers.HDNodeWallet.fromPhrase(seedPhrase, '', path1)
const path2 = m/44'/60'/0'/0/${1}
const derivedWallet2 = ethers.HDNodeWallet.fromPhrase(seedPhrase, '', path2)

const metaMaskAccount1 = '0x904c2b17cdf69198eed7004E6b4C99e4C1DdB930'
const metaMaskAccount2 = '0x8Cdd9312b3D5Aa52d9ADccE816F9cfB90363A76b'

console.log('Wallet Address:', wallet.address)
console.log('Metamask 1:', metaMaskAccount1)
console.log('Derived 1:', derivedWallet1.address)
console.log('Metamask 2:', metaMaskAccount2)
console.log('Derived 2:', derivedWallet2.address)
}

test()`

Results:

Wallet Address: 0x904c2b17cdf69198eed7004E6b4C99e4C1DdB930 Metamask 1: 0x904c2b17cdf69198eed7004E6b4C99e4C1DdB930 Derived 1: 0x904c2b17cdf69198eed7004E6b4C99e4C1DdB930 Metamask 2: 0x8Cdd9312b3D5Aa52d9ADccE816F9cfB90363A76b Derived 2: 0x8Cdd9312b3D5Aa52d9ADccE816F9cfB90363A76b

[Jouzep]

Hello thank you mate very good explanation

[niZmosis]

We found a work around but the issue you brought up is still a problem if you can reopen it.

[ricmoo]

I want to keep this open to further investigate. I’ll close this again if it is deemed not an issue.

There are also two functions getAccountPath and getIndexedAccountPath, depending if you want to match Ledger or if you want to match MetaMask. Each wallet will have chosen their own standard, so it will take some testing to figure out which to use. I see the MetaMask path followed more often though, but you can also use something like Ledger Live; when you import a mnemonic, if checks the first few accounts on both the Ledger and the MetaMask paths, and if it finds MetaMask transactions, switched to MetaMask mode and tags them with a little icon.

[ricmoo]

So, the above code should have thrown an error. I'm adding that now: the "m/" part of the path asserts that the depth of the node is 0, i.e. that you are computing the child from the "master" or root node.

I'm adding a constraint that it ensures this is the case, so the code in the OP would throw.

As an aside, for those that wish to compute a large number of child nodes, this should be about 5 times faster, as it keeps a reference to an intermediate node, so those calculations do not need to be replicated:

const wallet = ethers.HDNodeWallet.fromMnemonic(mnemonic, "m/44'/60'/0'/0");
const wallet1 = wallet.derivePath("0");
console.log(wallet1);
const wallet2 = wallet.derivePath("1");
console.log(wallet2);

// Or in a for loop:
for (let i = 0; i < 10; i++) {
  console.log(wallet.deriveChild(i));
}

[ricmoo]

These changes were published in v6.11.1.

Thanks! :)

[Sean329]

I think the initial issue for which this ticket was opened is still there with v6.11.1 :

input path="m/44'/60'/0'/0/1"
output path="m/44'/60'/0'/0/1/44'/60'/0'/0/1"

So, is this intentional and not an issue? Otherwise why this ticket is closed?

[ricmoo]

@Sean329 No that is not the intended behaviour.

This problem should be corrected in the above change, which is to throw if an attempt is made to declare the current node is the root (i.e. master) node when it isn’t. Can you provide sample code that demonstrates the issue you are having?

[Sean329]

@Sean329 No that is not the intended behaviour.

This problem should be corrected in the above change, which is to throw if an attempt is made to declare the current node is the root (i.e. master) node when it isn’t. Can you provide sample code that demonstrates the issue you are having?

@ricmoo Sure, I'm using v6.11.1 and running the code below, and plz look at the log:

const ethers = require("ethers");
const path0 = "44'/60'/0'/0/0";

const phrase = "word word word word word word word word word word word word";

const mnemonic = ethers.Mnemonic.fromPhrase(phrase);
const wallet = ethers.HDNodeWallet.fromMnemonic(mnemonic, path0);

console.log(wallet.path);  // output: m/44'/60'/0'/0/0

const path1 = "44'/60'/0'/0/1";
const wallet1 = wallet.derivePath(path1);

console.log(wallet1.path); // output: m/44'/60'/0'/0/0/44'/60'/0'/0/1

I expect the wallet1.path to be "m/44'/60'/0'/0/1" instead of being "m/44'/60'/0'/0/0/44'/60'/0'/0/1". Am I misunderstanding some concepts in here? Thanks.

[martines3000]

I am also getting this error:
Serialized Error: { code: 'INVALID_ARGUMENT', argument: 'path', value: 'm/44/1236/1/0/0', shortMessage: 'cannot derive root path (i.e. path starting with "m/") for a node at non-zero depth 5' }.

The code worked fine before the update to the latest version.

I am kinda confused now. Should my code not work or has the new update introduced a bug ?
Thank you for your help.

[niZmosis]

@Sean329 No that is not the intended behaviour.
This problem should be corrected in the above change, which is to throw if an attempt is made to declare the current node is the root (i.e. master) node when it isn’t. Can you provide sample code that demonstrates the issue you are having?

@ricmoo Sure, I'm using v6.11.1 and running the code below, and plz look at the log:

const ethers = require("ethers");
const path0 = "44'/60'/0'/0/0";

const phrase = "word word word word word word word word word word word word";

const mnemonic = ethers.Mnemonic.fromPhrase(phrase);
const wallet = ethers.HDNodeWallet.fromMnemonic(mnemonic, path0);

console.log(wallet.path);  // output: m/44'/60'/0'/0/0

const path1 = "44'/60'/0'/0/1";
const wallet1 = wallet.derivePath(path1);

console.log(wallet1.path); // output: m/44'/60'/0'/0/0/44'/60'/0'/0/1

I expect the wallet1.path to be "m/44'/60'/0'/0/1" instead of being "m/44'/60'/0'/0/0/44'/60'/0'/0/1". Am I misunderstanding some concepts in here? Thanks.

@Sean329 @martines3000

Here would be your updated code.

`
const basePath = "44'/60'/0'/0"; // or "m/44'/60'/0'/0"

const phrase = "word word word word word word word word word word word word"
const mnemonic = ethers.Mnemonic.fromPhrase(phrase);
const baseWallet = ethers.HDNodeWallet.fromMnemonic(mnemonic, basePath);

const wallet1 = baseWallet.derivePath('0');
console.log(wallet1.path);

const wallet2 = baseWallet.derivePath('1');
console.log(wallet2.path);
`

Notice our basePath leaves off the index, and then when deriving you use what ever index you'd like. Don't use the baseWallet directly as the path isn't complete. If you do provide a complete path when making the wallet, it will be valid. But if you go to derive a wallet from it, you will run into the path concatenation problem I showed earlier in the thread.

@ricmoo What do you think about renaming derivePath to deriveIndex and then instead of a string we pass it a number?

EDIT: ricmoo pointed out there is the deriveChild which takes in a number.

`
const basePath = "44'/60'/0'/0"; // or "m/44'/60'/0'/0"

const phrase = "word word word word word word word word word word word word"
const mnemonic = ethers.Mnemonic.fromPhrase(phrase);
const baseWallet = ethers.HDNodeWallet.fromMnemonic(mnemonic, basePath);

const walletViaPath = baseWallet.derivePath('0');
console.log(walletViaPath.path);

const walletViaChild = baseWallet.deriveChild(0);
console.log(walletViaChild.path);
`

[martines3000]

Thanks for the fast reply. Yes this indeed solved my issue.

I also double checked that I get the same addresses (as with the old approach) with a simple script and it looks good. 💯

EDIT: I didn't test it correctly. If I try it with the following code, I get a different address.

const baseWalletOld =
  HDNodeWallet.fromMnemonic(mnemonic).derivePath(`m/44/1236/1/0/0`);

const baseWalletNew = HDNodeWallet.fromMnemonic(
  mnemonic,
  `m/44/1236/1/0`
).derivePath('0');

console.log(`Old: ${baseWalletOld.address}`);
console.log(`New: ${baseWalletNew.address}`);

[niZmosis]

Thanks for the fast reply. Yes this indeed solved my issue.

I also double checked that I get the same addresses (as with the old approach) with a simple script and it looks good. 💯

EDIT: I didn't test it correctly. If I try it with the following code, I get a different address.

const baseWalletOld =
  HDNodeWallet.fromMnemonic(mnemonic).derivePath(`m/44/1236/1/0/0`);

const baseWalletNew = HDNodeWallet.fromMnemonic(
  mnemonic,
  `m/44/1236/1/0`
).derivePath('0');

console.log(`Old: ${baseWalletOld.address}`);
console.log(`New: ${baseWalletNew.address}`);

Your baseWalletOld isn't correct. When not providing the path to the fromMnemonic it will use the default path which is "export const defaultPath: string = "m/44'/60'/0'/0/0";" at which point the base path of the wallet object is now to long to be able to use the derivePath. Because of that default, your ".derivePath(m/44/1236/1/0/0)" is now going to concatenate to the defaults. So you must provide the base path like before without the index, then use the derivePath with the index you want, not the full path. Your baseWalletNew is the correct way to use it.

[ricmoo]

@niZmosis It already exists, but is called .deriveChild, which accepts a number. :)

@martines3000 Working previously was a bug. When you begin a path with m/ you are indicating you want to enforce the current not is the master node. Previously it was ignored, which is bad. Much of the time the higher nodes are not actually present in memory (a feature baked into the specification), so simply “jumping” to the master node isn’t possible. I could consider adding a feature in the future that would allow crawling back up to it, if enough state is in memory to reconstruct it, but I’d have to think about that more. To get the old address using the buggy version, you can simply leave the m/ off the second path to descend into the HD structure the same way. You should move those funds though, as they won’t be accessible using standard tools like ledger or MetaMask.

If you need any help with this, let me know. :)

[chiro-hiro]

@ricmoo .deriveChild will create a path m/44'/60'/0'/0/0/0 which is incompatible with other wallet. Is it an issue? I'm using 6.11.1

[niZmosis]

@ricmoo .deriveChild will create a path m/44'/60'/0'/0/0/0 which is incompatible with other wallet. Is it an issue? I'm using 6.11.1

What ever you use for the base path when making the wallet instance, will be used for the derived paths. If you don't specify a path when creating the wallet, it will prefix with m/44'/60'/0'/0/0 which is the path MetaMask uses.

[chiro-hiro]

@ricmoo .deriveChild will create a path m/44'/60'/0'/0/0/0 which is incompatible with other wallet. Is it an issue? I'm using 6.11.1

What ever you use for the base path when making the wallet instance, will be used for the derived paths. If you don't specify a path when creating the wallet, it will prefix with m/44'/60'/0'/0/0 which is the path MetaMask uses.

@niZmosis But my path got 1 extra /0 is that normal?

[niZmosis]

@ricmoo .deriveChild will create a path m/44'/60'/0'/0/0/0 which is incompatible with other wallet. Is it an issue? I'm using 6.11.1

What ever you use for the base path when making the wallet instance, will be used for the derived paths. If you don't specify a path when creating the wallet, it will prefix with m/44'/60'/0'/0/0 which is the path MetaMask uses.

@niZmosis But my path got 1 extra /0 is that normal?

No, it sounds like the base path of the wallet was set with the full path, so when you derive it is appending to that ("44'/60'/0'/0/0") instead of ("44'/60'/0'/0"). Pretty much the wallet object you make, you won't be using directly if you are planning on deriving wallets from it.

`
function test() {
// Base path is being used as a prefix for all derived paths
const basePath = "44'/60'/0'/0"; // or "m/44'/60'/0'/0"
const phrase = "escape joke bright reform stem industry cool announce hurt survey blossom wrap"
const mnemonic = ethers.Mnemonic.fromPhrase(phrase);

const baseWallet = ethers.HDNodeWallet.fromMnemonic(
mnemonic,
basePath
).deriveChild(0);

console.log(Address: ${baseWallet.address});
}
`