-
Notifications
You must be signed in to change notification settings - Fork 159
feat(trade): implement consent logic for restricted tokens list #6617
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds a complete RWA consent flow: types, Jotai storage atoms, geo lookup and restricted-token updater, hooks and modal (presentational + container), app-data consent propagation, trade validation gating, token identity helpers, CMS integration, and barrel exports. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TradeUI as Trade UI
participant RwaHook as useRwaTokenStatus
participant TokenStore as RestrictedTokens Atom
participant Geo as Geo Atom / useGeoStatus
participant Modal as RwaConsentModal
participant Consent as useRwaConsentStatus
participant Storage as localStorage
participant AppData as buildAppData
User->>TradeUI: Click Confirm
TradeUI->>RwaHook: request token status (input/output)
RwaHook->>TokenStore: lookup restricted token (consentHash, blockedCountries)
RwaHook->>Geo: ensure country (fetch if needed)
TokenStore-->>RwaHook: consentHash + blockedCountries
Geo-->>RwaHook: country | loading
alt Country is blocked
RwaHook-->>TradeUI: Restricted
TradeUI->>User: show "not available in your region"
else Consent required and missing
RwaHook-->>TradeUI: RequiredConsent
TradeUI->>Modal: open(consentHash, token)
Modal->>User: display terms
User->>Modal: Confirm
Modal->>Consent: confirmConsent(key)
Consent->>Storage: persist acceptedAt
Storage-->>Consent: stored
Consent-->>Modal: confirmed
Modal->>TradeUI: close
TradeUI->>AppData: include userConsent metadata
TradeUI->>User: proceed
else Consent already signed
RwaHook-->>TradeUI: ConsentIsSigned
TradeUI->>AppData: include existing consent metadata
TradeUI->>User: proceed
else Allowed
RwaHook-->>TradeUI: Allowed
TradeUI->>User: proceed
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
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 review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (2)
apps/cowswap-frontend/src/modules/rwa/types/rwaConsent.ts (1)
15-17: Consider validating or escaping key components to prevent collisions.The storage key construction uses colons as separators. If any of
wallet,issuer, ortosVersioncontain colons, it could lead to key collisions (e.g.,wallet="a:b", issuer="c"would collide withwallet="a", issuer="b:c").While unlikely in practice, consider either:
- Validating inputs to reject/escape special characters
- Using a more robust serialization (e.g.,
JSON.stringify([wallet, issuer, tosVersion]))apps/cowswap-frontend/src/modules/rwa/hooks/useRwaConsentStatus.ts (1)
15-20: Consider key stability foruseMemodependency.The
statusAtomis recreated whenever thekeyobject reference changes (line 17), even if its values are identical. While callers should useuseMemofor the key, you could add resilience here with a serialized dependency.export function useRwaConsentStatus(key: RwaConsentKey): UseRwaConsentStatusReturn { const consentAtom = getRwaConsentAtom(key) - const statusAtom = useMemo(() => createRwaConsentStatusAtom(key), [key]) + const statusAtom = useMemo( + () => createRwaConsentStatusAtom(key), + [key.wallet, key.issuer, key.tosVersion] + )This ensures the atom is only recreated when the actual values change, not just the object reference.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/cowswap-frontend/src/modules/rwa/components/RwaSelfCertificationModal/RwaSelfCertificationModal.tsx(1 hunks)apps/cowswap-frontend/src/modules/rwa/components/RwaSelfCertificationModal/index.ts(1 hunks)apps/cowswap-frontend/src/modules/rwa/hooks/useRwaConsentStatus.ts(1 hunks)apps/cowswap-frontend/src/modules/rwa/hooks/useRwaSelfCertification.tsx(1 hunks)apps/cowswap-frontend/src/modules/rwa/index.ts(1 hunks)apps/cowswap-frontend/src/modules/rwa/state/rwaConsentAtom.ts(1 hunks)apps/cowswap-frontend/src/modules/rwa/types/rwaConsent.ts(1 hunks)
🧰 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: 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.
Learnt from: alfetopito
Repo: cowprotocol/cowswap PR: 6234
File: libs/tokens/src/index.ts:1-4
Timestamp: 2025-09-11T08:25:51.460Z
Learning: In the cowprotocol/cowswap project, there is currently no SSR (Server-Side Rendering) support, so localStorage access at module import time does not cause SSR-related issues.
📚 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/components/RwaSelfCertificationModal/index.tsapps/cowswap-frontend/src/modules/rwa/components/RwaSelfCertificationModal/RwaSelfCertificationModal.tsxapps/cowswap-frontend/src/modules/rwa/state/rwaConsentAtom.tsapps/cowswap-frontend/src/modules/rwa/index.tsapps/cowswap-frontend/src/modules/rwa/hooks/useRwaConsentStatus.tsapps/cowswap-frontend/src/modules/rwa/hooks/useRwaSelfCertification.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:
apps/cowswap-frontend/src/modules/rwa/state/rwaConsentAtom.ts
📚 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:
apps/cowswap-frontend/src/modules/rwa/state/rwaConsentAtom.ts
📚 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/rwa/hooks/useRwaSelfCertification.tsx
🧬 Code graph analysis (4)
apps/cowswap-frontend/src/modules/rwa/components/RwaSelfCertificationModal/RwaSelfCertificationModal.tsx (3)
libs/types/src/common.ts (1)
Command(3-3)apps/cowswap-frontend/src/common/pure/Modal/index.tsx (1)
Modal(35-105)apps/cowswap-frontend/src/common/pure/ConfirmationModal/ConfirmationModalHeader.tsx (1)
ConfirmationModalHeader(46-53)
apps/cowswap-frontend/src/modules/rwa/state/rwaConsentAtom.ts (3)
apps/cowswap-frontend/src/modules/rwa/types/rwaConsent.ts (3)
RwaConsentRecord(3-7)RwaConsentKey(9-13)getRwaConsentStorageKey(15-17)apps/cowswap-frontend/public/emergency.js (1)
key(22-22)libs/core/src/jotaiStore.ts (1)
getJotaiMergerStorage(44-57)
apps/cowswap-frontend/src/modules/rwa/hooks/useRwaConsentStatus.ts (2)
apps/cowswap-frontend/src/modules/rwa/types/rwaConsent.ts (2)
GeoMode(1-1)RwaConsentKey(9-13)apps/cowswap-frontend/src/modules/rwa/state/rwaConsentAtom.ts (2)
getRwaConsentAtom(23-31)createRwaConsentStatusAtom(34-41)
apps/cowswap-frontend/src/modules/rwa/hooks/useRwaSelfCertification.tsx (4)
apps/cowswap-frontend/src/modules/rwa/types/rwaConsent.ts (2)
GeoMode(1-1)RwaConsentKey(9-13)libs/wallet/src/api/hooks.ts (1)
useWalletInfo(24-26)apps/cowswap-frontend/src/modules/rwa/hooks/useRwaConsentStatus.ts (1)
useRwaConsentStatus(15-49)apps/cowswap-frontend/src/modules/rwa/components/RwaSelfCertificationModal/RwaSelfCertificationModal.tsx (1)
RwaSelfCertificationModal(45-94)
🔇 Additional comments (3)
apps/cowswap-frontend/src/modules/rwa/components/RwaSelfCertificationModal/index.ts (1)
1-3: LGTM!Clean barrel export following the project's module organization pattern.
apps/cowswap-frontend/src/modules/rwa/index.ts (1)
1-6: LGTM!Clean module barrel export organizing all RWA consent functionality.
apps/cowswap-frontend/src/modules/rwa/hooks/useRwaConsentStatus.ts (1)
22-31: LGTM!The
confirmConsentcallback correctly updates the consent record with the providedgeoModeand timestamp.
...-frontend/src/modules/rwa/components/RwaSelfCertificationModal/RwaSelfCertificationModal.tsx
Outdated
Show resolved
Hide resolved
apps/cowswap-frontend/src/modules/rwa/hooks/useRwaSelfCertification.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/cowswap-frontend/src/locales/en-US.po (1)
1532-1534: Align “By clicking Confirm…” text with the “I Confirm” button labelThe explanatory sentence still references a
"Confirm"CTA, while the actual button label is"I Confirm". For legal clarity and UX consistency, consider either:
- Updating the sentence to
By clicking "I Confirm", you expressly represent and warrant that you are NOT:, or- Renaming the button to “Confirm” so it matches the copy.
This was already raised in a previous review and remains unresolved, so worth tightening before shipping.
Also applies to: 5964-5966
🧹 Nitpick comments (1)
apps/cowswap-frontend/src/locales/en-US.po (1)
238-240: Double‑check scope and intent of RWA disqualification bulletsThe disqualifying categories currently include:
- “A resident of any jurisdiction where trading securities or cryptographic tokens is regulated or prohibited by applicable laws.”
- “A U.S. Person or resident of the United States.”
- “A resident of the EU or EEA.”
- “A resident of any country subject to international sanctions (e.g., OFAC, UN lists).”
- Plus the follow‑up lines about selecting Cancel and being solely responsible for complying with local laws.
From a purely textual/readability standpoint these are consistent and clearly phrased. However, “regulated or prohibited” is extremely broad (almost all jurisdictions regulate securities / tokens), so in practice this could be read as excluding nearly everyone.
Recommend confirming with product/legal that this exact scope and phrasing is intentional for this consent flow, and adjusting the language (e.g. narrowing to “prohibited” or “where such trading is not legally permitted for you”) if that better reflects the intended restriction.
Also applies to: 544-547, 585-588, 1893-1896, 2009-2012, 3615-3618
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/cowswap-frontend/src/locales/en-US.po
🧰 Additional context used
🧠 Learnings (5)
📚 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
📚 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/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/locales/en-US.po
📚 Learning: 2025-07-24T16:43:47.639Z
Learnt from: cowdan
Repo: cowprotocol/cowswap PR: 6009
File: apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/HighFeeWarning/highFeeWarningHelpers.ts:18-20
Timestamp: 2025-07-24T16:43:47.639Z
Learning: In apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/HighFeeWarning/highFeeWarningHelpers.ts, the formatFeePercentage function uses ROUND_DOWN with toSignificant(2) for "at least X%" messaging. This ensures the displayed percentage is never higher than the actual fee, making the "at least" phrasing accurate. For example, if the actual fee is 25.4%, displaying "at least 25%" is correct, but "at least 26%" would be misleading.
Applied to files:
apps/cowswap-frontend/src/locales/en-US.po
📚 Learning: 2025-05-27T12:20:54.659Z
Learnt from: alfetopito
Repo: cowprotocol/cowswap PR: 5762
File: apps/cowswap-frontend/src/legacy/state/orders/utils.ts:499-503
Timestamp: 2025-05-27T12:20:54.659Z
Learning: In the CowSwap frontend, when displaying volume fees in the UI (like in ReceiptModal), zero fees (0 bps) should be treated as "free" and hidden from display. Only non-zero fees should show the "Total fee" line. This provides a cleaner UX by not cluttering the interface with "0.00%" fee displays.
Applied to files:
apps/cowswap-frontend/src/locales/en-US.po
⏰ 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 (2)
apps/cowswap-frontend/src/locales/en-US.po (2)
1055-1058: Geo‑undetermined and region‑blocked messages look coherentThe new copy:
- Explains why additional confirmation is needed when location cannot be determined (“e.g., due to VPN or privacy settings”) and states that access is limited to specific regions.
- Uses a clear, neutral hard‑block message (“This token is not available in your region”).
- Pairs well with the “Additional confirmation required for this token” header.
Wording is concise, user‑understandable, and consistent with a geo‑gated RWA flow.
Also applies to: 1359-1362, 1367-1370
4055-4058: Network costs / fees wording is internally consistentThe terminology updates:
- Labeling the row as “Network costs” in the amount breakdown,
- Explaining that “CoW Protocol covers the fees and costs by executing your order at a slightly better price than your limit price,” and
- Using “Network fees and costs” as the explanatory label in the receipt,
are consistent with each other and with the rest of the UI language around settlement costs vs protocol/partner fees. No issues from a copy or UX perspective.
Also applies to: 3588-3590, 5314-5316
RWA Geo-blocking & User Consent Flow
First PR for RWA geo-block implementation.
Implements geo-blocking and consent verification for restricted tokens (Ondo Finance RWA tokens).
Changes
Terms of Service
The consent references the Ondo Finance Terms of Service stored on IPFS:
bafkreidcn4bhj44nnethx6clfspkapahshqyq44adz674y7je5wyfiazsqRelated PRs
Configuration
Restricted countries can be configured via CMS:
Test Cases
Before testing - enable ff
isRwaGeoblockEnabled1. Geo-blocking behavior
2. Consent flow
3. Consent persistence
4. AppData integration
userConsentsin appDataHow to Test
Testing "Unknown Country" scenario
Testing "Restricted Country" scenario
Summary by CodeRabbit
New Features
Localization
Chores
✏️ Tip: You can customize this high-level summary in your review settings.