Skip to content

Commit 5e5cef8

Browse files
GuillaumeRxGuillaumeRx
authored
chore: Bulk register actions in InstitutionalSnapController (#41439)
## **Description** This PR updates the `InstitutionalSnapController` to use `registerMethodActionHandlers` and `MESSENGER_EXPOSED_METHODS` to register actions. ## **Changelog** CHANGELOG entry:null ## **Related issues** Fixes: ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Moderate risk because it changes how messenger actions are registered and renames a public hook method, which could break callers if any still reference the old handler name or types. > > **Overview** > Refactors `InstitutionalSnapController` messenger wiring to use `registerMethodActionHandlers` with an explicit `MESSENGER_EXPOSED_METHODS` list, and renames the hook implementation from `deferPublicationHook` to `publishHook`. > > Introduces an auto-generated `InstitutionalSnapController-method-action-types.ts` and updates controller messenger typings/imports (including `TransactionController` init messenger) to consume these generated action types. Tooling is updated by bumping `@metamask/messenger` and adding `@metamask/messenger-cli`, with new `messenger-action-types` check/generate steps integrated into `lint` and `lint:fix`. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 492d266. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 4c20c28 commit 5e5cef8

7 files changed

Lines changed: 86 additions & 68 deletions

File tree

‎app/scripts/controller-init/messengers/accounts/institutional-snap-controller-messenger.ts‎

Lines changed: 8 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -2,21 +2,9 @@ import { AccountsControllerGetAccountByAddressAction } from '@metamask/accounts-
22
import { Messenger } from '@metamask/messenger';
33
import { SnapControllerHandleRequestAction } from '@metamask/snaps-controllers';
44
import { TransactionControllerUpdateCustodialTransactionAction } from '@metamask/transaction-controller';
5-
import { InstitutionalSnapController } from '../../../controllers/institutional-snap/InstitutionalSnapController';
6-
import { RootMessenger } from '../../../lib/messenger';
7-
8-
const controllerName = 'InstitutionalSnapController';
9-
10-
export type InstitutionalSnapControllerPublishHookAction = {
11-
type: `${typeof controllerName}:publishHook`;
12-
handler: InstitutionalSnapController['deferPublicationHook'];
13-
};
145

15-
export type InstitutionalSnapControllerBeforeCheckPendingTransactionHookAction =
16-
{
17-
type: `${typeof controllerName}:beforeCheckPendingTransactionHook`;
18-
handler: InstitutionalSnapController['beforeCheckPendingTransactionHook'];
19-
};
6+
import { InstitutionalSnapControllerMethodActions } from '../../../controllers/institutional-snap/InstitutionalSnapController-method-action-types';
7+
import { RootMessenger } from '../../../lib/messenger';
208

219
export type InstitutionalSnapRequestSearchParameters = {
2210
from: string;
@@ -26,20 +14,14 @@ export type InstitutionalSnapRequestSearchParameters = {
2614
chainId: string;
2715
};
2816

29-
type AllowedActions =
17+
type Actions =
3018
| SnapControllerHandleRequestAction
3119
| AccountsControllerGetAccountByAddressAction
32-
| TransactionControllerUpdateCustodialTransactionAction;
33-
34-
type Actions =
35-
| AllowedActions
36-
| InstitutionalSnapControllerPublishHookAction
37-
| InstitutionalSnapControllerBeforeCheckPendingTransactionHookAction;
20+
| TransactionControllerUpdateCustodialTransactionAction
21+
| InstitutionalSnapControllerMethodActions;
3822

39-
export type InstitutionalSnapControllerMessenger = Messenger<
40-
'InstitutionalSnapControllerMessenger',
41-
Actions,
42-
never
23+
export type InstitutionalSnapControllerMessenger = ReturnType<
24+
typeof getInstitutionalSnapControllerMessenger
4325
>;
4426

4527
/**
@@ -54,7 +36,7 @@ export function getInstitutionalSnapControllerMessenger(
5436
messenger: RootMessenger<Actions, never>,
5537
) {
5638
const institutionalSnapControllerMessenger = new Messenger<
57-
typeof controllerName,
39+
'InstitutionalSnapController',
5840
Actions,
5941
never,
6042
typeof messenger

‎app/scripts/controller-init/messengers/transaction-controller-messenger.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ import { SubscriptionServiceAction } from '../../services/subscription/types';
6161
import {
6262
InstitutionalSnapControllerBeforeCheckPendingTransactionHookAction,
6363
InstitutionalSnapControllerPublishHookAction,
64-
} from './accounts/institutional-snap-controller-messenger';
64+
} from '../../controllers/institutional-snap/InstitutionalSnapController-method-action-types';
6565

6666
type AllowedActions = MessengerActions<TransactionControllerMessenger>;
6767

‎app/scripts/controllers/institutional-snap/InstitutionalSnapController-method-action-types.ts‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/**
2+
* This file is auto generated.
3+
* Do not edit manually.
4+
*/
5+
6+
import type { InstitutionalSnapController } from './InstitutionalSnapController';
7+
8+
export type InstitutionalSnapControllerPublishHookAction = {
9+
type: `InstitutionalSnapController:publishHook`;
10+
handler: InstitutionalSnapController['publishHook'];
11+
};
12+
13+
export type InstitutionalSnapControllerBeforeCheckPendingTransactionHookAction =
14+
{
15+
type: `InstitutionalSnapController:beforeCheckPendingTransactionHook`;
16+
handler: InstitutionalSnapController['beforeCheckPendingTransactionHook'];
17+
};
18+
19+
/**
20+
* Union of all InstitutionalSnapController action types.
21+
*/
22+
export type InstitutionalSnapControllerMethodActions =
23+
| InstitutionalSnapControllerPublishHookAction
24+
| InstitutionalSnapControllerBeforeCheckPendingTransactionHookAction;

‎app/scripts/controllers/institutional-snap/InstitutionalSnapController.test.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -158,9 +158,9 @@ describe('InstitutionalSnapController', () => {
158158
});
159159
});
160160

