feat(assets-controllers): Drop support for updating arbitary token rates #3639
Conversation
Updating and polling for token rates for arbitrary tokens is no longer supported. Instead the user's token list (from the `TokensController`) always dictates which tokens will be tracked by the `TokenRatesController`. See #3637 for more details.� This simplifies the API, dramatically simplifies the complexity of testing this controller thoroughly, and it resolves a recently introduced bug (#3638). Rates from non-tracked tokens can still be retrieved by calling the price service directly, using the `token-prices-service` module. Resolves #3637 Fixes #3638
| @@ -127,7 +128,12 @@ describe('TokenRatesController', () => { | |||
| allTokens: { | |||
| '0x1': { | |||
| [defaultSelectedAddress]: [ | |||
| { address: 'bar', decimals: 0, symbol: '', aggregators: [] }, | |||
| { | |||
| address: mockTokenAddress, | |||
There was a problem hiding this comment.
I had to update a bunch of test fixtures to use hex addresses rather than non-hex strings, as it threw an error when they were passed to toHex
| * @param chainId - The chain ID. | ||
| * @returns The list of tokens addresses for the current chain | ||
| */ | ||
| #getTokenAddresses(chainId: Hex): Hex[] { |
There was a problem hiding this comment.
I decided to drop the token list in favor of this function. This gives us exactly what we need (including the token => address mapping), doesn't require careful updating to keep in-sync, and doesn't take up additional memory (which might have been a concern if we tracked lists for all chains).
cc @mcmire , wanted to make sure you see this because for the "prevent redundant updates" issue, the solution made in the mobile patch used the updateTokenList function to determine whether the tokens changed. I think we can still do something similar with this pattern though, by calling this function before and after to see if it changed.
| this.tokenList = [...tokens, ...detectedTokens]; | ||
| allDetectedTokens[chainId]?.[this.config.selectedAddress] || []; | ||
|
|
||
| return [...tokens, ...detectedTokens].map((token) => toHex(token.address)); |
There was a problem hiding this comment.
Here is the toHex call that caused unit test issues
| const { chainId, nativeCurrency } = this.config; | ||
| const tokenContractAddresses = this.tokenList.map( | ||
| (token) => token.address, | ||
| ) as Hex[]; |
There was a problem hiding this comment.
Here is the as Hex call that was the reason we didn't need toHex prior to now
Explanation
Updating and polling for token rates for arbitrary tokens is no longer supported. Instead the user's token list (from the
TokensController) always dictates which tokens will be tracked by theTokenRatesController. See #3637 for more details.This simplifies the API, dramatically simplifies the complexity of testing this controller thoroughly, and it resolves a recently introduced bug (#3638).
Rates from non-tracked tokens can still be retrieved by calling the price service directly, using the
token-prices-servicemodule.References
Resolves #3637
Fixes #3638
Changelog
@metamask/assets-controllerstokenAddressesoption has been removed fromstartPollingByNetworkClientIdtokenContractAddressesoption has been removed fromupdateExchangeRatesByChainIdChecklist