diff options
| -rw-r--r-- | .gitignore | 3 | ||||
| -rw-r--r-- | app/scripts/controllers/transactions/index.js | 46 | ||||
| -rw-r--r-- | app/scripts/controllers/transactions/nonce-tracker.js | 16 | ||||
| -rw-r--r-- | app/scripts/controllers/transactions/pending-tx-tracker.js | 89 | ||||
| -rw-r--r-- | app/scripts/lib/util.js | 14 | ||||
| -rw-r--r-- | test/unit/app/controllers/transactions/pending-tx-test.js | 57 | ||||
| -rw-r--r-- | test/unit/app/controllers/transactions/tx-controller-test.js | 6 |
7 files changed, 78 insertions, 153 deletions
diff --git a/.gitignore b/.gitignore index 0e91a7d04..21a13c904 100644 --- a/.gitignore +++ b/.gitignore @@ -9,6 +9,7 @@ package # IDEs .idea .vscode +.sublime-project temp .tmp @@ -37,4 +38,4 @@ ui/app/css/output/ notes.txt .coveralls.yml -.nyc_output +.nyc_output
\ No newline at end of file diff --git a/app/scripts/controllers/transactions/index.js b/app/scripts/controllers/transactions/index.js index 7cb8af3a8..f84fd95ff 100644 --- a/app/scripts/controllers/transactions/index.js +++ b/app/scripts/controllers/transactions/index.js @@ -78,7 +78,7 @@ class TransactionController extends EventEmitter { }) this.txStateManager.store.subscribe(() => this.emit('update:badge')) - this._setupListners() + this._setupListeners() // memstore is computed from a few different stores this._updateMemstore() this.txStateManager.store.subscribe(() => this._updateMemstore()) @@ -382,8 +382,9 @@ class TransactionController extends EventEmitter { is called in constructor applies the listeners for pendingTxTracker txStateManager and blockTracker */ - _setupListners () { + _setupListeners () { this.txStateManager.on('tx:status-update', this.emit.bind(this, 'tx:status-update')) + this._setupBlockTrackerListener() this.pendingTxTracker.on('tx:warning', (txMeta) => { this.txStateManager.updateTx(txMeta, 'transactions/pending-tx-tracker#event: tx:warning') }) @@ -399,13 +400,6 @@ class TransactionController extends EventEmitter { txMeta.retryCount++ this.txStateManager.updateTx(txMeta, 'transactions/pending-tx-tracker#event: tx:retry') }) - - this.blockTracker.on('block', this.pendingTxTracker.checkForTxInBlock.bind(this.pendingTxTracker)) - // this is a little messy but until ethstore has been either - // removed or redone this is to guard against the race condition - this.blockTracker.on('latest', this.pendingTxTracker.resubmitPendingTxs.bind(this.pendingTxTracker)) - this.blockTracker.on('sync', this.pendingTxTracker.queryPendingTxs.bind(this.pendingTxTracker)) - } /** @@ -429,6 +423,40 @@ class TransactionController extends EventEmitter { }) } + _setupBlockTrackerListener () { + let listenersAreActive = false + const latestBlockHandler = this._onLatestBlock.bind(this) + const blockTracker = this.blockTracker + const txStateManager = this.txStateManager + + txStateManager.on('tx:status-update', updateSubscription) + updateSubscription() + + function updateSubscription() { + const pendingTxs = txStateManager.getPendingTransactions() + if (!listenersAreActive && pendingTxs.length > 0) { + blockTracker.on('latest', latestBlockHandler) + listenersAreActive = true + } else if (listenersAreActive && !pendingTxs.length) { + blockTracker.removeListener('latest', latestBlockHandler) + listenersAreActive = false + } + } + } + + async _onLatestBlock (blockNumber) { + try { + await this.pendingTxTracker.updatePendingTxs() + } catch (err) { + log.error(err) + } + try { + await this.pendingTxTracker.resubmitPendingTxs(blockNumber) + } catch (err) { + log.error(err) + } + } + /** Updates the memStore in transaction controller */ diff --git a/app/scripts/controllers/transactions/nonce-tracker.js b/app/scripts/controllers/transactions/nonce-tracker.js index 490118c89..fe2d25fca 100644 --- a/app/scripts/controllers/transactions/nonce-tracker.js +++ b/app/scripts/controllers/transactions/nonce-tracker.js @@ -35,7 +35,7 @@ class NonceTracker { * @typedef NonceDetails * @property {number} highestLocallyConfirmed - A hex string of the highest nonce on a confirmed transaction. * @property {number} nextNetworkNonce - The next nonce suggested by the eth_getTransactionCount method. - * @property {number} highetSuggested - The maximum between the other two, the number returned. + * @property {number} highestSuggested - The maximum between the other two, the number returned. */ /** @@ -75,14 +75,6 @@ class NonceTracker { return { nextNonce, nonceDetails, releaseLock } } - async _getCurrentBlock () { - const currentBlock = this.blockTracker.getCurrentBlock() - if (currentBlock) return currentBlock - return await new Promise((reject, resolve) => { - this.blockTracker.once('latest', resolve) - }) - } - async _globalMutexFree () { const globalMutex = this._lookupMutex('global') const release = await globalMutex.acquire() @@ -108,9 +100,8 @@ class NonceTracker { // calculate next nonce // we need to make sure our base count // and pending count are from the same block - const currentBlock = await this._getCurrentBlock() - const blockNumber = currentBlock.blockNumber - const baseCountBN = await this.ethQuery.getTransactionCount(address, blockNumber || 'latest') + const blockNumber = await this.blockTracker.getLatestBlock() + const baseCountBN = await this.ethQuery.getTransactionCount(address, blockNumber) const baseCount = baseCountBN.toNumber() assert(Number.isInteger(baseCount), `nonce-tracker - baseCount is not an integer - got: (${typeof baseCount}) "${baseCount}"`) const nonceDetails = { blockNumber, baseCount } @@ -171,6 +162,7 @@ class NonceTracker { return { name: 'local', nonce: highest, details: { startPoint, highest } } } + } module.exports = NonceTracker diff --git a/app/scripts/controllers/transactions/pending-tx-tracker.js b/app/scripts/controllers/transactions/pending-tx-tracker.js index e1bb67c90..e981e2991 100644 --- a/app/scripts/controllers/transactions/pending-tx-tracker.js +++ b/app/scripts/controllers/transactions/pending-tx-tracker.js @@ -24,60 +24,27 @@ class PendingTransactionTracker extends EventEmitter { super() this.query = new EthQuery(config.provider) this.nonceTracker = config.nonceTracker - // default is one day this.getPendingTransactions = config.getPendingTransactions this.getCompletedTransactions = config.getCompletedTransactions this.publishTransaction = config.publishTransaction this.confirmTransaction = config.confirmTransaction - this._checkPendingTxs() + this.updatePendingTxs() } /** - checks if a signed tx is in a block and - if it is included emits tx status as 'confirmed' - @param block {object}, a full block - @emits tx:confirmed - @emits tx:failed - */ - async checkForTxInBlock (blockNumber) { - const block = await this._getBlock(blockNumber) - const signedTxList = this.getPendingTransactions() - if (!signedTxList.length) return - signedTxList.forEach((txMeta) => { - const txHash = txMeta.hash - const txId = txMeta.id - - if (!txHash) { - const noTxHashErr = new Error('We had an error while submitting this transaction, please try again.') - noTxHashErr.name = 'NoTxHashError' - this.emit('tx:failed', txId, noTxHashErr) - return - } - - if (!block.transactions.length) return - - block.transactions.forEach((hash) => { - if (hash === txHash) { - this.confirmTransaction(txId) - } - }) - }) - } - - /** - asks the network for the transaction to see if a block number is included on it - if we have skipped/missed blocks - @param object - oldBlock newBlock + checks the network for signed txs and releases the nonce global lock if it is */ - queryPendingTxs ({ oldBlock, newBlock }) { - // check pending transactions on start - if (!oldBlock) { - this._checkPendingTxs() - return + async updatePendingTxs () { + const pendingTxs = this.getPendingTransactions() + // in order to keep the nonceTracker accurate we block it while updating pending transactions + const nonceGlobalLock = await this.nonceTracker.getGlobalLock() + try { + await Promise.all(pendingTxs.map((txMeta) => this._checkPendingTx(txMeta))) + } catch (err) { + log.error('PendingTransactionTracker - Error updating pending transactions') + log.error(err) } - // if we synced by more than one block, check for missed pending transactions - const diff = Number.parseInt(newBlock, 16) - Number.parseInt(oldBlock, 16) - if (diff > 1) this._checkPendingTxs() + nonceGlobalLock.releaseLock() } /** @@ -151,6 +118,7 @@ class PendingTransactionTracker extends EventEmitter { this.emit('tx:retry', txMeta) return txHash } + /** Ask the network for the transaction to see if it has been include in a block @param txMeta {Object} - the txMeta object @@ -180,9 +148,8 @@ class PendingTransactionTracker extends EventEmitter { } // get latest transaction status - let txParams try { - txParams = await this.query.getTransactionByHash(txHash) + const txParams = await this.query.getTransactionByHash(txHash) if (!txParams) return if (txParams.blockNumber) { this.confirmTransaction(txId) @@ -197,34 +164,6 @@ class PendingTransactionTracker extends EventEmitter { } /** - checks the network for signed txs and releases the nonce global lock if it is - */ - async _checkPendingTxs () { - const signedTxList = this.getPendingTransactions() - // in order to keep the nonceTracker accurate we block it while updating pending transactions - const nonceGlobalLock = await this.nonceTracker.getGlobalLock() - try { - await Promise.all(signedTxList.map((txMeta) => this._checkPendingTx(txMeta))) - } catch (err) { - log.error('PendingTransactionWatcher - Error updating pending transactions') - log.error(err) - } - nonceGlobalLock.releaseLock() - } - - async _getBlock (blockNumber) { - let block - while (!block) { - // block requests will sometimes return null due do the infura api - // being backed by multiple out-of-sync clients - block = await this.query.getBlockByNumber(blockNumber, false) - // if block is null, wait 1 sec then try again - if (!block) await timeout(1000) - } - return block - } - - /** checks to see if a confirmed txMeta has the same nonce @param txMeta {Object} - txMeta object @returns {boolean} diff --git a/app/scripts/lib/util.js b/app/scripts/lib/util.js index 431d1e59c..7ceb9da3c 100644 --- a/app/scripts/lib/util.js +++ b/app/scripts/lib/util.js @@ -99,7 +99,21 @@ function BnMultiplyByFraction (targetBN, numerator, denominator) { return targetBN.mul(numBN).div(denomBN) } +function applyListeners (listeners, emitter) { + Object.keys(listeners).forEach((key) => { + emitter.on(key, listeners[key]) + }) +} + +function removeListeners (listeners, emitter) { + Object.keys(listeners).forEach((key) => { + emitter.removeListener(key, listeners[key]) + }) +} + module.exports = { + removeListeners, + applyListeners, getStack, getEnvironmentType, sufficientBalance, diff --git a/test/unit/app/controllers/transactions/pending-tx-test.js b/test/unit/app/controllers/transactions/pending-tx-test.js index c0d033007..f06f1c0dd 100644 --- a/test/unit/app/controllers/transactions/pending-tx-test.js +++ b/test/unit/app/controllers/transactions/pending-tx-test.js @@ -7,7 +7,7 @@ const { createTestProviderTools } = require('../../../../stub/provider') const PendingTransactionTracker = require('../../../../../app/scripts/controllers/transactions/pending-tx-tracker') const MockTxGen = require('../../../../lib/mock-tx-gen') const sinon = require('sinon') -const noop = () => true +const noop =()=>true const currentNetworkId = 42 const otherNetworkId = 36 const privKey = new Buffer('8718b9618a37d1fc78c436511fc6df3c8258d3250635bba617f33003270ec03e', 'hex') @@ -108,56 +108,6 @@ describe('PendingTransactionTracker', function () { }) }) - describe('#checkForTxInBlock', function () { - it('should return if no pending transactions', function () { - // throw a type error if it trys to do anything on the block - // thus failing the test - const block = Proxy.revocable({}, {}).revoke() - pendingTxTracker.checkForTxInBlock(block) - }) - it('should emit \'tx:failed\' if the txMeta does not have a hash', function (done) { - const block = Proxy.revocable({}, {}).revoke() - pendingTxTracker.getPendingTransactions = () => [txMetaNoHash] - pendingTxTracker.once('tx:failed', (txId, err) => { - assert(txId, txMetaNoHash.id, 'should pass txId') - done() - }) - pendingTxTracker.checkForTxInBlock(block) - }) - }) - describe('#queryPendingTxs', function () { - it('should call #_checkPendingTxs if their is no oldBlock', function (done) { - let newBlock, oldBlock - newBlock = '0x01' - const originalFunction = pendingTxTracker._checkPendingTxs - pendingTxTracker._checkPendingTxs = () => { done() } - pendingTxTracker.queryPendingTxs({ oldBlock, newBlock }) - pendingTxTracker._checkPendingTxs = originalFunction - }) - it('should call #_checkPendingTxs if oldBlock and the newBlock have a diff of greater then 1', function (done) { - let newBlock, oldBlock - oldBlock = '0x01' - newBlock = '0x03' - const originalFunction = pendingTxTracker._checkPendingTxs - pendingTxTracker._checkPendingTxs = () => { done() } - pendingTxTracker.queryPendingTxs({ oldBlock, newBlock }) - pendingTxTracker._checkPendingTxs = originalFunction - }) - it('should not call #_checkPendingTxs if oldBlock and the newBlock have a diff of 1 or less', function (done) { - let newBlock, oldBlock - oldBlock = '0x1' - newBlock = '0x2' - const originalFunction = pendingTxTracker._checkPendingTxs - pendingTxTracker._checkPendingTxs = () => { - const err = new Error('should not call #_checkPendingTxs if oldBlock and the newBlock have a diff of 1 or less') - done(err) - } - pendingTxTracker.queryPendingTxs({ oldBlock, newBlock }) - pendingTxTracker._checkPendingTxs = originalFunction - done() - }) - }) - describe('#_checkPendingTx', function () { it('should emit \'tx:failed\' if the txMeta does not have a hash', function (done) { pendingTxTracker.once('tx:failed', (txId, err) => { @@ -187,7 +137,6 @@ describe('PendingTransactionTracker', function () { it('should warp all txMeta\'s in #_checkPendingTx', function (done) { pendingTxTracker.getPendingTransactions = () => txList pendingTxTracker._checkPendingTx = (tx) => { tx.resolve(tx) } - const list = txList.map Promise.all(txList.map((tx) => tx.processed)) .then((txCompletedList) => done()) .catch(done) @@ -201,7 +150,7 @@ describe('PendingTransactionTracker', function () { beforeEach(function () { const txMeta2 = txMeta3 = txMeta txList = [txMeta, txMeta2, txMeta3].map((tx) => { - tx.processed = new Promise ((resolve) => { tx.resolve = resolve }) + tx.processed = new Promise((resolve) => { tx.resolve = resolve }) return tx }) }) @@ -218,7 +167,7 @@ describe('PendingTransactionTracker', function () { pendingTxTracker.resubmitPendingTxs(blockNuberStub) }) it('should not emit \'tx:failed\' if the txMeta throws a known txError', function (done) { - knownErrors =[ + knownErrors = [ // geth ' Replacement transaction Underpriced ', ' known transaction', diff --git a/test/unit/app/controllers/transactions/tx-controller-test.js b/test/unit/app/controllers/transactions/tx-controller-test.js index c450ed3ed..b0cc0acda 100644 --- a/test/unit/app/controllers/transactions/tx-controller-test.js +++ b/test/unit/app/controllers/transactions/tx-controller-test.js @@ -1,4 +1,5 @@ const assert = require('assert') +const EventEmitter = require('events') const ethUtil = require('ethereumjs-util') const EthTx = require('ethereumjs-tx') const EthjsQuery = require('ethjs-query') @@ -26,12 +27,13 @@ describe('Transaction Controller', function () { provider = createTestProviderTools({ scaffold: providerResultStub }).provider query = new EthjsQuery(provider) fromAccount = getTestAccounts()[0] - + const blockTrackerStub = new EventEmitter() + blockTrackerStub.getCurrentBlock = noop txController = new TransactionController({ provider, networkStore: new ObservableStore(currentNetworkId), txHistoryLimit: 10, - blockTracker: { getCurrentBlock: noop, on: noop, once: noop }, + blockTracker: blockTrackerStub, signTransaction: (ethTx) => new Promise((resolve) => { ethTx.sign(fromAccount.key) resolve() |