Merge pull request #4511 from MetaMask/v4.7.4

Version 4.7.4

CHANGELOG.md

@@ -2,6 +2,11 @@
 
 ## Current Master
 
+## 4.7.4 Tue Jun 05 2018
+
+- Add diagnostic reporting for users with multiple HD keyrings
+- Throw explicit error when selected account is unset
+
 ## 4.7.3 Mon Jun 04 2018
 
 - Hide token now uses new modal

app/manifest.json

@@ -1,7 +1,7 @@
 {
   "name": "__MSG_appName__",
   "short_name": "__MSG_appName__",
-  "version": "4.7.3",
+  "version": "4.7.4",
   "manifest_version": 2,
   "author": "https://metamask.io",
   "description": "__MSG_appDescription__",

app/scripts/controllers/preferences.js

@@ -1,9 +1,7 @@
 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 {
 
@@ -34,8 +32,7 @@ class PreferencesController {
       lostIdentities: {},
     }, opts.initState)
 
-    this.getFirstTimeInfo = opts.getFirstTimeInfo || null
-    this.notifier = opts.notifier || notifier
+    this.diagnostics = opts.diagnostics
 
     this.store = new ObservableStore(initState)
   }
@@ -128,17 +125,9 @@ class PreferencesController {
     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)
+      if (this.diagnostics) this.diagnostics.reportOrphans(newlyLost)
 
+      // store lost accounts
       for (let key in newlyLost) {
         lostIdentities[key] = newlyLost[key]
       }

app/scripts/lib/bug-notifier.js

@@ -1,22 +0,0 @@
-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/lib/diagnostics-reporter.js

@@ -0,0 +1,71 @@
+class DiagnosticsReporter {
+
+  constructor ({ firstTimeInfo, version }) {
+    this.firstTimeInfo = firstTimeInfo
+    this.version = version
+  }
+
+  async reportOrphans(orphans) {
+    try {
+      return await this.submit({
+        accounts: Object.keys(orphans),
+        metadata: {
+          type: 'orphans',
+        },
+      })
+    } catch (err) {
+      console.error('DiagnosticsReporter - "reportOrphans" encountered an error:')
+      console.error(err)
+    }
+  }
+
+  async reportMultipleKeyrings(rawKeyrings) {
+    try {
+      const keyrings = await Promise.all(rawKeyrings.map(async (keyring, index) => {
+        return {
+          index,
+          type: keyring.type,
+          accounts: await keyring.getAccounts(),
+        }
+      }))
+      return await this.submit({
+        accounts: [],
+        metadata: {
+          type: 'keyrings',
+          keyrings,
+        },
+      })
+    } catch (err) {
+      console.error('DiagnosticsReporter - "reportMultipleKeyrings" encountered an error:')
+      console.error(err)
+    }
+  }
+
+  async submit (message) {
+    try {
+      // add metadata
+      message.metadata.version = this.version
+      message.metadata.firstTimeInfo = this.firstTimeInfo
+      return await postData(message)
+    } catch (err) {
+      console.error('DiagnosticsReporter - "submit" encountered an error:')
+      throw err
+    }
+  }
+
+}
+
+function postData(data) {
+  const uri = 'https://diagnostics.metamask.io/v1/orphanedAccounts'
+  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
+  })
+}
+
+module.exports = DiagnosticsReporter

app/scripts/metamask-controller.js

@@ -46,6 +46,7 @@ const GWEI_BN = new BN('1000000000')
 const percentile = require('percentile')
 const seedPhraseVerifier = require('./lib/seed-phrase-verifier')
 const cleanErrorStack = require('./lib/cleanErrorStack')
+const DiagnosticsReporter = require('./lib/diagnostics-reporter')
 const log = require('loglevel')
 
 module.exports = class MetamaskController extends EventEmitter {
@@ -64,6 +65,12 @@ module.exports = class MetamaskController extends EventEmitter {
     const initState = opts.initState || {}
     this.recordFirstTimeInfo(initState)
 
+    // metamask diagnostics reporter
+    this.diagnostics = opts.diagnostics || new DiagnosticsReporter({
+      firstTimeInfo: initState.firstTimeInfo,
+      version,
+    })
+
     // platform-specific api
     this.platform = opts.platform
 
@@ -85,7 +92,7 @@ module.exports = class MetamaskController extends EventEmitter {
     this.preferencesController = new PreferencesController({
       initState: initState.PreferencesController,
       initLangCode: opts.initLangCode,
-      getFirstTimeInfo: () => initState.firstTimeInfo,
+      diagnostics: this.diagnostics,
     })
 
     // currency controller
@@ -487,6 +494,12 @@ module.exports = class MetamaskController extends EventEmitter {
     await this.keyringController.submitPassword(password)
     const accounts = await this.keyringController.getAccounts()
 
+    // verify keyrings
+    const nonSimpleKeyrings = this.keyringController.keyrings.filter(keyring => keyring.type !== 'Simple Key Pair')
+    if (nonSimpleKeyrings.length > 1 && this.diagnostics) {
+      await this.diagnostics.reportMultipleKeyrings(nonSimpleKeyrings)
+    }
+
     await this.preferencesController.syncAddresses(accounts)
     return this.keyringController.fullUpdate()
   }

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

@@ -72,11 +72,6 @@ describe('MetaMaskController', function () {
     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)

ui/app/components/wallet-view.js

@@ -36,7 +36,6 @@ function mapStateToProps (state) {
     tokens: state.metamask.tokens,
     keyrings: state.metamask.keyrings,
     selectedAddress: selectors.getSelectedAddress(state),
-    selectedIdentity: selectors.getSelectedIdentity(state),
     selectedAccount: selectors.getSelectedAccount(state),
     selectedTokenAddress: state.metamask.selectedTokenAddress,
   }
@@ -99,21 +98,24 @@ WalletView.prototype.render = function () {
   const {
     responsiveDisplayClassname,
     selectedAddress,
-    selectedIdentity,
     keyrings,
     showAccountDetailModal,
     sidebarOpen,
     hideSidebar,
     history,
+    identities,
   } = this.props
   // temporary logs + fake extra wallets
   // console.log('walletview, selectedAccount:', selectedAccount)
 
   const checksummedAddress = checksumAddress(selectedAddress)
 
+  if (!selectedAddress) {
+    throw new Error('selectedAddress should not be ' + String(selectedAddress))
+  }
+
   const keyring = keyrings.find((kr) => {
-    return kr.accounts.includes(selectedAddress) ||
-      kr.accounts.includes(selectedIdentity.address)
+    return kr.accounts.includes(selectedAddress)
   })
 
   const type = keyring.type
@@ -145,7 +147,7 @@ WalletView.prototype.render = function () {
         h('span.account-name', {
           style: {},
         }, [
-          selectedIdentity.name,
+          identities[selectedAddress].name,
         ]),
 
         h('button.btn-clear.wallet-view__details-button.allcaps', this.context.t('details')),