From 92df9965ebd4a833817c32fd32f7e4533ec7fe19 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Wed, 21 Jun 2017 19:51:00 -0700 Subject: [PATCH] fix nonceTracker --- app/scripts/controllers/transactions.js | 51 +++++++------------------ app/scripts/lib/nonce-tracker.js | 14 +++---- test/unit/nonce-tracker-test.js | 18 +++------ 3 files changed, 25 insertions(+), 58 deletions(-) diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index c2f98e66a..59c8be8b7 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -25,7 +25,7 @@ module.exports = class TransactionController extends EventEmitter { this.blockTracker = opts.blockTracker this.nonceTracker = new NonceTracker({ provider: this.provider, - blockTracker: this.blockTracker, + blockTracker: this.provider._blockTracker, getPendingTransactions: (address) => this.getFilteredTxList({ from: address, status: 'submitted' }), }) this.query = opts.ethQuery @@ -176,33 +176,7 @@ module.exports = class TransactionController extends EventEmitter { }, {}) } - // 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) { + async approveTransaction (txId, cb = warn) { let nonceLock try { // approve @@ -215,9 +189,10 @@ module.exports = class TransactionController extends EventEmitter { this.updateTx(txMeta) // sign transaction const rawTx = await denodeify(this.signTransaction.bind(this))(txId) - await denodeify(this.publishTransaction.bind(this))(txId, rawTx) + await this.publishTransaction(txId, rawTx) // must set transaction to submitted/failed before releasing lock nonceLock.releaseLock() + cb() } catch (err) { this.setTxStatusFailed(txId, { errCode: err.errCode || err, @@ -226,7 +201,7 @@ module.exports = class TransactionController extends EventEmitter { // must set transaction to submitted/failed before releasing lock if (nonceLock) nonceLock.releaseLock() // continue with error chain - throw err + cb(err) } } @@ -260,16 +235,17 @@ module.exports = class TransactionController extends EventEmitter { }) } - publishTransaction (txId, rawTx, cb = warn) { + publishTransaction (txId, rawTx) { const txMeta = this.getTx(txId) txMeta.rawTx = rawTx this.updateTx(txMeta) - - this.txProviderUtils.publishTransaction(rawTx, (err, txHash) => { - if (err) return cb(err) - this.setTxHash(txId, txHash) - this.setTxStatusSubmitted(txId) - cb() + return new Promise((resolve, reject) => { + this.txProviderUtils.publishTransaction(rawTx, (err, txHash) => { + if (err) reject(err) + this.setTxHash(txId, txHash) + this.setTxStatusSubmitted(txId) + resolve() + }) }) } @@ -414,7 +390,6 @@ module.exports = class TransactionController extends EventEmitter { this.emit(`${txMeta.id}:${status}`, txId) if (status === 'submitted' || status === 'rejected') { this.emit(`${txMeta.id}:finished`, txMeta) - } this.updateTx(txMeta) this.emit('updateBadge') diff --git a/app/scripts/lib/nonce-tracker.js b/app/scripts/lib/nonce-tracker.js index ff2317a91..4ea511dec 100644 --- a/app/scripts/lib/nonce-tracker.js +++ b/app/scripts/lib/nonce-tracker.js @@ -2,7 +2,7 @@ const EthQuery = require('eth-query') class NonceTracker { - constructor({ blockTracker, provider, getPendingTransactions }) { + constructor ({ blockTracker, provider, getPendingTransactions }) { this.blockTracker = blockTracker this.ethQuery = new EthQuery(provider) this.getPendingTransactions = getPendingTransactions @@ -11,7 +11,7 @@ class NonceTracker { // releaseLock must be called // releaseLock must be called after adding signed tx to pending transactions (or discarding) - async getNonceLock(address) { + async getNonceLock (address) { // await lock free await this.lockMap[address] // take lock @@ -21,12 +21,12 @@ class NonceTracker { const blockNumber = currentBlock.number const pendingTransactions = this.getPendingTransactions(address) const baseCount = await this._getTxCount(address, blockNumber) - const nextNonce = parseInt(baseCount) + pendingTransactions.length + 1 + const nextNonce = parseInt(baseCount) + pendingTransactions.length // return next nonce and release cb return { nextNonce: nextNonce.toString(16), releaseLock } } - async _getCurrentBlock() { + async _getCurrentBlock () { const currentBlock = this.blockTracker.getCurrentBlock() if (currentBlock) return currentBlock return await Promise((reject, resolve) => { @@ -34,15 +34,13 @@ class NonceTracker { }) } - _takeLock(lockId) { + _takeLock (lockId) { let releaseLock = null // create and store lock const lock = new Promise((resolve, reject) => { releaseLock = resolve }) this.lockMap[lockId] = lock // setup lock teardown - lock.then(() => { - delete this.lockMap[lockId] - }) + lock.then(() => delete this.lockMap[lockId]) return releaseLock } diff --git a/test/unit/nonce-tracker-test.js b/test/unit/nonce-tracker-test.js index 5a05d6170..16cd6d008 100644 --- a/test/unit/nonce-tracker-test.js +++ b/test/unit/nonce-tracker-test.js @@ -3,31 +3,25 @@ const NonceTracker = require('../../app/scripts/lib/nonce-tracker') describe('Nonce Tracker', function () { let nonceTracker, provider, getPendingTransactions, pendingTxs - const noop = () => {} beforeEach(function () { - pendingTxs =[{ + pendingTxs = [{ 'status': 'submitted', 'txParams': { 'from': '0x7d3517b0d011698406d6e0aed8453f0be2697926', 'gas': '0x30d40', 'value': '0x0', - 'nonce': '0x1', + 'nonce': '0x0', }, }] getPendingTransactions = () => pendingTxs - provider = { sendAsync: (_, cb) => { cb(undefined , {result: '0x0'}) }, } + provider = { sendAsync: (_, cb) => { cb(undefined, {result: '0x0'}) } } nonceTracker = new NonceTracker({ blockTracker: { - getCurrentBlock: () => '0x11b568', - once: (...args) => { - setTimeout(() => { - args.pop()() - }, 5000) - } + getCurrentBlock: () => '0x11b568', }, provider, getPendingTransactions, @@ -38,8 +32,8 @@ describe('Nonce Tracker', function () { it('should work', async function (done) { this.timeout(15000) const nonceLock = await nonceTracker.getNonceLock('0x7d3517b0d011698406d6e0aed8453f0be2697926') - assert.equal(nonceLock.nextNonce, '2', 'nonce should be 2') - nonceLock.releaseLock() + assert.equal(nonceLock.nextNonce, '1', 'nonce should be 1') + await nonceLock.releaseLock() done() }) })