Await approval request in transaction controller (#19197)

app/scripts/background.js

@@ -676,10 +676,8 @@ export function setupController(
   //
   // User Interface setup
   //
+  updateBadge();
 
-  controller.txController.initApprovals().then(() => {
-    updateBadge();
-  });
   controller.txController.on(
     METAMASK_CONTROLLER_EVENTS.UPDATE_BADGE,
     updateBadge,
@@ -706,6 +704,8 @@ export function setupController(
     updateBadge,
   );
 
+  controller.txController.initApprovals();
+
   /**
    * Updates the Web Extension's "badge" number, on the little fox in the toolbar.
    * The number reflects the current number of pending transactions or message signatures needing user approval.

app/scripts/controllers/transactions/index.js

@@ -2,7 +2,7 @@ import EventEmitter from '@metamask/safe-event-emitter';
 import { ObservableStore } from '@metamask/obs-store';
 import { bufferToHex, keccak, toBuffer, isHexString } from 'ethereumjs-util';
 import EthQuery from 'ethjs-query';
-import { ethErrors } from 'eth-rpc-errors';
+import { errorCodes, ethErrors } from 'eth-rpc-errors';
 import { Common, Hardfork } from '@ethereumjs/common';
 import { TransactionFactory } from '@ethereumjs/tx';
 import { ApprovalType } from '@metamask/controller-utils';
@@ -202,7 +202,7 @@ export default class TransactionController extends EventEmitter {
         const approved = this.txStateManager.getApprovedTransactions();
         return [...pending, ...approved];
       },
-      approveTransaction: this.approveTransaction.bind(this),
+      approveTransaction: this._approveTransaction.bind(this),
       getCompletedTransactions:
         this.txStateManager.getConfirmedTransactions.bind(this.txStateManager),
     });
@@ -347,7 +347,7 @@ export default class TransactionController extends EventEmitter {
       `MetaMaskController newUnapprovedTransaction ${JSON.stringify(txParams)}`,
     );
 
-    const initialTxMeta = await this.addUnapprovedTransaction(
+    const { txMeta: initialTxMeta, isExisting } = await this._createTransaction(
       opts.method,
       txParams,
       opts.origin,
@@ -356,58 +356,59 @@ export default class TransactionController extends EventEmitter {
       opts.id,
     );
 
-    // listen for tx completion (success, fail)
+    const txId = initialTxMeta.id;
-    return new Promise((resolve, reject) => {
+    const isCompleted = this._isTransactionCompleted(initialTxMeta);
-      this.txStateManager.once(
+
-        `${initialTxMeta.id}:finished`,
+    const finishedPromise = isCompleted
-        (finishedTxMeta) => {
+      ? Promise.resolve(initialTxMeta)
-          switch (finishedTxMeta.status) {
+      : this._waitForTransactionFinished(txId);
-            case TransactionStatus.submitted:
+
-              return resolve(finishedTxMeta.hash);
+    if (!isExisting && !isCompleted) {
-            case TransactionStatus.rejected:
+      try {
-              return reject(
+        await this._requestTransactionApproval(initialTxMeta);
-                cleanErrorStack(
+      } catch (error) {
-                  ethErrors.provider.userRejectedRequest(
+        // Errors generated from final status using finished event
-                    'MetaMask Tx Signature: User denied transaction signature.',
+      }
-                  ),
+    }
-                ),
+
-              );
+    const finalTxMeta = await finishedPromise;
-            case TransactionStatus.failed:
+    const finalStatus = finalTxMeta?.status;
-              return reject(
+
-                cleanErrorStack(
+    switch (finalStatus) {
-                  ethErrors.rpc.internal(finishedTxMeta.err.message),
+      case TransactionStatus.submitted:
-                ),
+        return finalTxMeta.hash;
-              );
+      case TransactionStatus.rejected:
-            default:
+        throw cleanErrorStack(
-              return reject(
+          ethErrors.provider.userRejectedRequest(
-                cleanErrorStack(
+            'MetaMask Tx Signature: User denied transaction signature.',
-                  ethErrors.rpc.internal(
+          ),
-                    `MetaMask Tx Signature: Unknown problem: ${JSON.stringify(
+        );
-                      finishedTxMeta.txParams,
+      case TransactionStatus.failed:
-                    )}`,
+        throw cleanErrorStack(ethErrors.rpc.internal(finalTxMeta.err.message));
-                  ),
+      default:
-                ),
+        throw cleanErrorStack(
-              );
+          ethErrors.rpc.internal(
-          }
+            `MetaMask Tx Signature: Unknown problem: ${JSON.stringify(
-        },
+              finalTxMeta?.txParams,
-      );
+            )}`,
-    });
+          ),
+        );
+    }
   }
 
   /**
    * Creates approvals for all unapproved transactions in the txStateManager.
-   *
-   * @returns {Promise<void>}
    */
-  async initApprovals() {
+  initApprovals() {
     const unapprovedTxs = this.txStateManager.getUnapprovedTxList();
-    return Promise.all(
+
-      Object.values(unapprovedTxs).map((txMeta) =>
+    Object.values(unapprovedTxs).forEach((txMeta) => {
-        this._requestApproval(txMeta, {
+      this._requestTransactionApproval(txMeta, {
-          shouldShowRequest: false,
+        shouldShowRequest: false,
-        }),
+      }).catch((error) => {
-      ),
+        log.error('Error during persisted transaction approval', error);
-    );
+      });
+    });
   }
 
   // ====================================================================================================================================================
@@ -606,81 +607,6 @@ export default class TransactionController extends EventEmitter {
     return this._getTransaction(txId);
   }
 
-  /**
-   * updates a swap approval transaction with provided metadata and source token symbol
-   *  if the transaction state is unapproved.
-   *
-   * @param {string} txId
-   * @param {object} swapApprovalTransaction - holds the metadata and token symbol
-   * @param {string} swapApprovalTransaction.type
-   * @param {string} swapApprovalTransaction.sourceTokenSymbol
-   * @returns {TransactionMeta} the txMeta of the updated transaction
-   */
-  updateSwapApprovalTransaction(txId, { type, sourceTokenSymbol }) {
-    this._throwErrorIfNotUnapprovedTx(txId, 'updateSwapApprovalTransaction');
-
-    let swapApprovalTransaction = { type, sourceTokenSymbol };
-    // only update what is defined
-    swapApprovalTransaction = pickBy(swapApprovalTransaction);
-
-    const note = `Update Swap Approval Transaction for ${txId}`;
-    this._updateTransaction(txId, swapApprovalTransaction, note);
-    return this._getTransaction(txId);
-  }
-
-  /**
-   * updates a swap transaction with provided metadata and source token symbol
-   *  if the transaction state is unapproved.
-   *
-   * @param {string} txId
-   * @param {object} swapTransaction - holds the metadata
-   * @param {string} swapTransaction.sourceTokenSymbol
-   * @param {string} swapTransaction.destinationTokenSymbol
-   * @param {string} swapTransaction.type
-   * @param {string} swapTransaction.destinationTokenDecimals
-   * @param {string} swapTransaction.destinationTokenAddress
-   * @param {string} swapTransaction.swapMetaData
-   * @param {string} swapTransaction.swapTokenValue
-   * @param {string} swapTransaction.estimatedBaseFee
-   * @param {string} swapTransaction.approvalTxId
-   * @returns {TransactionMeta} the txMeta of the updated transaction
-   */
-  updateSwapTransaction(
-    txId,
-    {
-      sourceTokenSymbol,
-      destinationTokenSymbol,
-      type,
-      destinationTokenDecimals,
-      destinationTokenAddress,
-      swapMetaData,
-      swapTokenValue,
-      estimatedBaseFee,
-      approvalTxId,
-    },
-  ) {
-    this._throwErrorIfNotUnapprovedTx(txId, 'updateSwapTransaction');
-
-    let swapTransaction = {
-      sourceTokenSymbol,
-      destinationTokenSymbol,
-      type,
-      destinationTokenDecimals,
-      destinationTokenAddress,
-      swapMetaData,
-      swapTokenValue,
-      estimatedBaseFee,
-      approvalTxId,
-    };
-
-    // only update what is defined
-    swapTransaction = pickBy(swapTransaction);
-
-    const note = `Update Swap Transaction for ${txId}`;
-    this._updateTransaction(txId, swapTransaction, note);
-    return this._getTransaction(txId);
-  }
-
   /**
    * updates a transaction's user settings only if the transaction state is unapproved
    *
@@ -789,7 +715,7 @@ export default class TransactionController extends EventEmitter {
    * @param transactionType
    * @param sendFlowHistory
    * @param actionId
-   * @returns {txMeta}
+   * @param options
    */
   async addUnapprovedTransaction(
     txMethodType,
@@ -798,98 +724,30 @@ export default class TransactionController extends EventEmitter {
     transactionType,
     sendFlowHistory = [],
     actionId,
+    options,
   ) {
-    if (
+    const { txMeta, isExisting } = await this._createTransaction(
-      transactionType !== undefined &&
+      txMethodType,
-      !VALID_UNAPPROVED_TRANSACTION_TYPES.includes(transactionType)
+      txParams,
-    ) {
-      throw new Error(
-        `TransactionController - invalid transactionType value: ${transactionType}`,
-      );
-    }
-
-    // If a transaction is found with the same actionId, do not create a new speed-up transaction.
-    if (actionId) {
-      let existingTxMeta =
-        this.txStateManager.getTransactionWithActionId(actionId);
-      if (existingTxMeta) {
-        this.emit('newUnapprovedTx', existingTxMeta);
-        existingTxMeta = await this.addTransactionGasDefaults(existingTxMeta);
-        this._requestApproval(existingTxMeta);
-        return existingTxMeta;
-      }
-    }
-
-    // validate
-    const normalizedTxParams = txUtils.normalizeTxParams(txParams);
-    const eip1559Compatibility = await this.getEIP1559Compatibility();
-
-    txUtils.validateTxParams(normalizedTxParams, eip1559Compatibility);
-
-    /**
-     * `generateTxMeta` adds the default txMeta properties to the passed object.
-     * These include the tx's `id`. As we use the id for determining order of
-     * txes in the tx-state-manager, it is necessary to call the asynchronous
-     * method `determineTransactionType` after `generateTxMeta`.
-     */
-    let txMeta = this.txStateManager.generateTxMeta({
-      txParams: normalizedTxParams,
       origin,
+      transactionType,
       sendFlowHistory,
-    });
+      actionId,
-
+      options,
-    // Add actionId to txMeta to check if same actionId is seen again
-    // IF request to create transaction with same actionId is submitted again, new transaction will not be added for it.
-    if (actionId) {
-      txMeta.actionId = actionId;
-    }
-
-    if (origin === ORIGIN_METAMASK) {
-      // Assert the from address is the selected address
-      if (normalizedTxParams.from !== this.getSelectedAddress()) {
-        throw ethErrors.rpc.internal({
-          message: `Internally initiated transaction is using invalid account.`,
-          data: {
-            origin,
-            fromAddress: normalizedTxParams.from,
-            selectedAddress: this.getSelectedAddress(),
-          },
-        });
-      }
-    } else {
-      // Assert that the origin has permissions to initiate transactions from
-      // the specified address
-      const permittedAddresses = await this.getPermittedAccounts(origin);
-      if (!permittedAddresses.includes(normalizedTxParams.from)) {
-        throw ethErrors.provider.unauthorized({ data: { origin } });
-      }
-    }
-
-    const { type } = await determineTransactionType(
-      normalizedTxParams,
-      this.query,
     );
-    txMeta.type = transactionType || type;
+    if (isExisting) {
+      const isCompleted = this._isTransactionCompleted(txMeta);
 
-    // ensure value
+      return isCompleted
-    txMeta.txParams.value = txMeta.txParams.value
+        ? txMeta
-      ? addHexPrefix(txMeta.txParams.value)
+        : await this._waitForTransactionFinished(txMeta.id);
-      : '0x0';
-
-    if (txMethodType && this.securityProviderRequest) {
-      const securityProviderResponse = await this.securityProviderRequest(
-        txMeta,
-        txMethodType,
-      );
-
-      txMeta.securityProviderResponse = securityProviderResponse;
     }
 
-    this.addTransaction(txMeta);
+    if (options?.requireApproval === false) {
-    this.emit('newUnapprovedTx', txMeta);
+      await this._updateAndApproveTransaction(txMeta, actionId);
-
+    } else {
-    txMeta = await this.addTransactionGasDefaults(txMeta);
+      await this._requestTransactionApproval(txMeta, { actionId });
-    this._requestApproval(txMeta);
+    }
 
     return txMeta;
   }
@@ -1260,7 +1118,7 @@ export default class TransactionController extends EventEmitter {
     }
 
     this.addTransaction(newTxMeta);
-    await this.approveTransaction(newTxMeta.id, actionId, {
+    await this._approveTransaction(newTxMeta.id, actionId, {
       hasApprovalRequest: false,
     });
     return newTxMeta;
@@ -1320,9 +1178,7 @@ export default class TransactionController extends EventEmitter {
     }
 
     this.addTransaction(newTxMeta);
-    await this.approveTransaction(newTxMeta.id, actionId, {
+    await this._approveTransaction(newTxMeta.id, actionId);
-      hasApprovalRequest: false,
-    });
     return newTxMeta;
   }
 
@@ -1338,113 +1194,6 @@ export default class TransactionController extends EventEmitter {
     );
   }
 
-  /**
-   * updates and approves the transaction
-   *
-   * @param {object} txMeta
-   * @param {string} actionId
-   */
-  async updateAndApproveTransaction(txMeta, actionId) {
-    this.txStateManager.updateTransaction(
-      txMeta,
-      'confTx: user approved transaction',
-    );
-    await this.approveTransaction(txMeta.id, actionId);
-  }
-
-  /**
-   * sets the tx status to approved
-   * auto fills the nonce
-   * signs the transaction
-   * publishes the transaction
-   * if any of these steps fails the tx status will be set to failed
-   *
-   * @param {number} txId - the tx's Id
-   * @param {string} actionId - actionId passed from UI
-   * @param opts - options object
-   * @param opts.hasApprovalRequest - whether the transaction has an approval request
-   */
-  async approveTransaction(txId, actionId, { hasApprovalRequest = true } = {}) {
-    // TODO: Move this safety out of this function.
-    // Since this transaction is async,
-    // we need to keep track of what is currently being signed,
-    // So that we do not increment nonce + resubmit something
-    // that is already being incremented & signed.
-    const txMeta = this.txStateManager.getTransaction(txId);
-
-    ///: BEGIN:ONLY_INCLUDE_IN(build-mmi)
-    // MMI does not broadcast transactions, as that is the responsibility of the custodian
-    if (txMeta.custodyStatus) {
-      this.inProcessOfSigning.delete(txId);
-      await this.signTransaction(txId);
-      return;
-    }
-    ///: END:ONLY_INCLUDE_IN
-
-    if (this.inProcessOfSigning.has(txId)) {
-      return;
-    }
-    this.inProcessOfSigning.add(txId);
-    let nonceLock;
-    try {
-      // approve
-      this.txStateManager.setTxStatusApproved(txId);
-      if (hasApprovalRequest) {
-        this._acceptApproval(txMeta);
-      }
-      // get next nonce
-      const fromAddress = txMeta.txParams.from;
-      // wait for a nonce
-      let { customNonceValue } = txMeta;
-      customNonceValue = Number(customNonceValue);
-      nonceLock = await this.nonceTracker.getNonceLock(fromAddress);
-      // add nonce to txParams
-      // if txMeta has previousGasParams then it is a retry at same nonce with
-      // higher gas settings and therefor the nonce should not be recalculated
-      const nonce = txMeta.previousGasParams
-        ? txMeta.txParams.nonce
-        : nonceLock.nextNonce;
-      const customOrNonce =
-        customNonceValue === 0 ? customNonceValue : customNonceValue || nonce;
-
-      txMeta.txParams.nonce = addHexPrefix(customOrNonce.toString(16));
-      // add nonce debugging information to txMeta
-      txMeta.nonceDetails = nonceLock.nonceDetails;
-      if (customNonceValue) {
-        txMeta.nonceDetails.customNonceValue = customNonceValue;
-      }
-      this.txStateManager.updateTransaction(
-        txMeta,
-        'transactions#approveTransaction',
-      );
-      // sign transaction
-      const rawTx = await this.signTransaction(txId);
-      await this.publishTransaction(txId, rawTx, actionId);
-      this._trackTransactionMetricsEvent(
-        txMeta,
-        TransactionMetaMetricsEvent.approved,
-        actionId,
-      );
-      // must set transaction to submitted/failed before releasing lock
-      nonceLock.releaseLock();
-    } catch (err) {
-      // this is try-catch wrapped so that we can guarantee that the nonceLock is released
-      try {
-        this._failTransaction(txId, err, actionId);
-      } catch (err2) {
-        log.error(err2);
-      }
-      // must set transaction to submitted/failed before releasing lock
-      if (nonceLock) {
-        nonceLock.releaseLock();
-      }
-      // continue with error chain
-      throw err;
-    } finally {
-      this.inProcessOfSigning.delete(txId);
-    }
-  }
-
   async approveTransactionsWithSameNonce(listOfTxParams = []) {
     if (listOfTxParams.length === 0) {
       return '';
@@ -1779,24 +1528,6 @@ export default class TransactionController extends EventEmitter {
     }
   }
 
-  /**
-   * Convenience method for the ui thats sets the transaction to rejected
-   *
-   * @param {number} txId - the tx's Id
-   * @param {string} actionId - actionId passed from UI
-   * @returns {Promise<void>}
-   */
-  async cancelTransaction(txId, actionId) {
-    const txMeta = this.txStateManager.getTransaction(txId);
-    this.txStateManager.setTxStatusRejected(txId);
-    this._rejectApproval(txMeta);
-    this._trackTransactionMetricsEvent(
-      txMeta,
-      TransactionMetaMetricsEvent.rejected,
-      actionId,
-    );
-  }
-
   /**
    * Sets the txHas on the txMeta
    *
@@ -1835,6 +1566,368 @@ export default class TransactionController extends EventEmitter {
   //
   //           PRIVATE METHODS
   //
+
+  _isTransactionCompleted(txMeta) {
+    return [
+      TransactionStatus.submitted,
+      TransactionStatus.rejected,
+      TransactionStatus.failed,
+      TransactionStatus.dropped,
+      TransactionStatus.confirmed,
+    ].includes(txMeta.status);
+  }
+
+  async _waitForTransactionFinished(txId) {
+    return new Promise((resolve) => {
+      this.txStateManager.once(`${txId}:finished`, (txMeta) => {
+        resolve(txMeta);
+      });
+    });
+  }
+
+  async _createTransaction(
+    txMethodType,
+    txParams,
+    origin,
+    transactionType,
+    sendFlowHistory = [],
+    actionId,
+    options,
+  ) {
+    if (
+      transactionType !== undefined &&
+      !VALID_UNAPPROVED_TRANSACTION_TYPES.includes(transactionType)
+    ) {
+      throw new Error(
+        `TransactionController - invalid transactionType value: ${transactionType}`,
+      );
+    }
+
+    // If a transaction is found with the same actionId, do not create a new speed-up transaction.
+    if (actionId) {
+      let existingTxMeta =
+        this.txStateManager.getTransactionWithActionId(actionId);
+      if (existingTxMeta) {
+        existingTxMeta = await this.addTransactionGasDefaults(existingTxMeta);
+        return { txMeta: existingTxMeta, isExisting: true };
+      }
+    }
+
+    // validate
+    const normalizedTxParams = txUtils.normalizeTxParams(txParams);
+    const eip1559Compatibility = await this.getEIP1559Compatibility();
+
+    txUtils.validateTxParams(normalizedTxParams, eip1559Compatibility);
+
+    /**
+     * `generateTxMeta` adds the default txMeta properties to the passed object.
+     * These include the tx's `id`. As we use the id for determining order of
+     * txes in the tx-state-manager, it is necessary to call the asynchronous
+     * method `determineTransactionType` after `generateTxMeta`.
+     */
+    let txMeta = this.txStateManager.generateTxMeta({
+      txParams: normalizedTxParams,
+      origin,
+      sendFlowHistory,
+    });
+
+    // Add actionId to txMeta to check if same actionId is seen again
+    // IF request to create transaction with same actionId is submitted again, new transaction will not be added for it.
+    if (actionId) {
+      txMeta.actionId = actionId;
+    }
+
+    if (origin === ORIGIN_METAMASK) {
+      // Assert the from address is the selected address
+      if (normalizedTxParams.from !== this.getSelectedAddress()) {
+        throw ethErrors.rpc.internal({
+          message: `Internally initiated transaction is using invalid account.`,
+          data: {
+            origin,
+            fromAddress: normalizedTxParams.from,
+            selectedAddress: this.getSelectedAddress(),
+          },
+        });
+      }
+    } else {
+      // Assert that the origin has permissions to initiate transactions from
+      // the specified address
+      const permittedAddresses = await this.getPermittedAccounts(origin);
+      if (!permittedAddresses.includes(normalizedTxParams.from)) {
+        throw ethErrors.provider.unauthorized({ data: { origin } });
+      }
+    }
+
+    const { type } = await determineTransactionType(
+      normalizedTxParams,
+      this.query,
+    );
+    txMeta.type = transactionType || type;
+
+    // ensure value
+    txMeta.txParams.value = txMeta.txParams.value
+      ? addHexPrefix(txMeta.txParams.value)
+      : '0x0';
+
+    if (txMethodType && this.securityProviderRequest) {
+      const securityProviderResponse = await this.securityProviderRequest(
+        txMeta,
+        txMethodType,
+      );
+
+      txMeta.securityProviderResponse = securityProviderResponse;
+    }
+
+    this.addTransaction(txMeta);
+
+    txMeta = await this.addTransactionGasDefaults(txMeta);
+
+    if (
+      [TransactionType.swap, TransactionType.swapApproval].includes(
+        transactionType,
+      )
+    ) {
+      txMeta = await this._createSwapsTransaction(
+        options?.swaps,
+        transactionType,
+        txMeta,
+      );
+    }
+
+    return { txMeta, isExisting: false };
+  }
+
+  async _createSwapsTransaction(swapOptions, transactionType, txMeta) {
+    // The simulationFails property is added if the estimateGas call fails. In cases
+    // when no swaps approval tx is required, this indicates that the swap will likely
+    // fail. There was an earlier estimateGas call made by the swaps controller,
+    // but it is possible that external conditions have change since then, and
+    // a previously succeeding estimate gas call could now fail. By checking for
+    // the `simulationFails` property here, we can reduce the number of swap
+    // transactions that get published to the blockchain only to fail and thereby
+    // waste the user's funds on gas.
+    if (
+      transactionType === TransactionType.swap &&
+      swapOptions?.hasApproveTx === false &&
+      txMeta.simulationFails
+    ) {
+      await this._cancelTransaction(txMeta.id);
+      throw new Error('Simulation failed');
+    }
+
+    const swapsMeta = swapOptions?.meta;
+
+    if (!swapsMeta) {
+      return txMeta;
+    }
+
+    if (transactionType === TransactionType.swapApproval) {
+      this.emit('newSwapApproval', txMeta);
+      return this._updateSwapApprovalTransaction(txMeta.id, swapsMeta);
+    }
+
+    if (transactionType === TransactionType.swap) {
+      this.emit('newSwap', txMeta);
+      return this._updateSwapTransaction(txMeta.id, swapsMeta);
+    }
+
+    return txMeta;
+  }
+
+  /**
+   * updates a swap approval transaction with provided metadata and source token symbol
+   *  if the transaction state is unapproved.
+   *
+   * @param {string} txId
+   * @param {object} swapApprovalTransaction - holds the metadata and token symbol
+   * @param {string} swapApprovalTransaction.type
+   * @param {string} swapApprovalTransaction.sourceTokenSymbol
+   * @returns {TransactionMeta} the txMeta of the updated transaction
+   */
+  _updateSwapApprovalTransaction(txId, { type, sourceTokenSymbol }) {
+    this._throwErrorIfNotUnapprovedTx(txId, 'updateSwapApprovalTransaction');
+
+    let swapApprovalTransaction = { type, sourceTokenSymbol };
+    // only update what is defined
+    swapApprovalTransaction = pickBy(swapApprovalTransaction);
+
+    const note = `Update Swap Approval Transaction for ${txId}`;
+    this._updateTransaction(txId, swapApprovalTransaction, note);
+    return this._getTransaction(txId);
+  }
+
+  /**
+   * updates a swap transaction with provided metadata and source token symbol
+   *  if the transaction state is unapproved.
+   *
+   * @param {string} txId
+   * @param {object} swapTransaction - holds the metadata
+   * @param {string} swapTransaction.sourceTokenSymbol
+   * @param {string} swapTransaction.destinationTokenSymbol
+   * @param {string} swapTransaction.type
+   * @param {string} swapTransaction.destinationTokenDecimals
+   * @param {string} swapTransaction.destinationTokenAddress
+   * @param {string} swapTransaction.swapMetaData
+   * @param {string} swapTransaction.swapTokenValue
+   * @param {string} swapTransaction.estimatedBaseFee
+   * @param {string} swapTransaction.approvalTxId
+   * @returns {TransactionMeta} the txMeta of the updated transaction
+   */
+  _updateSwapTransaction(
+    txId,
+    {
+      sourceTokenSymbol,
+      destinationTokenSymbol,
+      type,
+      destinationTokenDecimals,
+      destinationTokenAddress,
+      swapMetaData,
+      swapTokenValue,
+      estimatedBaseFee,
+      approvalTxId,
+    },
+  ) {
+    this._throwErrorIfNotUnapprovedTx(txId, 'updateSwapTransaction');
+
+    let swapTransaction = {
+      sourceTokenSymbol,
+      destinationTokenSymbol,
+      type,
+      destinationTokenDecimals,
+      destinationTokenAddress,
+      swapMetaData,
+      swapTokenValue,
+      estimatedBaseFee,
+      approvalTxId,
+    };
+
+    // only update what is defined
+    swapTransaction = pickBy(swapTransaction);
+
+    const note = `Update Swap Transaction for ${txId}`;
+    this._updateTransaction(txId, swapTransaction, note);
+    return this._getTransaction(txId);
+  }
+
+  /**
+   * updates and approves the transaction
+   *
+   * @param {object} txMeta
+   * @param {string} actionId
+   */
+  async _updateAndApproveTransaction(txMeta, actionId) {
+    this.txStateManager.updateTransaction(
+      txMeta,
+      'confTx: user approved transaction',
+    );
+    await this._approveTransaction(txMeta.id, actionId);
+  }
+
+  /**
+   * sets the tx status to approved
+   * auto fills the nonce
+   * signs the transaction
+   * publishes the transaction
+   * if any of these steps fails the tx status will be set to failed
+   *
+   * @param {number} txId - the tx's Id
+   * @param {string} actionId - actionId passed from UI
+   */
+  async _approveTransaction(txId, actionId) {
+    // TODO: Move this safety out of this function.
+    // Since this transaction is async,
+    // we need to keep track of what is currently being signed,
+    // So that we do not increment nonce + resubmit something
+    // that is already being incremented & signed.
+    const txMeta = this.txStateManager.getTransaction(txId);
+
+    ///: BEGIN:ONLY_INCLUDE_IN(build-mmi)
+    // MMI does not broadcast transactions, as that is the responsibility of the custodian
+    if (txMeta.custodyStatus) {
+      this.inProcessOfSigning.delete(txId);
+      await this.signTransaction(txId);
+      return;
+    }
+    ///: END:ONLY_INCLUDE_IN
+
+    if (this.inProcessOfSigning.has(txId)) {
+      return;
+    }
+    this.inProcessOfSigning.add(txId);
+    let nonceLock;
+    try {
+      // approve
+      this.txStateManager.setTxStatusApproved(txId);
+      // get next nonce
+      const fromAddress = txMeta.txParams.from;
+      // wait for a nonce
+      let { customNonceValue } = txMeta;
+      customNonceValue = Number(customNonceValue);
+      nonceLock = await this.nonceTracker.getNonceLock(fromAddress);
+      // add nonce to txParams
+      // if txMeta has previousGasParams then it is a retry at same nonce with
+      // higher gas settings and therefor the nonce should not be recalculated
+      const nonce = txMeta.previousGasParams
+        ? txMeta.txParams.nonce
+        : nonceLock.nextNonce;
+      const customOrNonce =
+        customNonceValue === 0 ? customNonceValue : customNonceValue || nonce;
+
+      txMeta.txParams.nonce = addHexPrefix(customOrNonce.toString(16));
+      // add nonce debugging information to txMeta
+      txMeta.nonceDetails = nonceLock.nonceDetails;
+      if (customNonceValue) {
+        txMeta.nonceDetails.customNonceValue = customNonceValue;
+      }
+      this.txStateManager.updateTransaction(
+        txMeta,
+        'transactions#approveTransaction',
+      );
+      // sign transaction
+      const rawTx = await this.signTransaction(txId);
+      await this.publishTransaction(txId, rawTx, actionId);
+      this._trackTransactionMetricsEvent(
+        txMeta,
+        TransactionMetaMetricsEvent.approved,
+        actionId,
+      );
+      // must set transaction to submitted/failed before releasing lock
+      nonceLock.releaseLock();
+    } catch (err) {
+      // this is try-catch wrapped so that we can guarantee that the nonceLock is released
+      try {
+        this._failTransaction(txId, err, actionId);
+      } catch (err2) {
+        log.error(err2);
+      }
+      // must set transaction to submitted/failed before releasing lock
+      if (nonceLock) {
+        nonceLock.releaseLock();
+      }
+      // continue with error chain
+      throw err;
+    } finally {
+      this.inProcessOfSigning.delete(txId);
+    }
+  }
+
+  /**
+   * Convenience method for the ui thats sets the transaction to rejected
+   *
+   * @param {number} txId - the tx's Id
+   * @param {string} actionId - actionId passed from UI
+   * @returns {Promise<void>}
+   */
+  async _cancelTransaction(txId, actionId) {
+    const txMeta = this.txStateManager.getTransaction(txId);
+    this.txStateManager.setTxStatusRejected(txId);
+    this._trackTransactionMetricsEvent(
+      txMeta,
+      TransactionMetaMetricsEvent.rejected,
+      actionId,
+    );
+  }
+
   /** maps methods for convenience*/
   _mapMethods() {
     /** @returns {object} the state in transaction controller */
@@ -1923,7 +2016,7 @@ export default class TransactionController extends EventEmitter {
 
           // Line below will try to publish transaction which is in
           // APPROVED state at the time of controller bootup
-          this.approveTransaction(txMeta.id);
+          this._approveTransaction(txMeta.id);
 
           ///: BEGIN:ONLY_INCLUDE_IN(build-mmi)
         }
@@ -2662,56 +2755,70 @@ export default class TransactionController extends EventEmitter {
     );
   }
 
-  async _requestApproval(
+  // Approvals
+
+  async _requestTransactionApproval(
     txMeta,
+    { shouldShowRequest = true, actionId } = {},
+  ) {
+    let txId, result;
+
+    try {
+      txId = txMeta.id;
+      const { origin } = txMeta;
+
+      const approvalResult = await this._requestApproval(
+        String(txId),
+        origin,
+        { txId },
+        {
+          shouldShowRequest,
+        },
+      );
+
+      result = approvalResult.resultCallbacks;
+
+      const { value } = approvalResult;
+      const { txMeta: updatedTxMeta } = value;
+
+      await this._updateAndApproveTransaction(updatedTxMeta, actionId);
+
+      result?.success();
+    } catch (error) {
+      const transaction = this.txStateManager.getTransaction(txId);
+
+      if (transaction && !this._isTransactionCompleted(transaction)) {
+        if (error.code === errorCodes.provider.userRejectedRequest) {
+          await this._cancelTransaction(txId, actionId);
+        } else {
+          this._failTransaction(txId, error, actionId);
+        }
+      }
+
+      result?.error(error);
+
+      throw error;
+    }
+  }
+
+  async _requestApproval(
+    id,
+    origin,
+    requestData,
     { shouldShowRequest } = { shouldShowRequest: true },
   ) {
-    const id = this._getApprovalId(txMeta);
-    const { origin } = txMeta;
     const type = ApprovalType.Transaction;
-    const requestData = { txId: txMeta.id };
 
-    return this.messagingSystem
+    return this.messagingSystem.call(
-      .call(
+      'ApprovalController:addRequest',
-        'ApprovalController:addRequest',
+      {
-        {
-          id,
-          origin,
-          type,
-          requestData,
-        },
-        shouldShowRequest,
-      )
-      .catch(() => {
-        // Intentionally ignored as promise not currently used
-      });
-  }
-
-  _acceptApproval(txMeta) {
-    const id = this._getApprovalId(txMeta);
-
-    try {
-      this.messagingSystem.call('ApprovalController:acceptRequest', id);
-    } catch (error) {
-      log.error('Failed to accept transaction approval request', error);
-    }
-  }
-
-  _rejectApproval(txMeta) {
-    const id = this._getApprovalId(txMeta);
-
-    try {
-      this.messagingSystem.call(
-        'ApprovalController:rejectRequest',
         id,
-        new Error('Rejected'),
+        origin,
-      );
+        type,
-    } catch (error) {
+        requestData,
-      log.error('Failed to reject transaction approval request', error);
+        expectsResult: true,
-    }
+      },
-  }
+      shouldShowRequest,
-
+    );
-  _getApprovalId(txMeta) {
-    return String(txMeta.id);
   }
 }

app/scripts/metamask-controller.actions.test.js

@@ -5,7 +5,6 @@ import proxyquire from 'proxyquire';
 import { ApprovalRequestNotFoundError } from '@metamask/approval-controller';
 import { PermissionsRequestNotFoundError } from '@metamask/permission-controller';
 import nock from 'nock';
-import { ORIGIN_METAMASK } from '../../shared/constants/app';
 
 const Ganache = require('../../test/e2e/ganache');
 
@@ -237,35 +236,6 @@ describe('MetaMaskController', function () {
     });
   });
 
-  describe('#updateTransactionSendFlowHistory', function () {
-    it('two sequential calls with same history give same result', async function () {
-      const recipientAddress = '0xc42edfcc21ed14dda456aa0756c153f7985d8813';
-
-      await metamaskController.createNewVaultAndKeychain('test@123');
-      const accounts = await metamaskController.keyringController.getAccounts();
-      const txMeta = await metamaskController.getApi().addUnapprovedTransaction(
-        undefined,
-        {
-          from: accounts[0],
-          to: recipientAddress,
-        },
-        ORIGIN_METAMASK,
-      );
-
-      const [transaction1, transaction2] = await Promise.all([
-        metamaskController
-          .getApi()
-          .updateTransactionSendFlowHistory(txMeta.id, 2, ['foo1', 'foo2']),
-        Promise.resolve(1).then(() =>
-          metamaskController
-            .getApi()
-            .updateTransactionSendFlowHistory(txMeta.id, 2, ['foo1', 'foo2']),
-        ),
-      ]);
-      assert.deepEqual(transaction1, transaction2);
-    });
-  });
-
   describe('#removePermissionsFor', function () {
     it('should not propagate PermissionsRequestNotFoundError', function () {
       const error = new PermissionsRequestNotFoundError('123');
@@ -342,7 +312,7 @@ describe('MetaMaskController', function () {
   });
 
   describe('#resolvePendingApproval', function () {
-    it('should not propagate ApprovalRequestNotFoundError', function () {
+    it('should not propagate ApprovalRequestNotFoundError', async function () {
       const error = new ApprovalRequestNotFoundError('123');
       metamaskController.approvalController = {
         accept: () => {
@@ -350,7 +320,10 @@ describe('MetaMaskController', function () {
         },
       };
       // Line below will not throw error, in case it throws this test case will fail.
-      metamaskController.resolvePendingApproval('DUMMY_ID', 'DUMMY_VALUE');
+      await metamaskController.resolvePendingApproval(
+        'DUMMY_ID',
+        'DUMMY_VALUE',
+      );
     });
 
     it('should propagate Error other than ApprovalRequestNotFoundError', function () {
@@ -360,9 +333,11 @@ describe('MetaMaskController', function () {
           throw error;
         },
       };
-      assert.throws(() => {
+      assert.rejects(
-        metamaskController.resolvePendingApproval('DUMMY_ID', 'DUMMY_VALUE');
+        () =>
-      }, error);
+          metamaskController.resolvePendingApproval('DUMMY_ID', 'DUMMY_VALUE'),
+        error,
+      );
     });
   });
 

app/scripts/metamask-controller.js

@@ -1321,6 +1321,14 @@ export default class MetamaskController extends EventEmitter {
       initState.SmartTransactionsController,
     );
 
+    this.txController.on('newSwapApproval', (txMeta) => {
+      this.swapsController.setApproveTxId(txMeta.id);
+    });
+
+    this.txController.on('newSwap', (txMeta) => {
+      this.swapsController.setTradeTxId(txMeta.id);
+    });
+
     // ensure accountTracker updates balances after network change
     networkControllerMessenger.subscribe(
       'NetworkController:networkDidChange',
@@ -2197,10 +2205,7 @@ export default class MetamaskController extends EventEmitter {
       exportAccount: this.exportAccount.bind(this),
 
       // txController
-      cancelTransaction: txController.cancelTransaction.bind(txController),
       updateTransaction: txController.updateTransaction.bind(txController),
-      updateAndApproveTransaction:
-        txController.updateAndApproveTransaction.bind(txController),
       approveTransactionsWithSameNonce:
         txController.approveTransactionsWithSameNonce.bind(txController),
       createCancelTransaction: this.createCancelTransaction.bind(this),
@@ -2220,11 +2225,6 @@ export default class MetamaskController extends EventEmitter {
       updateTransactionSendFlowHistory:
         txController.updateTransactionSendFlowHistory.bind(txController),
 
-      updateSwapApprovalTransaction:
-        txController.updateSwapApprovalTransaction.bind(txController),
-      updateSwapTransaction:
-        txController.updateSwapTransaction.bind(txController),
-
       updatePreviousGasParams:
         txController.updatePreviousGasParams.bind(txController),
 
@@ -4427,9 +4427,9 @@ export default class MetamaskController extends EventEmitter {
     }
   };
 
-  resolvePendingApproval = (id, value) => {
+  resolvePendingApproval = async (id, value, options) => {
     try {
-      this.approvalController.accept(id, value);
+      await this.approvalController.accept(id, value, options);
     } catch (exp) {
       if (!(exp instanceof ApprovalRequestNotFoundError)) {
         throw exp;

lavamoat/build-system/policy.json

@@ -980,6 +980,7 @@
       "packages": {
         "@lavamoat/allow-scripts>@npmcli/run-script>node-gyp>npmlog>are-we-there-yet": true,
         "@lavamoat/allow-scripts>@npmcli/run-script>node-gyp>npmlog>gauge": true,
+        "@storybook/addon-mdx-gfm>@storybook/node-logger>npmlog>console-control-strings": true,
         "@storybook/react>@storybook/node-logger>npmlog>console-control-strings": true,
         "nyc>yargs>set-blocking": true
       }
@@ -1008,6 +1009,9 @@
         "@lavamoat/allow-scripts>@npmcli/run-script>node-gyp>npmlog>gauge>aproba": true,
         "@lavamoat/allow-scripts>@npmcli/run-script>node-gyp>npmlog>gauge>string-width": true,
         "@lavamoat/allow-scripts>@npmcli/run-script>node-gyp>npmlog>gauge>strip-ansi": true,
+        "@storybook/addon-mdx-gfm>@storybook/node-logger>npmlog>console-control-strings": true,
+        "@storybook/addon-mdx-gfm>@storybook/node-logger>npmlog>gauge>has-unicode": true,
+        "@storybook/addon-mdx-gfm>@storybook/node-logger>npmlog>gauge>wide-align": true,
         "@storybook/react>@storybook/node-logger>npmlog>console-control-strings": true,
         "@storybook/react>@storybook/node-logger>npmlog>gauge>has-unicode": true,
         "@storybook/react>@storybook/node-logger>npmlog>gauge>wide-align": true,
@@ -1133,11 +1137,33 @@
         "@metamask/jazzicon>color>color-convert>color-name": true
       }
     },
+    "@sentry/cli>mkdirp": {
+      "builtin": {
+        "fs": true,
+        "path.dirname": true,
+        "path.resolve": true
+      }
+    },
     "@storybook/addon-knobs>qs": {
       "packages": {
         "string.prototype.matchall>side-channel": true
       }
     },
+    "@storybook/addon-mdx-gfm>@storybook/node-logger>npmlog>gauge>has-unicode": {
+      "builtin": {
+        "os.type": true
+      },
+      "globals": {
+        "process.env.LANG": true,
+        "process.env.LC_ALL": true,
+        "process.env.LC_CTYPE": true
+      }
+    },
+    "@storybook/addon-mdx-gfm>@storybook/node-logger>npmlog>gauge>wide-align": {
+      "packages": {
+        "yargs>string-width": true
+      }
+    },
     "@storybook/core>@storybook/core-server>x-default-browser>default-browser-id>untildify>os-homedir": {
       "builtin": {
         "os.homedir": true
@@ -4886,9 +4912,20 @@
       },
       "packages": {
         "@storybook/core>@storybook/core-server>x-default-browser>default-browser-id>untildify>os-homedir": true,
+        "gulp-watch>chokidar>fsevents>node-pre-gyp>nopt>osenv>os-homedir": true,
         "gulp-watch>chokidar>fsevents>node-pre-gyp>nopt>osenv>os-tmpdir": true
       }
     },
+    "gulp-watch>chokidar>fsevents>node-pre-gyp>nopt>osenv>os-homedir": {
+      "builtin": {
+        "os.homedir": true
+      },
+      "globals": {
+        "process.env": true,
+        "process.getuid": true,
+        "process.platform": true
+      }
+    },
     "gulp-watch>chokidar>fsevents>node-pre-gyp>nopt>osenv>os-tmpdir": {
       "globals": {
         "process.env.SystemRoot": true,
@@ -4910,9 +4947,34 @@
         "setTimeout": true
       },
       "packages": {
+        "gulp-watch>chokidar>fsevents>node-pre-gyp>rimraf>glob": true,
         "nyc>glob": true
       }
     },
+    "gulp-watch>chokidar>fsevents>node-pre-gyp>rimraf>glob": {
+      "builtin": {
+        "assert": true,
+        "events.EventEmitter": true,
+        "fs": true,
+        "path.join": true,
+        "path.resolve": true,
+        "util": true
+      },
+      "globals": {
+        "console.error": true,
+        "process.cwd": true,
+        "process.nextTick": true,
+        "process.platform": true
+      },
+      "packages": {
+        "eslint>minimatch": true,
+        "gulp-watch>path-is-absolute": true,
+        "nyc>glob>fs.realpath": true,
+        "nyc>glob>inflight": true,
+        "pump>once": true,
+        "pumpify>inherits": true
+      }
+    },
     "gulp-watch>chokidar>fsevents>node-pre-gyp>semver": {
       "globals": {
         "console": true,
@@ -8246,14 +8308,7 @@
         "path.dirname": true
       },
       "packages": {
-        "stylelint>file-entry-cache>flat-cache>write>mkdirp": true
+        "@sentry/cli>mkdirp": true
-      }
-    },
-    "stylelint>file-entry-cache>flat-cache>write>mkdirp": {
-      "builtin": {
-        "fs": true,
-        "path.dirname": true,
-        "path.resolve": true
       }
     },
     "stylelint>global-modules": {

package.json

@@ -227,7 +227,7 @@
     "@metamask-institutional/transaction-update": "^0.1.21",
     "@metamask/address-book-controller": "^3.0.0",
     "@metamask/announcement-controller": "^4.0.0",
-    "@metamask/approval-controller": "^3.0.0",
+    "@metamask/approval-controller": "^3.1.0",
     "@metamask/assets-controllers": "^9.0.0",
     "@metamask/base-controller": "^3.0.0",
     "@metamask/browser-passworder": "^4.1.0",

ui/ducks/swaps/swaps.js

@@ -14,18 +14,12 @@ import {
   setInitialGasEstimate,
   setSwapsErrorKey,
   setSwapsTxGasPrice,
-  setApproveTxId,
-  setTradeTxId,
   stopPollingForQuotes,
-  updateAndApproveTx,
-  updateSwapApprovalTransaction,
-  updateSwapTransaction,
   resetBackgroundSwapsState,
   setSwapsLiveness,
   setSwapsFeatureFlags,
   setSelectedQuoteAggId,
   setSwapsTxGasLimit,
-  cancelTx,
   fetchSmartTransactionsLiveness,
   signAndSendSmartTransaction,
   updateSmartTransaction,
@@ -1191,20 +1185,23 @@ export const signAndSendTransactions = (
         approveTxParams.maxPriorityFeePerGas = maxPriorityFeePerGas;
         delete approveTxParams.gasPrice;
       }
-      const approveTxMeta = await addUnapprovedTransaction(
+
-        undefined,
-        { ...approveTxParams, amount: '0x0' },
-        TransactionType.swapApproval,
-      );
-      await dispatch(setApproveTxId(approveTxMeta.id));
-      finalApproveTxMeta = await dispatch(
-        updateSwapApprovalTransaction(approveTxMeta.id, {
-          type: TransactionType.swapApproval,
-          sourceTokenSymbol: sourceTokenInfo.symbol,
-        }),
-      );
       try {
-        await dispatch(updateAndApproveTx(finalApproveTxMeta, true));
+        finalApproveTxMeta = await addUnapprovedTransaction(
+          undefined,
+          { ...approveTxParams, amount: '0x0' },
+          TransactionType.swapApproval,
+          {
+            requireApproval: false,
+            swaps: {
+              hasApproveTx: true,
+              meta: {
+                type: TransactionType.swapApproval,
+                sourceTokenSymbol: sourceTokenInfo.symbol,
+              },
+            },
+          },
+        );
       } catch (e) {
         await dispatch(setSwapsErrorKey(SWAP_FAILED_ERROR));
         history.push(SWAPS_ERROR_ROUTE);
@@ -1212,47 +1209,34 @@ export const signAndSendTransactions = (
       }
     }
 
-    const tradeTxMeta = await addUnapprovedTransaction(
-      undefined,
-      usedTradeTxParams,
-      TransactionType.swap,
-    );
-    dispatch(setTradeTxId(tradeTxMeta.id));
-
-    // The simulationFails property is added during the transaction controllers
-    // addUnapprovedTransaction call if the estimateGas call fails. In cases
-    // when no approval is required, this indicates that the swap will likely
-    // fail. There was an earlier estimateGas call made by the swaps controller,
-    // but it is possible that external conditions have change since then, and
-    // a previously succeeding estimate gas call could now fail. By checking for
-    // the `simulationFails` property here, we can reduce the number of swap
-    // transactions that get published to the blockchain only to fail and thereby
-    // waste the user's funds on gas.
-    if (!approveTxParams && tradeTxMeta.simulationFails) {
-      await dispatch(cancelTx(tradeTxMeta, false));
-      await dispatch(setSwapsErrorKey(SWAP_FAILED_ERROR));
-      history.push(SWAPS_ERROR_ROUTE);
-      return;
-    }
-    const finalTradeTxMeta = await dispatch(
-      updateSwapTransaction(tradeTxMeta.id, {
-        estimatedBaseFee: decEstimatedBaseFee,
-        sourceTokenSymbol: sourceTokenInfo.symbol,
-        destinationTokenSymbol: destinationTokenInfo.symbol,
-        type: TransactionType.swap,
-        destinationTokenDecimals: destinationTokenInfo.decimals,
-        destinationTokenAddress: destinationTokenInfo.address,
-        swapMetaData,
-        swapTokenValue,
-        approvalTxId: finalApproveTxMeta?.id,
-      }),
-    );
     try {
-      await dispatch(updateAndApproveTx(finalTradeTxMeta, true));
+      await addUnapprovedTransaction(
+        undefined,
+        usedTradeTxParams,
+        TransactionType.swap,
+        {
+          requireApproval: false,
+          swaps: {
+            hasApproveTx: Boolean(approveTxParams),
+            meta: {
+              estimatedBaseFee: decEstimatedBaseFee,
+              sourceTokenSymbol: sourceTokenInfo.symbol,
+              destinationTokenSymbol: destinationTokenInfo.symbol,
+              type: TransactionType.swap,
+              destinationTokenDecimals: destinationTokenInfo.decimals,
+              destinationTokenAddress: destinationTokenInfo.address,
+              swapMetaData,
+              swapTokenValue,
+              approvalTxId: finalApproveTxMeta?.id,
+            },
+          },
+        },
+      );
     } catch (e) {
       const errorKey = e.message.includes('EthAppPleaseEnableContractData')
         ? CONTRACT_DATA_DISABLED_ERROR
         : SWAP_FAILED_ERROR;
+      console.error(e);
       await dispatch(setSwapsErrorKey(errorKey));
       history.push(SWAPS_ERROR_ROUTE);
       return;

ui/selectors/transactions.js

@@ -547,7 +547,9 @@ const hasUnapprovedTransactionsInCurrentNetwork = (state) => {
   const chainId = getCurrentChainId(state);
 
   const filteredUnapprovedTxInCurrentNetwork = unapprovedTxRequests.filter(
-    ({ id }) => transactionMatchesNetwork(unapprovedTxs[id], chainId),
+    ({ id }) =>
+      unapprovedTxs[id] &&
+      transactionMatchesNetwork(unapprovedTxs[id], chainId),
   );
 
   return filteredUnapprovedTxInCurrentNetwork.length > 0;

ui/store/actions.test.js

@@ -2031,7 +2031,7 @@ describe('Actions', () => {
       const store = mockStore();
 
       background.getApi.returns({
-        cancelTransaction: sinon.stub().callsFake((_1, _2, cb) => {
+        rejectPendingApproval: sinon.stub().callsFake((_1, _2, cb) => {
           cb();
         }),
         getState: sinon.stub().callsFake((cb) =>

ui/store/actions.ts

@@ -15,6 +15,7 @@ import { PayloadAction } from '@reduxjs/toolkit';
 import { GasFeeController } from '@metamask/gas-fee-controller';
 import { PermissionsRequest } from '@metamask/permission-controller';
 import { NonEmptyArray } from '@metamask/controller-utils';
+import { ethErrors } from 'eth-rpc-errors';
 import { getMethodDataAsync } from '../helpers/utils/transactions.util';
 import switchDirection from '../../shared/lib/switch-direction';
 import {
@@ -902,32 +903,6 @@ export function updatePreviousGasParams(
   };
 }
 
-// TODO: codeword: NOT_A_THUNK @brad-decker
-export function updateSwapApprovalTransaction(
-  txId: number,
-  txSwapApproval: TransactionMeta,
-): ThunkAction<
-  Promise<TransactionMeta>,
-  MetaMaskReduxState,
-  unknown,
-  AnyAction
-> {
-  return async () => {
-    let updatedTransaction: TransactionMeta;
-    try {
-      updatedTransaction = await submitRequestToBackground(
-        'updateSwapApprovalTransaction',
-        [txId, txSwapApproval],
-      );
-    } catch (error) {
-      logErrorWithMessage(error);
-      throw error;
-    }
-
-    return updatedTransaction;
-  };
-}
-
 export function updateEditableParams(
   txId: number,
   editableParams: Partial<TxParams>,
@@ -1044,32 +1019,6 @@ export function updateTransactionGasFees(
   };
 }
 
-// TODO: codeword: NOT_A_THUNK @brad-decker
-export function updateSwapTransaction(
-  txId: number,
-  txSwap: TransactionMeta,
-): ThunkAction<
-  Promise<TransactionMeta>,
-  MetaMaskReduxState,
-  unknown,
-  AnyAction
-> {
-  return async () => {
-    let updatedTransaction: TransactionMeta;
-    try {
-      updatedTransaction = await submitRequestToBackground(
-        'updateSwapTransaction',
-        [txId, txSwap],
-      );
-    } catch (error) {
-      logErrorWithMessage(error);
-      throw error;
-    }
-
-    return updatedTransaction;
-  };
-}
-
 export function updateTransaction(
   txMeta: TransactionMeta,
   dontShowLoadingIndicator: boolean,
@@ -1154,18 +1103,27 @@ export function addUnapprovedTransactionAndRouteToConfirmationPage(
  * @param method
  * @param txParams - the transaction parameters
  * @param type - The type of the transaction being added.
+ * @param options - Additional options for the transaction.
+ * @param options.requireApproval - Whether the transaction requires approval.
+ * @param options.swaps - Options specific to swaps transactions.
+ * @param options.swaps.hasApproveTx - Whether the swap required an approval transaction.
+ * @param options.swaps.meta - Additional transaction metadata required by swaps.
  * @returns
  */
 export async function addUnapprovedTransaction(
   method: string,
   txParams: TxParams,
   type: TransactionType,
+  options?: {
+    requireApproval?: boolean;
+    swaps?: { hasApproveTx?: boolean; meta?: Record<string, unknown> };
+  },
 ): Promise<TransactionMeta> {
   log.debug('background.addUnapprovedTransaction');
   const actionId = generateActionId();
   const txMeta = await submitRequestToBackground<TransactionMeta>(
     'addUnapprovedTransaction',
-    [method, txParams, ORIGIN_METAMASK, type, undefined, actionId],
+    [method, txParams, ORIGIN_METAMASK, type, undefined, actionId, options],
     actionId,
   );
   return txMeta;
@@ -1185,8 +1143,8 @@ export function updateAndApproveTx(
     return new Promise((resolve, reject) => {
       const actionId = generateActionId();
       callBackgroundMethod(
-        'updateAndApproveTransaction',
+        'resolvePendingApproval',
-        [txMeta, actionId],
+        [String(txMeta.id), { txMeta, actionId }, { waitForResult: true }],
         (err) => {
           dispatch(updateTransactionParams(txMeta.id, txMeta.txParams));
           dispatch(resetSendState());
@@ -1615,10 +1573,12 @@ export function cancelTx(
   return (dispatch: MetaMaskReduxDispatch) => {
     _showLoadingIndication && dispatch(showLoadingIndication());
     return new Promise<void>((resolve, reject) => {
-      const actionId = generateActionId();
       callBackgroundMethod(
-        'cancelTransaction',
+        'rejectPendingApproval',
-        [txMeta.id, actionId],
+        [
+          String(txMeta.id),
+          ethErrors.provider.userRejectedRequest().serialize(),
+        ],
         (error) => {
           if (error) {
             reject(error);
@@ -1663,15 +1623,21 @@ export function cancelTxs(
       const cancellations = txIds.map(
         (id) =>
           new Promise<void>((resolve, reject) => {
-            const actionId = generateActionId();
+            callBackgroundMethod(
-            callBackgroundMethod('cancelTransaction', [id, actionId], (err) => {
+              'rejectPendingApproval',
-              if (err) {
+              [
-                reject(err);
+                String(id),
-                return;
+                ethErrors.provider.userRejectedRequest().serialize(),
-              }
+              ],
+              (err) => {
+                if (err) {
+                  reject(err);
+                  return;
+                }
 
-              resolve();
+                resolve();
-            });
+              },
+            );
           }),
       );
 
@@ -3547,24 +3513,6 @@ export function setSwapsQuotesPollingLimitEnabled(
   };
 }
 
-export function setTradeTxId(
-  tradeTxId: string,
-): ThunkAction<void, MetaMaskReduxState, unknown, AnyAction> {
-  return async (dispatch: MetaMaskReduxDispatch) => {
-    await submitRequestToBackground('setTradeTxId', [tradeTxId]);
-    await forceUpdateMetamaskState(dispatch);
-  };
-}
-
-export function setApproveTxId(
-  approveTxId: string,
-): ThunkAction<void, MetaMaskReduxState, unknown, AnyAction> {
-  return async (dispatch: MetaMaskReduxDispatch) => {
-    await submitRequestToBackground('setApproveTxId', [approveTxId]);
-    await forceUpdateMetamaskState(dispatch);
-  };
-}
-
 export function safeRefetchQuotes(): ThunkAction<
   void,
   MetaMaskReduxState,

yarn.lock

@@ -24030,7 +24030,7 @@ __metadata:
     "@metamask-institutional/transaction-update": ^0.1.21
     "@metamask/address-book-controller": ^3.0.0
     "@metamask/announcement-controller": ^4.0.0
-    "@metamask/approval-controller": ^3.0.0
+    "@metamask/approval-controller": ^3.1.0
     "@metamask/assets-controllers": ^9.0.0
     "@metamask/auto-changelog": ^2.1.0
     "@metamask/base-controller": ^3.0.0