-
Notifications
You must be signed in to change notification settings - Fork 616
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
feat: query max amountIn based on max ticks willing to traverse #5889
Changes from 2 commits
1d233f5
16ca275
b2f0a37
01fd5bd
50cfbec
f719b04
3287b44
e78d4a8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -808,3 +808,133 @@ func edgeCaseInequalityBasedOnSwapStrategy(isZeroForOne bool, nextInitializedTic | |
} | ||
return nextInitializedTickSqrtPrice.LT(computedSqrtPrice) | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note to reviewer, |
||
// Returns the maximum amount of the tokenInDenom that can be swapped into the pool | ||
// to swap through all the liquidity from the current tick through the maxTicksCrossed tick, | ||
// but not exceed it. | ||
// | ||
// The protorev use case for this will be to call this method to X number of ticks based | ||
// on what is acceptable gas-wise and use the maxTokenIn as the upper bound for the binary | ||
// search if it's less than the default protorev binary search upper bound. This will then | ||
// allow the module to intelligently limit the gas-usage and compute time for assessing the | ||
// backrunning opportunities of routes that contain a concentrated liquidity pool(s). | ||
func (k Keeper) ComputeMaxInAmtGivenMaxTicksCrossed( | ||
ctx sdk.Context, | ||
poolId uint64, | ||
tokenInDenom string, | ||
maxTicksCrossed uint64, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just curious: since tick liquidity is subject to change, how is protorev going to bound the max tick a swap is going to cross? Is it something like X swap can cross upto 100 ticks given the gas to cross that 100 ticks is <50M I originally thought this function was going to be given X amt estimate how many ticks it will cross, but if this works then it's all g. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @stackman27 I previously thought we wanted "Given X amount, how many ticks will it cross?" but since Protorev needs to bound itself from consuming too much gas and compute time, and it seems to me liquidity doesn't really impact that, but number of ticks crossed does, so it cares more about the ticks crossed. If a method was made that given X amount how many ticks will it cross, and we put in $1k to swap in and it says it'll cross 1000 ticks, that would be too much and then the next action would be "okay how do I reduce the number of ticks crossed to something acceptable", so it feels like just querying directly for the acceptable number of ticks crossed and always using that amountIn bound is better than continuously trying in different amounts to identify what amount passes an acceptable amount of ticks. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i see one last question, so in the current implementation how are you going to decide the maxTickCrossed value? are you going to predetermine that 50M or 100M gas can cross X amt of ticks and generate routes for swap? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the answer to this is crossing ticks should cost a fixed amt of gas so yes, your description should be accurate (but admittedly I just made this based of Skip's design requirements and treated the actual use of the API as a black box) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @stackman27 @czarcas7ic Yes exactly. The plan is to make X number of ticks be a changeable variable (similar to pool points), and change that depending on how many CL pools we want to be able to use in backrunning. That is, if many new CL pools launch and almost all swaps are on CL pools, then we'd want to decrease the max number of ticks crossed per pool so that we can check 2-4 routes per swap without running out of the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sounds good, would love to review the PR when its ready too |
||
) (maxTokenIn, resultingTokenOut sdk.Coin, err error) { | ||
p, err := k.getPoolForSwap(ctx, poolId) | ||
if err != nil { | ||
return sdk.Coin{}, sdk.Coin{}, err | ||
} | ||
|
||
tokenOutDenom := "" | ||
|
||
if tokenInDenom != p.GetToken0() && tokenInDenom != p.GetToken1() { | ||
return sdk.Coin{}, sdk.Coin{}, types.TokenInDenomNotInPoolError{TokenInDenom: tokenInDenom} | ||
} | ||
|
||
if tokenInDenom == p.GetToken0() { | ||
tokenOutDenom = p.GetToken1() | ||
} else { | ||
tokenOutDenom = p.GetToken0() | ||
} | ||
|
||
swapStrategy, _, err := k.setupSwapStrategy(p, p.GetSpreadFactor(ctx), tokenInDenom, sdk.ZeroDec()) | ||
if err != nil { | ||
return sdk.Coin{}, sdk.Coin{}, err | ||
} | ||
|
||
// Initialize swap state | ||
swapState := newSwapState(types.MaxSpotPrice.RoundInt(), p, swapStrategy) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we use types.MaxSpotPrice? Do we decrement it until maxTicksCrossed ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah we basically never want the reason we stop an estimation to be due to a lack of funds, it should only ever strictly be due to the limit imposed by amt of ticks willing to traverse. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. in that case would TotalPoolLiquidity not be a good value to use? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I think that makes sense too, I can change it to that |
||
|
||
nextInitTickIter := swapStrategy.InitializeNextTickIterator(ctx, poolId, swapState.tick) | ||
defer nextInitTickIter.Close() | ||
|
||
fmt.Println("currTick", p.GetCurrentTick()) | ||
totalTokenOut := sdk.ZeroDec() | ||
|
||
for i := uint64(0); i < maxTicksCrossed; i++ { | ||
sqrtPriceStart := swapState.sqrtPrice | ||
// Iterator must be valid to be able to retrieve the next tick from it below. | ||
if !nextInitTickIter.Valid() { | ||
break | ||
} | ||
|
||
nextInitializedTick, err := types.TickIndexFromBytes(nextInitTickIter.Key()) | ||
if err != nil { | ||
return sdk.Coin{}, sdk.Coin{}, err | ||
} | ||
|
||
fmt.Println("nextInitializedTick", nextInitializedTick) | ||
|
||
_, nextInitializedTickSqrtPrice, err := math.TickToSqrtPrice(nextInitializedTick) | ||
if err != nil { | ||
return sdk.Coin{}, sdk.Coin{}, types.TickToSqrtPriceConversionError{NextTick: nextInitializedTick} | ||
} | ||
|
||
sqrtPriceTarget := swapStrategy.GetSqrtTargetPrice(nextInitializedTickSqrtPrice) | ||
|
||
computedSqrtPrice, amountOut, amountIn, spreadRewardChargeTotal := swapStrategy.ComputeSwapWithinBucketInGivenOut( | ||
swapState.sqrtPrice, | ||
sqrtPriceTarget, | ||
swapState.liquidity, | ||
swapState.amountSpecifiedRemaining, | ||
) | ||
|
||
fmt.Println("amountOut", amountOut) | ||
fmt.Println("amountIn", amountIn) | ||
|
||
swapState.sqrtPrice = computedSqrtPrice | ||
swapState.amountSpecifiedRemaining.SubMut(amountOut) | ||
swapState.amountCalculated.AddMut(amountIn.Add(spreadRewardChargeTotal)) | ||
|
||
totalTokenOut = totalTokenOut.Add(amountOut) | ||
|
||
nextInitializedTickSqrtPriceBigDec := osmomath.BigDecFromSDKDec(nextInitializedTickSqrtPrice) | ||
|
||
// We do not need to track spread rewards or uptime accums here since we are not actually swapping. | ||
if nextInitializedTickSqrtPriceBigDec.Equal(computedSqrtPrice) { | ||
nextInitializedTickInfo, err := ParseTickFromBz(nextInitTickIter.Value()) | ||
if err != nil { | ||
return sdk.Coin{}, sdk.Coin{}, err | ||
} | ||
liquidityNet := nextInitializedTickInfo.LiquidityNet | ||
// Move next tick iterator to the next tick as the tick is crossed. | ||
nextInitTickIter.Next() | ||
|
||
liquidityNet = swapState.swapStrategy.SetLiquidityDeltaSign(liquidityNet) | ||
// Update the swapState's liquidity with the new tick's liquidity | ||
swapState.liquidity.AddMut(liquidityNet) | ||
|
||
// Update the swapState's tick with the tick we retrieved liquidity from | ||
swapState.tick = swapStrategy.UpdateTickAfterCrossing(nextInitializedTick) | ||
} else if edgeCaseInequalityBasedOnSwapStrategy(swapStrategy.ZeroForOne(), nextInitializedTickSqrtPriceBigDec, computedSqrtPrice) { | ||
// If, based on the swap strategy, the computedSqrtPrice matches the edge case inequality, we return an error. | ||
// This is an edge case that occurs when swapping at/near tick boundaries that will be fixed in the next release. | ||
// For now, we return an error and ask the user to try again with a different swap amount. | ||
return sdk.Coin{}, sdk.Coin{}, types.ComputedSqrtPriceInequalityError{IsZeroForOne: swapStrategy.ZeroForOne(), ComputedSqrtPrice: computedSqrtPrice, NextInitializedTickSqrtPrice: nextInitializedTickSqrtPriceBigDec} | ||
} else if !sqrtPriceStart.Equal(computedSqrtPrice) { | ||
// Otherwise if the sqrtPrice calculated from ComputeSwapWithinBucketOutGivenIn(...) does not equal the sqrtPriceStart we started with at the | ||
// beginning of this iteration, we set the swapState tick to the corresponding tick of the computedSqrtPrice calculated from ComputeSwapWithinBucketOutGivenIn(...) | ||
newTick, err := math.CalculateSqrtPriceToTick(computedSqrtPrice) | ||
if err != nil { | ||
return sdk.Coin{}, sdk.Coin{}, err | ||
} | ||
swapState.tick = newTick | ||
} | ||
|
||
// If nothing was consumed from swapState.amountSpecifiedRemaining, we break the loop early. | ||
if amountOut.IsZero() { | ||
break | ||
} | ||
} | ||
|
||
maxAmt := swapState.amountCalculated.Ceil().TruncateInt() | ||
maxTokenIn = sdk.NewCoin(tokenInDenom, maxAmt) | ||
|
||
resultingTokenOut = sdk.NewCoin(tokenOutDenom, totalTokenOut.TruncateInt()) | ||
|
||
return maxTokenIn, resultingTokenOut, nil | ||
} |
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.
drive by lint (unchecked error)