fix: Cherry pick remove selectIdentities in favour of selectInternalA…

…ccounts #9724 (#10244) # Description - Cherry pick #9724 into the release. # Why? - fixes: #10161 - fixes: #10155 - fixes: #10079 # Testing - See [here](#9724 (comment)) and [here](#9724 (comment)) for manual testing steps. ## E2E - E2E test results can be found [here](https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/2618a2ea-ac7f-47f0-8e32-519232a48739).
MetaMask · Jul 5, 2024 · a2c0508 · a2c0508
1 parent cb511d6
commit a2c0508
Show file tree

Hide file tree

Showing 56 changed files with 1,141 additions and 1,119 deletions.
diff --git a/app/components/UI/AccountApproval/__snapshots__/index.test.tsx.snap b/app/components/UI/AccountApproval/__snapshots__/index.test.tsx.snap
@@ -334,7 +334,7 @@ exports[`AccountApproval should render correctly 1`] = `
               }
             }
           >
-            0xC496...a756
+            Account 2
           </Text>
           <Text
             accessibilityRole="text"

diff --git a/app/components/UI/AccountFromToInfoCard/AccountFromToInfoCard.test.tsx b/app/components/UI/AccountFromToInfoCard/AccountFromToInfoCard.test.tsx
@@ -39,19 +39,6 @@ const mockInitialState = {
           '0x326836cc6cd09B5aa59B81A7F72F25FcC0136b95': '0x5',
         },
       },
-      PreferencesController: {
-        selectedAddress: MOCK_ADDRESS_1,
-        identities: {
-          [MOCK_ADDRESS_1]: {
-            address: MOCK_ADDRESS_1,
-            name: 'Account 1',
-          },
-          [MOCK_ADDRESS_2]: {
-            address: MOCK_ADDRESS_2,
-            name: 'Account 2',
-          },
-        },
-      },
       AccountsController: MOCK_ACCOUNTS_CONTROLLER_STATE,
     },
   },

diff --git a/app/components/UI/AccountFromToInfoCard/AccountFromToInfoCard.tsx b/app/components/UI/AccountFromToInfoCard/AccountFromToInfoCard.tsx
@@ -9,7 +9,6 @@ import {
   selectChainId,
   selectTicker,
 } from '../../../selectors/networkController';
-import { selectIdentities } from '../../../selectors/preferencesController';
 import { collectConfusables } from '../../../util/confusables';
 import { decodeTransferData } from '../../../util/transactions';
 import { doENSReverseLookup } from '../../../util/ENSUtils';
@@ -20,10 +19,13 @@ import useExistingAddress from '../../hooks/useExistingAddress';
 import { AddressFrom, AddressTo } from '../AddressInputs';
 import createStyles from './AccountFromToInfoCard.styles';
 import { AccountFromToInfoCardProps } from './AccountFromToInfoCard.types';
+import { selectInternalAccounts } from '../../../selectors/accountsController';
+import { toLowerCaseEquals } from '../../../util/general';
+import { RootState } from '../../../reducers';
 
 const AccountFromToInfoCard = (props: AccountFromToInfoCardProps) => {
   const {
-    identities,
+    internalAccounts,
     chainId,
     onPressFromAddressIcon,
     ticker,
@@ -33,7 +35,6 @@ const AccountFromToInfoCard = (props: AccountFromToInfoCardProps) => {
   const {
     transaction: { from: rawFromAddress, data, to },
     transactionTo,
-    transactionToName,
     transactionFromName,
     selectedAsset,
     ensRecipient,
@@ -56,50 +57,88 @@ const AccountFromToInfoCard = (props: AccountFromToInfoCardProps) => {
   );
 
   useEffect(() => {
-    if (!fromAddress) {
-      return;
-    }
-    if (transactionFromName) {
-      setFromAccountName(transactionFromName);
-      return;
-    }
-    (async () => {
+    const fetchFromAccountDetails = async () => {
+      if (!fromAddress) {
+        return;
+      }
+
+      if (transactionFromName) {
+        if (fromAccountName !== transactionFromName) {
+          setFromAccountName(transactionFromName);
+        }
+        return;
+      }
+
       const fromEns = await doENSReverseLookup(fromAddress, chainId);
       if (fromEns) {
-        setFromAccountName(fromEns);
+        if (fromAccountName !== fromEns) {
+          setFromAccountName(fromEns);
+        }
       } else {
-        const { name: fromName } = identities[fromAddress];
-        setFromAccountName(fromName);
+        const accountWithMatchingFromAddress = internalAccounts.find(
+          (account) => toLowerCaseEquals(account.address, fromAddress),
+        );
+
+        const newName = accountWithMatchingFromAddress
+          ? accountWithMatchingFromAddress.metadata.name
+          : fromAddress;
+
+        if (fromAccountName !== newName) {
+          setFromAccountName(newName);
+        }
       }
-    })();
-  }, [fromAddress, identities, transactionFromName, chainId]);
+    };
+
+    fetchFromAccountDetails();
+  }, [
+    fromAddress,
+    transactionFromName,
+    chainId,
+    internalAccounts,
+    fromAccountName,
+  ]);
 
   useEffect(() => {
-    if (existingToAddress) {
-      setToAccountName(existingToAddress?.name);
-      return;
-    }
-    (async () => {
+    const fetchAccountDetails = async () => {
+      if (existingToAddress) {
+        if (toAccountName !== existingToAddress.name) {
+          setToAccountName(existingToAddress.name);
+        }
+        return;
+      }
+
       const toEns = await doENSReverseLookup(toAddress, chainId);
       if (toEns) {
-        setToAccountName(toEns);
-      } else if (identities[toAddress]) {
-        const { name: toName } = identities[toAddress];
-        setToAccountName(toName);
+        if (toAccountName !== toEns) {
+          setToAccountName(toEns);
+        }
+      } else {
+        const accountWithMatchingToAddress = internalAccounts.find((account) =>
+          toLowerCaseEquals(account.address, toAddress),
+        );
+
+        const newName = accountWithMatchingToAddress
+          ? accountWithMatchingToAddress.metadata.name
+          : toAddress;
+
+        if (toAccountName !== newName) {
+          setToAccountName(newName);
+        }
       }
-    })();
-  }, [existingToAddress, identities, chainId, toAddress, transactionToName]);
+    };
+
+    fetchAccountDetails();
+  }, [existingToAddress, chainId, toAddress, internalAccounts, toAccountName]);
 
   useEffect(() => {
-    const accountNames =
-      (identities &&
-        Object.keys(identities).map((hash) => identities[hash].name)) ||
-      [];
+    const accountNames = internalAccounts.map(
+      (account) => account.metadata.name,
+    );
     const isOwnAccount = ensRecipient && accountNames.includes(ensRecipient);
     if (ensRecipient && !isOwnAccount) {
       setConfusableCollection(collectConfusables(ensRecipient));
     }
-  }, [identities, ensRecipient]);
+  }, [internalAccounts, ensRecipient]);
 
   useEffect(() => {
     let toAddr;
@@ -172,8 +211,8 @@ const AccountFromToInfoCard = (props: AccountFromToInfoCardProps) => {
   );
 };
 
-const mapStateToProps = (state: any) => ({
-  identities: selectIdentities(state),
+const mapStateToProps = (state: RootState) => ({
+  internalAccounts: selectInternalAccounts(state),
   chainId: selectChainId(state),
   ticker: selectTicker(state),
 });

diff --git a/app/components/UI/AccountFromToInfoCard/AccountFromToInfoCard.types.tsx b/app/components/UI/AccountFromToInfoCard/AccountFromToInfoCard.types.tsx
@@ -1,9 +1,4 @@
-interface Identity {
-  address: string;
-  name: string;
-}
-
-type Identities = Record<string, Identity>;
+import { InternalAccount } from '@metamask/keyring-api';
 
 interface SelectedAsset {
   isETH: boolean;
@@ -26,7 +21,7 @@ export interface Transaction {
 }
 
 export interface AccountFromToInfoCardProps {
-  identities: Identities;
+  internalAccounts: InternalAccount[];
   chainId: string;
   onPressFromAddressIcon?: () => void;
   ticker?: string;

diff --git a/app/components/UI/AccountInfoCard/__snapshots__/index.test.tsx.snap b/app/components/UI/AccountInfoCard/__snapshots__/index.test.tsx.snap
@@ -232,7 +232,7 @@ exports[`AccountInfoCard should match snapshot 1`] = `
           }
         }
       >
