From e5a2159f3b6d3beef6cfd3cd5dab71661d1ddc38 Mon Sep 17 00:00:00 2001 From: Ariella Vu <20778143+digiwand@users.noreply.github.com> Date: Mon, 23 Sep 2024 21:59:59 +0800 Subject: [PATCH] fix: [cherrypick][V12.3.0] Redesign Signature Message date values (#27305) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Cherry-pick https://github.com/MetaMask/metamask-extension/pull/27249 for v12.3.0 Fix date values: - Formerly, conversations were converting values as if they were milliseconds. However, these values come from Solidity where these timestamps are unix timestamps in seconds. - Support -1 (no expiration) value - Display "expiry" as a date instead of a unix timestamp This PR fixes the conversion. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/27249?quickstart=1) Fixes: https://github.com/MetaMask/metamask-extension/issues/27137 1. Go to test-dapp 2. Request Permit or another TypedSign request 3. Observe deadlines and dates - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. - [ ] 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. --- app/_locales/en/messages.json | 3 + test/data/confirmations/typed_sign.ts | 27 +- .../confirmations/signatures/permit.spec.ts | 2 +- .../app/confirm/info/row/date.stories.tsx | 4 +- .../app/confirm/info/row/date.test.tsx | 4 +- ui/components/app/confirm/info/row/date.tsx | 11 +- ui/helpers/utils/util.js | 14 +- ui/helpers/utils/util.test.js | 8 +- .../personal-sign/siwe-sign/siwe-sign.tsx | 4 +- .../__snapshots__/typed-sign.test.tsx.snap | 589 +++++++++++++++++- .../info/typed-sign/typed-sign.test.tsx | 17 + .../row/__snapshots__/dataTree.test.tsx.snap | 99 +++ .../components/confirm/row/dataTree.test.tsx | 13 + .../components/confirm/row/dataTree.tsx | 23 +- .../__snapshots__/confirm.test.tsx.snap | 12 +- 15 files changed, 801 insertions(+), 29 deletions(-) diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json index c3683becebbd..75ff076121df 100644 --- a/app/_locales/en/messages.json +++ b/app/_locales/en/messages.json @@ -3253,6 +3253,9 @@ "nonceFieldHeading": { "message": "Custom nonce" }, + "none": { + "message": "None" + }, "notBusy": { "message": "Not busy" }, diff --git a/test/data/confirmations/typed_sign.ts b/test/data/confirmations/typed_sign.ts index 3dd236ca9fa6..a63423e17ae9 100644 --- a/test/data/confirmations/typed_sign.ts +++ b/test/data/confirmations/typed_sign.ts @@ -1,3 +1,4 @@ +import { TransactionType } from '@metamask/transaction-controller'; import { SignatureRequestType } from '../../../ui/pages/confirmations/types/confirm'; export const unapprovedTypedSignMsgV1 = { @@ -169,12 +170,13 @@ export const permitSignatureMsg = { data: '{"types":{"EIP712Domain":[{"name":"name","type":"string"},{"name":"version","type":"string"},{"name":"chainId","type":"uint256"},{"name":"verifyingContract","type":"address"}],"Permit":[{"name":"owner","type":"address"},{"name":"spender","type":"address"},{"name":"value","type":"uint256"},{"name":"nonce","type":"uint256"},{"name":"deadline","type":"uint256"}]},"primaryType":"Permit","domain":{"name":"MyToken","version":"1","verifyingContract":"0xCcCCccccCCCCcCCCCCCcCcCccCcCCCcCcccccccC","chainId":1},"message":{"owner":"0x935e73edb9ff52e23bac7f7e043a1ecd06d05477","spender":"0x5B38Da6a701c568545dCfcB03FcB875f56beddC4","value":3000,"nonce":0,"deadline":50000000000}}', from: '0x935e73edb9ff52e23bac7f7e043a1ecd06d05477', version: 'V4', + requestId: 14, signatureMethod: 'eth_signTypedData_v4', origin: 'https://metamask.github.io', }, } as SignatureRequestType; -export const permitBatchSignatureMsg = { +export const permitSignatureMsgWithNoDeadline = { id: '0b1787a0-1c44-11ef-b70d-e7064bd7b659', securityAlertResponse: { reason: 'loading', @@ -184,10 +186,30 @@ export const permitBatchSignatureMsg = { status: 'unapproved', time: 1716826404122, type: 'eth_signTypedData', + msgParams: { + data: '{"types":{"EIP712Domain":[{"name":"name","type":"string"},{"name":"version","type":"string"},{"name":"chainId","type":"uint256"},{"name":"verifyingContract","type":"address"}],"Permit":[{"name":"owner","type":"address"},{"name":"spender","type":"address"},{"name":"value","type":"uint256"},{"name":"nonce","type":"uint256"},{"name":"deadline","type":"uint256"}]},"primaryType":"Permit","domain":{"name":"MyToken","version":"1","verifyingContract":"0xCcCCccccCCCCcCCCCCCcCcCccCcCCCcCcccccccC","chainId":1},"message":{"owner":"0x935e73edb9ff52e23bac7f7e043a1ecd06d05477","spender":"0x5B38Da6a701c568545dCfcB03FcB875f56beddC4","value":3000,"nonce":0,"deadline":-1}}', + from: '0x935e73edb9ff52e23bac7f7e043a1ecd06d05477', + version: 'V4', + signatureMethod: 'eth_signTypedData_v4', + origin: 'https://metamask.github.io', + }, +} as SignatureRequestType; + +export const permitBatchSignatureMsg = { + id: '0b1787a0-1c44-11ef-b70d-e7064bd7b659', + securityAlertResponse: { + reason: 'loading', + result_type: 'validation_in_progress', + securityAlertId: 'ab21395f-2190-472f-8cfa-3d224e7529d8', + }, + status: 'unapproved', + time: 1716826404122, + type: TransactionType.signTypedData, msgParams: { data: '{"types":{"PermitBatch":[{"name":"details","type":"PermitDetails[]"},{"name":"spender","type":"address"},{"name":"sigDeadline","type":"uint256"}],"PermitDetails":[{"name":"token","type":"address"},{"name":"amount","type":"uint160"},{"name":"expiration","type":"uint48"},{"name":"nonce","type":"uint48"}],"EIP712Domain":[{"name":"name","type":"string"},{"name":"chainId","type":"uint256"},{"name":"verifyingContract","type":"address"}]},"domain":{"name":"Permit2","chainId":"1","verifyingContract":"0x000000000022d473030f116ddee9f6b43ac78ba3"},"primaryType":"PermitBatch","message":{"details":[{"token":"0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48","amount":"1461501637330902918203684832716283019655932542975","expiration":"1722887542","nonce":"5"},{"token":"0xb0b86991c6218b36c1d19d4a2e9eb0ce3606eb48","amount":"2461501637330902918203684832716283019655932542975","expiration":"1722887642","nonce":"6"}],"spender":"0x3fc91a3afd70395cd496c647d5a6cc9d4b2b7fad","sigDeadline":"1720297342"}}', from: '0x935e73edb9ff52e23bac7f7e043a1ecd06d05477', version: 'V4', + requestId: 15, signatureMethod: 'eth_signTypedData_v4', origin: 'https://metamask.github.io', }, @@ -202,11 +224,12 @@ export const permitSingleSignatureMsg = { }, status: 'unapproved', time: 1716826404122, - type: 'eth_signTypedData', + type: TransactionType.signTypedData, msgParams: { data: '{"types":{"PermitSingle":[{"name":"details","type":"PermitDetails"},{"name":"spender","type":"address"},{"name":"sigDeadline","type":"uint256"}],"PermitDetails":[{"name":"token","type":"address"},{"name":"amount","type":"uint160"},{"name":"expiration","type":"uint48"},{"name":"nonce","type":"uint48"}],"EIP712Domain":[{"name":"name","type":"string"},{"name":"chainId","type":"uint256"},{"name":"verifyingContract","type":"address"}]},"domain":{"name":"Permit2","chainId":"1","verifyingContract":"0x000000000022d473030f116ddee9f6b43ac78ba3"},"primaryType":"PermitSingle","message":{"details":{"token":"0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48","amount":"1461501637330902918203684832716283019655932542975","expiration":"1722887542","nonce":"5"},"spender":"0x3fc91a3afd70395cd496c647d5a6cc9d4b2b7fad","sigDeadline":"1720297342"}}', from: '0x935e73edb9ff52e23bac7f7e043a1ecd06d05477', version: 'V4', + requestId: 16, signatureMethod: 'eth_signTypedData_v4', origin: 'https://metamask.github.io', }, diff --git a/test/e2e/tests/confirmations/signatures/permit.spec.ts b/test/e2e/tests/confirmations/signatures/permit.spec.ts index a8191a15f229..6dadf732c246 100644 --- a/test/e2e/tests/confirmations/signatures/permit.spec.ts +++ b/test/e2e/tests/confirmations/signatures/permit.spec.ts @@ -123,7 +123,7 @@ async function assertInfoValues(driver: Driver) { }); const value = driver.findElement({ text: '3,000' }); const nonce = driver.findElement({ text: '0' }); - const deadline = driver.findElement({ text: '02 August 1971, 16:53' }); + const deadline = driver.findElement({ text: '09 June 3554, 16:53' }); assert.ok(await origin, 'origin'); assert.ok(await contractPetName, 'contractPetName'); diff --git a/ui/components/app/confirm/info/row/date.stories.tsx b/ui/components/app/confirm/info/row/date.stories.tsx index 971ba7624f3e..e0e6a9a07dec 100644 --- a/ui/components/app/confirm/info/row/date.stories.tsx +++ b/ui/components/app/confirm/info/row/date.stories.tsx @@ -18,9 +18,9 @@ const ConfirmInfoRowDateStory = { }, }; -export const DefaultStory = ({ date }) => ; +export const DefaultStory = ({ date }) => ; DefaultStory.args = { - date: 1633019124000, + date: 1633019124, }; export default ConfirmInfoRowDateStory; diff --git a/ui/components/app/confirm/info/row/date.test.tsx b/ui/components/app/confirm/info/row/date.test.tsx index eda8510e4e7c..8f6ac22f6c58 100644 --- a/ui/components/app/confirm/info/row/date.test.tsx +++ b/ui/components/app/confirm/info/row/date.test.tsx @@ -5,7 +5,9 @@ import { ConfirmInfoRowDate } from './date'; describe('ConfirmInfoRowDate', () => { it('should match snapshot', () => { - const { getByText } = render(); + const { getByText } = render( + , + ); expect(getByText('30 September 2021, 16:25')).toBeInTheDocument(); }); }); diff --git a/ui/components/app/confirm/info/row/date.tsx b/ui/components/app/confirm/info/row/date.tsx index 47fa8e4ea56a..278c1e514c2f 100644 --- a/ui/components/app/confirm/info/row/date.tsx +++ b/ui/components/app/confirm/info/row/date.tsx @@ -6,14 +6,17 @@ import { FlexWrap, TextColor, } from '../../../../../helpers/constants/design-system'; -import { formatUTCDate } from '../../../../../helpers/utils/util'; +import { formatUTCDateFromUnixTimestamp } from '../../../../../helpers/utils/util'; import { Box, Text } from '../../../../component-library'; export type ConfirmInfoRowDateProps = { - date: number; + /** timestamp as seconds since unix epoch e.g. Solidity block.timestamp (type uint256) value */ + unixTimestamp: number; }; -export const ConfirmInfoRowDate = ({ date }: ConfirmInfoRowDateProps) => ( +export const ConfirmInfoRowDate = ({ + unixTimestamp, +}: ConfirmInfoRowDateProps) => ( ( gap={2} > - {formatUTCDate(date)} + {formatUTCDateFromUnixTimestamp(unixTimestamp)} ); diff --git a/ui/helpers/utils/util.js b/ui/helpers/utils/util.js index 7eeab828a750..39c86e79bf75 100644 --- a/ui/helpers/utils/util.js +++ b/ui/helpers/utils/util.js @@ -39,13 +39,17 @@ export function formatDate(date, format = "M/d/y 'at' T") { return DateTime.fromMillis(date).toFormat(format); } -export const formatUTCDate = (dateInMillis) => { - if (!dateInMillis) { - return dateInMillis; +/** + * @param {number} unixTimestamp - timestamp as seconds since unix epoch + * @returns {string} formatted date string e.g. "14 July 2034, 22:22" + */ +export const formatUTCDateFromUnixTimestamp = (unixTimestamp) => { + if (!unixTimestamp) { + return unixTimestamp; } - return DateTime.fromMillis(dateInMillis) - .setZone('utc') + return DateTime.fromSeconds(unixTimestamp) + .toUTC() .toFormat('dd LLLL yyyy, HH:mm'); }; diff --git a/ui/helpers/utils/util.test.js b/ui/helpers/utils/util.test.js index e9485a262197..e10c70630dba 100644 --- a/ui/helpers/utils/util.test.js +++ b/ui/helpers/utils/util.test.js @@ -1044,15 +1044,15 @@ describe('util', () => { }); }); - describe('formatUTCDate', () => { + describe('formatUTCDateFromUnixTimestamp', () => { it('formats passed date string', () => { - expect(util.formatUTCDate(1633019124000)).toStrictEqual( - '30 September 2021, 16:25', + expect(util.formatUTCDateFromUnixTimestamp(2036528542)).toStrictEqual( + '14 July 2034, 22:22', ); }); it('returns empty string if empty string is passed', () => { - expect(util.formatUTCDate('')).toStrictEqual(''); + expect(util.formatUTCDateFromUnixTimestamp('')).toStrictEqual(''); }); }); diff --git a/ui/pages/confirmations/components/confirm/info/personal-sign/siwe-sign/siwe-sign.tsx b/ui/pages/confirmations/components/confirm/info/personal-sign/siwe-sign/siwe-sign.tsx index 8c00c16f9c02..a4ba9fcbc95f 100644 --- a/ui/pages/confirmations/components/confirm/info/personal-sign/siwe-sign/siwe-sign.tsx +++ b/ui/pages/confirmations/components/confirm/info/personal-sign/siwe-sign/siwe-sign.tsx @@ -66,7 +66,9 @@ const SIWESignInfo: React.FC = () => { {requestId && ( diff --git a/ui/pages/confirmations/components/confirm/info/typed-sign/__snapshots__/typed-sign.test.tsx.snap b/ui/pages/confirmations/components/confirm/info/typed-sign/__snapshots__/typed-sign.test.tsx.snap index b9b75a2f4ee5..9e0218a9c9f5 100644 --- a/ui/pages/confirmations/components/confirm/info/typed-sign/__snapshots__/typed-sign.test.tsx.snap +++ b/ui/pages/confirmations/components/confirm/info/typed-sign/__snapshots__/typed-sign.test.tsx.snap @@ -462,7 +462,594 @@ exports[`TypedSignInfo correctly renders permit sign type 1`] = ` class="mm-box mm-text mm-text--body-md mm-box--color-inherit" style="white-space: pre-wrap;" > - 02 August 1971, 16:53 + 09 June 3554, 16:53 +

+ + + + + + + + +`; + +exports[`TypedSignInfo correctly renders permit sign type with no deadline 1`] = ` +
+
+
+
+

+ Estimated changes +

+
+
+ +
+
+
+
+

+ You're giving the spender permission to spend this many tokens from your account. +

+
+
+
+
+

+ Spending cap +

+
+
+
+
+
+
+
+

+ 3,000 +

+
+
+
+
+
+ +

+ 0xCcCCc...ccccC +

+
+
+
+
+
+
+
+
+
+
+
+

+ Spender +

+
+
+
+ +

+ 0x5B38D...eddC4 +

+
+
+
+
+
+
+

+ Request from +

+
+
+ +
+
+
+
+

+ metamask.github.io +

+
+
+
+
+

+ Interacting with +

+
+
+
+ +

+ 0xCcCCc...ccccC +

+
+
+
+
+
+
+
+

+ Message +

+
+
+
+
+

+ Primary type: +

+
+
+

+ Permit +

+
+
+
+
+
+
+

+ Owner: +

+
+
+
+ +

+ 0x935E7...05477 +

+
+
+
+
+
+

+ Spender: +

+
+
+
+ +

+ 0x5B38D...eddC4 +

+
+
+
+
+
+

+ Value: +

+
+
+
+
+

+ 3,000 +

+
+
+
+
+
+
+

+ Nonce: +

+
+
+

+ 0 +

+
+
+
+
+

+ Deadline: +

+
+
+

+ None

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 1ac473dd40f8..089590d89618 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 @@ -5,6 +5,7 @@ import mockState from '../../../../../../../test/data/mock-state.json'; import { renderWithProvider } from '../../../../../../../test/lib/render-helpers'; import { permitSignatureMsg, + permitSignatureMsgWithNoDeadline, unapprovedTypedSignMsgV3, unapprovedTypedSignMsgV4, } from '../../../../../../../test/data/confirmations/typed_sign'; @@ -97,4 +98,20 @@ describe('TypedSignInfo', () => { const { container } = renderWithProvider(, mockStore); expect(container).toMatchSnapshot(); }); + + it('correctly renders permit sign type with no deadline', () => { + const state = { + ...mockState, + metamask: { + ...mockState.metamask, + useTransactionSimulations: true, + }, + confirm: { + currentConfirmation: permitSignatureMsgWithNoDeadline, + }, + }; + const mockStore = configureMockStore([])(state); + const { container } = renderWithProvider(, mockStore); + expect(container).toMatchSnapshot(); + }); }); diff --git a/ui/pages/confirmations/components/confirm/row/__snapshots__/dataTree.test.tsx.snap b/ui/pages/confirmations/components/confirm/row/__snapshots__/dataTree.test.tsx.snap index 06a50c385aed..1c341b09e71d 100644 --- a/ui/pages/confirmations/components/confirm/row/__snapshots__/dataTree.test.tsx.snap +++ b/ui/pages/confirmations/components/confirm/row/__snapshots__/dataTree.test.tsx.snap @@ -802,3 +802,102 @@ exports[`DataTree should match snapshot for permit signature type 1`] = `
`; + +exports[`DataTree should match snapshot for permit signature type and deadline is -1 (None) 1`] = ` +
+
+
+
+

+ Types: +

+
+
+

+

+
+
+
+

+ PrimaryType: +

+
+
+

+

+
+
+
+

+ Domain: +

+
+
+

+

+
+
+
+

+ Message: +

+
+
+

+ 3000 +

+
+
+
+
+`; diff --git a/ui/pages/confirmations/components/confirm/row/dataTree.test.tsx b/ui/pages/confirmations/components/confirm/row/dataTree.test.tsx index 699539d21a71..b427b6a8ee97 100644 --- a/ui/pages/confirmations/components/confirm/row/dataTree.test.tsx +++ b/ui/pages/confirmations/components/confirm/row/dataTree.test.tsx @@ -83,6 +83,19 @@ describe('DataTree', () => { expect(container).toMatchSnapshot(); }); + it('should match snapshot for permit signature type and deadline is -1 (None)', () => { + const mockPermitData = JSON.parse( + permitSignatureMsg.msgParams?.data as string, + ); + mockPermitData.message.deadline = '-1'; + + const { container } = renderWithProvider( + , + store, + ); + expect(container).toMatchSnapshot(); + }); + it('should match snapshot for order signature type', () => { const { container } = renderWithProvider( = { [Field.Deadline]: [...PRIMARY_TYPES_PERMIT], [Field.EndTime]: [...PRIMARY_TYPES_ORDER], [Field.Expiration]: [PrimaryType.PermitBatch, PrimaryType.PermitSingle], + [Field.Expiry]: [...PRIMARY_TYPES_PERMIT], [Field.SigDeadline]: [...PRIMARY_TYPES_PERMIT], [Field.StartTime]: [...PRIMARY_TYPES_ORDER], [Field.ValidTo]: [...PRIMARY_TYPES_ORDER], }; +/** + * Date values may include -1 to represent a null value + * e.g. + * {@see {@link https://eips.ethereum.org/EIPS/eip-2612}} + * "The deadline argument can be set to uint(-1) to create Permits that effectively never expire." + */ +const NONE_DATE_VALUE = -1; + const getTokenDecimalsOfDataTree = async ( dataTreeData: Record | TreeData[], ): Promise => { @@ -146,6 +157,8 @@ const DataField = memo( value: ValueType; tokenDecimals: number; }) => { + const t = useI18nContext(); + if (typeof value === 'object' && value !== null) { return ( ; + if (isDateField(label, primaryType) && Boolean(value)) { + const intValue = parseInt(value, 10); + + return intValue === NONE_DATE_VALUE ? ( + + ) : ( + + ); } if (isTokenUnitsField(label, primaryType)) { diff --git a/ui/pages/confirmations/confirm/__snapshots__/confirm.test.tsx.snap b/ui/pages/confirmations/confirm/__snapshots__/confirm.test.tsx.snap index 84bab3847b1e..cc1007342e1a 100644 --- a/ui/pages/confirmations/confirm/__snapshots__/confirm.test.tsx.snap +++ b/ui/pages/confirmations/confirm/__snapshots__/confirm.test.tsx.snap @@ -921,7 +921,7 @@ exports[`Confirm should match snapshot for signature - typed sign - V4 - PermitB class="mm-box mm-text mm-text--body-md mm-box--color-inherit" style="white-space: pre-wrap;" > - 20 January 1970, 22:34 + 05 August 2024, 19:52

@@ -1095,7 +1095,7 @@ exports[`Confirm should match snapshot for signature - typed sign - V4 - PermitB class="mm-box mm-text mm-text--body-md mm-box--color-inherit" style="white-space: pre-wrap;" > - 20 January 1970, 22:34 + 05 August 2024, 19:54

@@ -1218,7 +1218,7 @@ exports[`Confirm should match snapshot for signature - typed sign - V4 - PermitB class="mm-box mm-text mm-text--body-md mm-box--color-inherit" style="white-space: pre-wrap;" > - 20 January 1970, 21:51 + 06 July 2024, 20:22

@@ -1880,7 +1880,7 @@ exports[`Confirm should match snapshot for signature - typed sign - V4 - PermitS class="mm-box mm-text mm-text--body-md mm-box--color-inherit" style="white-space: pre-wrap;" > - 20 January 1970, 22:34 + 05 August 2024, 19:52

@@ -2001,7 +2001,7 @@ exports[`Confirm should match snapshot for signature - typed sign - V4 - PermitS class="mm-box mm-text mm-text--body-md mm-box--color-inherit" style="white-space: pre-wrap;" > - 20 January 1970, 21:51 + 06 July 2024, 20:22

@@ -3252,7 +3252,7 @@ exports[`Confirm should match snapshot for signature - typed sign - permit 1`] = class="mm-box mm-text mm-text--body-md mm-box--color-inherit" style="white-space: pre-wrap;" > - 02 August 1971, 16:53 + 09 June 3554, 16:53