Skip to content

Commit 4fc63db

Browse files
fix: Stop relying on account metadata for MultichainRoutingService (#3974)
Stop relying on `account.metadata.snap.enabled` for `MultichainRoutingService` as it is going away. Instead we can cross-check `getRunnableSnaps`. While doing this fix I also managed to re-use `getSupportedAccountsMetadata` in more cases. https://consensyssoftware.atlassian.net/browse/WPC-988 <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes routing eligibility checks for account Snaps by cross-referencing `SnapController:getRunnableSnaps`, which could impact which accounts/methods are considered routable if Snap/account metadata is inconsistent. Touches request routing and scope support detection, so regressions would affect multichain RPC handling. > > **Overview** > Stops relying on `account.metadata.snap.enabled` (and other snap metadata fields) to decide whether an account Snap can service a request, and instead gates supported accounts by whether their `metadata.snap.id` appears in `SnapController:getRunnableSnaps`. > > Refactors `MultichainRoutingService` to reuse `#getSupportedAccountsMetadata` for both request routing (`#getSnapAccountId`) and `isSupportedScope`, and extends it with an optional `method` filter. > > Updates multichain test fixtures and `InternalAccount` typing to remove the deprecated `metadata.snap.enabled`/`name` fields, and adjusts tests to mock `SnapController:getRunnableSnaps` (including updated call ordering expectations). > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit a5425d0. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 581474c commit 4fc63db

4 files changed

Lines changed: 77 additions & 23 deletions

File tree

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

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,13 @@ describe('MultichainRoutingService', () => {
3737
withSnapKeyring,
3838
});
3939

40+
rootMessenger.registerActionHandler(
41+
'SnapController:getRunnableSnaps',
42+
() => {
43+
return [getTruncatedSnap()];
44+
},
45+
);
46+
4047
rootMessenger.registerActionHandler(
4148
'AccountsController:listMultichainAccounts',
4249
() => MOCK_BTC_ACCOUNTS,
@@ -90,6 +97,13 @@ describe('MultichainRoutingService', () => {
9097
withSnapKeyring,
9198
});
9299

100+
rootMessenger.registerActionHandler(
101+
'SnapController:getRunnableSnaps',
102+
() => {
103+
return [getTruncatedSnap()];
104+
},
105+
);
106+
93107
rootMessenger.registerActionHandler(
94108
'AccountsController:listMultichainAccounts',
95109
() => MOCK_SOLANA_ACCOUNTS,
@@ -142,6 +156,13 @@ describe('MultichainRoutingService', () => {
142156
withSnapKeyring,
143157
});
144158

159+
rootMessenger.registerActionHandler(
160+
'SnapController:getRunnableSnaps',
161+
() => {
162+
return [getTruncatedSnap()];
163+
},
164+
);
165+
145166
rootMessenger.registerActionHandler(
146167
'AccountsController:listMultichainAccounts',
147168
() => MOCK_SOLANA_ACCOUNTS,
@@ -231,7 +252,7 @@ describe('MultichainRoutingService', () => {
231252
});
232253

233254
expect(messenger.call).toHaveBeenNthCalledWith(
234-
5,
255+
6,
235256
'SnapController:handleRequest',
236257
{
237258
snapId: MOCK_SNAP_ID,
@@ -302,6 +323,13 @@ describe('MultichainRoutingService', () => {
302323
withSnapKeyring,
303324
});
304325

326+
rootMessenger.registerActionHandler(
327+
'SnapController:getRunnableSnaps',
328+
() => {
329+
return [getTruncatedSnap()];
330+
},
331+
);
332+
305333
rootMessenger.registerActionHandler(
306334
'SnapController:handleRequest',
307335
async ({ handler }) => {
@@ -352,6 +380,13 @@ describe('MultichainRoutingService', () => {
352380
withSnapKeyring,
353381
});
354382

383+
rootMessenger.registerActionHandler(
384+
'SnapController:getRunnableSnaps',
385+
() => {
386+
return [getTruncatedSnap()];
387+
},
388+
);
389+
355390
rootMessenger.registerActionHandler(
356391
'SnapController:handleRequest',
357392
async ({ handler }) => {
@@ -405,6 +440,13 @@ describe('MultichainRoutingService', () => {
405440
withSnapKeyring,
406441
});
407442

443+
rootMessenger.registerActionHandler(
444+
'SnapController:getRunnableSnaps',
445+
() => {
446+
return [getTruncatedSnap()];
447+
},
448+
);
449+
408450
rootMessenger.registerActionHandler(
409451
'SnapController:handleRequest',
410452
async ({ handler }) => {
@@ -577,6 +619,13 @@ describe('MultichainRoutingService', () => {
577619
() => MOCK_SOLANA_ACCOUNTS,
578620
);
579621

622+
rootMessenger.registerActionHandler(
623+
'SnapController:getRunnableSnaps',
624+
() => {
625+
return [getTruncatedSnap()];
626+
},
627+
);
628+
580629
expect(
581630
messenger.call(
582631
'MultichainRoutingService:getSupportedAccounts',

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

Lines changed: 26 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -166,17 +166,7 @@ export class MultichainRoutingService {
166166
scope: CaipChainId,
167167
request: JsonRpcRequest,
168168
) {
169-
const accounts = this.#messenger
170-
.call('AccountsController:listMultichainAccounts', scope)
171-
.filter(
172-
(
173-
account: InternalAccount,
174-
): account is InternalAccount & {
175-
metadata: Required<InternalAccount['metadata']>;
176-
} =>
177-
Boolean(account.metadata.snap?.enabled) &&
178-
account.methods.includes(request.method),
179-
);
169+
const accounts = this.#getSupportedAccountsMetadata(scope, request.method);
180170

181171
// If no accounts can service the request, return null.
182172
if (accounts.length === 0) {
@@ -349,12 +339,31 @@ export class MultichainRoutingService {
349339
* Get a list of metadata for supported accounts for a given scope from the client.
350340
*
351341
* @param scope - The CAIP-2 scope.
342+
* @param method - An optional method that the account should support.
352343
* @returns A list of metadata for the supported accounts.
353344
*/
354-
#getSupportedAccountsMetadata(scope: CaipChainId): InternalAccount[] {
345+
#getSupportedAccountsMetadata(
346+
scope: CaipChainId,
347+
method?: string,
348+
): (InternalAccount & {
349+
metadata: Required<InternalAccount['metadata']>;
350+
})[] {
351+
const runnableSnaps = this.#messenger
352+
.call('SnapController:getRunnableSnaps')
353+
.map((snap) => snap.id);
354+
355355
return this.#messenger
356356
.call('AccountsController:listMultichainAccounts', scope)
357-
.filter((account: InternalAccount) => account.metadata.snap?.enabled);
357+
.filter(
358+
(
359+
account: InternalAccount,
360+
): account is InternalAccount & {
361+
metadata: Required<InternalAccount['metadata']>;
362+
} =>
363+
account.metadata.snap?.id !== undefined &&
364+
runnableSnaps.includes(account.metadata.snap?.id) &&
365+
(method === undefined || account.methods.includes(method)),
366+
);
358367
}
359368

360369
/**
@@ -395,10 +404,10 @@ export class MultichainRoutingService {
395404
* @returns True if the router can service the scope, otherwise false.
396405
*/
397406
isSupportedScope(scope: CaipChainId): boolean {
398-
const hasAccountSnap = this.#messenger
399-
.call('AccountsController:listMultichainAccounts', scope)
400-
.some((account: InternalAccount) => account.metadata.snap?.enabled);
401407
// We currently assume here that if one Snap exists that service the scope, we can service the scope generally.
402-
return hasAccountSnap || this.#getProtocolSnaps(scope).length > 0;
408+
return (
409+
this.#getSupportedAccountsMetadata(scope).length > 0 ||
410+
this.#getProtocolSnaps(scope).length > 0
411+
);
403412
}
404413
}

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,7 @@ export const MOCK_BTC_ACCOUNTS = [
2222
lastSelected: 1729154128902,
2323
name: 'Bitcoin Account',
2424
snap: {
25-
enabled: true,
2625
id: MOCK_SNAP_ID,
27-
name: 'Bitcoin',
2826
},
2927
},
3028
methods: ['sendBitcoin'],
@@ -50,9 +48,7 @@ export const MOCK_SOLANA_ACCOUNTS = [
5048
lastSelected: 1729154128902,
5149
name: 'Solana Account',
5250
snap: {
53-
enabled: true,
5451
id: MOCK_SNAP_ID,
55-
name: 'Solana',
5652
},
5753
},
5854
methods: ['signAndSendTransaction'],

packages/snaps-utils/src/account.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ export type InternalAccount = {
1515
scopes: CaipChainId[];
1616
metadata: {
1717
name: string;
18-
snap?: { id: SnapId; enabled: boolean; name: string };
18+
snap?: { id: SnapId };
1919
};
2020
};
2121

0 commit comments

Comments
 (0)