-        0xC495...D272
+        Account 1
       </Text>
       <Text
         accessibilityRole="text"

diff --git a/app/components/UI/AccountInfoCard/index.js b/app/components/UI/AccountInfoCard/index.js
@@ -25,11 +25,11 @@ import {
   selectCurrentCurrency,
 } from '../../../selectors/currencyRateController';
 import { selectAccounts } from '../../../selectors/accountTrackerController';
-import { selectIdentities } from '../../../selectors/preferencesController';
 import ApproveTransactionHeader from '../../Views/confirmations/components/ApproveTransactionHeader';
 import Text, {
   TextVariant,
 } from '../../../component-library/components/Texts/Text';
+import { selectInternalAccounts } from '../../../selectors/accountsController';
 
 const createStyles = (colors) =>
   StyleSheet.create({
@@ -109,9 +109,9 @@ class AccountInfoCard extends PureComponent {
      */
     accounts: PropTypes.object,
     /**
-     * List of accounts from the PreferencesController
+     * List of accounts from the AccountsController
      */
-    identities: PropTypes.object,
+    internalAccounts: PropTypes.array,
     /**
      * A number that specifies the ETH/USD conversion rate
      */
@@ -140,7 +140,7 @@ class AccountInfoCard extends PureComponent {
   render() {
     const {
       accounts,
-      identities,
+      internalAccounts,
       conversionRate,
       currentCurrency,
       operation,
@@ -160,7 +160,7 @@ class AccountInfoCard extends PureComponent {
       ? hexToBN(accounts[fromAddress].balance)
       : 0;
     const balance = `${renderFromWei(weiBalance)} ${getTicker(ticker)}`;
-    const accountLabel = renderAccountName(fromAddress, identities);
+    const accountLabel = renderAccountName(fromAddress, internalAccounts);
     const address = renderShortAddress(fromAddress);
     const dollarBalance = showFiatBalance
       ? weiToFiat(weiBalance, conversionRate, currentCurrency, 2)?.toUpperCase()
@@ -226,7 +226,7 @@ class AccountInfoCard extends PureComponent {
 
 const mapStateToProps = (state) => ({
   accounts: selectAccounts(state),
-  identities: selectIdentities(state),
+  internalAccounts: selectInternalAccounts(state),
   conversionRate: selectConversionRate(state),
   currentCurrency: selectCurrentCurrency(state),
   ticker: selectTicker(state),

diff --git a/app/components/UI/AccountOverview/index.js b/app/components/UI/AccountOverview/index.js
@@ -41,14 +41,17 @@ import AppConstants from '../../../core/AppConstants';
 import Engine from '../../../core/Engine';
 import { selectChainId } from '../../../selectors/networkController';
 import { selectCurrentCurrency } from '../../../selectors/currencyRateController';
-import { selectIdentities } from '../../../selectors/preferencesController';
-import { selectSelectedInternalAccountChecksummedAddress } from '../../../selectors/accountsController';
+import {
+  selectInternalAccounts,
+  selectSelectedInternalAccountChecksummedAddress,
+} from '../../../selectors/accountsController';
 import { createAccountSelectorNavDetails } from '../../Views/AccountSelector';
 import Text, {
   TextVariant,
 } from '../../../component-library/components/Texts/Text';
 import { withMetricsAwareness } from '../../../components/hooks/useMetrics';
 import { isPortfolioUrl } from '../../../util/url';
+import { toLowerCaseEquals } from '../../../util/general';
 
 const createStyles = (colors) =>
   StyleSheet.create({
@@ -157,9 +160,9 @@ class AccountOverview extends PureComponent {
      */
     selectedAddress: PropTypes.string,
     /**
-    /* Identities object required to get account name
+    /* InternalAccounts object required to get account name
     */
-    identities: PropTypes.object,
+    internalAccounts: PropTypes.object,
     /**
      * Object that represents the selected account
      */
@@ -234,8 +237,8 @@ class AccountOverview extends PureComponent {
   input = React.createRef();
 
   componentDidMount = () => {
-    const { identities, selectedAddress, onRef } = this.props;
-    const accountLabel = renderAccountName(selectedAddress, identities);
+    const { internalAccounts, selectedAddress, onRef } = this.props;
+    const accountLabel = renderAccountName(selectedAddress, internalAccounts);
     this.setState({ accountLabel });
     onRef && onRef(this);
     InteractionManager.runAfterInteractions(() => {
@@ -259,16 +262,18 @@ class AccountOverview extends PureComponent {
   }
 
   setAccountLabel = () => {
-    const { selectedAddress, identities } = this.props;
+    const { selectedAddress, internalAccounts } = this.props;
     const { accountLabel } = this.state;
 
-    const lastAccountLabel = identities[selectedAddress].name;
+    const accountWithMatchingToAddress = internalAccounts.find((account) =>
+      toLowerCaseEquals(account.address, selectedAddress),
+    );
 
     Engine.setAccountLabel(
       selectedAddress,
       this.isAccountLabelDefined(accountLabel)
         ? accountLabel
-        : lastAccountLabel,
+        : accountWithMatchingToAddress.metadata.name,
     );
     this.setState({ accountLabelEditable: false });
   };
@@ -278,17 +283,17 @@ class AccountOverview extends PureComponent {
   };
 
   setAccountLabelEditable = () => {
-    const { identities, selectedAddress } = this.props;
-    const accountLabel = renderAccountName(selectedAddress, identities);
+    const { internalAccounts, selectedAddress } = this.props;
+    const accountLabel = renderAccountName(selectedAddress, internalAccounts);
     this.setState({ accountLabelEditable: true, accountLabel });
     setTimeout(() => {
       this.input && this.input.current && this.input.current.focus();
     }, 100);
   };
 
   cancelAccountLabelEdition = () => {
-    const { identities, selectedAddress } = this.props;
-    const accountLabel = renderAccountName(selectedAddress, identities);
+    const { internalAccounts, selectedAddress } = this.props;
+    const accountLabel = renderAccountName(selectedAddress, internalAccounts);
     this.setState({ accountLabelEditable: false, accountLabel });
   };
 
@@ -461,7 +466,7 @@ class AccountOverview extends PureComponent {
 
 const mapStateToProps = (state) => ({
   selectedAddress: selectSelectedInternalAccountChecksummedAddress(state),
-  identities: selectIdentities(state),
+  internalAccounts: selectInternalAccounts(state),
   currentCurrency: selectCurrentCurrency(state),
   chainId: selectChainId(state),
   browserTabs: state.browser.tabs,
-Original file line number
+Diff line change
@@ Expand Up / @@ -334,7 +334,7 @@ exports[`AccountApproval should render correctly 1`] = ` @@
                   }
                 }
               >
-xC496...a756
+                Account 2
               </Text>
               <Text
                 accessibilityRole="text"
@@ Expand Down @@