-
Notifications
You must be signed in to change notification settings - Fork 32
feat(bridge): determine intermediate token #738
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: main
Are you sure you want to change the base?
Conversation
|
Warning Rate limit exceeded@shoom3301 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 1 minutes and 25 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughIntroduces a new intermediate token selection feature for the Bridging SDK. Implements a priority-based algorithm (HIGHEST → HIGH → MEDIUM → LOW) to select tokens from bridge provider options, adds token priority utilities, integrates into swap result flow, extends the swap settings interface with optional token correlation callback, and documents the selection mechanism. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
📦 GitHub Packages PublishedLast updated: Dec 9, 2025, 12:00:41 PM UTC The following packages have been published to GitHub Packages with pre-release version
InstallationThese packages require authentication to install from GitHub Packages. First, create a # Create .npmrc file in your project root
echo "@cowprotocol:registry=https://npm.pkg.github.com" > .npmrc
echo "//npm.pkg.github.com/:_authToken=YOUR_GITHUB_TOKEN" >> .npmrcTo get your GitHub token:
Then install any of the packages above, either by exact version (i.e. # Yarn
yarn add npm:@cowprotocol/cow-sdk@pr-738
# pnpm
pnpm install npm:@cowprotocol/cow-sdk@pr-738
# NPM
npm install npm:@cowprotocol/cow-sdk@pr-738Update to the latest version (only if you used the tag)Every commit will publish a new package. To upgrade to the latest version, run: # Yarn
yarn upgrade @cowprotocol/cow-sdk
# pnpm
pnpm update @cowprotocol/cow-sdk
# NPM
npm update @cowprotocol/cow-sdkView PackagesYou can view the published packages at: https://github.com/cowprotocol/cow-sdk/packages |
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 (5)
packages/bridging/src/BridgingSdk/determineIntermediateToken.test.ts (2)
55-62: Minor: The two error handling tests are identical.Both tests pass an empty array
[]and expectBridgeProviderQuoteError. Consider removing one or differentiating them (e.g., testing withundefinedtokens if the function signature changes, or testing with an array containing onlyundefinedelements).describe('error handling', () => { it('should throw error when no intermediate tokens provided', async () => { await expect(determineIntermediateToken(SupportedChainId.MAINNET, [])).rejects.toThrow(BridgeProviderQuoteError) }) - - it('should throw error when intermediateTokens is empty array', async () => { - await expect(determineIntermediateToken(SupportedChainId.MAINNET, [])).rejects.toThrow(BridgeProviderQuoteError) - }) })
184-204: Consider adding a test for the single-token optimization.The implementation has an early return when only one token is provided (bypasses priority calculation). Adding an explicit test for this case would improve coverage:
it('should return the only token when single token provided', async () => { const result = await determineIntermediateToken(SupportedChainId.MAINNET, [randomToken]) expect(result).toBe(randomToken) })packages/bridging/src/README.md (1)
86-108: Fix markdown linting issues in example scenarios.The static analysis tool flags two issues:
- Emphasis (
**Scenario X**) used instead of proper headings- Fenced code blocks missing language specifiers
-**Scenario 1: Stablecoin Available** -``` +#### Scenario 1: Stablecoin Available +```text Candidates: [DAI, WETH, USDC] Selected: USDC (HIGHEST priority - stablecoin)-Scenario 2: No Stablecoin
-+#### Scenario 2: No Stablecoin +text
Candidates: [RandomToken, WETH (correlated), NativeETH]
Selected: WETH (HIGH priority - correlated token)-**Scenario 3: Only Native and Random Tokens** -``` +#### Scenario 3: Only Native and Random Tokens +```text Candidates: [RandomToken1, NativeETH, RandomToken2] Selected: NativeETH (MEDIUM priority - native currency)-Scenario 4: All Low Priority
-+#### Scenario 4: All Low Priority +text
Candidates: [TokenA, TokenB, TokenC]
Selected: TokenA (first in list when all have same priority)packages/bridging/src/BridgingSdk/tokenPriority.ts (1)
63-65: Minor optimization: Cache the lowercase native currency address.
NATIVE_CURRENCY_ADDRESS.toLowerCase()is called on every invocation, but this is a constant. Consider caching it:+const NATIVE_ADDRESS_LOWER = NATIVE_CURRENCY_ADDRESS.toLowerCase() + /** * Checks if a token is the native blockchain currency (ETH, MATIC, AVAX, etc.) */ export function isNativeToken(tokenAddress: string): boolean { - return tokenAddress.toLowerCase() === NATIVE_CURRENCY_ADDRESS.toLowerCase() + return tokenAddress.toLowerCase() === NATIVE_ADDRESS_LOWER }packages/bridging/src/BridgingSdk/determineIntermediateToken.ts (1)
61-68: Consider pre-computing indices for stable sort.The
indexOf()call in the comparator is O(n) per comparison. For small token lists this is fine, but you could pre-compute indices in the mapping phase for O(n log n) overall:// Calculate priority for each token - const tokensWithPriority = intermediateTokens.map((token) => { + const tokensWithPriority = intermediateTokens.map((token, index) => { if (isHighPriorityToken(token.chainId, token.address)) { - return { token, priority: TokenPriority.HIGHEST } + return { token, priority: TokenPriority.HIGHEST, index } } if (isCorrelatedToken(token.address, correlatedTokens)) { - return { token, priority: TokenPriority.HIGH } + return { token, priority: TokenPriority.HIGH, index } } if (isNativeToken(token.address)) { - return { token, priority: TokenPriority.MEDIUM } + return { token, priority: TokenPriority.MEDIUM, index } } - return { token, priority: TokenPriority.LOW } + return { token, priority: TokenPriority.LOW, index } }) // Sort by priority (highest first), then by original order for stability tokensWithPriority.sort((a, b) => { if (a.priority !== b.priority) { return b.priority - a.priority // Higher priority first } // Maintain original order for tokens with same priority - return intermediateTokens.indexOf(a.token) - intermediateTokens.indexOf(b.token) + return a.index - b.index })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/contracts-ts/src/generated/packageVersion.tsis excluded by!**/generated/**
📒 Files selected for processing (6)
packages/bridging/src/BridgingSdk/determineIntermediateToken.test.ts(1 hunks)packages/bridging/src/BridgingSdk/determineIntermediateToken.ts(1 hunks)packages/bridging/src/BridgingSdk/getIntermediateSwapResult.ts(2 hunks)packages/bridging/src/BridgingSdk/tokenPriority.ts(1 hunks)packages/bridging/src/README.md(2 hunks)packages/trading/src/types.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: anxolin
Repo: cowprotocol/cow-sdk PR: 251
File: src/bridging/BridgingSdk/getQuoteWithBridging.ts:34-35
Timestamp: 2025-03-19T22:06:52.139Z
Learning: The implementation in the Across bridge provider deliberately selects only the first intermediate token from the list (`intermediateTokens[0]`) as a simplification for the initial implementation, with potential for future enhancement.
Learnt from: anxolin
Repo: cowprotocol/cow-sdk PR: 267
File: src/bridging/BridgingSdk/getCrossChainOrder.ts:44-46
Timestamp: 2025-04-14T20:37:56.543Z
Learning: The decoding of bridge appData (using provider.decodeBridgeHook) in the getCrossChainOrder function was intentionally left as a TODO comment for implementation in a future PR.
Learnt from: shoom3301
Repo: cowprotocol/cow-sdk PR: 324
File: src/bridging/types.ts:384-385
Timestamp: 2025-05-30T05:56:12.625Z
Learning: The BridgingDepositParams interface in src/bridging/types.ts includes a bridgingId: string field along with other deposit parameters like inputTokenAddress, outputTokenAddress, inputAmount, outputAmount, owner, quoteTimestamp, fillDeadline, recipient, sourceChainId, and destinationChainId.
📚 Learning: 2025-05-30T05:56:12.625Z
Learnt from: shoom3301
Repo: cowprotocol/cow-sdk PR: 324
File: src/bridging/types.ts:384-385
Timestamp: 2025-05-30T05:56:12.625Z
Learning: The BridgingDepositParams interface in src/bridging/types.ts includes a bridgingId: string field along with other deposit parameters like inputTokenAddress, outputTokenAddress, inputAmount, outputAmount, owner, quoteTimestamp, fillDeadline, recipient, sourceChainId, and destinationChainId.
Applied to files:
packages/bridging/src/BridgingSdk/tokenPriority.tspackages/bridging/src/BridgingSdk/determineIntermediateToken.test.tspackages/bridging/src/BridgingSdk/determineIntermediateToken.tspackages/bridging/src/BridgingSdk/getIntermediateSwapResult.tspackages/bridging/src/README.md
📚 Learning: 2025-08-12T09:15:28.459Z
Learnt from: alfetopito
Repo: cowprotocol/cow-sdk PR: 391
File: src/trading/getEthFlowTransaction.ts:80-85
Timestamp: 2025-08-12T09:15:28.459Z
Learning: In the CoW SDK codebase, ETH_FLOW_ADDRESSES and BARN_ETH_FLOW_ADDRESSES are typed as Record<SupportedChainId, string>, which requires all SupportedChainId enum values to have corresponding string entries. TypeScript compilation will fail if any chainId is missing from these mappings, making runtime guards for missing addresses unnecessary in these specific cases.
Applied to files:
packages/bridging/src/BridgingSdk/tokenPriority.ts
📚 Learning: 2025-11-06T11:26:21.337Z
Learnt from: shoom3301
Repo: cowprotocol/cow-sdk PR: 642
File: packages/bridging/src/BridgingSdk/BridgingSdk.ts:124-144
Timestamp: 2025-11-06T11:26:21.337Z
Learning: In the cow-sdk bridging module (packages/bridging/), all bridge providers return ChainInfo objects that are singleton instances imported from cowprotocol/sdk-config. The chain objects (mainnet, arbitrumOne, base, optimism, polygon, avalanche, gnosisChain, etc.) are exported as constants from packages/config/src/chains/details/ and are shared across all providers. This singleton pattern makes object reference comparison (e.g., with Array.includes()) safe for deduplicating networks returned by multiple providers, as the same chain will always be represented by the same object reference.
<!--
Applied to files:
packages/bridging/src/BridgingSdk/tokenPriority.tspackages/bridging/src/BridgingSdk/determineIntermediateToken.test.tspackages/bridging/src/README.md
📚 Learning: 2025-03-19T22:06:52.139Z
Learnt from: anxolin
Repo: cowprotocol/cow-sdk PR: 251
File: src/bridging/BridgingSdk/getQuoteWithBridging.ts:34-35
Timestamp: 2025-03-19T22:06:52.139Z
Learning: The implementation in the Across bridge provider deliberately selects only the first intermediate token from the list (`intermediateTokens[0]`) as a simplification for the initial implementation, with potential for future enhancement.
Applied to files:
packages/bridging/src/BridgingSdk/determineIntermediateToken.test.tspackages/bridging/src/BridgingSdk/determineIntermediateToken.tspackages/bridging/src/BridgingSdk/getIntermediateSwapResult.tspackages/bridging/src/README.md
📚 Learning: 2025-03-20T09:45:27.965Z
Learnt from: anxolin
Repo: cowprotocol/cow-sdk PR: 251
File: src/bridging/providers/across/AcrossBridgeProvider.ts:85-100
Timestamp: 2025-03-20T09:45:27.965Z
Learning: In the AcrossBridgeProvider's getIntermediateTokens method, not finding an intermediate token is expected behavior in some cases and should not be logged as a warning.
Applied to files:
packages/bridging/src/BridgingSdk/determineIntermediateToken.tspackages/bridging/src/BridgingSdk/getIntermediateSwapResult.ts
🧬 Code graph analysis (4)
packages/bridging/src/BridgingSdk/tokenPriority.ts (1)
packages/config/src/constants/tokens.ts (1)
NATIVE_CURRENCY_ADDRESS(4-4)
packages/bridging/src/BridgingSdk/determineIntermediateToken.test.ts (4)
packages/config/src/types/tokens.ts (1)
TokenInfo(6-14)packages/config/src/constants/tokens.ts (1)
NATIVE_CURRENCY_ADDRESS(4-4)packages/bridging/src/BridgingSdk/determineIntermediateToken.ts (1)
determineIntermediateToken(28-77)packages/bridging/src/errors.ts (1)
BridgeProviderQuoteError(13-21)
packages/bridging/src/BridgingSdk/determineIntermediateToken.ts (3)
packages/config/src/types/tokens.ts (1)
TokenInfo(6-14)packages/bridging/src/errors.ts (1)
BridgeProviderQuoteError(13-21)packages/bridging/src/BridgingSdk/tokenPriority.ts (3)
isHighPriorityToken(46-51)isCorrelatedToken(56-58)isNativeToken(63-65)
packages/bridging/src/BridgingSdk/getIntermediateSwapResult.ts (2)
packages/bridging/src/BridgingSdk/mock/bridgeRequestMocks.ts (1)
intermediateToken(25-25)packages/bridging/src/BridgingSdk/determineIntermediateToken.ts (1)
determineIntermediateToken(28-77)
🪛 markdownlint-cli2 (0.18.1)
packages/bridging/src/README.md
86-86: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
87-87: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
92-92: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
93-93: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
98-98: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
99-99: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
104-104: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
105-105: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (8)
packages/trading/src/types.ts (1)
151-152: LGTM!The new optional callback follows the existing pattern established by
getSlippageSuggestionand integrates cleanly with the bridging module'sdetermineIntermediateTokenfunction.packages/bridging/src/BridgingSdk/getIntermediateSwapResult.ts (1)
66-71: LGTM! Good enhancement over the previous first-token selection.The integration with
determineIntermediateTokenproperly passes the source chain ID and optional correlated tokens callback. Based on learnings, this replaces the previous deliberate simplification of selecting only the first token.packages/bridging/src/BridgingSdk/determineIntermediateToken.test.ts (1)
142-155: Good edge case coverage.The test correctly verifies that when
getCorrelatedTokensfails, the function gracefully falls back to basic priority and returns the first token (since bothrandomTokenandwethMainnethave LOW priority without the correlated tokens callback succeeding).packages/bridging/src/README.md (1)
42-82: Well-documented priority system.The documentation clearly explains the two-step cross-chain swap process and the 4-tier priority system. The descriptions align with the implementation in
determineIntermediateToken.ts.packages/bridging/src/BridgingSdk/tokenPriority.ts (2)
46-58: LGTM! Clean helper functions with consistent case handling.All functions properly normalize addresses to lowercase before comparison, ensuring case-insensitive matching.
7-41: No changes needed—OPTIMISM is intentionally excluded from HIGH_PRIORITY_TOKENS.OPTIMISM (chain ID 10) is defined in the
AdditionalTargetChainIdenum, notSupportedChainId. TheHIGH_PRIORITY_TOKENSobject is typed asPartial<Record<SupportedChainId, Set<string>>>, which means only chains in theSupportedChainIdenum are supported. This is by design:AdditionalTargetChainIdcontains bridging-only chains that are not part of the primary supported chains, so they cannot have entries inHIGH_PRIORITY_TOKENS.packages/bridging/src/BridgingSdk/determineIntermediateToken.ts (2)
8-13: LGTM!The enum values enable straightforward numeric sorting, and keeping it unexported maintains good encapsulation.
47-59: The invariant holds across all bridge provider implementations—intermediate tokens are always sourced fromsellTokenChainId.Line 48 uses
token.chainIdbecause all bridge providers (NearIntentsBridgeProvider, AcrossBridgeProvider, BungeeBridgeProvider) explicitly filter or organize returned tokens bysellTokenChainId. For example, NearIntentsBridgeProvider filters tokens wheretoken.chainId === sellTokenChainId, and AcrossBridgeProvider organizes its state bysupportedTokensState[sellTokenChainId]. This ensurestoken.chainIdalways equalssourceChainIdat this point. However, documenting this invariant or adding an explicit assertion would improve clarity.
| HIGHEST = 4, // USDC/USDT from hardcoded registry | ||
| HIGH = 3, // Tokens in CMS correlated tokens list | ||
| MEDIUM = 2, // Blockchain native token | ||
| LOW = 1, // Other 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.
This priority is inverted with the description. (Highest is 1)
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.
Sorry, I don't really see a problem, what do you mean?
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.

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:
🥇 Priority 1 - HIGHEST: Stablecoins (USDC/USDT)
🥈 Priority 2 - HIGH: Correlated Tokens
🥉 Priority 3 - MEDIUM: Native Chain Tokens
0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE📊 Priority 4 - LOW: Other Tokens
Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.