fix ledger unlocking using hdPath in accountDetails (#146)

MetaMask · May 26, 2022 · 239e4a7 · 239e4a7
1 parent d7d830b
commit 239e4a7
Show file tree

Hide file tree

Showing 2 changed files with 51 additions and 5 deletions.
diff --git a/index.js b/index.js
@@ -119,7 +119,7 @@ class LedgerBridgeKeyring extends EventEmitter {
     this.hdPath = hdPath
   }
 
-  unlock (hdPath) {
+  unlock (hdPath, updateHdk = true) {
     if (this.isUnlocked() && !hdPath) {
       return Promise.resolve('already unlocked')
     }
@@ -133,8 +133,10 @@ class LedgerBridgeKeyring extends EventEmitter {
       },
       ({ success, payload }) => {
         if (success) {
-          this.hdk.publicKey = Buffer.from(payload.publicKey, 'hex')
-          this.hdk.chainCode = Buffer.from(payload.chainCode, 'hex')
+          if (updateHdk) {
+            this.hdk.publicKey = Buffer.from(payload.publicKey, 'hex')
+            this.hdk.chainCode = Buffer.from(payload.chainCode, 'hex')
+          }
           resolve(payload.address)
         } else {
           reject(payload.error || new Error('Unknown error'))
@@ -373,7 +375,7 @@ class LedgerBridgeKeyring extends EventEmitter {
       throw new Error(`Ledger: Account for address '${checksummedAddress}' not found`)
     }
     const { hdPath } = this.accountDetails[checksummedAddress]
-    const unlockedAddress = await this.unlock(hdPath)
+    const unlockedAddress = await this.unlock(hdPath, false)
 
     // unlock resolves to the address for the given hdPath as reported by the ledger device
     // if that address is not the requested address, then this account belongs to a different device or seed

diff --git a/test/test-eth-ledger-bridge-keyring.js b/test/test-eth-ledger-bridge-keyring.js
@@ -214,6 +214,50 @@ describe('LedgerBridgeKeyring', function () {
         done()
       })
     })
+    it('should update hdk.publicKey if updateHdk is true', function (done) {
+      const ledgerKeyring = new LedgerBridgeKeyring()
+      ledgerKeyring.hdk = { publicKey: 'ABC' }
+
+      sandbox.on(ledgerKeyring, '_sendMessage', (_, cb) => {
+        cb({
+          success: true,
+          payload: {
+            publicKey:
+              '04197ced33b63059074b90ddecb9400c45cbc86210a20317b539b8cae84e573342149c3384ae45f27db68e75823323e97e03504b73ecbc47f5922b9b8144345e5a',
+            chainCode:
+              'ba0fb16e01c463d1635ec36f5adeb93a838adcd1526656c55f828f1e34002a8b',
+            address: fakeAccounts[1],
+          },
+        })
+      })
+
+      ledgerKeyring.unlock(`m/44'/60'/0'/1`).then((_) => {
+        assert.notDeepEqual(ledgerKeyring.hdk.publicKey, 'ABC')
+        done()
+      })
+    })
+    it('should not update hdk.publicKey if updateHdk is false', function (done) {
+      const ledgerKeyring = new LedgerBridgeKeyring()
+      ledgerKeyring.hdk = { publicKey: 'ABC' }
+
+      sandbox.on(ledgerKeyring, '_sendMessage', (_, cb) => {
+        cb({
+          success: true,
+          payload: {
+            publicKey:
+              '04197ced33b63059074b90ddecb9400c45cbc86210a20317b539b8cae84e573342149c3384ae45f27db68e75823323e97e03504b73ecbc47f5922b9b8144345e5a',
+            chainCode:
+              'ba0fb16e01c463d1635ec36f5adeb93a838adcd1526656c55f828f1e34002a8b',
+            address: fakeAccounts[1],
+          },
+        })
+      })
+
+      ledgerKeyring.unlock(`m/44'/60'/0'/1`, false).then((_) => {
+        assert.deepEqual(ledgerKeyring.hdk.publicKey, 'ABC')
+        done()
+      })
+    })
   })
 
   describe('setHdPath', function () {
@@ -276,7 +320,7 @@ describe('LedgerBridgeKeyring', function () {
     it('stores account details for bip44 accounts', function () {
       keyring.setHdPath(`m/44'/60'/0'/0/0`)
       keyring.setAccountToUnlock(1)
-      sandbox.on(keyring, 'unlock', (_) => Promise.resolve(fakeAccounts[1]))
+      sandbox.on(keyring, 'unlock', (_) => Promise.resolve(fakeAccounts[0]))
       return keyring.addAccounts(1)
         .then((accounts) => {
           assert.deepEqual(keyring.accountDetails[accounts[0]], {