From 797e63b37bc10b2aa3cb78e65024c4a68c099f0b Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Mon, 4 Jun 2018 13:28:46 -0700 Subject: [PATCH 01/17] Add failing test for unknown identity entry --- .../controllers/metamask-controller-test.js | 31 +++++++++++++++++-- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/test/unit/app/controllers/metamask-controller-test.js b/test/unit/app/controllers/metamask-controller-test.js index 4bc16e65e..266c3f258 100644 --- a/test/unit/app/controllers/metamask-controller-test.js +++ b/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,31 @@ 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]) + 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 +504,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 +538,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() From 7382bd0847145b58db8414ae015c41778e5ebb75 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Mon, 4 Jun 2018 13:43:26 -0700 Subject: [PATCH 02/17] Add identity synchronizing code Addresses #4475, where entries in the identities object do not necessarily have corresponding accounts in the vault. On password submission, this change passes known accounts to the preferencesController (responsible for nickname management), and removes unknown entries. Includes "TODO" notes for where we could log the issue to sentry or notify the user. --- app/scripts/controllers/preferences.js | 31 ++++++++++++++++++++++++++ app/scripts/metamask-controller.js | 18 ++++++++++++++- 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/app/scripts/controllers/preferences.js b/app/scripts/controllers/preferences.js index 760868ddf..426ee5a02 100644 --- a/app/scripts/controllers/preferences.js +++ b/app/scripts/controllers/preferences.js @@ -98,6 +98,37 @@ class PreferencesController { this.store.updateState({ identities }) } + /* + * Synchronizes identity entries with known accounts. + * Removes any unknown identities, and returns the resulting selected address. + * + * @param {Array} addresses known to the vault. + * @returns {Promise} selectedAddress the selected address. + */ + syncAddresses (addresses) { + const identities = this.store.getState().identities + + Object.keys(identities).forEach((identity) => { + if (!addresses.includes(identity)) { + delete identities[identity] + + // TODO: Report the bug to Sentry including the now-lost identity. + // TODO: Inform the user of the lost identity. + } + }) + + this.store.updateState({ identities }) + this.addAddresses(addresses) + + let selected = this.getSelectedAddress() + if (!addresses.includes(selected)) { + selected = addresses[0] + this.setSelectedAddress(selected) + } + + return selected + } + /** * Setter for the `selectedAddress` property * diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 96f976568..85c1fe09c 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -356,7 +356,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 +474,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} - 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. From f5d4acf53b2d518df1b2c0b9b983bbc5224fb670 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Mon, 4 Jun 2018 14:01:05 -0700 Subject: [PATCH 03/17] Add minimal user notification of issue. --- app/scripts/controllers/preferences.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/scripts/controllers/preferences.js b/app/scripts/controllers/preferences.js index 426ee5a02..8cb846476 100644 --- a/app/scripts/controllers/preferences.js +++ b/app/scripts/controllers/preferences.js @@ -113,7 +113,7 @@ class PreferencesController { delete identities[identity] // TODO: Report the bug to Sentry including the now-lost identity. - // TODO: Inform the user of the lost identity. + alert('Error 4486: MetaMask has encountered a very strange error. Please open a support issue immediately at support@metamask.io.') } }) From 8fcaa2cf56936388ef8dfc528ecbd2354adb201e Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Mon, 4 Jun 2018 14:05:56 -0700 Subject: [PATCH 04/17] Persist lost identities to storage for later analysis --- app/scripts/controllers/preferences.js | 6 ++++-- app/scripts/lib/4486-notifier.js | 29 ++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) create mode 100644 app/scripts/lib/4486-notifier.js diff --git a/app/scripts/controllers/preferences.js b/app/scripts/controllers/preferences.js index 8cb846476..38e93dea8 100644 --- a/app/scripts/controllers/preferences.js +++ b/app/scripts/controllers/preferences.js @@ -28,6 +28,7 @@ class PreferencesController { featureFlags: {}, currentLocale: opts.initLangCode, identities: {}, + lostIdentities: {}, }, opts.initState) this.store = new ObservableStore(initState) } @@ -106,18 +107,19 @@ class PreferencesController { * @returns {Promise} selectedAddress the selected address. */ syncAddresses (addresses) { - const identities = this.store.getState().identities + let { identities, lostIdentities } = this.store.getState() Object.keys(identities).forEach((identity) => { if (!addresses.includes(identity)) { delete identities[identity] + lostIdentities[identity] = identities[identity] // TODO: Report the bug to Sentry including the now-lost identity. alert('Error 4486: MetaMask has encountered a very strange error. Please open a support issue immediately at support@metamask.io.') } }) - this.store.updateState({ identities }) + this.store.updateState({ identities, lostIdentities }) this.addAddresses(addresses) let selected = this.getSelectedAddress() diff --git a/app/scripts/lib/4486-notifier.js b/app/scripts/lib/4486-notifier.js new file mode 100644 index 000000000..b1b153419 --- /dev/null +++ b/app/scripts/lib/4486-notifier.js @@ -0,0 +1,29 @@ +class BugNotifier { + notify (message) { + + postData('http://example.com/answer', {answer: 42}) + .then(data => console.log(data)) // JSON from `response.json()` call + .catch(error => console.error(error)) + } +} + +function postData(url, data) { + // Default options are marked with * + return fetch(url, { + body: JSON.stringify(data), // must match 'Content-Type' header + cache: 'no-cache', // *default, no-cache, reload, force-cache, only-if-cached + credentials: 'same-origin', // include, same-origin, *omit + headers: { + 'user-agent': 'Mozilla/4.0 MDN Example', + 'content-type': 'application/json' + }, + method: 'POST', // *GET, POST, PUT, DELETE, etc. + mode: 'cors', // no-cors, cors, *same-origin + redirect: 'follow', // manual, *follow, error + referrer: 'no-referrer', // *client, no-referrer + }) + .then(response => response.json()) // parses response to JSON +} + +module.exports = BugNotifier + From fd1ce4d741cc5992cc5d3d6109dc46cddf871ec2 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Mon, 4 Jun 2018 14:21:46 -0700 Subject: [PATCH 05/17] Begin adding unconfigured notifier --- app/scripts/controllers/preferences.js | 21 +++++++++++++++---- .../lib/{4486-notifier.js => bug-notifier.js} | 13 ++++-------- 2 files changed, 21 insertions(+), 13 deletions(-) rename app/scripts/lib/{4486-notifier.js => bug-notifier.js} (59%) diff --git a/app/scripts/controllers/preferences.js b/app/scripts/controllers/preferences.js index 38e93dea8..f822f61c5 100644 --- a/app/scripts/controllers/preferences.js +++ b/app/scripts/controllers/preferences.js @@ -1,6 +1,8 @@ const ObservableStore = require('obs-store') const normalizeAddress = require('eth-sig-util').normalize const extend = require('xtend') +const BugNotifier = require('../lib/bug-notifier') +const notifier = new BugNotifier() class PreferencesController { @@ -30,6 +32,7 @@ class PreferencesController { identities: {}, lostIdentities: {}, }, opts.initState) + this.store = new ObservableStore(initState) } // PUBLIC METHODS @@ -108,17 +111,27 @@ class PreferencesController { */ syncAddresses (addresses) { let { identities, lostIdentities } = this.store.getState() - Object.keys(identities).forEach((identity) => { if (!addresses.includes(identity)) { delete identities[identity] lostIdentities[identity] = identities[identity] - - // TODO: Report the bug to Sentry including the now-lost identity. - alert('Error 4486: MetaMask has encountered a very strange error. Please open a support issue immediately at support@metamask.io.') } }) + // Identities are no longer present. + if (Object.keys(lostIdentities).length > 0) { + + // timeout to prevent blocking the thread: + setTimeout(() => { + alert('Error 4486: MetaMask has encountered a very strange error. Please open a support issue immediately at support@metamask.io.') + }, 10) + + // Notify our servers: + const uri = + notifier.notify(uri, { accounts: Object.keys(lostIdentities) }) + .catch(log.error) + } + this.store.updateState({ identities, lostIdentities }) this.addAddresses(addresses) diff --git a/app/scripts/lib/4486-notifier.js b/app/scripts/lib/bug-notifier.js similarity index 59% rename from app/scripts/lib/4486-notifier.js rename to app/scripts/lib/bug-notifier.js index b1b153419..d6a2ed2c9 100644 --- a/app/scripts/lib/4486-notifier.js +++ b/app/scripts/lib/bug-notifier.js @@ -1,26 +1,21 @@ class BugNotifier { - notify (message) { - - postData('http://example.com/answer', {answer: 42}) + notify (uri, message) { + return postData(uri, message) .then(data => console.log(data)) // JSON from `response.json()` call .catch(error => console.error(error)) } } -function postData(url, data) { - // Default options are marked with * +function postData(uri, data) { + return fetch(url, { body: JSON.stringify(data), // must match 'Content-Type' header - cache: 'no-cache', // *default, no-cache, reload, force-cache, only-if-cached credentials: 'same-origin', // include, same-origin, *omit headers: { - 'user-agent': 'Mozilla/4.0 MDN Example', 'content-type': 'application/json' }, method: 'POST', // *GET, POST, PUT, DELETE, etc. mode: 'cors', // no-cors, cors, *same-origin - redirect: 'follow', // manual, *follow, error - referrer: 'no-referrer', // *client, no-referrer }) .then(response => response.json()) // parses response to JSON } From f3b385cb093cbd9706643c0feae2702adfcc11da Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Mon, 4 Jun 2018 14:22:34 -0700 Subject: [PATCH 06/17] Add reporting uri --- app/scripts/controllers/preferences.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/scripts/controllers/preferences.js b/app/scripts/controllers/preferences.js index f822f61c5..cff70272f 100644 --- a/app/scripts/controllers/preferences.js +++ b/app/scripts/controllers/preferences.js @@ -127,7 +127,7 @@ class PreferencesController { }, 10) // Notify our servers: - const uri = + const uri = 'https://diagnostics.metamask.io/v1/orphanedAccounts' notifier.notify(uri, { accounts: Object.keys(lostIdentities) }) .catch(log.error) } From b858cc4b1bf13c7c813fe48bb0b12f9eb3cc4a0d Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Mon, 4 Jun 2018 14:24:45 -0700 Subject: [PATCH 07/17] Only notify first time lost ids are detected --- app/scripts/controllers/preferences.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/app/scripts/controllers/preferences.js b/app/scripts/controllers/preferences.js index cff70272f..038bab37b 100644 --- a/app/scripts/controllers/preferences.js +++ b/app/scripts/controllers/preferences.js @@ -111,15 +111,17 @@ class PreferencesController { */ syncAddresses (addresses) { let { identities, lostIdentities } = this.store.getState() + + let newlyLost = {} Object.keys(identities).forEach((identity) => { if (!addresses.includes(identity)) { delete identities[identity] - lostIdentities[identity] = identities[identity] + newlyLost[identity] = identities[identity] } }) // Identities are no longer present. - if (Object.keys(lostIdentities).length > 0) { + if (Object.keys(newlyLost).length > 0) { // timeout to prevent blocking the thread: setTimeout(() => { @@ -130,6 +132,10 @@ class PreferencesController { const uri = 'https://diagnostics.metamask.io/v1/orphanedAccounts' notifier.notify(uri, { accounts: Object.keys(lostIdentities) }) .catch(log.error) + + for (let key in newlyLost) { + lostIdentities[key] = newlyLost[key] + } } this.store.updateState({ identities, lostIdentities }) From d07c664b2c31469d84dd3c84a929b8d4ba552e8c Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Mon, 4 Jun 2018 14:30:24 -0700 Subject: [PATCH 08/17] Fine tune error posting --- app/scripts/controllers/preferences.js | 2 +- app/scripts/lib/bug-notifier.js | 6 +----- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/app/scripts/controllers/preferences.js b/app/scripts/controllers/preferences.js index 038bab37b..18254af4b 100644 --- a/app/scripts/controllers/preferences.js +++ b/app/scripts/controllers/preferences.js @@ -130,7 +130,7 @@ class PreferencesController { // Notify our servers: const uri = 'https://diagnostics.metamask.io/v1/orphanedAccounts' - notifier.notify(uri, { accounts: Object.keys(lostIdentities) }) + notifier.notify(uri, { accounts: Object.keys(newlyLost) }) .catch(log.error) for (let key in newlyLost) { diff --git a/app/scripts/lib/bug-notifier.js b/app/scripts/lib/bug-notifier.js index d6a2ed2c9..42f943485 100644 --- a/app/scripts/lib/bug-notifier.js +++ b/app/scripts/lib/bug-notifier.js @@ -1,14 +1,11 @@ class BugNotifier { notify (uri, message) { return postData(uri, message) - .then(data => console.log(data)) // JSON from `response.json()` call - .catch(error => console.error(error)) } } function postData(uri, data) { - - return fetch(url, { + return fetch(uri, { body: JSON.stringify(data), // must match 'Content-Type' header credentials: 'same-origin', // include, same-origin, *omit headers: { @@ -17,7 +14,6 @@ function postData(uri, data) { method: 'POST', // *GET, POST, PUT, DELETE, etc. mode: 'cors', // no-cors, cors, *same-origin }) - .then(response => response.json()) // parses response to JSON } module.exports = BugNotifier From cd1e77c0f60cbc4832ff8adf184564fee4fb991b Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Mon, 4 Jun 2018 14:56:50 -0700 Subject: [PATCH 09/17] Add changelog entry --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1cf23ccbe..f4c2abd4d 100644 --- a/CHANGELOG.md +++ b/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. From 3bfc40c2848d3e814fca663fa039c261097976c3 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Mon, 4 Jun 2018 14:59:46 -0700 Subject: [PATCH 10/17] Add version to report --- app/scripts/controllers/preferences.js | 6 +++--- app/scripts/lib/bug-notifier.js | 4 +++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/app/scripts/controllers/preferences.js b/app/scripts/controllers/preferences.js index 18254af4b..39ca16f28 100644 --- a/app/scripts/controllers/preferences.js +++ b/app/scripts/controllers/preferences.js @@ -1,8 +1,8 @@ const ObservableStore = require('obs-store') const normalizeAddress = require('eth-sig-util').normalize const extend = require('xtend') -const BugNotifier = require('../lib/bug-notifier') -const notifier = new BugNotifier() +const notifier = require('../lib/bug-notifier') +const { version } = require('../../manifest.json') class PreferencesController { @@ -130,7 +130,7 @@ class PreferencesController { // Notify our servers: const uri = 'https://diagnostics.metamask.io/v1/orphanedAccounts' - notifier.notify(uri, { accounts: Object.keys(newlyLost) }) + notifier.notify(uri, { accounts: Object.keys(newlyLost), version }) .catch(log.error) for (let key in newlyLost) { diff --git a/app/scripts/lib/bug-notifier.js b/app/scripts/lib/bug-notifier.js index 42f943485..bfb3e9770 100644 --- a/app/scripts/lib/bug-notifier.js +++ b/app/scripts/lib/bug-notifier.js @@ -16,5 +16,7 @@ function postData(uri, data) { }) } -module.exports = BugNotifier +const notifier = new BugNotifier() + +module.exports = notifier From 0eacee8e45a3f9c3cbedf99ac79d0c37a8a9f87f Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Mon, 4 Jun 2018 15:03:31 -0700 Subject: [PATCH 11/17] Add first time info to bug report --- app/scripts/controllers/preferences.js | 5 ++++- app/scripts/metamask-controller.js | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/app/scripts/controllers/preferences.js b/app/scripts/controllers/preferences.js index 39ca16f28..70fbd1224 100644 --- a/app/scripts/controllers/preferences.js +++ b/app/scripts/controllers/preferences.js @@ -33,6 +33,8 @@ class PreferencesController { lostIdentities: {}, }, opts.initState) + this.getFirstTimeInfo = opts.getFirstTimeInfo || null + this.store = new ObservableStore(initState) } // PUBLIC METHODS @@ -130,7 +132,8 @@ class PreferencesController { // Notify our servers: const uri = 'https://diagnostics.metamask.io/v1/orphanedAccounts' - notifier.notify(uri, { accounts: Object.keys(newlyLost), version }) + const firstTimeInfo = this.getFirstTimeInfo ? this.getFirstTimeInfo() : {} + notifier.notify(uri, { accounts: Object.keys(newlyLost), version, firstTimeInfo }) .catch(log.error) for (let key in newlyLost) { diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 85c1fe09c..c753fc06f 100644 --- a/app/scripts/metamask-controller.js +++ b/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 From 7b87afb4b72b36a581912365501295b685007869 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Mon, 4 Jun 2018 15:06:21 -0700 Subject: [PATCH 12/17] Add bug info under metadata key --- app/scripts/controllers/preferences.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/app/scripts/controllers/preferences.js b/app/scripts/controllers/preferences.js index 70fbd1224..0bfb3b5a3 100644 --- a/app/scripts/controllers/preferences.js +++ b/app/scripts/controllers/preferences.js @@ -133,7 +133,13 @@ class PreferencesController { // Notify our servers: const uri = 'https://diagnostics.metamask.io/v1/orphanedAccounts' const firstTimeInfo = this.getFirstTimeInfo ? this.getFirstTimeInfo() : {} - notifier.notify(uri, { accounts: Object.keys(newlyLost), version, firstTimeInfo }) + notifier.notify(uri, { + accounts: Object.keys(newlyLost), + metadata: { + version, + firstTimeInfo, + }, + }) .catch(log.error) for (let key in newlyLost) { From 22754e3e1f496b2dde140eea56a88bc7237fda42 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Mon, 4 Jun 2018 15:10:51 -0700 Subject: [PATCH 13/17] Linted --- app/scripts/controllers/preferences.js | 1 + app/scripts/lib/bug-notifier.js | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/app/scripts/controllers/preferences.js b/app/scripts/controllers/preferences.js index 0bfb3b5a3..b5171214f 100644 --- a/app/scripts/controllers/preferences.js +++ b/app/scripts/controllers/preferences.js @@ -2,6 +2,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 { diff --git a/app/scripts/lib/bug-notifier.js b/app/scripts/lib/bug-notifier.js index bfb3e9770..4d305b894 100644 --- a/app/scripts/lib/bug-notifier.js +++ b/app/scripts/lib/bug-notifier.js @@ -9,7 +9,7 @@ function postData(uri, data) { body: JSON.stringify(data), // must match 'Content-Type' header credentials: 'same-origin', // include, same-origin, *omit headers: { - 'content-type': 'application/json' + 'content-type': 'application/json', }, method: 'POST', // *GET, POST, PUT, DELETE, etc. mode: 'cors', // no-cors, cors, *same-origin From 415ab2d5349903b3cc330b1fdc19afb737eca838 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Mon, 4 Jun 2018 15:17:03 -0700 Subject: [PATCH 14/17] Do not alert to user --- app/scripts/controllers/preferences.js | 5 ----- 1 file changed, 5 deletions(-) diff --git a/app/scripts/controllers/preferences.js b/app/scripts/controllers/preferences.js index b5171214f..30e00f298 100644 --- a/app/scripts/controllers/preferences.js +++ b/app/scripts/controllers/preferences.js @@ -126,11 +126,6 @@ class PreferencesController { // Identities are no longer present. if (Object.keys(newlyLost).length > 0) { - // timeout to prevent blocking the thread: - setTimeout(() => { - alert('Error 4486: MetaMask has encountered a very strange error. Please open a support issue immediately at support@metamask.io.') - }, 10) - // Notify our servers: const uri = 'https://diagnostics.metamask.io/v1/orphanedAccounts' const firstTimeInfo = this.getFirstTimeInfo ? this.getFirstTimeInfo() : {} From f07ca73e07c15e338e2f2d2ab794fa1c95619056 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Mon, 4 Jun 2018 15:18:12 -0700 Subject: [PATCH 15/17] Add comment --- app/scripts/controllers/preferences.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/scripts/controllers/preferences.js b/app/scripts/controllers/preferences.js index 30e00f298..b63dd5fcc 100644 --- a/app/scripts/controllers/preferences.js +++ b/app/scripts/controllers/preferences.js @@ -146,6 +146,8 @@ class PreferencesController { 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] From ae156e10872faae3040540a7f440af5882a79ec2 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Mon, 4 Jun 2018 15:26:01 -0700 Subject: [PATCH 16/17] Mock notifier in test --- app/scripts/controllers/preferences.js | 3 ++- test/unit/app/controllers/metamask-controller-test.js | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/app/scripts/controllers/preferences.js b/app/scripts/controllers/preferences.js index b63dd5fcc..942546528 100644 --- a/app/scripts/controllers/preferences.js +++ b/app/scripts/controllers/preferences.js @@ -35,6 +35,7 @@ class PreferencesController { }, opts.initState) this.getFirstTimeInfo = opts.getFirstTimeInfo || null + this.notifier = opts.notifier || notifier this.store = new ObservableStore(initState) } @@ -129,7 +130,7 @@ class PreferencesController { // Notify our servers: const uri = 'https://diagnostics.metamask.io/v1/orphanedAccounts' const firstTimeInfo = this.getFirstTimeInfo ? this.getFirstTimeInfo() : {} - notifier.notify(uri, { + this.notifier.notify(uri, { accounts: Object.keys(newlyLost), metadata: { version, diff --git a/test/unit/app/controllers/metamask-controller-test.js b/test/unit/app/controllers/metamask-controller-test.js index 266c3f258..7ec98766a 100644 --- a/test/unit/app/controllers/metamask-controller-test.js +++ b/test/unit/app/controllers/metamask-controller-test.js @@ -72,6 +72,11 @@ 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) From 41f292437dd6c7144e36efa8609a419c7dd88da7 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Mon, 4 Jun 2018 15:34:38 -0700 Subject: [PATCH 17/17] Record identity before deleting it --- app/scripts/controllers/preferences.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/scripts/controllers/preferences.js b/app/scripts/controllers/preferences.js index 942546528..2fe009f9a 100644 --- a/app/scripts/controllers/preferences.js +++ b/app/scripts/controllers/preferences.js @@ -119,8 +119,8 @@ class PreferencesController { let newlyLost = {} Object.keys(identities).forEach((identity) => { if (!addresses.includes(identity)) { - delete identities[identity] newlyLost[identity] = identities[identity] + delete identities[identity] } })