161-
describe('deferPublicationHook', () => {
161+
describe('publishHook', () => {
162162
it('should handle deferred publication', async () => {
163-
const result = await controller.deferPublicationHook(mockTransactionMeta);
163+
const result = await controller.publishHook(mockTransactionMeta);
164164

165165
expect(result).toBe(false);
166166
expect(messenger.call).toHaveBeenCalledWith(
@@ -206,7 +206,7 @@ describe('InstitutionalSnapController', () => {
206206
return {};
207207
});
208208

209-
const result = await controller.deferPublicationHook(mockTransactionMeta);
209+
const result = await controller.publishHook(mockTransactionMeta);
210210
expect(result).toBe(true);
211211
});
212212
});

‎app/scripts/controllers/institutional-snap/InstitutionalSnapController.ts‎

Lines changed: 12 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {
2020
InstitutionalSnapRequestSearchParameters,
2121
InstitutionalSnapResponse,
2222
} from './institutional-snap-controller.types';
23+
import type { InstitutionalSnapControllerMethodActions } from './InstitutionalSnapController-method-action-types';
2324

2425
const SNAP_ID = INSTITUTIONAL_WALLET_SNAP_ID;
2526

@@ -40,28 +41,21 @@ export type InstitutionalSnapControllerGetStateAction =
4041
InstitutionalSnapControllerControllerState
4142
>;
4243

43-
export type InstitutionalSnapControllerPublishHookAction = {
44-
type: `${typeof controllerName}:publishHook`;
45-
handler: InstitutionalSnapController['deferPublicationHook'];
46-
};
47-
48-
export type InstitutionalSnapControllerBeforeCheckPendingTransactionHookAction =
49-
{
50-
type: `${typeof controllerName}:beforeCheckPendingTransactionHook`;
51-
handler: InstitutionalSnapController['beforeCheckPendingTransactionHook'];
52-
};
53-
5444
export type InstitutionalSnapControllerStateChangeEvent =
5545
ControllerStateChangeEvent<
5646
typeof controllerName,
5747
InstitutionalSnapControllerControllerState
5848
>;
5949

50+
const MESSENGER_EXPOSED_METHODS = [
51+
'publishHook',
52+
'beforeCheckPendingTransactionHook',
53+
] as const;
54+
6055
type Actions =
6156
| AllowedActions
6257
| InstitutionalSnapControllerGetStateAction
63-
| InstitutionalSnapControllerPublishHookAction
64-
| InstitutionalSnapControllerBeforeCheckPendingTransactionHookAction;
58+
| InstitutionalSnapControllerMethodActions;
6559

6660
type Events = InstitutionalSnapControllerStateChangeEvent;
6761

@@ -100,12 +94,13 @@ export class InstitutionalSnapController extends BaseController<
10094
metadata,
10195
});
10296

103-
this.#registerMessageHandlers();
97+
this.messenger.registerMethodActionHandlers(
98+
this,
99+
MESSENGER_EXPOSED_METHODS,
100+
);
104101
}
105102

