Convert detect-tokens.js to typescript #23322
Assignees
Labels
Comments
metamaskbot
added
INVALID-ISSUE-TEMPLATE
|
The Shared Libraries team is currently in the last stages of replacing
|
Merged
13 tasks
|
@danjm @NiranjanaBinoy close this one as it is covered by shared libraries team |
MajorLift
added a commit
that referenced
this issue
Mar 14, 2024
…nDetectionController` (#22928) ## **Description** This commit replaces [`DetectTokensController`](https://github.com/MetaMask/metamask-extension/blob/68cc610976485d0071400cb2d4c3ea18cc6b15d9/app/scripts/controllers/detect-tokens.js) with the core repo's [`TokenDetectionController`](https://github.com/MetaMask/core/blob/main/packages/assets-controllers/src/TokenDetectionController.ts). This represents the final step of Shared Libraries' initiative to a) consolidate the core repo's `TokenDetectionController`, the extension's `DetectTokensController`, and relevant mobile patches to `@metamask/assets-controllers`, with the goal of b) migrating both extension and mobile to use the consolidated controller in core. This also represents a full conversion to TypeScript for `DetectTokensController` and its unit tests. [](https://codespaces.new/MetaMask/metamask-extension/pull/22928?quickstart=1) ## **Related issues** - Contributes to: - MetaMask/core#700 - MetaMask/core#1812 - Closes: - #23127 - #23322 - Blocked by: - MetaMask/core#3916 - MetaMask/core#3923 - MetaMask/core#4007 (`@metamask/assets-controllers` v26) ## **Manual testing steps** - Check whether issue no. 4 in this comment #23010 (comment) can be reproed in this branch. ## Changelog ### `metamask-extension` - `MetamaskController` class: - Define a `preferencesControllerMessenger` instance which only allows the `PreferencesController:getState` action and `PreferencesController:stateChange` event. - Add a subscription to the preferences-controller observable store, with a listener that publishes a `PreferencesController:stateChange` event. - `PreferencesController`: - Add `messenger` as an optional constructor options object property. - Register a `PreferencesController:getState` action handler. - **BREAKING:** Replace all imports of `toChecksumHexAddress` from `@metamask/controller-utils` with imports from the internal `shared/modules/hexstring-utils` module. - **BREAKING:** Bump `@metamask/assets-controllers` to `^26.0.0`. - Remove patch. - **BREAKING:** Bump `@metamask/accounts-controller` to `^11.0.0`. - Remove unused diff in patch. - **BREAKING:** Bump `@metamask/keyring-controller` to `^13.0.0`. - Remove patch. - **BREAKING:** Remove `@metamask/polling-controller` as a dependency. - Required by `test-yarn-dedupe` CI run. - Bump `@metamask/controller-utils` to `^8.0.4`. ### `TokenDetectionController` (compared to `DetectTokensController`) - **BREAKING:** Inherit from `StaticIntervalPollingController` instead of `StaticIntervalPollingControllerOnly` - Constructor and class fields: - **BREAKING:** Remove `preferences`, `network`, `tokenList`, `tokensController`, `assetsContractController`, `getCurrentSelectedAccount`, `getNetworkClientById` as constructor options, and add required option `getBalancesInSingleCall`. - **BREAKING:** Remove `disableLegacyInterval` class field and constructor option. - `#restartTokenDetection` always resets polling interval to default regardless of whether legacy or new polling is being used. - **BREAKING:** Add a `#disabled` private class field, which blocks all network requests if set to true, and add `disabled` as an optional constructor option, which defaults to 'true' if omitted. - **BREAKING:** Remove `isOpen` class field, and replace by adding `enable`, `disable` public methods. - Add optional constructor option `selectedAddress`. If omitted, its value is populated by calling the `AccountsController:getSelectedAccount` action. - Messenger: - **BREAKING:** Newly subscribe to the `PreferencesController:stateChange`, `AccountsController:selectedAccountChange`, `KeyringController:lock`, `KeyringController:unlock` events. - **BREAKING:** Newly allow messenger actions `AccountsController:getSelectedAccount`, `NetworkController:getNetworkClientById`, `NetworkController:getNetworkConfigurationByNetworkClientId`, `NetworkController:getState`, `KeyringController:getState`, `PreferencesController:getState`, `TokenListController:getState`, `TokensController:getState`, and `TokensController:addDetectedTokens`. - **BREAKING:** `detectTokens` replaces the `detectNewTokens` method. - **BREAKING:** Now expects an options object with optional properties `selectedAddress`, `networkClientId`, removing the `chainId` option. - **BREAKING:** Passes lists of full `Token` types to `TokensController:addDetectedTokens` instead of objects containing only `{ address, decimals, symbol }`. - Processes an arbitrary number of tokens in batches of 1000. - Previously, `detectTokens` was limited to two batches, with the first batch being limited to 1000 tokens. - If the `getBalancesInSingleCall` callback fails, it does not throw an error or exit early, and the method continues processing the next batch of tokens. - **BREAKING:** `#restartTokenDetection` is a private method instead of public. - **BREAKING:** Replace the `getChainIdFromNetworkStore` method with the private method `#getCorrectChainIdAndNetworkClientId`. - **BREAKING:** `#trackMetaMetricsEvents` is a private method instead of protected. - Passes string literals instead of extension shared constants into `_trackMetaMetricsEvent`. ### `TokensController` - **BREAKING:** Newly allows `NetworkController:getNetworkClientById` messenger action. - **BREAKING:** Newly subscribes to `NetworkController:networkDidChange`, `PreferencesController:stateChange`, `TokenListController:stateChange` events. - **BREAKING:** Unsubscribes from `NetworkController:stateChange` event. ## **Screenshots/Recordings** ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've clearly explained what problem this PR is solving and how it is solved. - [x] I've linked related issues - [x] I've included manual testing steps - [ ] I've included screenshots/recordings if applicable - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. - [x] I’ve properly set the pull request status: - [x] In case it's not yet "ready for review", I've set it to "draft". - [x] In case it's "ready for review", I've changed it from "draft" to "non-draft". ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
davidmurdoch
pushed a commit
that referenced
this issue
Mar 14, 2024
](https://codespaces.new/MetaMask/metamask-extension/pull/22928?quickstart=1){kind=link}
…nDetectionController` (#22928) ## **Description** This commit replaces [`DetectTokensController`](https://github.com/MetaMask/metamask-extension/blob/68cc610976485d0071400cb2d4c3ea18cc6b15d9/app/scripts/controllers/detect-tokens.js) with the core repo's [`TokenDetectionController`](https://github.com/MetaMask/core/blob/main/packages/assets-controllers/src/TokenDetectionController.ts). This represents the final step of Shared Libraries' initiative to a) consolidate the core repo's `TokenDetectionController`, the extension's `DetectTokensController`, and relevant mobile patches to `@metamask/assets-controllers`, with the goal of b) migrating both extension and mobile to use the consolidated controller in core. This also represents a full conversion to TypeScript for `DetectTokensController` and its unit tests. [](https://codespaces.new/MetaMask/metamask-extension/pull/22928?quickstart=1) ## **Related issues** - Contributes to: - MetaMask/core#700 - MetaMask/core#1812 - Closes: - #23127 - #23322 - Blocked by: - MetaMask/core#3916 - MetaMask/core#3923 - MetaMask/core#4007 (`@metamask/assets-controllers` v26) ## **Manual testing steps** - Check whether issue no. 4 in this comment #23010 (comment) can be reproed in this branch. ## Changelog ### `metamask-extension` - `MetamaskController` class: - Define a `preferencesControllerMessenger` instance which only allows the `PreferencesController:getState` action and `PreferencesController:stateChange` event. - Add a subscription to the preferences-controller observable store, with a listener that publishes a `PreferencesController:stateChange` event. - `PreferencesController`: - Add `messenger` as an optional constructor options object property. - Register a `PreferencesController:getState` action handler. - **BREAKING:** Replace all imports of `toChecksumHexAddress` from `@metamask/controller-utils` with imports from the internal `shared/modules/hexstring-utils` module. - **BREAKING:** Bump `@metamask/assets-controllers` to `^26.0.0`. - Remove patch. - **BREAKING:** Bump `@metamask/accounts-controller` to `^11.0.0`. - Remove unused diff in patch. - **BREAKING:** Bump `@metamask/keyring-controller` to `^13.0.0`. - Remove patch. - **BREAKING:** Remove `@metamask/polling-controller` as a dependency. - Required by `test-yarn-dedupe` CI run. - Bump `@metamask/controller-utils` to `^8.0.4`. ### `TokenDetectionController` (compared to `DetectTokensController`) - **BREAKING:** Inherit from `StaticIntervalPollingController` instead of `StaticIntervalPollingControllerOnly` - Constructor and class fields: - **BREAKING:** Remove `preferences`, `network`, `tokenList`, `tokensController`, `assetsContractController`, `getCurrentSelectedAccount`, `getNetworkClientById` as constructor options, and add required option `getBalancesInSingleCall`. - **BREAKING:** Remove `disableLegacyInterval` class field and constructor option. - `#restartTokenDetection` always resets polling interval to default regardless of whether legacy or new polling is being used. - **BREAKING:** Add a `#disabled` private class field, which blocks all network requests if set to true, and add `disabled` as an optional constructor option, which defaults to 'true' if omitted. - **BREAKING:** Remove `isOpen` class field, and replace by adding `enable`, `disable` public methods. - Add optional constructor option `selectedAddress`. If omitted, its value is populated by calling the `AccountsController:getSelectedAccount` action. - Messenger: - **BREAKING:** Newly subscribe to the `PreferencesController:stateChange`, `AccountsController:selectedAccountChange`, `KeyringController:lock`, `KeyringController:unlock` events. - **BREAKING:** Newly allow messenger actions `AccountsController:getSelectedAccount`, `NetworkController:getNetworkClientById`, `NetworkController:getNetworkConfigurationByNetworkClientId`, `NetworkController:getState`, `KeyringController:getState`, `PreferencesController:getState`, `TokenListController:getState`, `TokensController:getState`, and `TokensController:addDetectedTokens`. - **BREAKING:** `detectTokens` replaces the `detectNewTokens` method. - **BREAKING:** Now expects an options object with optional properties `selectedAddress`, `networkClientId`, removing the `chainId` option. - **BREAKING:** Passes lists of full `Token` types to `TokensController:addDetectedTokens` instead of objects containing only `{ address, decimals, symbol }`. - Processes an arbitrary number of tokens in batches of 1000. - Previously, `detectTokens` was limited to two batches, with the first batch being limited to 1000 tokens. - If the `getBalancesInSingleCall` callback fails, it does not throw an error or exit early, and the method continues processing the next batch of tokens. - **BREAKING:** `#restartTokenDetection` is a private method instead of public. - **BREAKING:** Replace the `getChainIdFromNetworkStore` method with the private method `#getCorrectChainIdAndNetworkClientId`. - **BREAKING:** `#trackMetaMetricsEvents` is a private method instead of protected. - Passes string literals instead of extension shared constants into `_trackMetaMetricsEvent`. ### `TokensController` - **BREAKING:** Newly allows `NetworkController:getNetworkClientById` messenger action. - **BREAKING:** Newly subscribes to `NetworkController:networkDidChange`, `PreferencesController:stateChange`, `TokenListController:stateChange` events. - **BREAKING:** Unsubscribes from `NetworkController:stateChange` event. ## **Screenshots/Recordings** ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've clearly explained what problem this PR is solving and how it is solved. - [x] I've linked related issues - [x] I've included manual testing steps - [ ] I've included screenshots/recordings if applicable - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. - [x] I’ve properly set the pull request status: - [x] In case it's not yet "ready for review", I've set it to "draft". - [x] In case it's "ready for review", I've changed it from "draft" to "non-draft". ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Part of #23018
The text was updated successfully, but these errors were encountered: