From b59b69acc4b94fd6466071fd9268b34e266c06f7 Mon Sep 17 00:00:00 2001 From: Monte Lai Date: Wed, 21 Feb 2024 12:32:29 +0800 Subject: [PATCH 1/7] feat: allow gas limits to be changed during #addPaymasterData --- .../src/UserOperationController.test.ts | 36 +++++++++++++++++++ .../src/UserOperationController.ts | 9 +++++ .../user-operation-controller/src/types.ts | 9 +++++ .../src/utils/validation.ts | 3 ++ 4 files changed, 57 insertions(+) diff --git a/packages/user-operation-controller/src/UserOperationController.test.ts b/packages/user-operation-controller/src/UserOperationController.test.ts index 678d4a94c0..17edcdee29 100644 --- a/packages/user-operation-controller/src/UserOperationController.test.ts +++ b/packages/user-operation-controller/src/UserOperationController.test.ts @@ -824,6 +824,42 @@ describe('UserOperationController', () => { expect(prepareMock).toHaveBeenCalledTimes(1); }); + it('uses gas limits suggested by snap during #addPaymasterData', async () => { + const controller = new UserOperationController(optionsMock); + const UPDATE_USER_OPERATION_WITH_GAS_LIMITS_RESPONSE_MOCK: UpdateUserOperationResponse = + { + paymasterAndData: '0xA', + callGasLimit: '0x123', + preVerificationGas: '0x456', + verificationGasLimit: '0x789', + }; + smartContractAccount.updateUserOperation.mockResolvedValue( + UPDATE_USER_OPERATION_WITH_GAS_LIMITS_RESPONSE_MOCK, + ); + const { id, hash } = await addUserOperation( + controller, + ADD_USER_OPERATION_REQUEST_MOCK, + { ...ADD_USER_OPERATION_OPTIONS_MOCK, smartContractAccount }, + ); + + await hash(); + + expect(Object.keys(controller.state.userOperations)).toHaveLength(1); + expect( + controller.state.userOperations[id].userOperation.callGasLimit, + ).toBe(UPDATE_USER_OPERATION_WITH_GAS_LIMITS_RESPONSE_MOCK.callGasLimit); + expect( + controller.state.userOperations[id].userOperation.verificationGasLimit, + ).toBe( + UPDATE_USER_OPERATION_WITH_GAS_LIMITS_RESPONSE_MOCK.verificationGasLimit, + ); + expect( + controller.state.userOperations[id].userOperation.preVerificationGas, + ).toBe( + UPDATE_USER_OPERATION_WITH_GAS_LIMITS_RESPONSE_MOCK.preVerificationGas, + ); + }); + 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); diff --git a/packages/user-operation-controller/src/UserOperationController.ts b/packages/user-operation-controller/src/UserOperationController.ts index bc3cabc7af..fab5242855 100644 --- a/packages/user-operation-controller/src/UserOperationController.ts +++ b/packages/user-operation-controller/src/UserOperationController.ts @@ -537,6 +537,15 @@ export class UserOperationController extends BaseController< validateUpdateUserOperationResponse(response); userOperation.paymasterAndData = response.paymasterAndData ?? EMPTY_BYTES; + if (response.callGasLimit) { + userOperation.callGasLimit = response.callGasLimit; + } + if (response.preVerificationGas) { + userOperation.preVerificationGas = response.preVerificationGas; + } + if (response.verificationGasLimit) { + userOperation.verificationGasLimit = response.verificationGasLimit; + } this.#updateMetadata(metadata); } diff --git a/packages/user-operation-controller/src/types.ts b/packages/user-operation-controller/src/types.ts index 9162189cad..70c165f12d 100644 --- a/packages/user-operation-controller/src/types.ts +++ b/packages/user-operation-controller/src/types.ts @@ -253,6 +253,15 @@ export type UpdateUserOperationResponse = { * Not required if a paymaster is not sponsoring the transaction. */ paymasterAndData?: string; + + /** + * The final gas limits for the user operation suggested by the smart contract account. + * The simulated gas limits may be different after the bundler estimates gas with the use + * of the paymaster. + */ + callGasLimit?: string; + preVerificationGas?: string; + verificationGasLimit?: string; }; /** diff --git a/packages/user-operation-controller/src/utils/validation.ts b/packages/user-operation-controller/src/utils/validation.ts index 362178a6a5..0e27a0bf2a 100644 --- a/packages/user-operation-controller/src/utils/validation.ts +++ b/packages/user-operation-controller/src/utils/validation.ts @@ -140,6 +140,9 @@ export function validateUpdateUserOperationResponse( const ValidResponse = optional( object({ paymasterAndData: optional(HexOrEmptyBytes), + callGasLimit: optional(HexOrEmptyBytes), + preVerificationGas: optional(HexOrEmptyBytes), + verificationGasLimit: optional(HexOrEmptyBytes), }), ); From 6b529f8151fd40d7f7bac442baf7d480134d7a4a Mon Sep 17 00:00:00 2001 From: Monte Lai Date: Thu, 22 Feb 2024 20:00:54 +0800 Subject: [PATCH 2/7] chore: bump keyring-api to 4.0.0 --- packages/keyring-controller/package.json | 2 +- yarn.lock | 16 +++++++++++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/packages/keyring-controller/package.json b/packages/keyring-controller/package.json index eb1315ceb2..5725bf7f90 100644 --- a/packages/keyring-controller/package.json +++ b/packages/keyring-controller/package.json @@ -37,7 +37,7 @@ "@metamask/eth-hd-keyring": "^7.0.1", "@metamask/eth-sig-util": "^7.0.1", "@metamask/eth-simple-keyring": "^6.0.1", - "@metamask/keyring-api": "^3.0.0", + "@metamask/keyring-api": "^4.0.0", "@metamask/message-manager": "^7.3.8", "@metamask/utils": "^8.3.0", "async-mutex": "^0.2.6", diff --git a/yarn.lock b/yarn.lock index fcc17862f2..19c0199fd3 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2226,6 +2226,20 @@ __metadata: languageName: node linkType: hard +"@metamask/keyring-api@npm:^4.0.0": + version: 4.0.0 + resolution: "@metamask/keyring-api@npm:4.0.0" + dependencies: + "@metamask/providers": ^14.0.1 + "@metamask/snaps-sdk": ^1.3.2 + "@metamask/utils": ^8.1.0 + "@types/uuid": ^9.0.1 + superstruct: ^1.0.3 + uuid: ^9.0.0 + checksum: 137521a967651fcc3435eb31d51c060a74199df28910c2c27a2472469db842f0fbe267d029138eb08854b8bee909886b6df038eb38dd08afbbca5e586f266d30 + languageName: node + linkType: hard + "@metamask/keyring-controller@^12.2.0, @metamask/keyring-controller@workspace:packages/keyring-controller": version: 0.0.0-use.local resolution: "@metamask/keyring-controller@workspace:packages/keyring-controller" @@ -2241,7 +2255,7 @@ __metadata: "@metamask/eth-hd-keyring": ^7.0.1 "@metamask/eth-sig-util": ^7.0.1 "@metamask/eth-simple-keyring": ^6.0.1 - "@metamask/keyring-api": ^3.0.0 + "@metamask/keyring-api": ^4.0.0 "@metamask/message-manager": ^7.3.8 "@metamask/scure-bip39": ^2.1.1 "@metamask/utils": ^8.3.0 From 4f5a6bdf238cb8553fab93eab3b83ec138c818a2 Mon Sep 17 00:00:00 2001 From: Monte Lai Date: Thu, 22 Feb 2024 20:04:04 +0800 Subject: [PATCH 3/7] fix: update to use Hex instead of HexOrEmptyBytes --- .../user-operation-controller/src/utils/validation.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/user-operation-controller/src/utils/validation.ts b/packages/user-operation-controller/src/utils/validation.ts index 0e27a0bf2a..4bbde3c1e0 100644 --- a/packages/user-operation-controller/src/utils/validation.ts +++ b/packages/user-operation-controller/src/utils/validation.ts @@ -1,5 +1,5 @@ import { TransactionType } from '@metamask/transaction-controller'; -import { isStrictHexString } from '@metamask/utils'; +import { HexStruct, isStrictHexString } from '@metamask/utils'; import type { Struct, StructError } from 'superstruct'; import { assert, @@ -140,9 +140,9 @@ export function validateUpdateUserOperationResponse( const ValidResponse = optional( object({ paymasterAndData: optional(HexOrEmptyBytes), - callGasLimit: optional(HexOrEmptyBytes), - preVerificationGas: optional(HexOrEmptyBytes), - verificationGasLimit: optional(HexOrEmptyBytes), + callGasLimit: optional(HexStruct), + preVerificationGas: optional(HexStruct), + verificationGasLimit: optional(HexStruct), }), ); From fea01e0d426b564fce9105acaa2235639f7bf480 Mon Sep 17 00:00:00 2001 From: Monte Lai Date: Mon, 26 Feb 2024 20:42:37 +0800 Subject: [PATCH 4/7] chore: bump keyring-api to 4 in AccountsController and AssetsController --- packages/accounts-controller/package.json | 2 +- packages/assets-controllers/package.json | 2 +- yarn.lock | 18 ++---------------- 3 files changed, 4 insertions(+), 18 deletions(-) diff --git a/packages/accounts-controller/package.json b/packages/accounts-controller/package.json index 0e6b608faf..1532ccd095 100644 --- a/packages/accounts-controller/package.json +++ b/packages/accounts-controller/package.json @@ -33,7 +33,7 @@ "dependencies": { "@metamask/base-controller": "^4.1.1", "@metamask/eth-snap-keyring": "^2.1.1", - "@metamask/keyring-api": "^3.0.0", + "@metamask/keyring-api": "^4.0.0", "@metamask/snaps-sdk": "^1.3.2", "@metamask/snaps-utils": "^5.1.2", "@metamask/utils": "^8.3.0", diff --git a/packages/assets-controllers/package.json b/packages/assets-controllers/package.json index a92c4b7f79..89601bac47 100644 --- a/packages/assets-controllers/package.json +++ b/packages/assets-controllers/package.json @@ -61,7 +61,7 @@ "devDependencies": { "@metamask/auto-changelog": "^3.4.4", "@metamask/ethjs-provider-http": "^0.3.0", - "@metamask/keyring-api": "^3.0.0", + "@metamask/keyring-api": "^4.0.0", "@types/jest": "^27.4.1", "@types/lodash": "^4.14.191", "@types/node": "^16.18.54", diff --git a/yarn.lock b/yarn.lock index 19c0199fd3..8e6a4f6be1 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1484,7 +1484,7 @@ __metadata: "@metamask/auto-changelog": ^3.4.4 "@metamask/base-controller": ^4.1.1 "@metamask/eth-snap-keyring": ^2.1.1 - "@metamask/keyring-api": ^3.0.0 + "@metamask/keyring-api": ^4.0.0 "@metamask/keyring-controller": ^12.2.0 "@metamask/snaps-controllers": ^4.0.0 "@metamask/snaps-sdk": ^1.3.2 @@ -1589,7 +1589,7 @@ __metadata: "@metamask/controller-utils": ^8.0.3 "@metamask/eth-query": ^4.0.0 "@metamask/ethjs-provider-http": ^0.3.0 - "@metamask/keyring-api": ^3.0.0 + "@metamask/keyring-api": ^4.0.0 "@metamask/keyring-controller": ^12.2.0 "@metamask/metamask-eth-abis": 3.0.0 "@metamask/network-controller": ^17.2.0 @@ -2212,20 +2212,6 @@ __metadata: languageName: node linkType: hard -"@metamask/keyring-api@npm:^3.0.0": - version: 3.0.0 - resolution: "@metamask/keyring-api@npm:3.0.0" - dependencies: - "@metamask/providers": ^14.0.1 - "@metamask/snaps-sdk": ^1.3.2 - "@metamask/utils": ^8.1.0 - "@types/uuid": ^9.0.1 - superstruct: ^1.0.3 - uuid: ^9.0.0 - checksum: 5e3fdc122789d605681070aa6ed6c656d5c9bb1f037fd4bf1ed2ec5fa453a0fc8b9663ddfd2106c122889682e2ae1c8ddd16913798f24821b22899f743ce1a31 - languageName: node - linkType: hard - "@metamask/keyring-api@npm:^4.0.0": version: 4.0.0 resolution: "@metamask/keyring-api@npm:4.0.0" From 79e4cc5a62128080b408af8b64641f52373b309c Mon Sep 17 00:00:00 2001 From: Monte Lai Date: Tue, 27 Feb 2024 21:45:05 +0800 Subject: [PATCH 5/7] fix: update smart contract account to return gas limits in updateUserOperation --- .../helpers/SnapSmartContractAccount.test.ts | 11 +++++++++++ .../src/helpers/SnapSmartContractAccount.ts | 19 +++++++++++++------ 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/packages/user-operation-controller/src/helpers/SnapSmartContractAccount.test.ts b/packages/user-operation-controller/src/helpers/SnapSmartContractAccount.test.ts index 83f7715754..f322323239 100644 --- a/packages/user-operation-controller/src/helpers/SnapSmartContractAccount.test.ts +++ b/packages/user-operation-controller/src/helpers/SnapSmartContractAccount.test.ts @@ -61,6 +61,9 @@ const PATCH_USER_OPERATION_RESPONSE_MOCK: Awaited< ReturnType > = { paymasterAndData: '0x123', + callGasLimit: '0x444', + verificationGasLimit: '0x555', + preVerificationGas: '0x667', }; const SIGN_USER_OPERATION_RESPONSE_MOCK: Awaited< @@ -222,6 +225,11 @@ describe('SnapSmartContractAccount', () => { expect(response).toStrictEqual({ paymasterAndData: PATCH_USER_OPERATION_RESPONSE_MOCK.paymasterAndData, + callGasLimit: PATCH_USER_OPERATION_RESPONSE_MOCK.callGasLimit, + preVerificationGas: + PATCH_USER_OPERATION_RESPONSE_MOCK.preVerificationGas, + verificationGasLimit: + PATCH_USER_OPERATION_RESPONSE_MOCK.verificationGasLimit, }); expect(patchMock).toHaveBeenCalledTimes(1); @@ -244,6 +252,9 @@ describe('SnapSmartContractAccount', () => { expect(response).toStrictEqual({ paymasterAndData: undefined, + callGasLimit: undefined, + preVerificationGas: undefined, + verificationGasLimit: undefined, }); }); }); diff --git a/packages/user-operation-controller/src/helpers/SnapSmartContractAccount.ts b/packages/user-operation-controller/src/helpers/SnapSmartContractAccount.ts index 7db5d57cb2..c8e77462c4 100644 --- a/packages/user-operation-controller/src/helpers/SnapSmartContractAccount.ts +++ b/packages/user-operation-controller/src/helpers/SnapSmartContractAccount.ts @@ -65,12 +65,16 @@ export class SnapSmartContractAccount implements SmartContractAccount { const { userOperation } = request; const { sender } = userOperation; - const { paymasterAndData: responsePaymasterAndData } = - await this.#messenger.call( - 'KeyringController:patchUserOperation', - sender, - userOperation, - ); + const { + paymasterAndData: responsePaymasterAndData, + verificationGasLimit, + preVerificationGas, + callGasLimit, + } = await this.#messenger.call( + 'KeyringController:patchUserOperation', + sender, + userOperation, + ); const paymasterAndData = responsePaymasterAndData === EMPTY_BYTES @@ -79,6 +83,9 @@ export class SnapSmartContractAccount implements SmartContractAccount { return { paymasterAndData, + verificationGasLimit, + preVerificationGas, + callGasLimit, }; } From 900b9cc5175f9151173194ef2016c8a050fd6f44 Mon Sep 17 00:00:00 2001 From: Monte Lai Date: Tue, 27 Feb 2024 21:59:19 +0800 Subject: [PATCH 6/7] fix: update test name --- .../src/UserOperationController.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/user-operation-controller/src/UserOperationController.test.ts b/packages/user-operation-controller/src/UserOperationController.test.ts index 17edcdee29..aacbd333f5 100644 --- a/packages/user-operation-controller/src/UserOperationController.test.ts +++ b/packages/user-operation-controller/src/UserOperationController.test.ts @@ -824,7 +824,7 @@ describe('UserOperationController', () => { expect(prepareMock).toHaveBeenCalledTimes(1); }); - it('uses gas limits suggested by snap during #addPaymasterData', async () => { + it('uses gas limits suggested by smart contract account during #addPaymasterData', async () => { const controller = new UserOperationController(optionsMock); const UPDATE_USER_OPERATION_WITH_GAS_LIMITS_RESPONSE_MOCK: UpdateUserOperationResponse = { From 790ae68d671516ab50778e94a8aff8bad4121209 Mon Sep 17 00:00:00 2001 From: Monte Lai Date: Wed, 20 Mar 2024 19:43:41 +0100 Subject: [PATCH 7/7] fix: use defineHexOrEmptyBytes --- .../src/utils/validation.test.ts | 20 ++++++++++++++++++- .../src/utils/validation.ts | 12 +++++------ 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/packages/user-operation-controller/src/utils/validation.test.ts b/packages/user-operation-controller/src/utils/validation.test.ts index 0b9af496a6..acca6482bf 100644 --- a/packages/user-operation-controller/src/utils/validation.test.ts +++ b/packages/user-operation-controller/src/utils/validation.test.ts @@ -432,7 +432,25 @@ describe('validation', () => { 'paymasterAndData', 'wrong type', 123, - 'Expected a value of type `Hexadecimal String`, but received: `123`', + 'Expected a value of type `Hexadecimal String or 0x`, but received: `123`', + ], + [ + 'callGasLimit', + 'wrong type', + 123, + 'Expected a value of type `Hexadecimal String or 0x`, but received: `123`', + ], + [ + 'preVerificationGas', + 'wrong type', + 123, + 'Expected a value of type `Hexadecimal String or 0x`, but received: `123`', + ], + [ + 'verificationGasLimit', + 'wrong type', + 123, + 'Expected a value of type `Hexadecimal String or 0x`, but received: `123`', ], ])( 'throws if %s is %s', diff --git a/packages/user-operation-controller/src/utils/validation.ts b/packages/user-operation-controller/src/utils/validation.ts index 4bbde3c1e0..10cffda287 100644 --- a/packages/user-operation-controller/src/utils/validation.ts +++ b/packages/user-operation-controller/src/utils/validation.ts @@ -1,5 +1,5 @@ import { TransactionType } from '@metamask/transaction-controller'; -import { HexStruct, isStrictHexString } from '@metamask/utils'; +import { isStrictHexString } from '@metamask/utils'; import type { Struct, StructError } from 'superstruct'; import { assert, @@ -135,14 +135,12 @@ export function validatePrepareUserOperationResponse( export function validateUpdateUserOperationResponse( response: UpdateUserOperationResponse, ) { - const HexOrEmptyBytes = defineHex(); - const ValidResponse = optional( object({ - paymasterAndData: optional(HexOrEmptyBytes), - callGasLimit: optional(HexStruct), - preVerificationGas: optional(HexStruct), - verificationGasLimit: optional(HexStruct), + paymasterAndData: optional(defineHexOrEmptyBytes()), + callGasLimit: optional(defineHexOrEmptyBytes()), + preVerificationGas: optional(defineHexOrEmptyBytes()), + verificationGasLimit: optional(defineHexOrEmptyBytes()), }), );