From 0584988688a471698e9b3ad05cb0597f0270ea9e Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Tue, 21 Feb 2017 14:25:47 -0800 Subject: Move sigUtil and keyrings to external modules These external modules now have their own test coverage and build enforcement. This allowed me to somewhat more easily add good tests around our personalSign strategy (held now in [eth-sig-util](https://github.com/flyswatter/eth-sig-util), and allow each of the keyrings to import that, etc. --- app/scripts/keyring-controller.js | 35 +++++++- app/scripts/keyrings/hd.js | 125 ---------------------------- app/scripts/keyrings/simple.js | 100 ---------------------- app/scripts/lib/config-manager.js | 2 +- app/scripts/lib/controllers/preferences.js | 2 +- app/scripts/lib/idStore-migrator.js | 2 +- app/scripts/lib/personal-message-manager.js | 118 ++++++++++++++++++++++++++ app/scripts/lib/sig-util.js | 28 ------- app/scripts/lib/tx-utils.js | 2 +- app/scripts/metamask-controller.js | 49 ++++++++++- 10 files changed, 202 insertions(+), 261 deletions(-) delete mode 100644 app/scripts/keyrings/hd.js delete mode 100644 app/scripts/keyrings/simple.js create mode 100644 app/scripts/lib/personal-message-manager.js delete mode 100644 app/scripts/lib/sig-util.js (limited to 'app/scripts') diff --git a/app/scripts/keyring-controller.js b/app/scripts/keyring-controller.js index b30161003..8c379b5b9 100644 --- a/app/scripts/keyring-controller.js +++ b/app/scripts/keyring-controller.js @@ -5,10 +5,10 @@ const EventEmitter = require('events').EventEmitter const ObservableStore = require('obs-store') const filter = require('promise-filter') const encryptor = require('browser-passworder') -const normalizeAddress = require('./lib/sig-util').normalize +const normalizeAddress = require('eth-sig-util').normalize // Keyrings: -const SimpleKeyring = require('./keyrings/simple') -const HdKeyring = require('./keyrings/hd') +const SimpleKeyring = require('eth-simple-keyring') +const HdKeyring = require('eth-hd-keyring') const keyringTypes = [ SimpleKeyring, HdKeyring, @@ -262,6 +262,35 @@ class KeyringController extends EventEmitter { }) } + // 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) + }) + } + + // Recover Personal Message + // @object msgParams + // + // returns Promise(@buffer signer) + // + // recovers a signature of the prefixed-style personalMessage signature. + recoverPersonalMessage (msgParams) { + const address = normalizeAddress(msgParams.from) + return this.getKeyringForAccount(address) + .then((keyring) => { + return keyring.recoverPersonalMessage(address, msgParams.data) + }) + } + // PRIVATE METHODS // // THESE METHODS ARE ONLY USED INTERNALLY TO THE KEYRING-CONTROLLER diff --git a/app/scripts/keyrings/hd.js b/app/scripts/keyrings/hd.js deleted file mode 100644 index 3a66f7868..000000000 --- a/app/scripts/keyrings/hd.js +++ /dev/null @@ -1,125 +0,0 @@ -const EventEmitter = require('events').EventEmitter -const hdkey = require('ethereumjs-wallet/hdkey') -const bip39 = require('bip39') -const ethUtil = require('ethereumjs-util') - -// *Internal Deps -const sigUtil = require('../lib/sig-util') - -// Options: -const hdPathString = `m/44'/60'/0'/0` -const type = 'HD Key Tree' - -class HdKeyring extends EventEmitter { - - /* PUBLIC METHODS */ - - constructor (opts = {}) { - super() - this.type = type - this.deserialize(opts) - } - - serialize () { - return Promise.resolve({ - mnemonic: this.mnemonic, - numberOfAccounts: this.wallets.length, - }) - } - - deserialize (opts = {}) { - this.opts = opts || {} - this.wallets = [] - this.mnemonic = null - this.root = null - - if (opts.mnemonic) { - this._initFromMnemonic(opts.mnemonic) - } - - if (opts.numberOfAccounts) { - return this.addAccounts(opts.numberOfAccounts) - } - - return Promise.resolve([]) - } - - addAccounts (numberOfAccounts = 1) { - if (!this.root) { - this._initFromMnemonic(bip39.generateMnemonic()) - } - - const oldLen = this.wallets.length - const newWallets = [] - for (let i = oldLen; i < numberOfAccounts + oldLen; i++) { - const child = this.root.deriveChild(i) - const wallet = child.getWallet() - newWallets.push(wallet) - this.wallets.push(wallet) - } - const hexWallets = newWallets.map(w => w.getAddress().toString('hex')) - return Promise.resolve(hexWallets) - } - - getAccounts () { - return Promise.resolve(this.wallets.map(w => w.getAddress().toString('hex'))) - } - - // tx is an instance of the ethereumjs-transaction class. - signTransaction (address, tx) { - const wallet = this._getWalletForAccount(address) - var privKey = wallet.getPrivateKey() - tx.sign(privKey) - return Promise.resolve(tx) - } - - // For eth_sign, we need to sign transactions: - // hd - signMessage (withAccount, data) { - const wallet = this._getWalletForAccount(withAccount) - const message = ethUtil.stripHexPrefix(data) - var privKey = wallet.getPrivateKey() - var msgSig = ethUtil.ecsign(new Buffer(message, 'hex'), privKey) - var rawMsgSig = ethUtil.bufferToHex(sigUtil.concatSig(msgSig.v, msgSig.r, msgSig.s)) - return Promise.resolve(rawMsgSig) - } - - // For eth_sign, we need to sign transactions: - newGethSignMessage (withAccount, msgHex) { - const wallet = this._getWalletForAccount(withAccount) - const privKey = wallet.getPrivateKey() - const msgBuffer = ethUtil.toBuffer(msgHex) - const msgHash = ethUtil.hashPersonalMessage(msgBuffer) - const msgSig = ethUtil.ecsign(msgHash, privKey) - const rawMsgSig = ethUtil.bufferToHex(sigUtil.concatSig(msgSig.v, msgSig.r, msgSig.s)) - return Promise.resolve(rawMsgSig) - } - - exportAccount (address) { - const wallet = this._getWalletForAccount(address) - return Promise.resolve(wallet.getPrivateKey().toString('hex')) - } - - - /* PRIVATE METHODS */ - - _initFromMnemonic (mnemonic) { - this.mnemonic = mnemonic - const seed = bip39.mnemonicToSeed(mnemonic) - this.hdWallet = hdkey.fromMasterSeed(seed) - this.root = this.hdWallet.derivePath(hdPathString) - } - - - _getWalletForAccount (account) { - const targetAddress = sigUtil.normalize(account) - return this.wallets.find((w) => { - const address = w.getAddress().toString('hex') - return ((address === targetAddress) || - (sigUtil.normalize(address) === targetAddress)) - }) - } -} - -HdKeyring.type = type -module.exports = HdKeyring diff --git a/app/scripts/keyrings/simple.js b/app/scripts/keyrings/simple.js deleted file mode 100644 index 82881aa2d..000000000 --- a/app/scripts/keyrings/simple.js +++ /dev/null @@ -1,100 +0,0 @@ -const EventEmitter = require('events').EventEmitter -const Wallet = require('ethereumjs-wallet') -const ethUtil = require('ethereumjs-util') -const type = 'Simple Key Pair' -const sigUtil = require('../lib/sig-util') - -class SimpleKeyring extends EventEmitter { - - /* PUBLIC METHODS */ - - constructor (opts) { - super() - this.type = type - this.opts = opts || {} - this.wallets = [] - } - - serialize () { - return Promise.resolve(this.wallets.map(w => w.getPrivateKey().toString('hex'))) - } - - deserialize (privateKeys = []) { - return new Promise((resolve, reject) => { - try { - this.wallets = privateKeys.map((privateKey) => { - const stripped = ethUtil.stripHexPrefix(privateKey) - const buffer = new Buffer(stripped, 'hex') - const wallet = Wallet.fromPrivateKey(buffer) - return wallet - }) - } catch (e) { - reject(e) - } - resolve() - }) - } - - addAccounts (n = 1) { - var newWallets = [] - for (var i = 0; i < n; i++) { - newWallets.push(Wallet.generate()) - } - this.wallets = this.wallets.concat(newWallets) - const hexWallets = newWallets.map(w => ethUtil.bufferToHex(w.getAddress())) - return Promise.resolve(hexWallets) - } - - getAccounts () { - return Promise.resolve(this.wallets.map(w => ethUtil.bufferToHex(w.getAddress()))) - } - - // tx is an instance of the ethereumjs-transaction class. - signTransaction (address, tx) { - const wallet = this._getWalletForAccount(address) - var privKey = wallet.getPrivateKey() - tx.sign(privKey) - return Promise.resolve(tx) - } - - // For eth_sign, we need to sign transactions: - signMessage (withAccount, data) { - const wallet = this._getWalletForAccount(withAccount) - const message = ethUtil.stripHexPrefix(data) - var privKey = wallet.getPrivateKey() - var msgSig = ethUtil.ecsign(new Buffer(message, 'hex'), privKey) - var rawMsgSig = ethUtil.bufferToHex(sigUtil.concatSig(msgSig.v, msgSig.r, msgSig.s)) - return Promise.resolve(rawMsgSig) - } - - // For eth_sign, we need to sign transactions: - - newGethSignMessage (withAccount, msgHex) { - const wallet = this._getWalletForAccount(withAccount) - const privKey = wallet.getPrivateKey() - const msgBuffer = ethUtil.toBuffer(msgHex) - const msgHash = ethUtil.hashPersonalMessage(msgBuffer) - const msgSig = ethUtil.ecsign(msgHash, privKey) - const rawMsgSig = ethUtil.bufferToHex(sigUtil.concatSig(msgSig.v, msgSig.r, msgSig.s)) - return Promise.resolve(rawMsgSig) - } - - exportAccount (address) { - const wallet = this._getWalletForAccount(address) - return Promise.resolve(wallet.getPrivateKey().toString('hex')) - } - - - /* PRIVATE METHODS */ - - _getWalletForAccount (account) { - const address = sigUtil.normalize(account) - let wallet = this.wallets.find(w => ethUtil.bufferToHex(w.getAddress()) === address) - if (!wallet) throw new Error('Simple Keyring - Unable to find matching address.') - return wallet - } - -} - -SimpleKeyring.type = type -module.exports = SimpleKeyring diff --git a/app/scripts/lib/config-manager.js b/app/scripts/lib/config-manager.js index 6267eab68..ea5e49b19 100644 --- a/app/scripts/lib/config-manager.js +++ b/app/scripts/lib/config-manager.js @@ -1,6 +1,6 @@ const MetamaskConfig = require('../config.js') const ethUtil = require('ethereumjs-util') -const normalize = require('./sig-util').normalize +const normalize = require('eth-sig-util').normalize const TESTNET_RPC = MetamaskConfig.network.testnet const MAINNET_RPC = MetamaskConfig.network.mainnet diff --git a/app/scripts/lib/controllers/preferences.js b/app/scripts/lib/controllers/preferences.js index dc9464c4e..c5e93a5b9 100644 --- a/app/scripts/lib/controllers/preferences.js +++ b/app/scripts/lib/controllers/preferences.js @@ -1,5 +1,5 @@ const ObservableStore = require('obs-store') -const normalizeAddress = require('../sig-util').normalize +const normalizeAddress = require('eth-sig-util').normalize class PreferencesController { diff --git a/app/scripts/lib/idStore-migrator.js b/app/scripts/lib/idStore-migrator.js index 655aed0af..1485beb48 100644 --- a/app/scripts/lib/idStore-migrator.js +++ b/app/scripts/lib/idStore-migrator.js @@ -1,6 +1,6 @@ const IdentityStore = require('./idStore') const HdKeyring = require('../keyrings/hd') -const sigUtil = require('./sig-util') +const sigUtil = require('eth-sig-util') const normalize = sigUtil.normalize const denodeify = require('denodeify') diff --git a/app/scripts/lib/personal-message-manager.js b/app/scripts/lib/personal-message-manager.js new file mode 100644 index 000000000..72dd1da96 --- /dev/null +++ b/app/scripts/lib/personal-message-manager.js @@ -0,0 +1,118 @@ +const EventEmitter = require('events') +const ObservableStore = require('obs-store') +const ethUtil = require('ethereumjs-util') +const createId = require('./random-id') + + +module.exports = class MessageManager extends EventEmitter{ + constructor (opts) { + super() + this.memStore = new ObservableStore({ + unapprovedPersonalMsgs: {}, + unapprovedPersonalMsgCount: 0, + }) + this.messages = [] + } + + get unapprovedPersonalMsgCount () { + return Object.keys(this.getUnapprovedMsgs()).length + } + + getUnapprovedMsgs () { + return this.messages.filter(msg => msg.status === 'unapproved') + .reduce((result, msg) => { result[msg.id] = msg; return result }, {}) + } + + addUnapprovedMessage (msgParams) { + msgParams.data = normalizeMsgData(msgParams.data) + // create txData obj with parameters and meta data + var time = (new Date()).getTime() + var msgId = createId() + var msgData = { + id: msgId, + msgParams: msgParams, + time: time, + status: 'unapproved', + } + this.addMsg(msgData) + + // signal update + this.emit('update') + return msgId + } + + addMsg (msg) { + this.messages.push(msg) + this._saveMsgList() + } + + getMsg (msgId) { + return this.messages.find(msg => msg.id === msgId) + } + + approveMessage (msgParams) { + this.setMsgStatusApproved(msgParams.metamaskId) + return this.prepMsgForSigning(msgParams) + } + + setMsgStatusApproved (msgId) { + this._setMsgStatus(msgId, 'approved') + } + + setMsgStatusSigned (msgId, rawSig) { + const msg = this.getMsg(msgId) + msg.rawSig = rawSig + this._updateMsg(msg) + this._setMsgStatus(msgId, 'signed') + } + + prepMsgForSigning (msgParams) { + delete msgParams.metamaskId + return Promise.resolve(msgParams) + } + + rejectMsg (msgId) { + this._setMsgStatus(msgId, 'rejected') + } + + // + // PRIVATE METHODS + // + + _setMsgStatus (msgId, status) { + const msg = this.getMsg(msgId) + if (!msg) throw new Error('MessageManager - Message not found for id: "${msgId}".') + msg.status = status + this._updateMsg(msg) + this.emit(`${msgId}:${status}`, msg) + if (status === 'rejected' || status === 'signed') { + this.emit(`${msgId}:finished`, msg) + } + } + + _updateMsg (msg) { + const index = this.messages.findIndex((message) => message.id === msg.id) + if (index !== -1) { + this.messages[index] = msg + } + this._saveMsgList() + } + + _saveMsgList () { + const unapprovedPersonalMsgs = this.getUnapprovedMsgs() + const unapprovedPersonalMsgCount = Object.keys(unapprovedPersonalMsgs).length + this.memStore.updateState({ unapprovedPersonalMsgs, unapprovedPersonalMsgCount }) + this.emit('updateBadge') + } + +} + +function normalizeMsgData(data) { + if (data.slice(0, 2) === '0x') { + // data is already hex + return data + } else { + // data is unicode, convert to hex + return ethUtil.bufferToHex(new Buffer(data, 'utf8')) + } +} diff --git a/app/scripts/lib/sig-util.js b/app/scripts/lib/sig-util.js deleted file mode 100644 index 193dda381..000000000 --- a/app/scripts/lib/sig-util.js +++ /dev/null @@ -1,28 +0,0 @@ -const ethUtil = require('ethereumjs-util') - -module.exports = { - - concatSig: function (v, r, s) { - const rSig = ethUtil.fromSigned(r) - const sSig = ethUtil.fromSigned(s) - const vSig = ethUtil.bufferToInt(v) - const rStr = padWithZeroes(ethUtil.toUnsigned(rSig).toString('hex'), 64) - const sStr = padWithZeroes(ethUtil.toUnsigned(sSig).toString('hex'), 64) - const vStr = ethUtil.stripHexPrefix(ethUtil.intToHex(vSig)) - return ethUtil.addHexPrefix(rStr.concat(sStr, vStr)).toString('hex') - }, - - normalize: function (address) { - if (!address) return - return ethUtil.addHexPrefix(address.toLowerCase()) - }, - -} - -function padWithZeroes (number, length) { - var myString = '' + number - while (myString.length < length) { - myString = '0' + myString - } - return myString -} diff --git a/app/scripts/lib/tx-utils.js b/app/scripts/lib/tx-utils.js index 5116cb93b..240a6ab47 100644 --- a/app/scripts/lib/tx-utils.js +++ b/app/scripts/lib/tx-utils.js @@ -2,7 +2,7 @@ const async = require('async') const EthQuery = require('eth-query') const ethUtil = require('ethereumjs-util') const Transaction = require('ethereumjs-tx') -const normalize = require('./sig-util').normalize +const normalize = require('eth-sig-util').normalize const BN = ethUtil.BN /* diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 29b13dc62..62242bd83 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -16,6 +16,7 @@ const CurrencyController = require('./lib/controllers/currency') const NoticeController = require('./notice-controller') const ShapeShiftController = require('./lib/controllers/shapeshift') const MessageManager = require('./lib/message-manager') +const PersonalMessageManager = require('./lib/personal-message-manager') const TxManager = require('./transaction-manager') const ConfigManager = require('./lib/config-manager') const extension = require('./lib/extension') @@ -23,6 +24,7 @@ const autoFaucet = require('./lib/auto-faucet') const nodeify = require('./lib/nodeify') const IdStoreMigrator = require('./lib/idStore-migrator') const accountImporter = require('./account-import-strategies') +const sigUtil = require('eth-sig-util') const version = require('../manifest.json').version @@ -105,6 +107,7 @@ module.exports = class MetamaskController extends EventEmitter { this.lookupNetwork() this.messageManager = new MessageManager() + this.personalMessageManager = new PersonalMessageManager() this.publicConfigStore = this.initPublicConfigStore() // TEMPORARY UNTIL FULL DEPRECATION: @@ -163,8 +166,13 @@ module.exports = class MetamaskController extends EventEmitter { }, // tx signing processTransaction: (txParams, cb) => this.newUnapprovedTransaction(txParams, cb), - // msg signing + // old style msg signing processMessage: this.newUnsignedMessage.bind(this), + + // new style msg signing + approvePersonalMessage: this.approvePersonalMessage.bind(this), + signPersonalMessage: this.signPersonalMessage.bind(this), + personalRecoverSigner: this.personalRecoverSigner.bind(this), }) return provider } @@ -209,6 +217,7 @@ module.exports = class MetamaskController extends EventEmitter { this.ethStore.getState(), this.txManager.memStore.getState(), this.messageManager.memStore.getState(), + this.personalMessageManager.memStore.getState(), this.keyringController.memStore.getState(), this.preferencesController.store.getState(), this.currencyController.store.getState(), @@ -449,6 +458,44 @@ module.exports = class MetamaskController extends EventEmitter { )(cb) } + // Prefixed Style Message Signing Methods: + approvePersonalMessage (cb) { + let msgId = this.personalMessageManager.addUnapprovedMessage(msgParams) + this.sendUpdate() + this.opts.showUnconfirmedMessage() + this.personalMessageManager.once(`${msgId}:finished`, (data) => { + switch (data.status) { + case 'signed': + return cb(null, data.rawSig) + case 'rejected': + return cb(new Error('MetaMask Message Signature: User denied transaction signature.')) + default: + return cb(new Error(`MetaMask Message Signature: Unknown problem: ${JSON.stringify(msgParams)}`)) + } + }) + } + + signPersonalMessage (msgParams) { + const msgId = msgParams.metamaskId + // sets the status op the message to 'approved' + // and removes the metamaskId for signing + return this.personalMessageManager.approveMessage(msgParams) + .then((cleanMsgParams) => { + // signs the message + return this.keyringController.signPersonalMessage(cleanMsgParams) + }) + .then((rawSig) => { + // tells the listener that the message has been signed + // and can be returned to the dapp + this.personalMessageManager.setMsgStatusSigned(msgId, rawSig) + return rawSig + }) + } + + personalRecoverSigner (msgParams) { + const recovered = sigUtil.recoverPersonalSignature(msgParams) + return Promise.resolve(recovered) + } markAccountsFound (cb) { this.configManager.setLostAccounts([]) -- cgit From 92fb07999a011fa6939c0068f15dd55a6bcd7506 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Tue, 21 Feb 2017 14:30:07 -0800 Subject: Point metamask-controller personalSignRecover method to keyring-controller --- app/scripts/metamask-controller.js | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) (limited to 'app/scripts') diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 62242bd83..06c133bb2 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -24,7 +24,6 @@ const autoFaucet = require('./lib/auto-faucet') const nodeify = require('./lib/nodeify') const IdStoreMigrator = require('./lib/idStore-migrator') const accountImporter = require('./account-import-strategies') -const sigUtil = require('eth-sig-util') const version = require('../manifest.json').version @@ -152,6 +151,8 @@ module.exports = class MetamaskController extends EventEmitter { // initializeProvider () { + const keyringController = this.keyringController + let provider = MetaMaskProvider({ static: { eth_syncing: false, @@ -171,8 +172,8 @@ module.exports = class MetamaskController extends EventEmitter { // new style msg signing approvePersonalMessage: this.approvePersonalMessage.bind(this), - signPersonalMessage: this.signPersonalMessage.bind(this), - personalRecoverSigner: this.personalRecoverSigner.bind(this), + signPersonalMessage: nodeify(this.signPersonalMessage).bind(this), + personalRecoverSigner: nodeify(keyringController.recoverPersonalMessage).bind(keyringController), }) return provider } @@ -459,7 +460,7 @@ module.exports = class MetamaskController extends EventEmitter { } // Prefixed Style Message Signing Methods: - approvePersonalMessage (cb) { + approvePersonalMessage (msgParams, cb) { let msgId = this.personalMessageManager.addUnapprovedMessage(msgParams) this.sendUpdate() this.opts.showUnconfirmedMessage() @@ -492,11 +493,6 @@ module.exports = class MetamaskController extends EventEmitter { }) } - personalRecoverSigner (msgParams) { - const recovered = sigUtil.recoverPersonalSignature(msgParams) - return Promise.resolve(recovered) - } - markAccountsFound (cb) { this.configManager.setLostAccounts([]) this.sendUpdate() -- cgit From 6c0916c28dc3d4f8eb449f92393a70545481ce30 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Tue, 21 Feb 2017 14:37:01 -0800 Subject: Fix reference --- app/scripts/lib/idStore-migrator.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app/scripts') diff --git a/app/scripts/lib/idStore-migrator.js b/app/scripts/lib/idStore-migrator.js index 1485beb48..62d21eee7 100644 --- a/app/scripts/lib/idStore-migrator.js +++ b/app/scripts/lib/idStore-migrator.js @@ -1,5 +1,5 @@ const IdentityStore = require('./idStore') -const HdKeyring = require('../keyrings/hd') +const HdKeyring = require('eth-hd-keyring') const sigUtil = require('eth-sig-util') const normalize = sigUtil.normalize const denodeify = require('denodeify') -- cgit From 8684fc40c78cb5293d85f751cf3927c14067d343 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Tue, 21 Feb 2017 14:41:55 -0800 Subject: Allow provider to init before keyringController --- app/scripts/metamask-controller.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'app/scripts') diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 06c133bb2..d58d1c22d 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -151,7 +151,6 @@ module.exports = class MetamaskController extends EventEmitter { // initializeProvider () { - const keyringController = this.keyringController let provider = MetaMaskProvider({ static: { @@ -173,7 +172,7 @@ module.exports = class MetamaskController extends EventEmitter { // new style msg signing approvePersonalMessage: this.approvePersonalMessage.bind(this), signPersonalMessage: nodeify(this.signPersonalMessage).bind(this), - personalRecoverSigner: nodeify(keyringController.recoverPersonalMessage).bind(keyringController), + personalRecoverSigner: nodeify(this.recoverPersonalMessage).bind(this), }) return provider } @@ -493,6 +492,11 @@ module.exports = class MetamaskController extends EventEmitter { }) } + recoverPersonalMessage (msgParams) { + const keyringController = this.keyringController + return keyringController.recoverPersonalMessage(msgParams) + } + markAccountsFound (cb) { this.configManager.setLostAccounts([]) this.sendUpdate() -- cgit From 564f920ae0a1be1aa08905f1b4cf6d081e9a5a0b Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 22 Feb 2017 16:23:13 -0800 Subject: Add personal sign actions and template --- app/scripts/metamask-controller.js | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'app/scripts') diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index d58d1c22d..b109918cf 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -241,6 +241,7 @@ module.exports = class MetamaskController extends EventEmitter { const preferencesController = this.preferencesController const txManager = this.txManager const messageManager = this.messageManager + const personalMessageManager = this.personalMessageManager const noticeController = this.noticeController return { @@ -285,6 +286,10 @@ module.exports = class MetamaskController extends EventEmitter { signMessage: this.signMessage.bind(this), cancelMessage: messageManager.rejectMsg.bind(messageManager), + // personalMessageManager + signPersonalMessage: this.signPersonalMessage.bind(this), + cancelPersonalMessage: personalMessageManager.rejectMsg.bind(personalMessageManager), + // notices checkNotices: noticeController.updateNoticesList.bind(noticeController), markNoticeRead: noticeController.markNoticeRead.bind(noticeController), -- cgit From 7ec25526b70473247a69ab4a3a1302e50b06f12b Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Thu, 23 Feb 2017 11:18:49 -0800 Subject: Add alternate UI for pending personal_sign messages --- app/scripts/lib/message-manager.js | 3 ++- app/scripts/lib/personal-message-manager.js | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) (limited to 'app/scripts') diff --git a/app/scripts/lib/message-manager.js b/app/scripts/lib/message-manager.js index ceaf8ee2f..711d5f159 100644 --- a/app/scripts/lib/message-manager.js +++ b/app/scripts/lib/message-manager.js @@ -33,6 +33,7 @@ module.exports = class MessageManager extends EventEmitter{ msgParams: msgParams, time: time, status: 'unapproved', + type: 'eth_sign', } this.addMsg(msgData) @@ -115,4 +116,4 @@ function normalizeMsgData(data) { // data is unicode, convert to hex return ethUtil.bufferToHex(new Buffer(data, 'utf8')) } -} \ No newline at end of file +} diff --git a/app/scripts/lib/personal-message-manager.js b/app/scripts/lib/personal-message-manager.js index 72dd1da96..65ad9200a 100644 --- a/app/scripts/lib/personal-message-manager.js +++ b/app/scripts/lib/personal-message-manager.js @@ -33,6 +33,7 @@ module.exports = class MessageManager extends EventEmitter{ msgParams: msgParams, time: time, status: 'unapproved', + type: 'personal_sign', } this.addMsg(msgData) -- cgit From 4697aca02c669b1787e72f0648b3043270867799 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Thu, 23 Feb 2017 14:23:45 -0800 Subject: Got personal_sign working Also fixed bug where signing would not close popup. --- app/scripts/background.js | 4 +++ app/scripts/lib/personal-message-manager.js | 4 +-- app/scripts/metamask-controller.js | 55 +++++++++++++++++++---------- 3 files changed, 42 insertions(+), 21 deletions(-) (limited to 'app/scripts') diff --git a/app/scripts/background.js b/app/scripts/background.js index 2e5a992b9..254737dec 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -15,6 +15,10 @@ const firstTimeState = require('./first-time-state') const STORAGE_KEY = 'metamask-config' const METAMASK_DEBUG = 'GULP_METAMASK_DEBUG' +const log = require('loglevel') +window.log = log +log.setDefaultLevel(METAMASK_DEBUG ? 'debug' : 'warn') + let popupIsOpen = false // state persistence diff --git a/app/scripts/lib/personal-message-manager.js b/app/scripts/lib/personal-message-manager.js index 65ad9200a..3b8510767 100644 --- a/app/scripts/lib/personal-message-manager.js +++ b/app/scripts/lib/personal-message-manager.js @@ -4,7 +4,7 @@ const ethUtil = require('ethereumjs-util') const createId = require('./random-id') -module.exports = class MessageManager extends EventEmitter{ +module.exports = class PersonalMessageManager extends EventEmitter{ constructor (opts) { super() this.memStore = new ObservableStore({ @@ -82,7 +82,7 @@ module.exports = class MessageManager extends EventEmitter{ _setMsgStatus (msgId, status) { const msg = this.getMsg(msgId) - if (!msg) throw new Error('MessageManager - Message not found for id: "${msgId}".') + if (!msg) throw new Error('PersonalMessageManager - Message not found for id: "${msgId}".') msg.status = status this._updateMsg(msg) this.emit(`${msgId}:${status}`, msg) diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index b109918cf..c301e2035 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -170,8 +170,7 @@ module.exports = class MetamaskController extends EventEmitter { processMessage: this.newUnsignedMessage.bind(this), // new style msg signing - approvePersonalMessage: this.approvePersonalMessage.bind(this), - signPersonalMessage: nodeify(this.signPersonalMessage).bind(this), + processPersonalMessage: this.newUnsignedPersonalMessage.bind(this), personalRecoverSigner: nodeify(this.recoverPersonalMessage).bind(this), }) return provider @@ -283,11 +282,11 @@ module.exports = class MetamaskController extends EventEmitter { cancelTransaction: txManager.cancelTransaction.bind(txManager), // messageManager - signMessage: this.signMessage.bind(this), + signMessage: nodeify(this.signMessage).bind(this), cancelMessage: messageManager.rejectMsg.bind(messageManager), // personalMessageManager - signPersonalMessage: this.signPersonalMessage.bind(this), + signPersonalMessage: nodeify(this.signPersonalMessage).bind(this), cancelPersonalMessage: personalMessageManager.rejectMsg.bind(personalMessageManager), // notices @@ -445,22 +444,39 @@ module.exports = class MetamaskController extends EventEmitter { }) } + newUnsignedPersonalMessage (msgParams, cb) { + let msgId = this.personalMessageManager.addUnapprovedMessage(msgParams) + this.sendUpdate() + this.opts.showUnconfirmedMessage() + this.personalMessageManager.once(`${msgId}:finished`, (data) => { + switch (data.status) { + case 'signed': + return cb(null, data.rawSig) + case 'rejected': + return cb(new Error('MetaMask Message Signature: User denied transaction signature.')) + default: + return cb(new Error(`MetaMask Message Signature: Unknown problem: ${JSON.stringify(msgParams)}`)) + } + }) + } + signMessage (msgParams, cb) { + log.info('MetaMaskController - signMessage') const msgId = msgParams.metamaskId - promiseToCallback( - // sets the status op the message to 'approved' - // and removes the metamaskId for signing - this.messageManager.approveMessage(msgParams) - .then((cleanMsgParams) => { - // signs the message - return this.keyringController.signMessage(cleanMsgParams) - }) - .then((rawSig) => { - // tells the listener that the message has been signed - // and can be returned to the dapp - this.messageManager.setMsgStatusSigned(msgId, rawSig) - }) - )(cb) + + // sets the status op the message to 'approved' + // and removes the metamaskId for signing + return this.messageManager.approveMessage(msgParams) + .then((cleanMsgParams) => { + // signs the message + return this.keyringController.signMessage(cleanMsgParams) + }) + .then((rawSig) => { + // tells the listener that the message has been signed + // and can be returned to the dapp + this.messageManager.setMsgStatusSigned(msgId, rawSig) + return this.getState() + }) } // Prefixed Style Message Signing Methods: @@ -481,6 +497,7 @@ module.exports = class MetamaskController extends EventEmitter { } signPersonalMessage (msgParams) { + log.info('MetaMaskController - signPersonalMessage') const msgId = msgParams.metamaskId // sets the status op the message to 'approved' // and removes the metamaskId for signing @@ -493,7 +510,7 @@ module.exports = class MetamaskController extends EventEmitter { // tells the listener that the message has been signed // and can be returned to the dapp this.personalMessageManager.setMsgStatusSigned(msgId, rawSig) - return rawSig + return this.getState() }) } -- cgit From 961a83769bd46334f5ecf72d00a32730d19866c3 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Thu, 23 Feb 2017 16:00:43 -0800 Subject: Fix cancel msg signing behavior. --- app/scripts/metamask-controller.js | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) (limited to 'app/scripts') diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index c301e2035..eace72c24 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -139,6 +139,7 @@ module.exports = class MetamaskController extends EventEmitter { this.ethStore.subscribe(this.sendUpdate.bind(this)) this.txManager.memStore.subscribe(this.sendUpdate.bind(this)) this.messageManager.memStore.subscribe(this.sendUpdate.bind(this)) + this.personalMessageManager.memStore.subscribe(this.sendUpdate.bind(this)) this.keyringController.memStore.subscribe(this.sendUpdate.bind(this)) this.preferencesController.store.subscribe(this.sendUpdate.bind(this)) this.currencyController.store.subscribe(this.sendUpdate.bind(this)) @@ -239,8 +240,6 @@ module.exports = class MetamaskController extends EventEmitter { const keyringController = this.keyringController const preferencesController = this.preferencesController const txManager = this.txManager - const messageManager = this.messageManager - const personalMessageManager = this.personalMessageManager const noticeController = this.noticeController return { @@ -283,11 +282,11 @@ module.exports = class MetamaskController extends EventEmitter { // messageManager signMessage: nodeify(this.signMessage).bind(this), - cancelMessage: messageManager.rejectMsg.bind(messageManager), + cancelMessage: this.cancelMessage.bind(this), // personalMessageManager signPersonalMessage: nodeify(this.signPersonalMessage).bind(this), - cancelPersonalMessage: personalMessageManager.rejectMsg.bind(personalMessageManager), + cancelPersonalMessage: this.cancelPersonalMessage.bind(this), // notices checkNotices: noticeController.updateNoticesList.bind(noticeController), @@ -437,7 +436,7 @@ module.exports = class MetamaskController extends EventEmitter { case 'signed': return cb(null, data.rawSig) case 'rejected': - return cb(new Error('MetaMask Message Signature: User denied transaction signature.')) + return cb(new Error('MetaMask Message Signature: User denied message signature.')) default: return cb(new Error(`MetaMask Message Signature: Unknown problem: ${JSON.stringify(msgParams)}`)) } @@ -445,6 +444,10 @@ module.exports = class MetamaskController extends EventEmitter { } newUnsignedPersonalMessage (msgParams, cb) { + if (!msgParams.from) { + return cb(new Error('MetaMask Message Signature: from field is required.')) + } + let msgId = this.personalMessageManager.addUnapprovedMessage(msgParams) this.sendUpdate() this.opts.showUnconfirmedMessage() @@ -453,7 +456,7 @@ module.exports = class MetamaskController extends EventEmitter { case 'signed': return cb(null, data.rawSig) case 'rejected': - return cb(new Error('MetaMask Message Signature: User denied transaction signature.')) + return cb(new Error('MetaMask Message Signature: User denied message signature.')) default: return cb(new Error(`MetaMask Message Signature: Unknown problem: ${JSON.stringify(msgParams)}`)) } @@ -479,6 +482,14 @@ module.exports = class MetamaskController extends EventEmitter { }) } + cancelMessage(msgId, cb) { + const messageManager = this.messageManager + messageManager.rejectMsg(msgId) + if (cb && typeof cb === 'function') { + cb(null, this.getState()) + } + } + // Prefixed Style Message Signing Methods: approvePersonalMessage (msgParams, cb) { let msgId = this.personalMessageManager.addUnapprovedMessage(msgParams) @@ -514,6 +525,14 @@ module.exports = class MetamaskController extends EventEmitter { }) } + cancelPersonalMessage(msgId, cb) { + const messageManager = this.personalMessageManager + messageManager.rejectMsg(msgId) + if (cb && typeof cb === 'function') { + cb(null, this.getState()) + } + } + recoverPersonalMessage (msgParams) { const keyringController = this.keyringController return keyringController.recoverPersonalMessage(msgParams) -- cgit From f2851402f39a12c2a11e5dea7312c55b81c481cb Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Fri, 24 Feb 2017 16:36:29 -0800 Subject: Mostly fix personal_recover --- app/scripts/keyring-controller.js | 11 +++++------ app/scripts/metamask-controller.js | 1 + 2 files changed, 6 insertions(+), 6 deletions(-) (limited to 'app/scripts') diff --git a/app/scripts/keyring-controller.js b/app/scripts/keyring-controller.js index 8c379b5b9..f2891db37 100644 --- a/app/scripts/keyring-controller.js +++ b/app/scripts/keyring-controller.js @@ -5,7 +5,8 @@ const EventEmitter = require('events').EventEmitter const ObservableStore = require('obs-store') const filter = require('promise-filter') const encryptor = require('browser-passworder') -const normalizeAddress = require('eth-sig-util').normalize +const sigUtil = require('eth-sig-util') +const normalizeAddress = sigUtil.normalize // Keyrings: const SimpleKeyring = require('eth-simple-keyring') const HdKeyring = require('eth-hd-keyring') @@ -284,11 +285,8 @@ class KeyringController extends EventEmitter { // // recovers a signature of the prefixed-style personalMessage signature. recoverPersonalMessage (msgParams) { - const address = normalizeAddress(msgParams.from) - return this.getKeyringForAccount(address) - .then((keyring) => { - return keyring.recoverPersonalMessage(address, msgParams.data) - }) + const address = sigUtil.recoverPersonalSignature(msgParams) + return Promise.resolve(address) } // PRIVATE METHODS @@ -500,6 +498,7 @@ class KeyringController extends EventEmitter { // 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([ diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index eace72c24..995db1c0a 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -534,6 +534,7 @@ module.exports = class MetamaskController extends EventEmitter { } recoverPersonalMessage (msgParams) { + log.debug(`MetaMaskController - recoverPersonalMessage: ${JSON.stringify(msgParams)}`) const keyringController = this.keyringController return keyringController.recoverPersonalMessage(msgParams) } -- cgit From 8c66260bdb903185aaf55944a04b850af2dd64b6 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Fri, 24 Feb 2017 17:07:54 -0800 Subject: Removed redundant personal_recover logic --- app/scripts/keyring-controller.js | 11 ----------- app/scripts/metamask-controller.js | 7 ------- 2 files changed, 18 deletions(-) (limited to 'app/scripts') diff --git a/app/scripts/keyring-controller.js b/app/scripts/keyring-controller.js index f2891db37..e1b1c4335 100644 --- a/app/scripts/keyring-controller.js +++ b/app/scripts/keyring-controller.js @@ -278,17 +278,6 @@ class KeyringController extends EventEmitter { }) } - // Recover Personal Message - // @object msgParams - // - // returns Promise(@buffer signer) - // - // recovers a signature of the prefixed-style personalMessage signature. - recoverPersonalMessage (msgParams) { - const address = sigUtil.recoverPersonalSignature(msgParams) - return Promise.resolve(address) - } - // PRIVATE METHODS // // THESE METHODS ARE ONLY USED INTERNALLY TO THE KEYRING-CONTROLLER diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 995db1c0a..f172c67a8 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -172,7 +172,6 @@ module.exports = class MetamaskController extends EventEmitter { // new style msg signing processPersonalMessage: this.newUnsignedPersonalMessage.bind(this), - personalRecoverSigner: nodeify(this.recoverPersonalMessage).bind(this), }) return provider } @@ -533,12 +532,6 @@ module.exports = class MetamaskController extends EventEmitter { } } - recoverPersonalMessage (msgParams) { - log.debug(`MetaMaskController - recoverPersonalMessage: ${JSON.stringify(msgParams)}`) - const keyringController = this.keyringController - return keyringController.recoverPersonalMessage(msgParams) - } - markAccountsFound (cb) { this.configManager.setLostAccounts([]) this.sendUpdate() -- cgit