fix(universal-router-sdk): handle ETH/WETH output in V4 split routes #462
Conversation
🤖 Claude Code Review
SummaryThis PR fixes handling of split routes when mixing V4 and non-V4 (V2/V3) swaps for ETH output. The issue is that V4 can handle native ETH directly, but V2/V3 require WETH. When splitting trades across both, you need consistent handling - this PR forces V4 to take WETH instead of ETH when there's a split route, then performs a final unwrap. Key Changes
AnalysisThe logic is correct. The ETH-WETH pool detection at lines 443-445 checks if one currency is native and the other is its wrapped equivalent: (lastPool.currency0.isNative && lastPool.currency1.wrapped.equals(lastPool.currency0.wrapped)) ||
(lastPool.currency1.isNative && lastPool.currency0.wrapped.equals(lastPool.currency1.wrapped))This correctly identifies pools like ETH/WETH where swapping direction doesn't matter for the output token type. The caching of The Forge tests verify the end-to-end behavior: recipient receives ETH (not WETH), router balance ends at 0, and USDC is correctly deducted. No issues found. 💡 Want a fresh review? Add a comment containing |
github-actions
bot
changed the title
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
📋 Review verdict: APPROVE
👆 The main review comment above is the source of truth for this PR review. It is automatically updated on each review cycle, so always refer to it for the most current feedback.
This formal review submission is for the verdict only.
Graphite Automations"Request reviewers once CI passes on sdks monorepo" took an action on this PR • (12/15/25)2 reviewers were added and 1 assignee was added to this PR based on Siyu Jiang (See-You John)'s automation. |
🤖 Claude Code ReviewStatus: The review was completed but there was an error posting inline comments. Raw Review OutputPlease check the workflow logs for details. |
🤖 Claude Code ReviewStatus: ❌ Error
|
🤖 Claude Code ReviewStatus: The review was completed but there was an error posting inline comments. Raw Review OutputPlease check the workflow logs for details. |
🤖 Claude Code ReviewStatus: ❌ Error
|
🤖 Claude Code ReviewStatus: The review was completed but there was an error posting inline comments. Raw Review OutputPlease check the workflow logs for details. |
🤖 Claude Code ReviewStatus: ❌ Error
|
| /** | ||
| * Checks if any route in a split trade ends with an ETH-WETH pool. | ||
| * | ||
| * When a split trade has routes ending in ETH-WETH pools alongside routes ending in WETH, |
There was a problem hiding this comment.
does this only affect pools ending with ETH-WETH or can it be any *-ETH pool? cc @jsy1218
There was a problem hiding this comment.
also curious if this change covers both exactIn and exactOut
There was a problem hiding this comment.
i think it should cover both exactIn and exactOut, because this change covers the tokenOut itself, and the exactIn and exactOut should have tokenOutput regardless. @dianakocsis to confirm
There was a problem hiding this comment.
good catch - seems like routes ending in ETH-WETH always fail for exactOut - fixing this now
|
@dianakocsis i think i prefer to have #476 merged first, then #462 should include as well |
🤖 Claude Code ReviewStatus: The review was completed but there was an error posting inline comments. Raw Review OutputPlease check the workflow logs for details. |
🤖 Claude Code ReviewStatus: ❌ Error
|
🤖 Claude Code ReviewStatus: The review was completed but there was an error posting inline comments. Raw Review OutputPlease check the workflow logs for details. |
🤖 Claude Code ReviewStatus: ❌ Error
|
PR Scope
Please title your PR according to the following types and scopes following conventional commits:
fix(SDK name):will trigger a patch versionchore(<type>):will not trigger any release and should be used for internal repo changes<type>(public):will trigger a patch version for non-code changes (e.g. README changes)feat(SDK name):will trigger a minor versionfeat(breaking):will trigger a major version for a breaking changeDescription
[Summary of the change, motivation, and context]
How Has This Been Tested?
[e.g. Manually, E2E tests, unit tests, Storybook]
Are there any breaking changes?
[e.g. Type definitions, API definitions]
If there are breaking changes, please ensure you bump the major version Bump the major version (by using the title
feat(breaking): ...), post a notice in #eng-sdks, and explicitly notify all Uniswap Labs consumers of the SDK.(Optional) Feedback Focus
[Specific parts of this PR you'd like feedback on, or that reviewers should pay closer attention to]
(Optional) Follow Ups
[Things that weren't addressed in this PR, ways you plan to build on this work, or other ways this work could be extended]
✨ Claude-Generated Content
Description
Fixes an issue where split routes containing both V4 and non-V4 (V2/V3) swaps would fail when the output currency is ETH. V4 pools handle ETH natively, but V2/V3 pools use WETH. When both route types are combined in a split trade, the V4 route must take WETH instead of ETH to ensure proper unwrapping at the end.
Changes
shouldForceV4UnwrapForSplitNativeOutputgetter toUniswapTradeclass to detect if any V4 route (including mixed routes) ends with an ETH-WETH pooloutputRequiresUnwrapto return true when output is native ETH and any V4 route has an ETH-WETH last pooladdV4Swapto accept ashouldForceV4UnwrapForSplitNativeOutputparameter and use WETH for the take operation when this condition is trueaddMixedSwapto pass the flag through and use WETH for the final take in mixed routes when applicableinterop.jsonHow Has This Been Tested?
Unit tests and Forge tests added for the following scenarios:
Are there any breaking changes?
No breaking changes. This is a bug fix that improves handling of edge cases in split route swaps.