Skip to content

Commit c9eaf77

Browse files
georgewrmarshallgeorgewrmarshall
authored
fix: Balance empty state incorrectly shown when price conversion unavailable (#38284)
## **Description** This PR fixes an issue where the balance empty state incorrectly appears when users have token balances, particularly on chains without price conversion data. **The problem:** PR #37366 introduced a balance empty state feature that shows a "Fund your wallet" message when accounts have $0 balance. However, the `selectAccountGroupBalanceForEmptyState` selector only checked `totalBalanceInUserCurrency` (fiat value) to determine if an account has a balance. This approach had multiple issues: 1. When price conversion data is unavailable (custom networks, price API failures), fiat balance equals $0 even when native tokens are present 2. The expensive `calculateBalanceForAllWallets()` function was called unnecessarily 3. The logic didn't check for actual token possession, which is what determines if a user can transact The issue became visible after PR #37004 turned on BIP-44 by default. Users with native tokens on custom networks (like Anvil chainId 1338) would see the empty state despite having a 10 ETH balance. **The solution:** The selector now directly checks for token balances (native and non-native) instead of relying on fiat conversion: 1. **Native token checks:** - **EVM chains**: Check `accountsByChainId` for non-zero hex balances (e.g., `0x8ac7230489e80000`) - **Non-EVM chains**: Check `multichainBalancesState.balances` for non-zero amounts (Solana, Bitcoin, etc.) 2. **Non-native token checks:** - **ERC-20 tokens**: Check `tokenBalancesState` for non-zero hex balances - **Other tokens**: Future support for SPL tokens, etc. **Code quality improvements:** - Uses established utilities from `@metamask/utils` (`isObject`, `hasProperty`) for robust type checking - Added helper function `getBalanceOrDefault()` for safe balance extraction - Uses `isEmptyHexString()` and `isZeroAmount()` utilities for reliable zero detection - Removed unnecessary defensive checks for unrealistic edge cases - Better performance: no longer calls expensive `calculateBalanceForAllWallets()` The empty state now correctly appears only when users have NO tokens (neither native nor non-native), regardless of price API availability. ## **Changelog** CHANGELOG entry: Fixed balance empty state incorrectly showing when price conversion data is unavailable but tokens are present ## **Related issues** Fixes: Issue exposed by #37004, related to #37366 ## **Manual testing steps** 1. Set up a local Anvil node: `anvil --port 8546 --chain-id 1338` 2. Add custom network in MetaMask: RPC URL `http://localhost:8546`, Chain ID `1338` 3. Send ETH to your account: `cast send <address> --value 10ether --rpc-url http://localhost:8546 --private-key 0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80` 4. Verify that the balance empty state does NOT appear 5. Create a fresh account with no balance 6. Verify that the balance empty state DOES appear correctly Alternative test without Anvil: 1. Use browser DevTools to mock price API failures 2. Verify accounts with native tokens don't show empty state 3. Verify accounts with $0 balance AND no native tokens DO show empty state ## **Screenshots/Recordings** ### **Before** Empty state incorrectly shown despite 10 ETH balance on custom network (chainId 1338) https://github.com/user-attachments/assets/ec78382f-4928-41c8-88fe-c25fc380f69c ### **After** Empty state correctly hidden when native token balances are present, regardless of price conversion availability https://github.com/user-attachments/assets/63997e02-3bf5-4511-a545-ca89b1a9dd6f Balance empty state displays as intended on completely empty account groups https://github.com/user-attachments/assets/0f12fd69-ee78-46f6-8de1-a8ce54cb56a1 ## **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/main/.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/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. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Refactors balance-empty-state logic to check actual token balances (EVM native, non-EVM native, ERC‑20) on mainnets and updates tests accordingly. > > - **Selectors**: > - Replace fiat-based check in `selectAccountGroupBalanceForEmptyState` with direct token presence checks: > - EVM native via `accountsByChainId` (hex non-zero using `isEmptyHexString`). > - Non-EVM native via `multichainBalances` (decimal non-zero using `isZeroAmount`). > - ERC-20 via `tokenBalances` (hex non-zero). > - Exclude all testnets using `selectAllMainnetNetworksEnabledMap`; only mainnet chains considered. > - Add helpers: `getBalanceOrDefault`, `selectAccountsByChainIdForBalances`; import `hasProperty`, `isObject`, `isEmptyHexString`, `isZeroAmount`. > - **Tests** (`ui/selectors/assets.test.ts`): > - Update to assert new balance-detection logic, including: > - Positive/zero balances for EVM and Solana. > - Ignoring EVM and non-EVM testnets. > - Decimal-zero handling (e.g., "0.00"). > - ERC-20 tokens without native balance. > - Mock `selectAssetsBySelectedAccountGroup` to return `{}` by default; remove reliance on `calculateBalanceForAllWallets` in these tests. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 039fe7e. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 5181212 commit c9eaf77

File tree

2 files changed

+368
-108
lines changed

2 files changed

+368
-108
lines changed

‎ui/selectors/assets.test.ts‎

Lines changed: 191 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ jest.mock('@metamask/assets-controllers', () => {
5151
percentChange: 0,
5252
userCurrency: 'usd',
5353
})),
54-
selectAssetsBySelectedAccountGroup: jest.fn(),
54+
selectAssetsBySelectedAccountGroup: jest.fn(() => ({})), // Returns empty object by default
5555
};
5656
});
5757

