fix(router-sdk): encodeMixedRouteToPath resolves the wrong pool token at a native/wrapped boundary - #716
Open
gomesalexandre wants to merge 1 commit into
Conversation
encodeMixedRouteToPath re-derived each hop's output token via currencyIn.equals(pool.token0) ? pool.token1 : pool.token0, which silently falls through to pool.token0 whenever neither pool token matches currencyIn by exact reference -- exactly what happens at a native/wrapped currency boundary, where MixedRouteSDK's constructor resolves the correct side via wrapped-equality rather than .equals(). Both branches (v4 and the legacy non-v4 fallback) now read each hop's currency directly from route.path, which the constructor already resolves correctly, mirroring the fix already applied to midPrice and getOutputOfPools in Uniswap#706 (ROUTE-886). Reproduced the divergence with a real route (USDC -[V4]-> ETH -[V3, WETH<->USDT]-> USDT): unfixed code encoded a path terminating at WETH's address instead of USDT's -- a genuine self-loop / wrong terminal token, not just a display nit. The existing test fixtures never caught this because every non-weth token address in them happens to sort below WETH's, which makes the fallthrough coincidentally correct; the new tests use a token that sorts above WETH's address specifically to break that coincidence. All 20 pre-existing fixtures remain byte-identical. Confirmed real red-before/green-after via git stash on both the source fix and the new regression tests.
gomesalexandre
marked this pull request as ready for review
September 1, 2026 21:32
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
encodeMixedRouteToPathre-derives each hop's output token via:This silently falls through to
pool.token0whenevercurrencyIndoesn't exactly matchpool.token0(by reference/.equals()) — which is exactly what happens at a native/wrapped currency boundary, whereMixedRouteSDK's constructor resolves the correct side via a wrapped-equality fallback (inputToken.wrapped.equals(pool.token0)etc.), not plain.equals().Both branches (the v4 branch and the legacy non-v4 fallback) had this same pattern. Both now read each hop's currency directly from
route.path, which the constructor already resolves correctly — this mirrors the fix already applied toMixedRouteSDK.midPriceandgetOutputOfPoolsin #706 (ROUTE-886), merged two days before I found this. That PR's own comment states the invariant this fix relies on:route.path[i + 1]is always pooli's own currency object.Why the existing 20 fixtures never caught it
Every non-WETH token address used in the existing test fixtures (
0x...0001,0x...0002,0x...0003) happens to sort below WETH's real address (0xC02aaa39...). Since there are only two tokens per pool, and the fallthrough always returnspool.token0, whenever the "other" token happens to betoken0(i.e. sorts below WETH), the fallthrough is coincidentally correct. It's only wrong when the non-WETH token sorts above WETH's address — none of the existing fixtures do.Repro
Confirmed via
git stashthat this is a genuine red-before/green-after — the encoded path silently pointed at the wrong terminal token (a WETH/WETH self-loop in the legacy-branch case tested), not a display nit.Consumers
Within this monorepo,
encodeMixedRouteToPathis used directly by:universal-router-sdk/src/entities/actions/uniswap.ts— building the actual swap command'spathargumentrouter-sdk/src/swapRouter.ts— the legacy swap-router path builderSo this isn't just a latent/unused public export — it's wired into real swap-calldata construction inside the same monorepo. I did not check external consumers (e.g.
smart-order-router, a separate repo) for how they construct routes that reach this encoder, so I can't speak to how often a real multihop route crosses a native/wrapped boundary at a non-default token ordering in production. What's proven here is the code-level defect and its consequence when it fires: the built swap path targets the wrong pool/token, which would either revert (no such pool) or, worse, route through an unintended pool.Testing
git stash.bun test).tsc --noEmitclean.bun run lintcurrently fails on unmodifiedmaintoo (ESLint: ... all files matching the glob are ignored) — confirmed viagit stashthat this is a pre-existing repo/tooling issue, not something this PR introduced or needs to fix.Codex review
Ran Codex adversarially (synchronous, own tracked PID). It did not return a verdict within ~7 minutes — killed the tracked PID cleanly (confirmed no lingering process) and did a thorough self-review instead, covering: semantic correctness of both branches (verified via the real repro above), whether
route.pathcan ever have an unexpected length relative toroute.pools(no — the constructor always pushes exactly one entry per pool after the initialpathInput, soroute.path.length === route.pools.length + 1by construction, androute.path[i+1]is always in bounds), whether the new tests are non-vacuous (confirmed via stash-based red-before/green-after), and whether any existing behavior regressed (confirmed via the full 374-test suite and byte-identical existing fixtures).Draft opened and flipped to ready per my usual workflow — real command output above, not descriptions.