From a641192693556bd3f9f88415ad3561e64595c780 Mon Sep 17 00:00:00 2001 From: Alexandr Kazachenko Date: Thu, 16 Oct 2025 13:07:54 +0500 Subject: [PATCH 01/17] feat(approve): adjust partial approves for buy orders --- .../src/common/hooks/useTokenAllowance.ts | 8 +-- .../TradeApproveToggle/TradeApproveToggle.tsx | 14 +++-- .../TradeApproveWithAffectedOrderList.tsx | 10 ++-- .../useIsApprovalOrPermitRequired.test.ts | 54 +++++++++---------- .../hooks/useIsApprovalOrPermitRequired.ts | 32 ++++++----- .../pure/TradeAllowanceDisplay/index.tsx | 24 +++++++++ .../pure/TradeAllowanceDisplay/styled.ts | 30 +++++++++++ .../trade/hooks/useAmountsToSignFromQuote.ts | 17 ++++-- .../hooks/useTradeFormValidationContext.ts | 2 +- libs/common-utils/src/fractionUtils.ts | 6 ++- 10 files changed, 137 insertions(+), 60 deletions(-) create mode 100644 apps/cowswap-frontend/src/modules/erc20Approve/pure/TradeAllowanceDisplay/index.tsx create mode 100644 apps/cowswap-frontend/src/modules/erc20Approve/pure/TradeAllowanceDisplay/styled.ts diff --git a/apps/cowswap-frontend/src/common/hooks/useTokenAllowance.ts b/apps/cowswap-frontend/src/common/hooks/useTokenAllowance.ts index a781ad032c..c544b16665 100644 --- a/apps/cowswap-frontend/src/common/hooks/useTokenAllowance.ts +++ b/apps/cowswap-frontend/src/common/hooks/useTokenAllowance.ts @@ -11,7 +11,6 @@ import useSWR, { SWRConfiguration, SWRResponse } from 'swr' import { useTokenContract } from 'common/hooks/useContract' - interface LastApproveParams { blockNumber: number tokenAddress: string @@ -46,11 +45,14 @@ export function useTokenAllowance( setLastApproveTx({}) }, [chainId, setLastApproveTx]) + // Important! Do not add erc20Contract to SWR deps, otherwise it will do unwanted node RPC calls! return useSWR( erc20Contract && targetOwner && targetSpender - ? [erc20Contract, targetOwner, targetSpender, chainId, lastApproveBlockNumber, 'useTokenAllowance'] + ? [targetOwner, targetSpender, chainId, lastApproveBlockNumber, tokenAddress, 'useTokenAllowance'] : null, - ([erc20Contract, targetOwner, targetSpender]) => { + ([targetOwner, targetSpender]) => { + if (!erc20Contract) return undefined + return erc20Contract.allowance(targetOwner, targetSpender).then((result) => result.toBigInt()) }, SWR_OPTIONS, diff --git a/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveToggle/TradeApproveToggle.tsx b/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveToggle/TradeApproveToggle.tsx index c1796fac0b..cf4b0bd1c9 100644 --- a/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveToggle/TradeApproveToggle.tsx +++ b/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveToggle/TradeApproveToggle.tsx @@ -15,13 +15,11 @@ export function TradeApproveToggle({ amountToApprove, updateModalState }: TradeA const setIsPartialApproveSelectedByUser = useSetIsPartialApproveSelectedByUser() return ( - <> - - + ) } diff --git a/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveWithAffectedOrderList/TradeApproveWithAffectedOrderList.tsx b/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveWithAffectedOrderList/TradeApproveWithAffectedOrderList.tsx index ed70966c9c..9ea40084e5 100644 --- a/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveWithAffectedOrderList/TradeApproveWithAffectedOrderList.tsx +++ b/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveWithAffectedOrderList/TradeApproveWithAffectedOrderList.tsx @@ -6,13 +6,14 @@ import { useGetPartialAmountToSignApprove, useIsApprovalOrPermitRequired, } from '../../hooks' +import { TradeAllowanceDisplay } from '../../pure/TradeAllowanceDisplay' import { useSetUserApproveAmountModalState } from '../../state' import { isMaxAmountToApprove } from '../../utils' import { ActiveOrdersWithAffectedPermit } from '../ActiveOrdersWithAffectedPermit' import { TradeApproveToggle } from '../TradeApproveToggle' export function TradeApproveWithAffectedOrderList(): ReactNode { - const isApproveRequired = useIsApprovalOrPermitRequired() + const { reason: isApproveRequired, currentAllowance } = useIsApprovalOrPermitRequired() const setUserApproveAmountModalState = useSetUserApproveAmountModalState() @@ -26,6 +27,8 @@ export function TradeApproveWithAffectedOrderList(): ReactNode { isApproveRequired === ApproveRequiredReason.Required || isApproveRequired === ApproveRequiredReason.Eip2612PermitRequired + const currencyToApprove = partialAmountToApprove?.currency + return ( <> {partialAmountToApprove && isApproveOrPartialPermitRequired && ( @@ -34,9 +37,10 @@ export function TradeApproveWithAffectedOrderList(): ReactNode { amountToApprove={partialAmountToApprove} /> )} - {showAffectedOrders && partialAmountToApprove && ( - + {typeof currentAllowance === 'bigint' && currencyToApprove && ( + )} + {showAffectedOrders && currencyToApprove && } ) } diff --git a/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useIsApprovalOrPermitRequired.test.ts b/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useIsApprovalOrPermitRequired.test.ts index db4d2f8559..5d261db0c0 100644 --- a/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useIsApprovalOrPermitRequired.test.ts +++ b/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useIsApprovalOrPermitRequired.test.ts @@ -87,7 +87,7 @@ describe('useIsApprovalOrPermitRequired', () => { currentAllowance: BigInt(0), }) - const { result } = renderHook(() => useIsApprovalOrPermitRequired()) + const { result } = renderHook(() => useIsApprovalOrPermitRequired().reason) expect(result.current).toBe(ApproveRequiredReason.Required) }) @@ -98,7 +98,7 @@ describe('useIsApprovalOrPermitRequired', () => { currentAllowance: BigInt(0), }) - const { result } = renderHook(() => useIsApprovalOrPermitRequired()) + const { result } = renderHook(() => useIsApprovalOrPermitRequired().reason) expect(result.current).toBe(ApproveRequiredReason.Required) }) @@ -109,7 +109,7 @@ describe('useIsApprovalOrPermitRequired', () => { currentAllowance: BigInt(1000000000000000000), }) - const { result } = renderHook(() => useIsApprovalOrPermitRequired()) + const { result } = renderHook(() => useIsApprovalOrPermitRequired().reason) expect(result.current).toBe(ApproveRequiredReason.NotRequired) }) @@ -123,7 +123,7 @@ describe('useIsApprovalOrPermitRequired', () => { }), ) - const { result } = renderHook(() => useIsApprovalOrPermitRequired()) + const { result } = renderHook(() => useIsApprovalOrPermitRequired().reason) expect(result.current).toBe(ApproveRequiredReason.NotRequired) }) @@ -135,7 +135,7 @@ describe('useIsApprovalOrPermitRequired', () => { }), ) - const { result } = renderHook(() => useIsApprovalOrPermitRequired()) + const { result } = renderHook(() => useIsApprovalOrPermitRequired().reason) expect(result.current).toBe(ApproveRequiredReason.NotRequired) }) @@ -147,7 +147,7 @@ describe('useIsApprovalOrPermitRequired', () => { }), ) - const { result } = renderHook(() => useIsApprovalOrPermitRequired()) + const { result } = renderHook(() => useIsApprovalOrPermitRequired().reason) expect(result.current).toBe(ApproveRequiredReason.NotRequired) }) @@ -157,7 +157,7 @@ describe('useIsApprovalOrPermitRequired', () => { it('should return NotRequired when isPartialApproveEnabled is false', () => { mockUseFeatureFlags.mockReturnValue({ isPartialApproveEnabled: false }) - const { result } = renderHook(() => useIsApprovalOrPermitRequired()) + const { result } = renderHook(() => useIsApprovalOrPermitRequired().reason) expect(result.current).toBe(ApproveRequiredReason.NotRequired) }) @@ -167,7 +167,7 @@ describe('useIsApprovalOrPermitRequired', () => { it('should return Eip2612PermitRequired for eip-2612 permit type', () => { mockUsePermitInfo.mockReturnValue({ type: 'eip-2612' }) - const { result } = renderHook(() => useIsApprovalOrPermitRequired()) + const { result } = renderHook(() => useIsApprovalOrPermitRequired().reason) expect(result.current).toBe(ApproveRequiredReason.Eip2612PermitRequired) }) @@ -175,7 +175,7 @@ describe('useIsApprovalOrPermitRequired', () => { it('should return DaiLikePermitRequired for dai-like permit type', () => { mockUsePermitInfo.mockReturnValue({ type: 'dai-like' }) - const { result } = renderHook(() => useIsApprovalOrPermitRequired()) + const { result } = renderHook(() => useIsApprovalOrPermitRequired().reason) expect(result.current).toBe(ApproveRequiredReason.DaiLikePermitRequired) }) @@ -183,7 +183,7 @@ describe('useIsApprovalOrPermitRequired', () => { it('should return NotRequired for unsupported permit type', () => { mockUsePermitInfo.mockReturnValue({ type: 'unsupported' }) - const { result } = renderHook(() => useIsApprovalOrPermitRequired()) + const { result } = renderHook(() => useIsApprovalOrPermitRequired().reason) expect(result.current).toBe(ApproveRequiredReason.NotRequired) }) @@ -193,7 +193,7 @@ describe('useIsApprovalOrPermitRequired', () => { it('should handle undefined trade state', () => { mockUseDerivedTradeState.mockReturnValue(null) - const { result } = renderHook(() => useIsApprovalOrPermitRequired()) + const { result } = renderHook(() => useIsApprovalOrPermitRequired().reason) expect(result.current).toBe(ApproveRequiredReason.NotRequired) }) @@ -201,7 +201,7 @@ describe('useIsApprovalOrPermitRequired', () => { it('should handle undefined permit info', () => { mockUsePermitInfo.mockReturnValue(undefined) - const { result } = renderHook(() => useIsApprovalOrPermitRequired()) + const { result } = renderHook(() => useIsApprovalOrPermitRequired().reason) expect(result.current).toBe(ApproveRequiredReason.NotRequired) }) @@ -213,7 +213,7 @@ describe('useIsApprovalOrPermitRequired', () => { currentAllowance: BigInt(0), }) - const { result } = renderHook(() => useIsApprovalOrPermitRequired()) + const { result } = renderHook(() => useIsApprovalOrPermitRequired().reason) expect(result.current).toBe(ApproveRequiredReason.Required) }) @@ -225,7 +225,7 @@ describe('useIsApprovalOrPermitRequired', () => { currentAllowance: BigInt(0), }) - const { result } = renderHook(() => useIsApprovalOrPermitRequired()) + const { result } = renderHook(() => useIsApprovalOrPermitRequired().reason) expect(result.current).toBe(ApproveRequiredReason.NotRequired) }) @@ -237,7 +237,7 @@ describe('useIsApprovalOrPermitRequired', () => { }), ) - const { result } = renderHook(() => useIsApprovalOrPermitRequired()) + const { result } = renderHook(() => useIsApprovalOrPermitRequired().reason) expect(result.current).toBe(ApproveRequiredReason.NotRequired) }) @@ -249,7 +249,7 @@ describe('useIsApprovalOrPermitRequired', () => { }), ) - const { result } = renderHook(() => useIsApprovalOrPermitRequired()) + const { result } = renderHook(() => useIsApprovalOrPermitRequired().reason) expect(result.current).toBe(ApproveRequiredReason.NotRequired) }) @@ -263,7 +263,7 @@ describe('useIsApprovalOrPermitRequired', () => { }) mockUsePermitInfo.mockReturnValue({ type: 'eip-2612' }) - const { result } = renderHook(() => useIsApprovalOrPermitRequired()) + const { result } = renderHook(() => useIsApprovalOrPermitRequired().reason) expect(result.current).toBe(ApproveRequiredReason.Eip2612PermitRequired) }) @@ -275,7 +275,7 @@ describe('useIsApprovalOrPermitRequired', () => { }) mockUsePermitInfo.mockReturnValue({ type: 'unsupported' }) - const { result } = renderHook(() => useIsApprovalOrPermitRequired()) + const { result } = renderHook(() => useIsApprovalOrPermitRequired().reason) expect(result.current).toBe(ApproveRequiredReason.Required) }) @@ -287,7 +287,7 @@ describe('useIsApprovalOrPermitRequired', () => { }) mockUsePermitInfo.mockReturnValue({ type: 'unsupported' }) - const { result } = renderHook(() => useIsApprovalOrPermitRequired()) + const { result } = renderHook(() => useIsApprovalOrPermitRequired().reason) expect(result.current).toBe(ApproveRequiredReason.NotRequired) }) @@ -296,19 +296,19 @@ describe('useIsApprovalOrPermitRequired', () => { describe('getPermitRequirements function', () => { it('should return correct permit requirements for different permit types', () => { mockUsePermitInfo.mockReturnValue({ type: 'eip-2612' }) - const { result: result1 } = renderHook(() => useIsApprovalOrPermitRequired()) + const { result: result1 } = renderHook(() => useIsApprovalOrPermitRequired().reason) expect(result1.current).toBe(ApproveRequiredReason.Eip2612PermitRequired) mockUsePermitInfo.mockReturnValue({ type: 'dai-like' }) - const { result: result2 } = renderHook(() => useIsApprovalOrPermitRequired()) + const { result: result2 } = renderHook(() => useIsApprovalOrPermitRequired().reason) expect(result2.current).toBe(ApproveRequiredReason.DaiLikePermitRequired) mockUsePermitInfo.mockReturnValue({ type: 'unsupported' }) - const { result: result3 } = renderHook(() => useIsApprovalOrPermitRequired()) + const { result: result3 } = renderHook(() => useIsApprovalOrPermitRequired().reason) expect(result3.current).toBe(ApproveRequiredReason.NotRequired) mockUsePermitInfo.mockReturnValue(undefined) - const { result: result4 } = renderHook(() => useIsApprovalOrPermitRequired()) + const { result: result4 } = renderHook(() => useIsApprovalOrPermitRequired().reason) expect(result4.current).toBe(ApproveRequiredReason.NotRequired) }) }) @@ -324,7 +324,7 @@ describe('useIsApprovalOrPermitRequired', () => { ) mockUsePermitInfo.mockReturnValue({ type: 'eip-2612' }) - const { result } = renderHook(() => useIsApprovalOrPermitRequired()) + const { result } = renderHook(() => useIsApprovalOrPermitRequired().reason) expect(result.current).toBe(ApproveRequiredReason.Eip2612PermitRequired) }) @@ -339,7 +339,7 @@ describe('useIsApprovalOrPermitRequired', () => { ) mockUsePermitInfo.mockReturnValue({ type: 'eip-2612' }) - const { result } = renderHook(() => useIsApprovalOrPermitRequired()) + const { result } = renderHook(() => useIsApprovalOrPermitRequired().reason) expect(result.current).toBe(ApproveRequiredReason.NotRequired) }) @@ -354,7 +354,7 @@ describe('useIsApprovalOrPermitRequired', () => { ) mockUsePermitInfo.mockReturnValue({ type: 'eip-2612' }) - const { result } = renderHook(() => useIsApprovalOrPermitRequired()) + const { result } = renderHook(() => useIsApprovalOrPermitRequired().reason) expect(result.current).toBe(ApproveRequiredReason.NotRequired) }) @@ -366,7 +366,7 @@ describe('useIsApprovalOrPermitRequired', () => { }) mockUsePermitInfo.mockReturnValue({ type: 'unsupported' }) - const { result } = renderHook(() => useIsApprovalOrPermitRequired()) + const { result } = renderHook(() => useIsApprovalOrPermitRequired().reason) expect(result.current).toBe(ApproveRequiredReason.NotRequired) }) diff --git a/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useIsApprovalOrPermitRequired.ts b/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useIsApprovalOrPermitRequired.ts index 659b131653..c5b8937344 100644 --- a/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useIsApprovalOrPermitRequired.ts +++ b/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useIsApprovalOrPermitRequired.ts @@ -1,3 +1,5 @@ +import { useMemo } from 'react' + import { useFeatureFlags } from '@cowprotocol/common-hooks' import { PermitType } from '@cowprotocol/permit-utils' import { Nullish } from '@cowprotocol/types' @@ -17,28 +19,32 @@ export enum ApproveRequiredReason { DaiLikePermitRequired, } -export function useIsApprovalOrPermitRequired(): ApproveRequiredReason { +export function useIsApprovalOrPermitRequired(): { reason: ApproveRequiredReason; currentAllowance: Nullish } { const amountToApprove = useGetAmountToSignApprove() const { isPartialApproveEnabled } = useFeatureFlags() - const { state: approvalState } = useApproveState(amountToApprove) + const { state: approvalState, currentAllowance } = useApproveState(amountToApprove) const { inputCurrency, tradeType } = useDerivedTradeState() || {} const { type } = usePermitInfo(inputCurrency, tradeType) || {} - if (amountToApprove?.equalTo('0')) { - return ApproveRequiredReason.NotRequired - } + const reason = (() => { + if (amountToApprove?.equalTo('0')) { + return ApproveRequiredReason.NotRequired + } - const isPermitSupported = type !== 'unsupported' + const isPermitSupported = type !== 'unsupported' - if (!isPermitSupported && isApprovalRequired(approvalState)) { - return ApproveRequiredReason.Required - } + if (!isPermitSupported && isApprovalRequired(approvalState)) { + return ApproveRequiredReason.Required + } - if (!isNewApproveFlowEnabled(tradeType, isPartialApproveEnabled)) { - return ApproveRequiredReason.NotRequired - } + if (!isNewApproveFlowEnabled(tradeType, isPartialApproveEnabled)) { + return ApproveRequiredReason.NotRequired + } + + return getPermitRequirements(type) + })() - return getPermitRequirements(type) + return useMemo(() => ({ reason, currentAllowance }), [reason, currentAllowance]) } function isApprovalRequired(approvalState: ApprovalState): boolean { diff --git a/apps/cowswap-frontend/src/modules/erc20Approve/pure/TradeAllowanceDisplay/index.tsx b/apps/cowswap-frontend/src/modules/erc20Approve/pure/TradeAllowanceDisplay/index.tsx new file mode 100644 index 0000000000..9dfa7eb5fb --- /dev/null +++ b/apps/cowswap-frontend/src/modules/erc20Approve/pure/TradeAllowanceDisplay/index.tsx @@ -0,0 +1,24 @@ +import { ReactNode } from 'react' + +import { TokenAmount } from '@cowprotocol/ui' +import { Currency, CurrencyAmount } from '@uniswap/sdk-core' + +import * as styledEl from './styled' + +interface TradeAllowanceDisplayProps { + currencyToApprove: Currency + currentAllowance: bigint +} + +export function TradeAllowanceDisplay({ currentAllowance, currencyToApprove }: TradeAllowanceDisplayProps): ReactNode { + const allowanceAmount = CurrencyAmount.fromRawAmount(currencyToApprove, currentAllowance.toString()) + + return ( + + Current allowance + + + + + ) +} diff --git a/apps/cowswap-frontend/src/modules/erc20Approve/pure/TradeAllowanceDisplay/styled.ts b/apps/cowswap-frontend/src/modules/erc20Approve/pure/TradeAllowanceDisplay/styled.ts new file mode 100644 index 0000000000..7fe8a54894 --- /dev/null +++ b/apps/cowswap-frontend/src/modules/erc20Approve/pure/TradeAllowanceDisplay/styled.ts @@ -0,0 +1,30 @@ +import { UI } from '@cowprotocol/ui' + +import styled from 'styled-components/macro' + +export const AllowanceWrapper = styled.div` + display: flex; + flex-direction: column; + gap: 6px; + padding: 12px 14px; + border-radius: 12px; + background: var(${UI.COLOR_BACKGROUND}); + color: var(${UI.COLOR_TEXT_PAPER}); + font-size: 13px; +` + +export const AllowanceLabel = styled.div` + font-size: 12px; + font-weight: 500; + color: var(${UI.COLOR_TEXT_OPACITY_70}); + letter-spacing: 0.03em; +` + +export const AllowanceAmount = styled.div` + font-size: 15px; + font-weight: 600; + color: var(${UI.COLOR_TEXT}); + display: flex; + align-items: center; + gap: 4px; +` diff --git a/apps/cowswap-frontend/src/modules/trade/hooks/useAmountsToSignFromQuote.ts b/apps/cowswap-frontend/src/modules/trade/hooks/useAmountsToSignFromQuote.ts index da2066c442..6f6c146fb7 100644 --- a/apps/cowswap-frontend/src/modules/trade/hooks/useAmountsToSignFromQuote.ts +++ b/apps/cowswap-frontend/src/modules/trade/hooks/useAmountsToSignFromQuote.ts @@ -1,10 +1,13 @@ import { useMemo } from 'react' -import { Currency, CurrencyAmount } from '@uniswap/sdk-core' +import { FractionUtils } from '@cowprotocol/common-utils' +import { Currency, CurrencyAmount, Percent } from '@uniswap/sdk-core' import { useDerivedTradeState } from './useDerivedTradeState' import { useGetReceiveAmountInfo } from './useGetReceiveAmountInfo' +const BUY_ORDER_APPROVE_AMOUNT_THRESHOLD = new Percent(1, 100) // 1% + export interface AmountsToSign { maximumSendSellAmount: CurrencyAmount minimumReceiveBuyAmount: CurrencyAmount @@ -16,7 +19,7 @@ export interface AmountsToSign { */ export function useAmountsToSignFromQuote(): AmountsToSign | null { const { isQuoteBasedOrder, inputCurrencyAmount, outputCurrencyAmount } = useDerivedTradeState() || {} - const { afterSlippage } = useGetReceiveAmountInfo() || {} + const { isSell, afterSlippage } = useGetReceiveAmountInfo() || {} return useMemo(() => { const maximumSendSellAmount = isQuoteBasedOrder ? afterSlippage?.sellAmount : inputCurrencyAmount @@ -24,6 +27,12 @@ export function useAmountsToSignFromQuote(): AmountsToSign | null { if (!maximumSendSellAmount || !minimumReceiveBuyAmount) return null - return { maximumSendSellAmount, minimumReceiveBuyAmount } - }, [isQuoteBasedOrder, inputCurrencyAmount, outputCurrencyAmount, afterSlippage]) + return { + // Add 1% threshold for buy orders to level out price/gas fluctuations + maximumSendSellAmount: isSell + ? maximumSendSellAmount + : FractionUtils.addPercent(maximumSendSellAmount, BUY_ORDER_APPROVE_AMOUNT_THRESHOLD), + minimumReceiveBuyAmount, + } + }, [isSell, isQuoteBasedOrder, inputCurrencyAmount, outputCurrencyAmount, afterSlippage]) } diff --git a/apps/cowswap-frontend/src/modules/tradeFormValidation/hooks/useTradeFormValidationContext.ts b/apps/cowswap-frontend/src/modules/tradeFormValidation/hooks/useTradeFormValidationContext.ts index 26ea8e8634..c99f8682f4 100644 --- a/apps/cowswap-frontend/src/modules/tradeFormValidation/hooks/useTradeFormValidationContext.ts +++ b/apps/cowswap-frontend/src/modules/tradeFormValidation/hooks/useTradeFormValidationContext.ts @@ -41,7 +41,7 @@ export function useTradeFormValidationContext(): TradeFormValidationCommonContex const isSafeReadonlyUser = gnosisSafeInfo?.isReadOnly === true - const isApproveRequired = useIsApprovalOrPermitRequired() + const isApproveRequired = useIsApprovalOrPermitRequired().reason const isInsufficientBalanceOrderAllowed = tradeType === TradeType.LIMIT_ORDER diff --git a/libs/common-utils/src/fractionUtils.ts b/libs/common-utils/src/fractionUtils.ts index c29bbe0568..d68f895408 100644 --- a/libs/common-utils/src/fractionUtils.ts +++ b/libs/common-utils/src/fractionUtils.ts @@ -1,6 +1,6 @@ import { FULL_PRICE_PRECISION } from '@cowprotocol/common-const' import { Nullish } from '@cowprotocol/types' -import { BigintIsh, Currency, CurrencyAmount, Fraction, Price, Rounding, Token } from '@uniswap/sdk-core' +import { BigintIsh, Currency, CurrencyAmount, Fraction, Percent, Price, Rounding, Token } from '@uniswap/sdk-core' import { BigNumber } from 'bignumber.js' import JSBI from 'jsbi' @@ -179,6 +179,10 @@ export class FractionUtils { static simplify(fraction: Fraction): Fraction { return reduce(trimZeros(fraction)) } + + static addPercent(amount: CurrencyAmount, percent: Percent): typeof amount { + return amount.add(amount.multiply(percent)) + } } const ZERO = JSBI.BigInt(0) From 933423f9450ce331adab2123ad15c0c2d91cbf85 Mon Sep 17 00:00:00 2001 From: Alexandr Kazachenko Date: Thu, 16 Oct 2025 13:32:52 +0500 Subject: [PATCH 02/17] fix: handle failed approve tx --- .../src/common/hooks/useTokenAllowance.ts | 13 +++++++++---- .../TradeApproveModal/useTradeApproveCallback.ts | 6 +++++- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/apps/cowswap-frontend/src/common/hooks/useTokenAllowance.ts b/apps/cowswap-frontend/src/common/hooks/useTokenAllowance.ts index c544b16665..b95fbbbd5e 100644 --- a/apps/cowswap-frontend/src/common/hooks/useTokenAllowance.ts +++ b/apps/cowswap-frontend/src/common/hooks/useTokenAllowance.ts @@ -18,6 +18,8 @@ interface LastApproveParams { const lastApproveTxBlockNumberAtom = atom>({}) +const LAST_APPROVE_TX_TIME_THRESHOLD = ms`1s` + const SWR_OPTIONS: SWRConfiguration = { ...SWR_NO_REFRESH_OPTIONS, revalidateIfStale: false, @@ -64,10 +66,13 @@ export function useUpdateLastApproveTxBlockNumber(): (params: LastApproveParams) return useCallback( (params: LastApproveParams) => { - setState((state) => ({ - ...state, - [params.tokenAddress.toLowerCase()]: params.blockNumber, - })) + // Wait 1 second before updating value to give some time to node to update the state + setTimeout(() => { + setState((state) => ({ + ...state, + [params.tokenAddress.toLowerCase()]: params.blockNumber, + })) + }, LAST_APPROVE_TX_TIME_THRESHOLD) }, [setState], ) diff --git a/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.ts b/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.ts index 5e02ef4984..b0b658a52a 100644 --- a/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.ts +++ b/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.ts @@ -54,7 +54,11 @@ export function useTradeApproveCallback(currency: Currency | undefined): TradeAp approvalAnalytics('Sign', symbol) // if ff is disabled - use old flow, hide modal when tx is sent !isPartialApproveEnabled && updateTradeApproveState({ currency: undefined, approveInProgress: false }) - return response?.wait() + return response?.wait().then((txResponse) => { + if (txResponse.status !== 1) { + throw new Error('Approval transaction failed') + } + }) }) .finally(() => { updateTradeApproveState({ currency: undefined, approveInProgress: false }) From e9f8d1f3274da27a3b11d9a6ae9a812a0ff1d3f7 Mon Sep 17 00:00:00 2001 From: Alexandr Kazachenko Date: Thu, 16 Oct 2025 13:35:01 +0500 Subject: [PATCH 03/17] chore: fix TradeAllowanceDisplay conditions --- .../TradeApproveWithAffectedOrderList.tsx | 22 +++++++++++-------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveWithAffectedOrderList/TradeApproveWithAffectedOrderList.tsx b/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveWithAffectedOrderList/TradeApproveWithAffectedOrderList.tsx index 9ea40084e5..213161281b 100644 --- a/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveWithAffectedOrderList/TradeApproveWithAffectedOrderList.tsx +++ b/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveWithAffectedOrderList/TradeApproveWithAffectedOrderList.tsx @@ -27,18 +27,22 @@ export function TradeApproveWithAffectedOrderList(): ReactNode { isApproveRequired === ApproveRequiredReason.Required || isApproveRequired === ApproveRequiredReason.Eip2612PermitRequired - const currencyToApprove = partialAmountToApprove?.currency + if (!partialAmountToApprove) return null + + const currencyToApprove = partialAmountToApprove.currency return ( <> - {partialAmountToApprove && isApproveOrPartialPermitRequired && ( - setUserApproveAmountModalState({ isModalOpen: true })} - amountToApprove={partialAmountToApprove} - /> - )} - {typeof currentAllowance === 'bigint' && currencyToApprove && ( - + {isApproveOrPartialPermitRequired && ( + <> + setUserApproveAmountModalState({ isModalOpen: true })} + amountToApprove={partialAmountToApprove} + /> + {typeof currentAllowance === 'bigint' && currencyToApprove && ( + + )} + )} {showAffectedOrders && currencyToApprove && } From 2f54b5a94d6a51aa63f209ea1b54cf049564d65c Mon Sep 17 00:00:00 2001 From: Alexandr Kazachenko Date: Thu, 16 Oct 2025 13:43:41 +0500 Subject: [PATCH 04/17] test: add tests for useTradeApproveCallback --- .../useTradeApproveCallback.test.ts | 416 ++++++++++++++++++ .../useTradeApproveCallback.ts | 2 + 2 files changed, 418 insertions(+) create mode 100644 apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.test.ts diff --git a/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.test.ts b/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.test.ts new file mode 100644 index 0000000000..987c5688ae --- /dev/null +++ b/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.test.ts @@ -0,0 +1,416 @@ +import { useCowAnalytics } from '@cowprotocol/analytics' +import { useTradeSpenderAddress } from '@cowprotocol/balances-and-allowances' +import { useFeatureFlags } from '@cowprotocol/common-hooks' +import { TransactionReceipt, TransactionResponse } from '@ethersproject/abstract-provider' +import { Token } from '@uniswap/sdk-core' + +import { renderHook, waitFor } from '@testing-library/react' + +import { CowSwapAnalyticsCategory } from 'common/analytics/types' + +import { useTradeApproveCallback } from './useTradeApproveCallback' + +import { useApproveCallback } from '../../hooks' +import { useUpdateTradeApproveState } from '../../state' + +jest.mock('@cowprotocol/analytics', () => ({ + useCowAnalytics: jest.fn(), + __resetGtmInstance: jest.fn(), +})) + +jest.mock('@cowprotocol/balances-and-allowances', () => ({ + useTradeSpenderAddress: jest.fn(), +})) + +jest.mock('@cowprotocol/common-hooks', () => ({ + useFeatureFlags: jest.fn(), +})) + +jest.mock('../../hooks', () => ({ + useApproveCallback: jest.fn(), +})) + +jest.mock('../../state', () => ({ + useUpdateTradeApproveState: jest.fn(), +})) + +const mockUseCowAnalytics = useCowAnalytics as jest.MockedFunction +const mockUseTradeSpenderAddress = useTradeSpenderAddress as jest.MockedFunction +const mockUseFeatureFlags = useFeatureFlags as jest.MockedFunction +const mockUseApproveCallback = useApproveCallback as jest.MockedFunction +const mockUseUpdateTradeApproveState = useUpdateTradeApproveState as jest.MockedFunction< + typeof useUpdateTradeApproveState +> + +describe('useTradeApproveCallback', () => { + const mockToken = new Token(1, '0x1234567890123456789012345678901234567890', 18, 'TEST', 'Test Token') + const mockSpenderAddress = '0xspenderaddress1234567890123456789012345678' + const mockAmount = BigInt('1000000000000000000') + + const mockSendEvent = jest.fn() + const mockUpdateTradeApproveState = jest.fn() + const mockApproveCallback = jest.fn() + + const createMockTransactionReceipt = (status: number): TransactionReceipt => ({ + to: mockSpenderAddress, + from: '0xfrom1234567890123456789012345678901234567890', + contractAddress: mockToken.address, + transactionIndex: 1, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + gasUsed: { toString: () => '21000' } as any, + logsBloom: '0x', + blockHash: '0xblockhash', + transactionHash: '0xtxhash', + logs: [], + blockNumber: 123456, + confirmations: 1, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + cumulativeGasUsed: { toString: () => '21000' } as any, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + effectiveGasPrice: { toString: () => '1000000000' } as any, + byzantium: true, + type: 2, + status, + }) + + const createMockTransactionResponse = (status: number): TransactionResponse => + ({ + hash: '0xtxhash', + confirmations: 0, + from: '0xfrom1234567890123456789012345678901234567890', + wait: jest.fn().mockResolvedValue(createMockTransactionReceipt(status)), + nonce: 1, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + gasLimit: { toString: () => '21000' } as any, + data: '0x', + // eslint-disable-next-line @typescript-eslint/no-explicit-any + value: { toString: () => '0' } as any, + chainId: 1, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + }) as any + + beforeEach(() => { + jest.clearAllMocks() + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + mockUseCowAnalytics.mockReturnValue({ sendEvent: mockSendEvent } as any) + mockUseTradeSpenderAddress.mockReturnValue(mockSpenderAddress) + mockUseFeatureFlags.mockReturnValue({ isPartialApproveEnabled: false }) + mockUseApproveCallback.mockReturnValue(mockApproveCallback) + mockUseUpdateTradeApproveState.mockReturnValue(mockUpdateTradeApproveState) + }) + + describe('successful approval flow', () => { + it('should successfully approve tokens and track analytics', async () => { + const mockTxResponse = createMockTransactionResponse(1) + mockApproveCallback.mockResolvedValue(mockTxResponse) + + const { result } = renderHook(() => useTradeApproveCallback(mockToken)) + + await result.current(mockAmount) + + await waitFor(() => { + expect(mockUpdateTradeApproveState).toHaveBeenCalledWith({ + currency: mockToken, + approveInProgress: true, + }) + expect(mockApproveCallback).toHaveBeenCalledWith(mockAmount) + expect(mockSendEvent).toHaveBeenCalledWith({ + category: CowSwapAnalyticsCategory.TRADE, + action: 'Send', + label: mockToken.symbol, + }) + expect(mockSendEvent).toHaveBeenCalledWith({ + category: CowSwapAnalyticsCategory.TRADE, + action: 'Sign', + label: mockToken.symbol, + }) + expect(mockUpdateTradeApproveState).toHaveBeenCalledWith({ + currency: undefined, + approveInProgress: false, + }) + }) + }) + + it('should return transaction receipt on successful approval', async () => { + const mockTxResponse = createMockTransactionResponse(1) + mockApproveCallback.mockResolvedValue(mockTxResponse) + + const { result } = renderHook(() => useTradeApproveCallback(mockToken)) + + await result.current(mockAmount) + + await waitFor(() => { + expect(mockUpdateTradeApproveState).toHaveBeenCalledWith({ + currency: undefined, + approveInProgress: false, + }) + }) + }) + + it('should wait for transaction to be mined', async () => { + const mockTxResponse = createMockTransactionResponse(1) + const mockWait = jest.fn().mockResolvedValue(createMockTransactionReceipt(1)) + mockTxResponse.wait = mockWait + mockApproveCallback.mockResolvedValue(mockTxResponse) + + const { result } = renderHook(() => useTradeApproveCallback(mockToken)) + + await result.current(mockAmount) + + await waitFor(() => { + expect(mockWait).toHaveBeenCalled() + }) + }) + }) + + describe('failed approval flow', () => { + it('should handle transaction failure (status !== 1)', async () => { + const mockTxResponse = createMockTransactionResponse(0) + mockApproveCallback.mockResolvedValue(mockTxResponse) + + const { result } = renderHook(() => useTradeApproveCallback(mockToken)) + + const receipt = await result.current(mockAmount) + + await waitFor(() => { + expect(receipt).toBeUndefined() + expect(mockUpdateTradeApproveState).toHaveBeenCalledWith({ + currency: undefined, + approveInProgress: false, + }) + expect(mockUpdateTradeApproveState).toHaveBeenCalledWith({ + error: expect.stringContaining('Approval transaction failed'), + }) + }) + }) + + it('should handle user rejection', async () => { + const rejectError = { code: 4001, message: 'User rejected the request' } + mockApproveCallback.mockRejectedValue(rejectError) + + const { result } = renderHook(() => useTradeApproveCallback(mockToken)) + + const receipt = await result.current(mockAmount) + + await waitFor(() => { + expect(receipt).toBeUndefined() + expect(mockUpdateTradeApproveState).toHaveBeenCalledWith({ + error: 'User rejected approval transaction', + }) + expect(mockUpdateTradeApproveState).toHaveBeenCalledWith({ + currency: undefined, + approveInProgress: false, + }) + }) + }) + + it('should handle generic errors and track analytics', async () => { + const genericError = { code: 500, message: 'Network error' } + mockApproveCallback.mockRejectedValue(genericError) + + const { result } = renderHook(() => useTradeApproveCallback(mockToken)) + + const receipt = await result.current(mockAmount) + + await waitFor(() => { + expect(receipt).toBeUndefined() + expect(mockSendEvent).toHaveBeenCalledWith({ + category: CowSwapAnalyticsCategory.TRADE, + action: 'Error', + label: mockToken.symbol, + value: 500, + }) + expect(mockUpdateTradeApproveState).toHaveBeenCalledWith({ + error: expect.any(String), + }) + }) + }) + + it('should handle errors without error codes', async () => { + const errorWithoutCode = new Error('Generic error') + mockApproveCallback.mockRejectedValue(errorWithoutCode) + + const { result } = renderHook(() => useTradeApproveCallback(mockToken)) + + const receipt = await result.current(mockAmount) + + await waitFor(() => { + expect(receipt).toBeUndefined() + expect(mockSendEvent).toHaveBeenCalledWith({ + category: CowSwapAnalyticsCategory.TRADE, + action: 'Error', + label: mockToken.symbol, + }) + }) + }) + }) + + describe('partial approval feature flag behavior', () => { + it('should hide modal immediately when feature is disabled and transaction is sent', async () => { + mockUseFeatureFlags.mockReturnValue({ isPartialApproveEnabled: false }) + const mockTxResponse = createMockTransactionResponse(1) + mockApproveCallback.mockResolvedValue(mockTxResponse) + + const { result } = renderHook(() => useTradeApproveCallback(mockToken)) + + await result.current(mockAmount) + + await waitFor(() => { + expect(mockSendEvent).toHaveBeenCalledWith({ + category: CowSwapAnalyticsCategory.TRADE, + action: 'Sign', + label: mockToken.symbol, + }) + expect(mockUpdateTradeApproveState).toHaveBeenCalledWith({ + currency: undefined, + approveInProgress: false, + }) + }) + }) + + it('should not hide modal early when feature is enabled', async () => { + mockUseFeatureFlags.mockReturnValue({ isPartialApproveEnabled: true }) + const mockTxResponse = createMockTransactionResponse(1) + mockApproveCallback.mockResolvedValue(mockTxResponse) + + const { result } = renderHook(() => useTradeApproveCallback(mockToken)) + + await result.current(mockAmount) + + await waitFor(() => { + expect(mockSendEvent).toHaveBeenCalledWith({ + category: CowSwapAnalyticsCategory.TRADE, + action: 'Sign', + label: mockToken.symbol, + }) + const callsBeforeFinally = mockUpdateTradeApproveState.mock.calls.filter( + (call) => call[0].currency === undefined && call[0].approveInProgress === false, + ) + expect(callsBeforeFinally).toHaveLength(1) + }) + }) + }) + + describe('useModals parameter', () => { + it('should show modal when useModals is true (default)', async () => { + const mockTxResponse = createMockTransactionResponse(1) + mockApproveCallback.mockResolvedValue(mockTxResponse) + + const { result } = renderHook(() => useTradeApproveCallback(mockToken)) + + await result.current(mockAmount) + + await waitFor(() => { + expect(mockUpdateTradeApproveState).toHaveBeenCalledWith({ + currency: mockToken, + approveInProgress: true, + }) + }) + }) + + it('should not show modal when useModals is false', async () => { + const mockTxResponse = createMockTransactionResponse(1) + mockApproveCallback.mockResolvedValue(mockTxResponse) + + const { result } = renderHook(() => useTradeApproveCallback(mockToken)) + + await result.current(mockAmount, { useModals: false }) + + await waitFor(() => { + const modalCalls = mockUpdateTradeApproveState.mock.calls.filter((call) => call[0].approveInProgress === true) + expect(modalCalls).toHaveLength(0) + }) + }) + + it('should still cleanup state even when useModals is false', async () => { + const mockTxResponse = createMockTransactionResponse(1) + mockApproveCallback.mockResolvedValue(mockTxResponse) + + const { result } = renderHook(() => useTradeApproveCallback(mockToken)) + + await result.current(mockAmount, { useModals: false }) + + await waitFor(() => { + expect(mockUpdateTradeApproveState).toHaveBeenCalledWith({ + currency: undefined, + approveInProgress: false, + }) + }) + }) + }) + + describe('state cleanup', () => { + it('should always reset state in finally block', async () => { + const mockTxResponse = createMockTransactionResponse(1) + mockApproveCallback.mockResolvedValue(mockTxResponse) + + const { result } = renderHook(() => useTradeApproveCallback(mockToken)) + + await result.current(mockAmount) + + await waitFor(() => { + expect(mockUpdateTradeApproveState).toHaveBeenCalledWith({ + currency: undefined, + approveInProgress: false, + }) + }) + }) + + it('should reset state even on error', async () => { + mockApproveCallback.mockRejectedValue(new Error('Test error')) + + const { result } = renderHook(() => useTradeApproveCallback(mockToken)) + + await result.current(mockAmount) + + await waitFor(() => { + expect(mockUpdateTradeApproveState).toHaveBeenCalledWith({ + currency: undefined, + approveInProgress: false, + }) + }) + }) + + it('should reset state even when transaction fails', async () => { + const mockTxResponse = createMockTransactionResponse(0) + mockApproveCallback.mockResolvedValue(mockTxResponse) + + const { result } = renderHook(() => useTradeApproveCallback(mockToken)) + + await result.current(mockAmount) + + await waitFor(() => { + expect(mockUpdateTradeApproveState).toHaveBeenCalledWith({ + currency: undefined, + approveInProgress: false, + }) + }) + }) + }) + + describe('memoization and re-renders', () => { + it('should return stable callback reference', () => { + const { result, rerender } = renderHook(() => useTradeApproveCallback(mockToken)) + + const firstCallback = result.current + + rerender() + + expect(result.current).toBe(firstCallback) + }) + + it('should update callback when dependencies change', () => { + const { result, rerender } = renderHook(({ currency }) => useTradeApproveCallback(currency), { + initialProps: { currency: mockToken }, + }) + + const firstCallback = result.current + + const newToken = new Token(1, '0x9876543210987654321098765432109876543210', 18, 'NEW', 'New Token') + rerender({ currency: newToken }) + + expect(result.current).not.toBe(firstCallback) + }) + }) +}) diff --git a/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.ts b/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.ts index b0b658a52a..95f056f81b 100644 --- a/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.ts +++ b/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.ts @@ -58,6 +58,8 @@ export function useTradeApproveCallback(currency: Currency | undefined): TradeAp if (txResponse.status !== 1) { throw new Error('Approval transaction failed') } + + return txResponse }) }) .finally(() => { From f583a9bb1fd3347123f7d4ae1803f7db680c9240 Mon Sep 17 00:00:00 2001 From: Alexandr Kazachenko Date: Thu, 16 Oct 2025 14:44:30 +0500 Subject: [PATCH 05/17] feat(approve): add optimistic allowance amount --- .../src/common/hooks/useTokenAllowance.ts | 73 +++--- .../getOptimisticAllowanceKey.ts | 7 + .../optimisticAllowancesAtom.ts | 5 + .../src/entities/optimisticAllowance/types.ts | 5 + .../useSetOptimisticAllowance.ts | 35 +++ .../TradeApproveModal/approveUtils.test.ts | 244 ++++++++++++++++++ .../TradeApproveModal/approveUtils.ts | 91 +++++++ .../useTradeApproveCallback.test.ts | 171 ++++++++++-- .../useTradeApproveCallback.ts | 36 ++- .../hooks/usePendingTransactionsContext.ts | 4 - .../services/finalizeEthereumTransaction.ts | 10 +- .../updaters/FinalizeTxUpdater/types.ts | 1 - 12 files changed, 612 insertions(+), 70 deletions(-) create mode 100644 apps/cowswap-frontend/src/entities/optimisticAllowance/getOptimisticAllowanceKey.ts create mode 100644 apps/cowswap-frontend/src/entities/optimisticAllowance/optimisticAllowancesAtom.ts create mode 100644 apps/cowswap-frontend/src/entities/optimisticAllowance/types.ts create mode 100644 apps/cowswap-frontend/src/entities/optimisticAllowance/useSetOptimisticAllowance.ts create mode 100644 apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/approveUtils.test.ts create mode 100644 apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/approveUtils.ts diff --git a/apps/cowswap-frontend/src/common/hooks/useTokenAllowance.ts b/apps/cowswap-frontend/src/common/hooks/useTokenAllowance.ts index b95fbbbd5e..52e8d13d10 100644 --- a/apps/cowswap-frontend/src/common/hooks/useTokenAllowance.ts +++ b/apps/cowswap-frontend/src/common/hooks/useTokenAllowance.ts @@ -1,24 +1,20 @@ -import { atom, useAtom, useSetAtom } from 'jotai' -import { useCallback, useEffect } from 'react' +import { useAtom } from 'jotai' +import { useEffect, useMemo } from 'react' import { useTradeSpenderAddress } from '@cowprotocol/balances-and-allowances' import { SWR_NO_REFRESH_OPTIONS } from '@cowprotocol/common-const' import { useWalletInfo } from '@cowprotocol/wallet' import { Token } from '@uniswap/sdk-core' +import { optimisticAllowancesAtom } from 'entities/optimisticAllowance/optimisticAllowancesAtom' import ms from 'ms.macro' import useSWR, { SWRConfiguration, SWRResponse } from 'swr' import { useTokenContract } from 'common/hooks/useContract' -interface LastApproveParams { - blockNumber: number - tokenAddress: string -} - -const lastApproveTxBlockNumberAtom = atom>({}) +import { getOptimisticAllowanceKey } from '../../entities/optimisticAllowance/getOptimisticAllowanceKey' -const LAST_APPROVE_TX_TIME_THRESHOLD = ms`1s` +const OPTIMISTIC_ALLOWANCE_TTL = ms`30s` const SWR_OPTIONS: SWRConfiguration = { ...SWR_NO_REFRESH_OPTIONS, @@ -36,21 +32,22 @@ export function useTokenAllowance( const { chainId, account } = useWalletInfo() const { contract: erc20Contract } = useTokenContract(tokenAddress) const tradeSpender = useTradeSpenderAddress() - const [lastApproveTx, setLastApproveTx] = useAtom(lastApproveTxBlockNumberAtom) + const [optimisticAllowances, setOptimisticAllowances] = useAtom(optimisticAllowancesAtom) const targetOwner = owner ?? account const targetSpender = spender ?? tradeSpender - const lastApproveBlockNumber = tokenAddress ? lastApproveTx[tokenAddress.toLowerCase()] : undefined - // Reset lastApproveTxBlockNumberAtom on network changes - useEffect(() => { - setLastApproveTx({}) - }, [chainId, setLastApproveTx]) + const optimisticAllowanceKey = useMemo(() => { + if (!tokenAddress || !targetOwner || !targetSpender) return null + return getOptimisticAllowanceKey({ chainId, tokenAddress, owner: targetOwner, spender: targetSpender }) + }, [chainId, tokenAddress, targetOwner, targetSpender]) + + const optimisticAllowance = optimisticAllowanceKey ? optimisticAllowances[optimisticAllowanceKey] : undefined // Important! Do not add erc20Contract to SWR deps, otherwise it will do unwanted node RPC calls! - return useSWR( + const swrResponse = useSWR( erc20Contract && targetOwner && targetSpender - ? [targetOwner, targetSpender, chainId, lastApproveBlockNumber, tokenAddress, 'useTokenAllowance'] + ? [targetOwner, targetSpender, chainId, tokenAddress, 'useTokenAllowance'] : null, ([targetOwner, targetSpender]) => { if (!erc20Contract) return undefined @@ -59,21 +56,33 @@ export function useTokenAllowance( }, SWR_OPTIONS, ) -} -export function useUpdateLastApproveTxBlockNumber(): (params: LastApproveParams) => void { - const setState = useSetAtom(lastApproveTxBlockNumberAtom) - - return useCallback( - (params: LastApproveParams) => { - // Wait 1 second before updating value to give some time to node to update the state - setTimeout(() => { - setState((state) => ({ - ...state, - [params.tokenAddress.toLowerCase()]: params.blockNumber, - })) - }, LAST_APPROVE_TX_TIME_THRESHOLD) - }, - [setState], + // Reset state on network changes + useEffect(() => { + setOptimisticAllowances({}) + }, [chainId, setOptimisticAllowances]) + + // Clean up expired optimistic allowances + useEffect(() => { + const now = Date.now() + const expiredKeys = Object.keys(optimisticAllowances).filter( + (key) => now - optimisticAllowances[key].timestamp > OPTIMISTIC_ALLOWANCE_TTL, + ) + + if (expiredKeys.length > 0) { + setOptimisticAllowances((state) => { + const newState = { ...state } + expiredKeys.forEach((key) => delete newState[key]) + return newState + }) + } + }, [optimisticAllowances, setOptimisticAllowances, swrResponse.data]) + + return useMemo( + () => ({ + ...swrResponse, + data: optimisticAllowance?.amount ?? swrResponse.data, + }), + [optimisticAllowance?.amount, swrResponse], ) } diff --git a/apps/cowswap-frontend/src/entities/optimisticAllowance/getOptimisticAllowanceKey.ts b/apps/cowswap-frontend/src/entities/optimisticAllowance/getOptimisticAllowanceKey.ts new file mode 100644 index 0000000000..28a67a1a39 --- /dev/null +++ b/apps/cowswap-frontend/src/entities/optimisticAllowance/getOptimisticAllowanceKey.ts @@ -0,0 +1,7 @@ +import { SetOptimisticAllowanceParams } from './useSetOptimisticAllowance' + +export function getOptimisticAllowanceKey( + params: Omit, +): string { + return `${params.chainId}-${params.tokenAddress.toLowerCase()}-${params.owner.toLowerCase()}-${params.spender.toLowerCase()}` +} diff --git a/apps/cowswap-frontend/src/entities/optimisticAllowance/optimisticAllowancesAtom.ts b/apps/cowswap-frontend/src/entities/optimisticAllowance/optimisticAllowancesAtom.ts new file mode 100644 index 0000000000..47363d77b0 --- /dev/null +++ b/apps/cowswap-frontend/src/entities/optimisticAllowance/optimisticAllowancesAtom.ts @@ -0,0 +1,5 @@ +import { atom } from 'jotai' + +import { OptimisticAllowance } from './types' + +export const optimisticAllowancesAtom = atom>({}) diff --git a/apps/cowswap-frontend/src/entities/optimisticAllowance/types.ts b/apps/cowswap-frontend/src/entities/optimisticAllowance/types.ts new file mode 100644 index 0000000000..3ef1ea8856 --- /dev/null +++ b/apps/cowswap-frontend/src/entities/optimisticAllowance/types.ts @@ -0,0 +1,5 @@ +export interface OptimisticAllowance { + amount: bigint + blockNumber: number + timestamp: number +} diff --git a/apps/cowswap-frontend/src/entities/optimisticAllowance/useSetOptimisticAllowance.ts b/apps/cowswap-frontend/src/entities/optimisticAllowance/useSetOptimisticAllowance.ts new file mode 100644 index 0000000000..7d33cec039 --- /dev/null +++ b/apps/cowswap-frontend/src/entities/optimisticAllowance/useSetOptimisticAllowance.ts @@ -0,0 +1,35 @@ +import { useSetAtom } from 'jotai' +import { useCallback } from 'react' + +import { getOptimisticAllowanceKey } from './getOptimisticAllowanceKey' +import { optimisticAllowancesAtom } from './optimisticAllowancesAtom' + +export interface SetOptimisticAllowanceParams { + tokenAddress: string + owner: string + spender: string + amount: bigint + blockNumber: number + chainId: number +} + +export function useSetOptimisticAllowance(): (params: SetOptimisticAllowanceParams) => void { + const setOptimisticAllowances = useSetAtom(optimisticAllowancesAtom) + + return useCallback( + (params: SetOptimisticAllowanceParams) => { + const key = getOptimisticAllowanceKey(params) + + // Set optimistic allowance immediately + setOptimisticAllowances((state) => ({ + ...state, + [key]: { + amount: params.amount, + blockNumber: params.blockNumber, + timestamp: Date.now(), + }, + })) + }, + [setOptimisticAllowances], + ) +} diff --git a/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/approveUtils.test.ts b/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/approveUtils.test.ts new file mode 100644 index 0000000000..155012d97c --- /dev/null +++ b/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/approveUtils.test.ts @@ -0,0 +1,244 @@ +import { SupportedChainId } from '@cowprotocol/cow-sdk' +import { defaultAbiCoder } from '@ethersproject/abi' +import { TransactionReceipt } from '@ethersproject/abstract-provider' +import { id } from '@ethersproject/hash' +import { Token } from '@uniswap/sdk-core' + +import { processApprovalTransaction } from './approveUtils' + +const APPROVAL_EVENT_TOPIC = id('Approval(address,address,uint256)') + +describe('processApprovalTransaction', () => { + const mockChainId = SupportedChainId.MAINNET + const mockTokenAddress = '0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48' + const mockAccount = '0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266' + const mockSpender = '0x9008D19f58AAbD9eD0D60971565AA8510560ab41' + const mockAmount = BigInt('1000000000000000000') + const mockBlockNumber = 123456 + + const mockToken = new Token(mockChainId, mockTokenAddress, 18, 'TEST', 'Test Token') + + // Helper to create padded address topic + const createAddressTopic = (address: string): string => { + return '0x' + '0'.repeat(24) + address.slice(2).toLowerCase() + } + + // Helper to encode approval amount + const encodeAmount = (amount: bigint): string => { + return defaultAbiCoder.encode(['uint256'], [amount.toString()]) + } + + const createMockTransactionReceipt = (status: number, logs: TransactionReceipt['logs'] = []): TransactionReceipt => { + return { + to: mockSpender, + from: mockAccount, + contractAddress: mockTokenAddress, + transactionIndex: 1, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + gasUsed: { toString: () => '21000' } as any, + logsBloom: '0x', + blockHash: '0xblockhash', + transactionHash: '0xtxhash', + logs, + blockNumber: mockBlockNumber, + confirmations: 1, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + cumulativeGasUsed: { toString: () => '21000' } as any, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + effectiveGasPrice: { toString: () => '1000000000' } as any, + byzantium: true, + type: 2, + status, + } + } + + const createApprovalLog = ( + tokenAddress: string, + owner: string, + spender: string, + amount: bigint, + ): TransactionReceipt['logs'][0] => { + return { + blockNumber: mockBlockNumber, + blockHash: '0xblockhash', + transactionIndex: 1, + removed: false, + address: tokenAddress, + data: encodeAmount(amount), + topics: [APPROVAL_EVENT_TOPIC, createAddressTopic(owner), createAddressTopic(spender)], + transactionHash: '0xtxhash', + logIndex: 0, + } + } + + describe('successful approval extraction', () => { + it('should extract approval data from valid transaction receipt', () => { + const approvalLog = createApprovalLog(mockTokenAddress, mockAccount, mockSpender, mockAmount) + const txReceipt = createMockTransactionReceipt(1, [approvalLog]) + + const result = processApprovalTransaction( + { + chainId: mockChainId, + currency: mockToken, + account: mockAccount, + spender: mockSpender, + }, + txReceipt, + ) + + expect(result).toEqual({ + tokenAddress: mockTokenAddress.toLowerCase(), + owner: mockAccount, + spender: mockSpender, + amount: mockAmount, + blockNumber: mockBlockNumber, + chainId: mockChainId, + }) + }) + + it('should handle zero approval amount (revoke approval)', () => { + const zeroAmount = BigInt('0') + const approvalLog = createApprovalLog(mockTokenAddress, mockAccount, mockSpender, zeroAmount) + const txReceipt = createMockTransactionReceipt(1, [approvalLog]) + + const result = processApprovalTransaction( + { + chainId: mockChainId, + currency: mockToken, + account: mockAccount, + spender: mockSpender, + }, + txReceipt, + ) + + expect(result).toEqual({ + tokenAddress: mockTokenAddress.toLowerCase(), + owner: mockAccount, + spender: mockSpender, + amount: zeroAmount, + blockNumber: mockBlockNumber, + chainId: mockChainId, + }) + }) + + it('should find correct approval log among multiple logs', () => { + const otherLog = { + blockNumber: mockBlockNumber, + blockHash: '0xblockhash', + transactionIndex: 1, + removed: false, + address: '0xOtherAddress000000000000000000000000000000', + data: '0x', + topics: ['0xothertopic'], + transactionHash: '0xtxhash', + logIndex: 0, + } + const approvalLog = createApprovalLog(mockTokenAddress, mockAccount, mockSpender, mockAmount) + const txReceipt = createMockTransactionReceipt(1, [otherLog, approvalLog, otherLog]) + + const result = processApprovalTransaction( + { + chainId: mockChainId, + currency: mockToken, + account: mockAccount, + spender: mockSpender, + }, + txReceipt, + ) + + expect(result).toEqual({ + tokenAddress: mockTokenAddress.toLowerCase(), + owner: mockAccount, + spender: mockSpender, + amount: mockAmount, + blockNumber: mockBlockNumber, + chainId: mockChainId, + }) + }) + }) + + describe('failed transaction handling', () => { + it('should throw error when transaction status is not 1', () => { + const approvalLog = createApprovalLog(mockTokenAddress, mockAccount, mockSpender, mockAmount) + const txReceipt = createMockTransactionReceipt(0, [approvalLog]) + + expect(() => + processApprovalTransaction( + { + chainId: mockChainId, + currency: mockToken, + account: mockAccount, + spender: mockSpender, + }, + txReceipt, + ), + ).toThrow('Approval transaction failed') + }) + + it('should throw error when transaction status is undefined', () => { + const approvalLog = createApprovalLog(mockTokenAddress, mockAccount, mockSpender, mockAmount) + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const txReceipt = createMockTransactionReceipt(undefined as any, [approvalLog]) + + expect(() => + processApprovalTransaction( + { + chainId: mockChainId, + currency: mockToken, + account: mockAccount, + spender: mockSpender, + }, + txReceipt, + ), + ).toThrow('Approval transaction failed') + }) + }) + + describe('real-world scenarios', () => { + it('should handle user changing approval amount in wallet', () => { + // User was asked to approve 1 token but changed to 5 in wallet + const requestedAmount = BigInt('1000000000000000000') + const actualAmount = BigInt('5000000000000000000') + + const approvalLog = createApprovalLog(mockTokenAddress, mockAccount, mockSpender, actualAmount) + const txReceipt = createMockTransactionReceipt(1, [approvalLog]) + + const result = processApprovalTransaction( + { + chainId: mockChainId, + currency: mockToken, + account: mockAccount, + spender: mockSpender, + }, + txReceipt, + ) + + expect(result?.amount).toBe(actualAmount) + expect(result?.amount).not.toBe(requestedAmount) + }) + + it('should handle multiple approval events and select correct one', () => { + const otherSpender = '0x1234567890123456789012345678901234567890' + const otherAmount = BigInt('2000000000000000000') + + // Create two approval logs - one for the target spender and one for another + const wrongApprovalLog = createApprovalLog(mockTokenAddress, mockAccount, otherSpender, otherAmount) + const correctApprovalLog = createApprovalLog(mockTokenAddress, mockAccount, mockSpender, mockAmount) + + const txReceipt = createMockTransactionReceipt(1, [wrongApprovalLog, correctApprovalLog]) + + const result = processApprovalTransaction( + { + chainId: mockChainId, + currency: mockToken, + account: mockAccount, + spender: mockSpender, + }, + txReceipt, + ) + + expect(result?.amount).toBe(mockAmount) + expect(result?.spender).toBe(mockSpender) + }) + }) +}) diff --git a/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/approveUtils.ts b/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/approveUtils.ts new file mode 100644 index 0000000000..a5d99463ff --- /dev/null +++ b/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/approveUtils.ts @@ -0,0 +1,91 @@ +import { SupportedChainId } from '@cowprotocol/cow-sdk' +import { Nullish } from '@cowprotocol/types' +import { defaultAbiCoder } from '@ethersproject/abi' +import type { TransactionReceipt } from '@ethersproject/abstract-provider' +import { getAddress } from '@ethersproject/address' +import { id } from '@ethersproject/hash' +import { Currency, Token } from '@uniswap/sdk-core' + +import { SetOptimisticAllowanceParams } from 'entities/optimisticAllowance/useSetOptimisticAllowance' + +// ERC20 Approval event signature: Approval(address indexed owner, address indexed spender, uint256 value) +const APPROVAL_EVENT_TOPIC = id('Approval(address,address,uint256)') + +interface ApprovalTransactionParams { + chainId: SupportedChainId + account: string | undefined + spender: string | undefined + currency: Nullish +} + +export function processApprovalTransaction( + { chainId, currency, account, spender }: ApprovalTransactionParams, + txResponse: TransactionReceipt, +): SetOptimisticAllowanceParams | null { + if (txResponse.status !== 1) { + throw new Error('Approval transaction failed') + } + + // Set optimistic allowance immediately after transaction is mined + // Extract the actual approved amount from transaction logs + if (currency && account && spender && chainId) { + const tokenAddress = (currency as Token).address + + if (tokenAddress) { + const approvedAmount = extractApprovalAmountFromLogs(txResponse, tokenAddress, account, spender) + + if (approvedAmount !== undefined) { + return { + tokenAddress: tokenAddress.toLowerCase(), + owner: account, + spender, + amount: approvedAmount, + blockNumber: txResponse.blockNumber, + chainId, + } + } + } + } + + return null +} + +/** + * Extracts the approved amount from the Approval event in transaction logs + * @param txReceipt Transaction receipt containing logs + * @param tokenAddress Token contract address + * @param owner Address of the token owner + * @param spender Address of the spender + * @returns The approved amount as bigint, or undefined if not found + */ +function extractApprovalAmountFromLogs( + txReceipt: TransactionReceipt, + tokenAddress: string, + owner: string, + spender: string, +): bigint | undefined { + try { + // Find the Approval event log + const approvalLog = txReceipt.logs.find((log) => { + // Check if it's from the token contract and has the Approval event signature + if (log.address.toLowerCase() !== tokenAddress.toLowerCase()) return false + if (log.topics[0] !== APPROVAL_EVENT_TOPIC) return false + + // Verify owner and spender match (topics[1] = owner, topics[2] = spender) + const logOwner = getAddress('0x' + log.topics[1].slice(26)) + const logSpender = getAddress('0x' + log.topics[2].slice(26)) + + return logOwner.toLowerCase() === owner.toLowerCase() && logSpender.toLowerCase() === spender.toLowerCase() + }) + + if (!approvalLog) return undefined + + // Parse the value from log data (3rd parameter of Approval event) + const value = defaultAbiCoder.decode(['uint256'], approvalLog.data)[0] + + return BigInt(value.toString()) + } catch (error) { + console.error('Error extracting approval amount from logs:', error) + return undefined + } +} diff --git a/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.test.ts b/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.test.ts index 987c5688ae..f01403e2fa 100644 --- a/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.test.ts +++ b/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.test.ts @@ -1,13 +1,16 @@ import { useCowAnalytics } from '@cowprotocol/analytics' import { useTradeSpenderAddress } from '@cowprotocol/balances-and-allowances' import { useFeatureFlags } from '@cowprotocol/common-hooks' +import { useWalletInfo } from '@cowprotocol/wallet' import { TransactionReceipt, TransactionResponse } from '@ethersproject/abstract-provider' import { Token } from '@uniswap/sdk-core' import { renderHook, waitFor } from '@testing-library/react' +import { useSetOptimisticAllowance } from 'entities/optimisticAllowance/useSetOptimisticAllowance' import { CowSwapAnalyticsCategory } from 'common/analytics/types' +import { processApprovalTransaction } from './approveUtils' import { useTradeApproveCallback } from './useTradeApproveCallback' import { useApproveCallback } from '../../hooks' @@ -22,6 +25,18 @@ jest.mock('@cowprotocol/balances-and-allowances', () => ({ useTradeSpenderAddress: jest.fn(), })) +jest.mock('@cowprotocol/wallet', () => ({ + useWalletInfo: jest.fn(), +})) + +jest.mock('entities/optimisticAllowance/useSetOptimisticAllowance', () => ({ + useSetOptimisticAllowance: jest.fn(), +})) + +jest.mock('./approveUtils', () => ({ + processApprovalTransaction: jest.fn(), +})) + jest.mock('@cowprotocol/common-hooks', () => ({ useFeatureFlags: jest.fn(), })) @@ -41,37 +56,48 @@ const mockUseApproveCallback = useApproveCallback as jest.MockedFunction +const mockUseWalletInfo = useWalletInfo as jest.MockedFunction +const mockUseSetOptimisticAllowance = useSetOptimisticAllowance as jest.MockedFunction +const mockProcessApprovalTransaction = processApprovalTransaction as jest.MockedFunction< + typeof processApprovalTransaction +> +// eslint-disable-next-line max-lines-per-function describe('useTradeApproveCallback', () => { - const mockToken = new Token(1, '0x1234567890123456789012345678901234567890', 18, 'TEST', 'Test Token') - const mockSpenderAddress = '0xspenderaddress1234567890123456789012345678' + const mockToken = new Token(1, '0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48', 18, 'TEST', 'Test Token') + const mockSpenderAddress = '0x9008D19f58AAbD9eD0D60971565AA8510560ab41' const mockAmount = BigInt('1000000000000000000') const mockSendEvent = jest.fn() const mockUpdateTradeApproveState = jest.fn() const mockApproveCallback = jest.fn() + const mockSetOptimisticAllowance = jest.fn() + const mockAccount = '0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266' // Valid address + const mockChainId = 1 - const createMockTransactionReceipt = (status: number): TransactionReceipt => ({ - to: mockSpenderAddress, - from: '0xfrom1234567890123456789012345678901234567890', - contractAddress: mockToken.address, - transactionIndex: 1, - // eslint-disable-next-line @typescript-eslint/no-explicit-any - gasUsed: { toString: () => '21000' } as any, - logsBloom: '0x', - blockHash: '0xblockhash', - transactionHash: '0xtxhash', - logs: [], - blockNumber: 123456, - confirmations: 1, - // eslint-disable-next-line @typescript-eslint/no-explicit-any - cumulativeGasUsed: { toString: () => '21000' } as any, - // eslint-disable-next-line @typescript-eslint/no-explicit-any - effectiveGasPrice: { toString: () => '1000000000' } as any, - byzantium: true, - type: 2, - status, - }) + const createMockTransactionReceipt = (status: number): TransactionReceipt => { + return { + to: mockSpenderAddress, + from: '0xfrom1234567890123456789012345678901234567890', + contractAddress: mockToken.address, + transactionIndex: 1, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + gasUsed: { toString: () => '21000' } as any, + logsBloom: '0x', + blockHash: '0xblockhash', + transactionHash: '0xtxhash', + logs: [], + blockNumber: 123456, + confirmations: 1, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + cumulativeGasUsed: { toString: () => '21000' } as any, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + effectiveGasPrice: { toString: () => '1000000000' } as any, + byzantium: true, + type: 2, + status, + } + } const createMockTransactionResponse = (status: number): TransactionResponse => ({ @@ -98,12 +124,95 @@ describe('useTradeApproveCallback', () => { mockUseFeatureFlags.mockReturnValue({ isPartialApproveEnabled: false }) mockUseApproveCallback.mockReturnValue(mockApproveCallback) mockUseUpdateTradeApproveState.mockReturnValue(mockUpdateTradeApproveState) + // eslint-disable-next-line @typescript-eslint/no-explicit-any + mockUseWalletInfo.mockReturnValue({ chainId: mockChainId, account: mockAccount } as any) + mockUseSetOptimisticAllowance.mockReturnValue(mockSetOptimisticAllowance) }) describe('successful approval flow', () => { + it('should extract and set optimistic allowance from transaction logs', async () => { + const mockTxResponse = createMockTransactionResponse(1) + mockApproveCallback.mockResolvedValue(mockTxResponse) + mockProcessApprovalTransaction.mockReturnValue({ + tokenAddress: mockToken.address.toLowerCase(), + owner: mockAccount, + spender: mockSpenderAddress, + amount: mockAmount, + blockNumber: 123456, + chainId: mockChainId, + }) + + const { result } = renderHook(() => useTradeApproveCallback(mockToken)) + + await result.current(mockAmount) + + await waitFor(() => { + expect(mockProcessApprovalTransaction).toHaveBeenCalled() + expect(mockSetOptimisticAllowance).toHaveBeenCalledWith({ + tokenAddress: mockToken.address.toLowerCase(), + owner: mockAccount, + spender: mockSpenderAddress, + amount: mockAmount, + blockNumber: 123456, + chainId: mockChainId, + }) + }) + }) + + it('should use actual approved amount from logs when user changes amount in wallet', async () => { + const userChangedAmount = BigInt('5000000000000000000') // User changed to 5 tokens instead of 1 + const mockTxResponse = createMockTransactionResponse(1) + mockApproveCallback.mockResolvedValue(mockTxResponse) + mockProcessApprovalTransaction.mockReturnValue({ + tokenAddress: mockToken.address.toLowerCase(), + owner: mockAccount, + spender: mockSpenderAddress, + amount: userChangedAmount, + blockNumber: 123456, + chainId: mockChainId, + }) + + const { result } = renderHook(() => useTradeApproveCallback(mockToken)) + + await result.current(mockAmount) // Request 1 token + + await waitFor(() => { + expect(mockProcessApprovalTransaction).toHaveBeenCalled() + expect(mockSetOptimisticAllowance).toHaveBeenCalledWith({ + tokenAddress: mockToken.address.toLowerCase(), + owner: mockAccount, + spender: mockSpenderAddress, + amount: userChangedAmount, // Should use the actual amount from logs, not mockAmount + blockNumber: 123456, + chainId: mockChainId, + }) + }) + }) + + it('should not set optimistic allowance when logs are missing', async () => { + const mockTxResponse = createMockTransactionResponse(1) + mockApproveCallback.mockResolvedValue(mockTxResponse) + mockProcessApprovalTransaction.mockReturnValue(null) // No approval data found + + const { result } = renderHook(() => useTradeApproveCallback(mockToken)) + + await result.current(mockAmount) + + await waitFor(() => { + expect(mockProcessApprovalTransaction).toHaveBeenCalled() + expect(mockUpdateTradeApproveState).toHaveBeenCalledWith({ + currency: undefined, + approveInProgress: false, + }) + }) + + expect(mockSetOptimisticAllowance).not.toHaveBeenCalled() + }) + it('should successfully approve tokens and track analytics', async () => { const mockTxResponse = createMockTransactionResponse(1) mockApproveCallback.mockResolvedValue(mockTxResponse) + mockProcessApprovalTransaction.mockReturnValue(null) const { result } = renderHook(() => useTradeApproveCallback(mockToken)) @@ -135,6 +244,7 @@ describe('useTradeApproveCallback', () => { it('should return transaction receipt on successful approval', async () => { const mockTxResponse = createMockTransactionResponse(1) mockApproveCallback.mockResolvedValue(mockTxResponse) + mockProcessApprovalTransaction.mockReturnValue(null) const { result } = renderHook(() => useTradeApproveCallback(mockToken)) @@ -153,6 +263,7 @@ describe('useTradeApproveCallback', () => { const mockWait = jest.fn().mockResolvedValue(createMockTransactionReceipt(1)) mockTxResponse.wait = mockWait mockApproveCallback.mockResolvedValue(mockTxResponse) + mockProcessApprovalTransaction.mockReturnValue(null) const { result } = renderHook(() => useTradeApproveCallback(mockToken)) @@ -168,6 +279,10 @@ describe('useTradeApproveCallback', () => { it('should handle transaction failure (status !== 1)', async () => { const mockTxResponse = createMockTransactionResponse(0) mockApproveCallback.mockResolvedValue(mockTxResponse) + // processApprovalTransaction will throw an error for status !== 1 + mockProcessApprovalTransaction.mockImplementation(() => { + throw new Error('Approval transaction failed') + }) const { result } = renderHook(() => useTradeApproveCallback(mockToken)) @@ -251,6 +366,7 @@ describe('useTradeApproveCallback', () => { mockUseFeatureFlags.mockReturnValue({ isPartialApproveEnabled: false }) const mockTxResponse = createMockTransactionResponse(1) mockApproveCallback.mockResolvedValue(mockTxResponse) + mockProcessApprovalTransaction.mockReturnValue(null) const { result } = renderHook(() => useTradeApproveCallback(mockToken)) @@ -273,6 +389,7 @@ describe('useTradeApproveCallback', () => { mockUseFeatureFlags.mockReturnValue({ isPartialApproveEnabled: true }) const mockTxResponse = createMockTransactionResponse(1) mockApproveCallback.mockResolvedValue(mockTxResponse) + mockProcessApprovalTransaction.mockReturnValue(null) const { result } = renderHook(() => useTradeApproveCallback(mockToken)) @@ -296,6 +413,7 @@ describe('useTradeApproveCallback', () => { it('should show modal when useModals is true (default)', async () => { const mockTxResponse = createMockTransactionResponse(1) mockApproveCallback.mockResolvedValue(mockTxResponse) + mockProcessApprovalTransaction.mockReturnValue(null) const { result } = renderHook(() => useTradeApproveCallback(mockToken)) @@ -312,6 +430,7 @@ describe('useTradeApproveCallback', () => { it('should not show modal when useModals is false', async () => { const mockTxResponse = createMockTransactionResponse(1) mockApproveCallback.mockResolvedValue(mockTxResponse) + mockProcessApprovalTransaction.mockReturnValue(null) const { result } = renderHook(() => useTradeApproveCallback(mockToken)) @@ -326,6 +445,7 @@ describe('useTradeApproveCallback', () => { it('should still cleanup state even when useModals is false', async () => { const mockTxResponse = createMockTransactionResponse(1) mockApproveCallback.mockResolvedValue(mockTxResponse) + mockProcessApprovalTransaction.mockReturnValue(null) const { result } = renderHook(() => useTradeApproveCallback(mockToken)) @@ -344,6 +464,7 @@ describe('useTradeApproveCallback', () => { it('should always reset state in finally block', async () => { const mockTxResponse = createMockTransactionResponse(1) mockApproveCallback.mockResolvedValue(mockTxResponse) + mockProcessApprovalTransaction.mockReturnValue(null) const { result } = renderHook(() => useTradeApproveCallback(mockToken)) @@ -375,6 +496,10 @@ describe('useTradeApproveCallback', () => { it('should reset state even when transaction fails', async () => { const mockTxResponse = createMockTransactionResponse(0) mockApproveCallback.mockResolvedValue(mockTxResponse) + // processApprovalTransaction will throw an error for status !== 1 + mockProcessApprovalTransaction.mockImplementation(() => { + throw new Error('Approval transaction failed') + }) const { result } = renderHook(() => useTradeApproveCallback(mockToken)) diff --git a/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.ts b/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.ts index 95f056f81b..04eab7ac0b 100644 --- a/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.ts +++ b/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.ts @@ -4,11 +4,16 @@ import { useCowAnalytics } from '@cowprotocol/analytics' import { useTradeSpenderAddress } from '@cowprotocol/balances-and-allowances' import { useFeatureFlags } from '@cowprotocol/common-hooks' import { errorToString, isRejectRequestProviderError } from '@cowprotocol/common-utils' +import { useWalletInfo } from '@cowprotocol/wallet' import { TransactionReceipt } from '@ethersproject/abstract-provider' import { Currency } from '@uniswap/sdk-core' +import { useSetOptimisticAllowance } from 'entities/optimisticAllowance/useSetOptimisticAllowance' + import { CowSwapAnalyticsCategory } from 'common/analytics/types' +import { processApprovalTransaction } from './approveUtils' + import { useApproveCallback } from '../../hooks' import { useUpdateTradeApproveState } from '../../state' @@ -26,6 +31,8 @@ export function useTradeApproveCallback(currency: Currency | undefined): TradeAp const symbol = currency?.symbol const cowAnalytics = useCowAnalytics() const { isPartialApproveEnabled } = useFeatureFlags() + const { chainId, account } = useWalletInfo() + const setOptimisticAllowance = useSetOptimisticAllowance() const approveCallback = useApproveCallback(currency, spender) @@ -59,6 +66,22 @@ export function useTradeApproveCallback(currency: Currency | undefined): TradeAp throw new Error('Approval transaction failed') } + // Set optimistic allowance immediately after transaction is mined + // Extract the actual approved amount from transaction logs + const approvedAmount = processApprovalTransaction( + { + currency, + account, + spender, + chainId, + }, + txResponse, + ) + + if (approvedAmount) { + setOptimisticAllowance(approvedAmount) + } + return txResponse }) }) @@ -80,6 +103,17 @@ export function useTradeApproveCallback(currency: Currency | undefined): TradeAp return undefined }) }, - [symbol, approveCallback, updateTradeApproveState, currency, approvalAnalytics, isPartialApproveEnabled], + [ + symbol, + approveCallback, + updateTradeApproveState, + currency, + approvalAnalytics, + isPartialApproveEnabled, + account, + spender, + chainId, + setOptimisticAllowance, + ], ) } diff --git a/apps/cowswap-frontend/src/modules/onchainTransactions/updaters/FinalizeTxUpdater/hooks/usePendingTransactionsContext.ts b/apps/cowswap-frontend/src/modules/onchainTransactions/updaters/FinalizeTxUpdater/hooks/usePendingTransactionsContext.ts index 4f7163c130..bf3992eabf 100644 --- a/apps/cowswap-frontend/src/modules/onchainTransactions/updaters/FinalizeTxUpdater/hooks/usePendingTransactionsContext.ts +++ b/apps/cowswap-frontend/src/modules/onchainTransactions/updaters/FinalizeTxUpdater/hooks/usePendingTransactionsContext.ts @@ -11,7 +11,6 @@ import { useGetTwapOrderById } from 'modules/twap/hooks/useGetTwapOrderById' import { useBlockNumber } from 'common/hooks/useBlockNumber' import { useGetReceipt } from 'common/hooks/useGetReceipt' -import { useUpdateLastApproveTxBlockNumber } from 'common/hooks/useTokenAllowance' import useNativeCurrency from 'lib/hooks/useNativeCurrency' import { CheckEthereumTransactions } from '../types' @@ -23,7 +22,6 @@ export function usePendingTransactionsContext(): CheckEthereumTransactions | nul const isSafeWallet = !!safeInfo const lastBlockNumber = useBlockNumber() - const updateLastApproveTxBlockNumber = useUpdateLastApproveTxBlockNumber() const dispatch = useAppDispatch() const cancelOrdersBatch = useCancelOrdersBatch() const getReceipt = useGetReceipt(chainId) @@ -50,7 +48,6 @@ export function usePendingTransactionsContext(): CheckEthereumTransactions | nul getTwapOrderById, transactionsCount, safeInfo, - updateLastApproveTxBlockNumber, } return params @@ -68,7 +65,6 @@ export function usePendingTransactionsContext(): CheckEthereumTransactions | nul cancelOrdersBatch, getTwapOrderById, safeInfo, - updateLastApproveTxBlockNumber, ], null, ) diff --git a/apps/cowswap-frontend/src/modules/onchainTransactions/updaters/FinalizeTxUpdater/services/finalizeEthereumTransaction.ts b/apps/cowswap-frontend/src/modules/onchainTransactions/updaters/FinalizeTxUpdater/services/finalizeEthereumTransaction.ts index 5be5b84a2b..7aa5ef91de 100644 --- a/apps/cowswap-frontend/src/modules/onchainTransactions/updaters/FinalizeTxUpdater/services/finalizeEthereumTransaction.ts +++ b/apps/cowswap-frontend/src/modules/onchainTransactions/updaters/FinalizeTxUpdater/services/finalizeEthereumTransaction.ts @@ -16,21 +16,13 @@ export function finalizeEthereumTransaction( params: CheckEthereumTransactions, safeTransactionHash?: string, ): void { - const { chainId, dispatch, updateLastApproveTxBlockNumber } = params + const { chainId, dispatch } = params const { hash } = transaction console.log(`[FinalizeTxUpdater] Transaction ${receipt.transactionHash} has been mined`, receipt, transaction) ONCHAIN_TRANSACTIONS_EVENTS.emit(OnchainTxEvents.BEFORE_TX_FINALIZE, { transaction, receipt }) - // Once approval tx is mined, we add the priority allowance to immediately allow the user to place orders - if (transaction.approval) { - updateLastApproveTxBlockNumber({ - blockNumber: receipt.blockNumber, - tokenAddress: transaction.approval.tokenAddress, - }) - } - dispatch( finalizeTransaction({ chainId, diff --git a/apps/cowswap-frontend/src/modules/onchainTransactions/updaters/FinalizeTxUpdater/types.ts b/apps/cowswap-frontend/src/modules/onchainTransactions/updaters/FinalizeTxUpdater/types.ts index 0053d6c3b8..33bc021ea1 100644 --- a/apps/cowswap-frontend/src/modules/onchainTransactions/updaters/FinalizeTxUpdater/types.ts +++ b/apps/cowswap-frontend/src/modules/onchainTransactions/updaters/FinalizeTxUpdater/types.ts @@ -22,5 +22,4 @@ export interface CheckEthereumTransactions { nativeCurrencySymbol: string cancelOrdersBatch: CancelOrdersBatchCallback safeInfo: GnosisSafeInfo | undefined - updateLastApproveTxBlockNumber: (params: { blockNumber: number; tokenAddress: string }) => void } From 62f2680376653059bad2ddc0588f3c894be08e37 Mon Sep 17 00:00:00 2001 From: Alexandr Kazachenko Date: Thu, 16 Oct 2025 15:20:54 +0500 Subject: [PATCH 06/17] fix(approve): do not go to confirm if not enough allowance --- .../TradeApproveButton/TradeApproveButton.tsx | 39 +- .../useTradeApproveCallback.ts | 6 +- .../hooks/useApproveAndSwap.test.ts | 442 ++++++++++++++++++ .../erc20Approve/hooks/useApproveAndSwap.ts | 69 +++ .../erc20Approve/hooks/useApproveCurrency.ts | 6 +- .../utils/getIsTradeApproveResult.ts | 13 + .../EthFlow/hooks/useEthFlowActions.ts | 4 +- 7 files changed, 538 insertions(+), 41 deletions(-) create mode 100644 apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveAndSwap.test.ts create mode 100644 apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveAndSwap.ts create mode 100644 apps/cowswap-frontend/src/modules/erc20Approve/utils/getIsTradeApproveResult.ts diff --git a/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveButton/TradeApproveButton.tsx b/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveButton/TradeApproveButton.tsx index 9ae25a83fa..52e0cd1ae7 100644 --- a/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveButton/TradeApproveButton.tsx +++ b/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveButton/TradeApproveButton.tsx @@ -1,4 +1,4 @@ -import React, { ReactNode, useCallback } from 'react' +import React, { ReactNode } from 'react' import { useTradeSpenderAddress } from '@cowprotocol/balances-and-allowances' import { ButtonSize, HoverTooltip, TokenSymbol } from '@cowprotocol/ui' @@ -6,16 +6,13 @@ import { Currency, CurrencyAmount } from '@uniswap/sdk-core' import { Trans } from '@lingui/macro' -import { useTokenSupportsPermit } from 'modules/permit' -import { TradeType } from 'modules/trade' - import * as styledEl from './styled' import { ButtonWrapper } from './styled' import { MAX_APPROVE_AMOUNT } from '../../constants' -import { useApprovalStateForSpender, useApproveCurrency, useGeneratePermitInAdvanceToTrade } from '../../hooks' +import { useApprovalStateForSpender, useApproveCurrency } from '../../hooks' +import { useApproveAndSwap } from '../../hooks/useApproveAndSwap' import { LegacyApproveButton } from '../../pure/LegacyApproveButton' -import { useIsPartialApproveSelectedByUser } from '../../state' import { ApprovalState } from '../../types' export interface TradeApproveButtonProps { @@ -35,44 +32,16 @@ export function TradeApproveButton(props: TradeApproveButtonProps): ReactNode { amountToApprove, children, enablePartialApprove, - confirmSwap, label, - ignorePermit, isDisabled, buttonSize = ButtonSize.DEFAULT, useModals = true, } = props - const isPartialApproveEnabledByUser = useIsPartialApproveSelectedByUser() const handleApprove = useApproveCurrency(amountToApprove, useModals) const spender = useTradeSpenderAddress() const { approvalState } = useApprovalStateForSpender(amountToApprove, spender) - const isPermitSupported = useTokenSupportsPermit(amountToApprove.currency, TradeType.SWAP) && !ignorePermit - const generatePermitToTrade = useGeneratePermitInAdvanceToTrade(amountToApprove) - - const approveAndSwap = useCallback(async (): Promise => { - if (isPermitSupported && confirmSwap) { - const isPermitSigned = await generatePermitToTrade() - if (isPermitSigned) { - confirmSwap() - } - - return - } - - const toApprove = isPartialApproveEnabledByUser ? BigInt(amountToApprove.quotient.toString()) : MAX_APPROVE_AMOUNT - const tx = await handleApprove(toApprove) - if (tx && confirmSwap) { - confirmSwap() - } - }, [ - isPermitSupported, - confirmSwap, - isPartialApproveEnabledByUser, - amountToApprove.quotient, - handleApprove, - generatePermitToTrade, - ]) + const approveAndSwap = useApproveAndSwap(props) if (!enablePartialApprove) { return ( diff --git a/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.ts b/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.ts index 04eab7ac0b..adf639dd48 100644 --- a/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.ts +++ b/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.ts @@ -21,8 +21,10 @@ interface TradeApproveCallbackParams { useModals: boolean } +export type TradeApproveResult = { txResponse: TransactionReceipt; approvedAmount: bigint | undefined } + export interface TradeApproveCallback { - (amount: bigint, params?: TradeApproveCallbackParams): Promise + (amount: bigint, params?: TradeApproveCallbackParams): Promise } export function useTradeApproveCallback(currency: Currency | undefined): TradeApproveCallback { @@ -82,7 +84,7 @@ export function useTradeApproveCallback(currency: Currency | undefined): TradeAp setOptimisticAllowance(approvedAmount) } - return txResponse + return { txResponse, approvedAmount: approvedAmount?.amount } }) }) .finally(() => { diff --git a/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveAndSwap.test.ts b/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveAndSwap.test.ts new file mode 100644 index 0000000000..7afdfb3e10 --- /dev/null +++ b/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveAndSwap.test.ts @@ -0,0 +1,442 @@ +import { TransactionReceipt } from '@ethersproject/abstract-provider' +import { Token } from '@uniswap/sdk-core' + +import { renderHook, waitFor } from '@testing-library/react' + +import { useApproveAndSwap } from './useApproveAndSwap' +import { useApproveCurrency } from './useApproveCurrency' +import { useGeneratePermitInAdvanceToTrade } from './useGeneratePermitInAdvanceToTrade' + +import { useTokenSupportsPermit } from '../../permit' +import { MAX_APPROVE_AMOUNT } from '../constants' +import { TradeApproveResult } from '../containers' +import { useIsPartialApproveSelectedByUser } from '../state' + +jest.mock('./useApproveCurrency') +jest.mock('./useGeneratePermitInAdvanceToTrade') +jest.mock('../../permit') +jest.mock('../state') + +const mockUseApproveCurrency = useApproveCurrency as jest.MockedFunction +const mockUseGeneratePermitInAdvanceToTrade = useGeneratePermitInAdvanceToTrade as jest.MockedFunction< + typeof useGeneratePermitInAdvanceToTrade +> +const mockUseTokenSupportsPermit = useTokenSupportsPermit as jest.MockedFunction +const mockUseIsPartialApproveSelectedByUser = useIsPartialApproveSelectedByUser as jest.MockedFunction< + typeof useIsPartialApproveSelectedByUser +> + +// eslint-disable-next-line max-lines-per-function +describe('useApproveAndSwap', () => { + const mockToken = new Token(1, '0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48', 18, 'TEST', 'Test Token') + const mockAmount = BigInt('1000000000000000000') + const mockAmountToApprove = { + currency: mockToken, + quotient: { toString: () => mockAmount.toString() }, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any + + const mockConfirmSwap = jest.fn() + const mockHandleApprove = jest.fn() + const mockGeneratePermitToTrade = jest.fn() + + const createMockTransactionReceipt = (): TransactionReceipt => { + return { + to: '0x9008D19f58AAbD9eD0D60971565AA8510560ab41', + from: '0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266', + contractAddress: mockToken.address, + transactionIndex: 1, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + gasUsed: { toString: () => '21000' } as any, + logsBloom: '0x', + blockHash: '0xblockhash', + transactionHash: '0xtxhash', + logs: [], + blockNumber: 123456, + confirmations: 1, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + cumulativeGasUsed: { toString: () => '21000' } as any, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + effectiveGasPrice: { toString: () => '1000000000' } as any, + byzantium: true, + type: 2, + status: 1, + } + } + + beforeEach(() => { + jest.clearAllMocks() + + mockUseApproveCurrency.mockReturnValue(mockHandleApprove) + mockUseGeneratePermitInAdvanceToTrade.mockReturnValue(mockGeneratePermitToTrade) + mockUseTokenSupportsPermit.mockReturnValue(false) + mockUseIsPartialApproveSelectedByUser.mockReturnValue(false) + }) + + describe('permit flow', () => { + it('should handle successful permit signing and call confirmSwap', async () => { + mockUseTokenSupportsPermit.mockReturnValue(true) + mockGeneratePermitToTrade.mockResolvedValue(true) + + const { result } = renderHook(() => + useApproveAndSwap({ + amountToApprove: mockAmountToApprove, + confirmSwap: mockConfirmSwap, + ignorePermit: false, + useModals: true, + }), + ) + + await result.current() + + await waitFor(() => { + expect(mockGeneratePermitToTrade).toHaveBeenCalled() + expect(mockConfirmSwap).toHaveBeenCalled() + expect(mockHandleApprove).not.toHaveBeenCalled() + }) + }) + + it('should not call confirmSwap if permit signing fails', async () => { + mockUseTokenSupportsPermit.mockReturnValue(true) + mockGeneratePermitToTrade.mockResolvedValue(false) + + const { result } = renderHook(() => + useApproveAndSwap({ + amountToApprove: mockAmountToApprove, + confirmSwap: mockConfirmSwap, + ignorePermit: false, + useModals: true, + }), + ) + + await result.current() + + await waitFor(() => { + expect(mockGeneratePermitToTrade).toHaveBeenCalled() + expect(mockConfirmSwap).not.toHaveBeenCalled() + expect(mockHandleApprove).not.toHaveBeenCalled() + }) + }) + + it('should not call confirmSwap if confirmSwap callback is not provided', async () => { + mockUseTokenSupportsPermit.mockReturnValue(true) + mockGeneratePermitToTrade.mockResolvedValue(true) + + const { result } = renderHook(() => + useApproveAndSwap({ + amountToApprove: mockAmountToApprove, + confirmSwap: undefined, + ignorePermit: false, + useModals: true, + }), + ) + + await result.current() + + await waitFor(() => { + expect(mockGeneratePermitToTrade).not.toHaveBeenCalled() + expect(mockConfirmSwap).not.toHaveBeenCalled() + }) + }) + + it('should skip permit flow when ignorePermit is true', async () => { + mockUseTokenSupportsPermit.mockReturnValue(true) + const mockTxReceipt = createMockTransactionReceipt() + const mockResult: TradeApproveResult = { + txResponse: mockTxReceipt, + approvedAmount: BigInt('2000000000000000000'), + } + mockHandleApprove.mockResolvedValue(mockResult) + + const { result } = renderHook(() => + useApproveAndSwap({ + amountToApprove: mockAmountToApprove, + confirmSwap: mockConfirmSwap, + ignorePermit: true, + useModals: true, + }), + ) + + await result.current() + + await waitFor(() => { + expect(mockGeneratePermitToTrade).not.toHaveBeenCalled() + expect(mockHandleApprove).toHaveBeenCalled() + expect(mockConfirmSwap).toHaveBeenCalled() + }) + }) + }) + + describe('approval flow with TradeApproveResult', () => { + it('should approve and call confirmSwap when approved amount is sufficient', async () => { + const mockTxReceipt = createMockTransactionReceipt() + const mockResult: TradeApproveResult = { + txResponse: mockTxReceipt, + approvedAmount: BigInt('2000000000000000000'), // More than required + } + mockHandleApprove.mockResolvedValue(mockResult) + + const { result } = renderHook(() => + useApproveAndSwap({ + amountToApprove: mockAmountToApprove, + confirmSwap: mockConfirmSwap, + ignorePermit: false, + useModals: true, + }), + ) + + await result.current() + + await waitFor(() => { + expect(mockHandleApprove).toHaveBeenCalledWith(MAX_APPROVE_AMOUNT) + expect(mockConfirmSwap).toHaveBeenCalled() + }) + }) + + it('should approve and call confirmSwap when approved amount equals required amount', async () => { + const mockTxReceipt = createMockTransactionReceipt() + const mockResult: TradeApproveResult = { + txResponse: mockTxReceipt, + approvedAmount: mockAmount, // Exactly what's required + } + mockHandleApprove.mockResolvedValue(mockResult) + + const { result } = renderHook(() => + useApproveAndSwap({ + amountToApprove: mockAmountToApprove, + confirmSwap: mockConfirmSwap, + ignorePermit: false, + useModals: true, + }), + ) + + await result.current() + + await waitFor(() => { + expect(mockHandleApprove).toHaveBeenCalledWith(MAX_APPROVE_AMOUNT) + expect(mockConfirmSwap).toHaveBeenCalled() + }) + }) + + it('should throw error when approved amount is insufficient', async () => { + const mockTxReceipt = createMockTransactionReceipt() + const mockResult: TradeApproveResult = { + txResponse: mockTxReceipt, + approvedAmount: BigInt('500000000000000000'), // Less than required + } + mockHandleApprove.mockResolvedValue(mockResult) + + const { result } = renderHook(() => + useApproveAndSwap({ + amountToApprove: mockAmountToApprove, + confirmSwap: mockConfirmSwap, + ignorePermit: false, + useModals: true, + }), + ) + + await expect(result.current()).rejects.toThrow('Approved amount is not sufficient!') + + expect(mockHandleApprove).toHaveBeenCalledWith(MAX_APPROVE_AMOUNT) + expect(mockConfirmSwap).not.toHaveBeenCalled() + }) + + it('should throw error when approved amount is undefined', async () => { + const mockTxReceipt = createMockTransactionReceipt() + const mockResult: TradeApproveResult = { + txResponse: mockTxReceipt, + approvedAmount: undefined, + } + mockHandleApprove.mockResolvedValue(mockResult) + + const { result } = renderHook(() => + useApproveAndSwap({ + amountToApprove: mockAmountToApprove, + confirmSwap: mockConfirmSwap, + ignorePermit: false, + useModals: true, + }), + ) + + await expect(result.current()).rejects.toThrow('Approved amount is not sufficient!') + + expect(mockHandleApprove).toHaveBeenCalledWith(MAX_APPROVE_AMOUNT) + expect(mockConfirmSwap).not.toHaveBeenCalled() + }) + + it('should use partial approve amount when user has enabled it', async () => { + mockUseIsPartialApproveSelectedByUser.mockReturnValue(true) + const mockTxReceipt = createMockTransactionReceipt() + const mockResult: TradeApproveResult = { + txResponse: mockTxReceipt, + approvedAmount: mockAmount, + } + mockHandleApprove.mockResolvedValue(mockResult) + + const { result } = renderHook(() => + useApproveAndSwap({ + amountToApprove: mockAmountToApprove, + confirmSwap: mockConfirmSwap, + ignorePermit: false, + useModals: true, + }), + ) + + await result.current() + + await waitFor(() => { + expect(mockHandleApprove).toHaveBeenCalledWith(mockAmount) + expect(mockConfirmSwap).toHaveBeenCalled() + }) + }) + }) + + describe('no transaction or confirmSwap', () => { + it('should not call confirmSwap when transaction is null', async () => { + mockHandleApprove.mockResolvedValue(null) + + const { result } = renderHook(() => + useApproveAndSwap({ + amountToApprove: mockAmountToApprove, + confirmSwap: mockConfirmSwap, + ignorePermit: false, + useModals: true, + }), + ) + + await result.current() + + await waitFor(() => { + expect(mockHandleApprove).toHaveBeenCalledWith(MAX_APPROVE_AMOUNT) + expect(mockConfirmSwap).not.toHaveBeenCalled() + }) + }) + + it('should not call confirmSwap when transaction is undefined', async () => { + mockHandleApprove.mockResolvedValue(undefined) + + const { result } = renderHook(() => + useApproveAndSwap({ + amountToApprove: mockAmountToApprove, + confirmSwap: mockConfirmSwap, + ignorePermit: false, + useModals: true, + }), + ) + + await result.current() + + await waitFor(() => { + expect(mockHandleApprove).toHaveBeenCalledWith(MAX_APPROVE_AMOUNT) + expect(mockConfirmSwap).not.toHaveBeenCalled() + }) + }) + + it('should not call confirmSwap when confirmSwap callback is not provided', async () => { + const mockTxReceipt = createMockTransactionReceipt() + const mockResult: TradeApproveResult = { + txResponse: mockTxReceipt, + approvedAmount: mockAmount, + } + mockHandleApprove.mockResolvedValue(mockResult) + + const { result } = renderHook(() => + useApproveAndSwap({ + amountToApprove: mockAmountToApprove, + confirmSwap: undefined, + ignorePermit: false, + useModals: true, + }), + ) + + await result.current() + + await waitFor(() => { + expect(mockHandleApprove).toHaveBeenCalledWith(MAX_APPROVE_AMOUNT) + expect(mockConfirmSwap).not.toHaveBeenCalled() + }) + }) + }) + + describe('useModals parameter', () => { + it('should pass useModals=true to useApproveCurrency', () => { + renderHook(() => + useApproveAndSwap({ + amountToApprove: mockAmountToApprove, + confirmSwap: mockConfirmSwap, + ignorePermit: false, + useModals: true, + }), + ) + + expect(mockUseApproveCurrency).toHaveBeenCalledWith(mockAmountToApprove, true) + }) + + it('should pass useModals=false to useApproveCurrency', () => { + renderHook(() => + useApproveAndSwap({ + amountToApprove: mockAmountToApprove, + confirmSwap: mockConfirmSwap, + ignorePermit: false, + useModals: false, + }), + ) + + expect(mockUseApproveCurrency).toHaveBeenCalledWith(mockAmountToApprove, false) + }) + + it('should pass useModals=undefined to useApproveCurrency when not specified', () => { + renderHook(() => + useApproveAndSwap({ + amountToApprove: mockAmountToApprove, + confirmSwap: mockConfirmSwap, + ignorePermit: false, + }), + ) + + expect(mockUseApproveCurrency).toHaveBeenCalledWith(mockAmountToApprove, undefined) + }) + }) + + describe('error handling', () => { + it('should propagate errors from handleApprove', async () => { + const mockError = new Error('Approval failed') + mockHandleApprove.mockRejectedValue(mockError) + + const { result } = renderHook(() => + useApproveAndSwap({ + amountToApprove: mockAmountToApprove, + confirmSwap: mockConfirmSwap, + ignorePermit: false, + useModals: true, + }), + ) + + await expect(result.current()).rejects.toThrow('Approval failed') + + expect(mockHandleApprove).toHaveBeenCalledWith(MAX_APPROVE_AMOUNT) + expect(mockConfirmSwap).not.toHaveBeenCalled() + }) + + it('should propagate errors from generatePermitToTrade', async () => { + mockUseTokenSupportsPermit.mockReturnValue(true) + const mockError = new Error('Permit generation failed') + mockGeneratePermitToTrade.mockRejectedValue(mockError) + + const { result } = renderHook(() => + useApproveAndSwap({ + amountToApprove: mockAmountToApprove, + confirmSwap: mockConfirmSwap, + ignorePermit: false, + useModals: true, + }), + ) + + await expect(result.current()).rejects.toThrow('Permit generation failed') + + expect(mockGeneratePermitToTrade).toHaveBeenCalled() + expect(mockHandleApprove).not.toHaveBeenCalled() + expect(mockConfirmSwap).not.toHaveBeenCalled() + }) + }) +}) diff --git a/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveAndSwap.ts b/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveAndSwap.ts new file mode 100644 index 0000000000..72f00ef44e --- /dev/null +++ b/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveAndSwap.ts @@ -0,0 +1,69 @@ +import { useCallback } from 'react' + +import { Currency, CurrencyAmount } from '@uniswap/sdk-core' + +import { useApproveCurrency } from './useApproveCurrency' +import { useGeneratePermitInAdvanceToTrade } from './useGeneratePermitInAdvanceToTrade' + +import { useTokenSupportsPermit } from '../../permit' +import { TradeType } from '../../trade' +import { MAX_APPROVE_AMOUNT } from '../constants' +import { useIsPartialApproveSelectedByUser } from '../state' +import { getIsTradeApproveResult } from '../utils/getIsTradeApproveResult' + +export interface ApproveAndSwapProps { + amountToApprove: CurrencyAmount + confirmSwap?: () => void + ignorePermit?: boolean + useModals?: boolean +} + +export function useApproveAndSwap({ + amountToApprove, + useModals, + ignorePermit, + confirmSwap, +}: ApproveAndSwapProps): () => Promise { + const isPartialApproveEnabledByUser = useIsPartialApproveSelectedByUser() + const handleApprove = useApproveCurrency(amountToApprove, useModals) + + const isPermitSupported = useTokenSupportsPermit(amountToApprove.currency, TradeType.SWAP) && !ignorePermit + const generatePermitToTrade = useGeneratePermitInAdvanceToTrade(amountToApprove) + + return useCallback(async (): Promise => { + if (isPermitSupported && confirmSwap) { + const isPermitSigned = await generatePermitToTrade() + if (isPermitSigned) { + confirmSwap() + } + + return + } + + const amountToApproveBig = BigInt(amountToApprove.quotient.toString()) + const toApprove = isPartialApproveEnabledByUser ? amountToApproveBig : MAX_APPROVE_AMOUNT + const tx = await handleApprove(toApprove) + + if (tx && confirmSwap) { + if (getIsTradeApproveResult(tx)) { + const approvedAmount = tx.approvedAmount + const isApprovedAmountSufficient = Boolean(approvedAmount && approvedAmount >= amountToApproveBig) + + if (isApprovedAmountSufficient) { + confirmSwap() + } else { + throw new Error('Approved amount is not sufficient!') + } + } else { + confirmSwap() + } + } + }, [ + isPermitSupported, + confirmSwap, + isPartialApproveEnabledByUser, + amountToApprove.quotient, + handleApprove, + generatePermitToTrade, + ]) +} diff --git a/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveCurrency.ts b/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveCurrency.ts index c9744c748c..1b8f6a9923 100644 --- a/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveCurrency.ts +++ b/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveCurrency.ts @@ -1,22 +1,22 @@ import { useCallback } from 'react' import { Nullish } from '@cowprotocol/types' -import { TransactionReceipt } from '@ethersproject/abstract-provider' import { SafeMultisigTransactionResponse } from '@safe-global/safe-core-sdk-types' import { Currency, CurrencyAmount } from '@uniswap/sdk-core' -import { useTradeApproveCallback } from 'modules/erc20Approve' +import { TradeApproveResult, useTradeApproveCallback } from 'modules/erc20Approve' import { useShouldZeroApprove, useZeroApprove } from 'modules/zeroApproval' export function useApproveCurrency( amountToApprove: CurrencyAmount | undefined, useModals = true, -): (amount: bigint) => Promise> { +): (amount: bigint) => Promise> { const currency = amountToApprove?.currency const tradeApproveCallback = useTradeApproveCallback(currency) const shouldZeroApprove = useShouldZeroApprove(amountToApprove, true) const zeroApprove = useZeroApprove(currency) + return useCallback( async (amount: bigint) => { if (await shouldZeroApprove()) { diff --git a/apps/cowswap-frontend/src/modules/erc20Approve/utils/getIsTradeApproveResult.ts b/apps/cowswap-frontend/src/modules/erc20Approve/utils/getIsTradeApproveResult.ts new file mode 100644 index 0000000000..c98d6a2a90 --- /dev/null +++ b/apps/cowswap-frontend/src/modules/erc20Approve/utils/getIsTradeApproveResult.ts @@ -0,0 +1,13 @@ +import { Nullish } from '@cowprotocol/types' +import { SafeMultisigTransactionResponse } from '@safe-global/safe-core-sdk-types' + +import { TradeApproveResult } from '../containers' + +export function getIsTradeApproveResult( + result: Nullish, +): result is TradeApproveResult { + if (!result) return false + const tradeApprove = result as TradeApproveResult + + return !!tradeApprove.txResponse +} 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 88d43ace1e..0f7a8db100 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 @@ -73,7 +73,9 @@ export function useEthFlowActions(callbacks: EthFlowActionCallbacks, partialAmou const approve = (useModals?: boolean): Promise => { return sendTransaction('approve', () => { - return callbacks.approve(amountToApprove, { useModals: !!useModals }).then((res) => res?.transactionHash) + return callbacks + .approve(amountToApprove, { useModals: !!useModals }) + .then((res) => res?.txResponse?.transactionHash) }) } From 37d07f1b5b0d3d99a5d1c538f999a1be4cde8549 Mon Sep 17 00:00:00 2001 From: Alexandr Kazachenko Date: Thu, 16 Oct 2025 15:26:33 +0500 Subject: [PATCH 07/17] chore: fix optimisticAllowance --- .../src/common/hooks/useTokenAllowance.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/apps/cowswap-frontend/src/common/hooks/useTokenAllowance.ts b/apps/cowswap-frontend/src/common/hooks/useTokenAllowance.ts index 52e8d13d10..3b805a3fbf 100644 --- a/apps/cowswap-frontend/src/common/hooks/useTokenAllowance.ts +++ b/apps/cowswap-frontend/src/common/hooks/useTokenAllowance.ts @@ -78,6 +78,15 @@ export function useTokenAllowance( } }, [optimisticAllowances, setOptimisticAllowances, swrResponse.data]) + // Trigger SWR revalidation when optimistic allowance is set + // This ensures the cache is updated with fresh data from the blockchain + useEffect(() => { + if (optimisticAllowance) { + // Revalidate in the background to get fresh data from blockchain + swrResponse.mutate() + } + }, [optimisticAllowance, swrResponse]) + return useMemo( () => ({ ...swrResponse, From 70e6b8e286b5d46647cd9b731a502309a7808ad9 Mon Sep 17 00:00:00 2001 From: Alexandr Kazachenko Date: Thu, 16 Oct 2025 15:31:02 +0500 Subject: [PATCH 08/17] chore: fix code duple --- .../containers/TradeApproveModal/useTradeApproveCallback.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.ts b/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.ts index adf639dd48..5a274bcc20 100644 --- a/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.ts +++ b/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.ts @@ -64,10 +64,6 @@ export function useTradeApproveCallback(currency: Currency | undefined): TradeAp // if ff is disabled - use old flow, hide modal when tx is sent !isPartialApproveEnabled && updateTradeApproveState({ currency: undefined, approveInProgress: false }) return response?.wait().then((txResponse) => { - if (txResponse.status !== 1) { - throw new Error('Approval transaction failed') - } - // Set optimistic allowance immediately after transaction is mined // Extract the actual approved amount from transaction logs const approvedAmount = processApprovalTransaction( From c0fd13cdce6b98b0f5130b248b62a1ca78d28cff Mon Sep 17 00:00:00 2001 From: Alexandr Kazachenko Date: Thu, 16 Oct 2025 15:46:38 +0500 Subject: [PATCH 09/17] chore: remove mutation --- .../src/common/hooks/useTokenAllowance.ts | 9 --------- 1 file changed, 9 deletions(-) diff --git a/apps/cowswap-frontend/src/common/hooks/useTokenAllowance.ts b/apps/cowswap-frontend/src/common/hooks/useTokenAllowance.ts index 3b805a3fbf..52e8d13d10 100644 --- a/apps/cowswap-frontend/src/common/hooks/useTokenAllowance.ts +++ b/apps/cowswap-frontend/src/common/hooks/useTokenAllowance.ts @@ -78,15 +78,6 @@ export function useTokenAllowance( } }, [optimisticAllowances, setOptimisticAllowances, swrResponse.data]) - // Trigger SWR revalidation when optimistic allowance is set - // This ensures the cache is updated with fresh data from the blockchain - useEffect(() => { - if (optimisticAllowance) { - // Revalidate in the background to get fresh data from blockchain - swrResponse.mutate() - } - }, [optimisticAllowance, swrResponse]) - return useMemo( () => ({ ...swrResponse, From 33e45158c3d462ba0689825044ac359cceff5749 Mon Sep 17 00:00:00 2001 From: Alexandr Kazachenko Date: Thu, 16 Oct 2025 15:50:41 +0500 Subject: [PATCH 10/17] chore: fix isApprovedAmountSufficient handling --- .../src/modules/erc20Approve/hooks/useApproveAndSwap.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveAndSwap.ts b/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveAndSwap.ts index 72f00ef44e..54b765fd33 100644 --- a/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveAndSwap.ts +++ b/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveAndSwap.ts @@ -8,7 +8,7 @@ import { useGeneratePermitInAdvanceToTrade } from './useGeneratePermitInAdvanceT import { useTokenSupportsPermit } from '../../permit' import { TradeType } from '../../trade' import { MAX_APPROVE_AMOUNT } from '../constants' -import { useIsPartialApproveSelectedByUser } from '../state' +import { useIsPartialApproveSelectedByUser, useUpdateTradeApproveState } from '../state' import { getIsTradeApproveResult } from '../utils/getIsTradeApproveResult' export interface ApproveAndSwapProps { @@ -26,6 +26,7 @@ export function useApproveAndSwap({ }: ApproveAndSwapProps): () => Promise { const isPartialApproveEnabledByUser = useIsPartialApproveSelectedByUser() const handleApprove = useApproveCurrency(amountToApprove, useModals) + const updateTradeApproveState = useUpdateTradeApproveState() const isPermitSupported = useTokenSupportsPermit(amountToApprove.currency, TradeType.SWAP) && !ignorePermit const generatePermitToTrade = useGeneratePermitInAdvanceToTrade(amountToApprove) @@ -52,7 +53,7 @@ export function useApproveAndSwap({ if (isApprovedAmountSufficient) { confirmSwap() } else { - throw new Error('Approved amount is not sufficient!') + updateTradeApproveState({ error: 'Approved amount is not sufficient!' }) } } else { confirmSwap() @@ -65,5 +66,6 @@ export function useApproveAndSwap({ amountToApprove.quotient, handleApprove, generatePermitToTrade, + updateTradeApproveState, ]) } From dce2fe74931d63c90203599db0acba71de7231d6 Mon Sep 17 00:00:00 2001 From: Alexandr Kazachenko Date: Thu, 16 Oct 2025 16:05:47 +0500 Subject: [PATCH 11/17] chore: fix tests --- .../hooks/useApproveAndSwap.test.ts | 29 +++++++++++++------ 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveAndSwap.test.ts b/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveAndSwap.test.ts index 7afdfb3e10..90f45e0f72 100644 --- a/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveAndSwap.test.ts +++ b/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveAndSwap.test.ts @@ -10,7 +10,7 @@ import { useGeneratePermitInAdvanceToTrade } from './useGeneratePermitInAdvanceT import { useTokenSupportsPermit } from '../../permit' import { MAX_APPROVE_AMOUNT } from '../constants' import { TradeApproveResult } from '../containers' -import { useIsPartialApproveSelectedByUser } from '../state' +import { useIsPartialApproveSelectedByUser, useUpdateTradeApproveState } from '../state' jest.mock('./useApproveCurrency') jest.mock('./useGeneratePermitInAdvanceToTrade') @@ -25,6 +25,9 @@ const mockUseTokenSupportsPermit = useTokenSupportsPermit as jest.MockedFunction const mockUseIsPartialApproveSelectedByUser = useIsPartialApproveSelectedByUser as jest.MockedFunction< typeof useIsPartialApproveSelectedByUser > +const mockUseUpdateTradeApproveState = useUpdateTradeApproveState as jest.MockedFunction< + typeof useUpdateTradeApproveState +> // eslint-disable-next-line max-lines-per-function describe('useApproveAndSwap', () => { @@ -39,6 +42,7 @@ describe('useApproveAndSwap', () => { const mockConfirmSwap = jest.fn() const mockHandleApprove = jest.fn() const mockGeneratePermitToTrade = jest.fn() + const mockUpdateTradeApproveState = jest.fn() const createMockTransactionReceipt = (): TransactionReceipt => { return { @@ -71,6 +75,7 @@ describe('useApproveAndSwap', () => { mockUseGeneratePermitInAdvanceToTrade.mockReturnValue(mockGeneratePermitToTrade) mockUseTokenSupportsPermit.mockReturnValue(false) mockUseIsPartialApproveSelectedByUser.mockReturnValue(false) + mockUseUpdateTradeApproveState.mockReturnValue(mockUpdateTradeApproveState) }) describe('permit flow', () => { @@ -218,7 +223,7 @@ describe('useApproveAndSwap', () => { }) }) - it('should throw error when approved amount is insufficient', async () => { + it('should set error state when approved amount is insufficient', async () => { const mockTxReceipt = createMockTransactionReceipt() const mockResult: TradeApproveResult = { txResponse: mockTxReceipt, @@ -235,13 +240,16 @@ describe('useApproveAndSwap', () => { }), ) - await expect(result.current()).rejects.toThrow('Approved amount is not sufficient!') + await result.current() - expect(mockHandleApprove).toHaveBeenCalledWith(MAX_APPROVE_AMOUNT) - expect(mockConfirmSwap).not.toHaveBeenCalled() + await waitFor(() => { + expect(mockHandleApprove).toHaveBeenCalledWith(MAX_APPROVE_AMOUNT) + expect(mockUpdateTradeApproveState).toHaveBeenCalledWith({ error: 'Approved amount is not sufficient!' }) + expect(mockConfirmSwap).not.toHaveBeenCalled() + }) }) - it('should throw error when approved amount is undefined', async () => { + it('should set error state when approved amount is undefined', async () => { const mockTxReceipt = createMockTransactionReceipt() const mockResult: TradeApproveResult = { txResponse: mockTxReceipt, @@ -258,10 +266,13 @@ describe('useApproveAndSwap', () => { }), ) - await expect(result.current()).rejects.toThrow('Approved amount is not sufficient!') + await result.current() - expect(mockHandleApprove).toHaveBeenCalledWith(MAX_APPROVE_AMOUNT) - expect(mockConfirmSwap).not.toHaveBeenCalled() + await waitFor(() => { + expect(mockHandleApprove).toHaveBeenCalledWith(MAX_APPROVE_AMOUNT) + expect(mockUpdateTradeApproveState).toHaveBeenCalledWith({ error: 'Approved amount is not sufficient!' }) + expect(mockConfirmSwap).not.toHaveBeenCalled() + }) }) it('should use partial approve amount when user has enabled it', async () => { From e4f31a9cff294792b2f5a64296e3b6f2a36bafb8 Mon Sep 17 00:00:00 2001 From: Alexandr Kazachenko Date: Thu, 16 Oct 2025 18:18:07 +0500 Subject: [PATCH 12/17] chore: merge with eth-flow changes --- .../TradeApproveModal/useApprovalAnalytics.ts | 21 +++ .../useHandleApprovalError.ts | 28 ++++ .../useTradeApproveCallback.ts | 132 ++++++++++-------- .../erc20Approve/hooks/useApproveAndSwap.ts | 35 +++-- .../erc20Approve/hooks/useApproveCurrency.ts | 4 +- .../utils/getIsTradeApproveResult.ts | 8 +- .../EthFlow/hooks/useEthFlowActions.ts | 3 +- 7 files changed, 150 insertions(+), 81 deletions(-) create mode 100644 apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useApprovalAnalytics.ts create mode 100644 apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useHandleApprovalError.ts diff --git a/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useApprovalAnalytics.ts b/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useApprovalAnalytics.ts new file mode 100644 index 0000000000..87524350c0 --- /dev/null +++ b/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useApprovalAnalytics.ts @@ -0,0 +1,21 @@ +import { useCallback } from 'react' + +import { useCowAnalytics } from '@cowprotocol/analytics' + +import { CowSwapAnalyticsCategory } from 'common/analytics/types' + +export function useApprovalAnalytics(): (action: string, symbol?: string, errorCode?: number | null) => void { + const cowAnalytics = useCowAnalytics() + + return useCallback( + (action: string, symbol?: string, errorCode?: number | null) => { + cowAnalytics.sendEvent({ + category: CowSwapAnalyticsCategory.TRADE, + action, + label: symbol, + ...(errorCode && { value: errorCode }), + }) + }, + [cowAnalytics], + ) +} diff --git a/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useHandleApprovalError.ts b/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useHandleApprovalError.ts new file mode 100644 index 0000000000..3bfb939062 --- /dev/null +++ b/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useHandleApprovalError.ts @@ -0,0 +1,28 @@ +import { useCallback } from 'react' + +import { errorToString, isRejectRequestProviderError } from '@cowprotocol/common-utils' + +import { useApprovalAnalytics } from './useApprovalAnalytics' + +import { useUpdateTradeApproveState } from '../../state' + +export function useHandleApprovalError(symbol: string | undefined): (error: unknown) => void { + const updateTradeApproveState = useUpdateTradeApproveState() + const approvalAnalytics = useApprovalAnalytics() + + return useCallback( + (error: unknown) => { + console.error('Error setting the allowance for token', error) + + if (isRejectRequestProviderError(error)) { + updateTradeApproveState({ error: 'User rejected approval transaction' }) + } else { + const errorCode = + error && typeof error === 'object' && 'code' in error && typeof error.code === 'number' ? error.code : null + approvalAnalytics('Error', symbol, errorCode) + updateTradeApproveState({ error: errorToString(error) }) + } + }, + [updateTradeApproveState, approvalAnalytics, symbol], + ) +} diff --git a/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.ts b/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.ts index 5a274bcc20..6e1c7a32f7 100644 --- a/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.ts +++ b/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.ts @@ -1,105 +1,112 @@ import { useCallback } from 'react' -import { useCowAnalytics } from '@cowprotocol/analytics' import { useTradeSpenderAddress } from '@cowprotocol/balances-and-allowances' import { useFeatureFlags } from '@cowprotocol/common-hooks' -import { errorToString, isRejectRequestProviderError } from '@cowprotocol/common-utils' import { useWalletInfo } from '@cowprotocol/wallet' -import { TransactionReceipt } from '@ethersproject/abstract-provider' +import type { TransactionReceipt, TransactionResponse } from '@ethersproject/abstract-provider' import { Currency } from '@uniswap/sdk-core' import { useSetOptimisticAllowance } from 'entities/optimisticAllowance/useSetOptimisticAllowance' -import { CowSwapAnalyticsCategory } from 'common/analytics/types' - import { processApprovalTransaction } from './approveUtils' +import { useApprovalAnalytics } from './useApprovalAnalytics' +import { useHandleApprovalError } from './useHandleApprovalError' import { useApproveCallback } from '../../hooks' import { useUpdateTradeApproveState } from '../../state' interface TradeApproveCallbackParams { useModals: boolean + waitForTxConfirmation?: boolean +} + +const DEFAULT_APPROVE_PARAMS: TradeApproveCallbackParams = { + useModals: true, + waitForTxConfirmation: false, } -export type TradeApproveResult = { txResponse: TransactionReceipt; approvedAmount: bigint | undefined } +export type TradeApproveResult = { txResponse: R; approvedAmount: bigint | undefined } + +export type GenerecTradeApproveResult = TradeApproveResult | TradeApproveResult export interface TradeApproveCallback { - (amount: bigint, params?: TradeApproveCallbackParams): Promise + ( + amount: bigint, + params?: TradeApproveCallbackParams & { + waitForTxConfirmation?: false + }, + ): Promise | undefined> + + ( + amount: bigint, + params: TradeApproveCallbackParams & { + waitForTxConfirmation: true + }, + ): Promise | undefined> } export function useTradeApproveCallback(currency: Currency | undefined): TradeApproveCallback { + const symbol = currency?.symbol + const updateTradeApproveState = useUpdateTradeApproveState() const spender = useTradeSpenderAddress() - const symbol = currency?.symbol - const cowAnalytics = useCowAnalytics() const { isPartialApproveEnabled } = useFeatureFlags() const { chainId, account } = useWalletInfo() const setOptimisticAllowance = useSetOptimisticAllowance() const approveCallback = useApproveCallback(currency, spender) - - const approvalAnalytics = useCallback( - (action: string, symbol?: string, errorCode?: number | null) => { - cowAnalytics.sendEvent({ - category: CowSwapAnalyticsCategory.TRADE, - action, - label: symbol, - ...(errorCode && { value: errorCode }), - }) - }, - [cowAnalytics], - ) + const approvalAnalytics = useApprovalAnalytics() + const handleApprovalError = useHandleApprovalError(symbol) return useCallback( - async (amount, { useModals = true } = { useModals: true }) => { + async (amount, { useModals = true, waitForTxConfirmation } = DEFAULT_APPROVE_PARAMS) => { if (useModals) { updateTradeApproveState({ currency, approveInProgress: true }) } approvalAnalytics('Send', symbol) - return approveCallback(amount) - .then((response) => { - approvalAnalytics('Sign', symbol) - // if ff is disabled - use old flow, hide modal when tx is sent - !isPartialApproveEnabled && updateTradeApproveState({ currency: undefined, approveInProgress: false }) - return response?.wait().then((txResponse) => { - // Set optimistic allowance immediately after transaction is mined - // Extract the actual approved amount from transaction logs - const approvedAmount = processApprovalTransaction( - { - currency, - account, - spender, - chainId, - }, - txResponse, - ) - - if (approvedAmount) { - setOptimisticAllowance(approvedAmount) - } - - return { txResponse, approvedAmount: approvedAmount?.amount } - }) - }) - .finally(() => { - updateTradeApproveState({ currency: undefined, approveInProgress: false }) - }) - .catch((error) => { - console.error('Error setting the allowance for token', error) - - if (isRejectRequestProviderError(error)) { - updateTradeApproveState({ error: 'User rejected approval transaction' }) - } else { - const errorCode = error?.code && typeof error.code === 'number' ? error.code : null + try { + const response = await approveCallback(amount) - approvalAnalytics('Error', symbol, errorCode) - updateTradeApproveState({ error: errorToString(error) }) + // if ff is disabled - use old flow, hide modal when tx is sent + if (!response || !isPartialApproveEnabled) { + updateTradeApproveState({ currency: undefined, approveInProgress: false }) + return undefined + } + + approvalAnalytics('Sign', symbol) + + if (waitForTxConfirmation) { + // need to wait response to run finally clause after that + const txResponse = await response.wait() + + // Set optimistic allowance immediately after transaction is mined + // Extract the actual approved amount from transaction logs + const approvedAmount = processApprovalTransaction( + { + currency, + account, + spender, + chainId, + }, + txResponse, + ) + + if (approvedAmount) { + setOptimisticAllowance(approvedAmount) } - return undefined - }) + return { txResponse, approvedAmount: approvedAmount?.amount } + } else { + return { txResponse: response, approvedAmount: undefined } + } + } catch (error) { + handleApprovalError(error) + return undefined + } finally { + updateTradeApproveState({ currency: undefined, approveInProgress: false }) + } }, [ symbol, @@ -112,6 +119,7 @@ export function useTradeApproveCallback(currency: Currency | undefined): TradeAp spender, chainId, setOptimisticAllowance, + handleApprovalError, ], - ) + ) as TradeApproveCallback } diff --git a/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveAndSwap.ts b/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveAndSwap.ts index 54b765fd33..812132cb42 100644 --- a/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveAndSwap.ts +++ b/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveAndSwap.ts @@ -1,5 +1,6 @@ import { useCallback } from 'react' +import { TransactionReceipt, TransactionResponse } from '@ethersproject/abstract-provider' import { Currency, CurrencyAmount } from '@uniswap/sdk-core' import { useApproveCurrency } from './useApproveCurrency' @@ -13,7 +14,7 @@ import { getIsTradeApproveResult } from '../utils/getIsTradeApproveResult' export interface ApproveAndSwapProps { amountToApprove: CurrencyAmount - confirmSwap?: () => void + onApproveConfirm?: (transactionHash?: string) => void ignorePermit?: boolean useModals?: boolean } @@ -22,7 +23,7 @@ export function useApproveAndSwap({ amountToApprove, useModals, ignorePermit, - confirmSwap, + onApproveConfirm, }: ApproveAndSwapProps): () => Promise { const isPartialApproveEnabledByUser = useIsPartialApproveSelectedByUser() const handleApprove = useApproveCurrency(amountToApprove, useModals) @@ -31,13 +32,23 @@ export function useApproveAndSwap({ const isPermitSupported = useTokenSupportsPermit(amountToApprove.currency, TradeType.SWAP) && !ignorePermit const generatePermitToTrade = useGeneratePermitInAdvanceToTrade(amountToApprove) - return useCallback(async (): Promise => { - if (isPermitSupported && confirmSwap) { + const handlePermit = useCallback(async () => { + if (isPermitSupported && onApproveConfirm) { const isPermitSigned = await generatePermitToTrade() if (isPermitSigned) { - confirmSwap() + onApproveConfirm() } + return true + } + + return false + }, [isPermitSupported, onApproveConfirm, generatePermitToTrade]) + + return useCallback(async (): Promise => { + const isPermitFlow = await handlePermit() + + if (isPermitFlow) { return } @@ -45,27 +56,29 @@ export function useApproveAndSwap({ const toApprove = isPartialApproveEnabledByUser ? amountToApproveBig : MAX_APPROVE_AMOUNT const tx = await handleApprove(toApprove) - if (tx && confirmSwap) { + if (tx && onApproveConfirm) { if (getIsTradeApproveResult(tx)) { const approvedAmount = tx.approvedAmount const isApprovedAmountSufficient = Boolean(approvedAmount && approvedAmount >= amountToApproveBig) if (isApprovedAmountSufficient) { - confirmSwap() + const hash = + (tx.txResponse as TransactionReceipt).transactionHash || (tx.txResponse as TransactionResponse).hash + + onApproveConfirm(hash) } else { updateTradeApproveState({ error: 'Approved amount is not sufficient!' }) } } else { - confirmSwap() + onApproveConfirm(tx.transactionHash) } } }, [ - isPermitSupported, - confirmSwap, + onApproveConfirm, isPartialApproveEnabledByUser, amountToApprove.quotient, handleApprove, - generatePermitToTrade, updateTradeApproveState, + handlePermit, ]) } diff --git a/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveCurrency.ts b/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveCurrency.ts index b7c7185614..9eb0427d61 100644 --- a/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveCurrency.ts +++ b/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveCurrency.ts @@ -4,13 +4,13 @@ import { Nullish } from '@cowprotocol/types' import { SafeMultisigTransactionResponse } from '@safe-global/safe-core-sdk-types' import { Currency, CurrencyAmount } from '@uniswap/sdk-core' -import { TradeApproveResult, useTradeApproveCallback } from 'modules/erc20Approve' +import { GenerecTradeApproveResult, useTradeApproveCallback } from 'modules/erc20Approve' import { useShouldZeroApprove, useZeroApprove } from 'modules/zeroApproval' export function useApproveCurrency( amountToApprove: CurrencyAmount | undefined, useModals = true, -): (amount: bigint) => Promise> { +): (amount: bigint) => Promise> { const currency = amountToApprove?.currency const tradeApproveCallback = useTradeApproveCallback(currency) diff --git a/apps/cowswap-frontend/src/modules/erc20Approve/utils/getIsTradeApproveResult.ts b/apps/cowswap-frontend/src/modules/erc20Approve/utils/getIsTradeApproveResult.ts index c98d6a2a90..f14d7ad661 100644 --- a/apps/cowswap-frontend/src/modules/erc20Approve/utils/getIsTradeApproveResult.ts +++ b/apps/cowswap-frontend/src/modules/erc20Approve/utils/getIsTradeApproveResult.ts @@ -1,13 +1,13 @@ import { Nullish } from '@cowprotocol/types' import { SafeMultisigTransactionResponse } from '@safe-global/safe-core-sdk-types' -import { TradeApproveResult } from '../containers' +import { GenerecTradeApproveResult } from '../containers' export function getIsTradeApproveResult( - result: Nullish, -): result is TradeApproveResult { + result: Nullish, +): result is GenerecTradeApproveResult { if (!result) return false - const tradeApprove = result as TradeApproveResult + const tradeApprove = result as GenerecTradeApproveResult return !!tradeApprove.txResponse } 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 2c845e9dc7..91b404db24 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 @@ -74,8 +74,7 @@ export function useEthFlowActions(callbacks: EthFlowActionCallbacks, amountToApp ? amountToApprove || MAX_APPROVE_AMOUNT : MAX_APPROVE_AMOUNT return sendTransaction('approve', () => { - return callbacks.approve(unitsToApprove) - .then((res) => res?.txResponse?.transactionHash) + return callbacks.approve(unitsToApprove).then((res) => res?.txResponse.hash) }) } From 54ad06da8e2e0dd5765266fc6b7ae87b6b6ae7c4 Mon Sep 17 00:00:00 2001 From: Alexandr Kazachenko Date: Thu, 16 Oct 2025 18:28:29 +0500 Subject: [PATCH 13/17] chore: fix tests --- .../useTradeApproveCallback.test.ts | 30 ++++--- .../hooks/useApproveAndSwap.test.ts | 86 +++++++++---------- 2 files changed, 63 insertions(+), 53 deletions(-) diff --git a/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.test.ts b/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.test.ts index f01403e2fa..a904ea0714 100644 --- a/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.test.ts +++ b/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.test.ts @@ -141,10 +141,11 @@ describe('useTradeApproveCallback', () => { blockNumber: 123456, chainId: mockChainId, }) + mockUseFeatureFlags.mockReturnValue({ isPartialApproveEnabled: true }) const { result } = renderHook(() => useTradeApproveCallback(mockToken)) - await result.current(mockAmount) + await result.current(mockAmount, { useModals: true, waitForTxConfirmation: true }) await waitFor(() => { expect(mockProcessApprovalTransaction).toHaveBeenCalled() @@ -171,10 +172,11 @@ describe('useTradeApproveCallback', () => { blockNumber: 123456, chainId: mockChainId, }) + mockUseFeatureFlags.mockReturnValue({ isPartialApproveEnabled: true }) const { result } = renderHook(() => useTradeApproveCallback(mockToken)) - await result.current(mockAmount) // Request 1 token + await result.current(mockAmount, { useModals: true, waitForTxConfirmation: true }) // Request 1 token await waitFor(() => { expect(mockProcessApprovalTransaction).toHaveBeenCalled() @@ -193,10 +195,11 @@ describe('useTradeApproveCallback', () => { const mockTxResponse = createMockTransactionResponse(1) mockApproveCallback.mockResolvedValue(mockTxResponse) mockProcessApprovalTransaction.mockReturnValue(null) // No approval data found + mockUseFeatureFlags.mockReturnValue({ isPartialApproveEnabled: true }) const { result } = renderHook(() => useTradeApproveCallback(mockToken)) - await result.current(mockAmount) + await result.current(mockAmount, { useModals: true, waitForTxConfirmation: true }) await waitFor(() => { expect(mockProcessApprovalTransaction).toHaveBeenCalled() @@ -213,10 +216,11 @@ describe('useTradeApproveCallback', () => { const mockTxResponse = createMockTransactionResponse(1) mockApproveCallback.mockResolvedValue(mockTxResponse) mockProcessApprovalTransaction.mockReturnValue(null) + mockUseFeatureFlags.mockReturnValue({ isPartialApproveEnabled: true }) const { result } = renderHook(() => useTradeApproveCallback(mockToken)) - await result.current(mockAmount) + await result.current(mockAmount, { useModals: true, waitForTxConfirmation: true }) await waitFor(() => { expect(mockUpdateTradeApproveState).toHaveBeenCalledWith({ @@ -264,10 +268,11 @@ describe('useTradeApproveCallback', () => { mockTxResponse.wait = mockWait mockApproveCallback.mockResolvedValue(mockTxResponse) mockProcessApprovalTransaction.mockReturnValue(null) + mockUseFeatureFlags.mockReturnValue({ isPartialApproveEnabled: true }) const { result } = renderHook(() => useTradeApproveCallback(mockToken)) - await result.current(mockAmount) + await result.current(mockAmount, { useModals: true, waitForTxConfirmation: true }) await waitFor(() => { expect(mockWait).toHaveBeenCalled() @@ -283,19 +288,22 @@ describe('useTradeApproveCallback', () => { mockProcessApprovalTransaction.mockImplementation(() => { throw new Error('Approval transaction failed') }) + mockUseFeatureFlags.mockReturnValue({ isPartialApproveEnabled: true }) const { result } = renderHook(() => useTradeApproveCallback(mockToken)) - const receipt = await result.current(mockAmount) + const receipt = await result.current(mockAmount, { useModals: true, waitForTxConfirmation: true }) await waitFor(() => { expect(receipt).toBeUndefined() + // Error should be set first expect(mockUpdateTradeApproveState).toHaveBeenCalledWith({ - currency: undefined, - approveInProgress: false, + error: expect.stringContaining('Approval transaction failed'), }) + // Then cleanup in finally block expect(mockUpdateTradeApproveState).toHaveBeenCalledWith({ - error: expect.stringContaining('Approval transaction failed'), + currency: undefined, + approveInProgress: false, }) }) }) @@ -373,11 +381,13 @@ describe('useTradeApproveCallback', () => { await result.current(mockAmount) await waitFor(() => { + // When feature is disabled, only "Send" event is tracked, not "Sign" expect(mockSendEvent).toHaveBeenCalledWith({ category: CowSwapAnalyticsCategory.TRADE, - action: 'Sign', + action: 'Send', label: mockToken.symbol, }) + // Modal should be hidden immediately after response expect(mockUpdateTradeApproveState).toHaveBeenCalledWith({ currency: undefined, approveInProgress: false, diff --git a/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveAndSwap.test.ts b/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveAndSwap.test.ts index 90f45e0f72..6e8837aef2 100644 --- a/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveAndSwap.test.ts +++ b/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveAndSwap.test.ts @@ -9,7 +9,7 @@ import { useGeneratePermitInAdvanceToTrade } from './useGeneratePermitInAdvanceT import { useTokenSupportsPermit } from '../../permit' import { MAX_APPROVE_AMOUNT } from '../constants' -import { TradeApproveResult } from '../containers' +import { TradeApproveResult } from '../containers/TradeApproveModal/useTradeApproveCallback' import { useIsPartialApproveSelectedByUser, useUpdateTradeApproveState } from '../state' jest.mock('./useApproveCurrency') @@ -39,7 +39,7 @@ describe('useApproveAndSwap', () => { // eslint-disable-next-line @typescript-eslint/no-explicit-any } as any - const mockConfirmSwap = jest.fn() + const mockOnApproveConfirm = jest.fn() const mockHandleApprove = jest.fn() const mockGeneratePermitToTrade = jest.fn() const mockUpdateTradeApproveState = jest.fn() @@ -79,14 +79,14 @@ describe('useApproveAndSwap', () => { }) describe('permit flow', () => { - it('should handle successful permit signing and call confirmSwap', async () => { + it('should handle successful permit signing and call onApproveConfirm', async () => { mockUseTokenSupportsPermit.mockReturnValue(true) mockGeneratePermitToTrade.mockResolvedValue(true) const { result } = renderHook(() => useApproveAndSwap({ amountToApprove: mockAmountToApprove, - confirmSwap: mockConfirmSwap, + onApproveConfirm: mockOnApproveConfirm, ignorePermit: false, useModals: true, }), @@ -96,19 +96,19 @@ describe('useApproveAndSwap', () => { await waitFor(() => { expect(mockGeneratePermitToTrade).toHaveBeenCalled() - expect(mockConfirmSwap).toHaveBeenCalled() + expect(mockOnApproveConfirm).toHaveBeenCalled() expect(mockHandleApprove).not.toHaveBeenCalled() }) }) - it('should not call confirmSwap if permit signing fails', async () => { + it('should not call onApproveConfirm if permit signing fails', async () => { mockUseTokenSupportsPermit.mockReturnValue(true) mockGeneratePermitToTrade.mockResolvedValue(false) const { result } = renderHook(() => useApproveAndSwap({ amountToApprove: mockAmountToApprove, - confirmSwap: mockConfirmSwap, + onApproveConfirm: mockOnApproveConfirm, ignorePermit: false, useModals: true, }), @@ -118,19 +118,19 @@ describe('useApproveAndSwap', () => { await waitFor(() => { expect(mockGeneratePermitToTrade).toHaveBeenCalled() - expect(mockConfirmSwap).not.toHaveBeenCalled() + expect(mockOnApproveConfirm).not.toHaveBeenCalled() expect(mockHandleApprove).not.toHaveBeenCalled() }) }) - it('should not call confirmSwap if confirmSwap callback is not provided', async () => { + it('should not call onApproveConfirm if onApproveConfirm callback is not provided', async () => { mockUseTokenSupportsPermit.mockReturnValue(true) mockGeneratePermitToTrade.mockResolvedValue(true) const { result } = renderHook(() => useApproveAndSwap({ amountToApprove: mockAmountToApprove, - confirmSwap: undefined, + onApproveConfirm: undefined, ignorePermit: false, useModals: true, }), @@ -140,14 +140,14 @@ describe('useApproveAndSwap', () => { await waitFor(() => { expect(mockGeneratePermitToTrade).not.toHaveBeenCalled() - expect(mockConfirmSwap).not.toHaveBeenCalled() + expect(mockOnApproveConfirm).not.toHaveBeenCalled() }) }) it('should skip permit flow when ignorePermit is true', async () => { mockUseTokenSupportsPermit.mockReturnValue(true) const mockTxReceipt = createMockTransactionReceipt() - const mockResult: TradeApproveResult = { + const mockResult: TradeApproveResult = { txResponse: mockTxReceipt, approvedAmount: BigInt('2000000000000000000'), } @@ -156,7 +156,7 @@ describe('useApproveAndSwap', () => { const { result } = renderHook(() => useApproveAndSwap({ amountToApprove: mockAmountToApprove, - confirmSwap: mockConfirmSwap, + onApproveConfirm: mockOnApproveConfirm, ignorePermit: true, useModals: true, }), @@ -167,7 +167,7 @@ describe('useApproveAndSwap', () => { await waitFor(() => { expect(mockGeneratePermitToTrade).not.toHaveBeenCalled() expect(mockHandleApprove).toHaveBeenCalled() - expect(mockConfirmSwap).toHaveBeenCalled() + expect(mockOnApproveConfirm).toHaveBeenCalled() }) }) }) @@ -175,7 +175,7 @@ describe('useApproveAndSwap', () => { describe('approval flow with TradeApproveResult', () => { it('should approve and call confirmSwap when approved amount is sufficient', async () => { const mockTxReceipt = createMockTransactionReceipt() - const mockResult: TradeApproveResult = { + const mockResult: TradeApproveResult = { txResponse: mockTxReceipt, approvedAmount: BigInt('2000000000000000000'), // More than required } @@ -184,7 +184,7 @@ describe('useApproveAndSwap', () => { const { result } = renderHook(() => useApproveAndSwap({ amountToApprove: mockAmountToApprove, - confirmSwap: mockConfirmSwap, + onApproveConfirm: mockOnApproveConfirm, ignorePermit: false, useModals: true, }), @@ -194,13 +194,13 @@ describe('useApproveAndSwap', () => { await waitFor(() => { expect(mockHandleApprove).toHaveBeenCalledWith(MAX_APPROVE_AMOUNT) - expect(mockConfirmSwap).toHaveBeenCalled() + expect(mockOnApproveConfirm).toHaveBeenCalled() }) }) it('should approve and call confirmSwap when approved amount equals required amount', async () => { const mockTxReceipt = createMockTransactionReceipt() - const mockResult: TradeApproveResult = { + const mockResult: TradeApproveResult = { txResponse: mockTxReceipt, approvedAmount: mockAmount, // Exactly what's required } @@ -209,7 +209,7 @@ describe('useApproveAndSwap', () => { const { result } = renderHook(() => useApproveAndSwap({ amountToApprove: mockAmountToApprove, - confirmSwap: mockConfirmSwap, + onApproveConfirm: mockOnApproveConfirm, ignorePermit: false, useModals: true, }), @@ -219,13 +219,13 @@ describe('useApproveAndSwap', () => { await waitFor(() => { expect(mockHandleApprove).toHaveBeenCalledWith(MAX_APPROVE_AMOUNT) - expect(mockConfirmSwap).toHaveBeenCalled() + expect(mockOnApproveConfirm).toHaveBeenCalled() }) }) it('should set error state when approved amount is insufficient', async () => { const mockTxReceipt = createMockTransactionReceipt() - const mockResult: TradeApproveResult = { + const mockResult: TradeApproveResult = { txResponse: mockTxReceipt, approvedAmount: BigInt('500000000000000000'), // Less than required } @@ -234,7 +234,7 @@ describe('useApproveAndSwap', () => { const { result } = renderHook(() => useApproveAndSwap({ amountToApprove: mockAmountToApprove, - confirmSwap: mockConfirmSwap, + onApproveConfirm: mockOnApproveConfirm, ignorePermit: false, useModals: true, }), @@ -245,13 +245,13 @@ describe('useApproveAndSwap', () => { await waitFor(() => { expect(mockHandleApprove).toHaveBeenCalledWith(MAX_APPROVE_AMOUNT) expect(mockUpdateTradeApproveState).toHaveBeenCalledWith({ error: 'Approved amount is not sufficient!' }) - expect(mockConfirmSwap).not.toHaveBeenCalled() + expect(mockOnApproveConfirm).not.toHaveBeenCalled() }) }) it('should set error state when approved amount is undefined', async () => { const mockTxReceipt = createMockTransactionReceipt() - const mockResult: TradeApproveResult = { + const mockResult: TradeApproveResult = { txResponse: mockTxReceipt, approvedAmount: undefined, } @@ -260,7 +260,7 @@ describe('useApproveAndSwap', () => { const { result } = renderHook(() => useApproveAndSwap({ amountToApprove: mockAmountToApprove, - confirmSwap: mockConfirmSwap, + onApproveConfirm: mockOnApproveConfirm, ignorePermit: false, useModals: true, }), @@ -271,14 +271,14 @@ describe('useApproveAndSwap', () => { await waitFor(() => { expect(mockHandleApprove).toHaveBeenCalledWith(MAX_APPROVE_AMOUNT) expect(mockUpdateTradeApproveState).toHaveBeenCalledWith({ error: 'Approved amount is not sufficient!' }) - expect(mockConfirmSwap).not.toHaveBeenCalled() + expect(mockOnApproveConfirm).not.toHaveBeenCalled() }) }) it('should use partial approve amount when user has enabled it', async () => { mockUseIsPartialApproveSelectedByUser.mockReturnValue(true) const mockTxReceipt = createMockTransactionReceipt() - const mockResult: TradeApproveResult = { + const mockResult: TradeApproveResult = { txResponse: mockTxReceipt, approvedAmount: mockAmount, } @@ -287,7 +287,7 @@ describe('useApproveAndSwap', () => { const { result } = renderHook(() => useApproveAndSwap({ amountToApprove: mockAmountToApprove, - confirmSwap: mockConfirmSwap, + onApproveConfirm: mockOnApproveConfirm, ignorePermit: false, useModals: true, }), @@ -297,7 +297,7 @@ describe('useApproveAndSwap', () => { await waitFor(() => { expect(mockHandleApprove).toHaveBeenCalledWith(mockAmount) - expect(mockConfirmSwap).toHaveBeenCalled() + expect(mockOnApproveConfirm).toHaveBeenCalled() }) }) }) @@ -309,7 +309,7 @@ describe('useApproveAndSwap', () => { const { result } = renderHook(() => useApproveAndSwap({ amountToApprove: mockAmountToApprove, - confirmSwap: mockConfirmSwap, + onApproveConfirm: mockOnApproveConfirm, ignorePermit: false, useModals: true, }), @@ -319,7 +319,7 @@ describe('useApproveAndSwap', () => { await waitFor(() => { expect(mockHandleApprove).toHaveBeenCalledWith(MAX_APPROVE_AMOUNT) - expect(mockConfirmSwap).not.toHaveBeenCalled() + expect(mockOnApproveConfirm).not.toHaveBeenCalled() }) }) @@ -329,7 +329,7 @@ describe('useApproveAndSwap', () => { const { result } = renderHook(() => useApproveAndSwap({ amountToApprove: mockAmountToApprove, - confirmSwap: mockConfirmSwap, + onApproveConfirm: mockOnApproveConfirm, ignorePermit: false, useModals: true, }), @@ -339,13 +339,13 @@ describe('useApproveAndSwap', () => { await waitFor(() => { expect(mockHandleApprove).toHaveBeenCalledWith(MAX_APPROVE_AMOUNT) - expect(mockConfirmSwap).not.toHaveBeenCalled() + expect(mockOnApproveConfirm).not.toHaveBeenCalled() }) }) it('should not call confirmSwap when confirmSwap callback is not provided', async () => { const mockTxReceipt = createMockTransactionReceipt() - const mockResult: TradeApproveResult = { + const mockResult: TradeApproveResult = { txResponse: mockTxReceipt, approvedAmount: mockAmount, } @@ -354,7 +354,7 @@ describe('useApproveAndSwap', () => { const { result } = renderHook(() => useApproveAndSwap({ amountToApprove: mockAmountToApprove, - confirmSwap: undefined, + onApproveConfirm: undefined, ignorePermit: false, useModals: true, }), @@ -364,7 +364,7 @@ describe('useApproveAndSwap', () => { await waitFor(() => { expect(mockHandleApprove).toHaveBeenCalledWith(MAX_APPROVE_AMOUNT) - expect(mockConfirmSwap).not.toHaveBeenCalled() + expect(mockOnApproveConfirm).not.toHaveBeenCalled() }) }) }) @@ -374,7 +374,7 @@ describe('useApproveAndSwap', () => { renderHook(() => useApproveAndSwap({ amountToApprove: mockAmountToApprove, - confirmSwap: mockConfirmSwap, + onApproveConfirm: mockOnApproveConfirm, ignorePermit: false, useModals: true, }), @@ -387,7 +387,7 @@ describe('useApproveAndSwap', () => { renderHook(() => useApproveAndSwap({ amountToApprove: mockAmountToApprove, - confirmSwap: mockConfirmSwap, + onApproveConfirm: mockOnApproveConfirm, ignorePermit: false, useModals: false, }), @@ -400,7 +400,7 @@ describe('useApproveAndSwap', () => { renderHook(() => useApproveAndSwap({ amountToApprove: mockAmountToApprove, - confirmSwap: mockConfirmSwap, + onApproveConfirm: mockOnApproveConfirm, ignorePermit: false, }), ) @@ -417,7 +417,7 @@ describe('useApproveAndSwap', () => { const { result } = renderHook(() => useApproveAndSwap({ amountToApprove: mockAmountToApprove, - confirmSwap: mockConfirmSwap, + onApproveConfirm: mockOnApproveConfirm, ignorePermit: false, useModals: true, }), @@ -426,7 +426,7 @@ describe('useApproveAndSwap', () => { await expect(result.current()).rejects.toThrow('Approval failed') expect(mockHandleApprove).toHaveBeenCalledWith(MAX_APPROVE_AMOUNT) - expect(mockConfirmSwap).not.toHaveBeenCalled() + expect(mockOnApproveConfirm).not.toHaveBeenCalled() }) it('should propagate errors from generatePermitToTrade', async () => { @@ -437,7 +437,7 @@ describe('useApproveAndSwap', () => { const { result } = renderHook(() => useApproveAndSwap({ amountToApprove: mockAmountToApprove, - confirmSwap: mockConfirmSwap, + onApproveConfirm: mockOnApproveConfirm, ignorePermit: false, useModals: true, }), @@ -447,7 +447,7 @@ describe('useApproveAndSwap', () => { expect(mockGeneratePermitToTrade).toHaveBeenCalled() expect(mockHandleApprove).not.toHaveBeenCalled() - expect(mockConfirmSwap).not.toHaveBeenCalled() + expect(mockOnApproveConfirm).not.toHaveBeenCalled() }) }) }) From 87ca67ad0212945833dd97b995f9b30dff4b4d64 Mon Sep 17 00:00:00 2001 From: limitofzero Date: Mon, 20 Oct 2025 02:28:36 +0400 Subject: [PATCH 14/17] fix: conflicts and tests --- .../useTradeApproveCallback.test.ts | 64 +++++++---- .../useTradeApproveCallback.ts | 69 +++++++---- .../hooks/useApproveAndSwap.test.ts | 8 +- .../useIsApprovalOrPermitRequired.test.ts | 108 +++++++++--------- 4 files changed, 149 insertions(+), 100 deletions(-) diff --git a/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.test.ts b/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.test.ts index a904ea0714..66f976e3d4 100644 --- a/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.test.ts +++ b/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.test.ts @@ -14,7 +14,7 @@ import { processApprovalTransaction } from './approveUtils' import { useTradeApproveCallback } from './useTradeApproveCallback' import { useApproveCallback } from '../../hooks' -import { useUpdateTradeApproveState } from '../../state' +import { useResetApproveProgressModalState, useUpdateApproveProgressModalState } from '../../state' jest.mock('@cowprotocol/analytics', () => ({ useCowAnalytics: jest.fn(), @@ -46,15 +46,19 @@ jest.mock('../../hooks', () => ({ })) jest.mock('../../state', () => ({ - useUpdateTradeApproveState: jest.fn(), + useUpdateApproveProgressModalState: jest.fn(), + useResetApproveProgressModalState: jest.fn(), })) const mockUseCowAnalytics = useCowAnalytics as jest.MockedFunction const mockUseTradeSpenderAddress = useTradeSpenderAddress as jest.MockedFunction const mockUseFeatureFlags = useFeatureFlags as jest.MockedFunction const mockUseApproveCallback = useApproveCallback as jest.MockedFunction -const mockUseUpdateTradeApproveState = useUpdateTradeApproveState as jest.MockedFunction< - typeof useUpdateTradeApproveState +const mockUseUpdateTradeApproveState = useUpdateApproveProgressModalState as jest.MockedFunction< + typeof useUpdateApproveProgressModalState +> +const mockUseResetApproveProgressModalState = useResetApproveProgressModalState as jest.MockedFunction< + typeof useResetApproveProgressModalState > const mockUseWalletInfo = useWalletInfo as jest.MockedFunction const mockUseSetOptimisticAllowance = useSetOptimisticAllowance as jest.MockedFunction @@ -70,6 +74,7 @@ describe('useTradeApproveCallback', () => { const mockSendEvent = jest.fn() const mockUpdateTradeApproveState = jest.fn() + const mockResetApproveProgressModalState = jest.fn() const mockApproveCallback = jest.fn() const mockSetOptimisticAllowance = jest.fn() const mockAccount = '0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266' // Valid address @@ -124,6 +129,7 @@ describe('useTradeApproveCallback', () => { mockUseFeatureFlags.mockReturnValue({ isPartialApproveEnabled: false }) mockUseApproveCallback.mockReturnValue(mockApproveCallback) mockUseUpdateTradeApproveState.mockReturnValue(mockUpdateTradeApproveState) + mockUseResetApproveProgressModalState.mockReturnValue(mockResetApproveProgressModalState) // eslint-disable-next-line @typescript-eslint/no-explicit-any mockUseWalletInfo.mockReturnValue({ chainId: mockChainId, account: mockAccount } as any) mockUseSetOptimisticAllowance.mockReturnValue(mockSetOptimisticAllowance) @@ -204,8 +210,10 @@ describe('useTradeApproveCallback', () => { await waitFor(() => { expect(mockProcessApprovalTransaction).toHaveBeenCalled() expect(mockUpdateTradeApproveState).toHaveBeenCalledWith({ - currency: undefined, + currency: mockToken, approveInProgress: false, + amountToApprove: undefined, + isPendingInProgress: false, }) }) @@ -226,6 +234,7 @@ describe('useTradeApproveCallback', () => { expect(mockUpdateTradeApproveState).toHaveBeenCalledWith({ currency: mockToken, approveInProgress: true, + amountToApprove: expect.any(Object), }) expect(mockApproveCallback).toHaveBeenCalledWith(mockAmount) expect(mockSendEvent).toHaveBeenCalledWith({ @@ -239,8 +248,10 @@ describe('useTradeApproveCallback', () => { label: mockToken.symbol, }) expect(mockUpdateTradeApproveState).toHaveBeenCalledWith({ - currency: undefined, + currency: mockToken, approveInProgress: false, + amountToApprove: undefined, + isPendingInProgress: false, }) }) }) @@ -256,8 +267,10 @@ describe('useTradeApproveCallback', () => { await waitFor(() => { expect(mockUpdateTradeApproveState).toHaveBeenCalledWith({ - currency: undefined, + currency: mockToken, approveInProgress: false, + amountToApprove: undefined, + isPendingInProgress: false, }) }) }) @@ -302,8 +315,10 @@ describe('useTradeApproveCallback', () => { }) // Then cleanup in finally block expect(mockUpdateTradeApproveState).toHaveBeenCalledWith({ - currency: undefined, + currency: mockToken, approveInProgress: false, + amountToApprove: undefined, + isPendingInProgress: false, }) }) }) @@ -322,8 +337,10 @@ describe('useTradeApproveCallback', () => { error: 'User rejected approval transaction', }) expect(mockUpdateTradeApproveState).toHaveBeenCalledWith({ - currency: undefined, + currency: mockToken, approveInProgress: false, + amountToApprove: undefined, + isPendingInProgress: false, }) }) }) @@ -388,10 +405,7 @@ describe('useTradeApproveCallback', () => { label: mockToken.symbol, }) // Modal should be hidden immediately after response - expect(mockUpdateTradeApproveState).toHaveBeenCalledWith({ - currency: undefined, - approveInProgress: false, - }) + expect(mockResetApproveProgressModalState).toHaveBeenCalled() }) }) @@ -411,10 +425,9 @@ describe('useTradeApproveCallback', () => { action: 'Sign', label: mockToken.symbol, }) - const callsBeforeFinally = mockUpdateTradeApproveState.mock.calls.filter( - (call) => call[0].currency === undefined && call[0].approveInProgress === false, - ) - expect(callsBeforeFinally).toHaveLength(1) + expect(mockUpdateTradeApproveState).toHaveBeenCalledWith({ + isPendingInProgress: true, + }) }) }) }) @@ -433,6 +446,7 @@ describe('useTradeApproveCallback', () => { expect(mockUpdateTradeApproveState).toHaveBeenCalledWith({ currency: mockToken, approveInProgress: true, + amountToApprove: expect.any(Object), }) }) }) @@ -463,8 +477,10 @@ describe('useTradeApproveCallback', () => { await waitFor(() => { expect(mockUpdateTradeApproveState).toHaveBeenCalledWith({ - currency: undefined, + currency: mockToken, approveInProgress: false, + amountToApprove: undefined, + isPendingInProgress: false, }) }) }) @@ -482,8 +498,10 @@ describe('useTradeApproveCallback', () => { await waitFor(() => { expect(mockUpdateTradeApproveState).toHaveBeenCalledWith({ - currency: undefined, + currency: mockToken, approveInProgress: false, + amountToApprove: undefined, + isPendingInProgress: false, }) }) }) @@ -497,8 +515,10 @@ describe('useTradeApproveCallback', () => { await waitFor(() => { expect(mockUpdateTradeApproveState).toHaveBeenCalledWith({ - currency: undefined, + currency: mockToken, approveInProgress: false, + amountToApprove: undefined, + isPendingInProgress: false, }) }) }) @@ -517,8 +537,10 @@ describe('useTradeApproveCallback', () => { await waitFor(() => { expect(mockUpdateTradeApproveState).toHaveBeenCalledWith({ - currency: undefined, + currency: mockToken, approveInProgress: false, + amountToApprove: undefined, + isPendingInProgress: false, }) }) }) diff --git a/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.ts b/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.ts index dfd075764c..cc8bae52dd 100644 --- a/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.ts +++ b/apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.ts @@ -15,6 +15,46 @@ import { useHandleApprovalError } from './useHandleApprovalError' import { useApproveCallback } from '../../hooks' import { useResetApproveProgressModalState, useUpdateApproveProgressModalState } from '../../state' +interface ProcessTransactionConfirmationParams { + response: TransactionResponse + currency: Currency | undefined + account: string | undefined + spender: string | undefined + chainId: number | undefined + setOptimisticAllowance: (data: { tokenAddress: string; owner: string; spender: string; amount: bigint; blockNumber: number; chainId: number }) => void +} + +async function processTransactionConfirmation({ + response, + currency, + account, + spender, + chainId, + setOptimisticAllowance, +}: ProcessTransactionConfirmationParams): Promise> { + const txResponse = await response.wait() + + if (!chainId) { + return { txResponse, approvedAmount: undefined } + } + + const approvedAmount = processApprovalTransaction( + { + currency, + account, + spender, + chainId, + }, + txResponse, + ) + + if (approvedAmount) { + setOptimisticAllowance(approvedAmount) + } + + return { txResponse, approvedAmount: approvedAmount?.amount } +} + interface TradeApproveCallbackParams { useModals: boolean waitForTxConfirmation?: boolean @@ -60,7 +100,6 @@ export function useTradeApproveCallback(currency: Currency | undefined): TradeAp const handleApprovalError = useHandleApprovalError(symbol) return useCallback( - // eslint-disable-next-line complexity async (amount, { useModals = true, waitForTxConfirmation } = DEFAULT_APPROVE_PARAMS) => { if (useModals) { const amountToApprove = currency ? CurrencyAmount.fromRawAmount(currency, amount.toString()) : undefined @@ -83,26 +122,14 @@ export function useTradeApproveCallback(currency: Currency | undefined): TradeAp approvalAnalytics('Sign', symbol) if (waitForTxConfirmation) { - // need to wait response to run finally clause after that - const txResponse = await response.wait() - - // Set optimistic allowance immediately after transaction is mined - // Extract the actual approved amount from transaction logs - const approvedAmount = processApprovalTransaction( - { - currency, - account, - spender, - chainId, - }, - txResponse, - ) - - if (approvedAmount) { - setOptimisticAllowance(approvedAmount) - } - - return { txResponse, approvedAmount: approvedAmount?.amount } + return await processTransactionConfirmation({ + response, + currency, + account, + spender, + chainId, + setOptimisticAllowance, + }) } else { return { txResponse: response, approvedAmount: undefined } } diff --git a/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveAndSwap.test.ts b/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveAndSwap.test.ts index 6e8837aef2..f4d0cd4138 100644 --- a/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveAndSwap.test.ts +++ b/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useApproveAndSwap.test.ts @@ -9,8 +9,8 @@ import { useGeneratePermitInAdvanceToTrade } from './useGeneratePermitInAdvanceT import { useTokenSupportsPermit } from '../../permit' import { MAX_APPROVE_AMOUNT } from '../constants' -import { TradeApproveResult } from '../containers/TradeApproveModal/useTradeApproveCallback' -import { useIsPartialApproveSelectedByUser, useUpdateTradeApproveState } from '../state' +import { TradeApproveResult } from '../containers' +import { useIsPartialApproveSelectedByUser, useUpdateApproveProgressModalState } from '../state' jest.mock('./useApproveCurrency') jest.mock('./useGeneratePermitInAdvanceToTrade') @@ -25,8 +25,8 @@ const mockUseTokenSupportsPermit = useTokenSupportsPermit as jest.MockedFunction const mockUseIsPartialApproveSelectedByUser = useIsPartialApproveSelectedByUser as jest.MockedFunction< typeof useIsPartialApproveSelectedByUser > -const mockUseUpdateTradeApproveState = useUpdateTradeApproveState as jest.MockedFunction< - typeof useUpdateTradeApproveState +const mockUseUpdateTradeApproveState = useUpdateApproveProgressModalState as jest.MockedFunction< + typeof useUpdateApproveProgressModalState > // eslint-disable-next-line max-lines-per-function diff --git a/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useIsApprovalOrPermitRequired.test.ts b/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useIsApprovalOrPermitRequired.test.ts index 5d261db0c0..2c25d86c79 100644 --- a/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useIsApprovalOrPermitRequired.test.ts +++ b/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useIsApprovalOrPermitRequired.test.ts @@ -87,9 +87,9 @@ describe('useIsApprovalOrPermitRequired', () => { currentAllowance: BigInt(0), }) - const { result } = renderHook(() => useIsApprovalOrPermitRequired().reason) + const { result } = renderHook(() => useIsApprovalOrPermitRequired()) - expect(result.current).toBe(ApproveRequiredReason.Required) + expect(result.current.reason).toBe(ApproveRequiredReason.Required) }) it('should return Required when approval state is PENDING', () => { @@ -98,9 +98,9 @@ describe('useIsApprovalOrPermitRequired', () => { currentAllowance: BigInt(0), }) - const { result } = renderHook(() => useIsApprovalOrPermitRequired().reason) + const { result } = renderHook(() => useIsApprovalOrPermitRequired()) - expect(result.current).toBe(ApproveRequiredReason.Required) + expect(result.current.reason).toBe(ApproveRequiredReason.Required) }) it('should return NotRequired when approval state is APPROVED', () => { @@ -109,9 +109,9 @@ describe('useIsApprovalOrPermitRequired', () => { currentAllowance: BigInt(1000000000000000000), }) - const { result } = renderHook(() => useIsApprovalOrPermitRequired().reason) + const { result } = renderHook(() => useIsApprovalOrPermitRequired()) - expect(result.current).toBe(ApproveRequiredReason.NotRequired) + expect(result.current.reason).toBe(ApproveRequiredReason.NotRequired) }) }) @@ -123,9 +123,9 @@ describe('useIsApprovalOrPermitRequired', () => { }), ) - const { result } = renderHook(() => useIsApprovalOrPermitRequired().reason) + const { result } = renderHook(() => useIsApprovalOrPermitRequired()) - expect(result.current).toBe(ApproveRequiredReason.NotRequired) + expect(result.current.reason).toBe(ApproveRequiredReason.NotRequired) }) it('should return NotRequired for ADVANCED_ORDERS', () => { @@ -135,9 +135,9 @@ describe('useIsApprovalOrPermitRequired', () => { }), ) - const { result } = renderHook(() => useIsApprovalOrPermitRequired().reason) + const { result } = renderHook(() => useIsApprovalOrPermitRequired()) - expect(result.current).toBe(ApproveRequiredReason.NotRequired) + expect(result.current.reason).toBe(ApproveRequiredReason.NotRequired) }) it('should return NotRequired for YIELD', () => { @@ -147,9 +147,9 @@ describe('useIsApprovalOrPermitRequired', () => { }), ) - const { result } = renderHook(() => useIsApprovalOrPermitRequired().reason) + const { result } = renderHook(() => useIsApprovalOrPermitRequired()) - expect(result.current).toBe(ApproveRequiredReason.NotRequired) + expect(result.current.reason).toBe(ApproveRequiredReason.NotRequired) }) }) @@ -157,9 +157,9 @@ describe('useIsApprovalOrPermitRequired', () => { it('should return NotRequired when isPartialApproveEnabled is false', () => { mockUseFeatureFlags.mockReturnValue({ isPartialApproveEnabled: false }) - const { result } = renderHook(() => useIsApprovalOrPermitRequired().reason) + const { result } = renderHook(() => useIsApprovalOrPermitRequired()) - expect(result.current).toBe(ApproveRequiredReason.NotRequired) + expect(result.current.reason).toBe(ApproveRequiredReason.NotRequired) }) }) @@ -167,25 +167,25 @@ describe('useIsApprovalOrPermitRequired', () => { it('should return Eip2612PermitRequired for eip-2612 permit type', () => { mockUsePermitInfo.mockReturnValue({ type: 'eip-2612' }) - const { result } = renderHook(() => useIsApprovalOrPermitRequired().reason) + const { result } = renderHook(() => useIsApprovalOrPermitRequired()) - expect(result.current).toBe(ApproveRequiredReason.Eip2612PermitRequired) + expect(result.current.reason).toBe(ApproveRequiredReason.Eip2612PermitRequired) }) it('should return DaiLikePermitRequired for dai-like permit type', () => { mockUsePermitInfo.mockReturnValue({ type: 'dai-like' }) - const { result } = renderHook(() => useIsApprovalOrPermitRequired().reason) + const { result } = renderHook(() => useIsApprovalOrPermitRequired()) - expect(result.current).toBe(ApproveRequiredReason.DaiLikePermitRequired) + expect(result.current.reason).toBe(ApproveRequiredReason.DaiLikePermitRequired) }) it('should return NotRequired for unsupported permit type', () => { mockUsePermitInfo.mockReturnValue({ type: 'unsupported' }) - const { result } = renderHook(() => useIsApprovalOrPermitRequired().reason) + const { result } = renderHook(() => useIsApprovalOrPermitRequired()) - expect(result.current).toBe(ApproveRequiredReason.NotRequired) + expect(result.current.reason).toBe(ApproveRequiredReason.NotRequired) }) }) @@ -193,17 +193,17 @@ describe('useIsApprovalOrPermitRequired', () => { it('should handle undefined trade state', () => { mockUseDerivedTradeState.mockReturnValue(null) - const { result } = renderHook(() => useIsApprovalOrPermitRequired().reason) + const { result } = renderHook(() => useIsApprovalOrPermitRequired()) - expect(result.current).toBe(ApproveRequiredReason.NotRequired) + expect(result.current.reason).toBe(ApproveRequiredReason.NotRequired) }) it('should handle undefined permit info', () => { mockUsePermitInfo.mockReturnValue(undefined) - const { result } = renderHook(() => useIsApprovalOrPermitRequired().reason) + const { result } = renderHook(() => useIsApprovalOrPermitRequired()) - expect(result.current).toBe(ApproveRequiredReason.NotRequired) + expect(result.current.reason).toBe(ApproveRequiredReason.NotRequired) }) it('should handle null amount to approve', () => { @@ -213,9 +213,9 @@ describe('useIsApprovalOrPermitRequired', () => { currentAllowance: BigInt(0), }) - const { result } = renderHook(() => useIsApprovalOrPermitRequired().reason) + const { result } = renderHook(() => useIsApprovalOrPermitRequired()) - expect(result.current).toBe(ApproveRequiredReason.Required) + expect(result.current.reason).toBe(ApproveRequiredReason.Required) }) it('should handle zero amount to approve', () => { @@ -225,9 +225,9 @@ describe('useIsApprovalOrPermitRequired', () => { currentAllowance: BigInt(0), }) - const { result } = renderHook(() => useIsApprovalOrPermitRequired().reason) + const { result } = renderHook(() => useIsApprovalOrPermitRequired()) - expect(result.current).toBe(ApproveRequiredReason.NotRequired) + expect(result.current.reason).toBe(ApproveRequiredReason.NotRequired) }) it('should handle undefined input currency', () => { @@ -237,9 +237,9 @@ describe('useIsApprovalOrPermitRequired', () => { }), ) - const { result } = renderHook(() => useIsApprovalOrPermitRequired().reason) + const { result } = renderHook(() => useIsApprovalOrPermitRequired()) - expect(result.current).toBe(ApproveRequiredReason.NotRequired) + expect(result.current.reason).toBe(ApproveRequiredReason.NotRequired) }) it('should handle undefined trade type', () => { @@ -249,9 +249,9 @@ describe('useIsApprovalOrPermitRequired', () => { }), ) - const { result } = renderHook(() => useIsApprovalOrPermitRequired().reason) + const { result } = renderHook(() => useIsApprovalOrPermitRequired()) - expect(result.current).toBe(ApproveRequiredReason.NotRequired) + expect(result.current.reason).toBe(ApproveRequiredReason.NotRequired) }) }) @@ -263,9 +263,9 @@ describe('useIsApprovalOrPermitRequired', () => { }) mockUsePermitInfo.mockReturnValue({ type: 'eip-2612' }) - const { result } = renderHook(() => useIsApprovalOrPermitRequired().reason) + const { result } = renderHook(() => useIsApprovalOrPermitRequired()) - expect(result.current).toBe(ApproveRequiredReason.Eip2612PermitRequired) + expect(result.current.reason).toBe(ApproveRequiredReason.Eip2612PermitRequired) }) it('should fall back to approval when permit is not supported but approval is needed', () => { @@ -275,9 +275,9 @@ describe('useIsApprovalOrPermitRequired', () => { }) mockUsePermitInfo.mockReturnValue({ type: 'unsupported' }) - const { result } = renderHook(() => useIsApprovalOrPermitRequired().reason) + const { result } = renderHook(() => useIsApprovalOrPermitRequired()) - expect(result.current).toBe(ApproveRequiredReason.Required) + expect(result.current.reason).toBe(ApproveRequiredReason.Required) }) it('should return NotRequired when both permit and approval are not needed', () => { @@ -287,29 +287,29 @@ describe('useIsApprovalOrPermitRequired', () => { }) mockUsePermitInfo.mockReturnValue({ type: 'unsupported' }) - const { result } = renderHook(() => useIsApprovalOrPermitRequired().reason) + const { result } = renderHook(() => useIsApprovalOrPermitRequired()) - expect(result.current).toBe(ApproveRequiredReason.NotRequired) + expect(result.current.reason).toBe(ApproveRequiredReason.NotRequired) }) }) describe('getPermitRequirements function', () => { it('should return correct permit requirements for different permit types', () => { mockUsePermitInfo.mockReturnValue({ type: 'eip-2612' }) - const { result: result1 } = renderHook(() => useIsApprovalOrPermitRequired().reason) - expect(result1.current).toBe(ApproveRequiredReason.Eip2612PermitRequired) + const { result: result1 } = renderHook(() => useIsApprovalOrPermitRequired()) + expect(result1.current.reason).toBe(ApproveRequiredReason.Eip2612PermitRequired) mockUsePermitInfo.mockReturnValue({ type: 'dai-like' }) - const { result: result2 } = renderHook(() => useIsApprovalOrPermitRequired().reason) - expect(result2.current).toBe(ApproveRequiredReason.DaiLikePermitRequired) + const { result: result2 } = renderHook(() => useIsApprovalOrPermitRequired()) + expect(result2.current.reason).toBe(ApproveRequiredReason.DaiLikePermitRequired) mockUsePermitInfo.mockReturnValue({ type: 'unsupported' }) - const { result: result3 } = renderHook(() => useIsApprovalOrPermitRequired().reason) - expect(result3.current).toBe(ApproveRequiredReason.NotRequired) + const { result: result3 } = renderHook(() => useIsApprovalOrPermitRequired()) + expect(result3.current.reason).toBe(ApproveRequiredReason.NotRequired) mockUsePermitInfo.mockReturnValue(undefined) - const { result: result4 } = renderHook(() => useIsApprovalOrPermitRequired().reason) - expect(result4.current).toBe(ApproveRequiredReason.NotRequired) + const { result: result4 } = renderHook(() => useIsApprovalOrPermitRequired()) + expect(result4.current.reason).toBe(ApproveRequiredReason.NotRequired) }) }) @@ -324,9 +324,9 @@ describe('useIsApprovalOrPermitRequired', () => { ) mockUsePermitInfo.mockReturnValue({ type: 'eip-2612' }) - const { result } = renderHook(() => useIsApprovalOrPermitRequired().reason) + const { result } = renderHook(() => useIsApprovalOrPermitRequired()) - expect(result.current).toBe(ApproveRequiredReason.Eip2612PermitRequired) + expect(result.current.reason).toBe(ApproveRequiredReason.Eip2612PermitRequired) }) it('should handle SWAP trade type with partial approve disabled', () => { @@ -339,9 +339,9 @@ describe('useIsApprovalOrPermitRequired', () => { ) mockUsePermitInfo.mockReturnValue({ type: 'eip-2612' }) - const { result } = renderHook(() => useIsApprovalOrPermitRequired().reason) + const { result } = renderHook(() => useIsApprovalOrPermitRequired()) - expect(result.current).toBe(ApproveRequiredReason.NotRequired) + expect(result.current.reason).toBe(ApproveRequiredReason.NotRequired) }) it('should handle non-SWAP trade type with partial approve enabled', () => { @@ -354,9 +354,9 @@ describe('useIsApprovalOrPermitRequired', () => { ) mockUsePermitInfo.mockReturnValue({ type: 'eip-2612' }) - const { result } = renderHook(() => useIsApprovalOrPermitRequired().reason) + const { result } = renderHook(() => useIsApprovalOrPermitRequired()) - expect(result.current).toBe(ApproveRequiredReason.NotRequired) + expect(result.current.reason).toBe(ApproveRequiredReason.NotRequired) }) it('should handle approval state UNKNOWN', () => { @@ -366,9 +366,9 @@ describe('useIsApprovalOrPermitRequired', () => { }) mockUsePermitInfo.mockReturnValue({ type: 'unsupported' }) - const { result } = renderHook(() => useIsApprovalOrPermitRequired().reason) + const { result } = renderHook(() => useIsApprovalOrPermitRequired()) - expect(result.current).toBe(ApproveRequiredReason.NotRequired) + expect(result.current.reason).toBe(ApproveRequiredReason.NotRequired) }) }) }) From 88466bbd7bd3d04780c41afd31f91d29fd1360bc Mon Sep 17 00:00:00 2001 From: Alexandr Kazachenko Date: Mon, 20 Oct 2025 13:05:09 +0200 Subject: [PATCH 15/17] chore: fix build --- .../hooks/useIsApprovalOrPermitRequired.test.ts | 16 ++++++++-------- .../hooks/useIsApprovalOrPermitRequired.ts | 1 + 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useIsApprovalOrPermitRequired.test.ts b/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useIsApprovalOrPermitRequired.test.ts index c7d3ba0177..fc07444524 100644 --- a/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useIsApprovalOrPermitRequired.test.ts +++ b/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useIsApprovalOrPermitRequired.test.ts @@ -175,7 +175,7 @@ describe('useIsApprovalOrPermitRequired', () => { useIsApprovalOrPermitRequired({ isBundlingSupportedOrEnabledForContext: null }), ) - expect(result.current).toBe(ApproveRequiredReason.NotRequired) + expect(result.current.reason).toBe(ApproveRequiredReason.NotRequired) }) }) @@ -188,7 +188,7 @@ describe('useIsApprovalOrPermitRequired', () => { useIsApprovalOrPermitRequired({ isBundlingSupportedOrEnabledForContext: null }), ) - expect(result.current).toBe(ApproveRequiredReason.NotRequired) + expect(result.current.reason).toBe(ApproveRequiredReason.NotRequired) }) it('should return NotRequired for native token even with zero amount', () => { @@ -199,7 +199,7 @@ describe('useIsApprovalOrPermitRequired', () => { useIsApprovalOrPermitRequired({ isBundlingSupportedOrEnabledForContext: null }), ) - expect(result.current).toBe(ApproveRequiredReason.NotRequired) + expect(result.current.reason).toBe(ApproveRequiredReason.NotRequired) }) it('should return NotRequired for native token even when bundling is enabled', () => { @@ -226,7 +226,7 @@ describe('useIsApprovalOrPermitRequired', () => { useIsApprovalOrPermitRequired({ isBundlingSupportedOrEnabledForContext: true }), ) - expect(result.current).toBe(ApproveRequiredReason.BundleApproveRequired) + expect(result.current.reason).toBe(ApproveRequiredReason.BundleApproveRequired) }) it('should return BundleApproveRequired when bundling is enabled regardless of permit support', () => { @@ -236,7 +236,7 @@ describe('useIsApprovalOrPermitRequired', () => { useIsApprovalOrPermitRequired({ isBundlingSupportedOrEnabledForContext: true }), ) - expect(result.current).toBe(ApproveRequiredReason.BundleApproveRequired) + expect(result.current.reason).toBe(ApproveRequiredReason.BundleApproveRequired) }) it('should return BundleApproveRequired when bundling is enabled and permit is not supported', () => { @@ -250,7 +250,7 @@ describe('useIsApprovalOrPermitRequired', () => { useIsApprovalOrPermitRequired({ isBundlingSupportedOrEnabledForContext: true }), ) - expect(result.current).toBe(ApproveRequiredReason.BundleApproveRequired) + expect(result.current.reason).toBe(ApproveRequiredReason.BundleApproveRequired) }) }) @@ -282,7 +282,7 @@ describe('useIsApprovalOrPermitRequired', () => { useIsApprovalOrPermitRequired({ isBundlingSupportedOrEnabledForContext: null }), ) - expect(result.current).toBe(ApproveRequiredReason.NotRequired) + expect(result.current.reason).toBe(ApproveRequiredReason.NotRequired) }) it('should return NotRequired for undefined permit type', () => { @@ -292,7 +292,7 @@ describe('useIsApprovalOrPermitRequired', () => { useIsApprovalOrPermitRequired({ isBundlingSupportedOrEnabledForContext: null }), ) - expect(result.current).toBe(ApproveRequiredReason.NotRequired) + expect(result.current.reason).toBe(ApproveRequiredReason.NotRequired) }) it('should return NotRequired for null permit type', () => { diff --git a/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useIsApprovalOrPermitRequired.ts b/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useIsApprovalOrPermitRequired.ts index dc30396bd8..cd3be2af02 100644 --- a/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useIsApprovalOrPermitRequired.ts +++ b/apps/cowswap-frontend/src/modules/erc20Approve/hooks/useIsApprovalOrPermitRequired.ts @@ -4,6 +4,7 @@ import { useFeatureFlags } from '@cowprotocol/common-hooks' import { getIsNativeToken } from '@cowprotocol/common-utils' import { PermitType } from '@cowprotocol/permit-utils' import { Nullish } from '@cowprotocol/types' +import { Currency, CurrencyAmount } from '@uniswap/sdk-core' import { usePermitInfo } from 'modules/permit' import { TradeType, useDerivedTradeState } from 'modules/trade' From 80b43bc6e7821947011f24729ff233d1e1be608e Mon Sep 17 00:00:00 2001 From: Alexandr Kazachenko Date: Mon, 20 Oct 2025 16:56:25 +0200 Subject: [PATCH 16/17] chore: fix confirm button text --- .../swap/hooks/useGetConfirmButtonLabel.ts | 26 +++++++++---------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/apps/cowswap-frontend/src/modules/swap/hooks/useGetConfirmButtonLabel.ts b/apps/cowswap-frontend/src/modules/swap/hooks/useGetConfirmButtonLabel.ts index af59e7c9fd..84bbd0ae5d 100644 --- a/apps/cowswap-frontend/src/modules/swap/hooks/useGetConfirmButtonLabel.ts +++ b/apps/cowswap-frontend/src/modules/swap/hooks/useGetConfirmButtonLabel.ts @@ -1,24 +1,22 @@ -import { useFeatureFlags } from '@cowprotocol/common-hooks' - -import { useIsApprovalOrPermitRequired } from 'modules/erc20Approve' +import { ApproveRequiredReason, useIsApprovalOrPermitRequired } from 'modules/erc20Approve' import { useIsCurrentTradeBridging } from 'modules/trade' -function getSwapLabel(isBridging: boolean): string { - return isBridging ? 'Swap and Bridge' : 'Swap' +enum ConfirmButtonLabel { + SWAP = 'Swap', + SWAP_BRIDGE = 'Swap and Bridge', + APPROVE_SWAP = 'Approve and Swap', + APPROVE_SWAP_BRIDGE = 'Approve, Swap & Bridge', } export function useGetConfirmButtonLabel(): string { const isCurrentTradeBridging = useIsCurrentTradeBridging() - const isPermitOrApproveRequired = useIsApprovalOrPermitRequired({ isBundlingSupportedOrEnabledForContext: false }) - const { isPartialApproveEnabled } = useFeatureFlags() - - if (!isPartialApproveEnabled) { - return getSwapLabel(isCurrentTradeBridging) - } + const { reason: approveRequiredReason } = useIsApprovalOrPermitRequired({ + isBundlingSupportedOrEnabledForContext: false, + }) - if (isPermitOrApproveRequired) { - return isCurrentTradeBridging ? 'Approve, Swap & Bridge' : 'Approve and Swap' + if (approveRequiredReason === ApproveRequiredReason.Required) { + return isCurrentTradeBridging ? ConfirmButtonLabel.APPROVE_SWAP_BRIDGE : ConfirmButtonLabel.APPROVE_SWAP } - return getSwapLabel(isCurrentTradeBridging) + return isCurrentTradeBridging ? ConfirmButtonLabel.SWAP_BRIDGE : ConfirmButtonLabel.SWAP } From a4582b91929d6c2e72af4e8201d6649cb2a118d6 Mon Sep 17 00:00:00 2001 From: Alexandr Kazachenko Date: Mon, 20 Oct 2025 18:20:50 +0200 Subject: [PATCH 17/17] fix(approve): fix approve button text --- .../OrderPartialApprove.tsx | 1 - .../TradeApproveButton/TradeApproveButton.tsx | 39 ++++++++++++------- .../permit/hooks/useGetCachedPermit.ts | 1 + .../permit/hooks/useHasCachedPermit.ts | 18 +++++++++ .../src/modules/permit/index.ts | 1 + .../modules/permit/state/permitCacheAtom.ts | 10 +++-- .../swap/containers/TradeButtons/index.tsx | 11 ++++-- .../swap/hooks/useGetConfirmButtonLabel.ts | 22 ----------- .../pure/TradeFormButtons/tradeButtonsMap.tsx | 1 - 9 files changed, 59 insertions(+), 45 deletions(-) create mode 100644 apps/cowswap-frontend/src/modules/permit/hooks/useHasCachedPermit.ts delete mode 100644 apps/cowswap-frontend/src/modules/swap/hooks/useGetConfirmButtonLabel.ts diff --git a/apps/cowswap-frontend/src/modules/account/containers/OrderPartialApprove/OrderPartialApprove.tsx b/apps/cowswap-frontend/src/modules/account/containers/OrderPartialApprove/OrderPartialApprove.tsx index b52d66742d..b2afbc976b 100644 --- a/apps/cowswap-frontend/src/modules/account/containers/OrderPartialApprove/OrderPartialApprove.tsx +++ b/apps/cowswap-frontend/src/modules/account/containers/OrderPartialApprove/OrderPartialApprove.tsx @@ -48,7 +48,6 @@ export function OrderPartialApprove({ )} void - ignorePermit?: boolean - label: string + label?: string buttonSize?: ButtonSize useModals?: boolean } @@ -33,7 +35,6 @@ export function TradeApproveButton(props: TradeApproveButtonProps): ReactNode { amountToApprove, children, enablePartialApprove, - label, isDisabled, buttonSize = ButtonSize.DEFAULT, useModals = true, @@ -41,9 +42,11 @@ export function TradeApproveButton(props: TradeApproveButtonProps): ReactNode { const handleApprove = useApproveCurrency(amountToApprove, useModals) const spender = useTradeSpenderAddress() + const isCurrentTradeBridging = useIsCurrentTradeBridging() const { approvalState } = useApprovalStateForSpender(amountToApprove, spender) const approveAndSwap = useApproveAndSwap(props) const approveWithPreventedDoubleExecution = usePreventDoubleExecution(approveAndSwap) + const { data: cachedPermit, isLoading: cachedPermitLoading } = useHasCachedPermit(amountToApprove) if (!enablePartialApprove) { return ( @@ -60,6 +63,10 @@ export function TradeApproveButton(props: TradeApproveButtonProps): ReactNode { } const isPending = approvalState === ApprovalState.PENDING + const noCachedPermit = !cachedPermitLoading && !cachedPermit + + const label = + props.label || (noCachedPermit ? (isCurrentTradeBridging ? 'Approve, Swap & Bridge' : 'Approve and Swap') : 'Swap') return ( {label}{' '} - - You must give the CoW Protocol smart contracts permission to use your{' '} - . If you approve the default amount, you will only have to - do this once per token. - - } - > - {isPending ? : } - + {noCachedPermit ? ( + + You must give the CoW Protocol smart contracts permission to use your{' '} + . If you approve the default amount, you will only have + to do this once per token. + + } + > + {isPending ? : } + + ) : null} ) diff --git a/apps/cowswap-frontend/src/modules/permit/hooks/useGetCachedPermit.ts b/apps/cowswap-frontend/src/modules/permit/hooks/useGetCachedPermit.ts index 22a2c8a080..6d036a71e3 100644 --- a/apps/cowswap-frontend/src/modules/permit/hooks/useGetCachedPermit.ts +++ b/apps/cowswap-frontend/src/modules/permit/hooks/useGetCachedPermit.ts @@ -29,6 +29,7 @@ export function useGetCachedPermit(): ( try { const eip2612Utils = getPermitUtilsInstance(chainId, provider, account) + // TODO: it might add unwanted node RPC requests // Always get the nonce for the real account, to know whether the cache should be invalidated // Static account should never need to pre-check the nonce as it'll never change once cached const nonce = account ? await eip2612Utils.getTokenNonce(tokenAddress, account) : undefined diff --git a/apps/cowswap-frontend/src/modules/permit/hooks/useHasCachedPermit.ts b/apps/cowswap-frontend/src/modules/permit/hooks/useHasCachedPermit.ts new file mode 100644 index 0000000000..683149cf4e --- /dev/null +++ b/apps/cowswap-frontend/src/modules/permit/hooks/useHasCachedPermit.ts @@ -0,0 +1,18 @@ +import { getWrappedToken } from '@cowprotocol/common-utils' +import { PermitHookData } from '@cowprotocol/permit-utils' +import { Currency, CurrencyAmount } from '@uniswap/sdk-core' + +import useSWR, { SWRResponse } from 'swr' + +import { useGetCachedPermit } from './useGetCachedPermit' + +export function useHasCachedPermit(amountToApprove: CurrencyAmount): SWRResponse { + const getCachedPermit = useGetCachedPermit() + const token = getWrappedToken(amountToApprove.currency) + const tokenAddress = token.address + const amount = BigInt(amountToApprove.quotient.toString()) + + return useSWR([tokenAddress, amount, token.chainId, 'useHasCachedPermit'], ([tokenAddress, amount]) => { + return getCachedPermit(tokenAddress, amount) + }) +} diff --git a/apps/cowswap-frontend/src/modules/permit/index.ts b/apps/cowswap-frontend/src/modules/permit/index.ts index cb734a99e6..a6cca91903 100644 --- a/apps/cowswap-frontend/src/modules/permit/index.ts +++ b/apps/cowswap-frontend/src/modules/permit/index.ts @@ -3,6 +3,7 @@ export * from './hooks/useGeneratePermitHook' export * from './hooks/usePermitInfo' export * from './hooks/usePermitCompatibleTokens' export * from './hooks/useTokenSupportsPermit' +export * from './hooks/useHasCachedPermit' export { useGetCachedPermit } from './hooks/useGetCachedPermit' export * from './types' export * from './utils/handlePermit' diff --git a/apps/cowswap-frontend/src/modules/permit/state/permitCacheAtom.ts b/apps/cowswap-frontend/src/modules/permit/state/permitCacheAtom.ts index c4d4c88b3d..3215d3bde2 100644 --- a/apps/cowswap-frontend/src/modules/permit/state/permitCacheAtom.ts +++ b/apps/cowswap-frontend/src/modules/permit/state/permitCacheAtom.ts @@ -6,7 +6,7 @@ import { GetPermitCacheParams, PermitCache, PermitCacheKeyParams, - StorePermitCacheParams + StorePermitCacheParams, } from '../types' /** @@ -14,14 +14,18 @@ import { * Should never change once it has been created. * Used exclusively for quote requests */ -export const staticPermitCacheAtom = atomWithStorage('staticPermitCache:v3', {}) +export const staticPermitCacheAtom = atomWithStorage('staticPermitCache:v3', {}, undefined, { + unstable_getOnInit: true, +}) /** * Atom that stores permit data for user permit requests. * Should be updated whenever the permit nonce is updated. * Used exclusively for order requests */ -export const userPermitCacheAtom = atomWithStorage('userPermitCache:v1', {}) +export const userPermitCacheAtom = atomWithStorage('userPermitCache:v1', {}, undefined, { + unstable_getOnInit: true, +}) /** * Atom to add/update permit cache data diff --git a/apps/cowswap-frontend/src/modules/swap/containers/TradeButtons/index.tsx b/apps/cowswap-frontend/src/modules/swap/containers/TradeButtons/index.tsx index 0011f2c2ba..f22d23504f 100644 --- a/apps/cowswap-frontend/src/modules/swap/containers/TradeButtons/index.tsx +++ b/apps/cowswap-frontend/src/modules/swap/containers/TradeButtons/index.tsx @@ -4,7 +4,12 @@ import { TokenWithLogo } from '@cowprotocol/common-const' import { useFeatureFlags } from '@cowprotocol/common-hooks' import { AddIntermediateToken } from 'modules/tokensList' -import { useIsNoImpactWarningAccepted, useTradeConfirmActions, useWrappedToken } from 'modules/trade' +import { + useIsCurrentTradeBridging, + useIsNoImpactWarningAccepted, + useTradeConfirmActions, + useWrappedToken, +} from 'modules/trade' import { TradeFormButtons, TradeFormValidation, @@ -17,7 +22,6 @@ import { useSafeMemoObject } from 'common/hooks/useSafeMemo' import { swapTradeButtonsMap } from './swapTradeButtonsMap' -import { useGetConfirmButtonLabel } from '../../hooks/useGetConfirmButtonLabel' import { useOnCurrencySelection } from '../../hooks/useOnCurrencySelection' import { useSwapDerivedState } from '../../hooks/useSwapDerivedState' import { useSwapFormState } from '../../hooks/useSwapFormState' @@ -50,10 +54,11 @@ export function TradeButtons({ const localFormValidation = useSwapFormState() const wrappedToken = useWrappedToken() const onCurrencySelection = useOnCurrencySelection() + const isCurrentTradeBridging = useIsCurrentTradeBridging() const confirmTrade = tradeConfirmActions.onOpen - const confirmText = useGetConfirmButtonLabel() + const confirmText = isCurrentTradeBridging ? 'Swap and Bridge' : 'Swap' const { isPartialApproveEnabled } = useFeatureFlags() // enable partial approve only for swap diff --git a/apps/cowswap-frontend/src/modules/swap/hooks/useGetConfirmButtonLabel.ts b/apps/cowswap-frontend/src/modules/swap/hooks/useGetConfirmButtonLabel.ts deleted file mode 100644 index 84bbd0ae5d..0000000000 --- a/apps/cowswap-frontend/src/modules/swap/hooks/useGetConfirmButtonLabel.ts +++ /dev/null @@ -1,22 +0,0 @@ -import { ApproveRequiredReason, useIsApprovalOrPermitRequired } from 'modules/erc20Approve' -import { useIsCurrentTradeBridging } from 'modules/trade' - -enum ConfirmButtonLabel { - SWAP = 'Swap', - SWAP_BRIDGE = 'Swap and Bridge', - APPROVE_SWAP = 'Approve and Swap', - APPROVE_SWAP_BRIDGE = 'Approve, Swap & Bridge', -} - -export function useGetConfirmButtonLabel(): string { - const isCurrentTradeBridging = useIsCurrentTradeBridging() - const { reason: approveRequiredReason } = useIsApprovalOrPermitRequired({ - isBundlingSupportedOrEnabledForContext: false, - }) - - if (approveRequiredReason === ApproveRequiredReason.Required) { - return isCurrentTradeBridging ? ConfirmButtonLabel.APPROVE_SWAP_BRIDGE : ConfirmButtonLabel.APPROVE_SWAP - } - - return isCurrentTradeBridging ? ConfirmButtonLabel.SWAP_BRIDGE : ConfirmButtonLabel.SWAP -} diff --git a/apps/cowswap-frontend/src/modules/tradeFormValidation/pure/TradeFormButtons/tradeButtonsMap.tsx b/apps/cowswap-frontend/src/modules/tradeFormValidation/pure/TradeFormButtons/tradeButtonsMap.tsx index 6cb84d05f8..676772938e 100644 --- a/apps/cowswap-frontend/src/modules/tradeFormValidation/pure/TradeFormButtons/tradeButtonsMap.tsx +++ b/apps/cowswap-frontend/src/modules/tradeFormValidation/pure/TradeFormButtons/tradeButtonsMap.tsx @@ -246,7 +246,6 @@ export const tradeButtonsMap: Record {defaultText}