Skip to content

Commit d219a72

Browse files
committed
fix(assets-controller): fix snap-backed multichain balance delivery
Fixes three related bugs that caused snap chain balances (Tron, Bitcoin, Solana, etc.) to never appear in AssetsController state. **Bug 1 — Chain discovery ignored `endowment:keyring` chainIds caveat** `#discoverKeyringSnaps` only read `chainIds` from `endowment:assets`. Snaps that declare supported chains solely on `endowment:keyring` (e.g. many wallet snaps) produced an empty `activeChains` after discovery. Fix: `#getChainIdsFromSnapPermissions` now prefers `endowment:assets` and falls back to `endowment:keyring` when the assets caveat is absent. **Bug 2 — Snap subscriptions never created (activeSubscriptions always 0)** `#subscribeAssetsBalance` in AssetsController passed only `#enabledChains` (EVM networks from NetworkController) to `#buildChainToAccountsMap`. Snap chains (`bip122:*`, `solana:*`, `tron:*`) were never in that set, so `remainingChains` had no overlap with SnapDataSource's `activeChains` → `assignedChains.length === 0` every call → SnapDataSource was immediately unsubscribed → `activeSubscriptions` stayed 0 and every push balance event was dispatched into nothing. Fix: augment the chain set with `snapDataSource.getActiveChainsSync()` so snap chains enter the assignment loop and are routed to SnapDataSource. **Bug 3 — Snaps with no `chainIds` caveat silently dropped (e.g. Tron)** When a snap has `endowment:keyring` but no `chainIds` caveat on either permission, discovery returns `null` and skips the snap. Tron balances arrived with `chainAllowed=false` and were discarded on every event. Fix: `#handleSnapBalancesUpdated` now detects a non-EVM chain that arrived after discovery completed but isn't registered, adds it to `activeChains` via `updateActiveChains`, and triggers `onActiveChainsUpdated` to re-subscribe AssetsController. EVM chains are explicitly excluded from this path — they belong to RpcDataSource. **Additional fixes** - Retry discovery in `#handleSnapBalancesUpdated` when a balance push arrives before `SnapController`/`PermissionController` are ready. - Fail-open in `subscribe` when discovery has not completed: register the subscription using requested chains so push events can be delivered once discovery finishes. - Subscribe to `PermissionController:stateChanged` (correct BaseController v2 event) instead of the deprecated `stateChange`.
1 parent 558518d commit d219a72

4 files changed

Lines changed: 313 additions & 46 deletions

File tree

packages/assets-controller/CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
99

1010
### Changed
1111

