Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: allow gas limits to be changed during #addPaymasterData #3942

Merged
merged 9 commits into from
Apr 9, 2024
Merged

feat: allow gas limits to be changed during #addPaymasterData #3942

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
b59b69a
feat: allow gas limits to be changed during #addPaymasterData
montelaidev Feb 21, 2024
6b529f8
chore: bump keyring-api to 4.0.0
montelaidev Feb 22, 2024
4f5a6bd
fix: update to use Hex instead of HexOrEmptyBytes
montelaidev Feb 22, 2024
fea01e0
chore: bump keyring-api to 4 in AccountsController and AssetsController
montelaidev Feb 26, 2024
79e4cc5
fix: update smart contract account to return gas limits in updateUser…
montelaidev Feb 27, 2024
900b9cc
fix: update test name
montelaidev Feb 27, 2024
5b40209
Merge remote-tracking branch 'origin/main' into fix/allow-gas-limit-u…
montelaidev Mar 19, 2024
790ae68
fix: use defineHexOrEmptyBytes
montelaidev Mar 20, 2024
8754243
Merge branch 'main' into fix/allow-gas-limit-update-during-patch-userop
montelaidev Apr 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/accounts-controller/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
"@ethereumjs/util": "^8.1.0",
"@metamask/base-controller": "^5.0.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",
Expand Down
2 changes: 1 addition & 1 deletion packages/assets-controllers/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,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",
Expand Down
2 changes: 1 addition & 1 deletion packages/keyring-controller/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,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": "^8.0.1",
"@metamask/utils": "^8.3.0",
"async-mutex": "^0.2.6",
Expand Down
36 changes: 36 additions & 0 deletions packages/user-operation-controller/src/UserOperationController.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -826,6 +826,42 @@ describe('UserOperationController', () => {
expect(prepareMock).toHaveBeenCalledTimes(1);
});

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 =
{
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);
Expand Down
9 changes: 9 additions & 0 deletions packages/user-operation-controller/src/UserOperationController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -537,6 +537,15 @@ export class UserOperationController extends BaseController<
validateUpdateUserOperationResponse(response);

userOperation.paymasterAndData = response.paymasterAndData ?? EMPTY_BYTES;
if (response.callGasLimit) {
matthewwalsh0 marked this conversation as resolved.
Show resolved Hide resolved
montelaidev marked this conversation as resolved.
Show resolved Hide resolved
userOperation.callGasLimit = response.callGasLimit;
}
if (response.preVerificationGas) {
userOperation.preVerificationGas = response.preVerificationGas;
}
if (response.verificationGasLimit) {
userOperation.verificationGasLimit = response.verificationGasLimit;
}

this.#updateMetadata(metadata);
}
Expand Down
11 changes: 11 additions & 0 deletions packages/user-operation-controller/src/helpers/SnapSmartContractAccount.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ const PATCH_USER_OPERATION_RESPONSE_MOCK: Awaited<
ReturnType<KeyringController['patchUserOperation']>
> = {
paymasterAndData: '0x123',
callGasLimit: '0x444',
verificationGasLimit: '0x555',
preVerificationGas: '0x667',
};

const SIGN_USER_OPERATION_RESPONSE_MOCK: Awaited<
Expand Down Expand Up @@ -222,6 +225,11 @@ describe('SnapSmartContractAccount', () => {

expect(response).toStrictEqual<UpdateUserOperationResponse>({
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);
Expand All @@ -244,6 +252,9 @@ describe('SnapSmartContractAccount', () => {

expect(response).toStrictEqual<UpdateUserOperationResponse>({
paymasterAndData: undefined,
callGasLimit: undefined,
preVerificationGas: undefined,
verificationGasLimit: undefined,
});
});
});
Expand Down
19 changes: 13 additions & 6 deletions packages/user-operation-controller/src/helpers/SnapSmartContractAccount.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -79,6 +83,9 @@ export class SnapSmartContractAccount implements SmartContractAccount {

return {
paymasterAndData,
verificationGasLimit,
preVerificationGas,
callGasLimit,
};
}

Expand Down
9 changes: 9 additions & 0 deletions packages/user-operation-controller/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
matthewwalsh0 marked this conversation as resolved.
Show resolved Hide resolved
};

/**
Expand Down
20 changes: 19 additions & 1 deletion packages/user-operation-controller/src/utils/validation.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
7 changes: 4 additions & 3 deletions packages/user-operation-controller/src/utils/validation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,11 +135,12 @@ export function validatePrepareUserOperationResponse(
export function validateUpdateUserOperationResponse(
response: UpdateUserOperationResponse,
) {
const HexOrEmptyBytes = defineHex();

const ValidResponse = optional(
matthewwalsh0 marked this conversation as resolved.
Show resolved Hide resolved
object({
paymasterAndData: optional(HexOrEmptyBytes),
paymasterAndData: optional(defineHexOrEmptyBytes()),
callGasLimit: optional(defineHexOrEmptyBytes()),
preVerificationGas: optional(defineHexOrEmptyBytes()),
verificationGasLimit: optional(defineHexOrEmptyBytes()),
}),
);

Expand Down
14 changes: 7 additions & 7 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1646,7 +1646,7 @@ __metadata:
"@metamask/auto-changelog": ^3.4.4
"@metamask/base-controller": ^5.0.1
"@metamask/eth-snap-keyring": ^2.1.1
"@metamask/keyring-api": ^3.0.0
"@metamask/keyring-api": ^4.0.0
"@metamask/keyring-controller": ^14.0.1
"@metamask/snaps-controllers": ^4.0.0
"@metamask/snaps-sdk": ^1.3.2
Expand Down Expand Up @@ -1764,7 +1764,7 @@ __metadata:
"@metamask/controller-utils": ^9.0.2
"@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": ^14.0.1
"@metamask/metamask-eth-abis": ^3.1.1
"@metamask/network-controller": ^18.1.0
Expand Down Expand Up @@ -2436,17 +2436,17 @@ __metadata:
languageName: node
linkType: hard

"@metamask/keyring-api@npm:^3.0.0":
version: 3.0.0
resolution: "@metamask/keyring-api@npm:3.0.0"
"@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: 5e3fdc122789d605681070aa6ed6c656d5c9bb1f037fd4bf1ed2ec5fa453a0fc8b9663ddfd2106c122889682e2ae1c8ddd16913798f24821b22899f743ce1a31
checksum: 137521a967651fcc3435eb31d51c060a74199df28910c2c27a2472469db842f0fbe267d029138eb08854b8bee909886b6df038eb38dd08afbbca5e586f266d30
languageName: node
linkType: hard

Expand All @@ -2466,7 +2466,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": ^8.0.1
"@metamask/scure-bip39": ^2.1.1
"@metamask/utils": ^8.3.0
Expand Down
Loading