Merge branch 'develop' into feat-migration-122-redesignedConfirmationsEnabled

Author: digiwand

.github/workflows/run-unit-tests.yml

@@ -1,32 +1,52 @@
-# WARNING! It is currently being investigated how to make this faster
-# DO NOT blindly copy this workflow, not noticing the slow down,
-# because suddenly our tests will take hours to pass CI.
-# Hopefully this comment here will help prevent that.
-# https://github.com/MetaMask/metamask-extension/issues/25680
-
 name: Run unit tests
 
 on:
+  push:
+    branches: [develop, master]
   pull_request:
     types: [opened,reopened,synchronize]
 
 jobs:
-  test-unit-jest:
+  test-unit:
     runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        shard: [1, 2, 3, 4, 5, 6]
     steps:
       - name: Checkout repository
         uses: actions/checkout@v4
 
       - name: Setup environment
         uses: ./.github/actions/setup-environment
 
-      - name: test:coverage:jest:dev
-        run: yarn test:coverage:jest:dev
+      - name: test:unit:coverage
+        run: yarn test:unit:coverage --shard=${{ matrix.shard }}/${{ strategy.job-total }}
+
+      - name: Rename coverage to shard coverage
+        run: mv coverage/coverage-final.json coverage/coverage-${{matrix.shard}}.json
+
+      - uses: actions/upload-artifact@v4
+        with:
+          name: coverage-${{matrix.shard}}
+          path: coverage/coverage-${{matrix.shard}}.json
+
+  report-coverage:
+    runs-on: ubuntu-latest
+    needs:
+      - test-unit
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v4
 
-      - name: test:coverage:jest
-        run: yarn test:coverage:jest
+      - name: Download coverage from shards
+        uses: actions/download-artifact@v4
+        with:
+          path: coverage
+          pattern: coverage-*
+          merge-multiple: true
 
-      - uses: codecov/codecov-action@e28ff129e5465c2c0dcc6f003fc735cb6ae0c673
+      - name: Upload coverage to Codecov
+        uses: codecov/codecov-action@e28ff129e5465c2c0dcc6f003fc735cb6ae0c673
         with:
           token: ${{ secrets.CODECOV_TOKEN }}
           fail_ci_if_error: true

.yarn/patches/@metamask-snaps-utils-npm-7.7.0-2cc1f044af.patch

