refactor: remove global network usage from signature confirmations (#…

…12905) ## **Description** Remove all references to the global network from signature confirmations. Specifically: - Remove usages of `selectChainId` selector. - Remove usages of `selectProviderType` selector. - Add `useSignatureRequest` hook to easily retrieve current `signatureRequest` data. - Add `selectSignatureRequestById` selector. ## **Related issues** Fixes: [#2024](MetaMask/mobile-planning#2024) ## **Manual testing steps** ## **Screenshots/Recordings** ### **Before** ### **After** ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/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-mobile/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.
MetaMask · Jan 13, 2025 · 68efd5e · 68efd5e
1 parent 8873337
commit 68efd5e
Show file tree

Hide file tree

Showing 14 changed files with 296 additions and 47 deletions.
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -77,7 +77,12 @@ module.exports = {
       files: [
         'app/components/UI/Name/**/*.{js,ts,tsx}',
         'app/components/UI/SimulationDetails/**/*.{js,ts,tsx}',
-        'app/components/hooks/DisplayName/**/*.{js,ts,tsx}'
+        'app/components/hooks/DisplayName/**/*.{js,ts,tsx}',
+        'app/components/Views/confirmations/components/Confirm/DataTree/**/*.{js,ts,tsx}',
+        'app/components/Views/confirmations/components/Confirm/Info/**/*.{js,ts,tsx}',
+        'app/components/Views/confirmations/components/PersonalSign/**/*.{js,ts,tsx}',
+        'app/components/Views/confirmations/components/SignatureRequest/**/*.{js,ts,tsx}',
+        'app/components/Views/confirmations/components/TypedSign/**/*.{js,ts,tsx}',
       ],
       rules: {
         'no-restricted-syntax': [

diff --git a/app/components/Views/confirmations/components/Confirm/Info/TypedSignV1/Message.tsx b/app/components/Views/confirmations/components/Confirm/Info/TypedSignV1/Message.tsx
@@ -1,10 +1,7 @@
 import React from 'react';
-import { useSelector } from 'react-redux';
-
-import { selectChainId } from '../../../../../../../selectors/networkController';
-import useApprovalRequest from '../../../../hooks/useApprovalRequest';
 import SignatureMessageSection from '../../SignatureMessageSection';
 import DataTree, { DataTreeInput } from '../../DataTree/DataTree';
+import { useSignatureRequest } from '../../../../hooks/useSignatureRequest';
 
 interface TypesSignDataV1 {
   name: string;
@@ -13,10 +10,11 @@ interface TypesSignDataV1 {
 }
 
 const Message = () => {
-  const { approvalRequest } = useApprovalRequest();
-  const chainId = useSelector(selectChainId);
+  const signatureRequest = useSignatureRequest();
+  const chainId = signatureRequest?.chainId as string;
 
-  const typedSignData = approvalRequest?.requestData?.data;
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  const typedSignData = signatureRequest?.messageParams?.data as any;
 
   if (!typedSignData) {
     return null;

diff --git a/app/components/Views/confirmations/components/Confirm/Info/TypedSignV3V4/Message.tsx b/app/components/Views/confirmations/components/Confirm/Info/TypedSignV3V4/Message.tsx
@@ -1,15 +1,14 @@
 import React from 'react';
 import { StyleSheet } from 'react-native';
-import { useSelector } from 'react-redux';
 
 import { strings } from '../../../../../../../../locales/i18n';
-import { selectChainId } from '../../../../../../../selectors/networkController';
-import useApprovalRequest from '../../../../hooks/useApprovalRequest';
 import { parseSanitizeTypedDataMessage } from '../../../../utils/signatures';
 import InfoRow from '../../../UI/InfoRow';
 import DataTree from '../../DataTree';
 import SignatureMessageSection from '../../SignatureMessageSection';
 import { DataTreeInput } from '../../DataTree/DataTree';
+import { useSignatureRequest } from '../../../../hooks/useSignatureRequest';
+import { Hex } from '@metamask/utils';
 
 const styles = StyleSheet.create({
   collpasedInfoRow: {
@@ -18,10 +17,12 @@ const styles = StyleSheet.create({
 });
 
 const Message = () => {
-  const { approvalRequest } = useApprovalRequest();
-  const chainId = useSelector(selectChainId);
+  const signatureRequest = useSignatureRequest();
+  const chainId = signatureRequest?.chainId as Hex;
 
-  const typedSignData = approvalRequest?.requestData?.data;
+  // Pending alignment of controller types.
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  const typedSignData = signatureRequest?.messageParams?.data as any;
 
   if (!typedSignData) {
     return null;

diff --git a/app/components/Views/confirmations/components/PersonalSign/PersonalSign.tsx b/app/components/Views/confirmations/components/PersonalSign/PersonalSign.tsx
@@ -23,13 +23,15 @@ import { PersonalSignProps } from './types';
 import { SigningBottomSheetSelectorsIDs } from '../../../../../../e2e/selectors/Browser/SigningBottomSheet.selectors';
 import { useMetrics } from '../../../../../components/hooks/useMetrics';
 import AppConstants from '../../../../../core/AppConstants';
-import { selectChainId } from '../../../../../selectors/networkController';
-import { store } from '../../../../../store';
 import Logger from '../../../../../util/Logger';
 import { getBlockaidMetricsParams } from '../../../../../util/blockaid';
 import createExternalSignModelNav from '../../../../../util/hardwareWallet/signatureUtils';
 import { getDecimalChainId } from '../../../../../util/networks';
 import { SecurityAlertResponse } from '../BlockaidBanner/BlockaidBanner.types';
+import { selectSignatureRequestById } from '../../../../../selectors/signatureController';
+import { selectNetworkTypeByChainId } from '../../../../../selectors/networkController';
+import { RootState } from '../../../../../reducers';
+import { Hex } from '@metamask/utils';
 
 /**
  * Converts a hexadecimal string to a utf8 string.
@@ -63,12 +65,23 @@ const PersonalSign = ({
   const navigation = useNavigation();
   const { trackEvent, createEventBuilder } = useMetrics();
   const [truncateMessage, setTruncateMessage] = useState<boolean>(false);
+
   const { securityAlertResponse } = useSelector(
     // TODO: Replace "any" with type
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
     (reduxState: any) => reduxState.signatureRequest,
   );
 
+  const signatureRequest = useSelector((state: RootState) =>
+    selectSignatureRequestById(state, messageParams.metamaskId),
+  );
+
+  const { chainId } = signatureRequest ?? {};
+
+  const networkType = useSelector((state: RootState) =>
+    selectNetworkTypeByChainId(state, chainId as Hex),
+  );
+
   // TODO: Replace "any" with type
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
   const { colors }: any = useTheme();
@@ -84,8 +97,6 @@ const PersonalSign = ({
 
   const getAnalyticsParams = useCallback((): AnalyticsParams => {
     const pageInfo = currentPageInformation || messageParams.meta || {};
-
-    const chainId = selectChainId(store.getState());
     const fallbackUrl = 'N/A';
 
     let urlHost = fallbackUrl;
@@ -101,7 +112,7 @@ const PersonalSign = ({
     let blockaidParams: Record<string, unknown> = {};
     if (securityAlertResponse) {
       blockaidParams = getBlockaidMetricsParams(
-        securityAlertResponse as SecurityAlertResponse,
+        securityAlertResponse as unknown as SecurityAlertResponse,
       );
     }
 
@@ -113,7 +124,7 @@ const PersonalSign = ({
       ...pageInfo.analytics,
       ...blockaidParams,
     };
-  }, [currentPageInformation, messageParams, securityAlertResponse]);
+  }, [chainId, currentPageInformation, messageParams, securityAlertResponse]);
 
   useEffect(() => {
     const onSignatureError = ({ error }: { error: Error }) => {
@@ -259,6 +270,7 @@ const PersonalSign = ({
       fromAddress={messageParams.from}
       origin={messageParams.origin}
       testID={SigningBottomSheetSelectorsIDs.PERSONAL_REQUEST}
+      networkType={networkType}
     >
       <View style={styles.messageWrapper}>{renderMessageText()}</View>
     </SignatureRequest>

diff --git a/app/components/Views/confirmations/components/PersonalSign/index.test.tsx b/app/components/Views/confirmations/components/PersonalSign/index.test.tsx
@@ -2,7 +2,7 @@ import React from 'react';
 import { shallow } from 'enzyme';
 import PersonalSign from '.';
 import configureMockStore from 'redux-mock-store';
-import { Provider } from 'react-redux';
+import { Provider, useSelector } from 'react-redux';
 import { WALLET_CONNECT_ORIGIN } from '../../../../../util/walletconnect';
 import SignatureRequest from '../SignatureRequest';
 import Engine from '../../../../../core/Engine';
@@ -12,6 +12,7 @@ import AppConstants from '../../../../../core/AppConstants';
 import { strings } from '../../../../../../locales/i18n';
 import { backgroundState } from '../../../../../util/test/initial-root-state';
 import { useMetrics } from '../../../../../components/hooks/useMetrics';
+import initialBackgroundState from '../../../../../util/test/initial-background-state.json';
 
 jest.mock('../../../../../components/hooks/useMetrics');
 jest.mock('../../../../../core/Engine', () => ({
@@ -61,20 +62,7 @@ const store = mockStore(initialState);
 
 jest.mock('react-redux', () => ({
   ...jest.requireActual('react-redux'),
-  // TODO: Replace "any" with type
-  // eslint-disable-next-line @typescript-eslint/no-explicit-any
-  useSelector: (callback: any) =>
-    callback({
-      signatureRequest: {
-        securityAlertResponse: {
-          description: '',
-          features: [],
-          providerRequestsCount: { eth_chainId: 1 },
-          reason: '',
-          result_type: 'Benign',
-        },
-      },
-    }),
+  useSelector: jest.fn(),
 }));
 
 jest.mock('../../../../../util/address', () => ({
@@ -117,6 +105,51 @@ function createWrapper({
 }
 
 describe('PersonalSign', () => {
+  const useSelectorMock = jest.mocked(useSelector);
+
+  beforeEach(() => {
+    useSelectorMock.mockReset();
+
+    useSelectorMock.mockImplementationOnce((callback) =>
+      callback({
+        signatureRequest: {
+          securityAlertResponse: {
+            description: '',
+            features: [],
+            providerRequestsCount: { eth_chainId: 1 },
+            reason: '',
+            result_type: 'Benign',
+          },
+        },
+      }),
+    );
+
+    useSelectorMock.mockImplementationOnce((callback) =>
+      callback({
+        engine: {
+          backgroundState: {
+            SignatureController: {
+              signatureRequests: {
+                [messageParamsMock.metamaskId]: {
+                  chainId: '1',
+                  messageParams: messageParamsMock,
+                },
+              },
+            },
+          },
+        },
+      }),
+    );
+
+    useSelectorMock.mockImplementationOnce((callback) =>
+      callback({
+        engine: {
+          backgroundState: initialBackgroundState,
+        },
+      }),
+    );
+  });
+
   it('should render correctly', () => {
     const wrapper = createWrapper();
     expect(wrapper).toMatchSnapshot();

diff --git a/app/components/Views/confirmations/components/SignatureRequest/index.js b/app/components/Views/confirmations/components/SignatureRequest/index.js
@@ -9,7 +9,6 @@ import { withMetricsAwareness } from '../../../../../components/hooks/useMetrics
 import ExtendedKeyringTypes from '../../../../../constants/keyringTypes';
 import { MetaMetricsEvents } from '../../../../../core/Analytics';
 import { selectSelectedInternalAccountFormattedAddress } from '../../../../../selectors/accountsController';
-import { selectProviderType } from '../../../../../selectors/networkController';
 import { fontStyles } from '../../../../../styles/common';
 import { isHardwareAccount } from '../../../../../util/address';
 import { getAnalyticsParams } from '../../../../../util/confirmation/signatureUtils';
@@ -149,7 +148,7 @@ class SignatureRequest extends PureComponent {
      */
     type: PropTypes.string,
     /**
-     * String representing the selected network
+     * String representing the associated network
      */
     networkType: PropTypes.string,
     /**
@@ -401,7 +400,6 @@ class SignatureRequest extends PureComponent {
 
 const mapStateToProps = (state) => ({
   selectedAddress: selectSelectedInternalAccountFormattedAddress(state),
-  networkType: selectProviderType(state),
   securityAlertResponse: state.signatureRequest.securityAlertResponse,
 });
 

diff --git a/app/components/Views/confirmations/components/TypedSign/index.js b/app/components/Views/confirmations/components/TypedSign/index.js
@@ -28,6 +28,8 @@ import { isExternalHardwareAccount } from '../../../../../util/address';
 import createExternalSignModelNav from '../../../../../util/hardwareWallet/signatureUtils';
 import { SigningBottomSheetSelectorsIDs } from '../../../../../../e2e/selectors/Browser/SigningBottomSheet.selectors';
 import { withMetricsAwareness } from '../../../../../components/hooks/useMetrics';
+import { selectNetworkTypeByChainId } from '../../../../../selectors/networkController';
+import { selectSignatureRequestById } from '../../../../../selectors/signatureController';
 
 const createStyles = (colors) =>
   StyleSheet.create({
@@ -95,6 +97,10 @@ class TypedSign extends PureComponent {
      * Metrics injected by withMetricsAwareness HOC
      */
     metrics: PropTypes.object,
+    /**
+     * String representing the associated network
+     */
+    networkType: PropTypes.string,
   };
 
   state = {
@@ -242,6 +248,7 @@ class TypedSign extends PureComponent {
       showExpandedMessage,
       toggleExpandedMessage,
       messageParams: { from },
+      networkType
     } = this.props;
     const { truncateMessage } = this.state;
     const messageWrapperStyles = [];
@@ -251,6 +258,7 @@ class TypedSign extends PureComponent {
     if (messageParams.version === 'V3') {
       domain = JSON.parse(messageParams.data).domain;
     }
+
     if (truncateMessage) {
       messageWrapperStyles.push(styles.truncatedMessageWrapper);
       if (Device.isIos()) {
@@ -278,6 +286,7 @@ class TypedSign extends PureComponent {
         type={typedSign[messageParams.version]}
         fromAddress={from}
         testID={SigningBottomSheetSelectorsIDs.TYPED_REQUEST}
+        networkType={networkType}
       >
         <View
           style={messageWrapperStyles}
@@ -293,8 +302,13 @@ class TypedSign extends PureComponent {
 
 TypedSign.contextType = ThemeContext;
 
-const mapStateToProps = (state) => ({
-  securityAlertResponse: state.signatureRequest.securityAlertResponse,
-});
+const mapStateToProps = (state, ownProps) => {
+  const signatureRequest = selectSignatureRequestById(state, ownProps.messageParams.metamaskId);
+
+  return {
+    networkType: selectNetworkTypeByChainId(state, signatureRequest?.chainId),
+    securityAlertResponse: state.signatureRequest.securityAlertResponse,
+  };
+};
 
 export default connect(mapStateToProps)(withMetricsAwareness(TypedSign));