Skip to content

Commit 9a7cc91

Browse files
fairlightethfairlighteth
committed
fix: isolate permit cache by spender
1 parent c03a2d3 commit 9a7cc91

13 files changed

Lines changed: 600 additions & 46 deletions

‎apps/cowswap-frontend/src/modules/permit/hooks/usePermitCompatibleTokens.test.ts‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
import { useAtomValue } from 'jotai'
2+
3+
import { getAddressKey } from '@cowprotocol/cow-sdk'
4+
import { PermitInfo } from '@cowprotocol/permit-utils'
5+
import { useWalletInfo } from '@cowprotocol/wallet'
6+
7+
import { renderHook } from '@testing-library/react'
8+
9+
import { useIsPermitEnabled } from 'common/hooks/featureFlags/useIsPermitEnabled'
10+
11+
import { usePermitCompatibleTokens } from './usePermitCompatibleTokens'
12+
import { usePreGeneratedPermitInfo } from './usePreGeneratedPermitInfo'
13+
14+
import { getPermittableTokenKey } from '../state/permittableTokensAtom'
15+
16+
jest.mock('jotai', () => ({
17+
...jest.requireActual('jotai'),
18+
useAtomValue: jest.fn(),
19+
}))
20+
21+
jest.mock('@cowprotocol/wallet', () => ({
22+
useWalletInfo: jest.fn(),
23+
}))
24+
25+
jest.mock('common/hooks/featureFlags/useIsPermitEnabled', () => ({
26+
useIsPermitEnabled: jest.fn(),
27+
}))
28+
29+
jest.mock('./usePreGeneratedPermitInfo', () => ({
30+
usePreGeneratedPermitInfo: jest.fn(),
31+
}))
32+
33+
const mockedUseAtomValue = useAtomValue as jest.MockedFunction<typeof useAtomValue>
34+
const mockedUseWalletInfo = useWalletInfo as jest.MockedFunction<typeof useWalletInfo>
35+
const mockedUseIsPermitEnabled = useIsPermitEnabled as jest.MockedFunction<typeof useIsPermitEnabled>
36+
const mockedUsePreGeneratedPermitInfo = usePreGeneratedPermitInfo as jest.MockedFunction<
37+
typeof usePreGeneratedPermitInfo
38+
>
39+
40+
describe('usePermitCompatibleTokens', () => {
41+
const chainId = 1
42+
const tokenAddress = '0x1234567890123456789012345678901234567890'
43+
const spender = '0x1111111111111111111111111111111111111111'
44+
const permitInfo: PermitInfo = { type: 'eip-2612', name: 'Test Token', version: '1' }
45+
46+
beforeEach(() => {
47+
jest.clearAllMocks()
48+
49+
mockedUseWalletInfo.mockReturnValue({ chainId } as ReturnType<typeof useWalletInfo>)
50+
mockedUseIsPermitEnabled.mockReturnValue(true)
51+
mockedUsePreGeneratedPermitInfo.mockReturnValue({ allPermitInfo: {}, isLoading: false })
52+
})
53+
54+
it('maps local token-spender permit info back to the token address', () => {
55+
const permitTokenKey = getPermittableTokenKey(tokenAddress, spender)
56+
57+
mockedUseAtomValue.mockReturnValue({
58+
[chainId]: {
59+
[permitTokenKey]: permitInfo,
60+
},
61+
})
62+
63+
const { result } = renderHook(() => usePermitCompatibleTokens())
64+
65+
expect(result.current[getAddressKey(tokenAddress)]).toBe(true)
66+
expect(result.current[permitTokenKey]).toBeUndefined()
67+
})
68+
})

‎apps/cowswap-frontend/src/modules/permit/hooks/usePermitCompatibleTokens.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import { useIsPermitEnabled } from 'common/hooks/featureFlags/useIsPermitEnabled
99

1010
import { usePreGeneratedPermitInfo } from './usePreGeneratedPermitInfo'
1111

12-
import { permittableTokensAtom } from '../state/permittableTokensAtom'
12+
import { getTokenAddressFromPermittableTokenKey, permittableTokensAtom } from '../state/permittableTokensAtom'
1313
import { PermitCompatibleTokens } from '../types'
1414

