fix: validate keyring type in EIP-5792 requests

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

@@ -16,7 +16,11 @@ import {
 } from '@metamask/eth-json-rpc-middleware';
 import { JsonRpcRequest } from '@metamask/utils';
 import { Messenger } from '@metamask/base-controller';
-import { AccountsControllerGetSelectedAccountAction } from '@metamask/accounts-controller';
+import {
+  AccountsControllerGetSelectedAccountAction,
+  AccountsControllerGetStateAction,
+  AccountsControllerState,
+} from '@metamask/accounts-controller';
 import { InternalAccount } from '@metamask/keyring-internal-api';
 import {
   AtomicCapabilityStatus,
@@ -105,6 +109,10 @@ describe('EIP-5792', () => {
     () => boolean
   > = jest.fn();
 
+  const getAccountsStateMock: jest.MockedFn<
+    AccountsControllerGetStateAction['handler']
+  > = jest.fn();
+
   let messenger: EIP5792Messenger;
 
   const sendCallsHooks = {
@@ -115,6 +123,12 @@ describe('EIP-5792', () => {
     validateSecurity: validateSecurityMock,
   };
 
+  const getCapabilitiesHooks = {
+    getDismissSmartAccountSuggestionEnabled:
+      getDismissSmartAccountSuggestionEnabledMock,
+    isAtomicBatchSupported: isAtomicBatchSupportedMock,
+  };
+
   beforeEach(() => {
     jest.resetAllMocks();
 
@@ -135,6 +149,11 @@ describe('EIP-5792', () => {
       getSelectedAccountMock,
     );
 
+    messenger.registerActionHandler(
+      'AccountsController:getState',
+      getAccountsStateMock,
+    );
+
     getNetworkClientByIdMock.mockReturnValue({
       configuration: {
         chainId: CHAIN_ID_MOCK,
@@ -155,6 +174,21 @@ describe('EIP-5792', () => {
         upgradeContractAddress: DELEGATION_ADDRESS_MOCK,
       },
     ]);
+
+    getAccountsStateMock.mockReturnValue({
+      internalAccounts: {
+        accounts: {
+          [FROM_MOCK]: {
+            address: FROM_MOCK,
+            metadata: {
+              keyring: {
+                type: 'HD Key Tree',
+              },
+            },
+          },
+        },
+      },
+    } as unknown as AccountsControllerState);
   });
 
   describe('processSendCalls', () => {
@@ -318,6 +352,32 @@ describe('EIP-5792', () => {
         ),
       ).rejects.toThrow(`EIP-7702 not supported on chain: ${CHAIN_ID_MOCK}`);
     });
+
+    it('throws if keyring type not supported', async () => {
+      getAccountsStateMock.mockReturnValue({
+        internalAccounts: {
+          accounts: {
+            [FROM_MOCK]: {
+              address: FROM_MOCK,
+              metadata: {
+                keyring: {
+                  type: 'Unsupported',
+                },
+              },
+            },
+          },
+        },
+      } as unknown as AccountsControllerState);
+
+      await expect(
+        processSendCalls(
+          sendCallsHooks,
+          messenger,
+          SEND_CALLS_MOCK,
+          REQUEST_MOCK,
+        ),
+      ).rejects.toThrow(`EIP-7702 upgrade not supported on account`);
+    });
   });
 
   describe('getCallsStatus', () => {
@@ -481,11 +541,8 @@ describe('EIP-5792', () => {
       ]);
 
       const capabilities = await getCapabilities(
-        {
-          getDismissSmartAccountSuggestionEnabled:
-            getDismissSmartAccountSuggestionEnabledMock,
-          isAtomicBatchSupported: isAtomicBatchSupportedMock,
-        },
+        getCapabilitiesHooks,
+        messenger,
         FROM_MOCK,
         [CHAIN_ID_MOCK],
       );
@@ -510,11 +567,8 @@ describe('EIP-5792', () => {
       ]);
 
       const capabilities = await getCapabilities(
-        {
-          getDismissSmartAccountSuggestionEnabled:
-            getDismissSmartAccountSuggestionEnabledMock,
-          isAtomicBatchSupported: isAtomicBatchSupportedMock,
-        },
+        getCapabilitiesHooks,
+        messenger,
         FROM_MOCK,
         [CHAIN_ID_MOCK],
       );
@@ -532,11 +586,8 @@ describe('EIP-5792', () => {
       isAtomicBatchSupportedMock.mockResolvedValueOnce([]);
 
       const capabilities = await getCapabilities(
-        {
-          getDismissSmartAccountSuggestionEnabled:
-            getDismissSmartAccountSuggestionEnabledMock,
-          isAtomicBatchSupported: isAtomicBatchSupportedMock,
-        },
+        getCapabilitiesHooks,
+        messenger,
         FROM_MOCK,
         [CHAIN_ID_MOCK],
       );
@@ -557,11 +608,8 @@ describe('EIP-5792', () => {
       getDismissSmartAccountSuggestionEnabledMock.mockReturnValue(true);
 
       const capabilities = await getCapabilities(
-        {
-          getDismissSmartAccountSuggestionEnabled:
-            getDismissSmartAccountSuggestionEnabledMock,
-          isAtomicBatchSupported: isAtomicBatchSupportedMock,
-        },
+        getCapabilitiesHooks,
+        messenger,
         FROM_MOCK,
         [CHAIN_ID_MOCK],
       );
@@ -580,11 +628,43 @@ describe('EIP-5792', () => {
       ]);
 
       const capabilities = await getCapabilities(
+        getCapabilitiesHooks,
+        messenger,
+        FROM_MOCK,
+        [CHAIN_ID_MOCK],
+      );
+
+      expect(capabilities).toStrictEqual({});
+    });
+
+    it('does not include atomic capability if keyring type not supported', async () => {
+      isAtomicBatchSupportedMock.mockResolvedValueOnce([
         {
-          getDismissSmartAccountSuggestionEnabled:
-            getDismissSmartAccountSuggestionEnabledMock,
-          isAtomicBatchSupported: isAtomicBatchSupportedMock,
+          chainId: CHAIN_ID_MOCK,
+          delegationAddress: undefined,
+          isSupported: false,
+          upgradeContractAddress: DELEGATION_ADDRESS_MOCK,
+        },
+      ]);
+
+      getAccountsStateMock.mockReturnValue({
+        internalAccounts: {
+          accounts: {
+            [FROM_MOCK]: {
+              address: FROM_MOCK,
+              metadata: {
+                keyring: {
+                  type: 'Unsupported',
+                },
+              },
+            },
+          },
         },
+      } as unknown as AccountsControllerState);
+
+      const capabilities = await getCapabilities(
+        getCapabilitiesHooks,
+        messenger,
         FROM_MOCK,
         [CHAIN_ID_MOCK],
       );

app/scripts/lib/transaction/eip5792.ts

@@ -19,11 +19,15 @@ import {
   SendCalls,
   SendCallsResult,
 } from '@metamask/eth-json-rpc-middleware';
-import { AccountsControllerGetSelectedAccountAction } from '@metamask/accounts-controller';
+import {
+  AccountsControllerGetSelectedAccountAction,
+  AccountsControllerGetStateAction,
+} from '@metamask/accounts-controller';
 import { generateSecurityAlertId } from '../ppom/ppom-util';
 import { EIP5792ErrorCode } from '../../../../shared/constants/transaction';
 
 type Actions =
+  | AccountsControllerGetStateAction
   | AccountsControllerGetSelectedAccountAction
   | NetworkControllerGetNetworkClientByIdAction
   | TransactionControllerGetStateAction;
@@ -37,6 +41,7 @@ export enum AtomicCapabilityStatus {
 }
 
 const VERSION = '2.0.0';
+const SUPPORTED_KEYRING_TYPES = ['HD Key Tree'];
 
 export async function processSendCalls(
   hooks: {
@@ -82,12 +87,14 @@ export async function processSendCalls(
   });
 
   const chainBatchSupport = batchSupport?.[0];
+  const keyringType = getAccountKeyringType(from, messenger) ?? '';
 
   validateSendCalls(
     params,
     dappChainId,
     dismissSmartAccountSuggestionEnabled,
     chainBatchSupport,
+    keyringType,
   );
 
   const securityAlertId = generateSecurityAlertId();
@@ -156,6 +163,7 @@ export async function getCapabilities(
     getDismissSmartAccountSuggestionEnabled: () => boolean;
     isAtomicBatchSupported: TransactionController['isAtomicBatchSupported'];
   },
+  messenger: EIP5792Messenger,
   address: Hex,
   chainIds: Hex[] | undefined,
 ) {
@@ -180,8 +188,15 @@ export async function getCapabilities(
 
       const isUpgradeDisabled = getDismissSmartAccountSuggestionEnabled();
 
+      const keyringType = getAccountKeyringType(address, messenger) ?? '';
+
+      const isSupportedAccount = SUPPORTED_KEYRING_TYPES.includes(keyringType);
+
       const canUpgrade =
-        !isUpgradeDisabled && upgradeContractAddress && !delegationAddress;
+        !isUpgradeDisabled &&
+        upgradeContractAddress &&
+        !delegationAddress &&
+        isSupportedAccount;
 
       if (!isSupported && !canUpgrade) {
         return acc;
@@ -208,11 +223,16 @@ function validateSendCalls(
   dappChainId: Hex,
   dismissSmartAccountSuggestionEnabled: boolean,
   chainBatchSupport: IsAtomicBatchSupportedResultEntry | undefined,
+  keyringType: string,
 ) {
   validateSendCallsVersion(sendCalls);
   validateSendCallsChainId(sendCalls, dappChainId, chainBatchSupport);
   validateCapabilities(sendCalls);
-  validateUserDisabled(dismissSmartAccountSuggestionEnabled, chainBatchSupport);
+  validateUpgrade(
+    dismissSmartAccountSuggestionEnabled,
+    chainBatchSupport,
+    keyringType,
+  );
 }
 
 function validateSendCallsVersion(sendCalls: SendCalls) {
@@ -277,9 +297,10 @@ function validateCapabilities(sendCalls: SendCalls) {
   }
 }
 
-function validateUserDisabled(
+function validateUpgrade(
   dismissSmartAccountSuggestionEnabled: boolean,
   chainBatchSupport: IsAtomicBatchSupportedResultEntry | undefined,
+  keyringType: string,
 ) {
   if (chainBatchSupport?.delegationAddress) {
     return;
@@ -291,6 +312,13 @@ function validateUserDisabled(
       'EIP-7702 upgrade disabled by the user',
     );
   }
+
+  if (!SUPPORTED_KEYRING_TYPES.includes(keyringType)) {
+    throw new JsonRpcError(
+      EIP5792ErrorCode.RejectedUpgrade,
+      'EIP-7702 upgrade not supported on account',
+    );
+  }
 }
 
 function getStatusCode(transactionMeta: TransactionMeta) {
@@ -312,3 +340,18 @@ function getStatusCode(transactionMeta: TransactionMeta) {
 
   return GetCallsStatusCode.PENDING;
 }
+
+function getAccountKeyringType(
+  accountAddress: Hex,
+  messenger: EIP5792Messenger,
+) {
+  const { accounts } = messenger.call(
+    'AccountsController:getState',
+  ).internalAccounts;
+
+  const account = Object.values(accounts).find(
+    (acc) => acc.address === accountAddress.toLowerCase(),
+  );
+
+  return account?.metadata?.keyring?.type;
+}

app/scripts/metamask-controller.js

@@ -2196,14 +2196,18 @@ export default class MetamaskController extends EventEmitter {
         this.controllerMessenger,
       ),
       getCallsStatus: getCallsStatus.bind(null, this.controllerMessenger),
-      getCapabilities: getCapabilities.bind(null, {
-        getDismissSmartAccountSuggestionEnabled: () =>
-          this.preferencesController.state.preferences
-            .dismissSmartAccountSuggestionEnabled,
-        isAtomicBatchSupported: this.txController.isAtomicBatchSupported.bind(
-          this.txController,
-        ),
-      }),
+      getCapabilities: getCapabilities.bind(
+        null,
+        {
+          getDismissSmartAccountSuggestionEnabled: () =>
+            this.preferencesController.state.preferences
+              .dismissSmartAccountSuggestionEnabled,
+          isAtomicBatchSupported: this.txController.isAtomicBatchSupported.bind(
+            this.txController,
+          ),
+        },
+        this.controllerMessenger,
+      ),
     });
 
     // ensure isClientOpenAndUnlocked is updated when memState updates