fix: transaction-controller – approveTransaction should not throw awa…

…y raw signed transaction (#4255)

## Explanation
Changed `approveTransaction` to use `#updateTransactionInternal`.
Addresses MetaMask/metamask-extension#24183
– Manually verified in extension.

@matthewwalsh0 observed:
The implementation of approveTransaction is throwing away the raw signed
transaction that signTransaction was adding to the metadata state by
holding onto a stale version of the tx metadata.

```typescript
function approveTransaction(id) {
  const updatedMetadata = ...
  
  // signTransaction calls updateTransaction
  // So any existing references to the txMetadata will be stale.
  this.signTransaction(updatedMetadata)
  updateTransaction(updatedMetadata) // BUG: updatedMetadata is now stale.
  //...
```

Long-term @matthewwalsh0 recommends replaying updateTransaction with the
more modern updateTransactionInternal everywhere to avoid bugs like
this.

## References

<!--
Are there any issues that this pull request is tied to? Are there other
links that reviewers should consult to understand these changes better?

For example:


-->

* Related to MetaMask/metamask-extension#24183

## Changelog

<!--
If you're making any consumer-facing changes, list those changes here as
if you were updating a changelog, using the template below as a guide.

(CATEGORY is one of BREAKING, ADDED, CHANGED, DEPRECATED, REMOVED, or
FIXED. For security-related issues, follow the Security Advisory
process.)

Please take care to name the exact pieces of the API you've added or
changed (e.g. types, interfaces, functions, or methods).

If there are any breaking changes, make sure to offer a solution for
consumers to follow once they upgrade to the changes.

Finally, if you're only making changes to development scripts or tests,
you may replace the template below with "None".
-->

### `@metamask/transaction-controller`

- **FIXED**: When resubmitting a pending transaction – if an 'already
seen' error is caught (which indicates that the tx is still pending in
the mempool), we should ignore the error instead of failing the
transaction.

## Checklist

- [ ] I've updated the test suite for new or updated code as appropriate
- [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [ ] I've highlighted breaking changes using the "BREAKING" category
above as appropriate

packages/transaction-controller/jest.config.js

@@ -17,10 +17,10 @@ module.exports = merge(baseConfig, {
   // An object that configures minimum threshold enforcement for coverage results
   coverageThreshold: {
     global: {
-      branches: 93.84,
-      functions: 98.65,
-      lines: 98.92,
-      statements: 98.93,
+      branches: 93.65,
+      functions: 98.38,
+      lines: 98.84,
+      statements: 98.85,
     },
   },
 

packages/transaction-controller/src/TransactionController.test.ts

@@ -4487,13 +4487,10 @@ describe('TransactionController', () => {
 
       expect(signSpy).toHaveBeenCalledTimes(1);
 
-      expect(updateTransactionSpy).toHaveBeenCalledTimes(2);
-      expect(updateTransactionSpy).toHaveBeenCalledWith(
-        expect.objectContaining({
-          txParams: expect.objectContaining(paramsMock),
-        }),
-        'TransactionController#approveTransaction - Transaction approved',
+      expect(transactionMeta.txParams).toStrictEqual(
+        expect.objectContaining(paramsMock),
       );
+      expect(updateTransactionSpy).toHaveBeenCalledTimes(1);
       expect(updateTransactionSpy).toHaveBeenCalledWith(
         expect.objectContaining({
           txParams: expect.objectContaining(paramsMock),
@@ -4518,7 +4515,6 @@ describe('TransactionController', () => {
         },
       });
       const signSpy = jest.spyOn(controller, 'sign');
-      const updateTransactionSpy = jest.spyOn(controller, 'updateTransaction');
 
       await controller.addTransaction(paramsMock, {
         origin: 'origin',
@@ -4531,7 +4527,6 @@ describe('TransactionController', () => {
       await wait(0);
 
       expect(signSpy).toHaveBeenCalledTimes(1);
-      expect(updateTransactionSpy).toHaveBeenCalledTimes(1);
     });
 
     it('adds a transaction, signs and skips publish the transaction', async () => {
@@ -4555,15 +4550,14 @@ describe('TransactionController', () => {
       mockTransactionApprovalRequest.approve();
       await wait(0);
 
-      expect(signSpy).toHaveBeenCalledTimes(1);
+      const transactionMeta = controller.state.transactions[0];
 
-      expect(updateTransactionSpy).toHaveBeenCalledTimes(2);
-      expect(updateTransactionSpy).toHaveBeenCalledWith(
-        expect.objectContaining({
-          txParams: expect.objectContaining(paramsMock),
-        }),
-        'TransactionController#approveTransaction - Transaction approved',
+      expect(transactionMeta.txParams).toStrictEqual(
+        expect.objectContaining(paramsMock),
       );
+
+      expect(signSpy).toHaveBeenCalledTimes(1);
+      expect(updateTransactionSpy).toHaveBeenCalledTimes(1);
       expect(updateTransactionSpy).toHaveBeenCalledWith(
         expect.objectContaining({
           txParams: expect.objectContaining(paramsMock),