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 version 44 #28472

Merged
merged 47 commits into from
Nov 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
f024224
upgrade
bergeron Nov 13, 2024
cfb46d1
remove unit test
bergeron Nov 13, 2024
2f36b23
Merge branch 'develop' into brian/upgrade-assets-controllers-43
bergeron Nov 13, 2024
25f5ed0
Update LavaMoat policies
metamaskbot Nov 13, 2024
6c216f8
yarn dedupe
bergeron Nov 13, 2024
2822c58
add unit test
bergeron Nov 13, 2024
3d5323a
Update LavaMoat policies
metamaskbot Nov 13, 2024
ae198fb
lint
bergeron Nov 13, 2024
e1f3843
Merge branch 'brian/upgrade-assets-controllers-43' of github.com:Meta…
bergeron Nov 13, 2024
b090b3c
lint and fix test
bergeron Nov 13, 2024
38fc254
onboarding
bergeron Nov 13, 2024
6338a00
fix race condition
bergeron Nov 14, 2024
6b22521
temporarily disable token detection polling
bergeron Nov 14, 2024
5206a35
patch
bergeron Nov 14, 2024
757d880
package json
bergeron Nov 14, 2024
abf8e11
fix unit test
bergeron Nov 14, 2024
c662098
update sentry state
bergeron Nov 14, 2024
8f1cfb1
sentry state
bergeron Nov 14, 2024
1411c2a
Merge branch 'develop' into brian/upgrade-assets-controllers-43
bergeron Nov 14, 2024
05554a3
Merge branch 'develop' into brian/upgrade-assets-controllers-43
bergeron Nov 14, 2024
729d07d
update patch
bergeron Nov 14, 2024
21fcda3
dont poll when locked
bergeron Nov 14, 2024
8b6391f
remove commented out code
bergeron Nov 14, 2024
b635ab8
fix e2e tests
bergeron Nov 14, 2024
ec38722
fix unit test
bergeron Nov 14, 2024
8978d6d
upgrade assets controllers to 44
bergeron Nov 14, 2024
79a451f
avoid calling hook different order
bergeron Nov 15, 2024
979a3af
lint
bergeron Nov 15, 2024
33d683f
Merge branch 'brian/upgrade-assets-controllers-43' into brian/upgrade…
bergeron Nov 15, 2024
dbaf0d1
lint
bergeron Nov 15, 2024
d399567
Merge branch 'develop' of github.com:MetaMask/metamask-extension into…
bergeron Nov 15, 2024
9affc1f
fix unit test and lint
bergeron Nov 15, 2024
63e01a0
fix unit test
bergeron Nov 15, 2024
527962b
lint
bergeron Nov 15, 2024
923af19
fix sentry state e2e tests
bergeron Nov 15, 2024
acacc07
fix unit test
bergeron Nov 15, 2024
52cf1df
fix unit test
bergeron Nov 15, 2024
5abcc71
fix unit tests
bergeron Nov 15, 2024
2bf29f6
Merge branch 'develop' into brian/upgrade-assets-controllers-44
bergeron Nov 15, 2024
5c55130
lint
bergeron Nov 15, 2024
5ff384a
Merge branch 'brian/upgrade-assets-controllers-44' of github.com:Meta…
bergeron Nov 15, 2024
9214479
lint
bergeron Nov 15, 2024
9facc09
fix e2e test updating balance after send
bergeron Nov 15, 2024
2e2a61d
undo file
bergeron Nov 15, 2024
0f3ae99
undo confirmations changes
bergeron Nov 15, 2024
d5413f3
fix unit tests
bergeron Nov 15, 2024
613a07a
remove empty line
bergeron Nov 15, 2024
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
3 changes: 3 additions & 0 deletions app/scripts/constants/sentry-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,9 @@ export const SENTRY_BACKGROUND_STATE = {
[AllProperties]: false,
},
},
TokenBalancesController: {
tokenBalances: false,
},
TokenRatesController: {
marketData: false,
},
Expand Down
39 changes: 39 additions & 0 deletions app/scripts/metamask-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
CodefiTokenPricesServiceV2,
RatesController,
fetchMultiExchangeRate,
TokenBalancesController,
} from '@metamask/assets-controllers';
import { JsonRpcEngine } from '@metamask/json-rpc-engine';
import { createEngineStream } from '@metamask/json-rpc-middleware-stream';
Expand Down Expand Up @@ -685,6 +686,7 @@ export default class MetamaskController extends EventEmitter {
'AccountsController:selectedEvmAccountChange',
'PreferencesController:stateChange',
'TokenListController:stateChange',
'NetworkController:stateChange',
],
});
this.tokensController = new TokensController({
Expand Down Expand Up @@ -893,6 +895,28 @@ export default class MetamaskController extends EventEmitter {
};
};

const tokenBalancesMessenger = this.controllerMessenger.getRestricted({
name: 'TokenBalancesController',
allowedActions: [
'NetworkController:getState',
'NetworkController:getNetworkClientById',
'TokensController:getState',
'PreferencesController:getState',
'AccountsController:getSelectedAccount',
],
allowedEvents: [
'PreferencesController:stateChange',
'TokensController:stateChange',
'NetworkController:stateChange',
],
});

this.tokenBalancesController = new TokenBalancesController({
messenger: tokenBalancesMessenger,
state: initState.TokenBalancesController,
interval: 30000,
});

const phishingControllerMessenger = this.controllerMessenger.getRestricted({
name: 'PhishingController',
});
Expand Down Expand Up @@ -2413,6 +2437,7 @@ export default class MetamaskController extends EventEmitter {
GasFeeController: this.gasFeeController,
TokenListController: this.tokenListController,
TokensController: this.tokensController,
TokenBalancesController: this.tokenBalancesController,
SmartTransactionsController: this.smartTransactionsController,
NftController: this.nftController,
PhishingController: this.phishingController,
Expand Down Expand Up @@ -2468,6 +2493,7 @@ export default class MetamaskController extends EventEmitter {
GasFeeController: this.gasFeeController,
TokenListController: this.tokenListController,
TokensController: this.tokensController,
TokenBalancesController: this.tokenBalancesController,
SmartTransactionsController: this.smartTransactionsController,
NftController: this.nftController,
SelectedNetworkController: this.selectedNetworkController,
Expand Down Expand Up @@ -3229,6 +3255,7 @@ export default class MetamaskController extends EventEmitter {
nftController,
nftDetectionController,
currencyRateController,
tokenBalancesController,
tokenDetectionController,
ensController,
tokenListController,
Expand Down Expand Up @@ -4047,6 +4074,14 @@ export default class MetamaskController extends EventEmitter {
tokenListStopPollingByPollingToken:
tokenListController.stopPollingByPollingToken.bind(tokenListController),

tokenBalancesStartPolling: tokenBalancesController.startPolling.bind(
tokenBalancesController,
),
tokenBalancesStopPollingByPollingToken:
tokenBalancesController.stopPollingByPollingToken.bind(
tokenBalancesController,
),

// GasFeeController
gasFeeStartPollingByNetworkClientId:
gasFeeController.startPollingByNetworkClientId.bind(gasFeeController),
Expand Down Expand Up @@ -6681,6 +6716,7 @@ export default class MetamaskController extends EventEmitter {
this.tokenRatesController.stopAllPolling();
this.tokenDetectionController.stopAllPolling();
this.tokenListController.stopAllPolling();
this.tokenBalancesController.stopAllPolling();
this.appStateController.clearPollingTokens();
} catch (error) {
console.error(error);
Expand Down Expand Up @@ -6921,6 +6957,9 @@ export default class MetamaskController extends EventEmitter {
await this._createTransactionNotifcation(transactionMeta);
await this._updateNFTOwnership(transactionMeta);
this._trackTransactionFailure(transactionMeta);
await this.tokenBalancesController.updateBalancesByChainId({
chainId: transactionMeta.chainId,
});
}

async _createTransactionNotifcation(transactionMeta) {
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@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/assets-controllers": "patch:@metamask/assets-controllers@patch%3A@metamask/assets-controllers@npm%253A44.0.0%23~/.yarn/patches/@metamask-assets-controllers-npm-44.0.0-c223d56176.patch%3A%3Aversion=44.0.0&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 @@ -314,6 +314,9 @@
"swapsFeatureFlags": {}
}
},
"TokenBalancesController": {
"tokenBalances": "object"
},
"TokenListController": {
"tokenList": "object",
"tokensChainsCache": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@
"0xe705": "object",
"0xe708": "object"
},
"tokenBalances": "object",
"preventPollingOnNetworkRestart": false,
"tokens": "object",
"ignoredTokens": "object",
Expand Down
2 changes: 2 additions & 0 deletions ui/components/app/assets/asset-list/asset-list.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ jest.mock('../../../../hooks/useIsOriginalNativeTokenSymbol', () => {
jest.mock('../../../../store/actions', () => {
return {
getTokenSymbol: jest.fn(),
tokenBalancesStartPolling: jest.fn().mockResolvedValue('pollingToken'),
tokenBalancesStopPollingByPollingToken: jest.fn(),
};
});

Expand Down
2 changes: 2 additions & 0 deletions ui/components/app/wallet-overview/btc-overview.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ jest.mock('../../../store/actions', () => ({
handleSnapRequest: jest.fn(),
sendMultichainTransaction: jest.fn(),
setDefaultHomeActiveTabName: jest.fn(),
tokenBalancesStartPolling: jest.fn().mockResolvedValue('pollingToken'),
tokenBalancesStopPollingByPollingToken: jest.fn(),
}));

const PORTOFOLIO_URL = 'https://portfolio.test';
Expand Down
2 changes: 2 additions & 0 deletions ui/components/app/wallet-overview/eth-overview.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ jest.mock('../../../ducks/locale/locale', () => ({

jest.mock('../../../store/actions', () => ({
startNewDraftTransaction: jest.fn(),
tokenBalancesStartPolling: jest.fn().mockResolvedValue('pollingToken'),
tokenBalancesStopPollingByPollingToken: jest.fn(),
}));

const mockGetIntlLocale = getIntlLocale;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ import {
AccountOverviewBtcProps,
} from './account-overview-btc';

jest.mock('../../../store/actions', () => ({
tokenBalancesStartPolling: jest.fn().mockResolvedValue('pollingToken'),
tokenBalancesStopPollingByPollingToken: jest.fn(),
}));

const defaultProps: AccountOverviewBtcProps = {
defaultHomeActiveTabName: null,
onTabClick: jest.fn(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ import {
AccountOverviewEthProps,
} from './account-overview-eth';

jest.mock('../../../store/actions', () => ({
tokenBalancesStartPolling: jest.fn().mockResolvedValue('pollingToken'),
tokenBalancesStopPollingByPollingToken: jest.fn(),
}));

const render = (props: AccountOverviewEthProps) => {
const store = configureStore({
metamask: mockState.metamask,
Expand Down
10 changes: 10 additions & 0 deletions ui/ducks/metamask/metamask.js
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,16 @@ export const getGasEstimateTypeByChainId = createSelector(
},
);

/**
* Returns the balances of imported and detected tokens across all accounts and chains.
*
* @param {*} state
* @returns { import('@metamask/assets-controllers').TokenBalancesControllerState['tokenBalances']}
*/
export function getTokenBalances(state) {
return state.metamask.tokenBalances;
}

export const getGasFeeEstimatesByChainId = createSelector(
getGasFeeControllerEstimatesByChainId,
getTransactionGasFeeEstimatesByChainId,
Expand Down
7 changes: 4 additions & 3 deletions ui/hooks/useAccountTotalFiatBalance.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
import { formatCurrency } from '../helpers/utils/confirm-tx.util';
import { getTokenFiatAmount } from '../helpers/utils/token-util';
import { roundToDecimalPlacesRemovingExtraZeroes } from '../helpers/utils/util';
import { useTokenTracker } from './useTokenTracker';
import { useTokenTracker } from './useTokenBalances';

export const useAccountTotalFiatBalance = (
account,
Expand Down Expand Up @@ -54,10 +54,11 @@ export const useAccountTotalFiatBalance = (
const primaryTokenImage = useSelector(getNativeCurrencyImage);
const nativeCurrency = useSelector(getNativeCurrency);

const { loading, tokensWithBalances } = useTokenTracker({
const loading = false;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't have an easy way to replicate the loading flag. Both because the token balances controller doesn't currently set any state indicating an update is in progress. And because in the multichain world, it may not be a single request we're waiting on. Each network will update and return separately. We don't have UI to indicate that specific balances are loading.

Copy link
Contributor Author

@bergeron bergeron Nov 15, 2024

Choose a reason for hiding this comment

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

Reading balances from state will have some UI benefits though. Since we don't have to wait for requests to return, we can always immediately show what we have in state, without an initial 'loading' flash. The values may be stale though, so in the future we may want to indicate which balances are very stale. Perhaps the controller could timestamp in state the time of the last update for each balance.

const { tokensWithBalances } = useTokenTracker({
chainId: currentChainId,
tokens,
address: account?.address,
includeFailedTokens: true,
hideZeroBalanceTokens: shouldHideZeroBalanceTokens,
});

Expand Down
41 changes: 9 additions & 32 deletions ui/hooks/useAccountTotalFiatBalance.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,35 +14,6 @@ const mockAccount = createMockInternalAccount({
address: '0x0836f5ed6b62baf60706fe3adc0ff0fd1df833da',
});

jest.mock('./useTokenTracker', () => {
return {
useTokenTracker: () => ({
loading: false,
tokensWithBalances: [
{
address: '0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48',
balance: '48573',
balanceError: null,
decimals: 6,
image: undefined,
isERC721: undefined,
string: '0.04857',
symbol: 'USDC',
},
{
address: '0x0bc529c00C6401aEF6D220BE8C6Ea1667F6Ad93e',
symbol: 'YFI',
balance: '1409247882142934',
decimals: 18,
string: '0.001409247882142934',
balanceError: null,
},
],
error: null,
}),
};
});

const renderUseAccountTotalFiatBalance = (address) => {
const state = {
...mockState,
Expand Down Expand Up @@ -78,7 +49,7 @@ const renderUseAccountTotalFiatBalance = (address) => {
},
...mockNetworkState({ chainId: CHAIN_IDS.MAINNET }),

detectedTokens: {
allTokens: {
'0x1': {
'0x0836f5ed6b62baf60706fe3adc0ff0fd1df833da': [
{
Expand All @@ -96,6 +67,14 @@ const renderUseAccountTotalFiatBalance = (address) => {
],
},
},
tokenBalances: {
[mockAccount.address]: {
[CHAIN_IDS.MAINNET]: {
'0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48': '0xBDBD',
'0x0bc529c00C6401aEF6D220BE8C6Ea1667F6Ad93e': '0x501B4176A64D6',
},
},
},
},
};

Expand All @@ -122,8 +101,6 @@ describe('useAccountTotalFiatBalance', () => {
address: '0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48',
symbol: 'USDC',
balance: '48573',
image: undefined,
isERC721: undefined,
decimals: 6,
string: 0.04857,
balanceError: null,
Expand Down
28 changes: 13 additions & 15 deletions ui/hooks/useMultichainAccountTotalFiatBalance.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,31 +15,21 @@ const mockTokenBalances = [
balance: '48573',
balanceError: null,
decimals: 6,
image: undefined,
isERC721: undefined,
string: '0.04857',
string: 0.04857,
symbol: 'USDC',
tokenFiatAmount: '0.05',
},
{
address: '0x0bc529c00C6401aEF6D220BE8C6Ea1667F6Ad93e',
symbol: 'YFI',
balance: '1409247882142934',
decimals: 18,
string: '0.001409247882142934',
string: 0.00141,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This looks like the values are changing, but it actually keeps them (mostly) the same. The old behavior of mocking ./useTokenTracker was modifying this array during the test, so this just updates to the same final result.

Screenshot 2024-11-15 at 11 22 22 AM

balanceError: null,
tokenFiatAmount: '7.52',
},
];

jest.mock('./useTokenTracker', () => {
return {
useTokenTracker: () => ({
loading: false,
tokensWithBalances: mockTokenBalances,
error: null,
}),
};
});

const mockAccount = createMockInternalAccount({
name: 'Account 1',
address: '0x0836f5ed6b62baf60706fe3adc0ff0fd1df833da',
Expand Down Expand Up @@ -104,7 +94,7 @@ const renderUseMultichainAccountTotalFiatBalance = (
},
...mockNetworkState({ chainId: CHAIN_IDS.MAINNET }),

detectedTokens: {
allTokens: {
'0x1': {
'0x0836f5ed6b62baf60706fe3adc0ff0fd1df833da': [
{
Expand All @@ -122,6 +112,14 @@ const renderUseMultichainAccountTotalFiatBalance = (
],
},
},
tokenBalances: {
[mockAccount.address]: {
[CHAIN_IDS.MAINNET]: {
'0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48': '0xBDBD',
'0x0bc529c00C6401aEF6D220BE8C6Ea1667F6Ad93e': '0x501B4176A64D6',
},
},
},
},
};

Expand Down
6 changes: 3 additions & 3 deletions ui/hooks/useMultichainAccountTotalFiatBalance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ export const useMultichainAccountTotalFiatBalance = (
tokensWithBalances: {
address: string;
symbol: string;
decimals: string;
isERC721: boolean;
image: string;
decimals: number;
isERC721?: boolean;
image?: string;
}[];
totalWeiBalance?: string;
totalBalance?: string;
Expand Down
Loading