diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index a861c0342..ce709bd28 100644 --- a/app/scripts/controllers/transactions.js +++ b/app/scripts/controllers/transactions.js @@ -72,6 +72,12 @@ module.exports = class TransactionController extends EventEmitter { }) this.pendingTxTracker.on('tx:failed', this.txStateManager.setTxStatusFailed.bind(this.txStateManager)) this.pendingTxTracker.on('tx:confirmed', this.txStateManager.setTxStatusConfirmed.bind(this.txStateManager)) + this.pendingTxTracker.on('tx:block-update', (txMeta, latestBlockNumber) => { + if (!txMeta.firstRetryBlockNumber) { + txMeta.firstRetryBlockNumber = latestBlockNumber + this.txStateManager.updateTx(txMeta, 'transactions/pending-tx-tracker#event: tx:block-update') + } + }) this.pendingTxTracker.on('tx:retry', (txMeta) => { if (!('retryCount' in txMeta)) txMeta.retryCount = 0 txMeta.retryCount++ diff --git a/app/scripts/lib/pending-tx-tracker.js b/app/scripts/lib/pending-tx-tracker.js index 0d7c6a92c..dc6e526fd 100644 --- a/app/scripts/lib/pending-tx-tracker.js +++ b/app/scripts/lib/pending-tx-tracker.js @@ -65,11 +65,11 @@ module.exports = class PendingTransactionTracker extends EventEmitter { } - resubmitPendingTxs () { + resubmitPendingTxs (block) { const pending = this.getPendingTransactions() // only try resubmitting if their are transactions to resubmit if (!pending.length) return - pending.forEach((txMeta) => this._resubmitTx(txMeta).catch((err) => { + pending.forEach((txMeta) => this._resubmitTx(txMeta, block.number).catch((err) => { /* Dont marked as failed if the error is a "known" transaction warning "there is already a transaction with the same sender-nonce @@ -101,13 +101,25 @@ module.exports = class PendingTransactionTracker extends EventEmitter { })) } - async _resubmitTx (txMeta) { + async _resubmitTx (txMeta, latestBlockNumber) { + if (!txMeta.firstRetryBlockNumber) { + this.emit('tx:block-update', txMeta, latestBlockNumber) + } + if (Date.now() > txMeta.time + this.retryTimePeriod) { const hours = (this.retryTimePeriod / 3.6e+6).toFixed(1) const err = new Error(`Gave up submitting after ${hours} hours.`) return this.emit('tx:failed', txMeta.id, err) } + const firstRetryBlockNumber = txMeta.firstRetryBlockNumber || latestBlockNumber + const txBlockDistance = Number.parseInt(latestBlockNumber, 16) - Number.parseInt(firstRetryBlockNumber, 16) + + const retryCount = txMeta.retryCount || 0 + + // Exponential backoff to limit retries at publishing + if (txBlockDistance <= Math.pow(2, retryCount) - 1) return + // Only auto-submit already-signed txs: if (!('rawTx' in txMeta)) return diff --git a/test/unit/pending-tx-test.js b/test/unit/pending-tx-test.js index 32117a194..961fa6baf 100644 --- a/test/unit/pending-tx-test.js +++ b/test/unit/pending-tx-test.js @@ -207,6 +207,7 @@ describe('PendingTransactionTracker', function () { }) describe('#resubmitPendingTxs', function () { + const blockStub = { number: '0x0' }; beforeEach(function () { const txMeta2 = txMeta3 = txMeta txList = [txMeta, txMeta2, txMeta3].map((tx) => { @@ -224,7 +225,7 @@ describe('PendingTransactionTracker', function () { Promise.all(txList.map((tx) => tx.processed)) .then((txCompletedList) => done()) .catch(done) - pendingTxTracker.resubmitPendingTxs() + pendingTxTracker.resubmitPendingTxs(blockStub) }) it('should not emit \'tx:failed\' if the txMeta throws a known txError', function (done) { knownErrors =[ @@ -251,7 +252,7 @@ describe('PendingTransactionTracker', function () { .then((txCompletedList) => done()) .catch(done) - pendingTxTracker.resubmitPendingTxs() + pendingTxTracker.resubmitPendingTxs(blockStub) }) it('should emit \'tx:warning\' if it encountered a real error', function (done) { pendingTxTracker.once('tx:warning', (txMeta, err) => { @@ -269,28 +270,74 @@ describe('PendingTransactionTracker', function () { .then((txCompletedList) => done()) .catch(done) - pendingTxTracker.resubmitPendingTxs() + pendingTxTracker.resubmitPendingTxs(blockStub) }) }) describe('#_resubmitTx', function () { - it('should publishing the transaction', function (done) { - const enoughBalance = '0x100000' - pendingTxTracker.getBalance = (address) => { - assert.equal(address, txMeta.txParams.from, 'Should pass the address') - return enoughBalance - } - pendingTxTracker.publishTransaction = async (rawTx) => { - assert.equal(rawTx, txMeta.rawTx, 'Should pass the rawTx') - } + const mockFirstRetryBlockNumber = '0x1' + let txMetaToTestExponentialBackoff - // Stubbing out current account state: - // Adding the fake tx: - pendingTxTracker._resubmitTx(txMeta) - .then(() => done()) - .catch((err) => { - assert.ifError(err, 'should not throw an error') - done(err) + beforeEach(() => { + pendingTxTracker.getBalance = (address) => { + assert.equal(address, txMeta.txParams.from, 'Should pass the address') + return enoughBalance + } + pendingTxTracker.publishTransaction = async (rawTx) => { + assert.equal(rawTx, txMeta.rawTx, 'Should pass the rawTx') + } + sinon.spy(pendingTxTracker, 'publishTransaction') + + txMetaToTestExponentialBackoff = Object.assign({}, txMeta, { + retryCount: 4, + firstRetryBlockNumber: mockFirstRetryBlockNumber, + }) + }) + + afterEach(() => { + pendingTxTracker.publishTransaction.reset() + }) + + it('should publish the transaction', function (done) { + const enoughBalance = '0x100000' + + // Stubbing out current account state: + // Adding the fake tx: + pendingTxTracker._resubmitTx(txMeta) + .then(() => done()) + .catch((err) => { + assert.ifError(err, 'should not throw an error') + done(err) + }) + + assert.equal(pendingTxTracker.publishTransaction.callCount, 1, 'Should call publish transaction') + }) + + it('should not publish the transaction if the limit of retries has been exceeded', function (done) { + const enoughBalance = '0x100000' + const mockLatestBlockNumber = '0x5' + + pendingTxTracker._resubmitTx(txMetaToTestExponentialBackoff, mockLatestBlockNumber) + .then(() => done()) + .catch((err) => { + assert.ifError(err, 'should not throw an error') + done(err) + }) + + assert.equal(pendingTxTracker.publishTransaction.callCount, 0, 'Should NOT call publish transaction') + }) + + it('should publish the transaction if the number of blocks since last retry exceeds the last set limit', function (done) { + const enoughBalance = '0x100000' + const mockLatestBlockNumber = '0x11' + + pendingTxTracker._resubmitTx(txMetaToTestExponentialBackoff, mockLatestBlockNumber) + .then(() => done()) + .catch((err) => { + assert.ifError(err, 'should not throw an error') + done(err) + }) + + assert.equal(pendingTxTracker.publishTransaction.callCount, 1, 'Should call publish transaction') }) - }) }) })