feat(solana): load token balances#7850
Open
shoom3301 wants to merge 43 commits into
Open
Conversation
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughThe PR adds Solana SPL and Token-2022 balance fetching, persistence, caching, network configuration, and frontend integration. It also replaces several EVM-only guards and adds explicit return types to combined-balance components. ChangesSolana balance support
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Wallet
participant BalanceHook
participant SolanaRPC
participant BalanceAtom
Wallet->>BalanceHook: provide Solana account and token state
BalanceHook->>SolanaRPC: fetch associated token accounts
SolanaRPC-->>BalanceHook: return account information
BalanceHook->>BalanceAtom: persist decoded balances and timestamps
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
shoom3301
marked this pull request as ready for review
July 14, 2026 13:33
elena-zh
requested changes
Jul 14, 2026
elena-zh
left a comment
elena-zh
left a comment
Contributor
There was a problem hiding this comment.
Hey @shoom3301 , no, it is not working: I can see only native balance
Same happens on other chains in this PR when being connected to MM and change chain using chain selector in sell/buy token lists
fairlighteth
self-requested a review
July 14, 2026 15:10
fairlighteth
left a comment
fairlighteth
left a comment
Contributor
There was a problem hiding this comment.
⚠️ AI Review (Codex GPT-5, worked 23m): Solana token-program and address handling need fixes
Finding: [BLOCKING] Resolve the token program before deriving ATAs
- Location:
fetchSolanaTokenBalances.ts:33-50 - Both ATA derivation and
unpackAccountare hard-coded toTOKEN_PROGRAM_ID. The program ID is part of the ATA seed, andunpackAccountverifies the account owner against it. - The enabled-by-default Solana token list already contains Token-2022 mints. For example, PYUSD is owned by
TOKEN_2022_PROGRAM_ID. The current code derives the wrong ATA and records0nwhen that address is absent, so held Token-2022 balances appear as zero.
Suggested fix
- Resolve each mint’s owner program and carry either
TOKEN_PROGRAM_IDorTOKEN_2022_PROGRAM_IDthrough both ATA derivation and unpacking. Alternatively, query the owner’s accounts under both programs. - Add service-level coverage for a Token-2022 mint. The current test mocks both SPL-token functions, so it cannot catch incorrect program selection.
Finding: [BLOCKING] Preserve Solana account keys and remove non-null assertions
- Location:
usePersistSolanaBalancesViaWebCalls.ts:60,116 connection!,account!, andaccount.toLowerCase()violate the mandatory root repository rules. Solana public keys are case-sensitive; lowercasing can alias different owners and persist balances under the wrong identity.- The immediate readers also lowercase the account in
useSwrConfigWithPauseForNetwork.ts:28andBalancesCacheUpdater.tsx:39-70, so the Solana-exposed reads and writes need migrating together.
Suggested fix
- Guard the query function or use React Query’s
skipTokeninstead of non-null assertions. - Key the timestamp, pause state, and balance cache consistently with
getAddressKey(account). - Add a regression test using a mixed-case Solana account through update and cache restoration.
Finding: [NON-BLOCKING] Isolate malformed mint entries from the batch
- Location:
fetchSolanaTokenBalances.ts:33 - The token-list sanitizer only checks the base58 character set and length. For example,
'z'.repeat(44)passes that check but decodes to 33 bytes, sonew PublicKey(mint)throws. - Because every ATA is constructed before the RPC request, one malformed remote or widget-list entry rejects balance loading for the entire token list.
Suggested fix
- Validate that entries decode to 32-byte public keys at the token-list boundary, and defensively isolate invalid entries in this service.
- Add a test proving one bad mint cannot prevent valid balances from loading.
Review scope and related context
- The existing changes-requested review already reports the end-to-end “only native balance” failure and other-chain selector regression, so those findings are not repeated.
- There is no author response or resolved thread to recheck, and the head remains
2a8d3ee. - Test, lint, typecheck, and agent-harness checks pass. The Cypress failure reproduces on the base PR, so it is not attributed to this change.
- After the runtime issue is fixed, the PR test plan should explicitly cover legacy SPL tokens, Token-2022 tokens, and account/chain switching.
🤖 Prompt for AI agents
Verify these findings against the current PR head and keep fixes scoped:
1. Make Solana balance loading program-aware for both the legacy Token Program and Token-2022, using the resolved program consistently for ATA derivation and account unpacking.
2. Remove the new non-null assertions and replace every Solana-exposed account-key lowercasing operation with getAddressKey.
3. Prevent one malformed mint from rejecting the complete balance request.
4. Add focused tests for Token-2022, mixed-case Solana account cache keys, and malformed-mint isolation.
Preserve existing EVM behavior and run targeted test, lint, and typecheck verification.
Generated using the pr-review skill from the CoW Protocol skills repo.
limitofzero
reviewed
Jul 14, 2026
Deploying swap-dev with
|
| Latest commit: |
a228233
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://605f57d1.swap-dev-5u6.pages.dev |
| Branch Preview URL: | https://solana-web-2.swap-dev-5u6.pages.dev |
Collaborator
Author
|
@elena-zh fixed, it works now. |
Contributor
LGTM, works as expected
|
limitofzero
approved these changes
Jul 15, 2026
Contributor
|
Hey @shoom3301 , great! Balances can be loaded now.
Thanks |
Collaborator
Author
|
@elena-zh thanks!
|
elena-zh
approved these changes
Jul 16, 2026
elena-zh
left a comment
elena-zh
left a comment
Contributor
There was a problem hiding this comment.
Thanks, opened an issue for the Account Proxy page in https://linear.app/cowswap/project/solana-support-cow-swap-and-sdk-9a9d4f04acc4/issues
![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: 5
🧹 Nitpick comments (1)
libs/common-const/src/networks.ts (1)
47-55: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUpdate the type cast to reflect non-EVM networks.
Since
RPC_URL_ENVSnow contains keys forNonEvmChains.SOLANAandNonEvmChains.BITCOIN, casting its keys toEvmChains[]is semantically inaccurate. UseTargetChainId[]instead.♻️ Proposed refactor
export const RPC_URLS: Record<TargetChainId, HttpsString> = ( - Object.keys(RPC_URL_ENVS) as unknown as EvmChains[] + Object.keys(RPC_URL_ENVS) as unknown as TargetChainId[] ).reduce( (acc, chainId) => { acc[Number(chainId) as TargetChainId] = getRpcUrl(Number(chainId) as TargetChainId) return acc }, {} as Record<TargetChainId, HttpsString>, )🤖 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/common-const/src/networks.ts` around lines 47 - 55, Update the Object.keys(RPC_URL_ENVS) cast in the RPC_URLS initialization to TargetChainId[] instead of EvmChains[], while preserving the existing reduce logic and TargetChainId indexing.
🤖 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/tokensList/pure/TokenListItem/index.tsx`:
- Around line 111-114: Update the stale comment above shouldShowBalances to
reflect that Solana balances are supported, and verify which bridge-only
destination chains support balance fetching. Keep shouldShowBalances enabled
only for supported chains, preserving the existing behavior that avoids
indefinite loading skeletons for unsupported destinations such as Bitcoin.
In
`@libs/balances-and-allowances/src/hooks/usePersistSolanaBalancesViaWebCalls.ts`:
- Around line 61-68: Update the useQuery options in
usePersistSolanaBalancesViaWebCalls to forward params.query?.refetchOnMount,
matching the EVM hook’s behavior and preserving caller control over refetching
on mount.
- Around line 136-153: The buildSolanaTokenMints function incorrectly defaults
unlisted mints to the classic SPL program. Resolve each missing mint’s token
program from its on-chain mint account owner, or query both classic and
Token-2022 ATA variants, while preserving the existing tokensByAddress metadata
path; add a regression test covering an unlisted Token-2022 mint and its real
balance.
In `@libs/balances-and-allowances/src/services/fetchSolanaTokenBalances.ts`:
- Around line 73-79: Update the ataInfos iteration in fetchSolanaTokenBalances
to catch TokenInvalidAccountOwnerError and TokenInvalidAccountSizeError from
unpackAccount, leaving the corresponding balances[index] at its default zero
value when either occurs. Preserve successful unpacking and balance assignment,
and avoid allowing malformed ATA data to abort processing of other accounts.
In `@libs/wallet/src/wagmi/config.ts`:
- Around line 82-87: Wrap the top-level localStorage access in the
hasRecentConnector initialization with try...catch, including both getItem calls
and the existing undefined guard. On a synchronous storage access failure such
as SecurityError, fall back to false so module evaluation continues without
crashing; preserve the current true result when either connector key is
available.
---
Nitpick comments:
In `@libs/common-const/src/networks.ts`:
- Around line 47-55: Update the Object.keys(RPC_URL_ENVS) cast in the RPC_URLS
initialization to TargetChainId[] instead of EvmChains[], while preserving the
existing reduce logic and TargetChainId indexing.
🪄 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: 04ec6e69-5fc5-4e43-b838-04dddcdcbf62
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (20)
apps/cowswap-frontend/package.jsonapps/cowswap-frontend/src/modules/combinedBalances/hooks/useTokensBalancesCombined.tsapps/cowswap-frontend/src/modules/combinedBalances/updater/BalancesCombinedUpdater.tsxapps/cowswap-frontend/src/modules/permit/hooks/usePermitInfo.tsapps/cowswap-frontend/src/modules/tokensList/pure/TokenListItem/index.tsxapps/cowswap-frontend/vite.config.mtslibs/balances-and-allowances/package.jsonlibs/balances-and-allowances/src/hooks/usePersistBalancesViaWebCalls.test.tsxlibs/balances-and-allowances/src/hooks/usePersistBalancesViaWebCalls.tslibs/balances-and-allowances/src/hooks/usePersistSolanaBalancesViaWebCalls.test.tsxlibs/balances-and-allowances/src/hooks/usePersistSolanaBalancesViaWebCalls.tslibs/balances-and-allowances/src/hooks/useSwrConfigWithPauseForNetwork.tslibs/balances-and-allowances/src/services/fetchSolanaTokenBalances.test.tslibs/balances-and-allowances/src/services/fetchSolanaTokenBalances.tslibs/balances-and-allowances/src/updaters/BalancesCacheUpdater.test.tsxlibs/balances-and-allowances/src/updaters/BalancesCacheUpdater.tsxlibs/common-const/src/networks.tslibs/common-const/src/types.test.tslibs/common-const/src/types.tslibs/wallet/src/wagmi/config.ts
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
4 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
Fixes: https://linear.app/cowswap/issue/FE-305/fetch-token-balances-from-blockchain
There is a new
usePersistSolanaBalancesViaWebCallswhich is similar tousePersistBalancesViaWebCallsbut for Solana.It uses
@solana/web3.js.connection.getMultipleAccountsInfo()to load balances and@solana/spl-tokento work with Solana tokens.Essentially, it should work the same as balance loading in EVM chains:
PriorityTokensUpdaterwhich updates currently selected tokens and tokens of pending orders every 8 secondsBalancesAndAllowancesUpdaterwhich updates all tokens every 31 secondsTo Test
Summary by CodeRabbit
New Features
Bug Fixes
Tests