From b93d09acc901ebea9e8311e15d11d70fc24b489f Mon Sep 17 00:00:00 2001 From: salimtb Date: Sun, 3 Nov 2024 00:12:46 +0100 Subject: [PATCH 1/8] fix: multi-chain-detection --- .../src/TokenDetectionController.ts | 180 +++++++++++------- 1 file changed, 108 insertions(+), 72 deletions(-) diff --git a/packages/assets-controllers/src/TokenDetectionController.ts b/packages/assets-controllers/src/TokenDetectionController.ts index 52a84bfc01..f5efe1c35b 100644 --- a/packages/assets-controllers/src/TokenDetectionController.ts +++ b/packages/assets-controllers/src/TokenDetectionController.ts @@ -147,7 +147,7 @@ export type TokenDetectionControllerMessenger = RestrictedControllerMessenger< /** The input to start polling for the {@link TokenDetectionController} */ type TokenDetectionPollingInput = { - networkClientId: NetworkClientId; + chainIds: Hex[]; address: string; }; @@ -394,28 +394,6 @@ export class TokenDetectionController extends StaticIntervalPollingController { - const isNetworkClientIdChanged = - this.#networkClientId !== selectedNetworkClientId; - - const { chainId: newChainId } = - this.#getCorrectChainIdAndNetworkClientId(selectedNetworkClientId); - this.#isDetectionEnabledForNetwork = - isTokenDetectionSupportedForNetwork(newChainId); - - if (isNetworkClientIdChanged && this.#isDetectionEnabledForNetwork) { - this.#networkClientId = selectedNetworkClientId; - await this.#restartTokenDetection({ - networkClientId: this.#networkClientId, - }); - } - }, - ); } /** @@ -501,6 +479,52 @@ export class TokenDetectionController extends StaticIntervalPollingController { + const configuration = networkConfigurationsByChainId[chainId]; + + if (!configuration) { + console.error(`No configuration found for chainId: ${chainId}`); + return { chainId, networkClientId: '' }; // TODO: handle this case where chainId is defined but there's no rpc added + } + + const { rpcEndpoints } = configuration; + + const matchingEndpoint = rpcEndpoints.find( + (endpoint) => endpoint.networkClientId === selectedNetworkClientId, + ); + + return { + chainId, + networkClientId: matchingEndpoint + ? matchingEndpoint.networkClientId + : rpcEndpoints[0].networkClientId, + }; + }); + } + #getCorrectChainIdAndNetworkClientId(networkClientId?: NetworkClientId): { chainId: Hex; networkClientId: NetworkClientId; @@ -533,14 +557,14 @@ export class TokenDetectionController extends StaticIntervalPollingController { if (!this.isActive) { return; } await this.detectTokens({ - networkClientId, + chainIds, selectedAddress: address, }); } @@ -551,17 +575,17 @@ export class TokenDetectionController extends StaticIntervalPollingController { await this.detectTokens({ - networkClientId, + chainIds, selectedAddress, }); this.setIntervalLength(DEFAULT_INTERVAL); @@ -572,14 +596,14 @@ export class TokenDetectionController extends StaticIntervalPollingController { if (!this.isActive) { @@ -588,56 +612,68 @@ export class TokenDetectionController extends StaticIntervalPollingController - this.#addDetectedTokens({ - tokensSlice, + const isTokenDetectionInactiveInMainnet = + !this.#isDetectionEnabledFromPreferences && + chainIdAgainstWhichToDetect === ChainId.mainnet; + + const { tokensChainsCache } = this.messagingSystem.call( + 'TokenListController:getState', + ); + this.#tokensChainsCache = isTokenDetectionInactiveInMainnet + ? this.#getConvertedStaticMainnetTokenList() + : tokensChainsCache ?? {}; + + const tokenCandidateSlices = this.#getSlicesOfTokensToDetect({ + chainId: chainIdAgainstWhichToDetect, selectedAddress: addressAgainstWhichToDetect, - networkClientId: networkClientIdAgainstWhichToDetect, + }); + + const accountAPIResult = await this.#addDetectedTokensViaAPI({ chainId: chainIdAgainstWhichToDetect, - }), - ); + selectedAddress: addressAgainstWhichToDetect, + tokenCandidateSlices, + }); + + if (accountAPIResult?.result === 'success') { + continue; + } + + const tokenDetectionPromises = tokenCandidateSlices.map((tokensSlice) => + this.#addDetectedTokens({ + tokensSlice, + selectedAddress: addressAgainstWhichToDetect, + networkClientId: networkClientIdAgainstWhichToDetect, + chainId: chainIdAgainstWhichToDetect, + }), + ); - await Promise.all(tokenDetectionPromises); + await Promise.all(tokenDetectionPromises); + } } #getSlicesOfTokensToDetect({ From 9ed5f2ffd80fa7759ea29c782086593498f96474 Mon Sep 17 00:00:00 2001 From: salimtb Date: Mon, 4 Nov 2024 15:19:10 +0100 Subject: [PATCH 2/8] fix: fix unit tests --- .../src/TokenDetectionController.test.ts | 97 ++++--------------- 1 file changed, 18 insertions(+), 79 deletions(-) diff --git a/packages/assets-controllers/src/TokenDetectionController.test.ts b/packages/assets-controllers/src/TokenDetectionController.test.ts index 7413ad9ff2..526fd00046 100644 --- a/packages/assets-controllers/src/TokenDetectionController.test.ts +++ b/packages/assets-controllers/src/TokenDetectionController.test.ts @@ -1422,67 +1422,6 @@ describe('TokenDetectionController', () => { }); describe('when "disabled" is false', () => { - it('should detect new tokens after switching network client id', async () => { - const mockGetBalancesInSingleCall = jest.fn().mockResolvedValue({ - [sampleTokenA.address]: new BN(1), - }); - const selectedAccount = createMockInternalAccount({ - address: '0x0000000000000000000000000000000000000001', - }); - await withController( - { - options: { - disabled: false, - getBalancesInSingleCall: mockGetBalancesInSingleCall, - }, - mocks: { - getAccount: selectedAccount, - getSelectedAccount: selectedAccount, - }, - }, - async ({ - mockTokenListGetState, - callActionSpy, - triggerNetworkDidChange, - }) => { - mockTokenListGetState({ - ...getDefaultTokenListState(), - tokensChainsCache: { - '0x89': { - timestamp: 0, - data: { - [sampleTokenA.address]: { - name: sampleTokenA.name, - symbol: sampleTokenA.symbol, - decimals: sampleTokenA.decimals, - address: sampleTokenA.address, - occurrences: 1, - aggregators: sampleTokenA.aggregators, - iconUrl: sampleTokenA.image, - }, - }, - }, - }, - }); - - triggerNetworkDidChange({ - ...getDefaultNetworkControllerState(), - selectedNetworkClientId: 'polygon', - }); - await advanceTime({ clock, duration: 1 }); - - expect(callActionSpy).toHaveBeenCalledWith( - 'TokensController:addDetectedTokens', - [sampleTokenA], - { - chainId: '0x89', - selectedAddress: selectedAccount.address, - }, - ); - }, - ); - }); - it('should not detect new tokens after switching to a chain that does not support token detection', async () => { const mockGetBalancesInSingleCall = jest.fn().mockResolvedValue({ [sampleTokenA.address]: new BN(1), @@ -2208,33 +2147,33 @@ describe('TokenDetectionController', () => { }); controller.startPolling({ - networkClientId: 'mainnet', + chainIds: ['0x1'], address: '0x1', }); controller.startPolling({ - networkClientId: 'sepolia', + chainIds: ['0xaa36a7'], address: '0xdeadbeef', }); controller.startPolling({ - networkClientId: 'goerli', + chainIds: ['0x5'], address: '0x3', }); await advanceTime({ clock, duration: 0 }); expect(spy.mock.calls).toMatchObject([ - [{ networkClientId: 'mainnet', selectedAddress: '0x1' }], - [{ networkClientId: 'sepolia', selectedAddress: '0xdeadbeef' }], - [{ networkClientId: 'goerli', selectedAddress: '0x3' }], + [{ chainIds: ['0x1'], selectedAddress: '0x1' }], + [{ chainIds: ['0xaa36a7'], selectedAddress: '0xdeadbeef' }], + [{ chainIds: ['0x5'], selectedAddress: '0x3' }], ]); await advanceTime({ clock, duration: DEFAULT_INTERVAL }); expect(spy.mock.calls).toMatchObject([ - [{ networkClientId: 'mainnet', selectedAddress: '0x1' }], - [{ networkClientId: 'sepolia', selectedAddress: '0xdeadbeef' }], - [{ networkClientId: 'goerli', selectedAddress: '0x3' }], - [{ networkClientId: 'mainnet', selectedAddress: '0x1' }], - [{ networkClientId: 'sepolia', selectedAddress: '0xdeadbeef' }], - [{ networkClientId: 'goerli', selectedAddress: '0x3' }], + [{ chainIds: ['0x1'], selectedAddress: '0x1' }], + [{ chainIds: ['0xaa36a7'], selectedAddress: '0xdeadbeef' }], + [{ chainIds: ['0x5'], selectedAddress: '0x3' }], + [{ chainIds: ['0x1'], selectedAddress: '0x1' }], + [{ chainIds: ['0xaa36a7'], selectedAddress: '0xdeadbeef' }], + [{ chainIds: ['0x5'], selectedAddress: '0x3' }], ]); }, ); @@ -2275,7 +2214,7 @@ describe('TokenDetectionController', () => { useTokenDetection: false, }); await controller.detectTokens({ - networkClientId: NetworkType.goerli, + chainIds: ['0x5'], selectedAddress: selectedAccount.address, }); expect(callActionSpy).not.toHaveBeenCalledWith( @@ -2319,7 +2258,7 @@ describe('TokenDetectionController', () => { useTokenDetection: false, }); await controller.detectTokens({ - networkClientId: NetworkType.mainnet, + chainIds: ['0x1'], selectedAddress: selectedAccount.address, }); expect(callActionSpy).toHaveBeenLastCalledWith( @@ -2381,7 +2320,7 @@ describe('TokenDetectionController', () => { }); await controller.detectTokens({ - networkClientId: NetworkType.mainnet, + chainIds: ['0x1'], selectedAddress: selectedAccount.address, }); @@ -2440,7 +2379,7 @@ describe('TokenDetectionController', () => { }); await controller.detectTokens({ - networkClientId: NetworkType.mainnet, + chainIds: ['0x1'], selectedAddress: selectedAccount.address, }); @@ -2505,7 +2444,7 @@ describe('TokenDetectionController', () => { }); await controller.detectTokens({ - networkClientId: NetworkType.mainnet, + chainIds: ['0x1'], }); expect(callActionSpy).toHaveBeenLastCalledWith( @@ -2634,7 +2573,7 @@ describe('TokenDetectionController', () => { // Act await controller.detectTokens({ - networkClientId: NetworkType.mainnet, + chainIds: ['0x1'], selectedAddress: selectedAccount.address, }); From 82e224efbaeea2faea435db8bb963bc34aff4d5a Mon Sep 17 00:00:00 2001 From: salimtb Date: Tue, 5 Nov 2024 15:52:52 +0100 Subject: [PATCH 3/8] fix: fix PR comments --- .../src/TokenDetectionController.test.ts | 48 ++- .../src/TokenDetectionController.ts | 316 ++++++++++++++---- .../mocks/mock-get-balances.ts | 18 + 3 files changed, 311 insertions(+), 71 deletions(-) diff --git a/packages/assets-controllers/src/TokenDetectionController.test.ts b/packages/assets-controllers/src/TokenDetectionController.test.ts index 526fd00046..b4092a3725 100644 --- a/packages/assets-controllers/src/TokenDetectionController.test.ts +++ b/packages/assets-controllers/src/TokenDetectionController.test.ts @@ -329,6 +329,7 @@ describe('TokenDetectionController', () => { { options: { getBalancesInSingleCall: mockGetBalancesInSingleCall, + useAccountsAPI: true, // USING ACCOUNTS API }, mocks: { getSelectedAccount: defaultSelectedAccount, @@ -363,13 +364,17 @@ describe('TokenDetectionController', () => { { options: { getBalancesInSingleCall: mockGetBalancesInSingleCall, + useAccountsAPI: true, // USING ACCOUNTS API }, mocks: { getAccount: selectedAccount, getSelectedAccount: selectedAccount, }, }, + + // Salim .... async ({ controller, mockTokenListGetState, callActionSpy }) => { + mockMultiChainAccountsService(); mockTokenListGetState({ ...getDefaultTokenListState(), tokensChainsCache: { @@ -392,6 +397,8 @@ describe('TokenDetectionController', () => { await controller.start(); + console.log(callActionSpy.mock.calls); + expect(callActionSpy).toHaveBeenCalledWith( 'TokensController:addDetectedTokens', [sampleTokenA], @@ -415,6 +422,7 @@ describe('TokenDetectionController', () => { { options: { getBalancesInSingleCall: mockGetBalancesInSingleCall, + useAccountsAPI: true, // USING ACCOUNTS API }, mocks: { getAccount: selectedAccount, @@ -428,6 +436,7 @@ describe('TokenDetectionController', () => { mockGetNetworkClientById, callActionSpy, }) => { + mockMultiChainAccountsService(); mockNetworkState({ ...getDefaultNetworkControllerState(), selectedNetworkClientId: 'polygon', @@ -473,7 +482,7 @@ describe('TokenDetectionController', () => { ); }); - it('should update detectedTokens when new tokens are detected', async () => { + it.skip('should update detectedTokens when new tokens are detected', async () => { const mockGetBalancesInSingleCall = jest.fn().mockResolvedValue({ [sampleTokenA.address]: new BN(1), [sampleTokenB.address]: new BN(1), @@ -487,6 +496,7 @@ describe('TokenDetectionController', () => { options: { getBalancesInSingleCall: mockGetBalancesInSingleCall, interval, + useAccountsAPI: true, // USING ACCOUNTS API }, mocks: { getAccount: selectedAccount, @@ -494,6 +504,7 @@ describe('TokenDetectionController', () => { }, }, async ({ controller, mockTokenListGetState, callActionSpy }) => { + mockMultiChainAccountsService(); const tokenListState = { ...getDefaultTokenListState(), tokensChainsCache: { @@ -551,6 +562,7 @@ describe('TokenDetectionController', () => { { options: { getBalancesInSingleCall: mockGetBalancesInSingleCall, + useAccountsAPI: true, // USING ACCOUNTS API }, mocks: { getAccount: selectedAccount, @@ -563,6 +575,7 @@ describe('TokenDetectionController', () => { mockTokenListGetState, callActionSpy, }) => { + mockMultiChainAccountsService(); mockTokensGetState({ ...getDefaultTokensState(), ignoredTokens: [sampleTokenA.address], @@ -604,12 +617,14 @@ describe('TokenDetectionController', () => { { options: { getBalancesInSingleCall: mockGetBalancesInSingleCall, + useAccountsAPI: true, // USING ACCOUNTS API }, mocks: { getSelectedAccount: defaultSelectedAccount, }, }, async ({ controller, mockTokenListGetState, callActionSpy }) => { + mockMultiChainAccountsService(); mockTokenListGetState({ ...getDefaultTokenListState(), tokensChainsCache: { @@ -666,6 +681,7 @@ describe('TokenDetectionController', () => { options: { disabled: false, getBalancesInSingleCall: mockGetBalancesInSingleCall, + useAccountsAPI: true, // USING ACCOUNTS API }, mocks: { getSelectedAccount: firstSelectedAccount, @@ -677,6 +693,7 @@ describe('TokenDetectionController', () => { triggerSelectedAccountChange, callActionSpy, }) => { + mockMultiChainAccountsService(); mockTokenListGetState({ ...getDefaultTokenListState(), tokensChainsCache: { @@ -725,6 +742,7 @@ describe('TokenDetectionController', () => { options: { disabled: false, getBalancesInSingleCall: mockGetBalancesInSingleCall, + useAccountsAPI: true, // USING ACCOUNTS API }, mocks: { getSelectedAccount: selectedAccount, @@ -735,6 +753,7 @@ describe('TokenDetectionController', () => { triggerSelectedAccountChange, callActionSpy, }) => { + mockMultiChainAccountsService(); mockTokenListGetState({ ...getDefaultTokenListState(), tokensChainsCache: { @@ -844,6 +863,7 @@ describe('TokenDetectionController', () => { options: { disabled: true, getBalancesInSingleCall: mockGetBalancesInSingleCall, + useAccountsAPI: true, // USING ACCOUNTS API }, mocks: { getSelectedAccount: firstSelectedAccount, @@ -914,6 +934,7 @@ describe('TokenDetectionController', () => { options: { disabled: false, getBalancesInSingleCall: mockGetBalancesInSingleCall, + useAccountsAPI: true, // USING ACCOUNTS API }, mocks: { getSelectedAccount: firstSelectedAccount, @@ -926,6 +947,7 @@ describe('TokenDetectionController', () => { triggerSelectedAccountChange, callActionSpy, }) => { + mockMultiChainAccountsService(); mockTokenListGetState({ ...getDefaultTokenListState(), tokensChainsCache: { @@ -978,6 +1000,7 @@ describe('TokenDetectionController', () => { options: { disabled: false, getBalancesInSingleCall: mockGetBalancesInSingleCall, + useAccountsAPI: true, // USING ACCOUNTS API }, mocks: { getSelectedAccount: selectedAccount, @@ -989,6 +1012,7 @@ describe('TokenDetectionController', () => { triggerPreferencesStateChange, callActionSpy, }) => { + mockMultiChainAccountsService(); mockGetAccount(selectedAccount); mockTokenListGetState({ ...getDefaultTokenListState(), @@ -1049,6 +1073,7 @@ describe('TokenDetectionController', () => { options: { disabled: false, getBalancesInSingleCall: mockGetBalancesInSingleCall, + useAccountsAPI: true, // USING ACCOUNTS API }, mocks: { getSelectedAccount: firstSelectedAccount, @@ -1061,6 +1086,7 @@ describe('TokenDetectionController', () => { triggerPreferencesStateChange, callActionSpy, }) => { + mockMultiChainAccountsService(); mockGetAccount(firstSelectedAccount); mockTokenListGetState({ ...getDefaultTokenListState(), @@ -1676,6 +1702,7 @@ describe('TokenDetectionController', () => { options: { disabled: false, getBalancesInSingleCall: mockGetBalancesInSingleCall, + useAccountsAPI: true, // USING ACCOUNTS API }, mocks: { getSelectedAccount: selectedAccount, @@ -1687,6 +1714,7 @@ describe('TokenDetectionController', () => { callActionSpy, triggerTokenListStateChange, }) => { + mockMultiChainAccountsService(); const tokenList = { [sampleTokenA.address]: { name: sampleTokenA.name, @@ -1890,6 +1918,7 @@ describe('TokenDetectionController', () => { options: { disabled: false, getBalancesInSingleCall: mockGetBalancesInSingleCall, + useAccountsAPI: true, // USING ACCOUNTS API }, mocks: { getSelectedAccount: selectedAccount, @@ -1901,6 +1930,7 @@ describe('TokenDetectionController', () => { triggerTokenListStateChange, controller, }) => { + mockMultiChainAccountsService(); const tokenListState = { ...getDefaultTokenListState(), tokensChainsCache: { @@ -1949,6 +1979,7 @@ describe('TokenDetectionController', () => { options: { disabled: false, getBalancesInSingleCall: mockGetBalancesInSingleCall, + useAccountsAPI: true, // USING ACCOUNTS API }, mocks: { getSelectedAccount: selectedAccount, @@ -1960,6 +1991,7 @@ describe('TokenDetectionController', () => { triggerTokenListStateChange, controller, }) => { + mockMultiChainAccountsService(); const tokenListState = { ...getDefaultTokenListState(), tokensChainsCache: { @@ -2026,6 +2058,7 @@ describe('TokenDetectionController', () => { options: { disabled: false, getBalancesInSingleCall: mockGetBalancesInSingleCall, + useAccountsAPI: true, // USING ACCOUNTS API }, mocks: { getSelectedAccount: selectedAccount, @@ -2037,6 +2070,7 @@ describe('TokenDetectionController', () => { triggerTokenListStateChange, controller, }) => { + mockMultiChainAccountsService(); const tokenListState = { ...getDefaultTokenListState(), tokensChainsCache: { @@ -2193,6 +2227,7 @@ describe('TokenDetectionController', () => { options: { disabled: false, getBalancesInSingleCall: mockGetBalancesInSingleCall, + useAccountsAPI: true, // USING ACCOUNTS API }, mocks: { getSelectedAccount: selectedAccount, @@ -2205,6 +2240,7 @@ describe('TokenDetectionController', () => { triggerPreferencesStateChange, callActionSpy, }) => { + mockMultiChainAccountsService(); mockNetworkState({ ...getDefaultNetworkControllerState(), selectedNetworkClientId: NetworkType.goerli, @@ -2224,7 +2260,7 @@ describe('TokenDetectionController', () => { ); }); - it('should detect and add tokens from the `@metamask/contract-metadata` legacy token list if token detection is disabled and current network is mainnet', async () => { + it.skip('should detect and add tokens from the `@metamask/contract-metadata` legacy token list if token detection is disabled and current network is mainnet', async () => { const mockGetBalancesInSingleCall = jest.fn().mockResolvedValue( Object.keys(STATIC_MAINNET_TOKEN_LIST).reduce>( (acc, address) => { @@ -2242,6 +2278,7 @@ describe('TokenDetectionController', () => { options: { disabled: false, getBalancesInSingleCall: mockGetBalancesInSingleCall, + useAccountsAPI: true, // USING ACCOUNTS API }, mocks: { getSelectedAccount: selectedAccount, @@ -2253,6 +2290,7 @@ describe('TokenDetectionController', () => { triggerPreferencesStateChange, callActionSpy, }) => { + mockMultiChainAccountsService(); triggerPreferencesStateChange({ ...getDefaultPreferencesState(), useTokenDetection: false, @@ -2292,6 +2330,7 @@ describe('TokenDetectionController', () => { options: { disabled: false, getBalancesInSingleCall: mockGetBalancesInSingleCall, + useAccountsAPI: true, // USING ACCOUNTS API }, mocks: { getSelectedAccount: selectedAccount, @@ -2299,6 +2338,7 @@ describe('TokenDetectionController', () => { }, }, async ({ controller, mockTokenListGetState, callActionSpy }) => { + mockMultiChainAccountsService(); mockTokenListGetState({ ...getDefaultTokenListState(), tokensChainsCache: { @@ -2351,6 +2391,7 @@ describe('TokenDetectionController', () => { disabled: false, getBalancesInSingleCall: mockGetBalancesInSingleCall, trackMetaMetricsEvent: mockTrackMetaMetricsEvent, + useAccountsAPI: true, // USING ACCOUNTS API }, mocks: { getSelectedAccount: selectedAccount, @@ -2358,6 +2399,7 @@ describe('TokenDetectionController', () => { }, }, async ({ controller, mockTokenListGetState }) => { + mockMultiChainAccountsService(); mockTokenListGetState({ ...getDefaultTokenListState(), tokensChainsCache: { @@ -2413,6 +2455,7 @@ describe('TokenDetectionController', () => { disabled: false, getBalancesInSingleCall: mockGetBalancesInSingleCall, trackMetaMetricsEvent: mockTrackMetaMetricsEvent, + useAccountsAPI: true, // USING ACCOUNTS API }, }, async ({ @@ -2421,6 +2464,7 @@ describe('TokenDetectionController', () => { mockTokenListGetState, callActionSpy, }) => { + mockMultiChainAccountsService(); // @ts-expect-error forcing an undefined value mockGetAccount(undefined); mockTokenListGetState({ diff --git a/packages/assets-controllers/src/TokenDetectionController.ts b/packages/assets-controllers/src/TokenDetectionController.ts index f5efe1c35b..ea155ed2dd 100644 --- a/packages/assets-controllers/src/TokenDetectionController.ts +++ b/packages/assets-controllers/src/TokenDetectionController.ts @@ -242,6 +242,35 @@ export class TokenDetectionController extends StaticIntervalPollingController hexToNumber(chainId)); + + const supportedNetworks = await this.getSupportedNetworks(); + + if ( + !supportedNetworks || + !chainIdNumbers.every((id) => supportedNetworks.includes(id)) + ) { + const supportedNetworksErrStr = (supportedNetworks ?? []).toString(); + throw new Error( + `Unsupported Network: supported networks ${supportedNetworksErrStr}, requested networks: ${chainIdNumbers.toString()}`, + ); + } + + const result = await fetchMultiChainBalances( + address, + { + networks: chainIdNumbers, + }, + this.platform, + ); + return result.balances; }, }; @@ -306,6 +335,7 @@ export class TokenDetectionController extends StaticIntervalPollingController((acc, { chainId }) => { + if (supportedNetworksOnAccountAPI.includes(hexToNumber(chainId))) { + acc.push(chainId); + } + return acc; + }, []); + + chainsToDetectUsingRpc = clientNetworksIdByChainId.reduce< + { chainId: `0x${string}`; networkClientId: string }[] + >((acc, { chainId, networkClientId }) => { + if (!supportedNetworksOnAccountAPI.includes(hexToNumber(chainId))) { + acc.push({ chainId, networkClientId }); + } + return acc; + }, []); + + const accountAPIResult = await this.#addDetectedTokensViaAPI({ + chainIds: chainsToDetectUsingAccountAPI, + selectedAddress: addressAgainstWhichToDetect, + }); + + if (accountAPIResult?.result === 'success') { + return; + } + // If API fails, add all chains from chainsToDetectUsingAccountAPI to chainsToDetectUsingRpc + chainsToDetectUsingAccountAPI.forEach((chainId) => { + const networkEntry = clientNetworksIdByChainId.find( + (network) => network.chainId === chainId, + ); + if (networkEntry) { + chainsToDetectUsingRpc.push({ + chainId: networkEntry.chainId, + networkClientId: networkEntry.networkClientId, + }); + } + }); + + console.log('result ----', accountAPIResult); + } else { + // Fallback to using all client networks if supportedNetworksOnAccountAPI is null + chainsToDetectUsingRpc = clientNetworksIdByChainId; + } + // Execute the rest in a loop for each pair of chainId and networkClientId in the result array for (const { chainId: chainIdAgainstWhichToDetect, networkClientId: networkClientIdAgainstWhichToDetect, - } of clientNetworksIdByChainId) { + } of chainsToDetectUsingRpc || []) { if (!isTokenDetectionSupportedForNetwork(chainIdAgainstWhichToDetect)) { continue; } @@ -653,16 +742,6 @@ export class TokenDetectionController extends StaticIntervalPollingController this.#addDetectedTokens({ tokensSlice, @@ -750,93 +829,190 @@ export class TokenDetectionController extends StaticIntervalPollingController { - const tokenBalances = await this.#accountsAPI - .getMultiChainBalances(selectedAddress, chainId) + // Fetch balances for multiple chain IDs at once + const tokenBalancesByChain = await this.#accountsAPI + .getMultiNetworksBalances(selectedAddress, chainIds) .catch(() => null); - if (!tokenBalances || tokenBalances.length === 0) { + if ( + !tokenBalancesByChain || + Object.keys(tokenBalancesByChain).length === 0 + ) { return { result: 'failed' } as const; } - const tokensWithBalance: Token[] = []; - const eventTokensDetails: string[] = []; + console.log('chainIds +++++', chainIds); + // Process each chain ID individually + for (const chainId of chainIds) { + const isTokenDetectionInactiveInMainnet = + !this.#isDetectionEnabledFromPreferences && + chainId === ChainId.mainnet; + const { tokensChainsCache } = this.messagingSystem.call( + 'TokenListController:getState', + ); + this.#tokensChainsCache = isTokenDetectionInactiveInMainnet + ? this.#getConvertedStaticMainnetTokenList() + : tokensChainsCache ?? {}; + + // Generate token candidates based on chainId and selectedAddress + const tokenCandidateSlices = this.#getSlicesOfTokensToDetect({ + chainId, + selectedAddress, + }); - const tokenCandidateSet = new Set(tokenCandidateSlices.flat()); + console.log('tokenBalancesByChain -----', tokenBalancesByChain); - tokenBalances.forEach((token) => { - const tokenAddress = token.address; + // Filter balances for the current chainId + const tokenBalances = tokenBalancesByChain.filter( + (balance) => balance.chainId === hexToNumber(chainId), + ); - // Make sure that the token to add is in our candidate list - // Ensures we don't add tokens we already own - if (!tokenCandidateSet.has(token.address)) { - return; - } + console.log('tokenBalances -----++++', tokenBalances); - // We need specific data from tokensChainsCache to correctly create a token - // So even if we have a token that was detected correctly by the API, if its missing data we cannot safely add it. - if (!this.#tokensChainsCache[chainId].data[token.address]) { - return; + if (!tokenBalances || tokenBalances.length === 0) { + continue; } - const { decimals, symbol, aggregators, iconUrl, name } = - this.#tokensChainsCache[chainId].data[token.address]; - eventTokensDetails.push(`${symbol} - ${tokenAddress}`); - tokensWithBalance.push({ - address: tokenAddress, - decimals, - symbol, - aggregators, - image: iconUrl, - isERC721: false, - name, - }); - }); + // Use helper function to filter tokens with balance for this chainId + const { tokensWithBalance, eventTokensDetails } = + this.#filterAndBuildTokensWithBalance( + tokenCandidateSlices, + tokenBalances, + chainId, + ); - if (tokensWithBalance.length) { - this.#trackMetaMetricsEvent({ - event: 'Token Detected', - category: 'Wallet', - properties: { - tokens: eventTokensDetails, - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/naming-convention - token_standard: ERC20, - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/naming-convention - asset_type: ASSET_TYPES.TOKEN, - }, - }); + console.log('tokensWithBalance +++++', tokensWithBalance); - await this.messagingSystem.call( - 'TokensController:addDetectedTokens', - tokensWithBalance, - { + if (tokensWithBalance.length) { + console.log( + 'INSIDE IF ===================>', + tokensWithBalance, selectedAddress, chainId, - }, - ); + ); + this.#trackMetaMetricsEvent({ + event: 'Token Detected', + category: 'Wallet', + properties: { + tokens: eventTokensDetails, + // TODO: Either fix this lint violation or explain why it's necessary to ignore. + // eslint-disable-next-line @typescript-eslint/naming-convention + token_standard: ERC20, + // TODO: Either fix this lint violation or explain why it's necessary to ignore. + // eslint-disable-next-line @typescript-eslint/naming-convention + asset_type: ASSET_TYPES.TOKEN, + }, + }); + + console.log( + 'INSIDE IF ===================>', + tokensWithBalance, + selectedAddress, + chainId, + ); + await this.messagingSystem.call( + 'TokensController:addDetectedTokens', + tokensWithBalance, + { + selectedAddress, + chainId, + }, + ); + } } return { result: 'success' } as const; }); } + /** + * Helper function to filter and build token data for detected tokens + * @param options.tokenCandidateSlices - these are tokens we know a user does not have (by checking the tokens controller). + * We will use these these token candidates to determine if a token found from the API is valid to be added on the users wallet. + * It will also prevent us to adding tokens a user already has + * @param tokenBalances - Tokens balances fetched from API + * @param chainId - The chain ID being processed + * @returns an object containing tokensWithBalance and eventTokensDetails arrays + */ + + #filterAndBuildTokensWithBalance( + tokenCandidateSlices: string[][], + tokenBalances: + | { + object: string; + type?: string; + timestamp?: string; + address: string; + symbol: string; + name: string; + decimals: number; + chainId: number; + balance: string; + }[] + | null, + chainId: Hex, + ) { + const tokensWithBalance: Token[] = []; + const eventTokensDetails: string[] = []; + + const tokenCandidateSet = new Set(tokenCandidateSlices.flat()); + + console.log('tokenCandidateSet -----', tokenCandidateSet); + console.log('tokenBalances -----', tokenBalances); + + tokenBalances?.forEach((token) => { + const tokenAddress = token.address; + + console.log('tokenAddress -----', tokenAddress); + + console.log('tokenCandidateSet -----', tokenCandidateSet); + console.log( + '!tokenCandidateSet.has(tokenAddress) -----', + !tokenCandidateSet.has(tokenAddress), + ); + + // Make sure the token to add is in our candidate list + if (!tokenCandidateSet.has(tokenAddress)) { + return; + } + + console.log('tokenAddress -----', tokenAddress); + + // Retrieve token data from cache to safely add it + const tokenData = this.#tokensChainsCache[chainId]?.data[tokenAddress]; + + console.log('tokenData ********', tokenData); + if (!tokenData) { + return; + } + + const { decimals, symbol, aggregators, iconUrl, name } = tokenData; + eventTokensDetails.push(`${symbol} - ${tokenAddress}`); + tokensWithBalance.push({ + address: tokenAddress, + decimals, + symbol, + aggregators, + image: iconUrl, + isERC721: false, + name, + }); + }); + + return { tokensWithBalance, eventTokensDetails }; + } + async #addDetectedTokens({ tokensSlice, selectedAddress, @@ -848,6 +1024,7 @@ export class TokenDetectionController extends StaticIntervalPollingController { + console.log('INSIDE addDetectedTokens:::::'); await safelyExecute(async () => { const balances = await this.#getBalancesInSingleCall( selectedAddress, @@ -872,6 +1049,7 @@ export class TokenDetectionController extends StaticIntervalPollingController Date: Tue, 5 Nov 2024 22:33:29 +0100 Subject: [PATCH 4/8] fix: refacto --- .../src/TokenDetectionController.ts | 229 ++++++++++-------- 1 file changed, 126 insertions(+), 103 deletions(-) diff --git a/packages/assets-controllers/src/TokenDetectionController.ts b/packages/assets-controllers/src/TokenDetectionController.ts index ea155ed2dd..5dbccaa31f 100644 --- a/packages/assets-controllers/src/TokenDetectionController.ts +++ b/packages/assets-controllers/src/TokenDetectionController.ts @@ -70,6 +70,11 @@ type TokenDetectionMap = { [P in keyof TokenListMap]: Omit; }; +type NetworkClient = { + chainId: Hex; + networkClientId: string; +}; + export const STATIC_MAINNET_TOKEN_LIST = Object.entries( contractMap, ).reduce((acc, [base, contract]) => { @@ -621,6 +626,103 @@ export class TokenDetectionController extends StaticIntervalPollingController { + if (supportedNetworks?.includes(hexToNumber(chainId))) { + chainsToDetectUsingAccountAPI.push(chainId); + } else { + chainsToDetectUsingRpc.push({ chainId, networkClientId }); + } + }); + + return { chainsToDetectUsingRpc, chainsToDetectUsingAccountAPI }; + } + + async #attemptAccountAPIDetection( + chainsToDetectUsingAccountAPI: Hex[], + addressToDetect: string, + ) { + return await this.#addDetectedTokensViaAPI({ + chainIds: chainsToDetectUsingAccountAPI, + selectedAddress: addressToDetect, + }); + } + + #addChainsToRpcDetection( + chainsToDetectUsingRpc: NetworkClient[], + chainsToDetectUsingAccountAPI: Hex[], + clientNetworks: NetworkClient[], + ): void { + chainsToDetectUsingAccountAPI.forEach((chainId) => { + const networkEntry = clientNetworks.find( + (network) => network.chainId === chainId, + ); + if (networkEntry) { + chainsToDetectUsingRpc.push({ + chainId: networkEntry.chainId, + networkClientId: networkEntry.networkClientId, + }); + } + }); + } + + #shouldDetectTokens(chainId: Hex): boolean { + if (!isTokenDetectionSupportedForNetwork(chainId)) { + return false; + } + if ( + !this.#isDetectionEnabledFromPreferences && + chainId !== ChainId.mainnet + ) { + return false; + } + + const isMainnetDetectionInactive = + !this.#isDetectionEnabledFromPreferences && chainId === ChainId.mainnet; + if (isMainnetDetectionInactive) { + this.#tokensChainsCache = this.#getConvertedStaticMainnetTokenList(); + } else { + const { tokensChainsCache } = this.messagingSystem.call( + 'TokenListController:getState', + ); + this.#tokensChainsCache = tokensChainsCache ?? {}; + } + + return true; + } + + async #detectTokensUsingRpc( + chainsToDetectUsingRpc: NetworkClient[], + addressToDetect: string, + ): Promise { + for (const { chainId, networkClientId } of chainsToDetectUsingRpc) { + if (!this.#shouldDetectTokens(chainId)) { + continue; + } + + const tokenCandidateSlices = this.#getSlicesOfTokensToDetect({ + chainId, + selectedAddress: addressToDetect, + }); + const tokenDetectionPromises = tokenCandidateSlices.map((tokensSlice) => + this.#addDetectedTokens({ + tokensSlice, + selectedAddress: addressToDetect, + networkClientId, + chainId, + }), + ); + + await Promise.all(tokenDetectionPromises); + } + } + /** * For each token in the token list provided by the TokenListController, checks the token's balance for the selected account address on the active network. * On mainnet, if token detection is disabled in preferences, ERC20 token auto detection will be triggered for each contract address in the legacy token list from the @metamask/contract-metadata repo. @@ -640,119 +742,40 @@ export class TokenDetectionController extends StaticIntervalPollingController((acc, { chainId }) => { - if (supportedNetworksOnAccountAPI.includes(hexToNumber(chainId))) { - acc.push(chainId); - } - return acc; - }, []); - - chainsToDetectUsingRpc = clientNetworksIdByChainId.reduce< - { chainId: `0x${string}`; networkClientId: string }[] - >((acc, { chainId, networkClientId }) => { - if (!supportedNetworksOnAccountAPI.includes(hexToNumber(chainId))) { - acc.push({ chainId, networkClientId }); - } - return acc; - }, []); - - const accountAPIResult = await this.#addDetectedTokensViaAPI({ - chainIds: chainsToDetectUsingAccountAPI, - selectedAddress: addressAgainstWhichToDetect, - }); + const supportedNetworks = await this.#accountsAPI.getSupportedNetworks(); + const { chainsToDetectUsingRpc, chainsToDetectUsingAccountAPI } = + this.#getChainsToDetect(clientNetworks, supportedNetworks); - if (accountAPIResult?.result === 'success') { + // Try detecting tokens via Account API first, fallback to RPC if API fails + if (supportedNetworks) { + const apiResult = await this.#attemptAccountAPIDetection( + chainsToDetectUsingAccountAPI, + addressToDetect, + ); + if (apiResult?.result === 'success') { return; } - // If API fails, add all chains from chainsToDetectUsingAccountAPI to chainsToDetectUsingRpc - chainsToDetectUsingAccountAPI.forEach((chainId) => { - const networkEntry = clientNetworksIdByChainId.find( - (network) => network.chainId === chainId, - ); - if (networkEntry) { - chainsToDetectUsingRpc.push({ - chainId: networkEntry.chainId, - networkClientId: networkEntry.networkClientId, - }); - } - }); - - console.log('result ----', accountAPIResult); - } else { - // Fallback to using all client networks if supportedNetworksOnAccountAPI is null - chainsToDetectUsingRpc = clientNetworksIdByChainId; - } - - // Execute the rest in a loop for each pair of chainId and networkClientId in the result array - for (const { - chainId: chainIdAgainstWhichToDetect, - networkClientId: networkClientIdAgainstWhichToDetect, - } of chainsToDetectUsingRpc || []) { - if (!isTokenDetectionSupportedForNetwork(chainIdAgainstWhichToDetect)) { - continue; - } - - if ( - !this.#isDetectionEnabledFromPreferences && - chainIdAgainstWhichToDetect !== ChainId.mainnet - ) { - continue; - } - - const isTokenDetectionInactiveInMainnet = - !this.#isDetectionEnabledFromPreferences && - chainIdAgainstWhichToDetect === ChainId.mainnet; - - const { tokensChainsCache } = this.messagingSystem.call( - 'TokenListController:getState', - ); - this.#tokensChainsCache = isTokenDetectionInactiveInMainnet - ? this.#getConvertedStaticMainnetTokenList() - : tokensChainsCache ?? {}; - - const tokenCandidateSlices = this.#getSlicesOfTokensToDetect({ - chainId: chainIdAgainstWhichToDetect, - selectedAddress: addressAgainstWhichToDetect, - }); - const tokenDetectionPromises = tokenCandidateSlices.map((tokensSlice) => - this.#addDetectedTokens({ - tokensSlice, - selectedAddress: addressAgainstWhichToDetect, - networkClientId: networkClientIdAgainstWhichToDetect, - chainId: chainIdAgainstWhichToDetect, - }), + // Fallback on RPC detection if Account API fails + this.#addChainsToRpcDetection( + chainsToDetectUsingRpc, + chainsToDetectUsingAccountAPI, + clientNetworks, ); - - await Promise.all(tokenDetectionPromises); } + + // If no supported networks, fallback to RPC with all client networks + const finalChainsToDetect = supportedNetworks + ? chainsToDetectUsingRpc + : clientNetworks; + await this.#detectTokensUsingRpc(finalChainsToDetect, addressToDetect); } #getSlicesOfTokensToDetect({ From 285dc76c60694e45eff0c690f63ba12ee0552863 Mon Sep 17 00:00:00 2001 From: salimtb Date: Wed, 6 Nov 2024 10:37:02 +0100 Subject: [PATCH 5/8] fix: fix test units --- .../src/TokenDetectionController.test.ts | 9 +-- .../src/TokenDetectionController.ts | 61 +++---------------- 2 files changed, 13 insertions(+), 57 deletions(-) diff --git a/packages/assets-controllers/src/TokenDetectionController.test.ts b/packages/assets-controllers/src/TokenDetectionController.test.ts index b4092a3725..fc0238d33f 100644 --- a/packages/assets-controllers/src/TokenDetectionController.test.ts +++ b/packages/assets-controllers/src/TokenDetectionController.test.ts @@ -397,8 +397,6 @@ describe('TokenDetectionController', () => { await controller.start(); - console.log(callActionSpy.mock.calls); - expect(callActionSpy).toHaveBeenCalledWith( 'TokensController:addDetectedTokens', [sampleTokenA], @@ -482,7 +480,7 @@ describe('TokenDetectionController', () => { ); }); - it.skip('should update detectedTokens when new tokens are detected', async () => { + it('should update detectedTokens when new tokens are detected', async () => { const mockGetBalancesInSingleCall = jest.fn().mockResolvedValue({ [sampleTokenA.address]: new BN(1), [sampleTokenB.address]: new BN(1), @@ -496,7 +494,6 @@ describe('TokenDetectionController', () => { options: { getBalancesInSingleCall: mockGetBalancesInSingleCall, interval, - useAccountsAPI: true, // USING ACCOUNTS API }, mocks: { getAccount: selectedAccount, @@ -2260,7 +2257,7 @@ describe('TokenDetectionController', () => { ); }); - it.skip('should detect and add tokens from the `@metamask/contract-metadata` legacy token list if token detection is disabled and current network is mainnet', async () => { + it('should detect and add tokens from the `@metamask/contract-metadata` legacy token list if token detection is disabled and current network is mainnet', async () => { const mockGetBalancesInSingleCall = jest.fn().mockResolvedValue( Object.keys(STATIC_MAINNET_TOKEN_LIST).reduce>( (acc, address) => { @@ -2278,7 +2275,7 @@ describe('TokenDetectionController', () => { options: { disabled: false, getBalancesInSingleCall: mockGetBalancesInSingleCall, - useAccountsAPI: true, // USING ACCOUNTS API + // useAccountsAPI: true, // USING ACCOUNTS API }, mocks: { getSelectedAccount: selectedAccount, diff --git a/packages/assets-controllers/src/TokenDetectionController.ts b/packages/assets-controllers/src/TokenDetectionController.ts index 5dbccaa31f..709e1ee852 100644 --- a/packages/assets-controllers/src/TokenDetectionController.ts +++ b/packages/assets-controllers/src/TokenDetectionController.ts @@ -262,6 +262,7 @@ export class TokenDetectionController extends StaticIntervalPollingController supportedNetworks.includes(id)) ) { + console.log('INSIDE IF -------'); const supportedNetworksErrStr = (supportedNetworks ?? []).toString(); throw new Error( `Unsupported Network: supported networks ${supportedNetworksErrStr}, requested networks: ${chainIdNumbers.toString()}`, @@ -534,28 +535,18 @@ export class TokenDetectionController extends StaticIntervalPollingController { const configuration = networkConfigurationsByChainId[chainId]; - if (!configuration) { - console.error(`No configuration found for chainId: ${chainId}`); - return { chainId, networkClientId: '' }; // TODO: handle this case where chainId is defined but there's no rpc added - } - - const { rpcEndpoints } = configuration; - - const matchingEndpoint = rpcEndpoints.find( - (endpoint) => endpoint.networkClientId === selectedNetworkClientId, - ); - return { chainId, - networkClientId: matchingEndpoint - ? matchingEndpoint.networkClientId - : rpcEndpoints[0].networkClientId, + networkClientId: + configuration.rpcEndpoints[configuration.defaultRpcEndpointIndex] + .networkClientId, }; }); } @@ -628,7 +619,7 @@ export class TokenDetectionController extends StaticIntervalPollingController balance.chainId === hexToNumber(chainId), ); - console.log('tokenBalances -----++++', tokenBalances); - if (!tokenBalances || tokenBalances.length === 0) { continue; } @@ -915,15 +904,7 @@ export class TokenDetectionController extends StaticIntervalPollingController', - tokensWithBalance, - selectedAddress, - chainId, - ); this.#trackMetaMetricsEvent({ event: 'Token Detected', category: 'Wallet', @@ -938,12 +919,6 @@ export class TokenDetectionController extends StaticIntervalPollingController', - tokensWithBalance, - selectedAddress, - chainId, - ); await this.messagingSystem.call( 'TokensController:addDetectedTokens', tokensWithBalance, @@ -991,31 +966,17 @@ export class TokenDetectionController extends StaticIntervalPollingController(tokenCandidateSlices.flat()); - console.log('tokenCandidateSet -----', tokenCandidateSet); - console.log('tokenBalances -----', tokenBalances); - tokenBalances?.forEach((token) => { const tokenAddress = token.address; - console.log('tokenAddress -----', tokenAddress); - - console.log('tokenCandidateSet -----', tokenCandidateSet); - console.log( - '!tokenCandidateSet.has(tokenAddress) -----', - !tokenCandidateSet.has(tokenAddress), - ); - // Make sure the token to add is in our candidate list if (!tokenCandidateSet.has(tokenAddress)) { return; } - console.log('tokenAddress -----', tokenAddress); - // Retrieve token data from cache to safely add it const tokenData = this.#tokensChainsCache[chainId]?.data[tokenAddress]; - console.log('tokenData ********', tokenData); if (!tokenData) { return; } @@ -1047,7 +1008,6 @@ export class TokenDetectionController extends StaticIntervalPollingController { - console.log('INSIDE addDetectedTokens:::::'); await safelyExecute(async () => { const balances = await this.#getBalancesInSingleCall( selectedAddress, @@ -1072,7 +1032,6 @@ export class TokenDetectionController extends StaticIntervalPollingController Date: Wed, 6 Nov 2024 13:10:32 +0100 Subject: [PATCH 6/8] fix: clean up + refacto --- .../src/TokenDetectionController.test.ts | 174 ++++++++++++++++++ .../src/TokenDetectionController.ts | 100 +++------- 2 files changed, 201 insertions(+), 73 deletions(-) diff --git a/packages/assets-controllers/src/TokenDetectionController.test.ts b/packages/assets-controllers/src/TokenDetectionController.test.ts index fc0238d33f..305594db82 100644 --- a/packages/assets-controllers/src/TokenDetectionController.test.ts +++ b/packages/assets-controllers/src/TokenDetectionController.test.ts @@ -49,6 +49,7 @@ import { STATIC_MAINNET_TOKEN_LIST, TokenDetectionController, controllerName, + mapChainIdWithTokenListMap, } from './TokenDetectionController'; import { getDefaultTokenListState, @@ -409,6 +410,80 @@ describe('TokenDetectionController', () => { ); }); + it('should not call add tokens if balance is not available on account api', async () => { + const mockGetBalancesInSingleCall = jest.fn().mockResolvedValue({ + [sampleTokenA.address]: new BN(1), + }); + + const selectedAccount = createMockInternalAccount({ + address: '0x0000000000000000000000000000000000000001', + }); + await withController( + { + options: { + getBalancesInSingleCall: mockGetBalancesInSingleCall, + useAccountsAPI: true, // USING ACCOUNTS API + }, + mocks: { + getAccount: selectedAccount, + getSelectedAccount: selectedAccount, + }, + }, + + async ({ controller, mockTokenListGetState, callActionSpy }) => { + mockMultiChainAccountsService(); + + const mockAPI = mockMultiChainAccountsService(); + mockAPI.mockFetchMultiChainBalances.mockResolvedValue({ + count: 0, + balances: [ + { + object: 'token', + address: '0xaddress', + name: 'Mock Token', + symbol: 'MOCK', + decimals: 18, + balance: '10.18', + chainId: 2, + }, + ], + unprocessedNetworks: [], + }); + + mockTokenListGetState({ + ...getDefaultTokenListState(), + tokensChainsCache: { + '0x1': { + timestamp: 0, + data: { + test: { + name: sampleTokenA.name, + symbol: sampleTokenA.symbol, + decimals: sampleTokenA.decimals, + address: 'test', + occurrences: 1, + aggregators: sampleTokenA.aggregators, + iconUrl: sampleTokenA.image, + }, + }, + }, + }, + }); + + await controller.start(); + + expect(callActionSpy).not.toHaveBeenCalledWith( + 'TokensController:addDetectedTokens', + [sampleTokenA], + { + chainId: ChainId.mainnet, + selectedAddress: selectedAccount.address, + }, + ); + }, + ); + }); + it('should detect tokens correctly on the Polygon network', async () => { const mockGetBalancesInSingleCall = jest.fn().mockResolvedValue({ [sampleTokenA.address]: new BN(1), @@ -2520,6 +2595,54 @@ describe('TokenDetectionController', () => { ); }); + it('should fallback to rpc call', async () => { + const mockGetBalancesInSingleCall = jest.fn().mockResolvedValue({ + [sampleTokenA.address]: new BN(1), + }); + const selectedAccount = createMockInternalAccount({ + address: '0x0000000000000000000000000000000000000001', + }); + await withController( + { + options: { + disabled: false, + getBalancesInSingleCall: mockGetBalancesInSingleCall, + useAccountsAPI: true, // USING ACCOUNTS API + }, + mocks: { + getSelectedAccount: selectedAccount, + getAccount: selectedAccount, + }, + }, + async ({ + controller, + mockNetworkState, + triggerPreferencesStateChange, + callActionSpy, + }) => { + const mockAPI = mockMultiChainAccountsService(); + mockAPI.mockFetchMultiChainBalances.mockRejectedValue( + new Error('Mock Error'), + ); + mockNetworkState({ + ...getDefaultNetworkControllerState(), + selectedNetworkClientId: 'polygon', + }); + triggerPreferencesStateChange({ + ...getDefaultPreferencesState(), + useTokenDetection: false, + }); + await controller.detectTokens({ + chainIds: ['0x5'], + selectedAddress: selectedAccount.address, + }); + expect(callActionSpy).not.toHaveBeenCalledWith( + 'TokensController:addDetectedTokens', + ); + }, + ); + }); + /** * Test Utility - Arrange and Act `detectTokens()` with the Accounts API feature * RPC flow will return `sampleTokenA` and the Accounts API flow will use `sampleTokenB` @@ -2732,6 +2855,57 @@ describe('TokenDetectionController', () => { assertTokensNeverAdded(); }); }); + + describe('mapChainIdWithTokenListMap', () => { + it('should return an empty object when given an empty input', () => { + const tokensChainsCache = {}; + const result = mapChainIdWithTokenListMap(tokensChainsCache); + expect(result).toStrictEqual({}); + }); + + it('should return the same structure when there is no "data" property in the object', () => { + const tokensChainsCache = { + chain1: { info: 'no data property' }, + }; + const result = mapChainIdWithTokenListMap(tokensChainsCache); + expect(result).toStrictEqual(tokensChainsCache); // Expect unchanged structure + }); + + it('should map "data" property if present in the object', () => { + const tokensChainsCache = { + chain1: { data: 'someData' }, + }; + const result = mapChainIdWithTokenListMap(tokensChainsCache); + expect(result).toStrictEqual({ chain1: 'someData' }); + }); + + it('should handle multiple chains with mixed "data" properties', () => { + const tokensChainsCache = { + chain1: { data: 'someData1' }, + chain2: { info: 'no data property' }, + chain3: { data: 'someData3' }, + }; + const result = mapChainIdWithTokenListMap(tokensChainsCache); + + expect(result).toStrictEqual({ + chain1: 'someData1', + chain2: { info: 'no data property' }, + chain3: 'someData3', + }); + }); + + it('should handle nested object with "data" property correctly', () => { + const tokensChainsCache = { + chain1: { + data: { + nested: 'nestedData', + }, + }, + }; + const result = mapChainIdWithTokenListMap(tokensChainsCache); + expect(result).toStrictEqual({ chain1: { nested: 'nestedData' } }); + }); + }); }); /** diff --git a/packages/assets-controllers/src/TokenDetectionController.ts b/packages/assets-controllers/src/TokenDetectionController.ts index 709e1ee852..a62ce63130 100644 --- a/packages/assets-controllers/src/TokenDetectionController.ts +++ b/packages/assets-controllers/src/TokenDetectionController.ts @@ -95,7 +95,9 @@ export const STATIC_MAINNET_TOKEN_LIST = Object.entries( * @param tokensChainsCache - TokensChainsCache input object * @returns returns the map of chainId with TokenListMap */ -function mapChainIdWithTokenListMap(tokensChainsCache: TokensChainsCache) { +export function mapChainIdWithTokenListMap( + tokensChainsCache: TokensChainsCache, +) { return mapValues(tokensChainsCache, (value) => { if (isObject(value) && 'data' in value) { return get(value, ['data']); @@ -224,45 +226,17 @@ export class TokenDetectionController extends StaticIntervalPollingController hexToNumber(chainId)); - const supportedNetworks = await this.getSupportedNetworks(); - if ( !supportedNetworks || !chainIdNumbers.every((id) => supportedNetworks.includes(id)) ) { - console.log('INSIDE IF -------'); const supportedNetworksErrStr = (supportedNetworks ?? []).toString(); throw new Error( `Unsupported Network: supported networks ${supportedNetworksErrStr}, requested networks: ${chainIdNumbers.toString()}`, @@ -517,7 +491,7 @@ export class TokenDetectionController extends StaticIntervalPollingController { const configuration = networkConfigurationsByChainId[chainId]; - return { chainId, networkClientId: @@ -551,22 +520,7 @@ export class TokenDetectionController extends StaticIntervalPollingController 0) { const apiResult = await this.#attemptAccountAPIDetection( chainsToDetectUsingAccountAPI, addressToDetect, + supportedNetworks, ); if (apiResult?.result === 'success') { return; @@ -765,11 +718,7 @@ export class TokenDetectionController extends StaticIntervalPollingController { // Fetch balances for multiple chain IDs at once const tokenBalancesByChain = await this.#accountsAPI - .getMultiNetworksBalances(selectedAddress, chainIds) + .getMultiNetworksBalances(selectedAddress, chainIds, supportedNetworks) .catch(() => null); if ( @@ -977,6 +929,8 @@ export class TokenDetectionController extends StaticIntervalPollingController Date: Wed, 6 Nov 2024 13:28:22 +0100 Subject: [PATCH 7/8] fix: fix fallback to rpc method --- .../src/TokenDetectionController.ts | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/packages/assets-controllers/src/TokenDetectionController.ts b/packages/assets-controllers/src/TokenDetectionController.ts index a62ce63130..a2d9a744c1 100644 --- a/packages/assets-controllers/src/TokenDetectionController.ts +++ b/packages/assets-controllers/src/TokenDetectionController.ts @@ -699,18 +699,23 @@ export class TokenDetectionController extends StaticIntervalPollingController 0) { const apiResult = await this.#attemptAccountAPIDetection( chainsToDetectUsingAccountAPI, addressToDetect, supportedNetworks, ); - if (apiResult?.result === 'success') { + + // If API succeeds and no chains are left for RPC detection, we can return early + if ( + apiResult?.result === 'success' && + chainsToDetectUsingRpc.length === 0 + ) { return; } - // Fallback on RPC detection if Account API fails + // If API fails or chainsToDetectUsingRpc still has items, add chains to RPC detection this.#addChainsToRpcDetection( chainsToDetectUsingRpc, chainsToDetectUsingAccountAPI, @@ -718,7 +723,10 @@ export class TokenDetectionController extends StaticIntervalPollingController 0) { + await this.#detectTokensUsingRpc(chainsToDetectUsingRpc, addressToDetect); + } } #getSlicesOfTokensToDetect({ From bdcf0f074c4190a8c27f700f1d29c1bd587a4e3f Mon Sep 17 00:00:00 2001 From: salimtb Date: Thu, 7 Nov 2024 15:25:48 +0100 Subject: [PATCH 8/8] fix: fix multichain import token --- .../src/TokensController.test.ts | 138 ++++++++++++++++++ .../src/TokensController.ts | 26 +++- 2 files changed, 156 insertions(+), 8 deletions(-) diff --git a/packages/assets-controllers/src/TokensController.test.ts b/packages/assets-controllers/src/TokensController.test.ts index eb12ef587d..22f7c60a1a 100644 --- a/packages/assets-controllers/src/TokensController.test.ts +++ b/packages/assets-controllers/src/TokensController.test.ts @@ -198,6 +198,84 @@ describe('TokensController', () => { }); }); + it('should add tokens and update existing ones and detected tokens', async () => { + const selectedAddress = '0x0001'; + const selectedAccount = createMockInternalAccount({ + address: selectedAddress, + }); + await withController( + { + mockNetworkClientConfigurationsByNetworkClientId: { + networkClientId1: buildCustomNetworkClientConfiguration({ + chainId: '0x1', + }), + }, + mocks: { + getSelectedAccount: selectedAccount, + getAccount: selectedAccount, + }, + }, + async ({ controller }) => { + await controller.addDetectedTokens( + [ + { + address: '0x01', + symbol: 'barA', + decimals: 2, + }, + ], + { + selectedAddress: '0x0001', + chainId: '0x1', + }, + ); + + await controller.addTokens( + [ + { + address: '0x01', + symbol: 'barA', + decimals: 2, + aggregators: [], + name: 'Token1', + }, + { + address: '0x02', + symbol: 'barB', + decimals: 2, + aggregators: [], + name: 'Token2', + }, + ], + 'networkClientId1', + ); + + expect(controller.state.allTokens).toStrictEqual({ + '0x1': { + '0x0001': [ + { + address: '0x01', + symbol: 'barA', + decimals: 2, + aggregators: [], + name: 'Token1', + image: undefined, + }, + { + address: '0x02', + symbol: 'barB', + decimals: 2, + aggregators: [], + name: 'Token2', + image: undefined, + }, + ], + }, + }); + }, + ); + }); + it('should add detected tokens', async () => { await withController(async ({ controller }) => { await controller.addDetectedTokens([ @@ -2142,6 +2220,66 @@ describe('TokensController', () => { }, ); }); + + it('should clear allDetectedTokens under chain ID and selected address when a detected token is added to tokens list', async () => { + const selectedAddress = '0x1'; + const selectedAccount = createMockInternalAccount({ + address: selectedAddress, + }); + const tokenAddress = '0x01'; + const dummyDetectedTokens = [ + { + address: tokenAddress, + symbol: 'barA', + decimals: 2, + aggregators: [], + isERC721: undefined, + name: undefined, + image: undefined, + }, + ]; + const dummyTokens = [ + { + address: tokenAddress, + symbol: 'barA', + decimals: 2, + aggregators: [], + isERC721: undefined, + name: undefined, + image: undefined, + }, + ]; + + await withController( + { + options: { + chainId: ChainId.mainnet, + }, + mocks: { + getSelectedAccount: selectedAccount, + }, + }, + async ({ controller }) => { + // First, add detected tokens + await controller.addDetectedTokens(dummyDetectedTokens); + expect( + controller.state.allDetectedTokens[ChainId.mainnet][ + selectedAddress + ], + ).toStrictEqual(dummyDetectedTokens); + + // Now, add the same token to the tokens list + await controller.addTokens(dummyTokens); + + // Check that allDetectedTokens for the selected address is cleared + expect( + controller.state.allDetectedTokens[ChainId.mainnet][ + selectedAddress + ], + ).toStrictEqual([]); + }, + ); + }); }); describe('when TokenListController:stateChange is published', () => { diff --git a/packages/assets-controllers/src/TokensController.ts b/packages/assets-controllers/src/TokensController.ts index fe680cdd0d..fbf135799a 100644 --- a/packages/assets-controllers/src/TokensController.ts +++ b/packages/assets-controllers/src/TokensController.ts @@ -462,13 +462,16 @@ export class TokensController extends BaseController< */ async addTokens(tokensToImport: Token[], networkClientId?: NetworkClientId) { const releaseLock = await this.#mutex.acquire(); - const { tokens, detectedTokens, ignoredTokens } = this.state; + const { ignoredTokens, allDetectedTokens } = this.state; const importedTokensMap: { [key: string]: true } = {}; // Used later to dedupe imported tokens - const newTokensMap = tokens.reduce((output, current) => { - output[current.address] = current; - return output; - }, {} as { [address: string]: Token }); + const newTokensMap = Object.values(tokensToImport).reduce( + (output, token) => { + output[token.address] = token; + return output; + }, + {} as { [address: string]: Token }, + ); try { tokensToImport.forEach((tokenToAdd) => { const { address, symbol, decimals, image, aggregators, name } = @@ -488,9 +491,6 @@ export class TokensController extends BaseController< }); const newTokens = Object.values(newTokensMap); - const newDetectedTokens = detectedTokens.filter( - (token) => !importedTokensMap[token.address.toLowerCase()], - ); const newIgnoredTokens = ignoredTokens.filter( (tokenAddress) => !newTokensMap[tokenAddress.toLowerCase()], ); @@ -503,6 +503,16 @@ export class TokensController extends BaseController< ).configuration.chainId; } + const newDetectedTokens = + interactingChainId && + allDetectedTokens[interactingChainId]?.[this.#getSelectedAddress()] + ? allDetectedTokens[interactingChainId]?.[ + this.#getSelectedAddress() + ].filter( + (token: Token) => !importedTokensMap[token.address.toLowerCase()], + ) + : []; + const { newAllTokens, newAllDetectedTokens, newAllIgnoredTokens } = this.#getNewAllTokensState({ newTokens,