Pluck key off of base path branch for Mnemonic

This reduces the tree walk complexity in wallet utils as it is assumed that we always walk relative. It also removes a HACK :)

4 files changed, 32 insertions, 35 deletions

diff --git a/packages/subproviders/src/subproviders/ledger.ts b/packages/subproviders/src/subproviders/ledger.ts
index 06c115105..d956a4f9d 100644
--- a/packages/subproviders/src/subproviders/ledger.ts
+++ b/packages/subproviders/src/subproviders/ledger.ts
@@ -26,6 +26,7 @@ const ASK_FOR_ON_DEVICE_CONFIRMATION = false;
 const SHOULD_GET_CHAIN_CODE = true;
 const DEFAULT_NUM_ADDRESSES_TO_FETCH = 10;
 const DEFAULT_ADDRESS_SEARCH_LIMIT = 1000;
+const INITIAL_KEY_DERIVATION_INDEX = 0;
 
 /**
  * Subprovider for interfacing with a user's [Ledger Nano S](https://www.ledgerwallet.com/products/ledger-nano-s).
@@ -203,10 +204,11 @@ export class LedgerSubprovider extends BaseWalletSubprovider {
     private async _initialDerivedKeyInfoAsync(): Promise<DerivedHDKeyInfo> {
         const ledgerClient = await this._createLedgerClientAsync();
 
+        const parentKeyDerivationPath = `m/${this._baseDerivationPath}`;
         let ledgerResponse;
         try {
             ledgerResponse = await ledgerClient.getAddress(
-                this._baseDerivationPath,
+                parentKeyDerivationPath,
                 this._shouldAlwaysAskForConfirmation,
                 SHOULD_GET_CHAIN_CODE,
             );
@@ -216,16 +218,14 @@ export class LedgerSubprovider extends BaseWalletSubprovider {
         const hdKey = new HDNode();
         hdKey.publicKey = new Buffer(ledgerResponse.publicKey, 'hex');
         hdKey.chainCode = new Buffer(ledgerResponse.chainCode, 'hex');
-        const derivationPath = `${this._baseDerivationPath}/0`;
-        const derivationIndex = 0;
-        return {
+        const address = walletUtils.addressOfHDKey(hdKey);
+        const initialDerivedKeyInfo = {
             hdKey,
-            address: ledgerResponse.address,
-            isChildKey: true,
+            address,
+            derivationPath: parentKeyDerivationPath,
             baseDerivationPath: this._baseDerivationPath,
-            derivationPath,
-            derivationIndex,
         };
+        return initialDerivedKeyInfo;
     }
     private _findDerivedKeyInfoForAddress(initalHDKey: DerivedHDKeyInfo, address: string): DerivedHDKeyInfo {
         const matchedDerivedKeyInfo = walletUtils.findDerivedKeyInfoForAddressIfExists(
diff --git a/packages/subproviders/src/subproviders/mnemonic_wallet.ts b/packages/subproviders/src/subproviders/mnemonic_wallet.ts
index 5dc1b3be7..287dcfd7d 100644
--- a/packages/subproviders/src/subproviders/mnemonic_wallet.ts
+++ b/packages/subproviders/src/subproviders/mnemonic_wallet.ts
@@ -14,8 +14,7 @@ import { PrivateKeyWalletSubprovider } from './private_key_wallet';
 const DEFAULT_BASE_DERIVATION_PATH = `44'/60'/0'/0`;
 const DEFAULT_NUM_ADDRESSES_TO_FETCH = 10;
 const DEFAULT_ADDRESS_SEARCH_LIMIT = 1000;
-const ROOT_KEY_DERIVATION_PATH = 'm/';
-const ROOT_KEY_DERIVATION_INDEX = 0;
+const INITIAL_KEY_DERIVATION_INDEX = 0;
 
 /**
  * This class implements the [web3-provider-engine](https://github.com/MetaMask/provider-engine) subprovider interface.
@@ -26,6 +25,8 @@ export class MnemonicWalletSubprovider extends BaseWalletSubprovider {
     private _addressSearchLimit: number;
     private _baseDerivationPath: string;
     private _derivedKeyInfo: DerivedHDKeyInfo;
+    private _mnemonic: string;
+
     /**
      * Instantiates a MnemonicWalletSubprovider. Defaults to baseDerivationPath set to `44'/60'/0'/0`.
      * This is the default in TestRPC/Ganache, it can be overridden if desired.
@@ -41,20 +42,10 @@ export class MnemonicWalletSubprovider extends BaseWalletSubprovider {
         assert.isNumber('addressSearchLimit', addressSearchLimit);
         super();
 
-        const seed = bip39.mnemonicToSeed(mnemonic);
-        const hdKey = HDNode.fromMasterSeed(seed);
+        this._mnemonic = mnemonic;
         this._baseDerivationPath = baseDerivationPath;
-        this._derivedKeyInfo = {
-            address: walletUtils.addressOfHDKey(hdKey),
-            // HACK this isn't the base path for this root key, but is is the base path
-            // we want all of the derived children to branched from
-            baseDerivationPath: this._baseDerivationPath,
-            derivationPath: ROOT_KEY_DERIVATION_PATH,
-            derivationIndex: ROOT_KEY_DERIVATION_INDEX,
-            hdKey,
-            isChildKey: false,
-        };
         this._addressSearchLimit = addressSearchLimit;
+        this._derivedKeyInfo = this._initialDerivedKeyInfo(this._baseDerivationPath);
     }
     /**
      * Retrieve the set derivation path
@@ -69,10 +60,7 @@ export class MnemonicWalletSubprovider extends BaseWalletSubprovider {
      */
     public setPath(baseDerivationPath: string) {
         this._baseDerivationPath = baseDerivationPath;
-        this._derivedKeyInfo = {
-            ...this._derivedKeyInfo,
-            baseDerivationPath,
-        };
+        this._derivedKeyInfo = this._initialDerivedKeyInfo(this._baseDerivationPath);
     }
     /**
      * Retrieve the accounts associated with the mnemonic.
@@ -140,4 +128,20 @@ export class MnemonicWalletSubprovider extends BaseWalletSubprovider {
         }
         return matchedDerivedKeyInfo;
     }
+    private _initialDerivedKeyInfo(baseDerivationPath: string): DerivedHDKeyInfo {
+        const seed = bip39.mnemonicToSeed(this._mnemonic);
+        const hdKey = HDNode.fromMasterSeed(seed);
+        // Walk down to base derivation level (i.e m/44'/60'/0') and create an initial key at that level
+        // all children will then be walked relative (i.e m/0)
+        const parentKeyDerivationPath = `m/${baseDerivationPath}`;
+        const parentHDKeyAtDerivationPath = hdKey.derive(parentKeyDerivationPath);
+        const address = walletUtils.addressOfHDKey(parentHDKeyAtDerivationPath);
+        const derivedKeyInfo = {
+            address,
+            baseDerivationPath,
+            derivationPath: parentKeyDerivationPath,
+            hdKey: parentHDKeyAtDerivationPath,
+        };
+        return derivedKeyInfo;
+    }
 }
diff --git a/packages/subproviders/src/types.ts b/packages/subproviders/src/types.ts
index b9d9d08ee..74ecec23b 100644
--- a/packages/subproviders/src/types.ts
+++ b/packages/subproviders/src/types.ts
@@ -120,11 +120,9 @@ export enum NonceSubproviderErrors {
 }
 export interface DerivedHDKeyInfo {
     address: string;
-    derivationIndex: number;
     baseDerivationPath: string;
     derivationPath: string;
     hdKey: HDNode;
-    isChildKey: boolean;
 }
 
 export type ErrorCallback = (err: Error | null, data?: any) => void;
diff --git a/packages/subproviders/src/utils/wallet_utils.ts b/packages/subproviders/src/utils/wallet_utils.ts
index a37597dff..4db876748 100644
--- a/packages/subproviders/src/utils/wallet_utils.ts
+++ b/packages/subproviders/src/utils/wallet_utils.ts
@@ -20,20 +20,15 @@ class DerivedHDKeyInfoIterator implements IterableIterator<DerivedHDKeyInfo> {
     public next(): IteratorResult<DerivedHDKeyInfo> {
         const baseDerivationPath = this._initialDerivedKey.baseDerivationPath;
         const derivationIndex = this._index;
-        const isChildKey = this._initialDerivedKey.isChildKey;
-        // If the DerivedHDKey is a child then we walk relative, if not we walk the full derivation path
         const fullDerivationPath = `m/${baseDerivationPath}/${derivationIndex}`;
-        const relativeDerivationPath = `m/${derivationIndex}`;
-        const path = isChildKey ? relativeDerivationPath : fullDerivationPath;
+        const path = `m/${derivationIndex}`;
         const hdKey = this._initialDerivedKey.hdKey.derive(path);
         const address = walletUtils.addressOfHDKey(hdKey);
         const derivedKey: DerivedHDKeyInfo = {
             address,
             hdKey,
             baseDerivationPath,
-            derivationIndex,
             derivationPath: fullDerivationPath,
-            isChildKey,
         };
         const done = this._index === this._searchLimit;
         this._index++;
@@ -78,7 +73,7 @@ export const walletUtils = {
         const ethereumAddressUnprefixed = ethUtil
             .publicToAddress(derivedPublicKey, shouldSanitizePublicKey)
             .toString('hex');
-        const address = ethUtil.addHexPrefix(ethereumAddressUnprefixed);
+        const address = ethUtil.addHexPrefix(ethereumAddressUnprefixed).toLowerCase();
         return address;
     },
 };