-
Notifications
You must be signed in to change notification settings - Fork 160
feat: move from gh cdn to aws #6591
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.
|
WalkthroughReplaced hard-coded raw.githubusercontent.com and other external URLs with a configurable Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (18)
🚧 Files skipped from review as they are similar to previous changes (13)
🧰 Additional context used🧠 Learnings (10)📚 Learning: 2025-09-25T08:49:32.256ZApplied to files:
📚 Learning: 2025-09-11T08:25:51.460ZApplied to files:
📚 Learning: 2025-10-10T20:28:16.565ZApplied to files:
📚 Learning: 2025-06-10T09:57:40.679ZApplied to files:
📚 Learning: 2025-08-12T06:33:19.348ZApplied to files:
📚 Learning: 2025-08-12T06:33:19.348ZApplied to files:
📚 Learning: 2025-07-18T08:07:55.497ZApplied to files:
📚 Learning: 2025-08-08T13:55:17.528ZApplied to files:
📚 Learning: 2025-08-08T13:56:18.009ZApplied to files:
📚 Learning: 2025-05-26T12:39:29.009ZApplied to files:
🧬 Code graph analysis (2)apps/cowswap-frontend/src/tradingSdk/TestReceiverAccountBridgeProvider.ts (1)
apps/cow-fi/services/tokens/index.ts (2)
🪛 ESLintapps/cow-fi/services/tokens/index.ts[error] 11-11: (import/order) ⏰ 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)
🔇 Additional comments (4)
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 |
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
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
🧹 Nitpick comments (3)
apps/explorer/src/utils/miscellaneous.ts (1)
43-55: Make deprecation message point to the intended replacementMarking
getImageUrlas deprecated is good, but@deprecated TODO5(daniel)doesn’t tell callers what to use instead. Consider changing it to something like@deprecated Use <new util> from <package> insteadonce the new logo URL helper is finalized, so migration is straightforward.libs/tokens/src/utils/trustTokenLogoUrl.ts (1)
17-28: Clarify what should replacetrustTokenLogoUrlThe deprecation marker is helpful, but
@deprecated TODO5(daniel)is vague. To ease future cleanup, consider updating it to reference the concrete replacement (for example, a CowFi-based logo helper) once agreed, so callers know what to migrate to.apps/explorer/src/hooks/useTokenList.ts (1)
32-38: Sepolia CowSwap list URL now matches shared token-list configUsing
https://files.cow.fi/token-lists/CowSwapSepolia.jsonfor Sepolia keeps this hook aligned withlibs/tokens/src/const/tokensList.jsonand the tokensList mocks. As a future clean‑up, you might consider centralizing the CowSwap list URLs (main + Sepolia) in a shared constant to avoid string drift across explorer, frontend, and libs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/cowswap-frontend/src/modules/permit/const.ts(1 hunks)apps/cowswap-frontend/src/modules/tokensList/mocks.ts(2 hunks)apps/explorer/src/hooks/useTokenList.ts(1 hunks)apps/explorer/src/utils/miscellaneous.ts(1 hunks)apps/widget-configurator/src/app/configurator/consts.ts(1 hunks)libs/common-const/src/cowprotocolTokenLogoUrl.ts(1 hunks)libs/tokens/src/const/lpTokensList.json(1 hunks)libs/tokens/src/const/tokensList.json(11 hunks)libs/tokens/src/state/tokenLists/tokenListsStateAtom.ts(1 hunks)libs/tokens/src/utils/trustTokenLogoUrl.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (14)
📓 Common learnings
Learnt from: alfetopito
Repo: cowprotocol/cowswap PR: 5992
File: libs/tokens/src/const/tokensList.json:135-167
Timestamp: 2025-07-18T08:07:55.497Z
Learning: Token lists for CoW Swap are maintained in a separate repository at https://github.com/cowprotocol/token-lists, not in the main cowswap repository. Issues related to missing token lists should be tracked in the token-lists repository.
📚 Learning: 2025-09-25T08:49:32.256Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6299
File: apps/cowswap-frontend/src/entities/bridgeProvider/useBridgeSupportedNetworks.test.tsx:62-67
Timestamp: 2025-09-25T08:49:32.256Z
Learning: In the cowswap-frontend codebase, when testing hooks that use multiple bridge providers, both providers are always properly mocked as complete jest.Mocked<BridgeProvider<BridgeQuoteResult>> objects with all required methods stubbed, ensuring no undefined returns that could break the hook logic.
Applied to files:
apps/cowswap-frontend/src/modules/tokensList/mocks.ts
📚 Learning: 2025-07-18T08:07:55.497Z
Learnt from: alfetopito
Repo: cowprotocol/cowswap PR: 5992
File: libs/tokens/src/const/tokensList.json:135-167
Timestamp: 2025-07-18T08:07:55.497Z
Learning: Token lists for CoW Swap are maintained in a separate repository at https://github.com/cowprotocol/token-lists, not in the main cowswap repository. Issues related to missing token lists should be tracked in the token-lists repository.
Applied to files:
apps/cowswap-frontend/src/modules/tokensList/mocks.tsapps/widget-configurator/src/app/configurator/consts.tslibs/tokens/src/state/tokenLists/tokenListsStateAtom.tslibs/tokens/src/const/lpTokensList.jsonlibs/common-const/src/cowprotocolTokenLogoUrl.tslibs/tokens/src/const/tokensList.jsonapps/explorer/src/hooks/useTokenList.ts
📚 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/tokensList/mocks.ts
📚 Learning: 2025-09-11T08:25:51.460Z
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.
Applied to files:
apps/cowswap-frontend/src/modules/tokensList/mocks.tsapps/explorer/src/hooks/useTokenList.ts
📚 Learning: 2025-07-28T16:26:08.051Z
Learnt from: cowdan
Repo: cowprotocol/cowswap PR: 6034
File: apps/cowswap-frontend-e2e/src/e2e/fiat-amounts.test.ts:44-47
Timestamp: 2025-07-28T16:26:08.051Z
Learning: In the cowswap codebase, using trivial placeholder tests like `it('should be true', () => { expect(true).to.be.true })` in e2e test files is an intentional pattern when disabling broken tests to keep CI green while maintaining build efficiency.
Applied to files:
apps/cowswap-frontend/src/modules/tokensList/mocks.ts
📚 Learning: 2025-09-25T08:48:53.495Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6299
File: apps/cowswap-frontend/src/entities/bridgeProvider/useBridgeSupportedNetworks.test.tsx:58-60
Timestamp: 2025-09-25T08:48:53.495Z
Learning: In the cowswap-frontend codebase, when writing SWR tests, the team prefers maximum test isolation by using `provider: () => new Map()` in SWRConfig wrappers, even if it recreates cache on every render, to ensure tests don't share any state.
Applied to files:
apps/cowswap-frontend/src/modules/tokensList/mocks.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/tokensList/mocks.tsapps/widget-configurator/src/app/configurator/consts.tslibs/tokens/src/state/tokenLists/tokenListsStateAtom.tslibs/tokens/src/const/lpTokensList.jsonlibs/tokens/src/const/tokensList.jsonapps/explorer/src/hooks/useTokenList.ts
📚 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/state/tokenLists/tokenListsStateAtom.tsapps/explorer/src/hooks/useTokenList.ts
📚 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/state/tokenLists/tokenListsStateAtom.tslibs/tokens/src/const/lpTokensList.json
📚 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/state/tokenLists/tokenListsStateAtom.tslibs/tokens/src/const/lpTokensList.json
📚 Learning: 2025-08-05T14:27:05.023Z
Learnt from: alfetopito
Repo: cowprotocol/cowswap PR: 5992
File: libs/wallet/src/web3-react/utils/switchChain.ts:36-38
Timestamp: 2025-08-05T14:27:05.023Z
Learning: In libs/wallet/src/web3-react/utils/switchChain.ts, the team prefers using Record<SupportedChainId, string | null> over Partial<Record<SupportedChainId, string>> for WALLET_RPC_SUGGESTION to enforce that all supported chain IDs have explicit values set, even if some might be null. This ensures compile-time completeness checking.
Applied to files:
libs/tokens/src/state/tokenLists/tokenListsStateAtom.ts
📚 Learning: 2025-05-26T12:39:29.009Z
Learnt from: cowdan
Repo: cowprotocol/cowswap PR: 5715
File: libs/common-const/src/tokens.ts:539-555
Timestamp: 2025-05-26T12:39:29.009Z
Learning: The team accepts using NATIVE_CURRENCY_ADDRESS as a temporary placeholder for COW token contract addresses on new networks (Polygon, Avalanche) until actual COW contracts are deployed.
Applied to files:
libs/common-const/src/cowprotocolTokenLogoUrl.ts
📚 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/explorer/src/hooks/useTokenList.ts
🔇 Additional comments (7)
apps/widget-configurator/src/app/configurator/consts.ts (1)
11-19: CoinGecko mainnet URL aligns with central token list configThe new
CoinGecko.1.jsonURL matches the mainnet CoinGecko entry inlibs/tokens/src/const/tokensList.json, so the widget’s defaults stay in sync with the shared token list config.libs/common-const/src/cowprotocolTokenLogoUrl.ts (1)
3-6: Verify new CowFi images root matches deployed bucket structureSwitching
tokenUrlRoottohttps://files.cow.fi/token-lists/imageskeeps the same${chainId}/${address}/logo.pngpattern, which looks consistent with how token-list images are organized. Given how widely this helper is used, please double‑check that the bucket actually exposes paths in this exact form (no extrasrc/publicsegment, trailing slash differences, etc.) so logos don’t regress.apps/cowswap-frontend/src/modules/permit/const.ts (1)
5-6: PermitInfo URL host migration looks good; confirm endpoint in runtimeUpdating
PRE_GENERATED_PERMIT_URLtohttps://files.cow.fi/token-lists/PermitInfofits the CDN migration. Please sanity‑check in dev/staging that consumers successfully fetch from this new endpoint (status 200 + expected JSON) so we don’t break pre‑generated permit loading.libs/tokens/src/const/tokensList.json (1)
1-195: Token-list CDN migration looks consistent; clarify remaining raw.githubusercontent.com sourcesThe new
files.cow.fiURLs for CoinGecko/Uniswap/CowSwap lists across chains look consistent with other modules (e.g. widget configurator default lists, explorer’suseTokenList, Sepolia CowSwap list), which helps centralize everything on the same CDN. Based on learnings, this matches the dedicated token-lists repo being surfaced via CowFi.There are still a few
raw.githubusercontent.comentries here (Ondo “cowswap-global-markets-token-list” on chains 1 & 56, and Balancer’s tokenlist on Arbitrum). If the PR’s scope is only CoW‑owned lists, that’s fine; otherwise, it might be worth confirming whether these third‑party lists are expected to stay on GitHub raw or if you plan to mirror them tofiles.cow.fias well to fully avoid GitHub rate‑limit issues.apps/cowswap-frontend/src/modules/tokensList/mocks.ts (1)
101-141: Mocks now align with CowFi-hosted Sepolia list and logoPointing
listsMock.sourceat the CowFi‑hostedCowSwapSepolia.jsonand updatingimportListsMock’s logo to the CowFi image keeps the mocks in sync with the production configuration without impacting runtime behavior. This should make tests and stories better reflect the real setup.libs/tokens/src/state/tokenLists/tokenListsStateAtom.ts (1)
17-37: UNISWAP token‑list URLs correctly migrated to files.cow.fiThe per‑chain URL mappings look consistent (chain IDs match filenames, Lens keeps the CoinGecko fallback), and the change is limited to swapping the host to
files.cow.fiwithout altering existing selection logic. From this file’s perspective, the migration is clean.If any of these lists turn out to be missing or need content tweaks, that should be handled in the separate
token-listsrepo rather than here. Based on learnings, token lists are maintained externally.libs/tokens/src/const/lpTokensList.json (1)
3-31: LP token list sources cleanly switched to files.cow.fiAll LP providers now use the new
files.cow.fi/token-lists/lp-tokens/...endpoints, with priorities and provider identifiers unchanged. This is a straightforward data‑source swap and looks good.
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/hook-dapp-lib/src/hookDappsRegistry.ts (1)
12-12: Incomplete CDN migration for internal hook DApps.The PR description states it replaces "all references to raw.githubusercontent.com" but four internal hook entries still use GitHub CDN:
- BUILD_CUSTOM_HOOK (line 12)
- CLAIM_GNO_FROM_VALIDATORS (line 22)
- PERMIT_HOOK_DAPP_ID (line 36)
- CLAIM_COW_AIRDROP (line 50)
While Bungee and Across DApps have been migrated to
files.cow.fi/cow-sdk, these internal entries remain on GitHub. Migrate these image URLs to the AWS CDN or document why they're intentionally excluded from the migration.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/cowswap-frontend/src/modules/orderProgressBar/pure/OrderProgressBar/index.cosmos.tsx(1 hunks)apps/cowswap-frontend/src/tradingSdk/TestReceiverAccountBridgeProvider.ts(1 hunks)apps/explorer/src/components/orders/OrderDetails/crossChainOrder.mock.ts(1 hunks)libs/hook-dapp-lib/src/hookDappsRegistry.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/cowswap-frontend/src/modules/orderProgressBar/pure/OrderProgressBar/index.cosmos.tsx
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-06-10T09:57:40.679Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 5787
File: apps/explorer/src/components/orders/BridgeDetailsTable/BridgeDetailsContent/index.tsx:33-40
Timestamp: 2025-06-10T09:57:40.679Z
Learning: In the bridge provider info structure (CrossChainOrder.provider.info), the `website` property is not optional and will always be present, so no conditional checking is needed when accessing `providerInfo.website`.
Applied to files:
apps/explorer/src/components/orders/OrderDetails/crossChainOrder.mock.tsapps/cowswap-frontend/src/tradingSdk/TestReceiverAccountBridgeProvider.ts
📚 Learning: 2025-09-25T08:49:32.256Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6299
File: apps/cowswap-frontend/src/entities/bridgeProvider/useBridgeSupportedNetworks.test.tsx:62-67
Timestamp: 2025-09-25T08:49:32.256Z
Learning: In the cowswap-frontend codebase, when testing hooks that use multiple bridge providers, both providers are always properly mocked as complete jest.Mocked<BridgeProvider<BridgeQuoteResult>> objects with all required methods stubbed, ensuring no undefined returns that could break the hook logic.
Applied to files:
apps/cowswap-frontend/src/tradingSdk/TestReceiverAccountBridgeProvider.ts
📚 Learning: 2025-08-12T05:57:08.021Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6138
File: libs/hook-dapp-lib/src/hookDappsRegistry.ts:1-1
Timestamp: 2025-08-12T05:57:08.021Z
Learning: The matchHooksToDapps function in libs/hook-dapp-lib/src/utils.ts provides backward compatibility for permit hooks through function selector detection (EIP_2612_PERMIT_SELECTOR and DAI_PERMIT_SELECTOR) rather than dappId matching, making it robust against dappId changes.
Applied to files:
libs/hook-dapp-lib/src/hookDappsRegistry.ts
🔇 Additional comments (2)
apps/explorer/src/components/orders/OrderDetails/crossChainOrder.mock.ts (1)
10-13: CDN logo URL update looks correct and non‑breakingThe
logoUrlnow points at thefiles.cow.fi/cow-sdk/bridging/providers/bungee/bungee-logo.pngpath, which matches the new CDN convention and keeps the mock structure intact. No runtime or typing impact here.libs/hook-dapp-lib/src/hookDappsRegistry.ts (1)
168-168: LGTM! URL migration correctly implemented.The Bungee and Across image URLs have been successfully migrated from GitHub CDN to the AWS-backed files.cow.fi domain, following a consistent path structure.
Also applies to: 181-181
apps/cowswap-frontend/src/tradingSdk/TestReceiverAccountBridgeProvider.ts
Outdated
Show resolved
Hide resolved
f598a67 to
b6a4b88
Compare
35c62c5 to
5b8d8fb
Compare
Summary
This change removes all references to
raw.githubusercontent.comand replaces them withfiles.cow.fi/cow-sdkBlocked by cowprotocol/cow-sdk#723
Testing
Background
This PR is part of https://linear.app/cowswap/issue/COW-136/token-lists-cdn
Moving away from our reliance on github CDN.
This is no longer viable as Github recently changed their rate limiting policy for non-logged in users.
Related PRs:
cowprotocol/cow-sdk#723
cowprotocol/cow-sdk#724
Summary by CodeRabbit
Chores
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.