@@ -0,0 +1,30 @@
+diff --git a/dist/chunk-37VHIRUJ.js b/dist/chunk-37VHIRUJ.js
+index a909a4ef20305665a07db5c25b4a9ff7eb0a447e..98dd75bf33a9716dc6cca96a38d184645f6ec033 100644
+--- a/dist/chunk-37VHIRUJ.js
++++ b/dist/chunk-37VHIRUJ.js
+@@ -53,8 +53,8 @@ function assertIsKeyringOrigins(value, ErrorWrapper) {
+ }
+ function createOriginRegExp(matcher) {
+   const escaped = matcher.replace(/[.*+?^${}()|[\]\\]/gu, "\\$&");
+-  const regex = escaped.replace(/\*/gu, ".*");
+-  return RegExp(regex, "u");
++  const regex = escaped.replace(/\\\*/gu, '.*');
++  return RegExp(`${regex}$`, 'u');
+ }
+ function checkAllowedOrigin(matcher, origin) {
+   if (matcher === "*" || matcher === origin) {
+diff --git a/dist/chunk-K2OTEZZZ.mjs b/dist/chunk-K2OTEZZZ.mjs
+index 15be5da7563a5bdf464d7e9c28ed6f04863e378a..7f38bf328e71c1feb2b8850ba050ce9e55801668 100644
+--- a/dist/chunk-K2OTEZZZ.mjs
++++ b/dist/chunk-K2OTEZZZ.mjs
+@@ -53,8 +53,8 @@ function assertIsKeyringOrigins(value, ErrorWrapper) {
+ }
+ function createOriginRegExp(matcher) {
+   const escaped = matcher.replace(/[.*+?^${}()|[\]\\]/gu, "\\$&");
+-  const regex = escaped.replace(/\*/gu, ".*");
+-  return RegExp(regex, "u");
++  const regex = escaped.replace(/\\\*/gu, '.*');
++  return RegExp(`${regex}$`, 'u');
+ }
+ function checkAllowedOrigin(matcher, origin) {
+   if (matcher === "*" || matcher === origin) {

app/scripts/controllers/mmi-controller.test.ts

@@ -456,26 +456,6 @@ describe('MMIController', function () {
     });
   });
 
-  describe('getCustodianAccountsByAddress', () => {
-    it('should return custodian accounts by address', async () => {
-      CUSTODIAN_TYPES['MOCK-CUSTODIAN-TYPE'] = {
-        keyringClass: { type: 'mock-keyring-class' },
-      };
-      mmiController.addKeyringIfNotExists = jest.fn().mockResolvedValue({
-        getCustodianAccounts: jest.fn().mockResolvedValue(['account1']),
-      });
-
-      const result = await mmiController.getCustodianAccountsByAddress(
-        'token',
-        'envName',
-        'address',
-        'mock-custodian-type',
-      );
-
-      expect(result).toEqual(['account1']);
-    });
-  });
-
   describe('getCustodianTransactionDeepLink', () => {
     it('should return a transaction deep link', async () => {
       mmiController.custodyController.getCustodyTypeByAddress = jest

app/scripts/controllers/mmi-controller.ts

@@ -570,33 +570,6 @@ export default class MMIController extends EventEmitter {
     return accounts;
   }
 
-  async getCustodianAccountsByAddress(
-    token: string,
-    envName: string,
-    address: string,
-    custodianType: string,
-  ) {
-    let keyring;
-
-    if (custodianType) {
-      const custodian = CUSTODIAN_TYPES[custodianType.toUpperCase()];
-      if (!custodian) {
-        throw new Error('No such custodian');
-      }
-
-      keyring = await this.addKeyringIfNotExists(custodian.keyringClass.type);
-    } else {
-      throw new Error('No custodian specified');
-    }
-
-    const accounts = await keyring.getCustodianAccounts(
-      token,
-      envName,
-      address,
-    );
-    return accounts;
-  }
-
   async getCustodianTransactionDeepLink(address: string, txId: string) {
     const custodyType = this.custodyController.getCustodyTypeByAddress(
       toChecksumHexAddress(address),

app/scripts/lib/ppom/ppom-middleware.test.ts

@@ -23,6 +23,7 @@ import { SecurityAlertResponse } from './types';
 jest.mock('./ppom-util');
 
 const SECURITY_ALERT_ID_MOCK = '123';
+const INTERNAL_ACCOUNT_ADDRESS = '0xec1adf982415d2ef5ec55899b9bfb8bc0f29251b';
 
 const SECURITY_ALERT_RESPONSE_MOCK: SecurityAlertResponse = {
   securityAlertId: SECURITY_ALERT_ID_MOCK,
@@ -69,6 +70,10 @@ const createMiddleware = (
     addSignatureSecurityAlertResponse: () => undefined,
   };
 
+  const accountsController = {
+    listAccounts: () => [{ address: INTERNAL_ACCOUNT_ADDRESS }],
+  };
+
   return createPPOMMiddleware(
     // TODO: Replace `any` with type
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
@@ -82,6 +87,8 @@ const createMiddleware = (
     // TODO: Replace `any` with type
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
     appStateController as any,
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    accountsController as any,
     updateSecurityAlertResponse,
   );
 };
@@ -206,6 +213,26 @@ describe('PPOMMiddleware', () => {
     expect(validateRequestWithPPOM).not.toHaveBeenCalled();
   });
 
+  it('does not do validation when request is send to users own account', async () => {
+    const middlewareFunction = createMiddleware();
+
+    const req = {
+      ...JsonRpcRequestStruct,
+      params: [{ to: INTERNAL_ACCOUNT_ADDRESS }],
+      method: 'eth_sendTransaction',
+      securityAlertResponse: undefined,
+    };
+
+    await middlewareFunction(
+      req,
+      { ...JsonRpcResponseStruct },
+      () => undefined,
+    );
+
+    expect(req.securityAlertResponse).toBeUndefined();
+    expect(validateRequestWithPPOM).not.toHaveBeenCalled();
+  });
+
   it('does not do validation for SIWE signature', async () => {
     const middlewareFunction = createMiddleware({
       securityAlertsEnabled: true,

app/scripts/lib/ppom/ppom-middleware.ts

@@ -1,3 +1,4 @@
+import { AccountsController } from '@metamask/accounts-controller';
 import { PPOMController } from '@metamask/ppom-validator';
 import { NetworkController } from '@metamask/network-controller';
 import {
@@ -8,6 +9,7 @@ import {
 } from '@metamask/utils';
 import { detectSIWE } from '@metamask/controller-utils';
 
+import { MESSAGE_TYPE } from '../../../../shared/constants/app';
 import { SIGNING_METHODS } from '../../../../shared/constants/transaction';
 import { PreferencesController } from '../../controllers/preferences';
 import { AppStateController } from '../../controllers/app-state';
@@ -41,6 +43,7 @@ const CONFIRMATION_METHODS = Object.freeze([
  * @param preferencesController - Instance of PreferenceController.
  * @param networkController - Instance of NetworkController.
  * @param appStateController
+ * @param accountsController - Instance of AccountsController.
  * @param updateSecurityAlertResponse
  * @returns PPOMMiddleware function.
  */
@@ -52,6 +55,7 @@ export function createPPOMMiddleware<
   preferencesController: PreferencesController,
   networkController: NetworkController,
   appStateController: AppStateController,
+  accountsController: AccountsController,
   updateSecurityAlertResponse: (
     method: string,
     signatureAlertId: string,
@@ -82,6 +86,17 @@ export function createPPOMMiddleware<
         return;
       }
 
+      if (req.method === MESSAGE_TYPE.ETH_SEND_TRANSACTION) {
+        const { to: toAddress } = req?.params?.[0] ?? {};
+        const internalAccounts = accountsController.listAccounts();
+        const isToInternalAccount = internalAccounts.some(
+          ({ address }) => address?.toLowerCase() === toAddress?.toLowerCase(),
+        );
+        if (isToInternalAccount) {
+          return;
+        }
+      }
+
       const securityAlertId = generateSecurityAlertId();
 
       validateRequestWithPPOM({

app/scripts/lib/transaction/util.test.ts

@@ -38,6 +38,8 @@ jest.mock('uuid', () => {
 
 const SECURITY_ALERT_ID_MOCK = '123';
 
+const INTERNAL_ACCOUNT_ADDRESS = '0xec1adf982415d2ef5ec55899b9bfb8bc0f29251b';
+
 const TRANSACTION_PARAMS_MOCK: TransactionParams = {
   from: '0x1',
 };
@@ -69,7 +71,8 @@ const TRANSACTION_REQUEST_MOCK: AddTransactionRequest = {
   transactionParams: TRANSACTION_PARAMS_MOCK,
   transactionOptions: TRANSACTION_OPTIONS_MOCK,
   waitForSubmit: false,
-} as AddTransactionRequest;
+  internalAccounts: [],
+} as unknown as AddTransactionRequest;
 
 const SECURITY_ALERT_RESPONSE_MOCK: SecurityAlertResponse = {
   result_type: BlockaidResultType.Malicious,
@@ -466,6 +469,37 @@ describe('Transaction Utils', () => {
         expect(validateRequestWithPPOMMock).toHaveBeenCalledTimes(0);
       });
 
+      it('send to users own acccount', async () => {
+        const sendRequest = {
+          ...request,
+          transactionParams: {
+            ...request.transactionParams,
+            to: INTERNAL_ACCOUNT_ADDRESS,
+          },
+        };
+        await addTransaction({
+          ...sendRequest,
+          securityAlertsEnabled: false,
+          chainId: '0x1',
+          internalAccounts: {
+            address: INTERNAL_ACCOUNT_ADDRESS,
+          } as InternalAccount,
+        });
+
+        expect(
+          request.transactionController.addTransaction,
+        ).toHaveBeenCalledTimes(1);
+
+        expect(
+          request.transactionController.addTransaction,
+        ).toHaveBeenCalledWith(
+          sendRequest.transactionParams,
+          TRANSACTION_OPTIONS_MOCK,
+        );
+
+        expect(validateRequestWithPPOMMock).toHaveBeenCalledTimes(0);
+      });
+
       it('unless chain is not supported', async () => {
         await addTransaction({
           ...request,

app/scripts/lib/transaction/util.ts

@@ -43,6 +43,7 @@ type BaseAddTransactionRequest = {
     securityAlertResponse: SecurityAlertResponse,
   ) => void;
   userOperationController: UserOperationController;
+  internalAccounts: InternalAccount[];
 };
 
 type FinalAddTransactionRequest = BaseAddTransactionRequest & {
@@ -223,6 +224,7 @@ function validateSecurity(request: AddTransactionRequest) {
     transactionOptions,
     transactionParams,
     updateSecurityAlertResponse,
+    internalAccounts,
   } = request;
 
   const { type } = transactionOptions;
@@ -240,6 +242,15 @@ function validateSecurity(request: AddTransactionRequest) {
     return;
   }
 
+  if (
+    internalAccounts.some(
+      ({ address }) =>
+        address.toLowerCase() === transactionParams.to?.toLowerCase(),
+    )
+  ) {
+    return;
+  }
+
   try {
     const { from, to, value, data } = transactionParams;
     const { actionId, origin } = transactionOptions;

app/scripts/metamask-controller.js

@@ -3419,10 +3419,6 @@ export default class MetamaskController extends EventEmitter {
       getCustodianAccounts: this.mmiController.getCustodianAccounts.bind(
         this.mmiController,
       ),
-      getCustodianAccountsByAddress:
-        this.mmiController.getCustodianAccountsByAddress.bind(
-          this.mmiController,
-        ),
       getCustodianTransactionDeepLink:
         this.mmiController.getCustodianTransactionDeepLink.bind(
           this.mmiController,
@@ -4602,6 +4598,7 @@ export default class MetamaskController extends EventEmitter {
     dappRequest,
   }) {
     return {
+      internalAccounts: this.accountsController.listAccounts(),
       dappRequest,
       networkClientId:
         dappRequest?.networkClientId ??
@@ -5203,6 +5200,7 @@ export default class MetamaskController extends EventEmitter {
         this.preferencesController,
         this.networkController,
         this.appStateController,
+        this.accountsController,
         this.updateSecurityAlertResponse.bind(this),
       ),
     );

development/build/transforms/remove-fenced-code.test.js

@@ -1,3 +1,6 @@
+/**
+ * @jest-environment node
+ */
 const buildUtils = require('@metamask/build-utils');
 const { createRemoveFencedCodeTransform } = require('./remove-fenced-code');
 const transformUtils = require('./utils');
@@ -36,6 +39,10 @@ describe('build/transforms/remove-fenced-code', () => {
       lintTransformedFileMock.mockImplementation(() => Promise.resolve());
     });
 
+    afterEach(() => {
+      jest.resetAllMocks();
+    });
+
     it('returns a PassThrough stream for files with ignored extensions', async () => {
       const fileContent = '"Valid JSON content"\n';
       const stream = createRemoveFencedCodeTransform(