@@ -1072,57 +1072,54 @@ describe('selectAccountGroupBalanceForEmptyState', () => {
10721072
};
10731073
};
10741074

1075-
const createMockBalanceResult = (balance = 750.25) => ({
1076-
wallets: {
1077-
'entropy:wallet1': {
1078-
totalBalanceInUserCurrency: balance * 2,
1079-
groups: {
1080-
'entropy:wallet1/group1': {
1081-
walletId: 'entropy:wallet1',
1082-
groupId: 'entropy:wallet1/group1',
1083-
totalBalanceInUserCurrency: balance,
1084-
userCurrency: 'usd',
1085-
},
1086-
},
1087-
},
1088-
},
1089-
userCurrency: 'usd',
1090-
});
1091-
10921075
it('should return true when balance is greater than 0 for EVM networks', () => {
10931076
const state = createMockStateWithEVMNetworks();
1094-
const expectedBalance = 750.25;
1095-
const mockResult = createMockBalanceResult(expectedBalance);
1096-
(calculateBalanceForAllWallets as jest.Mock).mockReturnValueOnce(
1097-
mockResult,
1098-
);
1077+
1078+
// Add accountsByChainId with non-zero EVM balance
1079+
state.metamask.accountsByChainId = {
1080+
'0x1': {
1081+
'0x0': {
1082+
address: '0x0',
1083+
balance: '0x8ac7230489e80000', // 10 ETH
1084+
},
1085+
},
1086+
};
10991087

11001088
const result = selectAccountGroupBalanceForEmptyState(state);
11011089

11021090
expect(result).toBe(true);
1103-
expect(calculateBalanceForAllWallets).toHaveBeenCalledTimes(1);
11041091
});
11051092

11061093
it('should return true when balance is greater than 0 for non-EVM networks like Solana', () => {
11071094
const state = createMockStateWithNonEVMNetworks();
1108-
const expectedBalance = 500.5;
1109-
const mockResult = createMockBalanceResult(expectedBalance);
1110-
(calculateBalanceForAllWallets as jest.Mock).mockReturnValueOnce(
1111-
mockResult,
1112-
);
1095+
1096+
// Add multichainBalancesState with non-zero Solana balance
1097+
state.metamask.balances = {
1098+
account2: {
1099+
'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501': {
1100+
amount: '10.5',
1101+
unit: 'SOL',
1102+
},
1103+
},
1104+
};
11131105

11141106
const result = selectAccountGroupBalanceForEmptyState(state);
11151107

11161108
expect(result).toBe(true);
1117-
expect(calculateBalanceForAllWallets).toHaveBeenCalledTimes(1);
11181109
});
11191110

