fix(bridge): do not crash calculating usd price in unsupported chain #5849
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Skipped Deployments
|
WalkthroughThe changes introduce a new USDC token for the Optimism chain, update type definitions to include additional chain IDs, and remove an unused token shorthand mapping. A guard clause is added to the USD token price calculation to handle missing USDC tokens more safely. The return type of a hook fetching bridge-supported networks was explicitly annotated. Changes
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
yarn install v1.22.22 ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
libs/common-const/src/tokens.ts (1)
416-424: Use checksum-cased address for the new Optimism USDC tokenAll other addresses in this file are EIP-55 checksum-cased, while the newly added Optimism address is all lowercase.
Keeping a consistent casing style avoids accidental double entries and prevents UX discrepancies when addresses are copy-pasted elsewhere.- '0x0b2c639c533813f4aa9d7837caf62653d097ff85', + '0x0B2C639c533813F4Aa9D7837cAF62653D097fF85',
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/cowswap-frontend/src/modules/usdAmount/state/usdTokenPricesAtom.ts(1 hunks)libs/common-const/src/tokens.ts(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Setup
- GitHub Check: Cypress
🔇 Additional comments (2)
libs/common-const/src/tokens.ts (2)
436-445: Index type still narrows toSupportedChainIdin downstream code
USDCis correctly widened to
Record<SupportedChainId | AdditionalTargetChainId, TokenWithLogo>here, but downstream look-ups (seeusdTokenPricesAtom.ts) still castcurrency.chainIdtoSupportedChainId.
This loses type-safety for Optimism and any future additional chains.Minimal fix in the consumer:
-const usdcToken = USDC[currency.chainId as SupportedChainId] +const usdcToken = USDC[currency.chainId as keyof typeof USDC](see separate comment in
usdTokenPricesAtom.ts)#!/bin/bash # Find remaining sites that still cast to SupportedChainId rg --context 2 -e 'as SupportedChainId' --type ts
1-1: RemovedTOKEN_SHORTHANDS: ensure no remaining references
TOKEN_SHORTHANDSwas deleted in this diff. If any module still imports or accesses it, the build will break.#!/bin/bash # Search for stale imports / usages rg -n "TOKEN_SHORTHANDS"
apps/cowswap
10000
-frontend/src/modules/usdAmount/state/usdTokenPricesAtom.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
libs/common-const/src/tokens.ts (1)
416-424: Use checksummed address forUSDC_OPTIMISMAll other token declarations follow the EIP-55 checksum format; the Optimism address is all lower-case.
This breaks strict–equality look-ups and makes human review harder.- '0x0b2c639c533813f4aa9d7837caf62653d097ff85', + '0x0B2C639c533813F4Aa9D7837Caf62653D097Ff85',
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/cowswap-frontend/src/modules/usdAmount/state/usdTokenPricesAtom.ts(1 hunks)libs/common-const/src/tokens.ts(5 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
libs/common-const/src/tokens.ts (3)
libs/common-const/src/types.ts (1)
TokenWithLogo(6-32)libs/widget-lib/src/types.ts (1)
SupportedChainId(4-4)libs/common-const/src/nativeAndWrappedTokens.ts (1)
WETH_MAINNET(132-132)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cypress
🔇 Additional comments (1)
libs/common-const/src/tokens.ts (1)
436-445: Update downstream look-ups now thatUSDCacceptsAdditionalTargetChainIdThe widened record is great, but be sure every consumer stops asserting
SupportedChainIdonly (seeusdTokenPricesAtom.ts).
A quick repo scan forUSDC[.* as SupportedChainIdwill surface the remaining call-sites.#!/bin/bash # Find unsafe casts that may now hide unsupported-chain bugs rg -n $'USDC\\[.* as SupportedChainId' | head -n 20
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
apps/cowswap-frontend/src/entities/bridgeProvider/useBridgeSupportedNetworks.ts (1)
10-12: Key may change every render – could cause unnecessary revalidations
bridgeProvideris an object reference that is likely recreated on each context update.
Including the plain object in the SWR key can trigger an unexpected re-fetch loop when its reference identity changes even though the underlying network set is unchanged.Consider using a stable identifier (e.g.
bridgeProvider.idor the provider’s chainId array) as part of the key, or stringify:- return useSWR([bridgeProvider, 'useBridgeSupportedNetworks'], ([bridgeProvider]) => { + return useSWR( + [bridgeProvider?.id, 'useBridgeSupportedNetworks'], // stable key + () => bridgeProvider?.getNetworks() + )This keeps revalidation predictable and prevents needless network calls.
🧹 Nitpick comments (2)
apps/cowswap-frontend/src/entities/bridgeProvider/useBridgeSupportedNetworks.ts (2)
3-3: ImportSWRResponseas a type-only importTree-shaking and build-time performance benefit slightly if
SWRResponseis imported with the TypeScripttypemodifier.-import useSWR, { SWRResponse } from 'swr' +import useSWR from 'swr' +import type { SWRResponse } from 'swr'
7-7: Specify the error type inSWRResponsefor stronger type safety
SWRResponse<Data = any, Error = any>defaults the error parameter toany, losing useful information.
IfbridgeProvider.getNetworks()rejects with a known error type (e.g.Error), declare it explicitly:-export function useBridgeSupportedNetworks(): SWRResponse<ChainInfo[]> { +export function useBridgeSupportedNetworks(): SWRResponse<ChainInfo[], Error> {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/cowswap-frontend/src/entities/bridgeProvider/useBridgeSupportedNetworks.ts(1 hunks)libs/common-const/src/tokens.ts(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/common-const/src/tokens.ts
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Cypress
- GitHub Check: Setup
Summary
Fixes https://www.notion.so/cownation/Crash-when-destination-token-on-optimism-2148da5f04ca80d89a49faef54abe124?pvs=25
To Test
See https://www.notion.so/cownation/Crash-when-destination-token-on-optimism-2148da5f04ca80d89a49faef54abe124?pvs=25
Summary by CodeRabbit
New Features
Bug Fixes
Chores