diff --git a/app/scripts/controllers/transactions/pending-tx-tracker.js b/app/scripts/controllers/transactions/pending-tx-tracker.js index 027686e9d..427a2f8f0 100644 --- a/app/scripts/controllers/transactions/pending-tx-tracker.js +++ b/app/scripts/controllers/transactions/pending-tx-tracker.js @@ -20,9 +20,25 @@ import EthQuery from 'ethjs-query' */ export default class PendingTransactionTracker extends EventEmitter { + /** + * We wait this many blocks before emitting a 'tx:dropped' event + * + * This is because we could be talking to a node that is out of sync. + * + * @type {number} + */ + DROPPED_BUFFER_COUNT = 3 + + /** + * A map of transaction hashes to the number of blocks we've seen + * since first considering it dropped + * + * @type {Map} + */ + droppedBlocksBufferByHash = new Map() + constructor (config) { super() - this.droppedBuffer = {} this.query = config.query || (new EthQuery(config.provider)) this.nonceTracker = config.nonceTracker this.getPendingTransactions = config.getPendingTransactions @@ -157,70 +173,61 @@ export default class PendingTransactionTracker extends EventEmitter { return } - // *note to self* hard failure point - const transactionReceipt = await this.query.getTransactionReceipt(txHash) - - // If another tx with the same nonce is mined, set as dropped. - const taken = await this._checkIfNonceIsTaken(txMeta) - 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, transactionReceipt) - - // 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 (typeof this.droppedBuffer[txHash] !== 'number') { - this.droppedBuffer[txHash] = 0 - } - - // 3 block count buffer - if (dropped && this.droppedBuffer[txHash] < 3) { - dropped = false - ++this.droppedBuffer[txHash] - } - - if (dropped && this.droppedBuffer[txHash] === 3) { - // clean up - delete this.droppedBuffer[txHash] - } - } catch (e) { - log.error(e) - } - - if (taken || dropped) { + if (await this._checkIfNonceIsTaken(txMeta)) { this.emit('tx:dropped', txId) return } - // get latest transaction status - if (transactionReceipt?.blockNumber) { - this.emit('tx:confirmed', txId, transactionReceipt) - } else { - const err = new Error('Missing transaction receipt or block number.') + try { + const transactionReceipt = await this.query.getTransactionReceipt(txHash) + if (transactionReceipt?.blockNumber) { + this.emit('tx:confirmed', txId, transactionReceipt) + return + } + } catch (err) { txMeta.warning = { error: err.message, message: 'There was a problem loading this transaction.', } this.emit('tx:warning', txMeta, err) + return + } + + if (await this._checkIfTxWasDropped(txMeta)) { + this.emit('tx:dropped', txId) + return } } /** - * Checks whether the nonce in the given {@code txMeta} is correct against the network + * Checks whether the nonce in the given {@code txMeta} is behind the network nonce + * * @param {Object} txMeta - the transaction metadata - * @param {Object} [transactionReceipt] - the transaction receipt * @returns {Promise} * @private */ - async _checkIfTxWasDropped (txMeta, transactionReceipt) { - const { txParams: { nonce, from } } = txMeta - const nextNonce = await this.query.getTransactionCount(from) - return !transactionReceipt?.blockNumber && parseInt(nextNonce) > parseInt(nonce) + async _checkIfTxWasDropped (txMeta) { + const { hash: txHash, txParams: { nonce, from } } = txMeta + const networkNonce = await this.query.getTransactionCount(from) + + if (parseInt(nonce) > parseInt(networkNonce)) { + return false + } + + if (!this.droppedBlocksBufferByHash.has(txHash)) { + this.droppedBlocksBufferByHash.set(txHash, 0) + } + + const currentBlockBuffer = this.droppedBlocksBufferByHash.get(txHash) + + if (currentBlockBuffer < this.DROPPED_BUFFER_COUNT) { + this.droppedBlocksBufferByHash.set(txHash, currentBlockBuffer + 1) + return false + } + + this.droppedBlocksBufferByHash.delete(txHash) + return true } /** diff --git a/test/unit/app/controllers/transactions/pending-tx-tracker-test.js b/test/unit/app/controllers/transactions/pending-tx-tracker-test.js index 602597849..e2b68c775 100644 --- a/test/unit/app/controllers/transactions/pending-tx-tracker-test.js +++ b/test/unit/app/controllers/transactions/pending-tx-tracker-test.js @@ -327,6 +327,8 @@ describe('PendingTransactionTracker', function () { confirmTransaction: sinon.spy(), }) + pendingTxTracker.DROPPED_BUFFER_COUNT = 0 + assert.ok(await pendingTxTracker._checkIfTxWasDropped({ id: 1, hash: '0x0593ee121b92e10d63150ad08b4b8f9c7857d1bd160195ee648fb9a0f8d00eeb', @@ -472,6 +474,53 @@ describe('PendingTransactionTracker', function () { }) describe('#_checkPendingTx', function () { + it("should emit 'tx:warning' if getTransactionReceipt rejects", async function () { + const txMeta = { + id: 1, + hash: '0x0593ee121b92e10d63150ad08b4b8f9c7857d1bd160195ee648fb9a0f8d00eeb', + status: 'submitted', + txParams: { + from: '0x1678a085c290ebd122dc42cba69373b5953b831d', + nonce: '0x1', + value: '0xfffff', + }, + history: [{}], + rawTx: '0xf86c808504a817c80082471d', + } + const pendingTxTracker = new PendingTransactionTracker({ + query: { + getTransactionReceipt: sinon.stub().rejects(), + getTransactionCount: sinon.stub().resolves('0x02'), + }, + nonceTracker: { + getGlobalLock: sinon.stub().resolves({ + releaseLock: sinon.spy(), + }), + }, + getPendingTransactions: sinon.stub().returns([]), + getCompletedTransactions: sinon.stub().returns([]), + publishTransaction: sinon.spy(), + confirmTransaction: sinon.spy(), + }) + const listeners = { + confirmed: sinon.spy(), + dropped: sinon.spy(), + failed: sinon.spy(), + warning: sinon.spy(), + } + + pendingTxTracker.once('tx:confirmed', listeners.confirmed) + pendingTxTracker.once('tx:dropped', listeners.dropped) + pendingTxTracker.once('tx:failed', listeners.failed) + pendingTxTracker.once('tx:warning', listeners.warning) + await pendingTxTracker._checkPendingTx(txMeta) + + assert.ok(listeners.dropped.notCalled, "should not emit 'tx:dropped") + assert.ok(listeners.confirmed.notCalled, "should not emit 'tx:confirmed'") + assert.ok(listeners.failed.notCalled, "should not emit 'tx:failed'") + assert.ok(listeners.warning.calledOnce, "should emit 'tx:warning'") + }) + it('should NOT emit anything if the tx is already not submitted', async function () { const pendingTxTracker = new PendingTransactionTracker({ query: sinon.spy(), @@ -489,11 +538,13 @@ describe('PendingTransactionTracker', function () { confirmed: sinon.spy(), dropped: sinon.spy(), failed: sinon.spy(), + warning: sinon.spy(), } pendingTxTracker.once('tx:confirmed', listeners.confirmed) pendingTxTracker.once('tx:dropped', listeners.dropped) pendingTxTracker.once('tx:failed', listeners.failed) + pendingTxTracker.once('tx:warning', listeners.warning) await pendingTxTracker._checkPendingTx({ 'status': 'confirmed', 'history': [{}], @@ -506,6 +557,7 @@ describe('PendingTransactionTracker', function () { assert.ok(listeners.failed.notCalled, "should not emit 'tx:failed'") assert.ok(listeners.confirmed.notCalled, "should not emit 'tx:confirmed'") assert.ok(listeners.dropped.notCalled, "should not emit 'tx:dropped'") + assert.ok(listeners.warning.notCalled, "should not emit 'tx:warning'") }) it("should emit 'tx:failed' if the txMeta does NOT have a hash", async function () { @@ -525,11 +577,13 @@ describe('PendingTransactionTracker', function () { confirmed: sinon.spy(), dropped: sinon.spy(), failed: sinon.spy(), + warning: sinon.spy(), } pendingTxTracker.once('tx:confirmed', listeners.confirmed) pendingTxTracker.once('tx:dropped', listeners.dropped) pendingTxTracker.once('tx:failed', listeners.failed) + pendingTxTracker.once('tx:warning', listeners.warning) await pendingTxTracker._checkPendingTx({ id: '2', history: [{}], @@ -540,6 +594,7 @@ describe('PendingTransactionTracker', function () { assert.ok(listeners.failed.calledOnceWithExactly('2', sinon.match.instanceOf(Error)), "should pass txId to 'tx:failed' listener") assert.ok(listeners.confirmed.notCalled, "should not emit 'tx:confirmed'") assert.ok(listeners.dropped.notCalled, "should not emit 'tx:dropped'") + assert.ok(listeners.warning.notCalled, "should not emit 'tx:warning'") }) it("should emit 'tx:dropped' if another tx with the same nonce succeeds", async function () { @@ -559,9 +614,9 @@ describe('PendingTransactionTracker', function () { 'hash': '0x2a919d2512ec963f524bfd9730fb66b6d5a2e399d1dd957abb5e2b544a12644b', }] const pendingTxTracker = new PendingTransactionTracker({ - query: sinon.stub({ - getTransactionReceipt: () => undefined, - }), + query: { + getTransactionReceipt: sinon.stub().resolves(null), + }, nonceTracker: { getGlobalLock: sinon.stub().resolves({ releaseLock: sinon.spy(), @@ -576,16 +631,19 @@ describe('PendingTransactionTracker', function () { confirmed: sinon.spy(), dropped: sinon.spy(), failed: sinon.spy(), + warning: sinon.spy(), } pendingTxTracker.once('tx:confirmed', listeners.confirmed) pendingTxTracker.once('tx:dropped', listeners.dropped) pendingTxTracker.once('tx:failed', listeners.failed) + pendingTxTracker.once('tx:warning', listeners.warning) await pendingTxTracker._checkPendingTx(txs[1]) assert.ok(listeners.dropped.calledOnceWithExactly('123')) assert.ok(listeners.confirmed.notCalled, "should not emit 'tx:confirmed'") assert.ok(listeners.failed.notCalled, "should not emit 'tx:failed'") + assert.ok(listeners.warning.notCalled, "should not emit 'tx:warning'") }) it("should emit 'tx:dropped' with the txMetas id only after the fourth call", async function () { @@ -603,7 +661,7 @@ describe('PendingTransactionTracker', function () { } const pendingTxTracker = new PendingTransactionTracker({ query: { - getTransactionReceipt: sinon.stub(), + getTransactionReceipt: sinon.stub().resolves(null), getTransactionCount: sinon.stub().resolves('0x02'), }, nonceTracker: { @@ -620,11 +678,13 @@ describe('PendingTransactionTracker', function () { confirmed: sinon.spy(), dropped: sinon.spy(), failed: sinon.spy(), + warning: sinon.spy(), } pendingTxTracker.once('tx:confirmed', listeners.confirmed) pendingTxTracker.once('tx:dropped', listeners.dropped) pendingTxTracker.once('tx:failed', listeners.failed) + pendingTxTracker.once('tx:warning', listeners.warning) await pendingTxTracker._checkPendingTx(txMeta) await pendingTxTracker._checkPendingTx(txMeta) await pendingTxTracker._checkPendingTx(txMeta) @@ -633,6 +693,7 @@ describe('PendingTransactionTracker', function () { assert.ok(listeners.dropped.calledOnceWithExactly(1)) assert.ok(listeners.confirmed.notCalled, "should not emit 'tx:confirmed'") assert.ok(listeners.failed.notCalled, "should not emit 'tx:failed'") + assert.ok(listeners.warning.notCalled, "should not emit 'tx:warning'") }) }) })