feat(v4-sdk): Create and Add Implementation #98
Conversation
…ion-createCallParameters
| import { ADDRESS_ZERO } from '../internalConstants' | ||
|
|
||
| // Uniswap v4 supports native pools. Those currencies are represented by the zero address. | ||
| // TODO: Figure out if this is how we should be handling weird edge case tokens like CELO/Polygon/etc.. |
There was a problem hiding this comment.
I believe this has been resolved and that any Currency's isNative value will be set according to how it should be "treated" in the interface
Graphite Automations"Request reviewers once CI passes on sdks monorepo" took an action on this PR • (09/18/24)1 reviewer was added and 1 assignee was added to this PR based on 's automation. |
sdks/v4-sdk/src/entities/position.ts
Outdated
| } | ||
|
|
||
| /** | ||
| * Returns the amount of token0 that this position's liquidity could be burned for at the current pool price | ||
| */ | ||
| public get amount0(): CurrencyAmount<Currency> { | ||
| throw new Error('Not implemented') | ||
| if (this._token0Amount === null) { |
There was a problem hiding this comment.
if (!this._token0Amount) will include the case it's undefined
There was a problem hiding this comment.
oh got it cool! updated. out of curiousity, could it ever be set to undefined instead of null ?
There was a problem hiding this comment.
Oh no, you're right.. I see it being initialized to null.
| this.pool.currency0, | ||
| SqrtPriceMath.getAmount0Delta( | ||
| TickMath.getSqrtRatioAtTick(this.tickLower), | ||
| TickMath.getSqrtRatioAtTick(this.tickUpper), |
There was a problem hiding this comment.
just double checking.. this should be tickLower and tickUpper if it's less than the tickLower?
There was a problem hiding this comment.
yep! good question.
So on L70 we check where the tickCurrent is.
If the tickCurrent is below the tickLower (tickCurrent < tickLower) that means the full range of liquidity is above the current price. I think about it like this in my head:
__________|______________|||||||__
^ ^ ^
__________tC____________tL_____tU
Anything below(to the left) of tC (tickCurrent) is going to be entirely in amount1 and anything above (to the right) of tickCurrent is going to be entirely in amount0.
Since this function wants to know what the amount0 is, then in this case we need to query for the amount between the tickLower and the tickUpper. Hopefully that makes sense!
There was a problem hiding this comment.
I don't think I fully understand. If amount0 is everything to the right of tC, wouldn't it be between tC and tL?
There was a problem hiding this comment.
yeah so everything to the right of tickCurrent is ALL of amount0 on the pool, since we are just working with a specific position (who is defined from tL to tU) we just care about the amount0 between tL and tU.
There was a problem hiding this comment.
if we wanted to know amount1 (where the state of the pool is the same in the above graph.. ie the position is entirely above the current tick) then amount1 = 0 since the position does not touch any of amount1. And you can verify this logic in the amount1() function call
| let nativeCurrency: Currency = position.pool.currency0.isNative | ||
| ? position.pool.currency0 | ||
| : position.pool.currency1 | ||
| value = position.pool.currency0.isNative ? toHex(amount0Max) : toHex(amount1Max) |
There was a problem hiding this comment.
Wanted to confirm by comparing to V3. Seems we are choosing the upper bound of slippage of the tokens rather than the inputed amount when 1 of the tokens is Native, is this desired?
There was a problem hiding this comment.
what do you mean rather than inputtedAmount? Can you link me to the line in v3?
There was a problem hiding this comment.
Yea was referring to this
We use amountDesired (inputtedAmount) instead of amountMax
There was a problem hiding this comment.
I think this is desired behavior because unlike in v3 we bound increasing positions by the maximum tokens required, so we need to send the MAX, and sweep any excess back to the user. This allows the price to slip "up" to your slippage.
| @@ -0,0 +1,524 @@ | |||
| // TODO: import this from npm | |||
| export const abi = [ | |||
There was a problem hiding this comment.
Ahh is this ABI currently on Sepolia?
There was a problem hiding this comment.
i think this is the most up to date abi yes, but let me know if you see discrepancies!
| return calldatas.length === 1 ? calldatas[0] : Multicall.INTERFACE.encodeFunctionData('multicall', [calldatas]) | ||
| return calldataList.length === 1 | ||
| ? calldataList[0] | ||
| : Multicall.INTERFACE.encodeFunctionData('multicall', [calldataList]) |
There was a problem hiding this comment.
Since calldataList is already an array, do we need to wrap it again in an array?
There was a problem hiding this comment.
because the mutlicall signature requires [], and the underlying functions being called in periphery require a list of actions (calldataList)
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
This PR will just implement
createCallParametersandaddCallParametersto reduce the size of the PR. All permit2 forwarding is also out of scope in this PR.Note that @zhongeric has started the impl for remove and collect but I will be removing them from this PR and finalizing those implementations in a different PR.
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]