Mostly added bad account detection

Currently riddled with logs, because the migrator is inexplicably returning before generating the new style accounts for comparison.

app/scripts/keyring-controller.js

@@ -113,6 +113,7 @@ module.exports = class KeyringController extends EventEmitter {
       conversionDate: this.configManager.getConversionDate(),
       keyringTypes: this.keyringTypes.map(krt => krt.type),
       identities: this.identities,
+      lostAccounts: this.configManager.getLostAccounts(),
     }
   }
 
@@ -623,7 +624,12 @@ module.exports = class KeyringController extends EventEmitter {
   migrateOldVaultIfAny (password) {
     const shouldMigrate = !!this.configManager.getWallet() && !this.configManager.getVault()
     return this.idStoreMigrator.migratedVaultForPassword(password)
-    .then((serialized) => {
+    .then((result) => {
+      console.log('migrator called back with')
+      console.dir(result)
+      const { serialized, lostAccounts } = result
+      console.dir({ serialized, lostAccounts })
+      this.configManager.setLostAccounts(lostAccounts)
       this.password = password
 
       if (serialized && shouldMigrate) {

app/scripts/keyrings/hd.js

@@ -38,16 +38,18 @@ class HdKeyring extends EventEmitter {
     }
 
     if ('numberOfAccounts' in opts) {
-      this.addAccounts(opts.numberOfAccounts)
+      console.log('number of accounts detected, adding accounts.')
+      return this.addAccounts(opts.numberOfAccounts)
     }
 
-    return Promise.resolve()
+    return Promise.resolve([])
   }
 
   addAccounts (numberOfAccounts = 1) {
     if (!this.root) {
       this._initFromMnemonic(bip39.generateMnemonic())
     }
+    console.log('attempting to add %s accounts', numberOfAccounts)
 
     const oldLen = this.wallets.length
     const newWallets = []
@@ -57,7 +59,9 @@ class HdKeyring extends EventEmitter {
       newWallets.push(wallet)
       this.wallets.push(wallet)
     }
+    console.log('hd has %s wallets', this.wallets.length)
     const hexWallets = newWallets.map(w => w.getAddress().toString('hex'))
+    console.log('hd calling back w promise of hex wallets ' + hexWallets)
     return Promise.resolve(hexWallets)
   }
 

app/scripts/lib/config-manager.js

@@ -432,3 +432,15 @@ ConfigManager.prototype.setGasMultiplier = function (gasMultiplier) {
   data.gasMultiplier = gasMultiplier
   this.setData(data)
 }
+
+ConfigManager.prototype.setLostAccounts = function (lostAccounts) {
+  var data = this.getData()
+  data.lostAccounts = lostAccounts
+  this.setData(data)
+}
+
+ConfigManager.prototype.getLostAccounts = function () {
+  var data = this.getData()
+  return ('lostAccounts' in data) && data.lostAccounts || []
+}
+

app/scripts/lib/idStore-migrator.js

@@ -1,5 +1,7 @@
 const IdentityStore = require('./idStore')
-
+const HdKeyring = require('../keyrings/hd')
+const sigUtil = require('./sig-util')
+const normalize = sigUtil.normalize
 
 module.exports = class IdentityStoreMigrator {
 
@@ -12,25 +14,34 @@ module.exports = class IdentityStoreMigrator {
   }
 
   migratedVaultForPassword (password) {
+    console.log('migrating vault for password')
     const hasOldVault = this.hasOldVault()
     const configManager = this.configManager
 
     if (!this.idStore) {
+      console.log('initializing id store')
       this.idStore = new IdentityStore({ configManager })
+      console.log('initialized')
     }
 
     if (!hasOldVault) {
+      console.log('no old vault recognized')
       return Promise.resolve(null)
     }
 
+    console.log('returning new promise')
     return new Promise((resolve, reject) => {
+      console.log('submitting password to idStore')
       this.idStore.submitPassword(password, (err) => {
+        console.log('returned ' + err)
         if (err) return reject(err)
-        try {
-          resolve(this.serializeVault())
-        } catch (e) {
-          reject(e)
-        }
+        console.log('serializing vault')
+        const serialized = this.serializeVault()
+        console.log('migrated and serialized into')
+        console.dir(serialized)
+        this.checkForErrors(serialized)
+        .then(resolve)
+        .catch(reject)
       })
     })
   }
@@ -45,6 +56,38 @@ module.exports = class IdentityStoreMigrator {
     }
   }
 
+  checkForErrors (serialized) {
+    console.log('checking for errors, first making hd wallet')
+    const hd = new HdKeyring()
+    return hd.deserialize(serialized)
+    .then(() => {
+      console.log('deserialized, now getting accounts')
+      console.dir(arguments)
+      return hd.getAccounts()
+    })
+    .then((hexAccounts) => {
+      console.log('hd returned accounts', hexAccounts)
+      const newAccounts = hexAccounts.map(normalize)
+      const oldAccounts = this.idStore._getAddresses().map(normalize)
+      const lostAccounts = oldAccounts.reduce((result, account) => {
+        if (newAccounts.includes(account)) {
+          return result
+        } else {
+          result.push(account)
+          return result
+        }
+      }, [])
+
+      console.log('migrator has')
+      console.dir({ newAccounts, oldAccounts, lostAccounts, hexAccounts })
+
+      return {
+        serialized,
+        lostAccounts,
+      }
+    })
+  }
+
   hasOldVault () {
     const wallet = this.configManager.getWallet()
     return wallet

test/integration/lib/keyring-controller-test.js

@@ -2,6 +2,7 @@ var KeyringController = require('../../../app/scripts/keyring-controller')
 var ConfigManager = require('../../../app/scripts/lib/config-manager')
 
 var oldStyleVault = require('../mocks/oldVault.json')
+var badStyleVault = require('../mocks/badVault.json')
 
 var STORAGE_KEY = 'metamask-config'
 var PASSWORD = '12345678'
@@ -41,6 +42,63 @@ QUnit.test('keyringController:submitPassword', function (assert) {
   this.keyringController.submitPassword(PASSWORD)
   .then((state) => {
     assert.ok(state.identities[FIRST_ADDRESS])
+    assert.equal(state.lostAccounts.length, 0, 'no lost accounts')
+    done()
+  })
+})
+
+QUnit.test('keyringController:setLocked', function (assert) {
+  var done = assert.async()
+  var self = this
+
+  this.keyringController.setLocked()
+  .then(function() {
+    assert.notOk(self.keyringController.password, 'password should be deallocated')
+    assert.deepEqual(self.keyringController.keyrings, [], 'keyrings should be deallocated')
+    done()
+  })
+  .catch((reason) => {
+    assert.ifError(reason)
+    done()
+  })
+})
+
+QUnit.module('Old Style Vaults with bad HD seed', {
+  beforeEach: function () {
+    window.localStorage[STORAGE_KEY] = JSON.stringify(badStyleVault)
+
+    this.configManager = new ConfigManager({
+      loadData: () => { return JSON.parse(window.localStorage[STORAGE_KEY]) },
+      setData: (data) => { window.localStorage[STORAGE_KEY] = JSON.stringify(data) },
+    })
+
+    this.keyringController = new KeyringController({
+      configManager: this.configManager,
+      getNetwork: () => { return '2' },
+    })
+
+    this.ethStore = {
+      addAccount: () => {},
+      removeAccount: () => {},
+    }
+
+    this.keyringController.setStore(this.ethStore)
+  }
+})
+
+QUnit.test('keyringController:isInitialized', function (assert) {
+  assert.ok(this.keyringController.getState().isInitialized)
+})
+
+QUnit.test('keyringController:submitPassword', function (assert) {
+  var done = assert.async()
+
+  this.keyringController.submitPassword(PASSWORD)
+  .then((state) => {
+    assert.ok(state.identities[FIRST_ADDRESS])
+    assert.equal(state.lostAccounts.length, 1, 'one lost account')
+    assert.equal(state.lostAccounts[0], 'e15D894BeCB0354c501AE69429B05143679F39e0'.toLowerCase())
+    assert.deepEqual(this.configManager.getLostAccounts(), state.lostAccounts, 'persisted')
     done()
   })
 })

test/integration/mocks/badVault.json

@@ -0,0 +1 @@
+{"meta":{"version":4},"data":{"fiatCurrency":"USD","conversionRate":8.34908448,"conversionDate":1481227505,"isConfirmed":true,"wallet":"{\"encSeed\":{\"encStr\":\"Te2KyAGY3S01bgUJ+7d4y3BOvr/8TKrXrkRZ29cGI6dgyedtN+YgTQxElC2td/pzuoXm7KeSfr+yAoFCvMgqFAJwRcX3arHOsMFQie8kp8mL5I65zwdg/HB2QecB4OJHytrxgApv2zZiKEo0kbu2cs8zYIn5wNlCBIHwgylYmHpUDIJcO1B4zg==\",\"nonce\":\"xnxqk4iy70bjt721F+KPLV4PNfBFNyct\"},\"ksData\":{\"m/44'/60'/0'/0\":{\"info\":{\"curve\":\"secp256k1\",\"purpose\":\"sign\"},\"encHdPathPriv\":{\"encStr\":\"vNrSjekRKLmaGFf77Uca9+aAebmDlvrBwtAV8YthpQ4OX/mXtLSycmnLsYdk4schaByfJvrm6/Mf9fxzOSaScJk+XvKw5XqNXedkDHtbWrmNnxFpuT+9tuB8Nupr3D9GZK9PgXhJD99/7Bn6Wk7/ne+PIDmbtdmx/SWmrdo3pg==\",\"nonce\":\"zqWq/gtJ5zfUVRWQQJkP/zoYjer6Rozj\"},\"hdIndex\":1,\"encPrivKeys\":{\"e15d894becb0354c501ae69429b05143679f39e0\":{\"key\":\"jBLQ9v1l5LOEY1C3kI8z7LpbJKHP1vpVfPAlz90MNSfa8Oe+XlxKQAGYs8Zb4fWm\",\"nonce\":\"fJyrSRo1t0RMNqp2MsneoJnYJWHQnSVY\"}},\"addresses\":[\"e15d894becb0354c501ae69429b05143679f39e0\"]}},\"encHdRootPriv\":{\"encStr\":\"mbvwiFBQGbjj4BJLmdeYzfYi8jb7gtFtwiCQOPfvmyz4h2/KMbHNGzumM16qRKpifioQXkhnBulMIQHaYg0Jwv1MoFsqHxHmuIAT+QP5XvJjz0MRl6708pHowmIVG+R8CZNTLqzE7XS8YkZ4ElRpTvLEM8Wngi5Sg287mQMP9w==\",\"nonce\":\"i5Tp2lQe92rXQzNhjZcu9fNNhfux6Wf4\"},\"salt\":\"FQpA8D9R/5qSp9WtQ94FILyfWZHMI6YZw6RmBYqK0N0=\",\"version\":2}","config":{"provider":{"type":"testnet"},"selectedAccount":"0xe15d894becb0354c501ae69429b05143679f39e0"},"isEthConfirmed":true,"transactions":[],"TOSHash":"a4f4e23f823a7ac51783e7ffba7914a911b09acdb97263296b7e14b527f80c5b","gasMultiplier":1}}

test/unit/idStore-migration-test.js

@@ -21,6 +21,10 @@ const mockVault = {
   account: '0x5d8de92c205279c10e5669f797b853ccef4f739a',
 }
 
+const badVault = {
+  seed: 'radar blur cabbage chef fix engine embark joy scheme fiction master release',
+}
+
 describe('IdentityStore to KeyringController migration', function() {
 
   // The stars of the show:
@@ -79,33 +83,9 @@ describe('IdentityStore to KeyringController migration', function() {
       keyringController.configManager.setWallet('something')
       const state = keyringController.getState()
       assert(state.isInitialized, 'old vault counted as initialized.')
+      console.dir(state)
+      assert.equal(state.lostAccounts.length, 0, 'no lost accounts')
     })
-
-    /*
-    it('should use the password to migrate the old vault', function(done) {
-      this.timeout(5000)
-      console.log('calling submitPassword')
-      console.dir(keyringController)
-      keyringController.submitPassword(password, function (err, state) {
-        assert.ifError(err, 'submitPassword threw error')
-
-        function log(str, dat) { console.log(str + ': ' + JSON.stringify(dat)) }
-
-        let newAccounts = keyringController.getAccounts()
-        log('new accounts: ', newAccounts)
-
-        let newAccount = ethUtil.addHexPrefix(newAccounts[0])
-        assert.equal(ethUtil.addHexPrefix(newAccount), mockVault.account, 'restored the correct account')
-        const newSeed = keyringController.keyrings[0].mnemonic
-        log('keyringController keyrings', keyringController.keyrings)
-        assert.equal(newSeed, mockVault.seed, 'seed phrase transferred.')
-
-        assert(configManager.getVault(), 'new type of vault is persisted')
-        done()
-      })
-    })
-    */
-
   })
 })
 

test/unit/keyring-controller-test.js

@@ -95,21 +95,6 @@ describe('KeyringController', function() {
     })
   })
 
-  describe('#migrateOldVaultIfAny', function() {
-    it('should return and init a new vault', function(done) {
-      keyringController.migrateOldVaultIfAny(password)
-      .then(() => {
-        assert(keyringController.configManager.getVault(), 'now has a vault')
-        assert(keyringController.password, 'has a password set')
-        done()
-      })
-      .catch((reason) => {
-        assert.ifError(reason)
-        done()
-      })
-    })
-  })
-
   describe('#createNickname', function() {
     it('should add the address to the identities hash', function() {
       const fakeAddress = '0x12345678'