Skip to content

Commit

Permalink
Use account snap by default in user operations (#3844)
Browse files Browse the repository at this point in the history 
Provide a default implementation for SmartContractAccount using SnapSmartContractAccount.
Delete user operation from state if rejected during approval.
Set transaction userFeeLevel to custom if using a paymaster.
Validate arguments when calling addUserOperationFromTransaction.
  • Loading branch information
@matthewwalsh0
matthewwalsh0 authored Jan 29, 2024
1 parent ac18b9c commit 4bc2d40
Show file tree
Hide file tree
Showing 13 changed files with 556 additions and 67 deletions.
3 changes: 3 additions & 0 deletions packages/user-operation-controller/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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"
},
Expand Down
82 changes: 61 additions & 21 deletions packages/user-operation-controller/src/UserOperationController.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { ApprovalType } from '@metamask/controller-utils';
import { errorCodes } from '@metamask/rpc-errors';
import {
determineTransactionType,
TransactionType,
Expand All @@ -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,
Expand Down Expand Up @@ -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';
Expand Down Expand Up @@ -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,
};
Expand Down Expand Up @@ -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';
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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 () => {
Expand Down
80 changes: 61 additions & 19 deletions packages/user-operation-controller/src/UserOperationController.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -95,7 +102,10 @@ export type UserOperationStateChange = {
export type UserOperationControllerActions =
| GetUserOperationState
| NetworkControllerGetNetworkClientByIdAction
| AddApprovalRequest;
| AddApprovalRequest
| KeyringControllerPrepareUserOperationAction
| KeyringControllerPatchUserOperationAction
| KeyringControllerSignUserOperationAction;

export type UserOperationControllerEvents = UserOperationStateChange;

Expand All @@ -117,6 +127,7 @@ export type UserOperationControllerOptions = {

export type AddUserOperationRequest = {
data?: string;
from: string;
maxFeePerGas?: string;
maxPriorityFeePerGas?: string;
to?: string;
Expand All @@ -138,7 +149,7 @@ export type AddUserOperationOptions = {
networkClientId: string;
origin: string;
requireApproval?: boolean;
smartContractAccount: SmartContractAccount;
smartContractAccount?: SmartContractAccount;
swaps?: AddUserOperationSwapOptions;
type?: TransactionType;
};
Expand All @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -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.
*/
Expand All @@ -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 {
Expand All @@ -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(
Expand All @@ -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,
Expand Down Expand Up @@ -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;

Expand All @@ -462,6 +491,7 @@ export class UserOperationController extends BaseController<
const response = await smartContractAccount.prepareUserOperation({
chainId,
data,
from,
to,
value,
});
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
Expand Down
Loading

0 comments on commit 4bc2d40

Please sign in to comment.