feat: implement consent for all flows #6769
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughCentralizes RWA-aware trade confirmation into a new Changes
Sequence Diagram(s)sequenceDiagram
participant UI as TradeButtons / ActionButtons
participant Hook as useConfirmTradeWithRwaCheck
participant RWA as RWA Module
participant Modal as RwaConsentModal
participant Trade as Trade Confirmation
rect rgb(240,248,255)
Note over UI,Trade: Trade confirmation with RWA consent check
end
UI->>Hook: confirmTrade()
Hook->>RWA: fetchRwaStatus(input/output tokens)
alt RWA consent required
Hook->>Modal: openRwaConsentModal(token, consentHash)
Modal->>RWA: user provides consent
RWA-->>Hook: consent recorded
else consent not required
Note over Hook: proceed to confirm
end
Hook->>Trade: open confirmation (tradeConfirmActions.onOpen) after onConfirmOpen
Trade->>UI: confirmation dialog opens
sequenceDiagram
participant Updater as RestrictedTokensListUpdater
participant CacheHook as useRestrictedTokensCache
participant IDB as IndexedDB
participant API as Token Lists API
rect rgb(240,248,255)
Note over Updater,API: Restricted tokens fetch & cache lifecycle
end
Updater->>CacheHook: read shouldFetch
alt shouldFetch = false
Note over CacheHook,IDB: use cached data — early exit
else shouldFetch = true
Updater->>API: fetch token lists (with timeout & retries)
API-->>Updater: token list responses
Updater->>Updater: build tokensMap, countriesPerToken, consentHashPerToken
Updater->>CacheHook: saveToCache(computedListState)
CacheHook->>IDB: persist cached state
CacheHook->>CacheHook: update runtime atom & last-update timestamp
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
libs/tokens/src/updaters/RestrictedTokensListUpdater/index.tsx (1)
9-14: Consider clarifyingMAX_RETRIESnaming.With
MAX_RETRIES = 1and the loopfor (let attempt = 0; attempt < retries; attempt++), this results in only 1 total attempt (no retries). The naming suggests it would be 1 original attempt + 1 retry = 2 total attempts.If the intent is 1 retry after failure, the loop should be
attempt <= retriesor change the constant toMAX_ATTEMPTS = 1.🔎 Option 1: Rename for clarity (if 1 attempt is intended)
-const MAX_RETRIES = 1 +const MAX_ATTEMPTS = 1Then update the function signature and loop accordingly.
🔎 Option 2: Fix loop to allow actual retries
- for (let attempt = 0; attempt < retries; attempt++) { + for (let attempt = 0; attempt <= retries; attempt++) {
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/cowswap-frontend/src/modules/limitOrders/containers/TradeButtons/index.tsxapps/cowswap-frontend/src/modules/swap/containers/TradeButtons/index.tsxapps/cowswap-frontend/src/modules/trade/hooks/useConfirmTradeWithRwaCheck.tsapps/cowswap-frontend/src/modules/trade/index.tsapps/cowswap-frontend/src/modules/twap/containers/ActionButtons/index.tsxlibs/tokens/src/hooks/useRestrictedTokensCache.tslibs/tokens/src/state/restrictedTokens/restrictedTokensAtom.tslibs/tokens/src/updaters/RestrictedTokensListUpdater/index.tsx
🧰 Additional context used
🧠 Learnings (13)
📓 Common learnings
Learnt from: fairlighteth
Repo: cowprotocol/cowswap PR: 6347
File: apps/cowswap-frontend/src/modules/trade/pure/TradeConfirmation/index.tsx:49-49
Timestamp: 2025-10-10T20:28:16.565Z
Learning: In apps/cowswap-frontend/src/modules/trade, TradeConfirmation follows a two-layer architecture: TradeConfirmationView (pure/stateless) in pure/TradeConfirmation/index.tsx renders the UI, while TradeConfirmation (container) in containers/TradeConfirmation/index.tsx wraps it to freeze props during pending trades (via useStableTradeConfirmationProps), wire in signing state (useSigningStep), and inject trade confirmation state (useTradeConfirmState). Consuming modules should import the container TradeConfirmation from 'modules/trade' to preserve this stateful behavior.
<!-- [add_learning]
When reviewing component refactoring in apps/cowswap-frontend/src/modules/trade, recognize the pattern where a pure view component (e.g., TradeConfirmationView) is separated from a stateful container (e.g., TradeConfirmation) that wraps it. The container adds runtime state management (prop stabilization, signing state, etc.) while the view remains testable and composable. Do not flag usages that import th...
Learnt from: limitofzero
Repo: cowprotocol/cowswap PR: 6351
File: apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.ts:87-121
Timestamp: 2025-10-13T19:41:31.440Z
Learning: In apps/cowswap-frontend/src/modules/erc20Approve, useApproveCallback returns Promise<TransactionResponse | undefined> and is distinct from useApproveCurrency, which can return Promise<TransactionReceipt | SafeMultisigTransactionResponse>. When reviewing approval flows, verify which hook is actually being used before flagging Safe wallet concerns.
📚 Learning: 2025-10-10T20:28:16.565Z
Learnt from: fairlighteth
Repo: cowprotocol/cowswap PR: 6347
File: apps/cowswap-frontend/src/modules/trade/pure/TradeConfirmation/index.tsx:49-49
Timestamp: 2025-10-10T20:28:16.565Z
Learning: In apps/cowswap-frontend/src/modules/trade, TradeConfirmation follows a two-layer architecture: TradeConfirmationView (pure/stateless) in pure/TradeConfirmation/index.tsx renders the UI, while TradeConfirmation (container) in containers/TradeConfirmation/index.tsx wraps it to freeze props during pending trades (via useStableTradeConfirmationProps), wire in signing state (useSigningStep), and inject trade confirmation state (useTradeConfirmState). Consuming modules should import the container TradeConfirmation from 'modules/trade' to preserve this stateful behavior.
<!-- [add_learning]
When reviewing component refactoring in apps/cowswap-frontend/src/modules/trade, recognize the pattern where a pure view component (e.g., TradeConfirmationView) is separated from a stateful container (e.g., TradeConfirmation) that wraps it. The container adds runtime state management (prop stabilization, signing state, etc.) while the view remains testable and composable. Do not flag usages that import th...
Applied to files:
apps/cowswap-frontend/src/modules/trade/index.tsapps/cowswap-frontend/src/modules/trade/hooks/useConfirmTradeWithRwaCheck.tsapps/cowswap-frontend/src/modules/swap/containers/TradeButtons/index.tsxapps/cowswap-frontend/src/modules/twap/containers/ActionButtons/index.tsxapps/cowswap-frontend/src/modules/limitOrders/containers/TradeButtons/index.tsx
📚 Learning: 2025-02-20T15:59:33.749Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 5443
File: apps/cowswap-frontend/src/modules/swap/containers/ConfirmSwapModalSetup/index.tsx:71-71
Timestamp: 2025-02-20T15:59:33.749Z
Learning: The swap module in apps/cowswap-frontend/src/modules/swap/ is marked for deletion in PR #5444 as part of the swap widget unification effort.
Applied to files:
apps/cowswap-frontend/src/modules/trade/index.tsapps/cowswap-frontend/src/modules/swap/containers/TradeButtons/index.tsx
📚 Learning: 2025-10-13T19:41:31.440Z
Learnt from: limitofzero
Repo: cowprotocol/cowswap PR: 6351
File: apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.ts:87-121
Timestamp: 2025-10-13T19:41:31.440Z
Learning: In apps/cowswap-frontend/src/modules/erc20Approve, useApproveCallback returns Promise<TransactionResponse | undefined> and is distinct from useApproveCurrency, which can return Promise<TransactionReceipt | SafeMultisigTransactionResponse>. When reviewing approval flows, verify which hook is actually being used before flagging Safe wallet concerns.
Applied to files:
apps/cowswap-frontend/src/modules/trade/index.tsapps/cowswap-frontend/src/modules/trade/hooks/useConfirmTradeWithRwaCheck.tsapps/cowswap-frontend/src/modules/swap/containers/TradeButtons/index.tsxapps/cowswap-frontend/src/modules/twap/containers/ActionButtons/index.tsxapps/cowswap-frontend/src/modules/limitOrders/containers/TradeButtons/index.tsx
📚 Learning: 2025-06-23T07:03:50.760Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 5859
File: apps/cowswap-frontend/src/modules/tradeQuote/hooks/useTradeQuotePolling.ts:76-82
Timestamp: 2025-06-23T07:03:50.760Z
Learning: In the useTradeQuotePolling hook, there are two useLayoutEffect hooks that work together: one resets the counter to 0 when the confirmation modal closes, and another automatically triggers pollQuote(false, true) whenever the counter reaches 0. This creates an intentional chain reaction for immediate quote updates.
Applied to files:
apps/cowswap-frontend/src/modules/trade/hooks/useConfirmTradeWithRwaCheck.tsapps/cowswap-frontend/src/modules/swap/containers/TradeButtons/index.tsxapps/cowswap-frontend/src/modules/twap/containers/ActionButtons/index.tsxapps/cowswap-frontend/src/modules/limitOrders/containers/TradeButtons/index.tsx
📚 Learning: 2025-07-24T16:42:53.154Z
Learnt from: cowdan
Repo: cowprotocol/cowswap PR: 6009
File: apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/HighFeeWarning/HighFeeWarningTooltipContent.tsx:23-33
Timestamp: 2025-07-24T16:42:53.154Z
Learning: In apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/HighFeeWarning/HighFeeWarningTooltipContent.tsx, the use of toFixed(2) for percentage formatting in tooltip content is intentional and differs from the banner message formatting that uses toSignificant(2, undefined, Rounding.ROUND_DOWN). This formatting difference serves different UX purposes and should not be flagged as inconsistent.
Applied to files:
apps/cowswap-frontend/src/modules/swap/containers/TradeButtons/index.tsxapps/cowswap-frontend/src/modules/limitOrders/containers/TradeButtons/index.tsx
📚 Learning: 2025-09-19T11:38:59.206Z
Learnt from: fairlighteth
Repo: cowprotocol/cowswap PR: 6232
File: apps/cowswap-frontend/src/modules/tokensList/pure/ChainsSelector/index.tsx:199-200
Timestamp: 2025-09-19T11:38:59.206Z
Learning: The makeBuildClickEvent function in apps/cowswap-frontend/src/modules/tokensList/pure/ChainsSelector/index.tsx takes five parameters: defaultChainId, contextLabel, mode, isSwapMode, and chainsCount. The chainsCount parameter is used to determine the CrossChain flag in analytics events.
Applied to files:
apps/cowswap-frontend/src/modules/swap/containers/TradeButtons/index.tsxapps/cowswap-frontend/src/modules/twap/containers/ActionButtons/index.tsxapps/cowswap-frontend/src/modules/limitOrders/containers/TradeButtons/index.tsx
📚 Learning: 2025-06-16T16:01:46.729Z
Learnt from: alfetopito
Repo: cowprotocol/cowswap PR: 5831
File: apps/cowswap-frontend/src/modules/application/containers/AppMenu/index.tsx:7-9
Timestamp: 2025-06-16T16:01:46.729Z
Learning: React Router v7 restructured packages - NavLink and other core routing components should be imported from 'react-router' (not 'react-router-dom'). In v7, 'react-router-dom' mainly re-exports for backward compatibility, while 'react-router' is the new preferred import path for most components.
Applied to files:
apps/cowswap-frontend/src/modules/swap/containers/TradeButtons/index.tsx
📚 Learning: 2025-06-16T15:58:00.268Z
Learnt from: alfetopito
Repo: cowprotocol/cowswap PR: 5830
File: apps/cowswap-frontend/src/modules/trade/containers/TradeWidget/index.tsx:1-2
Timestamp: 2025-06-16T15:58:00.268Z
Learning: JSX can be imported as a named export from React in modern React versions (React 17+). The import `import { JSX } from 'react'` is valid and does not cause compilation errors.
Applied to files:
apps/cowswap-frontend/src/modules/swap/containers/TradeButtons/index.tsx
📚 Learning: 2025-08-08T13:56:18.009Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6125
File: libs/tokens/src/updaters/TokensListsUpdater/index.tsx:29-31
Timestamp: 2025-08-08T13:56:18.009Z
Learning: In libs/tokens/src/updaters/TokensListsUpdater/index.tsx, the project’s current Jotai version requires using `unstable_getOnInit` (not `getOnInit`) in atomWithStorage options; keep `{ unstable_getOnInit: true }` until Jotai is upgraded.
Applied to files:
libs/tokens/src/hooks/useRestrictedTokensCache.tslibs/tokens/src/state/restrictedTokens/restrictedTokensAtom.tslibs/tokens/src/updaters/RestrictedTokensListUpdater/index.tsx
📚 Learning: 2025-08-08T13:55:17.528Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6125
File: libs/tokens/src/state/tokens/allTokensAtom.ts:78-78
Timestamp: 2025-08-08T13:55:17.528Z
Learning: In libs/tokens/src/state/tokens/allTokensAtom.ts (TypeScript/Jotai), the team prefers to wait for token lists to initialize (listsStatesListAtom non-empty) before returning tokens. No fallback to favorites/user-added/native tokens should be used when listsStatesList is empty.
Applied to files:
libs/tokens/src/hooks/useRestrictedTokensCache.tslibs/tokens/src/state/restrictedTokens/restrictedTokensAtom.tslibs/tokens/src/updaters/RestrictedTokensListUpdater/index.tsx
📚 Learning: 2025-08-12T06:33:19.348Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6137
File: libs/tokens/src/state/tokens/allTokensAtom.ts:34-65
Timestamp: 2025-08-12T06:33:19.348Z
Learning: In libs/tokens/src/state/tokens/allTokensAtom.ts, the parseTokenInfo() function returns a new instance of TokenInfo each time, making it safe to mutate properties like lpTokenProvider on the returned object without side effects.
Applied to files:
libs/tokens/src/hooks/useRestrictedTokensCache.tslibs/tokens/src/state/restrictedTokens/restrictedTokensAtom.tslibs/tokens/src/updaters/RestrictedTokensListUpdater/index.tsx
📚 Learning: 2025-08-12T06:33:19.348Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6137
File: libs/tokens/src/state/tokens/allTokensAtom.ts:34-65
Timestamp: 2025-08-12T06:33:19.348Z
Learning: In libs/tokens/src/utils/parseTokenInfo.ts, the parseTokenInfo() function returns a new instance of TokenInfo using object spread syntax ({ ...token, ... }), making it safe to mutate properties like lpTokenProvider on the returned object without side effects.
Applied to files:
libs/tokens/src/updaters/RestrictedTokensListUpdater/index.tsx
🧬 Code graph analysis (5)
apps/cowswap-frontend/src/modules/swap/containers/TradeButtons/index.tsx (2)
apps/cowswap-frontend/src/modules/swap/hooks/useSwapDerivedState.ts (1)
useSwapDerivedState(12-14)apps/cowswap-frontend/src/modules/trade/hooks/useConfirmTradeWithRwaCheck.ts (1)
useConfirmTradeWithRwaCheck(30-71)
libs/tokens/src/hooks/useRestrictedTokensCache.ts (1)
libs/tokens/src/state/restrictedTokens/restrictedTokensAtom.ts (4)
RestrictedTokenListState(11-16)restrictedTokensAtom(37-37)restrictedTokensCacheAtom(28-31)restrictedTokensLastUpdateAtom(39-44)
apps/cowswap-frontend/src/modules/twap/containers/ActionButtons/index.tsx (1)
apps/cowswap-frontend/src/modules/trade/hooks/useConfirmTradeWithRwaCheck.ts (1)
useConfirmTradeWithRwaCheck(30-71)
apps/cowswap-frontend/src/modules/limitOrders/containers/TradeButtons/index.tsx (1)
apps/cowswap-frontend/src/modules/trade/hooks/useConfirmTradeWithRwaCheck.ts (1)
useConfirmTradeWithRwaCheck(30-71)
libs/tokens/src/updaters/RestrictedTokensListUpdater/index.tsx (1)
libs/tokens/src/hooks/useRestrictedTokensCache.ts (1)
useRestrictedTokensCache(25-61)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Lint
- GitHub Check: Test
- GitHub Check: Build
🔇 Additional comments (14)
libs/tokens/src/hooks/useRestrictedTokensCache.ts (3)
1-18: LGTM! Clean utility setup.The imports, update interval constant, and
isTimeToUpdatehelper are well-structured. The 6-hour cache interval is reasonable for this use case.
25-41: Good cache hydration pattern with proper guard.The
hasLoadedFromCacheref correctly prevents redundant writes when thecachedStateatom value updates asynchronously. The one-time hydration from IndexedDB to runtime state is a sound pattern.
43-60: LGTM! Callback and return value are well-designed.The
saveToCachecallback correctly updates both runtime and persisted state atomically, and theshouldFetchlogic properly handles both time-based and initial-load scenarios.libs/tokens/src/updaters/RestrictedTokensListUpdater/index.tsx (4)
6-7: LGTM! Clean integration with the new cache hook.Good refactor to use
useRestrictedTokensCacheinstead of directly manipulating atoms.
24-38: LGTM! Robust timeout and delay utilities.The
fetchWithTimeoutcorrectly cleans up the timeout infinallyto prevent memory leaks, and usesAbortControllerfor proper fetch cancellation.
40-74: LGTM! Good error handling with smart retry logic.The retry logic correctly skips retries for validation errors (non-transient failures) and uses exponential backoff via
RETRY_DELAY_MS * (attempt + 1).
76-131: LGTM! Well-structured updater with proper cache integration.The effect correctly:
- Guards against disabled feature state
- Uses
shouldFetchfor cache-aware fetching- Handles per-list failures gracefully within
Promise.all- Persists the final state via the cache hook
The dependency array is correct since
saveToCacheis memoized withuseCallback.libs/tokens/src/state/restrictedTokens/restrictedTokensAtom.ts (2)
1-9: LGTM! Proper imports for persistence utilities.The imports correctly bring in
atomWithStorage,atomWithIdbStorage, andgetJotaiMergerStoragefor the new persistence layer.
25-44: LGTM! Well-designed persistence layer with proper Jotai patterns.Good implementation:
restrictedTokensCacheAtomuses IndexedDB for async persistence with versioned keyrestrictedTokensLastUpdateAtomusesunstable_getOnInit: truewhich is required for the current Jotai version (based on learnings)- Clear documentation comments explain the purpose of each atom
The separation between async cache and sync runtime state is a clean pattern for this use case.
apps/cowswap-frontend/src/modules/trade/index.ts (1)
10-10: LGTM! Clean public API addition.The new hook export follows the existing module pattern and properly exposes the centralized RWA consent handling functionality.
apps/cowswap-frontend/src/modules/limitOrders/containers/TradeButtons/index.tsx (1)
7-7: LGTM! Clean migration to centralized RWA consent flow.The change properly delegates RWA consent handling to the new hook while maintaining the component's existing public API and behavior.
Also applies to: 38-38
apps/cowswap-frontend/src/modules/trade/hooks/useConfirmTradeWithRwaCheck.ts (1)
1-71: LGTM! Well-structured centralized RWA consent handling.The hook properly encapsulates the RWA consent check logic with correct dependency arrays and edge case handling. The implementation correctly:
- Uses optional chaining to handle null/undefined derivedState
- Guards against null rwaTokenInfo before accessing it
- Properly memoizes the confirmTrade callback and return value
apps/cowswap-frontend/src/modules/twap/containers/ActionButtons/index.tsx (1)
33-43: LGTM! Analytics properly integrated with RWA consent flow.The analytics callback correctly fires only when the trade confirmation modal opens (not when the RWA consent modal is shown), as documented in the comment. The memoization chain for hookParams is properly structured.
apps/cowswap-frontend/src/modules/swap/containers/TradeButtons/index.tsx (1)
1-1: LGTM! Excellent refactor that eliminates code duplication.The changes successfully:
- Remove inline RWA consent handling logic
- Delegate to the centralized
useConfirmTradeWithRwaCheckhook- Simplify the component while maintaining identical external behavior
- Clean up unnecessary imports (useCallback, RWA-related hooks)
The refactor follows the learnings pattern where stateful logic is extracted to reusable hooks while keeping the component focused on rendering.
Also applies to: 11-11, 56-56, 72-72
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
libs/tokens/src/updaters/RestrictedTokensListUpdater/index.tsx (1)
9-11: Consider renamingMAX_RETRIEStoMAX_ATTEMPTSfor clarity.The constant
MAX_RETRIES = 1is used as the total number of fetch attempts (line 43), not the number of retries after an initial failure. Standard convention treats "retries" as additional attempts beyond the first, soMAX_RETRIES = 1would typically mean 2 total attempts. Renaming toMAX_ATTEMPTSwould eliminate this ambiguity.🔎 Suggested rename
-const MAX_RETRIES = 1 +const MAX_ATTEMPTS = 1And update line 40:
-async function fetchTokenList(url: string, retries = MAX_RETRIES): Promise<TokenInfo[]> { +async function fetchTokenList(url: string, attempts = MAX_ATTEMPTS): Promise<TokenInfo[]> {Then update the loop and error message accordingly.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
libs/tokens/src/updaters/RestrictedTokensListUpdater/index.tsx
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: fairlighteth
Repo: cowprotocol/cowswap PR: 6347
File: apps/cowswap-frontend/src/modules/trade/pure/TradeConfirmation/index.tsx:49-49
Timestamp: 2025-10-10T20:28:16.565Z
Learning: In apps/cowswap-frontend/src/modules/trade, TradeConfirmation follows a two-layer architecture: TradeConfirmationView (pure/stateless) in pure/TradeConfirmation/index.tsx renders the UI, while TradeConfirmation (container) in containers/TradeConfirmation/index.tsx wraps it to freeze props during pending trades (via useStableTradeConfirmationProps), wire in signing state (useSigningStep), and inject trade confirmation state (useTradeConfirmState). Consuming modules should import the container TradeConfirmation from 'modules/trade' to preserve this stateful behavior.
<!-- [add_learning]
When reviewing component refactoring in apps/cowswap-frontend/src/modules/trade, recognize the pattern where a pure view component (e.g., TradeConfirmationView) is separated from a stateful container (e.g., TradeConfirmation) that wraps it. The container adds runtime state management (prop stabilization, signing state, etc.) while the view remains testable and composable. Do not flag usages that import th...
Learnt from: limitofzero
Repo: cowprotocol/cowswap PR: 6351
File: apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.ts:87-121
Timestamp: 2025-10-13T19:41:31.440Z
Learning: In apps/cowswap-frontend/src/modules/erc20Approve, useApproveCallback returns Promise<TransactionResponse | undefined> and is distinct from useApproveCurrency, which can return Promise<TransactionReceipt | SafeMultisigTransactionResponse>. When reviewing approval flows, verify which hook is actually being used before flagging Safe wallet concerns.
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 5859
File: apps/cowswap-frontend/src/modules/tradeQuote/hooks/useTradeQuotePolling.ts:76-82
Timestamp: 2025-06-23T07:03:50.760Z
Learning: In the useTradeQuotePolling hook, there are two useLayoutEffect hooks that work together: one resets the counter to 0 when the confirmation modal closes, and another automatically triggers pollQuote(false, true) whenever the counter reaches 0. This creates an intentional chain reaction for immediate quote updates.
📚 Learning: 2025-08-08T13:56:18.009Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6125
File: libs/tokens/src/updaters/TokensListsUpdater/index.tsx:29-31
Timestamp: 2025-08-08T13:56:18.009Z
Learning: In libs/tokens/src/updaters/TokensListsUpdater/index.tsx, the project’s current Jotai version requires using `unstable_getOnInit` (not `getOnInit`) in atomWithStorage options; keep `{ unstable_getOnInit: true }` until Jotai is upgraded.
Applied to files:
libs/tokens/src/updaters/RestrictedTokensListUpdater/index.tsx
📚 Learning: 2025-08-08T13:55:17.528Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6125
File: libs/tokens/src/state/tokens/allTokensAtom.ts:78-78
Timestamp: 2025-08-08T13:55:17.528Z
Learning: In libs/tokens/src/state/tokens/allTokensAtom.ts (TypeScript/Jotai), the team prefers to wait for token lists to initialize (listsStatesListAtom non-empty) before returning tokens. No fallback to favorites/user-added/native tokens should be used when listsStatesList is empty.
Applied to files:
libs/tokens/src/updaters/RestrictedTokensListUpdater/index.tsx
📚 Learning: 2025-08-12T06:33:19.348Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6137
File: libs/tokens/src/state/tokens/allTokensAtom.ts:34-65
Timestamp: 2025-08-12T06:33:19.348Z
Learning: In libs/tokens/src/state/tokens/allTokensAtom.ts, the parseTokenInfo() function returns a new instance of TokenInfo each time, making it safe to mutate properties like lpTokenProvider on the returned object without side effects.
Applied to files:
libs/tokens/src/updaters/RestrictedTokensListUpdater/index.tsx
📚 Learning: 2025-08-12T06:33:19.348Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6137
File: libs/tokens/src/state/tokens/allTokensAtom.ts:34-65
Timestamp: 2025-08-12T06:33:19.348Z
Learning: In libs/tokens/src/utils/parseTokenInfo.ts, the parseTokenInfo() function returns a new instance of TokenInfo using object spread syntax ({ ...token, ... }), making it safe to mutate properties like lpTokenProvider on the returned object without side effects.
Applied to files:
libs/tokens/src/updaters/RestrictedTokensListUpdater/index.tsx
🧬 Code graph analysis (1)
libs/tokens/src/updaters/RestrictedTokensListUpdater/index.tsx (1)
libs/tokens/src/hooks/useRestrictedTokensCache.ts (1)
useRestrictedTokensCache(25-61)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Cypress
- GitHub Check: Setup
🔇 Additional comments (8)
libs/tokens/src/updaters/RestrictedTokensListUpdater/index.tsx (8)
6-7: LGTM: Clean integration of the new caching hook.The migration from direct atom usage to
useRestrictedTokensCacheproperly centralizes cache management, aligning with the PR's goal of introducing IndexedDB-backed restricted token caching.
24-38: LGTM: Robust timeout and delay implementations.The
fetchWithTimeoutfunction properly usesAbortControllerwith cleanup in thefinallyblock, ensuring the timeout is cleared regardless of success or failure. Thedelayhelper is a standard promise-based sleep implementation.
40-74: LGTM: Comprehensive fetch logic with retry and validation.The retry mechanism is well-implemented:
- Validates response structure to avoid retrying on malformed data (lines 53-55, 62-64)
- Applies exponential backoff with
RETRY_DELAY_MS * (attempt + 1)(line 68)- Properly propagates the last error when all attempts fail (line 73)
77-77: LGTM: Proper integration of the caching hook.Replacing direct atom manipulation with
useRestrictedTokensCachecleanly abstracts the cache coordination logic. The hook providesshouldFetchfor cache-aware fetching andsaveToCachefor unified state updates.
84-86: LGTM: Cache-aware early exit prevents unnecessary fetches.The
shouldFetchguard correctly optimizes by skipping network requests when the cache is valid and not stale, as determined byuseRestrictedTokensCache.
96-111: LGTM: Resilient list fetching with graceful degradation.The use of
Promise.allwith individualtry-catchblocks (lines 98-109) ensures that failures in fetching one restricted list don't block others. Individual errors are logged (line 108), allowing partial success, which is preferable to complete failure when one source is unavailable.
120-120: LGTM: Unified state persistence viasaveToCache.Calling
saveToCachecorrectly updates both the runtime atom and the IndexedDB-backed cache, replacing the previous direct atom setter approach.
127-127: LGTM: Effect dependencies are complete and correct.The dependency array includes all values consumed by the effect:
isRwaGeoblockEnabled(line 80),shouldFetch(line 84), andsaveToCache(line 120). SincesaveToCacheis memoized viauseCallbackin the hook, this won't cause unnecessary re-runs.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/cowswap-frontend/src/locales/en-US.po (1)
238-240: RWA consent strings correctly marked obsolete in i18n catalogueAll the RwaConsentModal-related entries in these ranges are now
#~ msgid/msgstr, which cleanly removes them from the active translation catalogue while preserving the text for reference. This matches the “remove i18n for consents” intent and keeps placeholders intact.If the long‑term plan is to hard‑code consent copy (for legal reasons) and never localize it again, this state is fine; otherwise, consider eventually deleting these obsolete entries entirely to keep the .po smaller and avoid confusion for translators.
Also applies to: 452-454, 544-546, 585-587, 1367-1369, 1893-1895, 2009-2011, 3615-3617, 5963-5965
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/cowswap-frontend/src/locales/en-US.poapps/cowswap-frontend/src/modules/rwa/containers/RwaConsentModalContainer/index.tsxapps/cowswap-frontend/src/modules/rwa/pure/RwaConsentModal/index.tsxapps/cowswap-frontend/src/modules/rwa/pure/RwaConsentModal/styled.ts
💤 Files with no reviewable changes (1)
- apps/cowswap-frontend/src/modules/rwa/pure/RwaConsentModal/styled.ts
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: fairlighteth
Repo: cowprotocol/cowswap PR: 6347
File: apps/cowswap-frontend/src/modules/trade/pure/TradeConfirmation/index.tsx:49-49
Timestamp: 2025-10-10T20:28:16.565Z
Learning: In apps/cowswap-frontend/src/modules/trade, TradeConfirmation follows a two-layer architecture: TradeConfirmationView (pure/stateless) in pure/TradeConfirmation/index.tsx renders the UI, while TradeConfirmation (container) in containers/TradeConfirmation/index.tsx wraps it to freeze props during pending trades (via useStableTradeConfirmationProps), wire in signing state (useSigningStep), and inject trade confirmation state (useTradeConfirmState). Consuming modules should import the container TradeConfirmation from 'modules/trade' to preserve this stateful behavior.
<!-- [add_learning]
When reviewing component refactoring in apps/cowswap-frontend/src/modules/trade, recognize the pattern where a pure view component (e.g., TradeConfirmationView) is separated from a stateful container (e.g., TradeConfirmation) that wraps it. The container adds runtime state management (prop stabilization, signing state, etc.) while the view remains testable and composable. Do not flag usages that import th...
📚 Learning: 2025-10-10T20:28:16.565Z
Learnt from: fairlighteth
Repo: cowprotocol/cowswap PR: 6347
File: apps/cowswap-frontend/src/modules/trade/pure/TradeConfirmation/index.tsx:49-49
Timestamp: 2025-10-10T20:28:16.565Z
Learning: In apps/cowswap-frontend/src/modules/trade, TradeConfirmation follows a two-layer architecture: TradeConfirmationView (pure/stateless) in pure/TradeConfirmation/index.tsx renders the UI, while TradeConfirmation (container) in containers/TradeConfirmation/index.tsx wraps it to freeze props during pending trades (via useStableTradeConfirmationProps), wire in signing state (useSigningStep), and inject trade confirmation state (useTradeConfirmState). Consuming modules should import the container TradeConfirmation from 'modules/trade' to preserve this stateful behavior.
<!-- [add_learning]
When reviewing component refactoring in apps/cowswap-frontend/src/modules/trade, recognize the pattern where a pure view component (e.g., TradeConfirmationView) is separated from a stateful container (e.g., TradeConfirmation) that wraps it. The container adds runtime state management (prop stabilization, signing state, etc.) while the view remains testable and composable. Do not flag usages that import th...
Applied to files:
apps/cowswap-frontend/src/modules/rwa/containers/RwaConsentModalContainer/index.tsxapps/cowswap-frontend/src/modules/rwa/pure/RwaConsentModal/index.tsxapps/cowswap-frontend/src/locales/en-US.po
📚 Learning: 2025-02-20T15:59:33.749Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 5443
File: apps/cowswap-frontend/src/modules/swap/containers/ConfirmSwapModalSetup/index.tsx:71-71
Timestamp: 2025-02-20T15:59:33.749Z
Learning: The swap module in apps/cowswap-frontend/src/modules/swap/ is marked for deletion in PR #5444 as part of the swap widget unification effort.
Applied to files:
apps/cowswap-frontend/src/modules/rwa/pure/RwaConsentModal/index.tsxapps/cowswap-frontend/src/locales/en-US.po
📚 Learning: 2025-07-24T16:42:53.154Z
Learnt from: cowdan
Repo: cowprotocol/cowswap PR: 6009
File: apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/HighFeeWarning/HighFeeWarningTooltipContent.tsx:23-33
Timestamp: 2025-07-24T16:42:53.154Z
Learning: In apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/HighFeeWarning/HighFeeWarningTooltipContent.tsx, the use of toFixed(2) for percentage formatting in tooltip content is intentional and differs from the banner message formatting that uses toSignificant(2, undefined, Rounding.ROUND_DOWN). This formatting difference serves different UX purposes and should not be flagged as inconsistent.
Applied to files:
apps/cowswap-frontend/src/modules/rwa/pure/RwaConsentModal/index.tsxapps/cowswap-frontend/src/locales/en-US.po
📚 Learning: 2025-10-13T19:41:31.440Z
Learnt from: limitofzero
Repo: cowprotocol/cowswap PR: 6351
File: apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.ts:87-121
Timestamp: 2025-10-13T19:41:31.440Z
Learning: In apps/cowswap-frontend/src/modules/erc20Approve, useApproveCallback returns Promise<TransactionResponse | undefined> and is distinct from useApproveCurrency, which can return Promise<TransactionReceipt | SafeMultisigTransactionResponse>. When reviewing approval flows, verify which hook is actually being used before flagging Safe wallet concerns.
Applied to files:
apps/cowswap-frontend/src/locales/en-US.po
🧬 Code graph analysis (2)
apps/cowswap-frontend/src/modules/rwa/containers/RwaConsentModalContainer/index.tsx (1)
apps/cowswap-frontend/src/modules/rwa/pure/RwaConsentModal/index.tsx (1)
RwaConsentModal(15-65)
apps/cowswap-frontend/src/modules/rwa/pure/RwaConsentModal/index.tsx (1)
apps/cowswap-frontend/src/modules/trade/containers/TradeConfirmModal/index.cosmos.tsx (1)
onConfirm(36-38)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Setup
- GitHub Check: Cypress
🔇 Additional comments (4)
apps/cowswap-frontend/src/modules/rwa/pure/RwaConsentModal/index.tsx (3)
9-16: LGTM: Props interface simplified consistently.The removal of
consentHashaligns with the broader changes to eliminate IPFS-based consent link display. The component now focuses purely on rendering the consent acknowledgment without needing the hash reference.
22-22: Internationalization removed as intended.The removal of translation wrappers aligns with the PR objective to "remove i18n for consents." Using hardcoded English text for legal/compliance statements is a common practice to ensure users acknowledge terms in a specific language.
Also applies to: 38-60
43-54: Verify legal text accuracy with compliance review.The explicit bullet points clearly enumerate disqualifying categories (U.S. Person, EU/EEA resident, sanctioned countries, restricted jurisdictions). Given the legal and compliance implications of this text, ensure it has been reviewed and approved by legal counsel.
apps/cowswap-frontend/src/modules/rwa/containers/RwaConsentModalContainer/index.tsx (1)
47-47: LGTM: Container correctly updated to match pure component interface.The removal of
consentHashfrom the props passed toRwaConsentModalis consistent with the updated component interface. The container appropriately maintains internal state management withconsentHash(viaconsentKey) while the pure component focuses on presentation. This follows the established pure/container architecture pattern.
limitofzero
changed the title
elena-zh
left a comment
elena-zh
left a comment
There was a problem hiding this comment.
Looks good!
There is only 1 UI nitpick: I think. it would be nice to change text alignment here from center to the left:
LMK please if I need to open another task to address it or it can be fixed here.
thank you @elena-zh , I will address this issue in the next pr |
Summary
Issue: https://www.notion.so/cownation/Add-Concent-pop-up-to-Limit-TWAP-tabs-2d38da5f04ca8098824ce5179a72d1e0
Implements consistent RWA (Real World Asset) consent handling across all trade forms and adds caching for restricted token lists.
Changes
To Test
Summary by CodeRabbit
New Features
Performance Improvements
✏️ Tip: You can customize this high-level summary in your review settings.