Skip to content

Commit 83c685d

Browse files
authored
fix: address list selector (#35592)
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/35592?quickstart=1) This PR introduces some changes to the address list code to fix minor bugs like crossing data between the accounts-controller and networks, update the network icons, and the address count. ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: Fixed minor bugs related to address list ## **Related issues** Fixes: ## **Manual testing steps** Pre-requisite: Enable Multichain Account State 2 1. Go to account details 2. Check that the networks number is correct according to the number of networks in the wallet 3. Go to address list 4. Check that the number of addresses are corresponding with the number of networks 5. Check that the network icons are correct ## **Screenshots/Recordings** https://github.com/user-attachments/assets/15ebb89d-412f-4931-be5f-8d76987d1235 ## **Pre-merge author checklist** - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.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.
1 parent 4200f2d commit 83c685d

File tree

8 files changed

+242
-116
lines changed

8 files changed

+242
-116
lines changed

ui/components/multichain-accounts/multichain-address-row/multichain-address-row.tsx

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
import React from 'react';
2+
import { CaipChainId, KnownCaipNamespace } from '@metamask/utils';
3+
24
import {
35
AlignItems,
46
BlockSize,
@@ -22,6 +24,7 @@ import {
2224
import { shortenAddress } from '../../../helpers/utils/util';
2325
import { useCopyToClipboard } from '../../../hooks/useCopyToClipboard';
2426
import { getImageForChainId } from '../../../selectors/multichain';
27+
import { convertCaipToHexChainId } from '../../../../shared/modules/network.utils';
2528

2629
type MultichainAddressRowProps = {
2730
/**
@@ -50,7 +53,13 @@ export const MultichainAddressRow = ({
5053
}: MultichainAddressRowProps) => {
5154
const [copied, handleCopy] = useCopyToClipboard();
5255

53-
const networkImageSrc = getImageForChainId(chainId);
56+
// We're mixing hex with caip chain ids so its necessary
57+
// to use the hex format for EVMs and caip for non EVMs.
58+
const networkImageSrc = getImageForChainId(
59+
chainId.startsWith(KnownCaipNamespace.Eip155)
60+
? convertCaipToHexChainId(chainId as CaipChainId)
61+
: chainId,
62+
);
5463
const truncatedAddress = shortenAddress(address);
5564

5665
const handleCopyClick = () => {

ui/components/multichain-accounts/multichain-address-rows-list/multichain-address-rows-list.test.tsx

Lines changed: 41 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,21 @@ import { render, screen, fireEvent } from '@testing-library/react';
33
import { Provider } from 'react-redux';
44
import configureStore from 'redux-mock-store';
55
import { InternalAccount } from '@metamask/keyring-internal-api';
6+
import { AccountGroupId } from '@metamask/account-api';
67
import { MultichainAddressRowsList } from './multichain-address-rows-list';
78

89
const mockStore = configureStore([]);
910

10-
const accounts: InternalAccount[] = [
11-
{
12-
id: '1',
11+
const WALLET_ID_MOCK = 'entropy:01K437Z7EJ0VCMFDE9TQKRV60A';
12+
13+
const GROUP_ID_MOCK = `${WALLET_ID_MOCK}/0`;
14+
15+
const ACCOUNT_ONE_ID_MOCK = 'account-one-id';
16+
const ACCOUNT_TWO_ID_MOCK = 'account-two-id';
17+
18+
const INTERNAL_ACCOUNTS_MOCK: Record<string, InternalAccount> = {
19+
[ACCOUNT_ONE_ID_MOCK]: {
20+
id: ACCOUNT_ONE_ID_MOCK,
1321
address: '0x1234567890abcdef1234567890abcdef12345678',
1422
metadata: {
1523
name: 'Ethereum Account',
@@ -19,10 +27,10 @@ const accounts: InternalAccount[] = [
1927
options: {},
2028
methods: [],
2129
type: 'eip155:eoa',
22-
scopes: ['eip155:*'],
30+
scopes: ['eip155:0'],
2331
},
24-
{
25-
id: '2',
32+
[ACCOUNT_TWO_ID_MOCK]: {
33+
id: ACCOUNT_TWO_ID_MOCK,
2634
address: 'DRpbCBMxVnDK7maPM5tGv6MvB3v1sRMC86PZ8okm21hy',
2735
metadata: {
2836
name: 'Solana Account',
@@ -32,20 +40,36 @@ const accounts: InternalAccount[] = [
3240
options: {},
3341
methods: [],
3442
type: 'solana:data-account',
35-
scopes: ['solana:*'],
43+
scopes: ['solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp'],
44+
},
45+
};
46+
47+
const ACCOUNT_TREE_MOCK = {
48+
wallets: {
49+
[WALLET_ID_MOCK]: {
50+
type: 'entropy',
51+
id: WALLET_ID_MOCK,
52+
metadata: {},
53+
groups: {
54+
[GROUP_ID_MOCK]: {
55+
type: 'multichain-account',
56+
id: GROUP_ID_MOCK,
57+
metadata: {},
58+
accounts: [ACCOUNT_ONE_ID_MOCK, ACCOUNT_TWO_ID_MOCK],
59+
},
60+
},
61+
},
3662
},
37-
];
63+
};
3864

3965
const createMockState = () => ({
4066
metamask: {
4167
completedOnboarding: true,
4268
internalAccounts: {
43-
accounts: {
44-
'1': accounts[0],
45-
'2': accounts[1],
46-
},
47-
selectedAccount: '1',
69+
accounts: INTERNAL_ACCOUNTS_MOCK,
70+
selectedAccount: ACCOUNT_ONE_ID_MOCK,
4871
},
72+
accountTree: ACCOUNT_TREE_MOCK,
4973
// EVM network configurations
5074
networkConfigurationsByChainId: {
5175
'0x1': {
@@ -144,11 +168,11 @@ const createMockState = () => ({
144168
},
145169
});
146170

147-
const renderComponent = (accountsList = accounts) => {
171+
const renderComponent = (groupId: AccountGroupId = GROUP_ID_MOCK) => {
148172
const store = mockStore(createMockState());
149173
return render(
150174
<Provider store={store}>
151-
<MultichainAddressRowsList accounts={accountsList} />
175+
<MultichainAddressRowsList groupId={groupId} />
152176
</Provider>,
153177
);
154178
};
@@ -208,8 +232,8 @@ describe('MultichainAddressRowsList', () => {
208232
expect(searchInput).toHaveValue('');
209233
});
210234

211-
it('handles empty accounts list', () => {
212-
renderComponent([]);
235+
it('handles invalid group', () => {
236+
renderComponent('invalid-group-id' as AccountGroupId);
213237

214238
expect(
215239
screen.getByTestId('multichain-address-rows-list'),

ui/components/multichain-accounts/multichain-address-rows-list/multichain-address-rows-list.tsx

Lines changed: 22 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import React, { useMemo } from 'react';
22
import { useSelector } from 'react-redux';
3-
import { InternalAccount } from '@metamask/keyring-internal-api';
4-
import { CaipChainId } from '@metamask/utils';
3+
import { type AccountGroupId } from '@metamask/account-api';
54
import { useI18nContext } from '../../../hooks/useI18nContext';
65
import {
76
BackgroundColor,
@@ -20,79 +19,42 @@ import {
2019
TextFieldSearchSize,
2120
} from '../../component-library';
2221
import { MultichainAddressRow } from '../multichain-address-row/multichain-address-row';
23-
import { getMultichainNetworkConfigurationsByChainId } from '../../../selectors/multichain/networks';
24-
import {
25-
NetworkAddressItem,
26-
sortNetworkAddressItems,
27-
getCompatibleNetworksForAccount,
28-
} from './utils';
22+
import { getInternalAccountListSpreadByScopesByGroupId } from '../../../selectors/multichain-accounts/account-tree';
2923

3024
export type MultichainAddressRowsListProps = {
3125
/**
32-
* Array of InternalAccount objects to determine compatible networks for
26+
* The account group ID.
3327
*/
34-
accounts?: InternalAccount[];
28+
groupId: AccountGroupId;
3529
};
3630

3731
export const MultichainAddressRowsList = ({
38-
accounts = [],
32+
groupId,
3933
}: MultichainAddressRowsListProps) => {
4034
const t = useI18nContext();
4135
const [searchPattern, setSearchPattern] = React.useState<string>('');
4236

43-
const [multichainNetworks] = useSelector(
44-
getMultichainNetworkConfigurationsByChainId,
37+
const getAccountsSpreadByNetworkByGroupId = useSelector((state) =>
38+
getInternalAccountListSpreadByScopesByGroupId(state, groupId),
4539
);
4640

47-
const allNetworks = useMemo(() => {
48-
const networks: Record<string, { name: string; chainId: CaipChainId }> = {};
49-
50-
if (!multichainNetworks) {
51-
return networks;
52-
}
53-
54-
Object.entries(multichainNetworks).forEach(([chainId, networkConfig]) => {
55-
if (networkConfig && networkConfig.name) {
56-
networks[chainId] = {
57-
name: networkConfig.name,
58-
chainId: chainId as CaipChainId,
59-
};
60-
}
61-
});
62-
63-
return networks;
64-
}, [multichainNetworks]);
65-
66-
// Generate network address items for all accounts and their compatible networks
67-
const networkAddressItems = useMemo(() => {
68-
const items: NetworkAddressItem[] = [];
69-
70-
accounts.forEach((account) => {
71-
const compatibleItems = getCompatibleNetworksForAccount(
72-
account,
73-
allNetworks,
74-
);
75-
items.push(...compatibleItems);
76-
});
77-
78-
return items;
79-
}, [accounts, allNetworks]);
80-
8141
const filteredItems = useMemo(() => {
8242
if (!searchPattern.trim()) {
83-
return sortNetworkAddressItems(networkAddressItems);
43+
return getAccountsSpreadByNetworkByGroupId;
8444
}
8545

8646
const pattern = searchPattern.toLowerCase();
87-
const filtered = networkAddressItems.filter((item) => {
88-
return (
89-
item.networkName.toLowerCase().includes(pattern) ||
90-
item.address.toLowerCase().includes(pattern)
91-
);
92-
});
93-
94-
return sortNetworkAddressItems(filtered);
95-
}, [networkAddressItems, searchPattern]);
47+
const filtered = getAccountsSpreadByNetworkByGroupId.filter(
48+
({ networkName, account }) => {
49+
return (
50+
networkName.toLowerCase().includes(pattern) ||
51+
account.address.toLowerCase().includes(pattern)
52+
);
53+
},
54+
);
55+
56+
return filtered;
57+
}, [getAccountsSpreadByNetworkByGroupId, searchPattern]);
9658

9759
const handleSearchChange = (event: React.ChangeEvent<HTMLInputElement>) => {
9860
setSearchPattern(event.target.value);
@@ -105,10 +67,10 @@ export const MultichainAddressRowsList = ({
10567
const renderedRows = useMemo(() => {
10668
return filteredItems.map((item, index) => (
10769
<MultichainAddressRow
108-
key={`${item.address}-${item.chainId}-${index}`}
109-
chainId={item.chainId}
70+
key={`${item.account.address}-${item.scope}-${index}`}
71+
chainId={item.scope}
11072
networkName={item.networkName}
111-
address={item.address}
73+
address={item.account.address}
11274
/>
11375
));
11476
}, [filteredItems]);

ui/helpers/utils/network-helper.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,8 @@
1+
import { NON_EVM_TESTNET_IDS } from '@metamask/multichain-network-controller';
2+
import { CaipChainId, type Hex, isCaipChainId } from '@metamask/utils';
3+
import { TEST_CHAINS } from '../../../shared/constants/network';
4+
import { convertCaipToHexChainId } from '../../../shared/modules/network.utils';
5+
16
export const getMatchedChain = (
27
decimalChainId: string,
38
safeChainsList: {
@@ -42,3 +47,21 @@ export const getMatchedNames = (
4247
return accumulator;
4348
}, []);
4449
};
50+
51+
/**
52+
* Checks if the given chain ID is a test network.
53+
*
54+
* @param chainId - The chain ID to check.
55+
* @returns True if the chain ID is a test network, false otherwise.
56+
*/
57+
export const isTestNetwork = (chainId: CaipChainId | Hex) => {
58+
if (!isCaipChainId(chainId)) {
59+
return TEST_CHAINS.includes(chainId);
60+
}
61+
62+
if (chainId.startsWith('eip155:')) {
63+
return TEST_CHAINS.includes(convertCaipToHexChainId(chainId));
64+
}
65+
66+
return NON_EVM_TESTNET_IDS.includes(chainId);
67+
};

ui/pages/multichain-accounts/multichain-account-address-list-page/multichain-account-address-list-page.tsx

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,7 @@ import {
1818
import { TextVariant } from '../../../helpers/constants/design-system';
1919
import { useI18nContext } from '../../../hooks/useI18nContext';
2020
import { MultichainAddressRowsList } from '../../../components/multichain-accounts/multichain-address-rows-list';
21-
import {
22-
getInternalAccountsFromGroupById,
23-
getMultichainAccountGroupById,
24-
} from '../../../selectors/multichain-accounts/account-tree';
21+
import { getMultichainAccountGroupById } from '../../../selectors/multichain-accounts/account-tree';
2522
import {
2623
AddressListQueryParams,
2724
AddressListSource,
@@ -37,12 +34,6 @@ export const MultichainAccountAddressListPage = () => {
3734
? (decodeURIComponent(accountGroupId) as AccountGroupId)
3835
: null;
3936

40-
const accounts = useSelector((state) =>
41-
decodedAccountGroupId
42-
? getInternalAccountsFromGroupById(state, decodedAccountGroupId)
43-
: [],
44-
);
45-
4637
const accountGroup = useSelector((state) =>
4738
decodedAccountGroupId
4839
? getMultichainAccountGroupById(state, decodedAccountGroupId)
@@ -78,7 +69,9 @@ export const MultichainAccountAddressListPage = () => {
7869
</Header>
7970
<Content>
8071
<Box flexDirection={BoxFlexDirection.Column}>
81-
<MultichainAddressRowsList accounts={accounts} />
72+
{decodedAccountGroupId ? (
73+
<MultichainAddressRowsList groupId={decodedAccountGroupId} />
74+
) : null}
8275
</Box>
8376
</Content>
8477
</Page>

ui/pages/multichain-accounts/multichain-account-details-page/multichain-account-details-page.test.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ describe('MultichainAccountDetailsPage', () => {
7575
it('displays the address count from the selector', () => {
7676
renderComponent();
7777

78-
expect(screen.getByText(/2 addresses/iu)).toBeInTheDocument();
78+
expect(screen.getByText(/10 addresses/iu)).toBeInTheDocument();
7979
});
8080

8181
it('calls history.goBack when back button is clicked', () => {

ui/selectors/multichain-accounts/account-tree.test.ts

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import {
3131
getNetworkAddressCount,
3232
getWallet,
3333
getAccountGroupsByAddress,
34+
getInternalAccountListSpreadByScopesByGroupId,
3435
} from './account-tree';
3536
import { MultichainAccountsState } from './account-tree.types';
3637
import {
@@ -1019,7 +1020,7 @@ describe('Multichain Accounts Selectors', () => {
10191020
ENTROPY_GROUP_1_ID as AccountGroupId,
10201021
);
10211022

1022-
expect(result).toBe(2);
1023+
expect(result).toBe(10);
10231024
});
10241025

10251026
it('returns 0 when the group does not exist', () => {
@@ -1233,4 +1234,43 @@ describe('Multichain Accounts Selectors', () => {
12331234
expect(result[1].id).toBe(LEDGER_GROUP_ID);
12341235
});
12351236
});
1237+
1238+
describe('getInternalAccountListSpreadByScopesByGroupId', () => {
1239+
it('returns internal accounts spread by scopes for a specific multichain group ID', () => {
1240+
const result = getInternalAccountListSpreadByScopesByGroupId(
1241+
typedMockState,
1242+
ENTROPY_GROUP_2_ID,
1243+
);
1244+
1245+
expect(result).toHaveLength(5);
1246+
expect(result[0]).toHaveProperty('scope', 'eip155:1');
1247+
expect(result[1]).toHaveProperty('scope', 'eip155:5');
1248+
expect(result[2]).toHaveProperty('scope', 'eip155:56');
1249+
expect(result[3]).toHaveProperty('scope', 'eip155:137');
1250+
expect(result[4]).toHaveProperty('scope', 'eip155:42161');
1251+
});
1252+
1253+
it('returns internal accounts spread by scopes for a specific single group ID', () => {
1254+
const result = getInternalAccountListSpreadByScopesByGroupId(
1255+
typedMockState,
1256+
LEDGER_GROUP_ID,
1257+
);
1258+
1259+
expect(result).toHaveLength(5);
1260+
expect(result[0]).toHaveProperty('scope', 'eip155:1');
1261+
expect(result[1]).toHaveProperty('scope', 'eip155:5');
1262+
expect(result[2]).toHaveProperty('scope', 'eip155:56');
1263+
expect(result[3]).toHaveProperty('scope', 'eip155:137');
1264+
expect(result[4]).toHaveProperty('scope', 'eip155:42161');
1265+
});
1266+
1267+
it('returns empty array when group ID does not exist', () => {
1268+
const result = getInternalAccountListSpreadByScopesByGroupId(
1269+
typedMockState,
1270+
'nonExistentGroupId' as AccountGroupId,
1271+
);
1272+
1273+
expect(result).toEqual([]);
1274+
});
1275+
});
12361276
});

0 commit comments

Comments
 (0)