From b52346388b8d4518ffb2eb34236c6d17579085f3 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 18 Jan 2017 15:17:08 -0800 Subject: [PATCH 1/7] Added new modular private key import system Now any strategy for importing a private key that can be described as a pure function can be very easily turned into a MetaMask import strategy. I've created a generic and reusable UI action called `importNewAccount(strategy, args)`. The `strategy` is a unique identifier defined in `app/scripts/account-import-strategies`, and the `args` will be passed to the member of the `strategies` array whose key matches the strategy string. Strategies return private key hex strings, and are used by the metamask-controller to create a new keyring, and select that new account, before calling back. This also implements @frankiebee's idea of showing the imported account when it's been imported (my oversight!). This commit only moves us to this architecture, keeping feature parity for private key import, but has some untested code for importing geth-style JSON files as well! --- .../account-import-strategies/index.js | 37 +++++++++++++++++++ app/scripts/metamask-controller.js | 11 ++++++ ui/app/accounts/import/private-key.js | 3 +- ui/app/actions.js | 16 ++++++++ 4 files changed, 65 insertions(+), 2 deletions(-) create mode 100644 app/scripts/account-import-strategies/index.js diff --git a/app/scripts/account-import-strategies/index.js b/app/scripts/account-import-strategies/index.js new file mode 100644 index 000000000..8f4456cdf --- /dev/null +++ b/app/scripts/account-import-strategies/index.js @@ -0,0 +1,37 @@ +const Wallet = require('ethereumjs-wallet') +const importers = require('ethereumjs-wallet/thirdparty') +const ethUtil = require('ethereumjs-util') + +const accountImporter = { + + importAccount(strategy, args) { + try { + const importer = this.strategies[strategy] + const wallet = importer.apply(null, args) + const privateKeyHex = walletToPrivateKey(wallet) + return Promise.resolve(privateKeyHex) + } catch (e) { + return Promise.reject(e) + } + }, + + strategies: { + 'Private Key': (privateKey) => { + const stripped = ethUtil.stripHexPrefix(privateKey) + const buffer = new Buffer(stripped, 'hex') + return Wallet.fromPrivateKey(buffer) + }, + 'JSON File': (input, password) => { + const wallet = importers.fromEtherWallet(input, password) + return walletToPrivateKey(wallet) + }, + }, + +} + +function walletToPrivateKey (wallet) { + const privateKeyBuffer = wallet.getPrivateKey() + return ethUtil.bufferToHex(privateKeyBuffer) +} + +module.exports = accountImporter diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 629216e42..7084bbc2d 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -13,6 +13,7 @@ const extension = require('./lib/extension') const autoFaucet = require('./lib/auto-faucet') const nodeify = require('./lib/nodeify') const IdStoreMigrator = require('./lib/idStore-migrator') +const accountImporter = require('./account-import-strategies') const version = require('../manifest.json').version module.exports = class MetamaskController extends EventEmitter { @@ -121,6 +122,16 @@ module.exports = class MetamaskController extends EventEmitter { .then((newState) => { cb(null, newState) }) .catch((reason) => { cb(reason) }) }, + importAccountWithStrategy: (strategy, args, cb) => { + accountImporter.importAccount(strategy, args) + .then((privateKey) => { + return keyringController.addNewKeyring('Simple Key Pair', [ privateKey ]) + }) + .then(keyring => keyring.getAccounts()) + .then((accounts) => keyringController.setSelectedAccount(accounts[0])) + .then(() => { cb(null, keyringController.fullUpdate()) }) + .catch((reason) => { cb(reason) }) + }, addNewAccount: nodeify(keyringController.addNewAccount).bind(keyringController), setSelectedAccount: nodeify(keyringController.setSelectedAccount).bind(keyringController), saveAccountLabel: nodeify(keyringController.saveAccountLabel).bind(keyringController), diff --git a/ui/app/accounts/import/private-key.js b/ui/app/accounts/import/private-key.js index 6b988a76b..b139a0374 100644 --- a/ui/app/accounts/import/private-key.js +++ b/ui/app/accounts/import/private-key.js @@ -2,7 +2,6 @@ const inherits = require('util').inherits const Component = require('react').Component const h = require('react-hyperscript') const connect = require('react-redux').connect -const type = 'Simple Key Pair' const actions = require('../../actions') module.exports = connect(mapStateToProps)(PrivateKeyImportView) @@ -64,6 +63,6 @@ PrivateKeyImportView.prototype.createKeyringOnEnter = function (event) { PrivateKeyImportView.prototype.createNewKeychain = function () { const input = document.getElementById('private-key-box') const privateKey = input.value - this.props.dispatch(actions.addNewKeyring(type, [ privateKey ])) + this.props.dispatch(actions.importNewAccount('Private Key', [ privateKey ])) } diff --git a/ui/app/actions.js b/ui/app/actions.js index 7934a329a..36efa4bc6 100644 --- a/ui/app/actions.js +++ b/ui/app/actions.js @@ -43,6 +43,7 @@ var actions = { createNewVaultAndRestore: createNewVaultAndRestore, createNewVaultInProgress: createNewVaultInProgress, addNewKeyring, + importNewAccount, addNewAccount, NEW_ACCOUNT_SCREEN: 'NEW_ACCOUNT_SCREEN', navigateToNewAccountScreen, @@ -264,6 +265,21 @@ function addNewKeyring (type, opts) { } } +function importNewAccount (strategy, args) { + return (dispatch) => { + dispatch(actions.showLoadingIndication()) + background.importAccountWithStrategy(strategy, args, (err, newState) => { + dispatch(actions.hideLoadingIndication()) + if (err) return dispatch(actions.displayWarning(err.message)) + dispatch(actions.updateMetamaskState(newState)) + dispatch({ + type: actions.SHOW_ACCOUNT_DETAIL, + value: newState.selectedAccount, + }) + }) + } +} + function navigateToNewAccountScreen() { return { type: this.NEW_ACCOUNT_SCREEN, From 9126652f2e8b5b612f894bbb6c46fb1ef7861d06 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 18 Jan 2017 16:09:21 -0800 Subject: [PATCH 2/7] Implement naieve JSON file importing Doesn't work on any JSON file I have, it's a very naieve strategy provided by ethereumjs-wallet. Will need to raise its sophistication before deploying to production. --- package.json | 1 + ui/app/accounts/import/index.js | 6 +-- ui/app/accounts/import/json.js | 75 ++++++++++++++++++++++++++++++++- 3 files changed, 77 insertions(+), 5 deletions(-) diff --git a/package.json b/package.json index 2c0c30523..828828753 100644 --- a/package.json +++ b/package.json @@ -84,6 +84,7 @@ "react-markdown": "^2.3.0", "react-redux": "^4.4.5", "react-select": "^1.0.0-rc.2", + "react-simple-file-input": "^1.0.0", "react-tooltip-component": "^0.3.0", "readable-stream": "^2.1.2", "redux": "^3.0.5", diff --git a/ui/app/accounts/import/index.js b/ui/app/accounts/import/index.js index 18a6b985c..96350852a 100644 --- a/ui/app/accounts/import/index.js +++ b/ui/app/accounts/import/index.js @@ -6,11 +6,11 @@ import Select from 'react-select' // Subviews const JsonImportView = require('./json.js') -const SeedImportView = require('./seed.js') const PrivateKeyImportView = require('./private-key.js') const menuItems = [ 'Private Key', + 'JSON File', ] module.exports = connect(mapStateToProps)(AccountImportSubview) @@ -81,10 +81,10 @@ AccountImportSubview.prototype.renderImportView = function() { const current = type || menuItems[0] switch (current) { - case 'HD Key Tree': - return h(SeedImportView) case 'Private Key': return h(PrivateKeyImportView) + case 'JSON File': + return h(JsonImportView) default: return h(JsonImportView) } diff --git a/ui/app/accounts/import/json.js b/ui/app/accounts/import/json.js index 22cf95cfd..1c2b331d4 100644 --- a/ui/app/accounts/import/json.js +++ b/ui/app/accounts/import/json.js @@ -2,11 +2,15 @@ const inherits = require('util').inherits const Component = require('react').Component const h = require('react-hyperscript') const connect = require('react-redux').connect +const actions = require('../../actions') +const FileInput = require('react-simple-file-input').default module.exports = connect(mapStateToProps)(JsonImportSubview) function mapStateToProps (state) { - return {} + return { + error: state.appState.warning, + } } inherits(JsonImportSubview, Component) @@ -15,13 +19,80 @@ function JsonImportSubview () { } JsonImportSubview.prototype.render = function () { + const { error } = this.props + return ( h('div', { style: { + display: 'flex', + flexDirection: 'column', + alignItems: 'center', + padding: '5px 15px 0px 15px', }, }, [ - `Upload your json file here!`, + + h('p', 'Used by a variety of different clients'), + + h(FileInput, { + readAs: 'text', + onLoad: this.onLoad.bind(this), + style: { + margin: '20px 0px 12px 20px', + fontSize: '15px', + }, + }), + + h('input.large-input.letter-spacey', { + type: 'password', + placeholder: 'Enter password', + id: 'json-password-box', + onKeyPress: this.createKeyringOnEnter.bind(this), + style: { + width: 260, + marginTop: 12, + }, + }), + + h('button.primary', { + onClick: this.createNewKeychain.bind(this), + style: { + margin: 12, + }, + }, 'Import'), + + error ? h('span.warning', error) : null, ]) ) } +JsonImportSubview.prototype.onLoad = function (event, file) { + this.setState({file: file, fileContents: event.target.result}) +} + +JsonImportSubview.prototype.createKeyringOnEnter = function (event) { + if (event.key === 'Enter') { + event.preventDefault() + this.createNewKeychain() + } +} + +JsonImportSubview.prototype.createNewKeychain = function () { + const state = this.state + const { fileContents } = state + + if (!fileContents) { + const message = 'You must select a file to import.' + return this.props.dispatch(actions.displayWarning(message)) + } + + const passwordInput = document.getElementById('json-password-box') + const password = passwordInput.value + + if (!password) { + const message = 'You must enter a password for the selected file.' + return this.props.dispatch(actions.displayWarning(message)) + } + + this.props.dispatch(actions.importNewAccount('JSON File', [ fileContents, password ])) +} + From 5d8a3dd99b0cebad48e2fdcc4c407d7d89d4717c Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 18 Jan 2017 16:45:39 -0800 Subject: [PATCH 3/7] Add ability to import v3 JSON wallets There is now a menu item labeled "JSON File" for importing, and it can digest either: - v1 MyEtherWallet JSON files - v3 Account files (used by Geth, Mist, and MyEtherWallet). Fixes #715 --- app/scripts/account-import-strategies/index.js | 18 +++++++++++++----- ui/app/actions.js | 5 +++-- ui/app/app.js | 5 +++-- ui/app/components/loading.js | 8 +++++++- ui/app/reducers/app.js | 1 + 5 files changed, 27 insertions(+), 10 deletions(-) diff --git a/app/scripts/account-import-strategies/index.js b/app/scripts/account-import-strategies/index.js index 8f4456cdf..d5124eb7f 100644 --- a/app/scripts/account-import-strategies/index.js +++ b/app/scripts/account-import-strategies/index.js @@ -7,8 +7,7 @@ const accountImporter = { importAccount(strategy, args) { try { const importer = this.strategies[strategy] - const wallet = importer.apply(null, args) - const privateKeyHex = walletToPrivateKey(wallet) + const privateKeyHex = importer.apply(null, args) return Promise.resolve(privateKeyHex) } catch (e) { return Promise.reject(e) @@ -18,11 +17,20 @@ const accountImporter = { strategies: { 'Private Key': (privateKey) => { const stripped = ethUtil.stripHexPrefix(privateKey) - const buffer = new Buffer(stripped, 'hex') - return Wallet.fromPrivateKey(buffer) + return stripped }, 'JSON File': (input, password) => { - const wallet = importers.fromEtherWallet(input, password) + let wallet + try { + wallet = importers.fromEtherWallet(input, password) + } catch (e) { + console.log('Attempt to import as EtherWallet format failed, trying V3...') + } + + if (!wallet) { + wallet = Wallet.fromV3(input, password, true) + } + return walletToPrivateKey(wallet) }, }, diff --git a/ui/app/actions.js b/ui/app/actions.js index 36efa4bc6..bf3617310 100644 --- a/ui/app/actions.js +++ b/ui/app/actions.js @@ -267,7 +267,7 @@ function addNewKeyring (type, opts) { function importNewAccount (strategy, args) { return (dispatch) => { - dispatch(actions.showLoadingIndication()) + dispatch(actions.showLoadingIndication('This may take a while, be patient.')) background.importAccountWithStrategy(strategy, args, (err, newState) => { dispatch(actions.hideLoadingIndication()) if (err) return dispatch(actions.displayWarning(err.message)) @@ -630,9 +630,10 @@ function useEtherscanProvider () { } } -function showLoadingIndication () { +function showLoadingIndication (message) { return { type: actions.SHOW_LOADING, + value: message, } } diff --git a/ui/app/app.js b/ui/app/app.js index 0e04c334c..d8dedd397 100644 --- a/ui/app/app.js +++ b/ui/app/app.js @@ -43,6 +43,7 @@ function mapStateToProps (state) { return { // state from plugin isLoading: state.appState.isLoading, + loadingMessage: state.appState.loadingMessage, isDisclaimerConfirmed: state.metamask.isDisclaimerConfirmed, noActiveNotices: state.metamask.noActiveNotices, isInitialized: state.metamask.isInitialized, @@ -64,7 +65,7 @@ function mapStateToProps (state) { App.prototype.render = function () { var props = this.props - const { isLoading, transForward } = props + const { isLoading, loadingMessage, transForward } = props return ( @@ -76,7 +77,7 @@ App.prototype.render = function () { }, }, [ - h(LoadingIndicator, { isLoading }), + h(LoadingIndicator, { isLoading, loadingMessage }), // app bar this.renderAppBar(), diff --git a/ui/app/components/loading.js b/ui/app/components/loading.js index ae735894f..88dc535df 100644 --- a/ui/app/components/loading.js +++ b/ui/app/components/loading.js @@ -12,7 +12,7 @@ function LoadingIndicator () { } LoadingIndicator.prototype.render = function () { - var isLoading = this.props.isLoading + const { isLoading, loadingMessage } = this.props return ( h(ReactCSSTransitionGroup, { @@ -37,8 +37,14 @@ LoadingIndicator.prototype.render = function () { h('img', { src: 'images/loading.svg', }), + + showMessageIfAny(loadingMessage), ]) : null, ]) ) } +function showMessageIfAny (loadingMessage) { + if (!loadingMessage) return null + return h('span', loadingMessage) +} diff --git a/ui/app/reducers/app.js b/ui/app/reducers/app.js index ae91272cc..6a2c93f78 100644 --- a/ui/app/reducers/app.js +++ b/ui/app/reducers/app.js @@ -386,6 +386,7 @@ function reduceApp (state, action) { case actions.SHOW_LOADING: return extend(appState, { isLoading: true, + loadingMessage: action.value, }) case actions.HIDE_LOADING: From b478ce5b778bd70a3ebfc89f2c680daa25a6f395 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 18 Jan 2017 17:24:41 -0800 Subject: [PATCH 4/7] Bump changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 51bea159c..adfec8be9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Current Master +- Add ability to import accounts in JSON file format (used by Mist, Geth, MyEtherWallet, and more!) + ## 3.1.0 2017-1-18 - Add ability to import accounts by private key. From b2623510aec78cbabdcb4bbfaf5c9c406f4e428a Mon Sep 17 00:00:00 2001 From: Frankie Date: Tue, 24 Jan 2017 13:19:26 -0800 Subject: [PATCH 5/7] 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) } From 42bb90341dde5699dca4ba7ad8891f26c1faf4a6 Mon Sep 17 00:00:00 2001 From: Frankie Date: Tue, 24 Jan 2017 13:21:15 -0800 Subject: [PATCH 6/7] 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 From 8ed657d5d574389a1b4c44d127ecf37f86cc7e64 Mon Sep 17 00:00:00 2001 From: Frankie Date: Tue, 24 Jan 2017 14:53:45 -0800 Subject: [PATCH 7/7] 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) }