From 8642ced310890c7a3202a2826a2c74fad1fefca3 Mon Sep 17 00:00:00 2001 From: Frankie Date: Tue, 24 Jan 2017 12:06:59 -0800 Subject: Fix issue where generating a new account would put it in loose keys --- app/scripts/keyring-controller.js | 9 ++++++--- app/scripts/metamask-controller.js | 7 ++++++- 2 files changed, 12 insertions(+), 4 deletions(-) (limited to 'app/scripts') diff --git a/app/scripts/keyring-controller.js b/app/scripts/keyring-controller.js index 86c93f5a3..95f0a1d63 100644 --- a/app/scripts/keyring-controller.js +++ b/app/scripts/keyring-controller.js @@ -259,9 +259,8 @@ module.exports = class KeyringController extends EventEmitter { // Calls the `addAccounts` method on the Keyring // in the kryings array at index `keyringNum`, // and then saves those changes. - addNewAccount (keyRingNum = 0) { - const ring = this.keyrings[keyRingNum] - return ring.addAccounts(1) + addNewAccount (selectedKeyring) { + return selectedKeyring.addAccounts(1) .then(this.setupAccounts.bind(this)) .then(this.persistAllKeyrings.bind(this)) .then(this.fullUpdate.bind(this)) @@ -587,6 +586,10 @@ module.exports = class KeyringController extends EventEmitter { return this.keyringTypes.find(kr => kr.type === type) } + getKeyringsByType (type) { + return this.keyrings.filter((keyring) => keyring.type === type) + } + // Get Accounts // returns Promise( @Array[ @string accounts ] ) // diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 629216e42..6b6424f2a 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -1,5 +1,6 @@ const EventEmitter = require('events') const extend = require('xtend') +const promiseToCallback = require('promise-to-callback') const EthStore = require('./lib/eth-store') const MetaMaskProvider = require('web3-provider-engine/zero.js') const KeyringController = require('./keyring-controller') @@ -121,7 +122,11 @@ module.exports = class MetamaskController extends EventEmitter { .then((newState) => { cb(null, newState) }) .catch((reason) => { cb(reason) }) }, - addNewAccount: nodeify(keyringController.addNewAccount).bind(keyringController), + addNewAccount: (cb) => { + const primaryKeyring = keyringController.getKeyringsByType('HD Key Tree')[0] + if (!primaryKeyring) return cb(new Error('MetamaskController - No HD Key Tree found')) + promiseToCallback(keyringController.addNewAccount(primaryKeyring))(cb) + }, setSelectedAccount: nodeify(keyringController.setSelectedAccount).bind(keyringController), saveAccountLabel: nodeify(keyringController.saveAccountLabel).bind(keyringController), exportAccount: nodeify(keyringController.exportAccount).bind(keyringController), -- cgit From 48ffea0142bf68a49f30887b3509786d7e751895 Mon Sep 17 00:00:00 2001 From: Frankie Date: Tue, 24 Jan 2017 12:28:05 -0800 Subject: Move the assumption of their only being one hd keyring when requesting seed words to metamaskController --- app/scripts/keyring-controller.js | 9 +++------ app/scripts/metamask-controller.js | 6 +++++- 2 files changed, 8 insertions(+), 7 deletions(-) (limited to 'app/scripts') diff --git a/app/scripts/keyring-controller.js b/app/scripts/keyring-controller.js index 95f0a1d63..7a46c7737 100644 --- a/app/scripts/keyring-controller.js +++ b/app/scripts/keyring-controller.js @@ -171,11 +171,8 @@ module.exports = class KeyringController extends EventEmitter { // // Used when creating a first vault, to allow confirmation. // Also used when revealing the seed words in the confirmation view. - placeSeedWords () { - const hdKeyrings = this.keyrings.filter((keyring) => keyring.type === 'HD Key Tree') - const firstKeyring = hdKeyrings[0] - if (!firstKeyring) throw new Error('KeyringController - No HD Key Tree found') - return firstKeyring.serialize() + placeSeedWords (selectedKeyring) { + return selectedKeyring.serialize() .then((serialized) => { const seedWords = serialized.mnemonic this.configManager.setSeedWords(seedWords) @@ -436,7 +433,7 @@ module.exports = class KeyringController extends EventEmitter { this.emit('newAccount', hexAccount) return this.setupAccounts(accounts) }).then(() => { - return this.placeSeedWords() + return this.placeSeedWords(this.getKeyringsByType('HD Key Tree')[0]) }) .then(this.persistAllKeyrings.bind(this)) } diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 6b6424f2a..a235be75b 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -107,7 +107,11 @@ module.exports = class MetamaskController extends EventEmitter { // forward directly to keyringController createNewVaultAndKeychain: nodeify(keyringController.createNewVaultAndKeychain).bind(keyringController), createNewVaultAndRestore: nodeify(keyringController.createNewVaultAndRestore).bind(keyringController), - placeSeedWords: nodeify(keyringController.placeSeedWords).bind(keyringController), + placeSeedWords: (cb) => { + const primaryKeyring = keyringController.getKeyringsByType('HD Key Tree')[0] + if (!primaryKeyring) return cb(new Error('MetamaskController - No HD Key Tree found')) + promiseToCallback(keyringController.placeSeedWords(primaryKeyring))(cb) + }, clearSeedWordCache: nodeify(keyringController.clearSeedWordCache).bind(keyringController), setLocked: nodeify(keyringController.setLocked).bind(keyringController), submitPassword: (password, cb) => { -- cgit From 8049c1fc07c248d978ced12746e26e1b3f2eb003 Mon Sep 17 00:00:00 2001 From: kumavis Date: Tue, 24 Jan 2017 13:21:55 -0800 Subject: keyring-controller - cleanup --- app/scripts/keyring-controller.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) (limited to 'app/scripts') diff --git a/app/scripts/keyring-controller.js b/app/scripts/keyring-controller.js index 7a46c7737..858ea19ee 100644 --- a/app/scripts/keyring-controller.js +++ b/app/scripts/keyring-controller.js @@ -423,11 +423,9 @@ module.exports = class KeyringController extends EventEmitter { createFirstKeyTree () { this.clearKeyrings() return this.addNewKeyring('HD Key Tree', {numberOfAccounts: 1}) - .then(() => { - return this.keyrings[0].getAccounts() - }) - .then((accounts) => { - const firstAccount = accounts[0] + .then((keyring) => { + const firstAccount = keyring.getAccounts()[0] + if (!firstAccount) throw new Error('KeyringController - No account found on keychain.') const hexAccount = normalize(firstAccount) this.configManager.setSelectedAccount(hexAccount) this.emit('newAccount', hexAccount) -- cgit From 01c88bb0bd522239c47d7045db39f3575ce460e4 Mon Sep 17 00:00:00 2001 From: kumavis Date: Tue, 24 Jan 2017 13:22:26 -0800 Subject: keyring-controller - cleanup --- app/scripts/keyring-controller.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'app/scripts') diff --git a/app/scripts/keyring-controller.js b/app/scripts/keyring-controller.js index 858ea19ee..5c75ca04a 100644 --- a/app/scripts/keyring-controller.js +++ b/app/scripts/keyring-controller.js @@ -422,9 +422,10 @@ module.exports = class KeyringController extends EventEmitter { // puts the current seed words into the state tree. createFirstKeyTree () { this.clearKeyrings() - return this.addNewKeyring('HD Key Tree', {numberOfAccounts: 1}) + return this.addNewKeyring('HD Key Tree', { numberOfAccounts: 1 }) .then((keyring) => { - const firstAccount = keyring.getAccounts()[0] + const accounts = keyring.getAccounts() + const firstAccount = accounts[0] if (!firstAccount) throw new Error('KeyringController - No account found on keychain.') const hexAccount = normalize(firstAccount) this.configManager.setSelectedAccount(hexAccount) -- cgit From 745fff0c80c49959aad9b0d214aff83cb05c3613 Mon Sep 17 00:00:00 2001 From: kumavis Date: Tue, 24 Jan 2017 13:29:48 -0800 Subject: hotfix - dont assume hd keyring position --- app/scripts/keyring-controller.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'app/scripts') diff --git a/app/scripts/keyring-controller.js b/app/scripts/keyring-controller.js index 86c93f5a3..76422bf6b 100644 --- a/app/scripts/keyring-controller.js +++ b/app/scripts/keyring-controller.js @@ -259,9 +259,11 @@ module.exports = class KeyringController extends EventEmitter { // Calls the `addAccounts` method on the Keyring // in the kryings array at index `keyringNum`, // and then saves those changes. - addNewAccount (keyRingNum = 0) { - const ring = this.keyrings[keyRingNum] - return ring.addAccounts(1) + addNewAccount () { + const hdKeyrings = this.keyrings.filter((keyring) => keyring.type === 'HD Key Tree') + const firstKeyring = hdKeyrings[0] + if (!firstKeyring) throw new Error('KeyringController - No HD Key Tree found') + return firstKeyring.addAccounts(1) .then(this.setupAccounts.bind(this)) .then(this.persistAllKeyrings.bind(this)) .then(this.fullUpdate.bind(this)) -- cgit From 463a56ff54b0d850c86348e260e5f7c17b138ccb Mon Sep 17 00:00:00 2001 From: kumavis Date: Tue, 24 Jan 2017 15:33:33 -0800 Subject: background controller - extract KeyringC.placeSeedWords to MetamaskC --- app/scripts/keyring-controller.js | 27 ++++++--------------------- app/scripts/metamask-controller.js | 11 ++++++++++- 2 files changed, 16 insertions(+), 22 deletions(-) (limited to 'app/scripts') diff --git a/app/scripts/keyring-controller.js b/app/scripts/keyring-controller.js index 5c75ca04a..741757c5a 100644 --- a/app/scripts/keyring-controller.js +++ b/app/scripts/keyring-controller.js @@ -150,12 +150,13 @@ module.exports = class KeyringController extends EventEmitter { mnemonic: seed, numberOfAccounts: 1, }) - }).then(() => { - const firstKeyring = this.keyrings[0] + }) + .then((firstKeyring) => { return firstKeyring.getAccounts() }) .then((accounts) => { const firstAccount = accounts[0] + if (!firstAccount) throw new Error('KeyringController - First Account not found.') const hexAccount = normalize(firstAccount) this.configManager.setSelectedAccount(hexAccount) return this.setupAccounts(accounts) @@ -164,22 +165,6 @@ module.exports = class KeyringController extends EventEmitter { .then(this.fullUpdate.bind(this)) } - // PlaceSeedWords - // returns Promise( @object state ) - // - // Adds the current vault's seed words to the UI's state tree. - // - // Used when creating a first vault, to allow confirmation. - // Also used when revealing the seed words in the confirmation view. - placeSeedWords (selectedKeyring) { - return selectedKeyring.serialize() - .then((serialized) => { - const seedWords = serialized.mnemonic - this.configManager.setSeedWords(seedWords) - return this.fullUpdate() - }) - } - // ClearSeedWordCache // // returns Promise( @string currentSelectedAccount ) @@ -424,15 +409,15 @@ module.exports = class KeyringController extends EventEmitter { this.clearKeyrings() return this.addNewKeyring('HD Key Tree', { numberOfAccounts: 1 }) .then((keyring) => { - const accounts = keyring.getAccounts() + return keyring.getAccounts() + }) + .then((accounts) => { const firstAccount = accounts[0] if (!firstAccount) throw new Error('KeyringController - No account found on keychain.') const hexAccount = normalize(firstAccount) this.configManager.setSelectedAccount(hexAccount) this.emit('newAccount', hexAccount) return this.setupAccounts(accounts) - }).then(() => { - return this.placeSeedWords(this.getKeyringsByType('HD Key Tree')[0]) }) .then(this.persistAllKeyrings.bind(this)) } diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index a235be75b..a1bb9a923 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -107,10 +107,19 @@ module.exports = class MetamaskController extends EventEmitter { // forward directly to keyringController createNewVaultAndKeychain: nodeify(keyringController.createNewVaultAndKeychain).bind(keyringController), createNewVaultAndRestore: nodeify(keyringController.createNewVaultAndRestore).bind(keyringController), + // Adds the current vault's seed words to the UI's state tree. + // + // Used when creating a first vault, to allow confirmation. + // Also used when revealing the seed words in the confirmation view. placeSeedWords: (cb) => { const primaryKeyring = keyringController.getKeyringsByType('HD Key Tree')[0] if (!primaryKeyring) return cb(new Error('MetamaskController - No HD Key Tree found')) - promiseToCallback(keyringController.placeSeedWords(primaryKeyring))(cb) + primaryKeyring.serialize() + .then((serialized) => { + const seedWords = serialized.mnemonic + this.configManager.setSeedWords(seedWords) + promiseToCallback(this.keyringController.fullUpdate())(cb) + }) }, clearSeedWordCache: nodeify(keyringController.clearSeedWordCache).bind(keyringController), setLocked: nodeify(keyringController.setLocked).bind(keyringController), -- cgit