1515
export function usePermitCompatibleTokens(): PermitCompatibleTokens {
@@ -43,10 +43,10 @@ export function usePermitCompatibleTokens(): PermitCompatibleTokens {
4343
)
4444
}
4545

46-
for (const address of Object.keys(localPermitInfoRef.current)) {
47-
const addressLowerCased = getAddressKey(address)
46+
for (const permitTokenKey of Object.keys(localPermitInfoRef.current)) {
47+
const addressLowerCased = getAddressKey(getTokenAddressFromPermittableTokenKey(permitTokenKey))
4848

49-
permitCompatibleTokens[addressLowerCased] = isSupportedPermitInfo(localPermitInfoRef.current[addressLowerCased])
49+
permitCompatibleTokens[addressLowerCased] = isSupportedPermitInfo(localPermitInfoRef.current[permitTokenKey])
5050
}
5151

5252
return permitCompatibleTokens

‎apps/cowswap-frontend/src/modules/permit/hooks/usePermitInfo.test.ts‎

Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,176 @@
1+
import { useAtomValue, useSetAtom } from 'jotai'
2+
3+
import { Token } from '@cowprotocol/currency'
4+
import { getTokenPermitInfo } from '@cowprotocol/permit-utils'
5+
import type { PermitInfo } from '@cowprotocol/permit-utils'
6+
import { useWalletInfo } from '@cowprotocol/wallet'
7+
8+
import { renderHook, waitFor } from '@testing-library/react'
9+
import { useConfig, usePublicClient } from 'wagmi'
10+
11+
import { TradeType } from 'modules/trade'
12+
13+
import { useIsPermitEnabled } from 'common/hooks/featureFlags/useIsPermitEnabled'
14+
15+
import { usePermitInfo } from './usePermitInfo'
16+
import { usePreGeneratedPermitInfoForToken } from './usePreGeneratedPermitInfoForToken'
17+
18+
import { getPermittableTokenKey } from '../state/permittableTokensAtom'
19+
20+
const defaultSpender = '0x9008D19f58AAbD9eD0D60971565AA8510560ab41'
21+
const customSpender = '0x1111111111111111111111111111111111111111'
22+
23+
jest.mock('jotai', () => ({
24+
...jest.requireActual('jotai'),
25+
useAtomValue: jest.fn(),
26+
useSetAtom: jest.fn(),
27+
}))
28+
29+
jest.mock('@cowprotocol/common-utils', () => ({
30+
COW_PROTOCOL_VAULT_RELAYER_ADDRESS: { 1: defaultSpender },
31+
getIsNativeToken: jest.fn().mockReturnValue(false),
32+
getWrappedToken: jest.fn((token) => token),
33+
isAddress: jest.fn(),
34+
}))
35+
36+
jest.mock('@cowprotocol/permit-utils', () => ({
37+
DEFAULT_MIN_GAS_LIMIT: 50000n,
38+
getTokenPermitInfo: jest.fn(),
39+
}))
40+
41+
jest.mock('@cowprotocol/wallet', () => ({
42+
useWalletInfo: jest.fn(),
43+
}))
44+
45+
jest.mock('wagmi', () => ({
46+
useConfig: jest.fn(),
47+
usePublicClient: jest.fn(),
48+
}))
49+
50+
jest.mock('common/hooks/featureFlags/useIsPermitEnabled', () => ({
51+
useIsPermitEnabled: jest.fn(),
52+
}))
53+
54+
jest.mock('modules/trade', () => ({
55+
TradeType: {
56+
SWAP: 'SWAP',
57+
LIMIT_ORDER: 'LIMIT_ORDER',
58+
ADVANCED_ORDERS: 'ADVANCED_ORDERS',
59+
YIELD: 'YIELD',
60+
},
61+
}))
62+
63+
jest.mock('./usePreGeneratedPermitInfoForToken', () => ({
64+
usePreGeneratedPermitInfoForToken: jest.fn(),
65+
}))
66+
67+
const mockedUseAtomValue = useAtomValue as jest.MockedFunction<typeof useAtomValue>
68+
const mockedUseSetAtom = useSetAtom as jest.MockedFunction<typeof useSetAtom>
69+
const mockedGetTokenPermitInfo = getTokenPermitInfo as jest.MockedFunction<typeof getTokenPermitInfo>
70+
const mockedUseWalletInfo = useWalletInfo as jest.MockedFunction<typeof useWalletInfo>
71+
const mockedUseConfig = useConfig as jest.MockedFunction<typeof useConfig>
72+
const mockedUsePublicClient = usePublicClient as jest.MockedFunction<typeof usePublicClient>
73+
const mockedUseIsPermitEnabled = useIsPermitEnabled as jest.MockedFunction<typeof useIsPermitEnabled>
74+
const mockedUsePreGeneratedPermitInfoForToken = usePreGeneratedPermitInfoForToken as jest.MockedFunction<
75+
typeof usePreGeneratedPermitInfoForToken
76+
>
77+
78+
const commonUtils = jest.requireMock('@cowprotocol/common-utils') as {
79+
isAddress: jest.MockedFunction<(value: string | undefined | null) => string | false>
80+
}
81+
82+
describe('usePermitInfo', () => {
83+
const token = new Token(1, '0x1234567890123456789012345678901234567890', 18, 'TEST', 'Test Token')
84+
const addPermitInfo = jest.fn()
85+
const defaultPermitInfo: PermitInfo = { type: 'eip-2612', name: 'Test Token', version: '1' }
86+
const fetchedPermitInfo: PermitInfo = { type: 'unsupported', name: 'Test Token' }
87+
88+
beforeEach(() => {
89+
jest.clearAllMocks()
90+
91+
mockedUseAtomValue.mockReturnValue({})
92+
mockedUseSetAtom.mockReturnValue(addPermitInfo)
93+
mockedUseWalletInfo.mockReturnValue({ chainId: 1 } as ReturnType<typeof useWalletInfo>)
94+
mockedUseConfig.mockReturnValue({} as ReturnType<typeof useConfig>)
95+
mockedUsePublicClient.mockReturnValue({} as ReturnType<typeof usePublicClient>)
96+
mockedUseIsPermitEnabled.mockReturnValue(true)
97+
mockedUsePreGeneratedPermitInfoForToken.mockImplementation(() => ({ permitInfo: undefined, isLoading: false }))
98+
mockedGetTokenPermitInfo.mockResolvedValue(fetchedPermitInfo)
99+
commonUtils.isAddress.mockImplementation((value) => {
100+
if (!value || !/^0x[a-fA-F0-9]{40}$/.test(value)) return false
101+
102+
return value
103+
})
104+
})
105+
106+
it('does not reuse cached permit info that belongs to a different spender', async () => {
107+
mockedUseAtomValue.mockReturnValue({
108+
1: {
109+
[getPermittableTokenKey(token.address, defaultSpender)]: defaultPermitInfo,
110+
},
111+
})
112+
113+
const { result } = renderHook(() => usePermitInfo(token, TradeType.SWAP, customSpender))
114+
115+
expect(result.current).toBeUndefined()
116+
117+
await waitFor(() => {
118+
expect(mockedGetTokenPermitInfo).toHaveBeenCalledWith(
119+
expect.objectContaining({
120+
spender: customSpender,
121+
tokenAddress: token.address,
122+
}),
123+
)
124+
})
125+
})
126+
127+
it('revalidates custom spenders instead of reusing pre-generated permit info', async () => {
128+
mockedUsePreGeneratedPermitInfoForToken.mockImplementation((tokenToCheck) => ({
129+
permitInfo: tokenToCheck ? defaultPermitInfo : undefined,
130+
isLoading: false,
131+
}))
132+
133+
const { result } = renderHook(() => usePermitInfo(token, TradeType.SWAP, customSpender))
134+
135+
expect(result.current).toBeUndefined()
136+
expect(mockedUsePreGeneratedPermitInfoForToken).toHaveBeenCalledWith(undefined)
137+
138+
await waitFor(() => {
139+
expect(mockedGetTokenPermitInfo).toHaveBeenCalledWith(
140+
expect.objectContaining({
141+
spender: customSpender,
142+
tokenAddress: token.address,
143+
}),
144+
)
145+
})
146+
147+
await waitFor(() => {
148+
expect(addPermitInfo).toHaveBeenCalledWith({
149+
chainId: 1,
150+
tokenAddress: token.address,
151+
spender: customSpender,
152+
permitInfo: fetchedPermitInfo,
153+
})
154+
})
155+
})
156+
157+
it('reuses pre-generated permit info for the default spender', () => {
158+
mockedUsePreGeneratedPermitInfoForToken.mockImplementation((tokenToCheck) => ({
159+
permitInfo: tokenToCheck ? defaultPermitInfo : undefined,
160+
isLoading: false,
161+
}))
162+
163+
const { result } = renderHook(() => usePermitInfo(token, TradeType.SWAP))
164+
165+
expect(result.current).toEqual(defaultPermitInfo)
166+
expect(mockedGetTokenPermitInfo).not.toHaveBeenCalled()
167+
})
168+
169+
it('skips permit lookup for an invalid custom spender', () => {
170+
const { result } = renderHook(() => usePermitInfo(token, TradeType.SWAP, '0x1'))
171+
172+
expect(result.current).toBeUndefined()
173+
expect(mockedUsePreGeneratedPermitInfoForToken).toHaveBeenCalledWith(undefined)
174+
expect(mockedGetTokenPermitInfo).not.toHaveBeenCalled()
175+
})
176+
})

‎apps/cowswap-frontend/src/modules/permit/hooks/usePermitInfo.ts‎

Lines changed: 43 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,12 @@
11
import { useAtomValue, useSetAtom } from 'jotai'
22
import { useEffect, useMemo } from 'react'
33

4-
import { getIsNativeToken, getWrappedToken, COW_PROTOCOL_VAULT_RELAYER_ADDRESS } from '@cowprotocol/common-utils'
4+
import {
5+
getIsNativeToken,
6+
getWrappedToken,
7+
COW_PROTOCOL_VAULT_RELAYER_ADDRESS,
8+
isAddress,
9+
} from '@cowprotocol/common-utils'
510
import { getAddressKey, mapSupportedNetworks, SupportedChainId } from '@cowprotocol/cow-sdk'
611
import { Currency } from '@cowprotocol/currency'
712
import { DEFAULT_MIN_GAS_LIMIT, getTokenPermitInfo, PermitInfo } from '@cowprotocol/permit-utils'
@@ -10,13 +15,17 @@ import { useWalletInfo } from '@cowprotocol/wallet'
1015
import { Nullish } from 'types'
1116
import { useConfig, usePublicClient } from 'wagmi'
1217

13-
import { TradeType } from 'modules/trade/types/TradeType'
18+
import { TradeType } from 'modules/trade'
1419

1520
import { useIsPermitEnabled } from 'common/hooks/featureFlags/useIsPermitEnabled'
1621

1722
import { usePreGeneratedPermitInfoForToken } from './usePreGeneratedPermitInfoForToken'
1823

19-
import { addPermitInfoForTokenAtom, permittableTokensAtom } from '../state/permittableTokensAtom'
24+
import {
25+
addPermitInfoForTokenAtom,
26+
getPermittableTokenKey,
27+
permittableTokensAtom,
28+
} from '../state/permittableTokensAtom'
2029
import { IsTokenPermittableResult } from '../types'
2130

2231
const ORDER_TYPE_SUPPORTS_PERMIT: Record<TradeType, boolean> = {
@@ -63,29 +72,31 @@ export function usePermitInfo(
6372
const isPermitSupported = !!tradeType && ORDER_TYPE_SUPPORTS_PERMIT[tradeType]
6473

6574
const isPermitEnabled = useIsPermitEnabled() && isPermitSupported
75+
const defaultSpender = chainId ? COW_PROTOCOL_VAULT_RELAYER_ADDRESS[chainId] : undefined
76+
const customSpenderAddress = customSpender ? isAddress(customSpender) || undefined : undefined
77+
const spender = customSpender ? customSpenderAddress : defaultSpender
78+
const shouldUsePreGeneratedInfo = !customSpender && spender === defaultSpender
6679

6780
const addPermitInfo = useAddPermitInfo()
68-
const permitInfo = usePermitInfoState(chainId, isPermitEnabled ? lowerCaseAddress : undefined)
81+
const permitInfo = usePermitInfoState(chainId, isPermitEnabled ? lowerCaseAddress : undefined, spender)
6982
const { permitInfo: preGeneratedInfo, isLoading: preGeneratedIsLoading } = usePreGeneratedPermitInfoForToken(
70-
isPermitEnabled && !isNative ? token : undefined,
83+
isPermitEnabled && !isNative && shouldUsePreGeneratedInfo ? token : undefined,
7184
)
72-
73-
const spender = customSpender || COW_PROTOCOL_VAULT_RELAYER_ADDRESS[chainId]
85+
const hasResolvedPermitInfo =
86+
permitInfo !== undefined || (shouldUsePreGeneratedInfo && preGeneratedInfo !== undefined)
87+
const shouldSkipPermitInfoLoad =
88+
!chainId ||
89+
!isPermitEnabled ||
90+
!lowerCaseAddress ||
91+
!spender ||
92+
!config ||
93+
!publicClient ||
94+
hasResolvedPermitInfo ||
95+
isNative ||
96+
preGeneratedIsLoading
7497

7598
useEffect(() => {
76-
if (
77-
!chainId ||
78-
!isPermitEnabled ||
79-
!lowerCaseAddress ||
80-
!config ||
81-
!publicClient ||
82-
permitInfo !== undefined ||
83-
isNative ||
84-
// Do not try to load when pre-generated info is loading
85-
preGeneratedIsLoading ||
86-
// Do not try to load when pre-generated exists
87-
preGeneratedInfo !== undefined
88-
) {
99+
if (shouldSkipPermitInfoLoad) {
89100
return
90101
}
91102

@@ -108,7 +119,7 @@ export function usePermitInfo(
108119
// TODO: there is a Single Responsibility Principle breach here. This hook should not be responsible for caching.
109120
// TODO: better to create a separate updater for caching.
110121
// Otherwise, we know it is permittable or not. Cache it.
111-
addPermitInfo({ chainId, tokenAddress: lowerCaseAddress, permitInfo: result })
122+
addPermitInfo({ chainId, tokenAddress: lowerCaseAddress, spender, permitInfo: result })
112123
}
113124
})
114125
}, [
@@ -122,14 +133,16 @@ export function usePermitInfo(
122133
permitInfo,
123134
preGeneratedInfo,
124135
preGeneratedIsLoading,
136+
shouldUsePreGeneratedInfo,
137+
shouldSkipPermitInfoLoad,
125138
spender,
126139
])
127140

128141
if (isNative) {
129142
return UNSUPPORTED
130143
}
131144

132-
return preGeneratedInfo ?? permitInfo
145+
return shouldUsePreGeneratedInfo ? (preGeneratedInfo ?? permitInfo) : permitInfo
133146
}
134147

135148
/**
@@ -141,12 +154,16 @@ function useAddPermitInfo() {
141154
return useSetAtom(addPermitInfoForTokenAtom)
142155
}
143156

144-
function usePermitInfoState(chainId: SupportedChainId, tokenAddress: string | undefined): IsTokenPermittableResult {
157+
function usePermitInfoState(
158+
chainId: SupportedChainId,
159+
tokenAddress: string | undefined,
160+
spender: string | undefined,
161+
): IsTokenPermittableResult {
145162
const permitableTokens = useAtomValue(permittableTokensAtom)
146163

147164
return useMemo(() => {
148-
if (!tokenAddress) return undefined
165+
if (!tokenAddress || !spender) return undefined
149166

150-
return permitableTokens[chainId]?.[getAddressKey(tokenAddress)]
151-
}, [chainId, permitableTokens, tokenAddress])
167+
return permitableTokens[chainId]?.[getPermittableTokenKey(tokenAddress, spender)]
168+
}, [chainId, permitableTokens, spender, tokenAddress])
152169
}

0 commit comments

Comments
 (0)