106-
async deferPublicationHook(
107-
transactionMeta: TransactionMeta,
108-
): Promise<boolean> {
103+
async publishHook(transactionMeta: TransactionMeta): Promise<boolean> {
109104
const shouldDefer = await this.#shouldDeferPublication(transactionMeta);
110105

111106
if (shouldDefer) {
@@ -128,18 +123,6 @@ export class InstitutionalSnapController extends BaseController<
128123
return !(await this.#shouldDeferPublication(transactionMeta));
129124
}
130125

131-
#registerMessageHandlers() {
132-
this.messenger.registerActionHandler(
133-
`${controllerName}:publishHook`,
134-
this.deferPublicationHook.bind(this),
135-
);
136-
137-
this.messenger.registerActionHandler(
138-
`${controllerName}:beforeCheckPendingTransactionHook`,
139-
this.beforeCheckPendingTransactionHook.bind(this),
140-
);
141-
}
142-
143126
async #handleSnapRequest(args: SnapRPCRequest) {
144127
const response = await this.messenger.call(
145128
'SnapController:handleRequest',

‎package.json‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -84,8 +84,8 @@
8484
"test:e2e:single": "node test/e2e/run-e2e-test.js",
8585
"ganache:start": "./development/run-ganache.sh",
8686
"sentry:publish": "node ./development/sentry-publish.js",
87-
"lint": "yarn lint:prettier && yarn lint:eslint && yarn lint:tsc && yarn lint:styles && yarn lint:images",
88-
"lint:fix": "yarn lint:prettier:fix && yarn lint:eslint:fix && yarn lint:styles:fix && yarn circular-deps:update && yarn lint:images:fix && yarn lint:baseline:fix",
87+
"lint": "yarn lint:prettier && yarn lint:eslint && yarn lint:tsc && yarn lint:styles && yarn lint:images && yarn messenger-action-types:check",
88+
"lint:fix": "yarn lint:prettier:fix && yarn lint:eslint:fix && yarn lint:styles:fix && yarn circular-deps:update && yarn lint:images:fix && yarn lint:baseline:fix && yarn messenger-action-types:generate",
8989
"lint:prettier": "prettier --check --cache -- '**/*.{json,md,mdx,yml}'",
9090
"lint:prettier:fix": "prettier --write --cache -- '**/*.{json,md,mdx,yml}'",
9191
"lint:changed": "node development/lint-changed.mts",
@@ -134,6 +134,8 @@
134134
"test-storybook": "test-storybook -c .storybook",
135135
"test-storybook:ci": "concurrently -k -s first -n \"SB,TEST\" -c \"magenta,blue\" \"yarn storybook:build && yarn http-server storybook-build --port 6006 \" \"wait-on tcp:6006 && echo 'Build done. Running storybook tests...' && yarn test-storybook\"",
136136
"githooks:install": "husky install",
137+
"messenger-action-types:check": "messenger-action-types app/ --check",
138+
"messenger-action-types:generate": "messenger-action-types app/ --generate",
137139
"fitness-functions": "tsx development/fitness-functions/index.ts",
138140
"generate-beta-commit": "node ./development/generate-beta-commit.js",
139141
"validate-branch-name": "validate-branch-name",
@@ -265,7 +267,7 @@
265267
"fast-xml-parser": "^5.5.7",
266268
"minimatch@npm:^10.1.1": "10.2.4",
267269
"@metamask/snaps-controllers": "^19.0.0",
268-
"@metamask/messenger@npm:^0.3.0": "^1.0.0"
270+
"@metamask/messenger@npm:^0.3.0": "^1.1.1"
269271
},
270272
"dependencies": {
271273
"@babel/runtime": "patch:@babel/runtime@npm%3A7.26.10#~/.yarn/patches/@babel-runtime-npm-7.26.10-fe8c62510a.patch",
@@ -351,7 +353,7 @@
351353
"@metamask/logo": "^4.0.0",
352354
"@metamask/message-manager": "^14.0.0",
353355
"@metamask/message-signing-snap": "1.1.4",
354-
"@metamask/messenger": "^1.0.0",
356+
"@metamask/messenger": "^1.1.1",
355357
"@metamask/metamask-eth-abis": "^3.1.1",
356358
"@metamask/multichain-account-service": "^8.0.1",
357359
"@metamask/multichain-api-client": "^0.10.1",
@@ -528,6 +530,7 @@
528530
"@metamask/eth-json-rpc-provider": "^6.0.0",
529531
"@metamask/forwarder": "^1.1.0",
530532
"@metamask/foundryup": "^1.0.1",
533+
"@metamask/messenger-cli": "^0.1.0",
531534
"@metamask/phishing-warning": "^5.1.0",
532535
"@metamask/preferences-controller": "^23.0.0",
533536
"@metamask/snap-account-abstraction-keyring-site": "^1.0.0",

‎yarn.lock‎

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7118,10 +7118,35 @@ __metadata:
71187118
languageName: node
71197119
linkType: hard
71207120

7121-
"@metamask/messenger@npm:^1.0.0":
7122-
version: 1.0.0
7123-
resolution: "@metamask/messenger@npm:1.0.0"
7124-
checksum: 10/ab1219a922d5acc86f2b1b557d79c75ca0c5f42572f50da8a2337bc5c8feb1ae95c0aaa2d2ee55b677acd4401fb2cc9c2dbacca7513edcddf20d88fb73fa7bea
7121+
"@metamask/messenger-cli@npm:^0.1.0":
7122+
version: 0.1.0
7123+
resolution: "@metamask/messenger-cli@npm:0.1.0"
7124+
dependencies:
7125+
"@metamask/utils": "npm:^11.9.0"
7126+
yargs: "npm:^17.7.2"
7127+
peerDependencies:
7128+
eslint: ">=8"
7129+
typescript: ">=5.0.0"
7130+
peerDependenciesMeta:
7131+
eslint:
7132+
optional: true
7133+
bin:
7134+
messenger-action-types: ./dist/cli.mjs
7135+
checksum: 10/dd359df00f2eba98dcfe5f3d8352d859f2e814220e709efe989ecb68c611ba32ea515fd6099a770a706066f1820abd07a134a2acf5fc6b883518c5d19991a8e2
7136+
languageName: node
7137+
linkType: hard
7138+
7139+
"@metamask/messenger@npm:^1.0.0, @metamask/messenger@npm:^1.1.1":
7140+
version: 1.1.1
7141+
resolution: "@metamask/messenger@npm:1.1.1"
7142+
dependencies:
7143+
"@metamask/utils": "npm:^11.9.0"
7144+
yargs: "npm:^17.7.2"
7145+
peerDependencies:
7146+
typescript: ">=5.0.0"
7147+
bin:
7148+
messenger-generate-action-types: ./dist/generate-action-types/cli.mjs
7149+
checksum: 10/a959af95e9e117aa0f7ad1c280f7817fef2c0b575c76837b1a6c884c9c9ef1dd0faeaef0c2c0c2035f68c7638d1f87cd172956ee962dec97d8ab6176fa6964e3
71257150
languageName: node
71267151
linkType: hard
71277152

@@ -34139,7 +34164,8 @@ __metadata:
3413934164
"@metamask/logo": "npm:^4.0.0"
3414034165
"@metamask/message-manager": "npm:^14.0.0"
3414134166
"@metamask/message-signing-snap": "npm:1.1.4"
34142-
"@metamask/messenger": "npm:^1.0.0"
34167+
"@metamask/messenger": "npm:^1.1.1"
34168+
"@metamask/messenger-cli": "npm:^0.1.0"
3414334169
"@metamask/metamask-eth-abis": "npm:^3.1.1"
3414434170
"@metamask/multichain-account-service": "npm:^8.0.1"
3414534171
"@metamask/multichain-api-client": "npm:^0.10.1"

0 commit comments

Comments
 (0)