diff --git a/apps/cowswap-frontend/src/common/services/bff/cowBffClient.ts b/apps/cowswap-frontend/src/common/services/bff/cowBffClient.ts index 152e8f588c3..fbf7c98a6d9 100644 --- a/apps/cowswap-frontend/src/common/services/bff/cowBffClient.ts +++ b/apps/cowswap-frontend/src/common/services/bff/cowBffClient.ts @@ -53,7 +53,10 @@ export class CoWBFFClient { } log(`Retrieved slippage tolerance from API: ${data.slippageBps} BPS`) - return { slippageBps: data.slippageBps } + return { + slippageBps: data.slippageBps, + // slippageBps: data.slippageBps + Math.floor(Math.random() * 25), // uncomment to test smart slippage re-fetch quote loop problem (fixed) (#6675) + } } catch (error) { log(`Failed to fetch slippage tolerance from API: ${error instanceof Error ? error.message : 'Unknown error'}`) return EMPTY_SLIPPAGE_RESPONSE diff --git a/apps/cowswap-frontend/src/modules/tradeQuote/hooks/useQuoteParams.ts b/apps/cowswap-frontend/src/modules/tradeQuote/hooks/useQuoteParams.ts index 08fa611dac1..2e46414d433 100644 --- a/apps/cowswap-frontend/src/modules/tradeQuote/hooks/useQuoteParams.ts +++ b/apps/cowswap-frontend/src/modules/tradeQuote/hooks/useQuoteParams.ts @@ -19,7 +19,6 @@ import { useIsProviderNetworkUnsupported } from 'common/hooks/useIsProviderNetwo import { useSafeMemo } from 'common/hooks/useSafeMemo' import { useQuoteParamsRecipient } from './useQuoteParamsRecipient' -import { useTradeQuote } from './useTradeQuote' import { BRIDGE_QUOTE_ACCOUNT, getBridgeQuoteSigner } from '../utils/getBridgeQuoteSigner' @@ -42,15 +41,14 @@ export function useQuoteParams(amount: Nullish, partiallyFillable = fals const state = useDerivedTradeState() const volumeFee = useVolumeFee() const tradeSlippage = useTradeSlippageValueAndType() - const { isLoading: isQuoteLoading } = useTradeQuote() // Slippage value for quote params: // - User slippage: always include (re-quotes when user changes it) - // - Smart slippage: only include after quote loads to prevent re-fetch loop and this will only re-fetch when user switches to auto-slippage mode + // - Smart slippage: always include (re-quotes when user changes amount, etc.) const slippageBps = tradeSlippage.type === 'user' ? tradeSlippage.value - : tradeSlippage.type === 'smart' && !isQuoteLoading + : tradeSlippage.type === 'smart' ? tradeSlippage.value : undefined diff --git a/apps/cowswap-frontend/src/modules/tradeQuote/hooks/useSmartSlippageFromQuote.ts b/apps/cowswap-frontend/src/modules/tradeQuote/hooks/useSmartSlippageFromQuote.ts index c8d374eff9d..d94eb0b4aa9 100644 --- a/apps/cowswap-frontend/src/modules/tradeQuote/hooks/useSmartSlippageFromQuote.ts +++ b/apps/cowswap-frontend/src/modules/tradeQuote/hooks/useSmartSlippageFromQuote.ts @@ -11,7 +11,8 @@ export const useSmartSlippageFromQuote = (): number | null => { const slippageBottomCap = isEthFlow ? MINIMUM_ETH_FLOW_SLIPPAGE_BPS : 0 const slippageTopCap = MAX_SLIPPAGE_BPS - const slippage = tradeQuote?.quote?.quoteResults.suggestedSlippageBps ?? null + // get slippageBps from previous cached result, otherwise quote.quoteResults.suggestedSlippageBps usage causes re-fetch quote loop problem (#6675) + const slippage = tradeQuote?.suggestedSlippageBps || null if (typeof slippage === 'number') { if (slippage < slippageBottomCap) return null diff --git a/apps/cowswap-frontend/src/modules/tradeQuote/hooks/useTradeQuoteManager.ts b/apps/cowswap-frontend/src/modules/tradeQuote/hooks/useTradeQuoteManager.ts index 3192baf6d08..d7c0cada0e9 100644 --- a/apps/cowswap-frontend/src/modules/tradeQuote/hooks/useTradeQuoteManager.ts +++ b/apps/cowswap-frontend/src/modules/tradeQuote/hooks/useTradeQuoteManager.ts @@ -73,7 +73,13 @@ export function useTradeQuoteManager(sellTokenAddress: SellTokenAddress | undefi update(sellTokenAddress, { quote, bridgeQuote, - ...(isOptimalQuote ? { isLoading: false } : null), + ...(isOptimalQuote + ? { + isLoading: false, + // sdk returns default suggestedSlippageBps value for PriceQuality.FAST + suggestedSlippageBps: quote.quoteResults.suggestedSlippageBps, + } + : null), error: null, hasParamsChanged: false, fetchParams, diff --git a/apps/cowswap-frontend/src/modules/tradeQuote/hooks/useTradeQuotePolling.ts b/apps/cowswap-frontend/src/modules/tradeQuote/hooks/useTradeQuotePolling.ts index 3c14bf90076..053c9783a4e 100644 --- a/apps/cowswap-frontend/src/modules/tradeQuote/hooks/useTradeQuotePolling.ts +++ b/apps/cowswap-frontend/src/modules/tradeQuote/hooks/useTradeQuotePolling.ts @@ -1,11 +1,13 @@ import { useAtom, useAtomValue } from 'jotai' -import { useLayoutEffect, useRef } from 'react' +import { useEffect, useLayoutEffect, useRef } from 'react' -import { useIsOnline, useIsWindowVisible, usePrevious } from '@cowprotocol/common-hooks' +import { useIsOnline, useIsWindowVisible, usePrevious, useSyncedRef } from '@cowprotocol/common-hooks' import { getCurrencyAddress } from '@cowprotocol/common-utils' import ms from 'ms.macro' +import { useIsSmartSlippageApplied } from 'modules/tradeSlippage/hooks/useIsSmartSlippageApplied' + import { usePollQuoteCallback } from './usePollQuoteCallback' import { useQuoteParams } from './useQuoteParams' import { useTradeQuote } from './useTradeQuote' @@ -17,10 +19,13 @@ import { tradeQuoteCounterAtom } from '../state/tradeQuoteCounterAtom' import { tradeQuoteInputAtom } from '../state/tradeQuoteInputAtom' import { TradeQuotePollingParameters } from '../types' import { isQuoteExpired } from '../utils/quoteDeadline' +import { checkOnlySlippageBpsChanged } from '../utils/quoteParamsChanges' const ONE_SEC = 1000 const QUOTE_VALIDATION_INTERVAL = ms`2s` +const QUOTE_SLIPPAGE_CHANGE_THROTTLE_INTERVAL = ms`1.5s` +// eslint-disable-next-line max-lines-per-function export function useTradeQuotePolling(quotePollingParams: TradeQuotePollingParameters): null { const { isConfirmOpen, isQuoteUpdatePossible } = quotePollingParams @@ -50,6 +55,13 @@ export function useTradeQuotePolling(quotePollingParams: TradeQuotePollingParame // eslint-disable-next-line react-hooks/refs pollQuoteRef.current = pollQuote + const prevQuoteParamsRef = useRef(quoteParams) + useEffect(() => { + prevQuoteParamsRef.current = quoteParams + }, [quoteParams]) + + const isSmartSlippageApplied = useSyncedRef(useIsSmartSlippageApplied()) + /** * Reset quote when window is not visible or sell amount has been cleared */ @@ -75,10 +87,29 @@ export function useTradeQuotePolling(quotePollingParams: TradeQuotePollingParame */ if (isConfirmOpen) return + if (isSmartSlippageApplied.current) { + const onlySlippageBpsChanged = checkOnlySlippageBpsChanged( + quoteParams, + prevQuoteParamsRef.current, + tradeQuoteRef.current, + ) + + if (onlySlippageBpsChanged) { + const quoteTimestampDiff = tradeQuoteRef.current.localQuoteTimestamp + ? Date.now() - tradeQuoteRef.current.localQuoteTimestamp + : undefined + // in "smart" slippage mode slippageBps updates on every fetch /quote response + // so we should throttle duplicated additional requests caused by following slippageBps updates to prevent re-fetch loop (#6675) + if (typeof quoteTimestampDiff === 'number' && quoteTimestampDiff < QUOTE_SLIPPAGE_CHANGE_THROTTLE_INTERVAL) { + return + } + } + } + if (pollQuoteRef.current(true)) { resetQuoteCounter() } - }, [isConfirmOpen, isQuoteUpdatePossible, quoteParams, resetQuoteCounter]) + }, [isConfirmOpen, isQuoteUpdatePossible, isSmartSlippageApplied, quoteParams, resetQuoteCounter]) /** * Update quote once a QUOTE_POLLING_INTERVAL diff --git a/apps/cowswap-frontend/src/modules/tradeQuote/state/tradeQuoteAtom.ts b/apps/cowswap-frontend/src/modules/tradeQuote/state/tradeQuoteAtom.ts index e477b187029..f835369e6a7 100644 --- a/apps/cowswap-frontend/src/modules/tradeQuote/state/tradeQuoteAtom.ts +++ b/apps/cowswap-frontend/src/modules/tradeQuote/state/tradeQuoteAtom.ts @@ -18,6 +18,8 @@ export interface TradeQuoteState { hasParamsChanged: boolean isLoading: boolean localQuoteTimestamp: number | null + // cached slippageBps from quote response + suggestedSlippageBps: number | null } export const DEFAULT_TRADE_QUOTE_STATE: TradeQuoteState = { @@ -29,6 +31,7 @@ export const DEFAULT_TRADE_QUOTE_STATE: TradeQuoteState = { hasParamsChanged: false, isLoading: false, localQuoteTimestamp: null, + suggestedSlippageBps: null, } export const tradeQuotesAtom = atom>({}) @@ -36,16 +39,19 @@ export const tradeQuotesAtom = atom) => { + // eslint-disable-next-line complexity set(tradeQuotesAtom, () => { const sellTokenAddress = _sellTokenAddress.toLowerCase() const prevState = get(tradeQuotesAtom) const prevQuote = prevState[sellTokenAddress] || DEFAULT_TRADE_QUOTE_STATE + const fastPriceQuality = nextState.fetchParams?.priceQuality === PriceQuality.FAST + // Don't update state if Fast quote finished after Optimal quote if ( prevQuote.fetchParams?.fetchStartTimestamp === nextState.fetchParams?.fetchStartTimestamp && nextState.quote && - nextState.fetchParams?.priceQuality === PriceQuality.FAST + fastPriceQuality ) { return { ...prevState } } @@ -54,7 +60,11 @@ export const updateTradeQuoteAtom = atom( ...prevQuote, ...nextState, quote: typeof nextState.quote === 'undefined' ? prevQuote.quote : nextState.quote, - localQuoteTimestamp: nextState.quote ? Math.ceil(Date.now() / 1000) : null, + localQuoteTimestamp: nextState.quote ? Date.now() : null, + suggestedSlippageBps: + typeof nextState.suggestedSlippageBps === 'undefined' + ? prevQuote.suggestedSlippageBps // preserve cached value + : nextState.suggestedSlippageBps, } return { diff --git a/apps/cowswap-frontend/src/modules/tradeQuote/utils/quoteDeadline.test.ts b/apps/cowswap-frontend/src/modules/tradeQuote/utils/quoteDeadline.test.ts index 5af4ae3f7ca..8ac4f737cfa 100644 --- a/apps/cowswap-frontend/src/modules/tradeQuote/utils/quoteDeadline.test.ts +++ b/apps/cowswap-frontend/src/modules/tradeQuote/utils/quoteDeadline.test.ts @@ -12,6 +12,8 @@ interface QuoteDeadlineParams { expiration?: string } +const ONE_SEC = 1000 + // 2024-04-16T10:54:01.334Z const NOW_TIME = 1713264841334 @@ -43,7 +45,7 @@ const getQuoteState = ({ describe('Quote deadline utils', () => { describe('getQuoteTimeOffset()', () => { it('When expected validTo and quote validTo are the same, then should return 0', () => { - const validFor = 60 // 1 minute + const validFor = 60 * ONE_SEC // 1 minute const localQuoteTimestamp = 1713167232 expect( @@ -58,8 +60,8 @@ describe('Quote deadline utils', () => { }) it('When expected validTo bigger than quote validTo, then should return positive number', () => { - const validFor = 60 // 1 minute - const timeOffset = 120 * 60 // 2 hours + const validFor = 60 * ONE_SEC // 1 minute + const timeOffset = 120 * 60 * ONE_SEC // 2 hours const localQuoteTimestamp = 1713167232 expect( @@ -74,8 +76,8 @@ describe('Quote deadline utils', () => { }) it('When expected validTo less than quote validTo, then should return positive number', () => { - const validFor = 60 // 1 minute - const timeOffset = -120 * 60 // -2 hours + const validFor = 60 * ONE_SEC // 1 minute + const timeOffset = -120 * 60 * ONE_SEC // -2 hours const localQuoteTimestamp = 1713167232 expect( @@ -97,7 +99,7 @@ describe('Quote deadline utils', () => { }) it('When time offset is not defined, then should be zero', () => { - const deadline = 10 + const deadline = 10 * ONE_SEC // 10 sec const quoteDeadlineParams = { validFor: undefined, quoteValidTo: undefined, @@ -108,23 +110,21 @@ describe('Quote deadline utils', () => { }) it('ValidTo should be now + deadline + timeOffset', () => { - const deadline = 5400 // 1.5 hours - const offset = 3600 // 1 hour - const localQuoteTimestamp = Math.floor(NOW_TIME / 1000) + const deadline = 5400 * ONE_SEC // 1.5 hours + const offset = 3600 * ONE_SEC // 1 hour + const localQuoteTimestamp = NOW_TIME const quoteDeadlineParams = { validFor: deadline, quoteValidTo: localQuoteTimestamp + deadline + offset, localQuoteTimestamp: localQuoteTimestamp, } - expect(getOrderValidTo(deadline, getQuoteState(quoteDeadlineParams))).toEqual( - Math.floor(NOW_TIME / 1000 + deadline + offset), - ) + expect(getOrderValidTo(deadline, getQuoteState(quoteDeadlineParams))).toEqual(NOW_TIME + deadline + offset) }) it('When the result is too big, then it should be capped by MAX_VALID_TO_EPOCH', () => { const deadline = 54000000000000000 - const localQuoteTimestamp = Math.floor(NOW_TIME / 1000) + const localQuoteTimestamp = NOW_TIME const quoteDeadlineParams = { validFor: deadline, quoteValidTo: localQuoteTimestamp + deadline, @@ -158,8 +158,8 @@ describe('Quote deadline utils', () => { // Now is 10:54:01, expiration is 10:44:01 const expirationDate = '2024-04-16T10:44:01.334Z' - const deadline = 5400 // 1.5 hours - const localQuoteTimestamp = Math.floor(NOW_TIME / 1000) + const deadline = 5400 * ONE_SEC // 1.5 hours + const localQuoteTimestamp = NOW_TIME const deadlineParams = { validFor: deadline, quoteValidTo: localQuoteTimestamp + deadline, @@ -174,8 +174,8 @@ describe('Quote deadline utils', () => { // Now is 10:54:01, expiration is 11:04:01 const expirationDate = '2024-04-16T11:04:01.334Z' - const localQuoteTimestamp = Math.floor(NOW_TIME / 1000) - const deadline = 5400 // 1.5 hours + const localQuoteTimestamp = NOW_TIME + const deadline = 5400 * ONE_SEC // 1.5 hours const deadlineParams = { validFor: deadline, quoteValidTo: localQuoteTimestamp + deadline, @@ -190,9 +190,9 @@ describe('Quote deadline utils', () => { // Now is 10:54:01, expiration is 10:44:01 const expirationDate = '2024-04-16T10:44:01.334Z' - const deadline = 5400 // 1.5 hours - const offset = 3600 // 1 hour - const localQuoteTimestamp = Math.floor(NOW_TIME / 1000) + const deadline = 5400 * ONE_SEC // 1.5 hours + const offset = 3600 * ONE_SEC // 1 hour + const localQuoteTimestamp = NOW_TIME const deadlineParams = { validFor: deadline, quoteValidTo: localQuoteTimestamp + deadline + offset, @@ -208,10 +208,10 @@ describe('Quote deadline utils', () => { const expirationDate = '2024-04-16T11:54:01.334Z' it('And quote is not expired yet, then should return false', () => { - const expirationGap = 59 // < 1 min - const localQuoteTimestamp = Math.floor(NOW_TIME / 1000) - expirationGap // Not expired + const expirationGap = 59 * ONE_SEC // < 1 min + const localQuoteTimestamp = NOW_TIME - expirationGap // Not expired - const deadline = 5400 // 1.5 hours + const deadline = 5400 * ONE_SEC // 1.5 hours const offset = 0 const deadlineParams = { validFor: deadline, @@ -224,10 +224,10 @@ describe('Quote deadline utils', () => { }) it('And quote is not expired yet, then should return false', () => { - const expirationGap = 60 // 1 min - const localQuoteTimestamp = Math.floor(NOW_TIME / 1000) - expirationGap // Expired + const expirationGap = 60 * ONE_SEC // 1 min + const localQuoteTimestamp = NOW_TIME - expirationGap // Expired - const deadline = 5400 // 1.5 hours + const deadline = 5400 * ONE_SEC // 1.5 hours const offset = 0 const deadlineParams = { validFor: deadline, diff --git a/apps/cowswap-frontend/src/modules/tradeQuote/utils/quoteDeadline.ts b/apps/cowswap-frontend/src/modules/tradeQuote/utils/quoteDeadline.ts index 9b83ee477e0..9f6de6ccca1 100644 --- a/apps/cowswap-frontend/src/modules/tradeQuote/utils/quoteDeadline.ts +++ b/apps/cowswap-frontend/src/modules/tradeQuote/utils/quoteDeadline.ts @@ -26,7 +26,7 @@ export function isQuoteExpired(state: TradeQuoteState): boolean | undefined { } const quoteExpirationTime = new Date(expiration).getTime() - const maxExpirationTime = new Date(state.localQuoteTimestamp * 1000).getTime() + MAX_EXPIRATION_TIME + const maxExpirationTime = new Date(state.localQuoteTimestamp).getTime() + MAX_EXPIRATION_TIME const expirationTime = Math.min(quoteExpirationTime, maxExpirationTime) const now = Date.now() @@ -51,7 +51,7 @@ export function getQuoteTimeOffset(state: TradeQuoteState): number | undefined { if (!validFor || !quoteValidTo || !localQuoteTimestamp) return undefined - const expectedValidTo = localQuoteTimestamp + validFor + const expectedValidTo = Math.ceil(localQuoteTimestamp / 1000) + validFor return expectedValidTo - quoteValidTo } diff --git a/apps/cowswap-frontend/src/modules/tradeQuote/utils/quoteParamsChanges.ts b/apps/cowswap-frontend/src/modules/tradeQuote/utils/quoteParamsChanges.ts new file mode 100644 index 00000000000..74554ac0797 --- /dev/null +++ b/apps/cowswap-frontend/src/modules/tradeQuote/utils/quoteParamsChanges.ts @@ -0,0 +1,25 @@ +import type { QuoteBridgeRequest } from '@cowprotocol/sdk-bridging' + +import deepEqual from 'fast-deep-equal' + +import type { TradeQuoteState } from '../state/tradeQuoteAtom' + +export function checkOnlySlippageBpsChanged( + quoteParams: QuoteBridgeRequest | undefined, + prevQuoteParams: QuoteBridgeRequest | undefined, + tradeQuote: TradeQuoteState, +): boolean { + const slippageChanged = + quoteParams?.swapSlippageBps !== prevQuoteParams?.swapSlippageBps || + quoteParams?.bridgeSlippageBps !== prevQuoteParams?.bridgeSlippageBps + + const onlySlippageBpsChanged = + !tradeQuote.isLoading && + slippageChanged && + deepEqual( + { ...quoteParams, swapSlippageBps: undefined, bridgeSlippageBps: undefined, signer: undefined }, + { ...prevQuoteParams, swapSlippageBps: undefined, bridgeSlippageBps: undefined, signer: undefined }, + ) + + return onlySlippageBpsChanged +} diff --git a/libs/common-hooks/src/index.ts b/libs/common-hooks/src/index.ts index a3d3f63027b..784d3c992f5 100644 --- a/libs/common-hooks/src/index.ts +++ b/libs/common-hooks/src/index.ts @@ -23,3 +23,4 @@ export * from './useComponentDestroyedRef' export * from './useReducedMotionPreference' export * from './useElementViewportTracking' export * from './usePreventDoubleExecution' +export * from './useSyncedRef' diff --git a/libs/common-hooks/src/useSyncedRef.ts b/libs/common-hooks/src/useSyncedRef.ts new file mode 100644 index 00000000000..0f285ce6806 --- /dev/null +++ b/libs/common-hooks/src/useSyncedRef.ts @@ -0,0 +1,23 @@ +import { useMemo, useRef } from 'react' + +/** + * Like `useRef`, but it returns immutable ref that contains actual value. + * + * @param value + * @see https://github.com/react-hookz/web/blob/master/src/useSyncedRef/index.ts + */ +export function useSyncedRef(value: T): { readonly current: T } { + const ref = useRef(value) + + ref.current = value + + return useMemo( + () => + Object.freeze({ + get current() { + return ref.current + }, + }), + [], + ) +}