Skip to content

Commit 54d632d

Browse files
fix: Manually construct keyring_resolveAccountAddress request (#3822)
Manually construct `keyring_resolveAccountAddress` request to bypass usage of `SnapKeyring`. This is required because the `SnapKeyring` abstraction is only available when the wallet in unlocked. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Switches address resolution in `MultichainRouter` to call `SnapController:handleRequest` with `HandlerType.OnKeyringRequest` and `keyring_resolveAccountAddress`, avoiding dependence on `SnapKeyring` during lock state. > > - Removes `resolveAccountAddress` from `SnapKeyring` and related test mocks/utilities > - Adds result validation (`isObject`/`assert`) and parses returned CAIP account ID > - Updates tests to mock `SnapController:handleRequest` for keyring address resolution and adjust expectations > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit d686e67. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 0278899 commit 54d632d

File tree

3 files changed

+64
-32
lines changed

3 files changed

+64
-32
lines changed

packages/snaps-controllers/src/multichain/MultichainRouter.test.ts

Lines changed: 41 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -230,17 +230,25 @@ describe('MultichainRouter', () => {
230230
it('throws if address resolution fails', async () => {
231231
const rootMessenger = getRootMultichainRouterMessenger();
232232
const messenger = getRestrictedMultichainRouterMessenger(rootMessenger);
233-
const withSnapKeyring = getMockWithSnapKeyring({
234-
// Simulate the Snap returning a bogus address
235-
resolveAccountAddress: jest.fn().mockResolvedValue({ address: 'foo' }),
236-
});
233+
const withSnapKeyring = getMockWithSnapKeyring();
237234

238235
/* eslint-disable-next-line no-new */
239236
new MultichainRouter({
240237
messenger,
241238
withSnapKeyring,
242239
});
243240

241+
rootMessenger.registerActionHandler(
242+
'SnapController:handleRequest',
243+
async ({ handler }) => {
244+
if (handler === HandlerType.OnKeyringRequest) {
245+
// Simulate the Snap returning a bogus address
246+
return { address: 'foo' };
247+
}
248+
throw new Error('Unmocked request');
249+
},
250+
);
251+
244252
rootMessenger.registerActionHandler(
245253
'AccountsController:listMultichainAccounts',
246254
() => MOCK_SOLANA_ACCOUNTS,
@@ -271,20 +279,28 @@ describe('MultichainRouter', () => {
271279
it('throws if address resolution returns an address that isnt available', async () => {
272280
const rootMessenger = getRootMultichainRouterMessenger();
273281
const messenger = getRestrictedMultichainRouterMessenger(rootMessenger);
274-
const withSnapKeyring = getMockWithSnapKeyring({
275-
// Simulate the Snap returning an unconnected address
276-
resolveAccountAddress: jest.fn().mockResolvedValue({
277-
address:
278-
'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp:7S3P4HxJpyyigGzodYwHtCxZyUQe9JiBMHyRWXArAaKa',
279-
}),
280-
});
282+
const withSnapKeyring = getMockWithSnapKeyring();
281283

282284
/* eslint-disable-next-line no-new */
283285
new MultichainRouter({
284286
messenger,
285287
withSnapKeyring,
286288
});
287289

290+
rootMessenger.registerActionHandler(
291+
'SnapController:handleRequest',
292+
async ({ handler }) => {
293+
if (handler === HandlerType.OnKeyringRequest) {
294+
// Simulate the Snap returning an unconnected address
295+
return {
296+
address:
297+
'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp:7S3P4HxJpyyigGzodYwHtCxZyUQe9JiBMHyRWXArAaKa',
298+
};
299+
}
300+
throw new Error('Unmocked request');
301+
},
302+
);
303+
288304
rootMessenger.registerActionHandler(
289305
'AccountsController:listMultichainAccounts',
290306
() => MOCK_SOLANA_ACCOUNTS,
@@ -315,19 +331,27 @@ describe('MultichainRouter', () => {
315331
it(`throws if address resolution returns a lower case address that isn't available`, async () => {
316332
const rootMessenger = getRootMultichainRouterMessenger();
317333
const messenger = getRestrictedMultichainRouterMessenger(rootMessenger);
318-
const withSnapKeyring = getMockWithSnapKeyring({
319-
// Simulate the Snap returning an unconnected address
320-
resolveAccountAddress: jest.fn().mockResolvedValue({
321-
address: `solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp:${MOCK_SOLANA_ACCOUNTS[0].address.toLowerCase()}`,
322-
}),
323-
});
334+
const withSnapKeyring = getMockWithSnapKeyring();
324335

325336
/* eslint-disable-next-line no-new */
326337
new MultichainRouter({
327338
messenger,
328339
withSnapKeyring,
329340
});
330341

342+
rootMessenger.registerActionHandler(
343+
'SnapController:handleRequest',
344+
async ({ handler }) => {
345+
if (handler === HandlerType.OnKeyringRequest) {
346+
// Simulate the Snap returning an unconnected address
347+
return {
348+
address: `solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp:${MOCK_SOLANA_ACCOUNTS[0].address.toLowerCase()}`,
349+
};
350+
}
351+
throw new Error('Unmocked request');
352+
},
353+
);
354+
331355
rootMessenger.registerActionHandler(
332356
'AccountsController:listMultichainAccounts',
333357
() => MOCK_SOLANA_ACCOUNTS,

packages/snaps-controllers/src/multichain/MultichainRouter.ts

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import type {
1616
import {
1717
assert,
1818
hasProperty,
19+
isObject,
1920
KnownCaipNamespace,
2021
parseCaipAccountId,
2122
} from '@metamask/utils';
@@ -52,11 +53,6 @@ type SnapKeyring = {
5253
params?: Json[] | Record<string, Json>;
5354
scope: CaipChainId;
5455
}) => Promise<Json>;
55-
resolveAccountAddress: (
56-
snapId: SnapId,
57-
scope: CaipChainId,
58-
request: Json,
59-
) => Promise<{ address: CaipAccountId } | null>;
6056
};
6157

6258
// Expecting a bound function that calls KeyringController.withKeyring selecting the Snap keyring
@@ -137,7 +133,9 @@ export class MultichainRouter {
137133
/**
138134
* Attempts to resolve the account address to use for a given request by inspecting the request itself.
139135
*
140-
* The request is sent to to an account Snap via the SnapKeyring that will attempt this resolution.
136+
* The request is sent to to an account Snap that will attempt this resolution.
137+
*
138+
* We manually construct the request instead of using the SnapKeyring, as the keyring may not be available.
141139
*
142140
* @param snapId - The ID of the Snap to send the request to.
143141
* @param scope - The CAIP-2 scope for the request.
@@ -151,10 +149,25 @@ export class MultichainRouter {
151149
request: JsonRpcRequest,
152150
) {
153151
try {
154-
const result = await this.#withSnapKeyring(async ({ keyring }) =>
155-
keyring.resolveAccountAddress(snapId, scope, request),
152+
const result = await this.#messenger.call(
153+
'SnapController:handleRequest',
154+
{
155+
snapId,
156+
origin: 'metamask',
157+
request: {
158+
method: 'keyring_resolveAccountAddress',
159+
params: {
160+
request,
161+
scope,
162+
},
163+
},
164+
handler: HandlerType.OnKeyringRequest,
165+
},
156166
);
157-
const address = result?.address;
167+
168+
assert(result === null || isObject(result));
169+
170+
const address = result?.address as CaipAccountId;
158171
return address ? parseCaipAccountId(address).address : null;
159172
} catch {
160173
throw rpcErrors.internal();

packages/snaps-controllers/src/test-utils/multichain.ts

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -94,9 +94,6 @@ export const MOCK_SOLANA_SNAP_PERMISSIONS: Record<
9494

9595
type MockSnapKeyring = {
9696
submitRequest: (request: unknown) => Promise<Json>;
97-
resolveAccountAddress: (
98-
options: unknown,
99-
) => Promise<{ address: CaipAccountId } | null>;
10097
};
10198

10299
type MockOperationCallback = ({
@@ -106,16 +103,14 @@ type MockOperationCallback = ({
106103
}) => Promise<any>;
107104

108105
export const getMockWithSnapKeyring = (
109-
{ submitRequest = jest.fn(), resolveAccountAddress = jest.fn() } = {
106+
{ submitRequest = jest.fn() } = {
110107
submitRequest: jest.fn(),
111-
resolveAccountAddress: jest.fn(),
112108
},
113109
): WithSnapKeyringFunction => {
114110
return async (callback: MockOperationCallback) =>
115111
callback({
116112
keyring: {
117113
submitRequest,
118-
resolveAccountAddress,
119114
},
120115
});
121116
};

0 commit comments

Comments
 (0)