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 ++++++- package.json | 1 + ui/app/actions.js | 4 ++-- 4 files changed, 15 insertions(+), 6 deletions(-) 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), diff --git a/package.json b/package.json index 2c0c30523..595d579f2 100644 --- a/package.json +++ b/package.json @@ -75,6 +75,7 @@ "polyfill-crypto.getrandomvalues": "^1.0.0", "post-message-stream": "^1.0.0", "promise-filter": "^1.1.0", + "promise-to-callback": "^1.0.0", "pumpify": "^1.3.4", "qrcode-npm": "0.0.3", "react": "^15.0.2", diff --git a/ui/app/actions.js b/ui/app/actions.js index 7934a329a..9a68d231a 100644 --- a/ui/app/actions.js +++ b/ui/app/actions.js @@ -270,8 +270,8 @@ function navigateToNewAccountScreen() { } } -function addNewAccount (ringNumber = 0) { - return callBackgroundThenUpdate(background.addNewAccount, ringNumber) +function addNewAccount () { + return callBackgroundThenUpdate(background.addNewAccount) } function showInfoPage () { -- 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(-) 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 de88a49243a54c6dfc3567bf30932f5a01d0c08d Mon Sep 17 00:00:00 2001 From: Frankie Date: Tue, 24 Jan 2017 12:29:20 -0800 Subject: add to CHANGELOG --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e98ce3758..ee8b60184 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Current Master +- Fix issue with HD accounts being generated as loose keys. + ## 3.1.1 2017-1-20 - Fix HD wallet seed export -- cgit From b2623510aec78cbabdcb4bbfaf5c9c406f4e428a Mon Sep 17 00:00:00 2001 From: Frankie Date: Tue, 24 Jan 2017 13:19:26 -0800 Subject: Fix issue where ConfTx view lets you approve txs when the account has insufficient balance --- ui/app/conf-tx.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/app/conf-tx.js b/ui/app/conf-tx.js index a6e03c3ed..d5c26d79f 100644 --- a/ui/app/conf-tx.js +++ b/ui/app/conf-tx.js @@ -136,7 +136,7 @@ ConfirmTxScreen.prototype.checkBalanceAgainstTx = function (txData) { var balance = account ? account.balance : '0x0' var maxCost = new BN(txData.maxCost) - var balanceBn = new BN(ethUtil.stripHexPrefix(balance), 16) + var balanceBn = new BN(ethUtil.stripHexPrefix(balance), 10) return maxCost.gt(balanceBn) } -- cgit From 42bb90341dde5699dca4ba7ad8891f26c1faf4a6 Mon Sep 17 00:00:00 2001 From: Frankie Date: Tue, 24 Jan 2017 13:21:15 -0800 Subject: add to CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fa825e860..cced970cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Current Master +- Fix rendering bug where the Confirm transaction view would lets you approve transactions when the account has insufficient balance. - Add ability to import accounts in JSON file format (used by Mist, Geth, MyEtherWallet, and more!) ## 3.1.1 2017-1-20 -- 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(-) 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(-) 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 +++++--- ui/app/actions.js | 4 ++-- 2 files changed, 7 insertions(+), 5 deletions(-) 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)) diff --git a/ui/app/actions.js b/ui/app/actions.js index 7934a329a..9a68d231a 100644 --- a/ui/app/actions.js +++ b/ui/app/actions.js @@ -270,8 +270,8 @@ function navigateToNewAccountScreen() { } } -function addNewAccount (ringNumber = 0) { - return callBackgroundThenUpdate(background.addNewAccount, ringNumber) +function addNewAccount () { + return callBackgroundThenUpdate(background.addNewAccount) } function showInfoPage () { -- cgit From e2b2e6d5e119ce918d7f383cdd300880e0dce5ce Mon Sep 17 00:00:00 2001 From: kumavis Date: Tue, 24 Jan 2017 14:04:29 -0800 Subject: 3.1.2 --- CHANGELOG.md | 5 +++++ app/manifest.json | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e98ce3758..02ff3969f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ ## Current Master + +## 3.1.2 2017-1-24 + +- Fix "New Account" default keychain + ## 3.1.1 2017-1-20 - Fix HD wallet seed export diff --git a/app/manifest.json b/app/manifest.json index c34b17e72..8662c8030 100644 --- a/app/manifest.json +++ b/app/manifest.json @@ -1,7 +1,7 @@ { "name": "MetaMask", "short_name": "Metamask", - "version": "3.1.1", + "version": "3.1.2", "manifest_version": 2, "author": "https://metamask.io", "description": "Ethereum Browser Extension", -- cgit From 8ed657d5d574389a1b4c44d127ecf37f86cc7e64 Mon Sep 17 00:00:00 2001 From: Frankie Date: Tue, 24 Jan 2017 14:53:45 -0800 Subject: fix base --- ui/app/conf-tx.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ui/app/conf-tx.js b/ui/app/conf-tx.js index d5c26d79f..1bd69f7d9 100644 --- a/ui/app/conf-tx.js +++ b/ui/app/conf-tx.js @@ -134,9 +134,9 @@ ConfirmTxScreen.prototype.checkBalanceAgainstTx = function (txData) { var address = txData.txParams.from || state.selectedAccount var account = state.accounts[address] var balance = account ? account.balance : '0x0' - var maxCost = new BN(txData.maxCost) + var maxCost = new BN(txData.maxCost, 16) - var balanceBn = new BN(ethUtil.stripHexPrefix(balance), 10) + var balanceBn = new BN(ethUtil.stripHexPrefix(balance), 16) return maxCost.gt(balanceBn) } -- 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 ++++++++++- test/unit/keyring-controller-test.js | 3 +++ ui/app/actions.js | 16 +++++++++++++++- 4 files changed, 34 insertions(+), 23 deletions(-) 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), diff --git a/test/unit/keyring-controller-test.js b/test/unit/keyring-controller-test.js index 37fd7175e..d6d2db817 100644 --- a/test/unit/keyring-controller-test.js +++ b/test/unit/keyring-controller-test.js @@ -41,6 +41,9 @@ describe('KeyringController', function() { state = newState done() }) + .catch((err) => { + done(err) + }) }) afterEach(function() { diff --git a/ui/app/actions.js b/ui/app/actions.js index 9a68d231a..5b2ad8a79 100644 --- a/ui/app/actions.js +++ b/ui/app/actions.js @@ -230,7 +230,21 @@ function createNewVaultAndRestore (password, seed) { } function createNewVaultAndKeychain (password) { - return callBackgroundThenUpdate(background.createNewVaultAndKeychain, password) + return (dispatch) => { + dispatch(actions.showLoadingIndication()) + background.createNewVaultAndKeychain(password, (err, newState) => { + if (err) { + return dispatch(actions.displayWarning(err.message)) + } + background.placeSeedWords((err, newState) => { + if (err) { + return dispatch(actions.displayWarning(err.message)) + } + dispatch(actions.hideLoadingIndication()) + dispatch(actions.updateMetamaskState(newState)) + }) + }) + } } function revealSeedConfirmation () { -- cgit