From 3044aa0ebe7aee8c9fb316231d5c92bb4e4becc1 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Sat, 26 Aug 2023 00:28:24 -0230 Subject: [PATCH] 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> --- .../edit-gas-fee-popover.test.js | 3 + .../edit-gas-item/edit-gas-item.test.js | 3 + .../edit-gas-tooltip/edit-gas-tooltip.test.js | 3 + .../gas-details-item-title.test.js | 3 + .../custody-confirm-link-modal.js | 4 +- ui/ducks/send/send.test.js | 15 ++++- ui/selectors/selectors.js | 66 ++++++++++++------- ui/store/actions.test.js | 6 ++ 8 files changed, 76 insertions(+), 27 deletions(-) diff --git a/ui/components/app/edit-gas-fee-popover/edit-gas-fee-popover.test.js b/ui/components/app/edit-gas-fee-popover/edit-gas-fee-popover.test.js index 3e371a05d..7d9c0adfa 100644 --- a/ui/components/app/edit-gas-fee-popover/edit-gas-fee-popover.test.js +++ b/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, diff --git a/ui/components/app/edit-gas-fee-popover/edit-gas-item/edit-gas-item.test.js b/ui/components/app/edit-gas-fee-popover/edit-gas-item/edit-gas-item.test.js index 44ac34b87..86f5ba097 100644 --- a/ui/components/app/edit-gas-fee-popover/edit-gas-item/edit-gas-item.test.js +++ b/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', diff --git a/ui/components/app/edit-gas-fee-popover/edit-gas-tooltip/edit-gas-tooltip.test.js b/ui/components/app/edit-gas-fee-popover/edit-gas-tooltip/edit-gas-tooltip.test.js index dbeda43ac..97797c110 100644 --- a/ui/components/app/edit-gas-fee-popover/edit-gas-tooltip/edit-gas-tooltip.test.js +++ b/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 }, }, diff --git a/ui/components/app/gas-details-item/gas-details-item-title/gas-details-item-title.test.js b/ui/components/app/gas-details-item/gas-details-item-title/gas-details-item-title.test.js index 9d8f36c4e..b57c7e32e 100644 --- a/ui/components/app/gas-details-item/gas-details-item-title/gas-details-item-title.test.js +++ b/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', }, }); diff --git a/ui/components/institutional/custody-confirm-link-modal/custody-confirm-link-modal.js b/ui/components/institutional/custody-confirm-link-modal/custody-confirm-link-modal.js index 430f1d4b9..815c4b9bf 100644 --- a/ui/components/institutional/custody-confirm-link-modal/custody-confirm-link-modal.js +++ b/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); diff --git a/ui/ducks/send/send.test.js b/ui/ducks/send/send.test.js index fa85ee1da..d85725de8 100644 --- a/ui/ducks/send/send.test.js +++ b/ui/ducks/send/send.test.js @@ -1598,6 +1598,9 @@ describe('Send Slice', () => { address: mockAddress1, }, }, + identities: { + [mockAddress1]: {}, + }, }, send: { ...getInitialSendStateWithExistingTxState({ @@ -2387,7 +2390,9 @@ describe('Send Slice', () => { addressBook: { [CHAIN_IDS.GOERLI]: {}, }, - identities: {}, + identities: { + [mockAddress1]: {}, + }, accounts: { [mockAddress1]: { address: mockAddress1, @@ -2522,7 +2527,9 @@ describe('Send Slice', () => { addressBook: { [CHAIN_IDS.GOERLI]: {}, }, - identities: {}, + identities: { + [mockAddress1]: {}, + }, accounts: { [mockAddress1]: { address: mockAddress1, @@ -2705,7 +2712,9 @@ describe('Send Slice', () => { addressBook: { [CHAIN_IDS.GOERLI]: {}, }, - identities: {}, + identities: { + [mockAddress1]: {}, + }, accounts: { [mockAddress1]: { address: mockAddress1, diff --git a/ui/selectors/selectors.js b/ui/selectors/selectors.js index a88b92797..a6d878d71 100644 --- a/ui/selectors/selectors.js +++ b/ui/selectors/selectors.js @@ -290,28 +290,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) => - Object.entries(currentAccounts).reduce( - (selectedAccounts, [accountID, account]) => { - if (account.balance === null || account.balance === undefined) { - return { - ...selectedAccounts, - [accountID]: { - ...account, - balance: cachedBalances && cachedBalances[accountID], - }, - }; - } - return { - ...selectedAccounts, - [accountID]: account, + (identities, balances, cachedBalances) => + Object.keys(identities).reduce((accounts, address) => { + // TODO: mix in the identity state here as well, consolidating this + // selector with `accountsWithSendEtherInfoSelector` + let account = {}; + + if (balances[address]) { + account = { + ...account, + ...balances[address], }; - }, - {}, - ), + } + + if (account.balance === null || account.balance === undefined) { + account = { + ...account, + balance: cachedBalances && cachedBalances[address], + }; + } + + return { + ...accounts, + [address]: account, + }; + }, {}), ); export function getSelectedAddress(state) { @@ -334,11 +344,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; } @@ -376,8 +398,8 @@ export const getMetaMaskAccountsConnected = createSelector( ); export function isBalanceCached(state) { - const selectedAccount = state.metamask.accounts[getSelectedAddress(state)]; - const selectedAccountBalance = selectedAccount && selectedAccount.balance; + const selectedAccountBalance = + getMetaMaskAccountBalances(state)[getSelectedAddress(state)]?.balance; const cachedBalance = getSelectedAccountCachedBalance(state); return Boolean(!selectedAccountBalance && cachedBalance); diff --git a/ui/store/actions.test.js b/ui/store/actions.test.js index f8ef8ec1e..86973d737 100644 --- a/ui/store/actions.test.js +++ b/ui/store/actions.test.js @@ -244,6 +244,9 @@ describe('Actions', () => { '0xAnotherAddress': '0x0', }, }, + identities: { + '0xAnotherAddress': {}, + }, }), ); @@ -1942,6 +1945,9 @@ describe('Actions', () => { balance: '0x0', }, }, + identities: { + '0xFirstAddress': {}, + }, cachedBalances: { '0x1': { '0xFirstAddress': '0x0',