From 96ebbde634c5f89793f72a2316a0c9aa2a38bc4b Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 27 Sep 2017 14:43:23 -0700 Subject: [PATCH 1/5] Fix Account Selection Do not select accounts on restore, only on creation and deliberate selection. Fixes #2164 --- app/scripts/controllers/preferences.js | 2 +- app/scripts/keyring-controller.js | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/app/scripts/controllers/preferences.js b/app/scripts/controllers/preferences.js index e45224593..bc4848421 100644 --- a/app/scripts/controllers/preferences.js +++ b/app/scripts/controllers/preferences.js @@ -22,7 +22,7 @@ class PreferencesController { }) } - getSelectedAddress (_address) { + getSelectedAddress () { return this.store.getState().selectedAddress } diff --git a/app/scripts/keyring-controller.js b/app/scripts/keyring-controller.js index 1a1904621..b5c51fd03 100644 --- a/app/scripts/keyring-controller.js +++ b/app/scripts/keyring-controller.js @@ -107,7 +107,6 @@ class KeyringController extends EventEmitter { const firstAccount = accounts[0] if (!firstAccount) throw new Error('KeyringController - First Account not found.') const hexAccount = normalizeAddress(firstAccount) - this.emit('newAccount', hexAccount) return this.setupAccounts(accounts) }) .then(this.persistAllKeyrings.bind(this, password)) @@ -207,6 +206,12 @@ class KeyringController extends EventEmitter { // and then saves those changes. addNewAccount (selectedKeyring) { return selectedKeyring.addAccounts(1) + .then((accounts) => { + accounts.forEach((hexAccount) => { + this.emit('newAccount', hexAccount) + }) + return accounts + }) .then(this.setupAccounts.bind(this)) .then(this.persistAllKeyrings.bind(this)) .then(this._updateMemStoreKeyrings.bind(this)) @@ -325,7 +330,6 @@ class KeyringController extends EventEmitter { const firstAccount = accounts[0] if (!firstAccount) throw new Error('KeyringController - No account found on keychain.') const hexAccount = normalizeAddress(firstAccount) - this.emit('newAccount', hexAccount) this.emit('newVault', hexAccount) return this.setupAccounts(accounts) }) From ea12be2c1bc488cfa5f85ced5472ec1515ddb00f Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 27 Sep 2017 14:44:44 -0700 Subject: [PATCH 2/5] Bump changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ad9888fd..5b54ed1a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ ## Current Master +- Fix bug where newly created accounts were not selected. +- Fix bug where selected account was not persisted between lockings. + ## 3.10.5 2017-9-27 - Fix block gas limit estimation. From aefd17ef9418148c1a35dbf72da7991366649f5c Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 27 Sep 2017 14:45:24 -0700 Subject: [PATCH 3/5] Remove dead reference --- app/scripts/keyring-controller.js | 1 - 1 file changed, 1 deletion(-) diff --git a/app/scripts/keyring-controller.js b/app/scripts/keyring-controller.js index b5c51fd03..4627b850e 100644 --- a/app/scripts/keyring-controller.js +++ b/app/scripts/keyring-controller.js @@ -106,7 +106,6 @@ class KeyringController extends EventEmitter { .then((accounts) => { const firstAccount = accounts[0] if (!firstAccount) throw new Error('KeyringController - First Account not found.') - const hexAccount = normalizeAddress(firstAccount) return this.setupAccounts(accounts) }) .then(this.persistAllKeyrings.bind(this, password)) From a246770866d242404ce275517c9223fc922e4312 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 27 Sep 2017 14:55:34 -0700 Subject: [PATCH 4/5] Commit to the eth-keyring-controller module --- app/scripts/keyring-controller.js | 599 ------------------------------ package.json | 2 +- 2 files changed, 1 insertion(+), 600 deletions(-) delete mode 100644 app/scripts/keyring-controller.js diff --git a/app/scripts/keyring-controller.js b/app/scripts/keyring-controller.js deleted file mode 100644 index 4627b850e..000000000 --- a/app/scripts/keyring-controller.js +++ /dev/null @@ -1,599 +0,0 @@ -const ethUtil = require('ethereumjs-util') -const BN = ethUtil.BN -const bip39 = require('bip39') -const EventEmitter = require('events').EventEmitter -const ObservableStore = require('obs-store') -const filter = require('promise-filter') -const encryptor = require('browser-passworder') -const sigUtil = require('eth-sig-util') -const normalizeAddress = sigUtil.normalize -// Keyrings: -const SimpleKeyring = require('eth-simple-keyring') -const HdKeyring = require('eth-hd-keyring') -const keyringTypes = [ - SimpleKeyring, - HdKeyring, -] - -class KeyringController extends EventEmitter { - - // PUBLIC METHODS - // - // THE FIRST SECTION OF METHODS ARE PUBLIC-FACING, - // MEANING THEY ARE USED BY CONSUMERS OF THIS CLASS. - // - // THEIR SURFACE AREA SHOULD BE CHANGED WITH GREAT CARE. - - constructor (opts) { - super() - const initState = opts.initState || {} - this.keyringTypes = keyringTypes - this.store = new ObservableStore(initState) - this.memStore = new ObservableStore({ - isUnlocked: false, - keyringTypes: this.keyringTypes.map(krt => krt.type), - keyrings: [], - identities: {}, - }) - - this.accountTracker = opts.accountTracker - this.encryptor = opts.encryptor || encryptor - this.keyrings = [] - this.getNetwork = opts.getNetwork - } - - // Full Update - // returns Promise( @object state ) - // - // Emits the `update` event and - // returns a Promise that resolves to the current state. - // - // Frequently used to end asynchronous chains in this class, - // indicating consumers can often either listen for updates, - // or accept a state-resolving promise to consume their results. - // - // Not all methods end with this, that might be a nice refactor. - fullUpdate () { - this.emit('update') - return Promise.resolve(this.memStore.getState()) - } - - // Create New Vault And Keychain - // @string password - The password to encrypt the vault with - // - // returns Promise( @object state ) - // - // Destroys any old encrypted storage, - // creates a new encrypted store with the given password, - // randomly creates a new HD wallet with 1 account, - // faucets that account on the testnet. - createNewVaultAndKeychain (password) { - return this.persistAllKeyrings(password) - .then(this.createFirstKeyTree.bind(this)) - .then(this.fullUpdate.bind(this)) - } - - // CreateNewVaultAndRestore - // @string password - The password to encrypt the vault with - // @string seed - The BIP44-compliant seed phrase. - // - // returns Promise( @object state ) - // - // Destroys any old encrypted storage, - // creates a new encrypted store with the given password, - // creates a new HD wallet from the given seed with 1 account. - createNewVaultAndRestore (password, seed) { - if (typeof password !== 'string') { - return Promise.reject('Password must be text.') - } - - if (!bip39.validateMnemonic(seed)) { - return Promise.reject(new Error('Seed phrase is invalid.')) - } - - this.clearKeyrings() - - return this.persistAllKeyrings(password) - .then(() => { - return this.addNewKeyring('HD Key Tree', { - mnemonic: seed, - numberOfAccounts: 1, - }) - }) - .then((firstKeyring) => { - return firstKeyring.getAccounts() - }) - .then((accounts) => { - const firstAccount = accounts[0] - if (!firstAccount) throw new Error('KeyringController - First Account not found.') - return this.setupAccounts(accounts) - }) - .then(this.persistAllKeyrings.bind(this, password)) - .then(this.fullUpdate.bind(this)) - } - - // Set Locked - // returns Promise( @object state ) - // - // This method deallocates all secrets, and effectively locks metamask. - setLocked () { - // set locked - this.password = null - this.memStore.updateState({ isUnlocked: false }) - // remove keyrings - this.keyrings = [] - this._updateMemStoreKeyrings() - return this.fullUpdate() - } - - // Submit Password - // @string password - // - // returns Promise( @object state ) - // - // Attempts to decrypt the current vault and load its keyrings - // into memory. - // - // Temporarily also migrates any old-style vaults first, as well. - // (Pre MetaMask 3.0.0) - submitPassword (password) { - return this.unlockKeyrings(password) - .then((keyrings) => { - this.keyrings = keyrings - return this.fullUpdate() - }) - } - - // Add New Keyring - // @string type - // @object opts - // - // returns Promise( @Keyring keyring ) - // - // Adds a new Keyring of the given `type` to the vault - // and the current decrypted Keyrings array. - // - // All Keyring classes implement a unique `type` string, - // and this is used to retrieve them from the keyringTypes array. - addNewKeyring (type, opts) { - const Keyring = this.getKeyringClassForType(type) - const keyring = new Keyring(opts) - return keyring.deserialize(opts) - .then(() => { - return keyring.getAccounts() - }) - .then((accounts) => { - return this.checkForDuplicate(type, accounts) - }) - .then((checkedAccounts) => { - this.keyrings.push(keyring) - return this.setupAccounts(checkedAccounts) - }) - .then(() => this.persistAllKeyrings()) - .then(() => this._updateMemStoreKeyrings()) - .then(() => this.fullUpdate()) - .then(() => { - return keyring - }) - } - - // For now just checks for simple key pairs - // but in the future - // should possibly add HD and other types - // - checkForDuplicate (type, newAccount) { - return this.getAccounts() - .then((accounts) => { - switch (type) { - case 'Simple Key Pair': - const isNotIncluded = !accounts.find((key) => key === newAccount[0] || key === ethUtil.stripHexPrefix(newAccount[0])) - return (isNotIncluded) ? Promise.resolve(newAccount) : Promise.reject(new Error('The account you\'re are trying to import is a duplicate')) - default: - return Promise.resolve(newAccount) - } - }) - } - - - // Add New Account - // @number keyRingNum - // - // returns Promise( @object state ) - // - // Calls the `addAccounts` method on the Keyring - // in the kryings array at index `keyringNum`, - // and then saves those changes. - addNewAccount (selectedKeyring) { - return selectedKeyring.addAccounts(1) - .then((accounts) => { - accounts.forEach((hexAccount) => { - this.emit('newAccount', hexAccount) - }) - return accounts - }) - .then(this.setupAccounts.bind(this)) - .then(this.persistAllKeyrings.bind(this)) - .then(this._updateMemStoreKeyrings.bind(this)) - .then(this.fullUpdate.bind(this)) - } - - // Save Account Label - // @string account - // @string label - // - // returns Promise( @string label ) - // - // Persists a nickname equal to `label` for the specified account. - saveAccountLabel (account, label) { - try { - const hexAddress = normalizeAddress(account) - // update state on diskStore - const state = this.store.getState() - const walletNicknames = state.walletNicknames || {} - walletNicknames[hexAddress] = label - this.store.updateState({ walletNicknames }) - // update state on memStore - const identities = this.memStore.getState().identities - identities[hexAddress].name = label - this.memStore.updateState({ identities }) - return Promise.resolve(label) - } catch (err) { - return Promise.reject(err) - } - } - - // Export Account - // @string address - // - // returns Promise( @string privateKey ) - // - // Requests the private key from the keyring controlling - // the specified address. - // - // Returns a Promise that may resolve with the private key string. - exportAccount (address) { - try { - return this.getKeyringForAccount(address) - .then((keyring) => { - return keyring.exportAccount(normalizeAddress(address)) - }) - } catch (e) { - return Promise.reject(e) - } - } - - - // SIGNING METHODS - // - // This method signs tx and returns a promise for - // TX Manager to update the state after signing - - signTransaction (ethTx, _fromAddress) { - const fromAddress = normalizeAddress(_fromAddress) - return this.getKeyringForAccount(fromAddress) - .then((keyring) => { - return keyring.signTransaction(fromAddress, ethTx) - }) - } - - // Sign Message - // @object msgParams - // - // returns Promise(@buffer rawSig) - // - // Attempts to sign the provided @object msgParams. - signMessage (msgParams) { - const address = normalizeAddress(msgParams.from) - return this.getKeyringForAccount(address) - .then((keyring) => { - return keyring.signMessage(address, msgParams.data) - }) - } - - // Sign Personal Message - // @object msgParams - // - // returns Promise(@buffer rawSig) - // - // Attempts to sign the provided @object msgParams. - // Prefixes the hash before signing as per the new geth behavior. - signPersonalMessage (msgParams) { - const address = normalizeAddress(msgParams.from) - return this.getKeyringForAccount(address) - .then((keyring) => { - return keyring.signPersonalMessage(address, msgParams.data) - }) - } - - // PRIVATE METHODS - // - // THESE METHODS ARE ONLY USED INTERNALLY TO THE KEYRING-CONTROLLER - // AND SO MAY BE CHANGED MORE LIBERALLY THAN THE ABOVE METHODS. - - // Create First Key Tree - // returns @Promise - // - // Clears the vault, - // creates a new one, - // creates a random new HD Keyring with 1 account, - // makes that account the selected account, - // faucets that account on testnet, - // puts the current seed words into the state tree. - createFirstKeyTree () { - this.clearKeyrings() - return this.addNewKeyring('HD Key Tree', { numberOfAccounts: 1 }) - .then((keyring) => { - return keyring.getAccounts() - }) - .then((accounts) => { - const firstAccount = accounts[0] - if (!firstAccount) throw new Error('KeyringController - No account found on keychain.') - const hexAccount = normalizeAddress(firstAccount) - this.emit('newVault', hexAccount) - return this.setupAccounts(accounts) - }) - .then(this.persistAllKeyrings.bind(this)) - } - - // Setup Accounts - // @array accounts - // - // returns @Promise(@object account) - // - // Initializes the provided account array - // Gives them numerically incremented nicknames, - // and adds them to the accountTracker for regular balance checking. - setupAccounts (accounts) { - return this.getAccounts() - .then((loadedAccounts) => { - const arr = accounts || loadedAccounts - return Promise.all(arr.map((account) => { - return this.getBalanceAndNickname(account) - })) - }) - } - - // Get Balance And Nickname - // @string account - // - // returns Promise( @string label ) - // - // Takes an account address and an iterator representing - // the current number of named accounts. - getBalanceAndNickname (account) { - if (!account) { - throw new Error('Problem loading account.') - } - const address = normalizeAddress(account) - this.accountTracker.addAccount(address) - return this.createNickname(address) - } - - // Create Nickname - // @string address - // - // returns Promise( @string label ) - // - // Takes an address, and assigns it an incremented nickname, persisting it. - createNickname (address) { - const hexAddress = normalizeAddress(address) - const identities = this.memStore.getState().identities - const currentIdentityCount = Object.keys(identities).length + 1 - const nicknames = this.store.getState().walletNicknames || {} - const existingNickname = nicknames[hexAddress] - const name = existingNickname || `Account ${currentIdentityCount}` - identities[hexAddress] = { - address: hexAddress, - name, - } - this.memStore.updateState({ identities }) - return this.saveAccountLabel(hexAddress, name) - } - - // Persist All Keyrings - // @password string - // - // returns Promise - // - // Iterates the current `keyrings` array, - // serializes each one into a serialized array, - // encrypts that array with the provided `password`, - // and persists that encrypted string to storage. - persistAllKeyrings (password = this.password) { - if (typeof password === 'string') { - this.password = password - this.memStore.updateState({ isUnlocked: true }) - } - return Promise.all(this.keyrings.map((keyring) => { - return Promise.all([keyring.type, keyring.serialize()]) - .then((serializedKeyringArray) => { - // Label the output values on each serialized Keyring: - return { - type: serializedKeyringArray[0], - data: serializedKeyringArray[1], - } - }) - })) - .then((serializedKeyrings) => { - return this.encryptor.encrypt(this.password, serializedKeyrings) - }) - .then((encryptedString) => { - this.store.updateState({ vault: encryptedString }) - return true - }) - } - - // Unlock Keyrings - // @string password - // - // returns Promise( @array keyrings ) - // - // Attempts to unlock the persisted encrypted storage, - // initializing the persisted keyrings to RAM. - unlockKeyrings (password) { - const encryptedVault = this.store.getState().vault - if (!encryptedVault) { - throw new Error('Cannot unlock without a previous vault.') - } - - return this.encryptor.decrypt(password, encryptedVault) - .then((vault) => { - this.password = password - this.memStore.updateState({ isUnlocked: true }) - vault.forEach(this.restoreKeyring.bind(this)) - return this.keyrings - }) - } - - // Restore Keyring - // @object serialized - // - // returns Promise( @Keyring deserialized ) - // - // Attempts to initialize a new keyring from the provided - // serialized payload. - // - // On success, returns the resulting @Keyring instance. - restoreKeyring (serialized) { - const { type, data } = serialized - - const Keyring = this.getKeyringClassForType(type) - const keyring = new Keyring() - return keyring.deserialize(data) - .then(() => { - return keyring.getAccounts() - }) - .then((accounts) => { - return this.setupAccounts(accounts) - }) - .then(() => { - this.keyrings.push(keyring) - this._updateMemStoreKeyrings() - return keyring - }) - } - - // Get Keyring Class For Type - // @string type - // - // Returns @class Keyring - // - // Searches the current `keyringTypes` array - // for a Keyring class whose unique `type` property - // matches the provided `type`, - // returning it if it exists. - getKeyringClassForType (type) { - 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 ] ) - // - // Returns the public addresses of all current accounts - // managed by all currently unlocked keyrings. - getAccounts () { - const keyrings = this.keyrings || [] - return Promise.all(keyrings.map(kr => kr.getAccounts())) - .then((keyringArrays) => { - return keyringArrays.reduce((res, arr) => { - return res.concat(arr) - }, []) - }) - } - - // Get Keyring For Account - // @string address - // - // returns Promise(@Keyring keyring) - // - // Returns the currently initialized keyring that manages - // the specified `address` if one exists. - getKeyringForAccount (address) { - const hexed = normalizeAddress(address) - log.debug(`KeyringController - getKeyringForAccount: ${hexed}`) - - return Promise.all(this.keyrings.map((keyring) => { - return Promise.all([ - keyring, - keyring.getAccounts(), - ]) - })) - .then(filter((candidate) => { - const accounts = candidate[1].map(normalizeAddress) - return accounts.includes(hexed) - })) - .then((winners) => { - if (winners && winners.length > 0) { - return winners[0][0] - } else { - throw new Error('No keyring found for the requested account.') - } - }) - } - - // Display For Keyring - // @Keyring keyring - // - // returns Promise( @Object { type:String, accounts:Array } ) - // - // Is used for adding the current keyrings to the state object. - displayForKeyring (keyring) { - return keyring.getAccounts() - .then((accounts) => { - return { - type: keyring.type, - accounts: accounts, - } - }) - } - - // Add Gas Buffer - // @string gas (as hexadecimal value) - // - // returns @string bufferedGas (as hexadecimal value) - // - // Adds a healthy buffer of gas to an initial gas estimate. - addGasBuffer (gas) { - const gasBuffer = new BN('100000', 10) - const bnGas = new BN(ethUtil.stripHexPrefix(gas), 16) - const correct = bnGas.add(gasBuffer) - return ethUtil.addHexPrefix(correct.toString(16)) - } - - // Clear Keyrings - // - // Deallocates all currently managed keyrings and accounts. - // Used before initializing a new vault. - clearKeyrings () { - let accounts - try { - accounts = Object.keys(this.accountTracker.store.getState()) - } catch (e) { - accounts = [] - } - accounts.forEach((address) => { - this.accountTracker.removeAccount(address) - }) - - // clear keyrings from memory - this.keyrings = [] - this.memStore.updateState({ - keyrings: [], - identities: {}, - }) - } - - _updateMemStoreKeyrings () { - Promise.all(this.keyrings.map(this.displayForKeyring)) - .then((keyrings) => { - this.memStore.updateState({ keyrings }) - }) - } - -} - -module.exports = KeyringController diff --git a/package.json b/package.json index c160cbfde..052d20a22 100644 --- a/package.json +++ b/package.json @@ -71,7 +71,7 @@ "eth-contract-metadata": "^1.1.4", "eth-hd-keyring": "^1.1.1", "eth-json-rpc-filters": "^1.2.1", - "eth-keyring-controller": "^1.0.1", + "eth-keyring-controller": "^2.0.0", "eth-phishing-detect": "^1.1.4", "eth-query": "^2.1.2", "eth-sig-util": "^1.2.2", From b473d440a36d7715582431ff0024d16e4da3bc36 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 27 Sep 2017 15:14:40 -0700 Subject: [PATCH 5/5] Version 3.10.6 --- CHANGELOG.md | 2 ++ app/manifest.json | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5b54ed1a6..599f340f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Current Master +## 3.10.6 2017-9-27 + - Fix bug where newly created accounts were not selected. - Fix bug where selected account was not persisted between lockings. diff --git a/app/manifest.json b/app/manifest.json index 4d02cd334..639f3fb4b 100644 --- a/app/manifest.json +++ b/app/manifest.json @@ -1,7 +1,7 @@ { "name": "MetaMask", "short_name": "Metamask", - "version": "3.10.5", + "version": "3.10.6", "manifest_version": 2, "author": "https://metamask.io", "description": "Ethereum Browser Extension",