Skip to content

Commit e1a71c7

Browse files
refactor: Use messenger for majority of Snaps RPC methods (#29970)
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until this PR meets the canonical Definition of Ready For Review in `docs/readme/ready-for-review.md`. In short: the template must be materially complete (not just section titles present), all status checks must be currently passing, and the only expected follow-up commits must be reviewer-driven. --> ## **Description** This PR completes a refactor to use the messenger to replace the majority of the method hooks used by Snaps RPC methods. This lets consolidate implementations across both clients and reduces the maintenance burden. There should be no functional difference to the user. ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: null ## **Related issues** https://consensyssoftware.atlassian.net/browse/WPC-995 <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Refactors Snaps RPC/permission wiring to rely on messenger calls and updates Snap-related package versions, which could subtly change authorization/unlock behavior or break Snap RPC methods if messenger action contracts differ. > > **Overview** > Refactors `snapMethodMiddlewareBuilder` and Snap permission specifications to remove direct `Engine.context` hook usage and instead rely on `controllerMessenger`/`initMessenger` (`waitUntil`, `call`) for unlock state, keyring creation, and other Snap RPC hooks. > > Updates `permissionControllerInit` to stop injecting a `KeyringController` client into Snap permission specs, removes the legacy `getMnemonic` helper/tests, and adjusts Snap/BackgroundBridge tests to mock the newer messenger APIs (e.g., `delegate`). > > Bumps `@metamask/snaps-controllers`, `@metamask/snaps-rpc-methods`, and `@metamask/snaps-sdk` to newer versions. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit cecac79. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent f2533f3 commit e1a71c7

11 files changed

Lines changed: 166 additions & 554 deletions

File tree

app/core/BackgroundBridge/BackgroundBridge.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -671,7 +671,6 @@ export class BackgroundBridge extends EventEmitter {
671671
if (!this.isMMSDK && !this.isWalletConnect) {
672672
engine.push(
673673
snapMethodMiddlewareBuilder(
674-
Engine.context,
675674
Engine.controllerMessenger,
676675
this.url,
677676
// We assume that origins connecting through the BackgroundBridge are websites

app/core/BackgroundBridge/BackgroundBridge.test.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ jest.mock('../Engine', () => ({
4343
subscribe: jest.fn(),
4444
tryUnsubscribe: jest.fn(),
4545
unsubscribe: jest.fn(),
46+
delegate: jest.fn(),
4647
},
4748
context: {
4849
AccountsController: {

app/core/Engine/controllers/permission-controller-init.ts

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -30,16 +30,7 @@ export const permissionControllerInit: MessengerClientInitFunction<
3030
>,
3131
PermissionControllerMessenger,
3232
PermissionControllerInitMessenger
33-
> = ({
34-
controllerMessenger,
35-
initMessenger,
36-
persistedState,
37-
getMessengerClient,
38-
}) => {
39-
///: BEGIN:ONLY_INCLUDE_IF(snaps)
40-
const keyringController = getMessengerClient('KeyringController');
41-
///: END:ONLY_INCLUDE_IF
42-
33+
> = ({ controllerMessenger, initMessenger, persistedState }) => {
4334
const controller = new PermissionController({
4435
messenger: controllerMessenger,
4536
state: persistedState.PermissionController,
@@ -65,9 +56,7 @@ export const permissionControllerInit: MessengerClientInitFunction<
6556
permissionSpecifications: {
6657
...getPermissionSpecifications(),
6758
///: BEGIN:ONLY_INCLUDE_IF(snaps)
68-
...getSnapPermissionSpecifications(initMessenger, {
69-
addNewKeyring: keyringController.addNewKeyring.bind(keyringController),
70-
}),
59+
...getSnapPermissionSpecifications(initMessenger),
7160
///: END:ONLY_INCLUDE_IF
7261
},
7362
unrestrictedMethods,

app/core/Snaps/SnapBridge.test.ts

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import { SnapId } from '@metamask/snaps-sdk';
2-
import { Json, JsonRpcRequest, PendingJsonRpcResponse } from '@metamask/utils';
2+
import { Json } from '@metamask/utils';
33
import ObjectMultiplex from '@metamask/object-multiplex';
4-
import { JsonRpcEngineNextCallback } from '@metamask/json-rpc-engine';
54
// eslint-disable-next-line import-x/no-nodejs-modules
65
import { Duplex } from 'stream';
76
import SnapBridge from './SnapBridge';
@@ -31,6 +30,16 @@ jest.mock('../Engine/Engine', () => ({
3130
];
3231
}
3332
}),
33+
delegate: jest.fn().mockImplementation((options) => {
34+
options.messenger.call = jest.fn().mockImplementation((action) => {
35+
if (action === 'KeyringController:getState') {
36+
return {
37+
isUnlocked: true,
38+
keyrings: [],
39+
};
40+
}
41+
});
42+
}),
3443
},
3544
context: {
3645
AccountsController: {
@@ -61,9 +70,6 @@ jest.mock('../Engine/Engine', () => ({
6170
},
6271
]),
6372
},
64-
ApprovalController: {
65-
addAndShowApprovalRequest: jest.fn(),
66-
},
6773
SelectedNetworkController: {
6874
getProviderAndBlockTracker: jest.fn().mockReturnValue({
6975
blockTracker: {},

app/core/Snaps/SnapBridge.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,6 @@ export default class SnapBridge {
177177

178178
engine.push(
179179
snapMethodMiddlewareBuilder(
180-
context,
181180
controllerMessenger,
182181
this.#snapId,
183182
SubjectType.Snap,
Lines changed: 37 additions & 243 deletions
Original file line numberDiff line numberDiff line change
@@ -1,48 +1,21 @@
11
///: BEGIN:ONLY_INCLUDE_IF(snaps)
22
import { createSnapsMethodMiddleware } from '@metamask/snaps-rpc-methods';
3-
import {
4-
RequestedPermissions,
5-
SubjectType,
6-
} from '@metamask/permission-controller';
7-
import { SnapRpcHookArgs } from '@metamask/snaps-utils';
8-
import { RestrictedMethods } from '../Permissions/constants';
3+
import { SubjectType } from '@metamask/permission-controller';
4+
import { Json } from '@metamask/utils';
5+
import { captureException } from '@sentry/react-native';
6+
import { AppState } from 'react-native';
7+
import { getVersion } from 'react-native-device-info';
8+
99
import { keyringSnapPermissionsBuilder } from '../SnapKeyring/keyringSnapsPermissions';
10-
import { SnapId } from '@metamask/snaps-sdk';
1110
import { RootExtendedMessenger, EngineContext } from '../Engine';
12-
import { handleSnapRequest } from './utils';
13-
import { captureException } from '@sentry/react-native';
14-
import {
15-
CronjobControllerCancelAction,
16-
CronjobControllerGetAction,
17-
SnapControllerClearSnapStateAction,
18-
SnapControllerGetPermittedSnapsAction,
19-
SnapControllerGetSnapAction,
20-
SnapControllerGetSnapFileAction,
21-
SnapControllerGetSnapStateAction,
22-
SnapControllerInstallSnapsAction,
23-
SnapControllerUpdateSnapStateAction,
24-
SnapInterfaceControllerCreateInterfaceAction,
25-
SnapInterfaceControllerResolveInterfaceAction,
26-
SnapInterfaceControllerUpdateInterfaceAction,
27-
SnapInterfaceControllerUpdateInterfaceStateAction,
28-
WebSocketServiceOpenAction,
29-
WebSocketServiceCloseAction,
30-
WebSocketServiceGetAllAction,
31-
WebSocketServiceSendMessageAction,
32-
} from '../Engine/controllers/snaps/constants';
33-
import { KeyringTypes } from '@metamask/keyring-controller';
3411
import { analytics } from '../../util/analytics/analytics';
3512
import { AnalyticsEventBuilder } from '../../util/analytics/AnalyticsEventBuilder';
36-
import { Json } from '@metamask/utils';
37-
import { CronjobControllerScheduleAction } from '@metamask/snaps-controllers';
3813
import { endTrace, trace } from '../../util/trace';
39-
import { AppState } from 'react-native';
40-
import { getVersion } from 'react-native-device-info';
4114

4215
export const trackSnapEvent = (eventPayload: {
4316
event: string;
44-
properties: Record<string, Json>;
45-
sensitiveProperties: Record<string, Json>;
17+
properties?: Record<string, Json>;
18+
sensitiveProperties?: Record<string, Json>;
4619
}) => {
4720
analytics.trackEvent(
4821
AnalyticsEventBuilder.createEventBuilder(eventPayload.event)
@@ -52,222 +25,43 @@ export const trackSnapEvent = (eventPayload: {
5225
);
5326
};
5427

55-
export function getSnapIdFromRequest(
56-
request: Record<string, unknown>,
57-
): SnapId | null {
58-
const { snapId } = request;
59-
return typeof snapId === 'string' ? (snapId as SnapId) : null;
60-
}
61-
// Snaps middleware
62-
/*
63-
from extension https://github.dev/MetaMask/metamask-extension/blob/1d5e8a78400d7aaaf2b3cbdb30cff9399061df34/app/scripts/metamask-controller.js#L3830-L3861
64-
*/
6528
const snapMethodMiddlewareBuilder = (
66-
engineContext: EngineContext,
6729
controllerMessenger: RootExtendedMessenger,
6830
origin: string,
6931
subjectType: SubjectType,
7032
) =>
71-
createSnapsMethodMiddleware(subjectType === SubjectType.Snap, {
72-
getUnlockPromise: () => {
73-
if (engineContext.KeyringController.isUnlocked()) {
74-
return Promise.resolve();
75-
}
76-
return new Promise<void>((resolve) => {
77-
controllerMessenger.subscribeOnceIf(
78-
'KeyringController:unlock',
79-
resolve,
80-
() => true,
81-
);
82-
});
83-
},
84-
getSnaps: controllerMessenger.call.bind(
85-
controllerMessenger,
86-
SnapControllerGetPermittedSnapsAction,
87-
origin,
88-
),
89-
requestPermissions: async (requestedPermissions: RequestedPermissions) =>
90-
await engineContext.PermissionController.requestPermissions(
91-
{ origin },
92-
requestedPermissions,
93-
),
94-
getPermissions: engineContext.PermissionController.getPermissions.bind(
95-
engineContext.PermissionController,
96-
origin,
97-
),
98-
hasPermission: engineContext.PermissionController.hasPermission.bind(
99-
engineContext.PermissionController,
100-
origin,
101-
),
102-
getAllowedKeyringMethods: keyringSnapPermissionsBuilder(origin),
103-
getSnapFile: controllerMessenger.call.bind(
104-
controllerMessenger,
105-
SnapControllerGetSnapFileAction,
106-
origin as SnapId,
107-
),
108-
installSnaps: controllerMessenger.call.bind(
109-
controllerMessenger,
110-
SnapControllerInstallSnapsAction,
111-
origin,
112-
),
113-
invokeSnap: engineContext.PermissionController.executeRestrictedMethod.bind(
114-
engineContext.PermissionController,
115-
origin,
116-
RestrictedMethods.wallet_snap,
117-
),
118-
createInterface: controllerMessenger.call.bind(
119-
controllerMessenger,
120-
SnapInterfaceControllerCreateInterfaceAction,
121-
origin as SnapId,
122-
),
123-
updateInterface: controllerMessenger.call.bind(
124-
controllerMessenger,
125-
SnapInterfaceControllerUpdateInterfaceAction,
126-
origin as SnapId,
127-
),
128-
getInterfaceContext: (id: string) =>
129-
controllerMessenger.call(
130-
'SnapInterfaceController:getInterface',
131-
origin as SnapId,
132-
id,
133-
).context,
134-
getInterfaceState: (id: string) =>
135-
controllerMessenger.call(
136-
'SnapInterfaceController:getInterfaceState',
137-
origin as SnapId,
138-
id,
139-
),
140-
resolveInterface: controllerMessenger.call.bind(
141-
controllerMessenger,
142-
SnapInterfaceControllerResolveInterfaceAction,
143-
origin as SnapId,
144-
),
145-
getSnap: controllerMessenger.call.bind(
146-
controllerMessenger,
147-
SnapControllerGetSnapAction,
148-
),
149-
trackError: (error: Error) => captureException(error),
150-
trackEvent: trackSnapEvent,
151-
openWebSocket: controllerMessenger.call.bind(
152-
controllerMessenger,
153-
WebSocketServiceOpenAction,
154-
origin as SnapId,
155-
),
156-
closeWebSocket: controllerMessenger.call.bind(
157-
controllerMessenger,
158-
WebSocketServiceCloseAction,
159-
origin as SnapId,
160-
),
161-
sendWebSocketMessage: controllerMessenger.call.bind(
162-
controllerMessenger,
163-
WebSocketServiceSendMessageAction,
164-
origin as SnapId,
165-
),
166-
getWebSockets: controllerMessenger.call.bind(
167-
controllerMessenger,
168-
WebSocketServiceGetAllAction,
169-
origin as SnapId,
170-
),
171-
updateInterfaceState: controllerMessenger.call.bind(
172-
controllerMessenger,
173-
SnapInterfaceControllerUpdateInterfaceStateAction,
174-
origin as SnapId,
175-
),
176-
handleSnapRpcRequest: async (request: Omit<SnapRpcHookArgs, 'origin'>) => {
177-
const snapId = getSnapIdFromRequest(request);
178-
179-
if (!snapId) {
180-
throw new Error(
181-
'snapMethodMiddlewareBuilder handleSnapRpcRequest: Invalid snap request: snapId not found',
182-
);
183-
}
184-
185-
return await handleSnapRequest(controllerMessenger, {
186-
snapId,
187-
origin,
188-
handler: request.handler,
189-
request: request.request,
190-
});
191-
},
192-
requestUserApproval:
193-
engineContext.ApprovalController.addAndShowApprovalRequest.bind(
194-
engineContext.ApprovalController,
195-
),
196-
getIsActive: () =>
197-
AppState.currentState === 'active' &&
198-
engineContext.KeyringController.isUnlocked(),
199-
getIsLocked: () => !engineContext.KeyringController.isUnlocked(),
200-
getVersion: () => {
201-
const baseVersion = getVersion();
202-
const buildType = process.env.METAMASK_BUILD_TYPE;
33+
createSnapsMethodMiddleware(
34+
subjectType === SubjectType.Snap,
35+
{
36+
getUnlockPromise: async () => {
37+
if (controllerMessenger.call('KeyringController:isUnlocked')) {
38+
return;
39+
}
40+
await controllerMessenger.waitUntil('KeyringController:unlock');
41+
},
42+
getAllowedKeyringMethods: keyringSnapPermissionsBuilder(origin),
43+
trackError: (error: Error) => captureException(error),
44+
trackEvent: trackSnapEvent,
45+
getIsActive: () =>
46+
AppState.currentState === 'active' &&
47+
controllerMessenger.call('KeyringController:isUnlocked'),
48+
getVersion: () => {
49+
const baseVersion = getVersion();
50+
const buildType = process.env.METAMASK_BUILD_TYPE;
20351

204-
if (buildType === 'main' || buildType === 'qa') {
205-
return baseVersion;
206-
}
52+
if (buildType === 'main' || buildType === 'qa') {
53+
return baseVersion;
54+
}
20755

208-
return `${baseVersion}-${buildType}.0`;
56+
return `${baseVersion}-${buildType}.0`;
57+
},
58+
// @ts-expect-error Type 'string' is not assignable to type 'TraceName'.
59+
startTrace: trace,
60+
// @ts-expect-error Type 'string' is not assignable to type 'TraceName'.
61+
endTrace,
20962
},
210-
getEntropySources: () => {
211-
const state = controllerMessenger.call('KeyringController:getState');
212-
213-
return state.keyrings
214-
.map((keyring, index) => {
215-
if (keyring.type === KeyringTypes.hd) {
216-
return {
217-
id: keyring.metadata.id,
218-
name: keyring.metadata.name,
219-
type: 'mnemonic',
220-
primary: index === 0,
221-
};
222-
}
223-
224-
return null;
225-
})
226-
.filter(Boolean);
227-
},
228-
clearSnapState: controllerMessenger.call.bind(
229-
controllerMessenger,
230-
SnapControllerClearSnapStateAction,
231-
origin as SnapId,
232-
),
233-
getSnapState: controllerMessenger.call.bind(
234-
controllerMessenger,
235-
SnapControllerGetSnapStateAction,
236-
origin as SnapId,
237-
),
238-
updateSnapState: controllerMessenger.call.bind(
239-
controllerMessenger,
240-
SnapControllerUpdateSnapStateAction,
241-
origin as SnapId,
242-
),
243-
scheduleBackgroundEvent: (
244-
event: Parameters<CronjobControllerScheduleAction['handler']>[0],
245-
) =>
246-
controllerMessenger.call('CronjobController:schedule', {
247-
...event,
248-
snapId: origin as SnapId,
249-
}),
250-
cancelBackgroundEvent: controllerMessenger.call.bind(
251-
controllerMessenger,
252-
CronjobControllerCancelAction,
253-
origin as SnapId,
254-
),
255-
getBackgroundEvents: controllerMessenger.call.bind(
256-
controllerMessenger,
257-
CronjobControllerGetAction,
258-
origin as SnapId,
259-
),
260-
getNetworkConfigurationByChainId: controllerMessenger.call.bind(
261-
controllerMessenger,
262-
'NetworkController:getNetworkConfigurationByChainId',
263-
),
264-
getNetworkClientById: controllerMessenger.call.bind(
265-
controllerMessenger,
266-
'NetworkController:getNetworkClientById',
267-
),
268-
startTrace: trace,
269-
endTrace,
270-
});
63+
controllerMessenger,
64+
);
27165

27266
export default snapMethodMiddlewareBuilder;
27367
///: END:ONLY_INCLUDE_IF

0 commit comments

Comments
 (0)