fix: cp-12.16.0 Follow up on the publishBatch hook (#31401)

## **Description**
- This PR is mainly refactoring based on the feedback in the [main
PR](#31267) that was
merged + it updates the STX controller to the latest version.

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

## **Related issues**

Fixes:

## **Manual testing steps**

1. Have STX enabled
2. Be on Ethereum mainnet
3. Go to the Send feature
4. You will be able to choose your gas token if you have enough balance
for it and its a supported token

## **Screenshots/Recordings**

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

### **Before**

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

### **After**

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

## **Pre-merge author checklist**

- [ ] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.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.

---------

Signed-off-by: dan437 <[email protected]>

Author: dan437

app/scripts/controller-init/confirmations/transaction-controller-init.ts

@@ -224,6 +224,26 @@ function getControllers(
   };
 }
 
+function getSmartTransactionCommonParams(flatState: ControllerFlatState) {
+  // UI state is required to support shared selectors to avoid duplicate logic in frontend and backend.
+  // Ideally all backend logic would instead rely on messenger event / state subscriptions.
+  const uiState = getUIState(flatState);
+
+  // @ts-expect-error Smart transaction selector types does not match controller state
+  const isSmartTransaction = getIsSmartTransaction(uiState);
+
+  // @ts-expect-error Smart transaction selector types does not match controller state
+  const featureFlags = getFeatureFlagsByChainId(uiState);
+
+  const isHardwareWalletAccount = isHardwareWallet(uiState);
+
+  return {
+    isSmartTransaction,
+    featureFlags,
+    isHardwareWalletAccount,
+  };
+}
+
 function publishSmartTransactionHook(
   transactionController: TransactionController,
   smartTransactionsController: SmartTransactionsController,
@@ -232,29 +252,22 @@ function publishSmartTransactionHook(
   transactionMeta: TransactionMeta,
   signedTransactionInHex: Hex,
 ) {
-  // UI state is required to support shared selectors to avoid duplicate logic in frontend and backend.
-  // Ideally all backend logic would instead rely on messenger event / state subscriptions.
-  const uiState = getUIState(flatState);
-
-  // @ts-expect-error Smart transaction selector types does not match controller state
-  const isSmartTransaction = getIsSmartTransaction(uiState);
+  const { isSmartTransaction, featureFlags, isHardwareWalletAccount } =
+    getSmartTransactionCommonParams(flatState);
 
   if (!isSmartTransaction) {
     // Will cause TransactionController to publish to the RPC provider as normal.
     return { transactionHash: undefined };
   }
 
-  // @ts-expect-error Smart transaction selector types does not match controller state
-  const featureFlags = getFeatureFlagsByChainId(uiState);
-
   return submitSmartTransactionHook({
     transactionMeta,
     signedTransactionInHex,
     transactionController,
     smartTransactionsController,
     controllerMessenger: hookControllerMessenger,
     isSmartTransaction,
-    isHardwareWallet: isHardwareWallet(uiState),
+    isHardwareWallet: isHardwareWalletAccount,
     // @ts-expect-error Smart transaction selector return type does not match FeatureFlags type from hook
     featureFlags,
   });
@@ -273,21 +286,16 @@ function publishBatchSmartTransactionHook({
   flatState: ControllerFlatState;
   transactions: PublishBatchHookTransaction[];
 }) {
-  // UI state is required to support shared selectors to avoid duplicate logic in frontend and backend.
-  // Ideally all backend logic would instead rely on messenger event / state subscriptions.
-  const uiState = getUIState(flatState);
-
-  // @ts-expect-error Smart transaction selector types does not match controller state
-  const isSmartTransaction = getIsSmartTransaction(uiState);
+  const { isSmartTransaction, featureFlags, isHardwareWalletAccount } =
+    getSmartTransactionCommonParams(flatState);
 
   if (!isSmartTransaction) {
     // Will cause TransactionController to publish to the RPC provider as normal.
-    return undefined;
+    throw new Error(
+      'publishBatchSmartTransactionHook: Smart Transaction is required for batch submissions',
+    );
   }
 
-  // @ts-expect-error Smart transaction selector types does not match controller state
-  const featureFlags = getFeatureFlagsByChainId(uiState);
-
   // Get transactionMeta based on the last transaction ID
   const lastTransaction = transactions[transactions.length - 1];
   const transactionMeta = getTransactionById(
@@ -297,10 +305,9 @@ function publishBatchSmartTransactionHook({
 
   // If we couldn't find the transaction, we should handle that gracefully
   if (!transactionMeta) {
-    console.warn(
-      `Publish batch hook: could not find transaction with id ${lastTransaction.id}`,
+    throw new Error(
+      `publishBatchSmartTransactionHook: Could not find transaction with id ${lastTransaction.id}`,
     );
-    return undefined;
   }
 
   return submitBatchSmartTransactionHook({
@@ -309,7 +316,7 @@ function publishBatchSmartTransactionHook({
     smartTransactionsController,
     controllerMessenger: hookControllerMessenger,
     isSmartTransaction,
-    isHardwareWallet: isHardwareWallet(uiState),
+    isHardwareWallet: isHardwareWalletAccount,
     // @ts-expect-error Smart transaction selector return type does not match FeatureFlags type from hook
     featureFlags,
     transactionMeta,

app/scripts/controller-init/messengers/transaction-controller-messenger.ts

@@ -98,11 +98,11 @@ export function getTransactionControllerInitMessenger(
       'SmartTransactionsController:smartTransaction',
     ],
     allowedActions: [
+      'ApprovalController:acceptRequest',
       'ApprovalController:addRequest',
       'ApprovalController:endFlow',
       'ApprovalController:startFlow',
       'ApprovalController:updateRequestState',
-      'ApprovalController:acceptRequest',
       'NetworkController:getEIP1559Compatibility',
       'RemoteFeatureFlagController:getState',
       'SwapsController:setApproveTxId',

app/scripts/lib/transaction/smart-transactions.test.ts

@@ -79,11 +79,13 @@ function withRequest<ReturnValue>(
   const startFlowSpy = jest.fn().mockResolvedValue({ id: 'approvalId' });
   messenger.registerActionHandler('ApprovalController:startFlow', startFlowSpy);
 
-  const addRequestSpy = jest.fn().mockImplementation(() => ({
-    then: (callback: () => void) => {
-      addRequestCallback = callback;
-    },
-  }));
+  const addRequestSpy = jest.fn().mockImplementation(() => {
+    return Promise.resolve().then(() => {
+      if (typeof addRequestCallback === 'function') {
+        addRequestCallback();
+      }
+    });
+  });
   messenger.registerActionHandler(
     'ApprovalController:addRequest',
     addRequestSpy,
@@ -142,7 +144,8 @@ function withRequest<ReturnValue>(
       txParams: {
         from: addressFrom,
         to: '0x1678a085c290ebd122dc42cba69373b5953b831d',
-        gasPrice: '0x77359400',
+        maxFeePerGas: '0x2fd8a58d7',
+        maxPriorityFeePerGas: '0xaa0f8a94',
         gas: '0x7b0d',
         nonce: '0x4b',
       },
@@ -551,6 +554,7 @@ describe('submitSmartTransactionHook', () => {
 
     const endFlowSpy = jest.fn();
     const acceptRequestSpy = jest.fn();
+    const addRequestSpy = jest.fn(() => Promise.resolve());
 
     // Create a mock messenger
     const mockMessenger = {
@@ -564,6 +568,9 @@ describe('submitSmartTransactionHook', () => {
         if (method === 'ApprovalController:acceptRequest') {
           return acceptRequestSpy(...args);
         }
+        if (method === 'ApprovalController:addRequest') {
+          return addRequestSpy();
+        }
         return undefined;
       }),
       subscribe: jest.fn(),
@@ -631,8 +638,9 @@ describe('submitBatchSmartTransactionHook', () => {
         },
       },
       async ({ request }) => {
-        const result = await submitBatchSmartTransactionHook(request);
-        expect(result).toEqual({ results: [] });
+        await expect(submitBatchSmartTransactionHook(request)).rejects.toThrow(
+          'submitBatch: Smart Transaction is required for batch submissions',
+        );
       },
     );
   });

app/scripts/lib/transaction/smart-transactions.ts

@@ -217,11 +217,11 @@ class SmartTransactionHook {
   }
 
   async submitBatch() {
-    // Will cause TransactionController to publish to the RPC provider as normal.
-    const useRegularTransactionSubmit = undefined;
-
+    // No fallback to regular transaction submission
     if (!this.#isSmartTransaction) {
-      return useRegularTransactionSubmit;
+      throw new Error(
+        'submitBatch: Smart Transaction is required for batch submissions',
+      );
     }
 
     if (this.#shouldShowStatusPage) {
@@ -233,7 +233,7 @@ class SmartTransactionHook {
       const uuid = submitTransactionResponse?.uuid;
 
       if (!uuid) {
-        throw new Error('No smart transaction UUID');
+        throw new Error('submitBatch: No smart transaction UUID');
       }
 
       await this.#processApprovalIfNeeded(uuid);
@@ -244,7 +244,7 @@ class SmartTransactionHook {
 
       if (transactionHash === null) {
         throw new Error(
-          'Transaction does not have a transaction hash, there was a problem',
+          'submitBatch: Transaction does not have a transaction hash, there was a problem',
         );
       }
 
@@ -263,24 +263,29 @@ class SmartTransactionHook {
 
       return submitBatchResponse;
     } catch (error) {
-      log.error('Error in smart transaction publish batch hook', error);
+      log.error(
+        'submitBatch: Error in smart transaction publish batch hook',
+        error,
+      );
       this.#onApproveOrReject();
       throw error;
     }
   }
 
-  /**
-   * Ends an existing approval flow and clears the shared approval flow ID
-   *
-   * @param approvalFlowId - The ID of the approval flow to end
-   * @returns Promise that resolves when the flow is successfully ended or errors are handled
-   */
-  async #endExistingApprovalFlow(approvalFlowId: string): Promise<void> {
+  async #endApprovalFlow(flowId: string): Promise<void> {
     try {
-      // End the existing flow
       await this.#controllerMessenger.call('ApprovalController:endFlow', {
-        id: approvalFlowId,
+        id: flowId,
       });
+    } catch (error) {
+      // If the flow is already ended, we can ignore the error.
+    }
+  }
+
+  async #endExistingApprovalFlow(approvalFlowId: string): Promise<void> {
+    try {
+      // End the existing flow
+      await this.#endApprovalFlow(approvalFlowId);
 
       // Accept the request to close the UI
       await this.#controllerMessenger.call(
@@ -327,13 +332,7 @@ class SmartTransactionHook {
       return;
     }
     this.#approvalFlowEnded = true;
-    try {
-      this.#controllerMessenger.call('ApprovalController:endFlow', {
-        id: this.#approvalFlowId,
-      });
-    } catch (error) {
-      // If the flow is already ended, we can ignore the error.
-    }
+    this.#endApprovalFlow(this.#approvalFlowId);
 
     // Clear the shared approval flow ID when we end the flow
     if (SmartTransactionHook.#sharedApprovalFlowId === this.#approvalFlowId) {

package.json

@@ -315,7 +315,7 @@
     "@metamask/scure-bip39": "^2.0.3",
     "@metamask/selected-network-controller": "^19.0.0",
     "@metamask/signature-controller": "^26.0.0",
-    "@metamask/smart-transactions-controller": "^16.2.0",
+    "@metamask/smart-transactions-controller": "^16.3.0",
     "@metamask/snaps-controllers": "^11.1.0",
     "@metamask/snaps-execution-environments": "^7.1.0",
     "@metamask/snaps-rpc-methods": "^12.0.0",

yarn.lock

@@ -6344,9 +6344,9 @@ __metadata:
   languageName: node
   linkType: hard
 
-"@metamask/smart-transactions-controller@npm:^16.2.0":
-  version: 16.2.0
-  resolution: "@metamask/smart-transactions-controller@npm:16.2.0"
+"@metamask/smart-transactions-controller@npm:^16.3.0":
+  version: 16.3.0
+  resolution: "@metamask/smart-transactions-controller@npm:16.3.0"
   dependencies:
     "@babel/runtime": "npm:^7.24.1"
     "@ethereumjs/tx": "npm:^5.2.1"
@@ -6368,7 +6368,7 @@ __metadata:
       optional: true
     "@metamask/approval-controller":
       optional: true
-  checksum: 10/3cf74a5ab01ce62e32d0c006d1519bdaccf3252636d83be9537c915caba049e151d981b74a58e4a1122defcf6a1d13d689d53b8f3cbbc31870d2896fa43a5b31
+  checksum: 10/0dc4fc7ce58c45af6598b8209646e8ce1eff7f31fc3fd7abe6ceae5d3ead9e4eafa7445eff6ecdc2b606fea9af30658adf5e2e15fce3bdbfdcee41bceb31795e
   languageName: node
   linkType: hard
 
@@ -27376,7 +27376,7 @@ __metadata:
     "@metamask/scure-bip39": "npm:^2.0.3"
     "@metamask/selected-network-controller": "npm:^19.0.0"
     "@metamask/signature-controller": "npm:^26.0.0"
-    "@metamask/smart-transactions-controller": "npm:^16.2.0"
+    "@metamask/smart-transactions-controller": "npm:^16.3.0"
     "@metamask/snaps-controllers": "npm:^11.1.0"
     "@metamask/snaps-execution-environments": "npm:^7.1.0"
     "@metamask/snaps-rpc-methods": "npm:^12.0.0"