fix(bridge): fix cross-chain swap quote details #6597
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughSwaps bridge intermediate-token resolution to address-based token lookups, splits receive-amount logic into swap vs cross-chain paths, introduces cross-chain receive-amount utilities and tests, refactors getReceiveAmountInfo to accept a params object, and updates imports/exports and UI display logic accordingly. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas requiring extra attention:
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 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
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/cowswap-frontend/src/modules/trade/utils/getCrossChainReceiveAmountInfo.test.ts (1)
40-44: Consider documenting thefeeBpsfield usage.The
bridgeFeeAmountsobject includes afeeBps: 14field that doesn't appear to be part of theBridgeFeeAmountsinterface defined intypes.ts(which only hasamountInSellCurrencyandamountInBuyCurrency). This extra field is ignored, but could cause confusion.Either remove the unused field or update the interface if it's meant to be used:
const bridgeFeeAmounts = { amountInBuyCurrency: 5700201003969n, amountInSellCurrency: 4694n, - feeBps: 14, }apps/cowswap-frontend/src/modules/trade/hooks/useGetSwapReceiveAmountInfo.ts (1)
56-64: Consider null safety for quote access chain.The access pattern
tradeQuote?.quote?.quoteResults.quoteResponsecould throw if intermediate properties exist but are unexpectedly shaped. While TypeScript types should prevent this, the optional chaining stops atquoteResponse, butquoteResponse?.quote?.sellTokencould also benefit from guarding.However, this is a minor concern since the hook consumers already handle
nullcurrencies gracefully.function useQuoteCurrencies(): ReceiveAmountCurrencies { const tradeQuote = useTradeQuote() const quoteResponse = tradeQuote?.quote?.quoteResults.quoteResponse - const inputCurrency = useTokenByAddress(quoteResponse?.quote?.sellToken.toLowerCase()) - const outputCurrency = useTokenByAddress(quoteResponse?.quote?.buyToken.toLowerCase()) + const inputCurrency = useTokenByAddress(quoteResponse?.quote?.sellToken?.toLowerCase()) + const outputCurrency = useTokenByAddress(quoteResponse?.quote?.buyToken?.toLowerCase()) return { inputCurrency, outputCurrency } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (32)
apps/cowswap-frontend/src/common/hooks/useGetExecutedBridgeSummary.ts(1 hunks)apps/cowswap-frontend/src/common/pure/CurrencyInputPanel/CurrencyInputPanel.tsx(1 hunks)apps/cowswap-frontend/src/common/pure/ReceiveAmountInfo/index.tsx(1 hunks)apps/cowswap-frontend/src/common/utils/getBridgeIntermediateTokenAddress.ts(1 hunks)apps/cowswap-frontend/src/modules/bridge/hooks/index.ts(0 hunks)apps/cowswap-frontend/src/modules/bridge/hooks/useBridgeQuoteAmounts.ts(2 hunks)apps/cowswap-frontend/src/modules/bridge/hooks/useQuoteBridgeContext.ts(1 hunks)apps/cowswap-frontend/src/modules/bridge/hooks/useQuoteSwapContext.ts(2 hunks)apps/cowswap-frontend/src/modules/bridge/hooks/useTryFindIntermediateToken.ts(0 hunks)apps/cowswap-frontend/src/modules/swap/containers/SwapWidget/index.tsx(3 hunks)apps/cowswap-frontend/src/modules/trade/containers/TradeBasicConfirmDetails/index.tsx(1 hunks)apps/cowswap-frontend/src/modules/trade/containers/TradeFeesAndCosts/index.tsx(1 hunks)apps/cowswap-frontend/src/modules/trade/containers/TradeWidget/TradeWidgetForm.tsx(0 hunks)apps/cowswap-frontend/src/modules/trade/hooks/useGetReceiveAmountInfo.ts(1 hunks)apps/cowswap-frontend/src/modules/trade/hooks/useGetSwapReceiveAmountInfo.ts(1 hunks)apps/cowswap-frontend/src/modules/trade/hooks/useReceiveAmounts.ts(1 hunks)apps/cowswap-frontend/src/modules/trade/hooks/useTryFindIntermediateTokenInTokensMap.ts(0 hunks)apps/cowswap-frontend/src/modules/trade/index.ts(1 hunks)apps/cowswap-frontend/src/modules/trade/pure/NetworkCostsRow/index.tsx(1 hunks)apps/cowswap-frontend/src/modules/trade/types/ReceiveAmountInfo.ts(2 hunks)apps/cowswap-frontend/src/modules/trade/utils/getCrossChainReceiveAmountInfo.test.ts(1 hunks)apps/cowswap-frontend/src/modules/trade/utils/getCrossChainReceiveAmountInfo.ts(1 hunks)apps/cowswap-frontend/src/modules/trade/utils/getOrderTypeReceiveAmounts.ts(1 hunks)apps/cowswap-frontend/src/modules/trade/utils/getReceiveAmountInfo.test.ts(4 hunks)apps/cowswap-frontend/src/modules/trade/utils/getReceiveAmountInfo.ts(2 hunks)apps/cowswap-frontend/src/modules/trade/utils/getTotalCosts.ts(1 hunks)apps/cowswap-frontend/src/modules/trade/utils/types.ts(1 hunks)apps/cowswap-frontend/src/modules/tradeFormValidation/hooks/useTradeFormValidationContext.ts(2 hunks)apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/TradeRateDetails/index.tsx(4 hunks)libs/tokens/src/hooks/tokens/useTokenByAddress.ts(1 hunks)libs/tokens/src/hooks/tokens/useTryFindToken.ts(1 hunks)libs/tokens/src/index.ts(1 hunks)
💤 Files with no reviewable changes (4)
- apps/cowswap-frontend/src/modules/trade/hooks/useTryFindIntermediateTokenInTokensMap.ts
- apps/cowswap-frontend/src/modules/bridge/hooks/useTryFindIntermediateToken.ts
- apps/cowswap-frontend/src/modules/bridge/hooks/index.ts
- apps/cowswap-frontend/src/modules/trade/containers/TradeWidget/TradeWidgetForm.tsx
🧰 Additional context used
🧠 Learnings (17)
📓 Common learnings
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.
Learnt from: limitofzero
Repo: cowprotocol/cowswap PR: 6351
File: apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.ts:87-121
Timestamp: 2025-10-13T19:41:31.440Z
Learning: In apps/cowswap-frontend/src/modules/erc20Approve, useApproveCallback returns Promise<TransactionResponse | undefined> and is distinct from useApproveCurrency, which can return Promise<TransactionReceipt | SafeMultisigTransactionResponse>. When reviewing approval flows, verify which hook is actually being used before flagging Safe wallet concerns.
📚 Learning: 2025-09-25T08:46:43.815Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6299
File: apps/cowswap-frontend/src/modules/tradeQuote/services/fetchAndProcessQuote.test.ts:376-385
Timestamp: 2025-09-25T08:46:43.815Z
Learning: In fetchAndProcessQuote.ts, when bridgingSdk.getBestQuote() returns null, no loading state reset is needed because loading state management is handled through onQuoteResult() callback for results and processQuoteError() for errors. The null case is intentionally designed not to trigger any manager methods.
Applied to files:
apps/cowswap-frontend/src/common/utils/getBridgeIntermediateTokenAddress.tsapps/cowswap-frontend/src/modules/bridge/hooks/useBridgeQuoteAmounts.tsapps/cowswap-frontend/src/modules/bridge/hooks/useQuoteBridgeContext.ts
📚 Learning: 2025-07-24T10:00:45.353Z
Learnt from: cowdan
Repo: cowprotocol/cowswap PR: 6009
File: apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/HighFeeWarning/hooks/useHighFeeWarning.ts:36-36
Timestamp: 2025-07-24T10:00:45.353Z
Learning: In the CowSwap frontend, when there's a bridgeFee present in the transaction, the isSell flag is always true for business reasons. This means bridge transactions are always structured as sell operations, which ensures consistent currency handling in fee percentage calculations.
Applied to files:
apps/cowswap-frontend/src/modules/trade/utils/types.tsapps/cowswap-frontend/src/modules/trade/utils/getOrderTypeReceiveAmounts.tsapps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/TradeRateDetails/index.tsxapps/cowswap-frontend/src/modules/bridge/hooks/useBridgeQuoteAmounts.tsapps/cowswap-frontend/src/modules/trade/types/ReceiveAmountInfo.tsapps/cowswap-frontend/src/modules/trade/utils/getReceiveAmountInfo.tsapps/cowswap-frontend/src/common/pure/ReceiveAmountInfo/index.tsxapps/cowswap-frontend/src/modules/trade/utils/getCrossChainReceiveAmountInfo.test.tsapps/cowswap-frontend/src/modules/trade/utils/getReceiveAmountInfo.test.ts
📚 Learning: 2025-10-10T20:28:16.565Z
Learnt from: fairlighteth
Repo: cowprotocol/cowswap PR: 6347
File: apps/cowswap-frontend/src/modules/trade/pure/TradeConfirmation/index.tsx:49-49
Timestamp: 2025-10-10T20:28:16.565Z
Learning: In apps/cowswap-frontend/src/modules/trade, TradeConfirmation follows a two-layer architecture: TradeConfirmationView (pure/stateless) in pure/TradeConfirmation/index.tsx renders the UI, while TradeConfirmation (container) in containers/TradeConfirmation/index.tsx wraps it to freeze props during pending trades (via useStableTradeConfirmationProps), wire in signing state (useSigningStep), and inject trade confirmation state (useTradeConfirmState). Consuming modules should import the container TradeConfirmation from 'modules/trade' to preserve this stateful behavior.
<!-- [add_learning]
When reviewing component refactoring in apps/cowswap-frontend/src/modules/trade, recognize the pattern where a pure view component (e.g., TradeConfirmationView) is separated from a stateful container (e.g., TradeConfirmation) that wraps it. The container adds runtime state management (prop stabilization, signing state, etc.) while the view remains testable and composable. Do not flag usages that import th...
Applied to files:
apps/cowswap-frontend/src/modules/trade/utils/types.tsapps/cowswap-frontend/src/common/pure/CurrencyInputPanel/CurrencyInputPanel.tsxapps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/TradeRateDetails/index.tsxapps/cowswap-frontend/src/modules/trade/hooks/useReceiveAmounts.tsapps/cowswap-frontend/src/modules/trade/pure/NetworkCostsRow/index.tsxapps/cowswap-frontend/src/modules/trade/containers/TradeFeesAndCosts/index.tsxapps/cowswap-frontend/src/common/hooks/useGetExecutedBridgeSummary.tsapps/cowswap-frontend/src/modules/trade/hooks/useGetReceiveAmountInfo.tsapps/cowswap-frontend/src/modules/trade/hooks/useGetSwapReceiveAmountInfo.tsapps/cowswap-frontend/src/modules/tradeFormValidation/hooks/useTradeFormValidationContext.tsapps/cowswap-frontend/src/modules/trade/index.tsapps/cowswap-frontend/src/modules/trade/containers/TradeBasicConfirmDetails/index.tsxapps/cowswap-frontend/src/modules/swap/containers/SwapWidget/index.tsxapps/cowswap-frontend/src/modules/trade/utils/getReceiveAmountInfo.test.tsapps/cowswap-frontend/src/modules/bridge/hooks/useQuoteSwapContext.ts
📚 Learning: 2025-07-24T16:42:53.154Z
Learnt from: cowdan
Repo: cowprotocol/cowswap PR: 6009
File: apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/HighFeeWarning/HighFeeWarningTooltipContent.tsx:23-33
Timestamp: 2025-07-24T16:42:53.154Z
Learning: In apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/HighFeeWarning/HighFeeWarningTooltipContent.tsx, the use of toFixed(2) for percentage formatting in tooltip content is intentional and differs from the banner message formatting that uses toSignificant(2, undefined, Rounding.ROUND_DOWN). This formatting difference serves different UX purposes and should not be flagged as inconsistent.
Applied to files:
apps/cowswap-frontend/src/modules/trade/utils/types.tsapps/cowswap-frontend/src/common/pure/CurrencyInputPanel/CurrencyInputPanel.tsxapps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/TradeRateDetails/index.tsxapps/cowswap-frontend/src/modules/trade/pure/NetworkCostsRow/index.tsxapps/cowswap-frontend/src/modules/trade/containers/TradeFeesAndCosts/index.tsxapps/cowswap-frontend/src/modules/trade/containers/TradeBasicConfirmDetails/index.tsxapps/cowswap-frontend/src/common/pure/ReceiveAmountInfo/index.tsxapps/cowswap-frontend/src/modules/swap/containers/SwapWidget/index.tsx
📚 Learning: 2025-11-19T10:18:23.717Z
Learnt from: limitofzero
Repo: cowprotocol/cowswap PR: 6537
File: apps/cowswap-frontend/src/modules/trade/pure/PartnerFeeRow/index.tsx:33-35
Timestamp: 2025-11-19T10:18:23.717Z
Learning: In apps/cowswap-frontend/src/modules/trade/pure/PartnerFeeRow/index.tsx, when there is no partner fee (amount is null/undefined, bps is missing, or amount equals 0), FreeFeeRow is rendered with withTimelineDot={false} hardcoded. This is intentional design—free fee rows should not show the timeline dot regardless of what the parent component passes, as they have a distinct visual treatment from actual fee rows.
Applied to files:
apps/cowswap-frontend/src/common/pure/CurrencyInputPanel/CurrencyInputPanel.tsxapps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/TradeRateDetails/index.tsxapps/cowswap-frontend/src/modules/trade/pure/NetworkCostsRow/index.tsxapps/cowswap-frontend/src/modules/trade/containers/TradeFeesAndCosts/index.tsxapps/cowswap-frontend/src/modules/trade/containers/TradeBasicConfirmDetails/index.tsxapps/cowswap-frontend/src/common/pure/ReceiveAmountInfo/index.tsx
📚 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/tradeWidgetAddons/containers/TradeRateDetails/index.tsxapps/cowswap-frontend/src/modules/trade/containers/TradeFeesAndCosts/index.tsxapps/cowswap-frontend/src/modules/bridge/hooks/useBridgeQuoteAmounts.tsapps/cowswap-frontend/src/modules/trade/hooks/useGetSwapReceiveAmountInfo.tsapps/cowswap-frontend/src/modules/trade/index.tsapps/cowswap-frontend/src/modules/swap/containers/SwapWidget/index.tsxapps/cowswap-frontend/src/modules/bridge/hooks/useQuoteSwapContext.ts
📚 Learning: 2025-10-13T19:41:31.440Z
Learnt from: limitofzero
Repo: cowprotocol/cowswap PR: 6351
File: apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.ts:87-121
Timestamp: 2025-10-13T19:41:31.440Z
Learning: In apps/cowswap-frontend/src/modules/erc20Approve, useApproveCallback returns Promise<TransactionResponse | undefined> and is distinct from useApproveCurrency, which can return Promise<TransactionReceipt | SafeMultisigTransactionResponse>. When reviewing approval flows, verify which hook is actually being used before flagging Safe wallet concerns.
Applied to files:
apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/TradeRateDetails/index.tsxapps/cowswap-frontend/src/modules/trade/hooks/useReceiveAmounts.tsapps/cowswap-frontend/src/common/hooks/useGetExecutedBridgeSummary.tsapps/cowswap-frontend/src/modules/bridge/hooks/useBridgeQuoteAmounts.tsapps/cowswap-frontend/src/modules/trade/hooks/useGetReceiveAmountInfo.tsapps/cowswap-frontend/src/modules/trade/hooks/useGetSwapReceiveAmountInfo.tsapps/cowswap-frontend/src/modules/tradeFormValidation/hooks/useTradeFormValidationContext.tsapps/cowswap-frontend/src/modules/trade/index.tsapps/cowswap-frontend/src/modules/trade/containers/TradeBasicConfirmDetails/index.tsxapps/cowswap-frontend/src/modules/swap/containers/SwapWidget/index.tsxapps/cowswap-frontend/src/modules/trade/utils/getReceiveAmountInfo.test.tsapps/cowswap-frontend/src/modules/bridge/hooks/useQuoteSwapContext.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/common/hooks/useGetExecutedBridgeSummary.tsapps/cowswap-frontend/src/modules/bridge/hooks/useBridgeQuoteAmounts.tsapps/cowswap-frontend/src/modules/trade/hooks/useGetSwapReceiveAmountInfo.tsapps/cowswap-frontend/src/modules/tradeFormValidation/hooks/useTradeFormValidationContext.tsapps/cowswap-frontend/src/modules/trade/utils/getCrossChainReceiveAmountInfo.test.tsapps/cowswap-frontend/src/modules/swap/containers/SwapWidget/index.tsxapps/cowswap-frontend/src/modules/trade/utils/getReceiveAmountInfo.test.tsapps/cowswap-frontend/src/modules/bridge/hooks/useQuoteBridgeContext.tsapps/cowswap-frontend/src/modules/bridge/hooks/useQuoteSwapContext.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/cowswap-frontend/src/modules/trade/index.tsapps/cowswap-frontend/src/modules/swap/containers/SwapWidget/index.tsxapps/cowswap-frontend/src/modules/bridge/hooks/useQuoteSwapContext.ts
📚 Learning: 2025-07-24T16:43:47.639Z
Learnt from: cowdan
Repo: cowprotocol/cowswap PR: 6009
File: apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/HighFeeWarning/highFeeWarningHelpers.ts:18-20
Timestamp: 2025-07-24T16:43:47.639Z
Learning: In apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/HighFeeWarning/highFeeWarningHelpers.ts, the formatFeePercentage function uses ROUND_DOWN with toSignificant(2) for "at least X%" messaging. This ensures the displayed percentage is never higher than the actual fee, making the "at least" phrasing accurate. For example, if the actual fee is 25.4%, displaying "at least 25%" is correct, but "at least 26%" would be misleading.
Applied to files:
apps/cowswap-frontend/src/modules/trade/utils/getTotalCosts.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/tokens/src/index.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/index.tsapps/cowswap-frontend/src/modules/swap/containers/SwapWidget/index.tsx
📚 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:
libs/tokens/src/index.tsapps/cowswap-frontend/src/modules/swap/containers/SwapWidget/index.tsx
📚 Learning: 2025-06-23T07:03:50.760Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 5859
File: apps/cowswap-frontend/src/modules/tradeQuote/hooks/useTradeQuotePolling.ts:76-82
Timestamp: 2025-06-23T07:03:50.760Z
Learning: In the useTradeQuotePolling hook, there are two useLayoutEffect hooks that work together: one resets the counter to 0 when the confirmation modal closes, and another automatically triggers pollQuote(false, true) whenever the counter reaches 0. This creates an intentional chain reaction for immediate quote updates.
Applied to files:
apps/cowswap-frontend/src/modules/swap/containers/SwapWidget/index.tsx
📚 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/modules/bridge/hooks/useQuoteSwapContext.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/bridge/hooks/useQuoteSwapContext.ts
🧬 Code graph analysis (14)
libs/tokens/src/hooks/tokens/useTryFindToken.ts (2)
libs/tokens/src/index.ts (2)
useTryFindToken(46-46)useTokenByAddress(45-45)libs/common-const/src/types.ts (1)
TokenWithLogo(6-36)
apps/cowswap-frontend/src/modules/trade/utils/getOrderTypeReceiveAmounts.ts (1)
apps/cowswap-frontend/src/modules/trade/types/ReceiveAmountInfo.ts (4)
ReceiveAmountInfo(20-47)OrderTypeReceiveAmounts(3-8)Currencies(10-13)BridgeFeeAmounts(15-18)
apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/TradeRateDetails/index.tsx (4)
apps/cowswap-frontend/src/modules/trade/hooks/useGetReceiveAmountInfo.ts (1)
useGetReceiveAmountInfo(16-43)apps/cowswap-frontend/src/modules/trade/hooks/useGetSwapReceiveAmountInfo.ts (1)
useGetSwapReceiveAmountInfo(21-25)apps/cowswap-frontend/src/modules/tradeQuote/hooks/useTradeQuote.ts (1)
useTradeQuote(14-29)apps/cowswap-frontend/src/modules/bridge/hooks/useBridgeQuoteAmounts.ts (1)
useBridgeQuoteAmounts(8-30)
libs/tokens/src/hooks/tokens/useTokenByAddress.ts (2)
libs/tokens/src/index.ts (2)
useTokenByAddress(45-45)useTokensByAddressMap(43-43)libs/common-const/src/types.ts (1)
TokenWithLogo(6-36)
apps/cowswap-frontend/src/common/hooks/useGetExecutedBridgeSummary.ts (3)
apps/cowswap-frontend/src/utils/getExecutedSummaryData.ts (1)
ExecutedSummaryData(13-20)libs/tokens/src/index.ts (1)
useTokenByAddress(45-45)libs/tokens/src/hooks/tokens/useTokenByAddress.ts (1)
useTokenByAddress(8-18)
apps/cowswap-frontend/src/modules/trade/hooks/useGetSwapReceiveAmountInfo.ts (5)
apps/cowswap-frontend/src/modules/trade/types/ReceiveAmountInfo.ts (1)
ReceiveAmountInfo(20-47)apps/cowswap-frontend/src/modules/trade/utils/getReceiveAmountInfo.ts (1)
getReceiveAmountInfo(18-61)apps/cowswap-frontend/src/modules/trade/utils/types.ts (1)
ReceiveAmountInfoParams(9-16)apps/cowswap-frontend/src/modules/trade/hooks/useDerivedTradeState.ts (1)
useDerivedTradeState(7-9)apps/cowswap-frontend/src/modules/tradeQuote/hooks/useTradeQuote.ts (1)
useTradeQuote(14-29)
apps/cowswap-frontend/src/modules/tradeFormValidation/hooks/useTradeFormValidationContext.ts (2)
libs/tokens/src/hooks/tokens/useTryFindToken.ts (1)
useTryFindToken(8-28)apps/cowswap-frontend/src/common/utils/getBridgeIntermediateTokenAddress.ts (1)
getBridgeIntermediateTokenAddress(4-6)
apps/cowswap-frontend/src/modules/trade/utils/getReceiveAmountInfo.ts (2)
apps/cowswap-frontend/src/modules/trade/types/ReceiveAmountInfo.ts (2)
Currencies(10-13)ReceiveAmountInfo(20-47)apps/cowswap-frontend/src/modules/trade/utils/types.ts (1)
ReceiveAmountInfoParams(9-16)
apps/cowswap-frontend/src/modules/trade/utils/getTotalCosts.ts (3)
apps/cowswap-frontend/src/modules/trade/types/ReceiveAmountInfo.ts (1)
ReceiveAmountInfo(20-47)apps/cowswap-frontend/src/modules/trade/utils/getOrderTypeReceiveAmounts.ts (1)
getOrderTypeReceiveAmounts(5-37)libs/common-utils/src/fractionUtils.ts (1)
FractionUtils(11-186)
apps/cowswap-frontend/src/common/pure/ReceiveAmountInfo/index.tsx (1)
apps/cowswap-frontend/src/common/pure/ReceiveAmountInfo/FeeItem.tsx (1)
FeeItem(18-40)
apps/cowswap-frontend/src/modules/trade/utils/getCrossChainReceiveAmountInfo.test.ts (5)
libs/common-const/src/tokens.ts (2)
USDT_BNB(453-461)USDC_BASE(277-284)libs/common-const/src/types.ts (1)
TokenWithLogo(6-36)libs/widget-lib/src/types.ts (1)
SupportedChainId(4-4)apps/cowswap-frontend/src/modules/trade/utils/getCrossChainReceiveAmountInfo.ts (1)
getCrossChainReceiveAmountInfo(8-23)apps/cowswap-frontend/src/modules/trade/utils/getOrderTypeReceiveAmounts.ts (1)
getOrderTypeReceiveAmounts(5-37)
apps/cowswap-frontend/src/modules/trade/utils/getReceiveAmountInfo.test.ts (1)
apps/cowswap-frontend/src/modules/trade/utils/getCrossChainReceiveAmountInfo.ts (1)
getCrossChainReceiveAmountInfo(8-23)
apps/cowswap-frontend/src/modules/bridge/hooks/useQuoteBridgeContext.ts (2)
apps/cowswap-frontend/src/modules/bridge/hooks/useBridgeQuoteAmounts.ts (1)
useBridgeQuoteAmounts(8-30)apps/cowswap-frontend/src/modules/bridge/hooks/index.ts (1)
useBridgeQuoteAmounts(4-4)
apps/cowswap-frontend/src/modules/bridge/hooks/useQuoteSwapContext.ts (1)
apps/cowswap-frontend/src/modules/trade/hooks/useGetSwapReceiveAmountInfo.ts (1)
useGetSwapReceiveAmountInfo(21-25)
🪛 GitHub Actions: CI
apps/cowswap-frontend/src/modules/trade/utils/getReceiveAmountInfo.test.ts
[error] 470-470: Test failure in getReceiveAmountInfo: bridge fee decimals calculation returned 50000000000 for amountInDestinationCurrency, but expected '0.05'.
🪛 Gitleaks (8.30.0)
apps/cowswap-frontend/src/modules/trade/utils/getCrossChainReceiveAmountInfo.test.ts
[high] 11-11: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (36)
apps/cowswap-frontend/src/common/utils/getBridgeIntermediateTokenAddress.ts (1)
1-6: LGTM! Clean utility for extracting intermediate token address.The function safely extracts the bridge intermediate token address with proper null handling via optional chaining and the
??operator. This centralizes access toquote.tradeParameters.sellTokenAddressand enables address-based token resolution in downstream components.apps/cowswap-frontend/src/modules/trade/hooks/useReceiveAmounts.ts (1)
6-6: LGTM! Import path updated to reflect utility reorganization.The import path now points to the dedicated
getOrderTypeReceiveAmountsutility module. Function signature and usage remain unchanged—this is a clean refactor with no behavioral impact.apps/cowswap-frontend/src/common/pure/ReceiveAmountInfo/index.tsx (1)
78-80: LGTM! Bridge costs now display in destination currency.The change from
amountInIntermediateCurrencytoamountInDestinationCurrencyaligns with the PR's goal to fix cross-chain quote displays. Showing bridge costs in the destination currency improves clarity for users by presenting costs in the currency they will actually receive.apps/cowswap-frontend/src/modules/bridge/hooks/useQuoteBridgeContext.ts (1)
20-20: LGTM! Removed overrideBridgeBuyAmount parameter.The call to
useBridgeQuoteAmounts()now uses the default behavior without theoverrideBridgeBuyAmountparameter. This aligns with the PR's refactor to remove hacky parameter overrides and rely on dedicated swap/cross-chain receive amount flows.apps/cowswap-frontend/src/modules/trade/containers/TradeBasicConfirmDetails/index.tsx (1)
21-21: LGTM! Import path updated for extracted utility.The import now points to the dedicated
getOrderTypeReceiveAmountsmodule. This extraction improves modularity with no behavioral changes.libs/tokens/src/index.ts (1)
45-46: LGTM! New token resolution hooks added to public API.The exports for
useTokenByAddressanduseTryFindTokenextend the tokens library's public API to support address-based token resolution and fallback lookups. These hooks enable the refactored bridge/swap flows to resolve intermediate tokens more cleanly.apps/cowswap-frontend/src/modules/trade/containers/TradeFeesAndCosts/index.tsx (1)
9-10: LGTM! Imports updated for modularized utilities.Both
getOrderTypeReceiveAmountsandgetTotalCostsare now imported from their dedicated modules, improving code organization. Function signatures and usage remain unchanged.libs/tokens/src/hooks/tokens/useTokenByAddress.ts (1)
1-18: LGTM! Clean implementation of address-based token lookup.The hook correctly:
- Handles nullish input with an early return
- Uses case-insensitive address matching via
toLowerCase()- Memoizes the result with proper dependencies
This provides a clean interface for resolving tokens by address throughout the bridge/trade flows.
apps/cowswap-frontend/src/modules/swap/containers/SwapWidget/index.tsx (1)
5-5: Intermediate token resolution refactor looks consistentSwitching to
useTryFindToken(getBridgeIntermediateTokenAddress(bridgeQuote))centralizes the bridge intermediate-token lookup on the shared tokens hooks and keeps the existingintermediateBuyToken/toBeImportedwiring intoTradeButtonsintact. NullbridgeQuoteis handled via the helper, so this should be a safe drop-in replacement.Also applies to: 32-32, 69-69
apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/TradeRateDetails/index.tsx (1)
14-15: Swap vs cross-chain receive info separation is wired correctly hereUsing
useGetSwapReceiveAmountInfo()alongsideuseGetReceiveAmountInfo()and gating on both before rendering full details matches the new split between swap-leg and cross-chain receive data. FeedingswapReceiveAmountInfo(plus optionalbridgeFeeAmounts.amountInIntermediateCurrency) intogetTotalCostskeeps total-cost computation anchored on the swap leg while still accounting for bridge fees, and the network-fee-only fallback remains intact.Also applies to: 47-49, 52-52, 69-71, 84-87
apps/cowswap-frontend/src/modules/tradeFormValidation/hooks/useTradeFormValidationContext.ts (1)
5-5: Intermediate token import flagging is safer and more explicit nowResolving the bridge intermediate token via
useTryFindToken(getBridgeIntermediateTokenAddress(tradeQuote.bridgeQuote))and then exposingintermediateTokenToBeImportedas!!intermediateBuyToken && toBeImportedensures validation logic only treats a token as “to be imported” when a concrete token was found via search, avoiding false positives when lookup fails.Also applies to: 15-15, 50-52, 69-73
libs/tokens/src/hooks/tokens/useTryFindToken.ts (1)
1-27: NewuseTryFindTokenhook cleanly encapsulates lookup+import semanticsThe hook correctly prefers
useTokenByAddressover the “non-existent token” search and exposes a simple{ token, toBeImported }contract, with memoization on the two underlying lookups. This nicely centralizes token/import resolution for bridge and swap flows.apps/cowswap-frontend/src/modules/trade/pure/NetworkCostsRow/index.tsx (1)
17-23: ExplicitReactNodereturn type is a straightforward typing improvementAdding
: ReactNodetoNetworkCostsRowtightens the typing without changing behavior; all props and JSX remain the same.apps/cowswap-frontend/src/common/pure/CurrencyInputPanel/CurrencyInputPanel.tsx (1)
280-287: Receive amount row now correctly follows presence ofreceiveAmountInfoAlways rendering
<ReceiveAmount>when bothreceiveAmountInfoandcurrencyare set removes the previous ad‑hoc hiding behavior and lets callsites control visibility viaCurrencyInfo.receiveAmountInfo. This is consistent with the new cross-chain receive-amount logic and should ensure “Receive (incl. costs)” appears whenever valid data is available.apps/cowswap-frontend/src/common/hooks/useGetExecutedBridgeSummary.ts (1)
3-11: Bridge execution summary now uses shared token lookup by addressUsing
useTokenByAddress(order?.buyToken)to derive the intermediate token, and then delegating togetExecutedSummaryDataWithSurplusTokenwhen available, aligns this hook with the centralized token registry and keeps the no-order case safely returningundefined.Also applies to: 14-22
apps/cowswap-frontend/src/modules/bridge/hooks/useQuoteSwapContext.ts (1)
7-7:useQuoteSwapContextcorrectly switches to swap‑only receive infoUsing
useGetSwapReceiveAmountInfo()here makes the swap quote context depend solely on the swap-leg receive data, while cross-chain specifics are handled elsewhere. Combined withuseBridgeQuoteAmounts, this keeps the swap metrics (sell, buy, min receive, USD values) internally consistent.Also applies to: 17-17
apps/cowswap-frontend/src/modules/trade/utils/getTotalCosts.ts (1)
8-39: LGTM! Well-structured cost aggregation utility.The function correctly handles:
- Accumulation of network, partner, and protocol fees
- Currency conversion when
additionalCostshas different decimals- Direct addition when currencies match
apps/cowswap-frontend/src/modules/trade/utils/types.ts (1)
1-22: LGTM! Clean type definitions for the refactored receive amount flow.The type hierarchy properly separates base swap parameters from cross-chain-specific fields. Using
bigintfor bridge fee amounts aligns with blockchain value handling.apps/cowswap-frontend/src/modules/trade/index.ts (1)
43-53: LGTM! Public API correctly updated for the refactored cross-chain flow.The new exports (
useGetSwapReceiveAmountInfo,getCrossChainReceiveAmountInfo,getTotalCosts,getOrderTypeReceiveAmounts) properly expose the refactored utilities for cross-chain receive amount calculations.apps/cowswap-frontend/src/modules/trade/utils/getCrossChainReceiveAmountInfo.test.ts (1)
9-24: LGTM! Comprehensive test with realistic cross-chain scenario.The test thoroughly validates the
getCrossChainReceiveAmountInfofunction with a real-world USDT (BNB) → USDC (Base) swap using BNB as the intermediate token. The detailed comments explaining each calculation step are excellent for maintainability.The static analysis warning about a "generic-api-key" on line 11 is a false positive — this is standard
appDataJSON containing order metadata, not a secret.apps/cowswap-frontend/src/modules/trade/utils/getCrossChainReceiveAmountInfo.ts (2)
8-23: LGTM! Clean extraction of cross-chain receive amount logic.The function properly delegates to
getReceiveAmountInfoand enriches the result with bridge fee information. The spread pattern preserves existing costs while adding the bridge fee.
25-39: The field mapping incalculateBridgeFeeis correct.The mapping aligns with the business logic that bridge transactions are always structured as sell operations:
amountInIntermediateCurrency←amountInBuyCurrency(fee in intermediate/buy currency)amountInDestinationCurrency←amountInSellCurrency(fee in final destination currency)This is confirmed by the test expectations and consistent with the transaction semantics.
apps/cowswap-frontend/src/modules/trade/utils/getOrderTypeReceiveAmounts.ts (3)
5-37: LGTM! Well-structured branching logic.The function cleanly separates protocol fee and non-protocol fee paths. The
hasProtocolFeecheck correctly handles both the undefined case and zero bps.
39-50: Bridge fee handling aligns with business invariant.The bridge fee subtraction only applies when
isSellis true, which is correct given the documented invariant that bridge transactions are always structured as sell operations. Based on retrieved learnings, this ensures consistent currency handling.
52-70: LGTM! Consistent implementation across both paths.Both helper functions follow the same pattern for computing amounts, with the only difference being the source of
amountBeforeFees(beforeNetworkCostsvsbeforeAllFees). The logic is clear and maintains consistency.Also applies to: 72-91
apps/cowswap-frontend/src/modules/trade/hooks/useGetSwapReceiveAmountInfo.ts (2)
21-25: LGTM! Clean hook composition.The hook properly memoizes the result and handles the null case when params are unavailable.
40-54: Good defensive guard against state mismatch.The check
orderKind !== orderParams?.kindat line 42 prevents returning stale or inconsistent data when the UI state and quote response are temporarily out of sync. This is a solid pattern for handling async state updates.apps/cowswap-frontend/src/modules/trade/types/ReceiveAmountInfo.ts (2)
10-18: LGTM! Well-defined interfaces for currency amounts.The
CurrenciesandBridgeFeeAmountsinterfaces provide clear, reusable types. The naming convention (amountInIntermediateCurrency,amountInDestinationCurrency) clearly indicates the semantic meaning of each field.
38-46: Clean type consolidation.Using the
Currenciestype for all the amount bundles improves consistency and reduces duplication. The optionalbridgeFeefield correctly uses the newBridgeFeeAmountstype.apps/cowswap-frontend/src/modules/bridge/hooks/useBridgeQuoteAmounts.ts (2)
8-11: LGTM! Simplified hook signature.Removing the
overrideBridgeBuyAmountparameter and using the dedicateduseGetSwapReceiveAmountInfohook for swap-specific data is a cleaner approach. The separation of concerns between cross-chain and swap-only receive info is now explicit.
13-29: Clear semantic distinction between swap and bridge amounts.The logic correctly differentiates:
swapBuyAmount(pre-slippage viaafterNetworkCosts)swapMinReceiveAmount(post-slippage viaafterSlippage)bridgeMinReceiveAmount(final destination amount post-slippage)This aligns with the PR objective to properly display "Receive (incl. costs)" for cross-chain quotes.
apps/cowswap-frontend/src/modules/trade/hooks/useGetReceiveAmountInfo.ts (2)
16-24: LGTM! Clean hook refactoring.The simplified signature removes the confusing boolean parameter. Using
useTryFindTokenwithgetBridgeIntermediateTokenAddressis a cleaner abstraction for intermediate token resolution.
26-42: Well-structured conditional for cross-chain flow.The explicit check for
intermediateCurrency && bridgeFeeAmounts && bridgeBuyAmountclearly gates the cross-chain path. The comment at line 32 helpfully documents why the currency override is necessary.apps/cowswap-frontend/src/modules/trade/utils/getReceiveAmountInfo.ts (3)
18-36: LGTM! Clean parameterization and SDK integration.The refactored signature with
ReceiveAmountInfoParamsis much cleaner than the previous multi-parameter approach. ThebuyAmountOverridehandling correctly updates both the currency context (line 23) and the order params (line 29).
38-60: Bridge fee intentionally deferred to cross-chain path.Setting
bridgeFee = undefinedhere is correct since this function now serves as the base calculation, with bridge fees added bygetCrossChainReceiveAmountInfowhen applicable.
63-110: Well-factored helper functions.The helper functions provide clean abstractions:
calculateNetworkFee: Handles the dual-currency network fee structuremapSellBuyAmounts: Generic sell/buy amount mappingmapFeeAmounts: Correctly selects currency based onisSellflagThe use of
.toString()for bigint-to-string conversion beforeCurrencyAmount.fromRawAmountis the standard pattern.
apps/cowswap-frontend/src/modules/trade/utils/getReceiveAmountInfo.test.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/cowswap-frontend/src/modules/trade/utils/getReceiveAmountInfo.test.ts (1)
395-434: Fix inputCurrency mismatch with sellToken.The test sets
sellToken: mainnetWeth.address(line 398) but usesinputCurrency: mainnetUsdc(line 416). The inputCurrency should match the sellToken being sold in the order. This inconsistency could lead to incorrect test behavior and doesn't accurately reflect a real cross-chain scenario.Apply this diff to fix the mismatch:
const result = getCrossChainReceiveAmountInfo({ orderParams, - inputCurrency: mainnetUsdc, + inputCurrency: mainnetWeth, outputCurrency: baseUsdc, slippagePercent, partnerFeeBps: undefined, intermediateCurrency: mainnetWeth, // intermediate currency with 18 decimals bridgeFeeAmounts, bridgeBuyAmount: 1n, protocolFeeBps: undefined, })
🧹 Nitpick comments (2)
apps/cowswap-frontend/src/modules/trade/utils/getReceiveAmountInfo.test.ts (2)
376-386: Consider using realistic bridgeBuyAmount values in tests.All three bridge fee tests use
bridgeBuyAmount: 1n, representing 1 wei (the smallest unit). While this doesn't break the current tests (which only verify fee decimals), using realistic values would make the tests more representative of actual usage and enable verification that bridgeBuyAmount is correctly incorporated into receive amount calculations.For example, the first test could use:
bridgeBuyAmount: BigInt('950000'), // 0.95 USDC (6 decimals) matching the buyAmountAlso applies to: 414-424, 455-465
356-476: Add test coverage for bridgeBuyAmount usage in receive amount calculations.The current bridge fee tests verify decimal adjustments but don't assert that
bridgeBuyAmountis correctly used to calculate the final receive amounts. Consider adding assertions that verify the receive amount reflects the bridgeBuyAmount value, ensuring getCrossChainReceiveAmountInfo properly maps this parameter to the destination currency amount.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/cowswap-frontend/src/modules/trade/utils/getReceiveAmountInfo.test.ts(4 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
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.
📚 Learning: 2025-10-13T19:41:31.440Z
Learnt from: limitofzero
Repo: cowprotocol/cowswap PR: 6351
File: apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.ts:87-121
Timestamp: 2025-10-13T19:41:31.440Z
Learning: In apps/cowswap-frontend/src/modules/erc20Approve, useApproveCallback returns Promise<TransactionResponse | undefined> and is distinct from useApproveCurrency, which can return Promise<TransactionReceipt | SafeMultisigTransactionResponse>. When reviewing approval flows, verify which hook is actually being used before flagging Safe wallet concerns.
Applied to files:
apps/cowswap-frontend/src/modules/trade/utils/getReceiveAmountInfo.test.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/modules/trade/utils/getReceiveAmountInfo.test.ts
📚 Learning: 2025-07-24T16:42:53.154Z
Learnt from: cowdan
Repo: cowprotocol/cowswap PR: 6009
File: apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/HighFeeWarning/HighFeeWarningTooltipContent.tsx:23-33
Timestamp: 2025-07-24T16:42:53.154Z
Learning: In apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/HighFeeWarning/HighFeeWarningTooltipContent.tsx, the use of toFixed(2) for percentage formatting in tooltip content is intentional and differs from the banner message formatting that uses toSignificant(2, undefined, Rounding.ROUND_DOWN). This formatting difference serves different UX purposes and should not be flagged as inconsistent.
Applied to files:
apps/cowswap-frontend/src/modules/trade/utils/getReceiveAmountInfo.test.ts
📚 Learning: 2025-07-24T10:00:45.353Z
Learnt from: cowdan
Repo: cowprotocol/cowswap PR: 6009
File: apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/HighFeeWarning/hooks/useHighFeeWarning.ts:36-36
Timestamp: 2025-07-24T10:00:45.353Z
Learning: In the CowSwap frontend, when there's a bridgeFee present in the transaction, the isSell flag is always true for business reasons. This means bridge transactions are always structured as sell operations, which ensures consistent currency handling in fee percentage calculations.
Applied to files:
apps/cowswap-frontend/src/modules/trade/utils/getReceiveAmountInfo.test.ts
🧬 Code graph analysis (1)
apps/cowswap-frontend/src/modules/trade/utils/getReceiveAmountInfo.test.ts (1)
apps/cowswap-frontend/src/modules/trade/utils/getCrossChainReceiveAmountInfo.ts (1)
getCrossChainReceiveAmountInfo(8-23)
⏰ 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)
- GitHub Check: Setup
- GitHub Check: Cypress
🔇 Additional comments (2)
apps/cowswap-frontend/src/modules/trade/utils/getReceiveAmountInfo.test.ts (2)
1-8: LGTM!The imports correctly reflect the refactored API, with getCrossChainReceiveAmountInfo and getOrderTypeReceiveAmounts now separated for clearer cross-chain vs regular swap testing.
436-475: Previous critical issue resolved.The outputCurrency is now correctly set to
baseWeth(line 458) to match thebuyTokenin orderParams. This resolves the pipeline failure identified in the previous review.
| const afterNetworkCosts = mapSellBuyAmounts(result.afterNetworkCosts, currencies) | ||
|
|
||
| const bridgeFee = calculateBridgeFee(outputCurrency, intermediateCurrency, bridgeFeeAmounts) | ||
| const bridgeFee = undefined |
There was a problem hiding this comment.
question: why it's undefined here ?
and if we use it like undefined, maybe just not to put it in the result?
costs: {
networkFee: calculateNetworkFee(result.costs.networkFee, currencies),
partnerFee: mapFeeAmounts(isSell, result.costs.partnerFee, currencies),
protocolFee: !!result.costs.protocolFee ? mapFeeAmounts(isSell, result.costs.protocolFee, currencies) : undefined,
// bridgeFee,
},
There was a problem hiding this comment.
100% makes sense, removed it.
Since now we have getReceiveAmountInfo and getCrossChainReceiveAmountInfo separate, we don't add bridgeFee in getReceiveAmountInfo
| if (!additionalCosts.currency.equals(fee.currency)) { | ||
| const additionalCostsFixed = CurrencyAmount.fromRawAmount( | ||
| fee.currency, | ||
| additionalCosts.currency.decimals !== fee.currency.decimals |
There was a problem hiding this comment.
nitpick: not for this pr, but I saw this logic few times in the code, is it possible to move it to helper? due to better readability. I always spend a lot of time to understand this nested constructions with adjusting decimals...
There was a problem hiding this comment.
I see the only place with that case, can you point to other places please?
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/cowswap-frontend/src/modules/trade/utils/getReceiveAmountInfo.ts (1)
10-13: Consider renaming to avoid confusion with the exportedCurrenciestype.The exported
Currenciesinterface inReceiveAmountInfo.tshas a different shape (sellAmount/buyAmountasCurrencyAmount). While this local interface works fine due to scoping, using the same name could cause confusion during maintenance.-interface Currencies { +interface CurrencyPair { inputCurrency: Currency outputCurrency: Currency }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/cowswap-frontend/src/modules/trade/utils/getReceiveAmountInfo.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
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.
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.
📚 Learning: 2025-07-24T10:00:45.353Z
Learnt from: cowdan
Repo: cowprotocol/cowswap PR: 6009
File: apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/HighFeeWarning/hooks/useHighFeeWarning.ts:36-36
Timestamp: 2025-07-24T10:00:45.353Z
Learning: In the CowSwap frontend, when there's a bridgeFee present in the transaction, the isSell flag is always true for business reasons. This means bridge transactions are always structured as sell operations, which ensures consistent currency handling in fee percentage calculations.
Applied to files:
apps/cowswap-frontend/src/modules/trade/utils/getReceiveAmountInfo.ts
📚 Learning: 2025-11-19T10:18:23.717Z
Learnt from: limitofzero
Repo: cowprotocol/cowswap PR: 6537
File: apps/cowswap-frontend/src/modules/trade/pure/PartnerFeeRow/index.tsx:33-35
Timestamp: 2025-11-19T10:18:23.717Z
Learning: In apps/cowswap-frontend/src/modules/trade/pure/PartnerFeeRow/index.tsx, when there is no partner fee (amount is null/undefined, bps is missing, or amount equals 0), FreeFeeRow is rendered with withTimelineDot={false} hardcoded. This is intentional design—free fee rows should not show the timeline dot regardless of what the parent component passes, as they have a distinct visual treatment from actual fee rows.
Applied to files:
apps/cowswap-frontend/src/modules/trade/utils/getReceiveAmountInfo.ts
🧬 Code graph analysis (1)
apps/cowswap-frontend/src/modules/trade/utils/getReceiveAmountInfo.ts (2)
apps/cowswap-frontend/src/modules/trade/types/ReceiveAmountInfo.ts (2)
Currencies(10-13)ReceiveAmountInfo(20-47)apps/cowswap-frontend/src/modules/trade/utils/types.ts (1)
ReceiveAmountInfoParams(9-16)
⏰ 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)
- GitHub Check: Setup
- GitHub Check: Cypress
🔇 Additional comments (4)
apps/cowswap-frontend/src/modules/trade/utils/getReceiveAmountInfo.ts (4)
1-8: LGTM!Clean imports with appropriate separation between external SDK dependencies and local types.
60-74: LGTM!Correctly maps SDK network fee bigint values to
CurrencyAmountobjects with appropriate currencies.
76-107: LGTM!Both helper functions are well-structured. The fee currency selection in
mapFeeAmountscorrectly usesoutputCurrencyfor sell orders andinputCurrencyfor buy orders, aligning with CoW Protocol's fee handling conventions.
22-36: This concern is not applicable. WhenbuyAmountOverrideis passed togetReceiveAmountInfo, it comes fromgetCrossChainReceiveAmountInfowhere it is created asCurrencyAmount.fromRawAmount(outputCurrency, ...)(line 11). Therefore,buyAmountOverride.currencyis guaranteed to be the sameoutputCurrencyobject whose decimals are used in thegetQuoteAmountsAndCostscall (line 32). No decimal mismatch can occur.
@limitofzero this is another problem I would like to solve in another PR.
Anyway, this is not new and already on prod, so I would address it in another interation. |
elena-zh
left a comment
elena-zh
left a comment
There was a problem hiding this comment.
I'm approving this PR as it does not break anything.
However, all calculations for swap and bridge amounts are wrong, and this is on Prod now.
I reported it in https://www.notion.so/cownation/124-swap-n-bridge-amounts-calculation-is-totally-off-2bf8da5f04ca802cba94fddf650db22e, and it would be great to address this issue in the upcoming release.
Summary
Fixes https://www.notion.so/cownation/Return-back-Receive-incl-costs-field-2aa8da5f04ca80ceac9ff9bde41ab44e
Welcome to my refactoring, which also fixed an issue with cross-chain quote details displaying.
I will guide you through the changes commit-by-commit, as it requires some context to understand the changes.
Code review
QuoteAmountsAndCosts, which is calculated in SDK. This object gives comprehensive data on quote amounts before/after all fees/costs/slippage/etc. The object contains mostlybigintvalues, but in CoW Swap we useCurrencyAmount<Currency>. Because of that, we haveuseGetReceiveAmountInfowhich mapsbigint->CurrencyAmount<Currency>.overrideBridgeBuyAmountargument to
useGetReceiveAmountInfohook for cross-chain order amounts displaying. By default,useGetReceiveAmountInforeturns data for the swap part of cross-chain order. Example: Sell 3.49 USDT (Bnb) to USDC (Base) with BNB (Bnb) as intermediate token. Swap part isUSDT (Bnb) -> BNB (Bnb). But for cross-chain orders we displayUSDT (Bnb) -> USDC (Base)and only display info about the intermediate token in expanded details. This hack serves exactly for this goal, it overrides the buyAmount inQuoteAmountsAndCostswith USDC (Base) instead of BNB (Bnb). As an intermediate step, I just removed this hacky argument: 0395415getTotalCostsinto a separate file to keep the code readable: 6f603f9QuoteAmountsAndCostsmapping for regular swaps and cross-chain swaps to make the flows independent and easy to reason about. As a preparation, I renamedgetReceiveAmountInfotogetCrossChainReceiveAmountInfobecause in fact in contains logic for both regular and cross-chain swaps. I'm going to extract the regular swaps logic a bit later. 03f1c14getOrderTypeReceiveAmountsinto a separate file: 063ce95getReceiveAmountInfofromgetCrossChainReceiveAmountInfo. Now one function is responsible only for regular swaps, another for cross-chain swaps. Also, now it's clear thatgetReceiveAmountInfoonly doesbigint->CurrencyAmount<Currency>mapping and nothing else. 4f5730buseGetReceiveAmountInfodepending on order type (regular/cross-chain). I also extracteduseReceiveAmountInfoParamshook to fix//eslint-disable-next-line complexitycomment. 3826a86useTryFindIntermediateTokenInTokensMapis a bit overcomplicated. It checks if order kind is notOrderKind.BUYfor cross-chain swaps. But this is not a case by design, so the check can be omitted. Without the check the hook becomes justuseTokenByAddressas a part oftokenslib. This changes made me notice a broken dependencies direction (seeFIXME: modules trade should not depend on modules/bridge), it will be addressed in the next step. 729bf07useTryFindIntermediateTokenis very similar to the existinguseSearchNonExistentTokenhook, so I refactored it intouseTryFindToken. The hook was moved frommodules/bridgetolibs/tokens, which fixed the mentioned dependencies direction issue. 14a7a11QuoteAmountsAndCosts(akaReceiveAmountInfo) for regular and cross-chain is the buyAmount. In one case it should be intermediate token amount in another destination token amount. In this commit, I mixed up logic and usedintermediateAmountinstead ofbridgeBuyAmount, but this will be fixed in another commit. The most important change here is that nowgetCrossChainReceiveAmountInfoextendsgetReceiveAmountInfoand do not duplicate the mappings. 132c9b6#diff-0ded1cbddcf9103cbe466fe8e5731ecde035dad850a8ecd53139062045a82ec2intermediateAmount->bridgeBuyAmount13e53abuseGetReceiveAmountInfo()anduseGetSwapReceiveAmountInfo(). First hook returns data depending on the swap type (regular/cross-chain), the second one always returns data of the swap part (sellToken -> intermediateToken). We need that because inSwap onsection we need to display values of the swap part which displayes values fromuseBridgeQuoteAmounts999014c#diff-57b3d3957c1052d64d05aa20cd1a4ef0dca1aed39478e417a2dc8aae1bade225Bridge viasection. Also added tests forgetCrossChainReceiveAmountInfo80c104eTo Test
Receive (incl. costs)should be displayed at the destination amount for quotes from Bridge intentsReceive (incl. costs)was hidden only for Near.Summary by CodeRabbit
New Features
Bug Fixes
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.