Merge pull request #4486 from MetaMask/i4475-ClearUnknownIdentities

Clear unknown identities

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ## Current Master
 
+- Fixes issue where old nicknames were kept around causing errors.
+
 ## 4.7.2 Sun Jun 03 2018
 
 - Fix bug preventing users from logging in. Internally accounts and identities were out of sync.

app/scripts/controllers/preferences.js

@@ -1,6 +1,9 @@
 const ObservableStore = require('obs-store')
 const normalizeAddress = require('eth-sig-util').normalize
 const extend = require('xtend')
+const notifier = require('../lib/bug-notifier')
+const log = require('loglevel')
+const { version } = require('../../manifest.json')
 
 class PreferencesController {
 
@@ -28,7 +31,12 @@ class PreferencesController {
       featureFlags: {},
       currentLocale: opts.initLangCode,
       identities: {},
+      lostIdentities: {},
     }, opts.initState)
+
+    this.getFirstTimeInfo = opts.getFirstTimeInfo || null
+    this.notifier = opts.notifier || notifier
+
     this.store = new ObservableStore(initState)
   }
 // PUBLIC METHODS
@@ -98,6 +106,58 @@ class PreferencesController {
     this.store.updateState({ identities })
   }
 
+  /*
+   * Synchronizes identity entries with known accounts.
+   * Removes any unknown identities, and returns the resulting selected address.
+   *
+   * @param {Array<string>} addresses known to the vault.
+   * @returns {Promise<string>} selectedAddress the selected address.
+   */
+  syncAddresses (addresses) {
+    let { identities, lostIdentities } = this.store.getState()
+
+    let newlyLost = {}
+    Object.keys(identities).forEach((identity) => {
+      if (!addresses.includes(identity)) {
+        newlyLost[identity] = identities[identity]
+        delete identities[identity]
+      }
+    })
+
+    // Identities are no longer present.
+    if (Object.keys(newlyLost).length > 0) {
+
+      // Notify our servers:
+      const uri = 'https://diagnostics.metamask.io/v1/orphanedAccounts'
+      const firstTimeInfo = this.getFirstTimeInfo ? this.getFirstTimeInfo() : {}
+      this.notifier.notify(uri, {
+        accounts: Object.keys(newlyLost),
+        metadata: {
+          version,
+          firstTimeInfo,
+        },
+      })
+      .catch(log.error)
+
+      for (let key in newlyLost) {
+        lostIdentities[key] = newlyLost[key]
+      }
+    }
+
+    this.store.updateState({ identities, lostIdentities })
+    this.addAddresses(addresses)
+
+    // If the selected account is no longer valid,
+    // select an arbitrary other account:
+    let selected = this.getSelectedAddress()
+    if (!addresses.includes(selected)) {
+      selected = addresses[0]
+      this.setSelectedAddress(selected)
+    }
+
+    return selected
+  }
+
   /**
    * Setter for the `selectedAddress` property
    *

app/scripts/lib/bug-notifier.js

@@ -0,0 +1,22 @@
+class BugNotifier {
+  notify (uri, message) {
+    return postData(uri, message)
+  }
+}
+
+function postData(uri, data) {
+  return fetch(uri, {
+    body: JSON.stringify(data), // must match 'Content-Type' header
+    credentials: 'same-origin', // include, same-origin, *omit
+    headers: {
+      'content-type': 'application/json',
+    },
+    method: 'POST', // *GET, POST, PUT, DELETE, etc.
+    mode: 'cors', // no-cors, cors, *same-origin
+  })
+}
+
+const notifier = new BugNotifier()
+
+module.exports = notifier
+

app/scripts/metamask-controller.js

@@ -85,6 +85,7 @@ module.exports = class MetamaskController extends EventEmitter {
     this.preferencesController = new PreferencesController({
       initState: initState.PreferencesController,
       initLangCode: opts.initLangCode,
+      getFirstTimeInfo: () => initState.firstTimeInfo,
     })
 
     // currency controller
@@ -356,7 +357,7 @@ module.exports = class MetamaskController extends EventEmitter {
       importAccountWithStrategy: nodeify(this.importAccountWithStrategy, this),
 
       // vault management
-      submitPassword: nodeify(keyringController.submitPassword, keyringController),
+      submitPassword: nodeify(this.submitPassword, this),
 
       // network management
       setProviderType: nodeify(networkController.setProviderType, networkController),
@@ -474,6 +475,22 @@ module.exports = class MetamaskController extends EventEmitter {
     }
   }
 
+  /*
+   * Submits the user's password and attempts to unlock the vault.
+   * Also synchronizes the preferencesController, to ensure its schema
+   * is up to date with known accounts once the vault is decrypted.
+   *
+   * @param {string} password - The user's password
+   * @returns {Promise<object>} - The keyringController update.
+   */
+  async submitPassword (password) {
+    await this.keyringController.submitPassword(password)
+    const accounts = await this.keyringController.getAccounts()
+
+    await this.preferencesController.syncAddresses(accounts)
+    return this.keyringController.fullUpdate()
+  }
+
   /**
    * @type Identity
    * @property {string} name - The account nickname.

test/unit/app/controllers/metamask-controller-test.js

@@ -45,7 +45,7 @@ describe('MetaMaskController', function () {
       encryptor: {
         encrypt: function (password, object) {
           this.object = object
-          return Promise.resolve()
+          return Promise.resolve('mock-encrypted')
         },
         decrypt: function () {
           return Promise.resolve(this.object)
@@ -62,6 +62,36 @@ describe('MetaMaskController', function () {
     sandbox.restore()
   })
 
+  describe('submitPassword', function () {
+    const password = 'password'
+
+    beforeEach(async function () {
+      await metamaskController.createNewVaultAndKeychain(password)
+    })
+
+    it('removes any identities that do not correspond to known accounts.', async function () {
+      const fakeAddress = '0xbad0'
+      metamaskController.preferencesController.addAddresses([fakeAddress])
+      metamaskController.preferencesController.notifier = {
+        notify: async () => {
+          return true
+        },
+      }
+      await metamaskController.submitPassword(password)
+
+      const identities = Object.keys(metamaskController.preferencesController.store.getState().identities)
+      const addresses = await metamaskController.keyringController.getAccounts()
+
+      identities.forEach((identity) => {
+        assert.ok(addresses.includes(identity), `addresses should include all IDs: ${identity}`)
+      })
+
+      addresses.forEach((address) => {
+        assert.ok(identities.includes(address), `identities should include all Addresses: ${address}`)
+      })
+    })
+  })
+
   describe('#getGasPrice', function () {
 
     it('gives the 50th percentile lowest accepted gas price from recentBlocksController', async function () {
@@ -479,7 +509,7 @@ describe('MetaMaskController', function () {
     it('errors when signing a message', async function () {
       await metamaskController.signPersonalMessage(personalMessages[0].msgParams)
       assert.equal(metamaskPersonalMsgs[msgId].status, 'signed')
-      assert.equal(metamaskPersonalMsgs[msgId].rawSig, '0x6a1b65e2b8ed53cf398a769fad24738f9fbe29841fe6854e226953542c4b6a173473cb152b6b1ae5f06d601d45dd699a129b0a8ca84e78b423031db5baa734741b')      
+      assert.equal(metamaskPersonalMsgs[msgId].rawSig, '0x6a1b65e2b8ed53cf398a769fad24738f9fbe29841fe6854e226953542c4b6a173473cb152b6b1ae5f06d601d45dd699a129b0a8ca84e78b423031db5baa734741b')
     })
   })
 
@@ -513,7 +543,7 @@ describe('MetaMaskController', function () {
     })
 
     it('sets up controller dnode api for trusted communication', function (done) {
-      streamTest = createThoughStream((chunk, enc, cb) => { 
+      streamTest = createThoughStream((chunk, enc, cb) => {
         assert.equal(chunk.name, 'controller')
         cb()
         done()