[token-detection-controller] Apply recent DetectTokensController up…

…dates (#3923) ## Explanation As a preparatory step for fully replacing the extension `DetectTokensController` with the consolidated core repo `TokenDetectionController`, `TokenDetectionController` needs to be updated with changes made to the extension `DetectTokensController` since #1813 was closed. #### Diff of `DetectTokensController` state - [MetaMask/metamask-extension@`5d285f7be5f7be981995dfa725aad97d81cc990a..85cd1c89039e900b452edb704ec37e9ccbd3e76a`#diff-323d0cf464](https://github.com/MetaMask/metamask-extension/compare/5d285f7be5f7be981995dfa725aad97d81cc990a..85cd1c89039e900b452edb704ec37e9ccbd3e76a#diff-323d0cf46498be3850b971474905354ea5ccf7fa13745ad1e6eba59c5b586830) ### Differences from extension `DetectTokensController` - Refactors logic for retrieving `chainId`, `networkClientId` into `this.#getCorrectChainIdAndNetworkClientId` - Uses `getNetworkConfigurationByNetworkClientId` action instead of `getNetworkClientById` to retrieve `chainId`. - If `networkClientId` is not supplied to the method, or it's supplied but `getNetworkConfigurationByNetworkClientId` returns `undefined`, finds `chainId` from `providerConfig`. - `detectTokens` replaces `detectNewTokens` - `detectTokens` accepts options object `{ selectedAddress, networkClientId }` instead of `{ selectedAddress, chainId, networkClientId }`. - Does not throw error if `getBalancesInSingleCall` fails. Also does not exit early -- continues looping. - Passes lists of full `Token` types to `TokensController:addDetectedTokens` instead of objects containing only `{ address, decimals, symbol }`. - `#trackMetaMetricsEvents` is a private method instead of protected. - Passes string literals instead of extension shared constants into `_trackMetaMetricsEvent`. ## References - Partially implements #3916 - Blocking #3918 - Changes adopted from: - MetaMask/metamask-extension#22898 - MetaMask/metamask-extension#22814 - #3914 - MetaMask/metamask-extension#21437 - Blocking (Followed by) #3938 ## Changelog ### [`@metamask/assets-controllers`](https://github.com/MetaMask/core/pull/3923/files#diff-ee47d03d53776b8dd530799a8047f5e32e36e35765620aeb50b294adc3339fab) ## 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 highlighted breaking changes using the "BREAKING" category above as appropriate --------- Co-authored-by: jiexi <[email protected]>
MetaMask · Feb 27, 2024 · 1d78bb5 · 1d78bb5
1 parent eda5b9d
commit 1d78bb5
Show file tree

Hide file tree

Showing 4 changed files with 210 additions and 117 deletions.
diff --git a/packages/assets-controllers/CHANGELOG.md b/packages/assets-controllers/CHANGELOG.md
@@ -10,17 +10,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Added
 
 - **BREAKING:** Adds `@metamask/accounts-controller` ^8.0.0 and `@metamask/keyring-controller` ^12.0.0 as dependencies and peer dependencies. ([#3775](https://github.com/MetaMask/core/pull/3775/)).
-- **BREAKING:** `TokenDetectionController` newly subscribes to the `PreferencesController:stateChange`, `AccountsController:selectedAccountChange`, `KeyringController:lock`, `KeyringController:unlock` events, and allows the `PreferencesController:getState` messenger action. ([#3775](https://github.com/MetaMask/core/pull/3775/))
+- **BREAKING:** `TokenDetectionController` newly subscribes to the `PreferencesController:stateChange`, `AccountsController:selectedAccountChange`, `KeyringController:lock`, `KeyringController:unlock` events, and allows messenger actions `AccountsController:getSelectedAccount`, `NetworkController:findNetworkClientIdByChainId`, `NetworkController:getNetworkConfigurationByNetworkClientId`, `NetworkController:getProviderConfig`, `KeyringController:getState`, `PreferencesController:getState`, `TokenListController:getState`, `TokensController:getState`, `TokensController:addDetectedTokens`. ([#3775](https://github.com/MetaMask/core/pull/3775/)), ([#3923](https://github.com/MetaMask/core/pull/3923/))
 - `TokensController` now exports `TokensControllerActions`, `TokensControllerGetStateAction`, `TokensControllerAddDetectedTokensAction`, `TokensControllerEvents`, `TokensControllerStateChangeEvent`. ([#3690](https://github.com/MetaMask/core/pull/3690/))
 
 ### Changed
 
-- **BREAKING:** `TokenDetectionController` is merged with `DetectTokensController` from the `metamask-extension` repo. ([#3775](https://github.com/MetaMask/core/pull/3775/))
+- **BREAKING:** `TokenDetectionController` is merged with `DetectTokensController` from the `metamask-extension` repo. ([#3775](https://github.com/MetaMask/core/pull/3775/), [#3923](https://github.com/MetaMask/core/pull/3923)), ([#3938](https://github.com/MetaMask/core/pull/3938))
   - **BREAKING:** `TokenDetectionController` now resets its polling interval to the default value of 3 minutes when token detection is triggered by external controller events `KeyringController:unlock`, `TokenListController:stateChange`, `PreferencesController:stateChange`, `AccountsController:selectedAccountChange`.
   - **BREAKING:** `TokenDetectionController` now refetches tokens on `NetworkController:networkDidChange` if the `networkClientId` is changed instead of `chainId`.
   - **BREAKING:** `TokenDetectionController` cannot initiate polling or token detection if `KeyringController` state is locked.
+  - **BREAKING:** The `detectTokens` method input option `accountAddress` has been renamed to `selectedAddress`.
   - **BREAKING:** The `detectTokens` method now excludes tokens that are already included in the `TokensController`'s `detectedTokens` list from the batch of incoming tokens it sends to the `TokensController` `addDetectedTokens` method.
   - **BREAKING:** The constructor for `TokenDetectionController` expects a new required proprerty `trackMetaMetricsEvent`, which defines the callback that is called in the `detectTokens` method.
+  - The constructor option `selectedAddress` no longer defaults to `''` if omitted. Instead, the correct address is assigned using the `AccountsController:getSelectedAccount` messenger action.
   - **BREAKING:** In Mainnet, even if the `PreferenceController`'s `useTokenDetection` option is set to false, automatic token detection is performed on the legacy token list (token data from the contract-metadata repo).
   - **BREAKING:** The `TokensState` type is now defined as a type alias rather than an interface. ([#3690](https://github.com/MetaMask/core/pull/3690/))
     - This is breaking because it could affect how this type is used with other types, such as `Json`, which does not support TypeScript interfaces.

diff --git a/packages/assets-controllers/jest.config.js b/packages/assets-controllers/jest.config.js
@@ -17,9 +17,9 @@ module.exports = merge(baseConfig, {
   // An object that configures minimum threshold enforcement for coverage results
   coverageThreshold: {
     global: {
-      branches: 88.8,
-      functions: 96.71,
-      lines: 97.34,
+      branches: 88.58,
+      functions: 96.98,
+      lines: 97.35,
       statements: 97.4,
     },
   },

diff --git a/packages/assets-controllers/src/TokenDetectionController.test.ts b/packages/assets-controllers/src/TokenDetectionController.test.ts
@@ -8,12 +8,14 @@ import {
 } from '@metamask/controller-utils';
 import type { InternalAccount } from '@metamask/keyring-api';
 import type { KeyringControllerState } from '@metamask/keyring-controller';
-import {
-  defaultState as defaultNetworkState,
-  type NetworkState,
-  type NetworkConfiguration,
-  type NetworkController,
+import type {
+  NetworkState,
+  NetworkConfiguration,
+  NetworkController,
+  ProviderConfig,
+  NetworkClientId,
 } from '@metamask/network-controller';
+import { defaultState as defaultNetworkState } from '@metamask/network-controller';
 import {
   getDefaultPreferencesState,
   type PreferencesState,
@@ -138,8 +140,11 @@ function buildTokenDetectionControllerMessenger(
   return controllerMessenger.getRestricted({
     name: controllerName,
     allowedActions: [
+      'AccountsController:getSelectedAccount',
       'KeyringController:getState',
+      'NetworkController:findNetworkClientIdByChainId',
       'NetworkController:getNetworkConfigurationByNetworkClientId',
+      'NetworkController:getProviderConfig',
       'TokensController:getState',
       'TokensController:addDetectedTokens',
       'TokenListController:getState',
@@ -338,7 +343,16 @@ describe('TokenDetectionController', () => {
             selectedAddress,
           },
         },
-        async ({ controller, mockTokenListGetState, callActionSpy }) => {
+        async ({
+          controller,
+          mockGetProviderConfig,
+          mockTokenListGetState,
+          callActionSpy,
+        }) => {
+          mockGetProviderConfig({
+            chainId: '0x89',
+          } as unknown as ProviderConfig);
+
           mockTokenListGetState({
             ...getDefaultTokenListState(),
             tokensChainsCache: {
@@ -1748,19 +1762,19 @@ describe('TokenDetectionController', () => {
           await advanceTime({ clock, duration: 0 });
 
           expect(spy.mock.calls).toMatchObject([
-            [{ networkClientId: 'mainnet', accountAddress: '0x1' }],
-            [{ networkClientId: 'sepolia', accountAddress: '0xdeadbeef' }],
-            [{ networkClientId: 'goerli', accountAddress: '0x3' }],
+            [{ networkClientId: 'mainnet', selectedAddress: '0x1' }],
+            [{ networkClientId: 'sepolia', selectedAddress: '0xdeadbeef' }],
+            [{ networkClientId: 'goerli', selectedAddress: '0x3' }],
           ]);
 
           await advanceTime({ clock, duration: DEFAULT_INTERVAL });
           expect(spy.mock.calls).toMatchObject([
-            [{ networkClientId: 'mainnet', accountAddress: '0x1' }],
-            [{ networkClientId: 'sepolia', accountAddress: '0xdeadbeef' }],
-            [{ networkClientId: 'goerli', accountAddress: '0x3' }],
-            [{ networkClientId: 'mainnet', accountAddress: '0x1' }],
-            [{ networkClientId: 'sepolia', accountAddress: '0xdeadbeef' }],
-            [{ networkClientId: 'goerli', accountAddress: '0x3' }],
+            [{ networkClientId: 'mainnet', selectedAddress: '0x1' }],
+            [{ networkClientId: 'sepolia', selectedAddress: '0xdeadbeef' }],
+            [{ networkClientId: 'goerli', selectedAddress: '0x3' }],
+            [{ networkClientId: 'mainnet', selectedAddress: '0x1' }],
+            [{ networkClientId: 'sepolia', selectedAddress: '0xdeadbeef' }],
+            [{ networkClientId: 'goerli', selectedAddress: '0x3' }],
           ]);
         },
       );
@@ -1793,7 +1807,7 @@ describe('TokenDetectionController', () => {
           });
           await controller.detectTokens({
             networkClientId: NetworkType.goerli,
-            accountAddress: selectedAddress,
+            selectedAddress,
           });
           expect(callActionSpy).not.toHaveBeenCalledWith(
             'TokensController:addDetectedTokens',
@@ -1833,7 +1847,7 @@ describe('TokenDetectionController', () => {
           });
           await controller.detectTokens({
             networkClientId: NetworkType.mainnet,
-            accountAddress: selectedAddress,
+            selectedAddress,
           });
           expect(callActionSpy).toHaveBeenLastCalledWith(
             'TokensController:addDetectedTokens',
@@ -1896,7 +1910,7 @@ describe('TokenDetectionController', () => {
 
           await controller.detectTokens({
             networkClientId: NetworkType.mainnet,
-            accountAddress: selectedAddress,
+            selectedAddress,
           });
 
           expect(callActionSpy).toHaveBeenCalledWith(
@@ -1951,7 +1965,7 @@ describe('TokenDetectionController', () => {
 
           await controller.detectTokens({
             networkClientId: NetworkType.mainnet,
-            accountAddress: selectedAddress,
+            selectedAddress,
           });
 
           expect(mockTrackMetaMetricsEvent).toHaveBeenCalledWith({
@@ -1983,10 +1997,14 @@ function getTokensPath(chainId: Hex) {
 
 type WithControllerCallback<ReturnValue> = ({
   controller,
+  mockGetSelectedAccount,
   mockKeyringGetState,
   mockTokensGetState,
   mockTokenListGetState,
   mockPreferencesGetState,
+  mockFindNetworkClientIdByChainId,
+  mockGetNetworkConfigurationByNetworkClientId,
+  mockGetProviderConfig,
   callActionSpy,
   triggerKeyringUnlock,
   triggerKeyringLock,
@@ -1996,13 +2014,18 @@ type WithControllerCallback<ReturnValue> = ({
   triggerNetworkDidChange,
 }: {
   controller: TokenDetectionController;
+  mockGetSelectedAccount: (address: string) => void;
   mockKeyringGetState: (state: KeyringControllerState) => void;
   mockTokensGetState: (state: TokensState) => void;
   mockTokenListGetState: (state: TokenListState) => void;
   mockPreferencesGetState: (state: PreferencesState) => void;
+  mockFindNetworkClientIdByChainId: (
+    handler: (chainId: Hex) => NetworkClientId,
+  ) => void;
   mockGetNetworkConfigurationByNetworkClientId: (
     handler: (networkClientId: string) => NetworkConfiguration,
   ) => void;
+  mockGetProviderConfig: (config: ProviderConfig) => void;
   callActionSpy: jest.SpyInstance;
   triggerKeyringUnlock: () => void;
   triggerKeyringLock: () => void;
@@ -2039,25 +2062,45 @@ async function withController<ReturnValue>(
   const controllerMessenger =
     messenger ?? new ControllerMessenger<AllowedActions, AllowedEvents>();
 
+  const mockGetSelectedAccount = jest.fn<InternalAccount, []>();
+  controllerMessenger.registerActionHandler(
+    'AccountsController:getSelectedAccount',
+    mockGetSelectedAccount.mockReturnValue({
+      address: '0x1',
+    } as InternalAccount),
+  );
   const mockKeyringState = jest.fn<KeyringControllerState, []>();
   controllerMessenger.registerActionHandler(
     'KeyringController:getState',
     mockKeyringState.mockReturnValue({
       isUnlocked: isKeyringUnlocked ?? true,
     } as KeyringControllerState),
   );
+  const mockFindNetworkClientIdByChainId = jest.fn<NetworkClientId, [Hex]>();
+  controllerMessenger.registerActionHandler(
+    'NetworkController:findNetworkClientIdByChainId',
+    mockFindNetworkClientIdByChainId.mockReturnValue(NetworkType.mainnet),
+  );
   const mockGetNetworkConfigurationByNetworkClientId = jest.fn<
     ReturnType<NetworkController['getNetworkConfigurationByNetworkClientId']>,
     Parameters<NetworkController['getNetworkConfigurationByNetworkClientId']>
   >();
   controllerMessenger.registerActionHandler(
     'NetworkController:getNetworkConfigurationByNetworkClientId',
     mockGetNetworkConfigurationByNetworkClientId.mockImplementation(
-      (networkClientId: string) => {
+      (networkClientId: NetworkClientId) => {
         return mockNetworkConfigurations[networkClientId];
       },
     ),
   );
+  const mockGetProviderConfig = jest.fn<ProviderConfig, []>();
+  controllerMessenger.registerActionHandler(
+    'NetworkController:getProviderConfig',
+    mockGetProviderConfig.mockReturnValue({
+      type: NetworkType.mainnet,
+      chainId: '0x1',
+    } as unknown as ProviderConfig),
+  );
   const mockTokensState = jest.fn<TokensState, []>();
   controllerMessenger.registerActionHandler(
     'TokensController:getState',
@@ -2096,6 +2139,9 @@ async function withController<ReturnValue>(
   try {
     return await fn({
       controller,
+      mockGetSelectedAccount: (address: string) => {
+        mockGetSelectedAccount.mockReturnValue({ address } as InternalAccount);
+      },
       mockKeyringGetState: (state: KeyringControllerState) => {
         mockKeyringState.mockReturnValue(state);
       },
@@ -2108,13 +2154,21 @@ async function withController<ReturnValue>(
       mockTokenListGetState: (state: TokenListState) => {
         mockTokenListState.mockReturnValue(state);
       },
+      mockFindNetworkClientIdByChainId: (
+        handler: (chainId: Hex) => NetworkClientId,
+      ) => {
+        mockFindNetworkClientIdByChainId.mockImplementation(handler);
+      },
       mockGetNetworkConfigurationByNetworkClientId: (
-        handler: (networkClientId: string) => NetworkConfiguration,
+        handler: (networkClientId: NetworkClientId) => NetworkConfiguration,
       ) => {
         mockGetNetworkConfigurationByNetworkClientId.mockImplementation(
           handler,
         );
       },
+      mockGetProviderConfig: (config: ProviderConfig) => {
+        mockGetProviderConfig.mockReturnValue(config);
+      },
       callActionSpy,
       triggerKeyringUnlock: () => {
         controllerMessenger.publish('KeyringController:unlock');