[token-detection-controller] Refactor detectTokens method (#3938)

## Explanation

Adds refactors and cosmetic, typing fixes to #3923.

- Extracts three methods from `TokenDetectionController`'s
`detectTokens` method:
  - `#getCorrectChainIdAndNetworkClientId`
  - `#getTokenListAndSlicesOfTokensToDetect`
  - `#addDetectedTokens`
- Maintains distinction between class fields `#selectedAddress`,
`#networkClientId` and corresponding parameters used in `detectTokens`
and its helper methods, so that `detectTokens` method can be used
independently of polling/passive detection.
- [Refactor
`#getCorrectChainIdAndNetworkClientId`](c75fb3b)
to remove `findNetworkClientIdByChainId` which might return
inconsistent/unexpected results, and replace it with `getState`,
`getNetworkClientById`
- [Add missing method return
types](5c2e887)
- [Type networkClientId as
`NetworkClientId`](05be5d2)
- [Fix excess properties from legacy token list, define `LegacyToken`,
`TokenDetectionMap`](ea4c5b8)
- Removes `#chainId` class field.

## References

- Closes #1614
- Fixes #3661
- Blocked by (Follows from) #3923
- Blocking #3916
- Blocking #3918

## Changelog

## 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: Brian Bergeron <brian.e.bergeron@gmail.com>
Co-authored-by: Elliot Winkler <elliot.winkler@gmail.com>

packages/assets-controllers/CHANGELOG.md

@@ -10,7 +10,7 @@ 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 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/))
+- **BREAKING:** `TokenDetectionController` newly subscribes to the `PreferencesController:stateChange`, `AccountsController:selectedAccountChange`, `KeyringController:lock`, `KeyringController:unlock` events, and allows messenger actions `AccountsController:getSelectedAccount`, `NetworkController:getNetworkClientById`, `NetworkController:getNetworkConfigurationByNetworkClientId`, `NetworkController:getState`, `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/), [#3938](https://github.com/MetaMask/core/pull/3938))
 - `TokensController` now exports `TokensControllerActions`, `TokensControllerGetStateAction`, `TokensControllerAddDetectedTokensAction`, `TokensControllerEvents`, `TokensControllerStateChangeEvent`. ([#3690](https://github.com/MetaMask/core/pull/3690/))
 
 ### Changed
@@ -25,11 +25,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   - 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.
+    - `TokensState` now extends the `Record` types, and it has an index signature of `string`, making it compatible with the `BaseControllerV2` state object constraint of `Record<string, Json>`.
+  - The `detectTokens` method can now process an arbitrary number of tokens in batches of 1000. ([#3938](https://github.com/MetaMask/core/pull/3938))
 
 ### Removed
 
-- **BREAKING:** `TokenDetectionController` constructor no longer accepts options `onPreferencesStateChange`, `getPreferencesState`, `getTokensState`, `addDetectedTokens`. ([#3690](https://github.com/MetaMask/core/pull/3690/), [#3775](https://github.com/MetaMask/core/pull/3775/))
+- **BREAKING:** `TokenDetectionController` constructor no longer accepts options `networkClientId`, `onPreferencesStateChange`, `getPreferencesState`, `getTokensState`, `addDetectedTokens`. ([#3690](https://github.com/MetaMask/core/pull/3690/), [#3775](https://github.com/MetaMask/core/pull/3775/), [#3938](https://github.com/MetaMask/core/pull/3938))
 - **BREAKING:** `TokenDetectionController` no longer allows the `NetworkController:stateChange` event. The `NetworkController:networkDidChange` event can be used instead. ([#3775](https://github.com/MetaMask/core/pull/3775/))
 - **BREAKING:** `TokensController` constructor no longer accepts options `onPreferencesStateChange`, `onNetworkDidChange`, `onTokenListStateChange`, `getNetworkClientById`. ([#3690](https://github.com/MetaMask/core/pull/3690/))
 - **BREAKING:** `TokenBalancesController` constructor no longer accepts options `onTokensStateChange`, `getSelectedAddress`. ([#3690](https://github.com/MetaMask/core/pull/3690/))

packages/assets-controllers/jest.config.js

@@ -17,10 +17,10 @@ module.exports = merge(baseConfig, {
   // An object that configures minimum threshold enforcement for coverage results
   coverageThreshold: {
     global: {
-      branches: 88.58,
-      functions: 96.98,
-      lines: 97.35,
-      statements: 97.4,
+      branches: 88.67,
+      functions: 97,
+      lines: 97.36,
+      statements: 97.41,
     },
   },