Fix account selectors when balances are missing (#20385)

* Fix account selectors when balances are missing

Some of the account selectors we use would return an empty set of
accounts if the `AccountTracker` state was missing. This resulted in UI
crashes when trying to access the current selected account.

The selectors have been updated to use the `identities` as the source-
of-truth for the full set of accounts. This ensures that even if the
balances are missing, each account will at least be represented by an
empty object.

* Fix unit test

* Fix another unit test

* Fix another unit test

* Fix another unit test

* Fix more unit tests

---------

Co-authored-by: legobeat <109787230+legobeat@users.noreply.github.com>

ui/components/app/edit-gas-fee-popover/edit-gas-fee-popover.test.js

@@ -61,6 +61,9 @@ const render = ({ txProps, contextProps } = {}) => {
           balance: '0x1F4',
         },
       },
+      identities: {
+        '0xAddress': {},
+      },
       selectedAddress: '0xAddress',
       featureFlags: { advancedInlineGas: true },
       gasFeeEstimates: MOCK_FEE_ESTIMATE,

ui/components/app/edit-gas-fee-popover/edit-gas-item/edit-gas-item.test.js

@@ -67,6 +67,9 @@ const renderComponent = ({
           balance: '0x176e5b6f173ebe66',
         },
       },
+      identities: {
+        '0xAddress': {},
+      },
       selectedAddress: '0xAddress',
       featureFlags: { advancedInlineGas: true },
       gasEstimateType: 'fee-market',

ui/components/app/edit-gas-fee-popover/edit-gas-tooltip/edit-gas-tooltip.test.js

@@ -41,6 +41,9 @@ const renderComponent = (componentProps) => {
           balance: '0x176e5b6f173ebe66',
         },
       },
+      identities: {
+        '0xAddress': {},
+      },
       selectedAddress: '0xAddress',
       featureFlags: { advancedInlineGas: true },
     },

ui/components/app/gas-details-item/gas-details-item-title/gas-details-item-title.test.js

@@ -28,6 +28,9 @@ const render = () => {
           balance: '0x176e5b6f173ebe66',
         },
       },
+      identities: {
+        '0xAddress': {},
+      },
       selectedAddress: '0xAddress',
     },
   });

ui/components/institutional/custody-confirm-link-modal/custody-confirm-link-modal.js

@@ -11,7 +11,7 @@ import withModalProps from '../../../helpers/higher-order-components/with-modal-
 import { toChecksumHexAddress } from '../../../../shared/modules/hexstring-utils';
 import { mmiActionsFactory } from '../../../store/institutional/institution-background';
 import { setSelectedAddress } from '../../../store/actions';
