From d595d0f9c0c103c3f78ad13b47cd2407129839c8 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Fri, 1 Dec 2023 12:18:33 -0800 Subject: [PATCH 01/82] Define types for BaseControllerV2 migration - Add types`TokenDetectionState`, `TokenDetectionControllerGetStateAction`, `TokenDetectionControllerActions`, `TokenDetectionControllerStateChangeEvent`, `TokenDetectionControllerEvents`, `TokenDetectionControllerMessenger` --- .../src/TokenDetectionController.ts | 59 +++++++++++++++---- 1 file changed, 49 insertions(+), 10 deletions(-) diff --git a/packages/assets-controllers/src/TokenDetectionController.ts b/packages/assets-controllers/src/TokenDetectionController.ts index 2b877058e1..70cb6ac990 100644 --- a/packages/assets-controllers/src/TokenDetectionController.ts +++ b/packages/assets-controllers/src/TokenDetectionController.ts @@ -1,11 +1,17 @@ -import type { BaseConfig, BaseState } from '@metamask/base-controller'; +import type { + RestrictedControllerMessenger, + ControllerGetStateAction, + ControllerStateChangeEvent, +} from '@metamask/base-controller'; import { safelyExecute, toChecksumHexAddress, } from '@metamask/controller-utils'; import type { NetworkClientId, - NetworkController, + NetworkControllerGetNetworkClientByIdAction, + NetworkControllerGetStateAction, + NetworkControllerStateChangeEvent, NetworkState, } from '@metamask/network-controller'; import { PollingControllerV1 } from '@metamask/polling-controller'; @@ -14,32 +20,65 @@ import type { Hex } from '@metamask/utils'; import type { AssetsContractController } from './AssetsContractController'; import { isTokenDetectionSupportedForNetwork } from './assetsUtil'; -import type { TokenListState } from './TokenListController'; +import type { + GetTokenListState, + TokenListStateChange, +} from './TokenListController'; import type { Token } from './TokenRatesController'; import type { TokensController, TokensState } from './TokensController'; const DEFAULT_INTERVAL = 180000; /** - * @type TokenDetectionConfig + * @type TokenDetectionState * - * TokenDetection configuration + * TokenDetection state * @property interval - Polling interval used to fetch new token rates * @property selectedAddress - Vault selected address * @property chainId - The chain ID of the current network + * @property disabled - Determines if this controller is enabled * @property isDetectionEnabledFromPreferences - Boolean to track if detection is enabled from PreferencesController * @property isDetectionEnabledForNetwork - Boolean to track if detected is enabled for current network */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface TokenDetectionConfig extends BaseConfig { +export type TokenDetectionState = { interval: number; selectedAddress: string; chainId: Hex; + disabled: boolean; isDetectionEnabledFromPreferences: boolean; isDetectionEnabledForNetwork: boolean; -} +}; + +const controllerName = 'TokenDetectionController'; + +export type TokenDetectionControllerGetStateAction = ControllerGetStateAction< + typeof controllerName, + TokenDetectionState +>; + +export type TokenDetectionControllerActions = + TokenDetectionControllerGetStateAction; + +type AllowedActions = + | NetworkControllerGetStateAction + | NetworkControllerGetNetworkClientByIdAction + | GetTokenListState; + +export type TokenDetectionControllerStateChangeEvent = + ControllerStateChangeEvent; + +export type TokenDetectionControllerEvents = + TokenDetectionControllerStateChangeEvent; + +type AllowedEvents = NetworkControllerStateChangeEvent | TokenListStateChange; + +export type TokenDetectionControllerMessenger = RestrictedControllerMessenger< + typeof controllerName, + TokenDetectionControllerActions | AllowedActions, + TokenDetectionControllerEvents | AllowedEvents, + AllowedActions['type'], + AllowedEvents['type'] +>; /** * Controller that passively polls on a set interval for Tokens auto detection From 56ca69263b4e0a72e9bd92b02a23bf570bbe56d8 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Fri, 1 Dec 2023 12:23:04 -0800 Subject: [PATCH 02/82] Upgrade `TokenDetectionController` to extend from `PollingController` - Remove methods that can be replaced with actions/events - Replace references to `this.config` with `this.state` - Replace `this.configure()` calls with `this.update()` --- .../src/TokenDetectionController.ts | 134 ++++++++---------- 1 file changed, 62 insertions(+), 72 deletions(-) diff --git a/packages/assets-controllers/src/TokenDetectionController.ts b/packages/assets-controllers/src/TokenDetectionController.ts index 70cb6ac990..2ff63a33da 100644 --- a/packages/assets-controllers/src/TokenDetectionController.ts +++ b/packages/assets-controllers/src/TokenDetectionController.ts @@ -14,7 +14,7 @@ import type { NetworkControllerStateChangeEvent, NetworkState, } from '@metamask/network-controller'; -import { PollingControllerV1 } from '@metamask/polling-controller'; +import { PollingController } from '@metamask/polling-controller'; import type { PreferencesState } from '@metamask/preferences-controller'; import type { Hex } from '@metamask/utils'; @@ -83,16 +83,16 @@ export type TokenDetectionControllerMessenger = RestrictedControllerMessenger< /** * Controller that passively polls on a set interval for Tokens auto detection */ -export class TokenDetectionController extends PollingControllerV1< - TokenDetectionConfig, - BaseState +export class TokenDetectionController extends PollingController< + typeof controllerName, + TokenDetectionState, + TokenDetectionControllerMessenger > { private intervalId?: ReturnType; /** * Name of this controller used during composition */ - override name = 'TokenDetectionController'; private readonly getBalancesInSingleCall: AssetsContractController['getBalancesInSingleCall']; @@ -100,80 +100,69 @@ export class TokenDetectionController extends PollingControllerV1< private readonly getTokensState: () => TokensState; - private readonly getTokenListState: () => TokenListState; - - private readonly getNetworkClientById: NetworkController['getNetworkClientById']; - /** * Creates a TokenDetectionController instance. * * @param options - The controller options. * @param options.onPreferencesStateChange - Allows subscribing to preferences controller state changes. - * @param options.onNetworkStateChange - Allows subscribing to network controller state changes. - * @param options.onTokenListStateChange - Allows subscribing to token list controller state changes. * @param options.getBalancesInSingleCall - Gets the balances of a list of tokens for the given address. * @param options.addDetectedTokens - Add a list of detected tokens. - * @param options.getTokenListState - Gets the current state of the TokenList controller. * @param options.getTokensState - Gets the current state of the Tokens controller. * @param options.getNetworkState - Gets the state of the network controller. * @param options.getPreferencesState - Gets the state of the preferences controller. - * @param options.getNetworkClientById - Gets the network client by ID. - * @param config - Initial options used to configure this controller. - * @param state - Initial state to set on this controller. + * @param options.messenger - The controller messaging system. + * @param options.state - Initial state to set on this controller. */ - constructor( - { - onPreferencesStateChange, - onNetworkStateChange, - onTokenListStateChange, - getBalancesInSingleCall, - addDetectedTokens, - getTokenListState, - getTokensState, - getNetworkState, - getPreferencesState, - getNetworkClientById, - }: { - onPreferencesStateChange: ( - listener: (preferencesState: PreferencesState) => void, - ) => void; - onNetworkStateChange: ( - listener: (networkState: NetworkState) => void, - ) => void; - onTokenListStateChange: ( - listener: (tokenListState: TokenListState) => void, - ) => void; - getBalancesInSingleCall: AssetsContractController['getBalancesInSingleCall']; - addDetectedTokens: TokensController['addDetectedTokens']; - getTokenListState: () => TokenListState; - getTokensState: () => TokensState; - getNetworkState: () => NetworkState; - getPreferencesState: () => PreferencesState; - getNetworkClientById: NetworkController['getNetworkClientById']; - }, - config?: Partial, - state?: Partial, - ) { + constructor({ + onPreferencesStateChange, + getBalancesInSingleCall, + addDetectedTokens, + getTokensState, + getNetworkState, + getPreferencesState, + messenger, + state, + }: { + onPreferencesStateChange: ( + listener: (preferencesState: PreferencesState) => void, + ) => void; + getBalancesInSingleCall: AssetsContractController['getBalancesInSingleCall']; + addDetectedTokens: TokensController['addDetectedTokens']; + getTokensState: () => TokensState; + getNetworkState: () => NetworkState; + getPreferencesState: () => PreferencesState; + messenger: TokenDetectionControllerMessenger; + state?: TokenDetectionState; + }) { const { providerConfig: { chainId: defaultChainId }, } = getNetworkState(); const { useTokenDetection: defaultUseTokenDetection } = getPreferencesState(); - super(config, state); - this.defaultConfig = { - interval: DEFAULT_INTERVAL, - selectedAddress: '', - disabled: true, - chainId: defaultChainId, - isDetectionEnabledFromPreferences: defaultUseTokenDetection, - isDetectionEnabledForNetwork: - isTokenDetectionSupportedForNetwork(defaultChainId), - ...config, - }; - - this.initialize(); - this.setIntervalLength(this.config.interval); + super({ + name: controllerName, + messenger, + state: { + interval: DEFAULT_INTERVAL, + selectedAddress: '', + disabled: true, + chainId: defaultChainId, + isDetectionEnabledFromPreferences: defaultUseTokenDetection, + isDetectionEnabledForNetwork: + isTokenDetectionSupportedForNetwork(defaultChainId), + ...state, + }, + metadata: { + interval: { persist: true, anonymous: false }, + selectedAddress: { persist: true, anonymous: false }, + chainId: { persist: true, anonymous: false }, + disabled: { persist: true, anonymous: false }, + isDetectionEnabledFromPreferences: { persist: true, anonymous: false }, + isDetectionEnabledForNetwork: { persist: true, anonymous: false }, + }, + }); + this.getTokensState = getTokensState; this.getTokenListState = getTokenListState; this.addDetectedTokens = addDetectedTokens; @@ -192,16 +181,17 @@ export class TokenDetectionController extends PollingControllerV1< const { selectedAddress: currentSelectedAddress, isDetectionEnabledFromPreferences, - } = this.config; + } = this.state; const isSelectedAddressChanged = selectedAddress !== currentSelectedAddress; const isDetectionChangedFromPreferences = isDetectionEnabledFromPreferences !== useTokenDetection; - this.configure({ + this.update(() => ({ + ...this.state, isDetectionEnabledFromPreferences: useTokenDetection, selectedAddress, - }); + })); if ( useTokenDetection && @@ -232,7 +222,7 @@ export class TokenDetectionController extends PollingControllerV1< * Start polling for detected tokens. */ async start() { - this.configure({ disabled: false }); + this.update(() => ({ ...this.state, disabled: false })); await this.startPolling(); } @@ -240,7 +230,7 @@ export class TokenDetectionController extends PollingControllerV1< * Stop polling for detected tokens. */ stop() { - this.configure({ disabled: true }); + this.update(() => ({ ...this.state, disabled: true })); this.stopPolling(); } @@ -256,12 +246,12 @@ export class TokenDetectionController extends PollingControllerV1< * @param interval - An interval on which to poll. */ private async startPolling(interval?: number): Promise { - interval && this.configure({ interval }, false, false); + interval && this.update(() => ({ ...this.state, interval })); this.stopPolling(); await this.detectTokens(); this.intervalId = setInterval(async () => { await this.detectTokens(); - }, this.config.interval); + }, this.state.interval); } private getCorrectChainId(networkClientId?: NetworkClientId) { @@ -292,12 +282,12 @@ export class TokenDetectionController extends PollingControllerV1< networkClientId?: NetworkClientId; accountAddress?: string; }) { - const { networkClientId, accountAddress } = options || {}; + const { networkClientId, accountAddress } = options ?? {}; const { disabled, isDetectionEnabledForNetwork, isDetectionEnabledFromPreferences, - } = this.config; + } = this.state; if ( disabled || !isDetectionEnabledForNetwork || @@ -306,7 +296,7 @@ export class TokenDetectionController extends PollingControllerV1< return; } const { tokens } = this.getTokensState(); - const selectedAddress = accountAddress || this.config.selectedAddress; + const selectedAddress = accountAddress ?? this.state.selectedAddress; const chainId = this.getCorrectChainId(networkClientId); const tokensAddresses = tokens.map( From 25ae501328edf657e44c06e4dcc82f87c0445b4f Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Fri, 1 Dec 2023 12:23:44 -0800 Subject: [PATCH 03/82] Replace event handlers with controller-messenger pattern --- .../src/TokenDetectionController.ts | 65 +++++++++++-------- 1 file changed, 39 insertions(+), 26 deletions(-) diff --git a/packages/assets-controllers/src/TokenDetectionController.ts b/packages/assets-controllers/src/TokenDetectionController.ts index 2ff63a33da..4c994f47b8 100644 --- a/packages/assets-controllers/src/TokenDetectionController.ts +++ b/packages/assets-controllers/src/TokenDetectionController.ts @@ -164,18 +164,19 @@ export class TokenDetectionController extends PollingController< }); this.getTokensState = getTokensState; - this.getTokenListState = getTokenListState; this.addDetectedTokens = addDetectedTokens; this.getBalancesInSingleCall = getBalancesInSingleCall; - this.getNetworkClientById = getNetworkClientById; - onTokenListStateChange(({ tokenList }) => { - const hasTokens = Object.keys(tokenList).length; + this.messagingSystem.subscribe( + 'TokenListController:stateChange', + ({ tokenList }) => { + const hasTokens = Object.keys(tokenList).length; - if (hasTokens) { - this.detectTokens(); - } - }); + if (hasTokens) { + this.detectTokens(); + } + }, + ); onPreferencesStateChange(({ selectedAddress, useTokenDetection }) => { const { @@ -201,21 +202,25 @@ export class TokenDetectionController extends PollingController< } }); - onNetworkStateChange(({ providerConfig: { chainId } }) => { - const { chainId: currentChainId } = this.config; - const isDetectionEnabledForNetwork = - isTokenDetectionSupportedForNetwork(chainId); - const isChainIdChanged = currentChainId !== chainId; - - this.configure({ - chainId, - isDetectionEnabledForNetwork, - }); - - if (isDetectionEnabledForNetwork && isChainIdChanged) { - this.detectTokens(); - } - }); + this.messagingSystem.subscribe( + 'NetworkController:stateChange', + ({ providerConfig: { chainId } }) => { + const { chainId: currentChainId } = this.state; + const isDetectionEnabledForNetwork = + isTokenDetectionSupportedForNetwork(chainId); + const isChainIdChanged = currentChainId !== chainId; + + this.update(() => ({ + ...this.state, + chainId, + isDetectionEnabledForNetwork, + })); + + if (isDetectionEnabledForNetwork && isChainIdChanged) { + this.detectTokens(); + } + }, + ); } /** @@ -256,9 +261,15 @@ export class TokenDetectionController extends PollingController< private getCorrectChainId(networkClientId?: NetworkClientId) { if (networkClientId) { - return this.getNetworkClientById(networkClientId).configuration.chainId; + const { + configuration: { chainId }, + } = this.messagingSystem.call( + 'NetworkController:getNetworkClientById', + networkClientId, + ); + return chainId; } - return this.config.chainId; + return this.state.chainId; } _executePoll( @@ -302,7 +313,9 @@ export class TokenDetectionController extends PollingController< const tokensAddresses = tokens.map( /* istanbul ignore next*/ (token) => token.address.toLowerCase(), ); - const { tokenList } = this.getTokenListState(); + const { tokenList } = this.messagingSystem.call( + 'TokenListController:getState', + ); const tokensToDetect: string[] = []; for (const address of Object.keys(tokenList)) { if (!tokensAddresses.includes(address)) { From d42cc0fa1986a7ad01c691f9389d329f636b5472 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 7 Dec 2023 07:45:35 -0800 Subject: [PATCH 04/82] [token-list-controller] Export `TokenListControllerMessenger` and define `AllowedEvents` --- .../assets-controllers/src/TokenListController.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/assets-controllers/src/TokenListController.ts b/packages/assets-controllers/src/TokenListController.ts index 3603760fef..19c181d727 100644 --- a/packages/assets-controllers/src/TokenListController.ts +++ b/packages/assets-controllers/src/TokenListController.ts @@ -68,12 +68,14 @@ export type TokenListControllerActions = GetTokenListState; type AllowedActions = NetworkControllerGetNetworkClientByIdAction; -type TokenListMessenger = RestrictedControllerMessenger< +type AllowedEvents = NetworkControllerStateChangeEvent; + +export type TokenListControllerMessenger = RestrictedControllerMessenger< typeof name, TokenListControllerActions | AllowedActions, - TokenListControllerEvents | NetworkControllerStateChangeEvent, + TokenListControllerEvents | AllowedEvents, AllowedActions['type'], - (TokenListControllerEvents | NetworkControllerStateChangeEvent)['type'] + AllowedEvents['type'] >; const metadata = { @@ -94,7 +96,7 @@ const defaultState: TokenListState = { export class TokenListController extends PollingController< typeof name, TokenListState, - TokenListMessenger + TokenListControllerMessenger > { private readonly mutex = new Mutex(); @@ -136,7 +138,7 @@ export class TokenListController extends PollingController< ) => void; interval?: number; cacheRefreshThreshold?: number; - messenger: TokenListMessenger; + messenger: TokenListControllerMessenger; state?: Partial; }) { super({ From 0a66c508b6c7678e7cd8d14b4b0a591716015e06 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 7 Dec 2023 07:46:51 -0800 Subject: [PATCH 05/82] Explicitly enumerate package-level exports --- packages/assets-controllers/src/index.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/assets-controllers/src/index.ts b/packages/assets-controllers/src/index.ts index 1977e0ee2d..1824028506 100644 --- a/packages/assets-controllers/src/index.ts +++ b/packages/assets-controllers/src/index.ts @@ -4,7 +4,8 @@ export * from './CurrencyRateController'; export * from './NftController'; export * from './NftDetectionController'; export * from './TokenBalancesController'; -export * from './TokenDetectionController'; +export type { TokenDetectionControllerMessenger } from './TokenDetectionController'; +export { TokenDetectionController } from './TokenDetectionController'; export * from './TokenListController'; export * from './TokenRatesController'; export * from './TokensController'; From 1b5b6351d634f487a6b916db538cafe0603ee749 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 7 Dec 2023 08:10:45 -0800 Subject: [PATCH 06/82] Move `TokenDetectionConfig` properties to private class fields in `TokenDetectionController` --- .../src/TokenDetectionController.ts | 151 +++++++++--------- 1 file changed, 77 insertions(+), 74 deletions(-) diff --git a/packages/assets-controllers/src/TokenDetectionController.ts b/packages/assets-controllers/src/TokenDetectionController.ts index 4c994f47b8..25922e1ae1 100644 --- a/packages/assets-controllers/src/TokenDetectionController.ts +++ b/packages/assets-controllers/src/TokenDetectionController.ts @@ -29,6 +29,7 @@ import type { TokensController, TokensState } from './TokensController'; const DEFAULT_INTERVAL = 180000; + /** * @type TokenDetectionState * @@ -82,13 +83,29 @@ export type TokenDetectionControllerMessenger = RestrictedControllerMessenger< /** * Controller that passively polls on a set interval for Tokens auto detection + * @property disabled - Determines if this controller is enabled + * @property interval - Polling interval used to fetch new token rates + * @property selectedAddress - Vault selected address + * @property chainId - The chain ID of the current network + * @property isDetectionEnabledFromPreferences - Boolean to track if detection is enabled from PreferencesController + * @property isDetectionEnabledForNetwork - Boolean to track if detected is enabled for current network */ export class TokenDetectionController extends PollingController< typeof controllerName, TokenDetectionState, TokenDetectionControllerMessenger > { - private intervalId?: ReturnType; + #disabled = true; + + #intervalId?: ReturnType; + + #chainId: Hex; + + #selectedAddress: string; + + #isDetectionEnabledFromPreferences: boolean; + + #isDetectionEnabledForNetwork: boolean; /** * Name of this controller used during composition @@ -104,25 +121,30 @@ export class TokenDetectionController extends PollingController< * Creates a TokenDetectionController instance. * * @param options - The controller options. + * @param options.messenger - The controller messaging system. + * @param options.interval - Polling interval used to fetch new token rates + * @param options.chainId - The chain ID of the current network + * @param options.selectedAddress - Vault selected address * @param options.onPreferencesStateChange - Allows subscribing to preferences controller state changes. * @param options.getBalancesInSingleCall - Gets the balances of a list of tokens for the given address. * @param options.addDetectedTokens - Add a list of detected tokens. * @param options.getTokensState - Gets the current state of the Tokens controller. - * @param options.getNetworkState - Gets the state of the network controller. * @param options.getPreferencesState - Gets the state of the preferences controller. - * @param options.messenger - The controller messaging system. - * @param options.state - Initial state to set on this controller. */ constructor({ + chainId, + selectedAddress, + interval = DEFAULT_INTERVAL, onPreferencesStateChange, getBalancesInSingleCall, addDetectedTokens, getTokensState, - getNetworkState, getPreferencesState, messenger, - state, }: { + chainId: Hex; + selectedAddress?: string; + interval?: number; onPreferencesStateChange: ( listener: (preferencesState: PreferencesState) => void, ) => void; @@ -143,26 +165,14 @@ export class TokenDetectionController extends PollingController< super({ name: controllerName, messenger, - state: { - interval: DEFAULT_INTERVAL, - selectedAddress: '', - disabled: true, - chainId: defaultChainId, - isDetectionEnabledFromPreferences: defaultUseTokenDetection, - isDetectionEnabledForNetwork: - isTokenDetectionSupportedForNetwork(defaultChainId), - ...state, - }, - metadata: { - interval: { persist: true, anonymous: false }, - selectedAddress: { persist: true, anonymous: false }, - chainId: { persist: true, anonymous: false }, - disabled: { persist: true, anonymous: false }, - isDetectionEnabledFromPreferences: { persist: true, anonymous: false }, - isDetectionEnabledForNetwork: { persist: true, anonymous: false }, - }, + state: {}, + metadata: {}, }); + this.setIntervalLength(interval); + this.#chainId = chainId; + this.#selectedAddress = selectedAddress ?? ''; + this.getTokensState = getTokensState; this.addDetectedTokens = addDetectedTokens; this.getBalancesInSingleCall = getBalancesInSingleCall; @@ -178,22 +188,20 @@ export class TokenDetectionController extends PollingController< }, ); - onPreferencesStateChange(({ selectedAddress, useTokenDetection }) => { - const { - selectedAddress: currentSelectedAddress, - isDetectionEnabledFromPreferences, - } = this.state; - const isSelectedAddressChanged = - selectedAddress !== currentSelectedAddress; - const isDetectionChangedFromPreferences = - isDetectionEnabledFromPreferences !== useTokenDetection; - - this.update(() => ({ - ...this.state, - isDetectionEnabledFromPreferences: useTokenDetection, - selectedAddress, - })); + onPreferencesStateChange( + async ({ selectedAddress: newSelectedAddress, useTokenDetection }) => { + const isSelectedAddressChanged = + this.#selectedAddress !== newSelectedAddress; + const isDetectionChangedFromPreferences = + this.#isDetectionEnabledFromPreferences !== useTokenDetection; + + this.#selectedAddress = newSelectedAddress; + this.#isDetectionEnabledFromPreferences = useTokenDetection; + if ( + useTokenDetection && + (isSelectedAddressChanged || isDetectionChangedFromPreferences) + ) { if ( useTokenDetection && (isSelectedAddressChanged || isDetectionChangedFromPreferences) @@ -201,33 +209,36 @@ export class TokenDetectionController extends PollingController< this.detectTokens(); } }); + } + }, + ); this.messagingSystem.subscribe( 'NetworkController:stateChange', - ({ providerConfig: { chainId } }) => { - const { chainId: currentChainId } = this.state; - const isDetectionEnabledForNetwork = - isTokenDetectionSupportedForNetwork(chainId); - const isChainIdChanged = currentChainId !== chainId; - - this.update(() => ({ - ...this.state, - chainId, - isDetectionEnabledForNetwork, - })); - - if (isDetectionEnabledForNetwork && isChainIdChanged) { - this.detectTokens(); + async ({ providerConfig: { chainId: newChainId } }) => { + this.#isDetectionEnabledForNetwork = + isTokenDetectionSupportedForNetwork(newChainId); + const isChainIdChanged = this.#chainId !== newChainId; + + this.#chainId = newChainId; + + if (this.#isDetectionEnabledForNetwork && isChainIdChanged) { + await this.detectTokens(); } }, ); + + this.#isDetectionEnabledFromPreferences = defaultUseTokenDetection; + this.#isDetectionEnabledForNetwork = isTokenDetectionSupportedForNetwork( + this.#chainId, + ); } /** * Start polling for detected tokens. */ async start() { - this.update(() => ({ ...this.state, disabled: false })); + this.#disabled = false; await this.startPolling(); } @@ -235,28 +246,25 @@ export class TokenDetectionController extends PollingController< * Stop polling for detected tokens. */ stop() { - this.update(() => ({ ...this.state, disabled: true })); + this.#disabled = true; this.stopPolling(); } private stopPolling() { - if (this.intervalId) { - clearInterval(this.intervalId); + if (this.#intervalId) { + clearInterval(this.#intervalId); } } /** * Starts a new polling interval. - * - * @param interval - An interval on which to poll. */ - private async startPolling(interval?: number): Promise { - interval && this.update(() => ({ ...this.state, interval })); + private async startPolling(): Promise { this.stopPolling(); await this.detectTokens(); - this.intervalId = setInterval(async () => { + this.#intervalId = setInterval(async () => { await this.detectTokens(); - }, this.state.interval); + }, this.getIntervalLength()); } private getCorrectChainId(networkClientId?: NetworkClientId) { @@ -267,9 +275,9 @@ export class TokenDetectionController extends PollingController< 'NetworkController:getNetworkClientById', networkClientId, ); - return chainId; + this.#chainId = chainId; } - return this.state.chainId; + return this.#chainId; } _executePoll( @@ -294,20 +302,15 @@ export class TokenDetectionController extends PollingController< accountAddress?: string; }) { const { networkClientId, accountAddress } = options ?? {}; - const { - disabled, - isDetectionEnabledForNetwork, - isDetectionEnabledFromPreferences, - } = this.state; if ( - disabled || - !isDetectionEnabledForNetwork || - !isDetectionEnabledFromPreferences + this.#disabled || + !this.#isDetectionEnabledForNetwork || + !this.#isDetectionEnabledFromPreferences ) { return; } const { tokens } = this.getTokensState(); - const selectedAddress = accountAddress ?? this.state.selectedAddress; + const selectedAddress = accountAddress ?? this.#selectedAddress; const chainId = this.getCorrectChainId(networkClientId); const tokensAddresses = tokens.map( From 94448cded3dd939abf38e6f5a43afda4e9759d40 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 7 Dec 2023 08:17:48 -0800 Subject: [PATCH 07/82] Define `controllerName`, `TokenDetectionState` as empty object, remove `getNetworkState` --- .../src/TokenDetectionController.ts | 37 ++++--------------- 1 file changed, 7 insertions(+), 30 deletions(-) diff --git a/packages/assets-controllers/src/TokenDetectionController.ts b/packages/assets-controllers/src/TokenDetectionController.ts index 25922e1ae1..b9fdb7ebdf 100644 --- a/packages/assets-controllers/src/TokenDetectionController.ts +++ b/packages/assets-controllers/src/TokenDetectionController.ts @@ -12,7 +12,6 @@ import type { NetworkControllerGetNetworkClientByIdAction, NetworkControllerGetStateAction, NetworkControllerStateChangeEvent, - NetworkState, } from '@metamask/network-controller'; import { PollingController } from '@metamask/polling-controller'; import type { PreferencesState } from '@metamask/preferences-controller'; @@ -29,28 +28,9 @@ import type { TokensController, TokensState } from './TokensController'; const DEFAULT_INTERVAL = 180000; +export const controllerName = 'TokenDetectionController'; -/** - * @type TokenDetectionState - * - * TokenDetection state - * @property interval - Polling interval used to fetch new token rates - * @property selectedAddress - Vault selected address - * @property chainId - The chain ID of the current network - * @property disabled - Determines if this controller is enabled - * @property isDetectionEnabledFromPreferences - Boolean to track if detection is enabled from PreferencesController - * @property isDetectionEnabledForNetwork - Boolean to track if detected is enabled for current network - */ -export type TokenDetectionState = { - interval: number; - selectedAddress: string; - chainId: Hex; - disabled: boolean; - isDetectionEnabledFromPreferences: boolean; - isDetectionEnabledForNetwork: boolean; -}; - -const controllerName = 'TokenDetectionController'; +export type TokenDetectionState = Record; export type TokenDetectionControllerGetStateAction = ControllerGetStateAction< typeof controllerName, @@ -60,7 +40,7 @@ export type TokenDetectionControllerGetStateAction = ControllerGetStateAction< export type TokenDetectionControllerActions = TokenDetectionControllerGetStateAction; -type AllowedActions = +export type AllowedActions = | NetworkControllerGetStateAction | NetworkControllerGetNetworkClientByIdAction | GetTokenListState; @@ -71,7 +51,9 @@ export type TokenDetectionControllerStateChangeEvent = export type TokenDetectionControllerEvents = TokenDetectionControllerStateChangeEvent; -type AllowedEvents = NetworkControllerStateChangeEvent | TokenListStateChange; +export type AllowedEvents = + | NetworkControllerStateChangeEvent + | TokenListStateChange; export type TokenDetectionControllerMessenger = RestrictedControllerMessenger< typeof controllerName, @@ -151,14 +133,9 @@ export class TokenDetectionController extends PollingController< getBalancesInSingleCall: AssetsContractController['getBalancesInSingleCall']; addDetectedTokens: TokensController['addDetectedTokens']; getTokensState: () => TokensState; - getNetworkState: () => NetworkState; getPreferencesState: () => PreferencesState; messenger: TokenDetectionControllerMessenger; - state?: TokenDetectionState; }) { - const { - providerConfig: { chainId: defaultChainId }, - } = getNetworkState(); const { useTokenDetection: defaultUseTokenDetection } = getPreferencesState(); @@ -300,7 +277,7 @@ export class TokenDetectionController extends PollingController< async detectTokens(options?: { networkClientId?: NetworkClientId; accountAddress?: string; - }) { + }): Promise { const { networkClientId, accountAddress } = options ?? {}; if ( this.#disabled || From bcfa0a1eed189f078720d5c06d302451c02738dc Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 7 Dec 2023 08:18:35 -0800 Subject: [PATCH 08/82] Fix: unregister internal `getState` action handler in constructor as state object is not used --- packages/assets-controllers/src/TokenDetectionController.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/assets-controllers/src/TokenDetectionController.ts b/packages/assets-controllers/src/TokenDetectionController.ts index b9fdb7ebdf..a54758ddfd 100644 --- a/packages/assets-controllers/src/TokenDetectionController.ts +++ b/packages/assets-controllers/src/TokenDetectionController.ts @@ -154,6 +154,10 @@ export class TokenDetectionController extends PollingController< this.addDetectedTokens = addDetectedTokens; this.getBalancesInSingleCall = getBalancesInSingleCall; + this.messagingSystem.unregisterActionHandler( + 'TokenDetectionController:getState', + ); + this.messagingSystem.subscribe( 'TokenListController:stateChange', ({ tokenList }) => { From b8dcf32241ec85d314f7324a49bcac7931f1b530 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 7 Dec 2023 08:19:04 -0800 Subject: [PATCH 09/82] Fix `sliceOfTokensToDetect` logic to handle arbitrary number of `tokensToDetect` --- .../assets-controllers/src/TokenDetectionController.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/assets-controllers/src/TokenDetectionController.ts b/packages/assets-controllers/src/TokenDetectionController.ts index a54758ddfd..b99ade79ba 100644 --- a/packages/assets-controllers/src/TokenDetectionController.ts +++ b/packages/assets-controllers/src/TokenDetectionController.ts @@ -307,11 +307,11 @@ export class TokenDetectionController extends PollingController< } } const sliceOfTokensToDetect = []; - sliceOfTokensToDetect[0] = tokensToDetect.slice(0, 1000); - sliceOfTokensToDetect[1] = tokensToDetect.slice( - 1000, - tokensToDetect.length - 1, - ); + for (let i = 0; i < tokensToDetect.length; i += 1000) { + sliceOfTokensToDetect.push( + tokensToDetect.slice(i, Math.min(i + 1000, tokensToDetect.length)), + ); + } /* istanbul ignore else */ if (!selectedAddress) { From a5ef65043dcb4ea11487f027c582a60384a02da7 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 7 Dec 2023 19:27:12 -0800 Subject: [PATCH 10/82] Various fixes for controller upgrade --- .../src/TokenDetectionController.ts | 67 +++++++++---------- 1 file changed, 30 insertions(+), 37 deletions(-) diff --git a/packages/assets-controllers/src/TokenDetectionController.ts b/packages/assets-controllers/src/TokenDetectionController.ts index b99ade79ba..081c28788a 100644 --- a/packages/assets-controllers/src/TokenDetectionController.ts +++ b/packages/assets-controllers/src/TokenDetectionController.ts @@ -93,11 +93,11 @@ export class TokenDetectionController extends PollingController< * Name of this controller used during composition */ - private readonly getBalancesInSingleCall: AssetsContractController['getBalancesInSingleCall']; + readonly #getBalancesInSingleCall: AssetsContractController['getBalancesInSingleCall']; - private readonly addDetectedTokens: TokensController['addDetectedTokens']; + readonly #addDetectedTokens: TokensController['addDetectedTokens']; - private readonly getTokensState: () => TokensState; + readonly #getTokensState: () => TokensState; /** * Creates a TokenDetectionController instance. @@ -115,7 +115,7 @@ export class TokenDetectionController extends PollingController< */ constructor({ chainId, - selectedAddress, + selectedAddress = '', interval = DEFAULT_INTERVAL, onPreferencesStateChange, getBalancesInSingleCall, @@ -148,11 +148,11 @@ export class TokenDetectionController extends PollingController< this.setIntervalLength(interval); this.#chainId = chainId; - this.#selectedAddress = selectedAddress ?? ''; + this.#selectedAddress = selectedAddress; - this.getTokensState = getTokensState; - this.addDetectedTokens = addDetectedTokens; - this.getBalancesInSingleCall = getBalancesInSingleCall; + this.#getTokensState = getTokensState; + this.#addDetectedTokens = addDetectedTokens; + this.#getBalancesInSingleCall = getBalancesInSingleCall; this.messagingSystem.unregisterActionHandler( 'TokenDetectionController:getState', @@ -170,7 +170,7 @@ export class TokenDetectionController extends PollingController< ); onPreferencesStateChange( - async ({ selectedAddress: newSelectedAddress, useTokenDetection }) => { + ({ selectedAddress: newSelectedAddress, useTokenDetection }) => { const isSelectedAddressChanged = this.#selectedAddress !== newSelectedAddress; const isDetectionChangedFromPreferences = @@ -183,20 +183,14 @@ export class TokenDetectionController extends PollingController< useTokenDetection && (isSelectedAddressChanged || isDetectionChangedFromPreferences) ) { - if ( - useTokenDetection && - (isSelectedAddressChanged || isDetectionChangedFromPreferences) - ) { - this.detectTokens(); - } - }); + this.detectTokens(); } }, ); this.messagingSystem.subscribe( 'NetworkController:stateChange', - async ({ providerConfig: { chainId: newChainId } }) => { + ({ providerConfig: { chainId: newChainId } }) => { this.#isDetectionEnabledForNetwork = isTokenDetectionSupportedForNetwork(newChainId); const isChainIdChanged = this.#chainId !== newChainId; @@ -204,7 +198,7 @@ export class TokenDetectionController extends PollingController< this.#chainId = newChainId; if (this.#isDetectionEnabledForNetwork && isChainIdChanged) { - await this.detectTokens(); + this.detectTokens(); } }, ); @@ -220,7 +214,7 @@ export class TokenDetectionController extends PollingController< */ async start() { this.#disabled = false; - await this.startPolling(); + await this.#startPolling(); } /** @@ -228,10 +222,10 @@ export class TokenDetectionController extends PollingController< */ stop() { this.#disabled = true; - this.stopPolling(); + this.#stopPolling(); } - private stopPolling() { + #stopPolling() { if (this.#intervalId) { clearInterval(this.#intervalId); } @@ -240,15 +234,15 @@ export class TokenDetectionController extends PollingController< /** * Starts a new polling interval. */ - private async startPolling(): Promise { - this.stopPolling(); + async #startPolling(): Promise { + this.#stopPolling(); await this.detectTokens(); - this.#intervalId = setInterval(async () => { - await this.detectTokens(); + this.#intervalId = setInterval(() => { + this.detectTokens(); }, this.getIntervalLength()); } - private getCorrectChainId(networkClientId?: NetworkClientId) { + #getCorrectChainId(networkClientId?: NetworkClientId) { if (networkClientId) { const { configuration: { chainId }, @@ -261,11 +255,11 @@ export class TokenDetectionController extends PollingController< return this.#chainId; } - _executePoll( + async _executePoll( networkClientId: string, options: { address: string }, ): Promise { - return this.detectTokens({ + return await this.detectTokens({ networkClientId, accountAddress: options.address, }); @@ -290,14 +284,13 @@ export class TokenDetectionController extends PollingController< ) { return; } - const { tokens } = this.getTokensState(); + const { tokens } = this.#getTokensState() ?? {}; const selectedAddress = accountAddress ?? this.#selectedAddress; - const chainId = this.getCorrectChainId(networkClientId); - - const tokensAddresses = tokens.map( + const chainId = this.#getCorrectChainId(networkClientId); + const tokensAddresses = tokens?.map( /* istanbul ignore next*/ (token) => token.address.toLowerCase(), ); - const { tokenList } = this.messagingSystem.call( + const { tokenList } = await this.messagingSystem.call( 'TokenListController:getState', ); const tokensToDetect: string[] = []; @@ -324,7 +317,7 @@ export class TokenDetectionController extends PollingController< } await safelyExecute(async () => { - const balances = await this.getBalancesInSingleCall( + const balances = await this.#getBalancesInSingleCall( selectedAddress, tokensSlice, ); @@ -332,7 +325,7 @@ export class TokenDetectionController extends PollingController< for (const tokenAddress of Object.keys(balances)) { let ignored; /* istanbul ignore else */ - const { ignoredTokens } = this.getTokensState(); + const { ignoredTokens } = this.#getTokensState(); if (ignoredTokens.length) { ignored = ignoredTokens.find( (ignoredTokenAddress) => @@ -342,7 +335,7 @@ export class TokenDetectionController extends PollingController< const caseInsensitiveTokenKey = Object.keys(tokenList).find( (i) => i.toLowerCase() === tokenAddress.toLowerCase(), - ) || ''; + ) ?? ''; if (ignored === undefined) { const { decimals, symbol, aggregators, iconUrl, name } = @@ -360,7 +353,7 @@ export class TokenDetectionController extends PollingController< } if (tokensToAdd.length) { - await this.addDetectedTokens(tokensToAdd, { + await this.#addDetectedTokens(tokensToAdd, { selectedAddress, chainId, }); From 72b36184abbf5bea8e900f3d210516fb414237d9 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 7 Dec 2023 21:02:16 -0800 Subject: [PATCH 11/82] Adapt tests to align with upgraded `TokenDetectionController` API --- packages/assets-controllers/jest.config.js | 8 +- .../src/TokenDetectionController.test.ts | 290 +++++++----------- 2 files changed, 114 insertions(+), 184 deletions(-) diff --git a/packages/assets-controllers/jest.config.js b/packages/assets-controllers/jest.config.js index 13bec0c96f..7c7fa8c850 100644 --- a/packages/assets-controllers/jest.config.js +++ b/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.2, - functions: 95.95, - lines: 96.25, - statements: 96.5, + branches: 88.54, + functions: 95.66, + lines: 96.46, + statements: 96.52, }, }, diff --git a/packages/assets-controllers/src/TokenDetectionController.test.ts b/packages/assets-controllers/src/TokenDetectionController.test.ts index 9eceb0751e..03892ab80a 100644 --- a/packages/assets-controllers/src/TokenDetectionController.test.ts +++ b/packages/assets-controllers/src/TokenDetectionController.test.ts @@ -1,3 +1,4 @@ +import type { AddApprovalRequest } from '@metamask/approval-controller'; import { ControllerMessenger } from '@metamask/base-controller'; import { ChainId, @@ -8,7 +9,6 @@ import { } from '@metamask/controller-utils'; import { defaultState as defaultNetworkState } from '@metamask/network-controller'; import type { - NetworkControllerStateChangeEvent, NetworkState, ProviderConfig, } from '@metamask/network-controller'; @@ -22,20 +22,23 @@ import { advanceTime } from '../../../tests/helpers'; import type { AssetsContractController } from './AssetsContractController'; import { formatAggregatorNames, - isTokenDetectionSupportedForNetwork, SupportedTokenDetectionNetworks, } from './assetsUtil'; import { TOKEN_END_POINT_API } from './token-service'; -import { TokenDetectionController } from './TokenDetectionController'; -import { TokenListController } from './TokenListController'; import type { - GetTokenListState, - TokenListStateChange, - TokenListToken, -} from './TokenListController'; + AllowedActions, + AllowedEvents, + TokenDetectionControllerMessenger, +} from './TokenDetectionController'; +import { + TokenDetectionController, + controllerName, +} from './TokenDetectionController'; +import { TokenListController } from './TokenListController'; +import type { TokenListToken } from './TokenListController'; import type { Token } from './TokenRatesController'; -import { TokensController } from './TokensController'; import type { TokensControllerMessenger } from './TokensController'; +import { TokensController } from './TokensController'; const DEFAULT_INTERVAL = 180000; @@ -96,8 +99,8 @@ const sampleTokenB: Token = { }; type MainControllerMessenger = ControllerMessenger< - GetTokenListState, - TokenListStateChange | NetworkControllerStateChangeEvent + AllowedActions | AddApprovalRequest, + AllowedEvents >; const getControllerMessenger = (): MainControllerMessenger => { @@ -122,6 +125,22 @@ const setupTokenListController = ( return { tokenList, tokenListMessenger }; }; +const buildTokenDetectionControllerMessenger: ( + controllerMessenger: MainControllerMessenger, +) => TokenDetectionControllerMessenger = (controllerMessenger) => + controllerMessenger.getRestricted({ + name: controllerName, + allowedActions: [ + 'NetworkController:getState', + 'NetworkController:getNetworkClientById', + 'TokenListController:getState', + ], + allowedEvents: [ + 'NetworkController:stateChange', + 'TokenListController:stateChange', + ], + }); + describe('TokenDetectionController', () => { let tokenDetection: TokenDetectionController; let preferences: PreferencesController; @@ -141,6 +160,16 @@ describe('TokenDetectionController', () => { providerConfig, }); }); + + controllerMessenger.registerActionHandler( + `NetworkController:getNetworkClientById`, + jest.fn().mockReturnValueOnce({ + configuration: providerConfig, + provider: {}, + blockTracker: {}, + destroy: jest.fn(), + }), + ); }; const mainnet = { chainId: ChainId.mainnet, @@ -189,30 +218,15 @@ describe('TokenDetectionController', () => { getBalancesInSingleCall = sinon.stub(); tokenDetection = new TokenDetectionController({ + chainId: ChainId.mainnet, onPreferencesStateChange: (listener) => preferences.subscribe(listener), - onNetworkStateChange: (listener) => - onNetworkDidChangeListeners.push(listener), - onTokenListStateChange: (listener) => - tokenListSetup.tokenListMessenger.subscribe( - `TokenListController:stateChange`, - listener, - ), getBalancesInSingleCall: getBalancesInSingleCall as unknown as AssetsContractController['getBalancesInSingleCall'], addDetectedTokens: tokensController.addDetectedTokens.bind(tokensController), getTokensState: () => tokensController.state, - getTokenListState: () => tokenList.state, - getNetworkState: () => defaultNetworkState, getPreferencesState: () => preferences.state, - getNetworkClientById: jest.fn().mockReturnValueOnce({ - configuration: { - chainId: ChainId.mainnet, - }, - provider: {}, - blockTracker: {}, - destroy: jest.fn(), - }), + messenger: buildTokenDetectionControllerMessenger(controllerMessenger), }); sinon @@ -227,25 +241,15 @@ describe('TokenDetectionController', () => { controllerMessenger.clearEventSubscriptions( 'NetworkController:stateChange', ); - }); - - it('should set default config', () => { - expect(tokenDetection.config).toStrictEqual({ - interval: DEFAULT_INTERVAL, - selectedAddress: '', - disabled: true, - chainId: ChainId.mainnet, - isDetectionEnabledForNetwork: true, - isDetectionEnabledFromPreferences: true, - }); + controllerMessenger.unregisterActionHandler( + 'NetworkController:getNetworkClientById', + ); }); it('should poll and detect tokens on interval while on supported networks', async () => { await new Promise(async (resolve) => { const mockTokens = sinon.stub(tokenDetection, 'detectTokens'); - tokenDetection.configure({ - interval: 10, - }); + tokenDetection.setIntervalLength(10); await tokenDetection.start(); expect(mockTokens.calledOnce).toBe(true); @@ -256,29 +260,10 @@ describe('TokenDetectionController', () => { }); }); - it('should detect supported networks correctly', () => { - tokenDetection.configure({ - chainId: SupportedTokenDetectionNetworks.mainnet, - }); - - expect( - isTokenDetectionSupportedForNetwork(tokenDetection.config.chainId), - ).toBe(true); - tokenDetection.configure({ chainId: SupportedTokenDetectionNetworks.bsc }); - expect( - isTokenDetectionSupportedForNetwork(tokenDetection.config.chainId), - ).toBe(true); - tokenDetection.configure({ chainId: ChainId.goerli }); - expect( - isTokenDetectionSupportedForNetwork(tokenDetection.config.chainId), - ).toBe(false); - }); - it('should not autodetect while not on supported networks', async () => { - tokenDetection.configure({ - selectedAddress: '0x1', - chainId: ChainId.goerli, - isDetectionEnabledForNetwork: false, + await tokenDetection.detectTokens({ + accountAddress: '0x1', + networkClientId: ChainId.goerli, }); getBalancesInSingleCall.resolves({ @@ -305,13 +290,20 @@ describe('TokenDetectionController', () => { type: NetworkType.mainnet, ticker: 'Aurora ETH', }; - preferences.update({ selectedAddress: '0x1' }); + const selectedAddress = '0x1'; + preferences.update({ selectedAddress }); changeNetwork(auroraMainnet); + await tokenDetection.start(); + getBalancesInSingleCall.resolves({ [sampleTokenA.address]: new BN(1), }); - await tokenDetection.start(); + + await tokenDetection.detectTokens({ + accountAddress: selectedAddress, + networkClientId: ChainId.aurora, + }); expect(tokensController.state.detectedTokens).toStrictEqual([sampleTokenA]); }); @@ -443,32 +435,17 @@ describe('TokenDetectionController', () => { const stub = sinon.stub(); const getBalancesInSingleCallMock = sinon.stub(); - let networkStateChangeListener: (state: any) => void; - const onNetworkStateChange = sinon.stub().callsFake((listener) => { - networkStateChangeListener = listener; - }); - tokenDetection = new TokenDetectionController( - { - onTokenListStateChange: stub, - onPreferencesStateChange: stub, - onNetworkStateChange, - getBalancesInSingleCall: getBalancesInSingleCallMock, - addDetectedTokens: stub, - getTokensState: () => tokensController.state, - getTokenListState: () => tokenList.state, - getNetworkState: () => defaultNetworkState, - getPreferencesState: () => preferences.state, - getNetworkClientById: jest.fn(), - }, - { - disabled: false, - isDetectionEnabledForNetwork: true, - isDetectionEnabledFromPreferences: true, - selectedAddress: '0x1', - chainId: ChainId.mainnet, - }, - ); + tokenDetection = new TokenDetectionController({ + chainId: ChainId.mainnet, + selectedAddress: '0x1', + onPreferencesStateChange: stub, + getBalancesInSingleCall: getBalancesInSingleCallMock, + addDetectedTokens: stub, + getTokensState: () => tokensController.state, + getPreferencesState: () => preferences.state, + messenger: buildTokenDetectionControllerMessenger(controllerMessenger), + }); await tokenDetection.start(); @@ -476,44 +453,29 @@ describe('TokenDetectionController', () => { getBalancesInSingleCallMock.reset(); tokenDetection.stop(); - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - await networkStateChangeListener!({ - providerConfig: { chainId: toHex(polygonDecimalChainId) }, + changeNetwork({ + chainId: toHex(polygonDecimalChainId), + type: NetworkType.goerli, + ticker: NetworksTicker.goerli, }); - expect(getBalancesInSingleCallMock.called).toBe(false); }); - it('should not call getBalancesInSingleCall if onTokenListStateChange is called with an empty token list', async () => { + it('should not call getBalancesInSingleCall if TokenListController is updated to have an empty token list', async () => { const stub = sinon.stub(); const getBalancesInSingleCallMock = sinon.stub(); - let tokenListStateChangeListener: (state: any) => void; - const onTokenListStateChange = sinon.stub().callsFake((listener) => { - tokenListStateChangeListener = listener; + tokenDetection = new TokenDetectionController({ + chainId: ChainId.mainnet, + onPreferencesStateChange: stub, + getBalancesInSingleCall: getBalancesInSingleCallMock, + addDetectedTokens: stub, + getTokensState: stub, + getPreferencesState: () => preferences.state, + messenger: buildTokenDetectionControllerMessenger(controllerMessenger), }); - tokenDetection = new TokenDetectionController( - { - onTokenListStateChange, - onPreferencesStateChange: stub, - onNetworkStateChange: stub, - getBalancesInSingleCall: getBalancesInSingleCallMock, - addDetectedTokens: stub, - getTokensState: stub, - getTokenListState: stub, - getNetworkState: () => defaultNetworkState, - getPreferencesState: () => preferences.state, - getNetworkClientById: jest.fn(), - }, - { - disabled: false, - isDetectionEnabledForNetwork: true, - isDetectionEnabledFromPreferences: true, - }, - ); - - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - await tokenListStateChangeListener!({ tokenList: {} }); + tokenList.clearingTokenListData(); + await tokenDetection.start(); expect(getBalancesInSingleCallMock.called).toBe(false); }); @@ -524,73 +486,44 @@ describe('TokenDetectionController', () => { const onPreferencesStateChange = sinon.stub().callsFake((listener) => { preferencesStateChangeListener = listener; }); - tokenDetection = new TokenDetectionController( - { - onPreferencesStateChange, - onTokenListStateChange: stub, - onNetworkStateChange: stub, - getBalancesInSingleCall: getBalancesInSingleCallMock, - addDetectedTokens: stub, - getTokensState: () => tokensController.state, - getTokenListState: () => tokenList.state, - getNetworkState: () => defaultNetworkState, - getPreferencesState: () => preferences.state, - getNetworkClientById: jest.fn(), - }, - { - disabled: false, - isDetectionEnabledForNetwork: true, - isDetectionEnabledFromPreferences: false, - selectedAddress: '0x1', - }, - ); + tokenDetection = new TokenDetectionController({ + chainId: ChainId.mainnet, + selectedAddress: '0x1', + onPreferencesStateChange, + getBalancesInSingleCall: getBalancesInSingleCallMock, + addDetectedTokens: stub, + getTokensState: () => tokensController.state, + getPreferencesState: () => preferences.state, + messenger: buildTokenDetectionControllerMessenger(controllerMessenger), + }); // eslint-disable-next-line @typescript-eslint/no-non-null-assertion await preferencesStateChangeListener!({ selectedAddress: '0x1', useTokenDetection: true, }); - + await tokenDetection.start(); expect(getBalancesInSingleCallMock.called).toBe(true); }); - it('should call getBalancesInSingleCall if onNetworkStateChange is called with a chainId that supports token detection and is changed', async () => { + it('should call getBalancesInSingleCall if network is changed to a chainId that supports token detection', async () => { const stub = sinon.stub(); const getBalancesInSingleCallMock = sinon.stub(); - let networkStateChangeListener: (state: any) => void; - const onNetworkStateChange = sinon.stub().callsFake((listener) => { - networkStateChangeListener = listener; - }); - tokenDetection = new TokenDetectionController( - { - onNetworkStateChange, - onTokenListStateChange: stub, - onPreferencesStateChange: stub, - getBalancesInSingleCall: getBalancesInSingleCallMock, - addDetectedTokens: stub, - getTokensState: () => tokensController.state, - getTokenListState: () => tokenList.state, - getNetworkState: () => defaultNetworkState, - getPreferencesState: () => preferences.state, - getNetworkClientById: jest.fn(), - }, - { - disabled: false, - isDetectionEnabledFromPreferences: true, - chainId: SupportedTokenDetectionNetworks.polygon, - isDetectionEnabledForNetwork: true, - selectedAddress: '0x1', - }, - ); - - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - await networkStateChangeListener!({ - providerConfig: { chainId: ChainId.mainnet }, + tokenDetection = new TokenDetectionController({ + chainId: SupportedTokenDetectionNetworks.polygon, + selectedAddress: '0x1', + onPreferencesStateChange: stub, + getBalancesInSingleCall: getBalancesInSingleCallMock, + addDetectedTokens: stub, + getTokensState: () => tokensController.state, + getPreferencesState: () => preferences.state, + messenger: buildTokenDetectionControllerMessenger(controllerMessenger), }); + changeNetwork(mainnet); + await tokenDetection.start(); expect(getBalancesInSingleCallMock.called).toBe(true); }); - describe('startPollingByNetworkClientId', () => { let clock: sinon.SinonFakeTimers; beforeEach(() => { @@ -640,16 +573,13 @@ describe('TokenDetectionController', () => { describe('detectTokens', () => { it('should detect and add tokens by networkClientId correctly', async () => { const selectedAddress = '0x1'; - tokenDetection.configure({ - disabled: false, - }); + preferences.update({ selectedAddress }); + changeNetwork(mainnet); + getBalancesInSingleCall.resolves({ [sampleTokenA.address]: new BN(1), }); - await tokenDetection.detectTokens({ - networkClientId: 'mainnet', - accountAddress: selectedAddress, - }); + await tokenDetection.start(); const tokens = tokensController.state.allDetectedTokens[ChainId.mainnet][ selectedAddress From 2f74472447df5eae292d5c296428e157d2cbdfe5 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Mon, 11 Dec 2023 08:51:21 -0800 Subject: [PATCH 12/82] Fix test to avoid using deprecated aurora network --- packages/assets-controllers/jest.config.js | 2 +- .../src/TokenDetectionController.test.ts | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/assets-controllers/jest.config.js b/packages/assets-controllers/jest.config.js index 7c7fa8c850..9a42d59f82 100644 --- a/packages/assets-controllers/jest.config.js +++ b/packages/assets-controllers/jest.config.js @@ -17,7 +17,7 @@ module.exports = merge(baseConfig, { // An object that configures minimum threshold enforcement for coverage results coverageThreshold: { global: { - branches: 88.54, + branches: 88.22, functions: 95.66, lines: 96.46, statements: 96.52, diff --git a/packages/assets-controllers/src/TokenDetectionController.test.ts b/packages/assets-controllers/src/TokenDetectionController.test.ts index 03892ab80a..d99ead6645 100644 --- a/packages/assets-controllers/src/TokenDetectionController.test.ts +++ b/packages/assets-controllers/src/TokenDetectionController.test.ts @@ -284,15 +284,15 @@ describe('TokenDetectionController', () => { expect(tokensController.state.detectedTokens).toStrictEqual([sampleTokenA]); }); - it('should detect tokens correctly on the Aurora network', async () => { - const auroraMainnet = { - chainId: ChainId.aurora, - type: NetworkType.mainnet, - ticker: 'Aurora ETH', + it('should detect tokens correctly on the Polygon network', async () => { + const polygonRpc = { + chainId: SupportedTokenDetectionNetworks.polygon, + type: NetworkType.rpc, + ticker: 'Polygon ETH', }; const selectedAddress = '0x1'; preferences.update({ selectedAddress }); - changeNetwork(auroraMainnet); + changeNetwork(polygonRpc); await tokenDetection.start(); @@ -302,7 +302,7 @@ describe('TokenDetectionController', () => { await tokenDetection.detectTokens({ accountAddress: selectedAddress, - networkClientId: ChainId.aurora, + networkClientId: SupportedTokenDetectionNetworks.polygon, }); expect(tokensController.state.detectedTokens).toStrictEqual([sampleTokenA]); }); From 720dc698e6faa4d8a5f4c07cf3996453f03d7f26 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Mon, 11 Dec 2023 11:15:58 -0800 Subject: [PATCH 13/82] Assume that `getTokensState` never returns `undefined` - Will throw runtime error if `getTokensState` doesn't return valid tokens state: ``` TypeError: Cannot destructure property 'tokens' of '((cov_1sp0cd8ows(...).s[79]++) , __classPrivateFieldGet(...).call(...))' as it is undefined. ``` --- .../assets-controllers/src/TokenDetectionController.test.ts | 2 +- packages/assets-controllers/src/TokenDetectionController.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/assets-controllers/src/TokenDetectionController.test.ts b/packages/assets-controllers/src/TokenDetectionController.test.ts index d99ead6645..2cedcd3c8f 100644 --- a/packages/assets-controllers/src/TokenDetectionController.test.ts +++ b/packages/assets-controllers/src/TokenDetectionController.test.ts @@ -469,7 +469,7 @@ describe('TokenDetectionController', () => { onPreferencesStateChange: stub, getBalancesInSingleCall: getBalancesInSingleCallMock, addDetectedTokens: stub, - getTokensState: stub, + getTokensState: () => tokensController.state, getPreferencesState: () => preferences.state, messenger: buildTokenDetectionControllerMessenger(controllerMessenger), }); diff --git a/packages/assets-controllers/src/TokenDetectionController.ts b/packages/assets-controllers/src/TokenDetectionController.ts index 081c28788a..65de2f485b 100644 --- a/packages/assets-controllers/src/TokenDetectionController.ts +++ b/packages/assets-controllers/src/TokenDetectionController.ts @@ -284,10 +284,10 @@ export class TokenDetectionController extends PollingController< ) { return; } - const { tokens } = this.#getTokensState() ?? {}; + const { tokens } = this.#getTokensState(); const selectedAddress = accountAddress ?? this.#selectedAddress; const chainId = this.#getCorrectChainId(networkClientId); - const tokensAddresses = tokens?.map( + const tokensAddresses = tokens.map( /* istanbul ignore next*/ (token) => token.address.toLowerCase(), ); const { tokenList } = await this.messagingSystem.call( From 74f74ba8486f62b1132a3c632c4b65b697634c46 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Mon, 11 Dec 2023 11:16:58 -0800 Subject: [PATCH 14/82] Remove unnecessary cleanup function calls --- .../assets-controllers/src/TokenDetectionController.test.ts | 6 ------ 1 file changed, 6 deletions(-) diff --git a/packages/assets-controllers/src/TokenDetectionController.test.ts b/packages/assets-controllers/src/TokenDetectionController.test.ts index 2cedcd3c8f..6981b08b99 100644 --- a/packages/assets-controllers/src/TokenDetectionController.test.ts +++ b/packages/assets-controllers/src/TokenDetectionController.test.ts @@ -238,12 +238,6 @@ describe('TokenDetectionController', () => { sinon.restore(); tokenDetection.stop(); tokenList.destroy(); - controllerMessenger.clearEventSubscriptions( - 'NetworkController:stateChange', - ); - controllerMessenger.unregisterActionHandler( - 'NetworkController:getNetworkClientById', - ); }); it('should poll and detect tokens on interval while on supported networks', async () => { From 7b3d4e45a3fef59e3bf9f0bbf1f6d7e749548000 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Mon, 11 Dec 2023 11:17:31 -0800 Subject: [PATCH 15/82] Add jsdoc for test utility functions --- .../src/TokenDetectionController.test.ts | 33 ++++++++++++++----- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/packages/assets-controllers/src/TokenDetectionController.test.ts b/packages/assets-controllers/src/TokenDetectionController.test.ts index 6981b08b99..34c6c1e57d 100644 --- a/packages/assets-controllers/src/TokenDetectionController.test.ts +++ b/packages/assets-controllers/src/TokenDetectionController.test.ts @@ -103,13 +103,22 @@ type MainControllerMessenger = ControllerMessenger< AllowedEvents >; -const getControllerMessenger = (): MainControllerMessenger => { +/** + * Returns a new `MainControllerMessenger` instance that can be used to create restricted messengers. + * @returns The new `MainControllerMessenger` instance. + */ +function getControllerMessenger(): MainControllerMessenger { return new ControllerMessenger(); -}; +} -const setupTokenListController = ( +/** + * Sets up a `TokenListController` and its restricted messenger. + * @param controllerMessenger - The main controller messenger. + * @returns An object containing the TokenListController and its restricted messenger. + */ +function setupTokenListController( controllerMessenger: MainControllerMessenger, -) => { +) { const tokenListMessenger = controllerMessenger.getRestricted({ name: 'TokenListController', allowedActions: [], @@ -123,12 +132,17 @@ const setupTokenListController = ( }); return { tokenList, tokenListMessenger }; -}; +} -const buildTokenDetectionControllerMessenger: ( - controllerMessenger: MainControllerMessenger, -) => TokenDetectionControllerMessenger = (controllerMessenger) => - controllerMessenger.getRestricted({ +/** + * Builds a messenger that `TokenDetectionController` can use to communicate with other controllers. + * @param controllerMessenger - The main controller messenger. + * @returns The restricted messenger. + */ +function buildTokenDetectionControllerMessenger( + controllerMessenger?: MainControllerMessenger, +): TokenDetectionControllerMessenger { + return (controllerMessenger ?? getControllerMessenger()).getRestricted({ name: controllerName, allowedActions: [ 'NetworkController:getState', @@ -140,6 +154,7 @@ const buildTokenDetectionControllerMessenger: ( 'TokenListController:stateChange', ], }); +} describe('TokenDetectionController', () => { let tokenDetection: TokenDetectionController; From 2bef9b81b573db9b459f26b544b3c67824ed233e Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Mon, 11 Dec 2023 11:18:50 -0800 Subject: [PATCH 16/82] Fix unnecessary `await` --- packages/assets-controllers/src/TokenDetectionController.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/assets-controllers/src/TokenDetectionController.ts b/packages/assets-controllers/src/TokenDetectionController.ts index 65de2f485b..f618289c1c 100644 --- a/packages/assets-controllers/src/TokenDetectionController.ts +++ b/packages/assets-controllers/src/TokenDetectionController.ts @@ -290,7 +290,7 @@ export class TokenDetectionController extends PollingController< const tokensAddresses = tokens.map( /* istanbul ignore next*/ (token) => token.address.toLowerCase(), ); - const { tokenList } = await this.messagingSystem.call( + const { tokenList } = this.messagingSystem.call( 'TokenListController:getState', ); const tokensToDetect: string[] = []; From 9c8491a0a98c512999b4f789b50c358c7e061df2 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Mon, 11 Dec 2023 14:39:39 -0800 Subject: [PATCH 17/82] Use new network-controller action/event - Replace `NetworkControlerGetNetworkClientByIdAction` with `NetworkCotnrollerGetNetworkConfigurationByNetworkClientId` - Use `NetworkControllerNetworkDidChangeEvent` --- .../src/TokenDetectionController.test.ts | 13 +++------- .../src/TokenDetectionController.ts | 25 ++++++++++--------- 2 files changed, 17 insertions(+), 21 deletions(-) diff --git a/packages/assets-controllers/src/TokenDetectionController.test.ts b/packages/assets-controllers/src/TokenDetectionController.test.ts index 34c6c1e57d..19bdedc0cd 100644 --- a/packages/assets-controllers/src/TokenDetectionController.test.ts +++ b/packages/assets-controllers/src/TokenDetectionController.test.ts @@ -145,12 +145,12 @@ function buildTokenDetectionControllerMessenger( return (controllerMessenger ?? getControllerMessenger()).getRestricted({ name: controllerName, allowedActions: [ - 'NetworkController:getState', - 'NetworkController:getNetworkClientById', + 'NetworkController:getNetworkConfigurationByNetworkClientId', 'TokenListController:getState', ], allowedEvents: [ 'NetworkController:stateChange', + 'NetworkController:networkDidChange', 'TokenListController:stateChange', ], }); @@ -177,13 +177,8 @@ describe('TokenDetectionController', () => { }); controllerMessenger.registerActionHandler( - `NetworkController:getNetworkClientById`, - jest.fn().mockReturnValueOnce({ - configuration: providerConfig, - provider: {}, - blockTracker: {}, - destroy: jest.fn(), - }), + `NetworkController:getNetworkConfigurationByNetworkClientId`, + jest.fn().mockReturnValueOnce(providerConfig), ); }; const mainnet = { diff --git a/packages/assets-controllers/src/TokenDetectionController.ts b/packages/assets-controllers/src/TokenDetectionController.ts index f618289c1c..d8611e25e0 100644 --- a/packages/assets-controllers/src/TokenDetectionController.ts +++ b/packages/assets-controllers/src/TokenDetectionController.ts @@ -9,8 +9,8 @@ import { } from '@metamask/controller-utils'; import type { NetworkClientId, - NetworkControllerGetNetworkClientByIdAction, - NetworkControllerGetStateAction, + NetworkControllerGetNetworkConfigurationByNetworkClientId, + NetworkControllerNetworkDidChangeEvent, NetworkControllerStateChangeEvent, } from '@metamask/network-controller'; import { PollingController } from '@metamask/polling-controller'; @@ -41,8 +41,7 @@ export type TokenDetectionControllerActions = TokenDetectionControllerGetStateAction; export type AllowedActions = - | NetworkControllerGetStateAction - | NetworkControllerGetNetworkClientByIdAction + | NetworkControllerGetNetworkConfigurationByNetworkClientId | GetTokenListState; export type TokenDetectionControllerStateChangeEvent = @@ -53,6 +52,7 @@ export type TokenDetectionControllerEvents = export type AllowedEvents = | NetworkControllerStateChangeEvent + | NetworkControllerNetworkDidChangeEvent | TokenListStateChange; export type TokenDetectionControllerMessenger = RestrictedControllerMessenger< @@ -189,7 +189,7 @@ export class TokenDetectionController extends PollingController< ); this.messagingSystem.subscribe( - 'NetworkController:stateChange', + 'NetworkController:networkDidChange', ({ providerConfig: { chainId: newChainId } }) => { this.#isDetectionEnabledForNetwork = isTokenDetectionSupportedForNetwork(newChainId); @@ -244,13 +244,14 @@ export class TokenDetectionController extends PollingController< #getCorrectChainId(networkClientId?: NetworkClientId) { if (networkClientId) { - const { - configuration: { chainId }, - } = this.messagingSystem.call( - 'NetworkController:getNetworkClientById', - networkClientId, - ); - this.#chainId = chainId; + const { chainId } = + this.messagingSystem.call( + 'NetworkController:getNetworkConfigurationByNetworkClientId', + networkClientId, + ) ?? {}; + if (chainId) { + this.#chainId = chainId; + } } return this.#chainId; } From 05ed86bb0c75a20fbe4337a0cf4b65c0cd8e9fec Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Mon, 11 Dec 2023 14:46:39 -0800 Subject: [PATCH 18/82] Fix tests --- packages/assets-controllers/jest.config.js | 8 ++--- .../src/TokenDetectionController.test.ts | 33 ++++++++++++------- 2 files changed, 26 insertions(+), 15 deletions(-) diff --git a/packages/assets-controllers/jest.config.js b/packages/assets-controllers/jest.config.js index 9a42d59f82..14057fb832 100644 --- a/packages/assets-controllers/jest.config.js +++ b/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.22, - functions: 95.66, - lines: 96.46, - statements: 96.52, + branches: 89.03, + functions: 96.49, + lines: 96.96, + statements: 97.02, }, }, diff --git a/packages/assets-controllers/src/TokenDetectionController.test.ts b/packages/assets-controllers/src/TokenDetectionController.test.ts index 19bdedc0cd..3796f0c531 100644 --- a/packages/assets-controllers/src/TokenDetectionController.test.ts +++ b/packages/assets-controllers/src/TokenDetectionController.test.ts @@ -186,6 +186,11 @@ describe('TokenDetectionController', () => { type: NetworkType.mainnet, ticker: NetworksTicker.mainnet, }; + const goerli = { + chainId: ChainId.goerli, + type: NetworkType.goerli, + ticker: NetworksTicker.goerli, + }; beforeEach(async () => { nock(TOKEN_END_POINT_API) @@ -265,15 +270,16 @@ describe('TokenDetectionController', () => { }); it('should not autodetect while not on supported networks', async () => { - await tokenDetection.detectTokens({ - accountAddress: '0x1', - networkClientId: ChainId.goerli, - }); + changeNetwork(goerli); + await tokenDetection.start(); getBalancesInSingleCall.resolves({ [sampleTokenA.address]: new BN(1), }); - await tokenDetection.start(); + await tokenDetection.detectTokens({ + accountAddress: '0x1', + networkClientId: ChainId.goerli, + }); expect(tokensController.state.detectedTokens).toStrictEqual([]); }); @@ -289,14 +295,14 @@ describe('TokenDetectionController', () => { }); it('should detect tokens correctly on the Polygon network', async () => { - const polygonRpc = { + const polygon = { chainId: SupportedTokenDetectionNetworks.polygon, type: NetworkType.rpc, ticker: 'Polygon ETH', }; const selectedAddress = '0x1'; preferences.update({ selectedAddress }); - changeNetwork(polygonRpc); + changeNetwork(polygon); await tokenDetection.start(); @@ -576,16 +582,21 @@ describe('TokenDetectionController', () => { describe('detectTokens', () => { it('should detect and add tokens by networkClientId correctly', async () => { - const selectedAddress = '0x1'; + const selectedAddress = '0x2'; preferences.update({ selectedAddress }); - changeNetwork(mainnet); + changeNetwork(goerli); + + await tokenDetection.start(); getBalancesInSingleCall.resolves({ [sampleTokenA.address]: new BN(1), }); - await tokenDetection.start(); + await tokenDetection.detectTokens({ + networkClientId: ChainId.goerli, + accountAddress: selectedAddress, + }); const tokens = - tokensController.state.allDetectedTokens[ChainId.mainnet][ + tokensController.state.allDetectedTokens[ChainId.goerli][ selectedAddress ]; expect(tokens).toStrictEqual([sampleTokenA]); From 8964e0d21d908f391b315eaad926c69f7437c485 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Mon, 11 Dec 2023 15:36:54 -0800 Subject: [PATCH 19/82] Move `unregisterActionhandler` calls for `TokenDetectionController:getState` to before `super` call in constructor --- packages/assets-controllers/jest.config.js | 2 +- packages/assets-controllers/src/TokenDetectionController.ts | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/assets-controllers/jest.config.js b/packages/assets-controllers/jest.config.js index 14057fb832..079ae91b83 100644 --- a/packages/assets-controllers/jest.config.js +++ b/packages/assets-controllers/jest.config.js @@ -20,7 +20,7 @@ module.exports = merge(baseConfig, { branches: 89.03, functions: 96.49, lines: 96.96, - statements: 97.02, + statements: 97.01, }, }, diff --git a/packages/assets-controllers/src/TokenDetectionController.ts b/packages/assets-controllers/src/TokenDetectionController.ts index d8611e25e0..06c16b3eaa 100644 --- a/packages/assets-controllers/src/TokenDetectionController.ts +++ b/packages/assets-controllers/src/TokenDetectionController.ts @@ -139,6 +139,8 @@ export class TokenDetectionController extends PollingController< const { useTokenDetection: defaultUseTokenDetection } = getPreferencesState(); + messenger.unregisterActionHandler('TokenDetectionController:getState'); + super({ name: controllerName, messenger, @@ -154,10 +156,6 @@ export class TokenDetectionController extends PollingController< this.#addDetectedTokens = addDetectedTokens; this.#getBalancesInSingleCall = getBalancesInSingleCall; - this.messagingSystem.unregisterActionHandler( - 'TokenDetectionController:getState', - ); - this.messagingSystem.subscribe( 'TokenListController:stateChange', ({ tokenList }) => { From 26509c18d2bf03f99db0fee98c17e08ef9e5a98e Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Mon, 11 Dec 2023 15:53:05 -0800 Subject: [PATCH 20/82] Remove `disabled` class field entirely --- packages/assets-controllers/src/TokenDetectionController.ts | 6 ------ 1 file changed, 6 deletions(-) diff --git a/packages/assets-controllers/src/TokenDetectionController.ts b/packages/assets-controllers/src/TokenDetectionController.ts index 06c16b3eaa..797a92c1d7 100644 --- a/packages/assets-controllers/src/TokenDetectionController.ts +++ b/packages/assets-controllers/src/TokenDetectionController.ts @@ -65,7 +65,6 @@ export type TokenDetectionControllerMessenger = RestrictedControllerMessenger< /** * Controller that passively polls on a set interval for Tokens auto detection - * @property disabled - Determines if this controller is enabled * @property interval - Polling interval used to fetch new token rates * @property selectedAddress - Vault selected address * @property chainId - The chain ID of the current network @@ -77,8 +76,6 @@ export class TokenDetectionController extends PollingController< TokenDetectionState, TokenDetectionControllerMessenger > { - #disabled = true; - #intervalId?: ReturnType; #chainId: Hex; @@ -211,7 +208,6 @@ export class TokenDetectionController extends PollingController< * Start polling for detected tokens. */ async start() { - this.#disabled = false; await this.#startPolling(); } @@ -219,7 +215,6 @@ export class TokenDetectionController extends PollingController< * Stop polling for detected tokens. */ stop() { - this.#disabled = true; this.#stopPolling(); } @@ -277,7 +272,6 @@ export class TokenDetectionController extends PollingController< }): Promise { const { networkClientId, accountAddress } = options ?? {}; if ( - this.#disabled || !this.#isDetectionEnabledForNetwork || !this.#isDetectionEnabledFromPreferences ) { From cb2bd08f7ad14314cbc594d9091cefcfcb37e586 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Mon, 11 Dec 2023 15:57:13 -0800 Subject: [PATCH 21/82] Remove unnecessary `tokenDetect.start()` calls from tests --- packages/assets-controllers/jest.config.js | 6 +++--- .../assets-controllers/src/TokenDetectionController.test.ts | 6 ------ 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/packages/assets-controllers/jest.config.js b/packages/assets-controllers/jest.config.js index 079ae91b83..e931960a34 100644 --- a/packages/assets-controllers/jest.config.js +++ b/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: 89.03, + branches: 89.01, functions: 96.49, - lines: 96.96, - statements: 97.01, + lines: 96.88, + statements: 96.94, }, }, diff --git a/packages/assets-controllers/src/TokenDetectionController.test.ts b/packages/assets-controllers/src/TokenDetectionController.test.ts index 3796f0c531..574bd38e05 100644 --- a/packages/assets-controllers/src/TokenDetectionController.test.ts +++ b/packages/assets-controllers/src/TokenDetectionController.test.ts @@ -271,7 +271,6 @@ describe('TokenDetectionController', () => { it('should not autodetect while not on supported networks', async () => { changeNetwork(goerli); - await tokenDetection.start(); getBalancesInSingleCall.resolves({ [sampleTokenA.address]: new BN(1), @@ -304,8 +303,6 @@ describe('TokenDetectionController', () => { preferences.update({ selectedAddress }); changeNetwork(polygon); - await tokenDetection.start(); - getBalancesInSingleCall.resolves({ [sampleTokenA.address]: new BN(1), }); @@ -485,7 +482,6 @@ describe('TokenDetectionController', () => { }); tokenList.clearingTokenListData(); - await tokenDetection.start(); expect(getBalancesInSingleCallMock.called).toBe(false); }); @@ -586,8 +582,6 @@ describe('TokenDetectionController', () => { preferences.update({ selectedAddress }); changeNetwork(goerli); - await tokenDetection.start(); - getBalancesInSingleCall.resolves({ [sampleTokenA.address]: new BN(1), }); From 92448034fb15e6d5f6fe26a830aa209856563d50 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Mon, 11 Dec 2023 16:00:46 -0800 Subject: [PATCH 22/82] Record CHANGELOG entries --- packages/assets-controllers/CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/assets-controllers/CHANGELOG.md b/packages/assets-controllers/CHANGELOG.md index 1791d2f4b0..7d4fed45a5 100644 --- a/packages/assets-controllers/CHANGELOG.md +++ b/packages/assets-controllers/CHANGELOG.md @@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Add `codefiTokenPricesServiceV2` ([#3600](https://github.com/MetaMask/core/pull/3600)) - This object can be used for the new `tokenPricesService` argument for TokenRatesController. It uses an internal API to fetch prices for tokens instead of CoinGecko. +- `TokenListController` now exports a `TokenListControllerMessenger` type ([#3609](https://github.com/MetaMask/core/pull/3609)). ### Changed - **BREAKING:** TokenRatesController now takes a required argument `tokenPricesService` ([#3600](https://github.com/MetaMask/core/pull/3600)) @@ -18,6 +19,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **BREAKING:** Change signature of `TokenRatesController.fetchAndMapExchangeRates` ([#3600](https://github.com/MetaMask/core/pull/3600)) - This method now takes an object with shape `{ tokenContractAddresses: Hex[]; chainId: Hex; nativeCurrency: string; }` rather than positional arguments - Update TokenListController to fetch prefiltered set of tokens from the API, reducing response data and removing the need for filtering logic ([#2054](https://github.com/MetaMask/core/pull/2054)) +- **BREAKING:** `TokenDetectionController` is upgraded to extend `PollingController` ([#3609](https://github.com/MetaMask/core/pull/3609)). + - The constructor now expects an options object as its only argument, with required properties `messenger`, `chainId`, `onPreferencesStateChange`, `getBalancesInSingleCall`, `addDetectedTokens`, `getTokensState`, `getPreferencesState`, and optional properties `interval`, `selectedAddress`. + - The state property is an empty object. +- The `detectTokens` method of `TokenDetectionController` can now handle any number of tokens. Previously the limit was 2000 ([#3609](https://github.com/MetaMask/core/pull/3609)). ### Removed - **BREAKING:** Remove `fetchExchangeRate` method from TokenRatesController ([#3600](https://github.com/MetaMask/core/pull/3600)) From 1274137ca7d7bdbb6d00587419e45fd486c41ae2 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Mon, 11 Dec 2023 17:03:29 -0800 Subject: [PATCH 23/82] Reorder constructor assignments --- .../assets-controllers/src/TokenDetectionController.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/assets-controllers/src/TokenDetectionController.ts b/packages/assets-controllers/src/TokenDetectionController.ts index 797a92c1d7..6e1ca68347 100644 --- a/packages/assets-controllers/src/TokenDetectionController.ts +++ b/packages/assets-controllers/src/TokenDetectionController.ts @@ -148,6 +148,10 @@ export class TokenDetectionController extends PollingController< this.setIntervalLength(interval); this.#chainId = chainId; this.#selectedAddress = selectedAddress; + this.#isDetectionEnabledFromPreferences = defaultUseTokenDetection; + this.#isDetectionEnabledForNetwork = isTokenDetectionSupportedForNetwork( + this.#chainId, + ); this.#getTokensState = getTokensState; this.#addDetectedTokens = addDetectedTokens; @@ -197,11 +201,6 @@ export class TokenDetectionController extends PollingController< } }, ); - - this.#isDetectionEnabledFromPreferences = defaultUseTokenDetection; - this.#isDetectionEnabledForNetwork = isTokenDetectionSupportedForNetwork( - this.#chainId, - ); } /** From 0c0476c4053f0be89d7d7ccf7aa2a179409e6ff7 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Mon, 11 Dec 2023 17:03:43 -0800 Subject: [PATCH 24/82] Await all `detectTokens()` calls --- .../src/TokenDetectionController.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/assets-controllers/src/TokenDetectionController.ts b/packages/assets-controllers/src/TokenDetectionController.ts index 6e1ca68347..10b737bda9 100644 --- a/packages/assets-controllers/src/TokenDetectionController.ts +++ b/packages/assets-controllers/src/TokenDetectionController.ts @@ -159,17 +159,17 @@ export class TokenDetectionController extends PollingController< this.messagingSystem.subscribe( 'TokenListController:stateChange', - ({ tokenList }) => { + async ({ tokenList }) => { const hasTokens = Object.keys(tokenList).length; if (hasTokens) { - this.detectTokens(); + await this.detectTokens(); } }, ); onPreferencesStateChange( - ({ selectedAddress: newSelectedAddress, useTokenDetection }) => { + async ({ selectedAddress: newSelectedAddress, useTokenDetection }) => { const isSelectedAddressChanged = this.#selectedAddress !== newSelectedAddress; const isDetectionChangedFromPreferences = @@ -182,14 +182,14 @@ export class TokenDetectionController extends PollingController< useTokenDetection && (isSelectedAddressChanged || isDetectionChangedFromPreferences) ) { - this.detectTokens(); + await this.detectTokens(); } }, ); this.messagingSystem.subscribe( 'NetworkController:networkDidChange', - ({ providerConfig: { chainId: newChainId } }) => { + async ({ providerConfig: { chainId: newChainId } }) => { this.#isDetectionEnabledForNetwork = isTokenDetectionSupportedForNetwork(newChainId); const isChainIdChanged = this.#chainId !== newChainId; @@ -197,7 +197,7 @@ export class TokenDetectionController extends PollingController< this.#chainId = newChainId; if (this.#isDetectionEnabledForNetwork && isChainIdChanged) { - this.detectTokens(); + await this.detectTokens(); } }, ); @@ -229,8 +229,8 @@ export class TokenDetectionController extends PollingController< async #startPolling(): Promise { this.#stopPolling(); await this.detectTokens(); - this.#intervalId = setInterval(() => { - this.detectTokens(); + this.#intervalId = setInterval(async () => { + await this.detectTokens(); }, this.getIntervalLength()); } From 8dc1fcd17cfdf1a2525431e52c25f9d199e887e9 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Mon, 11 Dec 2023 17:16:38 -0800 Subject: [PATCH 25/82] Test fixes --- packages/assets-controllers/jest.config.js | 6 +++--- .../src/TokenDetectionController.test.ts | 14 ++++++-------- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/packages/assets-controllers/jest.config.js b/packages/assets-controllers/jest.config.js index e931960a34..e7fa34f9ed 100644 --- a/packages/assets-controllers/jest.config.js +++ b/packages/assets-controllers/jest.config.js @@ -18,9 +18,9 @@ module.exports = merge(baseConfig, { coverageThreshold: { global: { branches: 89.01, - functions: 96.49, - lines: 96.88, - statements: 96.94, + functions: 96.22, + lines: 96.81, + statements: 96.87, }, }, diff --git a/packages/assets-controllers/src/TokenDetectionController.test.ts b/packages/assets-controllers/src/TokenDetectionController.test.ts index 574bd38e05..c9a159912a 100644 --- a/packages/assets-controllers/src/TokenDetectionController.test.ts +++ b/packages/assets-controllers/src/TokenDetectionController.test.ts @@ -222,7 +222,6 @@ describe('TokenDetectionController', () => { onNetworkDidChange: (listener) => onNetworkDidChangeListeners.push(listener), onTokenListStateChange: sinon.stub(), - getERC20TokenName: sinon.stub(), getNetworkClientById: sinon.stub() as any, messenger: undefined as unknown as TokensControllerMessenger, }); @@ -294,14 +293,13 @@ describe('TokenDetectionController', () => { }); it('should detect tokens correctly on the Polygon network', async () => { - const polygon = { - chainId: SupportedTokenDetectionNetworks.polygon, - type: NetworkType.rpc, - ticker: 'Polygon ETH', - }; const selectedAddress = '0x1'; preferences.update({ selectedAddress }); - changeNetwork(polygon); + changeNetwork({ + chainId: SupportedTokenDetectionNetworks.polygon, + type: NetworkType.goerli, + ticker: NetworksTicker.goerli, + }); getBalancesInSingleCall.resolves({ [sampleTokenA.address]: new BN(1), @@ -504,7 +502,7 @@ describe('TokenDetectionController', () => { }); // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - await preferencesStateChangeListener!({ + preferencesStateChangeListener!({ selectedAddress: '0x1', useTokenDetection: true, }); From fdb70d9230013b9c6b46cfb666439b82a51dc1b2 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Mon, 11 Dec 2023 20:26:15 -0800 Subject: [PATCH 26/82] Add `deferPollingStart` property --- packages/assets-controllers/jest.config.js | 6 +++--- .../src/TokenDetectionController.test.ts | 4 ++-- .../assets-controllers/src/TokenDetectionController.ts | 7 +++++++ 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/packages/assets-controllers/jest.config.js b/packages/assets-controllers/jest.config.js index e7fa34f9ed..655ba2e31b 100644 --- a/packages/assets-controllers/jest.config.js +++ b/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: 89.01, + branches: 89.04, functions: 96.22, - lines: 96.81, - statements: 96.87, + lines: 96.82, + statements: 96.88, }, }, diff --git a/packages/assets-controllers/src/TokenDetectionController.test.ts b/packages/assets-controllers/src/TokenDetectionController.test.ts index c9a159912a..22ef33cb1d 100644 --- a/packages/assets-controllers/src/TokenDetectionController.test.ts +++ b/packages/assets-controllers/src/TokenDetectionController.test.ts @@ -491,6 +491,7 @@ describe('TokenDetectionController', () => { preferencesStateChangeListener = listener; }); tokenDetection = new TokenDetectionController({ + deferPollingStart: false, chainId: ChainId.mainnet, selectedAddress: '0x1', onPreferencesStateChange, @@ -506,7 +507,6 @@ describe('TokenDetectionController', () => { selectedAddress: '0x1', useTokenDetection: true, }); - await tokenDetection.start(); expect(getBalancesInSingleCallMock.called).toBe(true); }); @@ -514,6 +514,7 @@ describe('TokenDetectionController', () => { const stub = sinon.stub(); const getBalancesInSingleCallMock = sinon.stub(); tokenDetection = new TokenDetectionController({ + deferPollingStart: false, chainId: SupportedTokenDetectionNetworks.polygon, selectedAddress: '0x1', onPreferencesStateChange: stub, @@ -525,7 +526,6 @@ describe('TokenDetectionController', () => { }); changeNetwork(mainnet); - await tokenDetection.start(); expect(getBalancesInSingleCallMock.called).toBe(true); }); describe('startPollingByNetworkClientId', () => { diff --git a/packages/assets-controllers/src/TokenDetectionController.ts b/packages/assets-controllers/src/TokenDetectionController.ts index 10b737bda9..24cd196dc8 100644 --- a/packages/assets-controllers/src/TokenDetectionController.ts +++ b/packages/assets-controllers/src/TokenDetectionController.ts @@ -101,6 +101,7 @@ export class TokenDetectionController extends PollingController< * * @param options - The controller options. * @param options.messenger - The controller messaging system. + * @param options.deferPollingStart - Defers polling for detected tokens until `start` is called. * @param options.interval - Polling interval used to fetch new token rates * @param options.chainId - The chain ID of the current network * @param options.selectedAddress - Vault selected address @@ -111,6 +112,7 @@ export class TokenDetectionController extends PollingController< * @param options.getPreferencesState - Gets the state of the preferences controller. */ constructor({ + deferPollingStart = true, chainId, selectedAddress = '', interval = DEFAULT_INTERVAL, @@ -121,6 +123,7 @@ export class TokenDetectionController extends PollingController< getPreferencesState, messenger, }: { + deferPollingStart?: boolean; chainId: Hex; selectedAddress?: string; interval?: number; @@ -201,6 +204,10 @@ export class TokenDetectionController extends PollingController< } }, ); + + if (!deferPollingStart) { + this.start(); + } } /** From 98e8d177bc387cc7df7b608c961d129de751ff5e Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Mon, 11 Dec 2023 21:15:13 -0800 Subject: [PATCH 27/82] Fix test --- packages/assets-controllers/jest.config.js | 2 +- .../assets-controllers/src/TokenDetectionController.test.ts | 6 +----- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/packages/assets-controllers/jest.config.js b/packages/assets-controllers/jest.config.js index 655ba2e31b..ab736a7578 100644 --- a/packages/assets-controllers/jest.config.js +++ b/packages/assets-controllers/jest.config.js @@ -17,7 +17,7 @@ module.exports = merge(baseConfig, { // An object that configures minimum threshold enforcement for coverage results coverageThreshold: { global: { - branches: 89.04, + branches: 88.92, functions: 96.22, lines: 96.82, statements: 96.88, diff --git a/packages/assets-controllers/src/TokenDetectionController.test.ts b/packages/assets-controllers/src/TokenDetectionController.test.ts index 22ef33cb1d..6a057bc65f 100644 --- a/packages/assets-controllers/src/TokenDetectionController.test.ts +++ b/packages/assets-controllers/src/TokenDetectionController.test.ts @@ -270,14 +270,10 @@ describe('TokenDetectionController', () => { it('should not autodetect while not on supported networks', async () => { changeNetwork(goerli); - getBalancesInSingleCall.resolves({ [sampleTokenA.address]: new BN(1), }); - await tokenDetection.detectTokens({ - accountAddress: '0x1', - networkClientId: ChainId.goerli, - }); + await tokenDetection.start(); expect(tokensController.state.detectedTokens).toStrictEqual([]); }); From 8ad12b5421663432157d7b8958e1ad447b7c9c40 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Tue, 12 Dec 2023 10:42:46 -0800 Subject: [PATCH 28/82] Fix test to avoid implicitly calling `detectTokens()` --- packages/assets-controllers/jest.config.js | 8 ++++---- .../src/TokenDetectionController.test.ts | 15 ++++++++++----- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/packages/assets-controllers/jest.config.js b/packages/assets-controllers/jest.config.js index ab736a7578..05f6d6efd5 100644 --- a/packages/assets-controllers/jest.config.js +++ b/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.92, - functions: 96.22, - lines: 96.82, - statements: 96.88, + branches: 89.11, + functions: 96.3, + lines: 96.73, + statements: 96.79, }, }, diff --git a/packages/assets-controllers/src/TokenDetectionController.test.ts b/packages/assets-controllers/src/TokenDetectionController.test.ts index 6a057bc65f..62c221466c 100644 --- a/packages/assets-controllers/src/TokenDetectionController.test.ts +++ b/packages/assets-controllers/src/TokenDetectionController.test.ts @@ -176,6 +176,9 @@ describe('TokenDetectionController', () => { }); }); + controllerMessenger.unregisterActionHandler( + `NetworkController:getNetworkConfigurationByNetworkClientId`, + ); controllerMessenger.registerActionHandler( `NetworkController:getNetworkConfigurationByNetworkClientId`, jest.fn().mockReturnValueOnce(providerConfig), @@ -243,6 +246,11 @@ describe('TokenDetectionController', () => { messenger: buildTokenDetectionControllerMessenger(controllerMessenger), }); + controllerMessenger.registerActionHandler( + `NetworkController:getNetworkConfigurationByNetworkClientId`, + jest.fn().mockReturnValueOnce(mainnet), + ); + sinon .stub(tokensController, '_detectIsERC721') .callsFake(() => Promise.resolve(false)); @@ -573,18 +581,15 @@ describe('TokenDetectionController', () => { describe('detectTokens', () => { it('should detect and add tokens by networkClientId correctly', async () => { const selectedAddress = '0x2'; - preferences.update({ selectedAddress }); - changeNetwork(goerli); - getBalancesInSingleCall.resolves({ [sampleTokenA.address]: new BN(1), }); await tokenDetection.detectTokens({ - networkClientId: ChainId.goerli, + networkClientId: ChainId.mainnet, accountAddress: selectedAddress, }); const tokens = - tokensController.state.allDetectedTokens[ChainId.goerli][ + tokensController.state.allDetectedTokens[ChainId.mainnet][ selectedAddress ]; expect(tokens).toStrictEqual([sampleTokenA]); From 5e94d1311db2e6cf3a3c9330bb84b6adabaabcae Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Tue, 12 Dec 2023 11:06:47 -0800 Subject: [PATCH 29/82] Fix `#getCorrectChainId` and polygon network test to pull network config for current `chainId` value if `networkClientId` is not provided --- packages/assets-controllers/jest.config.js | 8 ++++---- .../src/TokenDetectionController.test.ts | 13 ++++--------- .../src/TokenDetectionController.ts | 16 +++++++--------- 3 files changed, 15 insertions(+), 22 deletions(-) diff --git a/packages/assets-controllers/jest.config.js b/packages/assets-controllers/jest.config.js index 05f6d6efd5..90e1b4e92d 100644 --- a/packages/assets-controllers/jest.config.js +++ b/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: 89.11, - functions: 96.3, - lines: 96.73, - statements: 96.79, + branches: 89.19, + functions: 96.22, + lines: 96.82, + statements: 96.87, }, }, diff --git a/packages/assets-controllers/src/TokenDetectionController.test.ts b/packages/assets-controllers/src/TokenDetectionController.test.ts index 62c221466c..34010d3fb7 100644 --- a/packages/assets-controllers/src/TokenDetectionController.test.ts +++ b/packages/assets-controllers/src/TokenDetectionController.test.ts @@ -297,22 +297,17 @@ describe('TokenDetectionController', () => { }); it('should detect tokens correctly on the Polygon network', async () => { - const selectedAddress = '0x1'; - preferences.update({ selectedAddress }); + preferences.update({ selectedAddress: '0x2' }); changeNetwork({ chainId: SupportedTokenDetectionNetworks.polygon, - type: NetworkType.goerli, - ticker: NetworksTicker.goerli, + type: NetworkType.rpc, + ticker: NetworksTicker.rpc, }); getBalancesInSingleCall.resolves({ [sampleTokenA.address]: new BN(1), }); - - await tokenDetection.detectTokens({ - accountAddress: selectedAddress, - networkClientId: SupportedTokenDetectionNetworks.polygon, - }); + await tokenDetection.start(); expect(tokensController.state.detectedTokens).toStrictEqual([sampleTokenA]); }); diff --git a/packages/assets-controllers/src/TokenDetectionController.ts b/packages/assets-controllers/src/TokenDetectionController.ts index 24cd196dc8..d35a10e5d7 100644 --- a/packages/assets-controllers/src/TokenDetectionController.ts +++ b/packages/assets-controllers/src/TokenDetectionController.ts @@ -242,15 +242,13 @@ export class TokenDetectionController extends PollingController< } #getCorrectChainId(networkClientId?: NetworkClientId) { - if (networkClientId) { - const { chainId } = - this.messagingSystem.call( - 'NetworkController:getNetworkConfigurationByNetworkClientId', - networkClientId, - ) ?? {}; - if (chainId) { - this.#chainId = chainId; - } + const { chainId } = + this.messagingSystem.call( + 'NetworkController:getNetworkConfigurationByNetworkClientId', + networkClientId ?? this.#chainId, + ) ?? {}; + if (chainId) { + this.#chainId = chainId; } return this.#chainId; } From a2007f6b86c1115fcfd794c64d181543f862b0f0 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Tue, 12 Dec 2023 11:21:03 -0800 Subject: [PATCH 30/82] Update jest coverage thresholds --- packages/assets-controllers/jest.config.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/assets-controllers/jest.config.js b/packages/assets-controllers/jest.config.js index 90e1b4e92d..06fd8046cf 100644 --- a/packages/assets-controllers/jest.config.js +++ b/packages/assets-controllers/jest.config.js @@ -18,9 +18,9 @@ module.exports = merge(baseConfig, { coverageThreshold: { global: { branches: 89.19, - functions: 96.22, - lines: 96.82, - statements: 96.87, + functions: 96.3, + lines: 96.73, + statements: 96.79, }, }, From 3532bbc275187e36dcd2aea5cd2aa98479fd3ab6 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Tue, 12 Dec 2023 13:36:39 -0800 Subject: [PATCH 31/82] Remove `deferPollingStart` constructor option --- packages/assets-controllers/jest.config.js | 8 ++++---- .../src/TokenDetectionController.test.ts | 4 ++-- .../assets-controllers/src/TokenDetectionController.ts | 7 ------- 3 files changed, 6 insertions(+), 13 deletions(-) diff --git a/packages/assets-controllers/jest.config.js b/packages/assets-controllers/jest.config.js index 06fd8046cf..c21630b8ca 100644 --- a/packages/assets-controllers/jest.config.js +++ b/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: 89.19, - functions: 96.3, - lines: 96.73, - statements: 96.79, + branches: 89.16, + functions: 96.34, + lines: 96.77, + statements: 96.83, }, }, diff --git a/packages/assets-controllers/src/TokenDetectionController.test.ts b/packages/assets-controllers/src/TokenDetectionController.test.ts index 34010d3fb7..dcc1312331 100644 --- a/packages/assets-controllers/src/TokenDetectionController.test.ts +++ b/packages/assets-controllers/src/TokenDetectionController.test.ts @@ -490,7 +490,6 @@ describe('TokenDetectionController', () => { preferencesStateChangeListener = listener; }); tokenDetection = new TokenDetectionController({ - deferPollingStart: false, chainId: ChainId.mainnet, selectedAddress: '0x1', onPreferencesStateChange, @@ -500,6 +499,7 @@ describe('TokenDetectionController', () => { getPreferencesState: () => preferences.state, messenger: buildTokenDetectionControllerMessenger(controllerMessenger), }); + await tokenDetection.start(); // eslint-disable-next-line @typescript-eslint/no-non-null-assertion preferencesStateChangeListener!({ @@ -513,7 +513,6 @@ describe('TokenDetectionController', () => { const stub = sinon.stub(); const getBalancesInSingleCallMock = sinon.stub(); tokenDetection = new TokenDetectionController({ - deferPollingStart: false, chainId: SupportedTokenDetectionNetworks.polygon, selectedAddress: '0x1', onPreferencesStateChange: stub, @@ -523,6 +522,7 @@ describe('TokenDetectionController', () => { getPreferencesState: () => preferences.state, messenger: buildTokenDetectionControllerMessenger(controllerMessenger), }); + await tokenDetection.start(); changeNetwork(mainnet); expect(getBalancesInSingleCallMock.called).toBe(true); diff --git a/packages/assets-controllers/src/TokenDetectionController.ts b/packages/assets-controllers/src/TokenDetectionController.ts index d35a10e5d7..6a83ff5af1 100644 --- a/packages/assets-controllers/src/TokenDetectionController.ts +++ b/packages/assets-controllers/src/TokenDetectionController.ts @@ -101,7 +101,6 @@ export class TokenDetectionController extends PollingController< * * @param options - The controller options. * @param options.messenger - The controller messaging system. - * @param options.deferPollingStart - Defers polling for detected tokens until `start` is called. * @param options.interval - Polling interval used to fetch new token rates * @param options.chainId - The chain ID of the current network * @param options.selectedAddress - Vault selected address @@ -112,7 +111,6 @@ export class TokenDetectionController extends PollingController< * @param options.getPreferencesState - Gets the state of the preferences controller. */ constructor({ - deferPollingStart = true, chainId, selectedAddress = '', interval = DEFAULT_INTERVAL, @@ -123,7 +121,6 @@ export class TokenDetectionController extends PollingController< getPreferencesState, messenger, }: { - deferPollingStart?: boolean; chainId: Hex; selectedAddress?: string; interval?: number; @@ -204,10 +201,6 @@ export class TokenDetectionController extends PollingController< } }, ); - - if (!deferPollingStart) { - this.start(); - } } /** From 322d0741eb15c2f5acb71ada6ff6969f9be1d6f5 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Tue, 12 Dec 2023 13:46:04 -0800 Subject: [PATCH 32/82] Add controller types to package-level exports --- packages/assets-controllers/src/index.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/assets-controllers/src/index.ts b/packages/assets-controllers/src/index.ts index 1824028506..aa20b3c44a 100644 --- a/packages/assets-controllers/src/index.ts +++ b/packages/assets-controllers/src/index.ts @@ -4,7 +4,13 @@ export * from './CurrencyRateController'; export * from './NftController'; export * from './NftDetectionController'; export * from './TokenBalancesController'; -export type { TokenDetectionControllerMessenger } from './TokenDetectionController'; +export type { + TokenDetectionControllerMessenger, + TokenDetectionControllerActions, + TokenDetectionControllerGetStateAction, + TokenDetectionControllerEvents, + TokenDetectionControllerStateChangeEvent, +} from './TokenDetectionController'; export { TokenDetectionController } from './TokenDetectionController'; export * from './TokenListController'; export * from './TokenRatesController'; From b60ada59625295735d6c230ac877c7b3d53e090b Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Tue, 12 Dec 2023 13:49:48 -0800 Subject: [PATCH 33/82] Update CHANGELOG records --- packages/assets-controllers/CHANGELOG.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/assets-controllers/CHANGELOG.md b/packages/assets-controllers/CHANGELOG.md index 7d4fed45a5..0c38093492 100644 --- a/packages/assets-controllers/CHANGELOG.md +++ b/packages/assets-controllers/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add `codefiTokenPricesServiceV2` ([#3600](https://github.com/MetaMask/core/pull/3600)) - This object can be used for the new `tokenPricesService` argument for TokenRatesController. It uses an internal API to fetch prices for tokens instead of CoinGecko. - `TokenListController` now exports a `TokenListControllerMessenger` type ([#3609](https://github.com/MetaMask/core/pull/3609)). +- `TokenDetectionController` exports types `TokenDetectionControllerMessenger`, `TokenDetectionControllerActions`, `TokenDetectionControllerGetStateAction`, `TokenDetectionControllerEvents`, `TokenDetectionControllerStateChangeEvent` ([#3609](https://github.com/MetaMask/core/pull/3609)). ### Changed - **BREAKING:** TokenRatesController now takes a required argument `tokenPricesService` ([#3600](https://github.com/MetaMask/core/pull/3600)) @@ -19,9 +20,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **BREAKING:** Change signature of `TokenRatesController.fetchAndMapExchangeRates` ([#3600](https://github.com/MetaMask/core/pull/3600)) - This method now takes an object with shape `{ tokenContractAddresses: Hex[]; chainId: Hex; nativeCurrency: string; }` rather than positional arguments - Update TokenListController to fetch prefiltered set of tokens from the API, reducing response data and removing the need for filtering logic ([#2054](https://github.com/MetaMask/core/pull/2054)) -- **BREAKING:** `TokenDetectionController` is upgraded to extend `PollingController` ([#3609](https://github.com/MetaMask/core/pull/3609)). - - The constructor now expects an options object as its only argument, with required properties `messenger`, `chainId`, `onPreferencesStateChange`, `getBalancesInSingleCall`, `addDetectedTokens`, `getTokensState`, `getPreferencesState`, and optional properties `interval`, `selectedAddress`. - - The state property is an empty object. +- **BREAKING:** `TokenDetectionController` is upgraded to extend `BaseControllerV2` and `StaticIntervalPollingController` ([#3609](https://github.com/MetaMask/core/pull/3609)). + - The constructor now expects an options object as its only argument, with required properties `messenger`, `chainId`, required callbacks `onPreferencesStateChange`, `getBalancesInSingleCall`, `addDetectedTokens`, `getTokensState`, `getPreferencesState`, and optional properties `interval`, `selectedAddress`. + - The controller state is an empty object and the controller config object is deprecated. + - Polling can only be initiated by a `.start()` call made from a controller client. There is no longer an option to automatically start polling when the controller is instantiated. - The `detectTokens` method of `TokenDetectionController` can now handle any number of tokens. Previously the limit was 2000 ([#3609](https://github.com/MetaMask/core/pull/3609)). ### Removed From 6c22a3ad88c606b3eddb09e72a39f08bca321760 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Tue, 12 Dec 2023 13:58:14 -0800 Subject: [PATCH 34/82] Update jest coverage thresholds --- packages/assets-controllers/jest.config.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/assets-controllers/jest.config.js b/packages/assets-controllers/jest.config.js index c21630b8ca..c9ff65d902 100644 --- a/packages/assets-controllers/jest.config.js +++ b/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: 89.16, - functions: 96.34, - lines: 96.77, - statements: 96.83, + branches: 89.07, + functions: 96.38, + lines: 96.81, + statements: 96.87, }, }, From a26d568a516cc367c15c8f82ccf56d074aa56e4d Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Tue, 12 Dec 2023 16:24:53 -0800 Subject: [PATCH 35/82] Mock action handler return value instead of registering multiple handlers --- packages/assets-controllers/jest.config.js | 2 +- .../src/TokenDetectionController.test.ts | 13 ++++++------- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/packages/assets-controllers/jest.config.js b/packages/assets-controllers/jest.config.js index c9ff65d902..c823cd2856 100644 --- a/packages/assets-controllers/jest.config.js +++ b/packages/assets-controllers/jest.config.js @@ -17,7 +17,7 @@ module.exports = merge(baseConfig, { // An object that configures minimum threshold enforcement for coverage results coverageThreshold: { global: { - branches: 89.07, + branches: 88.96, functions: 96.38, lines: 96.81, statements: 96.87, diff --git a/packages/assets-controllers/src/TokenDetectionController.test.ts b/packages/assets-controllers/src/TokenDetectionController.test.ts index dcc1312331..53ec84c202 100644 --- a/packages/assets-controllers/src/TokenDetectionController.test.ts +++ b/packages/assets-controllers/src/TokenDetectionController.test.ts @@ -168,6 +168,7 @@ describe('TokenDetectionController', () => { >; const onNetworkDidChangeListeners: ((state: NetworkState) => void)[] = []; + const getNetworkConfigurationByNetworkClientIdHandler = jest.fn(); const changeNetwork = (providerConfig: ProviderConfig) => { onNetworkDidChangeListeners.forEach((listener) => { listener({ @@ -176,12 +177,8 @@ describe('TokenDetectionController', () => { }); }); - controllerMessenger.unregisterActionHandler( - `NetworkController:getNetworkConfigurationByNetworkClientId`, - ); - controllerMessenger.registerActionHandler( - `NetworkController:getNetworkConfigurationByNetworkClientId`, - jest.fn().mockReturnValueOnce(providerConfig), + getNetworkConfigurationByNetworkClientIdHandler.mockReturnValue( + providerConfig, ); }; const mainnet = { @@ -248,7 +245,9 @@ describe('TokenDetectionController', () => { controllerMessenger.registerActionHandler( `NetworkController:getNetworkConfigurationByNetworkClientId`, - jest.fn().mockReturnValueOnce(mainnet), + getNetworkConfigurationByNetworkClientIdHandler.mockReturnValueOnce( + mainnet, + ), ); sinon From 85e3f10a652d2266f67481ad063e0b0b1cbc7410 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Wed, 13 Dec 2023 09:37:16 -0800 Subject: [PATCH 36/82] Remove unregister action handler call in constructor and define `beforeEach` for `getBalancesInSingleCall` tests --- packages/assets-controllers/jest.config.js | 2 +- .../src/TokenDetectionController.test.ts | 180 ++++++++++-------- .../src/TokenDetectionController.ts | 2 - 3 files changed, 99 insertions(+), 85 deletions(-) diff --git a/packages/assets-controllers/jest.config.js b/packages/assets-controllers/jest.config.js index c823cd2856..a5a40099bb 100644 --- a/packages/assets-controllers/jest.config.js +++ b/packages/assets-controllers/jest.config.js @@ -20,7 +20,7 @@ module.exports = merge(baseConfig, { branches: 88.96, functions: 96.38, lines: 96.81, - statements: 96.87, + statements: 96.86, }, }, diff --git a/packages/assets-controllers/src/TokenDetectionController.test.ts b/packages/assets-controllers/src/TokenDetectionController.test.ts index 53ec84c202..c7af0968b2 100644 --- a/packages/assets-controllers/src/TokenDetectionController.test.ts +++ b/packages/assets-controllers/src/TokenDetectionController.test.ts @@ -430,102 +430,118 @@ describe('TokenDetectionController', () => { expect(tokensController.state.detectedTokens).toStrictEqual([sampleTokenA]); }); - it('should not call getBalancesInSingleCall after stopping polling, and then switching between networks that support token detection', async () => { - const polygonDecimalChainId = '137'; - nock(TOKEN_END_POINT_API) - .get(getTokensPath(toHex(polygonDecimalChainId))) - .reply(200, sampleTokenList); - - const stub = sinon.stub(); - const getBalancesInSingleCallMock = sinon.stub(); - - tokenDetection = new TokenDetectionController({ - chainId: ChainId.mainnet, - selectedAddress: '0x1', - onPreferencesStateChange: stub, - getBalancesInSingleCall: getBalancesInSingleCallMock, - addDetectedTokens: stub, - getTokensState: () => tokensController.state, - getPreferencesState: () => preferences.state, - messenger: buildTokenDetectionControllerMessenger(controllerMessenger), + describe('getBalancesInSingleCall', () => { + let stub: sinon.SinonStub; + let getBalancesInSingleCallMock: sinon.SinonStub< + Parameters, + ReturnType + >; + beforeEach(async () => { + stub = sinon.stub(); + getBalancesInSingleCallMock = sinon.stub(); + + controllerMessenger = getControllerMessenger(); + controllerMessenger.registerActionHandler( + `NetworkController:getNetworkConfigurationByNetworkClientId`, + getNetworkConfigurationByNetworkClientIdHandler.mockReturnValueOnce( + mainnet, + ), + ); + + const tokenListSetup = setupTokenListController(controllerMessenger); + tokenList = tokenListSetup.tokenList; + await tokenList.start(); }); - await tokenDetection.start(); + it('should not call getBalancesInSingleCall after stopping polling, and then switching between networks that support token detection', async () => { + const polygonDecimalChainId = '137'; + nock(TOKEN_END_POINT_API) + .get(getTokensPath(toHex(polygonDecimalChainId))) + .reply(200, sampleTokenList); + + tokenDetection = new TokenDetectionController({ + chainId: ChainId.mainnet, + selectedAddress: '0x1', + onPreferencesStateChange: stub, + getBalancesInSingleCall: getBalancesInSingleCallMock, + addDetectedTokens: stub, + getTokensState: () => tokensController.state, + getPreferencesState: () => preferences.state, + messenger: buildTokenDetectionControllerMessenger(controllerMessenger), + }); + await tokenDetection.start(); - expect(getBalancesInSingleCallMock.called).toBe(true); - getBalancesInSingleCallMock.reset(); + expect(getBalancesInSingleCallMock.called).toBe(true); + getBalancesInSingleCallMock.reset(); - tokenDetection.stop(); - changeNetwork({ - chainId: toHex(polygonDecimalChainId), - type: NetworkType.goerli, - ticker: NetworksTicker.goerli, + tokenDetection.stop(); + changeNetwork({ + chainId: toHex(polygonDecimalChainId), + type: NetworkType.goerli, + ticker: NetworksTicker.goerli, + }); + expect(getBalancesInSingleCallMock.called).toBe(false); }); - expect(getBalancesInSingleCallMock.called).toBe(false); - }); - it('should not call getBalancesInSingleCall if TokenListController is updated to have an empty token list', async () => { - const stub = sinon.stub(); - const getBalancesInSingleCallMock = sinon.stub(); - tokenDetection = new TokenDetectionController({ - chainId: ChainId.mainnet, - onPreferencesStateChange: stub, - getBalancesInSingleCall: getBalancesInSingleCallMock, - addDetectedTokens: stub, - getTokensState: () => tokensController.state, - getPreferencesState: () => preferences.state, - messenger: buildTokenDetectionControllerMessenger(controllerMessenger), + it('should not call getBalancesInSingleCall if TokenListController is updated to have an empty token list', async () => { + tokenDetection = new TokenDetectionController({ + chainId: ChainId.mainnet, + onPreferencesStateChange: stub, + getBalancesInSingleCall: getBalancesInSingleCallMock, + addDetectedTokens: stub, + getTokensState: () => tokensController.state, + getPreferencesState: () => preferences.state, + messenger: buildTokenDetectionControllerMessenger(controllerMessenger), + }); + + tokenList.clearingTokenListData(); + expect(getBalancesInSingleCallMock.called).toBe(false); }); - tokenList.clearingTokenListData(); - expect(getBalancesInSingleCallMock.called).toBe(false); - }); + it('should call getBalancesInSingleCall if onPreferencesStateChange is called with useTokenDetection being true and is changed', async () => { + let preferencesStateChangeListener: (state: any) => void; + const onPreferencesStateChange = sinon.stub().callsFake((listener) => { + preferencesStateChangeListener = listener; + }); - it('should call getBalancesInSingleCall if onPreferencesStateChange is called with useTokenDetection being true and is changed', async () => { - const stub = sinon.stub(); - const getBalancesInSingleCallMock = sinon.stub(); - let preferencesStateChangeListener: (state: any) => void; - const onPreferencesStateChange = sinon.stub().callsFake((listener) => { - preferencesStateChangeListener = listener; - }); - tokenDetection = new TokenDetectionController({ - chainId: ChainId.mainnet, - selectedAddress: '0x1', - onPreferencesStateChange, - getBalancesInSingleCall: getBalancesInSingleCallMock, - addDetectedTokens: stub, - getTokensState: () => tokensController.state, - getPreferencesState: () => preferences.state, - messenger: buildTokenDetectionControllerMessenger(controllerMessenger), - }); - await tokenDetection.start(); + tokenDetection = new TokenDetectionController({ + chainId: ChainId.mainnet, + selectedAddress: '0x1', + onPreferencesStateChange, + getBalancesInSingleCall: getBalancesInSingleCallMock, + addDetectedTokens: stub, + getTokensState: () => tokensController.state, + getPreferencesState: () => preferences.state, + messenger: buildTokenDetectionControllerMessenger(controllerMessenger), + }); + await tokenDetection.start(); - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - preferencesStateChangeListener!({ - selectedAddress: '0x1', - useTokenDetection: true, + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + preferencesStateChangeListener!({ + selectedAddress: '0x1', + useTokenDetection: true, + }); + expect(getBalancesInSingleCallMock.called).toBe(true); }); - expect(getBalancesInSingleCallMock.called).toBe(true); - }); - it('should call getBalancesInSingleCall if network is changed to a chainId that supports token detection', async () => { - const stub = sinon.stub(); - const getBalancesInSingleCallMock = sinon.stub(); - tokenDetection = new TokenDetectionController({ - chainId: SupportedTokenDetectionNetworks.polygon, - selectedAddress: '0x1', - onPreferencesStateChange: stub, - getBalancesInSingleCall: getBalancesInSingleCallMock, - addDetectedTokens: stub, - getTokensState: () => tokensController.state, - getPreferencesState: () => preferences.state, - messenger: buildTokenDetectionControllerMessenger(controllerMessenger), - }); - await tokenDetection.start(); + it('should call getBalancesInSingleCall if network is changed to a chainId that supports token detection', async () => { + tokenDetection = new TokenDetectionController({ + chainId: SupportedTokenDetectionNetworks.polygon, + selectedAddress: '0x1', + onPreferencesStateChange: stub, + getBalancesInSingleCall: getBalancesInSingleCallMock, + addDetectedTokens: stub, + getTokensState: () => tokensController.state, + getPreferencesState: () => preferences.state, + messenger: buildTokenDetectionControllerMessenger(controllerMessenger), + }); + await tokenDetection.start(); - changeNetwork(mainnet); - expect(getBalancesInSingleCallMock.called).toBe(true); + changeNetwork(mainnet); + expect(getBalancesInSingleCallMock.called).toBe(true); + }); }); + describe('startPollingByNetworkClientId', () => { let clock: sinon.SinonFakeTimers; beforeEach(() => { diff --git a/packages/assets-controllers/src/TokenDetectionController.ts b/packages/assets-controllers/src/TokenDetectionController.ts index 15bf87a4d9..5b1c9ae6ce 100644 --- a/packages/assets-controllers/src/TokenDetectionController.ts +++ b/packages/assets-controllers/src/TokenDetectionController.ts @@ -136,8 +136,6 @@ export class TokenDetectionController extends StaticIntervalPollingController< const { useTokenDetection: defaultUseTokenDetection } = getPreferencesState(); - messenger.unregisterActionHandler('TokenDetectionController:getState'); - super({ name: controllerName, messenger, From b9cbcba9f6877f4158623487f91a82d1b90ab24a Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Wed, 13 Dec 2023 09:42:34 -0800 Subject: [PATCH 37/82] Revert "Fix `sliceOfTokensToDetect` logic to handle arbitrary number of `tokensToDetect`" This reverts commit b8dcf32241ec85d314f7324a49bcac7931f1b530. --- .../assets-controllers/src/TokenDetectionController.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/assets-controllers/src/TokenDetectionController.ts b/packages/assets-controllers/src/TokenDetectionController.ts index 5b1c9ae6ce..0852a5a985 100644 --- a/packages/assets-controllers/src/TokenDetectionController.ts +++ b/packages/assets-controllers/src/TokenDetectionController.ts @@ -288,11 +288,11 @@ export class TokenDetectionController extends StaticIntervalPollingController< } } const sliceOfTokensToDetect = []; - for (let i = 0; i < tokensToDetect.length; i += 1000) { - sliceOfTokensToDetect.push( - tokensToDetect.slice(i, Math.min(i + 1000, tokensToDetect.length)), - ); - } + sliceOfTokensToDetect[0] = tokensToDetect.slice(0, 1000); + sliceOfTokensToDetect[1] = tokensToDetect.slice( + 1000, + tokensToDetect.length - 1, + ); /* istanbul ignore else */ if (!selectedAddress) { From fb7cbe21f7bf9a6c5aba7effae5c321a896b0aea Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Wed, 13 Dec 2023 18:58:15 -0800 Subject: [PATCH 38/82] set polygon network as rpc in test --- .../assets-controllers/src/TokenDetectionController.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/assets-controllers/src/TokenDetectionController.test.ts b/packages/assets-controllers/src/TokenDetectionController.test.ts index 5039dcb0ee..76d7774561 100644 --- a/packages/assets-controllers/src/TokenDetectionController.test.ts +++ b/packages/assets-controllers/src/TokenDetectionController.test.ts @@ -481,8 +481,8 @@ describe('TokenDetectionController', () => { tokenDetection.stop(); changeNetwork({ chainId: toHex(polygonDecimalChainId), - type: NetworkType.goerli, - ticker: NetworksTicker.goerli, + type: NetworkType.rpc, + ticker: NetworksTicker.rpc, }); expect(getBalancesInSingleCallMock.called).toBe(false); }); From 5d602a3b337a181bcac59b3160457464ee1e7820 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Wed, 13 Dec 2023 18:59:30 -0800 Subject: [PATCH 39/82] Fix test so that `preferencesStateChangeListener` triggers `detectTokens` without `.start()` call --- .../assets-controllers/src/TokenDetectionController.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/assets-controllers/src/TokenDetectionController.test.ts b/packages/assets-controllers/src/TokenDetectionController.test.ts index 76d7774561..a2e129307d 100644 --- a/packages/assets-controllers/src/TokenDetectionController.test.ts +++ b/packages/assets-controllers/src/TokenDetectionController.test.ts @@ -524,7 +524,7 @@ describe('TokenDetectionController', () => { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion preferencesStateChangeListener!({ - selectedAddress: '0x1', + selectedAddress: '0x2', useTokenDetection: true, }); expect(getBalancesInSingleCallMock.called).toBe(true); From a41a218e9c84b1bf337cebf7283889c56d3774cb Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Wed, 13 Dec 2023 19:00:39 -0800 Subject: [PATCH 40/82] Add `#networkClientId` class field and update using `seletedNetworkClientId` in `networkDidChange` listener --- .../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 0852a5a985..a75dca0d60 100644 --- a/packages/assets-controllers/src/TokenDetectionController.ts +++ b/packages/assets-controllers/src/TokenDetectionController.ts @@ -82,6 +82,8 @@ export class TokenDetectionController extends StaticIntervalPollingController< #selectedAddress: string; + #networkClientId: NetworkClientId; + #isDetectionEnabledFromPreferences: boolean; #isDetectionEnabledForNetwork: boolean; @@ -145,6 +147,7 @@ export class TokenDetectionController extends StaticIntervalPollingController< this.setIntervalLength(interval); this.#chainId = chainId; + this.#networkClientId = chainId; this.#selectedAddress = selectedAddress; this.#isDetectionEnabledFromPreferences = defaultUseTokenDetection; this.#isDetectionEnabledForNetwork = isTokenDetectionSupportedForNetwork( @@ -187,13 +190,18 @@ export class TokenDetectionController extends StaticIntervalPollingController< this.messagingSystem.subscribe( 'NetworkController:networkDidChange', - async ({ providerConfig: { chainId: newChainId } }) => { - this.#isDetectionEnabledForNetwork = - isTokenDetectionSupportedForNetwork(newChainId); + async ({ + providerConfig: { chainId: newChainId }, + selectedNetworkClientId, + }) => { const isChainIdChanged = this.#chainId !== newChainId; - this.#chainId = newChainId; + this.#networkClientId = selectedNetworkClientId; + + this.#isDetectionEnabledForNetwork = + isTokenDetectionSupportedForNetwork(newChainId); + if (this.#isDetectionEnabledForNetwork && isChainIdChanged) { await this.detectTokens(); } From 7e52c7c5d4e851639166e9073c007314e6dfd2d7 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Wed, 13 Dec 2023 19:02:07 -0800 Subject: [PATCH 41/82] Update CHANGELOG to reflect 'Revert "Fix `sliceOfTokensToDetect` logic to handle arbitrary number of `tokensToDetect`' --- packages/assets-controllers/CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/assets-controllers/CHANGELOG.md b/packages/assets-controllers/CHANGELOG.md index 0721e61c05..b933ddd7a8 100644 --- a/packages/assets-controllers/CHANGELOG.md +++ b/packages/assets-controllers/CHANGELOG.md @@ -24,7 +24,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - The constructor now expects an options object as its only argument, with required properties `messenger`, `chainId`, required callbacks `onPreferencesStateChange`, `getBalancesInSingleCall`, `addDetectedTokens`, `getTokensState`, `getPreferencesState`, and optional properties `interval`, `selectedAddress`. - The controller state is an empty object and the controller config object is deprecated. - Polling can only be initiated by a `.start()` call made from a controller client. There is no longer an option to automatically start polling when the controller is instantiated. -- The `detectTokens` method of `TokenDetectionController` can now handle any number of tokens. Previously the limit was 2000 ([#3609](https://github.com/MetaMask/core/pull/3609)). ### Removed - **BREAKING:** Remove `fetchExchangeRate` method from TokenRatesController ([#3600](https://github.com/MetaMask/core/pull/3600)) From e7dac31368583d6a8a1af032a80b98a510062016 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Wed, 13 Dec 2023 19:09:51 -0800 Subject: [PATCH 42/82] Update jsdoc for class fields --- packages/assets-controllers/src/TokenDetectionController.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/assets-controllers/src/TokenDetectionController.ts b/packages/assets-controllers/src/TokenDetectionController.ts index a75dca0d60..39bc11c4ba 100644 --- a/packages/assets-controllers/src/TokenDetectionController.ts +++ b/packages/assets-controllers/src/TokenDetectionController.ts @@ -65,9 +65,10 @@ export type TokenDetectionControllerMessenger = RestrictedControllerMessenger< /** * Controller that passively polls on a set interval for Tokens auto detection - * @property interval - Polling interval used to fetch new token rates - * @property selectedAddress - Vault selected address + * @property intervalId - Polling interval used to fetch new token rates * @property chainId - The chain ID of the current network + * @property selectedAddress - Vault selected address + * @property networkClientId - The network client ID of the current selected network * @property isDetectionEnabledFromPreferences - Boolean to track if detection is enabled from PreferencesController * @property isDetectionEnabledForNetwork - Boolean to track if detected is enabled for current network */ From 15ea19cfd5426cdec023987098de2d5cc4fa5a8c Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Wed, 13 Dec 2023 19:18:09 -0800 Subject: [PATCH 43/82] Fix mistaken assignment of chainId to networkClientId property --- .../assets-controllers/src/TokenDetectionController.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/assets-controllers/src/TokenDetectionController.test.ts b/packages/assets-controllers/src/TokenDetectionController.test.ts index a2e129307d..691f0cf4e4 100644 --- a/packages/assets-controllers/src/TokenDetectionController.test.ts +++ b/packages/assets-controllers/src/TokenDetectionController.test.ts @@ -601,7 +601,7 @@ describe('TokenDetectionController', () => { [sampleTokenA.address]: new BN(1), }); await tokenDetection.detectTokens({ - networkClientId: ChainId.mainnet, + networkClientId: NetworkType.mainnet, accountAddress: selectedAddress, }); const tokens = From 3e1416de48c225f000357fb2c84ca2e1ef03c8d4 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Wed, 13 Dec 2023 19:26:41 -0800 Subject: [PATCH 44/82] Revert "Add `#networkClientId` class field and update using `seletedNetworkClientId` in `networkDidChange` listener" This reverts commit a41a218e9c84b1bf337cebf7283889c56d3774cb. --- .../src/TokenDetectionController.ts | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/packages/assets-controllers/src/TokenDetectionController.ts b/packages/assets-controllers/src/TokenDetectionController.ts index 39bc11c4ba..08d77220e2 100644 --- a/packages/assets-controllers/src/TokenDetectionController.ts +++ b/packages/assets-controllers/src/TokenDetectionController.ts @@ -83,8 +83,6 @@ export class TokenDetectionController extends StaticIntervalPollingController< #selectedAddress: string; - #networkClientId: NetworkClientId; - #isDetectionEnabledFromPreferences: boolean; #isDetectionEnabledForNetwork: boolean; @@ -148,7 +146,6 @@ export class TokenDetectionController extends StaticIntervalPollingController< this.setIntervalLength(interval); this.#chainId = chainId; - this.#networkClientId = chainId; this.#selectedAddress = selectedAddress; this.#isDetectionEnabledFromPreferences = defaultUseTokenDetection; this.#isDetectionEnabledForNetwork = isTokenDetectionSupportedForNetwork( @@ -191,17 +188,12 @@ export class TokenDetectionController extends StaticIntervalPollingController< this.messagingSystem.subscribe( 'NetworkController:networkDidChange', - async ({ - providerConfig: { chainId: newChainId }, - selectedNetworkClientId, - }) => { - const isChainIdChanged = this.#chainId !== newChainId; - this.#chainId = newChainId; - - this.#networkClientId = selectedNetworkClientId; - + async ({ providerConfig: { chainId: newChainId } }) => { this.#isDetectionEnabledForNetwork = isTokenDetectionSupportedForNetwork(newChainId); + const isChainIdChanged = this.#chainId !== newChainId; + + this.#chainId = newChainId; if (this.#isDetectionEnabledForNetwork && isChainIdChanged) { await this.detectTokens(); From 123aff5f00c7fa56ceffa41576b1de5d650c5ebb Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Wed, 13 Dec 2023 19:45:08 -0800 Subject: [PATCH 45/82] Revert "Fix test so that `preferencesStateChangeListener` triggers `detectTokens` without `.start()` call" This reverts commit 5d602a3b337a181bcac59b3160457464ee1e7820. --- .../assets-controllers/src/TokenDetectionController.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/assets-controllers/src/TokenDetectionController.test.ts b/packages/assets-controllers/src/TokenDetectionController.test.ts index 691f0cf4e4..42ba53dcb4 100644 --- a/packages/assets-controllers/src/TokenDetectionController.test.ts +++ b/packages/assets-controllers/src/TokenDetectionController.test.ts @@ -524,7 +524,7 @@ describe('TokenDetectionController', () => { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion preferencesStateChangeListener!({ - selectedAddress: '0x2', + selectedAddress: '0x1', useTokenDetection: true, }); expect(getBalancesInSingleCallMock.called).toBe(true); From 7f2e58b39b457aa2cc9c60b28ddf821c74d6cc75 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Wed, 13 Dec 2023 19:45:52 -0800 Subject: [PATCH 46/82] Fix test so that `preferencesStateChangeListener` triggers `detectTokens` without `.start()` call --- .../assets-controllers/src/TokenDetectionController.test.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/assets-controllers/src/TokenDetectionController.test.ts b/packages/assets-controllers/src/TokenDetectionController.test.ts index 42ba53dcb4..6fd8e4476a 100644 --- a/packages/assets-controllers/src/TokenDetectionController.test.ts +++ b/packages/assets-controllers/src/TokenDetectionController.test.ts @@ -520,11 +520,10 @@ describe('TokenDetectionController', () => { getPreferencesState: () => preferences.state, messenger: buildTokenDetectionControllerMessenger(controllerMessenger), }); - await tokenDetection.start(); // eslint-disable-next-line @typescript-eslint/no-non-null-assertion preferencesStateChangeListener!({ - selectedAddress: '0x1', + selectedAddress: '0x2', useTokenDetection: true, }); expect(getBalancesInSingleCallMock.called).toBe(true); From d32cdf43d616fab43a50aa140282a7a66dffc9bd Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Wed, 13 Dec 2023 19:50:32 -0800 Subject: [PATCH 47/82] Add `#networkClientId` class field and update using `seletedNetworkClientId` in `networkDidChange` listener - add `NetworkControllerFindNetworkClientIdByChainId` as allowed action --- .../src/TokenDetectionController.test.ts | 26 ++++++++++++++----- .../src/TokenDetectionController.ts | 23 ++++++++++++---- 2 files changed, 37 insertions(+), 12 deletions(-) diff --git a/packages/assets-controllers/src/TokenDetectionController.test.ts b/packages/assets-controllers/src/TokenDetectionController.test.ts index 6fd8e4476a..4a6f209c75 100644 --- a/packages/assets-controllers/src/TokenDetectionController.test.ts +++ b/packages/assets-controllers/src/TokenDetectionController.test.ts @@ -146,6 +146,7 @@ function buildTokenDetectionControllerMessenger( name: controllerName, allowedActions: [ 'NetworkController:getNetworkConfigurationByNetworkClientId', + 'NetworkController:findNetworkClientIdByChainId', 'TokenListController:getState', ], allowedEvents: [ @@ -169,6 +170,7 @@ describe('TokenDetectionController', () => { const onNetworkDidChangeListeners: ((state: NetworkState) => void)[] = []; const getNetworkConfigurationByNetworkClientIdHandler = jest.fn(); + const findNetworkClientIdByChainIdHandler = jest.fn(); const changeNetwork = (providerConfig: ProviderConfig) => { onNetworkDidChangeListeners.forEach((listener) => { listener({ @@ -218,6 +220,17 @@ describe('TokenDetectionController', () => { // eslint-disable-next-line @typescript-eslint/no-explicit-any .callsFake(() => null as any); + controllerMessenger.registerActionHandler( + `NetworkController:getNetworkConfigurationByNetworkClientId`, + getNetworkConfigurationByNetworkClientIdHandler.mockReturnValueOnce( + mainnet, + ), + ); + controllerMessenger.registerActionHandler( + `NetworkController:findNetworkClientIdByChainId`, + findNetworkClientIdByChainIdHandler.mockReturnValue(NetworkType.mainnet), + ); + tokensController = new TokensController({ chainId: ChainId.mainnet, onPreferencesStateChange: (listener) => preferences.subscribe(listener), @@ -247,13 +260,6 @@ describe('TokenDetectionController', () => { messenger: buildTokenDetectionControllerMessenger(controllerMessenger), }); - controllerMessenger.registerActionHandler( - `NetworkController:getNetworkConfigurationByNetworkClientId`, - getNetworkConfigurationByNetworkClientIdHandler.mockReturnValueOnce( - mainnet, - ), - ); - sinon .stub(tokensController, '_detectIsERC721') .callsFake(() => Promise.resolve(false)); @@ -451,6 +457,12 @@ describe('TokenDetectionController', () => { mainnet, ), ); + controllerMessenger.registerActionHandler( + `NetworkController:findNetworkClientIdByChainId`, + findNetworkClientIdByChainIdHandler.mockReturnValue( + NetworkType.mainnet, + ), + ); const tokenListSetup = setupTokenListController(controllerMessenger); tokenList = tokenListSetup.tokenList; diff --git a/packages/assets-controllers/src/TokenDetectionController.ts b/packages/assets-controllers/src/TokenDetectionController.ts index 08d77220e2..6cfb13e6a2 100644 --- a/packages/assets-controllers/src/TokenDetectionController.ts +++ b/packages/assets-controllers/src/TokenDetectionController.ts @@ -10,6 +10,7 @@ import { import type { NetworkClientId, NetworkControllerGetNetworkConfigurationByNetworkClientId, + NetworkControllerFindNetworkClientIdByChainIdAction, NetworkControllerNetworkDidChangeEvent, NetworkControllerStateChangeEvent, } from '@metamask/network-controller'; @@ -42,6 +43,7 @@ export type TokenDetectionControllerActions = export type AllowedActions = | NetworkControllerGetNetworkConfigurationByNetworkClientId + | NetworkControllerFindNetworkClientIdByChainIdAction | GetTokenListState; export type TokenDetectionControllerStateChangeEvent = @@ -83,6 +85,8 @@ export class TokenDetectionController extends StaticIntervalPollingController< #selectedAddress: string; + #networkClientId: NetworkClientId; + #isDetectionEnabledFromPreferences: boolean; #isDetectionEnabledForNetwork: boolean; @@ -147,6 +151,10 @@ export class TokenDetectionController extends StaticIntervalPollingController< this.setIntervalLength(interval); this.#chainId = chainId; this.#selectedAddress = selectedAddress; + this.#networkClientId = this.messagingSystem.call( + 'NetworkController:findNetworkClientIdByChainId', + chainId, + ); this.#isDetectionEnabledFromPreferences = defaultUseTokenDetection; this.#isDetectionEnabledForNetwork = isTokenDetectionSupportedForNetwork( this.#chainId, @@ -188,13 +196,18 @@ export class TokenDetectionController extends StaticIntervalPollingController< this.messagingSystem.subscribe( 'NetworkController:networkDidChange', - async ({ providerConfig: { chainId: newChainId } }) => { - this.#isDetectionEnabledForNetwork = - isTokenDetectionSupportedForNetwork(newChainId); + async ({ + providerConfig: { chainId: newChainId }, + selectedNetworkClientId, + }) => { const isChainIdChanged = this.#chainId !== newChainId; - this.#chainId = newChainId; + this.#networkClientId = selectedNetworkClientId; + + this.#isDetectionEnabledForNetwork = + isTokenDetectionSupportedForNetwork(newChainId); + if (this.#isDetectionEnabledForNetwork && isChainIdChanged) { await this.detectTokens(); } @@ -237,7 +250,7 @@ export class TokenDetectionController extends StaticIntervalPollingController< const { chainId } = this.messagingSystem.call( 'NetworkController:getNetworkConfigurationByNetworkClientId', - networkClientId ?? this.#chainId, + networkClientId ?? this.#networkClientId, ) ?? {}; if (chainId) { this.#chainId = chainId; From 71d9967d3559111d528bf279d593167ed8e82409 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 14 Dec 2023 12:41:49 -0800 Subject: [PATCH 48/82] Fix test by publishing `networkDidChange` event in `changeNetwork` helper function --- .../assets-controllers/src/TokenDetectionController.test.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/assets-controllers/src/TokenDetectionController.test.ts b/packages/assets-controllers/src/TokenDetectionController.test.ts index 4a6f209c75..523693fc33 100644 --- a/packages/assets-controllers/src/TokenDetectionController.test.ts +++ b/packages/assets-controllers/src/TokenDetectionController.test.ts @@ -179,6 +179,11 @@ describe('TokenDetectionController', () => { }); }); + controllerMessenger.publish('NetworkController:networkDidChange', { + ...defaultNetworkState, + providerConfig, + }); + getNetworkConfigurationByNetworkClientIdHandler.mockReturnValue( providerConfig, ); @@ -552,7 +557,6 @@ describe('TokenDetectionController', () => { getPreferencesState: () => preferences.state, messenger: buildTokenDetectionControllerMessenger(controllerMessenger), }); - await tokenDetection.start(); changeNetwork(mainnet); expect(getBalancesInSingleCallMock.called).toBe(true); From 0dda603252cebeee328a7b409eaf35c863e4f418 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 14 Dec 2023 12:42:41 -0800 Subject: [PATCH 49/82] Add `#disabled` class field to ensure that passive token detection is blocked by `stop()` call, and only restored by `start(0` call --- .../assets-controllers/src/TokenDetectionController.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/assets-controllers/src/TokenDetectionController.ts b/packages/assets-controllers/src/TokenDetectionController.ts index 6cfb13e6a2..700131f805 100644 --- a/packages/assets-controllers/src/TokenDetectionController.ts +++ b/packages/assets-controllers/src/TokenDetectionController.ts @@ -71,6 +71,7 @@ export type TokenDetectionControllerMessenger = RestrictedControllerMessenger< * @property chainId - The chain ID of the current network * @property selectedAddress - Vault selected address * @property networkClientId - The network client ID of the current selected network + * @property disabled - Boolean to track if passive auto-detection is currently enabled in this controller * @property isDetectionEnabledFromPreferences - Boolean to track if detection is enabled from PreferencesController * @property isDetectionEnabledForNetwork - Boolean to track if detected is enabled for current network */ @@ -87,6 +88,8 @@ export class TokenDetectionController extends StaticIntervalPollingController< #networkClientId: NetworkClientId; + #disabled: boolean; + #isDetectionEnabledFromPreferences: boolean; #isDetectionEnabledForNetwork: boolean; @@ -148,6 +151,7 @@ export class TokenDetectionController extends StaticIntervalPollingController< metadata: {}, }); + this.#disabled = false; this.setIntervalLength(interval); this.#chainId = chainId; this.#selectedAddress = selectedAddress; @@ -219,6 +223,7 @@ export class TokenDetectionController extends StaticIntervalPollingController< * Start polling for detected tokens. */ async start() { + this.#disabled = false; await this.#startPolling(); } @@ -226,6 +231,7 @@ export class TokenDetectionController extends StaticIntervalPollingController< * Stop polling for detected tokens. */ stop() { + this.#disabled = true; this.#stopPolling(); } @@ -281,6 +287,7 @@ export class TokenDetectionController extends StaticIntervalPollingController< }): Promise { const { networkClientId, accountAddress } = options ?? {}; if ( + this.#disabled || !this.#isDetectionEnabledForNetwork || !this.#isDetectionEnabledFromPreferences ) { From c5eec2517fc0f981fd64cca6a3cf5d4dc7b0ba76 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 14 Dec 2023 12:45:39 -0800 Subject: [PATCH 50/82] Adjust jest coverage thresholds --- packages/assets-controllers/jest.config.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/assets-controllers/jest.config.js b/packages/assets-controllers/jest.config.js index a5a40099bb..11de507241 100644 --- a/packages/assets-controllers/jest.config.js +++ b/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.96, - functions: 96.38, - lines: 96.81, - statements: 96.86, + branches: 89.47, + functions: 97, + lines: 97.4, + statements: 97.45, }, }, From 1337d199b42db63cc94fa0a62cd645c5535a30d8 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Fri, 15 Dec 2023 10:14:07 -0800 Subject: [PATCH 51/82] Update packages/assets-controllers/src/TokenDetectionController.test.ts Co-authored-by: Elliot Winkler --- .../assets-controllers/src/TokenDetectionController.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/assets-controllers/src/TokenDetectionController.test.ts b/packages/assets-controllers/src/TokenDetectionController.test.ts index 523693fc33..1a3e4cd331 100644 --- a/packages/assets-controllers/src/TokenDetectionController.test.ts +++ b/packages/assets-controllers/src/TokenDetectionController.test.ts @@ -140,9 +140,9 @@ function setupTokenListController( * @returns The restricted messenger. */ function buildTokenDetectionControllerMessenger( - controllerMessenger?: MainControllerMessenger, + controllerMessenger: MainControllerMessenger = getControllerMessenger(), ): TokenDetectionControllerMessenger { - return (controllerMessenger ?? getControllerMessenger()).getRestricted({ + return controllerMessenger.getRestricted({ name: controllerName, allowedActions: [ 'NetworkController:getNetworkConfigurationByNetworkClientId', From 41f85a18f80873a9f7cfc027a70cadce25e8c243 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Fri, 15 Dec 2023 10:15:13 -0800 Subject: [PATCH 52/82] Update packages/assets-controllers/CHANGELOG.md --- packages/assets-controllers/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/assets-controllers/CHANGELOG.md b/packages/assets-controllers/CHANGELOG.md index 5286eea35d..d932c92059 100644 --- a/packages/assets-controllers/CHANGELOG.md +++ b/packages/assets-controllers/CHANGELOG.md @@ -13,7 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **BREAKING:** `TokenDetectionController` is upgraded to extend `BaseControllerV2` and `StaticIntervalPollingController` ([#3609](https://github.com/MetaMask/core/pull/3609)). - The constructor now expects an options object as its only argument, with required properties `messenger`, `chainId`, required callbacks `onPreferencesStateChange`, `getBalancesInSingleCall`, `addDetectedTokens`, `getTokensState`, `getPreferencesState`, and optional properties `interval`, `selectedAddress`. - The controller state is an empty object and the controller config object is deprecated. - - Polling can only be initiated by a `.start()` call made from a controller client. There is no longer an option to automatically start polling when the controller is instantiated. + - Polling can only be initiated by calling `start` or `startPollingByNetworkClientId` from a controller client. There is no longer an option to automatically start polling when the controller is instantiated. ## [21.0.0] ### Added From 8c331fac7bf8b370c9f1a7b046298b7db1ba49e4 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Fri, 15 Dec 2023 10:20:53 -0800 Subject: [PATCH 53/82] Edit test descriptions so that `getBalancesInSingleMock` is subject --- .../src/TokenDetectionController.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/assets-controllers/src/TokenDetectionController.test.ts b/packages/assets-controllers/src/TokenDetectionController.test.ts index 1a3e4cd331..a591df6ed9 100644 --- a/packages/assets-controllers/src/TokenDetectionController.test.ts +++ b/packages/assets-controllers/src/TokenDetectionController.test.ts @@ -474,7 +474,7 @@ describe('TokenDetectionController', () => { await tokenList.start(); }); - it('should not call getBalancesInSingleCall after stopping polling, and then switching between networks that support token detection', async () => { + it('should not be called after stopping polling, and then switching between networks that support token detection', async () => { const polygonDecimalChainId = '137'; nock(TOKEN_END_POINT_API) .get(getTokensPath(toHex(polygonDecimalChainId))) @@ -504,7 +504,7 @@ describe('TokenDetectionController', () => { expect(getBalancesInSingleCallMock.called).toBe(false); }); - it('should not call getBalancesInSingleCall if TokenListController is updated to have an empty token list', async () => { + it('should not be called if TokenListController is updated to have an empty token list', async () => { tokenDetection = new TokenDetectionController({ chainId: ChainId.mainnet, onPreferencesStateChange: stub, @@ -519,7 +519,7 @@ describe('TokenDetectionController', () => { expect(getBalancesInSingleCallMock.called).toBe(false); }); - it('should call getBalancesInSingleCall if onPreferencesStateChange is called with useTokenDetection being true and is changed', async () => { + it('should be called if onPreferencesStateChange is called with useTokenDetection being true and is changed', async () => { // TODO: Replace `any` with type // eslint-disable-next-line @typescript-eslint/no-explicit-any let preferencesStateChangeListener: (state: any) => void; @@ -546,7 +546,7 @@ describe('TokenDetectionController', () => { expect(getBalancesInSingleCallMock.called).toBe(true); }); - it('should call getBalancesInSingleCall if network is changed to a chainId that supports token detection', async () => { + it('should be called if network is changed to a chainId that supports token detection', async () => { tokenDetection = new TokenDetectionController({ chainId: SupportedTokenDetectionNetworks.polygon, selectedAddress: '0x1', From d40247e2fc2994f842b2dad8f6a843acbbdba2d0 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Fri, 15 Dec 2023 14:27:44 -0800 Subject: [PATCH 54/82] Fix incorrect package-level export --- packages/assets-controllers/src/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/assets-controllers/src/index.ts b/packages/assets-controllers/src/index.ts index c37217e571..aa20b3c44a 100644 --- a/packages/assets-controllers/src/index.ts +++ b/packages/assets-controllers/src/index.ts @@ -20,4 +20,4 @@ export { formatIconUrlWithProxy, getFormattedIpfsUrl, } from './assetsUtil'; -export { CodefiTokenPricesServiceV2 } from './token-prices-service'; +export { codefiTokenPricesServiceV2 } from './token-prices-service'; From 757ab0464e3561ca89a25cee65590a5e9a272470 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Fri, 15 Dec 2023 14:29:04 -0800 Subject: [PATCH 55/82] [tokens-controller] Add actions `getState`, `addDetectedTokens` and event `stateChange` --- packages/assets-controllers/CHANGELOG.md | 3 +- .../src/TokensController.ts | 62 ++++++++++++------- packages/assets-controllers/src/index.ts | 11 +++- 3 files changed, 52 insertions(+), 24 deletions(-) diff --git a/packages/assets-controllers/CHANGELOG.md b/packages/assets-controllers/CHANGELOG.md index d932c92059..7fe85115e1 100644 --- a/packages/assets-controllers/CHANGELOG.md +++ b/packages/assets-controllers/CHANGELOG.md @@ -6,7 +6,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Added -- `TokenListController` now exports a `TokenListControllerMessenger` type ([#3609](https://github.com/MetaMask/core/pull/3609)). +- `TokensController` exports types `TokensControllerGetStateAction`, `TokensControllerAddDetectedTokensAction`, `TokensControllerActions`, `TokensControllerStateChangeEvent`, `TokensControllerEvents` ([#3609](https://github.com/MetaMask/core/pull/3609)). +- `TokenListController` exports type `TokenListControllerMessenger` ([#3609](https://github.com/MetaMask/core/pull/3609)). - `TokenDetectionController` exports types `TokenDetectionControllerMessenger`, `TokenDetectionControllerActions`, `TokenDetectionControllerGetStateAction`, `TokenDetectionControllerEvents`, `TokenDetectionControllerStateChangeEvent` ([#3609](https://github.com/MetaMask/core/pull/3609)). ### Changed diff --git a/packages/assets-controllers/src/TokensController.ts b/packages/assets-controllers/src/TokensController.ts index 3633bddc8b..19081fb8b7 100644 --- a/packages/assets-controllers/src/TokensController.ts +++ b/packages/assets-controllers/src/TokensController.ts @@ -5,6 +5,8 @@ import type { BaseConfig, BaseState, RestrictedControllerMessenger, + ControllerGetStateAction, + ControllerStateChangeEvent, } from '@metamask/base-controller'; import { BaseControllerV1 } from '@metamask/base-controller'; import contractsMap from '@metamask/contract-metadata'; @@ -52,16 +54,13 @@ import type { Token } from './TokenRatesController'; * Tokens controller configuration * @property selectedAddress - Vault selected address */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface TokensConfig extends BaseConfig { +export type TokensConfig = BaseConfig & { selectedAddress: string; chainId: Hex; // TODO: Replace `any` with type // eslint-disable-next-line @typescript-eslint/no-explicit-any provider: any; -} +}; /** * @type SuggestedAssetMeta @@ -73,7 +72,7 @@ export interface TokensConfig extends BaseConfig { * @property asset - Asset suggested object * @property interactingAddress - Account address that requested watch asset */ -type SuggestedAssetMeta = { +export type SuggestedAssetMeta = { id: string; time: number; type: string; @@ -92,36 +91,55 @@ type SuggestedAssetMeta = { * @property allIgnoredTokens - Object containing hidden/ignored tokens by network and account * @property allDetectedTokens - Object containing tokens detected with non-zero balances */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface TokensState extends BaseState { - tokens: Token[]; - ignoredTokens: string[]; - detectedTokens: Token[]; - allTokens: { [chainId: Hex]: { [key: string]: Token[] } }; - allIgnoredTokens: { [chainId: Hex]: { [key: string]: string[] } }; - allDetectedTokens: { [chainId: Hex]: { [key: string]: Token[] } }; -} +export type TokensState = BaseState & + Record & { + tokens: Token[]; + ignoredTokens: string[]; + detectedTokens: Token[]; + allTokens: { [chainId: Hex]: { [key: string]: Token[] } }; + allIgnoredTokens: { [chainId: Hex]: { [key: string]: string[] } }; + allDetectedTokens: { [chainId: Hex]: { [key: string]: Token[] } }; + }; /** * The name of the {@link TokensController}. */ -const controllerName = 'TokensController'; +export const controllerName = 'TokensController'; + +export type TokensControllerGetStateAction = ControllerGetStateAction< + typeof controllerName, + TokensState +>; + +export type TokensControllerAddDetectedTokensAction = { + type: `${typeof controllerName}:addDetectedTokens`; + handler: TokensController['addDetectedTokens']; +}; + +export type TokensControllerActions = + | TokensControllerGetStateAction + | TokensControllerAddDetectedTokensAction; /** * The external actions available to the {@link TokensController}. */ -type AllowedActions = AddApprovalRequest; +export type AllowedActions = AddApprovalRequest; + +export type TokensControllerStateChangeEvent = ControllerStateChangeEvent< + typeof controllerName, + TokensState +>; + +export type TokensControllerEvents = TokensControllerStateChangeEvent; /** * The messenger of the {@link TokensController}. */ export type TokensControllerMessenger = RestrictedControllerMessenger< typeof controllerName, - AllowedActions, - never, - AllowedActions['type'], + TokensControllerActions | AllowedActions, + TokensControllerEvents, + (TokensControllerActions | AllowedActions)['type'], never >; diff --git a/packages/assets-controllers/src/index.ts b/packages/assets-controllers/src/index.ts index aa20b3c44a..5affc311eb 100644 --- a/packages/assets-controllers/src/index.ts +++ b/packages/assets-controllers/src/index.ts @@ -14,7 +14,16 @@ export type { export { TokenDetectionController } from './TokenDetectionController'; export * from './TokenListController'; export * from './TokenRatesController'; -export * from './TokensController'; +export type { + TokensState, + TokensControllerGetStateAction, + TokensControllerAddDetectedTokensAction, + TokensControllerActions, + TokensControllerStateChangeEvent, + TokensControllerEvents, + TokensControllerMessenger, +} from './TokensController'; +export { TokensController } from './TokensController'; export { isTokenDetectionSupportedForNetwork, formatIconUrlWithProxy, From 44e92cbb674dd492c25a24eacc16a559c3021a5a Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Fri, 15 Dec 2023 14:30:27 -0800 Subject: [PATCH 56/82] Replace `getTokensState`, `addDetectedTokens` callbacks with messenger actions --- .../src/TokenDetectionController.test.ts | 30 +++++++++------ .../src/TokenDetectionController.ts | 37 +++++++++---------- 2 files changed, 37 insertions(+), 30 deletions(-) diff --git a/packages/assets-controllers/src/TokenDetectionController.test.ts b/packages/assets-controllers/src/TokenDetectionController.test.ts index a591df6ed9..eb34518a44 100644 --- a/packages/assets-controllers/src/TokenDetectionController.test.ts +++ b/packages/assets-controllers/src/TokenDetectionController.test.ts @@ -147,6 +147,8 @@ function buildTokenDetectionControllerMessenger( allowedActions: [ 'NetworkController:getNetworkConfigurationByNetworkClientId', 'NetworkController:findNetworkClientIdByChainId', + 'TokensController:getState', + 'TokensController:addDetectedTokens', 'TokenListController:getState', ], allowedEvents: [ @@ -248,6 +250,15 @@ describe('TokenDetectionController', () => { messenger: undefined as unknown as TokensControllerMessenger, }); + controllerMessenger.registerActionHandler( + `TokensController:getState`, + () => tokensController.state, + ); + controllerMessenger.registerActionHandler( + `TokensController:addDetectedTokens`, + tokensController.addDetectedTokens.bind(tokensController), + ); + const tokenListSetup = setupTokenListController(controllerMessenger); tokenList = tokenListSetup.tokenList; await tokenList.start(); @@ -258,9 +269,6 @@ describe('TokenDetectionController', () => { onPreferencesStateChange: (listener) => preferences.subscribe(listener), getBalancesInSingleCall: getBalancesInSingleCall as unknown as AssetsContractController['getBalancesInSingleCall'], - addDetectedTokens: - tokensController.addDetectedTokens.bind(tokensController), - getTokensState: () => tokensController.state, getPreferencesState: () => preferences.state, messenger: buildTokenDetectionControllerMessenger(controllerMessenger), }); @@ -468,6 +476,14 @@ describe('TokenDetectionController', () => { NetworkType.mainnet, ), ); + controllerMessenger.registerActionHandler( + `TokensController:getState`, + () => tokensController.state, + ); + controllerMessenger.registerActionHandler( + `TokensController:addDetectedTokens`, + tokensController.addDetectedTokens.bind(tokensController), + ); const tokenListSetup = setupTokenListController(controllerMessenger); tokenList = tokenListSetup.tokenList; @@ -485,8 +501,6 @@ describe('TokenDetectionController', () => { selectedAddress: '0x1', onPreferencesStateChange: stub, getBalancesInSingleCall: getBalancesInSingleCallMock, - addDetectedTokens: stub, - getTokensState: () => tokensController.state, getPreferencesState: () => preferences.state, messenger: buildTokenDetectionControllerMessenger(controllerMessenger), }); @@ -509,8 +523,6 @@ describe('TokenDetectionController', () => { chainId: ChainId.mainnet, onPreferencesStateChange: stub, getBalancesInSingleCall: getBalancesInSingleCallMock, - addDetectedTokens: stub, - getTokensState: () => tokensController.state, getPreferencesState: () => preferences.state, messenger: buildTokenDetectionControllerMessenger(controllerMessenger), }); @@ -532,8 +544,6 @@ describe('TokenDetectionController', () => { selectedAddress: '0x1', onPreferencesStateChange, getBalancesInSingleCall: getBalancesInSingleCallMock, - addDetectedTokens: stub, - getTokensState: () => tokensController.state, getPreferencesState: () => preferences.state, messenger: buildTokenDetectionControllerMessenger(controllerMessenger), }); @@ -552,8 +562,6 @@ describe('TokenDetectionController', () => { selectedAddress: '0x1', onPreferencesStateChange: stub, getBalancesInSingleCall: getBalancesInSingleCallMock, - addDetectedTokens: stub, - getTokensState: () => tokensController.state, getPreferencesState: () => preferences.state, messenger: buildTokenDetectionControllerMessenger(controllerMessenger), }); diff --git a/packages/assets-controllers/src/TokenDetectionController.ts b/packages/assets-controllers/src/TokenDetectionController.ts index 700131f805..e2bc2132e6 100644 --- a/packages/assets-controllers/src/TokenDetectionController.ts +++ b/packages/assets-controllers/src/TokenDetectionController.ts @@ -25,7 +25,10 @@ import type { TokenListStateChange, } from './TokenListController'; import type { Token } from './TokenRatesController'; -import type { TokensController, TokensState } from './TokensController'; +import type { + TokensControllerAddDetectedTokensAction, + TokensControllerGetStateAction, +} from './TokensController'; const DEFAULT_INTERVAL = 180000; @@ -44,6 +47,8 @@ export type TokenDetectionControllerActions = export type AllowedActions = | NetworkControllerGetNetworkConfigurationByNetworkClientId | NetworkControllerFindNetworkClientIdByChainIdAction + | TokensControllerGetStateAction + | TokensControllerAddDetectedTokensAction | GetTokenListState; export type TokenDetectionControllerStateChangeEvent = @@ -100,10 +105,6 @@ export class TokenDetectionController extends StaticIntervalPollingController< readonly #getBalancesInSingleCall: AssetsContractController['getBalancesInSingleCall']; - readonly #addDetectedTokens: TokensController['addDetectedTokens']; - - readonly #getTokensState: () => TokensState; - /** * Creates a TokenDetectionController instance. * @@ -114,8 +115,6 @@ export class TokenDetectionController extends StaticIntervalPollingController< * @param options.selectedAddress - Vault selected address * @param options.onPreferencesStateChange - Allows subscribing to preferences controller state changes. * @param options.getBalancesInSingleCall - Gets the balances of a list of tokens for the given address. - * @param options.addDetectedTokens - Add a list of detected tokens. - * @param options.getTokensState - Gets the current state of the Tokens controller. * @param options.getPreferencesState - Gets the state of the preferences controller. */ constructor({ @@ -124,8 +123,6 @@ export class TokenDetectionController extends StaticIntervalPollingController< interval = DEFAULT_INTERVAL, onPreferencesStateChange, getBalancesInSingleCall, - addDetectedTokens, - getTokensState, getPreferencesState, messenger, }: { @@ -136,8 +133,6 @@ export class TokenDetectionController extends StaticIntervalPollingController< listener: (preferencesState: PreferencesState) => void, ) => void; getBalancesInSingleCall: AssetsContractController['getBalancesInSingleCall']; - addDetectedTokens: TokensController['addDetectedTokens']; - getTokensState: () => TokensState; getPreferencesState: () => PreferencesState; messenger: TokenDetectionControllerMessenger; }) { @@ -164,8 +159,6 @@ export class TokenDetectionController extends StaticIntervalPollingController< this.#chainId, ); - this.#getTokensState = getTokensState; - this.#addDetectedTokens = addDetectedTokens; this.#getBalancesInSingleCall = getBalancesInSingleCall; this.messagingSystem.subscribe( @@ -293,7 +286,7 @@ export class TokenDetectionController extends StaticIntervalPollingController< ) { return; } - const { tokens } = this.#getTokensState(); + const { tokens } = this.messagingSystem.call('TokensController:getState'); const selectedAddress = accountAddress ?? this.#selectedAddress; const chainId = this.#getCorrectChainId(networkClientId); const tokensAddresses = tokens.map( @@ -334,7 +327,9 @@ export class TokenDetectionController extends StaticIntervalPollingController< for (const tokenAddress of Object.keys(balances)) { let ignored; /* istanbul ignore else */ - const { ignoredTokens } = this.#getTokensState(); + const { ignoredTokens } = this.messagingSystem.call( + 'TokensController:getState', + ); if (ignoredTokens.length) { ignored = ignoredTokens.find( (ignoredTokenAddress) => @@ -362,10 +357,14 @@ export class TokenDetectionController extends StaticIntervalPollingController< } if (tokensToAdd.length) { - await this.#addDetectedTokens(tokensToAdd, { - selectedAddress, - chainId, - }); + this.messagingSystem.call( + 'TokensController:addDetectedTokens', + tokensToAdd, + { + selectedAddress, + chainId, + }, + ); } }); } From 4b0bda3256fd951c397f0a162ecf77017a6aca89 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Fri, 15 Dec 2023 14:51:30 -0800 Subject: [PATCH 57/82] Make `detectTokens` take empty options object as default argument --- .../assets-controllers/src/TokenDetectionController.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/assets-controllers/src/TokenDetectionController.ts b/packages/assets-controllers/src/TokenDetectionController.ts index e2bc2132e6..a62bca6ed1 100644 --- a/packages/assets-controllers/src/TokenDetectionController.ts +++ b/packages/assets-controllers/src/TokenDetectionController.ts @@ -274,11 +274,13 @@ export class TokenDetectionController extends StaticIntervalPollingController< * @param options.networkClientId - The ID of the network client to use. * @param options.accountAddress - The account address to use. */ - async detectTokens(options?: { + async detectTokens({ + networkClientId, + accountAddress, + }: { networkClientId?: NetworkClientId; accountAddress?: string; - }): Promise { - const { networkClientId, accountAddress } = options ?? {}; + } = {}): Promise { if ( this.#disabled || !this.#isDetectionEnabledForNetwork || From a007d1516eeeab088ed3190c61dbea517bbdbc88 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Fri, 15 Dec 2023 14:51:54 -0800 Subject: [PATCH 58/82] Remove assignment to `#chainId` in `#getCorrectChainId` --- packages/assets-controllers/src/TokenDetectionController.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/assets-controllers/src/TokenDetectionController.ts b/packages/assets-controllers/src/TokenDetectionController.ts index a62bca6ed1..f18462d77c 100644 --- a/packages/assets-controllers/src/TokenDetectionController.ts +++ b/packages/assets-controllers/src/TokenDetectionController.ts @@ -251,10 +251,7 @@ export class TokenDetectionController extends StaticIntervalPollingController< 'NetworkController:getNetworkConfigurationByNetworkClientId', networkClientId ?? this.#networkClientId, ) ?? {}; - if (chainId) { - this.#chainId = chainId; - } - return this.#chainId; + return chainId ?? this.#chainId; } async _executePoll( From f5e66114a14b07cf443eb75e338f99428907b1fb Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Fri, 15 Dec 2023 14:53:31 -0800 Subject: [PATCH 59/82] Edit test description --- .../assets-controllers/src/TokenDetectionController.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/assets-controllers/src/TokenDetectionController.test.ts b/packages/assets-controllers/src/TokenDetectionController.test.ts index eb34518a44..14be42cb55 100644 --- a/packages/assets-controllers/src/TokenDetectionController.test.ts +++ b/packages/assets-controllers/src/TokenDetectionController.test.ts @@ -531,7 +531,7 @@ describe('TokenDetectionController', () => { expect(getBalancesInSingleCallMock.called).toBe(false); }); - it('should be called if onPreferencesStateChange is called with useTokenDetection being true and is changed', async () => { + it('should be called if onPreferencesStateChange is called with useTokenDetection being true and selectedAddress is changed', async () => { // TODO: Replace `any` with type // eslint-disable-next-line @typescript-eslint/no-explicit-any let preferencesStateChangeListener: (state: any) => void; From b2a5af54274d3314318c8d2f5cc4481e6b0454e6 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Fri, 15 Dec 2023 14:57:34 -0800 Subject: [PATCH 60/82] Update changelog --- packages/assets-controllers/CHANGELOG.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/assets-controllers/CHANGELOG.md b/packages/assets-controllers/CHANGELOG.md index 7fe85115e1..dc933d1e7d 100644 --- a/packages/assets-controllers/CHANGELOG.md +++ b/packages/assets-controllers/CHANGELOG.md @@ -12,8 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - **BREAKING:** `TokenDetectionController` is upgraded to extend `BaseControllerV2` and `StaticIntervalPollingController` ([#3609](https://github.com/MetaMask/core/pull/3609)). - - The constructor now expects an options object as its only argument, with required properties `messenger`, `chainId`, required callbacks `onPreferencesStateChange`, `getBalancesInSingleCall`, `addDetectedTokens`, `getTokensState`, `getPreferencesState`, and optional properties `interval`, `selectedAddress`. - - The controller state is an empty object and the controller config object is deprecated. + - The constructor now expects an options object as its only argument, with required properties `messenger`, `chainId`, required callbacks `onPreferencesStateChange`, `getBalancesInSingleCall`, `getPreferencesState`, and optional properties `interval`, `selectedAddress`. Note that the `config` object is no longer used by the constructor. - Polling can only be initiated by calling `start` or `startPollingByNetworkClientId` from a controller client. There is no longer an option to automatically start polling when the controller is instantiated. ## [21.0.0] From aa9e3bad37deda17501b3475dcf07796a47e01b2 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Fri, 15 Dec 2023 15:04:20 -0800 Subject: [PATCH 61/82] Adjust jest coverage thresholds --- packages/assets-controllers/jest.config.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/assets-controllers/jest.config.js b/packages/assets-controllers/jest.config.js index 11de507241..3c492c8028 100644 --- a/packages/assets-controllers/jest.config.js +++ b/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: 89.47, - functions: 97, - lines: 97.4, - statements: 97.45, + branches: 88.98, + functions: 97.08, + lines: 97.42, + statements: 97.47, }, }, From f636e2527d0fab53c18d9aa153836a0643fc44f3 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Fri, 15 Dec 2023 15:13:18 -0800 Subject: [PATCH 62/82] Revert "Fix incorrect package-level export" This reverts commit d40247e2fc2994f842b2dad8f6a843acbbdba2d0. --- packages/assets-controllers/src/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/assets-controllers/src/index.ts b/packages/assets-controllers/src/index.ts index 5affc311eb..1eaa467125 100644 --- a/packages/assets-controllers/src/index.ts +++ b/packages/assets-controllers/src/index.ts @@ -29,4 +29,4 @@ export { formatIconUrlWithProxy, getFormattedIpfsUrl, } from './assetsUtil'; -export { codefiTokenPricesServiceV2 } from './token-prices-service'; +export { CodefiTokenPricesServiceV2 } from './token-prices-service'; From 173168f172ec1b1b56139c6a7ecb559c49ad7c36 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Fri, 15 Dec 2023 16:10:11 -0800 Subject: [PATCH 63/82] Use `getNetworkClientById` action in `#getCorrectChainId` method --- .../src/TokenDetectionController.ts | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/packages/assets-controllers/src/TokenDetectionController.ts b/packages/assets-controllers/src/TokenDetectionController.ts index f18462d77c..9477d56fd0 100644 --- a/packages/assets-controllers/src/TokenDetectionController.ts +++ b/packages/assets-controllers/src/TokenDetectionController.ts @@ -246,12 +246,13 @@ export class TokenDetectionController extends StaticIntervalPollingController< } #getCorrectChainId(networkClientId?: NetworkClientId) { - const { chainId } = - this.messagingSystem.call( - 'NetworkController:getNetworkConfigurationByNetworkClientId', - networkClientId ?? this.#networkClientId, - ) ?? {}; - return chainId ?? this.#chainId; + const { + configuration: { chainId }, + } = this.messagingSystem.call( + 'NetworkController:getNetworkClientById', + networkClientId ?? this.#networkClientId, + ); + return chainId; } async _executePoll( From d81c23d914e85f65564747e62d1e75d195e83184 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Fri, 15 Dec 2023 16:13:14 -0800 Subject: [PATCH 64/82] Replace `chainId` with `networkClientId` in constructor options. - Derive `chainId` from `networkClientId` using `#getCorrectChainId` method --- packages/assets-controllers/CHANGELOG.md | 2 +- .../src/TokenDetectionController.test.ts | 67 ++++++++++--------- .../src/TokenDetectionController.ts | 30 ++++----- 3 files changed, 47 insertions(+), 52 deletions(-) diff --git a/packages/assets-controllers/CHANGELOG.md b/packages/assets-controllers/CHANGELOG.md index dc933d1e7d..d84633aa55 100644 --- a/packages/assets-controllers/CHANGELOG.md +++ b/packages/assets-controllers/CHANGELOG.md @@ -12,7 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - **BREAKING:** `TokenDetectionController` is upgraded to extend `BaseControllerV2` and `StaticIntervalPollingController` ([#3609](https://github.com/MetaMask/core/pull/3609)). - - The constructor now expects an options object as its only argument, with required properties `messenger`, `chainId`, required callbacks `onPreferencesStateChange`, `getBalancesInSingleCall`, `getPreferencesState`, and optional properties `interval`, `selectedAddress`. Note that the `config` object is no longer used by the constructor. + - The constructor now expects an options object as its only argument, with required properties `messenger`, `networkClientId`, required callbacks `onPreferencesStateChange`, `getBalancesInSingleCall`, `getPreferencesState`, and optional properties `interval`, `selectedAddress`. Note that the `config` object is no longer used by the constructor. - Polling can only be initiated by calling `start` or `startPollingByNetworkClientId` from a controller client. There is no longer an option to automatically start polling when the controller is instantiated. ## [21.0.0] diff --git a/packages/assets-controllers/src/TokenDetectionController.test.ts b/packages/assets-controllers/src/TokenDetectionController.test.ts index 14be42cb55..d66c0c113d 100644 --- a/packages/assets-controllers/src/TokenDetectionController.test.ts +++ b/packages/assets-controllers/src/TokenDetectionController.test.ts @@ -145,8 +145,7 @@ function buildTokenDetectionControllerMessenger( return controllerMessenger.getRestricted({ name: controllerName, allowedActions: [ - 'NetworkController:getNetworkConfigurationByNetworkClientId', - 'NetworkController:findNetworkClientIdByChainId', + 'NetworkController:getNetworkClientById', 'TokensController:getState', 'TokensController:addDetectedTokens', 'TokenListController:getState', @@ -172,23 +171,25 @@ describe('TokenDetectionController', () => { const onNetworkDidChangeListeners: ((state: NetworkState) => void)[] = []; const getNetworkConfigurationByNetworkClientIdHandler = jest.fn(); - const findNetworkClientIdByChainIdHandler = jest.fn(); + const getNetworkClientByIdHandler = jest.fn(); const changeNetwork = (providerConfig: ProviderConfig) => { - onNetworkDidChangeListeners.forEach((listener) => { - listener({ - ...defaultNetworkState, - providerConfig, - }); - }); - controllerMessenger.publish('NetworkController:networkDidChange', { ...defaultNetworkState, providerConfig, + selectedNetworkClientId: providerConfig.type, }); getNetworkConfigurationByNetworkClientIdHandler.mockReturnValue( providerConfig, ); + getNetworkClientByIdHandler.mockReturnValue({ + configuration: { + chainId: providerConfig.chainId, + }, + provider: {}, + blockTracker: {}, + destroy: jest.fn(), + }); }; const mainnet = { chainId: ChainId.mainnet, @@ -228,14 +229,15 @@ describe('TokenDetectionController', () => { .callsFake(() => null as any); controllerMessenger.registerActionHandler( - `NetworkController:getNetworkConfigurationByNetworkClientId`, - getNetworkConfigurationByNetworkClientIdHandler.mockReturnValueOnce( - mainnet, - ), - ); - controllerMessenger.registerActionHandler( - `NetworkController:findNetworkClientIdByChainId`, - findNetworkClientIdByChainIdHandler.mockReturnValue(NetworkType.mainnet), + `NetworkController:getNetworkClientById`, + getNetworkClientByIdHandler.mockReturnValue({ + configuration: { + chainId: ChainId.mainnet, + }, + provider: {}, + blockTracker: {}, + destroy: jest.fn(), + }), ); tokensController = new TokensController({ @@ -265,7 +267,7 @@ describe('TokenDetectionController', () => { getBalancesInSingleCall = sinon.stub(); tokenDetection = new TokenDetectionController({ - chainId: ChainId.mainnet, + networkClientId: NetworkType.mainnet, onPreferencesStateChange: (listener) => preferences.subscribe(listener), getBalancesInSingleCall: getBalancesInSingleCall as unknown as AssetsContractController['getBalancesInSingleCall'], @@ -465,16 +467,15 @@ describe('TokenDetectionController', () => { controllerMessenger = getControllerMessenger(); controllerMessenger.registerActionHandler( - `NetworkController:getNetworkConfigurationByNetworkClientId`, - getNetworkConfigurationByNetworkClientIdHandler.mockReturnValueOnce( - mainnet, - ), - ); - controllerMessenger.registerActionHandler( - `NetworkController:findNetworkClientIdByChainId`, - findNetworkClientIdByChainIdHandler.mockReturnValue( - NetworkType.mainnet, - ), + `NetworkController:getNetworkClientById`, + getNetworkClientByIdHandler.mockReturnValue({ + configuration: { + chainId: ChainId.mainnet, + }, + provider: {}, + blockTracker: {}, + destroy: jest.fn(), + }), ); controllerMessenger.registerActionHandler( `TokensController:getState`, @@ -497,7 +498,7 @@ describe('TokenDetectionController', () => { .reply(200, sampleTokenList); tokenDetection = new TokenDetectionController({ - chainId: ChainId.mainnet, + networkClientId: NetworkType.mainnet, selectedAddress: '0x1', onPreferencesStateChange: stub, getBalancesInSingleCall: getBalancesInSingleCallMock, @@ -520,7 +521,7 @@ describe('TokenDetectionController', () => { it('should not be called if TokenListController is updated to have an empty token list', async () => { tokenDetection = new TokenDetectionController({ - chainId: ChainId.mainnet, + networkClientId: NetworkType.mainnet, onPreferencesStateChange: stub, getBalancesInSingleCall: getBalancesInSingleCallMock, getPreferencesState: () => preferences.state, @@ -540,7 +541,7 @@ describe('TokenDetectionController', () => { }); tokenDetection = new TokenDetectionController({ - chainId: ChainId.mainnet, + networkClientId: NetworkType.mainnet, selectedAddress: '0x1', onPreferencesStateChange, getBalancesInSingleCall: getBalancesInSingleCallMock, @@ -558,7 +559,7 @@ describe('TokenDetectionController', () => { it('should be called if network is changed to a chainId that supports token detection', async () => { tokenDetection = new TokenDetectionController({ - chainId: SupportedTokenDetectionNetworks.polygon, + networkClientId: 'polygon', selectedAddress: '0x1', onPreferencesStateChange: stub, getBalancesInSingleCall: getBalancesInSingleCallMock, diff --git a/packages/assets-controllers/src/TokenDetectionController.ts b/packages/assets-controllers/src/TokenDetectionController.ts index 9477d56fd0..b680837d58 100644 --- a/packages/assets-controllers/src/TokenDetectionController.ts +++ b/packages/assets-controllers/src/TokenDetectionController.ts @@ -9,10 +9,9 @@ import { } from '@metamask/controller-utils'; import type { NetworkClientId, - NetworkControllerGetNetworkConfigurationByNetworkClientId, - NetworkControllerFindNetworkClientIdByChainIdAction, NetworkControllerNetworkDidChangeEvent, NetworkControllerStateChangeEvent, + NetworkControllerGetNetworkClientByIdAction, } from '@metamask/network-controller'; import { StaticIntervalPollingController } from '@metamask/polling-controller'; import type { PreferencesState } from '@metamask/preferences-controller'; @@ -45,8 +44,7 @@ export type TokenDetectionControllerActions = TokenDetectionControllerGetStateAction; export type AllowedActions = - | NetworkControllerGetNetworkConfigurationByNetworkClientId - | NetworkControllerFindNetworkClientIdByChainIdAction + | NetworkControllerGetNetworkClientByIdAction | TokensControllerGetStateAction | TokensControllerAddDetectedTokensAction | GetTokenListState; @@ -111,14 +109,14 @@ export class TokenDetectionController extends StaticIntervalPollingController< * @param options - The controller options. * @param options.messenger - The controller messaging system. * @param options.interval - Polling interval used to fetch new token rates - * @param options.chainId - The chain ID of the current network + * @param options.networkClientId - The selected network client ID of the current network * @param options.selectedAddress - Vault selected address * @param options.onPreferencesStateChange - Allows subscribing to preferences controller state changes. * @param options.getBalancesInSingleCall - Gets the balances of a list of tokens for the given address. * @param options.getPreferencesState - Gets the state of the preferences controller. */ constructor({ - chainId, + networkClientId, selectedAddress = '', interval = DEFAULT_INTERVAL, onPreferencesStateChange, @@ -126,7 +124,7 @@ export class TokenDetectionController extends StaticIntervalPollingController< getPreferencesState, messenger, }: { - chainId: Hex; + networkClientId: NetworkClientId; selectedAddress?: string; interval?: number; onPreferencesStateChange: ( @@ -148,12 +146,11 @@ export class TokenDetectionController extends StaticIntervalPollingController< this.#disabled = false; this.setIntervalLength(interval); - this.#chainId = chainId; + + this.#networkClientId = networkClientId; this.#selectedAddress = selectedAddress; - this.#networkClientId = this.messagingSystem.call( - 'NetworkController:findNetworkClientIdByChainId', - chainId, - ); + this.#chainId = this.#getCorrectChainId(networkClientId); + this.#isDetectionEnabledFromPreferences = defaultUseTokenDetection; this.#isDetectionEnabledForNetwork = isTokenDetectionSupportedForNetwork( this.#chainId, @@ -193,15 +190,12 @@ export class TokenDetectionController extends StaticIntervalPollingController< this.messagingSystem.subscribe( 'NetworkController:networkDidChange', - async ({ - providerConfig: { chainId: newChainId }, - selectedNetworkClientId, - }) => { + async ({ selectedNetworkClientId }) => { + this.#networkClientId = selectedNetworkClientId; + const newChainId = this.#getCorrectChainId(selectedNetworkClientId); const isChainIdChanged = this.#chainId !== newChainId; this.#chainId = newChainId; - this.#networkClientId = selectedNetworkClientId; - this.#isDetectionEnabledForNetwork = isTokenDetectionSupportedForNetwork(newChainId); From d9bd4b179fe41f76de0af2961ae2bd98160d140c Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Mon, 18 Dec 2023 12:34:14 -0800 Subject: [PATCH 65/82] Remove `getNetworkConfigurationByClientId` handler from tests --- .../assets-controllers/src/TokenDetectionController.test.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/assets-controllers/src/TokenDetectionController.test.ts b/packages/assets-controllers/src/TokenDetectionController.test.ts index d66c0c113d..5b2799ce9d 100644 --- a/packages/assets-controllers/src/TokenDetectionController.test.ts +++ b/packages/assets-controllers/src/TokenDetectionController.test.ts @@ -170,7 +170,6 @@ describe('TokenDetectionController', () => { >; const onNetworkDidChangeListeners: ((state: NetworkState) => void)[] = []; - const getNetworkConfigurationByNetworkClientIdHandler = jest.fn(); const getNetworkClientByIdHandler = jest.fn(); const changeNetwork = (providerConfig: ProviderConfig) => { controllerMessenger.publish('NetworkController:networkDidChange', { @@ -179,9 +178,6 @@ describe('TokenDetectionController', () => { selectedNetworkClientId: providerConfig.type, }); - getNetworkConfigurationByNetworkClientIdHandler.mockReturnValue( - providerConfig, - ); getNetworkClientByIdHandler.mockReturnValue({ configuration: { chainId: providerConfig.chainId, From fefc64becbe390d48732fc1dab9936bda267a3c9 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Mon, 18 Dec 2023 12:35:02 -0800 Subject: [PATCH 66/82] test: adjust `getNetworkClientById` handler to return polygon chainId --- .../src/TokenDetectionController.test.ts | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/packages/assets-controllers/src/TokenDetectionController.test.ts b/packages/assets-controllers/src/TokenDetectionController.test.ts index 5b2799ce9d..97dc290475 100644 --- a/packages/assets-controllers/src/TokenDetectionController.test.ts +++ b/packages/assets-controllers/src/TokenDetectionController.test.ts @@ -510,7 +510,7 @@ describe('TokenDetectionController', () => { changeNetwork({ chainId: toHex(polygonDecimalChainId), type: NetworkType.rpc, - ticker: NetworksTicker.rpc, + ticker: 'MATIC', }); expect(getBalancesInSingleCallMock.called).toBe(false); }); @@ -550,7 +550,7 @@ describe('TokenDetectionController', () => { selectedAddress: '0x2', useTokenDetection: true, }); - expect(getBalancesInSingleCallMock.called).toBe(true); + expect(getBalancesInSingleCallMock.calledOnce).toBe(true); }); it('should be called if network is changed to a chainId that supports token detection', async () => { @@ -562,9 +562,17 @@ describe('TokenDetectionController', () => { getPreferencesState: () => preferences.state, messenger: buildTokenDetectionControllerMessenger(controllerMessenger), }); + getNetworkClientByIdHandler.mockReturnValue({ + configuration: { + chainId: SupportedTokenDetectionNetworks.polygon, + }, + provider: {}, + blockTracker: {}, + destroy: jest.fn(), + }); changeNetwork(mainnet); - expect(getBalancesInSingleCallMock.called).toBe(true); + expect(getBalancesInSingleCallMock.calledOnce).toBe(true); }); }); From 73eccbf25f42f340cbac0bb7892a3d32fce8b7aa Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Mon, 18 Dec 2023 12:36:29 -0800 Subject: [PATCH 67/82] test: remove unnecessary `.start()` calls --- .../src/TokenDetectionController.test.ts | 8 -------- 1 file changed, 8 deletions(-) diff --git a/packages/assets-controllers/src/TokenDetectionController.test.ts b/packages/assets-controllers/src/TokenDetectionController.test.ts index 97dc290475..6fc0710a5c 100644 --- a/packages/assets-controllers/src/TokenDetectionController.test.ts +++ b/packages/assets-controllers/src/TokenDetectionController.test.ts @@ -355,11 +355,8 @@ describe('TokenDetectionController', () => { it('should not add ignoredTokens to the tokens list if detected with balance', async () => { preferences.setSelectedAddress('0x0001'); - changeNetwork(mainnet); - await tokenDetection.start(); - await tokensController.addToken({ address: sampleTokenA.address, symbol: sampleTokenA.symbol, @@ -390,8 +387,6 @@ describe('TokenDetectionController', () => { preferences.setSelectedAddress('0x0001'); changeNetwork(mainnet); - await tokenDetection.start(); - await tokensController.addToken({ address: sampleTokenA.address, symbol: sampleTokenA.symbol, @@ -413,8 +408,6 @@ describe('TokenDetectionController', () => { preferences.update({ selectedAddress: '0x1' }); changeNetwork(mainnet); - await tokenDetection.start(); - getBalancesInSingleCall.resolves({ [sampleTokenA.address]: new BN(1), }); @@ -428,7 +421,6 @@ describe('TokenDetectionController', () => { }); it('should not detect tokens if there is no selectedAddress set', async () => { - await tokenDetection.start(); getBalancesInSingleCall.resolves({ [sampleTokenA.address]: new BN(1), }); From 5fe76743e6bac267557778baaede62ce43c2fccd Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Mon, 18 Dec 2023 14:12:10 -0800 Subject: [PATCH 68/82] Add `disabled` as constructor option and set it to block `#startPolling`, `_executePoll`, `_startPollingByNetworkClientId` calls --- .../src/TokenDetectionController.ts | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/packages/assets-controllers/src/TokenDetectionController.ts b/packages/assets-controllers/src/TokenDetectionController.ts index b680837d58..83b89ec51f 100644 --- a/packages/assets-controllers/src/TokenDetectionController.ts +++ b/packages/assets-controllers/src/TokenDetectionController.ts @@ -74,7 +74,7 @@ export type TokenDetectionControllerMessenger = RestrictedControllerMessenger< * @property chainId - The chain ID of the current network * @property selectedAddress - Vault selected address * @property networkClientId - The network client ID of the current selected network - * @property disabled - Boolean to track if passive auto-detection is currently enabled in this controller + * @property disabled - Boolean to track if network requests are blocked * @property isDetectionEnabledFromPreferences - Boolean to track if detection is enabled from PreferencesController * @property isDetectionEnabledForNetwork - Boolean to track if detected is enabled for current network */ @@ -108,6 +108,7 @@ export class TokenDetectionController extends StaticIntervalPollingController< * * @param options - The controller options. * @param options.messenger - The controller messaging system. + * @param options.disabled - If set to true, all network requests are blocked. * @param options.interval - Polling interval used to fetch new token rates * @param options.networkClientId - The selected network client ID of the current network * @param options.selectedAddress - Vault selected address @@ -119,6 +120,7 @@ export class TokenDetectionController extends StaticIntervalPollingController< networkClientId, selectedAddress = '', interval = DEFAULT_INTERVAL, + disabled = false, onPreferencesStateChange, getBalancesInSingleCall, getPreferencesState, @@ -127,6 +129,7 @@ export class TokenDetectionController extends StaticIntervalPollingController< networkClientId: NetworkClientId; selectedAddress?: string; interval?: number; + disabled?: boolean; onPreferencesStateChange: ( listener: (preferencesState: PreferencesState) => void, ) => void; @@ -144,7 +147,7 @@ export class TokenDetectionController extends StaticIntervalPollingController< metadata: {}, }); - this.#disabled = false; + this.#disabled = disabled; this.setIntervalLength(interval); this.#networkClientId = networkClientId; @@ -232,6 +235,9 @@ export class TokenDetectionController extends StaticIntervalPollingController< * Starts a new polling interval. */ async #startPolling(): Promise { + if (this.#disabled) { + return; + } this.#stopPolling(); await this.detectTokens(); this.#intervalId = setInterval(async () => { @@ -253,7 +259,12 @@ export class TokenDetectionController extends StaticIntervalPollingController< networkClientId: string, options: { address: string }, ): Promise { - return await this.detectTokens({ + if (this.#disabled) { + throw new Error( + 'Poll cannot be executed while network requests are disabled', + ); + } + await this.detectTokens({ networkClientId, accountAddress: options.address, }); From 116a22796fb611e0a5331d379f145b1a184f2983 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Mon, 18 Dec 2023 14:14:12 -0800 Subject: [PATCH 69/82] Adjust jest coverage thresholds --- packages/assets-controllers/jest.config.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/assets-controllers/jest.config.js b/packages/assets-controllers/jest.config.js index 3c492c8028..ed5fc4b833 100644 --- a/packages/assets-controllers/jest.config.js +++ b/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.98, + branches: 88.7, functions: 97.08, - lines: 97.42, - statements: 97.47, + lines: 97.22, + statements: 97.27, }, }, From acd022fb196c96ea88eb11331dbec5fe840875f2 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Tue, 19 Dec 2023 13:59:00 -0800 Subject: [PATCH 70/82] Remove unused comment --- packages/assets-controllers/src/TokenDetectionController.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/assets-controllers/src/TokenDetectionController.ts b/packages/assets-controllers/src/TokenDetectionController.ts index 83b89ec51f..a9a40556f0 100644 --- a/packages/assets-controllers/src/TokenDetectionController.ts +++ b/packages/assets-controllers/src/TokenDetectionController.ts @@ -97,10 +97,6 @@ export class TokenDetectionController extends StaticIntervalPollingController< #isDetectionEnabledForNetwork: boolean; - /** - * Name of this controller used during composition - */ - readonly #getBalancesInSingleCall: AssetsContractController['getBalancesInSingleCall']; /** From ee2154d28fb5c7ffa51067741eac5df95c13202f Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Wed, 20 Dec 2023 07:47:51 -0800 Subject: [PATCH 71/82] test: Restore publishes for `networkDidChange` event, mock polygon network client --- packages/assets-controllers/jest.config.js | 2 +- .../src/TokenDetectionController.test.ts | 63 +++++++++++++++++-- 2 files changed, 58 insertions(+), 7 deletions(-) diff --git a/packages/assets-controllers/jest.config.js b/packages/assets-controllers/jest.config.js index ed5fc4b833..f5db735b4e 100644 --- a/packages/assets-controllers/jest.config.js +++ b/packages/assets-controllers/jest.config.js @@ -17,7 +17,7 @@ module.exports = merge(baseConfig, { // An object that configures minimum threshold enforcement for coverage results coverageThreshold: { global: { - branches: 88.7, + branches: 88.81, functions: 97.08, lines: 97.22, statements: 97.27, diff --git a/packages/assets-controllers/src/TokenDetectionController.test.ts b/packages/assets-controllers/src/TokenDetectionController.test.ts index 6fc0710a5c..a8b9f05de5 100644 --- a/packages/assets-controllers/src/TokenDetectionController.test.ts +++ b/packages/assets-controllers/src/TokenDetectionController.test.ts @@ -7,7 +7,10 @@ import { convertHexToDecimal, toHex, } from '@metamask/controller-utils'; -import { defaultState as defaultNetworkState } from '@metamask/network-controller'; +import { + defaultState as defaultNetworkState, + NetworkClientType, +} from '@metamask/network-controller'; import type { NetworkState, ProviderConfig, @@ -172,6 +175,13 @@ describe('TokenDetectionController', () => { const onNetworkDidChangeListeners: ((state: NetworkState) => void)[] = []; const getNetworkClientByIdHandler = jest.fn(); const changeNetwork = (providerConfig: ProviderConfig) => { + onNetworkDidChangeListeners.forEach((listener) => { + listener({ + ...defaultNetworkState, + providerConfig, + }); + }); + controllerMessenger.publish('NetworkController:networkDidChange', { ...defaultNetworkState, providerConfig, @@ -186,17 +196,40 @@ describe('TokenDetectionController', () => { blockTracker: {}, destroy: jest.fn(), }); + + controllerMessenger.unregisterActionHandler( + 'NetworkController:getNetworkClientById', + ); + controllerMessenger.registerActionHandler( + 'NetworkController:getNetworkClientById', + getNetworkClientByIdHandler, + ); }; - const mainnet = { - chainId: ChainId.mainnet, - type: NetworkType.mainnet, - ticker: NetworksTicker.mainnet, - }; + const goerli = { chainId: ChainId.goerli, + id: 'goerli', type: NetworkType.goerli, ticker: NetworksTicker.goerli, }; + const polygon = { + chainId: SupportedTokenDetectionNetworks.polygon, + id: 'polygon', + type: NetworkType.rpc, + ticker: 'MATIC', + }; + const mockPolygonClient = { + configuration: { + ...polygon, + type: NetworkClientType.Custom, + }, + }; + const mainnet = { + chainId: ChainId.mainnet, + id: 'mainnet', + type: NetworkType.mainnet, + ticker: NetworksTicker.mainnet, + }; beforeEach(async () => { nock(TOKEN_END_POINT_API) @@ -224,6 +257,11 @@ describe('TokenDetectionController', () => { // eslint-disable-next-line @typescript-eslint/no-explicit-any .callsFake(() => null as any); + controllerMessenger.publish('NetworkController:networkDidChange', { + ...defaultNetworkState, + providerConfig: mainnet, + selectedNetworkClientId: NetworkType.mainnet, + }); controllerMessenger.registerActionHandler( `NetworkController:getNetworkClientById`, getNetworkClientByIdHandler.mockReturnValue({ @@ -454,6 +492,11 @@ describe('TokenDetectionController', () => { getBalancesInSingleCallMock = sinon.stub(); controllerMessenger = getControllerMessenger(); + controllerMessenger.publish('NetworkController:networkDidChange', { + ...defaultNetworkState, + providerConfig: mainnet, + selectedNetworkClientId: NetworkType.mainnet, + }); controllerMessenger.registerActionHandler( `NetworkController:getNetworkClientById`, getNetworkClientByIdHandler.mockReturnValue({ @@ -563,6 +606,14 @@ describe('TokenDetectionController', () => { destroy: jest.fn(), }); + controllerMessenger.unregisterActionHandler( + 'NetworkController:getNetworkClientById', + ); + controllerMessenger.registerActionHandler( + 'NetworkController:getNetworkClientById', + getNetworkClientByIdHandler.mockReturnValue(mockPolygonClient), + ); + changeNetwork(mainnet); expect(getBalancesInSingleCallMock.calledOnce).toBe(true); }); From 21f9b8c30af5bbe9c9006118123310db0fe83871 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Wed, 20 Dec 2023 07:49:12 -0800 Subject: [PATCH 72/82] Adjust jest coverage thresholds --- packages/assets-controllers/jest.config.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/assets-controllers/jest.config.js b/packages/assets-controllers/jest.config.js index f5db735b4e..da462aa049 100644 --- a/packages/assets-controllers/jest.config.js +++ b/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.81, + branches: 88.82, functions: 97.08, - lines: 97.22, - statements: 97.27, + lines: 97.23, + statements: 97.28, }, }, From de947613c5e25b000d8c34c2d2e22d969b3e4e76 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Wed, 20 Dec 2023 07:50:20 -0800 Subject: [PATCH 73/82] Revert "[tokens-controller] Add actions `getState`, `addDetectedTokens` and event `stateChange`" This reverts commit 757ab0464e3561ca89a25cee65590a5e9a272470. --- packages/assets-controllers/CHANGELOG.md | 3 +- .../src/TokensController.ts | 62 +++++++------------ packages/assets-controllers/src/index.ts | 11 +--- 3 files changed, 24 insertions(+), 52 deletions(-) diff --git a/packages/assets-controllers/CHANGELOG.md b/packages/assets-controllers/CHANGELOG.md index 7dc7ce6296..a7cb1a9b91 100644 --- a/packages/assets-controllers/CHANGELOG.md +++ b/packages/assets-controllers/CHANGELOG.md @@ -6,8 +6,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Added -- `TokensController` exports types `TokensControllerGetStateAction`, `TokensControllerAddDetectedTokensAction`, `TokensControllerActions`, `TokensControllerStateChangeEvent`, `TokensControllerEvents` ([#3609](https://github.com/MetaMask/core/pull/3609)). -- `TokenListController` exports type `TokenListControllerMessenger` ([#3609](https://github.com/MetaMask/core/pull/3609)). +- `TokenListController` now exports a `TokenListControllerMessenger` type ([#3609](https://github.com/MetaMask/core/pull/3609)). - `TokenDetectionController` exports types `TokenDetectionControllerMessenger`, `TokenDetectionControllerActions`, `TokenDetectionControllerGetStateAction`, `TokenDetectionControllerEvents`, `TokenDetectionControllerStateChangeEvent` ([#3609](https://github.com/MetaMask/core/pull/3609)). ### Changed diff --git a/packages/assets-controllers/src/TokensController.ts b/packages/assets-controllers/src/TokensController.ts index 19081fb8b7..3633bddc8b 100644 --- a/packages/assets-controllers/src/TokensController.ts +++ b/packages/assets-controllers/src/TokensController.ts @@ -5,8 +5,6 @@ import type { BaseConfig, BaseState, RestrictedControllerMessenger, - ControllerGetStateAction, - ControllerStateChangeEvent, } from '@metamask/base-controller'; import { BaseControllerV1 } from '@metamask/base-controller'; import contractsMap from '@metamask/contract-metadata'; @@ -54,13 +52,16 @@ import type { Token } from './TokenRatesController'; * Tokens controller configuration * @property selectedAddress - Vault selected address */ -export type TokensConfig = BaseConfig & { +// This interface was created before this ESLint rule was added. +// Convert to a `type` in a future major version. +// eslint-disable-next-line @typescript-eslint/consistent-type-definitions +export interface TokensConfig extends BaseConfig { selectedAddress: string; chainId: Hex; // TODO: Replace `any` with type // eslint-disable-next-line @typescript-eslint/no-explicit-any provider: any; -}; +} /** * @type SuggestedAssetMeta @@ -72,7 +73,7 @@ export type TokensConfig = BaseConfig & { * @property asset - Asset suggested object * @property interactingAddress - Account address that requested watch asset */ -export type SuggestedAssetMeta = { +type SuggestedAssetMeta = { id: string; time: number; type: string; @@ -91,55 +92,36 @@ export type SuggestedAssetMeta = { * @property allIgnoredTokens - Object containing hidden/ignored tokens by network and account * @property allDetectedTokens - Object containing tokens detected with non-zero balances */ -export type TokensState = BaseState & - Record & { - tokens: Token[]; - ignoredTokens: string[]; - detectedTokens: Token[]; - allTokens: { [chainId: Hex]: { [key: string]: Token[] } }; - allIgnoredTokens: { [chainId: Hex]: { [key: string]: string[] } }; - allDetectedTokens: { [chainId: Hex]: { [key: string]: Token[] } }; - }; +// This interface was created before this ESLint rule was added. +// Convert to a `type` in a future major version. +// eslint-disable-next-line @typescript-eslint/consistent-type-definitions +export interface TokensState extends BaseState { + tokens: Token[]; + ignoredTokens: string[]; + detectedTokens: Token[]; + allTokens: { [chainId: Hex]: { [key: string]: Token[] } }; + allIgnoredTokens: { [chainId: Hex]: { [key: string]: string[] } }; + allDetectedTokens: { [chainId: Hex]: { [key: string]: Token[] } }; +} /** * The name of the {@link TokensController}. */ -export const controllerName = 'TokensController'; - -export type TokensControllerGetStateAction = ControllerGetStateAction< - typeof controllerName, - TokensState ->; - -export type TokensControllerAddDetectedTokensAction = { - type: `${typeof controllerName}:addDetectedTokens`; - handler: TokensController['addDetectedTokens']; -}; - -export type TokensControllerActions = - | TokensControllerGetStateAction - | TokensControllerAddDetectedTokensAction; +const controllerName = 'TokensController'; /** * The external actions available to the {@link TokensController}. */ -export type AllowedActions = AddApprovalRequest; - -export type TokensControllerStateChangeEvent = ControllerStateChangeEvent< - typeof controllerName, - TokensState ->; - -export type TokensControllerEvents = TokensControllerStateChangeEvent; +type AllowedActions = AddApprovalRequest; /** * The messenger of the {@link TokensController}. */ export type TokensControllerMessenger = RestrictedControllerMessenger< typeof controllerName, - TokensControllerActions | AllowedActions, - TokensControllerEvents, - (TokensControllerActions | AllowedActions)['type'], + AllowedActions, + never, + AllowedActions['type'], never >; diff --git a/packages/assets-controllers/src/index.ts b/packages/assets-controllers/src/index.ts index 1eaa467125..c37217e571 100644 --- a/packages/assets-controllers/src/index.ts +++ b/packages/assets-controllers/src/index.ts @@ -14,16 +14,7 @@ export type { export { TokenDetectionController } from './TokenDetectionController'; export * from './TokenListController'; export * from './TokenRatesController'; -export type { - TokensState, - TokensControllerGetStateAction, - TokensControllerAddDetectedTokensAction, - TokensControllerActions, - TokensControllerStateChangeEvent, - TokensControllerEvents, - TokensControllerMessenger, -} from './TokensController'; -export { TokensController } from './TokensController'; +export * from './TokensController'; export { isTokenDetectionSupportedForNetwork, formatIconUrlWithProxy, From 4b6c1d2b5f17ff3798ebdb9faee7505ad1cd30fe Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Wed, 20 Dec 2023 08:03:25 -0800 Subject: [PATCH 74/82] test: Apply 'Revert "[tokens-controller] Add actions `getState`, `addDetectedTokens` and event `stateChange`" --- packages/assets-controllers/jest.config.js | 2 +- .../src/TokenDetectionController.test.ts | 34 +++++++--------- .../src/TokenDetectionController.ts | 40 ++++++++++--------- 3 files changed, 37 insertions(+), 39 deletions(-) diff --git a/packages/assets-controllers/jest.config.js b/packages/assets-controllers/jest.config.js index da462aa049..1ef36f44ca 100644 --- a/packages/assets-controllers/jest.config.js +++ b/packages/assets-controllers/jest.config.js @@ -17,7 +17,7 @@ module.exports = merge(baseConfig, { // An object that configures minimum threshold enforcement for coverage results coverageThreshold: { global: { - branches: 88.82, + branches: 88.8, functions: 97.08, lines: 97.23, statements: 97.28, diff --git a/packages/assets-controllers/src/TokenDetectionController.test.ts b/packages/assets-controllers/src/TokenDetectionController.test.ts index a8b9f05de5..1f534b91aa 100644 --- a/packages/assets-controllers/src/TokenDetectionController.test.ts +++ b/packages/assets-controllers/src/TokenDetectionController.test.ts @@ -149,8 +149,6 @@ function buildTokenDetectionControllerMessenger( name: controllerName, allowedActions: [ 'NetworkController:getNetworkClientById', - 'TokensController:getState', - 'TokensController:addDetectedTokens', 'TokenListController:getState', ], allowedEvents: [ @@ -286,15 +284,6 @@ describe('TokenDetectionController', () => { messenger: undefined as unknown as TokensControllerMessenger, }); - controllerMessenger.registerActionHandler( - `TokensController:getState`, - () => tokensController.state, - ); - controllerMessenger.registerActionHandler( - `TokensController:addDetectedTokens`, - tokensController.addDetectedTokens.bind(tokensController), - ); - const tokenListSetup = setupTokenListController(controllerMessenger); tokenList = tokenListSetup.tokenList; await tokenList.start(); @@ -305,6 +294,9 @@ describe('TokenDetectionController', () => { onPreferencesStateChange: (listener) => preferences.subscribe(listener), getBalancesInSingleCall: getBalancesInSingleCall as unknown as AssetsContractController['getBalancesInSingleCall'], + addDetectedTokens: + tokensController.addDetectedTokens.bind(tokensController), + getTokensState: () => tokensController.state, getPreferencesState: () => preferences.state, messenger: buildTokenDetectionControllerMessenger(controllerMessenger), }); @@ -508,14 +500,6 @@ describe('TokenDetectionController', () => { destroy: jest.fn(), }), ); - controllerMessenger.registerActionHandler( - `TokensController:getState`, - () => tokensController.state, - ); - controllerMessenger.registerActionHandler( - `TokensController:addDetectedTokens`, - tokensController.addDetectedTokens.bind(tokensController), - ); const tokenListSetup = setupTokenListController(controllerMessenger); tokenList = tokenListSetup.tokenList; @@ -533,6 +517,9 @@ describe('TokenDetectionController', () => { selectedAddress: '0x1', onPreferencesStateChange: stub, getBalancesInSingleCall: getBalancesInSingleCallMock, + addDetectedTokens: + tokensController.addDetectedTokens.bind(tokensController), + getTokensState: () => tokensController.state, getPreferencesState: () => preferences.state, messenger: buildTokenDetectionControllerMessenger(controllerMessenger), }); @@ -555,6 +542,9 @@ describe('TokenDetectionController', () => { networkClientId: NetworkType.mainnet, onPreferencesStateChange: stub, getBalancesInSingleCall: getBalancesInSingleCallMock, + addDetectedTokens: + tokensController.addDetectedTokens.bind(tokensController), + getTokensState: () => tokensController.state, getPreferencesState: () => preferences.state, messenger: buildTokenDetectionControllerMessenger(controllerMessenger), }); @@ -576,6 +566,9 @@ describe('TokenDetectionController', () => { selectedAddress: '0x1', onPreferencesStateChange, getBalancesInSingleCall: getBalancesInSingleCallMock, + addDetectedTokens: + tokensController.addDetectedTokens.bind(tokensController), + getTokensState: () => tokensController.state, getPreferencesState: () => preferences.state, messenger: buildTokenDetectionControllerMessenger(controllerMessenger), }); @@ -594,6 +587,9 @@ describe('TokenDetectionController', () => { selectedAddress: '0x1', onPreferencesStateChange: stub, getBalancesInSingleCall: getBalancesInSingleCallMock, + addDetectedTokens: + tokensController.addDetectedTokens.bind(tokensController), + getTokensState: () => tokensController.state, getPreferencesState: () => preferences.state, messenger: buildTokenDetectionControllerMessenger(controllerMessenger), }); diff --git a/packages/assets-controllers/src/TokenDetectionController.ts b/packages/assets-controllers/src/TokenDetectionController.ts index a9a40556f0..2bc662032e 100644 --- a/packages/assets-controllers/src/TokenDetectionController.ts +++ b/packages/assets-controllers/src/TokenDetectionController.ts @@ -24,10 +24,7 @@ import type { TokenListStateChange, } from './TokenListController'; import type { Token } from './TokenRatesController'; -import type { - TokensControllerAddDetectedTokensAction, - TokensControllerGetStateAction, -} from './TokensController'; +import type { TokensController, TokensState } from './TokensController'; const DEFAULT_INTERVAL = 180000; @@ -45,8 +42,6 @@ export type TokenDetectionControllerActions = export type AllowedActions = | NetworkControllerGetNetworkClientByIdAction - | TokensControllerGetStateAction - | TokensControllerAddDetectedTokensAction | GetTokenListState; export type TokenDetectionControllerStateChangeEvent = @@ -97,8 +92,12 @@ export class TokenDetectionController extends StaticIntervalPollingController< #isDetectionEnabledForNetwork: boolean; + readonly #addDetectedTokens: TokensController['addDetectedTokens']; + readonly #getBalancesInSingleCall: AssetsContractController['getBalancesInSingleCall']; + readonly #getTokensState: () => TokensState; + /** * Creates a TokenDetectionController instance. * @@ -109,7 +108,9 @@ export class TokenDetectionController extends StaticIntervalPollingController< * @param options.networkClientId - The selected network client ID of the current network * @param options.selectedAddress - Vault selected address * @param options.onPreferencesStateChange - Allows subscribing to preferences controller state changes. + * @param options.addDetectedTokens - Add a list of detected tokens. * @param options.getBalancesInSingleCall - Gets the balances of a list of tokens for the given address. + * @param options.getTokensState - Gets the current state of the Tokens controller. * @param options.getPreferencesState - Gets the state of the preferences controller. */ constructor({ @@ -119,7 +120,9 @@ export class TokenDetectionController extends StaticIntervalPollingController< disabled = false, onPreferencesStateChange, getBalancesInSingleCall, + addDetectedTokens, getPreferencesState, + getTokensState, messenger, }: { networkClientId: NetworkClientId; @@ -129,7 +132,9 @@ export class TokenDetectionController extends StaticIntervalPollingController< onPreferencesStateChange: ( listener: (preferencesState: PreferencesState) => void, ) => void; + addDetectedTokens: TokensController['addDetectedTokens']; getBalancesInSingleCall: AssetsContractController['getBalancesInSingleCall']; + getTokensState: () => TokensState; getPreferencesState: () => PreferencesState; messenger: TokenDetectionControllerMessenger; }) { @@ -155,7 +160,9 @@ export class TokenDetectionController extends StaticIntervalPollingController< this.#chainId, ); + this.#addDetectedTokens = addDetectedTokens; this.#getBalancesInSingleCall = getBalancesInSingleCall; + this.#getTokensState = getTokensState; this.messagingSystem.subscribe( 'TokenListController:stateChange', @@ -287,9 +294,10 @@ export class TokenDetectionController extends StaticIntervalPollingController< ) { return; } - const { tokens } = this.messagingSystem.call('TokensController:getState'); - const selectedAddress = accountAddress ?? this.#selectedAddress; + const { tokens } = this.#getTokensState(); + const selectedAddress = accountAddress || this.#selectedAddress; const chainId = this.#getCorrectChainId(networkClientId); + const tokensAddresses = tokens.map( /* istanbul ignore next*/ (token) => token.address.toLowerCase(), ); @@ -328,9 +336,7 @@ export class TokenDetectionController extends StaticIntervalPollingController< for (const tokenAddress of Object.keys(balances)) { let ignored; /* istanbul ignore else */ - const { ignoredTokens } = this.messagingSystem.call( - 'TokensController:getState', - ); + const { ignoredTokens } = this.#getTokensState(); if (ignoredTokens.length) { ignored = ignoredTokens.find( (ignoredTokenAddress) => @@ -358,14 +364,10 @@ export class TokenDetectionController extends StaticIntervalPollingController< } if (tokensToAdd.length) { - this.messagingSystem.call( - 'TokensController:addDetectedTokens', - tokensToAdd, - { - selectedAddress, - chainId, - }, - ); + await this.#addDetectedTokens(tokensToAdd, { + selectedAddress, + chainId, + }); } }); } From 541f8f448a9a14c12e19ddfc320626d420c66bea Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 21 Dec 2023 02:14:06 -0800 Subject: [PATCH 75/82] Adjust jest coverage threshold --- packages/assets-controllers/jest.config.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/assets-controllers/jest.config.js b/packages/assets-controllers/jest.config.js index 1ef36f44ca..cabdc33a86 100644 --- a/packages/assets-controllers/jest.config.js +++ b/packages/assets-controllers/jest.config.js @@ -17,7 +17,7 @@ module.exports = merge(baseConfig, { // An object that configures minimum threshold enforcement for coverage results coverageThreshold: { global: { - branches: 88.8, + branches: 88.51, functions: 97.08, lines: 97.23, statements: 97.28, From 798bb0905a1cea3e03f9fc5fcb59a0d41024bc52 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 21 Dec 2023 09:51:16 -0800 Subject: [PATCH 76/82] Use `getNetworkConfigurationByNetworkClientId` to replace `getNetworkClientById` --- packages/assets-controllers/jest.config.js | 2 +- .../src/TokenDetectionController.test.ts | 74 +++++-------------- .../src/TokenDetectionController.ts | 17 ++--- 3 files changed, 28 insertions(+), 65 deletions(-) diff --git a/packages/assets-controllers/jest.config.js b/packages/assets-controllers/jest.config.js index cabdc33a86..8b25905d86 100644 --- a/packages/assets-controllers/jest.config.js +++ b/packages/assets-controllers/jest.config.js @@ -17,7 +17,7 @@ module.exports = merge(baseConfig, { // An object that configures minimum threshold enforcement for coverage results coverageThreshold: { global: { - branches: 88.51, + branches: 88.4, functions: 97.08, lines: 97.23, statements: 97.28, diff --git a/packages/assets-controllers/src/TokenDetectionController.test.ts b/packages/assets-controllers/src/TokenDetectionController.test.ts index 1f534b91aa..b2e6e73f63 100644 --- a/packages/assets-controllers/src/TokenDetectionController.test.ts +++ b/packages/assets-controllers/src/TokenDetectionController.test.ts @@ -7,10 +7,7 @@ import { convertHexToDecimal, toHex, } from '@metamask/controller-utils'; -import { - defaultState as defaultNetworkState, - NetworkClientType, -} from '@metamask/network-controller'; +import { defaultState as defaultNetworkState } from '@metamask/network-controller'; import type { NetworkState, ProviderConfig, @@ -148,7 +145,7 @@ function buildTokenDetectionControllerMessenger( return controllerMessenger.getRestricted({ name: controllerName, allowedActions: [ - 'NetworkController:getNetworkClientById', + 'NetworkController:getNetworkConfigurationByNetworkClientId', 'TokenListController:getState', ], allowedEvents: [ @@ -171,7 +168,7 @@ describe('TokenDetectionController', () => { >; const onNetworkDidChangeListeners: ((state: NetworkState) => void)[] = []; - const getNetworkClientByIdHandler = jest.fn(); + const getNetworkConfigurationByNetworkClientIdHandler = jest.fn(); const changeNetwork = (providerConfig: ProviderConfig) => { onNetworkDidChangeListeners.forEach((listener) => { listener({ @@ -186,21 +183,16 @@ describe('TokenDetectionController', () => { selectedNetworkClientId: providerConfig.type, }); - getNetworkClientByIdHandler.mockReturnValue({ - configuration: { - chainId: providerConfig.chainId, - }, - provider: {}, - blockTracker: {}, - destroy: jest.fn(), + getNetworkConfigurationByNetworkClientIdHandler.mockReturnValue({ + chainId: providerConfig.chainId, }); controllerMessenger.unregisterActionHandler( - 'NetworkController:getNetworkClientById', + 'NetworkController:getNetworkConfigurationByNetworkClientId', ); controllerMessenger.registerActionHandler( - 'NetworkController:getNetworkClientById', - getNetworkClientByIdHandler, + 'NetworkController:getNetworkConfigurationByNetworkClientId', + getNetworkConfigurationByNetworkClientIdHandler, ); }; @@ -210,18 +202,6 @@ describe('TokenDetectionController', () => { type: NetworkType.goerli, ticker: NetworksTicker.goerli, }; - const polygon = { - chainId: SupportedTokenDetectionNetworks.polygon, - id: 'polygon', - type: NetworkType.rpc, - ticker: 'MATIC', - }; - const mockPolygonClient = { - configuration: { - ...polygon, - type: NetworkClientType.Custom, - }, - }; const mainnet = { chainId: ChainId.mainnet, id: 'mainnet', @@ -261,14 +241,9 @@ describe('TokenDetectionController', () => { selectedNetworkClientId: NetworkType.mainnet, }); controllerMessenger.registerActionHandler( - `NetworkController:getNetworkClientById`, - getNetworkClientByIdHandler.mockReturnValue({ - configuration: { - chainId: ChainId.mainnet, - }, - provider: {}, - blockTracker: {}, - destroy: jest.fn(), + `NetworkController:getNetworkConfigurationByNetworkClientId`, + getNetworkConfigurationByNetworkClientIdHandler.mockReturnValue({ + chainId: ChainId.mainnet, }), ); @@ -490,14 +465,9 @@ describe('TokenDetectionController', () => { selectedNetworkClientId: NetworkType.mainnet, }); controllerMessenger.registerActionHandler( - `NetworkController:getNetworkClientById`, - getNetworkClientByIdHandler.mockReturnValue({ - configuration: { - chainId: ChainId.mainnet, - }, - provider: {}, - blockTracker: {}, - destroy: jest.fn(), + `NetworkController:getNetworkConfigurationByNetworkClientId`, + getNetworkConfigurationByNetworkClientIdHandler.mockReturnValue({ + chainId: ChainId.mainnet, }), ); @@ -593,21 +563,15 @@ describe('TokenDetectionController', () => { getPreferencesState: () => preferences.state, messenger: buildTokenDetectionControllerMessenger(controllerMessenger), }); - getNetworkClientByIdHandler.mockReturnValue({ - configuration: { - chainId: SupportedTokenDetectionNetworks.polygon, - }, - provider: {}, - blockTracker: {}, - destroy: jest.fn(), - }); controllerMessenger.unregisterActionHandler( - 'NetworkController:getNetworkClientById', + 'NetworkController:getNetworkConfigurationByNetworkClientId', ); controllerMessenger.registerActionHandler( - 'NetworkController:getNetworkClientById', - getNetworkClientByIdHandler.mockReturnValue(mockPolygonClient), + 'NetworkController:getNetworkConfigurationByNetworkClientId', + getNetworkConfigurationByNetworkClientIdHandler.mockReturnValue({ + chainId: SupportedTokenDetectionNetworks.polygon, + }), ); changeNetwork(mainnet); diff --git a/packages/assets-controllers/src/TokenDetectionController.ts b/packages/assets-controllers/src/TokenDetectionController.ts index 2bc662032e..48284db5c3 100644 --- a/packages/assets-controllers/src/TokenDetectionController.ts +++ b/packages/assets-controllers/src/TokenDetectionController.ts @@ -11,7 +11,7 @@ import type { NetworkClientId, NetworkControllerNetworkDidChangeEvent, NetworkControllerStateChangeEvent, - NetworkControllerGetNetworkClientByIdAction, + NetworkControllerGetNetworkConfigurationByNetworkClientId, } from '@metamask/network-controller'; import { StaticIntervalPollingController } from '@metamask/polling-controller'; import type { PreferencesState } from '@metamask/preferences-controller'; @@ -41,7 +41,7 @@ export type TokenDetectionControllerActions = TokenDetectionControllerGetStateAction; export type AllowedActions = - | NetworkControllerGetNetworkClientByIdAction + | NetworkControllerGetNetworkConfigurationByNetworkClientId | GetTokenListState; export type TokenDetectionControllerStateChangeEvent = @@ -249,13 +249,12 @@ export class TokenDetectionController extends StaticIntervalPollingController< } #getCorrectChainId(networkClientId?: NetworkClientId) { - const { - configuration: { chainId }, - } = this.messagingSystem.call( - 'NetworkController:getNetworkClientById', - networkClientId ?? this.#networkClientId, - ); - return chainId; + const { chainId } = + this.messagingSystem.call( + 'NetworkController:getNetworkConfigurationByNetworkClientId', + networkClientId ?? this.#networkClientId, + ) ?? {}; + return chainId ?? this.#chainId; } async _executePoll( From 1552041bfd080a83b7cd95f86d38be074c9b6235 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 21 Dec 2023 09:54:09 -0800 Subject: [PATCH 77/82] Remove throw error in `_executePoll` --- packages/assets-controllers/src/TokenDetectionController.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/assets-controllers/src/TokenDetectionController.ts b/packages/assets-controllers/src/TokenDetectionController.ts index 48284db5c3..e17f9b7fb2 100644 --- a/packages/assets-controllers/src/TokenDetectionController.ts +++ b/packages/assets-controllers/src/TokenDetectionController.ts @@ -262,9 +262,7 @@ export class TokenDetectionController extends StaticIntervalPollingController< options: { address: string }, ): Promise { if (this.#disabled) { - throw new Error( - 'Poll cannot be executed while network requests are disabled', - ); + return; } await this.detectTokens({ networkClientId, From 6d020bcc469e9cc407cd54fc619963c49775b301 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 21 Dec 2023 09:59:47 -0800 Subject: [PATCH 78/82] Adjust jest coverage threshold --- packages/assets-controllers/jest.config.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/assets-controllers/jest.config.js b/packages/assets-controllers/jest.config.js index 8b25905d86..6d35a70f3a 100644 --- a/packages/assets-controllers/jest.config.js +++ b/packages/assets-controllers/jest.config.js @@ -17,7 +17,7 @@ module.exports = merge(baseConfig, { // An object that configures minimum threshold enforcement for coverage results coverageThreshold: { global: { - branches: 88.4, + branches: 88.36, functions: 97.08, lines: 97.23, statements: 97.28, From 05cf2bd3089d2b58b33bcb21ae3bed6f909c4f6b Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 21 Dec 2023 10:34:10 -0800 Subject: [PATCH 79/82] Set `disabled` to true by default and add `enable`, `disable` methods --- packages/assets-controllers/CHANGELOG.md | 4 ++-- .../src/TokenDetectionController.ts | 20 ++++++++++++++++--- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/packages/assets-controllers/CHANGELOG.md b/packages/assets-controllers/CHANGELOG.md index a7cb1a9b91..5580dfa631 100644 --- a/packages/assets-controllers/CHANGELOG.md +++ b/packages/assets-controllers/CHANGELOG.md @@ -8,11 +8,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - `TokenListController` now exports a `TokenListControllerMessenger` type ([#3609](https://github.com/MetaMask/core/pull/3609)). - `TokenDetectionController` exports types `TokenDetectionControllerMessenger`, `TokenDetectionControllerActions`, `TokenDetectionControllerGetStateAction`, `TokenDetectionControllerEvents`, `TokenDetectionControllerStateChangeEvent` ([#3609](https://github.com/MetaMask/core/pull/3609)). +- Add `enable` and `disable` methods to `TokenDetectionController`, which control whether the controller is able to make polling requests or all of its network calls are blocked ([#3609](https://github.com/MetaMask/core/pull/3609)). ### Changed - **BREAKING:** `TokenDetectionController` is upgraded to extend `BaseControllerV2` and `StaticIntervalPollingController` ([#3609](https://github.com/MetaMask/core/pull/3609)). - - The constructor now expects an options object as its only argument, with required properties `messenger`, `networkClientId`, required callbacks `onPreferencesStateChange`, `getBalancesInSingleCall`, `getPreferencesState`, and optional properties `interval`, `selectedAddress`. Note that the `config` object is no longer used by the constructor. - - Polling can only be initiated by calling `start` or `startPollingByNetworkClientId` from a controller client. There is no longer an option to automatically start polling when the controller is instantiated. + - The constructor now expects an options object as its only argument, with required properties `messenger`, `networkClientId`, required callbacks `onPreferencesStateChange`, `getBalancesInSingleCall`, `addDetectedTokens`, `getTokenState`, `getPreferencesState`, and optional properties `disabled`, `interval`, `selectedAddress`. Note that the `config` object is no longer used by the constructor. ## [22.0.0] ### Changed diff --git a/packages/assets-controllers/src/TokenDetectionController.ts b/packages/assets-controllers/src/TokenDetectionController.ts index e17f9b7fb2..19c4d90871 100644 --- a/packages/assets-controllers/src/TokenDetectionController.ts +++ b/packages/assets-controllers/src/TokenDetectionController.ts @@ -117,7 +117,7 @@ export class TokenDetectionController extends StaticIntervalPollingController< networkClientId, selectedAddress = '', interval = DEFAULT_INTERVAL, - disabled = false, + disabled = true, onPreferencesStateChange, getBalancesInSingleCall, addDetectedTokens, @@ -212,11 +212,25 @@ export class TokenDetectionController extends StaticIntervalPollingController< ); } + /** + * Allows controller to make active and passive polling requests + */ + enable() { + this.#disabled = false; + } + + /** + * Blocks controller from making network calls + */ + disable() { + this.#disabled = true; + } + /** * Start polling for detected tokens. */ async start() { - this.#disabled = false; + this.enable(); await this.#startPolling(); } @@ -224,7 +238,7 @@ export class TokenDetectionController extends StaticIntervalPollingController< * Stop polling for detected tokens. */ stop() { - this.#disabled = true; + this.disable(); this.#stopPolling(); } From d8ced017fee34fd583bf2f9c4a3d9a4daf0e2ded Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 21 Dec 2023 10:34:17 -0800 Subject: [PATCH 80/82] Revert "test: remove unnecessary `.start()` calls" This reverts commit 73eccbf25f42f340cbac0bb7892a3d32fce8b7aa. --- .../src/TokenDetectionController.test.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/assets-controllers/src/TokenDetectionController.test.ts b/packages/assets-controllers/src/TokenDetectionController.test.ts index b2e6e73f63..55e27e8e7b 100644 --- a/packages/assets-controllers/src/TokenDetectionController.test.ts +++ b/packages/assets-controllers/src/TokenDetectionController.test.ts @@ -360,8 +360,11 @@ describe('TokenDetectionController', () => { it('should not add ignoredTokens to the tokens list if detected with balance', async () => { preferences.setSelectedAddress('0x0001'); + changeNetwork(mainnet); + await tokenDetection.start(); + await tokensController.addToken({ address: sampleTokenA.address, symbol: sampleTokenA.symbol, @@ -392,6 +395,8 @@ describe('TokenDetectionController', () => { preferences.setSelectedAddress('0x0001'); changeNetwork(mainnet); + await tokenDetection.start(); + await tokensController.addToken({ address: sampleTokenA.address, symbol: sampleTokenA.symbol, @@ -413,6 +418,8 @@ describe('TokenDetectionController', () => { preferences.update({ selectedAddress: '0x1' }); changeNetwork(mainnet); + await tokenDetection.start(); + getBalancesInSingleCall.resolves({ [sampleTokenA.address]: new BN(1), }); @@ -426,6 +433,7 @@ describe('TokenDetectionController', () => { }); it('should not detect tokens if there is no selectedAddress set', async () => { + await tokenDetection.start(); getBalancesInSingleCall.resolves({ [sampleTokenA.address]: new BN(1), }); From 16e8badee8bb4d43e0d6935c87b250277529d004 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 21 Dec 2023 10:36:48 -0800 Subject: [PATCH 81/82] test: Adjust tests to `disabled` being true by default --- .../assets-controllers/src/TokenDetectionController.test.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/assets-controllers/src/TokenDetectionController.test.ts b/packages/assets-controllers/src/TokenDetectionController.test.ts index 55e27e8e7b..0bdb82b31e 100644 --- a/packages/assets-controllers/src/TokenDetectionController.test.ts +++ b/packages/assets-controllers/src/TokenDetectionController.test.ts @@ -540,6 +540,7 @@ describe('TokenDetectionController', () => { }); tokenDetection = new TokenDetectionController({ + disabled: false, networkClientId: NetworkType.mainnet, selectedAddress: '0x1', onPreferencesStateChange, @@ -561,6 +562,7 @@ describe('TokenDetectionController', () => { it('should be called if network is changed to a chainId that supports token detection', async () => { tokenDetection = new TokenDetectionController({ + disabled: false, networkClientId: 'polygon', selectedAddress: '0x1', onPreferencesStateChange: stub, @@ -603,6 +605,7 @@ describe('TokenDetectionController', () => { .mockImplementation(() => { return Promise.resolve(); }); + tokenDetection.enable(); tokenDetection.startPollingByNetworkClientId('mainnet', { address: '0x1', }); @@ -639,6 +642,7 @@ describe('TokenDetectionController', () => { getBalancesInSingleCall.resolves({ [sampleTokenA.address]: new BN(1), }); + tokenDetection.enable(); await tokenDetection.detectTokens({ networkClientId: NetworkType.mainnet, accountAddress: selectedAddress, From f67beadc3bef55428f9c3646ffc32548c4152a03 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 21 Dec 2023 10:38:50 -0800 Subject: [PATCH 82/82] Update changelog --- packages/assets-controllers/CHANGELOG.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/assets-controllers/CHANGELOG.md b/packages/assets-controllers/CHANGELOG.md index 5580dfa631..2c875368ce 100644 --- a/packages/assets-controllers/CHANGELOG.md +++ b/packages/assets-controllers/CHANGELOG.md @@ -8,11 +8,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - `TokenListController` now exports a `TokenListControllerMessenger` type ([#3609](https://github.com/MetaMask/core/pull/3609)). - `TokenDetectionController` exports types `TokenDetectionControllerMessenger`, `TokenDetectionControllerActions`, `TokenDetectionControllerGetStateAction`, `TokenDetectionControllerEvents`, `TokenDetectionControllerStateChangeEvent` ([#3609](https://github.com/MetaMask/core/pull/3609)). -- Add `enable` and `disable` methods to `TokenDetectionController`, which control whether the controller is able to make polling requests or all of its network calls are blocked ([#3609](https://github.com/MetaMask/core/pull/3609)). +- Add `enable` and `disable` methods to `TokenDetectionController`, which control whether the controller is able to make polling requests or all of its network calls are blocked. ([#3609](https://github.com/MetaMask/core/pull/3609)). + - Note that if the controller is initiated without the `disabled` constructor option set to `false`, the `enable` method will need to be called before the controller can make polling requests in response to subscribed events. ### Changed - **BREAKING:** `TokenDetectionController` is upgraded to extend `BaseControllerV2` and `StaticIntervalPollingController` ([#3609](https://github.com/MetaMask/core/pull/3609)). - - The constructor now expects an options object as its only argument, with required properties `messenger`, `networkClientId`, required callbacks `onPreferencesStateChange`, `getBalancesInSingleCall`, `addDetectedTokens`, `getTokenState`, `getPreferencesState`, and optional properties `disabled`, `interval`, `selectedAddress`. Note that the `config` object is no longer used by the constructor. + - The constructor now expects an options object as its only argument, with required properties `messenger`, `networkClientId`, required callbacks `onPreferencesStateChange`, `getBalancesInSingleCall`, `addDetectedTokens`, `getTokenState`, `getPreferencesState`, and optional properties `disabled`, `interval`, `selectedAddress`. ## [22.0.0] ### Changed