-
Notifications
You must be signed in to change notification settings - Fork 156
feat(bridge): enhance intermediate token determinition #6640
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds a centralized correlated-tokens entity (atom, updater, hook), exposes the hook from the entities index, threads a getCorrelatedTokens callback through polling and quote-fetch flows, moves correlated-tokens imports/exports to the new entity, updates executed-summary token handling, adds tests, and bumps several Cow Protocol SDK packages. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant PollHook as Polling Hook (usePollQuoteCallback)
participant State as CorrelatedTokens Entity (useGetCorrelatedTokensByChainId)
participant Service as fetchAndProcessQuote
participant SDK as bridgingSdk.getQuote
PollHook->>State: obtain getCorrelatedTokens fn
PollHook->>Service: call fetchAndProcessQuote(..., getCorrelatedTokens)
Service->>SDK: getQuote(..., advancedSettings { ..., getCorrelatedTokens })
SDK-->>Service: returns quote
Service-->>PollHook: returns processed quote
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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)
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 |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/cowswap-frontend/src/entities/correlatedTokens/hooks/useGetCorrelatedTokensByChainId.ts (1)
11-18: Consider removing unnecessary async wrapper.The callback function is declared as
asyncbut doesn't perform any asynchronous operations. Theasynckeyword causes unnecessary Promise wrapping overhead.If synchronous behavior is acceptable, apply this diff:
return useCallback( - async (chainId: SupportedChainId) => { + (chainId: SupportedChainId) => { const tokens = correlatedTokensByChain[chainId] if (!tokens) return [] return Object.keys(tokens) }, [correlatedTokensByChain], )And update the return type:
-export function useGetCorrelatedTokensByChainId(): (chainId: SupportedChainId) => Promise<string[]> { +export function useGetCorrelatedTokensByChainId(): (chainId: SupportedChainId) => string[] {However, if the async signature is intentional for API consistency or future async operations, this can be deferred.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (10)
apps/cowswap-frontend/src/entities/correlatedTokens/hooks/useGetCorrelatedTokensByChainId.ts(1 hunks)apps/cowswap-frontend/src/entities/correlatedTokens/index.ts(1 hunks)apps/cowswap-frontend/src/entities/correlatedTokens/updaters/CorrelatedTokensUpdater.tsx(1 hunks)apps/cowswap-frontend/src/modules/application/containers/App/Updaters.tsx(1 hunks)apps/cowswap-frontend/src/modules/tradeQuote/hooks/usePollQuoteCallback.ts(4 hunks)apps/cowswap-frontend/src/modules/tradeQuote/services/fetchAndProcessQuote.test.ts(1 hunks)apps/cowswap-frontend/src/modules/tradeQuote/services/fetchAndProcessQuote.ts(2 hunks)apps/cowswap-frontend/src/modules/volumeFee/index.ts(0 hunks)apps/cowswap-frontend/src/modules/volumeFee/state/volumeFeeAtom.ts(1 hunks)package.json(1 hunks)
💤 Files with no reviewable changes (1)
- apps/cowswap-frontend/src/modules/volumeFee/index.ts
🧰 Additional context used
🧠 Learnings (11)
📚 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/modules/tradeQuote/services/fetchAndProcessQuote.tsapps/cowswap-frontend/src/modules/tradeQuote/hooks/usePollQuoteCallback.tsapps/cowswap-frontend/src/modules/tradeQuote/services/fetchAndProcessQuote.test.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:
apps/cowswap-frontend/src/entities/correlatedTokens/index.tsapps/cowswap-frontend/src/modules/volumeFee/state/volumeFeeAtom.tsapps/cowswap-frontend/src/modules/tradeQuote/hooks/usePollQuoteCallback.tsapps/cowswap-frontend/src/entities/correlatedTokens/hooks/useGetCorrelatedTokensByChainId.tsapps/cowswap-frontend/src/modules/application/containers/App/Updaters.tsxapps/cowswap-frontend/src/entities/correlatedTokens/updaters/CorrelatedTokensUpdater.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:
apps/cowswap-frontend/src/entities/correlatedTokens/index.tsapps/cowswap-frontend/src/modules/volumeFee/state/volumeFeeAtom.tsapps/cowswap-frontend/src/modules/tradeQuote/hooks/usePollQuoteCallback.tsapps/cowswap-frontend/src/entities/correlatedTokens/hooks/useGetCorrelatedTokensByChainId.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:
apps/cowswap-frontend/src/entities/correlatedTokens/index.ts
📚 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/tradeQuote/hooks/usePollQuoteCallback.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/tradeQuote/hooks/usePollQuoteCallback.tsapps/cowswap-frontend/src/entities/correlatedTokens/hooks/useGetCorrelatedTokensByChainId.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/tradeQuote/hooks/usePollQuoteCallback.tsapps/cowswap-frontend/src/modules/tradeQuote/services/fetchAndProcessQuote.test.ts
📚 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/correlatedTokens/hooks/useGetCorrelatedTokensByChainId.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/entities/correlatedTokens/hooks/useGetCorrelatedTokensByChainId.tsapps/cowswap-frontend/src/modules/application/containers/App/Updaters.tsx
📚 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/application/containers/App/Updaters.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/application/containers/App/Updaters.tsx
🧬 Code graph analysis (2)
apps/cowswap-frontend/src/modules/tradeQuote/hooks/usePollQuoteCallback.ts (4)
apps/cowswap-frontend/src/entities/correlatedTokens/hooks/useGetCorrelatedTokensByChainId.ts (1)
useGetCorrelatedTokensByChainId(8-21)apps/cowswap-frontend/src/entities/correlatedTokens/index.ts (1)
useGetCorrelatedTokensByChainId(3-3)apps/cowswap-frontend/src/modules/tradeQuote/services/fetchAndProcessQuote.ts (1)
fetchAndProcessQuote(30-84)apps/cowswap-frontend/src/modules/twap/services/cancelTwapOrderTxs.ts (1)
appData(111-113)
apps/cowswap-frontend/src/modules/tradeQuote/services/fetchAndProcessQuote.test.ts (1)
apps/cowswap-frontend/src/modules/tradeQuote/services/fetchAndProcessQuote.ts (1)
fetchAndProcessQuote(30-84)
⏰ 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 (9)
package.json (1)
82-87: LGTM! Pre-release SDK versions properly pinned.All Cow Protocol SDK packages have been consistently updated to pre-release versions tied to PR #738 (commit 4aa6da12). This ensures the bridge intermediate token selection logic from the referenced SDK PR is available for testing.
apps/cowswap-frontend/src/modules/application/containers/App/Updaters.tsx (1)
12-12: LGTM! Import path updated to centralized location.The CorrelatedTokensUpdater import source has been correctly updated to use the centralized entities/correlatedTokens module instead of modules/volumeFee, aligning with the broader refactoring to consolidate correlated tokens functionality.
apps/cowswap-frontend/src/modules/volumeFee/state/volumeFeeAtom.ts (1)
7-7: LGTM! Import path centralized.The correlatedTokensAtom import has been correctly updated to reference the centralized entities/correlatedTokens module, maintaining consistency with the refactoring.
apps/cowswap-frontend/src/modules/tradeQuote/hooks/usePollQuoteCallback.ts (2)
1-1: LGTM! Import path standardized.Changed from
'jotai/index'to'jotai', which is the standard entry point for the library.
24-24: LGTM! Correlated tokens integration properly wired.The new
getCorrelatedTokensByChainIdhook is correctly integrated into the quote polling flow:
- Hook invoked at line 24
- Function passed to
fetchAndProcessQuoteat line 70- Properly included in the useCallback dependency array at line 99
This enables the SDK to apply the 4-tier priority system for intermediate token selection during cross-chain quotes.
Also applies to: 60-71, 91-100
apps/cowswap-frontend/src/modules/tradeQuote/services/fetchAndProcessQuote.test.ts (1)
261-293: LGTM! Test coverage added for getCorrelatedTokens forwarding.The new test case properly verifies that the
getCorrelatedTokenscallback is forwarded intoadvancedSettingswhen callingbridgingSdk.getQuote. The test:
- Creates a mock function with return value
- Passes it to
fetchAndProcessQuote- Asserts it's included in the
advancedSettingsobject passed togetQuoteapps/cowswap-frontend/src/entities/correlatedTokens/index.ts (1)
1-3: LGTM! Clean barrel exports for centralized access.The index properly exports the three key pieces of the correlatedTokens module:
correlatedTokensAtomfor stateCorrelatedTokensUpdaterfor state updatesuseGetCorrelatedTokensByChainIdfor consuming the dataThis provides a clean, centralized entry point for the feature.
apps/cowswap-frontend/src/modules/tradeQuote/services/fetchAndProcessQuote.ts (1)
37-37: LGTM! Optional parameter cleanly threaded to advancedSettings.The
getCorrelatedTokensparameter is:
- Declared as optional at line 37, maintaining backward compatibility
- Properly typed as
SwapAdvancedSettings['getCorrelatedTokens']- Included in the
advancedSettingsobject at line 51This enables the SDK to receive the correlated tokens callback for implementing the 4-tier intermediate token priority system.
Also applies to: 51-51
apps/cowswap-frontend/src/entities/correlatedTokens/updaters/CorrelatedTokensUpdater.tsx (1)
26-30: LGTM! Type annotations enhance code clarity.The explicit type annotations for both
querySerializer(params: unknown, returns: string) andCorrelatedTokensUpdater(returns: null) improve type safety and make the function signatures more self-documenting.
apps/cowswap-frontend/src/entities/correlatedTokens/hooks/useGetCorrelatedTokensByChainId.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/cowswap-frontend/src/entities/correlatedTokens/hooks/useGetCorrelatedTokensByChainId.ts (1)
17-17: Optional: consider deduplicating correlated token addresses.If
tokenscan contain multiple records that share the same address key,tokens.flatMap((token) => Object.keys(token))may yield duplicates. If the consumer ofgetCorrelatedTokensonly cares about unique addresses (and ordering is not important), you could normalize here:- return tokens.flatMap((token) => Object.keys(token)) + return Array.from(new Set(tokens.flatMap((token) => Object.keys(token))))Only worth doing if duplicates are expected and not meaningful to downstream logic.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/cowswap-frontend/src/modules/tradeQuote/hooks/__snapshots__/useTradeQuotePolling.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (1)
apps/cowswap-frontend/src/entities/correlatedTokens/hooks/useGetCorrelatedTokensByChainId.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 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/correlatedTokens/hooks/useGetCorrelatedTokensByChainId.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/entities/correlatedTokens/hooks/useGetCorrelatedTokensByChainId.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/entities/correlatedTokens/hooks/useGetCorrelatedTokensByChainId.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:
apps/cowswap-frontend/src/entities/correlatedTokens/hooks/useGetCorrelatedTokensByChainId.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/correlatedTokens/hooks/useGetCorrelatedTokensByChainId.ts
⏰ 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: Cypress
- GitHub Check: Setup
🔇 Additional comments (1)
apps/cowswap-frontend/src/entities/correlatedTokens/hooks/useGetCorrelatedTokensByChainId.ts (1)
8-20: Hook logic and typing look correct.useAtomValue + useCallback usage is sound, the closure over
correlatedTokensByChainis properly memoized via the dependency array, and the async signature(chainId: SupportedChainId) => Promise<string[]>is satisfied with sensible behavior ([]fallback when no data, flattened keys otherwise). No blocking issues from my side here.
elena-zh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @shoom3301 , cool!! Great!
However, I noticed that limit price is wrongly displayed when swap part is running



I don't observe such issue on neither on Prod nor on Dev

Also, there is an issue with amounts displaying on the Swap Traded pop-up:

Could you please take a look at it?
|
|
||
| if (!tokens) return [] | ||
|
|
||
| return tokens.flatMap((token) => Object.keys(token)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it be Set/filter so we dedup?
the current flatMap can return the same address multiple times (global + pair entries). Wrapping in a
Set before returning (e.g. return Array.from(new Set(tokens.flatMap(...)))) will dedup the list and avoid redundant intermediate-token requests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Fixed that
| }, | ||
| [correlatedTokensByChain], | ||
| ) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider dedicated unit test for this hook
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Added tests: e3560ea
|
Did a quick review on some code things and functionality. I saw when bridging USDC→USDT (e.g., Base→Avalanche), ETH is selected (and not USDT) because Near Intents doesn't offer USDT bridging on that route, or? Anyway I understand the SDK picks the best option by checking the best outcome (amount) by comparing all providers, so probably not relevant in the end. Other than @elena-zh 's points, this is a great feature to potentially provide even better prices/outcomes for the user. |
… feat/bridge-intermediate-token
@fairlighteth Correct, you can see that there is no USDT on Base in their API response: https://1click.chaindefuser.com/v0/tokens |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/cowswap-frontend/src/entities/correlatedTokens/hooks/useGetCorrelatedTokensByChainId.test.tsx (1)
41-80: Good test coverage for core behaviors.The tests effectively cover the three key behaviors: lowercase normalization, uppercase conversion, and deduplication. Consider adding an edge case test for when the chainId has no tokens in state (returns empty array).
Optional: Add a test case for missing chainId:
it('should return empty array when chainId has no tokens', async () => { const initialState = {} const { result } = renderHook(() => useGetCorrelatedTokensByChainId(), { wrapper: ({ children }) => <TestProvider initialState={initialState}>{children}</TestProvider>, }) const tokens = await result.current(SupportedChainId.MAINNET) expect(tokens).toEqual([]) })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/cowswap-frontend/src/entities/correlatedTokens/hooks/useGetCorrelatedTokensByChainId.test.tsx(1 hunks)apps/cowswap-frontend/src/entities/correlatedTokens/hooks/useGetCorrelatedTokensByChainId.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📚 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:
apps/cowswap-frontend/src/entities/correlatedTokens/hooks/useGetCorrelatedTokensByChainId.test.tsxapps/cowswap-frontend/src/entities/correlatedTokens/hooks/useGetCorrelatedTokensByChainId.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/correlatedTokens/hooks/useGetCorrelatedTokensByChainId.test.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:
apps/cowswap-frontend/src/entities/correlatedTokens/hooks/useGetCorrelatedTokensByChainId.test.tsxapps/cowswap-frontend/src/entities/correlatedTokens/hooks/useGetCorrelatedTokensByChainId.ts
📚 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/correlatedTokens/hooks/useGetCorrelatedTokensByChainId.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/entities/correlatedTokens/hooks/useGetCorrelatedTokensByChainId.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/entities/correlatedTokens/hooks/useGetCorrelatedTokensByChainId.ts
🧬 Code graph analysis (1)
apps/cowswap-frontend/src/entities/correlatedTokens/hooks/useGetCorrelatedTokensByChainId.ts (2)
apps/cowswap-frontend/src/entities/correlatedTokens/index.ts (2)
useGetCorrelatedTokensByChainId(3-3)correlatedTokensAtom(1-1)libs/widget-lib/src/types.ts (1)
SupportedChainId(4-4)
⏰ 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: Cypress
- GitHub Check: Setup
🔇 Additional comments (2)
apps/cowswap-frontend/src/entities/correlatedTokens/hooks/useGetCorrelatedTokensByChainId.test.tsx (1)
15-39: LGTM! Clean test scaffolding using standard Jotai patterns.The
HydrateAtomsandTestProvidersetup follows the idiomatic approach for testing Jotai-based hooks. Theas nevercast on line 36 is a reasonable workaround for the strict typing ofuseHydrateAtoms.apps/cowswap-frontend/src/entities/correlatedTokens/hooks/useGetCorrelatedTokensByChainId.ts (1)
8-27: LGTM! Implementation correctly addresses previous review feedback.The hook properly handles the
CorrelatedTokens[]array structure withflatMap, applies lowercase normalization, and deduplicates usingSet. TheuseCallbackdependency oncorrelatedTokensByChainensures the returned function updates when atom state changes.One minor observation: the function is
asyncbut performs no async operations. If thePromise<string[]>return type is intentional for API consistency with other token-fetching callbacks (e.g., for bridging SDK), this is fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/cowswap-frontend/src/common/hooks/useSwapAndBridgeContext.ts (1)
43-49: Intermediate token threading looks solid; consider tightening the fallback decimals assumptionExposing
intermediateTokenviaSwapAndBridgeContextsand resolving it with a map-first +TokenWithLogo.fromTokenfallback is a nice way to keep bridge UIs working in fresh sessions while token lists load. The new return memo also correctly includesintermediateTokenin both value and deps, so consumers likeActivityDetailscan rely on it.One nuance in the fallback on Line 76–82:
return TokenWithLogo.fromToken({ chainId: order.inputToken.chainId, address: order.buyToken, decimals: order.outputToken.decimals, // Use output token decimals as approximation symbol: t`Loading...`, name: t`Loading...`, })This assumes the “intermediate” token at
order.buyTokensharesdecimalswithorder.outputToken. If there are bridge routes where that doesn’t hold, anyCurrencyAmountbuilt fromintermediateToken(e.g.targetAmounts.sellAmount) will be temporarily mis-scaled untiltokensByAddressis populated.Suggestion (non-blocking):
- Either explicitly document this invariant in a short comment (e.g. “Bridge routes guarantee buyToken and final output token share decimals”), or
- Guard precision-sensitive paths (like
targetAmounts) to only use them once an enriched token fromtokensByAddressis available, keeping the placeholder mainly for logos / basic display.That would make the intent clear and avoid subtle surprises if a future route breaks the decimals coupling assumption.
Also applies to: 65-83, 233-236
apps/cowswap-frontend/src/modules/account/containers/Transaction/ActivityDetails.tsx (1)
277-281: Effective output token change is reasonable; please confirm behavior when intermediate ≠ final outputUsing
intermediateTokenfromuseSwapAndBridgeContextand defining:const effectiveOutputToken = intermediateToken ?? outputToken ... const outputAmount = CurrencyAmount.fromRawAmount(effectiveOutputToken, buyAmount.toString()) ... const rateOutputCurrencyAmount = isOrderFulfilled ? CurrencyAmount.fromRawAmount(effectiveOutputToken, executedBuyAmount?.toString() || '0') : outputAmountkeeps non-bridge orders unchanged (no intermediate token → falls back to
outputToken) and aligns bridge orders with the new intermediate-token selection, which is helpful for consistent logos and amount typing.The only thing to double‑check is semantics for bridge orders where the SDK’s “intermediate” token differs from the final destination token:
- In that case,
orderSummary.toandRateInfowill now be expressed in the intermediate token, not the final output token.- If the UX is intended to show the swap leg (input → intermediate) in this panel while
BridgeActivitySummarycovers the cross‑chain leg, this is fine.- If instead you want fulfilled bridge orders to always show rates in the final output token, you might need to branch on
isBridgeOrderand/or useswapAndBridgeOverview.targetCurrencyfor the fulfilled path.Please confirm which behavior is desired so this doesn’t accidentally regress how users read fulfilled bridge orders.
Also applies to: 334-337, 346-348
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/cowswap-frontend/src/common/hooks/useSwapAndBridgeContext.ts(2 hunks)apps/cowswap-frontend/src/modules/account/containers/Transaction/ActivityDetails.tsx(3 hunks)
🧰 Additional context used
🧠 Learnings (9)
📓 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-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:
apps/cowswap-frontend/src/common/hooks/useSwapAndBridgeContext.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/common/hooks/useSwapAndBridgeContext.tsapps/cowswap-frontend/src/modules/account/containers/Transaction/ActivityDetails.tsx
📚 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/common/hooks/useSwapAndBridgeContext.tsapps/cowswap-frontend/src/modules/account/containers/Transaction/ActivityDetails.tsx
📚 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:
apps/cowswap-frontend/src/common/hooks/useSwapAndBridgeContext.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/useSwapAndBridgeContext.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/common/hooks/useSwapAndBridgeContext.ts
📚 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/common/hooks/useSwapAndBridgeContext.ts
📚 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/account/containers/Transaction/ActivityDetails.tsx
🧬 Code graph analysis (2)
apps/cowswap-frontend/src/common/hooks/useSwapAndBridgeContext.ts (1)
libs/common-const/src/types.ts (1)
TokenWithLogo(6-36)
apps/cowswap-frontend/src/modules/account/containers/Transaction/ActivityDetails.tsx (1)
apps/cowswap-frontend/src/common/hooks/useSwapAndBridgeContext.ts (1)
useSwapAndBridgeContext(53-237)
⏰ 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
|
@elena-zh thanks!
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/cowswap-frontend/src/utils/__tests__/getExecutedSummaryData.test.ts (1)
262-274: Misleading comment on surplus calculation.The comment at line 262 states
// 0.025 USDC worth in 6 decimals, butsurplusRaw: '25000'is interpreted using the output token's decimals (baseWeth with 18 decimals) since this is a sell order. The expected result'0.000000000000025'at line 274 confirms the value is in 18-decimal precision, not 6.Consider updating the comment for clarity:
- surplusRaw: '25000', // 0.025 USDC worth in 6 decimals + surplusRaw: '25000', // raw surplus in output token (baseWeth) units
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/cowswap-frontend/src/common/hooks/useGetExecutedBridgeSummary.ts(1 hunks)apps/cowswap-frontend/src/utils/__tests__/getExecutedSummaryData.test.ts(11 hunks)apps/cowswap-frontend/src/utils/getExecutedSummaryData.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 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.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/common/hooks/useGetExecutedBridgeSummary.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/common/hooks/useGetExecutedBridgeSummary.ts
🧬 Code graph analysis (3)
apps/cowswap-frontend/src/common/hooks/useGetExecutedBridgeSummary.ts (3)
apps/cowswap-frontend/src/utils/getExecutedSummaryData.ts (2)
ExecutedSummaryData(12-19)getExecutedSummaryData(21-48)libs/tokens/src/hooks/tokens/useTokenByAddress.ts (1)
useTokenByAddress(8-18)libs/tokens/src/index.ts (1)
useTokenByAddress(45-45)
apps/cowswap-frontend/src/utils/__tests__/getExecutedSummaryData.test.ts (1)
apps/cowswap-frontend/src/utils/getExecutedSummaryData.ts (1)
getExecutedSummaryData(21-48)
apps/cowswap-frontend/src/utils/getExecutedSummaryData.ts (3)
apps/cowswap-frontend/src/common/types.ts (1)
GenericOrder(7-7)apps/cowswap-frontend/src/utils/orderUtils/parseOrder.ts (1)
parseOrder(64-134)apps/cowswap-frontend/src/utils/orderUtils/getFilledAmounts.ts (1)
getFilledAmounts(18-75)
⏰ 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/common/hooks/useGetExecutedBridgeSummary.ts (1)
16-23: LGTM! Clear documentation of cross-chain swap handling.The comment effectively explains the token flow for cross-chain orders. The simplified logic delegating token resolution to
getExecutedSummaryDatais cleaner.One consideration:
useTokenByAddress(order?.buyToken)looks up the token in the current chain's registry. For cross-chain orders whereorder.buyTokenis a destination-chain token address, the lookup would returnnullif that address doesn't exist on the source chain—causing fallback toorder.outputTokeninsidegetExecutedSummaryData. Verify this fallback behavior is intentional for the intermediate token selection logic.apps/cowswap-frontend/src/utils/__tests__/getExecutedSummaryData.test.ts (1)
92-111: Test suite properly updated to reflect API changes.The test suite has been correctly renamed and the test cases comprehensively cover:
- Sell orders with intermediate token override
- Buy orders (surplus stays in input token)
- Zero surplus fallback
- Cross-decimal scenarios
apps/cowswap-frontend/src/utils/getExecutedSummaryData.ts (2)
21-22: Clean API consolidation with proper fallback.The conditional
intermediateToken || order.outputTokenfor sell orders provides a clean fallback when no intermediate token is available, maintaining backward compatibility.
32-38: Verify decimal consistency between execution data and intermediate token.The
rawSurplusfromparsedOrder.executionData.surplusAmountmust be calculated using the same token decimals assurplusTokenfor theCurrencyAmount.fromRawAmountconversion to be accurate.For cross-chain orders, confirm that the backend/execution data calculates surplus based on the on-chain swap (input → intermediate token) rather than the full cross-chain path, ensuring decimal consistency when
intermediateTokendiffers fromorder.outputToken.
| const surplusToken = isSellOrder(order.kind) ? parsedOutputToken : parsedInputToken | ||
| return getExecutedSummaryDataWithSurplusToken(order, surplusToken) | ||
| } | ||
| export function getExecutedSummaryData(order: GenericOrder, intermediateToken: Nullish<Token>): ExecutedSummaryData { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A bit of context for #6629 fix.
- Check
resolveDisplayTokensfunction and you will see thatdefaultSurplusTokenis redundant, because in case of buy order we exit from the function with:
{
effectiveOutputToken: parsedOutputToken,
surplusDisplayToken: parsedInputToken,
surplusAmount: CurrencyAmount.fromRawAmount(parsedInputToken, rawSurplus),
}
- In the sell order case, we can replace
defaultSurplusTokenwithparsedOutputToken
- Further, you will see that
differsFromOutputTokenis actually the same asdiffersFromDefaultToken - What
differsFromOutputTokenmeans? It supposed to be always true in case of cross-chain swap, becausesurplusToken.chainId !== parsedOutputToken.chainId - Having that, we don't call
FractionUtils.adjustDecimalsAtoms()in case of cross-chain swap, but logically we should call it ONLY in case of cross-chain swap - Anyway,
rawSurplusis always in an intermediate token for cross-chain swaps, so we don't actually needresolveDisplayTokensfunction at all. - The main fix is: We define
intermediateTokenin any case (regular swap/cross-chain swap). In the first case it will be the same asorder.outputToken, in the second it will be different. - And finally, we just use
intermediateToken ?? order.outputTokeninstead of justorder.outputTokeneverywhere ingetExecutedSummaryData(). This makes the function calculate the values corresponding to only swap part.
Summary
Fixes #6629
SDK changes: cowprotocol/cow-sdk#738
First of all, there are two issues addressed in this PR:
#6629 - 00557eb
Intermediate token determination enhancement - all other commits
Before this change, we were just taking the first proposed intermediate token from the bridge provider.
In case if bridge provider returned an array of only one intermediate token, then it will be taken by default.
Otherwise, we will apply the following logic:
Tokens are evaluated and ranked using a 4-tier priority system:
🥇 HIGHEST: Stablecoins (USDC/USDT)
🥈 HIGH: Correlated Tokens
🥉 MEDIUM: Native Chain Tokens
0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE📊 LOW: Other Tokens
To Test
Summary by CodeRabbit
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.