Use account snap by default in user operations

packages/user-operation-controller/package.json

@@ -37,8 +37,10 @@
     "@metamask/controller-utils": "^8.0.2",
     "@metamask/eth-query": "^4.0.0",
     "@metamask/gas-fee-controller": "^13.0.0",
+    "@metamask/keyring-controller": "^12.2.0",
     "@metamask/network-controller": "^17.2.0",
     "@metamask/polling-controller": "^5.0.0",
+    "@metamask/rpc-errors": "^6.1.0",
     "@metamask/transaction-controller": "^21.0.0",
     "@metamask/utils": "^8.3.0",
     "ethereumjs-util": "^7.0.10",
@@ -60,6 +62,7 @@
   "peerDependencies": {
     "@metamask/approval-controller": "^5.1.2",
     "@metamask/gas-fee-controller": "^13.0.0",
+    "@metamask/keyring-controller": "^12.2.0",
     "@metamask/network-controller": "^17.2.0",
     "@metamask/transaction-controller": "^21.0.0"
   },

packages/user-operation-controller/src/UserOperationController.test.ts

@@ -1,4 +1,5 @@
 import { ApprovalType } from '@metamask/controller-utils';
+import { errorCodes } from '@metamask/rpc-errors';
 import {
   determineTransactionType,
   TransactionType,
@@ -9,6 +10,7 @@ import { EventEmitter } from 'stream';
 import { ADDRESS_ZERO, EMPTY_BYTES, VALUE_ZERO } from './constants';
 import * as BundlerHelper from './helpers/Bundler';
 import * as PendingUserOperationTrackerHelper from './helpers/PendingUserOperationTracker';
+import { SnapSmartContractAccount } from './helpers/SnapSmartContractAccount';
 import type { UserOperationMetadata } from './types';
 import {
   UserOperationStatus,
@@ -39,6 +41,7 @@ jest.mock('./utils/gas-fees');
 jest.mock('./utils/validation');
 jest.mock('./helpers/Bundler');
 jest.mock('./helpers/PendingUserOperationTracker');
+jest.mock('./helpers/SnapSmartContractAccount');
 
 const CHAIN_ID_MOCK = '0x5';
 const USER_OPERATION_HASH_MOCK = '0x123';
@@ -78,15 +81,16 @@ const SIGN_USER_OPERATION_RESPONSE_MOCK: SignUserOperationResponse = {
   signature: '0xB',
 };
 
-const ADD_USER_OPERATION_REQUEST_MOCK = {
+const ADD_USER_OPERATION_REQUEST_MOCK: AddUserOperationRequest = {
   data: '0x1',
+  from: '0x12',
   to: '0x2',
   value: '0x3',
   maxFeePerGas: '0x4',
   maxPriorityFeePerGas: '0x5',
 };
 
-const ADD_USER_OPERATION_OPTIONS_MOCK = {
+const ADD_USER_OPERATION_OPTIONS_MOCK: AddUserOperationOptions = {
   networkClientId: NETWORK_CLIENT_ID_MOCK,
   origin: ORIGIN_MOCK,
 };
@@ -257,10 +261,10 @@ describe('UserOperationController', () => {
     updateGasFeesMock
       .mockImplementationOnce(async ({ metadata }) => {
         metadata.userOperation.maxFeePerGas =
-          ADD_USER_OPERATION_REQUEST_MOCK.maxFeePerGas;
+          ADD_USER_OPERATION_REQUEST_MOCK.maxFeePerGas as string;
 
         metadata.userOperation.maxPriorityFeePerGas =
-          ADD_USER_OPERATION_REQUEST_MOCK.maxPriorityFeePerGas;
+          ADD_USER_OPERATION_REQUEST_MOCK.maxPriorityFeePerGas as string;
       })
       .mockImplementationOnce(async ({ metadata }) => {
         metadata.userOperation.maxFeePerGas = '0x6';
@@ -589,6 +593,26 @@ describe('UserOperationController', () => {
       );
     });
 
+    it('deletes user operation if rejected', async () => {
+      const controller = new UserOperationController(optionsMock);
+
+      const error = new Error(ERROR_MESSAGE_MOCK);
+      (error as unknown as Record<string, unknown>).code =
+        errorCodes.provider.userRejectedRequest;
+
+      approvalControllerAddRequestMock.mockClear();
+      approvalControllerAddRequestMock.mockRejectedValue(error);
+
+      const { hash } = await controller.addUserOperation(
+        ADD_USER_OPERATION_REQUEST_MOCK,
+        { ...ADD_USER_OPERATION_OPTIONS_MOCK, smartContractAccount },
+      );
+
+      await expect(hash()).rejects.toThrow(ERROR_MESSAGE_MOCK);
+
+      expect(Object.keys(controller.state.userOperations)).toHaveLength(0);
+    });
+
     // eslint-disable-next-line jest/expect-expect
     it('does not throw if hash function not invoked', async () => {
       const controller = new UserOperationController(optionsMock);
@@ -782,6 +806,24 @@ describe('UserOperationController', () => {
       expect(resultCallbackSuccessMock).not.toHaveBeenCalled();
     });
 
+    it('uses snap smart contract account if no smart contract account provided', async () => {
+      const prepareMock = jest.spyOn(
+        SnapSmartContractAccount.prototype,
+        'prepareUserOperation',
+      );
+
+      const controller = new UserOperationController(optionsMock);
+
+      await addUserOperation(controller, ADD_USER_OPERATION_REQUEST_MOCK, {
+        ...ADD_USER_OPERATION_OPTIONS_MOCK,
+        smartContractAccount: undefined,
+      });
+
+      await flushPromises();
+
+      expect(prepareMock).toHaveBeenCalledTimes(1);
+    });
+
     describe('if approval request resolved with updated transaction', () => {
       it('updates gas fees without regeneration if paymaster data not set', async () => {
         const controller = new UserOperationController(optionsMock);
@@ -1078,27 +1120,25 @@ describe('UserOperationController', () => {
       });
     });
 
-    if (method === 'addUserOperation') {
-      it('validates arguments', async () => {
-        const controller = new UserOperationController(optionsMock);
+    it('validates arguments', async () => {
+      const controller = new UserOperationController(optionsMock);
 
-        await addUserOperation(controller, ADD_USER_OPERATION_REQUEST_MOCK, {
-          ...ADD_USER_OPERATION_OPTIONS_MOCK,
-          smartContractAccount,
-        });
+      await addUserOperation(controller, ADD_USER_OPERATION_REQUEST_MOCK, {
+        ...ADD_USER_OPERATION_OPTIONS_MOCK,
+        smartContractAccount,
+      });
 
-        expect(validateAddUserOperationRequestMock).toHaveBeenCalledTimes(1);
-        expect(validateAddUserOperationRequestMock).toHaveBeenCalledWith(
-          ADD_USER_OPERATION_REQUEST_MOCK,
-        );
+      expect(validateAddUserOperationRequestMock).toHaveBeenCalledTimes(1);
+      expect(validateAddUserOperationRequestMock).toHaveBeenCalledWith(
+        ADD_USER_OPERATION_REQUEST_MOCK,
+      );
 
-        expect(validateAddUserOperationOptionsMock).toHaveBeenCalledTimes(1);
-        expect(validateAddUserOperationOptionsMock).toHaveBeenCalledWith({
-          ...ADD_USER_OPERATION_OPTIONS_MOCK,
-          smartContractAccount,
-        });
+      expect(validateAddUserOperationOptionsMock).toHaveBeenCalledTimes(1);
+      expect(validateAddUserOperationOptionsMock).toHaveBeenCalledWith({
+        ...ADD_USER_OPERATION_OPTIONS_MOCK,
+        smartContractAccount,
       });
-    }
+    });
 
     if (method === 'addUserOperationFromTransaction') {
       it('sets data as undefined if empty string', async () => {

packages/user-operation-controller/src/UserOperationController.ts

@@ -8,10 +8,16 @@ import { BaseController } from '@metamask/base-controller';
 import { ApprovalType } from '@metamask/controller-utils';
 import EthQuery from '@metamask/eth-query';
 import type { GasFeeState } from '@metamask/gas-fee-controller';
+import type {
+  KeyringControllerPrepareUserOperationAction,
+  KeyringControllerPatchUserOperationAction,
+  KeyringControllerSignUserOperationAction,
+} from '@metamask/keyring-controller';
 import type {
   NetworkControllerGetNetworkClientByIdAction,
   Provider,
 } from '@metamask/network-controller';
+import { errorCodes } from '@metamask/rpc-errors';
 import {
   determineTransactionType,
   type TransactionMeta,
@@ -27,6 +33,7 @@ import { v1 as random } from 'uuid';
 import { ADDRESS_ZERO, EMPTY_BYTES, VALUE_ZERO } from './constants';
 import { Bundler } from './helpers/Bundler';
 import { PendingUserOperationTracker } from './helpers/PendingUserOperationTracker';
+import { SnapSmartContractAccount } from './helpers/SnapSmartContractAccount';
 import { projectLogger as log } from './logger';
 import type {
   SmartContractAccount,
@@ -95,7 +102,10 @@ export type UserOperationStateChange = {
 export type UserOperationControllerActions =
   | GetUserOperationState
   | NetworkControllerGetNetworkClientByIdAction
-  | AddApprovalRequest;
+  | AddApprovalRequest
+  | KeyringControllerPrepareUserOperationAction
+  | KeyringControllerPatchUserOperationAction
+  | KeyringControllerSignUserOperationAction;
 
 export type UserOperationControllerEvents = UserOperationStateChange;
 
@@ -117,6 +127,7 @@ export type UserOperationControllerOptions = {
 
 export type AddUserOperationRequest = {
   data?: string;
+  from: string;
   maxFeePerGas?: string;
   maxPriorityFeePerGas?: string;
   to?: string;
@@ -138,7 +149,7 @@ export type AddUserOperationOptions = {
   networkClientId: string;
   origin: string;
   requireApproval?: boolean;
-  smartContractAccount: SmartContractAccount;
+  smartContractAccount?: SmartContractAccount;
   swaps?: AddUserOperationSwapOptions;
   type?: TransactionType;
 };
@@ -157,7 +168,9 @@ export type AddUserOperationResponse = {
 type UserOperationCache = {
   chainId: string;
   metadata: UserOperationMetadata;
-  options: AddUserOperationOptions;
+  options: AddUserOperationOptions & {
+    smartContractAccount: SmartContractAccount;
+  };
   provider: Provider;
   request: AddUserOperationRequest;
   transaction?: TransactionParams;
@@ -228,7 +241,9 @@ export class UserOperationController extends BaseController<
    * @param options.networkClientId - ID of the network client used to query the chain.
    * @param options.origin - Origin of the user operation, such as the hostname of a dApp.
    * @param options.requireApproval - Whether to require user approval before submitting the user operation. Defaults to true.
-   * @param options.smartContractAccount - Smart contract abstraction to provide the contract specific values such as call data and nonce.
+   * @param options.smartContractAccount - Smart contract abstraction to provide the contract specific values such as call data and nonce. Defaults to the current snap account.
+   * @param options.swaps - Swap metadata to record with the user operation.
+   * @param options.type - Type of the transaction.
    */
   async addUserOperation(
     request: AddUserOperationRequest,
@@ -248,7 +263,7 @@ export class UserOperationController extends BaseController<
    * @param options.networkClientId - ID of the network client used to query the chain.
    * @param options.origin - Origin of the user operation, such as the hostname of a dApp.
    * @param options.requireApproval - Whether to require user approval before submitting the user operation. Defaults to true.
-   * @param options.smartContractAccount - Smart contract abstraction to provide the contract specific values such as call data and nonce.
+   * @param options.smartContractAccount - Smart contract abstraction to provide the contract specific values such as call data and nonce. Defaults to the current snap account.
    * @param options.swaps - Swap metadata to record with the user operation.
    * @param options.type - Type of the transaction.
    */
@@ -258,18 +273,21 @@ export class UserOperationController extends BaseController<
   ): Promise<AddUserOperationResponse> {
     validateAddUserOperationOptions(options);
 
-    const { data, maxFeePerGas, maxPriorityFeePerGas, to, value } = transaction;
+    const { data, from, maxFeePerGas, maxPriorityFeePerGas, to, value } =
+      transaction;
 
-    return await this.#addUserOperation(
-      {
-        data: data === '' ? undefined : data,
-        maxFeePerGas,
-        maxPriorityFeePerGas,
-        to,
-        value,
-      },
-      { ...options, transaction },
-    );
+    const request: AddUserOperationRequest = {
+      data: data === '' ? undefined : data,
+      from,
+      maxFeePerGas,
+      maxPriorityFeePerGas,
+      to,
+      value,
+    };
+
+    validateAddUserOperationRequest(request);
+
+    return await this.#addUserOperation(request, { ...options, transaction });
   }
 
   startPollingByNetworkClientId(networkClientId: string): string {
@@ -284,7 +302,14 @@ export class UserOperationController extends BaseController<
   ): Promise<AddUserOperationResponse> {
     log('Adding user operation', { request, options });
 
-    const { networkClientId, origin, transaction, swaps } = options;
+    const {
+      networkClientId,
+      origin,
+      smartContractAccount: requestSmartContractAccount,
+      swaps,
+      transaction,
+    } = options;
+
     const { chainId, provider } = await this.#getProvider(networkClientId);
 
     const metadata = await this.#createMetadata(
@@ -294,10 +319,14 @@ export class UserOperationController extends BaseController<
       swaps,
     );
 
+    const smartContractAccount =
+      requestSmartContractAccount ??
+      new SnapSmartContractAccount(this.messagingSystem);
+
     const cache: UserOperationCache = {
       chainId,
       metadata,
-      options,
+      options: { ...options, smartContractAccount },
       provider,
       request,
       transaction,
@@ -435,7 +464,7 @@ export class UserOperationController extends BaseController<
     const { chainId, metadata, options, provider, request, transaction } =
       cache;
 
-    const { data, to, value } = request;
+    const { data, from, to, value } = request;
     const { id, transactionParams, userOperation } = metadata;
     const { smartContractAccount } = options;
 
@@ -462,6 +491,7 @@ export class UserOperationController extends BaseController<
     const response = await smartContractAccount.prepareUserOperation({
       chainId,
       data,
+      from,
       to,
       value,
     });
@@ -591,6 +621,12 @@ export class UserOperationController extends BaseController<
     metadata.status = UserOperationStatus.Failed;
 
     this.#updateMetadata(metadata);
+
+    if (
+      String(rawError.code) === String(errorCodes.provider.userRejectedRequest)
+    ) {
+      this.#deleteMetadata(id);
+    }
   }
 
   #createEmptyUserOperation(transaction?: TransactionParams): UserOperation {
@@ -619,6 +655,12 @@ export class UserOperationController extends BaseController<
     this.#updateTransaction(metadata);
   }
 
+  #deleteMetadata(id: string) {
+    this.update((state) => {
+      delete state.userOperations[id];
+    });
+  }
+
   #updateTransaction(metadata: UserOperationMetadata) {
     if (!metadata.transactionParams) {
       return;