diff --git a/app/components/Views/confirmations/components/info/musd-max-conversion-info/musd-max-conversion-info.test.tsx b/app/components/Views/confirmations/components/info/musd-max-conversion-info/musd-max-conversion-info.test.tsx index 3270b9f788f9..ac2fcc98f243 100644 --- a/app/components/Views/confirmations/components/info/musd-max-conversion-info/musd-max-conversion-info.test.tsx +++ b/app/components/Views/confirmations/components/info/musd-max-conversion-info/musd-max-conversion-info.test.tsx @@ -12,7 +12,6 @@ import { useIsTransactionPayLoading } from '../../../hooks/pay/useTransactionPay import { useConfirmActions } from '../../../hooks/useConfirmActions'; import { useAlerts } from '../../../context/alert-system-context'; import useFiatFormatter from '../../../../../UI/SimulationDetails/FiatDisplay/useFiatFormatter'; -import { useMMPayHardwareAccountAlert } from '../../../hooks/alerts/useMMPayHardwareAccountAlert'; import { Severity } from '../../../types/alerts'; const mockToken: AssetType = { @@ -90,10 +89,6 @@ jest.mock('../../rows/percentage-row', () => ({ PercentageRow: () => null, })); -jest.mock('../../../hooks/alerts/useMMPayHardwareAccountAlert', () => ({ - useMMPayHardwareAccountAlert: jest.fn(), -})); - const mockUseTransactionMetadataRequest = jest.mocked( useTransactionMetadataRequest, ); @@ -101,9 +96,6 @@ const mockUseIsTransactionPayLoading = jest.mocked(useIsTransactionPayLoading); const mockUseConfirmActions = jest.mocked(useConfirmActions); const mockUseAlerts = jest.mocked(useAlerts); const mockUseFiatFormatter = jest.mocked(useFiatFormatter); -const mockUseMMPayHardwareAccountAlert = jest.mocked( - useMMPayHardwareAccountAlert, -); function setupMocksForSuccessPath() { mockUseParams.mockReturnValue({ token: mockToken }); @@ -121,7 +113,6 @@ function setupMocksForSuccessPath() { mockUseFiatFormatter.mockReturnValue((value: { toString: () => string }) => value.toString(), ); - mockUseMMPayHardwareAccountAlert.mockReturnValue([]); } describe('MusdMaxConversionInfo', () => { @@ -275,15 +266,17 @@ describe('MusdMaxConversionInfo', () => { }); it('disables confirm button when hardware account alert is blocking', () => { - mockUseMMPayHardwareAccountAlert.mockReturnValue([ - { - key: 'MMPayHardwareAccount', - message: 'Hardware wallet not supported', - title: 'Not Supported', - severity: Severity.Danger, - isBlocking: true, - }, - ]); + mockUseAlerts.mockReturnValue({ + alerts: [ + { + key: 'MMPayHardwareAccount', + message: 'Hardware wallet not supported', + title: 'Not Supported', + severity: Severity.Danger, + isBlocking: true, + }, + ], + } as unknown as ReturnType); renderWithProvider(, { state: {} }); @@ -294,15 +287,17 @@ describe('MusdMaxConversionInfo', () => { }); it('shows hardware account alert title as button label', () => { - mockUseMMPayHardwareAccountAlert.mockReturnValue([ - { - key: 'MMPayHardwareAccount', - message: 'Hardware wallet not supported', - title: 'Not Supported', - severity: Severity.Danger, - isBlocking: true, - }, - ]); + mockUseAlerts.mockReturnValue({ + alerts: [ + { + key: 'MMPayHardwareAccount', + message: 'Hardware wallet not supported', + title: 'Not Supported', + severity: Severity.Danger, + isBlocking: true, + }, + ], + } as unknown as ReturnType); renderWithProvider(, { state: {} }); diff --git a/app/components/Views/confirmations/components/info/musd-max-conversion-info/musd-max-conversion-info.tsx b/app/components/Views/confirmations/components/info/musd-max-conversion-info/musd-max-conversion-info.tsx index 4a7b513936bc..636e48d22c03 100644 --- a/app/components/Views/confirmations/components/info/musd-max-conversion-info/musd-max-conversion-info.tsx +++ b/app/components/Views/confirmations/components/info/musd-max-conversion-info/musd-max-conversion-info.tsx @@ -25,7 +25,6 @@ import { MUSD_TOKEN, MUSD_TOKEN_ADDRESS, } from '../../../../../UI/Earn/constants/musd'; -import { useMMPayHardwareAccountAlert } from '../../../hooks/alerts/useMMPayHardwareAccountAlert'; /** * Navigation params for MusdMaxConversionInfo @@ -52,9 +51,8 @@ export const MusdMaxConversionInfo = () => { const { onConfirm } = useConfirmActions(); const { alerts } = useAlerts(); - const hardwareAccountAlerts = useMMPayHardwareAccountAlert(); - const blockingAlert = [...alerts, ...hardwareAccountAlerts].find( + const blockingAlert = alerts.find( (confirmationAlert) => confirmationAlert.isBlocking, ); diff --git a/app/components/Views/confirmations/hooks/alerts/useConfirmationAlerts.test.ts b/app/components/Views/confirmations/hooks/alerts/useConfirmationAlerts.test.ts index 3ce408ca41b2..3787fc637d24 100644 --- a/app/components/Views/confirmations/hooks/alerts/useConfirmationAlerts.test.ts +++ b/app/components/Views/confirmations/hooks/alerts/useConfirmationAlerts.test.ts @@ -30,6 +30,7 @@ import { useHeadlessBuyErrorAlert } from './useHeadlessBuyErrorAlert'; import { useTokenContractAlert } from './useTokenContractAlert'; import { useAddressPoisoningAlert } from './useAddressPoisoningAlert'; import { useAccountNoFundsAlert } from './useAccountNoFundsAlert'; +import { useMMPayHardwareAccountAlert } from './useMMPayHardwareAccountAlert'; jest.mock('./useBlockaidAlerts'); jest.mock('./useGasEstimateFailedAlert'); @@ -55,6 +56,7 @@ jest.mock('./useHeadlessBuyErrorAlert'); jest.mock('./useTokenContractAlert'); jest.mock('./useAddressPoisoningAlert'); jest.mock('./useAccountNoFundsAlert'); +jest.mock('./useMMPayHardwareAccountAlert'); describe('useConfirmationAlerts', () => { const ALERT_MESSAGE_MOCK = 'This is a test alert message.'; @@ -227,6 +229,7 @@ describe('useConfirmationAlerts', () => { (useTokenContractAlert as jest.Mock).mockReturnValue([]); (useAddressPoisoningAlert as jest.Mock).mockReturnValue([]); (useAccountNoFundsAlert as jest.Mock).mockReturnValue([]); + (useMMPayHardwareAccountAlert as jest.Mock).mockReturnValue([]); }); it('returns empty array if no alerts', () => { diff --git a/app/components/Views/confirmations/hooks/alerts/useConfirmationAlerts.ts b/app/components/Views/confirmations/hooks/alerts/useConfirmationAlerts.ts index 205273c8dd6f..2b357a547b61 100644 --- a/app/components/Views/confirmations/hooks/alerts/useConfirmationAlerts.ts +++ b/app/components/Views/confirmations/hooks/alerts/useConfirmationAlerts.ts @@ -24,6 +24,7 @@ import { useFirstTimeInteractionAlert } from './useFirstTimeInteractionAlert'; import { useAddressPoisoningAlert } from './useAddressPoisoningAlert'; import { useTokenContractAlert } from './useTokenContractAlert'; import { useAccountNoFundsAlert } from './useAccountNoFundsAlert'; +import { useMMPayHardwareAccountAlert } from './useMMPayHardwareAccountAlert'; function useSignatureAlerts(): Alert[] { const domainMismatchAlerts = useDomainMismatchAlerts(); @@ -53,6 +54,7 @@ function useTransactionAlerts(): Alert[] { const addressPoisoningAlert = useAddressPoisoningAlert(); const tokenContractAlert = useTokenContractAlert(); const accountNoFundsAlert = useAccountNoFundsAlert(); + const mmPayHardwareAccountAlert = useMMPayHardwareAccountAlert(); return useMemo( () => [ @@ -75,6 +77,7 @@ function useTransactionAlerts(): Alert[] { ...addressPoisoningAlert, ...tokenContractAlert, ...accountNoFundsAlert, + ...mmPayHardwareAccountAlert, ], [ gasEstimateFailedAlert, @@ -96,6 +99,7 @@ function useTransactionAlerts(): Alert[] { addressPoisoningAlert, tokenContractAlert, accountNoFundsAlert, + mmPayHardwareAccountAlert, ], ); } diff --git a/app/components/Views/confirmations/hooks/alerts/useMMPayHardwareAccountAlert.test.ts b/app/components/Views/confirmations/hooks/alerts/useMMPayHardwareAccountAlert.test.ts index 1b3d0848060a..fa4789d7f1db 100644 --- a/app/components/Views/confirmations/hooks/alerts/useMMPayHardwareAccountAlert.test.ts +++ b/app/components/Views/confirmations/hooks/alerts/useMMPayHardwareAccountAlert.test.ts @@ -4,6 +4,8 @@ import { AlertKeys } from '../../constants/alerts'; import { Severity } from '../../types/alerts'; import { strings } from '../../../../../../locales/i18n'; import { useTransactionMetadataRequest } from '../transactions/useTransactionMetadataRequest'; +import { useTransactionAccountOverride } from '../transactions/useTransactionAccountOverride'; +import { useTransactionPayFiatPayment } from '../pay/useTransactionPayData'; import { TransactionMeta, TransactionType, @@ -18,10 +20,15 @@ jest.mock('react-redux', () => ({ useSelector: jest.fn(), })); jest.mock('../transactions/useTransactionMetadataRequest'); +jest.mock('../transactions/useTransactionAccountOverride'); +jest.mock('../pay/useTransactionPayData'); jest.mock('../../../../../util/address'); const useSelectorMock = jest.mocked(useSelector); +const HARDWARE_ADDRESS = '0xabc'; +const OVERRIDE_ADDRESS = '0xdef'; + function runHook() { return renderHook(() => useMMPayHardwareAccountAlert()); } @@ -41,15 +48,24 @@ describe('useMMPayHardwareAccountAlert', () => { const useTransactionMetadataRequestMock = jest.mocked( useTransactionMetadataRequest, ); + const useTransactionAccountOverrideMock = jest.mocked( + useTransactionAccountOverride, + ); + const useTransactionPayFiatPaymentMock = jest.mocked( + useTransactionPayFiatPayment, + ); beforeEach(() => { jest.resetAllMocks(); useSelectorMock.mockReturnValue({ enabled: false }); + useTransactionAccountOverrideMock.mockReturnValue(undefined); + useTransactionPayFiatPaymentMock.mockReturnValue(undefined); useTransactionMetadataRequestMock.mockReturnValue({ + type: TransactionType.perpsDeposit, txParams: { - from: '0xabc', + from: HARDWARE_ADDRESS, }, } as TransactionMeta); @@ -72,6 +88,66 @@ describe('useMMPayHardwareAccountAlert', () => { expect(result.current).toStrictEqual([]); }); + it('returns no alert if transaction is not a pay transaction', () => { + isHardwareAccountMock.mockReturnValue(true); + useTransactionMetadataRequestMock.mockReturnValue({ + type: TransactionType.simpleSend, + txParams: { + from: HARDWARE_ADDRESS, + }, + } as TransactionMeta); + + const { result } = runHook(); + + expect(result.current).toStrictEqual([]); + }); + + it('checks account override instead of from when set', () => { + isHardwareAccountMock.mockImplementation( + (address) => address === OVERRIDE_ADDRESS, + ); + useTransactionAccountOverrideMock.mockReturnValue(OVERRIDE_ADDRESS); + useTransactionMetadataRequestMock.mockReturnValue({ + type: TransactionType.moneyAccountDeposit, + txParams: { + from: '0x123', + }, + } as TransactionMeta); + + const { result } = runHook(); + + expect(result.current).toStrictEqual([EXPECTED_ALERT]); + expect(isHardwareAccountMock).toHaveBeenCalledWith(OVERRIDE_ADDRESS); + }); + + it('returns no alert on withdraw when account override is hardware account', () => { + isHardwareAccountMock.mockImplementation( + (address) => address === OVERRIDE_ADDRESS, + ); + useTransactionAccountOverrideMock.mockReturnValue(OVERRIDE_ADDRESS); + useTransactionMetadataRequestMock.mockReturnValue({ + type: TransactionType.moneyAccountWithdraw, + txParams: { + from: '0x123', + }, + } as TransactionMeta); + + const { result } = runHook(); + + expect(result.current).toStrictEqual([]); + }); + + it('returns no alert when fiat payment method is selected', () => { + isHardwareAccountMock.mockReturnValue(true); + useTransactionPayFiatPaymentMock.mockReturnValue({ + selectedPaymentMethodId: 'payment-method-1', + } as ReturnType); + + const { result } = runHook(); + + expect(result.current).toStrictEqual([]); + }); + it('returns alert for Ledger wallet on mUSD conversion when feature flag is disabled', () => { isHardwareAccountMock.mockReturnValue(true); isQRHardwareAccountMock.mockReturnValue(false); @@ -79,7 +155,7 @@ describe('useMMPayHardwareAccountAlert', () => { useTransactionMetadataRequestMock.mockReturnValue({ type: TransactionType.musdConversion, txParams: { - from: '0xabc', + from: HARDWARE_ADDRESS, }, } as TransactionMeta); @@ -95,7 +171,7 @@ describe('useMMPayHardwareAccountAlert', () => { useTransactionMetadataRequestMock.mockReturnValue({ type: TransactionType.musdConversion, txParams: { - from: '0xabc', + from: HARDWARE_ADDRESS, }, } as TransactionMeta); @@ -111,7 +187,7 @@ describe('useMMPayHardwareAccountAlert', () => { useTransactionMetadataRequestMock.mockReturnValue({ type: TransactionType.musdConversion, txParams: { - from: '0xabc', + from: HARDWARE_ADDRESS, }, } as TransactionMeta); diff --git a/app/components/Views/confirmations/hooks/alerts/useMMPayHardwareAccountAlert.ts b/app/components/Views/confirmations/hooks/alerts/useMMPayHardwareAccountAlert.ts index a10d69f0097a..d0f235f1bbba 100644 --- a/app/components/Views/confirmations/hooks/alerts/useMMPayHardwareAccountAlert.ts +++ b/app/components/Views/confirmations/hooks/alerts/useMMPayHardwareAccountAlert.ts @@ -5,15 +5,23 @@ import { AlertKeys } from '../../constants/alerts'; import { Alert, Severity } from '../../types/alerts'; import { strings } from '../../../../../../locales/i18n'; import { useTransactionMetadataRequest } from '../transactions/useTransactionMetadataRequest'; -import { hasTransactionType } from '../../utils/transaction'; +import { useTransactionAccountOverride } from '../transactions/useTransactionAccountOverride'; +import { useTransactionPayFiatPayment } from '../pay/useTransactionPayData'; +import { + hasTransactionType, + isTransactionPayWithdraw, +} from '../../utils/transaction'; import { isHardwareAccount, isQRHardwareAccount, } from '../../../../../util/address'; import { selectMetaMaskPayHardwareFlags } from '../../../../../selectors/featureFlagController/confirmations'; +import { PAY_TRANSACTION_TYPES } from '../../constants/confirmations'; export function useMMPayHardwareAccountAlert(): Alert[] { const transactionMeta = useTransactionMetadataRequest(); + const accountOverride = useTransactionAccountOverride(); + const fiatPayment = useTransactionPayFiatPayment(); const { enabled: isHardwarePayEnabled } = useSelector( selectMetaMaskPayHardwareFlags, ); @@ -22,15 +30,31 @@ export function useMMPayHardwareAccountAlert(): Alert[] { txParams: { from }, } = transactionMeta ?? { txParams: {} }; + const isPayTransaction = hasTransactionType( + transactionMeta, + PAY_TRANSACTION_TYPES, + ); + const isMusdConversion = hasTransactionType(transactionMeta, [ TransactionType.musdConversion, ]); - const isHardwareWallet = isHardwareAccount(from ?? ''); - const isQRWallet = isQRHardwareAccount(from ?? ''); + // When set, accountOverride is the account paying for the transaction, + // except in withdraw (post-quote) flows where it is only the recipient + // and never signs. + const payingAccount = isTransactionPayWithdraw(transactionMeta) + ? from + : (accountOverride ?? from); + + const isHardwareWallet = isHardwareAccount(payingAccount ?? ''); + const isQRWallet = isQRHardwareAccount(payingAccount ?? ''); + + // Fiat payments are bought directly to the destination, so the paying + // account never signs. + const isFiatPayment = Boolean(fiatPayment?.selectedPaymentMethodId); return useMemo(() => { - if (!isHardwareWallet) { + if (!isPayTransaction || !isHardwareWallet || isFiatPayment) { return []; } @@ -47,5 +71,12 @@ export function useMMPayHardwareAccountAlert(): Alert[] { isBlocking: true, }, ]; - }, [isHardwareWallet, isHardwarePayEnabled, isMusdConversion, isQRWallet]); + }, [ + isFiatPayment, + isHardwareWallet, + isHardwarePayEnabled, + isMusdConversion, + isPayTransaction, + isQRWallet, + ]); } diff --git a/app/components/Views/confirmations/hooks/alerts/usePendingAmountAlerts.test.ts b/app/components/Views/confirmations/hooks/alerts/usePendingAmountAlerts.test.ts index 7c7c12ed99f3..5bab71e49e79 100644 --- a/app/components/Views/confirmations/hooks/alerts/usePendingAmountAlerts.test.ts +++ b/app/components/Views/confirmations/hooks/alerts/usePendingAmountAlerts.test.ts @@ -6,10 +6,6 @@ jest.mock('./useInsufficientPayTokenBalanceAlert', () => ({ useInsufficientPayTokenBalanceAlert: jest.fn(() => [{ id: 'alert-3' }]), })); -jest.mock('./useMMPayHardwareAccountAlert', () => ({ - useMMPayHardwareAccountAlert: () => [{ id: 'alert-1' }], -})); - jest.mock('./useInsufficientPredictBalanceAlert', () => ({ useInsufficientPredictBalanceAlert: () => [{ id: 'alert-4' }], })); @@ -56,7 +52,6 @@ describe('usePendingAmountAlerts', () => { ); expect(result.current).toStrictEqual([ - { id: 'alert-1' }, { id: 'alert-3' }, { id: 'alert-4' }, { id: 'alert-5' }, diff --git a/app/components/Views/confirmations/hooks/alerts/usePendingAmountAlerts.ts b/app/components/Views/confirmations/hooks/alerts/usePendingAmountAlerts.ts index 1c9049c0ca65..d48b71cc15c7 100644 --- a/app/components/Views/confirmations/hooks/alerts/usePendingAmountAlerts.ts +++ b/app/components/Views/confirmations/hooks/alerts/usePendingAmountAlerts.ts @@ -1,7 +1,6 @@ import { useMemo } from 'react'; import { Alert } from '../../types/alerts'; import { useInsufficientPayTokenBalanceAlert } from './useInsufficientPayTokenBalanceAlert'; -import { useMMPayHardwareAccountAlert } from './useMMPayHardwareAccountAlert'; import { useInsufficientPredictBalanceAlert } from './useInsufficientPredictBalanceAlert'; import { useInsufficientPerpsBalanceAlert } from './useInsufficientPerpsBalanceAlert'; import { useInsufficientMoneyAccountBalanceAlert } from './useInsufficientMoneyAccountBalanceAlert'; @@ -20,8 +19,6 @@ export function usePendingAmountAlerts({ pendingAmountUsd: pendingFiatAmount ?? '0', }); - const mmPayHardwareAccountAlert = useMMPayHardwareAccountAlert(); - const insufficientPredictBalanceAlert = useInsufficientPredictBalanceAlert({ pendingAmount: pendingTokenAmount ?? '0', }); @@ -47,7 +44,6 @@ export function usePendingAmountAlerts({ return useMemo( () => [ - ...mmPayHardwareAccountAlert, ...insufficientTokenFundsAlert, ...insufficientPredictBalanceAlert, ...insufficientPerpsBalanceAlert, @@ -58,7 +54,6 @@ export function usePendingAmountAlerts({ ], [ insufficientTokenFundsAlert, - mmPayHardwareAccountAlert, insufficientPredictBalanceAlert, insufficientPerpsBalanceAlert, insufficientMoneyAccountBalanceAlert, diff --git a/app/components/Views/confirmations/hooks/transactions/useTransactionCustomAmountAlerts.ts b/app/components/Views/confirmations/hooks/transactions/useTransactionCustomAmountAlerts.ts index cd5b218b0244..ba623c22ec0e 100644 --- a/app/components/Views/confirmations/hooks/transactions/useTransactionCustomAmountAlerts.ts +++ b/app/components/Views/confirmations/hooks/transactions/useTransactionCustomAmountAlerts.ts @@ -73,10 +73,16 @@ export function useTransactionCustomAmountAlerts({ }); }, [confirmationAlerts, isInputChanged, isKeyboardVisible]); - const alerts = useMemo( - () => [...pendingTokenAlerts, ...filteredAlerts], - [filteredAlerts, pendingTokenAlerts], - ); + const alerts = useMemo(() => { + const merged = [...pendingTokenAlerts, ...filteredAlerts]; + + // The hardware wallet alert can only be fixed by switching accounts, so + // its message takes priority over amount-level alerts. + return [ + ...merged.filter((a) => a.key === AlertKeys.MMPayHardwareAccount), + ...merged.filter((a) => a.key !== AlertKeys.MMPayHardwareAccount), + ]; + }, [filteredAlerts, pendingTokenAlerts]); const firstAlert = alerts?.[0];