Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: remove global network usage from signature confirmations #12905

Merged
merged 7 commits into from
Jan 13, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -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': [
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;
matthewwalsh0 marked this conversation as resolved.
Show resolved Hide resolved

if (!typedSignData) {
return null;
Expand Down
Original file line number Diff line number Diff line change
@@ -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: {
Expand All @@ -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;
matthewwalsh0 marked this conversation as resolved.
Show resolved Hide resolved

if (!typedSignData) {
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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,
matthewwalsh0 marked this conversation as resolved.
Show resolved Hide resolved
);

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();
Expand All @@ -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;
Expand All @@ -101,7 +112,7 @@ const PersonalSign = ({
let blockaidParams: Record<string, unknown> = {};
if (securityAlertResponse) {
blockaidParams = getBlockaidMetricsParams(
securityAlertResponse as SecurityAlertResponse,
securityAlertResponse as unknown as SecurityAlertResponse,
);
}

Expand All @@ -113,7 +124,7 @@ const PersonalSign = ({
...pageInfo.analytics,
...blockaidParams,
};
}, [currentPageInformation, messageParams, securityAlertResponse]);
}, [chainId, currentPageInformation, messageParams, securityAlertResponse]);

useEffect(() => {
const onSignatureError = ({ error }: { error: Error }) => {
Expand Down Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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', () => ({
Expand Down Expand Up @@ -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', () => ({
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -149,7 +148,7 @@ class SignatureRequest extends PureComponent {
*/
type: PropTypes.string,
/**
* String representing the selected network
* String representing the associated network
*/
networkType: PropTypes.string,
/**
Expand Down Expand Up @@ -401,7 +400,6 @@ class SignatureRequest extends PureComponent {

const mapStateToProps = (state) => ({
selectedAddress: selectSelectedInternalAccountFormattedAddress(state),
networkType: selectProviderType(state),
securityAlertResponse: state.signatureRequest.securityAlertResponse,
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down Expand Up @@ -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 = {
Expand Down Expand Up @@ -242,6 +248,7 @@ class TypedSign extends PureComponent {
showExpandedMessage,
toggleExpandedMessage,
messageParams: { from },
networkType
} = this.props;
const { truncateMessage } = this.state;
const messageWrapperStyles = [];
Expand All @@ -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()) {
Expand Down Expand Up @@ -278,6 +286,7 @@ class TypedSign extends PureComponent {
type={typedSign[messageParams.version]}
fromAddress={from}
testID={SigningBottomSheetSelectorsIDs.TYPED_REQUEST}
networkType={networkType}
>
<View
style={messageWrapperStyles}
Expand All @@ -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));
Loading
Loading