aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--app/scripts/controllers/transactions.js90
-rw-r--r--app/scripts/lib/nonce-tracker.js49
-rw-r--r--app/scripts/metamask-controller.js2
-rw-r--r--test/unit/tx-controller-test.js24
4 files changed, 118 insertions, 47 deletions
diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js
index d9d9849b1..c2f98e66a 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('rawBlock', this.checkForTxInBlock.bind(this))
@@ -170,29 +176,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) {
@@ -200,15 +235,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/lib/nonce-tracker.js b/app/scripts/lib/nonce-tracker.js
new file mode 100644
index 000000000..6e9d094bc
--- /dev/null
+++ b/app/scripts/lib/nonce-tracker.js
@@ -0,0 +1,49 @@
+const EthQuery = require('ethjs-query')
+
+class NonceTracker {
+
+ constructor({ blockTracker, provider, getPendingTransactions }) {
+ this.blockTracker = blockTracker
+ this.ethQuery = new EthQuery(provider)
+ this.getPendingTransactions = getPendingTransactions
+ this.lockMap = {}
+ }
+
+ // releaseLock must be called
+ // releaseLock must be called after adding signed tx to pending transactions (or discarding)
+ async getNonceLock(address) {
+ // await lock free
+ await this.lockMap[address]
+ // take lock
+ const releaseLock = this._takeLock(address)
+ // calculate next nonce
+ const currentBlock = await this._getCurrentBlock()
+ const blockNumber = currentBlock.number
+ const pendingTransactions = this.getPendingTransactions(address)
+ const baseCount = await this.ethQuery.getTransactionCount(address, blockNumber)
+ const nextNonce = baseCount + pendingTransactions
+ // return next nonce and release cb
+ return { nextNonce, releaseLock }
+ }
+
+ async _getCurrentBlock() {
+ const currentBlock = this.blockTracker.getCurrentBlock()
+ if (currentBlock) return currentBlock
+ return await Promise((reject, resolve) => {
+ this.blockTracker.once('latest', resolve)
+ })
+ }
+
+ _takeLock(lockId) {
+ let releaseLock = null
+ // create and store lock
+ const lock = new Promise((reject, resolve) => { releaseLock = resolve })
+ this.lockMap[lockId] = lock
+ // setup lock teardown
+ lock.then(() => delete this.lockMap[lockId])
+ return releaseLock
+ }
+
+}
+
+module.exports = NonceTracker
diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js
index de9a15924..755bf3289 100644
--- a/app/scripts/metamask-controller.js
+++ b/app/scripts/metamask-controller.js
@@ -295,7 +295,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 0d35cd62c..8ce6a5a65 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,14 +18,15 @@ describe('Transaction Controller', function () {
txController = new TransactionController({
networkStore: new ObservableStore(currentNetworkId),
txHistoryLimit: 10,
- provider: { _blockTracker: new EventEmitter()},
- 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 () {
@@ -270,26 +270,25 @@ describe('Transaction Controller', function () {
it('does not overwrite set values', function (done) {
+ this.timeout(15000)
const wrongValue = '0x05'
txController.addTx(txMeta)
const estimateStub = sinon.stub(txController.txProviderUtils.query, 'estimateGas')
- .callsArgWith(1, null, wrongValue)
+ .callsArgWithAsync(1, null, wrongValue)
const priceStub = sinon.stub(txController.txProviderUtils.query, 'gasPrice')
- .callsArgWith(0, null, wrongValue)
+ .callsArgWithAsync(0, null, wrongValue)
- const nonceStub = sinon.stub(txController.txProviderUtils.query, 'getTransactionCount')
- .callsArgWith(2, null, wrongValue)
const signStub = sinon.stub(txController, 'signTransaction')
- .callsArgWith(1, null, noop)
+ .callsArgWithAsync(1, null, noop)
const pubStub = sinon.stub(txController.txProviderUtils, 'publishTransaction')
- .callsArgWith(1, null, originalValue)
+ .callsArgWithAsync(1, null, originalValue)
- txController.approveTransaction(txMeta.id, (err) => {
+ txController.approveTransaction(txMeta.id).then((err) => {
assert.ifError(err, 'should not error')
const result = txController.getTx(txMeta.id)
@@ -297,15 +296,12 @@ describe('Transaction Controller', function () {
assert.equal(params.gas, originalValue, 'gas unmodified')
assert.equal(params.gasPrice, originalValue, 'gas price unmodified')
- assert.equal(params.nonce, originalValue, 'nonce unmodified')
assert.equal(result.hash, originalValue, 'hash was set')
estimateStub.restore()
priceStub.restore()
signStub.restore()
- nonceStub.restore()
pubStub.restore()
-
done()
})
})