feat: adjust dApp configuration for Solana as supported chain id #7525
Open
limitofzero wants to merge 22 commits into
Open
feat: adjust dApp configuration for Solana as supported chain id #7525limitofzero wants to merge 22 commits into
limitofzero wants to merge 22 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughConsolidates Solana support by migrating SOLANA to SupportedChainId, updates many ChangesSolana mainnet support migration
🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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 |
Deploying swap-dev with
|
| Latest commit: |
04c1a71
|
| Status: | 🚫 Build failed. |
…swap into feat/use-evm-chains-enum
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/cowswap-frontend/src/modules/yield/lpPageLinks.ts (1)
16-16: 💤 Low valueConsider adding explanatory comments for consistency.
Lines 33 and 50 include comments noting that Solana isn't supported by those providers, but lines 16 (COW_AMM_CHAINS) and 67 (PANCAKE_CHAINS) have no such comments. Adding comments would improve consistency and make the intent clearer to future maintainers.
💡 Suggested additions
- [SupportedChainId.SOLANA]: '', + [SupportedChainId.SOLANA]: '', // Doesn't supportAlso applies to: 67-67
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/cowswap-frontend/src/modules/yield/lpPageLinks.ts` at line 16, Add a short explanatory comment next to the SupportedChainId.SOLANA entries in the COW_AMM_CHAINS and PANCAKE_CHAINS maps to match the style used elsewhere; locate the SupportedChainId.SOLANA key in the COW_AMM_CHAINS constant and the PANCAKE_CHAINS constant and add a one-line note (e.g., "Solana not supported by COW AMM" / "Solana not supported by Pancake") explaining why the value is an empty string for consistency with the comments at lines 33 and 50.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/cowswap-frontend/src/modules/yield/lpPageLinks.ts`:
- Line 50: Fix the typo in the inline comment for SupportedChainId.SOLANA in
lpPageLinks.ts: change "Doesnt'" to the correct "Doesn't" so the comment reads
"Doesn't support" (locate the entry keyed by SupportedChainId.SOLANA in the
lpPageLinks mapping and update the comment only).
---
Nitpick comments:
In `@apps/cowswap-frontend/src/modules/yield/lpPageLinks.ts`:
- Line 16: Add a short explanatory comment next to the SupportedChainId.SOLANA
entries in the COW_AMM_CHAINS and PANCAKE_CHAINS maps to match the style used
elsewhere; locate the SupportedChainId.SOLANA key in the COW_AMM_CHAINS constant
and the PANCAKE_CHAINS constant and add a one-line note (e.g., "Solana not
supported by COW AMM" / "Solana not supported by Pancake") explaining why the
value is an empty string for consistency with the comments at lines 33 and 50.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2dd93acc-f041-451a-b67b-a68f2bcc9397
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
apps/cowswap-frontend/src/modules/yield/lpPageLinks.tslibs/balances-and-allowances/src/hooks/usePersistBalancesViaWebCalls.tslibs/common-const/src/tokens.tslibs/core/src/gnosisSafe/index.tslibs/tokens/src/hooks/tokens/useTokensByAddressMapForChain.tslibs/tokens/src/state/tokenLists/tokenListsStateAtom.ts
💤 Files with no reviewable changes (4)
- libs/tokens/src/state/tokenLists/tokenListsStateAtom.ts
- libs/core/src/gnosisSafe/index.ts
- libs/tokens/src/hooks/tokens/useTokensByAddressMapForChain.ts
- libs/common-const/src/tokens.ts
| [SupportedChainId.LINEA]: '', // TODO: confirm. This one is actually supported. | ||
| [SupportedChainId.PLASMA]: '', // TODO: confirm | ||
| [SupportedChainId.INK]: '', | ||
| [SupportedChainId.SOLANA]: '', // Doesnt' support |
Contributor
There was a problem hiding this comment.
Fix typo in comment.
The comment has a misplaced apostrophe: "Doesnt'" should be "Doesn't".
📝 Proposed fix
- [SupportedChainId.SOLANA]: '', // Doesnt' support
+ [SupportedChainId.SOLANA]: '', // Doesn't support📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| [SupportedChainId.SOLANA]: '', // Doesnt' support | |
| [SupportedChainId.SOLANA]: '', // Doesn't support |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/cowswap-frontend/src/modules/yield/lpPageLinks.ts` at line 50, Fix the
typo in the inline comment for SupportedChainId.SOLANA in lpPageLinks.ts: change
"Doesnt'" to the correct "Doesn't" so the comment reads "Doesn't support"
(locate the entry keyed by SupportedChainId.SOLANA in the lpPageLinks mapping
and update the comment only).
elena-zh
reviewed
May 22, 2026
Contributor
elena-zh
left a comment
elena-zh
left a comment
There was a problem hiding this comment.
Hey @limitofzero , works great!
The only thing that solana tokens images are broken on Explorer:
Could you please take a look at it?
Contributor
fairlighteth
left a comment
fairlighteth
left a comment
There was a problem hiding this comment.
AI findings:
- High: apps/cowswap-frontend/src/modules/application/containers/App/Updaters.tsx:110 now only fetches token lists for sourceChainId, while apps/cowswap-frontend/src/entities/bridgeProvider/useBridgeSupportedTokens.ts:22 expects destination-chain token lists via useTokensByAddressMapForChain(params?.buyChainId). Since the removed AdditionalChainTokensListsUpdater was the only target-chain fetch path, Solana destination lists/logos will stay empty unless the app is actually on Solana as the source chain. Likely related to Elena’s Explorer comment about broken Solana token images.
- Medium: apps/explorer/src/components/common/TokenImg/index.tsx:32 cannot load the new sol native image key from apps/explorer/src/utils/miscellaneous.ts:74: the filename regex does not include sol, and there is no apps/explorer/src/assets/img/tokens/sol.png. Native Solana image fallback resolves to nothing. This is the likely cause of Elena’s screenshot.
- Medium: libs/tokens/src/state/tokenLists/tokenListsStateAtom.ts:35 still has CodeRabbit’s valid issue: UNISWAP_TOKEN_LIST_URL[SupportedChainId.SOLANA] is '', but curatedListSourceAtom always returns a list source from it. In curated/widget mode that can propagate { source: '' }, causing fetch('') and a bogus enabled-list key.
- Low: libs/core/src/gnosisSafe/index.ts:20 maps Solana to '', so createSafeApiKitInstance(SOLANA) passes the “supported” check and builds https://api.safe.global/tx-service//api/. Prefer omitting Solana or guarding on a truthy Safe network id.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/i18n-extract.yml (1)
14-18: ⚡ Quick winScope
PACKAGE_READ_AUTH_TOKENto the install step instead of workflow-wide env.This token is only needed for dependency install; narrowing scope reduces accidental exposure across unrelated steps.
Patch suggestion
env: NODE_VERSION: lts/jod - # needed by tools/scripts/install-sdk-preview.js when package.json pins a `pr-NNN` - # SDK preview build — install:ci writes an .npmrc that auths to GitHub Packages. - PACKAGE_READ_AUTH_TOKEN: ${{ secrets.PACKAGE_READ_AUTH_TOKEN }} + # needed by tools/scripts/install-sdk-preview.js when package.json pins a `pr-NNN` + # SDK preview build — install:ci writes an .npmrc that auths to GitHub Packages. @@ - name: Install dependencies + env: + PACKAGE_READ_AUTH_TOKEN: ${{ secrets.PACKAGE_READ_AUTH_TOKEN }} run: pnpm run install:ci --frozen-lockfile🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/i18n-extract.yml around lines 14 - 18, The PACKAGE_READ_AUTH_TOKEN environment variable is declared at workflow-level under env: but is only needed during the dependency install step; remove PACKAGE_READ_AUTH_TOKEN from the top-level env block and add it to the install step's env instead (the step that runs npm install / yarn install or the script that invokes tools/scripts/install-sdk-preview.js) so only that step receives PACKAGE_READ_AUTH_TOKEN; use the same secret reference (${ { secrets.PACKAGE_READ_AUTH_TOKEN } }) when adding it to that step's env to preserve functionality.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/workflows/i18n-extract.yml:
- Around line 14-18: The PACKAGE_READ_AUTH_TOKEN environment variable is
declared at workflow-level under env: but is only needed during the dependency
install step; remove PACKAGE_READ_AUTH_TOKEN from the top-level env block and
add it to the install step's env instead (the step that runs npm install / yarn
install or the script that invokes tools/scripts/install-sdk-preview.js) so only
that step receives PACKAGE_READ_AUTH_TOKEN; use the same secret reference (${ {
secrets.PACKAGE_READ_AUTH_TOKEN } }) when adding it to that step's env to
preserve functionality.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 5fa08b0b-05d6-48c7-a27e-9b841bc3fe4a
📒 Files selected for processing (2)
.github/workflows/i18n-extract.ymlapps/explorer/src/modules/bridge/hooks/useBridgeProviderBuyTokens.ts
Contributor
Author
|
Contributor
Author
|
@elena-zh thank you, nice catch! fixed |
elena-zh
approved these changes
May 22, 2026
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
libs/core/src/gnosisSafe/index.ts (2)
90-94:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd guard for non-EVM chains.
getSafeWebUrlhas the same issue asgetSafeAccountUrl: it accepts anySupportedChainId(including non-EVM chains like Solana) but constructs a Gnosis Safe transaction URL. This produces semantically incorrect URLs for non-EVM chains where Safe doesn't exist.🛡️ Proposed fix
export function getSafeWebUrl(chainId: SupportedChainId, safeAddress: string, safeTxHash: string): string { + if (!(chainId in SAFE_API_NETWORK_ID)) { + throw new Error(`Gnosis Safe is not supported for chain ${chainId}`) + } const chainShortName = CHAIN_INFO[chainId].addressPrefix return `${SAFE_BASE_URL}/${chainShortName}:${safeAddress}/transactions/tx?id=multisig_${safeAddress}_${safeTxHash}` }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libs/core/src/gnosisSafe/index.ts` around lines 90 - 94, getSafeWebUrl currently builds a Gnosis Safe tx URL for any SupportedChainId (using CHAIN_INFO and SAFE_BASE_URL) which yields invalid URLs for non‑EVM chains; update getSafeWebUrl to first guard that the provided chainId supports EVM/Gnosis Safe (e.g. check a boolean like CHAIN_INFO[chainId].isEvm or ensure CHAIN_INFO[chainId].addressPrefix corresponds to an EVM chain) and if not, either throw a clear error or return an empty/undefined value consistent with getSafeAccountUrl behavior; reference the function name getSafeWebUrl, the CHAIN_INFO lookup, SupportedChainId type and SAFE_BASE_URL when making the change.
63-67:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAvoid generating Gnosis Safe app URLs for non-EVM chains in
getSafeAccountUrl.
getSafeAccountUrl(libs/core/src/gnosisSafe/index.ts, lines 63-67) acceptsSupportedChainIdand always builds a Safe URL fromCHAIN_INFO[chainId].addressPrefix.SAFE_API_NETWORK_IDis already used to gate Safe API instantiation for EVM chains, butgetSafeAccountUrlhas no equivalent guard—so for non-EVM chains likeSupportedChainId.SOLANA(which exists inCHAIN_INFO), it can generate an invalid/broken “safe app” link.Align
getSafeAccountUrl(and the same pattern ingetSafeWebUrl) with theSAFE_API_NETWORK_IDEVM-only guard and avoid rendering a Safe link for unsupported chains (e.g., return''/undefinedor narrow the input type), rather than constructing a URL unconditionally.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libs/core/src/gnosisSafe/index.ts` around lines 63 - 67, getSafeAccountUrl currently builds a Safe app URL for any chain using CHAIN_INFO[chainId].addressPrefix; update it (and the analogous getSafeWebUrl) to first check that the chain is an EVM-supported network (use SAFE_API_NETWORK_ID or a helper that verifies EVM-only support) and return an empty string (or undefined) for unsupported/non-EVM chains instead of constructing `${SAFE_BASE_URL}/${chainShortName}:${safeAddress}`; reference getSafeAccountUrl, getSafeWebUrl, CHAIN_INFO, SAFE_BASE_URL and SAFE_API_NETWORK_ID when making this guard.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@libs/core/src/gnosisSafe/index.ts`:
- Around line 90-94: getSafeWebUrl currently builds a Gnosis Safe tx URL for any
SupportedChainId (using CHAIN_INFO and SAFE_BASE_URL) which yields invalid URLs
for non‑EVM chains; update getSafeWebUrl to first guard that the provided
chainId supports EVM/Gnosis Safe (e.g. check a boolean like
CHAIN_INFO[chainId].isEvm or ensure CHAIN_INFO[chainId].addressPrefix
corresponds to an EVM chain) and if not, either throw a clear error or return an
empty/undefined value consistent with getSafeAccountUrl behavior; reference the
function name getSafeWebUrl, the CHAIN_INFO lookup, SupportedChainId type and
SAFE_BASE_URL when making the change.
- Around line 63-67: getSafeAccountUrl currently builds a Safe app URL for any
chain using CHAIN_INFO[chainId].addressPrefix; update it (and the analogous
getSafeWebUrl) to first check that the chain is an EVM-supported network (use
SAFE_API_NETWORK_ID or a helper that verifies EVM-only support) and return an
empty string (or undefined) for unsupported/non-EVM chains instead of
constructing `${SAFE_BASE_URL}/${chainShortName}:${safeAddress}`; reference
getSafeAccountUrl, getSafeWebUrl, CHAIN_INFO, SAFE_BASE_URL and
SAFE_API_NETWORK_ID when making this guard.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a4a086c1-b3ba-4420-a403-43941df525c9
📒 Files selected for processing (1)
libs/core/src/gnosisSafe/index.ts
Contributor
fairlighteth
left a comment
fairlighteth
left a comment
There was a problem hiding this comment.
Still some AI review findings that show up:
- Medium: Safe URL builders still accept SupportedChainId.SOLANA and generate invalid Safe links. createSafeApiKitInstance() was fixed, but libs/core/src/gnosisSafe/index.ts:63 and libs/core/src/gnosisSafe/index.ts:90 still build app.safe.global/solana:.... This leaves CodeRabbit’s final Safe-link comment unaddressed.
- Medium: The Solana destination token-list hydration path was removed without an equivalent replacement. apps/cowswap-frontend/src/modules/application/containers/App/Updaters.tsx:110 only runs
TokensListsUpdater for sourceChainId, while libs/tokens/src/hooks/tokens/useTokensByAddressMapForChain.ts:29 reads destination-chain list state. The new NearSolana list at libs/tokens/src/const/
tokensList.json:212 will not be fetched for normal EVM-to-Solana bridge flows. This means fairlighteth’s “target-chain token lists/logos stay empty” comment is not fully addressed. - Medium: Explorer’s native Solana local-image fallback is still broken. apps/explorer/src/utils/miscellaneous.ts:74 maps native Solana to sol, but apps/explorer/src/components/common/TokenImg/index.tsx:32
does not recognize sol, and there is no apps/explorer/src/assets/img/tokens/sol.png. The bridge-provider logoURI fallback may cover Elena’s reported screen, but the local fallback path still fails. - Low: PACKAGE_READ_AUTH_TOKEN is still workflow-wide in .github/workflows/i18n-extract.yml:14. CodeRabbit asked to scope it to the install step; that was not done.
- Low: The unresolved GitHub inline thread is still valid in apps/cowswap-frontend/src/modules/yield/lpPageLinks.ts:50: Doesnt' remains, and the requested explanatory comments at lines 16 and 67 are also still absent.
Also there are some merge conflicts.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
I've linked cowprotocol/cow-sdk#873 to this pr, so added some additional checks and solana fields in records/maps. Also I removed additional token list updater because all token lists moved in common.
P.S. Please don't pay attention on broken cow.fi, it's affected by invalidated npm token and I can't update it due to restricted permissions. It should work when we publish new cow-sdk version and I link it here
To Test
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Tests
Chores