feat: Integrate new TokenListController polling pattern [Core]
#4878
Conversation
gambinish
changed the title
…te tokenList only when chainId is selected in constructor
… base class, ensure tokenList only gets updated when on the selected chain
gambinish
changed the title
TokenListController polling pattern [Core]
|
@metamaskbot publish-preview |
|
Preview builds have been published. See these instructions for more information about preview builds. Expand for full list of packages and versions. |
| @@ -204,104 +204,110 @@ export class TokenListController extends StaticIntervalPollingController<TokenLi | |||
| tokenList: this.state.tokensChainsCache[this.chainId]?.data || {}, | |||
| }; | |||
| }); | |||
There was a problem hiding this comment.
We discussed it's probably a good idea to remove entries when their network is deleted. But I don't think it's necessary to do that now.
Untested example:
const chainIdsToRemove = [] as Hex[];
for (const chainId in Object.keys(this.state.tokensChainsCache)) {
if (!networkControllerState.networkConfigurationsByChainId[chainId as Hex]) {
chainIdsToRemove.push(chainId as Hex)
}
}
if (chainIdsToRemove.length > 0) {
this.update(() => {
return {
...this.state,
tokensChainsCache: chainIdsToRemove.reduce((acc, chainId) => {
delete acc[chainId]
return acc
}, this.state.tokensChainsCache)
}
})
}
| */ | ||
| async fetchTokenList(networkClientId?: NetworkClientId): Promise<void> { | ||
| async fetchTokenList(chainId: Hex): Promise<void> { |
There was a problem hiding this comment.
What are your thoughts on continuing to take a network client ID until we remove the global network completely? My thought is that clients might find it easier to pass this instead of a chain ID. The selected network client ID can be easily obtained from NetworkController state as selectedNetworkClientId and accessed within a Redux selector, while the client must obtain the chain ID by either doing the same thing that's happening here (use the messenger to get the network client by its ID, then get the chain ID that corresponds to it), or if within a Redux selector, loop through the networkConfigurationsByChainId to find the network configuration that matches the selectedNetworkClientId and then get the chainId off of that.
Ultimately I guess it depends on what the clients are currently doing. If we already have a way of accessing the current chain ID in a more or les type-safe way, then no worries. So, maybe this is change is just fine, but I wanted to double-check?
There was a problem hiding this comment.
I think that the move towards passing in the chain ID, rather than an networkClientId was an intentional one. You're right, it's one of the steps towards removing the global network selector entirely. Yes, the clients are currently being refactored to now execute a single poll per ChainId.
I'll defer to Brian here for more on these design decisions, but the pattern with the other AssetsControllers are also moving towards this same way of passing chainID instead of networkClientId.
There was a problem hiding this comment.
We're going to fetch token lists across all chains rather than just the current chain, so from the perspective of token lists we already no longer care about the current network. It'll just do Object.keys(networkConfigurationsByChainId) and fetch for each chain id. So the chain id input is already more convenient.
There was a problem hiding this comment.
Oh okay! Sounds good then. Thanks for the reply.
Explanation
TokenListController is currently responsible for maintaining a list of all tokens per chain. This dataset is accessible via
tokenListstate variable in metamask state. After this task is complete, this token list will migrate it's polling pattern to leverage the base class, to execute a single poll per chain, rather than on an interval for all chainsMove
TokenListControlleraway from being scoped to a single polling loop, to executing individual polling loops per chain, allowing it to be more UI based in its controls.The controller will accept
PollingInputsfrom the extension from various UI elements, and will be responsible for starting, stopping, and deduping polls as needed. Deduping is baked into the inherited base classStaticIntervalPollingControllerHere is the corresponding PR in extension that consumes and integrates with this controller. There will also be an additional mobile PR at some point to implement something similar.
References
https://github.com/MetaMask/MetaMask-planning/issues/3429
Changelog
@metamask/package-a@metamask/package-bChecklist