From c35058f741b74d3b5501d49d688f47ce7e51dded Mon Sep 17 00:00:00 2001 From: jpuri Date: Fri, 21 Aug 2026 09:07:29 +0530 Subject: [PATCH] fix(confirmations): hide irrelevant alerts on money account transactions --- .../useMultipleApprovalsAlerts.test.ts | 93 +++++++++++++------ .../useMultipleApprovalsAlerts.ts | 13 ++- .../alerts/useSelectedAccountAlerts.test.ts | 27 +++++- .../hooks/alerts/useSelectedAccountAlerts.ts | 12 ++- 4 files changed, 114 insertions(+), 31 deletions(-) diff --git a/ui/pages/confirmations/hooks/alerts/transactions/useMultipleApprovalsAlerts.test.ts b/ui/pages/confirmations/hooks/alerts/transactions/useMultipleApprovalsAlerts.test.ts index 4ce62fbc5425..e670c9790371 100644 --- a/ui/pages/confirmations/hooks/alerts/transactions/useMultipleApprovalsAlerts.test.ts +++ b/ui/pages/confirmations/hooks/alerts/transactions/useMultipleApprovalsAlerts.test.ts @@ -5,6 +5,7 @@ import { SimulationTokenBalanceChange, SimulationTokenStandard, TransactionMeta, + TransactionType, } from '@metamask/transaction-controller'; import { Hex } from '@metamask/utils'; import { BigNumber } from 'bignumber.js'; @@ -961,37 +962,75 @@ describe('useMultipleApprovalsAlerts', () => { }); }); - describe('when origin is in allow list', () => { - it('returns no alerts', () => { - const originAllowedMock = 'https://example.com'; - const nestedTransactions = [ - createMockNestedTransaction('0x123', TOKEN_ADDRESS_1), - ]; + describe('when transaction is a MetaMask Pay transaction', () => { + [ + TransactionType.moneyAccountDeposit, + TransactionType.moneyAccountWithdraw, + TransactionType.perpsDeposit, + ].forEach((nestedType) => { + it(`returns no alerts for ${nestedType} with unused approvals`, () => { + const nestedTransactions = [ + { + ...createMockNestedTransaction('0x123', TOKEN_ADDRESS_1), + type: nestedType, + }, + ]; + + mockParseApprovalTransactionData.mockReturnValue({ + name: 'approve', + amountOrTokenId: new BigNumber('1000'), + tokenAddress: undefined, + isRevokeAll: false, + }); - mockParseApprovalTransactionData.mockReturnValue({ - name: 'approve', - amountOrTokenId: new BigNumber('1000'), - tokenAddress: undefined, - isRevokeAll: false, - }); + const alerts = runHook({ + currentConfirmation: { + txParams: { from: ACCOUNT_ADDRESS }, + chainId: '0x5', + }, + nestedTransactions, + simulationData: { + tokenBalanceChanges: [], // no outflows - approval is unused + }, + approveBalanceChanges: [MOCK_APPROVAL_BALANCE_CHANGE], + }); - const alerts = runHook({ - currentConfirmation: { - txParams: { from: ACCOUNT_ADDRESS }, - chainId: '0x5', - origin: originAllowedMock, - }, - nestedTransactions, - simulationData: { - tokenBalanceChanges: [], - }, - approveBalanceChanges: [MOCK_APPROVAL_BALANCE_CHANGE], - remoteFeatureFlags: { - nonZeroUnusedApprovals: [originAllowedMock], - }, + expect(alerts).toEqual([]); }); + }); + + describe('when origin is in allow list', () => { + it('returns no alerts', () => { + const originAllowedMock = 'https://example.com'; + const nestedTransactions = [ + createMockNestedTransaction('0x123', TOKEN_ADDRESS_1), + ]; - expect(alerts).toHaveLength(0); + mockParseApprovalTransactionData.mockReturnValue({ + name: 'approve', + amountOrTokenId: new BigNumber('1000'), + tokenAddress: undefined, + isRevokeAll: false, + }); + + const alerts = runHook({ + currentConfirmation: { + txParams: { from: ACCOUNT_ADDRESS }, + chainId: '0x5', + origin: originAllowedMock, + }, + nestedTransactions, + simulationData: { + tokenBalanceChanges: [], + }, + approveBalanceChanges: [MOCK_APPROVAL_BALANCE_CHANGE], + remoteFeatureFlags: { + nonZeroUnusedApprovals: [originAllowedMock], + }, + }); + + expect(alerts).toHaveLength(0); + }); }); }); }); diff --git a/ui/pages/confirmations/hooks/alerts/transactions/useMultipleApprovalsAlerts.ts b/ui/pages/confirmations/hooks/alerts/transactions/useMultipleApprovalsAlerts.ts index df88e7413feb..c9b8019d693f 100644 --- a/ui/pages/confirmations/hooks/alerts/transactions/useMultipleApprovalsAlerts.ts +++ b/ui/pages/confirmations/hooks/alerts/transactions/useMultipleApprovalsAlerts.ts @@ -10,6 +10,7 @@ import { useMemo } from 'react'; import { useSelector } from 'react-redux'; import { TokenStandard } from '../../../../../../shared/constants/transaction'; import { parseApprovalTransactionData } from '../../../../../../shared/lib/transaction.utils'; +import { hasTransactionType } from '../../../../../../shared/lib/transactions.utils'; import { RowAlertKey } from '../../../../../components/app/confirm/info/row/constants'; import { Alert } from '../../../../../ducks/confirm-alerts/confirm-alerts'; import { Severity } from '../../../../../helpers/constants/design-system'; @@ -17,6 +18,7 @@ import { useAsyncResult } from '../../../../../hooks/useAsync'; import { useI18nContext } from '../../../../../hooks/useI18nContext'; import { getTokenStandardAndDetailsByChain } from '../../../../../store/actions'; import { useBatchApproveBalanceChanges } from '../../../components/confirm/info/hooks/useBatchApproveBalanceChanges'; +import { PAY_TRANSACTION_TYPES } from '../../../constants/pay'; import { useConfirmContext } from '../../../context/confirm'; import { getUseTransactionSimulations, @@ -298,11 +300,20 @@ export function useMultipleApprovalsAlerts(): Alert[] { return findUnusedApprovals(approvals, tokenOutflows); }, [approvals, tokenOutflows]); + // MetaMask Pay flows (money account deposits/withdrawals, perps, mUSD) + // batch their own approvals internally; mirrors mobile's + // `useBatchedUnusedApprovalsAlert` MM_PAY_TRANSACTION_TYPES skip. + const isPayTransaction = hasTransactionType( + currentConfirmation, + PAY_TRANSACTION_TYPES, + ); + const shouldShowAlert = unusedApprovals.length > 0 && Boolean(currentConfirmation?.simulationData) && isSimulationSupported && - !skipAlertOriginAllowed; + !skipAlertOriginAllowed && + !isPayTransaction; return useMemo(() => { if (!shouldShowAlert) { diff --git a/ui/pages/confirmations/hooks/alerts/useSelectedAccountAlerts.test.ts b/ui/pages/confirmations/hooks/alerts/useSelectedAccountAlerts.test.ts index 68487d37c7db..361390912266 100644 --- a/ui/pages/confirmations/hooks/alerts/useSelectedAccountAlerts.test.ts +++ b/ui/pages/confirmations/hooks/alerts/useSelectedAccountAlerts.test.ts @@ -1,4 +1,7 @@ -import { TransactionMeta } from '@metamask/transaction-controller'; +import { + TransactionMeta, + TransactionType, +} from '@metamask/transaction-controller'; import mockState from '../../../../../test/data/mock-state.json'; import { genUnapprovedContractInteractionConfirmation } from '../../../../../test/data/confirmations/contract-interaction'; @@ -66,6 +69,28 @@ describe('useSelectedAccountAlerts', () => { expect(result.current).toEqual(expectedAlert); }); + [ + TransactionType.moneyAccountDeposit, + TransactionType.moneyAccountWithdraw, + ].forEach((nestedType) => { + it(`does not return an alert for a ${nestedType} transaction from a different account`, () => { + const contractInteraction = { + ...genUnapprovedContractInteractionConfirmation({ + address: '0x0', + }), + type: TransactionType.batch, + nestedTransactions: [{ type: nestedType }], + }; + const { result } = renderHookWithConfirmContextProvider( + () => useSelectedAccountAlerts(), + getMockConfirmStateForTransaction( + contractInteraction as TransactionMeta, + ), + ); + expect(result.current).toEqual([]); + }); + }); + it('does not returns an alert for transaction if signing account is same as selected account', () => { const contractInteraction = genUnapprovedContractInteractionConfirmation({ address: '0x0dcd5d886577d5081b0c52e242ef29e70be3e7bc', diff --git a/ui/pages/confirmations/hooks/alerts/useSelectedAccountAlerts.ts b/ui/pages/confirmations/hooks/alerts/useSelectedAccountAlerts.ts index 1f24a9a2658b..b9da17b73536 100644 --- a/ui/pages/confirmations/hooks/alerts/useSelectedAccountAlerts.ts +++ b/ui/pages/confirmations/hooks/alerts/useSelectedAccountAlerts.ts @@ -10,6 +10,7 @@ import { Alert } from '../../../../ducks/confirm-alerts/confirm-alerts'; import { RowAlertKey } from '../../../../components/app/confirm/info/row/constants'; import { Severity } from '../../../../helpers/constants/design-system'; import { useI18nContext } from '../../../../hooks/useI18nContext'; +import { getMoneyAccountTransactionType } from '../../../../../shared/lib/transactions.utils'; import { SignatureRequestType } from '../../types/confirm'; import { useConfirmContext } from '../../context/confirm'; @@ -38,8 +39,15 @@ export const useSelectedAccountAlerts = (): Alert[] => { const confirmationAccountSameAsSelectedAccount = !fromAccount || isAccountFromSelectedAccountGroup; + // Money account deposits/withdrawals are wallet-initiated and always sent + // from the dedicated money account, never the selected account group, so + // the "different account" warning is noise there. + const isMoneyAccountTransaction = Boolean( + getMoneyAccountTransactionType(currentConfirmation as TransactionMeta), + ); + return useMemo((): Alert[] => { - if (confirmationAccountSameAsSelectedAccount) { + if (confirmationAccountSameAsSelectedAccount || isMoneyAccountTransaction) { return []; } @@ -52,5 +60,5 @@ export const useSelectedAccountAlerts = (): Alert[] => { message: t('alertSelectedAccountWarning'), }, ]; - }, [confirmationAccountSameAsSelectedAccount, t]); + }, [confirmationAccountSameAsSelectedAccount, isMoneyAccountTransaction, t]); };