From 4f52fb112d7552161d6503f71aca22df35ece644 Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Tue, 24 Jun 2025 16:13:10 +0100 Subject: [PATCH 01/11] Add slippage state and components --- .../controllers/app-state-controller.ts | 27 ++++ app/scripts/metamask-controller.js | 4 + .../simulation-settings-modal.tsx | 124 +++++++++++++++++- ui/pages/confirmations/selectors/confirm.ts | 16 +++ ui/pages/confirmations/types/confirm.ts | 2 + ui/store/actions.ts | 15 +++ 6 files changed, 187 insertions(+), 1 deletion(-) diff --git a/app/scripts/controllers/app-state-controller.ts b/app/scripts/controllers/app-state-controller.ts index c40516fc708..bdc27e60a0e 100644 --- a/app/scripts/controllers/app-state-controller.ts +++ b/app/scripts/controllers/app-state-controller.ts @@ -92,6 +92,8 @@ export type AppStateControllerState = { lastUpdatedAt: number | null; enableEnforcedSimulations: boolean; enableEnforcedSimulationsForTransactions: Record; + enforcedSimulationsSlippage: number; + enforcedSimulationsSlippageForTransactions: Record; }; const controllerName = 'AppStateController'; @@ -213,6 +215,8 @@ const getDefaultAppStateControllerState = (): AppStateControllerState => ({ lastUpdatedAt: null, enableEnforcedSimulations: true, enableEnforcedSimulationsForTransactions: {}, + enforcedSimulationsSlippage: 10, + enforcedSimulationsSlippageForTransactions: {}, ...getInitialStateOverrides(), }); @@ -398,6 +402,14 @@ const controllerMetadata = { persist: false, anonymous: true, }, + enforcedSimulationsSlippage: { + persist: true, + anonymous: true, + }, + enforcedSimulationsSlippageForTransactions: { + persist: false, + anonymous: true, + }, }; export class AppStateController extends BaseController< @@ -1148,4 +1160,19 @@ export class AppStateController extends BaseController< state.enableEnforcedSimulationsForTransactions[transactionId] = enabled; }); } + + setEnforcedSimulationsSlippage(value: number): void { + this.update((state) => { + state.enforcedSimulationsSlippage = value; + }); + } + + setEnforcedSimulationsSlippageForTransaction( + transactionId: string, + value: number, + ): void { + this.update((state) => { + state.enforcedSimulationsSlippageForTransactions[transactionId] = value; + }); + } } diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index e2d7f8af99e..feb286c83df 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -3835,6 +3835,10 @@ export default class MetamaskController extends EventEmitter { appStateController.setEnableEnforcedSimulationsForTransaction.bind( appStateController, ), + setEnforcedSimulationsSlippageForTransaction: + appStateController.setEnforcedSimulationsSlippageForTransaction.bind( + appStateController, + ), // EnsController tryReverseResolveAddress: diff --git a/ui/pages/confirmations/components/modals/simulation-settings-modal/simulation-settings-modal.tsx b/ui/pages/confirmations/components/modals/simulation-settings-modal/simulation-settings-modal.tsx index 00394c8c83d..37d19077dd9 100644 --- a/ui/pages/confirmations/components/modals/simulation-settings-modal/simulation-settings-modal.tsx +++ b/ui/pages/confirmations/components/modals/simulation-settings-modal/simulation-settings-modal.tsx @@ -7,6 +7,8 @@ import { useSelector } from 'react-redux'; import { AlignItems, BackgroundColor, + BlockSize, + BorderColor, BorderRadius, Display, FlexDirection, @@ -24,6 +26,8 @@ import { ModalHeader, ModalOverlay, Text, + TextField, + TextFieldType, } from '../../../../../components/component-library'; import { useI18nContext } from '../../../../../hooks/useI18nContext'; import ToggleButton from '../../../../../components/ui/toggle-button'; @@ -31,8 +35,13 @@ import { useConfirmContext } from '../../../context/confirm'; import { applyTransactionContainersExisting, setEnableEnforcedSimulationsForTransaction, + setEnforcedSimulationsSlippageForTransaction, } from '../../../../../store/actions'; -import { selectEnableEnforcedSimulations } from '../../../selectors'; +import { + selectEnableEnforcedSimulations, + selectEnforcedSimulationsDefaultSlippage, + selectEnforcedSimulationsSlippage, +} from '../../../selectors'; import { ConfirmMetamaskState } from '../../../types/confirm'; const Section = ({ children }: { children: React.ReactNode | string }) => { @@ -64,17 +73,54 @@ export const SimulationSettingsModal = ({ selectEnableEnforcedSimulations(state, transactionId), ); + const defaultSlippage = useSelector(selectEnforcedSimulationsDefaultSlippage); + + const savedSlippage = useSelector((state: ConfirmMetamaskState) => + selectEnforcedSimulationsSlippage(state, transactionId), + ); + const isEnforcedSimulationApplied = containerTypes?.includes( TransactionContainerType.EnforcedSimulations, ); const [enabled, setEnabled] = useState(isEnforcedSimulationsEnabled); + const [customSlippage, setCustomSlippage] = useState(savedSlippage); + + const [isCustomSlippage, setIsCustomSlippage] = useState( + savedSlippage !== defaultSlippage, + ); + + const handleCustomSlippageChange = useCallback( + (event: React.ChangeEvent) => { + const { value } = event.target; + const parsedValue = parseInt(value, 10); + setCustomSlippage(parsedValue); + }, + [], + ); + + const handleDefaultSlippageClick = useCallback(() => { + setIsCustomSlippage(false); + }, []); + + const handleCustomSlippageClick = useCallback(() => { + setIsCustomSlippage(true); + }, []); const handleUpdateClick = useCallback(async () => { await setEnableEnforcedSimulationsForTransaction(transactionId, enabled); let newContainerTypes = containerTypes || []; + const slippage = isCustomSlippage ? customSlippage : defaultSlippage; + + if (slippage !== savedSlippage) { + await setEnforcedSimulationsSlippageForTransaction( + transactionId, + slippage, + ); + } + if (!enabled && isEnforcedSimulationApplied) { newContainerTypes = newContainerTypes.filter( (type) => type !== TransactionContainerType.EnforcedSimulations, @@ -98,9 +144,13 @@ export const SimulationSettingsModal = ({ onClose?.(); }, [ containerTypes, + customSlippage, + defaultSlippage, enabled, + isCustomSlippage, isEnforcedSimulationApplied, onClose, + savedSlippage, transactionId, ]); @@ -150,6 +200,39 @@ export const SimulationSettingsModal = ({ {t('simulationSettingsModalEnforceToggleDescription')} +
+ Slippage tolerance + + + + {isCustomSlippage && ( + + )} + + + Set the percentage difference you're comfortable with for the + displayed balance changes. + +
); }; + +function Pill({ + isSelected = false, + onClick, + text, +}: { + isSelected?: boolean; + onClick?: () => void; + text: string; +}) { + return ( + + + {text} + + + ); +} diff --git a/ui/pages/confirmations/selectors/confirm.ts b/ui/pages/confirmations/selectors/confirm.ts index 957cd42bc59..3898da957a9 100644 --- a/ui/pages/confirmations/selectors/confirm.ts +++ b/ui/pages/confirmations/selectors/confirm.ts @@ -46,3 +46,19 @@ export function selectEnableEnforcedSimulations( state.metamask.enableEnforcedSimulations ); } + +export function selectEnforcedSimulationsDefaultSlippage( + state: ConfirmMetamaskState, +): number { + return state.metamask.enforcedSimulationsSlippage; +} + +export function selectEnforcedSimulationsSlippage( + state: ConfirmMetamaskState, + transactionId: string, +): number { + return ( + state.metamask.enforcedSimulationsSlippageForTransactions[transactionId] ?? + state.metamask.enforcedSimulationsSlippage + ); +} diff --git a/ui/pages/confirmations/types/confirm.ts b/ui/pages/confirmations/types/confirm.ts index d07323f1700..63698a401db 100644 --- a/ui/pages/confirmations/types/confirm.ts +++ b/ui/pages/confirmations/types/confirm.ts @@ -53,5 +53,7 @@ export type ConfirmMetamaskState = { signatureSecurityAlertResponses?: Record; enableEnforcedSimulations: boolean; enableEnforcedSimulationsForTransactions: Record; + enforcedSimulationsSlippage: number; + enforcedSimulationsSlippageForTransactions: Record; }; }; diff --git a/ui/store/actions.ts b/ui/store/actions.ts index c606fc62f48..c919a158833 100644 --- a/ui/store/actions.ts +++ b/ui/store/actions.ts @@ -1374,6 +1374,21 @@ export async function setEnableEnforcedSimulationsForTransaction( } } +export async function setEnforcedSimulationsSlippageForTransaction( + transactionId: string, + value: number, +): void { + try { + await submitRequestToBackground( + 'setEnforcedSimulationsSlippageForTransaction', + [transactionId, value], + ); + } catch (error) { + logErrorWithMessage(error); + throw error; + } +} + // TODO: Not a thunk, but rather a wrapper around a background call export function updateTransactionGasFees( txId: string, From 924d7e8a7818d0f17a6f08fc60da98ecffb56c2f Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Tue, 24 Jun 2025 18:24:26 +0100 Subject: [PATCH 02/11] Apply slippage --- .../containers/enforced-simulations.test.ts | 1 + .../containers/enforced-simulations.ts | 36 ++++++++++++++++--- .../lib/transaction/containers/util.ts | 17 ++++++++- 3 files changed, 48 insertions(+), 6 deletions(-) diff --git a/app/scripts/lib/transaction/containers/enforced-simulations.test.ts b/app/scripts/lib/transaction/containers/enforced-simulations.test.ts index f66d2f98399..6dce5091d1e 100644 --- a/app/scripts/lib/transaction/containers/enforced-simulations.test.ts +++ b/app/scripts/lib/transaction/containers/enforced-simulations.test.ts @@ -71,6 +71,7 @@ describe('Enforced Simulations Utils', () => { chainId: CHAIN_ID_MOCK, messenger, simulationData: cloneDeep(SIMULATION_DATA_MOCK), + slippage: 10, txParams: TX_PARAMS_MOCK, }; }); diff --git a/app/scripts/lib/transaction/containers/enforced-simulations.ts b/app/scripts/lib/transaction/containers/enforced-simulations.ts index b61d21b04d6..a12f5797070 100644 --- a/app/scripts/lib/transaction/containers/enforced-simulations.ts +++ b/app/scripts/lib/transaction/containers/enforced-simulations.ts @@ -5,6 +5,7 @@ import { TransactionParams, } from '@metamask/transaction-controller'; import { Hex, createProjectLogger, hexToNumber } from '@metamask/utils'; +import { BigNumber } from 'bignumber.js'; import { TransactionControllerInitMessenger } from '../../../controller-init/messengers/transaction-controller-messenger'; import { DeleGatorEnvironment, @@ -30,12 +31,14 @@ export async function enforceSimulations({ chainId, messenger, simulationData, + slippage, txParams, useRealSignature = false, }: { chainId: Hex; messenger: TransactionControllerInitMessenger; simulationData: SimulationData; + slippage: number; txParams: TransactionParams; useRealSignature?: boolean; }) { @@ -48,6 +51,7 @@ export async function enforceSimulations({ accountAddress: from, environment: delegationEnvironment, simulationData, + slippage, }); log('Delegation', delegation); @@ -88,12 +92,19 @@ function generateDelegation({ accountAddress, environment, simulationData, + slippage, }: { accountAddress: Hex; environment: DeleGatorEnvironment; simulationData: SimulationData; + slippage: number; }): UnsignedDelegation { - const caveats = generateCaveats(accountAddress, environment, simulationData); + const caveats = generateCaveats( + accountAddress, + environment, + simulationData, + slippage, + ); log('Caveats', caveats); @@ -137,18 +148,21 @@ function generateCaveats( recipient: Hex, environment: DeleGatorEnvironment, simulationData: SimulationData, + slippage: number, ) { const caveatBuilder = createCaveatBuilder(environment); const { nativeBalanceChange, tokenBalanceChanges = [] } = simulationData; if (nativeBalanceChange) { const { difference, isDecrease: enforceDecrease } = nativeBalanceChange; - const delta = BigInt(difference); + const delta = applySlippage(difference, slippage, enforceDecrease); log('Caveat - Native Balance Change', { enforceDecrease, recipient, - delta, + delta: BigInt(difference), + slippage, + deltaWithSlippage: delta, }); caveatBuilder.addCaveat( @@ -168,14 +182,16 @@ function generateCaveats( id: tokenIdHex, } = tokenChange; - const delta = BigInt(difference); + const delta = applySlippage(difference, slippage, enforceDecrease); const tokenId = tokenIdHex ? BigInt(tokenIdHex) : 0n; log('Caveat - Token Balance Change', { enforceDecrease, token, recipient, - delta, + delta: BigInt(difference), + slippage, + deltaWithSlippage: delta, }); switch (standard) { @@ -218,3 +234,13 @@ function generateCaveats( return caveatBuilder.build(); } + +function applySlippage( + value: Hex, + slippage: number, + isDecrease: boolean, +): bigint { + const valueBN = new BigNumber(value); + const slippageMultiplier = (100 + (isDecrease ? slippage : -slippage)) / 100; + return BigInt(valueBN.mul(slippageMultiplier).toFixed(0)); +} diff --git a/app/scripts/lib/transaction/containers/util.ts b/app/scripts/lib/transaction/containers/util.ts index 033202a366e..4084492ef61 100644 --- a/app/scripts/lib/transaction/containers/util.ts +++ b/app/scripts/lib/transaction/containers/util.ts @@ -24,7 +24,12 @@ export async function applyTransactionContainers({ }): Promise<{ updateTransaction: (transaction: TransactionMeta) => void; }> { - const { chainId, simulationData, txParamsOriginal } = transactionMeta; + const { + chainId, + id: transactionid, + simulationData, + txParamsOriginal, + } = transactionMeta; const finalMetadata = cloneDeep(transactionMeta); if (txParamsOriginal) { @@ -32,10 +37,20 @@ export async function applyTransactionContainers({ } if (types.includes(TransactionContainerType.EnforcedSimulations)) { + const appControllerState = await messenger.call( + 'AppStateController:getState', + ); + + const slippage = + appControllerState.enforcedSimulationsSlippageForTransactions[ + transactionid + ] ?? appControllerState.enforcedSimulationsSlippage; + const { updateTransaction } = await enforceSimulations({ chainId, messenger, simulationData: simulationData ?? { tokenBalanceChanges: [] }, + slippage, txParams: finalMetadata.txParams, useRealSignature: isApproved, }); From 9a6ec46df06e96af5a44fc55f956242503af051e Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Wed, 25 Jun 2025 14:20:44 +0100 Subject: [PATCH 03/11] Create slippage local component --- .../simulation-settings-modal.tsx | 153 ++++++++++-------- 1 file changed, 83 insertions(+), 70 deletions(-) diff --git a/ui/pages/confirmations/components/modals/simulation-settings-modal/simulation-settings-modal.tsx b/ui/pages/confirmations/components/modals/simulation-settings-modal/simulation-settings-modal.tsx index 37d19077dd9..798b7141dbe 100644 --- a/ui/pages/confirmations/components/modals/simulation-settings-modal/simulation-settings-modal.tsx +++ b/ui/pages/confirmations/components/modals/simulation-settings-modal/simulation-settings-modal.tsx @@ -1,4 +1,4 @@ -import React, { useCallback, useState } from 'react'; +import React, { useCallback, useEffect, useState } from 'react'; import { TransactionContainerType, TransactionMeta, @@ -73,51 +73,22 @@ export const SimulationSettingsModal = ({ selectEnableEnforcedSimulations(state, transactionId), ); - const defaultSlippage = useSelector(selectEnforcedSimulationsDefaultSlippage); - - const savedSlippage = useSelector((state: ConfirmMetamaskState) => - selectEnforcedSimulationsSlippage(state, transactionId), - ); - const isEnforcedSimulationApplied = containerTypes?.includes( TransactionContainerType.EnforcedSimulations, ); const [enabled, setEnabled] = useState(isEnforcedSimulationsEnabled); - const [customSlippage, setCustomSlippage] = useState(savedSlippage); - - const [isCustomSlippage, setIsCustomSlippage] = useState( - savedSlippage !== defaultSlippage, - ); - - const handleCustomSlippageChange = useCallback( - (event: React.ChangeEvent) => { - const { value } = event.target; - const parsedValue = parseInt(value, 10); - setCustomSlippage(parsedValue); - }, - [], - ); - - const handleDefaultSlippageClick = useCallback(() => { - setIsCustomSlippage(false); - }, []); - - const handleCustomSlippageClick = useCallback(() => { - setIsCustomSlippage(true); - }, []); + const [updatedSlippage, setUpdatedSlippage] = useState(); const handleUpdateClick = useCallback(async () => { await setEnableEnforcedSimulationsForTransaction(transactionId, enabled); let newContainerTypes = containerTypes || []; - const slippage = isCustomSlippage ? customSlippage : defaultSlippage; - - if (slippage !== savedSlippage) { + if (updatedSlippage !== undefined) { await setEnforcedSimulationsSlippageForTransaction( transactionId, - slippage, + updatedSlippage, ); } @@ -144,14 +115,11 @@ export const SimulationSettingsModal = ({ onClose?.(); }, [ containerTypes, - customSlippage, - defaultSlippage, enabled, - isCustomSlippage, isEnforcedSimulationApplied, onClose, - savedSlippage, transactionId, + updatedSlippage, ]); return ( @@ -200,39 +168,10 @@ export const SimulationSettingsModal = ({ {t('simulationSettingsModalEnforceToggleDescription')} -
- Slippage tolerance - - - - {isCustomSlippage && ( - - )} - - - Set the percentage difference you're comfortable with for the - displayed balance changes. - -
+ void; + transactionId: string; +}) { + const defaultSlippage = useSelector(selectEnforcedSimulationsDefaultSlippage); + + const savedSlippage = useSelector((state: ConfirmMetamaskState) => + selectEnforcedSimulationsSlippage(state, transactionId), + ); + + const [customSlippage, setCustomSlippage] = useState(savedSlippage); + + const [isCustomSlippage, setIsCustomSlippage] = useState( + savedSlippage !== defaultSlippage, + ); + + const handleCustomSlippageChange = useCallback( + (event: React.ChangeEvent) => { + const { value } = event.target; + const parsedValue = parseInt(value, 10); + setCustomSlippage(parsedValue); + }, + [], + ); + + const handleDefaultSlippageClick = useCallback(() => { + setIsCustomSlippage(false); + }, []); + + const handleCustomSlippageClick = useCallback(() => { + setIsCustomSlippage(true); + }, []); + + const slippage = isCustomSlippage ? customSlippage : defaultSlippage; + const changeValue = slippage === savedSlippage ? undefined : slippage; + + useEffect(() => { + onChange?.(changeValue); + }, [changeValue, onChange]); + + return ( +
+ Slippage tolerance + + + + + {isCustomSlippage && ( + + )} + + Set the percentage difference you're comfortable with for the displayed + balance changes. + +
+ ); +} + function Pill({ isSelected = false, onClick, From 7929e46af1e2e02acf4a124eb71e5028feb4f630 Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Thu, 26 Jun 2025 12:11:50 +0100 Subject: [PATCH 04/11] Add controller unit tests --- .../controllers/app-state-controller.test.ts | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/app/scripts/controllers/app-state-controller.test.ts b/app/scripts/controllers/app-state-controller.test.ts index 780496cfd62..8c7c7992478 100644 --- a/app/scripts/controllers/app-state-controller.test.ts +++ b/app/scripts/controllers/app-state-controller.test.ts @@ -652,6 +652,32 @@ describe('AppStateController', () => { }); }); }); + + describe('setEnforcedSimulationsSlippage', () => { + it('updates the enforcedSimulationsSlippage state', async () => { + await withController(({ controller }) => { + controller.setEnforcedSimulationsSlippage(23); + expect(controller.state.enforcedSimulationsSlippage).toBe(23); + }); + }); + }); + + describe('setEnforcedSimulationsSlippageForTransaction', () => { + it('updates the enforcedSimulationsSlippageForTransactions state', async () => { + await withController(({ controller }) => { + controller.setEnforcedSimulationsSlippageForTransaction( + TRANSACTION_ID_MOCK, + 25, + ); + + expect( + controller.state.enforcedSimulationsSlippageForTransactions, + ).toStrictEqual({ + [TRANSACTION_ID_MOCK]: 25, + }); + }); + }); + }); }); type WithControllerOptions = { From e094925fe7710101e53d9810873117344801addc Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Thu, 26 Jun 2025 13:24:36 +0100 Subject: [PATCH 05/11] Add util unit tests Accept transactionMeta in util. --- .../containers/enforced-simulations.test.ts | 152 ++++++++++++++++-- .../containers/enforced-simulations.ts | 52 ++++-- .../lib/transaction/containers/util.ts | 21 +-- 3 files changed, 184 insertions(+), 41 deletions(-) diff --git a/app/scripts/lib/transaction/containers/enforced-simulations.test.ts b/app/scripts/lib/transaction/containers/enforced-simulations.test.ts index 6dce5091d1e..5beefcbe7c3 100644 --- a/app/scripts/lib/transaction/containers/enforced-simulations.test.ts +++ b/app/scripts/lib/transaction/containers/enforced-simulations.test.ts @@ -9,7 +9,12 @@ import { cloneDeep } from 'lodash'; import { DELEGATOR_CONTRACTS } from '@metamask/delegation-deployments'; import { Hex, remove0x } from '@metamask/utils'; import { DelegationControllerSignDelegationAction } from '@metamask/delegation-controller'; +import { toHex } from '@metamask/controller-utils'; import { TransactionControllerInitMessenger } from '../../../controller-init/messengers/transaction-controller-messenger'; +import { + AppStateControllerGetStateAction, + AppStateControllerState, +} from '../../../controllers/app-state-controller'; import { enforceSimulations } from './enforced-simulations'; const TOKEN_MOCK = '0x4567890abcdef1234567890abcdef1234567890a' as Hex; @@ -35,12 +40,20 @@ const TX_PARAMS_MOCK: TransactionParams = { }; const TRANSACTION_META_MOCK: TransactionMeta = { - txParams: {}, + chainId: CHAIN_ID_MOCK, + id: '123-456', + simulationData: SIMULATION_DATA_MOCK, + txParams: TX_PARAMS_MOCK, } as TransactionMeta; describe('Enforced Simulations Utils', () => { let messenger: TransactionControllerInitMessenger; let options: Parameters[0]; + let simulationData: SimulationData; + + const getAppStateMock: jest.MockedFn< + AppStateControllerGetStateAction['handler'] + > = jest.fn(); const signDelegationMock: jest.MockedFn< DelegationControllerSignDelegationAction['handler'] @@ -50,10 +63,16 @@ describe('Enforced Simulations Utils', () => { jest.resetAllMocks(); const baseMessenger = new Messenger< - DelegationControllerSignDelegationAction, + | AppStateControllerGetStateAction + | DelegationControllerSignDelegationAction, never >(); + baseMessenger.registerActionHandler( + 'AppStateController:getState', + getAppStateMock, + ); + baseMessenger.registerActionHandler( 'DelegationController:signDelegation', signDelegationMock, @@ -61,19 +80,26 @@ describe('Enforced Simulations Utils', () => { messenger = baseMessenger.getRestricted({ name: 'TransactionControllerInitMessenger', - allowedActions: ['DelegationController:signDelegation'], + allowedActions: [ + 'AppStateController:getState', + 'DelegationController:signDelegation', + ], allowedEvents: [], }); + getAppStateMock.mockReturnValue({ + enforcedSimulationsSlippage: 10, + enforcedSimulationsSlippageForTransactions: {}, + } as AppStateControllerState); + signDelegationMock.mockResolvedValue(DELEGATION_SIGNATURE_MOCK); options = { - chainId: CHAIN_ID_MOCK, messenger, - simulationData: cloneDeep(SIMULATION_DATA_MOCK), - slippage: 10, - txParams: TX_PARAMS_MOCK, + transactionMeta: cloneDeep(TRANSACTION_META_MOCK), }; + + simulationData = options.transactionMeta.simulationData as SimulationData; }); describe('enforceSimulations', () => { @@ -124,7 +150,7 @@ describe('Enforced Simulations Utils', () => { }); it('includes erc20 token balance change caveat', async () => { - options.simulationData.tokenBalanceChanges = [ + simulationData.tokenBalanceChanges = [ { ...BALANCE_CHANGE_MOCK, address: TOKEN_MOCK, @@ -147,7 +173,7 @@ describe('Enforced Simulations Utils', () => { }); it('includes erc721 token balance change caveat', async () => { - options.simulationData.tokenBalanceChanges = [ + simulationData.tokenBalanceChanges = [ { ...BALANCE_CHANGE_MOCK, address: TOKEN_MOCK, @@ -170,7 +196,7 @@ describe('Enforced Simulations Utils', () => { }); it('includes erc1155 token balance change caveat', async () => { - options.simulationData.tokenBalanceChanges = [ + simulationData.tokenBalanceChanges = [ { ...BALANCE_CHANGE_MOCK, address: TOKEN_MOCK, @@ -211,5 +237,111 @@ describe('Enforced Simulations Utils', () => { ), ); }); + + describe('applies slippage', () => { + it('if decrease', async () => { + simulationData.tokenBalanceChanges = [ + { + ...BALANCE_CHANGE_MOCK, + difference: toHex(100000), + address: TOKEN_MOCK, + standard: SimulationTokenStandard.erc20, + }, + ]; + + getAppStateMock.mockReturnValue({ + enforcedSimulationsSlippage: 23, + enforcedSimulationsSlippageForTransactions: {}, + } as AppStateControllerState); + + const { updateTransaction } = await enforceSimulations(options); + + const newTransaction = cloneDeep(TRANSACTION_META_MOCK); + updateTransaction?.(newTransaction); + + expect(newTransaction.txParams.data).toStrictEqual( + expect.stringContaining(remove0x(toHex(123000)).toLowerCase()), + ); + }); + + it('if increase', async () => { + simulationData.tokenBalanceChanges = [ + { + ...BALANCE_CHANGE_MOCK, + isDecrease: false, + difference: toHex(100000), + address: TOKEN_MOCK, + standard: SimulationTokenStandard.erc20, + }, + ]; + + getAppStateMock.mockReturnValue({ + enforcedSimulationsSlippage: 23, + enforcedSimulationsSlippageForTransactions: {}, + } as AppStateControllerState); + + const { updateTransaction } = await enforceSimulations(options); + + const newTransaction = cloneDeep(TRANSACTION_META_MOCK); + updateTransaction?.(newTransaction); + + expect(newTransaction.txParams.data).toStrictEqual( + expect.stringContaining(remove0x(toHex(77000)).toLowerCase()), + ); + }); + + it('if overridden', async () => { + simulationData.tokenBalanceChanges = [ + { + ...BALANCE_CHANGE_MOCK, + isDecrease: false, + difference: toHex(100000), + address: TOKEN_MOCK, + standard: SimulationTokenStandard.erc20, + }, + ]; + + getAppStateMock.mockReturnValue({ + enforcedSimulationsSlippage: 10, + enforcedSimulationsSlippageForTransactions: { + [TRANSACTION_META_MOCK.id]: 15, + }, + } as AppStateControllerState); + + const { updateTransaction } = await enforceSimulations(options); + + const newTransaction = cloneDeep(TRANSACTION_META_MOCK); + updateTransaction?.(newTransaction); + + expect(newTransaction.txParams.data).toStrictEqual( + expect.stringContaining(remove0x(toHex(85000)).toLowerCase()), + ); + }); + + // @ts-expect-error Wrong `it` type + it.each([ + SimulationTokenStandard.erc721, + SimulationTokenStandard.erc1155, + ])('unless token is %s', async (standard: SimulationTokenStandard) => { + simulationData.tokenBalanceChanges = [ + { + ...BALANCE_CHANGE_MOCK, + isDecrease: false, + difference: toHex(100000), + address: TOKEN_MOCK, + standard, + }, + ]; + + const { updateTransaction } = await enforceSimulations(options); + + const newTransaction = cloneDeep(TRANSACTION_META_MOCK); + updateTransaction?.(newTransaction); + + expect(newTransaction.txParams.data).not.toStrictEqual( + expect.stringContaining(remove0x(toHex(90000)).toLowerCase()), + ); + }); + }); }); }); diff --git a/app/scripts/lib/transaction/containers/enforced-simulations.ts b/app/scripts/lib/transaction/containers/enforced-simulations.ts index a12f5797070..30d22a65bc3 100644 --- a/app/scripts/lib/transaction/containers/enforced-simulations.ts +++ b/app/scripts/lib/transaction/containers/enforced-simulations.ts @@ -28,24 +28,30 @@ const MOCK_DELEGATION_SIGNATURE = '0x2261a7810ed3e9cde160895909e138e2f68adb2da86fcf98ea0840701df107721fb369ab9b52550ea98832c09f8185284aca4c94bd345e867a4f4461868dd7751b'; export async function enforceSimulations({ - chainId, messenger, - simulationData, - slippage, - txParams, + transactionMeta, useRealSignature = false, }: { - chainId: Hex; messenger: TransactionControllerInitMessenger; - simulationData: SimulationData; - slippage: number; - txParams: TransactionParams; + transactionMeta: TransactionMeta; useRealSignature?: boolean; }) { + log('Enforcing simulations', { + transactionMeta, + useRealSignature, + }); + + const { + chainId, + simulationData = { tokenBalanceChanges: [] }, + txParams, + } = transactionMeta; + const from = txParams.from as Hex; const chainIdDecimal = hexToNumber(chainId); const delegationEnvironment = getDeleGatorEnvironment(chainIdDecimal); const delegationManagerAddress = delegationEnvironment.DelegationManager; + const slippage = getSlippage(messenger, transactionMeta.id); const delegation = generateDelegation({ accountAddress: from, @@ -182,16 +188,23 @@ function generateCaveats( id: tokenIdHex, } = tokenChange; - const delta = applySlippage(difference, slippage, enforceDecrease); + const delta = BigInt(difference); + + const deltaWithSlippage = applySlippage( + difference, + slippage, + enforceDecrease, + ); + const tokenId = tokenIdHex ? BigInt(tokenIdHex) : 0n; log('Caveat - Token Balance Change', { enforceDecrease, token, recipient, - delta: BigInt(difference), + delta, slippage, - deltaWithSlippage: delta, + deltaWithSlippage, }); switch (standard) { @@ -201,7 +214,7 @@ function generateCaveats( enforceDecrease, token, recipient, - delta, + deltaWithSlippage, ); break; @@ -235,6 +248,21 @@ function generateCaveats( return caveatBuilder.build(); } +function getSlippage( + messenger: TransactionControllerInitMessenger, + transactionId: string, +): number { + const appControllerState = messenger.call('AppStateController:getState'); + const defaultValue = appControllerState.enforcedSimulationsSlippage; + + const transactionOverride = + appControllerState.enforcedSimulationsSlippageForTransactions[ + transactionId + ]; + + return transactionOverride ?? defaultValue; +} + function applySlippage( value: Hex, slippage: number, diff --git a/app/scripts/lib/transaction/containers/util.ts b/app/scripts/lib/transaction/containers/util.ts index 4084492ef61..d2329b2ba55 100644 --- a/app/scripts/lib/transaction/containers/util.ts +++ b/app/scripts/lib/transaction/containers/util.ts @@ -24,12 +24,7 @@ export async function applyTransactionContainers({ }): Promise<{ updateTransaction: (transaction: TransactionMeta) => void; }> { - const { - chainId, - id: transactionid, - simulationData, - txParamsOriginal, - } = transactionMeta; + const { txParamsOriginal } = transactionMeta; const finalMetadata = cloneDeep(transactionMeta); if (txParamsOriginal) { @@ -37,21 +32,9 @@ export async function applyTransactionContainers({ } if (types.includes(TransactionContainerType.EnforcedSimulations)) { - const appControllerState = await messenger.call( - 'AppStateController:getState', - ); - - const slippage = - appControllerState.enforcedSimulationsSlippageForTransactions[ - transactionid - ] ?? appControllerState.enforcedSimulationsSlippage; - const { updateTransaction } = await enforceSimulations({ - chainId, messenger, - simulationData: simulationData ?? { tokenBalanceChanges: [] }, - slippage, - txParams: finalMetadata.txParams, + transactionMeta: finalMetadata, useRealSignature: isApproved, }); From ee68ad4d1a8ca38355eb07e939b6827379f0367b Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Sat, 28 Jun 2025 21:40:28 +0100 Subject: [PATCH 06/11] Split click handler --- .../simulation-settings-modal.tsx | 128 +++++++++++------- 1 file changed, 81 insertions(+), 47 deletions(-) diff --git a/ui/pages/confirmations/components/modals/simulation-settings-modal/simulation-settings-modal.tsx b/ui/pages/confirmations/components/modals/simulation-settings-modal/simulation-settings-modal.tsx index 798b7141dbe..6aad24817cd 100644 --- a/ui/pages/confirmations/components/modals/simulation-settings-modal/simulation-settings-modal.tsx +++ b/ui/pages/confirmations/components/modals/simulation-settings-modal/simulation-settings-modal.tsx @@ -80,35 +80,54 @@ export const SimulationSettingsModal = ({ const [enabled, setEnabled] = useState(isEnforcedSimulationsEnabled); const [updatedSlippage, setUpdatedSlippage] = useState(); - const handleUpdateClick = useCallback(async () => { - await setEnableEnforcedSimulationsForTransaction(transactionId, enabled); + const handleUpdateSlippage = useCallback(async () => { + if (updatedSlippage === undefined) { + return false; + } - let newContainerTypes = containerTypes || []; + await setEnforcedSimulationsSlippageForTransaction( + transactionId, + updatedSlippage, + ); - if (updatedSlippage !== undefined) { - await setEnforcedSimulationsSlippageForTransaction( - transactionId, - updatedSlippage, - ); - } + return true; + }, [transactionId, updatedSlippage]); + + const handleDisable = useCallback(async () => { + await setEnableEnforcedSimulationsForTransaction(transactionId, false); + + const newContainerTypes = (containerTypes ?? []).filter( + (type) => type !== TransactionContainerType.EnforcedSimulations, + ); + + await applyTransactionContainersExisting(transactionId, newContainerTypes); + }, [containerTypes, transactionId]); + + const handleEnable = useCallback(async () => { + const newContainerTypes = [ + ...(containerTypes ?? []), + TransactionContainerType.EnforcedSimulations, + ]; + + await applyTransactionContainersExisting(transactionId, newContainerTypes); + await setEnableEnforcedSimulationsForTransaction(transactionId, true); + }, [containerTypes, transactionId]); + + const handleUpdateClick = useCallback(async () => { + const slippageUpdated = await handleUpdateSlippage(); if (!enabled && isEnforcedSimulationApplied) { - newContainerTypes = newContainerTypes.filter( - (type) => type !== TransactionContainerType.EnforcedSimulations, - ); + await handleDisable(); } if (enabled && !isEnforcedSimulationApplied) { - newContainerTypes = [ - ...newContainerTypes, - TransactionContainerType.EnforcedSimulations, - ]; + await handleEnable(); } - if (newContainerTypes.length !== containerTypes?.length) { + if (enabled && isEnforcedSimulationApplied && slippageUpdated) { await applyTransactionContainersExisting( transactionId, - newContainerTypes, + containerTypes ?? [], ); } @@ -116,10 +135,12 @@ export const SimulationSettingsModal = ({ }, [ containerTypes, enabled, + handleDisable, + handleEnable, + handleUpdateSlippage, isEnforcedSimulationApplied, onClose, transactionId, - updatedSlippage, ]); return ( @@ -144,34 +165,13 @@ export const SimulationSettingsModal = ({ flexDirection={FlexDirection.Column} gap={3} > -
- - - {t('simulationSettingsModalEnforceToggle')} - - setEnabled(!enabled)} - /> - - - {t('simulationSettingsModalEnforceToggleDescription')} - -
- + + {enabled && ( + + )} void; +}) { + const t = useI18nContext(); + + return ( +
+ + + {t('simulationSettingsModalEnforceToggle')} + + onChange?.(!enabled)} + /> + + + {t('simulationSettingsModalEnforceToggleDescription')} + +
+ ); +} + function Slippage({ onChange, transactionId, From ae0748c344a7413578c494cddb96d58454b60c35 Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Sat, 28 Jun 2025 22:02:16 +0100 Subject: [PATCH 07/11] Add translations --- app/_locales/en/messages.json | 9 +++++++++ app/_locales/en_GB/messages.json | 9 +++++++++ .../simulation-settings-modal.tsx | 10 ++++++---- 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json index c4d31aa55a4..538c519f94d 100644 --- a/app/_locales/en/messages.json +++ b/app/_locales/en/messages.json @@ -5389,6 +5389,15 @@ "simulationErrorMessageV2": { "message": "We were not able to estimate gas. There might be an error in the contract and this transaction may fail." }, + "simulationSettingsModalEnforceSlippage": { + "message": "Slippage tolerance" + }, + "simulationSettingsModalEnforceSlippageCustom": { + "message": "Custom" + }, + "simulationSettingsModalEnforceSlippageDescription": { + "message": "Set a difference you're comfortable with for balance changes. Transactions won't go through if the difference is higher." + }, "simulationSettingsModalEnforceToggle": { "message": "Enforce balance changes" }, diff --git a/app/_locales/en_GB/messages.json b/app/_locales/en_GB/messages.json index c4d31aa55a4..538c519f94d 100644 --- a/app/_locales/en_GB/messages.json +++ b/app/_locales/en_GB/messages.json @@ -5389,6 +5389,15 @@ "simulationErrorMessageV2": { "message": "We were not able to estimate gas. There might be an error in the contract and this transaction may fail." }, + "simulationSettingsModalEnforceSlippage": { + "message": "Slippage tolerance" + }, + "simulationSettingsModalEnforceSlippageCustom": { + "message": "Custom" + }, + "simulationSettingsModalEnforceSlippageDescription": { + "message": "Set a difference you're comfortable with for balance changes. Transactions won't go through if the difference is higher." + }, "simulationSettingsModalEnforceToggle": { "message": "Enforce balance changes" }, diff --git a/ui/pages/confirmations/components/modals/simulation-settings-modal/simulation-settings-modal.tsx b/ui/pages/confirmations/components/modals/simulation-settings-modal/simulation-settings-modal.tsx index 6aad24817cd..7cd677c2388 100644 --- a/ui/pages/confirmations/components/modals/simulation-settings-modal/simulation-settings-modal.tsx +++ b/ui/pages/confirmations/components/modals/simulation-settings-modal/simulation-settings-modal.tsx @@ -225,6 +225,7 @@ function Slippage({ onChange?: (slippage: number | undefined) => void; transactionId: string; }) { + const t = useI18nContext(); const defaultSlippage = useSelector(selectEnforcedSimulationsDefaultSlippage); const savedSlippage = useSelector((state: ConfirmMetamaskState) => @@ -263,7 +264,9 @@ function Slippage({ return (
- Slippage tolerance + + {t('simulationSettingsModalEnforceSlippage')} + @@ -285,8 +288,7 @@ function Slippage({ /> )} - Set the percentage difference you're comfortable with for the displayed - balance changes. + {t('simulationSettingsModalEnforceSlippageDescription')}
); From cf035222944fbea76e154d5ef713fff04c04f12c Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Sat, 28 Jun 2025 22:28:29 +0100 Subject: [PATCH 08/11] Add modal unit tests --- shared/types/background.ts | 2 + test/data/mock-state.json | 2 + .../simulation-settings-modal.test.tsx | 69 +++++++++++++++++-- .../simulation-settings-modal.tsx | 6 ++ 4 files changed, 73 insertions(+), 6 deletions(-) diff --git a/shared/types/background.ts b/shared/types/background.ts index df2d528a1c7..5f767cd1bd5 100644 --- a/shared/types/background.ts +++ b/shared/types/background.ts @@ -136,6 +136,8 @@ export type ControllerStatePropertiesEnumerated = { throttledOrigins: AppStateControllerState['throttledOrigins']; enableEnforcedSimulations: AppStateControllerState['enableEnforcedSimulations']; enableEnforcedSimulationsForTransactions: AppStateControllerState['enableEnforcedSimulationsForTransactions']; + enforcedSimulationsSlippage: AppStateControllerState['enforcedSimulationsSlippage']; + enforcedSimulationsSlippageForTransactions: AppStateControllerState['enforcedSimulationsSlippageForTransactions']; quoteRequest: BridgeControllerState['quoteRequest']; quotes: BridgeControllerState['quotes']; quotesInitialLoadTime: BridgeControllerState['quotesInitialLoadTime']; diff --git a/test/data/mock-state.json b/test/data/mock-state.json index c9dc55eccf0..fec32753b03 100644 --- a/test/data/mock-state.json +++ b/test/data/mock-state.json @@ -63,6 +63,8 @@ "allTokens": {}, "enableEnforcedSimulations": true, "enableEnforcedSimulationsForTransactions": {}, + "enforcedSimulationsSlippage": 10, + "enforcedSimulationsSlippageForTransactions": {}, "tokenBalances": {}, "use4ByteResolution": true, "ipfsGateway": "dweb.link", diff --git a/ui/pages/confirmations/components/modals/simulation-settings-modal/simulation-settings-modal.test.tsx b/ui/pages/confirmations/components/modals/simulation-settings-modal/simulation-settings-modal.test.tsx index 73633e4ec7b..2489420c949 100644 --- a/ui/pages/confirmations/components/modals/simulation-settings-modal/simulation-settings-modal.test.tsx +++ b/ui/pages/confirmations/components/modals/simulation-settings-modal/simulation-settings-modal.test.tsx @@ -6,12 +6,13 @@ import { TransactionContainerType, TransactionParams, } from '@metamask/transaction-controller'; +import { fireEvent } from '@testing-library/react'; import configureStore from '../../../../../store/store'; import { renderWithConfirmContextProvider } from '../../../../../../test/lib/confirmations/render-helpers'; import { applyTransactionContainersExisting, setEnableEnforcedSimulationsForTransaction, - updateEditableParams, + setEnforcedSimulationsSlippageForTransaction, } from '../../../../../store/actions'; import { getMockConfirmStateForTransaction } from '../../../../../../test/data/confirmations/helper'; import { genUnapprovedContractInteractionConfirmation } from '../../../../../../test/data/confirmations/contract-interaction'; @@ -50,15 +51,16 @@ describe('SimulationSettingsModal', () => { setEnableEnforcedSimulationsForTransaction, ); + const setEnforcedSimulationsSlippageForTransactionMock = jest.mocked( + setEnforcedSimulationsSlippageForTransaction, + ); + const applyTransactionContainersExistingMock = jest.mocked( applyTransactionContainersExisting, ); - const updateEditableParamsMock = jest.mocked(updateEditableParams); - beforeEach(() => { jest.resetAllMocks(); - updateEditableParamsMock.mockReturnValue(async () => ({}) as never); }); describe('renders', () => { @@ -127,7 +129,7 @@ describe('SimulationSettingsModal', () => { it('sets enforced simulations enabled', async () => { const { getByTestId } = render({ metamaskState: { - enableEnforcedSimulations: true, + enableEnforcedSimulations: false, enableEnforcedSimulationsForTransactions: {}, }, }); @@ -142,7 +144,7 @@ describe('SimulationSettingsModal', () => { expect( setEnableEnforcedSimulationsForTransactionMock, - ).toHaveBeenCalledWith(TRANSACTION_ID_MOCK, false); + ).toHaveBeenCalledWith(TRANSACTION_ID_MOCK, true); }); it('applies enforced simulations if enabled and not already applied', async () => { @@ -194,5 +196,60 @@ describe('SimulationSettingsModal', () => { [], ); }); + + it('updates slippage if custom', async () => { + const { getByTestId } = render({ + metamaskState: { + enableEnforcedSimulations: true, + enableEnforcedSimulationsForTransactions: {}, + enforcedSimulationsSlippage: 10, + }, + }); + + await act(async () => { + getByTestId('simulation-settings-modal-slippage-custom').click(); + }); + + await act(async () => { + const input = getByTestId( + 'simulation-settings-modal-slippage-custom-input', + ).querySelector('input') as HTMLInputElement; + + fireEvent.change(input, { target: { value: '20' } }); + }); + + await act(async () => { + getByTestId('simulation-settings-modal-update').click(); + }); + + expect( + setEnforcedSimulationsSlippageForTransactionMock, + ).toHaveBeenCalledWith(TRANSACTION_ID_MOCK, 20); + }); + + it('updates slippage if default', async () => { + const { getByTestId } = render({ + metamaskState: { + enableEnforcedSimulations: true, + enableEnforcedSimulationsForTransactions: {}, + enforcedSimulationsSlippage: 15, + enforcedSimulationsSlippageForTransactions: { + [TRANSACTION_ID_MOCK]: 20, + }, + }, + }); + + await act(async () => { + getByTestId('simulation-settings-modal-slippage-default').click(); + }); + + await act(async () => { + getByTestId('simulation-settings-modal-update').click(); + }); + + expect( + setEnforcedSimulationsSlippageForTransactionMock, + ).toHaveBeenCalledWith(TRANSACTION_ID_MOCK, 15); + }); }); }); diff --git a/ui/pages/confirmations/components/modals/simulation-settings-modal/simulation-settings-modal.tsx b/ui/pages/confirmations/components/modals/simulation-settings-modal/simulation-settings-modal.tsx index 7cd677c2388..1037c39f375 100644 --- a/ui/pages/confirmations/components/modals/simulation-settings-modal/simulation-settings-modal.tsx +++ b/ui/pages/confirmations/components/modals/simulation-settings-modal/simulation-settings-modal.tsx @@ -269,11 +269,13 @@ function Slippage({ {isCustomSlippage && ( void; text: string; }) { return ( Date: Sun, 29 Jun 2025 01:25:30 +0100 Subject: [PATCH 09/11] Fix linting and E2E tests --- .../errors-after-init-opt-in-background-state.json | 4 +++- .../state-snapshots/errors-after-init-opt-in-ui-state.json | 4 +++- test/integration/data/onboarding-completion-route.json | 2 ++ ui/pages/confirmations/selectors/confirm.test.ts | 2 ++ 4 files changed, 10 insertions(+), 2 deletions(-) diff --git a/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-background-state.json b/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-background-state.json index b34a6809daf..a7630542381 100644 --- a/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-background-state.json +++ b/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-background-state.json @@ -60,7 +60,9 @@ "termsOfUseLastAgreed": "number", "snapsInstallPrivacyWarningShown": true, "enableEnforcedSimulations": true, - "enableEnforcedSimulationsForTransactions": "object" + "enableEnforcedSimulationsForTransactions": "object", + "enforcedSimulationsSlippage": "number", + "enforcedSimulationsSlippageForTransactions": "object" }, "ApprovalController": { "pendingApprovals": "object", diff --git a/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-ui-state.json b/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-ui-state.json index 955f3fc0eee..3539e5edde4 100644 --- a/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-ui-state.json +++ b/test/e2e/tests/metrics/state-snapshots/errors-after-init-opt-in-ui-state.json @@ -373,7 +373,9 @@ "socialBackupsMetadata": "object", "srpSessionData": "object", "enableEnforcedSimulations": true, - "enableEnforcedSimulationsForTransactions": "object" + "enableEnforcedSimulationsForTransactions": "object", + "enforcedSimulationsSlippage": "number", + "enforcedSimulationsSlippageForTransactions": "object" }, "ramps": "object", "send": "object", diff --git a/test/integration/data/onboarding-completion-route.json b/test/integration/data/onboarding-completion-route.json index 03becaf0458..7ca11d9f582 100644 --- a/test/integration/data/onboarding-completion-route.json +++ b/test/integration/data/onboarding-completion-route.json @@ -96,6 +96,8 @@ }, "encryptionKey": "{\"key\":{\"alg\":\"A256GCM\",\"ext\":true,\"k\":\"IWP6tqC2WeYfTxhHhTFU2m1hhO9GD5YhgYe2iRzu5GU\",\"key_ops\":[\"encrypt\",\"decrypt\"],\"kty\":\"oct\"},\"derivationOptions\":{\"algorithm\":\"PBKDF2\",\"params\":{\"iterations\":600000}}}", "encryptionSalt": "6AA1VP9N1aU5bglaSiAhmE9PdLH3YCqiKJagv+wQCR8=", + "enforcedSimulationsSlippage": "number", + "enforcedSimulationsSlippageForTransactions": "object", "ensEntries": {}, "ensResolutionsByAddress": {}, "estimatedGasFeeTimeBounds": {}, diff --git a/ui/pages/confirmations/selectors/confirm.test.ts b/ui/pages/confirmations/selectors/confirm.test.ts index 7460c9fb54d..062762e8b7b 100644 --- a/ui/pages/confirmations/selectors/confirm.test.ts +++ b/ui/pages/confirmations/selectors/confirm.test.ts @@ -41,6 +41,8 @@ describe('confirm selectors', () => { approvalFlows: [], enableEnforcedSimulations: false, enableEnforcedSimulationsForTransactions: {}, + enforcedSimulationsSlippage: 0, + enforcedSimulationsSlippageForTransactions: {}, }, }; From fb18c9120cc2ef132388ea0128d4a4408b561767 Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Thu, 10 Jul 2025 22:42:49 +0100 Subject: [PATCH 10/11] Remove pills Add settings hook. --- .../simulation-settings-modal.test.tsx | 31 +- .../simulation-settings-modal.tsx | 334 +++++++----------- 2 files changed, 122 insertions(+), 243 deletions(-) diff --git a/ui/pages/confirmations/components/modals/simulation-settings-modal/simulation-settings-modal.test.tsx b/ui/pages/confirmations/components/modals/simulation-settings-modal/simulation-settings-modal.test.tsx index 2489420c949..0ad88483fbc 100644 --- a/ui/pages/confirmations/components/modals/simulation-settings-modal/simulation-settings-modal.test.tsx +++ b/ui/pages/confirmations/components/modals/simulation-settings-modal/simulation-settings-modal.test.tsx @@ -197,7 +197,7 @@ describe('SimulationSettingsModal', () => { ); }); - it('updates slippage if custom', async () => { + it('updates slippage', async () => { const { getByTestId } = render({ metamaskState: { enableEnforcedSimulations: true, @@ -206,10 +206,6 @@ describe('SimulationSettingsModal', () => { }, }); - await act(async () => { - getByTestId('simulation-settings-modal-slippage-custom').click(); - }); - await act(async () => { const input = getByTestId( 'simulation-settings-modal-slippage-custom-input', @@ -226,30 +222,5 @@ describe('SimulationSettingsModal', () => { setEnforcedSimulationsSlippageForTransactionMock, ).toHaveBeenCalledWith(TRANSACTION_ID_MOCK, 20); }); - - it('updates slippage if default', async () => { - const { getByTestId } = render({ - metamaskState: { - enableEnforcedSimulations: true, - enableEnforcedSimulationsForTransactions: {}, - enforcedSimulationsSlippage: 15, - enforcedSimulationsSlippageForTransactions: { - [TRANSACTION_ID_MOCK]: 20, - }, - }, - }); - - await act(async () => { - getByTestId('simulation-settings-modal-slippage-default').click(); - }); - - await act(async () => { - getByTestId('simulation-settings-modal-update').click(); - }); - - expect( - setEnforcedSimulationsSlippageForTransactionMock, - ).toHaveBeenCalledWith(TRANSACTION_ID_MOCK, 15); - }); }); }); diff --git a/ui/pages/confirmations/components/modals/simulation-settings-modal/simulation-settings-modal.tsx b/ui/pages/confirmations/components/modals/simulation-settings-modal/simulation-settings-modal.tsx index 1037c39f375..f582ad89123 100644 --- a/ui/pages/confirmations/components/modals/simulation-settings-modal/simulation-settings-modal.tsx +++ b/ui/pages/confirmations/components/modals/simulation-settings-modal/simulation-settings-modal.tsx @@ -1,4 +1,4 @@ -import React, { useCallback, useEffect, useState } from 'react'; +import React, { useCallback, useState } from 'react'; import { TransactionContainerType, TransactionMeta, @@ -7,8 +7,6 @@ import { useSelector } from 'react-redux'; import { AlignItems, BackgroundColor, - BlockSize, - BorderColor, BorderRadius, Display, FlexDirection, @@ -39,7 +37,6 @@ import { } from '../../../../../store/actions'; import { selectEnableEnforcedSimulations, - selectEnforcedSimulationsDefaultSlippage, selectEnforcedSimulationsSlippage, } from '../../../selectors'; import { ConfirmMetamaskState } from '../../../types/confirm'; @@ -59,12 +56,69 @@ const Section = ({ children }: { children: React.ReactNode | string }) => { ); }; -export const SimulationSettingsModal = ({ - onClose, +const EnforceToggle = ({ + enabled, + onChange, }: { - onClose?: () => void; + enabled: boolean; + onChange?: (value: boolean) => void; }) => { const t = useI18nContext(); + + return ( +
+ + + {t('simulationSettingsModalEnforceToggle')} + + onChange?.(!enabled)} + /> + + + {t('simulationSettingsModalEnforceToggleDescription')} + +
+ ); +}; + +const Slippage = ({ + onChange, + slippage, +}: { + onChange?: (slippage: number) => void; + slippage: number; +}) => { + const t = useI18nContext(); + + return ( +
+ + {t('simulationSettingsModalEnforceSlippage')} + + onChange?.(Number(e.target.value))} + value={slippage} + endAccessory={'%'} + /> + + {t('simulationSettingsModalEnforceSlippageDescription')} + +
+ ); +}; + +function useSimulationSettings() { const { currentConfirmation } = useConfirmContext(); const { containerTypes, id: transactionId } = currentConfirmation || {}; @@ -77,72 +131,86 @@ export const SimulationSettingsModal = ({ TransactionContainerType.EnforcedSimulations, ); - const [enabled, setEnabled] = useState(isEnforcedSimulationsEnabled); - const [updatedSlippage, setUpdatedSlippage] = useState(); - - const handleUpdateSlippage = useCallback(async () => { - if (updatedSlippage === undefined) { - return false; - } - - await setEnforcedSimulationsSlippageForTransaction( - transactionId, - updatedSlippage, - ); - - return true; - }, [transactionId, updatedSlippage]); - - const handleDisable = useCallback(async () => { - await setEnableEnforcedSimulationsForTransaction(transactionId, false); - - const newContainerTypes = (containerTypes ?? []).filter( - (type) => type !== TransactionContainerType.EnforcedSimulations, - ); + const savedSlippage = useSelector((state: ConfirmMetamaskState) => + selectEnforcedSimulationsSlippage(state, transactionId), + ); - await applyTransactionContainersExisting(transactionId, newContainerTypes); - }, [containerTypes, transactionId]); + const [enabled, setEnabled] = useState(isEnforcedSimulationsEnabled); + const [slippage, setSlippage] = useState(savedSlippage); - const handleEnable = useCallback(async () => { - const newContainerTypes = [ - ...(containerTypes ?? []), - TransactionContainerType.EnforcedSimulations, - ]; + const updateSettings = useCallback(async () => { + const slippageUpdated = slippage !== savedSlippage; + const isEnabled = enabled && !isEnforcedSimulationApplied; + const isDisabled = !enabled && isEnforcedSimulationApplied; - await applyTransactionContainersExisting(transactionId, newContainerTypes); - await setEnableEnforcedSimulationsForTransaction(transactionId, true); - }, [containerTypes, transactionId]); + if (slippageUpdated) { + await setEnforcedSimulationsSlippageForTransaction( + transactionId, + slippage, + ); + } - const handleUpdateClick = useCallback(async () => { - const slippageUpdated = await handleUpdateSlippage(); + const newContainerTypes = [...(containerTypes ?? [])]; - if (!enabled && isEnforcedSimulationApplied) { - await handleDisable(); + if (isEnabled) { + newContainerTypes.push(TransactionContainerType.EnforcedSimulations); } - if (enabled && !isEnforcedSimulationApplied) { - await handleEnable(); + if (isDisabled) { + newContainerTypes.splice( + newContainerTypes.indexOf(TransactionContainerType.EnforcedSimulations), + 1, + ); + + await setEnableEnforcedSimulationsForTransaction(transactionId, false); } - if (enabled && isEnforcedSimulationApplied && slippageUpdated) { + if ( + newContainerTypes.length !== containerTypes?.length || + slippageUpdated + ) { await applyTransactionContainersExisting( transactionId, - containerTypes ?? [], + newContainerTypes, ); } - onClose?.(); + if (isEnabled) { + await setEnableEnforcedSimulationsForTransaction(transactionId, true); + } }, [ containerTypes, enabled, - handleDisable, - handleEnable, - handleUpdateSlippage, isEnforcedSimulationApplied, - onClose, + savedSlippage, + slippage, transactionId, ]); + return { + enabled, + setEnabled, + slippage, + setSlippage, + updateSettings, + }; +} + +export const SimulationSettingsModal = ({ + onClose, +}: { + onClose?: () => void; +}) => { + const t = useI18nContext(); + + const { enabled, setEnabled, slippage, setSlippage, updateSettings } = + useSimulationSettings(); + + const handleUpdateClick = useCallback(async () => { + await updateSettings(); + onClose?.(); + }, [updateSettings, onClose]); + return ( - {enabled && ( - - )} + {enabled && } ); }; - -function EnforceToggle({ - enabled, - onChange, -}: { - enabled: boolean; - onChange?: (value: boolean) => void; -}) { - const t = useI18nContext(); - - return ( -
- - - {t('simulationSettingsModalEnforceToggle')} - - onChange?.(!enabled)} - /> - - - {t('simulationSettingsModalEnforceToggleDescription')} - -
- ); -} - -function Slippage({ - onChange, - transactionId, -}: { - onChange?: (slippage: number | undefined) => void; - transactionId: string; -}) { - const t = useI18nContext(); - const defaultSlippage = useSelector(selectEnforcedSimulationsDefaultSlippage); - - const savedSlippage = useSelector((state: ConfirmMetamaskState) => - selectEnforcedSimulationsSlippage(state, transactionId), - ); - - const [customSlippage, setCustomSlippage] = useState(savedSlippage); - - const [isCustomSlippage, setIsCustomSlippage] = useState( - savedSlippage !== defaultSlippage, - ); - - const handleCustomSlippageChange = useCallback( - (event: React.ChangeEvent) => { - const { value } = event.target; - const parsedValue = parseInt(value, 10); - setCustomSlippage(parsedValue); - }, - [], - ); - - const handleDefaultSlippageClick = useCallback(() => { - setIsCustomSlippage(false); - }, []); - - const handleCustomSlippageClick = useCallback(() => { - setIsCustomSlippage(true); - }, []); - - const slippage = isCustomSlippage ? customSlippage : defaultSlippage; - const changeValue = slippage === savedSlippage ? undefined : slippage; - - useEffect(() => { - onChange?.(changeValue); - }, [changeValue, onChange]); - - return ( -
- - {t('simulationSettingsModalEnforceSlippage')} - - - - - - {isCustomSlippage && ( - - )} - - {t('simulationSettingsModalEnforceSlippageDescription')} - -
- ); -} - -function Pill({ - dataTestId, - isSelected = false, - onClick, - text, -}: { - dataTestId?: string; - isSelected?: boolean; - onClick?: () => void; - text: string; -}) { - return ( - - - {text} - - - ); -} From ae29a8ad026c7851d18c4d2c0695a81ca7484d35 Mon Sep 17 00:00:00 2001 From: Matthew Walsh Date: Thu, 10 Jul 2025 22:59:42 +0100 Subject: [PATCH 11/11] Remove translation key --- app/_locales/en/messages.json | 3 --- app/_locales/en_GB/messages.json | 3 --- 2 files changed, 6 deletions(-) diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json index 538c519f94d..664ac94b10e 100644 --- a/app/_locales/en/messages.json +++ b/app/_locales/en/messages.json @@ -5392,9 +5392,6 @@ "simulationSettingsModalEnforceSlippage": { "message": "Slippage tolerance" }, - "simulationSettingsModalEnforceSlippageCustom": { - "message": "Custom" - }, "simulationSettingsModalEnforceSlippageDescription": { "message": "Set a difference you're comfortable with for balance changes. Transactions won't go through if the difference is higher." }, diff --git a/app/_locales/en_GB/messages.json b/app/_locales/en_GB/messages.json index 538c519f94d..664ac94b10e 100644 --- a/app/_locales/en_GB/messages.json +++ b/app/_locales/en_GB/messages.json @@ -5392,9 +5392,6 @@ "simulationSettingsModalEnforceSlippage": { "message": "Slippage tolerance" }, - "simulationSettingsModalEnforceSlippageCustom": { - "message": "Custom" - }, "simulationSettingsModalEnforceSlippageDescription": { "message": "Set a difference you're comfortable with for balance changes. Transactions won't go through if the difference is higher." },