diff --git a/app/components/Views/confirmations/hooks/transactions/useTransactionConfirm.test.ts b/app/components/Views/confirmations/hooks/transactions/useTransactionConfirm.test.ts index 0c6e3e60f193..f858c2c07ff6 100644 --- a/app/components/Views/confirmations/hooks/transactions/useTransactionConfirm.test.ts +++ b/app/components/Views/confirmations/hooks/transactions/useTransactionConfirm.test.ts @@ -31,6 +31,7 @@ import { useParams } from '../../../../../util/navigation/navUtils'; import { PayWithOption } from '../../components/confirm/confirm-component'; import { useFiatConfirm } from '../pay/useFiatConfirm'; import { useHandleHwSend } from '../../../../UI/HardwareWallet/Swaps/useHandleHwSend'; +import { useTransactionPayingAccount } from './useTransactionPayingAccount'; const mockNavigate = jest.fn(); const mockGoBack = jest.fn(); @@ -55,6 +56,7 @@ jest.mock('../../../../../util/navigation/navUtils', () => ({ jest.mock('../../../../UI/HardwareWallet/Swaps/useHandleHwSend', () => ({ useHandleHwSend: jest.fn(), })); +jest.mock('./useTransactionPayingAccount'); jest.mock('@react-navigation/native', () => ({ ...jest.requireActual('@react-navigation/native'), @@ -65,6 +67,8 @@ jest.mock('@react-navigation/native', () => ({ })); const CHAIN_ID_MOCK = '0x123'; +const SOFTWARE_SIGNER_ADDRESS = '0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266'; +const HARDWARE_PAYER_ADDRESS = '0x70997970C51812dc3A010C7d01b50e0d17dc79C8'; // ---------- Top-level mocks (referenced by beforeEach and tests) ---------- @@ -84,6 +88,9 @@ const useTransactionMetadataRequestMock = jest.mocked( ); const isHardwareAccountMock = jest.mocked(isHardwareAccount); const useHandleHwSendMock = jest.mocked(useHandleHwSend); +const useTransactionPayingAccountMock = jest.mocked( + useTransactionPayingAccount, +); const onFiatConfirmMock = jest.fn(); const useParamsMock = jest.mocked(useParams); @@ -194,6 +201,7 @@ describe('useTransactionConfirm', () => { }); isHardwareAccountMock.mockReturnValue(false); + useTransactionPayingAccountMock.mockReturnValue(undefined); useHandleHwSendMock.mockReturnValue({ shouldDefer: jest.fn(() => false), @@ -326,6 +334,32 @@ describe('useTransactionConfirm', () => { ); }); + it('waits for result when a hardware payer differs from the software signer', async () => { + useTransactionPayingAccountMock.mockReturnValue(HARDWARE_PAYER_ADDRESS); + isHardwareAccountMock.mockImplementation( + (address) => address === HARDWARE_PAYER_ADDRESS, + ); + useTransactionPayQuotesMock.mockReturnValue([ + {} as TransactionPayQuote, + ]); + useTransactionMetadataRequestMock.mockReturnValue({ + id: transactionIdMock, + chainId: CHAIN_ID_MOCK, + txParams: { from: SOFTWARE_SIGNER_ADDRESS }, + } as unknown as TransactionMeta); + + const { result } = renderHook(); + + await act(async () => { + await result.current.onConfirm(); + }); + + expect(onApprovalConfirm).toHaveBeenCalledWith( + expect.objectContaining({ waitForResult: true }), + expect.anything(), + ); + }); + it('calls tryEnableEvmNetwork', async () => { const tryEnableEvmNetworkMock = jest.fn(); @@ -635,7 +669,7 @@ describe('useTransactionConfirm', () => { }); describe('isGasFeeSponsored override', () => { - it('clears isGasFeeSponsored when gasless is not supported', async () => { + it('passes gas sponsorship metadata through when gasless is not supported', async () => { useIsGaslessSupportedMock.mockReturnValue({ isSmartTransaction: false, isSupported: false, @@ -658,7 +692,7 @@ describe('useTransactionConfirm', () => { expect(onApprovalConfirm).toHaveBeenCalledWith(expect.anything(), { txMeta: expect.objectContaining({ - isGasFeeSponsored: false, + isGasFeeSponsored: true, }), }); }); @@ -691,7 +725,7 @@ describe('useTransactionConfirm', () => { }); }); - it('clears isGasFeeSponsored for revoke delegation when gasless is supported', async () => { + it('passes gas sponsorship metadata through for revoke delegation when gasless is supported', async () => { useIsGaslessSupportedMock.mockReturnValue({ isSmartTransaction: true, isSupported: true, @@ -715,12 +749,12 @@ describe('useTransactionConfirm', () => { expect(onApprovalConfirm).toHaveBeenCalledWith(expect.anything(), { txMeta: expect.objectContaining({ - isGasFeeSponsored: false, + isGasFeeSponsored: true, }), }); }); - it('clears isGasFeeSponsored even without selectedGasFeeToken', async () => { + it('passes gas sponsorship metadata through even without selectedGasFeeToken', async () => { useIsGaslessSupportedMock.mockReturnValue({ isSmartTransaction: false, isSupported: false, @@ -747,7 +781,7 @@ describe('useTransactionConfirm', () => { expect(onApprovalConfirm).toHaveBeenCalledWith(expect.anything(), { txMeta: expect.objectContaining({ - isGasFeeSponsored: false, + isGasFeeSponsored: true, }), }); }); @@ -790,6 +824,32 @@ describe('useTransactionConfirm', () => { }); }); + it('appends the fee-token transfer to existing smart transaction batches', async () => { + useTransactionMetadataRequestMock.mockReturnValue({ + id: transactionIdMock, + chainId: CHAIN_ID_MOCK, + txParams: {}, + batchTransactions: [ + { data: '0xexisting', to: '0xexisting', value: '0x0' }, + ], + } as unknown as TransactionMeta); + + const { result } = renderHook(); + + await act(async () => { + await result.current.onConfirm(); + }); + + expect(onApprovalConfirm).toHaveBeenCalledWith(expect.anything(), { + txMeta: expect.objectContaining({ + batchTransactions: [ + expect.objectContaining({ data: '0xexisting' }), + expect.objectContaining({ data: '0xabc' }), + ], + }), + }); + }); + it('does nothing if selectedGasFeeToken missing', async () => { useSelectedGasFeeTokenMock.mockReturnValue( undefined as unknown as ReturnType, @@ -856,8 +916,8 @@ describe('useTransactionConfirm', () => { }); }); - describe('handleGasless7702', () => { - it('sets isExternalSign when selectedGasFeeToken is present and not smart transaction', async () => { + describe('selected gas fee token handling', () => { + it('keeps external-sign metadata out of the confirmation request when selectedGasFeeToken is present and not smart transaction', async () => { isSendBundleSupportedMock.mockReturnValue(Promise.resolve(false)); useSelectedGasFeeTokenMock.mockReturnValue({ @@ -871,13 +931,11 @@ describe('useTransactionConfirm', () => { }); expect(onApprovalConfirm).toHaveBeenCalledWith(expect.anything(), { - txMeta: expect.objectContaining({ - isExternalSign: true, - }), + txMeta: expect.not.objectContaining({ isExternalSign: true }), }); }); - it('sets isExternalSign when selectedGasFeeToken is present and smart transaction but the chain does not support send bundle', async () => { + it('keeps external-sign metadata out of the confirmation request when selectedGasFeeToken is present and smart transaction but the chain does not support send bundle', async () => { isSendBundleSupportedMock.mockReturnValue(Promise.resolve(false)); useSelectedGasFeeTokenMock.mockReturnValue({ @@ -891,9 +949,32 @@ describe('useTransactionConfirm', () => { }); expect(onApprovalConfirm).toHaveBeenCalledWith(expect.anything(), { - txMeta: expect.objectContaining({ - isExternalSign: true, - }), + txMeta: expect.not.objectContaining({ isExternalSign: true }), + }); + }); + + it('uses the software signer capability when the fee payer is a hardware account', async () => { + useTransactionPayingAccountMock.mockReturnValue(HARDWARE_PAYER_ADDRESS); + isHardwareAccountMock.mockImplementation( + (address) => address === HARDWARE_PAYER_ADDRESS, + ); + useSelectedGasFeeTokenMock.mockReturnValue({ + transferTransaction: { data: '0xabc' }, + } as unknown as ReturnType); + useTransactionMetadataRequestMock.mockReturnValue({ + id: transactionIdMock, + chainId: CHAIN_ID_MOCK, + txParams: { from: SOFTWARE_SIGNER_ADDRESS }, + } as unknown as TransactionMeta); + + const { result } = renderHook(); + + await act(async () => { + await result.current.onConfirm(); + }); + + expect(onApprovalConfirm).toHaveBeenCalledWith(expect.anything(), { + txMeta: expect.not.objectContaining({ isExternalSign: true }), }); }); @@ -913,16 +994,21 @@ describe('useTransactionConfirm', () => { }); }); - it('does nothing if isGasFeeTokenIgnoredIfBalance', async () => { + it('keeps the confirmation request free of client-side external-sign hints when the fee token is ignored for native balance', async () => { isSendBundleSupportedMock.mockReturnValue(Promise.resolve(false)); - + useIsGaslessSupportedMock.mockReturnValue({ + isSupported: false, + isSmartTransaction: false, + pending: false, + }); useSelectedGasFeeTokenMock.mockReturnValue({ transferTransaction: { data: '0xabc' }, } as unknown as ReturnType); - useTransactionMetadataRequestMock.mockReturnValue({ id: transactionIdMock, + isGasFeeSponsored: true, isGasFeeTokenIgnoredIfBalance: true, + txParams: { from: SOFTWARE_SIGNER_ADDRESS }, } as unknown as TransactionMeta); const { result } = renderHook(); @@ -937,16 +1023,8 @@ describe('useTransactionConfirm', () => { }); }); - describe('isExternalSign revert for unsupported accounts', () => { - // Regression: on gas-sponsorship chains (e.g. Monad, SEI) the - // TransactionController sets `isExternalSign = true` from - // `isGasFeeSponsored` during gas simulation regardless of account type. - // For hardware wallets no relay is eligible (HW cannot hold an EIP-7702 - // delegation), so leaving the flag set skips device signing and an empty - // `0x` payload reaches eth_sendRawTransaction. The fix reverts the flag - // whenever gasless sponsorship cannot apply for the account/chain. - - it('reverts isExternalSign when gasless is unsupported (hardware wallet on sponsored chain)', async () => { + describe('isExternalSign handling', () => { + it('keeps the confirmation request free of external-sign hints when gasless is unsupported (hardware wallet on sponsored chain)', async () => { isHardwareAccountMock.mockReturnValue(true); useIsGaslessSupportedMock.mockReturnValue({ isSupported: false, @@ -957,7 +1035,6 @@ describe('useTransactionConfirm', () => { id: transactionIdMock, chainId: CHAIN_ID_MOCK, isGasFeeSponsored: true, - isExternalSign: true, txParams: { from: '0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266' }, } as unknown as TransactionMeta); @@ -968,14 +1045,11 @@ describe('useTransactionConfirm', () => { }); expect(onApprovalConfirm).toHaveBeenCalledWith(expect.anything(), { - txMeta: expect.objectContaining({ - isExternalSign: false, - isGasFeeSponsored: false, - }), + txMeta: expect.not.objectContaining({ isExternalSign: true }), }); }); - it('keeps isExternalSign when gasless is supported (EOA relay path intact)', async () => { + it('keeps the confirmation request free of external-sign hints when gasless is supported (EOA relay path intact)', async () => { isHardwareAccountMock.mockReturnValue(false); useIsGaslessSupportedMock.mockReturnValue({ isSupported: true, @@ -986,7 +1060,6 @@ describe('useTransactionConfirm', () => { id: transactionIdMock, chainId: CHAIN_ID_MOCK, isGasFeeSponsored: true, - isExternalSign: true, txParams: { from: '0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266' }, } as unknown as TransactionMeta); @@ -997,10 +1070,7 @@ describe('useTransactionConfirm', () => { }); expect(onApprovalConfirm).toHaveBeenCalledWith(expect.anything(), { - txMeta: expect.objectContaining({ - isExternalSign: true, - isGasFeeSponsored: true, - }), + txMeta: expect.not.objectContaining({ isExternalSign: true }), }); }); }); @@ -1086,6 +1156,39 @@ describe('useTransactionConfirm', () => { expect(onApprovalConfirm).not.toHaveBeenCalled(); }); + it('forwards sponsored metadata to the hardware signer without client-side external-sign hints', async () => { + const spy = setupHwSend(); + useIsGaslessSupportedMock.mockReturnValue({ + isSupported: false, + isSmartTransaction: false, + pending: false, + }); + useTransactionMetadataRequestMock.mockReturnValue({ + id: transactionIdMock, + chainId: CHAIN_ID_MOCK, + type: TransactionType.simpleSend, + isGasFeeSponsored: true, + txParams: { + from: HARDWARE_PAYER_ADDRESS, + to: SOFTWARE_SIGNER_ADDRESS, + value: '0x64', + }, + } as unknown as TransactionMeta); + + const { result } = renderHook(); + + await act(async () => { + await result.current.onConfirm(); + }); + + expect(spy).toHaveBeenCalledWith( + expect.objectContaining({ + isGasFeeSponsored: true, + }), + ); + expect(onApprovalConfirm).not.toHaveBeenCalled(); + }); + it('ERC-20 send (tokenMethodTransfer): routes through handleHwSend', async () => { // The decoded amount + symbol now live inside useHandleHwSend; here we // assert the branch fires for ERC-20 transfers and the prepared diff --git a/app/components/Views/confirmations/hooks/transactions/useTransactionConfirm.ts b/app/components/Views/confirmations/hooks/transactions/useTransactionConfirm.ts index a17a1fe5925c..cade94c00a56 100644 --- a/app/components/Views/confirmations/hooks/transactions/useTransactionConfirm.ts +++ b/app/components/Views/confirmations/hooks/transactions/useTransactionConfirm.ts @@ -20,8 +20,6 @@ import { } from '../../components/confirm/confirm-component'; import { createProjectLogger } from '@metamask/utils'; import { useSelectedGasFeeToken } from '../gas/useGasFeeToken'; -import { shouldApplyGasFeeSponsorship } from '../../utils/transaction'; -import { useIsGaslessSupported } from '../gas/useIsGaslessSupported'; import { useGaslessSupportedSmartTransactions } from '../gas/useGaslessSupportedSmartTransactions'; import { cloneDeep } from 'lodash'; import { useTransactionPayQuotes } from '../pay/useTransactionPayData'; @@ -61,8 +59,6 @@ export function useTransactionConfirm() { const { isSupported: isGaslessSupportedSTX, isSmartTransaction } = useGaslessSupportedSmartTransactions(); - const { isSupported: isGaslessSupported } = useIsGaslessSupported(); - // Signer of the confirmed transaction; gates signing-related paths. const isSignerHardwareWallet = isHardwareAccount( transactionMetadata?.txParams?.from ?? '', @@ -96,17 +92,6 @@ export function useTransactionConfirm() { [selectedGasFeeToken, isGasFeeTokenIgnoredIfBalance], ); - const handleGasless7702 = useCallback( - (updatedMetadata: TransactionMeta) => { - if (!selectedGasFeeToken || isGasFeeTokenIgnoredIfBalance) { - return; - } - - updatedMetadata.isExternalSign = true; - }, - [isGasFeeTokenIgnoredIfBalance, selectedGasFeeToken], - ); - const navigateOnConfirm = useCallback(() => { if (!transactionMetadata) { return; @@ -193,27 +178,8 @@ export function useTransactionConfirm() { const updatedMetadata = cloneDeep(transactionMetadata); - // Sponsorship eligibility is account-specific (HW wallets are excluded), - // unlike the controller's account-agnostic simulation result. - const isGaslessEligible = shouldApplyGasFeeSponsorship({ - transactionMeta: transactionMetadata, - isGaslessSupported, - }); - updatedMetadata.isGasFeeSponsored = isGaslessEligible; - - // The controller sets `isExternalSign` from `isGasFeeSponsored` for any - // account. When gasless isn't eligible, revert it or signing is skipped - // and an empty `'0x'` reaches `eth_sendRawTransaction`. - const isExternalSignStale = - Boolean(transactionMetadata.isExternalSign) && !isGaslessEligible; - if (isExternalSignStale) { - updatedMetadata.isExternalSign = false; - } - if (isGaslessSupportedSTX) { handleSmartTransaction(updatedMetadata); - } else if (selectedGasFeeToken && !isSignerHardwareWallet) { - handleGasless7702(updatedMetadata); } if (shouldDeferHwSend(updatedMetadata)) { @@ -243,12 +209,10 @@ export function useTransactionConfirm() { } }, [ - handleGasless7702, - shouldDeferHwSend, + shouldDeferHwSend, deferHwSend, handleSmartTransaction, isFiatPaymentSelected, - isGaslessSupported, isGaslessSupportedSTX, navigateOnConfirm, onFiatConfirm, diff --git a/app/util/transactions/hooks/index.test.ts b/app/util/transactions/hooks/index.test.ts index cc756536d3de..a95ae54fb1c3 100644 --- a/app/util/transactions/hooks/index.test.ts +++ b/app/util/transactions/hooks/index.test.ts @@ -25,6 +25,7 @@ import { } from '../../smart-transactions/smart-publish-hook'; import { accountSupports7702 } from '../account-supports-7702'; import { getTransactionById } from '..'; +import { isRelaySupported } from '../transaction-relay'; import { isSendBundleSupported } from '../sentinel-api'; import { Delegation7702PublishHook } from './delegation-7702-publish'; import { @@ -47,6 +48,7 @@ jest.mock('../../../store', () => ({ jest.mock('../../smart-transactions/smart-publish-hook'); jest.mock('../account-supports-7702'); jest.mock('..'); +jest.mock('../transaction-relay'); jest.mock('../sentinel-api'); jest.mock('./delegation-7702-publish'); @@ -147,6 +149,8 @@ describe('getTransactionControllerHooks', () => { expect(hooks).toStrictEqual( expect.objectContaining({ + isSponsored: expect.any(Function), + shouldSign: expect.any(Function), beforePublish: expect.any(Function), beforeSign: expect.any(Function), publish: expect.any(Function), @@ -155,6 +159,25 @@ describe('getTransactionControllerHooks', () => { ); }); + it('returns sponsorship and signing decisions', async () => { + jest.mocked(selectShouldUseSmartTransaction).mockReturnValue(false); + jest.mocked(isSendBundleSupported).mockResolvedValue(false); + jest.mocked(isRelaySupported).mockResolvedValue(false); + + const request = buildRequest(); + const hooks = getTransactionControllerHooks(request); + + await expect( + hooks.isSponsored?.({ transactionMeta: MOCK_TRANSACTION_META }), + ).resolves.toBe(false); + await expect( + hooks.shouldSign?.({ + transactionMeta: MOCK_TRANSACTION_META, + isSponsored: false, + }), + ).resolves.toBe(true); + }); + it('delegates Predict beforePublish and beforeSign through the init messenger', async () => { const request = buildRequest(); const hooks = getTransactionControllerHooks(request); diff --git a/app/util/transactions/hooks/index.ts b/app/util/transactions/hooks/index.ts index 1726febe97cc..367e4e21bb73 100644 --- a/app/util/transactions/hooks/index.ts +++ b/app/util/transactions/hooks/index.ts @@ -18,6 +18,7 @@ import { import { Hex } from '@metamask/utils'; import type { RootState } from '../../../reducers'; +import { isHardwareAccount } from '../../address'; import { getSmartTransactionsFeatureFlagsForChain, selectShouldUseSmartTransaction, @@ -39,6 +40,7 @@ import { type SubmitSmartTransactionRequest, } from '../../smart-transactions/smart-publish-hook'; import { getTransactionById } from '..'; +import { isRelaySupported } from '../transaction-relay'; import { accountSupports7702 } from '../account-supports-7702'; import { isSendBundleSupported } from '../sentinel-api'; import { Delegation7702PublishHook } from './delegation-7702-publish'; @@ -66,6 +68,8 @@ export function getTransactionControllerHooks( request: TransactionControllerHookRequest, ): TransactionControllerOptions['hooks'] { return { + isSponsored: isSponsoredHook(request), + shouldSign: shouldSignHook(request), beforePublish: beforePublishHook(request), beforeSign: beforeSignHook(request), // @ts-expect-error - TransactionController actually sends a signedTx as a second argument, but its type doesn't reflect that. @@ -87,6 +91,87 @@ async function getNextNonce( return toHex(nonceLock.nextNonce); } +type TransactionApprovalDecision = { + signingMode: 'local' | 'external'; + sponsorshipEnabled: boolean; +}; + +async function getTransactionApprovalDecision( + { getState }: TransactionControllerHookRequest, + transactionMeta: TransactionMeta, +): Promise { + const state = getState(); + const { chainId, txParams } = transactionMeta; + + const shouldUseSmartTransaction = selectShouldUseSmartTransaction( + state, + chainId, + ); + const sendBundleSupport = await isSendBundleSupported(chainId); + const isSmartTransactionAndBundleSupported = Boolean( + shouldUseSmartTransaction && sendBundleSupport, + ); + + const fromAddress = txParams?.from; + const isHardwareWallet = Boolean(fromAddress && isHardwareAccount(fromAddress)); + + const shouldCheck7702Eligibility = + !isHardwareWallet && !isSmartTransactionAndBundleSupported; + + const is7702Supported = Boolean( + !isHardwareWallet && + shouldCheck7702Eligibility && + (await isRelaySupported(chainId)) && + txParams?.to !== undefined, + ); + + const requiresExternalSigning = + Boolean(transactionMeta.selectedGasFeeToken) && + !transactionMeta.isGasFeeTokenIgnoredIfBalance && + !isHardwareWallet && + !isSmartTransactionAndBundleSupported; + + const sponsorshipEnabled = + Boolean(transactionMeta.isGasFeeSponsored) && + (isSmartTransactionAndBundleSupported || is7702Supported); + + const signingMode: 'local' | 'external' = requiresExternalSigning + ? 'external' + : 'local'; + + return { + signingMode, + sponsorshipEnabled, + }; +} + +function isSponsoredHook(request: TransactionControllerHookRequest) { + return async ({ transactionMeta }: { transactionMeta: TransactionMeta }) => { + const { sponsorshipEnabled } = await getTransactionApprovalDecision( + request, + transactionMeta, + ); + + return sponsorshipEnabled; + }; +} + +function shouldSignHook(request: TransactionControllerHookRequest) { + return async ({ + transactionMeta, + }: { + transactionMeta: TransactionMeta; + isSponsored: boolean; + }) => { + const { signingMode } = await getTransactionApprovalDecision( + request, + transactionMeta, + ); + + return signingMode === 'local'; + }; +} + function beforePublishHook({ initMessenger, }: TransactionControllerHookRequest) {