Resubmit approved transactions on new block (#5752)

* Add beginning of test

* Resubmit approved transactions on new block

May fix #4343 and related issues, where an error could leave
transactions stranded in the approved state.

* Remove unused test

* Re-approve transactions when retrying approved

* Add retry approved test

* Include approved in pending tx count

* Fix getPendingTxs()

* Linted

* Only throw hash error in submitted state

* Only check submitted txs for block inclusion

* Fix test expectations

2 files changed, 20 insertions, 4 deletions

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')
     })
   })
 })