refactor: remove global network usage from transaction confirmations (#…

…28236) ## **Description** Remove usages of global network selectors from transaction confirmation React components and hooks. Specifically: - Remove usages of the following selectors: - `getConversionRate` - `getCurrentChainId` - `getNativeCurrency` - `getNetworkIdentifier` - `getNftContracts` - `getNfts` - `getProviderConfig` - `getRpcPrefsForCurrentProvider` - Add new selectors: - `selectConversionRateByChainId` - `selectNftsByChainId` - `selectNftContractsByChainId` - `selectNetworkIdentifierByChainId` - Add ESLint rule to prevent further usage of global network selectors in confirmations directory. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/28236?quickstart=1) ## **Related issues** Fixes: [#3469](MetaMask/MetaMask-planning#3469) [#3373](MetaMask/MetaMask-planning#3373) [#3486](MetaMask/MetaMask-planning#3486) [#3487](MetaMask/MetaMask-planning#3487) ## **Manual testing steps** Full regression of all transaction confirmations and related functionality. ## **Screenshots/Recordings** ### **Before** ### **After** ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --------- Co-authored-by: Jyoti Puri <jyotipuri@gmail.com>
MetaMask · Nov 8, 2024 · c564383 · c564383
1 parent 04dd7d3
commit c564383
Show file tree

Hide file tree

Showing 49 changed files with 416 additions and 257 deletions.
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -472,5 +472,30 @@ module.exports = {
         '@metamask/design-tokens/color-no-hex': 'off',
       },
     },
+    {
+      files: ['ui/pages/confirmations/**/*.{js,ts,tsx}'],
+      rules: {
+        'no-restricted-syntax': [
+          'error',
+          {
+            selector: `ImportSpecifier[imported.name=/${[
+              'getConversionRate',
+              'getCurrentChainId',
+              'getNativeCurrency',
+              'getNetworkIdentifier',
+              'getNftContracts',
+              'getNfts',
+              'getProviderConfig',
+              'getRpcPrefsForCurrentProvider',
+              'getUSDConversionRate',
+              'isCurrentProviderCustom',
+            ]
+              .map((method) => `(${method})`)
+              .join('|')}/]`,
+            message: 'Avoid using global network selectors in confirmations',
+          },
+        ],
+      },
+    },
   ],
 };
diff --git a/.storybook/preview.js b/.storybook/preview.js
@@ -15,6 +15,7 @@ import MetaMetricsProviderStorybook from './metametrics';
 import testData from './test-data.js';
 import { Router } from 'react-router-dom';
 import { createBrowserHistory } from 'history';
+import { MemoryRouter } from 'react-router-dom';
 import { setBackgroundConnection } from '../ui/store/background-connection';
 import { metamaskStorybookTheme } from './metamask-storybook-theme';
 import { DocsContainer } from '@storybook/addon-docs';
