Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 7 commits into from
Jul 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions test/data/mock-state.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": [
Expand Down
3 changes: 3 additions & 0 deletions test/e2e/fixture-builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ function defaultFixture() {
rpcUrl: 'http://localhost:8545',
ticker: 'ETH',
type: 'rpc',
id: 'networkConfigurationId',
},
networkConfigurations: {
networkConfigurationId: {
Expand Down Expand Up @@ -346,6 +347,7 @@ function onboardingFixture() {
rpcUrl: 'http://localhost:8545',
chainId: CHAIN_IDS.LOCALHOST,
nickname: 'Localhost 8545',
id: 'networkConfigurationId',
},
networkConfigurations: {
networkConfigurationId: {
Expand All @@ -355,6 +357,7 @@ function onboardingFixture() {
rpcUrl: 'http://localhost:8545',
ticker: 'ETH',
networkConfigurationId: 'networkConfigurationId',
type: 'rpc',
},
},
},
Expand Down
10 changes: 6 additions & 4 deletions test/e2e/tests/custom-rpc-history.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
},
},
})
Expand Down Expand Up @@ -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',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
18 changes: 16 additions & 2 deletions ui/components/app/nft-details/nft-details.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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,
},
},
};
Expand Down Expand Up @@ -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',
},
},
},
Expand Down Expand Up @@ -239,7 +252,8 @@ describe('NFT Details', () => {
metamask: {
...mockState.metamask,
providerConfig: {
chainId: '0xaa36a7',
chainId: CHAIN_IDS.SEPOLIA,
type: NETWORK_TYPES.SEPOLIA,
},
},
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
getCurrentChainId,
getNonTestNetworks,
getTestNetworks,
getCurrentNetwork,
} from '../../../selectors';
import ToggleButton from '../../ui/toggle-button';
import {
Expand Down Expand Up @@ -60,6 +61,9 @@ export const NetworkListMenu = ({ onClose }) => {

const showTestNetworks = useSelector(getShowTestNetworks);
const currentChainId = useSelector(getCurrentChainId);

const currentNetwork = useSelector(getCurrentNetwork);

const dispatch = useDispatch();
const history = useHistory();
const trackEvent = useContext(MetaMetricsContext);
Expand All @@ -76,7 +80,8 @@ export const NetworkListMenu = ({ onClose }) => {
if (!lineaMainnetReleased && network.providerType === 'linea-mainnet') {
return null;
}
const isCurrentNetwork = currentChainId === network.chainId;
const isCurrentNetwork = currentNetwork.id === network.id;

const canDeleteNetwork =
!isCurrentNetwork && !UNREMOVABLE_CHAIN_IDS.includes(network.chainId);

Expand Down
13 changes: 11 additions & 2 deletions ui/selectors/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -1193,13 +1197,15 @@ export function getTestNetworks(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,
nickname: SEPOLIA_DISPLAY_NAME,
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,
Expand All @@ -1210,6 +1216,7 @@ export function getTestNetworks(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(
Expand All @@ -1232,6 +1239,7 @@ export function getNonTestNetworks(state) {
},
providerType: NETWORK_TYPES.MAINNET,
ticker: CURRENCY_SYMBOLS.ETH,
id: NETWORK_TYPES.MAINNET,
},
{
chainId: CHAIN_IDS.LINEA_MAINNET,
Expand All @@ -1242,6 +1250,7 @@ export function getNonTestNetworks(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(
Expand Down
40 changes: 40 additions & 0 deletions ui/selectors/selectors.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 and Linea with showTestNetworks off', () => {
const networks = selectors.getAllEnabledNetworks({
Expand Down