diff options
-rw-r--r-- | app/scripts/controllers/transactions/index.js | 1 | ||||
-rw-r--r-- | app/scripts/controllers/transactions/pending-tx-tracker.js | 53 | ||||
-rw-r--r-- | test/unit/app/controllers/transactions/pending-tx-test.js | 72 |
3 files changed, 113 insertions, 13 deletions
diff --git a/app/scripts/controllers/transactions/index.js b/app/scripts/controllers/transactions/index.js index 79dba7833..dd497a11e 100644 --- a/app/scripts/controllers/transactions/index.js +++ b/app/scripts/controllers/transactions/index.js @@ -555,6 +555,7 @@ class TransactionController extends EventEmitter { }) this.pendingTxTracker.on('tx:failed', this.txStateManager.setTxStatusFailed.bind(this.txStateManager)) this.pendingTxTracker.on('tx:confirmed', (txId) => this.confirmTransaction(txId)) + this.pendingTxTracker.on('tx:dropped', this.txStateManager.setTxStatusDropped.bind(this.txStateManager)) this.pendingTxTracker.on('tx:block-update', (txMeta, latestBlockNumber) => { if (!txMeta.firstRetryBlockNumber) { txMeta.firstRetryBlockNumber = latestBlockNumber diff --git a/app/scripts/controllers/transactions/pending-tx-tracker.js b/app/scripts/controllers/transactions/pending-tx-tracker.js index 4bf40b1db..bc11f6633 100644 --- a/app/scripts/controllers/transactions/pending-tx-tracker.js +++ b/app/scripts/controllers/transactions/pending-tx-tracker.js @@ -22,6 +22,7 @@ const EthQuery = require('ethjs-query') class PendingTransactionTracker extends EventEmitter { constructor (config) { super() + this.droppedBuffer = {} this.query = new EthQuery(config.provider) this.nonceTracker = config.nonceTracker this.getPendingTransactions = config.getPendingTransactions @@ -139,22 +140,42 @@ class PendingTransactionTracker extends EventEmitter { 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 another tx with the same nonce is mined, set as failed. + // If another tx with the same nonce is mined, set as dropped. const taken = await this._checkIfNonceIsTaken(txMeta) - if (taken) { - const nonceTakenErr = new Error('Another transaction with this nonce has been mined.') - nonceTakenErr.name = 'NonceTakenErr' - return this.emit('tx:failed', txId, nonceTakenErr) + let dropped + try { + // check the network if the nonce is ahead the tx + // and the tx has not been mined into a block + + dropped = await this._checkIftxWasDropped(txMeta) + // the dropped buffer is in case we ask a node for the tx + // that is behind the node we asked for tx count + // IS A SECURITY FOR HITTING NODES IN INFURA THAT COULD GO OUT + // OF SYNC. + // on the next block event it will return fire as dropped + if (dropped && !this.droppedBuffer[txHash]) { + this.droppedBuffer[txHash] = true + dropped = false + } else if (dropped && this.droppedBuffer[txHash]) { + // clean up + delete this.droppedBuffer[txHash] + } + + } catch (e) { + log.error(e) + } + if (taken || dropped) { + return this.emit('tx:dropped', txId) } // get latest transaction status try { - const txParams = await this.query.getTransactionByHash(txHash) - if (!txParams) return - if (txParams.blockNumber) { + const { blockNumber } = await this.query.getTransactionByHash(txHash) || {} + if (blockNumber) { this.emit('tx:confirmed', txId) } } catch (err) { @@ -165,6 +186,22 @@ class PendingTransactionTracker extends EventEmitter { this.emit('tx:warning', txMeta, err) } } + /** + checks to see if if the tx's nonce has been used by another transaction + @param txMeta {Object} - txMeta object + @emits tx:dropped + @returns {boolean} + */ + + async _checkIftxWasDropped (txMeta) { + const { txParams: { nonce, from }, hash } = txMeta + const nextNonce = await this.query.getTransactionCount(from) + const { blockNumber } = await this.query.getTransactionByHash(hash) || {} + if (!blockNumber && parseInt(nextNonce) > parseInt(nonce)) { + return true + } + return false + } /** checks to see if a confirmed txMeta has the same nonce diff --git a/test/unit/app/controllers/transactions/pending-tx-test.js b/test/unit/app/controllers/transactions/pending-tx-test.js index 1c5f59f5a..b37ac2766 100644 --- a/test/unit/app/controllers/transactions/pending-tx-test.js +++ b/test/unit/app/controllers/transactions/pending-tx-test.js @@ -58,7 +58,7 @@ describe('PendingTransactionTracker', function () { } }) - it('should become failed if another tx with the same nonce succeeds', async function () { + it('should emit dropped if another tx with the same nonce succeeds', async function () { // SETUP const txGen = new MockTxGen() @@ -84,17 +84,16 @@ describe('PendingTransactionTracker', function () { // THE EXPECTATION const spy = sinon.spy() - pendingTxTracker.on('tx:failed', (txId, err) => { + pendingTxTracker.on('tx:dropped', (txId) => { assert.equal(txId, pending.id, 'should fail the pending tx') - assert.equal(err.name, 'NonceTakenErr', 'should emit a nonce taken error.') - spy(txId, err) + spy(txId) }) // THE METHOD await pendingTxTracker._checkPendingTx(pending) // THE ASSERTION - assert.ok(spy.calledWith(pending.id), 'tx failed should be emitted') + assert.ok(spy.calledWith(pending.id), 'tx dropped should be emitted') }) }) @@ -107,6 +106,38 @@ describe('PendingTransactionTracker', function () { pendingTxTracker._checkPendingTx(txMetaNoHash) }) + it('should emit tx:dropped with the txMetas id only after the second call', function (done) { + txMeta = { + id: 1, + hash: '0x0593ee121b92e10d63150ad08b4b8f9c7857d1bd160195ee648fb9a0f8d00eeb', + status: 'submitted', + txParams: { + from: '0x1678a085c290ebd122dc42cba69373b5953b831d', + nonce: '0x1', + value: '0xfffff', + }, + history: [{}], + rawTx: '0xf86c808504a817c800827b0d940c62bb85faa3311a998d3aba8098c1235c564966880de0b6b3a7640000802aa08ff665feb887a25d4099e40e11f0fef93ee9608f404bd3f853dd9e84ed3317a6a02ec9d3d1d6e176d4d2593dd760e74ccac753e6a0ea0d00cc9789d0d7ff1f471d', + } + + providerResultStub['eth_getTransactionCount'] = '0x02' + providerResultStub['eth_getTransactionByHash'] = {} + pendingTxTracker.once('tx:dropped', (id) => { + if (id === txMeta.id) { + delete providerResultStub['eth_getTransactionCount'] + delete providerResultStub['eth_getTransactionByHash'] + return done() + } else { + done(new Error('wrong tx Id')) + } + }) + + pendingTxTracker._checkPendingTx(txMeta).then(() => { + pendingTxTracker._checkPendingTx(txMeta).catch(done) + }).catch(done) + }) + + it('should should return if query does not return txParams', function () { providerResultStub.eth_getTransactionByHash = null pendingTxTracker._checkPendingTx(txMeta) @@ -283,6 +314,37 @@ describe('PendingTransactionTracker', function () { }) }) + describe('#_checkIftxWasDropped', () => { + const txMeta = { + id: 1, + hash: '0x0593ee121b92e10d63150ad08b4b8f9c7857d1bd160195ee648fb9a0f8d00eeb', + status: 'submitted', + txParams: { + from: '0x1678a085c290ebd122dc42cba69373b5953b831d', + nonce: '0x1', + value: '0xfffff', + }, + rawTx: '0xf86c808504a817c800827b0d940c62bb85faa3311a998d3aba8098c1235c564966880de0b6b3a7640000802aa08ff665feb887a25d4099e40e11f0fef93ee9608f404bd3f853dd9e84ed3317a6a02ec9d3d1d6e176d4d2593dd760e74ccac753e6a0ea0d00cc9789d0d7ff1f471d', + } + it('should return false when the nonce is the suggested network nonce', (done) => { + providerResultStub['eth_getTransactionCount'] = '0x01' + providerResultStub['eth_getTransactionByHash'] = {} + pendingTxTracker._checkIftxWasDropped(txMeta).then((dropped) => { + assert(!dropped, 'should be false') + done() + }).catch(done) + }) + + it('should return true when the network nonce is higher then the txMeta nonce', function (done) { + providerResultStub['eth_getTransactionCount'] = '0x02' + providerResultStub['eth_getTransactionByHash'] = {} + pendingTxTracker._checkIftxWasDropped(txMeta).then((dropped) => { + assert(dropped, 'should be true') + done() + }).catch(done) + }) + }) + describe('#_checkIfNonceIsTaken', function () { beforeEach(function () { const confirmedTxList = [{ |