Skip to content

Commit

Permalink
[token-detection-controller] Refactor detectTokens method (#3938)
Browse files Browse the repository at this point in the history
## 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 <[email protected]>
Co-authored-by: Elliot Winkler <[email protected]>
  • Loading branch information
3 people authored Mar 4, 2024
1 parent a680605 commit 34bf8f0
Show file tree
Hide file tree
Showing 4 changed files with 203 additions and 187 deletions.
7 changes: 4 additions & 3 deletions packages/assets-controllers/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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/))
Expand Down
8 changes: 4 additions & 4 deletions packages/assets-controllers/jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
},

Expand Down
Loading

0 comments on commit 34bf8f0

Please sign in to comment.