@@ -147,7 +148,7 @@ const metamaskDecorator = (story, context) => {
 
   return (
     <Provider store={store}>
-      <Router history={history}>
+      <MemoryRouter>
         <MetaMetricsProviderStorybook>
           <AlertMetricsProvider
             metrics={{
@@ -165,7 +166,7 @@ const metamaskDecorator = (story, context) => {
             </I18nProvider>
           </AlertMetricsProvider>
         </MetaMetricsProviderStorybook>
-      </Router>
+      </MemoryRouter>
     </Provider>
   );
 };

diff --git a/test/data/confirmations/contract-interaction.ts b/test/data/confirmations/contract-interaction.ts
@@ -1,4 +1,5 @@
 import {
+  CHAIN_IDS,
   SimulationData,
   TransactionMeta,
   TransactionStatus,
@@ -18,7 +19,7 @@ export const CONTRACT_INTERACTION_SENDER_ADDRESS =
 
 export const DEPOSIT_METHOD_DATA = '0xd0e30db0';
 
-export const CHAIN_ID = '0xaa36a7';
+export const CHAIN_ID = CHAIN_IDS.GOERLI;
 
 export const genUnapprovedContractInteractionConfirmation = ({
   address = CONTRACT_INTERACTION_SENDER_ADDRESS,

diff --git a/...omponents/advanced-gas-fee-popover/advanced-gas-fee-defaults/advanced-gas-fee-defaults.js b/...omponents/advanced-gas-fee-popover/advanced-gas-fee-defaults/advanced-gas-fee-defaults.js
@@ -11,8 +11,7 @@ import {
 } from '../../../../../helpers/constants/design-system';
 import {
   getAdvancedGasFeeValues,
-  getCurrentChainId,
-  getNetworkIdentifier,
+  selectNetworkIdentifierByChainId,
 } from '../../../../../selectors';
 import { setAdvancedGasFee } from '../../../../../store/actions';
 import { useGasFeeContext } from '../../../../../contexts/gasFee';
@@ -35,11 +34,14 @@ const AdvancedGasFeeDefaults = () => {
     10,
   ).toString();
   const advancedGasFeeValues = useSelector(getAdvancedGasFeeValues);
-  // This will need to use a different chainId in multinetwork
-  const chainId = useSelector(getCurrentChainId);
-  const networkIdentifier = useSelector(getNetworkIdentifier);
   const { updateTransactionEventFragment } = useTransactionEventFragment();
-  const { editGasMode } = useGasFeeContext();
+  const { editGasMode, transaction } = useGasFeeContext();
+  const { chainId } = transaction;
+
+  const networkIdentifier = useSelector((state) =>
+    selectNetworkIdentifierByChainId(state, chainId),
+  );
+
   const [isDefaultSettingsSelected, setDefaultSettingsSelected] = useState(
     Boolean(advancedGasFeeValues) &&
       advancedGasFeeValues.maxBaseFee === maxBaseFee &&

diff --git a/...ents/advanced-gas-fee-popover/advanced-gas-fee-defaults/advanced-gas-fee-defaults.test.js b/...ents/advanced-gas-fee-popover/advanced-gas-fee-defaults/advanced-gas-fee-defaults.test.js
@@ -21,6 +21,7 @@ import { mockNetworkState } from '../../../../../../test/stub/networks';
 import AdvancedGasFeeDefaults from './advanced-gas-fee-defaults';
 
 const TEXT_SELECTOR = 'Save these values as my default for the Goerli network.';
+const CHAIN_ID_MOCK = CHAIN_IDS.GOERLI;
 
 jest.mock('../../../../../store/actions', () => ({
   gasFeeStartPollingByNetworkClientId: jest
@@ -43,7 +44,7 @@ const render = async (defaultGasParams, contextParams) => {
     metamask: {
       ...mockState.metamask,
       ...defaultGasParams,
-      ...mockNetworkState({ chainId: CHAIN_IDS.GOERLI }),
+      ...mockNetworkState({ chainId: CHAIN_ID_MOCK }),
       accounts: {
         [mockSelectedInternalAccount.address]: {
           address: mockSelectedInternalAccount.address,
@@ -71,6 +72,7 @@ const render = async (defaultGasParams, contextParams) => {
       (result = renderWithProvider(
         <GasFeeContextProvider
           transaction={{
+            chainId: CHAIN_ID_MOCK,
             userFeeLevel: 'medium',
           }}
           {...contextParams}

diff --git a/ui/pages/confirmations/components/confirm-hexdata/confirm-hexdata.test.js b/ui/pages/confirmations/components/confirm-hexdata/confirm-hexdata.test.js
@@ -1,20 +1,33 @@
 import React from 'react';
 
+import { CHAIN_IDS } from '@metamask/transaction-controller';
 import mockState from '../../../../../test/data/mock-state.json';
 import { renderWithProvider } from '../../../../../test/jest';
 
 import configureStore from '../../../../store/store';
+import { mockNetworkState } from '../../../../../test/stub/networks';
 import ConfirmHexData from './confirm-hexdata';
 
 jest.mock('../../../../../shared/lib/fetch-with-cache');
 
+const CHAIN_ID_MOCK = CHAIN_IDS.GOERLI;
+
+const STATE_MOCK = {
+  ...mockState,
+  metamask: {
+    ...mockState.metamask,
+    ...mockNetworkState({ chainId: CHAIN_ID_MOCK }),
+  },
+};
+
 describe('ConfirmHexData', () => {
-  const store = configureStore(mockState);
+  const store = configureStore(STATE_MOCK);
 
   it('should render function type', async () => {
     const { findByText } = renderWithProvider(
       <ConfirmHexData
         txData={{
+          chainId: CHAIN_ID_MOCK,
           txParams: {
             to: '0x8eeee1781fd885ff5ddef7789486676961873d12',
             data: '0x608060405234801',
@@ -34,6 +47,7 @@ describe('ConfirmHexData', () => {
       const { container } = renderWithProvider(
         <ConfirmHexData
           txData={{
+            chainId: CHAIN_ID_MOCK,
             txParams: {
               data,
             },
@@ -51,6 +65,7 @@ describe('ConfirmHexData', () => {
     const { container } = renderWithProvider(
       <ConfirmHexData
         txData={{
+          chainId: CHAIN_ID_MOCK,
           txParams: {
             data: '0x608060405234801',
           },
@@ -67,6 +82,7 @@ describe('ConfirmHexData', () => {
     const { getByText } = renderWithProvider(
       <ConfirmHexData
         txData={{
+          chainId: CHAIN_ID_MOCK,
           txParams: {},
           origin: 'https://metamask.github.io',
           type: 'transfer',

diff --git a/ui/pages/confirmations/components/confirm-page-container/confirm-page-container.component.js b/ui/pages/confirmations/components/confirm-page-container/confirm-page-container.component.js
@@ -44,8 +44,8 @@ import {
   getAddressBookEntry,
   getInternalAccounts,
   getMetadataContractName,
-  getNetworkIdentifier,
   getSwapsDefaultToken,
+  selectNetworkIdentifierByChainId,
 } from '../../../../selectors';
 import useRamps from '../../../../hooks/ramps/useRamps/useRamps';
 ///: BEGIN:ONLY_INCLUDE_IF(build-main,build-beta,build-flask)
@@ -120,7 +120,6 @@ const ConfirmPageContainer = (props) => {
     useState(false);
   const isBuyableChain = useSelector(getIsNativeTokenBuyable);
   const contact = useSelector((state) => getAddressBookEntry(state, toAddress));
-  const networkIdentifier = useSelector(getNetworkIdentifier);
   const defaultToken = useSelector(getSwapsDefaultToken);
   const accountBalance = defaultToken.string;
   const internalAccounts = useSelector(getInternalAccounts);
@@ -139,8 +138,13 @@ const ConfirmPageContainer = (props) => {
   const shouldDisplayWarning =
     contentComponent && disabled && (errorKey || errorMessage);
 
-  const networkName =
-    NETWORK_TO_NAME_MAP[currentTransaction.chainId] || networkIdentifier;
+  const { chainId } = currentTransaction;
+
+  const networkIdentifier = useSelector((state) =>
+    selectNetworkIdentifierByChainId(state, chainId),
+  );
+
+  const networkName = NETWORK_TO_NAME_MAP[chainId] || networkIdentifier;
 
   const fetchCollectionBalance = useCallback(async () => {
     const tokenBalance = await fetchTokenBalance(

diff --git a/ui/pages/confirmations/components/confirm-page-container/confirm-page-container.container.js b/ui/pages/confirmations/components/confirm-page-container/confirm-page-container.container.js
@@ -1,7 +1,6 @@
 import { connect } from 'react-redux';
 import {
   getAddressBookEntry,
-  getNetworkIdentifier,
   getSwapsDefaultToken,
   getMetadataContractName,
   getAccountName,
@@ -12,7 +11,6 @@ import ConfirmPageContainer from './confirm-page-container.component';
 function mapStateToProps(state, ownProps) {
   const to = ownProps.toAddress;
   const contact = getAddressBookEntry(state, to);
-  const networkIdentifier = getNetworkIdentifier(state);
   const defaultToken = getSwapsDefaultToken(state);
   const accountBalance = defaultToken.string;
   const internalAccounts = getInternalAccounts(state);
@@ -26,7 +24,6 @@ function mapStateToProps(state, ownProps) {
     toMetadataName,
     recipientIsOwnedAccount: Boolean(ownedAccountName),
     to,
-    networkIdentifier,
     accountBalance,
   };
 }

diff --git a/ui/pages/confirmations/components/confirm-subtitle/confirm-subtitle.test.js b/ui/pages/confirmations/components/confirm-subtitle/confirm-subtitle.test.js
@@ -1,11 +1,11 @@
 import React from 'react';
 import { ERC1155, ERC721 } from '@metamask/controller-utils';
 
+import { CHAIN_IDS } from '@metamask/transaction-controller';
 import mockState from '../../../../../test/data/mock-state.json';
 import { renderWithProvider } from '../../../../../test/lib/render-helpers';
 import configureStore from '../../../../store/store';
 import { getSelectedInternalAccountFromMockState } from '../../../../../test/jest/mocks';
-import { getProviderConfig } from '../../../../ducks/metamask/metamask';
 import ConfirmSubTitle from './confirm-subtitle';
 
 const mockSelectedInternalAccount =
@@ -51,14 +51,15 @@ describe('ConfirmSubTitle', () => {
     mockState.metamask.preferences.showFiatInTestnets = false;
     mockState.metamask.allNftContracts = {
       [mockSelectedInternalAccount.address]: {
-        [getProviderConfig(mockState).chainId]: [{ address: '0x9' }],
+        [CHAIN_IDS.GOERLI]: [{ address: '0x9' }],
       },
     };
     store = configureStore(mockState);
 
     const { findByText } = renderWithProvider(
       <ConfirmSubTitle
         txData={{
+          chainId: CHAIN_IDS.GOERLI,
           txParams: {
             to: '0x9',
           },

diff --git a/...s/components/confirm/info/approve/approve-static-simulation/approve-static-simulation.tsx b/...s/components/confirm/info/approve/approve-static-simulation/approve-static-simulation.tsx
@@ -32,6 +32,7 @@ export const ApproveStaticSimulation = () => {
     transactionMeta?.txParams?.to,
     transactionMeta?.txParams?.from,
     transactionMeta?.txParams?.data,
+    transactionMeta?.chainId,
   );
 
   const decimals = initialDecimals || '0';

diff --git a/ui/pages/confirmations/components/confirm/info/approve/approve.tsx b/ui/pages/confirmations/components/confirm/info/approve/approve.tsx
@@ -30,6 +30,7 @@ const ApproveInfo = () => {
     transactionMeta.txParams.to,
     transactionMeta.txParams.from,
     transactionMeta.txParams.data,
+    transactionMeta.chainId,
   );
 
   const { spendingCap, pending } = useApproveTokenSimulation(

diff --git a/...tions/components/confirm/info/approve/edit-spending-cap-modal/edit-spending-cap-modal.tsx b/...tions/components/confirm/info/approve/edit-spending-cap-modal/edit-spending-cap-modal.tsx
@@ -54,6 +54,7 @@ export const EditSpendingCapModal = ({
     transactionMeta.txParams.to,
     transactionMeta.txParams.from,
     transactionMeta.txParams.data,
+    transactionMeta.chainId,
   );
 
   const accountBalance = calcTokenAmount(

diff --git a/ui/pages/confirmations/components/confirm/info/approve/spending-cap/spending-cap.tsx b/ui/pages/confirmations/components/confirm/info/approve/spending-cap/spending-cap.tsx
@@ -87,6 +87,7 @@ export const SpendingCap = ({
     transactionMeta.txParams.to,
     transactionMeta.txParams.from,
     transactionMeta.txParams.data,
+    transactionMeta.chainId,
   );
 
   const accountBalance = calcTokenAmount(

diff --git a/ui/pages/confirmations/components/confirm/info/hooks/use-token-values.ts b/ui/pages/confirmations/components/confirm/info/hooks/use-token-values.ts
@@ -17,6 +17,7 @@ export const useTokenValues = (transactionMeta: TransactionMeta) => {
     transactionMeta.txParams.to,
     transactionMeta.txParams.from,
     transactionMeta.txParams.data,
+    transactionMeta.chainId,
   );
 
   const decodedResponse = useDecodedTransactionData();

diff --git a/ui/pages/confirmations/components/confirm/info/hooks/useFeeCalculations.ts b/ui/pages/confirmations/components/confirm/info/hooks/useFeeCalculations.ts
@@ -12,10 +12,12 @@ import {
   multiplyHexes,
 } from '../../../../../../../shared/modules/conversion.utils';
 import { Numeric } from '../../../../../../../shared/modules/Numeric';
-import { getConversionRate } from '../../../../../../ducks/metamask/metamask';
 import { useFiatFormatter } from '../../../../../../hooks/useFiatFormatter';
 import { useGasFeeEstimates } from '../../../../../../hooks/useGasFeeEstimates';
-import { getCurrentCurrency } from '../../../../../../selectors';
+import {
+  getCurrentCurrency,
+  selectConversionRateByChainId,
+} from '../../../../../../selectors';
 import { getMultichainNetwork } from '../../../../../../selectors/multichain';
 import { HEX_ZERO } from '../shared/constants';
 import { useEIP1559TxFees } from './useEIP1559TxFees';
@@ -30,9 +32,13 @@ const EMPTY_FEES = {
 
 export function useFeeCalculations(transactionMeta: TransactionMeta) {
   const currentCurrency = useSelector(getCurrentCurrency);
-  const conversionRate = useSelector(getConversionRate);
+  const { chainId } = transactionMeta;
   const fiatFormatter = useFiatFormatter();
 
+  const conversionRate = useSelector((state) =>
+    selectConversionRateByChainId(state, chainId),
+  );
+
   const multichainNetwork = useSelector(getMultichainNetwork);
   const ticker = multichainNetwork?.network?.ticker;
 

diff --git a/...onfirmations/components/confirm/info/shared/advanced-details/advanced-details.stories.tsx b/...onfirmations/components/confirm/info/shared/advanced-details/advanced-details.stories.tsx
@@ -22,6 +22,6 @@ const Story = {
 
 export default Story;
 
-export const DefaultStory = () => <AdvancedDetails />;
+export const DefaultStory = () => <AdvancedDetails overrideVisibility />;
 
 DefaultStory.storyName = 'Default';
diff --git a/.../confirmations/components/confirm/info/shared/native-send-heading/native-send-heading.tsx b/.../confirmations/components/confirm/info/shared/native-send-heading/native-send-heading.tsx
@@ -14,7 +14,6 @@ import {
 } from '../../../../../../../components/component-library';
 import Tooltip from '../../../../../../../components/ui/tooltip';
 import { getIntlLocale } from '../../../../../../../ducks/locale/locale';
-import { getConversionRate } from '../../../../../../../ducks/metamask/metamask';
 import {
   AlignItems,
   Display,
@@ -25,7 +24,10 @@ import {
 } from '../../../../../../../helpers/constants/design-system';
 import { MIN_AMOUNT } from '../../../../../../../hooks/useCurrencyDisplay';
 import { useFiatFormatter } from '../../../../../../../hooks/useFiatFormatter';
-import { getPreferences } from '../../../../../../../selectors';
+import {
+  getPreferences,
+  selectConversionRateByChainId,
+} from '../../../../../../../selectors';
 import { getMultichainNetwork } from '../../../../../../../selectors/multichain';
 import { useConfirmContext } from '../../../../../context/confirm';
 import {
@@ -38,11 +40,16 @@ const NativeSendHeading = () => {
   const { currentConfirmation: transactionMeta } =
     useConfirmContext<TransactionMeta>();
 
+  const { chainId } = transactionMeta;
+
   const nativeAssetTransferValue = new BigNumber(
     transactionMeta.txParams.value as string,
   ).dividedBy(new BigNumber(10).pow(18));
 
-  const conversionRate = useSelector(getConversionRate);
+  const conversionRate = useSelector((state) =>
+    selectConversionRateByChainId(state, chainId),
+  );
+
   const fiatValue =
     conversionRate &&
     nativeAssetTransferValue &&