From b0bd20ed7b75112d2bf7e0fef5291affb1ffcf08 Mon Sep 17 00:00:00 2001 From: fairlighteth <31534717+fairlighteth@users.noreply.github.com> Date: Sat, 6 Jun 2026 10:03:24 +0100 Subject: [PATCH 1/5] docs: open approval and permit integrity plan From 82158cebb498bc9d8851b4a2ba31a5c2a40a0e0f Mon Sep 17 00:00:00 2001 From: fairlighteth <31534717+fairlighteth@users.noreply.github.com> Date: Sat, 6 Jun 2026 10:39:24 +0100 Subject: [PATCH 2/5] fix: align approval hooks and permit spender checks --- .../src/common/hooks/useNeedsApproval.ts | 8 +- .../modules/erc20Approve/hooks/useApproval.ts | 2 +- .../hooks/useApproveCurrency.test.ts | 29 +++---- .../erc20Approve/hooks/useApproveCurrency.ts | 18 ++-- .../erc20Approve/hooks/useApproveState.ts | 7 +- .../useGeneratePermitInAdvanceToTrade.test.ts | 42 +++++++++- .../useGeneratePermitInAdvanceToTrade.ts | 19 ++++- .../EthFlow/hooks/useEthFlowActions.ts | 4 +- .../ethFlow/containers/EthFlow/index.tsx | 4 +- .../src/modules/injectedWidget/index.ts | 1 + .../callOnBeforeApprovalWidgetHook.ts | 33 ++++++++ .../services/safeBundleFlow/index.ts | 22 +++-- .../hooks/useSafeBundleFlowContext.ts | 2 +- .../safeBundleFlow/safeBundleApprovalFlow.ts | 17 +++- .../safeBundleFlow/safeBundleEthFlow.ts | 24 +++++- .../twap/hooks/useTwapOrderCreationContext.ts | 4 +- .../src/lib/generatePermitHook.test.ts | 84 +++++++++++++++++++ .../src/lib/generatePermitHook.ts | 40 +++++++-- 18 files changed, 295 insertions(+), 65 deletions(-) create mode 100644 apps/cowswap-frontend/src/modules/injectedWidget/services/callOnBeforeApprovalWidgetHook.ts create mode 100644 libs/permit-utils/src/lib/generatePermitHook.test.ts diff --git a/apps/cowswap-frontend/src/common/hooks/useNeedsApproval.ts b/apps/cowswap-frontend/src/common/hooks/useNeedsApproval.ts index b6148749eb2..ad6ce8d7345 100644 --- a/apps/cowswap-frontend/src/common/hooks/useNeedsApproval.ts +++ b/apps/cowswap-frontend/src/common/hooks/useNeedsApproval.ts @@ -19,16 +19,16 @@ import { useTokenAllowance } from './useTokenAllowance' * @param amount * @returns {boolean} */ -export function useNeedsApproval(amount: Nullish>): boolean { - const spender = useTradeSpenderAddress() +export function useNeedsApproval(amount: Nullish>, spender?: string): boolean { + const tradeSpender = useTradeSpenderAddress() const token = amount ? getWrappedToken(amount.currency) : undefined - const allowance = useTokenAllowance(token) + const allowance = useTokenAllowance(token, undefined, spender ?? tradeSpender) if (typeof allowance === 'undefined') { return true } - if (!token || !amount || !spender) { + if (!token || !amount || !(spender ?? tradeSpender)) { return false } diff --git a/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproval.ts b/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproval.ts index 12621fe5542..bec2afe7f59 100644 --- a/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproval.ts +++ b/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproval.ts @@ -24,7 +24,7 @@ export function useApprovalStateForSpender( const token = currency && !getIsNativeToken(currency) ? currency : undefined const currentAllowance = useTokenAllowance(token, account ?? undefined, spender) - const { state: approvalState } = useApproveState(amountToApprove) + const { state: approvalState } = useApproveState(amountToApprove, spender) return useMemo(() => { return { approvalState, currentAllowance: currentAllowance?.data } diff --git a/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveCurrency.test.ts b/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveCurrency.test.ts index cd628927c67..0434bc9afc2 100644 --- a/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveCurrency.test.ts +++ b/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveCurrency.test.ts @@ -5,7 +5,7 @@ import { useWalletInfo } from '@cowprotocol/wallet' import { renderHook, waitFor } from '@testing-library/react' import { useTradeApproveCallback } from 'modules/erc20Approve' -import { callWidgetHook } from 'modules/injectedWidget' +import { callOnBeforeApprovalWidgetHook } from 'modules/injectedWidget' import { useShouldZeroApprove, useZeroApprove } from 'modules/zeroApproval' import { useApproveCurrency } from './useApproveCurrency' @@ -23,7 +23,7 @@ jest.mock('modules/erc20Approve', () => ({ })) jest.mock('modules/injectedWidget', () => ({ - callWidgetHook: jest.fn(), + callOnBeforeApprovalWidgetHook: jest.fn(), })) jest.mock('modules/zeroApproval', () => ({ @@ -34,7 +34,9 @@ jest.mock('modules/zeroApproval', () => ({ const mockUseTradeSpenderAddress = useTradeSpenderAddress as jest.MockedFunction const mockUseWalletInfo = useWalletInfo as jest.MockedFunction const mockUseTradeApproveCallback = useTradeApproveCallback as jest.MockedFunction -const mockCallWidgetHook = callWidgetHook as jest.MockedFunction +const mockCallOnBeforeApprovalWidgetHook = callOnBeforeApprovalWidgetHook as jest.MockedFunction< + typeof callOnBeforeApprovalWidgetHook +> const mockUseShouldZeroApprove = useShouldZeroApprove as jest.MockedFunction const mockUseZeroApprove = useZeroApprove as jest.MockedFunction @@ -54,7 +56,7 @@ describe('useApproveCurrency', () => { mockUseTradeSpenderAddress.mockReturnValue(spenderAddress) mockUseWalletInfo.mockReturnValue({ account } as ReturnType) mockUseTradeApproveCallback.mockReturnValue(tradeApproveCallback) - mockCallWidgetHook.mockResolvedValue(true) + mockCallOnBeforeApprovalWidgetHook.mockResolvedValue(true) shouldZeroApprove.mockResolvedValue(false) mockUseShouldZeroApprove.mockReturnValue(shouldZeroApprove) mockUseZeroApprove.mockReturnValue(zeroApprove) @@ -66,18 +68,11 @@ describe('useApproveCurrency', () => { await result.current(approveAmount) await waitFor(() => { - expect(mockCallWidgetHook).toHaveBeenCalledWith('ON_BEFORE_APPROVAL', { - chainId: mockToken.chainId, - sellToken: expect.objectContaining({ - address: mockToken.address, - chainId: mockToken.chainId, - decimals: mockToken.decimals, - name: mockToken.name, - symbol: mockToken.symbol, - }), - sellAmount: approveAmount.toString(), - walletAddress: account, + expect(mockCallOnBeforeApprovalWidgetHook).toHaveBeenCalledWith({ + account, + amountToApprove, spenderAddress, + approvalAmount: approveAmount, }) expect(tradeApproveCallback).toHaveBeenCalledWith(approveAmount, { useModals: true, @@ -87,14 +82,14 @@ describe('useApproveCurrency', () => { }) it('does not run on-chain approval when widget hook blocks it', async () => { - mockCallWidgetHook.mockResolvedValue(false) + mockCallOnBeforeApprovalWidgetHook.mockResolvedValue(false) const { result } = renderHook(() => useApproveCurrency(amountToApprove, true)) await result.current(approveAmount) await waitFor(() => { - expect(mockCallWidgetHook).toHaveBeenCalled() + expect(mockCallOnBeforeApprovalWidgetHook).toHaveBeenCalled() expect(shouldZeroApprove).not.toHaveBeenCalled() expect(zeroApprove).not.toHaveBeenCalled() expect(tradeApproveCallback).not.toHaveBeenCalled() diff --git a/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveCurrency.ts b/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveCurrency.ts index 11ff2070712..9267a69dc97 100644 --- a/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveCurrency.ts +++ b/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveCurrency.ts @@ -1,15 +1,13 @@ import { useCallback } from 'react' import { useTradeSpenderAddress } from '@cowprotocol/balances-and-allowances' -import { currencyAmountToTokenAmount } from '@cowprotocol/common-utils' import { Currency, CurrencyAmount } from '@cowprotocol/currency' import { Nullish } from '@cowprotocol/types' import { useWalletInfo } from '@cowprotocol/wallet' -import { WidgetHookEvents } from '@cowprotocol/widget-lib' import type { SafeMultisigTransactionResponse } from '@safe-global/types-kit' import { GenerecTradeApproveResult, useTradeApproveCallback } from 'modules/erc20Approve' -import { callWidgetHook } from 'modules/injectedWidget' +import { callOnBeforeApprovalWidgetHook } from 'modules/injectedWidget' import { useShouldZeroApprove, useZeroApprove } from 'modules/zeroApproval' export type ApproveCurrencyCallback = ( @@ -32,17 +30,11 @@ export function useApproveCurrency( async (amount: bigint) => { if (!account || !tradeSpenderAddress || !amountToApprove) return null - const tokenAmount = currencyAmountToTokenAmount(amountToApprove) - const isWidgetHookPassed = await callWidgetHook(WidgetHookEvents.ON_BEFORE_APPROVAL, { - chainId: tokenAmount.currency.chainId, - sellToken: { - ...tokenAmount.currency, - name: tokenAmount.currency.name || '', - symbol: tokenAmount.currency.symbol || '', - }, - sellAmount: amount.toString(), - walletAddress: account, + const isWidgetHookPassed = await callOnBeforeApprovalWidgetHook({ + account, + amountToApprove, spenderAddress: tradeSpenderAddress, + approvalAmount: amount, }) if (!isWidgetHookPassed) return null diff --git a/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveState.ts b/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveState.ts index 3a515020fb0..e5c4359417a 100644 --- a/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveState.ts +++ b/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveState.ts @@ -14,13 +14,16 @@ import { useTokenAllowance } from 'common/hooks/useTokenAllowance' import { ApprovalState } from '../types' import { getApprovalState } from '../utils' -export function useApproveState(amountToApprove: Nullish>): { +export function useApproveState( + amountToApprove: Nullish>, + spender?: string, +): { state: ApprovalState currentAllowance: Nullish } { const token = getCurrencyToApprove(amountToApprove) const tokenAddress = token?.address ? getAddressKey(token.address) : undefined - const currentAllowance = useTokenAllowance(token).data + const currentAllowance = useTokenAllowance(token, undefined, spender).data const pendingApproval = useHasPendingApproval(tokenAddress) const approvalStateBase = useSafeMemo(() => { diff --git a/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useGeneratePermitInAdvanceToTrade.test.ts b/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useGeneratePermitInAdvanceToTrade.test.ts index 93b0694342e..5735cb4b754 100644 --- a/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useGeneratePermitInAdvanceToTrade.test.ts +++ b/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useGeneratePermitInAdvanceToTrade.test.ts @@ -1,9 +1,11 @@ +import { useTradeSpenderAddress } from '@cowprotocol/balances-and-allowances' import { getWrappedToken } from '@cowprotocol/common-utils' import { CurrencyAmount, Token } from '@cowprotocol/currency' import { useWalletInfo, WalletInfo } from '@cowprotocol/wallet' import { renderHook } from '@testing-library/react' +import { callOnBeforeApprovalWidgetHook } from 'modules/injectedWidget' import { IsTokenPermittableResult, useGeneratePermitHook, usePermitInfo } from 'modules/permit' import { TradeType } from 'modules/trade' @@ -11,6 +13,10 @@ import { useGeneratePermitInAdvanceToTrade } from './useGeneratePermitInAdvanceT import { useResetApproveProgressModalState, useUpdateApproveProgressModalState } from '../' +jest.mock('@cowprotocol/balances-and-allowances', () => ({ + useTradeSpenderAddress: jest.fn(), +})) + jest.mock('@cowprotocol/common-utils', () => ({ ...jest.requireActual('@cowprotocol/common-utils'), getWrappedToken: jest.fn(), @@ -25,6 +31,10 @@ jest.mock('modules/permit', () => ({ usePermitInfo: jest.fn(), })) +jest.mock('modules/injectedWidget', () => ({ + callOnBeforeApprovalWidgetHook: jest.fn(), +})) + jest.mock('modules/trade', () => ({ TradeType: { SWAP: 'SWAP', @@ -36,8 +46,12 @@ jest.mock('../', () => ({ useResetApproveProgressModalState: jest.fn(), })) +const mockUseTradeSpenderAddress = useTradeSpenderAddress as jest.MockedFunction const mockGetWrappedToken = getWrappedToken as jest.MockedFunction const mockUseWalletInfo = useWalletInfo as jest.MockedFunction +const mockCallOnBeforeApprovalWidgetHook = callOnBeforeApprovalWidgetHook as jest.MockedFunction< + typeof callOnBeforeApprovalWidgetHook +> const mockUseGeneratePermitHook = useGeneratePermitHook as jest.MockedFunction const mockUsePermitInfo = usePermitInfo as jest.MockedFunction const mockUseUpdateApproveProgressModalState = useUpdateApproveProgressModalState as jest.MockedFunction< @@ -47,6 +61,7 @@ const mockUseResetApproveProgressModalState = useResetApproveProgressModalState typeof useResetApproveProgressModalState > +// eslint-disable-next-line max-lines-per-function describe('useGeneratePermitInAdvanceToTrade', () => { const mockToken = new Token(1, '0x1234567890123456789012345678901234567890', 18, 'TEST', 'Test Token') const mockWrappedToken = new Token(1, '0x0987654321098765432109876543210987654321', 18, 'WETH', 'Wrapped Ether') @@ -55,14 +70,17 @@ describe('useGeneratePermitInAdvanceToTrade', () => { const mockPermitInfo = { type: 'eip-2612' as const } const mockUpdateApproveProgressModalState = jest.fn() const mockResetApproveProgressModalState = jest.fn() + const mockSpenderAddress = '0x9008D19f58AAbD9eD0D60971565AA8510560ab41' const mockGeneratePermit = jest.fn() beforeEach(() => { jest.clearAllMocks() + mockUseTradeSpenderAddress.mockReturnValue(mockSpenderAddress) mockGetWrappedToken.mockReturnValue(mockWrappedToken as unknown as ReturnType) mockUseWalletInfo.mockReturnValue({ account: mockAccount, chainId: 1 } as WalletInfo) + mockCallOnBeforeApprovalWidgetHook.mockResolvedValue(true) mockUseGeneratePermitHook.mockReturnValue(mockGeneratePermit) mockUsePermitInfo.mockReturnValue(mockPermitInfo) mockUseUpdateApproveProgressModalState.mockReturnValue(mockUpdateApproveProgressModalState) @@ -85,7 +103,7 @@ describe('useGeneratePermitInAdvanceToTrade', () => { it('should call usePermitInfo with wrapped token and SWAP trade type', () => { renderHook(() => useGeneratePermitInAdvanceToTrade(mockAmountToApprove)) - expect(mockUsePermitInfo).toHaveBeenCalledWith(mockWrappedToken, TradeType.SWAP) + expect(mockUsePermitInfo).toHaveBeenCalledWith(mockWrappedToken, TradeType.SWAP, mockSpenderAddress) }) }) @@ -98,6 +116,7 @@ describe('useGeneratePermitInAdvanceToTrade', () => { const result_value = await generatePermit() expect(result_value).toBe(false) + expect(mockCallOnBeforeApprovalWidgetHook).not.toHaveBeenCalled() expect(mockGeneratePermit).not.toHaveBeenCalled() }) @@ -109,6 +128,7 @@ describe('useGeneratePermitInAdvanceToTrade', () => { const result_value = await generatePermit() expect(result_value).toBe(false) + expect(mockCallOnBeforeApprovalWidgetHook).not.toHaveBeenCalled() expect(mockGeneratePermit).not.toHaveBeenCalled() }) @@ -120,6 +140,7 @@ describe('useGeneratePermitInAdvanceToTrade', () => { const result_value = await generatePermit() expect(result_value).toBe(false) + expect(mockCallOnBeforeApprovalWidgetHook).not.toHaveBeenCalled() expect(mockGeneratePermit).not.toHaveBeenCalled() }) @@ -133,6 +154,11 @@ describe('useGeneratePermitInAdvanceToTrade', () => { const result_value = await generatePermit() expect(result_value).toBe(true) + expect(mockCallOnBeforeApprovalWidgetHook).toHaveBeenCalledWith({ + account: mockAccount, + amountToApprove: mockAmountToApprove, + spenderAddress: mockSpenderAddress, + }) expect(mockGeneratePermit).toHaveBeenCalledWith({ inputToken: { name: mockWrappedToken.name || '', @@ -141,11 +167,25 @@ describe('useGeneratePermitInAdvanceToTrade', () => { account: mockAccount, permitInfo: mockPermitInfo, amount: BigInt(mockAmountToApprove.quotient.toString()), + customSpender: mockSpenderAddress, preSignCallback: expect.any(Function), postSignCallback: expect.any(Function), }) }) + it('should stop before generating a permit when widget approval hook blocks it', async () => { + mockCallOnBeforeApprovalWidgetHook.mockResolvedValue(false) + + const { result } = renderHook(() => useGeneratePermitInAdvanceToTrade(mockAmountToApprove)) + + const generatePermit = result.current + const result_value = await generatePermit() + + expect(result_value).toBe(false) + expect(mockCallOnBeforeApprovalWidgetHook).toHaveBeenCalled() + expect(mockGeneratePermit).not.toHaveBeenCalled() + }) + it('should return false when generatePermit returns null', async () => { mockGeneratePermit.mockResolvedValue(null) diff --git a/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useGeneratePermitInAdvanceToTrade.ts b/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useGeneratePermitInAdvanceToTrade.ts index 95bc25ddfa5..28d0e1fadc9 100644 --- a/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useGeneratePermitInAdvanceToTrade.ts +++ b/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useGeneratePermitInAdvanceToTrade.ts @@ -1,9 +1,11 @@ import { useCallback } from 'react' +import { useTradeSpenderAddress } from '@cowprotocol/balances-and-allowances' import { getWrappedToken, isRejectRequestProviderError } from '@cowprotocol/common-utils' import { Currency, CurrencyAmount } from '@cowprotocol/currency' import { useWalletInfo } from '@cowprotocol/wallet' +import { callOnBeforeApprovalWidgetHook } from 'modules/injectedWidget' import { useGeneratePermitHook, usePermitInfo } from 'modules/permit' import { TradeType } from 'modules/trade' @@ -14,12 +16,23 @@ export function useGeneratePermitInAdvanceToTrade(amountToApprove: CurrencyAmoun const updateApproveProgressModalState = useUpdateApproveProgressModalState() const resetApproveProgressModalState = useResetApproveProgressModalState() const { account } = useWalletInfo() + const tradeSpenderAddress = useTradeSpenderAddress() const token = getWrappedToken(amountToApprove.currency) - const permitInfo = usePermitInfo(token, TradeType.SWAP) + const permitInfo = usePermitInfo(token, TradeType.SWAP, tradeSpenderAddress) return useCallback(async () => { - if (!account || !permitInfo) return false + if (!account || !permitInfo || !tradeSpenderAddress) return false + + const isWidgetHookPassed = await callOnBeforeApprovalWidgetHook({ + account, + amountToApprove, + spenderAddress: tradeSpenderAddress, + }) + + if (!isWidgetHookPassed) { + return false + } const preSignCallback = (): void => updateApproveProgressModalState({ @@ -34,6 +47,7 @@ export function useGeneratePermitInAdvanceToTrade(amountToApprove: CurrencyAmoun account, permitInfo, amount: BigInt(amountToApprove.quotient.toString()), + customSpender: tradeSpenderAddress, preSignCallback, postSignCallback: resetApproveProgressModalState, }) @@ -52,6 +66,7 @@ export function useGeneratePermitInAdvanceToTrade(amountToApprove: CurrencyAmoun generatePermit, permitInfo, resetApproveProgressModalState, + tradeSpenderAddress, token.address, token.name, updateApproveProgressModalState, diff --git a/apps/cowswap-frontend/src/modules/ethFlow/containers/EthFlow/hooks/useEthFlowActions.ts b/apps/cowswap-frontend/src/modules/ethFlow/containers/EthFlow/hooks/useEthFlowActions.ts index 7a7903060c0..5b8c8aec413 100644 --- a/apps/cowswap-frontend/src/modules/ethFlow/containers/EthFlow/hooks/useEthFlowActions.ts +++ b/apps/cowswap-frontend/src/modules/ethFlow/containers/EthFlow/hooks/useEthFlowActions.ts @@ -8,7 +8,7 @@ import { useWalletInfo } from '@cowprotocol/wallet' import { WrapUnwrapCallback } from 'legacy/hooks/useWrapCallback' import { Field } from 'legacy/state/types' -import { MAX_APPROVE_AMOUNT, TradeApproveCallback } from 'modules/erc20Approve' +import { ApproveCurrencyCallback, MAX_APPROVE_AMOUNT } from 'modules/erc20Approve' import { useIsInfiniteApproveDisabledInWidget } from 'modules/injectedWidget' import { useSwapPartialApprovalToggleState } from 'modules/swap/hooks/useSwapSettings' import { useOnCurrencySelection, useTradeConfirmActions } from 'modules/trade' @@ -16,7 +16,7 @@ import { useOnCurrencySelection, useTradeConfirmActions } from 'modules/trade' import { updateEthFlowContextAtom } from '../../../state/ethFlowContextAtom' export interface EthFlowActionCallbacks { - approve: TradeApproveCallback + approve: ApproveCurrencyCallback wrap: WrapUnwrapCallback | null directSwap: Command dismiss: Command diff --git a/apps/cowswap-frontend/src/modules/ethFlow/containers/EthFlow/index.tsx b/apps/cowswap-frontend/src/modules/ethFlow/containers/EthFlow/index.tsx index 357a5e9105e..bd64d4eb960 100644 --- a/apps/cowswap-frontend/src/modules/ethFlow/containers/EthFlow/index.tsx +++ b/apps/cowswap-frontend/src/modules/ethFlow/containers/EthFlow/index.tsx @@ -11,10 +11,10 @@ import { useSingleActivityDescriptor } from 'legacy/hooks/useRecentActivity' import { WrapUnwrapCallback } from 'legacy/hooks/useWrapCallback' import { + useApproveCurrency, useApproveState, useIsPartialApproveSelectedByUser, usePartialApproveAmountModalState, - useTradeApproveCallback, useUpdatePartialApproveAmountModalState, } from 'modules/erc20Approve' import { useWrappedToken } from 'modules/trade' @@ -63,7 +63,7 @@ export function EthFlowModal({ const isPartialApproveSelectedByUser = useIsPartialApproveSelectedByUser() const currencyToApprove = isPartialApproveSelectedByUser ? (amountSetByUser ?? wrappedAmount) : undefined - const approveCallback = useTradeApproveCallback(wrapped) + const approveCallback = useApproveCurrency(wrappedAmount, true) const ethFlowActions = useEthFlowActions( { diff --git a/apps/cowswap-frontend/src/modules/injectedWidget/index.ts b/apps/cowswap-frontend/src/modules/injectedWidget/index.ts index 6527624fad9..c4beb3c2488 100644 --- a/apps/cowswap-frontend/src/modules/injectedWidget/index.ts +++ b/apps/cowswap-frontend/src/modules/injectedWidget/index.ts @@ -8,6 +8,7 @@ export { useInjectedWidgetPalette } from './hooks/useInjectedWidgetPalette' export { WidgetMarkdownContent } from './pure/WidgetMarkdownContent' export { callWidgetHook } from './services/callWidgetHook' +export { callOnBeforeApprovalWidgetHook } from './services/callOnBeforeApprovalWidgetHook' export { buildOrderWidgetHookPayload, buildOrdersWidgetHookPayload, diff --git a/apps/cowswap-frontend/src/modules/injectedWidget/services/callOnBeforeApprovalWidgetHook.ts b/apps/cowswap-frontend/src/modules/injectedWidget/services/callOnBeforeApprovalWidgetHook.ts new file mode 100644 index 00000000000..8d3ca828617 --- /dev/null +++ b/apps/cowswap-frontend/src/modules/injectedWidget/services/callOnBeforeApprovalWidgetHook.ts @@ -0,0 +1,33 @@ +import { currencyAmountToTokenAmount } from '@cowprotocol/common-utils' +import { Currency, CurrencyAmount } from '@cowprotocol/currency' +import { WidgetHookEvents } from '@cowprotocol/widget-lib' + +import { callWidgetHook } from './callWidgetHook' + +interface CallOnBeforeApprovalWidgetHookParams { + amountToApprove: CurrencyAmount + account: string + spenderAddress: string + approvalAmount?: bigint +} + +export function callOnBeforeApprovalWidgetHook({ + amountToApprove, + account, + spenderAddress, + approvalAmount, +}: CallOnBeforeApprovalWidgetHookParams): Promise { + const tokenAmount = currencyAmountToTokenAmount(amountToApprove) + + return callWidgetHook(WidgetHookEvents.ON_BEFORE_APPROVAL, { + chainId: tokenAmount.currency.chainId, + sellToken: { + ...tokenAmount.currency, + name: tokenAmount.currency.name || '', + symbol: tokenAmount.currency.symbol || '', + }, + sellAmount: (approvalAmount ?? BigInt(amountToApprove.quotient.toString())).toString(), + walletAddress: account, + spenderAddress, + }) +} diff --git a/apps/cowswap-frontend/src/modules/limitOrders/services/safeBundleFlow/index.ts b/apps/cowswap-frontend/src/modules/limitOrders/services/safeBundleFlow/index.ts index df33cdec9c2..86ab6ab16ad 100644 --- a/apps/cowswap-frontend/src/modules/limitOrders/services/safeBundleFlow/index.ts +++ b/apps/cowswap-frontend/src/modules/limitOrders/services/safeBundleFlow/index.ts @@ -13,6 +13,7 @@ import { partialOrderUpdate } from 'legacy/state/orders/utils' import { mapUnsignedOrderToOrder, wrapErrorInOperatorError } from 'legacy/utils/trade' import { removePermitHookFromAppData } from 'modules/appData' +import { callOnBeforeApprovalWidgetHook } from 'modules/injectedWidget' import { LOW_RATE_THRESHOLD_PERCENT } from 'modules/limitOrders/const/trade' import { PriceImpactDeclineError, SafeBundleFlowContext } from 'modules/limitOrders/services/types' import { LimitOrdersSettingsState } from 'modules/limitOrders/state/limitOrdersSettingsAtom' @@ -47,7 +48,7 @@ export async function safeBundleFlow({ analytics: TradeFlowAnalytics beforeTrade?: Command config: Config -}): Promise { +}): Promise { logTradeFlow(LOG_PREFIX, 'STEP 1: confirm price impact') const isTooLowRate = params.rateImpact < LOW_RATE_THRESHOLD_PERCENT @@ -68,15 +69,26 @@ export async function safeBundleFlow({ orderType: UiOrderType.LIMIT, } - logTradeFlow(LOG_PREFIX, 'STEP 2: send transaction') - analytics.approveAndPresign(swapFlowAnalyticsContext) - beforeTrade?.() - const { chainId, postOrderParams, spender, dispatch, sendBatchTransactions } = params const validTo = calculateLimitOrdersDeadline(settingsState, params.quoteState) try { + const isWidgetHookPassed = await callOnBeforeApprovalWidgetHook({ + account, + amountToApprove: inputAmount, + spenderAddress: spender, + approvalAmount: maxUint256, + }) + + if (!isWidgetHookPassed) { + return undefined + } + + logTradeFlow(LOG_PREFIX, 'STEP 2: send transaction') + analytics.approveAndPresign(swapFlowAnalyticsContext) + beforeTrade?.() + // For now, bundling ALWAYS includes 2 steps: approve and presign. // In the feature users will be able to sort/add steps as they see fit logTradeFlow(LOG_PREFIX, 'STEP 2: build approval tx') diff --git a/apps/cowswap-frontend/src/modules/tradeFlow/hooks/useSafeBundleFlowContext.ts b/apps/cowswap-frontend/src/modules/tradeFlow/hooks/useSafeBundleFlowContext.ts index fb1cf216115..6c46163eb95 100644 --- a/apps/cowswap-frontend/src/modules/tradeFlow/hooks/useSafeBundleFlowContext.ts +++ b/apps/cowswap-frontend/src/modules/tradeFlow/hooks/useSafeBundleFlowContext.ts @@ -22,7 +22,7 @@ export function useSafeBundleFlowContext(): SafeBundleFlowContext | null { // todo check for safe wallet const { maximumSendSellAmount } = useAmountsToSignFromQuote() || {} - const needsApproval = useNeedsApproval(maximumSendSellAmount) + const needsApproval = useNeedsApproval(maximumSendSellAmount, spender) const tokenAddress = useMemo(() => { return maximumSendSellAmount ? getCurrencyAddress(maximumSendSellAmount.currency) : undefined }, [maximumSendSellAmount]) diff --git a/apps/cowswap-frontend/src/modules/tradeFlow/services/safeBundleFlow/safeBundleApprovalFlow.ts b/apps/cowswap-frontend/src/modules/tradeFlow/services/safeBundleFlow/safeBundleApprovalFlow.ts index 9c51ba9be15..c63e5d9abb6 100644 --- a/apps/cowswap-frontend/src/modules/tradeFlow/services/safeBundleFlow/safeBundleApprovalFlow.ts +++ b/apps/cowswap-frontend/src/modules/tradeFlow/services/safeBundleFlow/safeBundleApprovalFlow.ts @@ -12,6 +12,7 @@ import { partialOrderUpdate } from 'legacy/state/orders/utils' import { mapUnsignedOrderToOrder, wrapErrorInOperatorError } from 'legacy/utils/trade' import { removePermitHookFromAppData } from 'modules/appData' +import { callOnBeforeApprovalWidgetHook } from 'modules/injectedWidget' import { buildApproveTx } from 'modules/operations/bundle/buildApproveTx' import { buildZeroApproveTx } from 'modules/operations/bundle/buildZeroApproveTx' import { emitPostedOrderEvent } from 'modules/orders' @@ -67,10 +68,20 @@ export async function safeBundleApprovalFlow({ const tradeAmounts = { inputAmount, outputAmount } const isBridgingOrder = inputAmount.currency.chainId !== outputAmount.currency.chainId - analytics.approveAndPresign(swapFlowAnalyticsContext) - tradeConfirmActions.onSign(tradeAmounts) - try { + const isWidgetHookPassed = await callOnBeforeApprovalWidgetHook({ + account, + amountToApprove, + spenderAddress: spender, + }) + + if (!isWidgetHookPassed) { + return false + } + + analytics.approveAndPresign(swapFlowAnalyticsContext) + tradeConfirmActions.onSign(tradeAmounts) + // For now, bundling ALWAYS includes 2 steps: approve and presign. // In the feature users will be able to sort/add steps as they see fit logTradeFlow(LOG_PREFIX, 'STEP 2: build approval tx') diff --git a/apps/cowswap-frontend/src/modules/tradeFlow/services/safeBundleFlow/safeBundleEthFlow.ts b/apps/cowswap-frontend/src/modules/tradeFlow/services/safeBundleFlow/safeBundleEthFlow.ts index 3119d57a2a8..0c2eb684f0b 100644 --- a/apps/cowswap-frontend/src/modules/tradeFlow/services/safeBundleFlow/safeBundleEthFlow.ts +++ b/apps/cowswap-frontend/src/modules/tradeFlow/services/safeBundleFlow/safeBundleEthFlow.ts @@ -12,6 +12,7 @@ import { partialOrderUpdate } from 'legacy/state/orders/utils' import { mapUnsignedOrderToOrder, type PostOrderParams, wrapErrorInOperatorError } from 'legacy/utils/trade' import { removePermitHookFromAppData } from 'modules/appData' +import { callOnBeforeApprovalWidgetHook } from 'modules/injectedWidget' import { buildApproveTx } from 'modules/operations/bundle/buildApproveTx' import { buildWrapTx } from 'modules/operations/bundle/buildWrapTx' import { emitPostedOrderEvent } from 'modules/orders' @@ -26,7 +27,7 @@ import { SafeBundleFlowContext, TradeFlowContext } from '../../types/TradeFlowCo const LOG_PREFIX = 'SAFE BUNDLE ETH FLOW' // TODO: Break down this large function into smaller functions -// eslint-disable-next-line max-lines-per-function +// eslint-disable-next-line complexity, max-lines-per-function export async function safeBundleEthFlow( tradeContext: TradeFlowContext, safeBundleContext: SafeBundleFlowContext, @@ -62,11 +63,8 @@ export async function safeBundleEthFlow( const { account, recipientAddressOrName, kind } = orderParams const isBridgingOrder = inputAmount.currency.chainId !== outputAmount.currency.chainId - analytics.wrapApproveAndPresign(swapFlowAnalyticsContext) const nativeAmountInWei = inputAmount.quotient.toString() const tradeAmounts = { inputAmount, outputAmount } - - tradeConfirmActions.onSign(tradeAmounts) try { const txs: MetaTransactionData[] = [] @@ -83,6 +81,19 @@ export async function safeBundleEthFlow( logTradeFlow(LOG_PREFIX, 'STEP 3: [optional] build approval tx') if (needsApproval) { + const isWidgetHookPassed = await callOnBeforeApprovalWidgetHook({ + account, + amountToApprove, + spenderAddress: spender, + }) + + if (!isWidgetHookPassed) { + return false + } + + analytics.wrapApproveAndPresign(swapFlowAnalyticsContext) + tradeConfirmActions.onSign(tradeAmounts) + const approveTx = await buildApproveTx({ tokenAddress: wrappedNativeContract.address, spender, @@ -97,6 +108,11 @@ export async function safeBundleEthFlow( }) } + if (!needsApproval) { + analytics.wrapApproveAndPresign(swapFlowAnalyticsContext) + tradeConfirmActions.onSign(tradeAmounts) + } + orderParams.appData = await removePermitHookFromAppData(orderParams.appData, typedHooks) logTradeFlow(LOG_PREFIX, 'STEP 4: post order') diff --git a/apps/cowswap-frontend/src/modules/twap/hooks/useTwapOrderCreationContext.ts b/apps/cowswap-frontend/src/modules/twap/hooks/useTwapOrderCreationContext.ts index efe6c5fcb26..97124834992 100644 --- a/apps/cowswap-frontend/src/modules/twap/hooks/useTwapOrderCreationContext.ts +++ b/apps/cowswap-frontend/src/modules/twap/hooks/useTwapOrderCreationContext.ts @@ -29,10 +29,10 @@ export function useTwapOrderCreationContext( ): TwapOrderCreationContext | null { const composableCowContract = useComposableCowContractData() const composableCowChainId = composableCowContract.chainId - const needsApproval = useNeedsApproval(inputAmount) + const spender = useTradeSpenderAddress() + const needsApproval = useNeedsApproval(inputAmount, spender) const erc20ContractData = useTokenContract(inputAmount?.currency.address) const erc20ChainId = erc20ContractData.chainId - const spender = useTradeSpenderAddress() const needsZeroApproval = useNeedsZeroApproval(inputAmount?.currency, spender, inputAmount) const currentBlockFactoryAddress = composableCowChainId != null ? CURRENT_BLOCK_FACTORY_ADDRESS[composableCowChainId as SupportedChainId] : null diff --git a/libs/permit-utils/src/lib/generatePermitHook.test.ts b/libs/permit-utils/src/lib/generatePermitHook.test.ts new file mode 100644 index 00000000000..0b3f4c4c3cc --- /dev/null +++ b/libs/permit-utils/src/lib/generatePermitHook.test.ts @@ -0,0 +1,84 @@ +import { Address } from 'viem' +import { estimateGas } from 'wagmi/actions' + +import { generatePermitHook } from './generatePermitHook' + +import { buildEip2612PermitCallData } from '../utils/buildPermitCallData' + +import type { PermitHookParams } from '../types' +import type { Config } from 'wagmi' + +jest.mock('../const', () => ({ + DEFAULT_PERMIT_GAS_LIMIT: 50000n, + DEFAULT_PERMIT_VALUE: 1n, + PERMIT_ACCOUNT: { + address: '0x0000000000000000000000000000000000000001', + }, +})) + +jest.mock('wagmi/actions', () => ({ + estimateGas: jest.fn(), +})) + +jest.mock('../utils/buildPermitCallData', () => ({ + buildEip2612PermitCallData: jest.fn(), + buildDaiLikePermitCallData: jest.fn(), +})) + +const mockEstimateGas = estimateGas as jest.MockedFunction +const mockBuildEip2612PermitCallData = buildEip2612PermitCallData as jest.MockedFunction< + typeof buildEip2612PermitCallData +> + +describe('generatePermitHook request cache', () => { + const config = {} as Config + const tokenAddress = '0x1111111111111111111111111111111111111111' as Address + const account = '0x2222222222222222222222222222222222222222' as Address + const spender = '0x3333333333333333333333333333333333333333' + const otherSpender = '0x4444444444444444444444444444444444444444' + + const eip2612Utils = { + getTokenNonce: jest.fn().mockResolvedValue(7), + } + + function createParams(customSpender = spender): PermitHookParams { + return { + chainId: 1, + config, + eip2612Utils: eip2612Utils as unknown as PermitHookParams['eip2612Utils'], + inputToken: { + address: tokenAddress, + name: 'Test Token', + }, + permitInfo: { + type: 'eip-2612', + name: 'Test Token', + version: '1', + }, + spender: customSpender, + account, + amount: 123n, + nonce: 7, + } + } + + beforeEach(() => { + jest.clearAllMocks() + + mockEstimateGas.mockResolvedValue(45000n) + mockBuildEip2612PermitCallData.mockResolvedValue('0xpermit') + }) + + it('reuses the in-flight request when the spender is unchanged', async () => { + const [first, second] = await Promise.all([generatePermitHook(createParams()), generatePermitHook(createParams())]) + + expect(first).toEqual(second) + expect(mockBuildEip2612PermitCallData).toHaveBeenCalledTimes(1) + }) + + it('does not reuse the in-flight request when the spender changes', async () => { + await Promise.all([generatePermitHook(createParams()), generatePermitHook(createParams(otherSpender))]) + + expect(mockBuildEip2612PermitCallData).toHaveBeenCalledTimes(2) + }) +}) diff --git a/libs/permit-utils/src/lib/generatePermitHook.ts b/libs/permit-utils/src/lib/generatePermitHook.ts index d60da8bf2cf..dd61c62abda 100644 --- a/libs/permit-utils/src/lib/generatePermitHook.ts +++ b/libs/permit-utils/src/lib/generatePermitHook.ts @@ -1,3 +1,4 @@ +import { getAddressKey } from '@cowprotocol/cow-sdk' import { PERMIT_HOOK_DAPP_ID } from '@cowprotocol/hook-dapp-lib' import { Address, Hex } from 'viem' @@ -17,11 +18,35 @@ const REQUESTS_CACHE: { [permitKey: string]: Promise const USER_REJECTION_CODES = [4001, -32000] const USER_REJECTION_MESSAGES = ['user denied', 'user rejected', 'rejected transaction', 'transaction was rejected'] -// eslint-disable-next-line @typescript-eslint/no-explicit-any -function isUserRejectionError(error: any): boolean { +function hasUserRejectionCode(error: unknown): boolean { + return ( + typeof error === 'object' && + error !== null && + 'code' in error && + USER_REJECTION_CODES.includes(error.code as number) + ) +} + +function getErrorMessage(error: unknown): string { + if (typeof error === 'string') { + return error.toLowerCase() + } + + if (typeof error === 'object' && error !== null && 'message' in error && typeof error.message === 'string') { + return error.message.toLowerCase() + } + + return '' +} + +function isUserRejectionError(error: unknown): boolean { if (!error) return false - if (USER_REJECTION_CODES.includes(error.code)) return true - const message = (typeof error === 'string' ? error : error.message)?.toLowerCase() || '' + if (hasUserRejectionCode(error)) { + return true + } + + const message = getErrorMessage(error) + return USER_REJECTION_MESSAGES.some((msg) => message.includes(msg)) } @@ -160,6 +185,9 @@ async function calculateGasLimit({ } function getCacheKey(params: PermitHookParams): string { - const { inputToken, chainId, account, amount } = params - return `${inputToken.address.toLowerCase()}-${chainId}${account ? `-${account.toLowerCase()}` : ''}${amount ? `-${amount.toString()}` : ''}` + const { inputToken, chainId, account, amount, spender } = params + + return `${getAddressKey(inputToken.address)}-${chainId}-${getAddressKey(spender)}${ + account ? `-${getAddressKey(account)}` : '' + }${amount ? `-${amount.toString()}` : ''}` } From 3567a3c3e2ca4d50546b0af8051339067ee97fc3 Mon Sep 17 00:00:00 2001 From: fairlighteth <31534717+fairlighteth@users.noreply.github.com> Date: Sun, 7 Jun 2026 09:39:49 +0100 Subject: [PATCH 3/5] fix: isolate permit checks by spender - key permit support caches by spender - bypass default pre-generated permit data for custom spenders - restore eth-flow approval typing so app checks pass --- .../src/modules/erc20Approve/utils/index.ts | 1 + .../EthFlow/hooks/useEthFlowActions.ts | 17 +- .../ethFlow/containers/EthFlow/index.tsx | 2 +- .../permit/hooks/usePermitInfo.test.ts | 158 ++++++++++++++++++ .../src/modules/permit/hooks/usePermitInfo.ts | 61 ++++--- .../permit/state/permittableTokensAtom.ts | 10 +- .../src/modules/permit/types.ts | 1 + .../src/modules/swap/index.ts | 1 + .../src/lib/getTokenPermitInfo.test.ts | 93 +++++++++++ .../src/lib/getTokenPermitInfo.ts | 6 +- 10 files changed, 312 insertions(+), 38 deletions(-) create mode 100644 apps/cowswap-frontend/src/modules/permit/hooks/usePermitInfo.test.ts create mode 100644 libs/permit-utils/src/lib/getTokenPermitInfo.test.ts diff --git a/apps/cowswap-frontend/src/modules/erc20Approve/utils/index.ts b/apps/cowswap-frontend/src/modules/erc20Approve/utils/index.ts index b4e1b36664e..ce00c319f8c 100644 --- a/apps/cowswap-frontend/src/modules/erc20Approve/utils/index.ts +++ b/apps/cowswap-frontend/src/modules/erc20Approve/utils/index.ts @@ -1,2 +1,3 @@ export { getApprovalState } from './getApprovalState' +export { getIsTradeApproveResult } from './getIsTradeApproveResult' export * from './isMaxAmountToApprove' diff --git a/apps/cowswap-frontend/src/modules/ethFlow/containers/EthFlow/hooks/useEthFlowActions.ts b/apps/cowswap-frontend/src/modules/ethFlow/containers/EthFlow/hooks/useEthFlowActions.ts index 5b8c8aec413..36aecde5a9a 100644 --- a/apps/cowswap-frontend/src/modules/ethFlow/containers/EthFlow/hooks/useEthFlowActions.ts +++ b/apps/cowswap-frontend/src/modules/ethFlow/containers/EthFlow/hooks/useEthFlowActions.ts @@ -8,9 +8,9 @@ import { useWalletInfo } from '@cowprotocol/wallet' import { WrapUnwrapCallback } from 'legacy/hooks/useWrapCallback' import { Field } from 'legacy/state/types' -import { ApproveCurrencyCallback, MAX_APPROVE_AMOUNT } from 'modules/erc20Approve' +import { ApproveCurrencyCallback, getIsTradeApproveResult, MAX_APPROVE_AMOUNT } from 'modules/erc20Approve' import { useIsInfiniteApproveDisabledInWidget } from 'modules/injectedWidget' -import { useSwapPartialApprovalToggleState } from 'modules/swap/hooks/useSwapSettings' +import { useSwapPartialApprovalToggleState } from 'modules/swap' import { useOnCurrencySelection, useTradeConfirmActions } from 'modules/trade' import { updateEthFlowContextAtom } from '../../../state/ethFlowContextAtom' @@ -77,10 +77,15 @@ export function useEthFlowActions(callbacks: EthFlowActionCallbacks, amountToApp return sendTransaction('approve', () => { return callbacks.approve(unitsToApprove).then((res): string | undefined => { - const tx = res?.txResponse - return (tx && 'transactionHash' in tx ? tx.transactionHash : (tx as { hash?: string })?.hash) as - | string - | undefined + if (!res) return undefined + + if (getIsTradeApproveResult(res)) { + const tx = res.txResponse + + return 'transactionHash' in tx ? tx.transactionHash : tx.hash + } + + return res.transactionHash || undefined }) }) } diff --git a/apps/cowswap-frontend/src/modules/ethFlow/containers/EthFlow/index.tsx b/apps/cowswap-frontend/src/modules/ethFlow/containers/EthFlow/index.tsx index bd64d4eb960..d853937460b 100644 --- a/apps/cowswap-frontend/src/modules/ethFlow/containers/EthFlow/index.tsx +++ b/apps/cowswap-frontend/src/modules/ethFlow/containers/EthFlow/index.tsx @@ -63,7 +63,7 @@ export function EthFlowModal({ const isPartialApproveSelectedByUser = useIsPartialApproveSelectedByUser() const currencyToApprove = isPartialApproveSelectedByUser ? (amountSetByUser ?? wrappedAmount) : undefined - const approveCallback = useApproveCurrency(wrappedAmount, true) + const approveCallback = useApproveCurrency(wrappedAmount ?? undefined, true) const ethFlowActions = useEthFlowActions( { diff --git a/apps/cowswap-frontend/src/modules/permit/hooks/usePermitInfo.test.ts b/apps/cowswap-frontend/src/modules/permit/hooks/usePermitInfo.test.ts new file mode 100644 index 00000000000..a167afe5b8a --- /dev/null +++ b/apps/cowswap-frontend/src/modules/permit/hooks/usePermitInfo.test.ts @@ -0,0 +1,158 @@ +import { useAtomValue, useSetAtom } from 'jotai' + +import { Token } from '@cowprotocol/currency' +import { getTokenPermitInfo } from '@cowprotocol/permit-utils' +import type { PermitInfo } from '@cowprotocol/permit-utils' +import { useWalletInfo } from '@cowprotocol/wallet' + +import { renderHook, waitFor } from '@testing-library/react' +import { useConfig, usePublicClient } from 'wagmi' + +import { TradeType } from 'modules/trade' + +import { useIsPermitEnabled } from 'common/hooks/featureFlags/useIsPermitEnabled' + +import { usePermitInfo } from './usePermitInfo' +import { usePreGeneratedPermitInfoForToken } from './usePreGeneratedPermitInfoForToken' + +import { getPermittableTokenKey } from '../state/permittableTokensAtom' + +const defaultSpender = '0x9008D19f58AAbD9eD0D60971565AA8510560ab41' +const customSpender = '0x1111111111111111111111111111111111111111' + +jest.mock('jotai', () => ({ + ...jest.requireActual('jotai'), + useAtomValue: jest.fn(), + useSetAtom: jest.fn(), +})) + +jest.mock('@cowprotocol/common-utils', () => ({ + COW_PROTOCOL_VAULT_RELAYER_ADDRESS: { 1: defaultSpender }, + getIsNativeToken: jest.fn().mockReturnValue(false), + getWrappedToken: jest.fn((token) => token), +})) + +jest.mock('@cowprotocol/permit-utils', () => ({ + DEFAULT_MIN_GAS_LIMIT: 50000n, + getTokenPermitInfo: jest.fn(), +})) + +jest.mock('@cowprotocol/wallet', () => ({ + useWalletInfo: jest.fn(), +})) + +jest.mock('wagmi', () => ({ + useConfig: jest.fn(), + usePublicClient: jest.fn(), +})) + +jest.mock('common/hooks/featureFlags/useIsPermitEnabled', () => ({ + useIsPermitEnabled: jest.fn(), +})) + +jest.mock('modules/trade', () => ({ + TradeType: { + SWAP: 'SWAP', + LIMIT_ORDER: 'LIMIT_ORDER', + ADVANCED_ORDERS: 'ADVANCED_ORDERS', + YIELD: 'YIELD', + }, +})) + +jest.mock('./usePreGeneratedPermitInfoForToken', () => ({ + usePreGeneratedPermitInfoForToken: jest.fn(), +})) + +const mockedUseAtomValue = useAtomValue as jest.MockedFunction +const mockedUseSetAtom = useSetAtom as jest.MockedFunction +const mockedGetTokenPermitInfo = getTokenPermitInfo as jest.MockedFunction +const mockedUseWalletInfo = useWalletInfo as jest.MockedFunction +const mockedUseConfig = useConfig as jest.MockedFunction +const mockedUsePublicClient = usePublicClient as jest.MockedFunction +const mockedUseIsPermitEnabled = useIsPermitEnabled as jest.MockedFunction +const mockedUsePreGeneratedPermitInfoForToken = usePreGeneratedPermitInfoForToken as jest.MockedFunction< + typeof usePreGeneratedPermitInfoForToken +> + +describe('usePermitInfo', () => { + const token = new Token(1, '0x1234567890123456789012345678901234567890', 18, 'TEST', 'Test Token') + const addPermitInfo = jest.fn() + const defaultPermitInfo: PermitInfo = { type: 'eip-2612', name: 'Test Token', version: '1' } + const fetchedPermitInfo: PermitInfo = { type: 'unsupported', name: 'Test Token' } + + beforeEach(() => { + jest.clearAllMocks() + + mockedUseAtomValue.mockReturnValue({}) + mockedUseSetAtom.mockReturnValue(addPermitInfo) + mockedUseWalletInfo.mockReturnValue({ chainId: 1 } as ReturnType) + mockedUseConfig.mockReturnValue({} as ReturnType) + mockedUsePublicClient.mockReturnValue({} as ReturnType) + mockedUseIsPermitEnabled.mockReturnValue(true) + mockedUsePreGeneratedPermitInfoForToken.mockImplementation(() => ({ permitInfo: undefined, isLoading: false })) + mockedGetTokenPermitInfo.mockResolvedValue(fetchedPermitInfo) + }) + + it('does not reuse cached permit info that belongs to a different spender', async () => { + mockedUseAtomValue.mockReturnValue({ + 1: { + [getPermittableTokenKey(token.address, defaultSpender)]: defaultPermitInfo, + }, + }) + + const { result } = renderHook(() => usePermitInfo(token, TradeType.SWAP, customSpender)) + + expect(result.current).toBeUndefined() + + await waitFor(() => { + expect(mockedGetTokenPermitInfo).toHaveBeenCalledWith( + expect.objectContaining({ + spender: customSpender, + tokenAddress: token.address, + }), + ) + }) + }) + + it('revalidates custom spenders instead of reusing pre-generated permit info', async () => { + mockedUsePreGeneratedPermitInfoForToken.mockImplementation((tokenToCheck) => ({ + permitInfo: tokenToCheck ? defaultPermitInfo : undefined, + isLoading: false, + })) + + const { result } = renderHook(() => usePermitInfo(token, TradeType.SWAP, customSpender)) + + expect(result.current).toBeUndefined() + expect(mockedUsePreGeneratedPermitInfoForToken).toHaveBeenCalledWith(undefined) + + await waitFor(() => { + expect(mockedGetTokenPermitInfo).toHaveBeenCalledWith( + expect.objectContaining({ + spender: customSpender, + tokenAddress: token.address, + }), + ) + }) + + await waitFor(() => { + expect(addPermitInfo).toHaveBeenCalledWith({ + chainId: 1, + tokenAddress: token.address, + spender: customSpender, + permitInfo: fetchedPermitInfo, + }) + }) + }) + + it('reuses pre-generated permit info for the default spender', () => { + mockedUsePreGeneratedPermitInfoForToken.mockImplementation((tokenToCheck) => ({ + permitInfo: tokenToCheck ? defaultPermitInfo : undefined, + isLoading: false, + })) + + const { result } = renderHook(() => usePermitInfo(token, TradeType.SWAP)) + + expect(result.current).toEqual(defaultPermitInfo) + expect(mockedGetTokenPermitInfo).not.toHaveBeenCalled() + }) +}) diff --git a/apps/cowswap-frontend/src/modules/permit/hooks/usePermitInfo.ts b/apps/cowswap-frontend/src/modules/permit/hooks/usePermitInfo.ts index 149b4916a89..f56f1f0cf92 100644 --- a/apps/cowswap-frontend/src/modules/permit/hooks/usePermitInfo.ts +++ b/apps/cowswap-frontend/src/modules/permit/hooks/usePermitInfo.ts @@ -10,13 +10,17 @@ import { useWalletInfo } from '@cowprotocol/wallet' import { Nullish } from 'types' import { useConfig, usePublicClient } from 'wagmi' -import { TradeType } from 'modules/trade/types/TradeType' +import { TradeType } from 'modules/trade' import { useIsPermitEnabled } from 'common/hooks/featureFlags/useIsPermitEnabled' import { usePreGeneratedPermitInfoForToken } from './usePreGeneratedPermitInfoForToken' -import { addPermitInfoForTokenAtom, permittableTokensAtom } from '../state/permittableTokensAtom' +import { + addPermitInfoForTokenAtom, + getPermittableTokenKey, + permittableTokensAtom, +} from '../state/permittableTokensAtom' import { IsTokenPermittableResult } from '../types' const ORDER_TYPE_SUPPORTS_PERMIT: Record = { @@ -63,29 +67,30 @@ export function usePermitInfo( const isPermitSupported = !!tradeType && ORDER_TYPE_SUPPORTS_PERMIT[tradeType] const isPermitEnabled = useIsPermitEnabled() && isPermitSupported + const defaultSpender = chainId ? COW_PROTOCOL_VAULT_RELAYER_ADDRESS[chainId] : undefined + const spender = customSpender || defaultSpender + const shouldUsePreGeneratedInfo = spender === defaultSpender const addPermitInfo = useAddPermitInfo() - const permitInfo = usePermitInfoState(chainId, isPermitEnabled ? lowerCaseAddress : undefined) + const permitInfo = usePermitInfoState(chainId, isPermitEnabled ? lowerCaseAddress : undefined, spender) const { permitInfo: preGeneratedInfo, isLoading: preGeneratedIsLoading } = usePreGeneratedPermitInfoForToken( - isPermitEnabled && !isNative ? token : undefined, + isPermitEnabled && !isNative && shouldUsePreGeneratedInfo ? token : undefined, ) - - const spender = customSpender || COW_PROTOCOL_VAULT_RELAYER_ADDRESS[chainId] + const hasResolvedPermitInfo = + permitInfo !== undefined || (shouldUsePreGeneratedInfo && preGeneratedInfo !== undefined) + const shouldSkipPermitInfoLoad = + !chainId || + !isPermitEnabled || + !lowerCaseAddress || + !spender || + !config || + !publicClient || + hasResolvedPermitInfo || + isNative || + preGeneratedIsLoading useEffect(() => { - if ( - !chainId || - !isPermitEnabled || - !lowerCaseAddress || - !config || - !publicClient || - permitInfo !== undefined || - isNative || - // Do not try to load when pre-generated info is loading - preGeneratedIsLoading || - // Do not try to load when pre-generated exists - preGeneratedInfo !== undefined - ) { + if (shouldSkipPermitInfoLoad) { return } @@ -108,7 +113,7 @@ export function usePermitInfo( // TODO: there is a Single Responsibility Principle breach here. This hook should not be responsible for caching. // TODO: better to create a separate updater for caching. // Otherwise, we know it is permittable or not. Cache it. - addPermitInfo({ chainId, tokenAddress: lowerCaseAddress, permitInfo: result }) + addPermitInfo({ chainId, tokenAddress: lowerCaseAddress, spender, permitInfo: result }) } }) }, [ @@ -122,6 +127,8 @@ export function usePermitInfo( permitInfo, preGeneratedInfo, preGeneratedIsLoading, + shouldUsePreGeneratedInfo, + shouldSkipPermitInfoLoad, spender, ]) @@ -129,7 +136,7 @@ export function usePermitInfo( return UNSUPPORTED } - return preGeneratedInfo ?? permitInfo + return shouldUsePreGeneratedInfo ? (preGeneratedInfo ?? permitInfo) : permitInfo } /** @@ -141,12 +148,16 @@ function useAddPermitInfo() { return useSetAtom(addPermitInfoForTokenAtom) } -function usePermitInfoState(chainId: SupportedChainId, tokenAddress: string | undefined): IsTokenPermittableResult { +function usePermitInfoState( + chainId: SupportedChainId, + tokenAddress: string | undefined, + spender: string | undefined, +): IsTokenPermittableResult { const permitableTokens = useAtomValue(permittableTokensAtom) return useMemo(() => { - if (!tokenAddress) return undefined + if (!tokenAddress || !spender) return undefined - return permitableTokens[chainId]?.[getAddressKey(tokenAddress)] - }, [chainId, permitableTokens, tokenAddress]) + return permitableTokens[chainId]?.[getPermittableTokenKey(tokenAddress, spender)] + }, [chainId, permitableTokens, spender, tokenAddress]) } diff --git a/apps/cowswap-frontend/src/modules/permit/state/permittableTokensAtom.ts b/apps/cowswap-frontend/src/modules/permit/state/permittableTokensAtom.ts index 6dd51a78a36..5842ed0dbba 100644 --- a/apps/cowswap-frontend/src/modules/permit/state/permittableTokensAtom.ts +++ b/apps/cowswap-frontend/src/modules/permit/state/permittableTokensAtom.ts @@ -10,6 +10,10 @@ import { AddPermitTokenParams } from '../types' type PermittableTokens = Record +export function getPermittableTokenKey(tokenAddress: string, spender: string): string { + return `${getAddressKey(tokenAddress)}-${getAddressKey(spender)}` +} + /** * Atom that stores the permittable tokens info for each chain on localStorage. * It's meant to be shared across different tabs, thus no special storage handling. @@ -18,7 +22,7 @@ type PermittableTokens = Record */ export const permittableTokensAtom = atomWithStorage>( - 'permittableTokens:v3', + 'permittableTokens:v4', mapSupportedNetworks({}), getJotaiMergerStorage(), ) @@ -28,13 +32,13 @@ export const permittableTokensAtom = atomWithStorage { + (get, set, { chainId, tokenAddress, spender, permitInfo }: AddPermitTokenParams) => { const permittableTokens = { ...get(permittableTokensAtom) } const permittableTokensForChain = permittableTokens[chainId] || {} permittableTokens[chainId] = { ...permittableTokensForChain, - [getAddressKey(tokenAddress)]: permitInfo, + [getPermittableTokenKey(tokenAddress, spender)]: permitInfo, } set(permittableTokensAtom, permittableTokens) diff --git a/apps/cowswap-frontend/src/modules/permit/types.ts b/apps/cowswap-frontend/src/modules/permit/types.ts index f0f15548d23..45532011c43 100644 --- a/apps/cowswap-frontend/src/modules/permit/types.ts +++ b/apps/cowswap-frontend/src/modules/permit/types.ts @@ -7,6 +7,7 @@ import { AppDataInfo, TypedAppDataHooks } from 'modules/appData' export type AddPermitTokenParams = { chainId: SupportedChainId tokenAddress: string + spender: string permitInfo: PermitInfo } diff --git a/apps/cowswap-frontend/src/modules/swap/index.ts b/apps/cowswap-frontend/src/modules/swap/index.ts index 214c9400186..9a9d773c8a5 100644 --- a/apps/cowswap-frontend/src/modules/swap/index.ts +++ b/apps/cowswap-frontend/src/modules/swap/index.ts @@ -4,6 +4,7 @@ export { useSwapRawState } from './hooks/useSwapRawState' export { useSwapFlowContext } from './hooks/useSwapFlowContext' export { useUpdateSwapRawState } from './hooks/useUpdateSwapRawState' export { useSwapDerivedStateToFill } from './hooks/useSwapDerivedState' +export { useSwapPartialApprovalToggleState } from './hooks/useSwapSettings' export { SwapUpdaters } from './updaters' export { swapDerivedStateAtom } from './state/swapRawStateAtom' export { DeprecatedNetworkBanner } from './containers/DeprecatedNetworkBanner/DeprecatedNetworkBanner.container' diff --git a/libs/permit-utils/src/lib/getTokenPermitInfo.test.ts b/libs/permit-utils/src/lib/getTokenPermitInfo.test.ts new file mode 100644 index 00000000000..3ed8fb5391d --- /dev/null +++ b/libs/permit-utils/src/lib/getTokenPermitInfo.test.ts @@ -0,0 +1,93 @@ +import { estimateGas } from 'wagmi/actions' + +import { getPermitUtilsInstance } from './getPermitUtilsInstance' +import { getTokenPermitInfo } from './getTokenPermitInfo' + +import { buildEip2612PermitCallData } from '../utils/buildPermitCallData' +import { getEip712Domain } from '../utils/getEip712Domain' + +import type { GetTokenPermitInfoParams } from '../types' +import type { Address } from 'viem' +import type { Config } from 'wagmi' + +jest.mock('../const', () => ({ + DEFAULT_MIN_GAS_LIMIT: 50000n, + DEFAULT_PERMIT_VALUE: 1n, + PERMIT_ACCOUNT: { + address: '0x0000000000000000000000000000000000000001', + }, +})) + +jest.mock('./getPermitUtilsInstance', () => ({ + getPermitUtilsInstance: jest.fn(), +})) + +jest.mock('../utils/getEip712Domain', () => ({ + getEip712Domain: jest.fn(), +})) + +jest.mock('../utils/buildPermitCallData', () => ({ + buildEip2612PermitCallData: jest.fn(), + buildDaiLikePermitCallData: jest.fn(), +})) + +jest.mock('wagmi/actions', () => ({ + estimateGas: jest.fn(), +})) + +const mockedEstimateGas = estimateGas as jest.MockedFunction +const mockedGetPermitUtilsInstance = getPermitUtilsInstance as jest.MockedFunction +const mockedGetEip712Domain = getEip712Domain as jest.MockedFunction +const mockedBuildEip2612PermitCallData = buildEip2612PermitCallData as jest.MockedFunction< + typeof buildEip2612PermitCallData +> + +describe('getTokenPermitInfo request cache', () => { + const config = {} as Config + const publicClient = {} + const spender = '0x3333333333333333333333333333333333333333' + const otherSpender = '0x4444444444444444444444444444444444444444' + + function createParams(tokenAddress: Address, currentSpender = spender): GetTokenPermitInfoParams { + return { + tokenAddress, + chainId: 1, + spender: currentSpender, + config, + publicClient, + minGasLimit: 50000n, + } + } + + beforeEach(() => { + jest.clearAllMocks() + + mockedGetPermitUtilsInstance.mockResolvedValue({ + getTokenNonce: jest.fn().mockResolvedValue(7), + } as Awaited>) + mockedGetEip712Domain.mockResolvedValue({ name: 'Test Token', version: '1' }) + mockedBuildEip2612PermitCallData.mockResolvedValue('0xpermit') + mockedEstimateGas.mockResolvedValue(60000n) + }) + + it('reuses the in-flight request when the spender is unchanged', async () => { + const tokenAddress = '0x1111111111111111111111111111111111111111' as Address + + await Promise.all([getTokenPermitInfo(createParams(tokenAddress)), getTokenPermitInfo(createParams(tokenAddress))]) + + expect(mockedBuildEip2612PermitCallData).toHaveBeenCalledTimes(1) + expect(mockedEstimateGas).toHaveBeenCalledTimes(1) + }) + + it('does not reuse the in-flight request when the spender changes', async () => { + const tokenAddress = '0x2222222222222222222222222222222222222222' as Address + + await Promise.all([ + getTokenPermitInfo(createParams(tokenAddress)), + getTokenPermitInfo(createParams(tokenAddress, otherSpender)), + ]) + + expect(mockedBuildEip2612PermitCallData).toHaveBeenCalledTimes(2) + expect(mockedEstimateGas).toHaveBeenCalledTimes(2) + }) +}) diff --git a/libs/permit-utils/src/lib/getTokenPermitInfo.ts b/libs/permit-utils/src/lib/getTokenPermitInfo.ts index 9ad59994cf0..2f28968411c 100644 --- a/libs/permit-utils/src/lib/getTokenPermitInfo.ts +++ b/libs/permit-utils/src/lib/getTokenPermitInfo.ts @@ -1,4 +1,4 @@ -import { getTokenId } from '@cowprotocol/cow-sdk' +import { getAddressKey, getTokenId } from '@cowprotocol/cow-sdk' import { Config } from 'wagmi' import { estimateGas } from 'wagmi/actions' @@ -33,8 +33,8 @@ const REQUESTS_CACHE: Record> = {} const UNSUPPORTED: PermitInfo = { type: 'unsupported' } export async function getTokenPermitInfo(params: GetTokenPermitInfoParams): Promise { - const { tokenAddress, chainId } = params - const key = getTokenId({ address: tokenAddress, chainId }) + const { tokenAddress, chainId, spender } = params + const key = `${getTokenId({ address: tokenAddress, chainId })}-${getAddressKey(spender)}` const cached = REQUESTS_CACHE[key] From 268a7199ad693ed26eb3a14583af3610ed59a265 Mon Sep 17 00:00:00 2001 From: fairlighteth <31534717+fairlighteth@users.noreply.github.com> Date: Thu, 9 Jul 2026 16:29:01 +0100 Subject: [PATCH 4/5] fix: harden permit cache follow-ups --- .../hooks/usePermitCompatibleTokens.test.ts | 68 +++++++++++++++++++ .../permit/hooks/usePermitCompatibleTokens.ts | 8 +-- .../permit/state/permittableTokensAtom.ts | 4 ++ .../src/lib/getTokenPermitInfo.test.ts | 27 ++++++++ .../src/lib/getTokenPermitInfo.ts | 11 +++ 5 files changed, 114 insertions(+), 4 deletions(-) create mode 100644 apps/cowswap-frontend/src/modules/permit/hooks/usePermitCompatibleTokens.test.ts diff --git a/apps/cowswap-frontend/src/modules/permit/hooks/usePermitCompatibleTokens.test.ts b/apps/cowswap-frontend/src/modules/permit/hooks/usePermitCompatibleTokens.test.ts new file mode 100644 index 00000000000..8981da3d30b --- /dev/null +++ b/apps/cowswap-frontend/src/modules/permit/hooks/usePermitCompatibleTokens.test.ts @@ -0,0 +1,68 @@ +import { useAtomValue } from 'jotai' + +import { getAddressKey } from '@cowprotocol/cow-sdk' +import { PermitInfo } from '@cowprotocol/permit-utils' +import { useWalletInfo } from '@cowprotocol/wallet' + +import { renderHook } from '@testing-library/react' + +import { useIsPermitEnabled } from 'common/hooks/featureFlags/useIsPermitEnabled' + +import { usePermitCompatibleTokens } from './usePermitCompatibleTokens' +import { usePreGeneratedPermitInfo } from './usePreGeneratedPermitInfo' + +import { getPermittableTokenKey } from '../state/permittableTokensAtom' + +jest.mock('jotai', () => ({ + ...jest.requireActual('jotai'), + useAtomValue: jest.fn(), +})) + +jest.mock('@cowprotocol/wallet', () => ({ + useWalletInfo: jest.fn(), +})) + +jest.mock('common/hooks/featureFlags/useIsPermitEnabled', () => ({ + useIsPermitEnabled: jest.fn(), +})) + +jest.mock('./usePreGeneratedPermitInfo', () => ({ + usePreGeneratedPermitInfo: jest.fn(), +})) + +const mockedUseAtomValue = useAtomValue as jest.MockedFunction +const mockedUseWalletInfo = useWalletInfo as jest.MockedFunction +const mockedUseIsPermitEnabled = useIsPermitEnabled as jest.MockedFunction +const mockedUsePreGeneratedPermitInfo = usePreGeneratedPermitInfo as jest.MockedFunction< + typeof usePreGeneratedPermitInfo +> + +describe('usePermitCompatibleTokens', () => { + const chainId = 1 + const tokenAddress = '0x1234567890123456789012345678901234567890' + const spender = '0x1111111111111111111111111111111111111111' + const permitInfo: PermitInfo = { type: 'eip-2612', name: 'Test Token', version: '1' } + + beforeEach(() => { + jest.clearAllMocks() + + mockedUseWalletInfo.mockReturnValue({ chainId } as ReturnType) + mockedUseIsPermitEnabled.mockReturnValue(true) + mockedUsePreGeneratedPermitInfo.mockReturnValue({ allPermitInfo: {}, isLoading: false }) + }) + + it('maps local token-spender permit info back to the token address', () => { + const permitTokenKey = getPermittableTokenKey(tokenAddress, spender) + + mockedUseAtomValue.mockReturnValue({ + [chainId]: { + [permitTokenKey]: permitInfo, + }, + }) + + const { result } = renderHook(() => usePermitCompatibleTokens()) + + expect(result.current[getAddressKey(tokenAddress)]).toBe(true) + expect(result.current[permitTokenKey]).toBeUndefined() + }) +}) diff --git a/apps/cowswap-frontend/src/modules/permit/hooks/usePermitCompatibleTokens.ts b/apps/cowswap-frontend/src/modules/permit/hooks/usePermitCompatibleTokens.ts index 638348067c2..cb0755f36ab 100644 --- a/apps/cowswap-frontend/src/modules/permit/hooks/usePermitCompatibleTokens.ts +++ b/apps/cowswap-frontend/src/modules/permit/hooks/usePermitCompatibleTokens.ts @@ -9,7 +9,7 @@ import { useIsPermitEnabled } from 'common/hooks/featureFlags/useIsPermitEnabled import { usePreGeneratedPermitInfo } from './usePreGeneratedPermitInfo' -import { permittableTokensAtom } from '../state/permittableTokensAtom' +import { getTokenAddressFromPermittableTokenKey, permittableTokensAtom } from '../state/permittableTokensAtom' import { PermitCompatibleTokens } from '../types' export function usePermitCompatibleTokens(): PermitCompatibleTokens { @@ -43,10 +43,10 @@ export function usePermitCompatibleTokens(): PermitCompatibleTokens { ) } - for (const address of Object.keys(localPermitInfoRef.current)) { - const addressLowerCased = getAddressKey(address) + for (const permitTokenKey of Object.keys(localPermitInfoRef.current)) { + const addressLowerCased = getAddressKey(getTokenAddressFromPermittableTokenKey(permitTokenKey)) - permitCompatibleTokens[addressLowerCased] = isSupportedPermitInfo(localPermitInfoRef.current[addressLowerCased]) + permitCompatibleTokens[addressLowerCased] = isSupportedPermitInfo(localPermitInfoRef.current[permitTokenKey]) } return permitCompatibleTokens diff --git a/apps/cowswap-frontend/src/modules/permit/state/permittableTokensAtom.ts b/apps/cowswap-frontend/src/modules/permit/state/permittableTokensAtom.ts index 5842ed0dbba..0cd3c1ad10c 100644 --- a/apps/cowswap-frontend/src/modules/permit/state/permittableTokensAtom.ts +++ b/apps/cowswap-frontend/src/modules/permit/state/permittableTokensAtom.ts @@ -14,6 +14,10 @@ export function getPermittableTokenKey(tokenAddress: string, spender: string): s return `${getAddressKey(tokenAddress)}-${getAddressKey(spender)}` } +export function getTokenAddressFromPermittableTokenKey(permitTokenKey: string): string { + return permitTokenKey.split('-')[0] +} + /** * Atom that stores the permittable tokens info for each chain on localStorage. * It's meant to be shared across different tabs, thus no special storage handling. diff --git a/libs/permit-utils/src/lib/getTokenPermitInfo.test.ts b/libs/permit-utils/src/lib/getTokenPermitInfo.test.ts index 3ed8fb5391d..35891862790 100644 --- a/libs/permit-utils/src/lib/getTokenPermitInfo.test.ts +++ b/libs/permit-utils/src/lib/getTokenPermitInfo.test.ts @@ -90,4 +90,31 @@ describe('getTokenPermitInfo request cache', () => { expect(mockedBuildEip2612PermitCallData).toHaveBeenCalledTimes(2) expect(mockedEstimateGas).toHaveBeenCalledTimes(2) }) + + it('does not cache transient error results', async () => { + const tokenAddress = '0x5555555555555555555555555555555555555555' as Address + const consoleDebugSpy = jest.spyOn(console, 'debug').mockImplementation(() => undefined) + + mockedGetPermitUtilsInstance + .mockResolvedValueOnce({ + getTokenNonce: jest.fn().mockRejectedValue(new Error('RPC timeout')), + } as Awaited>) + .mockResolvedValueOnce({ + getTokenNonce: jest.fn().mockResolvedValue(7), + } as Awaited>) + + try { + await expect(getTokenPermitInfo(createParams(tokenAddress))).resolves.toEqual({ error: 'RPC timeout' }) + await expect(getTokenPermitInfo(createParams(tokenAddress))).resolves.toEqual({ + type: 'eip-2612', + name: 'Test Token', + version: '1', + }) + + expect(mockedGetPermitUtilsInstance).toHaveBeenCalledTimes(2) + expect(mockedEstimateGas).toHaveBeenCalledTimes(1) + } finally { + consoleDebugSpy.mockRestore() + } + }) }) diff --git a/libs/permit-utils/src/lib/getTokenPermitInfo.ts b/libs/permit-utils/src/lib/getTokenPermitInfo.ts index 2f28968411c..c53237398ec 100644 --- a/libs/permit-utils/src/lib/getTokenPermitInfo.ts +++ b/libs/permit-utils/src/lib/getTokenPermitInfo.ts @@ -43,6 +43,17 @@ export async function getTokenPermitInfo(params: GetTokenPermitInfoParams): Prom } const request = actuallyCheckTokenIsPermittable(params) + .then((result) => { + if ('error' in result) { + delete REQUESTS_CACHE[key] + } + + return result + }) + .catch((error) => { + delete REQUESTS_CACHE[key] + throw error + }) REQUESTS_CACHE[key] = request From e579f425f5b9497aa1f02a4646bd7dd4d8728eca Mon Sep 17 00:00:00 2001 From: fairlighteth <31534717+fairlighteth@users.noreply.github.com> Date: Thu, 9 Jul 2026 17:12:56 +0100 Subject: [PATCH 5/5] fix: harden approval follow-ups --- .../src/common/hooks/useNeedsApproval.test.ts | 73 +++++++++++++++++++ .../src/common/hooks/useNeedsApproval.ts | 13 ++-- .../hooks/TransactionHooksMod.tsx | 11 ++- .../erc20Approve/hooks/useApproveState.ts | 2 +- .../useGeneratePermitInAdvanceToTrade.test.ts | 24 ++++++ .../useGeneratePermitInAdvanceToTrade.ts | 7 +- .../ethFlow/containers/EthFlow/index.tsx | 7 +- .../utils/getEthFlowCurrencyToApprove.test.ts | 52 +++++++++++++ .../utils/getEthFlowCurrencyToApprove.ts | 19 +++++ .../permit/hooks/usePermitInfo.test.ts | 18 +++++ .../src/modules/permit/hooks/usePermitInfo.ts | 12 ++- .../twap/hooks/useTwapOrderCreationContext.ts | 1 + .../hooks/useNeedsZeroApproval.ts | 14 +++- .../common-utils/src/getIsNativeToken.test.ts | 27 +++++++ libs/common-utils/src/getIsNativeToken.ts | 4 +- libs/permit-utils/src/index.ts | 2 +- .../src/lib/generatePermitHook.test.ts | 14 ++++ .../src/lib/generatePermitHook.ts | 4 +- 18 files changed, 281 insertions(+), 23 deletions(-) create mode 100644 apps/cowswap-frontend/src/common/hooks/useNeedsApproval.test.ts create mode 100644 apps/cowswap-frontend/src/modules/ethFlow/containers/EthFlow/utils/getEthFlowCurrencyToApprove.test.ts create mode 100644 apps/cowswap-frontend/src/modules/ethFlow/containers/EthFlow/utils/getEthFlowCurrencyToApprove.ts create mode 100644 libs/common-utils/src/getIsNativeToken.test.ts diff --git a/apps/cowswap-frontend/src/common/hooks/useNeedsApproval.test.ts b/apps/cowswap-frontend/src/common/hooks/useNeedsApproval.test.ts new file mode 100644 index 00000000000..068e9f9527c --- /dev/null +++ b/apps/cowswap-frontend/src/common/hooks/useNeedsApproval.test.ts @@ -0,0 +1,73 @@ +import { useTradeSpenderAddress } from '@cowprotocol/balances-and-allowances' +import { CurrencyAmount, Token } from '@cowprotocol/currency' + +import { renderHook } from '@testing-library/react' +import { SWRResponse } from 'swr' + +import { useNeedsApproval } from './useNeedsApproval' +import { useTokenAllowance } from './useTokenAllowance' + +jest.mock('@cowprotocol/balances-and-allowances', () => ({ + useTradeSpenderAddress: jest.fn(), +})) + +jest.mock('./useTokenAllowance', () => ({ + useTokenAllowance: jest.fn(), +})) + +const mockUseTradeSpenderAddress = useTradeSpenderAddress as jest.MockedFunction +const mockUseTokenAllowance = useTokenAllowance as jest.MockedFunction + +describe('useNeedsApproval', () => { + const spender = '0x0000000000000000000000000000000000000001' + const token = new Token(1, '0x1234567890123456789012345678901234567890', 18, 'TEST', 'Test Token') + const amount = CurrencyAmount.fromRawAmount(token, '100') + + function mockAllowance(data: bigint | undefined): void { + mockUseTokenAllowance.mockReturnValue({ data } as SWRResponse) + } + + beforeEach(() => { + jest.clearAllMocks() + mockUseTradeSpenderAddress.mockReturnValue(spender) + mockAllowance(0n) + }) + + it('returns false when amount is missing', () => { + const { result } = renderHook(() => useNeedsApproval(null)) + + expect(result.current).toBe(false) + }) + + it('returns false when spender is missing', () => { + mockUseTradeSpenderAddress.mockReturnValue(undefined) + + const { result } = renderHook(() => useNeedsApproval(amount)) + + expect(result.current).toBe(false) + }) + + it('returns true when allowance is not loaded yet', () => { + mockAllowance(undefined) + + const { result } = renderHook(() => useNeedsApproval(amount)) + + expect(result.current).toBe(true) + }) + + it('returns true when allowance is insufficient', () => { + mockAllowance(99n) + + const { result } = renderHook(() => useNeedsApproval(amount)) + + expect(result.current).toBe(true) + }) + + it('returns false when allowance is sufficient', () => { + mockAllowance(100n) + + const { result } = renderHook(() => useNeedsApproval(amount)) + + expect(result.current).toBe(false) + }) +}) diff --git a/apps/cowswap-frontend/src/common/hooks/useNeedsApproval.ts b/apps/cowswap-frontend/src/common/hooks/useNeedsApproval.ts index ad6ce8d7345..1754de025cf 100644 --- a/apps/cowswap-frontend/src/common/hooks/useNeedsApproval.ts +++ b/apps/cowswap-frontend/src/common/hooks/useNeedsApproval.ts @@ -22,15 +22,16 @@ import { useTokenAllowance } from './useTokenAllowance' export function useNeedsApproval(amount: Nullish>, spender?: string): boolean { const tradeSpender = useTradeSpenderAddress() const token = amount ? getWrappedToken(amount.currency) : undefined - const allowance = useTokenAllowance(token, undefined, spender ?? tradeSpender) + const approvalSpender = spender ?? tradeSpender + const allowance = useTokenAllowance(token, undefined, approvalSpender) - if (typeof allowance === 'undefined') { - return true + if (!token || !amount || !approvalSpender) { + return false } - if (!token || !amount || !(spender ?? tradeSpender)) { - return false + if (allowance.data === undefined) { + return true } - return isEnoughAmount(amount, allowance?.data) === false + return isEnoughAmount(amount, allowance.data) === false } diff --git a/apps/cowswap-frontend/src/legacy/state/enhancedTransactions/hooks/TransactionHooksMod.tsx b/apps/cowswap-frontend/src/legacy/state/enhancedTransactions/hooks/TransactionHooksMod.tsx index 5e80444f6f5..e3a2423d294 100644 --- a/apps/cowswap-frontend/src/legacy/state/enhancedTransactions/hooks/TransactionHooksMod.tsx +++ b/apps/cowswap-frontend/src/legacy/state/enhancedTransactions/hooks/TransactionHooksMod.tsx @@ -19,14 +19,15 @@ export function useAllTransactions(): { [txHash: string]: EnhancedTransactionDet } // returns whether a token has a pending approval transaction -export function useHasPendingApproval(tokenAddress: string | undefined): boolean { +export function useHasPendingApproval(tokenAddress: string | undefined, approvalSpender?: string): boolean { const allTransactions = useAllTransactions() const spender = useTradeSpenderAddress() + const targetSpender = approvalSpender ?? spender return useMemo( () => typeof tokenAddress === 'string' && - typeof spender === 'string' && + typeof targetSpender === 'string' && Object.keys(allTransactions).some((hash) => { const tx = allTransactions[hash] if (!tx || tx.receipt || tx.replacementType || tx.errorMessage) return false @@ -34,8 +35,10 @@ export function useHasPendingApproval(tokenAddress: string | undefined): boolean const approval = tx.approval if (!approval) return false - return areAddressesEqual(approval.spender, spender) && areAddressesEqual(approval.tokenAddress, tokenAddress) + return ( + areAddressesEqual(approval.spender, targetSpender) && areAddressesEqual(approval.tokenAddress, tokenAddress) + ) }), - [allTransactions, spender, tokenAddress], + [allTransactions, targetSpender, tokenAddress], ) } diff --git a/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveState.ts b/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveState.ts index e5c4359417a..7c41f9f2379 100644 --- a/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveState.ts +++ b/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveState.ts @@ -24,7 +24,7 @@ export function useApproveState( const token = getCurrencyToApprove(amountToApprove) const tokenAddress = token?.address ? getAddressKey(token.address) : undefined const currentAllowance = useTokenAllowance(token, undefined, spender).data - const pendingApproval = useHasPendingApproval(tokenAddress) + const pendingApproval = useHasPendingApproval(tokenAddress, spender) const approvalStateBase = useSafeMemo(() => { return getApprovalState(amountToApprove, currentAllowance, pendingApproval) diff --git a/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useGeneratePermitInAdvanceToTrade.test.ts b/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useGeneratePermitInAdvanceToTrade.test.ts index 5735cb4b754..5c843aebbd8 100644 --- a/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useGeneratePermitInAdvanceToTrade.test.ts +++ b/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useGeneratePermitInAdvanceToTrade.test.ts @@ -1,6 +1,7 @@ import { useTradeSpenderAddress } from '@cowprotocol/balances-and-allowances' import { getWrappedToken } from '@cowprotocol/common-utils' import { CurrencyAmount, Token } from '@cowprotocol/currency' +import { DEFAULT_PERMIT_VALUE } from '@cowprotocol/permit-utils' import { useWalletInfo, WalletInfo } from '@cowprotocol/wallet' import { renderHook } from '@testing-library/react' @@ -157,6 +158,7 @@ describe('useGeneratePermitInAdvanceToTrade', () => { expect(mockCallOnBeforeApprovalWidgetHook).toHaveBeenCalledWith({ account: mockAccount, amountToApprove: mockAmountToApprove, + approvalAmount: BigInt(mockAmountToApprove.quotient.toString()), spenderAddress: mockSpenderAddress, }) expect(mockGeneratePermit).toHaveBeenCalledWith({ @@ -186,6 +188,28 @@ describe('useGeneratePermitInAdvanceToTrade', () => { expect(mockGeneratePermit).not.toHaveBeenCalled() }) + it('should report max approval amount to the widget hook for DAI-like permits', async () => { + mockUsePermitInfo.mockReturnValue({ type: 'dai-like' }) + mockGeneratePermit.mockResolvedValue({ signature: '0x123' }) + + const { result } = renderHook(() => useGeneratePermitInAdvanceToTrade(mockAmountToApprove)) + + await result.current() + + expect(mockCallOnBeforeApprovalWidgetHook).toHaveBeenCalledWith({ + account: mockAccount, + amountToApprove: mockAmountToApprove, + approvalAmount: DEFAULT_PERMIT_VALUE, + spenderAddress: mockSpenderAddress, + }) + expect(mockGeneratePermit).toHaveBeenCalledWith( + expect.objectContaining({ + amount: BigInt(mockAmountToApprove.quotient.toString()), + permitInfo: { type: 'dai-like' }, + }), + ) + }) + it('should return false when generatePermit returns null', async () => { mockGeneratePermit.mockResolvedValue(null) diff --git a/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useGeneratePermitInAdvanceToTrade.ts b/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useGeneratePermitInAdvanceToTrade.ts index 28d0e1fadc9..395f7dbfec0 100644 --- a/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useGeneratePermitInAdvanceToTrade.ts +++ b/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useGeneratePermitInAdvanceToTrade.ts @@ -3,6 +3,7 @@ import { useCallback } from 'react' import { useTradeSpenderAddress } from '@cowprotocol/balances-and-allowances' import { getWrappedToken, isRejectRequestProviderError } from '@cowprotocol/common-utils' import { Currency, CurrencyAmount } from '@cowprotocol/currency' +import { DEFAULT_PERMIT_VALUE } from '@cowprotocol/permit-utils' import { useWalletInfo } from '@cowprotocol/wallet' import { callOnBeforeApprovalWidgetHook } from 'modules/injectedWidget' @@ -24,10 +25,14 @@ export function useGeneratePermitInAdvanceToTrade(amountToApprove: CurrencyAmoun return useCallback(async () => { if (!account || !permitInfo || !tradeSpenderAddress) return false + const permitAmount = BigInt(amountToApprove.quotient.toString()) + const hookApprovalAmount = permitInfo.type === 'dai-like' ? DEFAULT_PERMIT_VALUE : permitAmount + const isWidgetHookPassed = await callOnBeforeApprovalWidgetHook({ account, amountToApprove, spenderAddress: tradeSpenderAddress, + approvalAmount: hookApprovalAmount, }) if (!isWidgetHookPassed) { @@ -46,7 +51,7 @@ export function useGeneratePermitInAdvanceToTrade(amountToApprove: CurrencyAmoun inputToken: { name: token.name || '', address: token.address as `0x${string}` }, account, permitInfo, - amount: BigInt(amountToApprove.quotient.toString()), + amount: permitAmount, customSpender: tradeSpenderAddress, preSignCallback, postSignCallback: resetApproveProgressModalState, diff --git a/apps/cowswap-frontend/src/modules/ethFlow/containers/EthFlow/index.tsx b/apps/cowswap-frontend/src/modules/ethFlow/containers/EthFlow/index.tsx index d853937460b..ed03d8d6899 100644 --- a/apps/cowswap-frontend/src/modules/ethFlow/containers/EthFlow/index.tsx +++ b/apps/cowswap-frontend/src/modules/ethFlow/containers/EthFlow/index.tsx @@ -25,6 +25,7 @@ import { useEthFlowActions } from './hooks/useEthFlowActions' import useRemainingNativeTxsAndCosts from './hooks/useRemainingNativeTxsAndCosts' import { useSetupEthFlow } from './hooks/useSetupEthFlow' import { getDerivedEthFlowState } from './utils/getDerivedEthFlowState' +import { getEthFlowCurrencyToApprove } from './utils/getEthFlowCurrencyToApprove' import { EthFlowModalContent } from '../../pure/EthFlowModalContent' import { WrappingPreviewProps } from '../../pure/WrappingPreview' @@ -61,7 +62,11 @@ export function EthFlowModal({ const { amountSetByUser } = usePartialApproveAmountModalState() || {} const updatePartialApproveAmountModalState = useUpdatePartialApproveAmountModalState() const isPartialApproveSelectedByUser = useIsPartialApproveSelectedByUser() - const currencyToApprove = isPartialApproveSelectedByUser ? (amountSetByUser ?? wrappedAmount) : undefined + const currencyToApprove = getEthFlowCurrencyToApprove({ + amountSetByUser, + isPartialApproveSelectedByUser, + wrappedAmount, + }) const approveCallback = useApproveCurrency(wrappedAmount ?? undefined, true) diff --git a/apps/cowswap-frontend/src/modules/ethFlow/containers/EthFlow/utils/getEthFlowCurrencyToApprove.test.ts b/apps/cowswap-frontend/src/modules/ethFlow/containers/EthFlow/utils/getEthFlowCurrencyToApprove.test.ts new file mode 100644 index 00000000000..49c0a1e5cfb --- /dev/null +++ b/apps/cowswap-frontend/src/modules/ethFlow/containers/EthFlow/utils/getEthFlowCurrencyToApprove.test.ts @@ -0,0 +1,52 @@ +import { CurrencyAmount, Token } from '@cowprotocol/currency' + +import { getEthFlowCurrencyToApprove } from './getEthFlowCurrencyToApprove' + +describe('getEthFlowCurrencyToApprove', () => { + const wrappedNative = new Token(1, '0x1111111111111111111111111111111111111111', 18, 'WETH', 'Wrapped Ether') + const otherToken = new Token(1, '0x2222222222222222222222222222222222222222', 18, 'TEST', 'Test Token') + + const wrappedAmount = CurrencyAmount.fromRawAmount(wrappedNative, '100') + const sameCurrencyAmountSetByUser = CurrencyAmount.fromRawAmount(wrappedNative, '25') + const staleAmountSetByUser = CurrencyAmount.fromRawAmount(otherToken, '25') + + it('returns undefined when partial approval is not selected', () => { + expect( + getEthFlowCurrencyToApprove({ + amountSetByUser: sameCurrencyAmountSetByUser, + isPartialApproveSelectedByUser: false, + wrappedAmount, + }), + ).toBeUndefined() + }) + + it('uses the current wrapped amount when the user has not set a custom amount', () => { + expect( + getEthFlowCurrencyToApprove({ + amountSetByUser: undefined, + isPartialApproveSelectedByUser: true, + wrappedAmount, + }), + ).toBe(wrappedAmount) + }) + + it('uses the user amount when it belongs to the current wrapped token', () => { + expect( + getEthFlowCurrencyToApprove({ + amountSetByUser: sameCurrencyAmountSetByUser, + isPartialApproveSelectedByUser: true, + wrappedAmount, + }), + ).toBe(sameCurrencyAmountSetByUser) + }) + + it('ignores a stale user amount from a different token', () => { + expect( + getEthFlowCurrencyToApprove({ + amountSetByUser: staleAmountSetByUser, + isPartialApproveSelectedByUser: true, + wrappedAmount, + }), + ).toBe(wrappedAmount) + }) +}) diff --git a/apps/cowswap-frontend/src/modules/ethFlow/containers/EthFlow/utils/getEthFlowCurrencyToApprove.ts b/apps/cowswap-frontend/src/modules/ethFlow/containers/EthFlow/utils/getEthFlowCurrencyToApprove.ts new file mode 100644 index 00000000000..647ec83251f --- /dev/null +++ b/apps/cowswap-frontend/src/modules/ethFlow/containers/EthFlow/utils/getEthFlowCurrencyToApprove.ts @@ -0,0 +1,19 @@ +import { Currency, CurrencyAmount } from '@cowprotocol/currency' + +interface GetEthFlowCurrencyToApproveParams { + amountSetByUser: CurrencyAmount | undefined + isPartialApproveSelectedByUser: boolean + wrappedAmount: CurrencyAmount | null +} + +export function getEthFlowCurrencyToApprove({ + amountSetByUser, + isPartialApproveSelectedByUser, + wrappedAmount, +}: GetEthFlowCurrencyToApproveParams): CurrencyAmount | undefined { + if (!isPartialApproveSelectedByUser) return undefined + if (!wrappedAmount) return undefined + if (!amountSetByUser) return wrappedAmount + + return amountSetByUser.currency.equals(wrappedAmount.currency) ? amountSetByUser : wrappedAmount +} diff --git a/apps/cowswap-frontend/src/modules/permit/hooks/usePermitInfo.test.ts b/apps/cowswap-frontend/src/modules/permit/hooks/usePermitInfo.test.ts index a167afe5b8a..c4d341ba15a 100644 --- a/apps/cowswap-frontend/src/modules/permit/hooks/usePermitInfo.test.ts +++ b/apps/cowswap-frontend/src/modules/permit/hooks/usePermitInfo.test.ts @@ -30,6 +30,7 @@ jest.mock('@cowprotocol/common-utils', () => ({ COW_PROTOCOL_VAULT_RELAYER_ADDRESS: { 1: defaultSpender }, getIsNativeToken: jest.fn().mockReturnValue(false), getWrappedToken: jest.fn((token) => token), + isAddress: jest.fn(), })) jest.mock('@cowprotocol/permit-utils', () => ({ @@ -74,6 +75,10 @@ const mockedUsePreGeneratedPermitInfoForToken = usePreGeneratedPermitInfoForToke typeof usePreGeneratedPermitInfoForToken > +const commonUtils = jest.requireMock('@cowprotocol/common-utils') as { + isAddress: jest.MockedFunction<(value: string | undefined | null) => string | false> +} + describe('usePermitInfo', () => { const token = new Token(1, '0x1234567890123456789012345678901234567890', 18, 'TEST', 'Test Token') const addPermitInfo = jest.fn() @@ -91,6 +96,11 @@ describe('usePermitInfo', () => { mockedUseIsPermitEnabled.mockReturnValue(true) mockedUsePreGeneratedPermitInfoForToken.mockImplementation(() => ({ permitInfo: undefined, isLoading: false })) mockedGetTokenPermitInfo.mockResolvedValue(fetchedPermitInfo) + commonUtils.isAddress.mockImplementation((value) => { + if (!value || !/^0x[a-fA-F0-9]{40}$/.test(value)) return false + + return value + }) }) it('does not reuse cached permit info that belongs to a different spender', async () => { @@ -155,4 +165,12 @@ describe('usePermitInfo', () => { expect(result.current).toEqual(defaultPermitInfo) expect(mockedGetTokenPermitInfo).not.toHaveBeenCalled() }) + + it('skips permit lookup for an invalid custom spender', () => { + const { result } = renderHook(() => usePermitInfo(token, TradeType.SWAP, '0x1')) + + expect(result.current).toBeUndefined() + expect(mockedUsePreGeneratedPermitInfoForToken).toHaveBeenCalledWith(undefined) + expect(mockedGetTokenPermitInfo).not.toHaveBeenCalled() + }) }) diff --git a/apps/cowswap-frontend/src/modules/permit/hooks/usePermitInfo.ts b/apps/cowswap-frontend/src/modules/permit/hooks/usePermitInfo.ts index f56f1f0cf92..9651fe7b6e1 100644 --- a/apps/cowswap-frontend/src/modules/permit/hooks/usePermitInfo.ts +++ b/apps/cowswap-frontend/src/modules/permit/hooks/usePermitInfo.ts @@ -1,7 +1,12 @@ import { useAtomValue, useSetAtom } from 'jotai' import { useEffect, useMemo } from 'react' -import { getIsNativeToken, getWrappedToken, COW_PROTOCOL_VAULT_RELAYER_ADDRESS } from '@cowprotocol/common-utils' +import { + getIsNativeToken, + getWrappedToken, + COW_PROTOCOL_VAULT_RELAYER_ADDRESS, + isAddress, +} from '@cowprotocol/common-utils' import { getAddressKey, mapSupportedNetworks, SupportedChainId } from '@cowprotocol/cow-sdk' import { Currency } from '@cowprotocol/currency' import { DEFAULT_MIN_GAS_LIMIT, getTokenPermitInfo, PermitInfo } from '@cowprotocol/permit-utils' @@ -68,8 +73,9 @@ export function usePermitInfo( const isPermitEnabled = useIsPermitEnabled() && isPermitSupported const defaultSpender = chainId ? COW_PROTOCOL_VAULT_RELAYER_ADDRESS[chainId] : undefined - const spender = customSpender || defaultSpender - const shouldUsePreGeneratedInfo = spender === defaultSpender + const customSpenderAddress = customSpender ? isAddress(customSpender) || undefined : undefined + const spender = customSpender ? customSpenderAddress : defaultSpender + const shouldUsePreGeneratedInfo = !customSpender && spender === defaultSpender const addPermitInfo = useAddPermitInfo() const permitInfo = usePermitInfoState(chainId, isPermitEnabled ? lowerCaseAddress : undefined, spender) diff --git a/apps/cowswap-frontend/src/modules/twap/hooks/useTwapOrderCreationContext.ts b/apps/cowswap-frontend/src/modules/twap/hooks/useTwapOrderCreationContext.ts index 97124834992..21441a5dc3a 100644 --- a/apps/cowswap-frontend/src/modules/twap/hooks/useTwapOrderCreationContext.ts +++ b/apps/cowswap-frontend/src/modules/twap/hooks/useTwapOrderCreationContext.ts @@ -43,6 +43,7 @@ export function useTwapOrderCreationContext( !erc20ContractData?.contract || !spender || !currentBlockFactoryAddress || + needsZeroApproval === undefined || composableCowChainId !== erc20ChainId ) return null diff --git a/apps/cowswap-frontend/src/modules/zeroApproval/hooks/useNeedsZeroApproval.ts b/apps/cowswap-frontend/src/modules/zeroApproval/hooks/useNeedsZeroApproval.ts index 7efbd9e5aed..6143059e02d 100644 --- a/apps/cowswap-frontend/src/modules/zeroApproval/hooks/useNeedsZeroApproval.ts +++ b/apps/cowswap-frontend/src/modules/zeroApproval/hooks/useNeedsZeroApproval.ts @@ -11,11 +11,15 @@ export function useNeedsZeroApproval( token: Nullish, spender: Nullish, sellAmount: Nullish>, -): boolean { +): boolean | undefined { const config = useConfig() - const [shouldZeroApprove, setShouldZeroApprove] = useState(false) + const [shouldZeroApprove, setShouldZeroApprove] = useState(undefined) useEffect(() => { + let isStale = false + + setShouldZeroApprove(undefined) + if (!token?.address || !spender || !sellAmount || !config) return shouldZeroApproveFn({ @@ -25,8 +29,14 @@ export function useNeedsZeroApproval( forceApprove: true, config, }).then((res) => { + if (isStale) return + setShouldZeroApprove(!!res) }) + + return () => { + isStale = true + } // eslint-disable-next-line react-hooks/exhaustive-deps }, [token?.address, sellAmount?.quotient?.toString(), spender, config]) diff --git a/libs/common-utils/src/getIsNativeToken.test.ts b/libs/common-utils/src/getIsNativeToken.test.ts new file mode 100644 index 00000000000..747fea31262 --- /dev/null +++ b/libs/common-utils/src/getIsNativeToken.test.ts @@ -0,0 +1,27 @@ +import { NATIVE_CURRENCIES } from '@cowprotocol/common-const' +import { SupportedChainId } from '@cowprotocol/cow-sdk' +import { Token } from '@cowprotocol/currency' + +import { getIsNativeToken } from './getIsNativeToken' + +describe('getIsNativeToken', () => { + it('does not treat an ERC20 token as native only because it uses the native symbol', () => { + const fakeEth = new Token( + SupportedChainId.MAINNET, + '0x1234567890123456789012345678901234567890', + 18, + 'ETH', + 'Fake ETH', + ) + + expect(getIsNativeToken(fakeEth)).toBe(false) + }) + + it('treats the canonical native token object as native', () => { + expect(getIsNativeToken(NATIVE_CURRENCIES[SupportedChainId.MAINNET])).toBe(true) + }) + + it('keeps symbol matching for route token ids', () => { + expect(getIsNativeToken(SupportedChainId.MAINNET, 'ETH')).toBe(true) + }) +}) diff --git a/libs/common-utils/src/getIsNativeToken.ts b/libs/common-utils/src/getIsNativeToken.ts index 101d9067c52..926e5ee09eb 100644 --- a/libs/common-utils/src/getIsNativeToken.ts +++ b/libs/common-utils/src/getIsNativeToken.ts @@ -1,5 +1,5 @@ import { NATIVE_CURRENCIES, TokenWithLogo } from '@cowprotocol/common-const' -import { SupportedChainId } from '@cowprotocol/cow-sdk' +import { areAddressesEqual, SupportedChainId } from '@cowprotocol/cow-sdk' import { Currency, NativeCurrency } from '@cowprotocol/currency' import { doesTokenMatchSymbolOrAddress } from './doesTokenMatchSymbolOrAddress' @@ -29,5 +29,5 @@ export function getIsNativeToken(chainIdOrTokenParams: SupportedChainId | Curren // When token is from Bridge, it's not in the list of native tokens if (!nativeToken) return false - return doesTokenMatchSymbolOrAddress(nativeToken, tokenId) + return areAddressesEqual(nativeToken.address, tokenId) } diff --git a/libs/permit-utils/src/index.ts b/libs/permit-utils/src/index.ts index 3884d589e1c..356257d475a 100644 --- a/libs/permit-utils/src/index.ts +++ b/libs/permit-utils/src/index.ts @@ -1,4 +1,4 @@ -export { DEFAULT_MIN_GAS_LIMIT, PERMIT_ACCOUNT } from './const' +export { DEFAULT_MIN_GAS_LIMIT, DEFAULT_PERMIT_VALUE, PERMIT_ACCOUNT } from './const' export { checkIsCallDataAValidPermit } from './lib/checkIsCallDataAValidPermit' export { generatePermitHook } from './lib/generatePermitHook' diff --git a/libs/permit-utils/src/lib/generatePermitHook.test.ts b/libs/permit-utils/src/lib/generatePermitHook.test.ts index 0b3f4c4c3cc..9b2c9cb8ce8 100644 --- a/libs/permit-utils/src/lib/generatePermitHook.test.ts +++ b/libs/permit-utils/src/lib/generatePermitHook.test.ts @@ -81,4 +81,18 @@ describe('generatePermitHook request cache', () => { expect(mockBuildEip2612PermitCallData).toHaveBeenCalledTimes(2) }) + + it('preserves an explicit zero amount in permit calldata', async () => { + await generatePermitHook({ ...createParams(), amount: 0n }) + + expect(mockBuildEip2612PermitCallData).toHaveBeenCalledWith( + expect.objectContaining({ + callDataParams: expect.arrayContaining([ + expect.objectContaining({ + value: '0', + }), + ]), + }), + ) + }) }) diff --git a/libs/permit-utils/src/lib/generatePermitHook.ts b/libs/permit-utils/src/lib/generatePermitHook.ts index dd61c62abda..b0f8ebb345d 100644 --- a/libs/permit-utils/src/lib/generatePermitHook.ts +++ b/libs/permit-utils/src/lib/generatePermitHook.ts @@ -100,7 +100,7 @@ async function generatePermitHookRaw(params: PermitHookParams): Promise