From c2b8dada91c90788dcd81a0318c52a66b4b769d1 Mon Sep 17 00:00:00 2001 From: Sergey Ukustov Date: Fri, 29 Sep 2017 19:24:08 +0300 Subject: Add eth_signTypedData handler --- app/scripts/background.js | 3 +- app/scripts/lib/typed-message-manager.js | 108 +++++++++++++++++++++++++++++++ app/scripts/metamask-controller.js | 52 +++++++++++++++ 3 files changed, 162 insertions(+), 1 deletion(-) create mode 100644 app/scripts/lib/typed-message-manager.js (limited to 'app') diff --git a/app/scripts/background.js b/app/scripts/background.js index 195881e15..3e560d302 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -124,7 +124,8 @@ function setupController (initState) { var unapprovedTxCount = controller.txController.getUnapprovedTxCount() var unapprovedMsgCount = controller.messageManager.unapprovedMsgCount var unapprovedPersonalMsgs = controller.personalMessageManager.unapprovedPersonalMsgCount - var count = unapprovedTxCount + unapprovedMsgCount + unapprovedPersonalMsgs + var unapprovedTypedMsgs = controller.typedMessageManager.unapprovedTypedMessagesCount + var count = unapprovedTxCount + unapprovedMsgCount + unapprovedPersonalMsgs + unapprovedTypedMsgs if (count) { label = String(count) } diff --git a/app/scripts/lib/typed-message-manager.js b/app/scripts/lib/typed-message-manager.js new file mode 100644 index 000000000..e3efdb45d --- /dev/null +++ b/app/scripts/lib/typed-message-manager.js @@ -0,0 +1,108 @@ +const EventEmitter = require('events') +const ObservableStore = require('obs-store') +const createId = require('./random-id') + + +module.exports = class TypedMessageManager extends EventEmitter { + constructor (opts) { + super() + this.memStore = new ObservableStore({ + unapprovedTypedMessages: {}, + unapprovedTypedMessagesCount: 0, + }) + this.messages = [] + } + + get unapprovedTypedMessagesCount () { + 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) { + log.debug(`TypedMessageManager addUnapprovedMessage: ${JSON.stringify(msgParams)}`) + // 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', + type: 'eth_signTypedData', + } + 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('TypedMessageManager - 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 unapprovedTypedMessages = this.getUnapprovedMsgs() + const unapprovedTypedMessagesCount = Object.keys(unapprovedTypedMessages).length + this.memStore.updateState({ unapprovedTypedMessages, unapprovedTypedMessagesCount }) + this.emit('updateBadge') + } + +} diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 5b3161bc6..0eeb708fc 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -25,6 +25,7 @@ const InfuraController = require('./controllers/infura') const BlacklistController = require('./controllers/blacklist') const MessageManager = require('./lib/message-manager') const PersonalMessageManager = require('./lib/personal-message-manager') +const TypedMessageManager = require('./lib/typed-message-manager') const TransactionController = require('./controllers/transactions') const BalancesController = require('./controllers/computed-balances') const ConfigManager = require('./lib/config-manager') @@ -154,6 +155,7 @@ module.exports = class MetamaskController extends EventEmitter { this.networkController.lookupNetwork() this.messageManager = new MessageManager() this.personalMessageManager = new PersonalMessageManager() + this.typedMessageManager = new TypedMessageManager() this.publicConfigStore = this.initPublicConfigStore() // manual disk state subscriptions @@ -195,6 +197,7 @@ module.exports = class MetamaskController extends EventEmitter { this.balancesController.store.subscribe(this.sendUpdate.bind(this)) this.messageManager.memStore.subscribe(this.sendUpdate.bind(this)) this.personalMessageManager.memStore.subscribe(this.sendUpdate.bind(this)) + this.typedMessageManager.memStore.subscribe(this.sendUpdate.bind(this)) this.keyringController.memStore.subscribe(this.sendUpdate.bind(this)) this.preferencesController.store.subscribe(this.sendUpdate.bind(this)) this.addressBookController.store.subscribe(this.sendUpdate.bind(this)) @@ -234,6 +237,7 @@ module.exports = class MetamaskController extends EventEmitter { processMessage: this.newUnsignedMessage.bind(this), // personal_sign msg signing processPersonalMessage: this.newUnsignedPersonalMessage.bind(this), + processTypedMessage: this.newUnsignedTypedMessage.bind(this), }) } @@ -276,6 +280,7 @@ module.exports = class MetamaskController extends EventEmitter { this.txController.memStore.getState(), this.messageManager.memStore.getState(), this.personalMessageManager.memStore.getState(), + this.typedMessageManager.memStore.getState(), this.keyringController.memStore.getState(), this.balancesController.store.getState(), this.preferencesController.store.getState(), @@ -354,6 +359,10 @@ module.exports = class MetamaskController extends EventEmitter { signPersonalMessage: nodeify(this.signPersonalMessage, this), cancelPersonalMessage: this.cancelPersonalMessage.bind(this), + // personalMessageManager + signTypedMessage: nodeify(this.signTypedMessage, this), + cancelTypedMessage: this.cancelTypedMessage.bind(this), + // notices checkNotices: noticeController.updateNoticesList.bind(noticeController), markNoticeRead: noticeController.markNoticeRead.bind(noticeController), @@ -546,6 +555,23 @@ module.exports = class MetamaskController extends EventEmitter { }) } + newUnsignedTypedMessage (msgParams, cb) { + const msgId = this.typedMessageManager.addUnapprovedMessage(msgParams) + this.sendUpdate() + this.opts.showUnconfirmedMessage() + this.typedMessageManager.once(`${msgId}:finished`, (data) => { + console.log(data) + switch (data.status) { + case 'signed': + return cb(null, data.rawSig) + case 'rejected': + return cb(new Error('MetaMask Message Signature: User denied message 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 @@ -608,6 +634,24 @@ module.exports = class MetamaskController extends EventEmitter { }) } + signTypedMessage (msgParams) { + log.info('MetaMaskController - signTypedMessage') + const msgId = msgParams.metamaskId + // sets the status op the message to 'approved' + // and removes the metamaskId for signing + return this.typedMessageManager.approveMessage(msgParams) + .then((cleanMsgParams) => { + // signs the message + return this.keyringController.signTypedMessage(cleanMsgParams) + }) + .then((rawSig) => { + // tells the listener that the message has been signed + // and can be returned to the dapp + this.typedMessageManager.setMsgStatusSigned(msgId, rawSig) + return this.getState() + }) + } + cancelPersonalMessage (msgId, cb) { const messageManager = this.personalMessageManager messageManager.rejectMsg(msgId) @@ -616,6 +660,14 @@ module.exports = class MetamaskController extends EventEmitter { } } + cancelTypedMessage (msgId, cb) { + const messageManager = this.typedMessageManager + messageManager.rejectMsg(msgId) + if (cb && typeof cb === 'function') { + cb(null, this.getState()) + } + } + markAccountsFound (cb) { this.configManager.setLostAccounts([]) this.sendUpdate() -- cgit From 9bc80d998eda937e3a8f95fa5e04fcba66e8a6f8 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Thu, 5 Oct 2017 14:39:35 -0700 Subject: Add signTypedData input validations --- app/scripts/lib/typed-message-manager.js | 11 +++++++++++ app/scripts/metamask-controller.js | 13 +++++++++---- 2 files changed, 20 insertions(+), 4 deletions(-) (limited to 'app') diff --git a/app/scripts/lib/typed-message-manager.js b/app/scripts/lib/typed-message-manager.js index e3efdb45d..e041ae9f3 100644 --- a/app/scripts/lib/typed-message-manager.js +++ b/app/scripts/lib/typed-message-manager.js @@ -1,6 +1,7 @@ const EventEmitter = require('events') const ObservableStore = require('obs-store') const createId = require('./random-id') +const assert = require('assert') module.exports = class TypedMessageManager extends EventEmitter { @@ -23,6 +24,8 @@ module.exports = class TypedMessageManager extends EventEmitter { } addUnapprovedMessage (msgParams) { + this.validateParams(msgParams) + log.debug(`TypedMessageManager addUnapprovedMessage: ${JSON.stringify(msgParams)}`) // create txData obj with parameters and meta data var time = (new Date()).getTime() @@ -41,6 +44,14 @@ module.exports = class TypedMessageManager extends EventEmitter { return msgId } + validateParams (params) { + assert.equal(typeof params, 'object', 'Params should ben an object.') + assert.ok('data' in params, 'Params must include a data field.') + assert.ok('from' in params, 'Params must include a from field.') + assert.ok(Array.isArray(params.data), 'Data should be an array.') + assert.equal(typeof params.from, 'string', 'From field must be a string.') + } + addMsg (msg) { this.messages.push(msg) this._saveMsgList() diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 8f773a72b..727f48f1c 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -566,11 +566,16 @@ module.exports = class MetamaskController extends EventEmitter { } newUnsignedTypedMessage (msgParams, cb) { - const msgId = this.typedMessageManager.addUnapprovedMessage(msgParams) - this.sendUpdate() - this.opts.showUnconfirmedMessage() + let msgId + try { + msgId = this.typedMessageManager.addUnapprovedMessage(msgParams) + this.sendUpdate() + this.opts.showUnconfirmedMessage() + } catch (e) { + return cb(e) + } + this.typedMessageManager.once(`${msgId}:finished`, (data) => { - console.log(data) switch (data.status) { case 'signed': return cb(null, data.rawSig) -- cgit From 0146b55d6d7bd8717b3f3ad071c64744e21a93fd Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Fri, 6 Oct 2017 11:33:14 -0700 Subject: Check status of pending transactions on startup Fixes #1531 --- app/scripts/lib/pending-tx-tracker.js | 1 + 1 file changed, 1 insertion(+) (limited to 'app') diff --git a/app/scripts/lib/pending-tx-tracker.js b/app/scripts/lib/pending-tx-tracker.js index 8a626e222..5049cc4b4 100644 --- a/app/scripts/lib/pending-tx-tracker.js +++ b/app/scripts/lib/pending-tx-tracker.js @@ -26,6 +26,7 @@ module.exports = class PendingTransactionTracker extends EventEmitter { this.retryTimePeriod = config.retryTimePeriod || 86400000 this.getPendingTransactions = config.getPendingTransactions this.publishTransaction = config.publishTransaction + this._checkPendingTxs() } // checks if a signed tx is in a block and -- cgit From be4f7b33f4f0885f2c0f5f4d537f6e9793f3fa30 Mon Sep 17 00:00:00 2001 From: kumavis Date: Fri, 6 Oct 2017 12:36:08 -0700 Subject: nodeify - allow callback to be optional --- app/scripts/lib/nodeify.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) (limited to 'app') diff --git a/app/scripts/lib/nodeify.js b/app/scripts/lib/nodeify.js index 832d6c6d3..19c3c8337 100644 --- a/app/scripts/lib/nodeify.js +++ b/app/scripts/lib/nodeify.js @@ -1,10 +1,18 @@ const promiseToCallback = require('promise-to-callback') +const noop = function(){} module.exports = function nodeify (fn, context) { return function(){ const args = [].slice.call(arguments) - const callback = args.pop() - if (typeof callback !== 'function') throw new Error('callback is not a function') + const lastArg = args[args.length-1] + const lastArgIsCallback = typeof lastArg === 'function' + let callback + if (lastArgIsCallback) { + callback = lastArg + args.pop() + } else { + callback = noop + } promiseToCallback(fn.apply(context, args))(callback) } } -- cgit From 94513cae7bf3c8310ae6a248e12a9b7dd73e306f Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Fri, 6 Oct 2017 12:50:33 -0700 Subject: Provide method for tx tracker to refer to all txs --- app/scripts/controllers/transactions.js | 1 + app/scripts/lib/tx-state-manager.js | 8 +++++++- 2 files changed, 8 insertions(+), 1 deletion(-) (limited to 'app') diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index a0f983deb..ef659a300 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -62,6 +62,7 @@ module.exports = class TransactionController extends EventEmitter { retryTimePeriod: 86400000, // Retry 3500 blocks, or about 1 day. publishTransaction: (rawTx) => this.query.sendRawTransaction(rawTx), getPendingTransactions: this.txStateManager.getPendingTransactions.bind(this.txStateManager), + getCompletedTransactions: this.txStateManager.getConfirmedTransactions.bind(this.txStateManager), }) this.txStateManager.store.subscribe(() => this.emit('update:badge')) diff --git a/app/scripts/lib/tx-state-manager.js b/app/scripts/lib/tx-state-manager.js index cf8117864..2250403f6 100644 --- a/app/scripts/lib/tx-state-manager.js +++ b/app/scripts/lib/tx-state-manager.js @@ -46,6 +46,12 @@ module.exports = class TransactionStateManger extends EventEmitter { return this.getFilteredTxList(opts) } + getConfirmedTransactions (address) { + const opts = { status: 'confirmed' } + if (address) opts.from = address + return this.getFilteredTxList(opts) + } + addTx (txMeta) { this.once(`${txMeta.id}:signed`, function (txId) { this.removeAllListeners(`${txMeta.id}:rejected`) @@ -242,4 +248,4 @@ module.exports = class TransactionStateManger extends EventEmitter { _saveTxList (transactions) { this.store.updateState({ transactions }) } -} \ No newline at end of file +} -- cgit From a417fab0ebd71d22f51a8e30590c259b32164fd2 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Fri, 6 Oct 2017 12:51:13 -0700 Subject: When checking pending txs, check for successful txs with same nonce. If a successful tx with the same nonce exists, transition tx to the failed state. Fixes #2294 --- app/scripts/lib/pending-tx-tracker.js | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) (limited to 'app') diff --git a/app/scripts/lib/pending-tx-tracker.js b/app/scripts/lib/pending-tx-tracker.js index 8a626e222..2d8f22ae8 100644 --- a/app/scripts/lib/pending-tx-tracker.js +++ b/app/scripts/lib/pending-tx-tracker.js @@ -25,6 +25,7 @@ module.exports = class PendingTransactionTracker extends EventEmitter { // default is one day this.retryTimePeriod = config.retryTimePeriod || 86400000 this.getPendingTransactions = config.getPendingTransactions + this.getCompletedTransactions = config.getCompletedTransactions this.publishTransaction = config.publishTransaction } @@ -120,6 +121,7 @@ module.exports = class PendingTransactionTracker extends EventEmitter { async _checkPendingTx (txMeta) { const txHash = txMeta.hash const txId = txMeta.id + // extra check in case there was an uncaught error during the // signature and submission process if (!txHash) { @@ -128,6 +130,15 @@ module.exports = class PendingTransactionTracker extends EventEmitter { this.emit('tx:failed', txId, noTxHashErr) return } + + // If another tx with the same nonce is mined, set as failed. + const taken = await this._checkIfNonceIsTaken(txMeta) + if (taken) { + const nonceTakenErr = new Error('Another transaction with this nonce has been mined.') + nonceTakenErr.name = 'NonceTakenErr' + return this.emit('tx:failed', txId, nonceTakenErr) + } + // get latest transaction status let txParams try { @@ -159,4 +170,13 @@ module.exports = class PendingTransactionTracker extends EventEmitter { } nonceGlobalLock.releaseLock() } + + async _checkIfNonceIsTaken (txMeta) { + const completed = this.getCompletedTransactions() + const sameNonce = completed.filter((otherMeta) => { + return otherMeta.txParams.nonce === txMeta.txParams.nonce + }) + return sameNonce.length > 0 + } + } -- cgit From bc396a7417ecfe9855ec84af0cb08fd033c42bf5 Mon Sep 17 00:00:00 2001 From: kumavis Date: Fri, 6 Oct 2017 13:02:34 -0700 Subject: lint fix - nodeify --- app/scripts/lib/nodeify.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app') diff --git a/app/scripts/lib/nodeify.js b/app/scripts/lib/nodeify.js index 19c3c8337..d24e92206 100644 --- a/app/scripts/lib/nodeify.js +++ b/app/scripts/lib/nodeify.js @@ -4,7 +4,7 @@ const noop = function(){} module.exports = function nodeify (fn, context) { return function(){ const args = [].slice.call(arguments) - const lastArg = args[args.length-1] + const lastArg = args[args.length - 1] const lastArgIsCallback = typeof lastArg === 'function' let callback if (lastArgIsCallback) { -- cgit From a1696f89a8764f17c10298a45160abf8fc7dce5e Mon Sep 17 00:00:00 2001 From: Sergey Ukustov Date: Sat, 7 Oct 2017 00:38:13 +0300 Subject: Validate data format for eth_signTypedData --- app/scripts/lib/typed-message-manager.js | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'app') diff --git a/app/scripts/lib/typed-message-manager.js b/app/scripts/lib/typed-message-manager.js index e041ae9f3..8b760790e 100644 --- a/app/scripts/lib/typed-message-manager.js +++ b/app/scripts/lib/typed-message-manager.js @@ -2,6 +2,7 @@ const EventEmitter = require('events') const ObservableStore = require('obs-store') const createId = require('./random-id') const assert = require('assert') +const sigUtil = require('eth-sig-util') module.exports = class TypedMessageManager extends EventEmitter { @@ -50,6 +51,9 @@ module.exports = class TypedMessageManager extends EventEmitter { assert.ok('from' in params, 'Params must include a from field.') assert.ok(Array.isArray(params.data), 'Data should be an array.') assert.equal(typeof params.from, 'string', 'From field must be a string.') + assert.doesNotThrow(() => { + sigUtil.typedSignatureHash(params.data) + }, 'Expected EIP712 typed data') } addMsg (msg) { -- cgit