12+
- SnapDataSource now subscribes to `PermissionController:stateChange` instead of deprecated `PermissionController:stateChange`. Hosts that restrict or delegate events into the `AssetsController` messenger must delegate `PermissionController:stateChange`; delegating only `stateChange` will prevent permission-driven snap rediscovery. ([#8857](https://github.com/MetaMask/core/pull/8857))
1213
- Bump `@metamask/transaction-controller` from `^65.3.0` to `^66.0.0` ([#8796](https://github.com/MetaMask/core/pull/8796), [#8848](https://github.com/MetaMask/core/pull/8848))
1314
- Bump `@metamask/core-backend` from `^6.2.2` to `^6.3.0` ([#8813](https://github.com/MetaMask/core/pull/8813))
1415
- Bump `@metamask/phishing-controller` from `^17.1.2` to `^17.2.0` ([#8819](https://github.com/MetaMask/core/pull/8819))
@@ -17,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1718

1819
### Fixed
1920

21+
- **SnapDataSource:** Read supported CAIP-2 chain IDs from `endowment:keyring` when `endowment:assets` has no `chainIds` caveat (common for some wallet snaps); retry discovery when a balance push arrives before `activeChains` is populated; accept balance payloads while discovery is still empty so updates are not dropped; register subscriptions in that window so `AccountsController:accountBalancesUpdated` can reach `onAssetsUpdate`. ([#8857](https://github.com/MetaMask/core/pull/8857))
2022
- Non-EVM assets with a `slip44` asset namespace (e.g. Bitcoin, Solana native, TRON) are now correctly typed as `native` instead of `erc20` in `assetsInfo` ([#8811](https://github.com/MetaMask/core/pull/8811))
2123
- Solana SPL tokens (CAIP-19 `solana:.../token:<address>`) are now correctly typed as `spl` instead of `erc20` in `assetsInfo` ([#8811](https://github.com/MetaMask/core/pull/8811))
2224

packages/assets-controller/src/AssetsController.ts

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,7 @@ import type {
3737
NetworkEnablementControllerEvents,
3838
NetworkEnablementControllerState,
3939
} from '@metamask/network-enablement-controller';
40-
import type {
41-
GetPermissions,
42-
PermissionControllerStateChange,
43-
} from '@metamask/permission-controller';
40+
import type { GetPermissions } from '@metamask/permission-controller';
4441
import { PhishingControllerBulkScanTokensAction } from '@metamask/phishing-controller';
4542
import type { PreferencesControllerStateChangeEvent } from '@metamask/preferences-controller';
4643
import type {
@@ -79,7 +76,7 @@ import type { PriceDataSourceConfig } from './data-sources/PriceDataSource';
7976
import { PriceDataSource } from './data-sources/PriceDataSource';
8077
import type { RpcDataSourceConfig } from './data-sources/RpcDataSource';
8178
import { RpcDataSource } from './data-sources/RpcDataSource';
82-
import type { AccountsControllerAccountBalancesUpdatedEvent } from './data-sources/SnapDataSource';
79+
import type { SnapDataSourceAllowedEvents } from './data-sources/SnapDataSource';
8380
import { SnapDataSource } from './data-sources/SnapDataSource';
8481
import type { StakedBalanceDataSourceConfig } from './data-sources/StakedBalanceDataSource';
8582
import { StakedBalanceDataSource } from './data-sources/StakedBalanceDataSource';
@@ -335,8 +332,7 @@ type AllowedEvents =
335332
// StakedBalanceDataSource
336333
| NetworkEnablementControllerEvents
337334
// SnapDataSource
338-
| AccountsControllerAccountBalancesUpdatedEvent
339-
| PermissionControllerStateChange
335+
| SnapDataSourceAllowedEvents
340336
// BackendWebsocketDataSource
341337
| BackendWebSocketServiceEvents;
342338

@@ -2565,9 +2561,13 @@ export class AssetsController extends BaseController<
25652561
accounts: InternalAccount[],
25662562
chainIds: ChainId[],
25672563
): void {
2564+
// Snap data source handles non-EVM chains that are never in #enabledChains.
2565+
// Include its active chains so snap-backed accounts (bip122, solana, tron…)
2566+
// appear in chainToAccounts and receive a subscription.
2567+
const snapActiveChains = this.#snapDataSource.getActiveChainsSync();
25682568
const chainToAccounts = this.#buildChainToAccountsMap(
25692569
accounts,
2570-
new Set(chainIds),
2570+
new Set([...chainIds, ...snapActiveChains]),
25712571
);
25722572
const remainingChains = new Set(chainToAccounts.keys());
25732573
// When basic functionality is on, use all balance data sources; when off, RPC only.

packages/assets-controller/src/data-sources/SnapDataSource.test.ts

Lines changed: 218 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,31 @@ function createMockPermissions(
146146
} as unknown as SubjectPermissions<PermissionConstraint>;
147147
}
148148

149+
/**
150+
* Mock permissions where chainIds exist only on `endowment:keyring` (no assets permission).
151+
*
152+
* @param chainIds - Chain IDs for the caveat.
153+
* @returns Permissions object.
154+
*/
155+
function createMockPermissionsKeyringOnly(
156+
chainIds: ChainId[],
157+
): SubjectPermissions<PermissionConstraint> {
158+
return {
159+
[KEYRING_PERMISSION]: {
160+
id: 'mock-keyring-permission-id',
161+
parentCapability: KEYRING_PERMISSION,
162+
invoker: 'test',
163+
date: Date.now(),
164+
caveats: [
165+
{
166+
type: 'chainIds',
167+
value: chainIds,
168+
},
169+
],
170+
},
171+
} as unknown as SubjectPermissions<PermissionConstraint>;
172+
}
173+
149174
/**
150175
* Creates a mock handler for SnapController:handleRequest
151176
*
@@ -171,13 +196,26 @@ function createMockHandleRequest(
171196

172197
function setupController(
173198
options: {
174-
installedSnaps?: Record<string, { version: string; chainIds?: ChainId[] }>;
199+
installedSnaps?: Record<
200+
string,
201+
{
202+
version: string;
203+
chainIds?: ChainId[];
204+
keyringOnlyChainIds?: ChainId[];
205+
}
206+
>;
175207
accountAssets?: string[];
176208
balances?: Record<string, { amount: string; unit: string }>;
177209
configuredNetworks?: ChainId[];
210+
getRunnableSnapsImplementation?: () => unknown;
178211
} = {},
179212
): SetupResult {
180-
const { installedSnaps = {}, accountAssets = [], balances = {} } = options;
213+
const {
214+
installedSnaps = {},
215+
accountAssets = [],
216+
balances = {},
217+
getRunnableSnapsImplementation,
218+
} = options;
181219

182220
const rootMessenger = new Messenger<MockAnyNamespace, AllActions, AllEvents>({
183221
namespace: MOCK_ANY_NAMESPACE,
@@ -200,7 +238,10 @@ function setupController(
200238
'SnapController:handleRequest',
201239
'PermissionController:getPermissions',
202240
],
203-
events: ['AccountsController:accountBalancesUpdated'],
241+
events: [
242+
'AccountsController:accountBalancesUpdated',
243+
'PermissionController:stateChange',
244+
],
204245
});
205246

206247
const assetsUpdateHandler = jest.fn().mockResolvedValue(undefined);
@@ -218,9 +259,12 @@ function setupController(
218259
);
219260

220261
// Register SnapController action handlers
221-
const mockGetRunnableSnaps = jest
222-
.fn()
223-
.mockReturnValue(snapsForGetRunnableSnaps);
262+
const mockGetRunnableSnaps = jest.fn(
263+
getRunnableSnapsImplementation ??
264+
((): typeof snapsForGetRunnableSnaps => {
265+
return snapsForGetRunnableSnaps;
266+
}),
267+
);
224268
rootMessenger.registerActionHandler(
225269
'SnapController:getRunnableSnaps',
226270
mockGetRunnableSnaps,
@@ -236,9 +280,12 @@ function setupController(
236280
// Returns permissions with chainIds caveat based on installed snaps config
237281
const mockGetPermissions = jest.fn().mockImplementation((snapId: string) => {
238282
const snapConfig = installedSnaps[snapId];
239-
if (snapConfig?.chainIds) {
283+
if (snapConfig?.chainIds?.length) {
240284
return createMockPermissions(snapConfig.chainIds);
241285
}
286+
if (snapConfig?.keyringOnlyChainIds?.length) {
287+
return createMockPermissionsKeyringOnly(snapConfig.keyringOnlyChainIds);
288+
}
242289
return undefined;
243290
});
244291
rootMessenger.registerActionHandler(
@@ -409,6 +456,166 @@ describe('SnapDataSource', () => {
409456
cleanup();
410457
});
411458

459+
it('discovers chain IDs from endowment:keyring when assets permission has no chainIds caveat', async () => {
460+
const { controller, cleanup } = setupController({
461+
installedSnaps: {
462+
[SOLANA_SNAP_ID]: {
463+
version: '1.0.0',
464+
keyringOnlyChainIds: [SOLANA_MAINNET, TRON_MAINNET],
465+
},
466+
},
467+
});
468+
await new Promise(process.nextTick);
469+
470+
const chains = await controller.getActiveChains();
471+
expect(chains).toStrictEqual(
472+
expect.arrayContaining([SOLANA_MAINNET, TRON_MAINNET]),
473+
);
474+
475+
cleanup();
476+
});
477+
478+
it('forwards accountBalancesUpdated while snap discovery has not completed (fail-open)', async () => {
479+
const assetsUpdateHandler = jest.fn().mockResolvedValue(undefined);
480+
const { controller, triggerBalancesUpdated, cleanup } = setupController({
481+
getRunnableSnapsImplementation: () => {
482+
throw new Error('SnapController not ready');
483+
},
484+
});
485+
await new Promise(process.nextTick);
486+
487+
await controller.subscribe({
488+
request: createDataRequest({
489+
chainIds: [SOLANA_MAINNET],
490+
}),
491+
subscriptionId: 'sub-fail-open',
492+
isUpdate: false,
493+
onAssetsUpdate: assetsUpdateHandler,
494+
});
495+
496+
triggerBalancesUpdated({
497+
balances: {
498+
'mock-account-id': {
499+
[MOCK_SOL_ASSET]: { amount: '1', unit: 'SOL' },
500+
},
501+
},
502+
});
503+
504+
expect(assetsUpdateHandler).toHaveBeenCalledTimes(1);
505+
expect(assetsUpdateHandler).toHaveBeenCalledWith(
506+
expect.objectContaining({
507+
updateMode: 'merge',
508+
assetsBalance: {
509+
'mock-account-id': {
510+
[MOCK_SOL_ASSET]: { amount: '1' },
511+
},
512+
},
513+
}),
514+
);
515+
516+
cleanup();
517+
});
518+
519+
it('does not fail-open after discovery completes with no supported chains', async () => {
520+
const assetsUpdateHandler = jest.fn().mockResolvedValue(undefined);
521+
const { controller, triggerBalancesUpdated, cleanup } = setupController({
522+
installedSnaps: {
523+
[SOLANA_SNAP_ID]: { version: '1.0.0' },
524+
},
525+
});
526+
await new Promise(process.nextTick);
527+
528+
await controller.subscribe({
529+
request: createDataRequest({
530+
chainIds: [SOLANA_MAINNET],
531+
}),
532+
subscriptionId: 'sub-no-discovered-chains',
533+
isUpdate: false,
534+
onAssetsUpdate: assetsUpdateHandler,
535+
});
536+
537+
triggerBalancesUpdated({
538+
balances: {
539+
'mock-account-id': {
540+
[MOCK_SOL_ASSET]: { amount: '1', unit: 'SOL' },
541+
},
542+
},
543+
});
544+
545+
await new Promise(process.nextTick);
546+
547+
expect(assetsUpdateHandler).not.toHaveBeenCalled();
548+
549+
cleanup();
550+
});
551+
552+
it('keeps last discovered chains when rediscovery fails', async () => {
553+
const assetsUpdateHandler = jest.fn().mockResolvedValue(undefined);
554+
let shouldFailRediscovery = false;
555+
const { controller, messenger, triggerBalancesUpdated, cleanup } =
556+
setupController({
557+
installedSnaps: {
558+
[SOLANA_SNAP_ID]: { version: '1.0.0', chainIds: [SOLANA_MAINNET] },
559+
},
560+
getRunnableSnapsImplementation: () => {
561+
if (shouldFailRediscovery) {
562+
throw new Error('SnapController not ready');
563+
}
564+
return [
565+
{
566+
id: SOLANA_SNAP_ID,
567+
version: '1.0.0',
568+
enabled: true,
569+
blocked: false,
570+
},
571+
];
572+
},
573+
});
574+
await new Promise(process.nextTick);
575+
576+
expect(await controller.getActiveChains()).toStrictEqual([SOLANA_MAINNET]);
577+
578+
shouldFailRediscovery = true;
579+
messenger.publish('PermissionController:stateChange', { subjects: {} }, []);
580+
581+
expect(await controller.getActiveChains()).toStrictEqual([SOLANA_MAINNET]);
582+
583+
await controller.subscribe({
584+
request: createDataRequest({
585+
chainIds: [SOLANA_MAINNET],
586+
}),
587+
subscriptionId: 'sub-after-failed-rediscovery',
588+
isUpdate: false,
589+
onAssetsUpdate: assetsUpdateHandler,
590+
});
591+
592+
assetsUpdateHandler.mockClear();
593+
triggerBalancesUpdated({
594+
balances: {
595+
'mock-account-id': {
596+
[MOCK_SOL_ASSET]: { amount: '1', unit: 'SOL' },
597+
['eip155:1/slip44:60' as Caip19AssetId]: {
598+
amount: '1',
599+
unit: 'ETH',
600+
},
601+
},
602+
},
603+
});
604+
605+
expect(assetsUpdateHandler).toHaveBeenCalledTimes(1);
606+
expect(assetsUpdateHandler).toHaveBeenCalledWith(
607+
expect.objectContaining({
608+
assetsBalance: {
609+
'mock-account-id': {
610+
[MOCK_SOL_ASSET]: { amount: '1' },
611+
},
612+
},
613+
}),
614+
);
615+
616+
cleanup();
617+
});
618+
412619
it('fetch returns empty response for accounts without snap metadata', async () => {
413620
const { controller, cleanup } = setupController({
414621
installedSnaps: {
@@ -922,7 +1129,10 @@ describe('SnapDataSource', () => {
9221129
'SnapController:handleRequest',
9231130
'PermissionController:getPermissions',
9241131
],
925-
events: ['AccountsController:accountBalancesUpdated'],
1132+
events: [
1133+
'AccountsController:accountBalancesUpdated',
1134+
'PermissionController:stateChange',
1135+
],
9261136
});
9271137
rootMessenger.registerActionHandler(
9281138
'SnapController:getRunnableSnaps',

0 commit comments

Comments
 (0)