From 6247e54fcc11d4d8d857a977d00eee8012afb92f Mon Sep 17 00:00:00 2001 From: kumavis Date: Tue, 5 Jun 2018 11:15:58 -0700 Subject: [PATCH 1/5] add multivault detection to diagnostics reporting --- app/scripts/metamask-controller.js | 33 ++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index c753fc06f..ad1d6d6a7 100644 --- a/app/scripts/metamask-controller.js +++ b/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 notifier = require('./lib/bug-notifier') const log = require('loglevel') module.exports = class MetamaskController extends EventEmitter { @@ -64,6 +65,9 @@ module.exports = class MetamaskController extends EventEmitter { const initState = opts.initState || {} this.recordFirstTimeInfo(initState) + // metamask diagnostics reporter + this.notifier = opts.notifier || notifier + // platform-specific api this.platform = opts.platform @@ -487,6 +491,35 @@ module.exports = class MetamaskController extends EventEmitter { await this.keyringController.submitPassword(password) const accounts = await this.keyringController.getAccounts() + // verify keyrings + try { + const nonSimpleKeyrings = this.keyringController.keyrings.filter(keyring => keyring.type !== 'Simple Key Pair') + if (nonSimpleKeyrings.length > 1) { + const keyrings = await Promise.all(nonSimpleKeyrings.map(async (keyring, index) => { + return { + index, + type: keyring.type, + accounts: await keyring.getAccounts() + } + })) + // unexpected number of keyrings, report to diagnostics + const uri = 'https://diagnostics.metamask.io/v1/orphanedAccounts' + const firstTimeInfo = this.getFirstTimeInfo ? this.getFirstTimeInfo() : {} + await this.notifier.notify(uri, { + accounts: [], + metadata: { + type: 'keyrings', + keyrings, + version, + firstTimeInfo, + }, + }) + } + } catch (err) { + console.error('Keyring validation error:') + console.error(err) + } + await this.preferencesController.syncAddresses(accounts) return this.keyringController.fullUpdate() } From 20bdba3d1710a070d06c2a395f92d948b9396d47 Mon Sep 17 00:00:00 2001 From: kumavis Date: Tue, 5 Jun 2018 11:51:27 -0700 Subject: [PATCH 2/5] diagnostics - rewrite bug-notifier as diagnostics-reporter --- app/scripts/controllers/preferences.js | 19 ++--- app/scripts/lib/bug-notifier.js | 22 ------ app/scripts/lib/diagnostics-reporter.js | 71 +++++++++++++++++++ app/scripts/metamask-controller.js | 38 +++------- .../controllers/metamask-controller-test.js | 5 -- 5 files changed, 84 insertions(+), 71 deletions(-) delete mode 100644 app/scripts/lib/bug-notifier.js create mode 100644 app/scripts/lib/diagnostics-reporter.js diff --git a/app/scripts/controllers/preferences.js b/app/scripts/controllers/preferences.js index 2fe009f9a..a5d8cc27b 100644 --- a/app/scripts/controllers/preferences.js +++ b/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] } diff --git a/app/scripts/lib/bug-notifier.js b/app/scripts/lib/bug-notifier.js deleted file mode 100644 index 4d305b894..000000000 --- a/app/scripts/lib/bug-notifier.js +++ /dev/null @@ -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 - diff --git a/app/scripts/lib/diagnostics-reporter.js b/app/scripts/lib/diagnostics-reporter.js new file mode 100644 index 000000000..6c77923bd --- /dev/null +++ b/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 { + 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(), + } + })) + 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:') + console.error(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 diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index ad1d6d6a7..4b0b00306 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -46,7 +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 notifier = require('./lib/bug-notifier') +const DiagnosticsReporter = require('./lib/diagnostics-reporter') const log = require('loglevel') module.exports = class MetamaskController extends EventEmitter { @@ -66,7 +66,10 @@ module.exports = class MetamaskController extends EventEmitter { this.recordFirstTimeInfo(initState) // metamask diagnostics reporter - this.notifier = opts.notifier || notifier + this.diagnostics = opts.diagnostics || new DiagnosticsReporter({ + firstTimeInfo: initState.firstTimeInfo, + version, + }) // platform-specific api this.platform = opts.platform @@ -89,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 @@ -492,32 +495,9 @@ module.exports = class MetamaskController extends EventEmitter { const accounts = await this.keyringController.getAccounts() // verify keyrings - try { - const nonSimpleKeyrings = this.keyringController.keyrings.filter(keyring => keyring.type !== 'Simple Key Pair') - if (nonSimpleKeyrings.length > 1) { - const keyrings = await Promise.all(nonSimpleKeyrings.map(async (keyring, index) => { - return { - index, - type: keyring.type, - accounts: await keyring.getAccounts() - } - })) - // unexpected number of keyrings, report to diagnostics - const uri = 'https://diagnostics.metamask.io/v1/orphanedAccounts' - const firstTimeInfo = this.getFirstTimeInfo ? this.getFirstTimeInfo() : {} - await this.notifier.notify(uri, { - accounts: [], - metadata: { - type: 'keyrings', - keyrings, - version, - firstTimeInfo, - }, - }) - } - } catch (err) { - console.error('Keyring validation error:') - console.error(err) + const nonSimpleKeyrings = this.keyringController.keyrings.filter(keyring => keyring.type !== 'Simple Key Pair') + if (nonSimpleKeyrings.length > 1) { + if (this.diagnostics) await this.reportMultipleKeyrings(nonSimpleKeyrings) } await this.preferencesController.syncAddresses(accounts) diff --git a/test/unit/app/controllers/metamask-controller-test.js b/test/unit/app/controllers/metamask-controller-test.js index 7ec98766a..266c3f258 100644 --- a/test/unit/app/controllers/metamask-controller-test.js +++ b/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) From ece5cfc7858cb3e8077fad2c11f165a437e60413 Mon Sep 17 00:00:00 2001 From: kumavis Date: Tue, 5 Jun 2018 11:53:21 -0700 Subject: [PATCH 3/5] lint - fix diagnostics reporter --- app/scripts/lib/diagnostics-reporter.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/scripts/lib/diagnostics-reporter.js b/app/scripts/lib/diagnostics-reporter.js index 6c77923bd..534a0fa8a 100644 --- a/app/scripts/lib/diagnostics-reporter.js +++ b/app/scripts/lib/diagnostics-reporter.js @@ -10,7 +10,7 @@ class DiagnosticsReporter { await this.submit({ accounts: Object.keys(orphans), metadata: { - type: 'orphans' + type: 'orphans', }, }) } catch (err) { From 36a0574f566ba2b9aac53396721c0075dd1107e5 Mon Sep 17 00:00:00 2001 From: kumavis Date: Tue, 5 Jun 2018 12:20:24 -0700 Subject: [PATCH 4/5] diagnostics - minor fixes --- app/scripts/lib/diagnostics-reporter.js | 6 +++--- app/scripts/metamask-controller.js | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/scripts/lib/diagnostics-reporter.js b/app/scripts/lib/diagnostics-reporter.js index 534a0fa8a..aa4ca6e26 100644 --- a/app/scripts/lib/diagnostics-reporter.js +++ b/app/scripts/lib/diagnostics-reporter.js @@ -7,7 +7,7 @@ class DiagnosticsReporter { async reportOrphans(orphans) { try { - await this.submit({ + return await this.submit({ accounts: Object.keys(orphans), metadata: { type: 'orphans', @@ -28,7 +28,7 @@ class DiagnosticsReporter { accounts: await keyring.getAccounts(), } })) - await this.submit({ + return await this.submit({ accounts: [], metadata: { type: 'keyrings', @@ -49,7 +49,7 @@ class DiagnosticsReporter { return await postData(message) } catch (err) { console.error('DiagnosticsReporter - "submit" encountered an error:') - console.error(err) + throw err } } diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 4b0b00306..18a2e7c48 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -496,8 +496,8 @@ module.exports = class MetamaskController extends EventEmitter { // verify keyrings const nonSimpleKeyrings = this.keyringController.keyrings.filter(keyring => keyring.type !== 'Simple Key Pair') - if (nonSimpleKeyrings.length > 1) { - if (this.diagnostics) await this.reportMultipleKeyrings(nonSimpleKeyrings) + if (nonSimpleKeyrings.length > 1 && this.diagnostics) { + await this.reportMultipleKeyrings(nonSimpleKeyrings) } await this.preferencesController.syncAddresses(accounts) From 60e61e6834a4502fcd9d19e1417725c301c79a13 Mon Sep 17 00:00:00 2001 From: kumavis Date: Tue, 5 Jun 2018 12:36:15 -0700 Subject: [PATCH 5/5] diagnostics - fix reportMultipleKeyrings call --- app/scripts/metamask-controller.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 18a2e7c48..1bb0af5ee 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -497,7 +497,7 @@ module.exports = class MetamaskController extends EventEmitter { // verify keyrings const nonSimpleKeyrings = this.keyringController.keyrings.filter(keyring => keyring.type !== 'Simple Key Pair') if (nonSimpleKeyrings.length > 1 && this.diagnostics) { - await this.reportMultipleKeyrings(nonSimpleKeyrings) + await this.diagnostics.reportMultipleKeyrings(nonSimpleKeyrings) } await this.preferencesController.syncAddresses(accounts)