fix: validate token structure in useBridgeSupportedTokens and TokenWi… #6539
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughReplaces direct token mapping in the bridge-supported-tokens hook with a filtering reduce that skips malformed tokens (missing chainId/address) and wraps valid entries via TokenWithLogo.fromToken; TokenWithLogo now requires/exports logoURI and enforces presence of chainId and address at runtime. Changes
Sequence Diagram(s)sequenceDiagram
participant Hook as BridgeHook
participant API as BridgeAPI
participant TokenClass as TokenWithLogo
Note over Hook,API: fetch supported tokens
Hook->>API: request supported tokens
API-->>Hook: result { tokens: [...], isRouteAvailable }
Note over Hook: reduce over result.tokens
Hook->>Hook: for each token\n- if missing chainId/address -> log warning, skip
Hook->>TokenClass: TokenWithLogo.fromToken(validToken, logoURI)
TokenClass-->>Hook: TokenWithLogo instance
Note over Hook: derive tokens array\nset isRouteAvailable = (tokens.length>0 ? result.isRouteAvailable : false)
Hook-->>Consumer: { tokens: [...TokenWithLogo], isRouteAvailable }
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 (1)
🧰 Additional context used🧠 Learnings (4)📚 Learning: 2025-09-25T08:49:32.256ZApplied to files:
📚 Learning: 2025-08-05T14:27:05.023ZApplied to files:
📚 Learning: 2025-08-08T13:55:17.528ZApplied to files:
📚 Learning: 2025-08-12T06:33:19.348ZApplied to files:
🧬 Code graph analysis (1)apps/cowswap-frontend/src/entities/bridgeProvider/useBridgeSupportedTokens.ts (1)
⏰ 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 (3)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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 |
elena-zh
left a comment
elena-zh
left a comment
There was a problem hiding this comment.
Hey @fairlighteth , thank you! I have not been able to face a crash during my tests.
![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 (2)
libs/common-const/src/types.ts (1)
8-10: Good defensive validation; consider also checking for null chainId.The validation correctly guards against malformed tokens and provides a clear error message. The check prevents the white-screen crash described in the PR objectives.
Consider using loose equality to also catch
nullchainId:- if (!token || token.chainId === undefined || !token.address) { + if (!token || token.chainId == null || !token.address) { throw new Error('TokenWithLogo.fromToken requires a token with chainId and address') }This would catch both
undefinedandnullvalues, making the validation more robust against various malformed provider responses.apps/cowswap-frontend/src/entities/bridgeProvider/useBridgeSupportedTokens.ts (1)
46-49: Effective validation that prevents crashes while maintaining observability.The validation correctly filters malformed tokens during aggregation and logs warnings for visibility. This aligns with the PR objective of preventing the white-screen crash when bridge providers return invalid data.
For consistency with the suggestion in
types.ts, consider also catchingnullchainId:- if (!token || !token.address || token.chainId === undefined) { + if (!token || !token.address || token.chainId == null) { console.warn('[bridgeTokens] Ignoring malformed token', token) return }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/cowswap-frontend/src/entities/bridgeProvider/useBridgeSupportedTokens.ts(2 hunks)libs/common-const/src/types.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📚 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:
apps/cowswap-frontend/src/entities/bridgeProvider/useBridgeSupportedTokens.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/entities/bridgeProvider/useBridgeSupportedTokens.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/entities/bridgeProvider/useBridgeSupportedTokens.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/entities/bridgeProvider/useBridgeSupportedTokens.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/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/common-const/src/types.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/common-const/src/types.ts
🔇 Additional comments (2)
apps/cowswap-frontend/src/entities/bridgeProvider/useBridgeSupportedTokens.ts (2)
73-74: Correct logic that addresses the core issue.The derivation ensures
isRouteAvailableisfalsewhen no valid tokens remain after filtering, preventing the blank-screen regression described in the PR objectives. The approach correctly handles the scenario where providers reportisRouteAvailable: truebut return only malformed tokens.
77-78: LGTM!The return statement correctly uses the derived values, ensuring consumers receive consistent and validated state where
isRouteAvailableaccurately reflects token availability.
Summary
Related to #6531 and #6429
Guard bridge token aggregation against malformed provider entries so they no longer crash the selector (
apps/cowswap-frontend/src/entities/bridgeProvider/useBridgeSupportedTokens.ts:45-79) and add a sanity check insideTokenWithLogo.fromTokento surface misuse early (libs/common-const/src/types.ts:7-22). The bridge UI now reports routes as unavailable whenever every upstream token is discarded, preventing the blank-screen regression observed when switching chains in bridge mode.To Test
Open production-like build (Base → Arbitrum bridge flow)
[bridgeTokens] Ignoring malformed tokeninstead of an error boundarySmoke test other token selectors
TokenWithLogo.fromTokenBackground
The bridge provider occasionally returns
nulltokens even whenisRouteAvailableistrue, which previously causedtoken.chainIddereferences to explode. These changes sanitize external data and keep the selector functional despite upstream glitches.Summary by CodeRabbit
Bug Fixes
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.