-
Notifications
You must be signed in to change notification settings - Fork 64
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
fix(router-sdk): Address wrapped/native currency conditions in mixed routes with v4 #81
Conversation
Graphite Automations"Request reviewers once CI passes on sdks monorepo" took an action on this PR • (08/28/24)1 reviewer was added and 1 assignee was added to this PR based on 's automation. |
type TPool = Pair | V3Pool | V4Pool | ||
|
||
export function isValidTokenPath(prevPool: TPool, currentPool: TPool, inputToken: Currency): boolean { | ||
if (currentPool.involvesToken(inputToken as Token)) return true |
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 should always be true if inputToken is not native right, a comment could be helpful
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.
I have to put as Token
bc v2/v3 pools only accept token parameters..so I'm basically hacking typescript so it will compile. this will work if the inputToken IS native as type Currency too....
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.
I kind of want to go into v2/v3-sdks change these inputs to Currency
and then throw an error if it's not type Token
(a subset of Currency
) then I can remove all of the as Token
's from this sdk
@@ -4,6 +4,7 @@ import { Currency, Price, Token } from '@uniswap/sdk-core' | |||
import { Pair } from '@uniswap/v2-sdk' | |||
import { Pool as V3Pool } from '@uniswap/v3-sdk' | |||
import { Pool as V4Pool } from '@uniswap/v4-sdk' | |||
import { isValidTokenPath } from '../../utils/isValidTokenPath' | |||
|
|||
type TPool = Pair | V3Pool | V4Pool |
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.
nit: as I was coding v4 routing in SOR, I realized getting this type exported can be beneficial
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.
yah I also need to DRY and put this somewhere, I keep meaning to do taht
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.
I'm afraid of needing new approvals, so I'm just gonna merge..
Description
fixes #67
Valid Routes:
v2/v3 --> v4 WETH unwraps to ETH
v4 --> v2/v3 ETH wraps to WETH
Invalid Routes:
v4 --> v4 WETH unwraps to ETH
v4 --> v4 ETH wraps to WETH
v4 routes must trade through WETH/ETH pools to make this conversion
How Has This Been Tested?
unit tests
Are there any breaking changes?
I don't think so
(Optional) Feedback Focus
make sure all conditions are met.
(Optional) Follow Ups