From e5651cfe3580db9ed49a31b3b6fe6e00f1b9b47e 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 ## **Description** 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) ## **Related issues** Fixes #26320 ## **Manual testing steps** * 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. ## **Screenshots/Recordings** N/A ## **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. --- .../assets/nfts/nft-details/nft-details.tsx | 2 +- .../nfts/nft-details/nft-full-image.tsx | 2 +- .../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 +++++++++++++++++++ 9 files changed, 137 insertions(+), 12 deletions(-) diff --git a/ui/components/app/assets/nfts/nft-details/nft-details.tsx b/ui/components/app/assets/nfts/nft-details/nft-details.tsx index 0ca9a83c6ec5..0064dc38976c 100644 --- a/ui/components/app/assets/nfts/nft-details/nft-details.tsx +++ b/ui/components/app/assets/nfts/nft-details/nft-details.tsx @@ -345,7 +345,7 @@ export default function NftDetails({ nft }: { nft: Nft }) { alt={image ? nftImageAlt : ''} name={name} tokenId={tokenId} - networkName={currentChain.nickname} + networkName={currentChain.nickname ?? ''} networkSrc={currentChain.rpcPrefs?.imageUrl} isIpfsURL={isIpfsURL} onClick={handleImageClick} diff --git a/ui/components/app/assets/nfts/nft-details/nft-full-image.tsx b/ui/components/app/assets/nfts/nft-details/nft-full-image.tsx index 68562bc6d857..3d09cba1ddc4 100644 --- a/ui/components/app/assets/nfts/nft-details/nft-full-image.tsx +++ b/ui/components/app/assets/nfts/nft-details/nft-full-image.tsx @@ -82,7 +82,7 @@ export default function NftFullImage() { alt={image ? nftImageAlt : ''} name={name} tokenId={tokenId} - networkName={currentChain.nickname} + networkName={currentChain.nickname ?? ''} networkSrc={currentChain.rpcPrefs?.imageUrl} isIpfsURL={isIpfsURL} badgeWrapperClassname="badge-wrapper" 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 56aa58ed4f74..ea382450fbd1 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 8fd320c388cc..b8a02a46e7e5 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 a8546bc5fdec..f27d60af9756 100644 --- a/ui/selectors/selectors.test.js +++ b/ui/selectors/selectors.test.js @@ -782,6 +782,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,