From f9d762a463b4a178a0c31f6bafd369d0690c2d44 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Fri, 2 Aug 2024 16:49:31 -0230 Subject: [PATCH] fix: Fix `currentNetwork` selector when current network config is missing (#26327) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `providerConfig` state is not guaranteed to match a built-in or custom network. It should in the vast majority of cases, but there are some edge cases where that would not hold true. For example: * If the wallet crashed partway through removing the current network * If the user has had the same network selected since before we introduced the `networkConfigurations` state The `currentNetwork` selector has been updated to handle this case by returning a configuration object constructed from the `providerConfig` state directly. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/26327?quickstart=1) Fixes #26320 * Create a dev build and proceed through onboarding * Run this command in the background console to set `providerConfig` to a custom network that isn't present in the user's tracked network configurations: ``` chrome.storage.local.get( null, (state) => { state.data.NetworkController.providerConfig = { rpcUrl: 'https://mainnet.optimism.io', chainId: '0xa', ticker: 'ETH', type: 'rpc', }; chrome.storage.local.set(state, () => chrome.runtime.reload()); } ); ``` * See that the UI can be opened and does not crash. N/A - [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. - [ ] 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. --- .../asset-picker/asset-picker.tsx | 2 +- ui/components/multichain/nft-item/nft-item.js | 2 +- .../pages/send/components/network-picker.tsx | 2 +- .../add-ethereum-chain.test.js.snap | 6 +- .../switch-ethereum-chain.test.js.snap | 12 ++- ui/selectors/selectors.js | 25 ++++- ui/selectors/selectors.test.js | 96 +++++++++++++++++++ 7 files changed, 135 insertions(+), 10 deletions(-) diff --git a/ui/components/multichain/asset-picker-amount/asset-picker/asset-picker.tsx b/ui/components/multichain/asset-picker-amount/asset-picker/asset-picker.tsx index 100c6708fe7e..8f50ba28f9be 100644 --- a/ui/components/multichain/asset-picker-amount/asset-picker/asset-picker.tsx +++ b/ui/components/multichain/asset-picker-amount/asset-picker/asset-picker.tsx @@ -184,7 +184,7 @@ export function AssetPicker({ badge={ { return ( dispatch(toggleNetworkMenu())} data-testid="send-page-network-picker" diff --git a/ui/pages/confirmations/confirmation/templates/__snapshots__/add-ethereum-chain.test.js.snap b/ui/pages/confirmations/confirmation/templates/__snapshots__/add-ethereum-chain.test.js.snap index 47294c6b5f80..b20b71194b00 100644 --- a/ui/pages/confirmations/confirmation/templates/__snapshots__/add-ethereum-chain.test.js.snap +++ b/ui/pages/confirmations/confirmation/templates/__snapshots__/add-ethereum-chain.test.js.snap @@ -17,12 +17,14 @@ exports[`add-ethereum-chain confirmation should match snapshot 1`] = `
- ? + T
+ > + Test initial state + - ? + T + > + Test initial state + - ? + T + > + Test initial state + []} allNetworks - All network configurations. + * @param {Record} providerConfig - The configuration for the current network's provider. + * @returns {{ + * chainId: `0x${string}`; + * id?: string; + * nickname?: string; + * providerType?: string; + * rpcPrefs?: { blockExplorerUrl?: string; imageUrl?: string; }; + * rpcUrl: string; + * ticker: string; + * }} networkConfiguration - Configuration for the current network. + */ (allNetworks, providerConfig) => { const filter = providerConfig.type === 'rpc' ? (network) => network.id === providerConfig.id : (network) => network.id === providerConfig.type; - return allNetworks.find(filter); + return ( + allNetworks.find(filter) ?? { + chainId: providerConfig.chainId, + nickname: providerConfig.nickname, + rpcPrefs: providerConfig.rpcPrefs, + rpcUrl: providerConfig.rpcUrl, + ticker: providerConfig.ticker, + } + ); }, ); diff --git a/ui/selectors/selectors.test.js b/ui/selectors/selectors.test.js index 2ea7ff87d5dc..f9b175d3eb35 100644 --- a/ui/selectors/selectors.test.js +++ b/ui/selectors/selectors.test.js @@ -738,6 +738,102 @@ describe('Selectors', () => { }); describe('#getCurrentNetwork', () => { + it('returns built-in network configuration', () => { + const modifiedMockState = { + ...mockState, + metamask: { + ...mockState.metamask, + providerConfig: { + ...mockState.metamask.providerConfig, + chainId: '0x1', + type: 'sepolia', + }, + }, + }; + const currentNetwork = selectors.getCurrentNetwork(modifiedMockState); + + expect(currentNetwork).toMatchInlineSnapshot(` + { + "chainId": "0xaa36a7", + "id": "sepolia", + "nickname": "Sepolia", + "providerType": "sepolia", + "removable": false, + "rpcUrl": "https://sepolia.infura.io/v3/undefined", + "ticker": "SepoliaETH", + } + `); + }); + + it('returns custom network configuration', () => { + const mockNetworkConfigurationId = 'mock-network-config-id'; + const modifiedMockState = { + ...mockState, + metamask: { + ...mockState.metamask, + networkConfigurations: { + ...mockState.networkConfigurations, + [mockNetworkConfigurationId]: { + rpcUrl: 'https://mock-rpc-endpoint.test', + chainId: '0x9999', + ticker: 'TST', + id: mockNetworkConfigurationId, + }, + }, + providerConfig: { + rpcUrl: 'https://mock-rpc-endpoint.test', + chainId: '0x9999', + ticker: 'TST', + id: mockNetworkConfigurationId, + type: 'rpc', + }, + }, + }; + + const currentNetwork = selectors.getCurrentNetwork(modifiedMockState); + + expect(currentNetwork).toMatchInlineSnapshot(` + { + "blockExplorerUrl": undefined, + "chainId": "0x9999", + "id": "mock-network-config-id", + "removable": true, + "rpcPrefs": { + "imageUrl": undefined, + }, + "rpcUrl": "https://mock-rpc-endpoint.test", + "ticker": "TST", + } + `); + }); + + it('returns custom network configuration that is missing from networkConfigurations state', () => { + const modifiedMockState = { + ...mockState, + metamask: { + ...mockState.metamask, + providerConfig: { + rpcUrl: 'https://mock-rpc-endpoint.test', + chainId: '0x9999', + ticker: 'TST', + type: 'rpc', + }, + }, + }; + + const currentNetwork = selectors.getCurrentNetwork(modifiedMockState); + + expect(currentNetwork).toMatchInlineSnapshot(` + { + "chainId": "0x9999", + "nickname": undefined, + "rpcPrefs": undefined, + "rpcUrl": "https://mock-rpc-endpoint.test", + "ticker": "TST", + } + `); + }); + it('returns the correct custom network when there is a chainId collision', () => { const modifiedMockState = { ...mockState,