feat: mixed route support native currency routing based on whether v4 pool is involved #716
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
| ( | ||
| token: Currency, | ||
| tokenValidation: TokenValidationResult | undefined | ||
| ): boolean => { |
There was a problem hiding this comment.
this seems identical with inline method at line 187. Maybe create a common function?
| metric.putMetric( | ||
| 'MixedGetRoutesLoad', | ||
| Date.now() - beforeGetRoutes, | ||
| MetricLoggerUnit.Milliseconds | ||
| ); | ||
|
|
||
| return { | ||
| routes: routes, | ||
| routes: routes.concat(v2V3OnlyRoutes), |
There was a problem hiding this comment.
could those contain dups? if so, is that a problem later on?
There was a problem hiding this comment.
meanwhile dup is not a problem, because best-swap-route will select a single best route, it's better if we do dedup here. I will think about how to do it here.
jsy1218
force-pushed
the
siyujiang/route-290-top-level-alpha-router-native-currencyin-currencyin
branch
from
c745fdc
to
f9f1901
Compare
jsy1218
force-pushed
the
siyujiang/route-291-special-treat-mixed-route-native-currency-routing-only-when
branch
3 times, most recently
from
5a3f322
to
235e884
Compare
jsy1218
changed the base branch from
siyujiang/route-290-top-level-alpha-router-native-currencyin-currencyin
to
graphite-base/716
jsy1218
force-pushed
the
siyujiang/route-291-special-treat-mixed-route-native-currency-routing-only-when
branch
from
235e884
to
46e4a3e
Compare
jsy1218
force-pushed
the
graphite-base/716
branch
from
929a331
to
101cf3d
Compare
… pool is involved
jsy1218
force-pushed
the
siyujiang/route-291-special-treat-mixed-route-native-currency-routing-only-when
branch
from
46e4a3e
to
35d8213
Compare
Graphite Automations"Request reviewers once CI passes on smart-order-router repo" took an action on this PR • (09/24/24)4 reviewers were added and 1 assignee was added to this PR based on 's automation. |
|
After spoke to Alice, I realized this PR is not desirable. Mixed route can have a situation, where some are v2/v3 route with WETH input, meanwhile some are v4 route with ETH input. This is extremely difficult for universal-router-sdk to handle well. I'm not going to optimize for the case, where ETH -> X best quote is a mixed route, where v4 is ETH -> X, meanwhile v2/v3 is WETH -> X. But rather, I will keep the existing mixed routing logic, which is v4 returns WETH -> X, and v2/v3 returns WETH -> X. universal-router-sdk will understand to always wrap input, given mixed routing. This means close this PR, and meanwhile the existing logic as well. |
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
feature
What is the current behavior? (You can also link to an open issue here)
mixed route cannot support non-native currency routing for v2 and v3 only.
What is the new behavior (if this is a feature change)?
we will be able to support non-native currency routing for v2 and v3 only.
Other information:
this is for backward compatibility with existing mixed routing against v2 and v3