Fix #19941 - Correctly show network name and selection when chainIds collide (#19947)

test/data/mock-state.json

@@ -115,13 +115,22 @@
       "type": "rpc",
       "chainId": "0x5",
       "ticker": "ETH",
-      "id": "testNetworkConfigurationId"
+      "id": "chain5"
     },
     "networkConfigurations": {
       "testNetworkConfigurationId": {
         "rpcUrl": "https://testrpc.com",
         "chainId": "0x1",
-        "nickname": "Custom Mainnet RPC"
+        "nickname": "Custom Mainnet RPC",
+        "type": "rpc",
+        "id": "testNetworkConfigurationId"
+      },
+      "chain5": {
+        "type": "rpc",
+        "chainId": "0x5",
+        "ticker": "ETH",
+        "nickname": "Chain 5",
+        "id": "chain5"
       }
     },
     "keyrings": [

test/e2e/fixture-builder.js

@@ -215,6 +215,7 @@ function defaultFixture() {
           rpcUrl: 'http://localhost:8545',
           ticker: 'ETH',
           type: 'rpc',
+          id: 'networkConfigurationId',
         },
         networkConfigurations: {
           networkConfigurationId: {
@@ -346,6 +347,7 @@ function onboardingFixture() {
           rpcUrl: 'http://localhost:8545',
           chainId: CHAIN_IDS.LOCALHOST,
           nickname: 'Localhost 8545',
+          id: 'networkConfigurationId',
         },
         networkConfigurations: {
           networkConfigurationId: {
@@ -355,6 +357,7 @@ function onboardingFixture() {
             rpcUrl: 'http://localhost:8545',
             ticker: 'ETH',
             networkConfigurationId: 'networkConfigurationId',
+            type: 'rpc',
           },
         },
       },

test/e2e/tests/custom-rpc-history.spec.js

@@ -196,19 +196,21 @@ describe('Stores custom RPC history', function () {
         fixtures: new FixtureBuilder()
           .withNetworkController({
             networkConfigurations: {
-              networkConfigurationId: {
+              networkConfigurationIdOne: {
                 rpcUrl: 'http://127.0.0.1:8545/1',
                 chainId: '0x539',
                 ticker: 'ETH',
                 nickname: 'http://127.0.0.1:8545/1',
                 rpcPrefs: {},
+                type: 'rpc',
               },
-              networkConfigurationId2: {
+              networkConfigurationIdTwo: {
                 rpcUrl: 'http://127.0.0.1:8545/2',
                 chainId: '0x539',
                 ticker: 'ETH',
                 nickname: 'http://127.0.0.1:8545/2',
                 rpcPrefs: {},
+                type: 'rpc',
               },
             },
           })
@@ -248,14 +250,14 @@ describe('Stores custom RPC history', function () {
         fixtures: new FixtureBuilder()
           .withNetworkController({
             networkConfigurations: {
-              networkConfigurationId: {
+              networkConfigurationIdOne: {
                 rpcUrl: 'http://127.0.0.1:8545/1',
                 chainId: '0x539',
                 ticker: 'ETH',
                 nickname: 'http://127.0.0.1:8545/1',
                 rpcPrefs: {},
               },
-              networkConfigurationId2: {
+              networkConfigurationIdTwo: {
                 rpcUrl: 'http://127.0.0.1:8545/2',
                 chainId: '0x539',
                 ticker: 'ETH',

ui/components/app/nft-details/__snapshots__/nft-details.test.js.snap

@@ -66,7 +66,7 @@ exports[`NFT Details should match minimal props and state snapshot 1`] = `
                 class="box mm-text mm-avatar-base mm-avatar-base--size-sm mm-avatar-network nft-item__network-badge mm-text--body-sm mm-text--text-transform-uppercase box--display-flex box--flex-direction-row box--justify-content-center box--align-items-center box--color-text-default box--background-color-background-alternative box--rounded-full box--border-color-background-default box--border-width-2 box--border-style-solid"
                 data-testid="nft-network-badge"
               >
-                G
+                C
               </div>
             </div>
           </div>

ui/components/app/nft-details/nft-details.test.js

@@ -13,6 +13,12 @@ import {
   removeAndIgnoreNft,
   setRemoveNftMessage,
 } from '../../../store/actions';
+import {
+  CHAIN_IDS,
+  CURRENCY_SYMBOLS,
+  MAINNET_DISPLAY_NAME,
+  NETWORK_TYPES,
+} from '../../../../shared/constants/network';
 import NftDetails from './nft-details';
 
 jest.mock('copy-to-clipboard');
@@ -172,7 +178,10 @@ describe('NFT Details', () => {
         metamask: {
           ...mockState.metamask,
           providerConfig: {
-            chainId: '0x1',
+            chainId: CHAIN_IDS.MAINNET,
+            type: NETWORK_TYPES.MAINNET,
+            ticker: CURRENCY_SYMBOLS.ETH,
+            nickname: MAINNET_DISPLAY_NAME,
           },
         },
       };
@@ -203,12 +212,16 @@ describe('NFT Details', () => {
           ...mockState.metamask,
           providerConfig: {
             chainId: '0x89',
+            type: 'rpc',
+            id: 'custom-mainnet',
           },
           networkConfigurations: {
             testNetworkConfigurationId: {
               rpcUrl: 'https://testrpc.com',
               chainId: '0x89',
               nickname: 'Custom Mainnet RPC',
+              type: 'rpc',
+              id: 'custom-mainnet',
             },
           },
         },
@@ -239,7 +252,8 @@ describe('NFT Details', () => {
         metamask: {
           ...mockState.metamask,
           providerConfig: {
-            chainId: '0xaa36a7',
+            chainId: CHAIN_IDS.SEPOLIA,
+            type: NETWORK_TYPES.SEPOLIA,
           },
         },
       };

ui/components/multichain/app-header/__snapshots__/app-header.test.js.snap

@@ -214,12 +214,12 @@ exports[`App Header should match snapshot 1`] = `
           <div
             class="box mm-text mm-avatar-base mm-avatar-base--size-xs mm-avatar-network mm-picker-network__avatar-network mm-text--body-xs mm-text--text-transform-uppercase box--display-flex box--flex-direction-row box--justify-content-center box--align-items-center box--color-text-default box--background-color-background-alternative box--rounded-full box--border-color-transparent box--border-style-solid box--border-width-1"
           >
-            G
+            C
           </div>
           <p
             class="box mm-text mm-text--body-sm mm-text--ellipsis box--flex-direction-row box--color-text-default"
           >
-            Goerli
+            Chain 5
           </p>
           <span
             class="box mm-picker-network__arrow-down-icon mm-icon mm-icon--size-xs box--display-inline-block box--flex-direction-row box--color-icon-default"

ui/components/multichain/network-list-menu/network-list-menu.js

@@ -17,6 +17,7 @@ import {
   getShowTestNetworks,
   getAllEnabledNetworks,
   getCurrentChainId,
+  getCurrentNetwork,
 } from '../../../selectors';
 import Box from '../../ui/box/box';
 import ToggleButton from '../../ui/toggle-button';
@@ -53,6 +54,9 @@ export const NetworkListMenu = ({ onClose }) => {
   const networks = useSelector(getAllEnabledNetworks);
   const showTestNetworks = useSelector(getShowTestNetworks);
   const currentChainId = useSelector(getCurrentChainId);
+
+  const currentNetwork = useSelector(getCurrentNetwork);
+
   const dispatch = useDispatch();
   const history = useHistory();
   const trackEvent = useContext(MetaMetricsContext);
@@ -91,7 +95,7 @@ export const NetworkListMenu = ({ onClose }) => {
             ) {
               return null;
             }
-            const isCurrentNetwork = currentChainId === network.chainId;
+            const isCurrentNetwork = currentNetwork.id === network.id;
             const canDeleteNetwork =
               !isCurrentNetwork &&
               !UNREMOVABLE_CHAIN_IDS.includes(network.chainId);

ui/selectors/selectors.js

@@ -1170,9 +1170,13 @@ export function getNetworkConfigurations(state) {
 
 export function getCurrentNetwork(state) {
   const allNetworks = getAllNetworks(state);
-  const currentChainId = getCurrentChainId(state);
+  const providerConfig = getProviderConfig(state);
 
-  return allNetworks.find((network) => network.chainId === currentChainId);
+  const filter =
+    providerConfig.type === 'rpc'
+      ? (network) => network.id === providerConfig.id
+      : (network) => network.id === providerConfig.type;
+  return allNetworks.find(filter);
 }
 
 export function getAllEnabledNetworks(state) {
@@ -1200,6 +1204,7 @@ export function getAllNetworks(state) {
       },
       providerType: NETWORK_TYPES.MAINNET,
       ticker: CURRENCY_SYMBOLS.ETH,
+      id: NETWORK_TYPES.MAINNET,
     },
     {
       chainId: CHAIN_IDS.LINEA_MAINNET,
@@ -1210,6 +1215,7 @@ export function getAllNetworks(state) {
       },
       providerType: NETWORK_TYPES.LINEA_MAINNET,
       ticker: TEST_NETWORK_TICKER_MAP[NETWORK_TYPES.LINEA_MAINNET],
+      id: NETWORK_TYPES.LINEA_MAINNET,
     },
     // Custom networks added by the user
     ...Object.values(networkConfigurations).filter(
@@ -1222,6 +1228,7 @@ export function getAllNetworks(state) {
       rpcUrl: CHAIN_ID_TO_RPC_URL_MAP[CHAIN_IDS.GOERLI],
       providerType: NETWORK_TYPES.GOERLI,
       ticker: TEST_NETWORK_TICKER_MAP[NETWORK_TYPES.GOERLI],
+      id: NETWORK_TYPES.GOERLI,
     },
     {
       chainId: CHAIN_IDS.SEPOLIA,
@@ -1229,6 +1236,7 @@ export function getAllNetworks(state) {
       rpcUrl: CHAIN_ID_TO_RPC_URL_MAP[CHAIN_IDS.SEPOLIA],
       providerType: NETWORK_TYPES.SEPOLIA,
       ticker: TEST_NETWORK_TICKER_MAP[NETWORK_TYPES.SEPOLIA],
+      id: NETWORK_TYPES.SEPOLIA,
     },
     {
       chainId: CHAIN_IDS.LINEA_GOERLI,
@@ -1239,6 +1247,7 @@ export function getAllNetworks(state) {
       },
       providerType: NETWORK_TYPES.LINEA_GOERLI,
       ticker: TEST_NETWORK_TICKER_MAP[NETWORK_TYPES.LINEA_GOERLI],
+      id: NETWORK_TYPES.LINEA_GOERLI,
     },
     // Localhosts
     ...Object.values(networkConfigurations).filter(

ui/selectors/selectors.test.js

@@ -316,6 +316,46 @@ describe('Selectors', () => {
     });
   });
 
+  describe('#getCurrentNetwork', () => {
+    it('returns the correct custom network when there is a chainId collision', () => {
+      const modifiedMockState = {
+        ...mockState,
+        metamask: {
+          ...mockState.metamask,
+          providerConfig: {
+            ...mockState.metamask.networkConfigurations
+              .testNetworkConfigurationId,
+            // 0x1 would collide with Ethereum Mainnet
+            chainId: '0x1',
+            // type of "rpc" signals custom network
+            type: 'rpc',
+          },
+        },
+      };
+
+      const currentNetwork = selectors.getCurrentNetwork(modifiedMockState);
+      expect(currentNetwork.nickname).toBe('Custom Mainnet RPC');
+      expect(currentNetwork.chainId).toBe('0x1');
+    });
+
+    it('returns the correct mainnet network when there is a chainId collision', () => {
+      const modifiedMockState = {
+        ...mockState,
+        metamask: {
+          ...mockState.metamask,
+          providerConfig: {
+            ...mockState.metamask.providerConfig,
+            chainId: '0x1',
+            // Changing type to 'mainnet' represents Ethereum Mainnet
+            type: 'mainnet',
+          },
+        },
+      };
+      const currentNetwork = selectors.getCurrentNetwork(modifiedMockState);
+      expect(currentNetwork.nickname).toBe('Ethereum Mainnet');
+    });
+  });
+
   describe('#getAllEnabledNetworks', () => {
     it('returns only MainNet with showTestNetworks off', () => {
       const networks = selectors.getAllEnabledNetworks({

ui/store/actions.ts

@@ -19,12 +19,9 @@ import { PayloadAction } from '@reduxjs/toolkit';
 import { GasFeeController } from '@metamask/gas-fee-controller';
 import { PermissionsRequest } from '@metamask/permission-controller';
 import { NonEmptyArray } from '@metamask/controller-utils';
-<<<<<<< HEAD
 ///: BEGIN:ONLY_INCLUDE_IN(keyring-snaps)
 import { HandlerType } from '@metamask/snaps-utils';
 ///: END:ONLY_INCLUDE_IN
-=======
->>>>>>> master
 import { getMethodDataAsync } from '../helpers/utils/transactions.util';
 import switchDirection from '../../shared/lib/switch-direction';
 import {