From 7add8acc8244f185a2462f14f368d931d16783bc Mon Sep 17 00:00:00 2001
From: Bilal <44588480+BZahory@users.noreply.github.com>
Date: Tue, 25 Jun 2024 12:51:40 -0400
Subject: [PATCH] fix: do not use STX for swap+send approval (#25510)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

Paired w/ @dan437 and @forest-diggs-consensys

We removed STX functionality for Swap+Send in #25422. However, we still
get the STX flow because the `swapApproval` type we use is still routed
as an STX.

Since only Swap+Send uses the `swapApproval` type for STXs routed
through `submitSmartTransactionHook`, we can exclude it in the same way
we excluded the `swapAndSend` type in #25422.

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/25510?quickstart=1)

## **Related issues**

Fixes: #25356

## **Manual testing steps**

1. Enable Smart Transactions in settings
2. Switch to Ethereum Mainnet
3. Submit a swap+send transaction that requires an approval (i.e. from
an ERC-20 w/o an existing approval)
4. Ensure that the STX screen does not show after submitting
5. Ensure that the contract interaction isn't marked as "unapproved"

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**


https://github.com/MetaMask/metamask-extension/assets/44588480/d3fb13ca-4aa0-43a8-8689-37f7e49af10a

<!-- [screenshots/recordings] -->

### **After**


https://github.com/MetaMask/metamask-extension/assets/44588480/d34ab118-ed69-4418-8c35-06774099e092

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [x] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.
---
 app/scripts/lib/transaction/smart-transactions.test.ts | 7 +++++++
 app/scripts/lib/transaction/smart-transactions.ts      | 8 ++++++--
 ui/ducks/send/send.js                                  | 1 +
 3 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/app/scripts/lib/transaction/smart-transactions.test.ts b/app/scripts/lib/transaction/smart-transactions.test.ts
index 04c029c3ef8b..eed52a909f67 100644
--- a/app/scripts/lib/transaction/smart-transactions.test.ts
+++ b/app/scripts/lib/transaction/smart-transactions.test.ts
@@ -141,6 +141,13 @@ describe('submitSmartTransactionHook', () => {
     expect(result).toEqual({ transactionHash: undefined });
   });
 
+  it('falls back to regular transaction submit if the transaction type is "swapApproval"', async () => {
+    const request: SubmitSmartTransactionRequestMocked = createRequest();
+    request.transactionMeta.type = TransactionType.swapApproval;
+    const result = await submitSmartTransactionHook(request);
+    expect(result).toEqual({ transactionHash: undefined });
+  });
+
   it('falls back to regular transaction submit if /getFees throws an error', async () => {
     const request: SubmitSmartTransactionRequestMocked = createRequest();
     jest
diff --git a/app/scripts/lib/transaction/smart-transactions.ts b/app/scripts/lib/transaction/smart-transactions.ts
index 97e37965d6ea..21a95e94494d 100644
--- a/app/scripts/lib/transaction/smart-transactions.ts
+++ b/app/scripts/lib/transaction/smart-transactions.ts
@@ -121,8 +121,12 @@ class SmartTransactionHook {
   }
 
   async submit() {
-    const isUnsupportedTransactionTypeForSmartTransaction =
-      this.#transactionMeta?.type === TransactionType.swapAndSend;
+    const isUnsupportedTransactionTypeForSmartTransaction = this
+      .#transactionMeta?.type
+      ? [TransactionType.swapAndSend, TransactionType.swapApproval].includes(
+          this.#transactionMeta.type,
+        )
+      : false;
 
     // Will cause TransactionController to publish to the RPC provider as normal.
     const useRegularTransactionSubmit = { transactionHash: undefined };
diff --git a/ui/ducks/send/send.js b/ui/ducks/send/send.js
index ce7ddc4e9bfe..50039e02f650 100644
--- a/ui/ducks/send/send.js
+++ b/ui/ducks/send/send.js
@@ -3017,6 +3017,7 @@ export function signTransaction(history) {
             { ...bestQuote.approvalNeeded, amount: '0x0' },
             {
               requireApproval: false,
+              // TODO: create new type for swap+send approvals; works as stopgap bc swaps doesn't use this type for STXs in `submitSmartTransactionHook` (via `TransactionController`)
               type: TransactionType.swapApproval,
               swaps: {
                 hasApproveTx: true,