diff --git a/apps/cowswap-frontend/src/locales/en-US.po b/apps/cowswap-frontend/src/locales/en-US.po index 285d7f24980..8a9a444c7d9 100644 --- a/apps/cowswap-frontend/src/locales/en-US.po +++ b/apps/cowswap-frontend/src/locales/en-US.po @@ -1956,7 +1956,6 @@ msgstr "Pick your own code or generate one. Once saved, it becomes permanently l #~ msgstr "By clicking Confirm, you expressly represent and warrant that you are NOT:" #: apps/cowswap-frontend/src/modules/ordersTable/pure/ReceiptModal/ReceiptModal.modal.tsx -#: apps/cowswap-frontend/src/modules/trade/pure/TotalFeeRow/index.tsx msgid "Total fee" msgstr "Total fee" diff --git a/apps/cowswap-frontend/src/modules/permit/hooks/usePermitCompatibleTokens.test.ts b/apps/cowswap-frontend/src/modules/permit/hooks/usePermitCompatibleTokens.test.ts new file mode 100644 index 00000000000..e483d28dd08 --- /dev/null +++ b/apps/cowswap-frontend/src/modules/permit/hooks/usePermitCompatibleTokens.test.ts @@ -0,0 +1,114 @@ +import { useAtomValue } from 'jotai' + +import { COW_PROTOCOL_VAULT_RELAYER_ADDRESS } from '@cowprotocol/common-utils' +import { getAddressKey } from '@cowprotocol/cow-sdk' +import { PermitInfo } from '@cowprotocol/permit-utils' +import { useWalletInfo } from '@cowprotocol/wallet' + +import { renderHook } from '@testing-library/react' + +import { useIsPermitEnabled } from 'common/hooks/featureFlags/useIsPermitEnabled' + +import { usePermitCompatibleTokens } from './usePermitCompatibleTokens' +import { usePreGeneratedPermitInfo } from './usePreGeneratedPermitInfo' + +import { getPermittableTokenKey } from '../state/permittableTokensAtom' + +jest.mock('jotai', () => ({ + ...jest.requireActual('jotai'), + useAtomValue: jest.fn(), +})) + +jest.mock('@cowprotocol/wallet', () => ({ + useWalletInfo: jest.fn(), +})) + +jest.mock('common/hooks/featureFlags/useIsPermitEnabled', () => ({ + useIsPermitEnabled: jest.fn(), +})) + +jest.mock('./usePreGeneratedPermitInfo', () => ({ + usePreGeneratedPermitInfo: jest.fn(), +})) + +const mockedUseAtomValue = useAtomValue as jest.MockedFunction +const mockedUseWalletInfo = useWalletInfo as jest.MockedFunction +const mockedUseIsPermitEnabled = useIsPermitEnabled as jest.MockedFunction +const mockedUsePreGeneratedPermitInfo = usePreGeneratedPermitInfo as jest.MockedFunction< + typeof usePreGeneratedPermitInfo +> + +describe('usePermitCompatibleTokens', () => { + const chainId = 1 + const tokenAddress = '0x1234567890123456789012345678901234567890' + const spender = COW_PROTOCOL_VAULT_RELAYER_ADDRESS[chainId] + const customSpender = '0x2222222222222222222222222222222222222222' + const permitInfo: PermitInfo = { type: 'eip-2612', name: 'Test Token', version: '1' } + + beforeEach(() => { + jest.clearAllMocks() + + mockedUseWalletInfo.mockReturnValue({ chainId } as ReturnType) + mockedUseIsPermitEnabled.mockReturnValue(true) + mockedUsePreGeneratedPermitInfo.mockReturnValue({ allPermitInfo: {}, isLoading: false }) + }) + + it('maps local token-spender permit info back to the token address', () => { + const permitTokenKey = getPermittableTokenKey(tokenAddress, spender) + + mockedUseAtomValue.mockReturnValue({ + [chainId]: { + [permitTokenKey]: permitInfo, + }, + }) + + const { result } = renderHook(() => usePermitCompatibleTokens()) + + expect(result.current[getAddressKey(tokenAddress)]).toBe(true) + expect(result.current[permitTokenKey]).toBeUndefined() + }) + + it('ignores custom-spender results when mapping default permit compatibility', () => { + mockedUsePreGeneratedPermitInfo.mockReturnValue({ + allPermitInfo: { [getAddressKey(tokenAddress)]: permitInfo }, + isLoading: false, + }) + mockedUseAtomValue.mockReturnValue({ + [chainId]: { + [getPermittableTokenKey(tokenAddress, customSpender)]: { type: 'unsupported' }, + }, + }) + + const { result } = renderHook(() => usePermitCompatibleTokens()) + + expect(result.current[getAddressKey(tokenAddress)]).toBe(true) + }) + + it('allows the default-spender result to override pre-generated compatibility', () => { + mockedUsePreGeneratedPermitInfo.mockReturnValue({ + allPermitInfo: { [getAddressKey(tokenAddress)]: permitInfo }, + isLoading: false, + }) + mockedUseAtomValue.mockReturnValue({ + [chainId]: { + [getPermittableTokenKey(tokenAddress, spender)]: { type: 'unsupported' }, + }, + }) + + const { result } = renderHook(() => usePermitCompatibleTokens()) + + expect(result.current[getAddressKey(tokenAddress)]).toBe(false) + }) + + it('does not advertise permit compatibility from a custom-spender result alone', () => { + mockedUseAtomValue.mockReturnValue({ + [chainId]: { + [getPermittableTokenKey(tokenAddress, customSpender)]: permitInfo, + }, + }) + + const { result } = renderHook(() => usePermitCompatibleTokens()) + + expect(result.current[getAddressKey(tokenAddress)]).toBeUndefined() + }) +}) diff --git a/apps/cowswap-frontend/src/modules/permit/hooks/usePermitCompatibleTokens.ts b/apps/cowswap-frontend/src/modules/permit/hooks/usePermitCompatibleTokens.ts index 638348067c2..7d7d6918485 100644 --- a/apps/cowswap-frontend/src/modules/permit/hooks/usePermitCompatibleTokens.ts +++ b/apps/cowswap-frontend/src/modules/permit/hooks/usePermitCompatibleTokens.ts @@ -1,6 +1,7 @@ import { useAtomValue } from 'jotai' import { useMemo, useRef } from 'react' +import { COW_PROTOCOL_VAULT_RELAYER_ADDRESS } from '@cowprotocol/common-utils' import { getAddressKey } from '@cowprotocol/cow-sdk' import { isSupportedPermitInfo } from '@cowprotocol/permit-utils' import { useWalletInfo } from '@cowprotocol/wallet' @@ -9,7 +10,11 @@ import { useIsPermitEnabled } from 'common/hooks/featureFlags/useIsPermitEnabled import { usePreGeneratedPermitInfo } from './usePreGeneratedPermitInfo' -import { permittableTokensAtom } from '../state/permittableTokensAtom' +import { + getPermittableTokenKey, + getTokenAddressFromPermittableTokenKey, + permittableTokensAtom, +} from '../state/permittableTokensAtom' import { PermitCompatibleTokens } from '../types' export function usePermitCompatibleTokens(): PermitCompatibleTokens { @@ -17,6 +22,7 @@ export function usePermitCompatibleTokens(): PermitCompatibleTokens { const permitInfoAllChains = useAtomValue(permittableTokensAtom) const localPermitInfo = permitInfoAllChains[chainId] || {} const { allPermitInfo } = usePreGeneratedPermitInfo() + const defaultSpender = COW_PROTOCOL_VAULT_RELAYER_ADDRESS[chainId] const isPermitEnabled = useIsPermitEnabled() @@ -43,14 +49,18 @@ export function usePermitCompatibleTokens(): PermitCompatibleTokens { ) } - for (const address of Object.keys(localPermitInfoRef.current)) { - const addressLowerCased = getAddressKey(address) + for (const permitTokenKey of Object.keys(localPermitInfoRef.current)) { + const tokenAddress = getTokenAddressFromPermittableTokenKey(permitTokenKey) + + if (!defaultSpender || permitTokenKey !== getPermittableTokenKey(tokenAddress, defaultSpender)) continue + + const addressLowerCased = getAddressKey(tokenAddress) - permitCompatibleTokens[addressLowerCased] = isSupportedPermitInfo(localPermitInfoRef.current[addressLowerCased]) + permitCompatibleTokens[addressLowerCased] = isSupportedPermitInfo(localPermitInfoRef.current[permitTokenKey]) } return permitCompatibleTokens // Reducing unnecessary re-renders // eslint-disable-next-line react-hooks/exhaustive-deps - }, [stableRef, isPermitEnabled]) + }, [defaultSpender, stableRef, isPermitEnabled]) } diff --git a/apps/cowswap-frontend/src/modules/permit/hooks/usePermitInfo.test.ts b/apps/cowswap-frontend/src/modules/permit/hooks/usePermitInfo.test.ts new file mode 100644 index 00000000000..c4d341ba15a --- /dev/null +++ b/apps/cowswap-frontend/src/modules/permit/hooks/usePermitInfo.test.ts @@ -0,0 +1,176 @@ +import { useAtomValue, useSetAtom } from 'jotai' + +import { Token } from '@cowprotocol/currency' +import { getTokenPermitInfo } from '@cowprotocol/permit-utils' +import type { PermitInfo } from '@cowprotocol/permit-utils' +import { useWalletInfo } from '@cowprotocol/wallet' + +import { renderHook, waitFor } from '@testing-library/react' +import { useConfig, usePublicClient } from 'wagmi' + +import { TradeType } from 'modules/trade' + +import { useIsPermitEnabled } from 'common/hooks/featureFlags/useIsPermitEnabled' + +import { usePermitInfo } from './usePermitInfo' +import { usePreGeneratedPermitInfoForToken } from './usePreGeneratedPermitInfoForToken' + +import { getPermittableTokenKey } from '../state/permittableTokensAtom' + +const defaultSpender = '0x9008D19f58AAbD9eD0D60971565AA8510560ab41' +const customSpender = '0x1111111111111111111111111111111111111111' + +jest.mock('jotai', () => ({ + ...jest.requireActual('jotai'), + useAtomValue: jest.fn(), + useSetAtom: jest.fn(), +})) + +jest.mock('@cowprotocol/common-utils', () => ({ + COW_PROTOCOL_VAULT_RELAYER_ADDRESS: { 1: defaultSpender }, + getIsNativeToken: jest.fn().mockReturnValue(false), + getWrappedToken: jest.fn((token) => token), + isAddress: jest.fn(), +})) + +jest.mock('@cowprotocol/permit-utils', () => ({ + DEFAULT_MIN_GAS_LIMIT: 50000n, + getTokenPermitInfo: jest.fn(), +})) + +jest.mock('@cowprotocol/wallet', () => ({ + useWalletInfo: jest.fn(), +})) + +jest.mock('wagmi', () => ({ + useConfig: jest.fn(), + usePublicClient: jest.fn(), +})) + +jest.mock('common/hooks/featureFlags/useIsPermitEnabled', () => ({ + useIsPermitEnabled: jest.fn(), +})) + +jest.mock('modules/trade', () => ({ + TradeType: { + SWAP: 'SWAP', + LIMIT_ORDER: 'LIMIT_ORDER', + ADVANCED_ORDERS: 'ADVANCED_ORDERS', + YIELD: 'YIELD', + }, +})) + +jest.mock('./usePreGeneratedPermitInfoForToken', () => ({ + usePreGeneratedPermitInfoForToken: jest.fn(), +})) + +const mockedUseAtomValue = useAtomValue as jest.MockedFunction +const mockedUseSetAtom = useSetAtom as jest.MockedFunction +const mockedGetTokenPermitInfo = getTokenPermitInfo as jest.MockedFunction +const mockedUseWalletInfo = useWalletInfo as jest.MockedFunction +const mockedUseConfig = useConfig as jest.MockedFunction +const mockedUsePublicClient = usePublicClient as jest.MockedFunction +const mockedUseIsPermitEnabled = useIsPermitEnabled as jest.MockedFunction +const mockedUsePreGeneratedPermitInfoForToken = usePreGeneratedPermitInfoForToken as jest.MockedFunction< + typeof usePreGeneratedPermitInfoForToken +> + +const commonUtils = jest.requireMock('@cowprotocol/common-utils') as { + isAddress: jest.MockedFunction<(value: string | undefined | null) => string | false> +} + +describe('usePermitInfo', () => { + const token = new Token(1, '0x1234567890123456789012345678901234567890', 18, 'TEST', 'Test Token') + const addPermitInfo = jest.fn() + const defaultPermitInfo: PermitInfo = { type: 'eip-2612', name: 'Test Token', version: '1' } + const fetchedPermitInfo: PermitInfo = { type: 'unsupported', name: 'Test Token' } + + beforeEach(() => { + jest.clearAllMocks() + + mockedUseAtomValue.mockReturnValue({}) + mockedUseSetAtom.mockReturnValue(addPermitInfo) + mockedUseWalletInfo.mockReturnValue({ chainId: 1 } as ReturnType) + mockedUseConfig.mockReturnValue({} as ReturnType) + mockedUsePublicClient.mockReturnValue({} as ReturnType) + mockedUseIsPermitEnabled.mockReturnValue(true) + mockedUsePreGeneratedPermitInfoForToken.mockImplementation(() => ({ permitInfo: undefined, isLoading: false })) + mockedGetTokenPermitInfo.mockResolvedValue(fetchedPermitInfo) + commonUtils.isAddress.mockImplementation((value) => { + if (!value || !/^0x[a-fA-F0-9]{40}$/.test(value)) return false + + return value + }) + }) + + it('does not reuse cached permit info that belongs to a different spender', async () => { + mockedUseAtomValue.mockReturnValue({ + 1: { + [getPermittableTokenKey(token.address, defaultSpender)]: defaultPermitInfo, + }, + }) + + const { result } = renderHook(() => usePermitInfo(token, TradeType.SWAP, customSpender)) + + expect(result.current).toBeUndefined() + + await waitFor(() => { + expect(mockedGetTokenPermitInfo).toHaveBeenCalledWith( + expect.objectContaining({ + spender: customSpender, + tokenAddress: token.address, + }), + ) + }) + }) + + it('revalidates custom spenders instead of reusing pre-generated permit info', async () => { + mockedUsePreGeneratedPermitInfoForToken.mockImplementation((tokenToCheck) => ({ + permitInfo: tokenToCheck ? defaultPermitInfo : undefined, + isLoading: false, + })) + + const { result } = renderHook(() => usePermitInfo(token, TradeType.SWAP, customSpender)) + + expect(result.current).toBeUndefined() + expect(mockedUsePreGeneratedPermitInfoForToken).toHaveBeenCalledWith(undefined) + + await waitFor(() => { + expect(mockedGetTokenPermitInfo).toHaveBeenCalledWith( + expect.objectContaining({ + spender: customSpender, + tokenAddress: token.address, + }), + ) + }) + + await waitFor(() => { + expect(addPermitInfo).toHaveBeenCalledWith({ + chainId: 1, + tokenAddress: token.address, + spender: customSpender, + permitInfo: fetchedPermitInfo, + }) + }) + }) + + it('reuses pre-generated permit info for the default spender', () => { + mockedUsePreGeneratedPermitInfoForToken.mockImplementation((tokenToCheck) => ({ + permitInfo: tokenToCheck ? defaultPermitInfo : undefined, + isLoading: false, + })) + + const { result } = renderHook(() => usePermitInfo(token, TradeType.SWAP)) + + expect(result.current).toEqual(defaultPermitInfo) + expect(mockedGetTokenPermitInfo).not.toHaveBeenCalled() + }) + + it('skips permit lookup for an invalid custom spender', () => { + const { result } = renderHook(() => usePermitInfo(token, TradeType.SWAP, '0x1')) + + expect(result.current).toBeUndefined() + expect(mockedUsePreGeneratedPermitInfoForToken).toHaveBeenCalledWith(undefined) + expect(mockedGetTokenPermitInfo).not.toHaveBeenCalled() + }) +}) diff --git a/apps/cowswap-frontend/src/modules/permit/hooks/usePermitInfo.ts b/apps/cowswap-frontend/src/modules/permit/hooks/usePermitInfo.ts index 149b4916a89..b9e57ff3c36 100644 --- a/apps/cowswap-frontend/src/modules/permit/hooks/usePermitInfo.ts +++ b/apps/cowswap-frontend/src/modules/permit/hooks/usePermitInfo.ts @@ -1,7 +1,12 @@ import { useAtomValue, useSetAtom } from 'jotai' import { useEffect, useMemo } from 'react' -import { getIsNativeToken, getWrappedToken, COW_PROTOCOL_VAULT_RELAYER_ADDRESS } from '@cowprotocol/common-utils' +import { + getIsNativeToken, + getWrappedToken, + COW_PROTOCOL_VAULT_RELAYER_ADDRESS, + isAddress, +} from '@cowprotocol/common-utils' import { getAddressKey, mapSupportedNetworks, SupportedChainId } from '@cowprotocol/cow-sdk' import { Currency } from '@cowprotocol/currency' import { DEFAULT_MIN_GAS_LIMIT, getTokenPermitInfo, PermitInfo } from '@cowprotocol/permit-utils' @@ -10,13 +15,17 @@ import { useWalletInfo } from '@cowprotocol/wallet' import { Nullish } from 'types' import { useConfig, usePublicClient } from 'wagmi' -import { TradeType } from 'modules/trade/types/TradeType' +import { TradeType } from 'modules/trade' import { useIsPermitEnabled } from 'common/hooks/featureFlags/useIsPermitEnabled' import { usePreGeneratedPermitInfoForToken } from './usePreGeneratedPermitInfoForToken' -import { addPermitInfoForTokenAtom, permittableTokensAtom } from '../state/permittableTokensAtom' +import { + addPermitInfoForTokenAtom, + getPermittableTokenKey, + permittableTokensAtom, +} from '../state/permittableTokensAtom' import { IsTokenPermittableResult } from '../types' const ORDER_TYPE_SUPPORTS_PERMIT: Record = { @@ -63,29 +72,31 @@ export function usePermitInfo( const isPermitSupported = !!tradeType && ORDER_TYPE_SUPPORTS_PERMIT[tradeType] const isPermitEnabled = useIsPermitEnabled() && isPermitSupported + const defaultSpender = chainId ? COW_PROTOCOL_VAULT_RELAYER_ADDRESS[chainId] : undefined + const customSpenderAddress = customSpender ? isAddress(customSpender) || undefined : undefined + const spender = customSpender ? customSpenderAddress : defaultSpender + const shouldUsePreGeneratedInfo = !customSpender const addPermitInfo = useAddPermitInfo() - const permitInfo = usePermitInfoState(chainId, isPermitEnabled ? lowerCaseAddress : undefined) + const permitInfo = usePermitInfoState(chainId, isPermitEnabled ? lowerCaseAddress : undefined, spender) const { permitInfo: preGeneratedInfo, isLoading: preGeneratedIsLoading } = usePreGeneratedPermitInfoForToken( - isPermitEnabled && !isNative ? token : undefined, + isPermitEnabled && !isNative && shouldUsePreGeneratedInfo ? token : undefined, ) - - const spender = customSpender || COW_PROTOCOL_VAULT_RELAYER_ADDRESS[chainId] + const hasResolvedPermitInfo = + permitInfo !== undefined || (shouldUsePreGeneratedInfo && preGeneratedInfo !== undefined) + const shouldSkipPermitInfoLoad = + !chainId || + !isPermitEnabled || + !lowerCaseAddress || + !spender || + !config || + !publicClient || + hasResolvedPermitInfo || + isNative || + preGeneratedIsLoading useEffect(() => { - if ( - !chainId || - !isPermitEnabled || - !lowerCaseAddress || - !config || - !publicClient || - permitInfo !== undefined || - isNative || - // Do not try to load when pre-generated info is loading - preGeneratedIsLoading || - // Do not try to load when pre-generated exists - preGeneratedInfo !== undefined - ) { + if (shouldSkipPermitInfoLoad) { return } @@ -108,7 +119,7 @@ export function usePermitInfo( // TODO: there is a Single Responsibility Principle breach here. This hook should not be responsible for caching. // TODO: better to create a separate updater for caching. // Otherwise, we know it is permittable or not. Cache it. - addPermitInfo({ chainId, tokenAddress: lowerCaseAddress, permitInfo: result }) + addPermitInfo({ chainId, tokenAddress: lowerCaseAddress, spender, permitInfo: result }) } }) }, [ @@ -122,6 +133,8 @@ export function usePermitInfo( permitInfo, preGeneratedInfo, preGeneratedIsLoading, + shouldUsePreGeneratedInfo, + shouldSkipPermitInfoLoad, spender, ]) @@ -129,7 +142,7 @@ export function usePermitInfo( return UNSUPPORTED } - return preGeneratedInfo ?? permitInfo + return shouldUsePreGeneratedInfo ? (preGeneratedInfo ?? permitInfo) : permitInfo } /** @@ -141,12 +154,16 @@ function useAddPermitInfo() { return useSetAtom(addPermitInfoForTokenAtom) } -function usePermitInfoState(chainId: SupportedChainId, tokenAddress: string | undefined): IsTokenPermittableResult { +function usePermitInfoState( + chainId: SupportedChainId, + tokenAddress: string | undefined, + spender: string | undefined, +): IsTokenPermittableResult { const permitableTokens = useAtomValue(permittableTokensAtom) return useMemo(() => { - if (!tokenAddress) return undefined + if (!tokenAddress || !spender) return undefined - return permitableTokens[chainId]?.[getAddressKey(tokenAddress)] - }, [chainId, permitableTokens, tokenAddress]) + return permitableTokens[chainId]?.[getPermittableTokenKey(tokenAddress, spender)] + }, [chainId, permitableTokens, spender, tokenAddress]) } diff --git a/apps/cowswap-frontend/src/modules/permit/state/permittableTokensAtom.ts b/apps/cowswap-frontend/src/modules/permit/state/permittableTokensAtom.ts index 6dd51a78a36..0cd3c1ad10c 100644 --- a/apps/cowswap-frontend/src/modules/permit/state/permittableTokensAtom.ts +++ b/apps/cowswap-frontend/src/modules/permit/state/permittableTokensAtom.ts @@ -10,6 +10,14 @@ import { AddPermitTokenParams } from '../types' type PermittableTokens = Record +export function getPermittableTokenKey(tokenAddress: string, spender: string): string { + return `${getAddressKey(tokenAddress)}-${getAddressKey(spender)}` +} + +export function getTokenAddressFromPermittableTokenKey(permitTokenKey: string): string { + return permitTokenKey.split('-')[0] +} + /** * Atom that stores the permittable tokens info for each chain on localStorage. * It's meant to be shared across different tabs, thus no special storage handling. @@ -18,7 +26,7 @@ type PermittableTokens = Record */ export const permittableTokensAtom = atomWithStorage>( - 'permittableTokens:v3', + 'permittableTokens:v4', mapSupportedNetworks({}), getJotaiMergerStorage(), ) @@ -28,13 +36,13 @@ export const permittableTokensAtom = atomWithStorage { + (get, set, { chainId, tokenAddress, spender, permitInfo }: AddPermitTokenParams) => { const permittableTokens = { ...get(permittableTokensAtom) } const permittableTokensForChain = permittableTokens[chainId] || {} permittableTokens[chainId] = { ...permittableTokensForChain, - [getAddressKey(tokenAddress)]: permitInfo, + [getPermittableTokenKey(tokenAddress, spender)]: permitInfo, } set(permittableTokensAtom, permittableTokens) diff --git a/apps/cowswap-frontend/src/modules/permit/types.ts b/apps/cowswap-frontend/src/modules/permit/types.ts index f0f15548d23..45532011c43 100644 --- a/apps/cowswap-frontend/src/modules/permit/types.ts +++ b/apps/cowswap-frontend/src/modules/permit/types.ts @@ -7,6 +7,7 @@ import { AppDataInfo, TypedAppDataHooks } from 'modules/appData' export type AddPermitTokenParams = { chainId: SupportedChainId tokenAddress: string + spender: string permitInfo: PermitInfo } diff --git a/libs/common-utils/src/getIsNativeToken.test.ts b/libs/common-utils/src/getIsNativeToken.test.ts new file mode 100644 index 00000000000..747fea31262 --- /dev/null +++ b/libs/common-utils/src/getIsNativeToken.test.ts @@ -0,0 +1,27 @@ +import { NATIVE_CURRENCIES } from '@cowprotocol/common-const' +import { SupportedChainId } from '@cowprotocol/cow-sdk' +import { Token } from '@cowprotocol/currency' + +import { getIsNativeToken } from './getIsNativeToken' + +describe('getIsNativeToken', () => { + it('does not treat an ERC20 token as native only because it uses the native symbol', () => { + const fakeEth = new Token( + SupportedChainId.MAINNET, + '0x1234567890123456789012345678901234567890', + 18, + 'ETH', + 'Fake ETH', + ) + + expect(getIsNativeToken(fakeEth)).toBe(false) + }) + + it('treats the canonical native token object as native', () => { + expect(getIsNativeToken(NATIVE_CURRENCIES[SupportedChainId.MAINNET])).toBe(true) + }) + + it('keeps symbol matching for route token ids', () => { + expect(getIsNativeToken(SupportedChainId.MAINNET, 'ETH')).toBe(true) + }) +}) diff --git a/libs/common-utils/src/getIsNativeToken.ts b/libs/common-utils/src/getIsNativeToken.ts index 101d9067c52..926e5ee09eb 100644 --- a/libs/common-utils/src/getIsNativeToken.ts +++ b/libs/common-utils/src/getIsNativeToken.ts @@ -1,5 +1,5 @@ import { NATIVE_CURRENCIES, TokenWithLogo } from '@cowprotocol/common-const' -import { SupportedChainId } from '@cowprotocol/cow-sdk' +import { areAddressesEqual, SupportedChainId } from '@cowprotocol/cow-sdk' import { Currency, NativeCurrency } from '@cowprotocol/currency' import { doesTokenMatchSymbolOrAddress } from './doesTokenMatchSymbolOrAddress' @@ -29,5 +29,5 @@ export function getIsNativeToken(chainIdOrTokenParams: SupportedChainId | Curren // When token is from Bridge, it's not in the list of native tokens if (!nativeToken) return false - return doesTokenMatchSymbolOrAddress(nativeToken, tokenId) + return areAddressesEqual(nativeToken.address, tokenId) } diff --git a/libs/permit-utils/src/index.ts b/libs/permit-utils/src/index.ts index 3884d589e1c..356257d475a 100644 --- a/libs/permit-utils/src/index.ts +++ b/libs/permit-utils/src/index.ts @@ -1,4 +1,4 @@ -export { DEFAULT_MIN_GAS_LIMIT, PERMIT_ACCOUNT } from './const' +export { DEFAULT_MIN_GAS_LIMIT, DEFAULT_PERMIT_VALUE, PERMIT_ACCOUNT } from './const' export { checkIsCallDataAValidPermit } from './lib/checkIsCallDataAValidPermit' export { generatePermitHook } from './lib/generatePermitHook' diff --git a/libs/permit-utils/src/lib/generatePermitHook.test.ts b/libs/permit-utils/src/lib/generatePermitHook.test.ts new file mode 100644 index 00000000000..8a0de8ab006 --- /dev/null +++ b/libs/permit-utils/src/lib/generatePermitHook.test.ts @@ -0,0 +1,128 @@ +import { Address } from 'viem' +import { estimateGas } from 'wagmi/actions' + +import { generatePermitHook } from './generatePermitHook' + +import { buildDaiLikePermitCallData, buildEip2612PermitCallData } from '../utils/buildPermitCallData' + +import type { PermitHookParams } from '../types' +import type { Config } from 'wagmi' + +jest.mock('../const', () => ({ + DEFAULT_PERMIT_GAS_LIMIT: 50000n, + DEFAULT_PERMIT_VALUE: 1n, + PERMIT_ACCOUNT: { + address: '0x0000000000000000000000000000000000000001', + }, +})) + +jest.mock('wagmi/actions', () => ({ + estimateGas: jest.fn(), +})) + +jest.mock('../utils/buildPermitCallData', () => ({ + buildEip2612PermitCallData: jest.fn(), + buildDaiLikePermitCallData: jest.fn(), +})) + +const mockEstimateGas = estimateGas as jest.MockedFunction +const mockBuildEip2612PermitCallData = buildEip2612PermitCallData as jest.MockedFunction< + typeof buildEip2612PermitCallData +> +const mockBuildDaiLikePermitCallData = buildDaiLikePermitCallData as jest.MockedFunction< + typeof buildDaiLikePermitCallData +> + +describe('generatePermitHook request cache', () => { + const config = {} as Config + const tokenAddress = '0x1111111111111111111111111111111111111111' as Address + const account = '0x2222222222222222222222222222222222222222' as Address + const spender = '0x3333333333333333333333333333333333333333' + const otherSpender = '0x4444444444444444444444444444444444444444' + + const eip2612Utils = { + getTokenNonce: jest.fn().mockResolvedValue(7), + } + + function createParams(overrides: Partial = {}): PermitHookParams { + return { + chainId: 1, + config, + eip2612Utils: eip2612Utils as unknown as PermitHookParams['eip2612Utils'], + inputToken: { + address: tokenAddress, + name: 'Test Token', + }, + permitInfo: { + type: 'eip-2612', + name: 'Test Token', + version: '1', + }, + spender, + account, + amount: 123n, + nonce: 7, + ...overrides, + } + } + + beforeEach(() => { + jest.clearAllMocks() + + mockEstimateGas.mockResolvedValue(45000n) + mockBuildEip2612PermitCallData.mockResolvedValue('0xpermit') + mockBuildDaiLikePermitCallData.mockResolvedValue('0xpermit') + }) + + it('reuses the in-flight request when the spender is unchanged', async () => { + const [first, second] = await Promise.all([generatePermitHook(createParams()), generatePermitHook(createParams())]) + + expect(first).toEqual(second) + expect(mockBuildEip2612PermitCallData).toHaveBeenCalledTimes(1) + }) + + it('does not reuse the in-flight request when the spender changes', async () => { + await Promise.all([generatePermitHook(createParams()), generatePermitHook(createParams({ spender: otherSpender }))]) + + expect(mockBuildEip2612PermitCallData).toHaveBeenCalledTimes(2) + }) + + it('does not reuse the in-flight request when the nonce changes', async () => { + await Promise.all([generatePermitHook(createParams()), generatePermitHook(createParams({ nonce: 8 }))]) + + expect(mockBuildEip2612PermitCallData).toHaveBeenCalledTimes(2) + }) + + it.each([ + ['name', { type: 'eip-2612' as const, name: 'Other Token', version: '1' }], + ['version', { type: 'eip-2612' as const, name: 'Test Token', version: '2' }], + ])('does not reuse the in-flight request when the permit %s changes', async (_, permitInfo) => { + await Promise.all([generatePermitHook(createParams()), generatePermitHook(createParams({ permitInfo }))]) + + expect(mockBuildEip2612PermitCallData).toHaveBeenCalledTimes(2) + }) + + it('does not reuse the in-flight request when the permit type changes', async () => { + await Promise.all([ + generatePermitHook(createParams()), + generatePermitHook(createParams({ permitInfo: { type: 'dai-like', name: 'Test Token', version: '1' } })), + ]) + + expect(mockBuildEip2612PermitCallData).toHaveBeenCalledTimes(1) + expect(mockBuildDaiLikePermitCallData).toHaveBeenCalledTimes(1) + }) + + it('preserves an explicit zero amount in permit calldata', async () => { + await generatePermitHook({ ...createParams(), amount: 0n }) + + expect(mockBuildEip2612PermitCallData).toHaveBeenCalledWith( + expect.objectContaining({ + callDataParams: expect.arrayContaining([ + expect.objectContaining({ + value: '0', + }), + ]), + }), + ) + }) +}) diff --git a/libs/permit-utils/src/lib/generatePermitHook.ts b/libs/permit-utils/src/lib/generatePermitHook.ts index d60da8bf2cf..a87edd465f9 100644 --- a/libs/permit-utils/src/lib/generatePermitHook.ts +++ b/libs/permit-utils/src/lib/generatePermitHook.ts @@ -1,3 +1,4 @@ +import { getAddressKey } from '@cowprotocol/cow-sdk' import { PERMIT_HOOK_DAPP_ID } from '@cowprotocol/hook-dapp-lib' import { Address, Hex } from 'viem' @@ -17,11 +18,35 @@ const REQUESTS_CACHE: { [permitKey: string]: Promise const USER_REJECTION_CODES = [4001, -32000] const USER_REJECTION_MESSAGES = ['user denied', 'user rejected', 'rejected transaction', 'transaction was rejected'] -// eslint-disable-next-line @typescript-eslint/no-explicit-any -function isUserRejectionError(error: any): boolean { +function hasUserRejectionCode(error: unknown): boolean { + return ( + typeof error === 'object' && + error !== null && + 'code' in error && + USER_REJECTION_CODES.includes(error.code as number) + ) +} + +function getErrorMessage(error: unknown): string { + if (typeof error === 'string') { + return error.toLowerCase() + } + + if (typeof error === 'object' && error !== null && 'message' in error && typeof error.message === 'string') { + return error.message.toLowerCase() + } + + return '' +} + +function isUserRejectionError(error: unknown): boolean { if (!error) return false - if (USER_REJECTION_CODES.includes(error.code)) return true - const message = (typeof error === 'string' ? error : error.message)?.toLowerCase() || '' + if (hasUserRejectionCode(error)) { + return true + } + + const message = getErrorMessage(error) + return USER_REJECTION_MESSAGES.some((msg) => message.includes(msg)) } @@ -75,7 +100,7 @@ async function generatePermitHookRaw(params: PermitHookParams): Promise ({ + DEFAULT_MIN_GAS_LIMIT: 50000n, + DEFAULT_PERMIT_VALUE: 1n, + PERMIT_ACCOUNT: { + address: '0x0000000000000000000000000000000000000001', + }, +})) + +jest.mock('./getPermitUtilsInstance', () => ({ + getPermitUtilsInstance: jest.fn(), +})) + +jest.mock('../utils/getEip712Domain', () => ({ + getEip712Domain: jest.fn(), +})) + +jest.mock('../utils/buildPermitCallData', () => ({ + buildEip2612PermitCallData: jest.fn(), + buildDaiLikePermitCallData: jest.fn(), +})) + +jest.mock('wagmi/actions', () => ({ + estimateGas: jest.fn(), +})) + +const mockedEstimateGas = estimateGas as jest.MockedFunction +const mockedGetPermitUtilsInstance = getPermitUtilsInstance as jest.MockedFunction +const mockedGetEip712Domain = getEip712Domain as jest.MockedFunction +const mockedBuildEip2612PermitCallData = buildEip2612PermitCallData as jest.MockedFunction< + typeof buildEip2612PermitCallData +> + +describe('getTokenPermitInfo request cache', () => { + const config = {} as Config + const publicClient = {} + const spender = '0x3333333333333333333333333333333333333333' + const otherSpender = '0x4444444444444444444444444444444444444444' + + function createParams( + tokenAddress: Address, + overrides: Pick, 'amount' | 'minGasLimit' | 'spender'> = {}, + ): GetTokenPermitInfoParams { + return { + tokenAddress, + chainId: 1, + spender, + config, + publicClient, + minGasLimit: 50000n, + ...overrides, + } + } + + beforeEach(() => { + jest.clearAllMocks() + + mockedGetPermitUtilsInstance.mockResolvedValue({ + getTokenNonce: jest.fn().mockResolvedValue(7), + } as Awaited>) + mockedGetEip712Domain.mockResolvedValue({ name: 'Test Token', version: '1' }) + mockedBuildEip2612PermitCallData.mockResolvedValue('0xpermit') + mockedEstimateGas.mockResolvedValue(60000n) + }) + + it('reuses the in-flight request when the spender is unchanged', async () => { + const tokenAddress = '0x1111111111111111111111111111111111111111' as Address + + await Promise.all([getTokenPermitInfo(createParams(tokenAddress)), getTokenPermitInfo(createParams(tokenAddress))]) + + expect(mockedBuildEip2612PermitCallData).toHaveBeenCalledTimes(1) + expect(mockedEstimateGas).toHaveBeenCalledTimes(1) + }) + + it('does not reuse the in-flight request when the spender changes', async () => { + const tokenAddress = '0x2222222222222222222222222222222222222222' as Address + + await Promise.all([ + getTokenPermitInfo(createParams(tokenAddress)), + getTokenPermitInfo(createParams(tokenAddress, { spender: otherSpender })), + ]) + + expect(mockedBuildEip2612PermitCallData).toHaveBeenCalledTimes(2) + expect(mockedEstimateGas).toHaveBeenCalledTimes(2) + }) + + it('does not reuse the in-flight request when the amount changes', async () => { + const tokenAddress = '0x6666666666666666666666666666666666666666' as Address + + await Promise.all([ + getTokenPermitInfo(createParams(tokenAddress, { amount: 1n })), + getTokenPermitInfo(createParams(tokenAddress, { amount: 2n })), + ]) + + expect(mockedBuildEip2612PermitCallData).toHaveBeenCalledTimes(2) + expect(mockedEstimateGas).toHaveBeenCalledTimes(2) + }) + + it('does not reuse the in-flight request when the minimum gas limit changes', async () => { + const tokenAddress = '0x7777777777777777777777777777777777777777' as Address + + await Promise.all([ + getTokenPermitInfo(createParams(tokenAddress, { minGasLimit: 50000n })), + getTokenPermitInfo(createParams(tokenAddress, { minGasLimit: 70000n })), + ]) + + expect(mockedBuildEip2612PermitCallData).toHaveBeenCalledTimes(2) + expect(mockedEstimateGas).toHaveBeenCalledTimes(2) + }) + + it('does not cache transient error results', async () => { + const tokenAddress = '0x5555555555555555555555555555555555555555' as Address + const consoleDebugSpy = jest.spyOn(console, 'debug').mockImplementation(() => undefined) + + mockedGetPermitUtilsInstance + .mockResolvedValueOnce({ + getTokenNonce: jest.fn().mockRejectedValue(new Error('RPC timeout')), + } as Awaited>) + .mockResolvedValueOnce({ + getTokenNonce: jest.fn().mockResolvedValue(7), + } as Awaited>) + + try { + await expect(getTokenPermitInfo(createParams(tokenAddress))).resolves.toEqual({ error: 'RPC timeout' }) + await expect(getTokenPermitInfo(createParams(tokenAddress))).resolves.toEqual({ + type: 'eip-2612', + name: 'Test Token', + version: '1', + }) + + expect(mockedGetPermitUtilsInstance).toHaveBeenCalledTimes(2) + expect(mockedEstimateGas).toHaveBeenCalledTimes(1) + } finally { + consoleDebugSpy.mockRestore() + } + }) +}) diff --git a/libs/permit-utils/src/lib/getTokenPermitInfo.ts b/libs/permit-utils/src/lib/getTokenPermitInfo.ts index 9ad59994cf0..9a163d46b95 100644 --- a/libs/permit-utils/src/lib/getTokenPermitInfo.ts +++ b/libs/permit-utils/src/lib/getTokenPermitInfo.ts @@ -1,4 +1,4 @@ -import { getTokenId } from '@cowprotocol/cow-sdk' +import { getAddressKey, getTokenId } from '@cowprotocol/cow-sdk' import { Config } from 'wagmi' import { estimateGas } from 'wagmi/actions' @@ -33,8 +33,10 @@ const REQUESTS_CACHE: Record> = {} const UNSUPPORTED: PermitInfo = { type: 'unsupported' } export async function getTokenPermitInfo(params: GetTokenPermitInfoParams): Promise { - const { tokenAddress, chainId } = params - const key = getTokenId({ address: tokenAddress, chainId }) + const { tokenAddress, chainId, spender, amount = DEFAULT_PERMIT_VALUE, minGasLimit = DEFAULT_MIN_GAS_LIMIT } = params + const key = `${getTokenId({ address: tokenAddress, chainId })}-${getAddressKey( + spender, + )}-${amount.toString()}-${minGasLimit.toString()}` const cached = REQUESTS_CACHE[key] @@ -43,6 +45,17 @@ export async function getTokenPermitInfo(params: GetTokenPermitInfoParams): Prom } const request = actuallyCheckTokenIsPermittable(params) + .then((result) => { + if ('error' in result) { + delete REQUESTS_CACHE[key] + } + + return result + }) + .catch((error) => { + delete REQUESTS_CACHE[key] + throw error + }) REQUESTS_CACHE[key] = request