diff options
-rw-r--r-- | app/scripts/keyring-controller.js | 10 | ||||
-rw-r--r-- | app/scripts/lib/tx-utils.js | 48 | ||||
-rw-r--r-- | app/scripts/metamask-controller.js | 55 | ||||
-rw-r--r-- | app/scripts/transaction-manager.js | 181 | ||||
-rw-r--r-- | package.json | 3 | ||||
-rw-r--r-- | test/unit/metamask-controller-test.js | 18 | ||||
-rw-r--r-- | test/unit/tx-manager-test.js | 31 |
7 files changed, 215 insertions, 131 deletions
diff --git a/app/scripts/keyring-controller.js b/app/scripts/keyring-controller.js index d4c0d863e..df2910187 100644 --- a/app/scripts/keyring-controller.js +++ b/app/scripts/keyring-controller.js @@ -317,13 +317,11 @@ module.exports = class KeyringController extends EventEmitter { // This method signs tx and returns a promise for // TX Manager to update the state after signing - signTransaction (ethTx, selectedAddress, txId) { - const address = normalize(selectedAddress) - return this.getKeyringForAccount(address) + signTransaction (ethTx, _fromAddress) { + const fromAddress = normalize(_fromAddress) + return this.getKeyringForAccount(fromAddress) .then((keyring) => { - return keyring.signTransaction(address, ethTx) - }).then((tx) => { - return {tx, txId} + return keyring.signTransaction(fromAddress, ethTx) }) } // Add Unconfirmed Message diff --git a/app/scripts/lib/tx-utils.js b/app/scripts/lib/tx-utils.js index d1fb98f42..eba537d0a 100644 --- a/app/scripts/lib/tx-utils.js +++ b/app/scripts/lib/tx-utils.js @@ -1,6 +1,8 @@ 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 BN = ethUtil.BN /* @@ -14,6 +16,7 @@ module.exports = class txProviderUtils { this.provider = provider this.query = new EthQuery(provider) } + analyzeGasUsage (txData, cb) { var self = this this.query.getBlockByNumber('latest', true, (err, block) => { @@ -71,4 +74,49 @@ module.exports = class txProviderUtils { const correct = bnGas.add(gasBuffer) return ethUtil.addHexPrefix(correct.toString(16)) } + + fillInTxParams (txParams, cb) { + let fromAddress = txParams.from + let reqs = {} + + if (isUndef(txParams.gas)) reqs.gas = (cb) => this.query.estimateGas(txParams, cb) + if (isUndef(txParams.gasPrice)) reqs.gasPrice = (cb) => this.query.gasPrice(cb) + if (isUndef(txParams.nonce)) reqs.nonce = (cb) => this.query.getTransactionCount(fromAddress, 'pending', cb) + + async.parallel(reqs, function(err, result) { + if (err) return cb(err) + // write results to txParams obj + Object.assign(txParams, result) + cb() + }) + } + + // builds ethTx from txParams object + buildEthTxFromParams (txParams, gasMultiplier = 1) { + // apply gas multiplyer + let gasPrice = new BN(ethUtil.stripHexPrefix(txParams.gasPrice), 16) + // multiply and divide by 100 so as to add percision to integer mul + gasPrice = gasPrice.mul(new BN(gasMultiplier * 100, 10)).div(new BN(100, 10)) + txParams.gasPrice = ethUtil.intToHex(gasPrice.toNumber()) + // normalize values + txParams.to = normalize(txParams.to) + txParams.from = normalize(txParams.from) + txParams.value = normalize(txParams.value) + txParams.data = normalize(txParams.data) + txParams.gasLimit = normalize(txParams.gasLimit || txParams.gas) + txParams.nonce = normalize(txParams.nonce) + // build ethTx + const ethTx = new Transaction(txParams) + return ethTx + } + + publishTransaction (rawTx, cb) { + this.query.sendRawTransaction(rawTx, cb) + } } + +// util + +function isUndef(value) { + return value === undefined +}
\ No newline at end of file diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 1fc97e81d..67c35dd67 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -45,6 +45,7 @@ module.exports = class MetamaskController extends EventEmitter { getSelectedAccount: this.configManager.getSelectedAccount.bind(this.configManager), getGasMultiplier: this.configManager.getGasMultiplier.bind(this.configManager), getNetwork: this.getStateNetwork.bind(this), + signTransaction: this.keyringController.signTransaction.bind(this.keyringController), provider: this.provider, blockTracker: this.provider, }) @@ -188,26 +189,7 @@ module.exports = class MetamaskController extends EventEmitter { cb(null, result) }, // tx signing - approveTransaction: this.newUnsignedTransaction.bind(this), - signTransaction: (txParams, cb) => { - this.txManager.formatTxForSigining(txParams) - .then(({ethTx, address, txId}) => { - return this.keyringController.signTransaction(ethTx, address, txId) - }) - .then(({tx, txId}) => { - return this.txManager.resolveSignedTransaction({tx, txId}) - }) - .then((rawTx) => { - cb(null, rawTx) - this.sendUpdate() - this.txManager.emit(`${txParams.metamaskId}:signingComplete`) - }) - .catch((err) => { - console.error(err) - cb(err) - }) - }, - + processTransaction: (txParams, cb) => this.newUnapprovedTransaction(txParams, cb), // msg signing approveMessage: this.newUnsignedMessage.bind(this), signMessage: (...args) => { @@ -256,24 +238,29 @@ module.exports = class MetamaskController extends EventEmitter { return publicConfigStore } - newUnsignedTransaction (txParams, onTxDoneCb) { - const txManager = this.txManager - const err = this.enforceTxValidations(txParams) - if (err) return onTxDoneCb(err) - txManager.addUnapprovedTransaction(txParams, onTxDoneCb, (err, txData) => { - if (err) return onTxDoneCb(err) + newUnapprovedTransaction (txParams, cb) { + this.txManager.addUnapprovedTransaction(txParams, (err, txMeta) => { + if (err) return cb(err) this.sendUpdate() - this.opts.showUnapprovedTx(txParams, txData, onTxDoneCb) + this.opts.showUnapprovedTx(txMeta) + // listen for tx completion (success, fail) + this.txManager.once(`${txMeta.id}:submitted`, successHandler) + this.txManager.once(`${txMeta.id}:rejected`, failHandler) + function successHandler(rawTx) { + removeHandlers() + cb(null, rawTx) + } + function failHandler() { + removeHandlers() + cb(new Error('User denied message signature.')) + } + function removeHandlers() { + this.txManager.removeListener(`${txMeta.id}:submitted`, successHandler) + this.txManager.removeListener(`${txMeta.id}:rejected`, failHandler) + } }) } - enforceTxValidations (txParams) { - if (('value' in txParams) && txParams.value.indexOf('-') === 0) { - const msg = `Invalid transaction value of ${txParams.value} not a positive number.` - return new Error(msg) - } - } - newUnsignedMessage (msgParams, cb) { var state = this.keyringController.getState() if (!state.isUnlocked) { diff --git a/app/scripts/transaction-manager.js b/app/scripts/transaction-manager.js index 6becfa6d1..ec08b6af7 100644 --- a/app/scripts/transaction-manager.js +++ b/app/scripts/transaction-manager.js @@ -1,11 +1,10 @@ const EventEmitter = require('events') +const async = require('async') const extend = require('xtend') +const Semaphore = require('semaphore') const ethUtil = require('ethereumjs-util') -const Transaction = require('ethereumjs-tx') -const BN = ethUtil.BN const TxProviderUtil = require('./lib/tx-utils') const createId = require('./lib/random-id') -const normalize = require('./lib/sig-util').normalize module.exports = class TransactionManager extends EventEmitter { constructor (opts) { @@ -15,11 +14,14 @@ module.exports = class TransactionManager extends EventEmitter { this.txHistoryLimit = opts.txHistoryLimit this.getSelectedAccount = opts.getSelectedAccount this.provider = opts.provider + this.query = opts.query this.blockTracker = opts.blockTracker this.txProviderUtils = new TxProviderUtil(this.provider) this.blockTracker.on('block', this.checkForTxInBlock.bind(this)) this.getGasMultiplier = opts.getGasMultiplier this.getNetwork = opts.getNetwork + this.signEthTx = opts.signTransaction + this.nonceLock = Semaphore(1) } getState () { @@ -37,7 +39,7 @@ module.exports = class TransactionManager extends EventEmitter { } // Adds a tx to the txlist - addTx (txMeta, onTxDoneCb = warn) { + addTx (txMeta) { var txList = this.getTxList() var txHistoryLimit = this.txHistoryLimit @@ -53,16 +55,11 @@ module.exports = class TransactionManager extends EventEmitter { txList.push(txMeta) this._saveTxList(txList) - // keep the onTxDoneCb around in a listener - // for after approval/denial (requires user interaction) - // This onTxDoneCb fires completion to the Dapp's write operation. this.once(`${txMeta.id}:signed`, function (txId) { this.removeAllListeners(`${txMeta.id}:rejected`) - onTxDoneCb(null, true) }) this.once(`${txMeta.id}:rejected`, function (txId) { this.removeAllListeners(`${txMeta.id}:signed`) - onTxDoneCb(null, false) }) this.emit('updateBadge') @@ -93,28 +90,35 @@ module.exports = class TransactionManager extends EventEmitter { return this.getTxsByMetaData('status', 'signed').length } - addUnapprovedTransaction (txParams, onTxDoneCb, cb) { - // create txData obj with parameters and meta data - var time = (new Date()).getTime() - var txId = createId() - txParams.metamaskId = txId - txParams.metamaskNetworkId = this.getNetwork() - var txData = { - id: txId, - txParams: txParams, - time: time, - status: 'unapproved', - gasMultiplier: this.getGasMultiplier() || 1, - metamaskNetworkId: this.getNetwork(), - } - this.txProviderUtils.analyzeGasUsage(txData, this.txDidComplete.bind(this, txData, onTxDoneCb, cb)) - // calculate metadata for tx - } - - txDidComplete (txMeta, onTxDoneCb, cb, err) { - if (err) return cb(err) - this.addTx(txMeta, onTxDoneCb) - cb(null, txMeta) + addUnapprovedTransaction (txParams, done) { + let txMeta + async.waterfall([ + // validate + (cb) => this.validateTxParams(txParams, cb), + // prepare txMeta + (cb) => { + // create txMeta obj with parameters and meta data + let time = (new Date()).getTime() + let txId = createId() + txParams.metamaskId = txId + txParams.metamaskNetworkId = this.getNetwork() + txMeta = { + id: txId, + time: time, + status: 'unapproved', + gasMultiplier: this.getGasMultiplier() || 1, + metamaskNetworkId: this.getNetwork(), + txParams: txParams, + } + // calculate metadata for tx + this.txProviderUtils.analyzeGasUsage(txMeta, cb) + }, + // save txMeta + (cb) => { + this.addTx(txMeta) + cb(null, txMeta) + }, + ], done) } getUnapprovedTxList () { @@ -127,8 +131,23 @@ module.exports = class TransactionManager extends EventEmitter { } approveTransaction (txId, cb = warn) { - this.setTxStatusSigned(txId) - this.once(`${txId}:signingComplete`, cb) + const self = this + // approve + self.setTxStatusApproved(txId) + // only allow one tx at a time for atomic nonce usage + self.nonceLock.take(() => { + // begin signature process + async.waterfall([ + (cb) => self.fillInTxParams(txId, cb), + (cb) => self.signTransaction(txId, cb), + (rawTx, cb) => self.publishTransaction(txId, rawTx, cb), + ], (err) => { + self.nonceLock.leave() + // TODO: move tx to error state + if (err) return cb(err) + cb() + }) + }) } cancelTransaction (txId, cb = warn) { @@ -136,38 +155,52 @@ module.exports = class TransactionManager extends EventEmitter { cb() } - // formats txParams so the keyringController can sign it - formatTxForSigining (txParams) { - var address = txParams.from - var metaTx = this.getTx(txParams.metamaskId) - var gasMultiplier = metaTx.gasMultiplier - var gasPrice = new BN(ethUtil.stripHexPrefix(txParams.gasPrice), 16) - gasPrice = gasPrice.mul(new BN(gasMultiplier * 100, 10)).div(new BN(100, 10)) - txParams.gasPrice = ethUtil.intToHex(gasPrice.toNumber()) - - // normalize values - txParams.to = normalize(txParams.to) - txParams.from = normalize(txParams.from) - txParams.value = normalize(txParams.value) - txParams.data = normalize(txParams.data) - txParams.gasLimit = normalize(txParams.gasLimit || txParams.gas) - txParams.nonce = normalize(txParams.nonce) - const ethTx = new Transaction(txParams) - var txId = txParams.metamaskId - return Promise.resolve({ethTx, address, txId}) + fillInTxParams (txId, cb) { + let txMeta = this.getTx(txId) + this.txProviderUtils.fillInTxParams(txMeta.txParams, (err) => { + if (err) return cb(err) + this.updateTx(txMeta) + cb() + }) + } + + signTransaction (txId, cb) { + let txMeta = this.getTx(txId) + let txParams = txMeta.txParams + let fromAddress = txParams.from + let ethTx = this.txProviderUtils.buildEthTxFromParams(txParams, txMeta.gasMultiplier) + this.signEthTx(ethTx, fromAddress).then(() => { + this.updateTxAsSigned(txMeta.id, ethTx) + cb(null, ethUtil.bufferToHex(ethTx.serialize())) + }).catch((err) => { + cb(err) + }) + } + + publishTransaction (txId, rawTx, cb) { + this.txProviderUtils.publishTransaction(rawTx, (err) => { + if (err) return cb(err) + this.setTxStatusSubmitted(txId, rawTx) + cb() + }) + } + + validateTxParams (txParams, cb) { + if (('value' in txParams) && txParams.value.indexOf('-') === 0) { + cb(new Error(`Invalid transaction value of ${txParams.value} not a positive number.`)) + } else { + cb() + } } // receives a signed tx object and updates the tx hash - // and pass it to the cb to be sent off - resolveSignedTransaction ({tx, txId, cb = warn}) { + updateTxAsSigned (txId, ethTx) { // Add the tx hash to the persisted meta-tx object - var txHash = ethUtil.bufferToHex(tx.hash()) - var metaTx = this.getTx(txId) - metaTx.hash = txHash - this.updateTx(metaTx) - var rawTx = ethUtil.bufferToHex(tx.serialize()) - return Promise.resolve(rawTx) - + let txHash = ethUtil.bufferToHex(ethTx.hash()) + let txMeta = this.getTx(txId) + txMeta.hash = txHash + this.updateTx(txMeta) + this.setTxStatusSigned(txMeta.id) } /* @@ -212,23 +245,32 @@ module.exports = class TransactionManager extends EventEmitter { return txMeta.status } + // should update the status of the tx to 'rejected'. + setTxStatusRejected (txId) { + this._setTxStatus(txId, 'rejected') + } + + // should update the status of the tx to 'approved'. + setTxStatusApproved (txId) { + this._setTxStatus(txId, 'approved') + } // should update the status of the tx to 'signed'. setTxStatusSigned (txId) { this._setTxStatus(txId, 'signed') - this.emit('updateBadge') } - // should update the status of the tx to 'rejected'. - setTxStatusRejected (txId) { - this._setTxStatus(txId, 'rejected') - this.emit('updateBadge') + // should update the status of the tx to 'submitted'. + setTxStatusSubmitted (txId, rawTx) { + this._setTxStatus(txId, 'submitted', rawTx) } + // should update the status of the tx to 'confirmed'. setTxStatusConfirmed (txId) { this._setTxStatus(txId, 'confirmed') } + // merges txParams obj onto txData.txParams // use extend to ensure that all fields are filled updateTxParams (txId, txParams) { @@ -266,13 +308,16 @@ module.exports = class TransactionManager extends EventEmitter { // should set the status in txData // - `'unapproved'` the user has not responded // - `'rejected'` the user has responded no! + // - `'approved'` the user has approved the tx // - `'signed'` the tx is signed // - `'submitted'` the tx is sent to a server // - `'confirmed'` the tx has been included in a block. - _setTxStatus (txId, status) { + // "value" is an optional parameter to emit + _setTxStatus (txId, status, value) { var txMeta = this.getTx(txId) txMeta.status = status - this.emit(`${txMeta.id}:${status}`, txId) + this.emit(`${txMeta.id}:${status}`, value) + this.emit('updateBadge') this.updateTx(txMeta) } diff --git a/package.json b/package.json index 0d0835a86..52708fdab 100644 --- a/package.json +++ b/package.json @@ -89,13 +89,14 @@ "redux-logger": "^2.3.1", "redux-thunk": "^1.0.2", "sandwich-expando": "^1.0.5", + "semaphore": "^1.0.5", "textarea-caret": "^3.0.1", "three.js": "^0.73.2", "through2": "^2.0.1", "valid-url": "^1.0.9", "vreme": "^3.0.2", "web3": "0.17.0-beta", - "web3-provider-engine": "^8.2.0", + "web3-provider-engine": "^8.4.0", "web3-stream-provider": "^2.0.6", "xtend": "^4.0.1" }, diff --git a/test/unit/metamask-controller-test.js b/test/unit/metamask-controller-test.js index 414610404..a6164c9a0 100644 --- a/test/unit/metamask-controller-test.js +++ b/test/unit/metamask-controller-test.js @@ -25,24 +25,6 @@ describe('MetaMaskController', function() { this.sinon.restore() }) - describe('#enforceTxValidations', function () { - it('returns null for positive values', function() { - var sample = { - value: '0x01' - } - var res = controller.enforceTxValidations(sample) - assert.equal(res, null, 'no error') - }) - - - it('returns error for negative values', function() { - var sample = { - value: '-0x01' - } - var res = controller.enforceTxValidations(sample) - assert.ok(res, 'error') - }) - }) }) diff --git a/test/unit/tx-manager-test.js b/test/unit/tx-manager-test.js index be16facad..d5d386234 100644 --- a/test/unit/tx-manager-test.js +++ b/test/unit/tx-manager-test.js @@ -18,6 +18,27 @@ describe('Transaction Manager', function() { }) }) + describe('#validateTxParams', function () { + it('returns null for positive values', function() { + var sample = { + value: '0x01' + } + var res = txManager.validateTxParams(sample, (err) => { + assert.equal(err, null, 'no error') + }) + }) + + + it('returns error for negative values', function() { + var sample = { + value: '-0x01' + } + var res = txManager.validateTxParams(sample, (err) => { + assert.ok(err, 'error') + }) + }) + }) + describe('#getTxList', function() { it('when new should return empty array', function() { var result = txManager.getTxList() @@ -100,11 +121,12 @@ describe('Transaction Manager', function() { it('should emit a signed event to signal the exciton of callback', (done) => { this.timeout(10000) var tx = { id: 1, status: 'unapproved' } - let onTxDoneCb = function (err, txId) { + let onTxDoneCb = function () { assert(true, 'event listener has been triggered and onTxDoneCb executed') done() } - txManager.addTx(tx, onTxDoneCb) + txManager.addTx(tx) + txManager.on('1:signed', onTxDoneCb) txManager.setTxStatusSigned(1) }) }) @@ -112,7 +134,7 @@ describe('Transaction Manager', function() { describe('#setTxStatusRejected', function() { it('sets the tx status to rejected', function() { var tx = { id: 1, status: 'unapproved' } - txManager.addTx(tx, onTxDoneCb) + txManager.addTx(tx) txManager.setTxStatusRejected(1) var result = txManager.getTxList() assert.ok(Array.isArray(result)) @@ -123,11 +145,12 @@ describe('Transaction Manager', function() { it('should emit a rejected event to signal the exciton of callback', (done) => { this.timeout(10000) var tx = { id: 1, status: 'unapproved' } + txManager.addTx(tx) let onTxDoneCb = function (err, txId) { assert(true, 'event listener has been triggered and onTxDoneCb executed') done() } - txManager.addTx(tx, onTxDoneCb) + txManager.on('1:rejected', onTxDoneCb) txManager.setTxStatusRejected(1) }) |