diff options
author | kumavis <kumavis@users.noreply.github.com> | 2017-10-07 04:17:45 +0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2017-10-07 04:17:45 +0800 |
commit | 9adf77e3fe324d1366d2caf0044bfd223965d4fa (patch) | |
tree | d4ace491cfd417fc9afca2733b141b92e2a157d2 | |
parent | 53da368d2370edbb78eca063ff701c0492abbff6 (diff) | |
parent | a417fab0ebd71d22f51a8e30590c259b32164fd2 (diff) | |
download | tangerine-wallet-browser-9adf77e3fe324d1366d2caf0044bfd223965d4fa.tar tangerine-wallet-browser-9adf77e3fe324d1366d2caf0044bfd223965d4fa.tar.gz tangerine-wallet-browser-9adf77e3fe324d1366d2caf0044bfd223965d4fa.tar.bz2 tangerine-wallet-browser-9adf77e3fe324d1366d2caf0044bfd223965d4fa.tar.lz tangerine-wallet-browser-9adf77e3fe324d1366d2caf0044bfd223965d4fa.tar.xz tangerine-wallet-browser-9adf77e3fe324d1366d2caf0044bfd223965d4fa.tar.zst tangerine-wallet-browser-9adf77e3fe324d1366d2caf0044bfd223965d4fa.zip |
Merge pull request #2306 from MetaMask/i2294-MarkTxAsFailedAfterSameNonceIsMined
Mark tx as failed after tx with same nonce is mined
-rw-r--r-- | app/scripts/controllers/transactions.js | 1 | ||||
-rw-r--r-- | app/scripts/lib/pending-tx-tracker.js | 20 | ||||
-rw-r--r-- | app/scripts/lib/tx-state-manager.js | 8 | ||||
-rw-r--r-- | test/unit/pending-tx-test.js | 54 |
4 files changed, 81 insertions, 2 deletions
diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index a0f983deb..ef659a300 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -62,6 +62,7 @@ module.exports = class TransactionController extends EventEmitter { retryTimePeriod: 86400000, // Retry 3500 blocks, or about 1 day. publishTransaction: (rawTx) => this.query.sendRawTransaction(rawTx), getPendingTransactions: this.txStateManager.getPendingTransactions.bind(this.txStateManager), + getCompletedTransactions: this.txStateManager.getConfirmedTransactions.bind(this.txStateManager), }) this.txStateManager.store.subscribe(() => this.emit('update:badge')) diff --git a/app/scripts/lib/pending-tx-tracker.js b/app/scripts/lib/pending-tx-tracker.js index 8a626e222..2d8f22ae8 100644 --- a/app/scripts/lib/pending-tx-tracker.js +++ b/app/scripts/lib/pending-tx-tracker.js @@ -25,6 +25,7 @@ module.exports = class PendingTransactionTracker extends EventEmitter { // default is one day this.retryTimePeriod = config.retryTimePeriod || 86400000 this.getPendingTransactions = config.getPendingTransactions + this.getCompletedTransactions = config.getCompletedTransactions this.publishTransaction = config.publishTransaction } @@ -120,6 +121,7 @@ module.exports = class PendingTransactionTracker extends EventEmitter { async _checkPendingTx (txMeta) { const txHash = txMeta.hash const txId = txMeta.id + // extra check in case there was an uncaught error during the // signature and submission process if (!txHash) { @@ -128,6 +130,15 @@ module.exports = class PendingTransactionTracker extends EventEmitter { this.emit('tx:failed', txId, noTxHashErr) return } + + // If another tx with the same nonce is mined, set as failed. + 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) + } + // get latest transaction status let txParams try { @@ -159,4 +170,13 @@ module.exports = class PendingTransactionTracker extends EventEmitter { } nonceGlobalLock.releaseLock() } + + async _checkIfNonceIsTaken (txMeta) { + const completed = this.getCompletedTransactions() + const sameNonce = completed.filter((otherMeta) => { + return otherMeta.txParams.nonce === txMeta.txParams.nonce + }) + return sameNonce.length > 0 + } + } diff --git a/app/scripts/lib/tx-state-manager.js b/app/scripts/lib/tx-state-manager.js index cf8117864..2250403f6 100644 --- a/app/scripts/lib/tx-state-manager.js +++ b/app/scripts/lib/tx-state-manager.js @@ -46,6 +46,12 @@ module.exports = class TransactionStateManger extends EventEmitter { return this.getFilteredTxList(opts) } + getConfirmedTransactions (address) { + const opts = { status: 'confirmed' } + if (address) opts.from = address + return this.getFilteredTxList(opts) + } + addTx (txMeta) { this.once(`${txMeta.id}:signed`, function (txId) { this.removeAllListeners(`${txMeta.id}:rejected`) @@ -242,4 +248,4 @@ module.exports = class TransactionStateManger extends EventEmitter { _saveTxList (transactions) { this.store.updateState({ transactions }) } -}
\ No newline at end of file +} diff --git a/test/unit/pending-tx-test.js b/test/unit/pending-tx-test.js index 6b62bb5b1..32421a44f 100644 --- a/test/unit/pending-tx-test.js +++ b/test/unit/pending-tx-test.js @@ -5,6 +5,8 @@ const ObservableStore = require('obs-store') const clone = require('clone') const { createStubedProvider } = require('../stub/provider') const PendingTransactionTracker = require('../../app/scripts/lib/pending-tx-tracker') +const MockTxGen = require('../lib/mock-tx-gen') +const sinon = require('sinon') const noop = () => true const currentNetworkId = 42 const otherNetworkId = 36 @@ -46,10 +48,60 @@ describe('PendingTransactionTracker', function () { } }, getPendingTransactions: () => {return []}, + getCompletedTransactions: () => {return []}, publishTransaction: () => {}, }) }) + describe('_checkPendingTx state management', function () { + let stub + + afterEach(function () { + if (stub) { + stub.restore() + } + }) + + it('should become failed if another tx with the same nonce succeeds', async function () { + + // SETUP + const txGen = new MockTxGen() + + txGen.generate({ + id: '456', + value: '0x01', + hash: '0xbad', + status: 'confirmed', + nonce: '0x01', + }, { count: 1 }) + + const pending = txGen.generate({ + id: '123', + value: '0x02', + hash: '0xfad', + status: 'submitted', + nonce: '0x01', + }, { count: 1 })[0] + + stub = sinon.stub(pendingTxTracker, 'getCompletedTransactions') + .returns(txGen.txs) + + // THE EXPECTATION + const spy = sinon.spy() + pendingTxTracker.on('tx:failed', (txId, err) => { + assert.equal(txId, pending.id, 'should fail the pending tx') + assert.equal(err.name, 'NonceTakenErr', 'should emit a nonce taken error.') + spy(txId, err) + }) + + // THE METHOD + await pendingTxTracker._checkPendingTx(pending) + + // THE ASSERTION + assert.ok(spy.calledWith(pending.id), 'tx failed should be emitted') + }) + }) + describe('#checkForTxInBlock', function () { it('should return if no pending transactions', function () { // throw a type error if it trys to do anything on the block @@ -239,4 +291,4 @@ describe('PendingTransactionTracker', function () { }) }) }) -})
\ No newline at end of file +}) |