Skip to content

Commit

Permalink
fix: Fix currentNetwork selector when current network config is mis…
Browse files Browse the repository at this point in the history
…sing (#26327)

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.
  • Loading branch information
Gudahtt committed Aug 2, 2024
1 parent 517409d commit f9d762a
Show file tree
Hide file tree
Showing 7 changed files with 135 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ export function AssetPicker({
badge={
<AvatarNetwork
size={AvatarNetworkSize.Xs}
name={currentNetwork?.nickname}
name={currentNetwork?.nickname ?? ''}
src={currentNetwork?.rpcPrefs?.imageUrl}
backgroundColor={testNetworkBackgroundColor}
borderColor={
Expand Down
2 changes: 1 addition & 1 deletion ui/components/multichain/nft-item/nft-item.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ NftItem.propTypes = {
/**
* Image that represents the network
*/
networkSrc: PropTypes.string.isRequired,
networkSrc: PropTypes.string,
/**
* Token ID of the NFT
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export const SendPageNetworkPicker = () => {
return (
<SendPageRow>
<PickerNetwork
label={currentNetwork?.nickname}
label={currentNetwork?.nickname ?? ''}
src={currentNetwork?.rpcPrefs?.imageUrl}
onClick={() => dispatch(toggleNetworkMenu())}
data-testid="send-page-network-picker"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,14 @@ exports[`add-ethereum-chain confirmation should match snapshot 1`] = `
<div
class="mm-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 mm-box--display-flex mm-box--justify-content-center mm-box--align-items-center mm-box--color-text-default mm-box--background-color-background-alternative mm-box--rounded-full mm-box--border-color-transparent box--border-style-solid box--border-width-1"
>
?
T
</div>
<span
class="mm-box mm-text mm-text--body-sm mm-text--ellipsis mm-box--color-text-default"
data-testid="network-display"
/>
>
Test initial state
</span>
<span
class="mm-box mm-picker-network__arrow-down-icon mm-icon mm-icon--size-xs mm-box--margin-left-auto mm-box--display-none mm-box--color-icon-default"
style="mask-image: url('./images/icons/arrow-down.svg');"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,14 @@ exports[`switch-ethereum-chain confirmation should match snapshot 1`] = `
<div
class="mm-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 mm-box--display-flex mm-box--justify-content-center mm-box--align-items-center mm-box--color-text-default mm-box--background-color-background-alternative mm-box--rounded-full mm-box--border-color-transparent box--border-style-solid box--border-width-1"
>
?
T
</div>
<span
class="mm-box mm-text mm-text--body-sm mm-text--ellipsis mm-box--color-text-default"
data-testid="network-display"
/>
>
Test initial state
</span>
<span
class="mm-box mm-picker-network__arrow-down-icon mm-icon mm-icon--size-xs mm-box--margin-left-auto mm-box--display-none mm-box--color-icon-default"
style="mask-image: url('./images/icons/arrow-down.svg');"
Expand Down Expand Up @@ -131,12 +133,14 @@ exports[`switch-ethereum-chain confirmation should show alert if there are pendi
<div
class="mm-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 mm-box--display-flex mm-box--justify-content-center mm-box--align-items-center mm-box--color-text-default mm-box--background-color-background-alternative mm-box--rounded-full mm-box--border-color-transparent box--border-style-solid box--border-width-1"
>
?
T
</div>
<span
class="mm-box mm-text mm-text--body-sm mm-text--ellipsis mm-box--color-text-default"
data-testid="network-display"
/>
>
Test initial state
</span>
<span
class="mm-box mm-picker-network__arrow-down-icon mm-icon mm-icon--size-xs mm-box--margin-left-auto mm-box--display-none mm-box--color-icon-default"
style="mask-image: url('./images/icons/arrow-down.svg');"
Expand Down
25 changes: 24 additions & 1 deletion ui/selectors/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -1800,12 +1800,35 @@ export function getNumberOfAllUnapprovedTransactionsAndMessages(state) {
export const getCurrentNetwork = createDeepEqualSelector(
getAllNetworks,
getProviderConfig,
/**
* Get the current network configuration.
*
* @param {Record<string, unknown>[]} allNetworks - All network configurations.
* @param {Record<string, unknown>} 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,
}
);
},
);

Expand Down
96 changes: 96 additions & 0 deletions ui/selectors/selectors.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit f9d762a

Please sign in to comment.