diff options
| -rw-r--r-- | CHANGELOG.md | 2 | ||||
| -rw-r--r-- | app/scripts/controllers/transactions/index.js | 7 | ||||
| -rw-r--r-- | app/scripts/controllers/transactions/pending-tx-tracker.js | 6 | ||||
| -rw-r--r-- | app/scripts/controllers/transactions/tx-state-manager.js | 11 | ||||
| -rw-r--r-- | test/unit/app/controllers/transactions/pending-tx-test.js | 15 | ||||
| -rw-r--r-- | test/unit/app/controllers/transactions/tx-controller-test.js | 9 |
6 files changed, 44 insertions, 6 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 85ac342dc..2c284c4ce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Current Develop Branch +- Resubmit approved transactions on new block, to fix bug where an error can stick transactions in this state. + ## 5.0.2 Friday November 9 2018 - Fixed bug that caused accounts to update slowly to sites. #5717 diff --git a/app/scripts/controllers/transactions/index.js b/app/scripts/controllers/transactions/index.js index 9f2290924..9cf822d34 100644 --- a/app/scripts/controllers/transactions/index.js +++ b/app/scripts/controllers/transactions/index.js @@ -82,7 +82,12 @@ class TransactionController extends EventEmitter { provider: this.provider, nonceTracker: this.nonceTracker, publishTransaction: (rawTx) => this.query.sendRawTransaction(rawTx), - getPendingTransactions: this.txStateManager.getPendingTransactions.bind(this.txStateManager), + getPendingTransactions: () => { + const pending = this.txStateManager.getPendingTransactions() + const approved = this.txStateManager.getApprovedTransactions() + return [...pending, ...approved] + }, + approveTransaction: this.approveTransaction.bind(this), getCompletedTransactions: this.txStateManager.getConfirmedTransactions.bind(this.txStateManager), }) diff --git a/app/scripts/controllers/transactions/pending-tx-tracker.js b/app/scripts/controllers/transactions/pending-tx-tracker.js index 70cac096b..44a50a589 100644 --- a/app/scripts/controllers/transactions/pending-tx-tracker.js +++ b/app/scripts/controllers/transactions/pending-tx-tracker.js @@ -27,6 +27,7 @@ class PendingTransactionTracker extends EventEmitter { this.getPendingTransactions = config.getPendingTransactions this.getCompletedTransactions = config.getCompletedTransactions this.publishTransaction = config.publishTransaction + this.approveTransaction = config.approveTransaction this.confirmTransaction = config.confirmTransaction } @@ -108,7 +109,7 @@ class PendingTransactionTracker extends EventEmitter { if (txBlockDistance <= Math.pow(2, retryCount) - 1) return // Only auto-submit already-signed txs: - if (!('rawTx' in txMeta)) return + if (!('rawTx' in txMeta)) return this.approveTransaction(txMeta.id) const rawTx = txMeta.rawTx const txHash = await this.publishTransaction(rawTx) @@ -129,6 +130,9 @@ class PendingTransactionTracker extends EventEmitter { const txHash = txMeta.hash const txId = txMeta.id + // Only check submitted txs + if (txMeta.status !== 'submitted') return + // extra check in case there was an uncaught error during the // signature and submission process if (!txHash) { diff --git a/app/scripts/controllers/transactions/tx-state-manager.js b/app/scripts/controllers/transactions/tx-state-manager.js index 58c48e34e..62319507d 100644 --- a/app/scripts/controllers/transactions/tx-state-manager.js +++ b/app/scripts/controllers/transactions/tx-state-manager.js @@ -83,6 +83,17 @@ class TransactionStateManager extends EventEmitter { /** @param [address] {string} - hex prefixed address to sort the txMetas for [optional] + @returns {array} the tx list whos status is approved if no address is provide + returns all txMetas who's status is approved for the current network + */ + getApprovedTransactions(address) { + const opts = { status: 'approved' } + if (address) opts.from = address + return this.getFilteredTxList(opts) + } + + /** + @param [address] {string} - hex prefixed address to sort the txMetas for [optional] @returns {array} the tx list whos status is submitted if no address is provide returns all txMetas who's status is submitted for the current network */ diff --git a/test/unit/app/controllers/transactions/pending-tx-test.js b/test/unit/app/controllers/transactions/pending-tx-test.js index ba15f1953..85b0969f5 100644 --- a/test/unit/app/controllers/transactions/pending-tx-test.js +++ b/test/unit/app/controllers/transactions/pending-tx-test.js @@ -24,7 +24,7 @@ describe('PendingTransactionTracker', function () { } txMetaNoHash = { id: 2, - status: 'signed', + status: 'submitted', txParams: { from: '0x1678a085c290ebd122dc42cba69373b5953b831d'}, } @@ -212,6 +212,7 @@ describe('PendingTransactionTracker', function () { pendingTxTracker.publishTransaction = async (rawTx) => { assert.equal(rawTx, txMeta.rawTx, 'Should pass the rawTx') } + pendingTxTracker.approveTransaction = async () => {} sinon.spy(pendingTxTracker, 'publishTransaction') txMetaToTestExponentialBackoff = Object.assign({}, txMeta, { @@ -266,6 +267,18 @@ describe('PendingTransactionTracker', function () { assert.equal(pendingTxTracker.publishTransaction.callCount, 1, 'Should call publish transaction') }) + + it('should call opts.approveTransaction with the id if the tx is not signed', async () => { + const stubTx = { + id: 40, + } + const approveMock = sinon.stub(pendingTxTracker, 'approveTransaction') + + pendingTxTracker._resubmitTx(stubTx) + + assert.ok(approveMock.called) + approveMock.restore() + }) }) describe('#_checkIfNonceIsTaken', function () { diff --git a/test/unit/app/controllers/transactions/tx-controller-test.js b/test/unit/app/controllers/transactions/tx-controller-test.js index ea58aa560..7f1d8e6f5 100644 --- a/test/unit/app/controllers/transactions/tx-controller-test.js +++ b/test/unit/app/controllers/transactions/tx-controller-test.js @@ -313,6 +313,7 @@ describe('Transaction Controller', function () { assert.equal(params.gas, originalValue, 'gas unmodified') assert.equal(params.gasPrice, originalValue, 'gas price unmodified') assert.equal(result.hash, originalValue, `hash was set \n got: ${result.hash} \n expected: ${originalValue}`) + assert.equal(result.status, 'submitted', 'Should have reached the submitted status.') signStub.restore() pubStub.restore() done() @@ -469,9 +470,11 @@ describe('Transaction Controller', function () { { id: 7, status: 'failed', metamaskNetworkId: currentNetworkId, txParams: {} }, ]) }) - it('should show only submitted transactions as pending transasction', function () { - assert(txController.pendingTxTracker.getPendingTransactions().length, 1) - assert(txController.pendingTxTracker.getPendingTransactions()[0].status, 'submitted') + it('should show only submitted and approved transactions as pending transasction', function () { + assert(txController.pendingTxTracker.getPendingTransactions().length, 2) + const states = txController.pendingTxTracker.getPendingTransactions().map(tx => tx.status) + assert(states.includes('approved'), 'includes approved') + assert(states.includes('submitted'), 'includes submitted') }) }) }) |