From 2367e4e1a6a5b29f83bdb8e7e81b0974ce889812 Mon Sep 17 00:00:00 2001 From: Jyoti Puri Date: Fri, 15 Nov 2024 17:20:28 +0530 Subject: [PATCH] feat: UI changes to show decoding data for permits (#28342) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Changes in permit simulation component to show decoding api results if present. Changes will currently not be visible as signature controller update is not yet done. ## **Related issues** Fixes: https://github.com/MetaMask/MetaMask-planning/issues/3554 ## **Manual testing steps** 1. Locally enable signature decoding using env variables 2. Submit a permit on test dapp 3. Check simulation section ## **Screenshots/Recordings** Screenshot 2024-11-07 at 3 07 29 PM ## **Pre-merge author checklist** - [X] 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). - [X] I've completed the PR template to the best of my ability - [X] I’ve included tests if applicable - [ ] 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-extension/blob/develop/.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. --------- Co-authored-by: MetaMask Bot --- .../static-simulation/static-simulation.tsx | 18 +- .../permit-simulation.test.tsx.snap | 43 ++- .../decoded-simulation.test.tsx.snap | 9 + .../decoded-simulation.test.tsx | 21 ++ .../decoded-simulation/decoded-simulation.tsx | 7 + .../decoded-simulation/index.ts | 1 + .../default-simulation.test.tsx.snap | 246 ++++++++++++++++++ .../default-simulation.test.tsx | 60 +++++ .../default-simulation/default-simulation.tsx | 118 +++++++++ .../default-simulation/index.ts | 1 + .../permit-simulation.test.tsx | 51 +++- .../permit-simulation/permit-simulation.tsx | 120 +-------- ui/pages/confirmations/types/confirm.ts | 3 + 13 files changed, 566 insertions(+), 132 deletions(-) create mode 100644 ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/decoded-simulation/__snapshots__/decoded-simulation.test.tsx.snap create mode 100644 ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/decoded-simulation/decoded-simulation.test.tsx create mode 100644 ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/decoded-simulation/decoded-simulation.tsx create mode 100644 ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/decoded-simulation/index.ts create mode 100644 ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/default-simulation/__snapshots__/default-simulation.test.tsx.snap create mode 100644 ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/default-simulation/default-simulation.test.tsx create mode 100644 ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/default-simulation/default-simulation.tsx create mode 100644 ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/default-simulation/index.ts diff --git a/ui/pages/confirmations/components/confirm/info/shared/static-simulation/static-simulation.tsx b/ui/pages/confirmations/components/confirm/info/shared/static-simulation/static-simulation.tsx index 7901854b91d1..cf8ad9cd5773 100644 --- a/ui/pages/confirmations/components/confirm/info/shared/static-simulation/static-simulation.tsx +++ b/ui/pages/confirmations/components/confirm/info/shared/static-simulation/static-simulation.tsx @@ -1,22 +1,36 @@ import React from 'react'; + +import { Box } from '../../../../../../../components/component-library'; import { ConfirmInfoRow, ConfirmInfoRowText, } from '../../../../../../../components/app/confirm/info/row'; import { ConfirmInfoSection } from '../../../../../../../components/app/confirm/info/row/section'; +import { + Display, + JustifyContent, +} from '../../../../../../../helpers/constants/design-system'; +import Preloader from '../../../../../../../components/ui/icon/preloader'; const StaticSimulation: React.FC<{ title: string; titleTooltip: string; description: string; simulationElements: React.ReactNode; -}> = ({ title, titleTooltip, description, simulationElements }) => { + isLoading?: boolean; +}> = ({ title, titleTooltip, description, simulationElements, isLoading }) => { return ( - {simulationElements} + {isLoading ? ( + + + + ) : ( + simulationElements + )} ); }; 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 f35e2218cbfb..16e7f5fa79d9 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 @@ -1,6 +1,14 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`PermitSimulation renders component correctly 1`] = ` +exports[`PermitSimulation should not render default simulation if decodingLoading is true 1`] = ` +
+
+ "DECODED SIMULATION IMPLEMENTATION" +
+
+`; + +exports[`PermitSimulation should render default simulation if decoding api does not return result 1`] = `
`; -exports[`PermitSimulation renders correctly for NFT permit 1`] = ` +exports[`PermitSimulation should render default simulation if decoding api returns error 1`] = `
- You're giving someone else permission to withdraw NFTs from your account. + You're giving the spender permission to spend this many tokens from your account.

@@ -190,7 +198,7 @@ exports[`PermitSimulation renders correctly for NFT permit 1`] = `

- Withdraw + Spending cap

@@ -207,14 +215,25 @@ exports[`PermitSimulation renders correctly for NFT permit 1`] = `
-
-

+

- #3606393 -

+

+ 30 +

+
- 0xC3644...1FE88 + 0xCcCCc...ccccC

diff --git a/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/decoded-simulation/__snapshots__/decoded-simulation.test.tsx.snap b/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/decoded-simulation/__snapshots__/decoded-simulation.test.tsx.snap new file mode 100644 index 000000000000..9c190acccee9 --- /dev/null +++ b/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/decoded-simulation/__snapshots__/decoded-simulation.test.tsx.snap @@ -0,0 +1,9 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`DecodedSimulation renders component correctly 1`] = ` +
+
+ "DECODED SIMULATION IMPLEMENTATION" +
+
+`; diff --git a/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/decoded-simulation/decoded-simulation.test.tsx b/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/decoded-simulation/decoded-simulation.test.tsx new file mode 100644 index 000000000000..00e0fbadb496 --- /dev/null +++ b/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/decoded-simulation/decoded-simulation.test.tsx @@ -0,0 +1,21 @@ +import React from 'react'; +import configureMockStore from 'redux-mock-store'; + +import { getMockTypedSignConfirmStateForRequest } from '../../../../../../../../../test/data/confirmations/helper'; +import { renderWithConfirmContextProvider } from '../../../../../../../../../test/lib/confirmations/render-helpers'; +import { permitSignatureMsg } from '../../../../../../../../../test/data/confirmations/typed_sign'; +import PermitSimulation from './decoded-simulation'; + +describe('DecodedSimulation', () => { + it('renders component correctly', async () => { + const state = getMockTypedSignConfirmStateForRequest(permitSignatureMsg); + const mockStore = configureMockStore([])(state); + + const { container } = renderWithConfirmContextProvider( + , + mockStore, + ); + + expect(container).toMatchSnapshot(); + }); +}); diff --git a/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/decoded-simulation/decoded-simulation.tsx b/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/decoded-simulation/decoded-simulation.tsx new file mode 100644 index 000000000000..f4767dbee264 --- /dev/null +++ b/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/decoded-simulation/decoded-simulation.tsx @@ -0,0 +1,7 @@ +import React from 'react'; + +const DecodedSimulation: React.FC = () => ( +
"DECODED SIMULATION IMPLEMENTATION"
+); + +export default DecodedSimulation; diff --git a/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/decoded-simulation/index.ts b/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/decoded-simulation/index.ts new file mode 100644 index 000000000000..a13982b1b09d --- /dev/null +++ b/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/decoded-simulation/index.ts @@ -0,0 +1 @@ +export { default as DecodedSimulation } from './decoded-simulation'; diff --git a/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/default-simulation/__snapshots__/default-simulation.test.tsx.snap b/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/default-simulation/__snapshots__/default-simulation.test.tsx.snap new file mode 100644 index 000000000000..a378d0befcc4 --- /dev/null +++ b/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/default-simulation/__snapshots__/default-simulation.test.tsx.snap @@ -0,0 +1,246 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`DefaultSimulation renders component correctly 1`] = ` +
+
+
+
+
+

+ Estimated changes +

+
+
+ +
+
+
+
+
+

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

+
+
+
+
+
+

+ Spending cap +

+
+
+
+
+
+
+
+
+

+ 30 +

+
+
+
+
+
+ +

+ 0xCcCCc...ccccC +

+
+
+
+
+
+
+
+
+
+`; + +exports[`DefaultSimulation renders correctly for NFT permit 1`] = ` +
+
+
+
+
+

+ Estimated changes +

+
+
+ +
+
+
+
+
+

+ You're giving someone else permission to withdraw NFTs from your account. +

+
+
+
+
+
+

+ Withdraw +

+
+
+
+
+
+
+
+

+ #3606393 +

+
+
+
+
+ +

+ 0xC3644...1FE88 +

+
+
+
+
+
+
+
+
+
+`; diff --git a/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/default-simulation/default-simulation.test.tsx b/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/default-simulation/default-simulation.test.tsx new file mode 100644 index 000000000000..b9e095292bce --- /dev/null +++ b/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/default-simulation/default-simulation.test.tsx @@ -0,0 +1,60 @@ +import React from 'react'; +import configureMockStore from 'redux-mock-store'; +import { act } from 'react-dom/test-utils'; + +import { getMockTypedSignConfirmStateForRequest } from '../../../../../../../../../test/data/confirmations/helper'; +import { renderWithConfirmContextProvider } from '../../../../../../../../../test/lib/confirmations/render-helpers'; +import { + permitNFTSignatureMsg, + permitSignatureMsg, +} from '../../../../../../../../../test/data/confirmations/typed_sign'; +import { memoizedGetTokenStandardAndDetails } from '../../../../../../utils/token'; +import DefaultSimulation from './default-simulation'; + +jest.mock('../../../../../../../../store/actions', () => { + return { + getTokenStandardAndDetails: jest + .fn() + .mockResolvedValue({ decimals: 2, standard: 'ERC20' }), + }; +}); + +describe('DefaultSimulation', () => { + afterEach(() => { + jest.clearAllMocks(); + + /** Reset memoized function using getTokenStandardAndDetails for each test */ + memoizedGetTokenStandardAndDetails?.cache?.clear?.(); + }); + + it('renders component correctly', async () => { + const state = getMockTypedSignConfirmStateForRequest(permitSignatureMsg); + const mockStore = configureMockStore([])(state); + + await act(async () => { + const { container, findByText } = renderWithConfirmContextProvider( + , + mockStore, + ); + + expect(await findByText('30')).toBeInTheDocument(); + expect(container).toMatchSnapshot(); + }); + }); + + it('renders correctly for NFT permit', async () => { + const state = getMockTypedSignConfirmStateForRequest(permitNFTSignatureMsg); + const mockStore = configureMockStore([])(state); + + await act(async () => { + const { container, findByText } = renderWithConfirmContextProvider( + , + mockStore, + ); + + expect(await findByText('Withdraw')).toBeInTheDocument(); + expect(await findByText('#3606393')).toBeInTheDocument(); + expect(container).toMatchSnapshot(); + }); + }); +}); diff --git a/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/default-simulation/default-simulation.tsx b/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/default-simulation/default-simulation.tsx new file mode 100644 index 000000000000..c39a88585e35 --- /dev/null +++ b/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/default-simulation/default-simulation.tsx @@ -0,0 +1,118 @@ +import { Hex } from '@metamask/utils'; +import React from 'react'; +import { PrimaryType } from '../../../../../../../../../shared/constants/signatures'; +import { parseTypedDataMessage } from '../../../../../../../../../shared/modules/transaction.utils'; +import { ConfirmInfoRow } from '../../../../../../../../components/app/confirm/info/row'; +import { Box } from '../../../../../../../../components/component-library'; +import { + Display, + FlexDirection, +} from '../../../../../../../../helpers/constants/design-system'; +import { useI18nContext } from '../../../../../../../../hooks/useI18nContext'; +import { useConfirmContext } from '../../../../../../context/confirm'; +import { SignatureRequestType } from '../../../../../../types/confirm'; +import StaticSimulation from '../../../shared/static-simulation/static-simulation'; +import PermitSimulationValueDisplay from '../value-display/value-display'; + +function extractTokenDetailsByPrimaryType( + message: Record, + primaryType: PrimaryType, +): object[] | unknown { + let tokenDetails; + + switch (primaryType) { + case PrimaryType.PermitBatch: + case PrimaryType.PermitSingle: + tokenDetails = message?.details; + break; + case PrimaryType.PermitBatchTransferFrom: + case PrimaryType.PermitTransferFrom: + tokenDetails = message?.permitted; + break; + default: + break; + } + + const isNonArrayObject = tokenDetails && !Array.isArray(tokenDetails); + + return isNonArrayObject ? [tokenDetails] : tokenDetails; +} + +const DefaultPermitSimulation: React.FC = () => { + const t = useI18nContext(); + const { currentConfirmation } = useConfirmContext(); + const msgData = currentConfirmation.msgParams?.data; + const chainId = currentConfirmation.chainId as Hex; + const { + domain: { verifyingContract }, + message, + message: { tokenId }, + primaryType, + } = parseTypedDataMessage(msgData as string); + const isNFT = tokenId !== undefined; + + const tokenDetails = extractTokenDetailsByPrimaryType(message, primaryType); + + const TokenDetail = ({ + token, + amount, + i, + }: { + token: Hex | string; + amount: number | string; + i: number; + }) => ( + + ); + + const SpendingCapRow = ( + + + {Array.isArray(tokenDetails) ? ( + + {tokenDetails.map( + ( + { token, amount }: { token: string; amount: string }, + i: number, + ) => ( + + ), + )} + + ) : ( + + )} + + + ); + + return ( + + ); +}; + +export default DefaultPermitSimulation; diff --git a/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/default-simulation/index.ts b/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/default-simulation/index.ts new file mode 100644 index 000000000000..b9f6c48b65db --- /dev/null +++ b/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/default-simulation/index.ts @@ -0,0 +1 @@ +export { default as DefaultSimulation } from './default-simulation'; diff --git a/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/permit-simulation.test.tsx b/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/permit-simulation.test.tsx index 0d67715867d9..d0183d1ac2e7 100644 --- a/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/permit-simulation.test.tsx +++ b/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/permit-simulation.test.tsx @@ -1,13 +1,11 @@ import React from 'react'; import configureMockStore from 'redux-mock-store'; import { act } from 'react-dom/test-utils'; +import { waitFor } from '@testing-library/dom'; import { getMockTypedSignConfirmStateForRequest } from '../../../../../../../../test/data/confirmations/helper'; import { renderWithConfirmContextProvider } from '../../../../../../../../test/lib/confirmations/render-helpers'; -import { - permitNFTSignatureMsg, - permitSignatureMsg, -} from '../../../../../../../../test/data/confirmations/typed_sign'; +import { permitSignatureMsg } from '../../../../../../../../test/data/confirmations/typed_sign'; import { memoizedGetTokenStandardAndDetails } from '../../../../../utils/token'; import PermitSimulation from './permit-simulation'; @@ -27,8 +25,12 @@ describe('PermitSimulation', () => { memoizedGetTokenStandardAndDetails?.cache?.clear?.(); }); - it('renders component correctly', async () => { - const state = getMockTypedSignConfirmStateForRequest(permitSignatureMsg); + it('should render default simulation if decoding api does not return result', async () => { + const state = getMockTypedSignConfirmStateForRequest({ + ...permitSignatureMsg, + decodingLoading: false, + decodingData: undefined, + }); const mockStore = configureMockStore([])(state); await act(async () => { @@ -42,8 +44,18 @@ describe('PermitSimulation', () => { }); }); - it('renders correctly for NFT permit', async () => { - const state = getMockTypedSignConfirmStateForRequest(permitNFTSignatureMsg); + it('should render default simulation if decoding api returns error', async () => { + const state = getMockTypedSignConfirmStateForRequest({ + ...permitSignatureMsg, + decodingLoading: false, + decodingData: { + stateChanges: null, + error: { + message: 'some error', + type: 'SOME_ERROR', + }, + }, + }); const mockStore = configureMockStore([])(state); await act(async () => { @@ -52,9 +64,28 @@ describe('PermitSimulation', () => { mockStore, ); - expect(await findByText('Withdraw')).toBeInTheDocument(); - expect(await findByText('#3606393')).toBeInTheDocument(); + expect(await findByText('30')).toBeInTheDocument(); expect(container).toMatchSnapshot(); }); }); + + it('should not render default simulation if decodingLoading is true', async () => { + const state = getMockTypedSignConfirmStateForRequest({ + ...permitSignatureMsg, + decodingLoading: true, + }); + const mockStore = configureMockStore([])(state); + + await act(async () => { + const { container, queryByTestId } = renderWithConfirmContextProvider( + , + mockStore, + ); + + await waitFor(() => { + expect(queryByTestId('30')).not.toBeInTheDocument(); + expect(container).toMatchSnapshot(); + }); + }); + }); }); diff --git a/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/permit-simulation.tsx b/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/permit-simulation.tsx index 855f7837ba42..0b4d9eed22d4 100644 --- a/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/permit-simulation.tsx +++ b/ui/pages/confirmations/components/confirm/info/typed-sign/permit-simulation/permit-simulation.tsx @@ -1,118 +1,22 @@ -import { Hex } from '@metamask/utils'; import React from 'react'; -import { PrimaryType } from '../../../../../../../../shared/constants/signatures'; -import { parseTypedDataMessage } from '../../../../../../../../shared/modules/transaction.utils'; -import { ConfirmInfoRow } from '../../../../../../../components/app/confirm/info/row'; -import { Box } from '../../../../../../../components/component-library'; -import { - Display, - FlexDirection, -} from '../../../../../../../helpers/constants/design-system'; -import { useI18nContext } from '../../../../../../../hooks/useI18nContext'; -import { useConfirmContext } from '../../../../../context/confirm'; -import { SignatureRequestType } from '../../../../../types/confirm'; -import StaticSimulation from '../../shared/static-simulation/static-simulation'; -import PermitSimulationValueDisplay from './value-display/value-display'; - -function extractTokenDetailsByPrimaryType( - message: Record, - primaryType: PrimaryType, -): object[] | unknown { - let tokenDetails; - - switch (primaryType) { - case PrimaryType.PermitBatch: - case PrimaryType.PermitSingle: - tokenDetails = message?.details; - break; - case PrimaryType.PermitBatchTransferFrom: - case PrimaryType.PermitTransferFrom: - tokenDetails = message?.permitted; - break; - default: - break; - } - - const isNonArrayObject = tokenDetails && !Array.isArray(tokenDetails); - return isNonArrayObject ? [tokenDetails] : tokenDetails; -} +import { SignatureRequestType } from '../../../../../types/confirm'; +import { useConfirmContext } from '../../../../../context/confirm'; +import { DefaultSimulation } from './default-simulation'; +import { DecodedSimulation } from './decoded-simulation'; const PermitSimulation: React.FC = () => { - const t = useI18nContext(); const { currentConfirmation } = useConfirmContext(); - const msgData = currentConfirmation.msgParams?.data; - const chainId = currentConfirmation.chainId as Hex; - const { - domain: { verifyingContract }, - message, - message: { tokenId }, - primaryType, - } = parseTypedDataMessage(msgData as string); - const isNFT = tokenId !== undefined; - - const tokenDetails = extractTokenDetailsByPrimaryType(message, primaryType); + const { decodingLoading, decodingData } = currentConfirmation; - const TokenDetail = ({ - token, - amount, - i, - }: { - token: Hex | string; - amount: number | string; - i: number; - }) => ( - - ); - - const SpendingCapRow = ( - - - {Array.isArray(tokenDetails) ? ( - - {tokenDetails.map( - ( - { token, amount }: { token: string; amount: string }, - i: number, - ) => ( - - ), - )} - - ) : ( - - )} - - - ); + if ( + decodingData?.error || + (decodingData === undefined && decodingLoading !== true) + ) { + return ; + } - return ( - - ); + return ; }; export default PermitSimulation; diff --git a/ui/pages/confirmations/types/confirm.ts b/ui/pages/confirmations/types/confirm.ts index b7c0c1160b3c..fd344fcc2354 100644 --- a/ui/pages/confirmations/types/confirm.ts +++ b/ui/pages/confirmations/types/confirm.ts @@ -1,4 +1,5 @@ import { ApprovalControllerState } from '@metamask/approval-controller'; +import { DecodingData } from '@metamask/signature-controller'; import { SIWEMessage } from '@metamask/controller-utils'; import { TransactionMeta, @@ -38,6 +39,8 @@ export type SignatureRequestType = { type: TransactionType; custodyId?: string; securityAlertResponse?: SecurityAlertResponse; + decodingLoading?: boolean; + decodingData?: DecodingData; }; export type Confirmation = SignatureRequestType | TransactionMeta;