From b3492d9c17e62332c17bb082c23db30512e2b881 Mon Sep 17 00:00:00 2001 From: kumavis Date: Wed, 14 Jun 2017 23:44:02 -0700 Subject: [PATCH] transaction controller - use nonce-tracker --- app/scripts/controllers/transactions.js | 90 ++++++++++++++++--------- app/scripts/metamask-controller.js | 2 +- test/unit/tx-controller-test.js | 7 +- 3 files changed, 63 insertions(+), 36 deletions(-) diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index 2db8041eb..e7fe9927e 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -4,9 +4,10 @@ const extend = require('xtend') const Semaphore = require('semaphore') const ObservableStore = require('obs-store') const ethUtil = require('ethereumjs-util') +const denodeify = require('denodeify') const TxProviderUtil = require('../lib/tx-utils') const createId = require('../lib/random-id') -const denodeify = require('denodeify') +const NonceTracker = require('../lib/nonce-tracker') const RETRY_LIMIT = 200 @@ -22,6 +23,11 @@ module.exports = class TransactionController extends EventEmitter { this.txHistoryLimit = opts.txHistoryLimit this.provider = opts.provider this.blockTracker = opts.blockTracker + this.nonceTracker = new NonceTracker({ + provider: this.provider, + blockTracker: this.blockTracker, + getPendingTransactions: (address) => this.getFilteredTxList({ from: address, status: 'submitted' }), + }) this.query = opts.ethQuery this.txProviderUtils = new TxProviderUtil(this.query) this.blockTracker.on('block', this.checkForTxInBlock.bind(this)) @@ -169,29 +175,58 @@ module.exports = class TransactionController extends EventEmitter { }, {}) } - approveTransaction (txId, cb = warn) { - 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() - if (err) { - this.setTxStatusFailed(txId, { - errCode: err.errCode || err, - message: err.message || 'Transaction failed during approval', - }) - return cb(err) - } - cb() + // approveTransaction (txId, cb = warn) { + // promiseToCallback((async () => { + // // approve + // self.setTxStatusApproved(txId) + // // get next nonce + // const txMeta = this.getTx(txId) + // const fromAddress = txMeta.txParams.from + // const { nextNonce, releaseLock } = await this.nonceTracker.getNonceLock(fromAddress) + // txMeta.txParams.nonce = nonce + // this.updateTx(txMeta) + // // sign transaction + // const rawTx = await denodeify(self.signTransaction.bind(self))(txId) + // await denodeify(self.publishTransaction.bind(self))(txId, rawTx) + // })())((err) => { + // if (err) { + // this.setTxStatusFailed(txId, { + // errCode: err.errCode || err, + // message: err.message || 'Transaction failed during approval', + // }) + // } + // // must set transaction to submitted/failed before releasing lock + // releaseLock() + // cb(err) + // }) + // } + + async approveTransaction (txId) { + let nonceLock + try { + // approve + this.setTxStatusApproved(txId) + // get next nonce + const txMeta = this.getTx(txId) + const fromAddress = txMeta.txParams.from + nonceLock = await this.nonceTracker.getNonceLock(fromAddress) + txMeta.txParams.nonce = nonceLock.nextNonce + this.updateTx(txMeta) + // sign transaction + const rawTx = await denodeify(this.signTransaction.bind(this))(txId) + await denodeify(this.publishTransaction.bind(this))(txId, rawTx) + // must set transaction to submitted/failed before releasing lock + nonceLock.releaseLock() + } catch (err) { + this.setTxStatusFailed(txId, { + errCode: err.errCode || err, + message: err.message || 'Transaction failed during approval', }) - }) + // must set transaction to submitted/failed before releasing lock + if (nonceLock) nonceLock.releaseLock() + // continue with error chain + throw err + } } cancelTransaction (txId, cb = warn) { @@ -199,15 +234,6 @@ module.exports = class TransactionController extends EventEmitter { cb() } - fillInTxParams (txId, cb) { - const txMeta = this.getTx(txId) - this.txProviderUtils.fillInTxParams(txMeta.txParams, (err) => { - if (err) return cb(err) - this.updateTx(txMeta) - cb() - }) - } - getChainId () { const networkState = this.networkStore.getState() const getChainId = parseInt(networkState) diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index a7eb3d056..006a32eac 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -290,7 +290,7 @@ module.exports = class MetamaskController extends EventEmitter { exportAccount: nodeify(keyringController.exportAccount).bind(keyringController), // txController - approveTransaction: txController.approveTransaction.bind(txController), + approveTransaction: nodeify(txController.approveTransaction).bind(txController), cancelTransaction: txController.cancelTransaction.bind(txController), updateAndApproveTransaction: this.updateAndApproveTx.bind(this), diff --git a/test/unit/tx-controller-test.js b/test/unit/tx-controller-test.js index f0d8a706e..7c8d1761d 100644 --- a/test/unit/tx-controller-test.js +++ b/test/unit/tx-controller-test.js @@ -1,5 +1,4 @@ const assert = require('assert') -const EventEmitter = require('events') const ethUtil = require('ethereumjs-util') const EthTx = require('ethereumjs-tx') const EthQuery = require('eth-query') @@ -19,13 +18,15 @@ describe('Transaction Controller', function () { txController = new TransactionController({ networkStore: new ObservableStore(currentNetworkId), txHistoryLimit: 10, - blockTracker: new EventEmitter(), - ethQuery: new EthQuery(new EventEmitter()), + blockTracker: { getCurrentBlock: noop, on: noop }, + provider: { sendAsync: noop }, + ethQuery: new EthQuery({ sendAsync: noop }), signTransaction: (ethTx) => new Promise((resolve) => { ethTx.sign(privKey) resolve() }), }) + txController.nonceTracker.getNonceLock = () => Promise.resolve({ nextNonce: 0, releaseLock: noop }) }) describe('#validateTxParams', function () {