From f8b5dc1c1b26b07098b1f3f826517312772dc587 Mon Sep 17 00:00:00 2001 From: juanmigdr Date: Fri, 31 Oct 2025 11:43:52 +0100 Subject: [PATCH 01/16] feat: support adding non-evm tokens --- .../MultichainAssetsController.test.ts | 136 ++++++++++++++++++ .../MultichainAssetsController.ts | 66 ++++++++- .../MultichainBalancesController.test.ts | 90 ++++++++++++ .../MultichainBalancesController.ts | 12 ++ 4 files changed, 303 insertions(+), 1 deletion(-) diff --git a/packages/assets-controllers/src/MultichainAssetsController/MultichainAssetsController.test.ts b/packages/assets-controllers/src/MultichainAssetsController/MultichainAssetsController.test.ts index b9e6af42bb8..756dbd5347d 100644 --- a/packages/assets-controllers/src/MultichainAssetsController/MultichainAssetsController.test.ts +++ b/packages/assets-controllers/src/MultichainAssetsController/MultichainAssetsController.test.ts @@ -921,6 +921,142 @@ describe('MultichainAssetsController', () => { }); }); + describe('addAsset', () => { + it('should add a single asset to account assets list', async () => { + const { controller } = setupController({ + state: { + accountsAssets: { + [mockSolanaAccount.id]: [ + 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/slip44:501', + ], + }, + assetsMetadata: mockGetMetadataReturnValue.assets, + allIgnoredAssets: {}, + } as MultichainAssetsControllerState, + }); + + const assetToAdd = + 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:Gh9ZwEmdLJ8DscKNTkTqPbNwLNNBjuSzaG9Vp2KGtKJr'; + + const result = await controller.addAsset( + assetToAdd, + mockSolanaAccount.id, + ); + + expect(result).toStrictEqual([ + 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/slip44:501', + assetToAdd, + ]); + expect( + controller.state.accountsAssets[mockSolanaAccount.id], + ).toStrictEqual([ + 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/slip44:501', + assetToAdd, + ]); + }); + + it('should not add duplicate assets', async () => { + const existingAsset = + 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/slip44:501'; + const { controller } = setupController({ + state: { + accountsAssets: { + [mockSolanaAccount.id]: [existingAsset], + }, + assetsMetadata: mockGetMetadataReturnValue.assets, + allIgnoredAssets: {}, + } as MultichainAssetsControllerState, + }); + + const result = await controller.addAsset( + existingAsset, + mockSolanaAccount.id, + ); + + expect(result).toStrictEqual([existingAsset]); + expect( + controller.state.accountsAssets[mockSolanaAccount.id], + ).toStrictEqual([existingAsset]); + }); + + it('should remove asset from ignored list when added', async () => { + const assetToAdd = 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/slip44:501'; + const { controller } = setupController({ + state: { + accountsAssets: {}, + assetsMetadata: mockGetMetadataReturnValue.assets, + allIgnoredAssets: { + [mockSolanaAccount.id]: [assetToAdd], + }, + } as MultichainAssetsControllerState, + }); + + const result = await controller.addAsset( + assetToAdd, + mockSolanaAccount.id, + ); + + expect(result).toStrictEqual([assetToAdd]); + expect( + controller.state.accountsAssets[mockSolanaAccount.id], + ).toStrictEqual([assetToAdd]); + expect( + controller.state.allIgnoredAssets[mockSolanaAccount.id], + ).toBeUndefined(); + }); + + it('should handle adding asset to account with no existing assets', async () => { + const assetToAdd = 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/slip44:501'; + const { controller } = setupController({ + state: { + accountsAssets: {}, + assetsMetadata: mockGetMetadataReturnValue.assets, + allIgnoredAssets: {}, + } as MultichainAssetsControllerState, + }); + + const result = await controller.addAsset( + assetToAdd, + mockSolanaAccount.id, + ); + + expect(result).toStrictEqual([assetToAdd]); + expect( + controller.state.accountsAssets[mockSolanaAccount.id], + ).toStrictEqual([assetToAdd]); + }); + + it('should publish accountAssetListUpdated event when asset is added', async () => { + const { controller, messenger } = setupController({ + state: { + accountsAssets: {}, + assetsMetadata: mockGetMetadataReturnValue.assets, + allIgnoredAssets: {}, + } as MultichainAssetsControllerState, + }); + + const assetToAdd = 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/slip44:501'; + + // Set up event listener to capture the published event + const eventListener = jest.fn(); + messenger.subscribe( + 'MultichainAssetsController:accountAssetListUpdated', + eventListener, + ); + + await controller.addAsset(assetToAdd, mockSolanaAccount.id); + + expect(eventListener).toHaveBeenCalledWith({ + assets: { + [mockSolanaAccount.id]: { + added: [assetToAdd], + removed: [], + }, + }, + }); + }); + }); + describe('asset detection with ignored assets', () => { it('should filter out ignored assets when account assets are updated', async () => { const ignoredAsset = 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/slip44:501'; diff --git a/packages/assets-controllers/src/MultichainAssetsController/MultichainAssetsController.ts b/packages/assets-controllers/src/MultichainAssetsController/MultichainAssetsController.ts index 6f05f676f59..775f38addce 100644 --- a/packages/assets-controllers/src/MultichainAssetsController/MultichainAssetsController.ts +++ b/packages/assets-controllers/src/MultichainAssetsController/MultichainAssetsController.ts @@ -85,6 +85,11 @@ export type MultichainAssetsControllerIgnoreAssetsAction = { handler: MultichainAssetsController['ignoreAssets']; }; +export type MultichainAssetsControllerAddAssetAction = { + type: `${typeof controllerName}:addAsset`; + handler: MultichainAssetsController['addAsset']; +}; + /** * Returns the state of the {@link MultichainAssetsController}. */ @@ -108,7 +113,8 @@ export type MultichainAssetsControllerStateChangeEvent = export type MultichainAssetsControllerActions = | MultichainAssetsControllerGetStateAction | MultichainAssetsControllerGetAssetMetadataAction - | MultichainAssetsControllerIgnoreAssetsAction; + | MultichainAssetsControllerIgnoreAssetsAction + | MultichainAssetsControllerAddAssetAction; /** * Events emitted by {@link MultichainAssetsController}. @@ -259,6 +265,11 @@ export class MultichainAssetsController extends BaseController< 'MultichainAssetsController:ignoreAssets', this.ignoreAssets.bind(this), ); + + this.messenger.registerActionHandler( + 'MultichainAssetsController:addAsset', + this.addAsset.bind(this), + ); } /** @@ -296,6 +307,59 @@ export class MultichainAssetsController extends BaseController< }); } + /** + * Adds a single asset to the stored asset list for a specific account. + * + * @param assetId - The CAIP asset ID to add. + * @param accountId - The account ID to add the asset to. + * @returns The updated asset list for the account. + */ + async addAsset( + assetId: CaipAssetType, + accountId: string, + ): Promise { + return this.#withControllerLock(async () => { + // Refresh metadata for the asset + await this.#refreshAssetsMetadata([assetId]); + + this.update((state) => { + // Initialize account assets if it doesn't exist + if (!state.accountsAssets[accountId]) { + state.accountsAssets[accountId] = []; + } + + // Add asset if it doesn't already exist + if (!state.accountsAssets[accountId].includes(assetId)) { + state.accountsAssets[accountId].push(assetId); + } + + // Remove from ignored list if it exists there (inline logic like EVM) + if (state.allIgnoredAssets[accountId]) { + state.allIgnoredAssets[accountId] = state.allIgnoredAssets[ + accountId + ].filter((asset) => asset !== assetId); + + // Clean up empty arrays + if (state.allIgnoredAssets[accountId].length === 0) { + delete state.allIgnoredAssets[accountId]; + } + } + }); + + // Publish event to notify other controllers (balances, rates) about the new asset + this.messenger.publish(`${controllerName}:accountAssetListUpdated`, { + assets: { + [accountId]: { + added: [assetId], + removed: [], + }, + }, + }); + + return this.state.accountsAssets[accountId] || []; + }); + } + /** * Checks if an asset is ignored for a specific account. * diff --git a/packages/assets-controllers/src/MultichainBalancesController/MultichainBalancesController.test.ts b/packages/assets-controllers/src/MultichainBalancesController/MultichainBalancesController.test.ts index 9c3ec74912d..9315586c59f 100644 --- a/packages/assets-controllers/src/MultichainBalancesController/MultichainBalancesController.test.ts +++ b/packages/assets-controllers/src/MultichainBalancesController/MultichainBalancesController.test.ts @@ -643,6 +643,96 @@ describe('MultichainBalancesController', () => { }, }); }); + + it('sets balance to zero for assets that were added but have no balance from snap', async () => { + const mockSolanaAccountId1 = mockListSolanaAccounts[0].id; + + const existingBalancesState = { + [mockSolanaAccountId1]: { + 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:existingToken': { + amount: '5.00000000', + unit: 'SOL', + }, + }, + }; + + const { + controller, + messenger, + mockSnapHandleRequest, + mockListMultichainAccounts, + } = setupController({ + state: { + balances: existingBalancesState, + }, + mocks: { + handleMockGetAssetsState: { + accountsAssets: { + [mockSolanaAccountId1]: [ + 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:existingToken', + ], + }, + }, + handleRequestReturnValue: { + 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:existingToken': { + amount: '5.00000000', + unit: 'SOL', + }, + }, + listMultichainAccounts: [mockListSolanaAccounts[0]], + }, + }); + + mockSnapHandleRequest.mockReset(); + mockListMultichainAccounts.mockReset(); + + mockListMultichainAccounts.mockReturnValue(mockListSolanaAccounts); + + // Mock snap returning balance for only one asset, not the newly added ones + mockSnapHandleRequest.mockResolvedValueOnce({ + 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:newTokenWithBalance': { + amount: '1.00000000', + unit: 'SOL', + }, + // Note: newTokenWithoutBalance is not returned by snap, so it should get 0 balance + }); + + // Simulate adding assets where some have balance and some don't + messenger.publish('MultichainAssetsController:accountAssetListUpdated', { + assets: { + [mockSolanaAccountId1]: { + added: [ + 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:newTokenWithBalance', + 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:newTokenWithoutBalance', + ], + removed: [], + }, + }, + }); + + await waitForAllPromises(); + + expect(controller.state.balances).toStrictEqual({ + [mockSolanaAccountId1]: { + // Existing balance should remain + 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:existingToken': { + amount: '5.00000000', + unit: 'SOL', + }, + // New asset with balance from snap + 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:newTokenWithBalance': { + amount: '1.00000000', + unit: 'SOL', + }, + // New asset without balance from snap should get zero balance + 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:newTokenWithoutBalance': + { + amount: '0', + unit: '', + }, + }, + }); + }); }); it('resumes updating balances after unlocking KeyringController', async () => { diff --git a/packages/assets-controllers/src/MultichainBalancesController/MultichainBalancesController.ts b/packages/assets-controllers/src/MultichainBalancesController/MultichainBalancesController.ts index 892f0b06079..911a46f4db9 100644 --- a/packages/assets-controllers/src/MultichainBalancesController/MultichainBalancesController.ts +++ b/packages/assets-controllers/src/MultichainBalancesController/MultichainBalancesController.ts @@ -223,6 +223,8 @@ export class MultichainBalancesController extends BaseController< return; } + const accountsMap = new Map(accounts.map((acc) => [acc.accountId, acc])); + this.update((state: Draft) => { for (const [accountId, accountBalances] of Object.entries( balancesToUpdate, @@ -233,11 +235,21 @@ export class MultichainBalancesController extends BaseController< ) { state.balances[accountId] = accountBalances; } else { + const acc = accountsMap.get(accountId); + + const assetsWithoutBalance = new Set(acc?.assets || []); + for (const assetId in accountBalances) { if (!state.balances[accountId][assetId]) { state.balances[accountId][assetId] = accountBalances[assetId]; + assetsWithoutBalance.delete(assetId as CaipAssetType); } } + + // If an asset is added and has 0 balance add it to the balances (triggered when addAsset action is called) + for (const assetId of assetsWithoutBalance) { + state.balances[accountId][assetId] = { amount: '0', unit: '' }; + } } } }); From c5510a867618a44466f29ebde6bcc022840d702c Mon Sep 17 00:00:00 2001 From: juanmigdr Date: Fri, 31 Oct 2025 11:47:26 +0100 Subject: [PATCH 02/16] chore: updated comment --- .../MultichainBalancesController.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/assets-controllers/src/MultichainBalancesController/MultichainBalancesController.ts b/packages/assets-controllers/src/MultichainBalancesController/MultichainBalancesController.ts index 911a46f4db9..32ee1934521 100644 --- a/packages/assets-controllers/src/MultichainBalancesController/MultichainBalancesController.ts +++ b/packages/assets-controllers/src/MultichainBalancesController/MultichainBalancesController.ts @@ -246,7 +246,7 @@ export class MultichainBalancesController extends BaseController< } } - // If an asset is added and has 0 balance add it to the balances (triggered when addAsset action is called) + // Triggered when an asset is added to the accountAssets list manually for (const assetId of assetsWithoutBalance) { state.balances[accountId][assetId] = { amount: '0', unit: '' }; } From e4a61947bd1269db7e97d4622918c4d78a9370b0 Mon Sep 17 00:00:00 2001 From: juanmigdr Date: Fri, 31 Oct 2025 11:49:26 +0100 Subject: [PATCH 03/16] chore: updated changelog --- packages/assets-controllers/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/assets-controllers/CHANGELOG.md b/packages/assets-controllers/CHANGELOG.md index 40415f1b2c4..b87102508c1 100644 --- a/packages/assets-controllers/CHANGELOG.md +++ b/packages/assets-controllers/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Added `addAsset` to allow adding assets for non-EVM chains ([#7016](https://github.com/MetaMask/core/pull/7016)) + ## [86.0.0] ### Changed From 7dbcd10898e5405db5b37f7b651a8f2ec853459e Mon Sep 17 00:00:00 2001 From: juanmigdr Date: Mon, 3 Nov 2025 13:38:16 +0100 Subject: [PATCH 04/16] chore: updated function to accept multiple assets --- .../MultichainAssetsController.test.ts | 131 ++++++++++++++++-- .../MultichainAssetsController.ts | 75 ++++++---- 2 files changed, 170 insertions(+), 36 deletions(-) diff --git a/packages/assets-controllers/src/MultichainAssetsController/MultichainAssetsController.test.ts b/packages/assets-controllers/src/MultichainAssetsController/MultichainAssetsController.test.ts index 756dbd5347d..1d4498f3242 100644 --- a/packages/assets-controllers/src/MultichainAssetsController/MultichainAssetsController.test.ts +++ b/packages/assets-controllers/src/MultichainAssetsController/MultichainAssetsController.test.ts @@ -1,6 +1,7 @@ import { deriveStateFromMetadata } from '@metamask/base-controller'; import type { AccountAssetListUpdatedEventPayload, + CaipAssetType, CaipAssetTypeOrId, } from '@metamask/keyring-api'; import { @@ -921,7 +922,7 @@ describe('MultichainAssetsController', () => { }); }); - describe('addAsset', () => { + describe('addAssets', () => { it('should add a single asset to account assets list', async () => { const { controller } = setupController({ state: { @@ -938,8 +939,8 @@ describe('MultichainAssetsController', () => { const assetToAdd = 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:Gh9ZwEmdLJ8DscKNTkTqPbNwLNNBjuSzaG9Vp2KGtKJr'; - const result = await controller.addAsset( - assetToAdd, + const result = await controller.addAssets( + [assetToAdd], mockSolanaAccount.id, ); @@ -968,8 +969,8 @@ describe('MultichainAssetsController', () => { } as MultichainAssetsControllerState, }); - const result = await controller.addAsset( - existingAsset, + const result = await controller.addAssets( + [existingAsset], mockSolanaAccount.id, ); @@ -991,8 +992,8 @@ describe('MultichainAssetsController', () => { } as MultichainAssetsControllerState, }); - const result = await controller.addAsset( - assetToAdd, + const result = await controller.addAssets( + [assetToAdd], mockSolanaAccount.id, ); @@ -1015,8 +1016,8 @@ describe('MultichainAssetsController', () => { } as MultichainAssetsControllerState, }); - const result = await controller.addAsset( - assetToAdd, + const result = await controller.addAssets( + [assetToAdd], mockSolanaAccount.id, ); @@ -1044,7 +1045,7 @@ describe('MultichainAssetsController', () => { eventListener, ); - await controller.addAsset(assetToAdd, mockSolanaAccount.id); + await controller.addAssets([assetToAdd], mockSolanaAccount.id); expect(eventListener).toHaveBeenCalledWith({ assets: { @@ -1055,6 +1056,116 @@ describe('MultichainAssetsController', () => { }, }); }); + + it('should add multiple assets from the same chain', async () => { + const { controller } = setupController({ + state: { + accountsAssets: { + [mockSolanaAccount.id]: [ + 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/slip44:501', + ], + }, + assetsMetadata: mockGetMetadataReturnValue.assets, + allIgnoredAssets: {}, + } as MultichainAssetsControllerState, + }); + + const assetsToAdd: CaipAssetType[] = [ + 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:Gh9ZwEmdLJ8DscKNTkTqPbNwLNNBjuSzaG9Vp2KGtKJr', + 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:AnotherTokenAddress', + ]; + + const result = await controller.addAssets( + assetsToAdd, + mockSolanaAccount.id, + ); + + expect(result).toStrictEqual([ + 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/slip44:501', + ...assetsToAdd, + ]); + expect( + controller.state.accountsAssets[mockSolanaAccount.id], + ).toStrictEqual([ + 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/slip44:501', + ...assetsToAdd, + ]); + }); + + it('should throw error when assets are from different chains', async () => { + const { controller } = setupController({ + state: { + accountsAssets: {}, + assetsMetadata: mockGetMetadataReturnValue.assets, + allIgnoredAssets: {}, + } as MultichainAssetsControllerState, + }); + + const assetsFromDifferentChains: CaipAssetType[] = [ + 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/slip44:501', + 'eip155:1/slip44:60', // Ethereum asset + ]; + + await expect( + controller.addAssets(assetsFromDifferentChains, mockSolanaAccount.id), + ).rejects.toThrow( + 'All assets must belong to the same chain. Found assets from chains: solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1, eip155:1', + ); + }); + + it('should return existing assets when empty array is provided', async () => { + const existingAsset = + 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/slip44:501'; + const { controller } = setupController({ + state: { + accountsAssets: { + [mockSolanaAccount.id]: [existingAsset], + }, + assetsMetadata: mockGetMetadataReturnValue.assets, + allIgnoredAssets: {}, + } as MultichainAssetsControllerState, + }); + + const result = await controller.addAssets([], mockSolanaAccount.id); + + expect(result).toStrictEqual([existingAsset]); + }); + + it('should only publish event for newly added assets', async () => { + const existingAsset = + 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/slip44:501'; + const newAsset = 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:NewToken'; + + const { controller, messenger } = setupController({ + state: { + accountsAssets: { + [mockSolanaAccount.id]: [existingAsset], + }, + assetsMetadata: mockGetMetadataReturnValue.assets, + allIgnoredAssets: {}, + } as MultichainAssetsControllerState, + }); + + const eventListener = jest.fn(); + messenger.subscribe( + 'MultichainAssetsController:accountAssetListUpdated', + eventListener, + ); + + await controller.addAssets( + [existingAsset, newAsset], + mockSolanaAccount.id, + ); + + expect(eventListener).toHaveBeenCalledWith({ + assets: { + [mockSolanaAccount.id]: { + added: [newAsset], // Only the new asset should be in the event + removed: [], + }, + }, + }); + }); }); describe('asset detection with ignored assets', () => { diff --git a/packages/assets-controllers/src/MultichainAssetsController/MultichainAssetsController.ts b/packages/assets-controllers/src/MultichainAssetsController/MultichainAssetsController.ts index 775f38addce..a6638ab2a5c 100644 --- a/packages/assets-controllers/src/MultichainAssetsController/MultichainAssetsController.ts +++ b/packages/assets-controllers/src/MultichainAssetsController/MultichainAssetsController.ts @@ -85,9 +85,9 @@ export type MultichainAssetsControllerIgnoreAssetsAction = { handler: MultichainAssetsController['ignoreAssets']; }; -export type MultichainAssetsControllerAddAssetAction = { - type: `${typeof controllerName}:addAsset`; - handler: MultichainAssetsController['addAsset']; +export type MultichainAssetsControllerAddAssetsAction = { + type: `${typeof controllerName}:addAssets`; + handler: MultichainAssetsController['addAssets']; }; /** @@ -114,7 +114,7 @@ export type MultichainAssetsControllerActions = | MultichainAssetsControllerGetStateAction | MultichainAssetsControllerGetAssetMetadataAction | MultichainAssetsControllerIgnoreAssetsAction - | MultichainAssetsControllerAddAssetAction; + | MultichainAssetsControllerAddAssetsAction; /** * Events emitted by {@link MultichainAssetsController}. @@ -267,8 +267,8 @@ export class MultichainAssetsController extends BaseController< ); this.messenger.registerActionHandler( - 'MultichainAssetsController:addAsset', - this.addAsset.bind(this), + 'MultichainAssetsController:addAssets', + this.addAssets.bind(this), ); } @@ -308,19 +308,37 @@ export class MultichainAssetsController extends BaseController< } /** - * Adds a single asset to the stored asset list for a specific account. + * Adds multiple assets to the stored asset list for a specific account. + * All assets must belong to the same chain. * - * @param assetId - The CAIP asset ID to add. - * @param accountId - The account ID to add the asset to. + * @param assetIds - Array of CAIP asset IDs to add (must be from same chain). + * @param accountId - The account ID to add the assets to. * @returns The updated asset list for the account. + * @throws Error if assets are from different chains. */ - async addAsset( - assetId: CaipAssetType, + async addAssets( + assetIds: CaipAssetType[], accountId: string, ): Promise { + if (assetIds.length === 0) { + return this.state.accountsAssets[accountId] || []; + } + + // Validate that all assets are from the same chain + const chainIds = new Set( + assetIds.map((assetId) => parseCaipAssetType(assetId).chainId), + ); + if (chainIds.size > 1) { + throw new Error( + `All assets must belong to the same chain. Found assets from chains: ${Array.from(chainIds).join(', ')}`, + ); + } + return this.#withControllerLock(async () => { - // Refresh metadata for the asset - await this.#refreshAssetsMetadata([assetId]); + // Refresh metadata for all assets + await this.#refreshAssetsMetadata(assetIds); + + const addedAssets: CaipAssetType[] = []; this.update((state) => { // Initialize account assets if it doesn't exist @@ -328,16 +346,19 @@ export class MultichainAssetsController extends BaseController< state.accountsAssets[accountId] = []; } - // Add asset if it doesn't already exist - if (!state.accountsAssets[accountId].includes(assetId)) { - state.accountsAssets[accountId].push(assetId); + // Add assets if they don't already exist + for (const assetId of assetIds) { + if (!state.accountsAssets[accountId].includes(assetId)) { + state.accountsAssets[accountId].push(assetId); + addedAssets.push(assetId); + } } - // Remove from ignored list if it exists there (inline logic like EVM) + // Remove from ignored list if they exist there (inline logic like EVM) if (state.allIgnoredAssets[accountId]) { state.allIgnoredAssets[accountId] = state.allIgnoredAssets[ accountId - ].filter((asset) => asset !== assetId); + ].filter((asset) => !assetIds.includes(asset)); // Clean up empty arrays if (state.allIgnoredAssets[accountId].length === 0) { @@ -346,15 +367,17 @@ export class MultichainAssetsController extends BaseController< } }); - // Publish event to notify other controllers (balances, rates) about the new asset - this.messenger.publish(`${controllerName}:accountAssetListUpdated`, { - assets: { - [accountId]: { - added: [assetId], - removed: [], + // Publish event to notify other controllers (balances, rates) about the new assets + if (addedAssets.length > 0) { + this.messenger.publish(`${controllerName}:accountAssetListUpdated`, { + assets: { + [accountId]: { + added: addedAssets, + removed: [], + }, }, - }, - }); + }); + } return this.state.accountsAssets[accountId] || []; }); From 28f910853e2d12c4a95da77c55c3c5a120e778b3 Mon Sep 17 00:00:00 2001 From: juanmigdr Date: Mon, 3 Nov 2025 13:40:56 +0100 Subject: [PATCH 05/16] chore: update changelog --- packages/assets-controllers/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/assets-controllers/CHANGELOG.md b/packages/assets-controllers/CHANGELOG.md index b87102508c1..137a0d2bf0b 100644 --- a/packages/assets-controllers/CHANGELOG.md +++ b/packages/assets-controllers/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- Added `addAsset` to allow adding assets for non-EVM chains ([#7016](https://github.com/MetaMask/core/pull/7016)) +- Added `addAssets` to allow adding multiple assets for non-EVM chains ([#7016](https://github.com/MetaMask/core/pull/7016)) ## [86.0.0] From 7887a129dbc620b798ec9a6eb4142d4330108f73 Mon Sep 17 00:00:00 2001 From: juanmigdr Date: Mon, 3 Nov 2025 17:54:03 +0100 Subject: [PATCH 06/16] fix: token is not removed from ignored tokens list when added back due to case insensiteivity --- packages/assets-controllers/src/TokensController.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/assets-controllers/src/TokensController.ts b/packages/assets-controllers/src/TokensController.ts index b5350c60979..686340c0394 100644 --- a/packages/assets-controllers/src/TokensController.ts +++ b/packages/assets-controllers/src/TokensController.ts @@ -534,7 +534,7 @@ export class TokensController extends BaseController< aggregators, name, }; - newTokensMap[address] = formattedToken; + newTokensMap[checksumAddress] = formattedToken; importedTokensMap[address.toLowerCase()] = true; return formattedToken; }); @@ -542,7 +542,9 @@ export class TokensController extends BaseController< const newIgnoredTokens = allIgnoredTokens[interactingChainId]?.[ this.#getSelectedAddress() - ]?.filter((tokenAddress) => !newTokensMap[tokenAddress.toLowerCase()]); + ]?.filter( + (tokenAddress) => !newTokensMap[toChecksumHexAddress(tokenAddress)], + ); const detectedTokensForGivenChain = interactingChainId ? allDetectedTokens?.[interactingChainId]?.[this.#getSelectedAddress()] From 9b1aac19d34c4832f4822c2f45648fff8e6b3e58 Mon Sep 17 00:00:00 2001 From: Mathieu Artu Date: Mon, 3 Nov 2025 15:21:58 +0100 Subject: [PATCH 07/16] feat(multichain-account-service): add per-provider throttling for non-EVM account creation (#7000) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Explanation ## References Related to [MUL-1222](https://consensyssoftware.atlassian.net/browse/MUL-1222), [MUL-1221](https://consensyssoftware.atlassian.net/browse/MUL-1221) ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [x] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/contributing.md#updating-changelogs), highlighting breaking changes as necessary - [ ] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes [MUL-1222]: https://consensyssoftware.atlassian.net/browse/MUL-1222?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ --- > [!NOTE] > Adds per-provider concurrency limits for non-EVM account creation (Solana defaults to 3), refactors creation flow to optionally await/aggregate errors, and updates providers/tests accordingly. > > - **Core** > - Introduce `withMaxConcurrency` in `SnapAccountProvider` using a semaphore; add optional `maxConcurrency` to `SnapAccountProviderConfig`. > - Add `toRejectedErrorMessage` util and factor non‑EVM creation into `MultichainAccountWallet.#createNonEvmAccounts`, supporting await-all with aggregated errors and background mode logging. > - Forward optional `providerConfigs` safely (`?.`) and allow Solana-specific config (including `maxConcurrency`). > - **Providers** > - Update `SolAccountProvider`, `BtcAccountProvider`, and `TrxAccountProvider` to use `SnapAccountProviderConfig` and throttle `createAccounts` via `withMaxConcurrency`. > - Default Solana `maxConcurrency: 3`; others unthrottled by default. > - **Tests** > - Add concurrency tests for `SnapAccountProvider`. > - Update wallet tests to verify aggregated non‑EVM failures when awaiting all and provider config forwarding. > - **Docs** > - Update `CHANGELOG.md` with new throttling behavior and defaults. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 6e2148641ba44deabd1a26b79d466ce4efb6a391. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). [MUL-1221]: https://consensyssoftware.atlassian.net/browse/MUL-1221?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ --- .../multichain-account-service/CHANGELOG.md | 6 + .../src/MultichainAccountService.test.ts | 8 +- .../src/MultichainAccountService.ts | 6 +- .../src/MultichainAccountWallet.test.ts | 47 ++++++ .../src/MultichainAccountWallet.ts | 151 ++++++++++------- .../src/providers/BtcAccountProvider.ts | 57 +++---- .../src/providers/EvmAccountProvider.ts | 2 +- .../src/providers/SnapAccountProvider.test.ts | 153 +++++++++++++++++- .../src/providers/SnapAccountProvider.ts | 53 +++++- .../src/providers/SolAccountProvider.ts | 48 +++--- .../src/providers/TrxAccountProvider.ts | 59 +++---- .../multichain-account-service/src/utils.ts | 12 ++ 12 files changed, 440 insertions(+), 162 deletions(-) create mode 100644 packages/multichain-account-service/src/utils.ts diff --git a/packages/multichain-account-service/CHANGELOG.md b/packages/multichain-account-service/CHANGELOG.md index 91b96b4055b..edba796d74e 100644 --- a/packages/multichain-account-service/CHANGELOG.md +++ b/packages/multichain-account-service/CHANGELOG.md @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add per-provider throttling for non-EVM account creation to improve performance on low-end devices ([#7000](https://github.com/MetaMask/core/pull/7000)) + - Solana provider is now limited to 3 concurrent account creations by default when creating multichain account groups. + - Other providers remain unthrottled by default. + ## [2.0.1] ### Fixed diff --git a/packages/multichain-account-service/src/MultichainAccountService.test.ts b/packages/multichain-account-service/src/MultichainAccountService.test.ts index 424b5f49805..144d709033e 100644 --- a/packages/multichain-account-service/src/MultichainAccountService.test.ts +++ b/packages/multichain-account-service/src/MultichainAccountService.test.ts @@ -200,6 +200,7 @@ describe('MultichainAccountService', () => { }, }, [SOL_ACCOUNT_PROVIDER_NAME]: { + maxConcurrency: 3, discovery: { timeoutMs: 5000, maxAttempts: 4, @@ -218,11 +219,11 @@ describe('MultichainAccountService', () => { expect(mocks.EvmAccountProvider.constructor).toHaveBeenCalledWith( messenger, - providerConfigs[EvmAccountProvider.NAME], + providerConfigs?.[EvmAccountProvider.NAME], ); expect(mocks.SolAccountProvider.constructor).toHaveBeenCalledWith( messenger, - providerConfigs[SolAccountProvider.NAME], + providerConfigs?.[SolAccountProvider.NAME], ); }); @@ -232,6 +233,7 @@ describe('MultichainAccountService', () => { // NOTE: We use constants here, since `*AccountProvider` are mocked, thus, their `.NAME` will // be `undefined`. [SOL_ACCOUNT_PROVIDER_NAME]: { + maxConcurrency: 3, discovery: { timeoutMs: 5000, maxAttempts: 4, @@ -255,7 +257,7 @@ describe('MultichainAccountService', () => { ); expect(mocks.SolAccountProvider.constructor).toHaveBeenCalledWith( messenger, - providerConfigs[SolAccountProvider.NAME], + providerConfigs?.[SolAccountProvider.NAME], ); }); }); diff --git a/packages/multichain-account-service/src/MultichainAccountService.ts b/packages/multichain-account-service/src/MultichainAccountService.ts index 776e057b139..b628ab0558c 100644 --- a/packages/multichain-account-service/src/MultichainAccountService.ts +++ b/packages/multichain-account-service/src/MultichainAccountService.ts @@ -18,14 +18,16 @@ import { MultichainAccountWallet } from './MultichainAccountWallet'; import type { EvmAccountProviderConfig, NamedAccountProvider, - SolAccountProviderConfig, } from './providers'; import { AccountProviderWrapper, isAccountProviderWrapper, } from './providers/AccountProviderWrapper'; import { EvmAccountProvider } from './providers/EvmAccountProvider'; -import { SolAccountProvider } from './providers/SolAccountProvider'; +import { + SolAccountProvider, + type SolAccountProviderConfig, +} from './providers/SolAccountProvider'; import type { MultichainAccountServiceMessenger } from './types'; export const serviceName = 'MultichainAccountService'; diff --git a/packages/multichain-account-service/src/MultichainAccountWallet.test.ts b/packages/multichain-account-service/src/MultichainAccountWallet.test.ts index d87a2361881..40dde775ad7 100644 --- a/packages/multichain-account-service/src/MultichainAccountWallet.test.ts +++ b/packages/multichain-account-service/src/MultichainAccountWallet.test.ts @@ -364,6 +364,53 @@ describe('MultichainAccountWallet', () => { 'Unable to create multichain account group for index: 1', ); }); + + it('aggregates non-EVM failures when waiting for all providers', async () => { + const startingIndex = 0; + + const mockEvmAccount = MockAccountBuilder.from(MOCK_HD_ACCOUNT_1) + .withEntropySource(MOCK_HD_KEYRING_1.metadata.id) + .withGroupIndex(startingIndex) + .get(); + + const { wallet, providers } = setup({ + providers: [ + setupNamedAccountProvider({ accounts: [mockEvmAccount], index: 0 }), + setupNamedAccountProvider({ + name: 'Non-EVM Provider', + accounts: [], + index: 1, + }), + ], + }); + + const nextIndex = 1; + const nextEvmAccount = MockAccountBuilder.from(mockEvmAccount) + .withGroupIndex(nextIndex) + .get(); + + const [evmProvider, solProvider] = providers; + evmProvider.createAccounts.mockResolvedValueOnce([nextEvmAccount]); + evmProvider.getAccounts.mockReturnValueOnce([nextEvmAccount]); + evmProvider.getAccount.mockReturnValueOnce(nextEvmAccount); + + const warnSpy = jest.spyOn(console, 'warn').mockImplementation(); + + const SOL_PROVIDER_ERROR = 'SOL create failed'; + solProvider.createAccounts.mockRejectedValueOnce( + new Error(SOL_PROVIDER_ERROR), + ); + + await expect( + wallet.createMultichainAccountGroup(nextIndex, { + waitForAllProvidersToFinishCreatingAccounts: true, + }), + ).rejects.toThrow( + `Unable to create multichain account group for index: ${nextIndex}:\n- Error: ${SOL_PROVIDER_ERROR}`, + ); + + expect(warnSpy).toHaveBeenCalled(); + }); }); describe('createNextMultichainAccountGroup', () => { diff --git a/packages/multichain-account-service/src/MultichainAccountWallet.ts b/packages/multichain-account-service/src/MultichainAccountWallet.ts index f90c0a73694..b1d6b3df96a 100644 --- a/packages/multichain-account-service/src/MultichainAccountWallet.ts +++ b/packages/multichain-account-service/src/MultichainAccountWallet.ts @@ -26,6 +26,7 @@ import { import { MultichainAccountGroup } from './MultichainAccountGroup'; import { EvmAccountProvider, type NamedAccountProvider } from './providers'; import type { MultichainAccountServiceMessenger } from './types'; +import { toRejectedErrorMessage } from './utils'; /** * The context for a provider discovery. @@ -220,6 +221,65 @@ export class MultichainAccountWallet< } } + /** + * Create accounts with non‑EVM providers. Optional throttling is managed by each provider internally. + * When awaitAll is true, waits for all providers and throws if any failed. + * When false, starts work in background and logs errors without throwing. + * + * @param options - Method options. + * @param options.groupIndex - The group index to create accounts for. + * @param options.providers - The non‑EVM account providers. + * @param options.awaitAll - Whether to wait for all providers to finish. + * @throws If awaitAll is true and any provider fails to create accounts. + * @returns A promise that resolves when done (if awaitAll is true) or immediately (if false). + */ + async #createNonEvmAccounts({ + groupIndex, + providers, + awaitAll, + }: { + groupIndex: number; + providers: NamedAccountProvider[]; + awaitAll: boolean; + }): Promise { + if (awaitAll) { + const tasks = providers.map((provider) => + provider.createAccounts({ + entropySource: this.#entropySource, + groupIndex, + }), + ); + + const results = await Promise.allSettled(tasks); + if (results.some((r) => r.status === 'rejected')) { + const errorMessage = toRejectedErrorMessage( + `Unable to create multichain account group for index: ${groupIndex}`, + results, + ); + + this.#log(`${WARNING_PREFIX} ${errorMessage}`); + console.warn(errorMessage); + throw new Error(errorMessage); + } + return; + } + + // Background mode: start tasks and log errors. + // Optional throttling is handled internally by each provider based on its config. + providers.forEach((provider) => { + // eslint-disable-next-line no-void + void provider + .createAccounts({ + entropySource: this.#entropySource, + groupIndex, + }) + .catch((error) => { + const errorMessage = `Unable to create multichain account group for index: ${groupIndex} (background mode with provider "${provider.getName()}")`; + this.#log(`${WARNING_PREFIX} ${errorMessage}:`, error); + }); + }); + } + /** * Gets multichain account for a given ID. * The default group ID will default to the multichain account with index 0. @@ -335,70 +395,39 @@ export class MultichainAccountWallet< this.#log(`Creating new group for index ${groupIndex}...`); - if (options?.waitForAllProvidersToFinishCreatingAccounts) { - // Create account with all providers and await them. - const results = await Promise.allSettled( - this.#providers.map((provider) => - provider.createAccounts({ - entropySource: this.#entropySource, - groupIndex, - }), - ), - ); + // Extract the EVM provider from the list of providers. + // We always await EVM account creation first. + const [evmProvider, ...otherProviders] = this.#providers; + assert( + evmProvider instanceof EvmAccountProvider, + 'EVM account provider must be first', + ); - // If any of the provider failed to create their accounts, then we consider the - // multichain account group to have failed too. - if (results.some((result) => result.status === 'rejected')) { - // NOTE: Some accounts might still have been created on other account providers. We - // don't rollback them. - const error = `Unable to create multichain account group for index: ${groupIndex}`; - - let message = `${error}:`; - for (const result of results) { - if (result.status === 'rejected') { - message += `\n- ${result.reason}`; - } - } - this.#log(`${WARNING_PREFIX} ${message}`); - console.warn(message); + try { + await evmProvider.createAccounts({ + entropySource: this.#entropySource, + groupIndex, + }); + } catch (error) { + const errorMessage = `Unable to create multichain account group for index: ${groupIndex} with provider "${evmProvider.getName()}". Error: ${(error as Error).message}`; + this.#log(`${ERROR_PREFIX} ${errorMessage}:`, error); + throw new Error(errorMessage); + } - throw new Error(error); - } + // We then create accounts with other providers (some being throttled if configured). + // Depending on the options, we either await all providers or run them in background. + if (options?.waitForAllProvidersToFinishCreatingAccounts) { + await this.#createNonEvmAccounts({ + groupIndex, + providers: otherProviders, + awaitAll: true, + }); } else { - // Extract the EVM provider from the list of providers. - // We will only await the EVM provider to create its accounts, while - // all other providers will be started in the background. - const [evmProvider, ...otherProviders] = this.#providers; - assert( - evmProvider instanceof EvmAccountProvider, - 'EVM account provider must be first', - ); - - // Create account with the EVM provider first and await it. - // If it fails, we don't start creating accounts with other providers. - try { - await evmProvider.createAccounts({ - entropySource: this.#entropySource, - groupIndex, - }); - } catch (error) { - const errorMessage = `Unable to create multichain account group for index: ${groupIndex} with provider "${evmProvider.getName()}". Error: ${(error as Error).message}`; - this.#log(`${ERROR_PREFIX} ${errorMessage}:`, error); - throw new Error(errorMessage); - } - - // Create account with other providers in the background - otherProviders.forEach((provider) => { - provider - .createAccounts({ - entropySource: this.#entropySource, - groupIndex, - }) - .catch((error) => { - // Log errors from background providers but don't fail the operation - const errorMessage = `Could not to create account with provider "${provider.getName()}" for multichain account group index: ${groupIndex}`; - this.#log(`${WARNING_PREFIX} ${errorMessage}:`, error); - }); + // eslint-disable-next-line no-void + void this.#createNonEvmAccounts({ + groupIndex, + providers: otherProviders, + awaitAll: false, }); } diff --git a/packages/multichain-account-service/src/providers/BtcAccountProvider.ts b/packages/multichain-account-service/src/providers/BtcAccountProvider.ts index 8e004f82bf9..a938a02f7ab 100644 --- a/packages/multichain-account-service/src/providers/BtcAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/BtcAccountProvider.ts @@ -7,20 +7,14 @@ import type { SnapId } from '@metamask/snaps-sdk'; import { HandlerType } from '@metamask/snaps-utils'; import type { Json, JsonRpcRequest } from '@metamask/utils'; -import { SnapAccountProvider } from './SnapAccountProvider'; +import { + SnapAccountProvider, + type SnapAccountProviderConfig, +} from './SnapAccountProvider'; import { withRetry, withTimeout } from './utils'; import type { MultichainAccountServiceMessenger } from '../types'; -export type BtcAccountProviderConfig = { - discovery: { - maxAttempts: number; - timeoutMs: number; - backOffMs: number; - }; - createAccounts: { - timeoutMs: number; - }; -}; +export type BtcAccountProviderConfig = SnapAccountProviderConfig; export const BTC_ACCOUNT_PROVIDER_NAME = 'Bitcoin' as const; @@ -31,8 +25,6 @@ export class BtcAccountProvider extends SnapAccountProvider { readonly #client: KeyringClient; - readonly #config: BtcAccountProviderConfig; - constructor( messenger: MultichainAccountServiceMessenger, config: BtcAccountProviderConfig = { @@ -46,11 +38,10 @@ export class BtcAccountProvider extends SnapAccountProvider { }, }, ) { - super(BtcAccountProvider.BTC_SNAP_ID, messenger); + super(BtcAccountProvider.BTC_SNAP_ID, messenger, config); this.#client = this.#getKeyringClientFromSnapId( BtcAccountProvider.BTC_SNAP_ID, ); - this.#config = config; } getName(): string { @@ -88,20 +79,22 @@ export class BtcAccountProvider extends SnapAccountProvider { entropySource: EntropySourceId; groupIndex: number; }): Promise[]> { - const createAccount = await this.getRestrictedSnapAccountCreator(); - - const account = await withTimeout( - createAccount({ - entropySource, - index, - addressType: BtcAccountType.P2wpkh, - scope: BtcScope.Mainnet, - }), - this.#config.createAccounts.timeoutMs, - ); - - assertIsBip44Account(account); - return [account]; + return this.withMaxConcurrency(async () => { + const createAccount = await this.getRestrictedSnapAccountCreator(); + + const account = await withTimeout( + createAccount({ + entropySource, + index, + addressType: BtcAccountType.P2wpkh, + scope: BtcScope.Mainnet, + }), + this.config.createAccounts.timeoutMs, + ); + + assertIsBip44Account(account); + return [account]; + }); } async discoverAccounts({ @@ -119,11 +112,11 @@ export class BtcAccountProvider extends SnapAccountProvider { entropySource, groupIndex, ), - this.#config.discovery.timeoutMs, + this.config.discovery.timeoutMs, ), { - maxAttempts: this.#config.discovery.maxAttempts, - backOffMs: this.#config.discovery.backOffMs, + maxAttempts: this.config.discovery.maxAttempts, + backOffMs: this.config.discovery.backOffMs, }, ); diff --git a/packages/multichain-account-service/src/providers/EvmAccountProvider.ts b/packages/multichain-account-service/src/providers/EvmAccountProvider.ts index 50c5e256833..530d4e59093 100644 --- a/packages/multichain-account-service/src/providers/EvmAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/EvmAccountProvider.ts @@ -10,7 +10,6 @@ import type { } from '@metamask/keyring-internal-api'; import type { Provider } from '@metamask/network-controller'; import { add0x, assert, bytesToHex, type Hex } from '@metamask/utils'; -import type { MultichainAccountServiceMessenger } from 'src/types'; import { assertAreBip44Accounts, @@ -18,6 +17,7 @@ import { BaseBip44AccountProvider, } from './BaseBip44AccountProvider'; import { withRetry, withTimeout } from './utils'; +import type { MultichainAccountServiceMessenger } from '../types'; const ETH_MAINNET_CHAIN_ID = '0x1'; diff --git a/packages/multichain-account-service/src/providers/SnapAccountProvider.test.ts b/packages/multichain-account-service/src/providers/SnapAccountProvider.test.ts index 647421fac7e..2ac3c1d1734 100644 --- a/packages/multichain-account-service/src/providers/SnapAccountProvider.test.ts +++ b/packages/multichain-account-service/src/providers/SnapAccountProvider.test.ts @@ -1,7 +1,87 @@ -import { isSnapAccountProvider } from './SnapAccountProvider'; +import type { Bip44Account } from '@metamask/account-api'; +import type { EntropySourceId, KeyringAccount } from '@metamask/keyring-api'; +import type { SnapId } from '@metamask/snaps-sdk'; + +import { + isSnapAccountProvider, + SnapAccountProvider, +} from './SnapAccountProvider'; import { SolAccountProvider } from './SolAccountProvider'; +import { + getMultichainAccountServiceMessenger, + getRootMessenger, +} from '../tests'; import type { MultichainAccountServiceMessenger } from '../types'; +const THROTTLED_OPERATION_DELAY_MS = 10; +const TEST_SNAP_ID = 'npm:@metamask/test-snap' as SnapId; +const TEST_ENTROPY_SOURCE = 'test-entropy-source' as EntropySourceId; + +// Helper to create a tracked provider that monitors concurrent execution +const createTrackedProvider = (maxConcurrency: number) => { + const tracker: { + startLog: number[]; + endLog: number[]; + activeCount: number; + maxActiveCount: number; + } = { + startLog: [], + endLog: [], + activeCount: 0, + maxActiveCount: 0, + }; + + class TrackedProvider extends SnapAccountProvider { + getName(): string { + return 'Test Provider'; + } + + isAccountCompatible(): boolean { + return true; + } + + async discoverAccounts(): Promise[]> { + return []; + } + + async createAccounts(options: { + entropySource: EntropySourceId; + groupIndex: number; + }): Promise[]> { + return this.withMaxConcurrency(async () => { + tracker.startLog.push(options.groupIndex); + tracker.activeCount += 1; + tracker.maxActiveCount = Math.max( + tracker.maxActiveCount, + tracker.activeCount, + ); + await new Promise((resolve) => + setTimeout(resolve, THROTTLED_OPERATION_DELAY_MS), + ); + tracker.activeCount -= 1; + tracker.endLog.push(options.groupIndex); + return []; + }); + } + } + + const messenger = getMultichainAccountServiceMessenger(getRootMessenger()); + const config = { + maxConcurrency, + createAccounts: { + timeoutMs: 5000, + }, + discovery: { + timeoutMs: 2000, + maxAttempts: 3, + backOffMs: 1000, + }, + }; + const provider = new TrackedProvider(TEST_SNAP_ID, messenger, config); + + return { provider, tracker }; +}; + describe('SnapAccountProvider', () => { describe('isSnapAccountProvider', () => { it('returns false for plain object with snapId property', () => { @@ -47,4 +127,75 @@ describe('SnapAccountProvider', () => { expect(isSnapAccountProvider(solProvider)).toBe(true); }); }); + + describe('withMaxConcurrency', () => { + afterEach(() => { + jest.clearAllTimers(); + jest.useRealTimers(); + }); + + it('throttles createAccounts when maxConcurrency is finite', async () => { + const { provider, tracker } = createTrackedProvider(2); // Allow only 2 concurrent operations + + // Start 4 concurrent calls + const promises = [0, 1, 2, 3].map((index) => + provider.createAccounts({ + entropySource: TEST_ENTROPY_SOURCE, + groupIndex: index, + }), + ); + + await Promise.all(promises); + + // All operations should complete + expect(tracker.startLog).toHaveLength(4); + expect(tracker.endLog).toHaveLength(4); + + // With maxConcurrency=2, never more than 2 should run concurrently + expect(tracker.maxActiveCount).toBe(2); + + // First 2 should start immediately, next 2 should wait + expect(tracker.startLog.slice(0, 2).sort()).toStrictEqual([0, 1]); + }); + + it('does not throttle when maxConcurrency is Infinity', async () => { + const { provider, tracker } = createTrackedProvider(Infinity); // No throttling + + // Start 4 concurrent calls + const promises = [0, 1, 2, 3].map((index) => + provider.createAccounts({ + entropySource: TEST_ENTROPY_SOURCE, + groupIndex: index, + }), + ); + + await Promise.all(promises); + + // All 4 operations should complete + expect(tracker.startLog).toHaveLength(4); + + // With no throttling, all 4 should have been able to run concurrently + expect(tracker.maxActiveCount).toBe(4); + }); + + it('respects concurrency limit across multiple calls', async () => { + const { provider, tracker } = createTrackedProvider(1); // Only 1 concurrent operation + + // Start 3 concurrent calls + const promises = [0, 1, 2].map((index) => + provider.createAccounts({ + entropySource: TEST_ENTROPY_SOURCE, + groupIndex: index, + }), + ); + + await Promise.all(promises); + + // Verify all completed + expect(tracker.endLog).toHaveLength(3); + + // With maxConcurrency=1, never more than 1 should run at a time + expect(tracker.maxActiveCount).toBe(1); + }); + }); }); diff --git a/packages/multichain-account-service/src/providers/SnapAccountProvider.ts b/packages/multichain-account-service/src/providers/SnapAccountProvider.ts index 6b1e814f9ca..a549600ee30 100644 --- a/packages/multichain-account-service/src/providers/SnapAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/SnapAccountProvider.ts @@ -4,21 +4,70 @@ import type { EntropySourceId, KeyringAccount } from '@metamask/keyring-api'; import { KeyringTypes } from '@metamask/keyring-controller'; import type { InternalAccount } from '@metamask/keyring-internal-api'; import type { Json, SnapId } from '@metamask/snaps-sdk'; -import type { MultichainAccountServiceMessenger } from 'src/types'; +import { Semaphore } from 'async-mutex'; import { BaseBip44AccountProvider } from './BaseBip44AccountProvider'; +import type { MultichainAccountServiceMessenger } from '../types'; export type RestrictedSnapKeyringCreateAccount = ( options: Record, ) => Promise; +export type SnapAccountProviderConfig = { + maxConcurrency?: number; + discovery: { + maxAttempts: number; + timeoutMs: number; + backOffMs: number; + }; + createAccounts: { + timeoutMs: number; + }; +}; + export abstract class SnapAccountProvider extends BaseBip44AccountProvider { readonly snapId: SnapId; - constructor(snapId: SnapId, messenger: MultichainAccountServiceMessenger) { + protected readonly config: SnapAccountProviderConfig; + + readonly #queue?: Semaphore; + + constructor( + snapId: SnapId, + messenger: MultichainAccountServiceMessenger, + config: SnapAccountProviderConfig, + ) { super(messenger); this.snapId = snapId; + + const maxConcurrency = config.maxConcurrency ?? Infinity; + this.config = { + ...config, + maxConcurrency, + }; + + // Create semaphore only if concurrency is limited + if (isFinite(maxConcurrency)) { + this.#queue = new Semaphore(maxConcurrency); + } + } + + /** + * Wraps an async operation with concurrency limiting based on maxConcurrency config. + * If maxConcurrency is Infinity (the default), the operation runs immediately without throttling. + * Otherwise, it's queued through the semaphore to respect the concurrency limit. + * + * @param operation - The async operation to execute. + * @returns The result of the operation. + */ + protected async withMaxConcurrency( + operation: () => Promise, + ): Promise { + if (this.#queue) { + return this.#queue.runExclusive(operation); + } + return operation(); } protected async getRestrictedSnapAccountCreator(): Promise { diff --git a/packages/multichain-account-service/src/providers/SolAccountProvider.ts b/packages/multichain-account-service/src/providers/SolAccountProvider.ts index 05a447757be..635104f5f4a 100644 --- a/packages/multichain-account-service/src/providers/SolAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/SolAccountProvider.ts @@ -11,21 +11,15 @@ import { KeyringClient } from '@metamask/keyring-snap-client'; import type { SnapId } from '@metamask/snaps-sdk'; import { HandlerType } from '@metamask/snaps-utils'; import type { Json, JsonRpcRequest } from '@metamask/utils'; -import type { MultichainAccountServiceMessenger } from 'src/types'; -import { SnapAccountProvider } from './SnapAccountProvider'; +import { + SnapAccountProvider, + type SnapAccountProviderConfig, +} from './SnapAccountProvider'; import { withRetry, withTimeout } from './utils'; +import type { MultichainAccountServiceMessenger } from '../types'; -export type SolAccountProviderConfig = { - discovery: { - maxAttempts: number; - timeoutMs: number; - backOffMs: number; - }; - createAccounts: { - timeoutMs: number; - }; -}; +export type SolAccountProviderConfig = SnapAccountProviderConfig; export const SOL_ACCOUNT_PROVIDER_NAME = 'Solana' as const; @@ -36,11 +30,10 @@ export class SolAccountProvider extends SnapAccountProvider { readonly #client: KeyringClient; - readonly #config: SolAccountProviderConfig; - constructor( messenger: MultichainAccountServiceMessenger, config: SolAccountProviderConfig = { + maxConcurrency: 3, discovery: { timeoutMs: 2000, maxAttempts: 3, @@ -51,11 +44,10 @@ export class SolAccountProvider extends SnapAccountProvider { }, }, ) { - super(SolAccountProvider.SOLANA_SNAP_ID, messenger); + super(SolAccountProvider.SOLANA_SNAP_ID, messenger, config); this.#client = this.#getKeyringClientFromSnapId( SolAccountProvider.SOLANA_SNAP_ID, ); - this.#config = config; } getName(): string { @@ -98,7 +90,7 @@ export class SolAccountProvider extends SnapAccountProvider { const createAccount = await this.getRestrictedSnapAccountCreator(); const account = await withTimeout( createAccount({ entropySource, derivationPath }), - this.#config.createAccounts.timeoutMs, + this.config.createAccounts.timeoutMs, ); // Ensure entropy is present before type assertion validation @@ -120,14 +112,16 @@ export class SolAccountProvider extends SnapAccountProvider { entropySource: EntropySourceId; groupIndex: number; }): Promise[]> { - const derivationPath = `m/44'/501'/${groupIndex}'/0'`; - const account = await this.#createAccount({ - entropySource, - groupIndex, - derivationPath, + return this.withMaxConcurrency(async () => { + const derivationPath = `m/44'/501'/${groupIndex}'/0'`; + const account = await this.#createAccount({ + entropySource, + groupIndex, + derivationPath, + }); + + return [account]; }); - - return [account]; } async discoverAccounts({ @@ -145,11 +139,11 @@ export class SolAccountProvider extends SnapAccountProvider { entropySource, groupIndex, ), - this.#config.discovery.timeoutMs, + this.config.discovery.timeoutMs, ), { - maxAttempts: this.#config.discovery.maxAttempts, - backOffMs: this.#config.discovery.backOffMs, + maxAttempts: this.config.discovery.maxAttempts, + backOffMs: this.config.discovery.backOffMs, }, ); diff --git a/packages/multichain-account-service/src/providers/TrxAccountProvider.ts b/packages/multichain-account-service/src/providers/TrxAccountProvider.ts index d62e2c45096..c2c1f660be1 100644 --- a/packages/multichain-account-service/src/providers/TrxAccountProvider.ts +++ b/packages/multichain-account-service/src/providers/TrxAccountProvider.ts @@ -7,21 +7,15 @@ import { KeyringClient } from '@metamask/keyring-snap-client'; import type { SnapId } from '@metamask/snaps-sdk'; import { HandlerType } from '@metamask/snaps-utils'; import type { Json, JsonRpcRequest } from '@metamask/utils'; -import type { MultichainAccountServiceMessenger } from 'src/types'; -import { SnapAccountProvider } from './SnapAccountProvider'; +import { + SnapAccountProvider, + type SnapAccountProviderConfig, +} from './SnapAccountProvider'; import { withRetry, withTimeout } from './utils'; +import type { MultichainAccountServiceMessenger } from '../types'; -export type TrxAccountProviderConfig = { - discovery: { - maxAttempts: number; - timeoutMs: number; - backOffMs: number; - }; - createAccounts: { - timeoutMs: number; - }; -}; +export type TrxAccountProviderConfig = SnapAccountProviderConfig; export const TRX_ACCOUNT_PROVIDER_NAME = 'Tron' as const; @@ -32,8 +26,6 @@ export class TrxAccountProvider extends SnapAccountProvider { readonly #client: KeyringClient; - readonly #config: TrxAccountProviderConfig; - constructor( messenger: MultichainAccountServiceMessenger, config: TrxAccountProviderConfig = { @@ -47,11 +39,10 @@ export class TrxAccountProvider extends SnapAccountProvider { }, }, ) { - super(TrxAccountProvider.TRX_SNAP_ID, messenger); + super(TrxAccountProvider.TRX_SNAP_ID, messenger, config); this.#client = this.#getKeyringClientFromSnapId( TrxAccountProvider.TRX_SNAP_ID, ); - this.#config = config; } getName(): string { @@ -89,20 +80,22 @@ export class TrxAccountProvider extends SnapAccountProvider { entropySource: EntropySourceId; groupIndex: number; }): Promise[]> { - const createAccount = await this.getRestrictedSnapAccountCreator(); - - const account = await withTimeout( - createAccount({ - entropySource, - index, - addressType: TrxAccountType.Eoa, - scope: TrxScope.Mainnet, - }), - this.#config.createAccounts.timeoutMs, - ); - - assertIsBip44Account(account); - return [account]; + return this.withMaxConcurrency(async () => { + const createAccount = await this.getRestrictedSnapAccountCreator(); + + const account = await withTimeout( + createAccount({ + entropySource, + index, + addressType: TrxAccountType.Eoa, + scope: TrxScope.Mainnet, + }), + this.config.createAccounts.timeoutMs, + ); + + assertIsBip44Account(account); + return [account]; + }); } async discoverAccounts({ @@ -120,11 +113,11 @@ export class TrxAccountProvider extends SnapAccountProvider { entropySource, groupIndex, ), - this.#config.discovery.timeoutMs, + this.config.discovery.timeoutMs, ), { - maxAttempts: this.#config.discovery.maxAttempts, - backOffMs: this.#config.discovery.backOffMs, + maxAttempts: this.config.discovery.maxAttempts, + backOffMs: this.config.discovery.backOffMs, }, ); diff --git a/packages/multichain-account-service/src/utils.ts b/packages/multichain-account-service/src/utils.ts new file mode 100644 index 00000000000..b7fa12338cd --- /dev/null +++ b/packages/multichain-account-service/src/utils.ts @@ -0,0 +1,12 @@ +export const toRejectedErrorMessage = ( + prefix: string, + results: PromiseSettledResult[], +) => { + let errorMessage = `${prefix}:`; + for (const r of results) { + if (r.status === 'rejected') { + errorMessage += `\n- ${r.reason}`; + } + } + return errorMessage; +}; From df0e524e1f294973e30e48164e37cc35b8882377 Mon Sep 17 00:00:00 2001 From: Lwin <147362763+lwin-kyaw@users.noreply.github.com> Date: Mon, 3 Nov 2025 22:40:42 +0800 Subject: [PATCH 08/16] feat: updates Subscription Controller exports (#7037) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit …in controller exports ## Explanation Updates Subscription Controller exports. ## References ## Checklist - [ ] I've updated the test suite for new or updated code as appropriate - [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [ ] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/contributing.md#updating-changelogs), highlighting breaking changes as necessary - [ ] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes --- > [!NOTE] > Exports `SubscriptionControllerSubmitSponsorshipIntentsAction` and `SubmitSponsorshipIntentsMethodParams`, and updates the changelog accordingly. > > - **Subscription Controller exports**: > - Add `SubscriptionControllerSubmitSponsorshipIntentsAction` to `src/index.ts`. > - Add `SubmitSponsorshipIntentsMethodParams` to `src/index.ts`. > - **Docs**: > - Update `CHANGELOG.md` to note the new `SubscriptionControllerSubmitSponsorshipIntentsAction` export. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit e8e08ff65e02266043cfe0784a562e253bfde6a3. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- packages/subscription-controller/CHANGELOG.md | 1 + packages/subscription-controller/src/index.ts | 2 ++ 2 files changed, 3 insertions(+) diff --git a/packages/subscription-controller/CHANGELOG.md b/packages/subscription-controller/CHANGELOG.md index 948f318a638..61ee456a691 100644 --- a/packages/subscription-controller/CHANGELOG.md +++ b/packages/subscription-controller/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Added new property, `isSponsorshipSupported` to the ControllerState, `pricing.paymentMethods.chains`. ([#7035](https://github.com/MetaMask/core/pull/7035)) +- Added `SubscriptionControllerSubmitSponsorshipIntentsAction` in the controller exports. ([#7037](https://github.com/MetaMask/core/pull/7037)) ### Changed diff --git a/packages/subscription-controller/src/index.ts b/packages/subscription-controller/src/index.ts index 4538324c66c..7805c902a83 100644 --- a/packages/subscription-controller/src/index.ts +++ b/packages/subscription-controller/src/index.ts @@ -15,6 +15,7 @@ export type { SubscriptionControllerMessenger, SubscriptionControllerOptions, SubscriptionControllerStateChangeEvent, + SubscriptionControllerSubmitSponsorshipIntentsAction, AllowedActions, AllowedEvents, } from './SubscriptionController'; @@ -58,6 +59,7 @@ export type { UpdatePaymentMethodCardRequest, UpdatePaymentMethodCardResponse, CachedLastSelectedPaymentMethod, + SubmitSponsorshipIntentsMethodParams, } from './types'; export { CRYPTO_PAYMENT_METHOD_ERRORS, From defacc6a8b027ccbca5ba0c5379232fc6f70fc6f Mon Sep 17 00:00:00 2001 From: Mathieu Artu Date: Mon, 3 Nov 2025 15:57:13 +0100 Subject: [PATCH 09/16] Release/657.0.0 (#7041) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Explanation Minor release of `@metamask/multichain-account-service`. Changelog: ```md ### Added - Add per-provider throttling for non-EVM account creation to improve performance on low-end devices ([#7000](https://github.com/MetaMask/core/pull/7000)) - Solana provider is now limited to 3 concurrent account creations by default when creating multichain account groups. - Other providers remain unthrottled by default. ``` Mobile test-drive PR: https://github.com/MetaMask/metamask-mobile/pull/21905 (QA'd ✅ ) ## References ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [x] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/contributing.md#updating-changelogs), highlighting breaking changes as necessary - [ ] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes --- > [!NOTE] > Publish @metamask/multichain-account-service 2.1.0 adding per-provider throttling (Solana capped at 3 concurrent) and update dependents; bump monorepo to 657.0.0. > > - **multichain-account-service (`packages/multichain-account-service`)**: > - Release `2.1.0` with per-provider throttling for non-EVM account creation. > - Solana provider limited to 3 concurrent creations by default. > - Update `CHANGELOG.md` and version references. > - **Dependents**: > - Bump `@metamask/multichain-account-service` devDependency to `^2.1.0` in `packages/account-tree-controller` and `packages/assets-controllers`. > - **Repo**: > - Bump monorepo version to `657.0.0`. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit b4773ca35adc00346e87dac3fa00c1eeb7b5c57f. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- package.json | 2 +- packages/account-tree-controller/package.json | 2 +- packages/assets-controllers/package.json | 2 +- packages/multichain-account-service/CHANGELOG.md | 5 ++++- packages/multichain-account-service/package.json | 2 +- yarn.lock | 6 +++--- 6 files changed, 11 insertions(+), 8 deletions(-) diff --git a/package.json b/package.json index 301779746bc..52dd0708f56 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@metamask/core-monorepo", - "version": "656.0.0", + "version": "657.0.0", "private": true, "description": "Monorepo for packages shared between MetaMask clients", "repository": { diff --git a/packages/account-tree-controller/package.json b/packages/account-tree-controller/package.json index 697c0a737b0..a0d5cd4b0ec 100644 --- a/packages/account-tree-controller/package.json +++ b/packages/account-tree-controller/package.json @@ -63,7 +63,7 @@ "@metamask/auto-changelog": "^3.4.4", "@metamask/keyring-api": "^21.0.0", "@metamask/keyring-controller": "^24.0.0", - "@metamask/multichain-account-service": "^2.0.1", + "@metamask/multichain-account-service": "^2.1.0", "@metamask/profile-sync-controller": "^26.0.0", "@metamask/providers": "^22.1.0", "@metamask/snaps-controllers": "^14.0.1", diff --git a/packages/assets-controllers/package.json b/packages/assets-controllers/package.json index b2203020cef..74041ae3772 100644 --- a/packages/assets-controllers/package.json +++ b/packages/assets-controllers/package.json @@ -91,7 +91,7 @@ "@metamask/keyring-controller": "^24.0.0", "@metamask/keyring-internal-api": "^9.0.0", "@metamask/keyring-snap-client": "^8.0.0", - "@metamask/multichain-account-service": "^2.0.1", + "@metamask/multichain-account-service": "^2.1.0", "@metamask/network-controller": "^25.0.0", "@metamask/permission-controller": "^12.1.0", "@metamask/phishing-controller": "^15.0.0", diff --git a/packages/multichain-account-service/CHANGELOG.md b/packages/multichain-account-service/CHANGELOG.md index edba796d74e..3b42f9f7e14 100644 --- a/packages/multichain-account-service/CHANGELOG.md +++ b/packages/multichain-account-service/CHANGELOG.md @@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [2.1.0] + ### Added - Add per-provider throttling for non-EVM account creation to improve performance on low-end devices ([#7000](https://github.com/MetaMask/core/pull/7000)) @@ -256,7 +258,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add `MultichainAccountService` ([#6141](https://github.com/MetaMask/core/pull/6141)), ([#6165](https://github.com/MetaMask/core/pull/6165)) - This service manages multichain accounts/wallets. -[Unreleased]: https://github.com/MetaMask/core/compare/@metamask/multichain-account-service@2.0.1...HEAD +[Unreleased]: https://github.com/MetaMask/core/compare/@metamask/multichain-account-service@2.1.0...HEAD +[2.1.0]: https://github.com/MetaMask/core/compare/@metamask/multichain-account-service@2.0.1...@metamask/multichain-account-service@2.1.0 [2.0.1]: https://github.com/MetaMask/core/compare/@metamask/multichain-account-service@2.0.0...@metamask/multichain-account-service@2.0.1 [2.0.0]: https://github.com/MetaMask/core/compare/@metamask/multichain-account-service@1.6.2...@metamask/multichain-account-service@2.0.0 [1.6.2]: https://github.com/MetaMask/core/compare/@metamask/multichain-account-service@1.6.1...@metamask/multichain-account-service@1.6.2 diff --git a/packages/multichain-account-service/package.json b/packages/multichain-account-service/package.json index db9f1177ce4..d7cd29cde08 100644 --- a/packages/multichain-account-service/package.json +++ b/packages/multichain-account-service/package.json @@ -1,6 +1,6 @@ { "name": "@metamask/multichain-account-service", - "version": "2.0.1", + "version": "2.1.0", "description": "Service to manage multichain accounts", "keywords": [ "MetaMask", diff --git a/yarn.lock b/yarn.lock index 0681edf3950..ddcd00176fa 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2577,7 +2577,7 @@ __metadata: "@metamask/keyring-api": "npm:^21.0.0" "@metamask/keyring-controller": "npm:^24.0.0" "@metamask/messenger": "npm:^0.3.0" - "@metamask/multichain-account-service": "npm:^2.0.1" + "@metamask/multichain-account-service": "npm:^2.1.0" "@metamask/profile-sync-controller": "npm:^26.0.0" "@metamask/providers": "npm:^22.1.0" "@metamask/snaps-controllers": "npm:^14.0.1" @@ -2790,7 +2790,7 @@ __metadata: "@metamask/keyring-snap-client": "npm:^8.0.0" "@metamask/messenger": "npm:^0.3.0" "@metamask/metamask-eth-abis": "npm:^3.1.1" - "@metamask/multichain-account-service": "npm:^2.0.1" + "@metamask/multichain-account-service": "npm:^2.1.0" "@metamask/network-controller": "npm:^25.0.0" "@metamask/permission-controller": "npm:^12.1.0" "@metamask/phishing-controller": "npm:^15.0.0" @@ -4165,7 +4165,7 @@ __metadata: languageName: node linkType: hard -"@metamask/multichain-account-service@npm:^2.0.1, @metamask/multichain-account-service@workspace:packages/multichain-account-service": +"@metamask/multichain-account-service@npm:^2.1.0, @metamask/multichain-account-service@workspace:packages/multichain-account-service": version: 0.0.0-use.local resolution: "@metamask/multichain-account-service@workspace:packages/multichain-account-service" dependencies: From 0f8363d4985302776b75f5e48abc736c5f956d53 Mon Sep 17 00:00:00 2001 From: Bernardo Garces Chapero Date: Mon, 3 Nov 2025 15:15:46 +0000 Subject: [PATCH 10/16] change: set defi controller to refresh only one account (#6944) - Updates DeFiPositionsController to only update the selected EVM account - Adds timeout & retry mechanism - Fixes issue with DeFi polling starting before onboarding due to subscription to `KeyringController:unlocked` event Draft PR for extension: https://github.com/MetaMask/metamask-extension/pull/37215 Draft PR for mobile: https://github.com/MetaMask/metamask-mobile/pull/21657 Related to https://consensyssoftware.atlassian.net/browse/ASSETS-1238 - [X] I've updated the test suite for new or updated code as appropriate - [X] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [X] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/contributing.md#updating-changelogs), highlighting breaking changes as necessary - [X] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes --- > [!NOTE] > Updates DeFi positions to only refresh the selected EVM address, gates updates on relevant events, and adds an 8s timeout with a single retry to the API fetch. > > - **DeFiPositionsController** > - Refreshes DeFi positions for only the selected EVM account (`_executePoll`, `#updateAccountPositions`). > - Replaces AccountsController dependencies with `AccountTreeController:getAccountsFromSelectedAccountGroup` to derive selected EVM address. > - Event changes: > - Stops polling on `KeyringController:lock` (unchanged). > - Removes `KeyringController:unlock` and `AccountsController:accountAdded` listeners. > - Adds `AccountTreeController:selectedAccountGroupChange` to refresh selected address. > - `TransactionController:transactionConfirmed` now updates only if it matches the selected address. > - **Fetch behavior** > - `fetch-positions`: wraps API call with `timeoutWithRetry` (8s timeout, 1 retry). > - Adds `utils/timeout-with-retry` with unit tests. > - **Tests** > - Update unit tests to new selected-account-only behavior and new events. > - **Changelog** > - Documents breaking changes to events/behavior and new timeout+retry. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit fe032936694dbf6234420a14e38bdd636868d69c. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- packages/assets-controllers/CHANGELOG.md | 16 ++ .../DeFiPositionsController.test.ts | 182 ++++++++---------- .../DeFiPositionsController.ts | 104 ++++------ .../fetch-positions.ts | 11 +- .../src/utils/timeout-with-retry.test.ts | 109 +++++++++++ .../src/utils/timeout-with-retry.ts | 39 ++++ 6 files changed, 290 insertions(+), 171 deletions(-) create mode 100644 packages/assets-controllers/src/utils/timeout-with-retry.test.ts create mode 100644 packages/assets-controllers/src/utils/timeout-with-retry.ts diff --git a/packages/assets-controllers/CHANGELOG.md b/packages/assets-controllers/CHANGELOG.md index 137a0d2bf0b..a4f9659de3b 100644 --- a/packages/assets-controllers/CHANGELOG.md +++ b/packages/assets-controllers/CHANGELOG.md @@ -9,8 +9,24 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- Add `AssetsByAccountGroup` to list of exported types ([#6983](https://github.com/MetaMask/core/pull/6983)) - Added `addAssets` to allow adding multiple assets for non-EVM chains ([#7016](https://github.com/MetaMask/core/pull/7016)) +### Changed + +- `isNative` is inferred as `true` for all non-evm assets with slip44 as its asset namespace ([#6983](https://github.com/MetaMask/core/pull/6983)) +- **BREAKING:** Modify DeFi position fetching behaviour ([#6944](https://github.com/MetaMask/core/pull/6944)) + - The fetch request to the API times out after 8 seconds and attempts a single retry + - Refresh only updates the selected evm address + - `KeyringController:unlock` no longer starts polling + - `AccountsController:accountAdded` no longer updates DeFi positions + - `AccountTreeController:selectedAccountGroupChange` updates DeFi positions for the selected address + - `TransactionController:transactionConfirmed` only updates DeFi positions if the transaction is for the selected address + +### Fixed + +- Fixed token is not removed from ignored tokens list when added back due to case insensiteivity ([#7016](https://github.com/MetaMask/core/pull/7016)) + ## [86.0.0] ### Changed diff --git a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsController.test.ts b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsController.test.ts index 302a28319f5..7d7c8d9101a 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsController.test.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsController.test.ts @@ -1,5 +1,5 @@ import { deriveStateFromMetadata } from '@metamask/base-controller'; -import { BtcAccountType } from '@metamask/keyring-api'; +import { BtcAccountType, EthAccountType } from '@metamask/keyring-api'; import { MOCK_ANY_NAMESPACE, Messenger, @@ -23,17 +23,21 @@ import type { TransactionMeta, } from '../../../transaction-controller/src/types'; -const OWNER_ACCOUNTS = [ +const GROUP_ACCOUNTS = [ createMockInternalAccount({ id: 'mock-id-1', address: '0x0000000000000000000000000000000000000001', + type: EthAccountType.Eoa, }), createMockInternalAccount({ - id: 'mock-id-2', - address: '0x0000000000000000000000000000000000000002', + id: 'mock-id-btc-1', + type: BtcAccountType.P2wpkh, }), +]; + +const GROUP_ACCOUNTS_NO_EVM = [ createMockInternalAccount({ - id: 'mock-id-btc', + id: 'mock-id-btc-3', type: BtcAccountType.P2wpkh, }), ]; @@ -59,6 +63,7 @@ type RootMessenger = Messenger< * @param config.mockFetchPositions - The mock fetch positions function * @param config.mockGroupDeFiPositions - The mock group positions function * @param config.mockCalculateDefiMetrics - The mock calculate metrics function + * @param config.mockGroupAccounts - The mock group accounts function * @returns The controller instance, trigger functions, and spies */ function setupController({ @@ -67,21 +72,22 @@ function setupController({ mockFetchPositions = jest.fn(), mockGroupDeFiPositions = jest.fn(), mockCalculateDefiMetrics = jest.fn(), + mockGroupAccounts = GROUP_ACCOUNTS, }: { isEnabled?: () => boolean; mockFetchPositions?: jest.Mock; mockGroupDeFiPositions?: jest.Mock; mockCalculateDefiMetrics?: jest.Mock; mockTrackEvent?: jest.Mock; + mockGroupAccounts?: InternalAccount[]; } = {}) { const messenger: RootMessenger = new Messenger({ namespace: MOCK_ANY_NAMESPACE, }); - const mockListAccounts = jest.fn().mockReturnValue(OWNER_ACCOUNTS); messenger.registerActionHandler( - 'AccountsController:listAccounts', - mockListAccounts, + 'AccountTreeController:getAccountsFromSelectedAccountGroup', + () => mockGroupAccounts, ); const defiPositionControllerMessenger = new Messenger< @@ -95,12 +101,11 @@ function setupController({ }); messenger.delegate({ messenger: defiPositionControllerMessenger, - actions: ['AccountsController:listAccounts'], + actions: ['AccountTreeController:getAccountsFromSelectedAccountGroup'], events: [ - 'KeyringController:unlock', 'KeyringController:lock', 'TransactionController:transactionConfirmed', - 'AccountsController:accountAdded', + 'AccountTreeController:selectedAccountGroupChange', ], }); @@ -132,10 +137,6 @@ function setupController({ const updateSpy = jest.spyOn(controller, 'update' as never); - const triggerUnlock = (): void => { - messenger.publish('KeyringController:unlock'); - }; - const triggerLock = (): void => { messenger.publish('KeyringController:lock'); }; @@ -148,19 +149,19 @@ function setupController({ } as TransactionMeta); }; - const triggerAccountAdded = (account: Partial): void => { + const triggerAccountGroupChange = (): void => { messenger.publish( - 'AccountsController:accountAdded', - account as InternalAccount, + 'AccountTreeController:selectedAccountGroupChange', + 'entropy:test/0', + '', ); }; return { controller, - triggerUnlock, triggerLock, triggerTransactionConfirmed, - triggerAccountAdded, + triggerAccountGroupChange, buildPositionsFetcherSpy, updateSpy, mockFetchPositions, @@ -198,26 +199,8 @@ describe('DeFiPositionsController', () => { expect(stopAllPollingSpy).toHaveBeenCalled(); }); - it('starts polling if the keyring is unlocked', async () => { - const { controller, triggerUnlock } = setupController(); - const startPollingSpy = jest.spyOn(controller, 'startPolling'); - - triggerUnlock(); - - await flushPromises(); - - expect(startPollingSpy).toHaveBeenCalled(); - }); - - it('fetches positions for all accounts when polling', async () => { - const mockFetchPositions = jest.fn().mockImplementation((address) => { - // eslint-disable-next-line jest/no-conditional-in-test - if (OWNER_ACCOUNTS[0].address === address) { - return 'mock-fetch-data-1'; - } - - throw new Error('Error fetching positions'); - }); + it('fetches positions for the selected account when polling', async () => { + const mockFetchPositions = jest.fn().mockResolvedValue('mock-fetch-data-1'); const mockGroupDeFiPositions = jest .fn() .mockReturnValue('mock-grouped-data-1'); @@ -233,17 +216,15 @@ describe('DeFiPositionsController', () => { expect(controller.state).toStrictEqual({ allDeFiPositions: { - [OWNER_ACCOUNTS[0].address]: 'mock-grouped-data-1', - [OWNER_ACCOUNTS[1].address]: null, + [GROUP_ACCOUNTS[0].address]: 'mock-grouped-data-1', }, allDeFiPositionsCount: {}, }); expect(buildPositionsFetcherSpy).toHaveBeenCalled(); - expect(mockFetchPositions).toHaveBeenCalledWith(OWNER_ACCOUNTS[0].address); - expect(mockFetchPositions).toHaveBeenCalledWith(OWNER_ACCOUNTS[1].address); - expect(mockFetchPositions).toHaveBeenCalledTimes(2); + expect(mockFetchPositions).toHaveBeenCalledWith(GROUP_ACCOUNTS[0].address); + expect(mockFetchPositions).toHaveBeenCalledTimes(1); expect(mockGroupDeFiPositions).toHaveBeenCalledWith('mock-fetch-data-1'); expect(mockGroupDeFiPositions).toHaveBeenCalledTimes(1); @@ -293,19 +274,19 @@ describe('DeFiPositionsController', () => { mockGroupDeFiPositions, }); - triggerTransactionConfirmed(OWNER_ACCOUNTS[0].address); + triggerTransactionConfirmed(GROUP_ACCOUNTS[0].address); await flushPromises(); expect(controller.state).toStrictEqual({ allDeFiPositions: { - [OWNER_ACCOUNTS[0].address]: 'mock-grouped-data-1', + [GROUP_ACCOUNTS[0].address]: 'mock-grouped-data-1', }, allDeFiPositionsCount: {}, }); expect(buildPositionsFetcherSpy).toHaveBeenCalled(); - expect(mockFetchPositions).toHaveBeenCalledWith(OWNER_ACCOUNTS[0].address); + expect(mockFetchPositions).toHaveBeenCalledWith(GROUP_ACCOUNTS[0].address); expect(mockFetchPositions).toHaveBeenCalledTimes(1); expect(mockGroupDeFiPositions).toHaveBeenCalledWith('mock-fetch-data-1'); @@ -326,7 +307,33 @@ describe('DeFiPositionsController', () => { isEnabled: () => false, }); - triggerTransactionConfirmed(OWNER_ACCOUNTS[0].address); + triggerTransactionConfirmed(GROUP_ACCOUNTS[0].address); + await flushPromises(); + + expect(controller.state).toStrictEqual( + getDefaultDefiPositionsControllerState(), + ); + + expect(buildPositionsFetcherSpy).toHaveBeenCalled(); + + expect(mockFetchPositions).not.toHaveBeenCalled(); + + expect(mockGroupDeFiPositions).not.toHaveBeenCalled(); + + expect(updateSpy).not.toHaveBeenCalled(); + }); + + it('does not fetch positions for an account when a transaction is confirmed for a different than the selected account', async () => { + const { + controller, + triggerTransactionConfirmed, + buildPositionsFetcherSpy, + updateSpy, + mockFetchPositions, + mockGroupDeFiPositions, + } = setupController(); + + triggerTransactionConfirmed('0x0000000000000000000000000000000000000002'); await flushPromises(); expect(controller.state).toStrictEqual( @@ -342,7 +349,7 @@ describe('DeFiPositionsController', () => { expect(updateSpy).not.toHaveBeenCalled(); }); - it('fetches positions for an account when a new account is added', async () => { + it('fetches positions for the selected evm account when the account group changes', async () => { const mockFetchPositions = jest.fn().mockResolvedValue('mock-fetch-data-1'); const mockGroupDeFiPositions = jest .fn() @@ -350,7 +357,7 @@ describe('DeFiPositionsController', () => { const { controller, - triggerAccountAdded, + triggerAccountGroupChange, buildPositionsFetcherSpy, updateSpy, } = setupController({ @@ -358,23 +365,19 @@ describe('DeFiPositionsController', () => { mockGroupDeFiPositions, }); - const newAccountAddress = '0x0000000000000000000000000000000000000003'; - triggerAccountAdded({ - type: 'eip155:eoa', - address: newAccountAddress, - }); + triggerAccountGroupChange(); await flushPromises(); expect(controller.state).toStrictEqual({ allDeFiPositions: { - [newAccountAddress]: 'mock-grouped-data-1', + [GROUP_ACCOUNTS[0].address]: 'mock-grouped-data-1', }, allDeFiPositionsCount: {}, }); expect(buildPositionsFetcherSpy).toHaveBeenCalled(); - expect(mockFetchPositions).toHaveBeenCalledWith(newAccountAddress); + expect(mockFetchPositions).toHaveBeenCalledWith(GROUP_ACCOUNTS[0].address); expect(mockFetchPositions).toHaveBeenCalledTimes(1); expect(mockGroupDeFiPositions).toHaveBeenCalledWith('mock-fetch-data-1'); @@ -383,22 +386,19 @@ describe('DeFiPositionsController', () => { expect(updateSpy).toHaveBeenCalledTimes(1); }); - it('does not fetch positions for an account when a new account is added and the controller is disabled', async () => { + it('does not fetch positions when the account group changes and there is no evm account', async () => { const { controller, - triggerAccountAdded, + triggerAccountGroupChange, buildPositionsFetcherSpy, updateSpy, mockFetchPositions, mockGroupDeFiPositions, } = setupController({ - isEnabled: () => false, + mockGroupAccounts: GROUP_ACCOUNTS_NO_EVM, }); - triggerAccountAdded({ - type: 'eip155:eoa', - address: '0x0000000000000000000000000000000000000003', - }); + triggerAccountGroupChange(); await flushPromises(); expect(controller.state).toStrictEqual( @@ -430,19 +430,7 @@ describe('DeFiPositionsController', () => { }, }; - const mockMetric2 = { - event: 'mock-event', - category: 'mock-category', - properties: { - totalPositions: 2, - totalMarketValueUSD: 2, - }, - }; - - const mockCalculateDefiMetrics = jest - .fn() - .mockReturnValueOnce(mockMetric1) - .mockReturnValueOnce(mockMetric2); + const mockCalculateDefiMetrics = jest.fn().mockReturnValueOnce(mockMetric1); const { controller } = setupController({ mockGroupDeFiPositions, @@ -454,19 +442,15 @@ describe('DeFiPositionsController', () => { expect(mockCalculateDefiMetrics).toHaveBeenCalled(); expect(mockCalculateDefiMetrics).toHaveBeenCalledWith( - controller.state.allDeFiPositions[OWNER_ACCOUNTS[0].address], + controller.state.allDeFiPositions[GROUP_ACCOUNTS[0].address], ); expect(controller.state.allDeFiPositionsCount).toStrictEqual({ - [OWNER_ACCOUNTS[0].address]: mockMetric1.properties.totalPositions, - [OWNER_ACCOUNTS[1].address]: mockMetric2.properties.totalPositions, + [GROUP_ACCOUNTS[0].address]: mockMetric1.properties.totalPositions, }); - expect(mockTrackEvent).toHaveBeenNthCalledWith(1, mockMetric1); - expect(mockTrackEvent).toHaveBeenNthCalledWith(2, mockMetric2); - expect(mockTrackEvent).toHaveBeenCalledTimes(2); - expect(mockTrackEvent).toHaveBeenNthCalledWith(1, mockMetric1); - expect(mockTrackEvent).toHaveBeenNthCalledWith(2, mockMetric2); + expect(mockTrackEvent).toHaveBeenCalledWith(mockMetric1); + expect(mockTrackEvent).toHaveBeenCalledTimes(1); }); it('only calls track metric when position count changes', async () => { @@ -484,20 +468,10 @@ describe('DeFiPositionsController', () => { }, }; - const mockMetric2 = { - event: 'mock-event', - category: 'mock-category', - properties: { - totalPositions: 2, - totalMarketValueUSD: 2, - }, - }; - const mockCalculateDefiMetrics = jest .fn() .mockReturnValueOnce(mockMetric1) - .mockReturnValueOnce(mockMetric2) - .mockReturnValueOnce(mockMetric2); + .mockReturnValueOnce(mockMetric1); const { controller, triggerTransactionConfirmed } = setupController({ mockGroupDeFiPositions, @@ -505,23 +479,21 @@ describe('DeFiPositionsController', () => { mockTrackEvent, }); - triggerTransactionConfirmed(OWNER_ACCOUNTS[0].address); - triggerTransactionConfirmed(OWNER_ACCOUNTS[0].address); - triggerTransactionConfirmed(OWNER_ACCOUNTS[0].address); + triggerTransactionConfirmed(GROUP_ACCOUNTS[0].address); + triggerTransactionConfirmed(GROUP_ACCOUNTS[0].address); await flushPromises(); expect(mockCalculateDefiMetrics).toHaveBeenCalled(); expect(mockCalculateDefiMetrics).toHaveBeenCalledWith( - controller.state.allDeFiPositions[OWNER_ACCOUNTS[0].address], + controller.state.allDeFiPositions[GROUP_ACCOUNTS[0].address], ); expect(controller.state.allDeFiPositionsCount).toStrictEqual({ - [OWNER_ACCOUNTS[0].address]: mockMetric2.properties.totalPositions, + [GROUP_ACCOUNTS[0].address]: mockMetric1.properties.totalPositions, }); - expect(mockTrackEvent).toHaveBeenCalledTimes(2); - expect(mockTrackEvent).toHaveBeenNthCalledWith(1, mockMetric1); - expect(mockTrackEvent).toHaveBeenNthCalledWith(2, mockMetric2); + expect(mockTrackEvent).toHaveBeenCalledTimes(1); + expect(mockTrackEvent).toHaveBeenCalledWith(mockMetric1); }); describe('metadata', () => { diff --git a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsController.ts b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsController.ts index 06db940f5ca..b317414ee4f 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsController.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/DeFiPositionsController.ts @@ -1,14 +1,15 @@ import type { - AccountsControllerAccountAddedEvent, - AccountsControllerListAccountsAction, -} from '@metamask/accounts-controller'; + AccountTreeControllerGetAccountsFromSelectedAccountGroupAction, + AccountTreeControllerSelectedAccountGroupChangeEvent, +} from '@metamask/account-tree-controller'; import type { ControllerGetStateAction, ControllerStateChangeEvent, StateMetadata, } from '@metamask/base-controller'; -import type { KeyringControllerUnlockEvent } from '@metamask/keyring-controller'; +import { isEvmAccountType } from '@metamask/keyring-api'; import type { KeyringControllerLockEvent } from '@metamask/keyring-controller'; +import type { InternalAccount } from '@metamask/keyring-internal-api'; import type { Messenger } from '@metamask/messenger'; import { StaticIntervalPollingController } from '@metamask/polling-controller'; import type { TransactionControllerTransactionConfirmedEvent } from '@metamask/transaction-controller'; @@ -21,12 +22,9 @@ import { groupDeFiPositions, type GroupedDeFiPositions, } from './group-defi-positions'; -import { reduceInBatchesSerially } from '../assetsUtil'; const TEN_MINUTES_IN_MS = 600_000; -const FETCH_POSITIONS_BATCH_SIZE = 10; - const controllerName = 'DeFiPositionsController'; export type GroupedDeFiPositionsPerChain = { @@ -109,16 +107,16 @@ export type DeFiPositionsControllerStateChangeEvent = /** * The external actions available to the {@link DeFiPositionsController}. */ -export type AllowedActions = AccountsControllerListAccountsAction; +export type AllowedActions = + AccountTreeControllerGetAccountsFromSelectedAccountGroupAction; /** * The external events available to the {@link DeFiPositionsController}. */ export type AllowedEvents = - | KeyringControllerUnlockEvent | KeyringControllerLockEvent | TransactionControllerTransactionConfirmedEvent - | AccountsControllerAccountAddedEvent; + | AccountTreeControllerSelectedAccountGroupChangeEvent; /** * The messenger of the {@link DeFiPositionsController}. @@ -174,10 +172,6 @@ export class DeFiPositionsController extends StaticIntervalPollingController()< this.#fetchPositions = buildPositionFetcher(); this.#isEnabled = isEnabled; - this.messenger.subscribe('KeyringController:unlock', () => { - this.startPolling(null); - }); - this.messenger.subscribe('KeyringController:lock', () => { this.stopAllPolling(); }); @@ -185,22 +179,30 @@ export class DeFiPositionsController extends StaticIntervalPollingController()< this.messenger.subscribe( 'TransactionController:transactionConfirmed', async (transactionMeta) => { - if (!this.#isEnabled()) { + const selectedAddress = this.#getSelectedEvmAdress(); + + if ( + !selectedAddress || + selectedAddress.toLowerCase() !== + transactionMeta.txParams.from.toLowerCase() + ) { return; } - await this.#updateAccountPositions(transactionMeta.txParams.from); + await this.#updateAccountPositions(selectedAddress); }, ); this.messenger.subscribe( - 'AccountsController:accountAdded', - async (account) => { - if (!this.#isEnabled() || !account.type.startsWith('eip155:')) { + 'AccountTreeController:selectedAccountGroupChange', + async () => { + const selectedAddress = this.#getSelectedEvmAdress(); + + if (!selectedAddress) { return; } - await this.#updateAccountPositions(account.address); + await this.#updateAccountPositions(selectedAddress); }, ); @@ -212,57 +214,24 @@ export class DeFiPositionsController extends StaticIntervalPollingController()< return; } - const accounts = this.messenger.call('AccountsController:listAccounts'); - - const initialResult: { - accountAddress: string; - positions: GroupedDeFiPositionsPerChain | null; - }[] = []; - - const results = await reduceInBatchesSerially({ - initialResult, - values: accounts, - batchSize: FETCH_POSITIONS_BATCH_SIZE, - eachBatch: async (workingResult, batch) => { - const batchResults = ( - await Promise.all( - batch.map(async ({ address: accountAddress, type }) => { - if (type.startsWith('eip155:')) { - const positions = - await this.#fetchAccountPositions(accountAddress); - - return { - accountAddress, - positions, - }; - } - - return undefined; - }), - ) - ).filter(Boolean) as { - accountAddress: string; - positions: GroupedDeFiPositionsPerChain | null; - }[]; - - return [...workingResult, ...batchResults]; - }, - }); + const selectedAddress = this.#getSelectedEvmAdress(); - const allDefiPositions = results.reduce( - (acc, { accountAddress, positions }) => { - acc[accountAddress] = positions; - return acc; - }, - {} as DeFiPositionsControllerState['allDeFiPositions'], - ); + if (!selectedAddress) { + return; + } + + const accountPositions = await this.#fetchAccountPositions(selectedAddress); this.update((state) => { - state.allDeFiPositions = allDefiPositions; + state.allDeFiPositions[selectedAddress] = accountPositions; }); } async #updateAccountPositions(accountAddress: string): Promise { + if (!this.#isEnabled()) { + return; + } + const accountPositionsPerChain = await this.#fetchAccountPositions(accountAddress); @@ -314,4 +283,11 @@ export class DeFiPositionsController extends StaticIntervalPollingController()< this.#trackEvent?.(defiMetrics); } } + + #getSelectedEvmAdress(): string | undefined { + return this.messenger + .call('AccountTreeController:getAccountsFromSelectedAccountGroup') + .find((account: InternalAccount) => isEvmAccountType(account.type)) + ?.address; + } } diff --git a/packages/assets-controllers/src/DeFiPositionsController/fetch-positions.ts b/packages/assets-controllers/src/DeFiPositionsController/fetch-positions.ts index cd05d1921c8..427ca587081 100644 --- a/packages/assets-controllers/src/DeFiPositionsController/fetch-positions.ts +++ b/packages/assets-controllers/src/DeFiPositionsController/fetch-positions.ts @@ -1,3 +1,5 @@ +import { timeoutWithRetry } from '../utils/timeout-with-retry'; + export type DefiPositionResponse = AdapterResponse<{ tokens: ProtocolToken[]; }>; @@ -58,6 +60,9 @@ export type Balance = { // TODO: Update with prod API URL when available export const DEFI_POSITIONS_API_URL = 'https://defiadapters.api.cx.metamask.io'; +const EIGHT_SECONDS_IN_MS = 8_000; +const MAX_RETRIES = 1; + /** * Builds a function that fetches DeFi positions for a given account address * @@ -65,8 +70,10 @@ export const DEFI_POSITIONS_API_URL = 'https://defiadapters.api.cx.metamask.io'; */ export function buildPositionFetcher() { return async (accountAddress: string): Promise => { - const defiPositionsResponse = await fetch( - `${DEFI_POSITIONS_API_URL}/positions/${accountAddress}`, + const defiPositionsResponse = await timeoutWithRetry( + () => fetch(`${DEFI_POSITIONS_API_URL}/positions/${accountAddress}`), + EIGHT_SECONDS_IN_MS, + MAX_RETRIES, ); if (defiPositionsResponse.status !== 200) { diff --git a/packages/assets-controllers/src/utils/timeout-with-retry.test.ts b/packages/assets-controllers/src/utils/timeout-with-retry.test.ts new file mode 100644 index 00000000000..6b4ec9caa9b --- /dev/null +++ b/packages/assets-controllers/src/utils/timeout-with-retry.test.ts @@ -0,0 +1,109 @@ +import { timeoutWithRetry } from './timeout-with-retry'; +import { flushPromises } from '../../../../tests/helpers'; + +describe('timeoutWithRetry', () => { + const timeout = 1000; + + beforeEach(() => { + jest.useFakeTimers(); + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + it('returns the result when call completes before timeout', async () => { + const mockCall = jest.fn(async () => 'success'); + + const resultPromise = timeoutWithRetry(mockCall, timeout, 0); + jest.runAllTimers(); + const result = await resultPromise; + + expect(result).toBe('success'); + expect(mockCall).toHaveBeenCalledTimes(1); + }); + + describe('retry behaviour', () => { + it('throws when maxRetries is negative', async () => { + const mockCall = jest.fn(async () => 'success'); + + await expect(() => + timeoutWithRetry(mockCall, timeout, -1), + ).rejects.toThrow('maxRetries must be greater than or equal to 0'); + }); + + it('returns the result when call completes just before timeout', async () => { + const mockCall = createMockCallWithRetries(timeout, 0); + + const resultPromise = timeoutWithRetry(mockCall, timeout, 0); + jest.runAllTimers(); + const result = await resultPromise; + + expect(result).toBe('success'); + expect(mockCall).toHaveBeenCalledTimes(1); + }); + + it('succeeds after multiple retries', async () => { + const mockCall = createMockCallWithRetries(timeout, 2); + + const resultPromise = timeoutWithRetry(mockCall, timeout, 3); + jest.runAllTimers(); + await flushPromises(); + jest.runAllTimers(); + const result = await resultPromise; + + expect(result).toBe('success'); + expect(mockCall).toHaveBeenCalledTimes(3); + }); + + it('throws when all retries are exhausted', async () => { + const mockCall = createMockCallWithRetries(timeout, 2); + + const resultPromise = timeoutWithRetry(mockCall, timeout, 1); + jest.runAllTimers(); + await flushPromises(); + jest.runAllTimers(); + + await expect(resultPromise).rejects.toThrow('timeout'); + expect(mockCall).toHaveBeenCalledTimes(2); + }); + }); + + describe('non-timeout errors', () => { + it('throws immediately on non-timeout error without retrying', async () => { + const customError = new Error('custom error'); + const mockCall = jest.fn(async () => { + throw customError; + }); + + const resultPromise = timeoutWithRetry(mockCall, timeout, 0); + jest.runAllTimers(); + + await expect(resultPromise).rejects.toThrow('custom error'); + expect(mockCall).toHaveBeenCalledTimes(1); + }); + }); +}); + +/** + * @param timeout - The timeout in milliseconds. + * @param timeoutsBeforeSuccess - The number of timeouts before the call succeeds. + * @returns A mock call function that times out for a specific number of times before returning 'success'. + */ +function createMockCallWithRetries( + timeout: number, + timeoutsBeforeSuccess: number, +) { + let callCount = 0; + const mockCall = jest.fn(async () => { + callCount += 1; + + if (callCount < timeoutsBeforeSuccess + 1) { + await new Promise((resolve) => setTimeout(resolve, timeout + 1)); + } + + return 'success'; + }); + + return mockCall; +} diff --git a/packages/assets-controllers/src/utils/timeout-with-retry.ts b/packages/assets-controllers/src/utils/timeout-with-retry.ts new file mode 100644 index 00000000000..90e0be1b964 --- /dev/null +++ b/packages/assets-controllers/src/utils/timeout-with-retry.ts @@ -0,0 +1,39 @@ +import { assert } from '@metamask/utils'; + +const TIMEOUT_ERROR = new Error('timeout'); + +/** + * + * @param call - The async function to call. + * @param timeout - Timeout in milliseconds for each call attempt. + * @param maxRetries - Maximum number of retries on timeout. + * @returns The resolved value of the call, or throws the last error if not a timeout or retries exhausted. + */ +// eslint-disable-next-line consistent-return +export async function timeoutWithRetry Promise>( + call: T, + timeout: number, + maxRetries: number, + // @ts-expect-error TS2366: Assertion guarantees loop executes +): Promise>> { + assert(maxRetries >= 0, 'maxRetries must be greater than or equal to 0'); + + let attempt = 0; + + while (attempt <= maxRetries) { + try { + return (await Promise.race([ + call(), + new Promise((_resolve, reject) => + setTimeout(() => reject(TIMEOUT_ERROR), timeout), + ), + ])) as Awaited>; + } catch (err) { + if (err === TIMEOUT_ERROR && attempt < maxRetries) { + attempt += 1; + continue; + } + throw err; + } + } +} From be8c8a0a2f0d82581462a54b225f8b400fd7f8b9 Mon Sep 17 00:00:00 2001 From: Bernardo Garces Chapero Date: Mon, 3 Nov 2025 15:29:16 +0000 Subject: [PATCH 11/16] feat: better detection of native multichain assets (#6983) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently, non-evm native assets were identified by a static list. With this change, they will be identified when using `slip44` as part of the asset namespace, instead of `token`. - [ ] I've updated the test suite for new or updated code as appropriate - [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [ ] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/contributing.md#updating-changelogs), highlighting breaking changes as necessary - [ ] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes --- > [!NOTE] > Infer `isNative` for non‑EVM assets from `slip44` namespace and export `AssetsByAccountGroup` type. > > - **Selectors** > - Update `selectAllMultichainAssets` in `selectors/token-selectors.ts` to set `isNative` when `caipAsset.assetNamespace === 'slip44'` (remove static `MULTICHAIN_NATIVE_ASSET_IDS`). > - **Exports** > - Export `AssetsByAccountGroup` from `selectors/token-selectors` via `src/index.ts`. > - **Docs** > - Update `CHANGELOG.md` for the new export and `isNative` detection change. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit f2bf02bda2f67b82aa0d8194ba51cc940e5ceba8. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- packages/assets-controllers/src/index.ts | 1 + .../src/selectors/token-selectors.ts | 10 ++-------- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/packages/assets-controllers/src/index.ts b/packages/assets-controllers/src/index.ts index 22ed9936b6c..3b9cde2216f 100644 --- a/packages/assets-controllers/src/index.ts +++ b/packages/assets-controllers/src/index.ts @@ -224,6 +224,7 @@ export { calculateBalanceChangeForAccountGroup, } from './balances'; export type { + AssetsByAccountGroup, AccountGroupAssets, Asset, AssetListState, diff --git a/packages/assets-controllers/src/selectors/token-selectors.ts b/packages/assets-controllers/src/selectors/token-selectors.ts index f72959eabc4..123c26bb027 100644 --- a/packages/assets-controllers/src/selectors/token-selectors.ts +++ b/packages/assets-controllers/src/selectors/token-selectors.ts @@ -20,7 +20,7 @@ import type { TokenBalancesControllerState } from '../TokenBalancesController'; import type { Token, TokenRatesControllerState } from '../TokenRatesController'; import type { TokensControllerState } from '../TokensController'; -type AssetsByAccountGroup = { +export type AssetsByAccountGroup = { [accountGroupId: AccountGroupId]: AccountGroupAssets; }; @@ -28,12 +28,6 @@ export type AccountGroupAssets = { [network: string]: Asset[]; }; -// If this gets out of hand with other chains, we should probably have a permanent object that defines them -const MULTICHAIN_NATIVE_ASSET_IDS = [ - `bip122:000000000019d6689c085ae165831e93/slip44:0`, - `solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501`, -]; - type EvmAccountType = Extract; type MultichainAccountType = Exclude< InternalAccount['type'], @@ -401,7 +395,7 @@ const selectAllMultichainAssets = createAssetListSelector( groupChainAssets.push({ accountType: type as MultichainAccountType, assetId, - isNative: MULTICHAIN_NATIVE_ASSET_IDS.includes(assetId), + isNative: caipAsset.assetNamespace === 'slip44', image: assetMetadata.iconUrl, name: assetMetadata.name ?? assetMetadata.symbol ?? asset, symbol: assetMetadata.symbol ?? asset, From c661e705c794881b89ae753b37c6d0827ad94744 Mon Sep 17 00:00:00 2001 From: juanmigdr Date: Mon, 3 Nov 2025 18:21:47 +0100 Subject: [PATCH 12/16] chore: added tests for ignore tokens fix --- .../src/TokensController.test.ts | 62 +++++++++++++++++++ .../src/TokensController.ts | 2 +- 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/packages/assets-controllers/src/TokensController.test.ts b/packages/assets-controllers/src/TokensController.test.ts index d5d8d114566..0078012e0d1 100644 --- a/packages/assets-controllers/src/TokensController.test.ts +++ b/packages/assets-controllers/src/TokensController.test.ts @@ -2925,6 +2925,68 @@ describe('TokensController', () => { ); }); + it('should clear nest allIgnoredTokens when re-adding tokens with different address case via addTokens', async () => { + const selectedAddress = '0x1'; + const selectedAccount = createMockInternalAccount({ + address: selectedAddress, + }); + + const tokenAddressFromAPI = '0x7DA14988E4F390C2E34ED41DF1814467D3ADE0C3'; + const checksummedAddress = '0x7da14988E4f390C2E34ed41DF1814467D3aDe0c3'; + + const dummyTokens = [ + { + address: tokenAddressFromAPI, + symbol: 'PEPE', + decimals: 18, + aggregators: [], + image: undefined, + }, + ]; + + await withController( + { + options: { + chainId: ChainId.mainnet, + }, + mocks: { + getSelectedAccount: selectedAccount, + }, + }, + async ({ controller }) => { + await controller.addTokens(dummyTokens, 'mainnet'); + expect( + controller.state.allTokens[ChainId.mainnet][selectedAddress][0] + .address, + ).toBe(checksummedAddress); + + controller.ignoreTokens([tokenAddressFromAPI], 'mainnet'); + expect( + controller.state.allIgnoredTokens[ChainId.mainnet][selectedAddress], + ).toStrictEqual([checksummedAddress]); + + expect( + controller.state.allTokens[ChainId.mainnet][selectedAddress], + ).toStrictEqual([]); + + await controller.addTokens(dummyTokens, 'mainnet'); + + // Should remove ignored token despite case difference + expect( + controller.state.allIgnoredTokens[ChainId.mainnet][selectedAddress], + ).toStrictEqual([]); + + expect( + controller.state.allTokens[ChainId.mainnet][selectedAddress], + ).toHaveLength(1); + expect( + controller.state.allTokens[ChainId.mainnet][selectedAddress][0] + .address, + ).toBe(checksummedAddress); + }, + ); + }); + it('should clear nest allDetectedTokens under chain ID and selected address when an detected token is added to tokens list', async () => { const selectedAddress = '0x1'; const selectedAccount = createMockInternalAccount({ diff --git a/packages/assets-controllers/src/TokensController.ts b/packages/assets-controllers/src/TokensController.ts index 686340c0394..62cac130a75 100644 --- a/packages/assets-controllers/src/TokensController.ts +++ b/packages/assets-controllers/src/TokensController.ts @@ -516,7 +516,7 @@ export class TokensController extends BaseController< ...tokensToImport, ].reduce( (output, token) => { - output[token.address] = token; + output[toChecksumHexAddress(token.address)] = token; return output; }, {} as { [address: string]: Token }, From 4be226439e88cc666cb24655964357d1602d792e Mon Sep 17 00:00:00 2001 From: juanmigdr Date: Mon, 3 Nov 2025 18:54:56 +0100 Subject: [PATCH 13/16] chore: added some more tests --- .../MultichainAssetsController.test.ts | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/packages/assets-controllers/src/MultichainAssetsController/MultichainAssetsController.test.ts b/packages/assets-controllers/src/MultichainAssetsController/MultichainAssetsController.test.ts index 1d4498f3242..750b6d048dd 100644 --- a/packages/assets-controllers/src/MultichainAssetsController/MultichainAssetsController.test.ts +++ b/packages/assets-controllers/src/MultichainAssetsController/MultichainAssetsController.test.ts @@ -1166,6 +1166,64 @@ describe('MultichainAssetsController', () => { }, }); }); + + it('should not publish event when no new assets are added', async () => { + const existingAsset = + 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/slip44:501'; + + const { controller, messenger } = setupController({ + state: { + accountsAssets: { + [mockSolanaAccount.id]: [existingAsset], + }, + assetsMetadata: mockGetMetadataReturnValue.assets, + allIgnoredAssets: {}, + } as MultichainAssetsControllerState, + }); + + const eventListener = jest.fn(); + messenger.subscribe( + 'MultichainAssetsController:accountAssetListUpdated', + eventListener, + ); + + await controller.addAssets([existingAsset], mockSolanaAccount.id); + + // Event should not be published since no new assets were added + expect(eventListener).not.toHaveBeenCalled(); + }); + + it('should partially remove assets from ignored list when only some are added', async () => { + const ignoredAsset1 = 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/slip44:501'; + const ignoredAsset2 = 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:Token1'; + const ignoredAsset3 = 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:Token2'; + + const { controller } = setupController({ + state: { + accountsAssets: {}, + assetsMetadata: mockGetMetadataReturnValue.assets, + allIgnoredAssets: { + [mockSolanaAccount.id]: [ignoredAsset1, ignoredAsset2, ignoredAsset3], + }, + } as MultichainAssetsControllerState, + }); + + // Only add two of the three ignored assets + await controller.addAssets( + [ignoredAsset1, ignoredAsset2], + mockSolanaAccount.id, + ); + + // Should have added the two assets + expect( + controller.state.accountsAssets[mockSolanaAccount.id], + ).toStrictEqual([ignoredAsset1, ignoredAsset2]); + + // Should have only the third asset remaining in ignored list + expect( + controller.state.allIgnoredAssets[mockSolanaAccount.id], + ).toStrictEqual([ignoredAsset3]); + }); }); describe('asset detection with ignored assets', () => { From 51ad704fbf508afcbc96db36b86c7c3c6da23bc3 Mon Sep 17 00:00:00 2001 From: juanmigdr Date: Mon, 3 Nov 2025 19:00:11 +0100 Subject: [PATCH 14/16] chore: prettier --- .../MultichainAssetsController.test.ts | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/packages/assets-controllers/src/MultichainAssetsController/MultichainAssetsController.test.ts b/packages/assets-controllers/src/MultichainAssetsController/MultichainAssetsController.test.ts index 750b6d048dd..15d3a24f159 100644 --- a/packages/assets-controllers/src/MultichainAssetsController/MultichainAssetsController.test.ts +++ b/packages/assets-controllers/src/MultichainAssetsController/MultichainAssetsController.test.ts @@ -1194,16 +1194,23 @@ describe('MultichainAssetsController', () => { }); it('should partially remove assets from ignored list when only some are added', async () => { - const ignoredAsset1 = 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/slip44:501'; - const ignoredAsset2 = 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:Token1'; - const ignoredAsset3 = 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:Token2'; + const ignoredAsset1 = + 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/slip44:501'; + const ignoredAsset2 = + 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:Token1'; + const ignoredAsset3 = + 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:Token2'; const { controller } = setupController({ state: { accountsAssets: {}, assetsMetadata: mockGetMetadataReturnValue.assets, allIgnoredAssets: { - [mockSolanaAccount.id]: [ignoredAsset1, ignoredAsset2, ignoredAsset3], + [mockSolanaAccount.id]: [ + ignoredAsset1, + ignoredAsset2, + ignoredAsset3, + ], }, } as MultichainAssetsControllerState, }); From 66692cf1513c0b58dba1aba6a3f9b41e291ff227 Mon Sep 17 00:00:00 2001 From: juanmigdr Date: Mon, 3 Nov 2025 19:14:10 +0100 Subject: [PATCH 15/16] chore: increase coverage --- .../ERC1155/ERC1155Standard.test.ts | 134 ++++++++++++++++++ 1 file changed, 134 insertions(+) diff --git a/packages/assets-controllers/src/Standards/NftStandards/ERC1155/ERC1155Standard.test.ts b/packages/assets-controllers/src/Standards/NftStandards/ERC1155/ERC1155Standard.test.ts index f1e7d341a56..f1fedc83af7 100644 --- a/packages/assets-controllers/src/Standards/NftStandards/ERC1155/ERC1155Standard.test.ts +++ b/packages/assets-controllers/src/Standards/NftStandards/ERC1155/ERC1155Standard.test.ts @@ -330,4 +330,138 @@ describe('ERC1155Standard', () => { ); }); }); + + describe('getDetails error handling', () => { + it('should throw error for non-ERC1155 contract', async () => { + // Mock failed ERC1155 interface check + nock('https://mainnet.infura.io:443', { encodedQueryParams: true }) + .post('/v3/341eacb578dd44a1a049cbc5f6fd4035') + .reply(200, { + jsonrpc: '2.0', + id: 1, + result: + '0x0000000000000000000000000000000000000000000000000000000000000000', + }); + + await expect( + erc1155Standard.getDetails(ERC1155_ADDRESS, 'https://ipfs.gateway.com'), + ).rejects.toThrow("This isn't a valid ERC1155 contract"); + }); + + it('should handle JSON parsing errors gracefully', async () => { + // Mock successful ERC1155 interface check + nock('https://mainnet.infura.io:443', { encodedQueryParams: true }) + .post('/v3/341eacb578dd44a1a049cbc5f6fd4035') + .reply(200, { + jsonrpc: '2.0', + id: 1, + result: + '0x0000000000000000000000000000000000000000000000000000000000000001', + }) + .persist(); + + // Mock tokenURI call to return a valid URI + nock('https://mainnet.infura.io:443', { encodedQueryParams: true }) + .post('/v3/341eacb578dd44a1a049cbc5f6fd4035') + .reply(200, { + jsonrpc: '2.0', + id: 2, + result: + '0x0000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000001968747470733a2f2f6578616d706c652e636f6d2f746f6b656e2f31000000', + }); + + // Mock the metadata fetch to return invalid JSON + nock('https://example.com') + .get('/token/1') + .reply(200, 'invalid json content'); + + const details = await erc1155Standard.getDetails( + ERC1155_ADDRESS, + 'https://ipfs.gateway.com', + SAMPLE_TOKEN_ID, + ); + + // Should still return details even if JSON parsing fails + expect(details.standard).toBe('ERC1155'); + expect(details.image).toBeUndefined(); // Image should be undefined due to JSON parse error + }); + + it('should handle IPFS URLs in image metadata', async () => { + // Mock successful ERC1155 interface check + nock('https://mainnet.infura.io:443', { encodedQueryParams: true }) + .post('/v3/341eacb578dd44a1a049cbc5f6fd4035') + .reply(200, { + jsonrpc: '2.0', + id: 1, + result: + '0x0000000000000000000000000000000000000000000000000000000000000001', + }) + .persist(); + + // Mock tokenURI call + nock('https://mainnet.infura.io:443', { encodedQueryParams: true }) + .post('/v3/341eacb578dd44a1a049cbc5f6fd4035') + .reply(200, { + jsonrpc: '2.0', + id: 2, + result: + '0x0000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000001968747470733a2f2f6578616d706c652e636f6d2f746f6b656e2f31000000', + }); + + // Mock metadata with IPFS image URL + nock('https://example.com').get('/token/1').reply(200, { + name: 'Test NFT', + image: 'ipfs://QmTestHash/image.png', + }); + + const details = await erc1155Standard.getDetails( + ERC1155_ADDRESS, + 'https://ipfs.gateway.com', + SAMPLE_TOKEN_ID, + ); + + expect(details.standard).toBe('ERC1155'); + expect(details.image).toBe( + 'https://ipfs.gateway.com/ipfs/QmTestHash/image.png', + ); + }); + + it('should handle network timeout when fetching metadata', async () => { + // Mock successful ERC1155 interface check + nock('https://mainnet.infura.io:443', { encodedQueryParams: true }) + .post('/v3/341eacb578dd44a1a049cbc5f6fd4035') + .reply(200, { + jsonrpc: '2.0', + id: 1, + result: + '0x0000000000000000000000000000000000000000000000000000000000000001', + }) + .persist(); + + // Mock tokenURI call + nock('https://mainnet.infura.io:443', { encodedQueryParams: true }) + .post('/v3/341eacb578dd44a1a049cbc5f6fd4035') + .reply(200, { + jsonrpc: '2.0', + id: 2, + result: + '0x0000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000001968747470733a2f2f6578616d706c652e636f6d2f746f6b656e2f31000000', + }); + + // Mock network timeout + nock('https://example.com') + .get('/token/1') + .replyWithError('Network timeout'); + + const details = await erc1155Standard.getDetails( + ERC1155_ADDRESS, + 'https://ipfs.gateway.com', + SAMPLE_TOKEN_ID, + ); + + // Should handle timeout gracefully and still return basic details + expect(details.standard).toBe('ERC1155'); + expect(details.image).toBeUndefined(); + }); + }); }); From 6b72b0d809e6cee7a656aa91d3938dfc70d12415 Mon Sep 17 00:00:00 2001 From: juanmigdr Date: Mon, 3 Nov 2025 19:50:00 +0100 Subject: [PATCH 16/16] chore: fixed broken tests --- .../ERC1155/ERC1155Standard.test.ts | 163 +++++++----------- 1 file changed, 60 insertions(+), 103 deletions(-) diff --git a/packages/assets-controllers/src/Standards/NftStandards/ERC1155/ERC1155Standard.test.ts b/packages/assets-controllers/src/Standards/NftStandards/ERC1155/ERC1155Standard.test.ts index f1fedc83af7..d35857522a2 100644 --- a/packages/assets-controllers/src/Standards/NftStandards/ERC1155/ERC1155Standard.test.ts +++ b/packages/assets-controllers/src/Standards/NftStandards/ERC1155/ERC1155Standard.test.ts @@ -331,137 +331,94 @@ describe('ERC1155Standard', () => { }); }); - describe('getDetails error handling', () => { - it('should throw error for non-ERC1155 contract', async () => { - // Mock failed ERC1155 interface check - nock('https://mainnet.infura.io:443', { encodedQueryParams: true }) - .post('/v3/341eacb578dd44a1a049cbc5f6fd4035') - .reply(200, { - jsonrpc: '2.0', - id: 1, - result: - '0x0000000000000000000000000000000000000000000000000000000000000000', - }); - - await expect( - erc1155Standard.getDetails(ERC1155_ADDRESS, 'https://ipfs.gateway.com'), - ).rejects.toThrow("This isn't a valid ERC1155 contract"); - }); - - it('should handle JSON parsing errors gracefully', async () => { + describe('getDetails optional parameters', () => { + it('should return details without tokenURI when no tokenId is provided', async () => { // Mock successful ERC1155 interface check - nock('https://mainnet.infura.io:443', { encodedQueryParams: true }) - .post('/v3/341eacb578dd44a1a049cbc5f6fd4035') - .reply(200, { - jsonrpc: '2.0', - id: 1, - result: - '0x0000000000000000000000000000000000000000000000000000000000000001', - }) - .persist(); - - // Mock tokenURI call to return a valid URI - nock('https://mainnet.infura.io:443', { encodedQueryParams: true }) - .post('/v3/341eacb578dd44a1a049cbc5f6fd4035') - .reply(200, { - jsonrpc: '2.0', - id: 2, - result: - '0x0000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000001968747470733a2f2f6578616d706c652e636f6d2f746f6b656e2f31000000', - }); - - // Mock the metadata fetch to return invalid JSON - nock('https://example.com') - .get('/token/1') - .reply(200, 'invalid json content'); + jest + .spyOn(erc1155Standard, 'contractSupportsBase1155Interface') + .mockResolvedValue(true); + jest.spyOn(erc1155Standard, 'getAssetSymbol').mockResolvedValue('TEST'); + jest + .spyOn(erc1155Standard, 'getAssetName') + .mockResolvedValue('Test Token'); + const ipfsGateway = 'https://ipfs.gateway.com'; const details = await erc1155Standard.getDetails( ERC1155_ADDRESS, - 'https://ipfs.gateway.com', - SAMPLE_TOKEN_ID, + ipfsGateway, + // No tokenId parameter to test the optional parameter behavior ); - // Should still return details even if JSON parsing fails expect(details.standard).toBe('ERC1155'); - expect(details.image).toBeUndefined(); // Image should be undefined due to JSON parse error + expect(details.tokenURI).toBeUndefined(); // Should be undefined when no tokenId + expect(details.symbol).toBe('TEST'); + expect(details.name).toBe('Test Token'); + + // Restore original methods + jest.restoreAllMocks(); }); - it('should handle IPFS URLs in image metadata', async () => { + it('should convert IPFS URIs to gateway URLs', async () => { // Mock successful ERC1155 interface check - nock('https://mainnet.infura.io:443', { encodedQueryParams: true }) - .post('/v3/341eacb578dd44a1a049cbc5f6fd4035') - .reply(200, { - jsonrpc: '2.0', - id: 1, - result: - '0x0000000000000000000000000000000000000000000000000000000000000001', - }) - .persist(); - - // Mock tokenURI call - nock('https://mainnet.infura.io:443', { encodedQueryParams: true }) - .post('/v3/341eacb578dd44a1a049cbc5f6fd4035') - .reply(200, { - jsonrpc: '2.0', - id: 2, - result: - '0x0000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000001968747470733a2f2f6578616d706c652e636f6d2f746f6b656e2f31000000', - }); - - // Mock metadata with IPFS image URL - nock('https://example.com').get('/token/1').reply(200, { - name: 'Test NFT', - image: 'ipfs://QmTestHash/image.png', - }); + jest + .spyOn(erc1155Standard, 'contractSupportsBase1155Interface') + .mockResolvedValue(true); + jest.spyOn(erc1155Standard, 'getAssetSymbol').mockResolvedValue('TEST'); + jest + .spyOn(erc1155Standard, 'getAssetName') + .mockResolvedValue('Test Token'); + // Mock getTokenURI to return IPFS URI + jest + .spyOn(erc1155Standard, 'getTokenURI') + .mockResolvedValue( + 'ipfs://QmYwAPJzv5CZsnA625s3Xf2nemtYgPpHdWEz79ojWnPbdG', + ); + const ipfsGateway = 'https://ipfs.gateway.com'; const details = await erc1155Standard.getDetails( ERC1155_ADDRESS, - 'https://ipfs.gateway.com', + ipfsGateway, SAMPLE_TOKEN_ID, ); expect(details.standard).toBe('ERC1155'); - expect(details.image).toBe( - 'https://ipfs.gateway.com/ipfs/QmTestHash/image.png', - ); + expect(details.tokenURI).toContain('ipfs.gateway.com'); // Should be converted from IPFS URI + + // Restore original methods + jest.restoreAllMocks(); }); - it('should handle network timeout when fetching metadata', async () => { + it('should handle metadata fetching with network errors', async () => { // Mock successful ERC1155 interface check - nock('https://mainnet.infura.io:443', { encodedQueryParams: true }) - .post('/v3/341eacb578dd44a1a049cbc5f6fd4035') - .reply(200, { - jsonrpc: '2.0', - id: 1, - result: - '0x0000000000000000000000000000000000000000000000000000000000000001', - }) - .persist(); - - // Mock tokenURI call - nock('https://mainnet.infura.io:443', { encodedQueryParams: true }) - .post('/v3/341eacb578dd44a1a049cbc5f6fd4035') - .reply(200, { - jsonrpc: '2.0', - id: 2, - result: - '0x0000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000001968747470733a2f2f6578616d706c652e636f6d2f746f6b656e2f31000000', - }); - - // Mock network timeout + jest + .spyOn(erc1155Standard, 'contractSupportsBase1155Interface') + .mockResolvedValue(true); + jest.spyOn(erc1155Standard, 'getAssetSymbol').mockResolvedValue('TEST'); + jest + .spyOn(erc1155Standard, 'getAssetName') + .mockResolvedValue('Test Token'); + jest + .spyOn(erc1155Standard, 'getTokenURI') + .mockResolvedValue('https://example.com/metadata.json'); + + // Mock fetch to fail - this tests the catch block in getDetails nock('https://example.com') - .get('/token/1') - .replyWithError('Network timeout'); + .get('/metadata.json') + .replyWithError('Network error'); + const ipfsGateway = 'https://ipfs.gateway.com'; const details = await erc1155Standard.getDetails( ERC1155_ADDRESS, - 'https://ipfs.gateway.com', + ipfsGateway, SAMPLE_TOKEN_ID, ); - // Should handle timeout gracefully and still return basic details expect(details.standard).toBe('ERC1155'); - expect(details.image).toBeUndefined(); + expect(details.tokenURI).toBe('https://example.com/metadata.json'); + expect(details.image).toBeUndefined(); // Should be undefined due to fetch error + + // Restore original methods + jest.restoreAllMocks(); }); }); });