11201111
it('should return false when balance is 0', () => {
11211112
const state = createMockStateWithEVMNetworks();
1122-
const mockResult = createMockBalanceResult(0);
1123-
(calculateBalanceForAllWallets as jest.Mock).mockReturnValueOnce(
1124-
mockResult,
1125-
);
1113+
1114+
// Add accountsByChainId with zero EVM balance
1115+
state.metamask.accountsByChainId = {
1116+
'0x1': {
1117+
'0x0': {
1118+
address: '0x0',
1119+
balance: '0x0',
1120+
},
1121+
},
1122+
};
11261123

11271124
const result = selectAccountGroupBalanceForEmptyState(state);
11281125

@@ -1131,22 +1128,28 @@ describe('selectAccountGroupBalanceForEmptyState', () => {
11311128

11321129
it('should return true for small positive balances', () => {
11331130
const state = createMockStateWithEVMNetworks();
1134-
const mockResult = createMockBalanceResult(0.01);
1135-
(calculateBalanceForAllWallets as jest.Mock).mockReturnValueOnce(
1136-
mockResult,
1137-
);
1131+
1132+
// Add accountsByChainId with small positive EVM balance
1133+
state.metamask.accountsByChainId = {
1134+
'0x1': {
1135+
'0x0': {
1136+
address: '0x0',
1137+
balance: '0x2386f26fc10000', // 0.01 ETH
1138+
},
1139+
},
1140+
};
11381141

11391142
const result = selectAccountGroupBalanceForEmptyState(state);
11401143

11411144
expect(result).toBe(true);
11421145
});
11431146

1144-
it('should return false for negative balances', () => {
1147+
it('should return false when no balances are set', () => {
11451148
const state = createMockStateWithEVMNetworks();
1146-
const mockResult = createMockBalanceResult(-10);
1147-
(calculateBalanceForAllWallets as jest.Mock).mockReturnValueOnce(
1148-
mockResult,
1149-
);
1149+
1150+
// No balances set at all
1151+
state.metamask.accountsByChainId = {};
1152+
state.metamask.balances = {};
11501153

11511154
const result = selectAccountGroupBalanceForEmptyState(state);
11521155

@@ -1155,47 +1158,161 @@ describe('selectAccountGroupBalanceForEmptyState', () => {
11551158

11561159
it('should exclude EVM testnets from balance calculation', () => {
11571160
const state = createMockStateWithEVMNetworks(true); // Include EVM testnets
1158-
const mockResult = createMockBalanceResult();
1159-
(calculateBalanceForAllWallets as jest.Mock).mockReturnValueOnce(
1160-
mockResult,
1161-
);
11621161

1163-
selectAccountGroupBalanceForEmptyState(state);
1162+
// Add balances for both mainnet and testnet
1163+
state.metamask.accountsByChainId = {
1164+
'0x1': {
1165+
// Ethereum mainnet
1166+
'0x0': {
1167+
address: '0x0',
1168+
balance: '0x0', // Zero on mainnet
1169+
},
1170+
},
1171+
'0xaa36a7': {
1172+
// Sepolia testnet (should be ignored)
1173+
'0x0': {
1174+
address: '0x0',
1175+
balance: '0x8ac7230489e80000', // 10 ETH on testnet
1176+
},
1177+
},
1178+
};
11641179

1165-
const callArgs = (calculateBalanceForAllWallets as jest.Mock).mock.calls[0];
1166-
const enabledNetworkMap = callArgs[9]; // 10th argument is the network map
1180+
const result = selectAccountGroupBalanceForEmptyState(state);
11671181

1168-
// Should not include Sepolia (0xaa36a7) or Linea Sepolia (0xe705)
1169-
expect(enabledNetworkMap?.eip155).not.toHaveProperty('0xaa36a7');
1170-
expect(enabledNetworkMap?.eip155).not.toHaveProperty('0xe705');
1171-
// Should include mainnet networks
1172-
expect(enabledNetworkMap?.eip155).toHaveProperty('0x1');
1173-
expect(enabledNetworkMap?.eip155).toHaveProperty('0x89');
1182+
// Should return false because testnet balance is ignored
1183+
expect(result).toBe(false);
11741184
});
11751185

11761186
it('should exclude non-EVM testnets like Solana from balance calculation', () => {
11771187
const state = createMockStateWithNonEVMNetworks(true); // Include non-EVM testnets
1178-
const mockResult = createMockBalanceResult();
1179-
(calculateBalanceForAllWallets as jest.Mock).mockReturnValueOnce(
1180-
mockResult,
1181-
);
11821188

1183-
selectAccountGroupBalanceForEmptyState(state);
1189+
// Add balances for both mainnet and testnet
1190+
state.metamask.balances = {
1191+
account2: {
1192+
'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501': {
1193+
// Mainnet
1194+
amount: '0',
1195+
unit: 'SOL',
1196+
},
1197+
'solana:4uhcVJyU9pJkvQyS88uRDiswHXSCkY3z/slip44:501': {
1198+
// Testnet (should be ignored)
1199+
amount: '10.5',
1200+
unit: 'SOL',
1201+
},
1202+
},
1203+
};
1204+
1205+
const result = selectAccountGroupBalanceForEmptyState(state);
11841206

1185-
const callArgs = (calculateBalanceForAllWallets as jest.Mock).mock.calls[0];
1186-
const enabledNetworkMap = callArgs[9];
1207+
// Should return false because testnet balance is ignored
1208+
expect(result).toBe(false);
1209+
});
11871210

1188-
// Should not include Solana testnet or devnet
1189-
expect(enabledNetworkMap?.solana).not.toHaveProperty(
1190-
'solana:4uhcVJyU9pJkvQyS88uRDiswHXSCkY3z',
1191-
);
1192-
expect(enabledNetworkMap?.solana).not.toHaveProperty(
1193-
'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1',
1194-
);
1195-
// Should include Solana mainnet
1196-
expect(enabledNetworkMap?.solana).toHaveProperty(
1197-
'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp',
1198-
);
1211+
describe('native token balance checks', () => {
1212+
it('should return true when EVM native token balance exists', () => {
1213+
const state = createMockStateWithEVMNetworks();
1214+
1215+
// Add accountsByChainId with non-zero EVM balance
1216+
state.metamask.accountsByChainId = {
1217+
'0x1': {
1218+
'0x0': {
1219+
address: '0x0',
1220+
balance: '0x8ac7230489e80000', // 10 ETH
1221+
},
1222+
},
1223+
};
1224+
1225+
const result = selectAccountGroupBalanceForEmptyState(state);
1226+
1227+
expect(result).toBe(true);
1228+
});
1229+
1230+
it('should return true when non-EVM native token balance exists', () => {
1231+
const state = createMockStateWithNonEVMNetworks();
1232+
1233+
// Add multichainBalancesState with non-zero Solana balance
1234+
state.metamask.balances = {
1235+
account2: {
1236+
'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501': {
1237+
amount: '10.5',
1238+
unit: 'SOL',
1239+
},
1240+
},
1241+
};
1242+
1243+
const result = selectAccountGroupBalanceForEmptyState(state);
1244+
1245+
expect(result).toBe(true);
1246+
});
1247+
1248+
it('should return false when no native token balances exist', () => {
1249+
const state = createMockStateWithEVMNetworks();
1250+
1251+
// Add accountsByChainId with zero EVM balance
1252+
state.metamask.accountsByChainId = {
1253+
'0x1': {
1254+
'0x0': {
1255+
address: '0x0',
1256+
balance: '0x0',
1257+
},
1258+
},
1259+
};
1260+
1261+
// Add multichainBalancesState with zero balance
1262+
state.metamask.balances = {};
1263+
1264+
const result = selectAccountGroupBalanceForEmptyState(state);
1265+
1266+
expect(result).toBe(false);
1267+
});
1268+
1269+
it('should return false when non-EVM balance is decimal zero like "0.0" or "0.00"', () => {
1270+
const state = createMockStateWithNonEVMNetworks();
1271+
1272+
// Add multichainBalancesState with decimal zero Solana balance
1273+
state.metamask.balances = {
1274+
account2: {
1275+
'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501': {
1276+
amount: '0.00', // Decimal zero
1277+
unit: 'SOL',
1278+
},
1279+
},
1280+
};
1281+
1282+
const result = selectAccountGroupBalanceForEmptyState(state);
1283+
1284+
expect(result).toBe(false);
1285+
});
1286+
1287+
it('should return true when user has ERC-20 tokens but no native tokens', () => {
1288+
const state = createMockStateWithEVMNetworks();
1289+
1290+
// Add accountsByChainId with zero EVM balance
1291+
state.metamask.accountsByChainId = {
1292+
'0x1': {
1293+
'0x0': {
1294+
address: '0x0',
1295+
balance: '0x0', // No ETH
1296+
},
1297+
},
1298+
};
1299+
1300+
// Add tokenBalances with ERC-20 tokens
1301+
state.metamask.tokenBalances = {
1302+
'0x0': {
1303+
// account address
1304+
'0x1': {
1305+
// Ethereum mainnet
1306+
'0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48': '0xde0b6b3a7640000', // USDC balance
1307+
},
1308+
},
1309+
};
1310+
1311+
const result = selectAccountGroupBalanceForEmptyState(state);
1312+
1313+
// Should return true because user has non-native tokens
1314+
expect(result).toBe(true);
1315+
});
11991316
});
12001317
});
12011318

0 commit comments

Comments
 (0)