Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Upgrade assets controllers to 43 with multichain polling for token lists + detection #28447

Merged
merged 27 commits into from
Nov 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
diff --git a/dist/assetsUtil.cjs b/dist/assetsUtil.cjs
index e90a1b6767bc8ac54b7a4d580035cf5db6861dca..a5e0f03d2541b4e3540431ef2e6e4b60fb7ae9fe 100644
index c2e83cf6caee19152aa164f1333cfef7b681e900..590b6de6e9d20ca402b82ac56b0929ab8c16c932 100644
--- a/dist/assetsUtil.cjs
+++ b/dist/assetsUtil.cjs
@@ -3,6 +3,7 @@ var __importDefault = (this && this.__importDefault) || function (mod) {
return (mod && mod.__esModule) ? mod : { "default": mod };
};
Object.defineProperty(exports, "__esModule", { value: true });
+function _interopRequireWildcard(obj) { if (obj && obj.__esModule) { return obj; } else { var newObj = {}; if (obj != null) { for (var key in obj) { if (Object.prototype.hasOwnProperty.call(obj, key)) { newObj[key] = obj[key]; } } } newObj.default = obj; return newObj; } }
exports.fetchTokenContractExchangeRates = exports.reduceInBatchesSerially = exports.divideIntoBatches = exports.ethersBigNumberToBN = exports.addUrlProtocolPrefix = exports.getFormattedIpfsUrl = exports.getIpfsCIDv1AndPath = exports.removeIpfsProtocolPrefix = exports.isTokenListSupportedForNetwork = exports.isTokenDetectionSupportedForNetwork = exports.SupportedTokenDetectionNetworks = exports.formatIconUrlWithProxy = exports.formatAggregatorNames = exports.hasNewCollectionFields = exports.compareNftMetadata = exports.TOKEN_PRICES_BATCH_SIZE = void 0;
exports.fetchTokenContractExchangeRates = exports.reduceInBatchesSerially = exports.divideIntoBatches = exports.ethersBigNumberToBN = exports.addUrlProtocolPrefix = exports.getFormattedIpfsUrl = exports.getIpfsCIDv1AndPath = exports.removeIpfsProtocolPrefix = exports.isTokenListSupportedForNetwork = exports.isTokenDetectionSupportedForNetwork = exports.SupportedStakedBalanceNetworks = exports.SupportedTokenDetectionNetworks = exports.formatIconUrlWithProxy = exports.formatAggregatorNames = exports.hasNewCollectionFields = exports.compareNftMetadata = exports.TOKEN_PRICES_BATCH_SIZE = void 0;
const controller_utils_1 = require("@metamask/controller-utils");
const utils_1 = require("@metamask/utils");
@@ -221,7 +222,7 @@ async function getIpfsCIDv1AndPath(ipfsUrl) {
@@ -233,7 +234,7 @@ async function getIpfsCIDv1AndPath(ipfsUrl) {
const index = url.indexOf('/');
const cid = index !== -1 ? url.substring(0, index) : url;
const path = index !== -1 ? url.substring(index) : undefined;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
diff --git a/dist/TokenDetectionController.cjs b/dist/TokenDetectionController.cjs
index ab23c95d667357db365f925c4c4acce4736797f8..8fd5efde7a3c24080f8a43f79d10300e8c271245 100644
--- a/dist/TokenDetectionController.cjs
+++ b/dist/TokenDetectionController.cjs
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -204,13 +204,10 @@ class TokenDetectionController extends (0, polling_controller_1.StaticIntervalPo
// Try detecting tokens via Account API first if conditions allow
if (supportedNetworks && chainsToDetectUsingAccountAPI.length > 0) {
const apiResult = await __classPrivateFieldGet(this, _TokenDetectionController_instances, "m", _TokenDetectionController_attemptAccountAPIDetection).call(this, chainsToDetectUsingAccountAPI, addressToDetect, supportedNetworks);
- // If API succeeds and no chains are left for RPC detection, we can return early
- if (apiResult?.result === 'success' &&
- chainsToDetectUsingRpc.length === 0) {
- return;
+ // If the account API call failed, have those chains fall back to RPC detection
+ if (apiResult?.result === 'failed') {
+ __classPrivateFieldGet(this, _TokenDetectionController_instances, "m", _TokenDetectionController_addChainsToRpcDetection).call(this, chainsToDetectUsingRpc, chainsToDetectUsingAccountAPI, clientNetworks);
}
- // If API fails or chainsToDetectUsingRpc still has items, add chains to RPC detection
- __classPrivateFieldGet(this, _TokenDetectionController_instances, "m", _TokenDetectionController_addChainsToRpcDetection).call(this, chainsToDetectUsingRpc, chainsToDetectUsingAccountAPI, clientNetworks);
}
// Proceed with RPC detection if there are chains remaining in chainsToDetectUsingRpc
if (chainsToDetectUsingRpc.length > 0) {
@@ -446,8 +443,7 @@ async function _TokenDetectionController_addDetectedTokensViaAPI({ selectedAddre
const tokenBalancesByChain = await __classPrivateFieldGet(this, _TokenDetectionController_accountsAPI, "f")
.getMultiNetworksBalances(selectedAddress, chainIds, supportedNetworks)
.catch(() => null);
- if (!tokenBalancesByChain ||
- Object.keys(tokenBalancesByChain).length === 0) {
+ if (tokenBalancesByChain === null) {
return { result: 'failed' };
}
// Process each chain ID individually
diff --git a/dist/TokenDetectionController.mjs b/dist/TokenDetectionController.mjs
index f75eb5c2c74f2a9d15a79760985111171dc938e1..ebc30bb915cc39dabf49f9e0da84a7948ae1ed48 100644
--- a/dist/TokenDetectionController.mjs
+++ b/dist/TokenDetectionController.mjs
@@ -205,13 +205,10 @@ export class TokenDetectionController extends StaticIntervalPollingController()
// Try detecting tokens via Account API first if conditions allow
if (supportedNetworks && chainsToDetectUsingAccountAPI.length > 0) {
const apiResult = await __classPrivateFieldGet(this, _TokenDetectionController_instances, "m", _TokenDetectionController_attemptAccountAPIDetection).call(this, chainsToDetectUsingAccountAPI, addressToDetect, supportedNetworks);
- // If API succeeds and no chains are left for RPC detection, we can return early
- if (apiResult?.result === 'success' &&
- chainsToDetectUsingRpc.length === 0) {
- return;
+ // If the account API call failed, have those chains fall back to RPC detection
+ if (apiResult?.result === 'failed') {
+ __classPrivateFieldGet(this, _TokenDetectionController_instances, "m", _TokenDetectionController_addChainsToRpcDetection).call(this, chainsToDetectUsingRpc, chainsToDetectUsingAccountAPI, clientNetworks);
}
- // If API fails or chainsToDetectUsingRpc still has items, add chains to RPC detection
- __classPrivateFieldGet(this, _TokenDetectionController_instances, "m", _TokenDetectionController_addChainsToRpcDetection).call(this, chainsToDetectUsingRpc, chainsToDetectUsingAccountAPI, clientNetworks);
}
// Proceed with RPC detection if there are chains remaining in chainsToDetectUsingRpc
if (chainsToDetectUsingRpc.length > 0) {
@@ -446,8 +443,7 @@ async function _TokenDetectionController_addDetectedTokensViaAPI({ selectedAddre
const tokenBalancesByChain = await __classPrivateFieldGet(this, _TokenDetectionController_accountsAPI, "f")
.getMultiNetworksBalances(selectedAddress, chainIds, supportedNetworks)
.catch(() => null);
- if (!tokenBalancesByChain ||
- Object.keys(tokenBalancesByChain).length === 0) {
+ if (tokenBalancesByChain === null) {
return { result: 'failed' };
}
// Process each chain ID individually
38 changes: 17 additions & 21 deletions app/scripts/metamask-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -2606,24 +2606,12 @@ export default class MetamaskController extends EventEmitter {
this.accountTrackerController.start();
this.txController.startIncomingTransactionPolling();
this.tokenDetectionController.enable();

const preferencesControllerState = this.preferencesController.state;

if (this.#isTokenListPollingRequired(preferencesControllerState)) {
this.tokenListController.start();
}
}

stopNetworkRequests() {
this.accountTrackerController.stop();
this.txController.stopIncomingTransactionPolling();
this.tokenDetectionController.disable();

const preferencesControllerState = this.preferencesController.state;

if (this.#isTokenListPollingRequired(preferencesControllerState)) {
this.tokenListController.stop();
}
}

resetStates(resetMethods) {
Expand Down Expand Up @@ -3243,6 +3231,7 @@ export default class MetamaskController extends EventEmitter {
currencyRateController,
tokenDetectionController,
ensController,
tokenListController,
gasFeeController,
metaMetricsController,
networkController,
Expand Down Expand Up @@ -4045,6 +4034,19 @@ export default class MetamaskController extends EventEmitter {
tokenRatesController,
),

tokenDetectionStartPolling: tokenDetectionController.startPolling.bind(
tokenDetectionController,
),
tokenDetectionStopPollingByPollingToken:
tokenDetectionController.stopPollingByPollingToken.bind(
tokenDetectionController,
),

tokenListStartPolling:
tokenListController.startPolling.bind(tokenListController),
tokenListStopPollingByPollingToken:
tokenListController.stopPollingByPollingToken.bind(tokenListController),

// GasFeeController
gasFeeStartPollingByNetworkClientId:
gasFeeController.startPollingByNetworkClientId.bind(gasFeeController),
Expand Down Expand Up @@ -4408,6 +4410,7 @@ export default class MetamaskController extends EventEmitter {
if (balance === '0x0') {
// This account has no balance, so check for tokens
await this.tokenDetectionController.detectTokens({
chainIds: [chainId],
selectedAddress: address,
});

Expand Down Expand Up @@ -6676,6 +6679,8 @@ export default class MetamaskController extends EventEmitter {
this.gasFeeController.stopAllPolling();
this.currencyRateController.stopAllPolling();
this.tokenRatesController.stopAllPolling();
this.tokenDetectionController.stopAllPolling();
this.tokenListController.stopAllPolling();
this.appStateController.clearPollingTokens();
} catch (error) {
console.error(error);
Expand Down Expand Up @@ -7211,15 +7216,6 @@ export default class MetamaskController extends EventEmitter {
}

this.tokenListController.updatePreventPollingOnNetworkRestart(!newEnabled);
Copy link
Contributor Author

@bergeron bergeron Nov 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've never understood the purpose of preventPollingOnNetworkRestart, so I don't know whether it still makes sense. When it's set to true, all it seems to do is reset the state when switching chains. Keeping it around for now.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw this same thing and had similar thoughts. Sounds good


if (newEnabled) {
log.debug('Started token list controller polling');
this.tokenListController.start();
} else {
log.debug('Stopped token list controller polling');
this.tokenListController.clearingTokenListData();
this.tokenListController.stop();
}
}

#isTokenListPollingRequired(preferencesControllerState) {
Expand Down
109 changes: 0 additions & 109 deletions app/scripts/metamask-controller.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2301,115 +2301,6 @@ describe('MetaMaskController', () => {
});
});

describe('token list controller', () => {
it('stops polling if petnames, simulations, and token detection disabled', async () => {
expect(TokenListController.prototype.stop).not.toHaveBeenCalled();

expect(
TokenListController.prototype.clearingTokenListData,
).not.toHaveBeenCalled();

await simulatePreferencesChange({
useTransactionSimulations: false,
useTokenDetection: false,
preferences: {
petnamesEnabled: false,
},
});

expect(TokenListController.prototype.stop).toHaveBeenCalledTimes(1);

expect(
TokenListController.prototype.clearingTokenListData,
).toHaveBeenCalledTimes(1);
});

it.each([
[
'petnames',
{
preferences: { petnamesEnabled: false },
useTokenDetection: true,
useTransactionSimulations: true,
},
],
[
'simulations',
{
preferences: { petnamesEnabled: true },
useTokenDetection: true,
useTransactionSimulations: false,
},
],
[
'token detection',
{
preferences: { petnamesEnabled: true },
useTokenDetection: false,
useTransactionSimulations: true,
},
],
])(
'does not stop polling if only %s disabled',
async (_, preferences) => {
expect(TokenListController.prototype.stop).not.toHaveBeenCalled();

expect(
TokenListController.prototype.clearingTokenListData,
).not.toHaveBeenCalled();

await simulatePreferencesChange(preferences);

expect(TokenListController.prototype.stop).not.toHaveBeenCalled();

expect(
TokenListController.prototype.clearingTokenListData,
).not.toHaveBeenCalled();
},
);

it.each([
[
'petnames',
{
preferences: { petnamesEnabled: true },
useTokenDetection: false,
useTransactionSimulations: false,
},
],
[
'simulations',
{
preferences: { petnamesEnabled: false },
useTokenDetection: false,
useTransactionSimulations: true,
},
],
[
'token detection',
{
preferences: { petnamesEnabled: false },
useTokenDetection: true,
useTransactionSimulations: false,
},
],
])('starts polling if only %s enabled', async (_, preferences) => {
expect(TokenListController.prototype.start).not.toHaveBeenCalled();

await simulatePreferencesChange({
useTransactionSimulations: false,
useTokenDetection: false,
preferences: {
petnamesEnabled: false,
},
});

await simulatePreferencesChange(preferences);

expect(TokenListController.prototype.start).toHaveBeenCalledTimes(1);
});
});

describe('MultichainRatesController start/stop', () => {
const mockEvmAccount = createMockInternalAccount();
const mockNonEvmAccount = {
Expand Down
1 change: 1 addition & 0 deletions lavamoat/browserify/beta/policy.json
Original file line number Diff line number Diff line change
Expand Up @@ -686,6 +686,7 @@
"packages": {
"@ensdomains/content-hash>multicodec>uint8arrays>multiformats": true,
"@ethereumjs/tx>@ethereumjs/util": true,
"@ethersproject/bignumber": true,
"@ethersproject/contracts": true,
"@ethersproject/providers": true,
"@metamask/abi-utils": true,
Expand Down
1 change: 1 addition & 0 deletions lavamoat/browserify/flask/policy.json
Original file line number Diff line number Diff line change
Expand Up @@ -686,6 +686,7 @@
"packages": {
"@ensdomains/content-hash>multicodec>uint8arrays>multiformats": true,
"@ethereumjs/tx>@ethereumjs/util": true,
"@ethersproject/bignumber": true,
"@ethersproject/contracts": true,
"@ethersproject/providers": true,
"@metamask/abi-utils": true,
Expand Down
1 change: 1 addition & 0 deletions lavamoat/browserify/main/policy.json
Original file line number Diff line number Diff line change
Expand Up @@ -686,6 +686,7 @@
"packages": {
"@ensdomains/content-hash>multicodec>uint8arrays>multiformats": true,
"@ethereumjs/tx>@ethereumjs/util": true,
"@ethersproject/bignumber": true,
"@ethersproject/contracts": true,
"@ethersproject/providers": true,
"@metamask/abi-utils": true,
Expand Down
1 change: 1 addition & 0 deletions lavamoat/browserify/mmi/policy.json
Original file line number Diff line number Diff line change
Expand Up @@ -778,6 +778,7 @@
"packages": {
"@ensdomains/content-hash>multicodec>uint8arrays>multiformats": true,
"@ethereumjs/tx>@ethereumjs/util": true,
"@ethersproject/bignumber": true,
"@ethersproject/contracts": true,
"@ethersproject/providers": true,
"@metamask/abi-utils": true,
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@
"@metamask/address-book-controller": "^6.0.0",
"@metamask/announcement-controller": "^7.0.0",
"@metamask/approval-controller": "^7.0.0",
"@metamask/assets-controllers": "patch:@metamask/assets-controllers@npm%3A42.0.0#~/.yarn/patches/@metamask-assets-controllers-npm-42.0.0-57b3d695bb.patch",
"@metamask/assets-controllers": "patch:@metamask/assets-controllers@patch%3A@metamask/assets-controllers@npm%253A43.1.1%23~/.yarn/patches/@metamask-assets-controllers-npm-43.1.1-c223d56176.patch%3A%3Aversion=43.1.1&hash=5a94c2#~/.yarn/patches/@metamask-assets-controllers-patch-9e00573eb4.patch",
"@metamask/base-controller": "^7.0.0",
"@metamask/bitcoin-wallet-snap": "^0.8.2",
"@metamask/browser-passworder": "^4.3.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,11 @@
"MultichainBalancesController": { "balances": "object" },
"MultichainRatesController": {
"fiatCurrency": "usd",
"rates": { "btc": { "conversionDate": 0, "conversionRate": 0 } },
"cryptocurrencies": ["btc"]
"rates": {
"btc": { "conversionDate": 0, "conversionRate": 0 },
"sol": { "conversionDate": 0, "conversionRate": 0 }
},
"cryptocurrencies": ["btc", "sol"]
},
"NameController": { "names": "object", "nameSources": "object" },
"NetworkController": {
Expand Down Expand Up @@ -313,7 +316,13 @@
},
"TokenListController": {
"tokenList": "object",
"tokensChainsCache": {},
"tokensChainsCache": {
"0x1": "object",
"0x539": "object",
"0xaa36a7": "object",
"0xe705": "object",
"0xe708": "object"
},
"preventPollingOnNetworkRestart": false
},
"TokenRatesController": { "marketData": "object" },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,13 @@
"gasEstimateType": "none",
"nonRPCGasFeeApisDisabled": "boolean",
"tokenList": "object",
"tokensChainsCache": {},
"tokensChainsCache": {
"0x1": "object",
"0x539": "object",
"0xaa36a7": "object",
"0xe705": "object",
"0xe708": "object"
},
"preventPollingOnNetworkRestart": false,
"tokens": "object",
"ignoredTokens": "object",
Expand All @@ -198,8 +204,11 @@
"lastFetchedBlockNumbers": "object",
"submitHistory": "object",
"fiatCurrency": "usd",
"rates": { "btc": { "conversionDate": 0, "conversionRate": 0 } },
"cryptocurrencies": ["btc"],
"rates": {
"btc": { "conversionDate": 0, "conversionRate": 0 },
"sol": { "conversionDate": 0, "conversionRate": 0 }
},
"cryptocurrencies": ["btc", "sol"],
"snaps": "object",
"jobs": "object",
"database": null,
Expand Down
9 changes: 9 additions & 0 deletions test/e2e/tests/tokens/add-hide-token.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,15 @@ describe('Add existing token using search', function () {
{
fixtures: new FixtureBuilder({ inputChainId: CHAIN_IDS.BSC })
.withPreferencesController({ useTokenDetection: true })
.withTokenListController({
tokenList: [
{
name: 'Basic Attention Token',
symbol: 'BAT',
address: '0x0d8775f648430679a709e98d2b0cb6250d2887ef',
},
],
})
.build(),
ganacheOptions: {
...defaultGanacheOptions,
Expand Down
Loading