-import { getMetaMaskAccountsRaw } from '../../../selectors';
+import { getMetaMaskIdentities } from '../../../selectors';
 import {
   getMMIAddressFromModalOrAddress,
   getCustodyAccountDetails,
@@ -42,7 +42,7 @@ const CustodyConfirmLink = ({ hideModal }) => {
   const dispatch = useDispatch();
   const mmiActions = mmiActionsFactory();
   const trackEvent = useContext(MetaMetricsContext);
-  const mmiAccounts = useSelector(getMetaMaskAccountsRaw);
+  const mmiAccounts = useSelector(getMetaMaskIdentities);
   const address = useSelector(getMMIAddressFromModalOrAddress);
   const custodyAccountDetails = useSelector(getCustodyAccountDetails);
   const { custodians } = useSelector(getMMIConfiguration);

ui/ducks/send/send.test.js

@@ -77,6 +77,8 @@ import { draftTransactionInitialState, editExistingTransaction } from '.';
 
 const mockStore = createMockStore([thunk]);
 
+const mockAddress1 = '0xdafea492d9c6733ae3d56b7ed1adb60692c98123';
+
 jest.mock('./send', () => {
   const actual = jest.requireActual('./send');
   return {
@@ -1063,7 +1065,7 @@ describe('Send Slice', () => {
     describe('QR Code Detected', () => {
       const qrCodestate = getInitialSendStateWithExistingTxState({
         recipient: {
-          address: '0xAddress',
+          address: mockAddress1,
         },
       });
 
@@ -1102,7 +1104,7 @@ describe('Send Slice', () => {
 
         const draftTransaction = getTestUUIDTx(result);
 
-        expect(draftTransaction.recipient.address).toStrictEqual('0xAddress');
+        expect(draftTransaction.recipient.address).toStrictEqual(mockAddress1);
         expect(draftTransaction.recipient.error).toStrictEqual(
           INVALID_RECIPIENT_ADDRESS_ERROR,
         );
@@ -1115,7 +1117,7 @@ describe('Send Slice', () => {
           ...INITIAL_SEND_STATE_FOR_EXISTING_DRAFT,
           selectedAccount: {
             balance: '0x0',
-            address: '0xAddress',
+            address: mockAddress1,
           },
         };
 
@@ -1144,7 +1146,7 @@ describe('Send Slice', () => {
           ...INITIAL_SEND_STATE_FOR_EXISTING_DRAFT,
           selectedAccount: {
             balance: '0x0',
-            address: '0xAddress',
+            address: mockAddress1,
           },
         };
 
@@ -1158,7 +1160,7 @@ describe('Send Slice', () => {
         const result = sendReducer(olderState, action);
 
         expect(result.selectedAccount.balance).toStrictEqual('0x0');
-        expect(result.selectedAccount.address).toStrictEqual('0xAddress');
+        expect(result.selectedAccount.address).toStrictEqual(mockAddress1);
       });
     });
 
@@ -1167,13 +1169,13 @@ describe('Send Slice', () => {
         const accountsChangedState = {
           ...getInitialSendStateWithExistingTxState({
             fromAccount: {
-              address: '0xAddress',
+              address: mockAddress1,
               balance: '0x0',
             },
           }),
           stage: SEND_STAGES.EDIT,
           selectedAccount: {
-            address: '0xAddress',
+            address: mockAddress1,
             balance: '0x0',
           },
         };
@@ -1182,7 +1184,7 @@ describe('Send Slice', () => {
           type: 'ACCOUNT_CHANGED',
           payload: {
             account: {
-              address: '0xAddress',
+              address: mockAddress1,
               balance: '0x1',
             },
           },
@@ -1201,13 +1203,13 @@ describe('Send Slice', () => {
         const accountsChangedState = {
           ...getInitialSendStateWithExistingTxState({
             fromAccount: {
-              address: '0xAddress',
+              address: mockAddress1,
               balance: '0x0',
             },
           }),
           stage: SEND_STAGES.EDIT,
           selectedAccount: {
-            address: '0xAddress',
+            address: mockAddress1,
             balance: '0x0',
           },
         };
@@ -1231,7 +1233,7 @@ describe('Send Slice', () => {
           ...INITIAL_SEND_STATE_FOR_EXISTING_DRAFT,
           stage: SEND_STAGES.EDIT,
           selectedAccount: {
-            address: '0xAddress',
+            address: mockAddress1,
             balance: '0x0',
           },
         };
@@ -1274,23 +1276,23 @@ describe('Send Slice', () => {
                 1559: true,
               },
             },
-            selectedAddress: '0xAddress',
+            selectedAddress: mockAddress1,
-            identities: { '0xAddress': { address: '0xAddress' } },
+            identities: { [mockAddress1]: { address: mockAddress1 } },
             keyrings: [
               {
                 type: KeyringType.hdKeyTree,
-                accounts: ['0xAddress'],
+                accounts: [mockAddress1],
               },
             ],
             accounts: {
-              '0xAddress': {
+              [mockAddress1]: {
-                address: '0xAddress',
+                address: mockAddress1,
                 balance: '0x0',
               },
             },
             cachedBalances: {
               0x5: {
-                '0xAddress': '0x0',
+                [mockAddress1]: '0x0',
               },
             },
             providerConfig: {
@@ -1580,14 +1582,17 @@ describe('Send Slice', () => {
           },
           cachedBalances: {
             [CHAIN_IDS.GOERLI]: {
-              '0xAddress': '0x0',
+              [mockAddress1]: '0x0',
             },
           },
           accounts: {
-            '0xAddress': {
+            [mockAddress1]: {
-              address: '0xAddress',
+              address: mockAddress1,
             },
           },
+          identities: {
+            [mockAddress1]: {},
+          },
         },
         send: {
           ...getInitialSendStateWithExistingTxState({
@@ -1607,7 +1612,7 @@ describe('Send Slice', () => {
             userInputHexData: '',
           }),
           selectedAccount: {
-            address: '0xAddress',
+            address: mockAddress1,
           },
         },
       };
@@ -2379,16 +2384,18 @@ describe('Send Slice', () => {
             addressBook: {
               [CHAIN_IDS.GOERLI]: {},
             },
-            identities: {},
+            identities: {
+              [mockAddress1]: {},
+            },
             accounts: {
-              '0xAddress': {
+              [mockAddress1]: {
-                address: '0xAddress',
+                address: mockAddress1,
                 balance: '0x0',
               },
             },
             cachedBalances: {
               [CHAIN_IDS.GOERLI]: {
-                '0xAddress': '0x0',
+                [mockAddress1]: '0x0',
               },
             },
             tokenList: {},
@@ -2397,7 +2404,7 @@ describe('Send Slice', () => {
                 id: 1,
                 txParams: {
                   data: '',
-                  from: '0xAddress',
+                  from: mockAddress1,
                   to: '0xRecipientAddress',
                   gas: GAS_LIMITS.SIMPLE,
                   gasPrice: '0x3b9aca00', // 1000000000
@@ -2414,7 +2421,7 @@ describe('Send Slice', () => {
             ...getInitialSendStateWithExistingTxState({
               id: 1,
               fromAccount: {
-                address: '0xAddress',
+                address: mockAddress1,
               },
             }),
           },
@@ -2443,7 +2450,7 @@ describe('Send Slice', () => {
               type: AssetType.native,
             },
             fromAccount: {
-              address: '0xAddress',
+              address: mockAddress1,
               balance: '0x0',
             },
             gas: {
@@ -2514,16 +2521,18 @@ describe('Send Slice', () => {
             addressBook: {
               [CHAIN_IDS.GOERLI]: {},
             },
-            identities: {},
+            identities: {
+              [mockAddress1]: {},
+            },
             accounts: {
-              '0xAddress': {
+              [mockAddress1]: {
-                address: '0xAddress',
+                address: mockAddress1,
                 balance: '0x0',
               },
             },
             cachedBalances: {
               [CHAIN_IDS.GOERLI]: {
-                '0xAddress': '0x0',
+                [mockAddress1]: '0x0',
               },
             },
             tokenList: {},
@@ -2533,10 +2542,10 @@ describe('Send Slice', () => {
                 txParams: {
                   data: generateERC721TransferData({
                     toAddress: BURN_ADDRESS,
-                    fromAddress: '0xAddress',
+                    fromAddress: mockAddress1,
                     tokenId: BigNumber.from(15000).toString(),
                   }),
-                  from: '0xAddress',
+                  from: mockAddress1,
                   to: '0xNftAddress',
                   gas: GAS_LIMITS.BASE_TOKEN_ESTIMATE,
                   gasPrice: '0x3b9aca00', // 1000000000
@@ -2586,7 +2595,7 @@ describe('Send Slice', () => {
               type: AssetType.native,
             },
             fromAccount: {
-              address: '0xAddress',
+              address: mockAddress1,
               balance: '0x0',
             },
             gas: {
@@ -2697,16 +2706,18 @@ describe('Send Slice', () => {
           addressBook: {
             [CHAIN_IDS.GOERLI]: {},
           },
-          identities: {},
+          identities: {
+            [mockAddress1]: {},
+          },
           accounts: {
-            '0xAddress': {
+            [mockAddress1]: {
-              address: '0xAddress',
+              address: mockAddress1,
               balance: '0x0',
             },
           },
           cachedBalances: {
             [CHAIN_IDS.GOERLI]: {
-              '0xAddress': '0x0',
+              [mockAddress1]: '0x0',
             },
           },
           unapprovedTxs: {
@@ -2722,7 +2733,7 @@ describe('Send Slice', () => {
                     decimals: 18,
                   },
                 }),
-                from: '0xAddress',
+                from: mockAddress1,
                 to: '0xTokenAddress',
                 gas: GAS_LIMITS.BASE_TOKEN_ESTIMATE,
                 gasPrice: '0x3b9aca00', // 1000000000
@@ -2740,7 +2751,7 @@ describe('Send Slice', () => {
             },
           }),
           selectedAccount: {
-            address: '0xAddress',
+            address: mockAddress1,
             balance: '0x0',
           },
           stage: SEND_STAGES.EDIT,
@@ -2777,7 +2788,7 @@ describe('Send Slice', () => {
             type: AssetType.native,
           },
           fromAccount: {
-            address: '0xAddress',
+            address: mockAddress1,
             balance: '0x0',
           },
           gas: {

ui/selectors/selectors.js

@@ -281,28 +281,38 @@ export function deprecatedGetCurrentNetworkId(state) {
   return state.metamask.networkId ?? 'loading';
 }
 
+/**
+ * Get MetaMask accounts, including account name and balance.
+ */
 export const getMetaMaskAccounts = createSelector(
-  getMetaMaskAccountsRaw,
+  getMetaMaskIdentities,
+  getMetaMaskAccountBalances,
   getMetaMaskCachedBalances,
-  (currentAccounts, cachedBalances) =>
+  (identities, balances, cachedBalances) =>
-    Object.entries(currentAccounts).reduce(
+    Object.keys(identities).reduce((accounts, address) => {
-      (selectedAccounts, [accountID, account]) => {
+      // TODO: mix in the identity state here as well, consolidating this
-        if (account.balance === null || account.balance === undefined) {
+      // selector with `accountsWithSendEtherInfoSelector`
-          return {
+      let account = {};
-            ...selectedAccounts,
+
-            [accountID]: {
+      if (balances[address]) {
+        account = {
           ...account,
-              balance: cachedBalances && cachedBalances[accountID],
+          ...balances[address],
-            },
         };
       }
-        return {
+
-          ...selectedAccounts,
+      if (account.balance === null || account.balance === undefined) {
-          [accountID]: account,
+        account = {
+          ...account,
+          balance: cachedBalances && cachedBalances[address],
         };
-      },
+      }
-      {},
+
-    ),
+      return {
+        ...accounts,
+        [address]: account,
+      };
+    }, {}),
 );
 
 export function getSelectedAddress(state) {
@@ -325,11 +335,23 @@ export function getMetaMaskKeyrings(state) {
   return state.metamask.keyrings;
 }
 
+/**
+ * Get identity state.
+ *
+ * @param {object} state - Redux state
+ * @returns {object} A map of account addresses to identities (which includes the account name)
+ */
 export function getMetaMaskIdentities(state) {
   return state.metamask.identities;
 }
 
-export function getMetaMaskAccountsRaw(state) {
+/**
+ * Get account balances state.
+ *
+ * @param {object} state - Redux state
+ * @returns {object} A map of account addresses to account objects (which includes the account balance)
+ */
+export function getMetaMaskAccountBalances(state) {
   return state.metamask.accounts;
 }
 
@@ -368,7 +390,7 @@ export const getMetaMaskAccountsConnected = createSelector(
 
 export function isBalanceCached(state) {
   const selectedAccountBalance =
-    state.metamask.accounts[getSelectedAddress(state)].balance;
+    getMetaMaskAccountBalances(state)[getSelectedAddress(state)]?.balance;
   const cachedBalance = getSelectedAccountCachedBalance(state);
 
   return Boolean(!selectedAccountBalance && cachedBalance);

ui/store/actions.test.js

@@ -244,6 +244,9 @@ describe('Actions', () => {
               '0xAnotherAddress': '0x0',
             },
           },
+          identities: {
+            '0xAnotherAddress': {},
+          },
         }),
       );
 
@@ -1876,6 +1879,9 @@ describe('Actions', () => {
                 balance: '0x0',
               },
             },
+            identities: {
+              '0xFirstAddress': {},
+            },
             cachedBalances: {
               '0x1': {
                 '0xFirstAddress': '0x0',