From 85cf4306af7cd050fe877400074b2a9f7af9c361 Mon Sep 17 00:00:00 2001 From: Jyoti Puri Date: Wed, 12 Jun 2024 18:57:30 +0530 Subject: [PATCH] feat: permit signature copy changes (#24975) --- app/_locales/en/messages.json | 14 ++-- .../permit-simulation.test.tsx.snap | 2 +- .../info/typed-sign/typed-sign.test.tsx | 4 +- .../confirm/info/typed-sign/typed-sign.tsx | 15 ++-- .../components/confirm/title/title.test.tsx | 19 ++++- .../components/confirm/title/title.tsx | 70 +++++++++++-------- ui/pages/confirmations/constants/index.ts | 6 ++ ui/pages/confirmations/utils/confirm.test.ts | 21 ++++++ ui/pages/confirmations/utils/confirm.ts | 19 +++++ 9 files changed, 127 insertions(+), 43 deletions(-) diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json index df4e33085230..fcc425becf68 100644 --- a/app/_locales/en/messages.json +++ b/app/_locales/en/messages.json @@ -521,9 +521,6 @@ "message": "Approved on $1 for $2", "description": "$1 is the approval date for a permission. $2 is the AvatarGroup component displaying account images." }, - "approvingTo": { - "message": "Approving to" - }, "areYouSure": { "message": "Are you sure?" }, @@ -906,9 +903,15 @@ "confirmTitleDescContractInteractionTransaction": { "message": "Only confirm this transaction if you fully understand the content and trust the requesting site." }, + "confirmTitleDescPermitSignature": { + "message": "This site wants permission to spend your tokens." + }, "confirmTitleDescSignature": { "message": "Only confirm this message if you approve the content and trust the requesting site." }, + "confirmTitlePermitSignature": { + "message": "Spending cap request" + }, "confirmTitleSignature": { "message": "Signature request" }, @@ -3788,7 +3791,7 @@ "message": "Connected sites are now permissions" }, "permitSimulationDetailInfo": { - "message": "This transaction gives permission to withdraw your tokens" + "message": "You're giving the spender permission to spend this many tokens from your account." }, "personalAddressDetected": { "message": "Personal address detected. Input the token contract address." @@ -4911,6 +4914,9 @@ "spendLimitTooLarge": { "message": "Spend limit too large" }, + "spender": { + "message": "Spender" + }, "spendingCap": { "message": "Spending cap" }, diff --git a/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/__snapshots__/permit-simulation.test.tsx.snap b/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/__snapshots__/permit-simulation.test.tsx.snap index e96dce4d0201..b5deeb02f612 100644 --- a/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/__snapshots__/permit-simulation.test.tsx.snap +++ b/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/__snapshots__/permit-simulation.test.tsx.snap @@ -40,7 +40,7 @@ exports[`PermitSimulation renders component correctly 1`] = ` class="mm-box mm-text mm-text--body-md mm-box--color-inherit" style="white-space: pre-wrap;" > - This transaction gives permission to withdraw your tokens + You're giving the spender permission to spend this many tokens from your account.

diff --git a/ui/pages/confirmations/components/confirm/info/typed-sign/typed-sign.test.tsx b/ui/pages/confirmations/components/confirm/info/typed-sign/typed-sign.test.tsx index dc1d093d57bb..dd6782af6644 100644 --- a/ui/pages/confirmations/components/confirm/info/typed-sign/typed-sign.test.tsx +++ b/ui/pages/confirmations/components/confirm/info/typed-sign/typed-sign.test.tsx @@ -76,7 +76,7 @@ describe('TypedSignInfo', () => { expect(getByText('Estimated changes')).toBeDefined(); }); - it('displays "Approving to" for permit signature type', () => { + it('displays "Spender" for permit signature type', () => { const state = { ...mockState, confirm: { @@ -85,6 +85,6 @@ describe('TypedSignInfo', () => { }; const mockStore = configureMockStore([])(state); const { getByText } = renderWithProvider(, mockStore); - expect(getByText('Approving to')).toBeDefined(); + expect(getByText('Spender')).toBeDefined(); }); }); diff --git a/ui/pages/confirmations/components/confirm/info/typed-sign/typed-sign.tsx b/ui/pages/confirmations/components/confirm/info/typed-sign/typed-sign.tsx index ed4cb30f3360..5ff582bde243 100644 --- a/ui/pages/confirmations/components/confirm/info/typed-sign/typed-sign.tsx +++ b/ui/pages/confirmations/components/confirm/info/typed-sign/typed-sign.tsx @@ -15,9 +15,11 @@ import { BackgroundColor, BorderRadius, } from '../../../../../../helpers/constants/design-system'; -import { EIP712_PRIMARY_TYPE_PERMIT } from '../../../../constants'; import { SignatureRequestType } from '../../../../types/confirm'; -import { parseTypedDataMessage } from '../../../../utils'; +import { + isPermitSignatureRequest, + parseTypedDataMessage, +} from '../../../../utils'; import { ConfirmInfoRowTypedSignData } from '../../row/typed-sign-data/typedSignData'; import { PermitSimulation } from './permit-simulation'; @@ -33,23 +35,24 @@ const TypedSignInfo: React.FC = () => { const { domain: { verifyingContract }, - primaryType, message: { spender }, } = parseTypedDataMessage(currentConfirmation.msgParams.data as string); + const isPermit = isPermitSignatureRequest(currentConfirmation); + return ( <> - {primaryType === EIP712_PRIMARY_TYPE_PERMIT && } + {isPermit && } - {primaryType === EIP712_PRIMARY_TYPE_PERMIT && ( + {isPermit && ( <> - + diff --git a/ui/pages/confirmations/components/confirm/title/title.test.tsx b/ui/pages/confirmations/components/confirm/title/title.test.tsx index dfd94d6f6617..f0f330653e5c 100644 --- a/ui/pages/confirmations/components/confirm/title/title.test.tsx +++ b/ui/pages/confirmations/components/confirm/title/title.test.tsx @@ -1,6 +1,11 @@ import React from 'react'; import configureMockStore from 'redux-mock-store'; import { TransactionType } from '@metamask/transaction-controller'; + +import { + permitSignatureMsg, + unapprovedTypedSignMsgV4, +} from '../../../../../../test/data/confirmations/typed_sign'; import { renderWithProvider } from '../../../../../../test/lib/render-helpers'; import { Confirmation } from '../../../types/confirm'; import { Severity } from '../../../../../helpers/constants/design-system'; @@ -38,9 +43,21 @@ describe('ConfirmTitle', () => { ).toBeInTheDocument(); }); + it('should render the title and description for a permit signature', () => { + const mockStore = configureMockStore([])( + genMockState(permitSignatureMsg as Confirmation), + ); + const { getByText } = renderWithProvider(, mockStore); + + expect(getByText('Spending cap request')).toBeInTheDocument(); + expect( + getByText('This site wants permission to spend your tokens.'), + ).toBeInTheDocument(); + }); + it('should render the title and description for typed signature', () => { const mockStore = configureMockStore([])( - genMockState({ type: TransactionType.signTypedData }), + genMockState(unapprovedTypedSignMsgV4 as Confirmation), ); const { getByText } = renderWithProvider(, mockStore); diff --git a/ui/pages/confirmations/components/confirm/title/title.tsx b/ui/pages/confirmations/components/confirm/title/title.tsx index ea18fcbadd63..a38abbb55433 100644 --- a/ui/pages/confirmations/components/confirm/title/title.tsx +++ b/ui/pages/confirmations/components/confirm/title/title.tsx @@ -9,10 +9,11 @@ import { } from '../../../../../helpers/constants/design-system'; import { useI18nContext } from '../../../../../hooks/useI18nContext'; import { currentConfirmationSelector } from '../../../../../selectors'; -import { Confirmation } from '../../../types/confirm'; import useAlerts from '../../../../../hooks/useAlerts'; import { getHighestSeverity } from '../../../../../components/app/alert-system/utils'; import GeneralAlert from '../../../../../components/app/alert-system/general-alert/general-alert'; +import { Confirmation, SignatureRequestType } from '../../../types/confirm'; +import { isPermitSignatureRequest } from '../../../utils'; function ConfirmBannerAlert({ ownerId }: { ownerId: string }) { const t = useI18nContext(); @@ -49,45 +50,56 @@ function ConfirmBannerAlert({ ownerId }: { ownerId: string }) { ); } +type IntlFunction = (str: string) => string; + +const getTitle = (t: IntlFunction, confirmation?: Confirmation) => { + switch (confirmation?.type) { + case TransactionType.contractInteraction: + return t('confirmTitleTransaction'); + case TransactionType.personalSign: + return t('confirmTitleSignature'); + case TransactionType.signTypedData: + return isPermitSignatureRequest(confirmation as SignatureRequestType) + ? t('confirmTitlePermitSignature') + : t('confirmTitleSignature'); + default: + return ''; + } +}; + +const getDescription = (t: IntlFunction, confirmation?: Confirmation) => { + switch (confirmation?.type) { + case TransactionType.contractInteraction: + return t('confirmTitleDescContractInteractionTransaction'); + case TransactionType.personalSign: + return t('confirmTitleDescSignature'); + case TransactionType.signTypedData: + return isPermitSignatureRequest(confirmation as SignatureRequestType) + ? t('confirmTitleDescPermitSignature') + : t('confirmTitleDescSignature'); + default: + return ''; + } +}; + const ConfirmTitle: React.FC = memo(() => { const t = useI18nContext(); - const currentConfirmation = useSelector( - currentConfirmationSelector, - ) as Confirmation; + const currentConfirmation = useSelector(currentConfirmationSelector); - const typeToTitleTKey: Partial> = useMemo( - () => ({ - [TransactionType.personalSign]: t('confirmTitleSignature'), - [TransactionType.signTypedData]: t('confirmTitleSignature'), - [TransactionType.contractInteraction]: t('confirmTitleTransaction'), - }), - [], + const title = useMemo( + () => getTitle(t as IntlFunction, currentConfirmation), + [currentConfirmation], ); - const typeToDescTKey: Partial> = useMemo( - () => ({ - [TransactionType.personalSign]: t('confirmTitleDescSignature'), - [TransactionType.signTypedData]: t('confirmTitleDescSignature'), - [TransactionType.contractInteraction]: t( - 'confirmTitleDescContractInteractionTransaction', - ), - }), - [], + const description = useMemo( + () => getDescription(t as IntlFunction, currentConfirmation), + [currentConfirmation], ); if (!currentConfirmation) { return null; } - const title = - typeToTitleTKey[ - currentConfirmation.type || TransactionType.contractInteraction - ]; - const description = - typeToDescTKey[ - currentConfirmation.type || TransactionType.contractInteraction - ]; - return ( <> diff --git a/ui/pages/confirmations/constants/index.ts b/ui/pages/confirmations/constants/index.ts index 88af9e200997..055fcea53f6f 100644 --- a/ui/pages/confirmations/constants/index.ts +++ b/ui/pages/confirmations/constants/index.ts @@ -3,3 +3,9 @@ import { CHAINLIST_CHAIN_IDS_MAP } from '../../../../shared/constants/network'; export const EIP712_PRIMARY_TYPE_PERMIT = 'Permit'; export const IGNORE_GAS_LIMIT_CHAIN_IDS = [CHAINLIST_CHAIN_IDS_MAP.MANTLE]; + +export const TYPED_SIGNATURE_VERSIONS = { + V1: 'V1', + V3: 'V3', + V4: 'V4', +}; diff --git a/ui/pages/confirmations/utils/confirm.test.ts b/ui/pages/confirmations/utils/confirm.test.ts index e358a6076408..4e2edbaad6e8 100644 --- a/ui/pages/confirmations/utils/confirm.test.ts +++ b/ui/pages/confirmations/utils/confirm.test.ts @@ -3,6 +3,12 @@ import { ApprovalType } from '@metamask/controller-utils'; import { TransactionType } from '@metamask/transaction-controller'; import { + permitSignatureMsg, + unapprovedTypedSignMsgV4, +} from '../../../../test/data/confirmations/typed_sign'; +import { SignatureRequestType } from '../types/confirm'; +import { + isPermitSignatureRequest, isSignatureApprovalRequest, isSignatureTransactionType, parseSanitizeTypedDataMessage, @@ -71,4 +77,19 @@ describe('confirm util', () => { expect(result).toStrictEqual(false); }); }); + + describe('isPermitSignatureRequest', () => { + it('returns true for permit signature requests', () => { + const result = isPermitSignatureRequest( + permitSignatureMsg as SignatureRequestType, + ); + expect(result).toStrictEqual(true); + }); + it('returns false for request not of type permit signature', () => { + const result = isPermitSignatureRequest( + unapprovedTypedSignMsgV4 as SignatureRequestType, + ); + expect(result).toStrictEqual(false); + }); + }); }); diff --git a/ui/pages/confirmations/utils/confirm.ts b/ui/pages/confirmations/utils/confirm.ts index 6eddab209e67..abc57fa02483 100644 --- a/ui/pages/confirmations/utils/confirm.ts +++ b/ui/pages/confirmations/utils/confirm.ts @@ -5,6 +5,10 @@ import { Json } from '@metamask/utils'; import { sanitizeMessage } from '../../../helpers/utils/util'; import { SignatureRequestType } from '../types/confirm'; +import { + EIP712_PRIMARY_TYPE_PERMIT, + TYPED_SIGNATURE_VERSIONS, +} from '../constants'; export const REDESIGN_APPROVAL_TYPES = [ ApprovalType.EthSignTypedData, @@ -47,3 +51,18 @@ export const parseSanitizeTypedDataMessage = (dataToParse: string) => { export const isSIWESignatureRequest = (request: SignatureRequestType) => request.msgParams?.siwe?.isSIWEMessage; + +export const isPermitSignatureRequest = (request: SignatureRequestType) => { + if ( + !request || + !isSignatureTransactionType(request) || + request.type !== 'eth_signTypedData' || + request.msgParams?.version?.toUpperCase() === TYPED_SIGNATURE_VERSIONS.V1 + ) { + return false; + } + const { primaryType } = parseTypedDataMessage( + request.msgParams?.data as string, + ); + return primaryType === EIP712_PRIMARY_TYPE_PERMIT; +};