diff --git a/app/apptesting/concentrated_liquidity.go b/app/apptesting/concentrated_liquidity.go index 72de6f80f35..fca7071a817 100644 --- a/app/apptesting/concentrated_liquidity.go +++ b/app/apptesting/concentrated_liquidity.go @@ -1,10 +1,12 @@ package apptesting import ( + "fmt" "time" sdk "github.com/cosmos/cosmos-sdk/types" + clmath "github.com/osmosis-labs/osmosis/v16/x/concentrated-liquidity/math" clmodel "github.com/osmosis-labs/osmosis/v16/x/concentrated-liquidity/model" "github.com/osmosis-labs/osmosis/v16/x/concentrated-liquidity/types" @@ -141,3 +143,57 @@ func (s *KeeperTestHelper) SetupConcentratedLiquidityDenomsAndPoolCreation() { defaultParams.AuthorizedQuoteDenoms = append(defaultParams.AuthorizedQuoteDenoms, ETH, USDC, BAR, BAZ, FOO, UOSMO, STAKE) s.App.ConcentratedLiquidityKeeper.SetParams(s.Ctx, defaultParams) } + +// PriceToTickRoundDown takes a price and returns the corresponding tick index. +// If tickSpacing is provided, the tick index will be rounded down to the nearest multiple of tickSpacing. +// CONTRACT: tickSpacing must be smaller or equal to the max of 1 << 63 - 1. +// This is not a concern because we have authorized tick spacings that are smaller than this max, +// and we don't expect to ever require it to be this large. +func (s *KeeperTestHelper) PriceToTickRoundDownSpacing(price sdk.Dec, tickSpacing uint64) (int64, error) { + tickIndex, err := s.PriceToTick(price) + if err != nil { + return 0, err + } + + tickIndex, err = clmath.RoundDownTickToSpacing(tickIndex, int64(tickSpacing)) + if err != nil { + return 0, err + } + + return tickIndex, nil +} + +// CalculatePriceToTick takes in a price and returns the corresponding tick index. +// This function does not take into consideration tick spacing. +// NOTE: This is really returning a "Bucket index". Bucket index `b` corresponds to +// all prices in range [TickToSqrtPrice(b), TickToSqrtPrice(b+1)). +// We make an erroneous assumption here, that bucket index `b` corresponds to +// all prices in range [TickToPrice(b), TickToPrice(b+1)). +// This currently makes this function unsuitable for the state machine. +func (s *KeeperTestHelper) CalculatePriceToTick(price sdk.Dec) (tickIndex int64) { + // TODO: Make truncate, since this defines buckets as + // [TickToPrice(b - .5), TickToPrice(b+.5)) + return clmath.CalculatePriceToTickDec(price).RoundInt64() +} + +// PriceToTick takes a price and returns the corresponding tick index assuming +// tick spacing of 1. +func (s *KeeperTestHelper) PriceToTick(price sdk.Dec) (int64, error) { + if price.Equal(sdk.OneDec()) { + return 0, nil + } + + if price.IsNegative() { + return 0, fmt.Errorf("price must be greater than zero") + } + + if price.GT(types.MaxSpotPrice) || price.LT(types.MinSpotPrice) { + return 0, types.PriceBoundError{ProvidedPrice: price, MinSpotPrice: types.MinSpotPrice, MaxSpotPrice: types.MaxSpotPrice} + } + + // Determine the tick that corresponds to the price + // This does not take into account the tickSpacing + tickIndex := s.CalculatePriceToTick(price) + + return tickIndex, nil +} diff --git a/tests/cl-genesis-positions/convert.go b/tests/cl-genesis-positions/convert.go index fabde1e98a1..cd2cad53134 100644 --- a/tests/cl-genesis-positions/convert.go +++ b/tests/cl-genesis-positions/convert.go @@ -134,12 +134,20 @@ func ConvertSubgraphToOsmosisGenesis(positionCreatorAddresses []sdk.AccAddress, continue } - lowerTickOsmosis, err := math.PriceToTickRoundDown(lowerPrice, pool.GetTickSpacing()) + sqrtPriceLower, err := lowerPrice.ApproxRoot(2) + if err != nil { + panic(err) + } + lowerTickOsmosis, err := math.SqrtPriceToTickRoundDownSpacing(sqrtPriceLower, pool.GetTickSpacing()) if err != nil { panic(err) } - upperTickOsmosis, err := math.PriceToTickRoundDown(upperPrice, pool.GetTickSpacing()) + sqrtPriceUpper, err := upperPrice.ApproxRoot(2) + if err != nil { + panic(err) + } + upperTickOsmosis, err := math.SqrtPriceToTickRoundDownSpacing(sqrtPriceUpper, pool.GetTickSpacing()) if err != nil { panic(err) } diff --git a/x/concentrated-liquidity/keeper_test.go b/x/concentrated-liquidity/keeper_test.go index 85b5d878cc6..c5acd4f5a54 100644 --- a/x/concentrated-liquidity/keeper_test.go +++ b/x/concentrated-liquidity/keeper_test.go @@ -50,10 +50,15 @@ var ( FullRangeLiquidityAmt = sdk.MustNewDecFromStr("70710678.118654752940000000") DefaultTickSpacing = uint64(100) PoolCreationFee = poolmanagertypes.DefaultParams().PoolCreationFee - DefaultExponentConsecutivePositionLowerTick, _ = math.PriceToTickRoundDown(sdk.NewDec(5500), DefaultTickSpacing) - DefaultExponentConsecutivePositionUpperTick, _ = math.PriceToTickRoundDown(sdk.NewDec(6250), DefaultTickSpacing) - DefaultExponentOverlappingPositionLowerTick, _ = math.PriceToTickRoundDown(sdk.NewDec(4000), DefaultTickSpacing) - DefaultExponentOverlappingPositionUpperTick, _ = math.PriceToTickRoundDown(sdk.NewDec(4999), DefaultTickSpacing) + sqrt4000 = sdk.MustNewDecFromStr("63.245553203367586640") + sqrt4994 = sdk.MustNewDecFromStr("70.668238976219012614") + sqrt4999 = sdk.MustNewDecFromStr("70.703606697254136612") + sqrt5500 = sdk.MustNewDecFromStr("74.161984870956629487") + sqrt6250 = sdk.MustNewDecFromStr("79.056941504209483300") + DefaultExponentConsecutivePositionLowerTick, _ = math.SqrtPriceToTickRoundDownSpacing(sqrt5500, DefaultTickSpacing) + DefaultExponentConsecutivePositionUpperTick, _ = math.SqrtPriceToTickRoundDownSpacing(sqrt6250, DefaultTickSpacing) + DefaultExponentOverlappingPositionLowerTick, _ = math.SqrtPriceToTickRoundDownSpacing(sqrt4000, DefaultTickSpacing) + DefaultExponentOverlappingPositionUpperTick, _ = math.SqrtPriceToTickRoundDownSpacing(sqrt4999, DefaultTickSpacing) BAR = "bar" FOO = "foo" InsufficientFundsError = fmt.Errorf("insufficient funds") diff --git a/x/concentrated-liquidity/lp.go b/x/concentrated-liquidity/lp.go index eb5da97da7e..16905410421 100644 --- a/x/concentrated-liquidity/lp.go +++ b/x/concentrated-liquidity/lp.go @@ -66,13 +66,13 @@ func (k Keeper) createPosition(ctx sdk.Context, poolId uint64, owner sdk.AccAddr } // Transform the provided ticks into their corresponding sqrtPrices. - priceLowerTick, priceUpperTick, sqrtPriceLowerTick, sqrtPriceUpperTick, err := math.TicksToSqrtPrice(lowerTick, upperTick) + _, _, sqrtPriceLowerTick, sqrtPriceUpperTick, err := math.TicksToSqrtPrice(lowerTick, upperTick) if err != nil { return 0, sdk.Int{}, sdk.Int{}, sdk.Dec{}, 0, 0, err } // If multiple ticks can represent the same spot price, ensure we are using the largest of those ticks. - lowerTick, upperTick, err = roundTickToCanonicalPriceTick(lowerTick, upperTick, priceLowerTick, priceUpperTick, pool.GetTickSpacing()) + lowerTick, upperTick, err = roundTickToCanonicalPriceTick(lowerTick, upperTick, sqrtPriceLowerTick, sqrtPriceUpperTick, pool.GetTickSpacing()) if err != nil { return 0, sdk.Int{}, sdk.Int{}, sdk.Dec{}, 0, 0, err } @@ -473,7 +473,7 @@ func (k Keeper) initializeInitialPositionForPool(ctx sdk.Context, pool types.Con // Calculate the initial tick from the initial spot price // We round down here so that the tick is rounded to // the nearest possible value given the tick spacing. - initialTick, err := math.PriceToTickRoundDown(initialSpotPrice, pool.GetTickSpacing()) + initialTick, err := math.SqrtPriceToTickRoundDownSpacing(initialCurSqrtPrice, pool.GetTickSpacing()) if err != nil { return err } diff --git a/x/concentrated-liquidity/lp_test.go b/x/concentrated-liquidity/lp_test.go index 235985e4472..546afd146c3 100644 --- a/x/concentrated-liquidity/lp_test.go +++ b/x/concentrated-liquidity/lp_test.go @@ -1703,7 +1703,10 @@ func (s *KeeperTestSuite) TestInitializeInitialPositionForPool() { amount1Desired: sdk.NewInt(100_000_051), tickSpacing: 1, expectedCurrSqrtPrice: sqrt(100_000_051), - expectedTick: 72000001, + // We expect the returned tick to always be rounded down. + // In this case, tick 72000000 corresponds to 100_000_000, + // while 72000001 corresponds to 100_000_100. + expectedTick: 72000000, }, "error: amount0Desired is zero": { amount0Desired: sdk.ZeroInt(), diff --git a/x/concentrated-liquidity/math/tick.go b/x/concentrated-liquidity/math/tick.go index 20ac2d8e32b..4023929d5b0 100644 --- a/x/concentrated-liquidity/math/tick.go +++ b/x/concentrated-liquidity/math/tick.go @@ -1,6 +1,7 @@ package math import ( + "errors" "fmt" sdk "github.com/cosmos/cosmos-sdk/types" @@ -91,60 +92,44 @@ func TickToPrice(tickIndex int64) (price sdk.Dec, err error) { return price, nil } -// PriceToTick takes a price and returns the corresponding tick index assuming -// tick spacing of 1. -func PriceToTick(price sdk.Dec) (int64, error) { - if price.Equal(sdk.OneDec()) { - return 0, nil +// RoundDownTickToSpacing rounds the tick index down to the nearest tick spacing if the tickIndex is in between authorized tick values +// Note that this is Euclidean modulus. +// The difference from default Go modulus is that Go default results +// in a negative remainder when the dividend is negative. +// Consider example tickIndex = -17, tickSpacing = 10 +// tickIndexModulus = tickIndex % tickSpacing = -7 +// tickIndexModulus = -7 + 10 = 3 +// tickIndex = -17 - 3 = -20 +func RoundDownTickToSpacing(tickIndex int64, tickSpacing int64) (int64, error) { + tickIndexModulus := tickIndex % tickSpacing + if tickIndexModulus < 0 { + tickIndexModulus += tickSpacing } - if price.IsNegative() { - return 0, fmt.Errorf("price must be greater than zero") + if tickIndexModulus != 0 { + tickIndex = tickIndex - tickIndexModulus } - if price.GT(types.MaxSpotPrice) || price.LT(types.MinSpotPrice) { - return 0, types.PriceBoundError{ProvidedPrice: price, MinSpotPrice: types.MinSpotPrice, MaxSpotPrice: types.MaxSpotPrice} + // Defense-in-depth check to ensure that the tick index is within the authorized range + // Should never get here. + if tickIndex > types.MaxTick || tickIndex < types.MinTick { + return 0, types.TickIndexNotWithinBoundariesError{ActualTick: tickIndex, MinTick: types.MinTick, MaxTick: types.MaxTick} } - // Determine the tick that corresponds to the price - // This does not take into account the tickSpacing - tickIndex := CalculatePriceToTick(price) - return tickIndex, nil } -// PriceToTickRoundDown takes a price and returns the corresponding tick index. -// If tickSpacing is provided, the tick index will be rounded down to the nearest multiple of tickSpacing. -// CONTRACT: tickSpacing must be smaller or equal to the max of 1 << 63 - 1. -// This is not a concern because we have authorized tick spacings that are smaller than this max, -// and we don't expect to ever require it to be this large. -func PriceToTickRoundDown(price sdk.Dec, tickSpacing uint64) (int64, error) { - tickIndex, err := PriceToTick(price) +// SqrtPriceToTickRoundDown converts the given sqrt price to its corresponding tick rounded down +// to the nearest tick spacing. +func SqrtPriceToTickRoundDownSpacing(sqrtPrice sdk.Dec, tickSpacing uint64) (int64, error) { + tickIndex, err := CalculateSqrtPriceToTick(sqrtPrice) if err != nil { return 0, err } - // Round the tick index down to the nearest tick spacing if the tickIndex is in between authorized tick values - // Note that this is Euclidean modulus. - // The difference from default Go modulus is that Go default results - // in a negative remainder when the dividend is negative. - // Consider example tickIndex = -17, tickSpacing = 10 - // tickIndexModulus = tickIndex % tickSpacing = -7 - // tickIndexModulus = -7 + 10 = 3 - // tickIndex = -17 - 3 = -20 - tickIndexModulus := tickIndex % int64(tickSpacing) - if tickIndexModulus < 0 { - tickIndexModulus += int64(tickSpacing) - } - - if tickIndexModulus != 0 { - tickIndex = tickIndex - tickIndexModulus - } - - // Defense-in-depth check to ensure that the tick index is within the authorized range - // Should never get here. - if tickIndex > types.MaxTick || tickIndex < types.MinTick { - return 0, types.TickIndexNotWithinBoundariesError{ActualTick: tickIndex, MinTick: types.MinTick, MaxTick: types.MaxTick} + tickIndex, err = RoundDownTickToSpacing(tickIndex, int64(tickSpacing)) + if err != nil { + return 0, err } return tickIndex, nil @@ -166,13 +151,9 @@ func powTenBigDec(exponent int64) osmomath.BigDec { return bigNegPowersOfTen[-exponent] } -// CalculatePriceToTick takes in a price and returns the corresponding tick index. -// This function does not take into consideration tick spacing. -// NOTE: This is really returning a "Bucket index". Bucket index `b` corresponds to -// all prices in range [TickToPrice(b), TickToPrice(b+1)). -func CalculatePriceToTick(price sdk.Dec) (tickIndex int64) { +func CalculatePriceToTickDec(price sdk.Dec) (tickIndex sdk.Dec) { if price.Equal(sdkOneDec) { - return 0 + return sdk.ZeroDec() } // The approach here is to try determine which "geometric spacing" are we in. @@ -204,9 +185,73 @@ func CalculatePriceToTick(price sdk.Dec) (tickIndex int64) { ticksFilledByCurrentSpacing := priceInThisExponent.Quo(geoSpacing.additiveIncrementPerTick) // we get the bucket index by: // * taking the bucket index of the smallest price in this tick - // * adding to it the number of ticks "completely" filled by the current spacing - // the latter is the truncation of the division above - // TODO: This should be rounding down? - tickIndex = geoSpacing.initialTick + ticksFilledByCurrentSpacing.SDKDec().RoundInt64() + // * adding to it the number of ticks filled by the current spacing + // We leave rounding of this to the caller + // (NOTE: You'd expect it to be number of ticks "completely" filled by the current spacing, + // which would be truncation. However price may have errors, hence it being callers job) + tickIndex = ticksFilledByCurrentSpacing.SDKDec() + tickIndex = tickIndex.Add(sdk.NewDec(geoSpacing.initialTick)) return tickIndex } + +// CalculateSqrtPriceToTick takes in a square root and returns the corresponding tick index. +// This function does not take into consideration tick spacing. +func CalculateSqrtPriceToTick(sqrtPrice sdk.Dec) (tickIndex int64, err error) { + // SqrtPrice may have errors, so we take the tick obtained from the price + // and move it in a +/- 1 tick range based on the sqrt price those ticks would imply. + price := sqrtPrice.Mul(sqrtPrice) + tick := CalculatePriceToTickDec(price) + truncatedTick := tick.TruncateInt64() + + // We have a candidate bucket index `t`. We discern here if: + // * sqrtPrice in [TickToSqrtPrice(t - 1), TickToSqrtPrice(t)) + // * sqrtPrice in [TickToSqrtPrice(t), TickToSqrtPrice(t + 1)) + // * sqrtPrice in [TickToSqrtPrice(t+1), TickToSqrtPrice(t + 2)) + // * sqrtPrice not in either. + // We handle boundary checks, by saying that if our candidate is the min tick, + // set the candidate to min tick + 1. + // If our candidate is at or above max tick - 1, set the candidate to max tick - 2. + // This is because to check tick t + 1, we need to go to t + 2, so to not go over + // max tick during these checks, we need to shift it down by 2. + // We check this at max tick - 1 instead of max tick, since we expect the output to + // have some error that can push us over the tick boundary. + outOfBounds := false + if truncatedTick <= types.MinTick { + truncatedTick = types.MinTick + 1 + outOfBounds = true + } else if truncatedTick >= types.MaxTick-1 { + truncatedTick = types.MaxTick - 2 + outOfBounds = true + } + + _, sqrtPriceTmin1, errM1 := TickToSqrtPrice(truncatedTick - 1) + _, sqrtPriceT, errT := TickToSqrtPrice(truncatedTick) + _, sqrtPriceTplus1, errP1 := TickToSqrtPrice(truncatedTick + 1) + _, sqrtPriceTplus2, errP2 := TickToSqrtPrice(truncatedTick + 2) + if errM1 != nil || errT != nil || errP1 != nil || errP2 != nil { + return 0, errors.New("internal error in computing square roots within CalculateSqrtPriceToTick") + } + + // We error if sqrtPriceT is above sqrtPriceTplus2 or below sqrtPriceTmin1. + // For cases where calculated tick does not fall on a limit (min/max tick), the upper end is exclusive. + // For cases where calculated tick falls on a limit, the upper end is inclusive, since the actual tick is + // already shifted and making it exclusive would make min/max tick impossible to reach by construction. + // We do this primary for code simplicity, as alternatives would require more branching and special cases. + if (!outOfBounds && sqrtPrice.GTE(sqrtPriceTplus2)) || (outOfBounds && sqrtPrice.GT(sqrtPriceTplus2)) || sqrtPrice.LT(sqrtPriceTmin1) { + return 0, fmt.Errorf("sqrt price to tick could not find a satisfying tick index. Hit bounds: %v", outOfBounds) + } + + // We expect this case to only be hit when the original provided sqrt price is exactly equal to the max sqrt price. + if sqrtPrice.Equal(sqrtPriceTplus2) { + return truncatedTick + 2, nil + } + + // The remaining cases handle shifting tick index by +/- 1. + if sqrtPrice.GTE(sqrtPriceTplus1) { + return truncatedTick + 1, nil + } + if sqrtPrice.GTE(sqrtPriceT) { + return truncatedTick, nil + } + return truncatedTick - 1, nil +} diff --git a/x/concentrated-liquidity/math/tick_test.go b/x/concentrated-liquidity/math/tick_test.go index 578bbbebd8e..1e5e4d2b979 100644 --- a/x/concentrated-liquidity/math/tick_test.go +++ b/x/concentrated-liquidity/math/tick_test.go @@ -376,10 +376,10 @@ func (suite *ConcentratedMathTestSuite) TestPriceToTick() { suite.Run(name, func() { // surpress error here, we only listen to errors from system under test. - tick, _ := math.PriceToTick(tc.price) + tick, _ := suite.PriceToTick(tc.price) // With tick spacing of one, no rounding should occur. - tickRoundDown, err := math.PriceToTickRoundDown(tc.price, one) + tickRoundDown, err := suite.PriceToTickRoundDownSpacing(tc.price, one) if tc.expectedError != nil { suite.Require().Error(err) suite.Require().ErrorContains(err, tc.expectedError.Error()) @@ -453,7 +453,7 @@ func (suite *ConcentratedMathTestSuite) TestPriceToTickRoundDown() { tc := tc suite.Run(name, func() { - tick, err := math.PriceToTickRoundDown(tc.price, tc.tickSpacing) + tick, err := suite.PriceToTickRoundDownSpacing(tc.price, tc.tickSpacing) suite.Require().NoError(err) suite.Require().Equal(tc.tickExpected, tick) @@ -591,7 +591,7 @@ func (suite *ConcentratedMathTestSuite) TestTickToSqrtPricePriceToTick_InverseRe tickSpacing := uint64(1) // 1. Compute tick from price. - tickFromPrice, err := math.PriceToTickRoundDown(tc.price, tickSpacing) + tickFromPrice, err := suite.PriceToTickRoundDownSpacing(tc.price, tickSpacing) suite.Require().NoError(err) suite.Require().Equal(tc.tickExpected, tickFromPrice) @@ -607,7 +607,7 @@ func (suite *ConcentratedMathTestSuite) TestTickToSqrtPricePriceToTick_InverseRe suite.Require().Equal(expectedPrice, price) // 3. Compute tick from inverse price (inverse tick) - inverseTickFromPrice, err := math.PriceToTickRoundDown(price, tickSpacing) + inverseTickFromPrice, err := suite.PriceToTickRoundDownSpacing(price, tickSpacing) suite.Require().NoError(err) // Make sure original tick and inverse tick match. @@ -624,7 +624,7 @@ func (suite *ConcentratedMathTestSuite) TestTickToSqrtPricePriceToTick_InverseRe // suite.Require().Equal(expectedPrice.String(), priceFromSqrtPrice.String()) // 5. Compute tick from sqrt price from the original tick. - inverseTickFromSqrtPrice, err := math.PriceToTickRoundDown(priceFromSqrtPrice, tickSpacing) + inverseTickFromSqrtPrice, err := suite.PriceToTickRoundDownSpacing(priceFromSqrtPrice, tickSpacing) suite.Require().NoError(err) suite.Require().Equal(tickFromPrice, inverseTickFromSqrtPrice, "expected: %s, actual: %s", tickFromPrice, inverseTickFromSqrtPrice) @@ -650,7 +650,7 @@ func (suite *ConcentratedMathTestSuite) TestPriceToTick_ErrorCases() { tc := tc suite.Run(name, func() { - tickFromPrice, err := math.PriceToTick(tc.price) + tickFromPrice, err := suite.PriceToTick(tc.price) suite.Require().Error(err) suite.Require().Equal(tickFromPrice, int64(0)) }) @@ -709,8 +709,114 @@ func (suite *ConcentratedMathTestSuite) TestCalculatePriceToTick() { } for name, t := range testCases { suite.Run(name, func() { - tickIndex := math.CalculatePriceToTick(t.price) + tickIndex := suite.CalculatePriceToTick(t.price) suite.Require().Equal(t.expectedTickIndex, tickIndex) }) } } + +func (s *ConcentratedMathTestSuite) TestSqrtPriceToTickRoundDownSpacing() { + testCases := map[string]struct { + sqrtPrice sdk.Dec + tickSpacing uint64 + tickExpected int64 + }{ + "sqrt price of 1 (tick spacing 1)": { + sqrtPrice: sdk.OneDec(), + tickSpacing: 1, + tickExpected: 0, + }, + "sqrt price exactly on boundary of next tick (tick spacing 1)": { + sqrtPrice: sdk.MustNewDecFromStr("1.000000499999875000"), + tickSpacing: 1, + tickExpected: 1, + }, + "sqrt price one ULP below boundary of next tick (tick spacing 1)": { + sqrtPrice: sdk.MustNewDecFromStr("1.000000499999874999"), + tickSpacing: 1, + tickExpected: 0, + }, + "sqrt price corresponding to bucket 99 (tick spacing 100)": { + sqrtPrice: sdk.MustNewDecFromStr("1.000049498774935650"), + tickSpacing: defaultTickSpacing, + tickExpected: 0, + }, + "sqrt price exactly on bucket 100 (tick spacing 100)": { + sqrtPrice: sdk.MustNewDecFromStr("1.000049998750062496"), + tickSpacing: defaultTickSpacing, + tickExpected: 100, + }, + "sqrt price one ULP below bucket 100 (tick spacing 100)": { + sqrtPrice: sdk.MustNewDecFromStr("1.000049998750062495"), + tickSpacing: defaultTickSpacing, + tickExpected: 0, + }, + "sqrt price exactly on bucket -100 (tick spacing 100)": { + sqrtPrice: sdk.MustNewDecFromStr("0.999994999987499937"), + tickSpacing: defaultTickSpacing, + tickExpected: -100, + }, + "sqrt price one ULP below bucket -100 (tick spacing 100)": { + sqrtPrice: sdk.MustNewDecFromStr("0.999994999987499936"), + tickSpacing: defaultTickSpacing, + tickExpected: -200, + }, + "sqrt price exactly on tick -101 (tick spacing 100)": { + sqrtPrice: sdk.MustNewDecFromStr("0.999994949987248685"), + tickSpacing: defaultTickSpacing, + tickExpected: -200, + }, + "sqrt price exactly equal to max sqrt price": { + sqrtPrice: types.MaxSqrtPrice, + tickSpacing: defaultTickSpacing, + tickExpected: types.MaxTick, + }, + "sqrt price exactly equal to min sqrt price": { + sqrtPrice: types.MinSqrtPrice, + tickSpacing: defaultTickSpacing, + tickExpected: types.MinTick, + }, + "sqrt price equal to max sqrt price minus one ULP": { + sqrtPrice: types.MaxSqrtPrice.Sub(sdk.SmallestDec()), + tickSpacing: defaultTickSpacing, + tickExpected: types.MaxTick - defaultTickSpacing, + }, + "sqrt price corresponds exactly to max tick - 1 (tick spacing 1)": { + // Calculated using TickToSqrtPrice(types.MaxTick - 1) + sqrtPrice: sdk.MustNewDecFromStr("9999999499999987499.999374999960937497"), + tickSpacing: 1, + tickExpected: types.MaxTick - 1, + }, + "sqrt price one ULP below max tick - 1 (tick spacing 1)": { + // Calculated using TickToSqrtPrice(types.MaxTick - 1) - 1 ULP + sqrtPrice: sdk.MustNewDecFromStr("9999999499999987499.999374999960937496"), + tickSpacing: 1, + tickExpected: types.MaxTick - 2, + }, + "sqrt price one ULP below max tick - 1 (tick spacing 100)": { + // Calculated using TickToSqrtPrice(types.MaxTick - 1) - 1 ULP + sqrtPrice: sdk.MustNewDecFromStr("9999999499999987499.999374999960937496"), + tickSpacing: defaultTickSpacing, + tickExpected: types.MaxTick - defaultTickSpacing, + }, + } + for name, tc := range testCases { + s.Run(name, func() { + tickIndex, err := math.SqrtPriceToTickRoundDownSpacing(tc.sqrtPrice, tc.tickSpacing) + s.Require().NoError(err) + s.Require().Equal(tc.tickExpected, tickIndex) + + // Ensure returned bucket properly encapsulates given sqrt price, skipping the upper bound + // check if we're on the max tick + _, inverseSqrtPrice, err := math.TickToSqrtPrice(tickIndex) + s.Require().NoError(err) + s.Require().True(inverseSqrtPrice.LTE(tc.sqrtPrice)) + + if tc.tickExpected != types.MaxTick { + _, inverseSqrtPriceTickAbove, err := math.TickToSqrtPrice(tickIndex + int64(tc.tickSpacing)) + s.Require().NoError(err) + s.Require().True(inverseSqrtPriceTickAbove.GT(tc.sqrtPrice)) + } + }) + } +} diff --git a/x/concentrated-liquidity/position_test.go b/x/concentrated-liquidity/position_test.go index 5efe7ba3e29..ac578fba335 100644 --- a/x/concentrated-liquidity/position_test.go +++ b/x/concentrated-liquidity/position_test.go @@ -2409,42 +2409,63 @@ func (s *KeeperTestSuite) TestCreateFullRangePositionLocked() { } } +// TestTickRoundingEdgeCase tests an edge case where incorrect tick rounding would cause LP funds to be drained. +func (s *KeeperTestSuite) TestTickRoundingEdgeCase() { + s.SetupTest() + pool := s.PrepareConcentratedPool() + + testAccs := apptesting.CreateRandomAccounts(3) + firstPositionAddr := testAccs[0] + secondPositionAddr := testAccs[1] + + // Create two identical positions with the initial assets set such that both positions are fully in one asset + firstPositionAssets := sdk.NewCoins(sdk.NewCoin(ETH, sdk.NewInt(9823358512)), sdk.NewCoin(USDC, sdk.NewInt(8985893232))) + firstPosLiq, firstPosId := s.SetupPosition(pool.GetId(), firstPositionAddr, firstPositionAssets, -68720000, -68710000, true) + secondPositionAssets := sdk.NewCoins(sdk.NewCoin(ETH, sdk.NewInt(9823358512)), sdk.NewCoin(USDC, sdk.NewInt(8985893232))) + secondPosLiq, secondPosId := s.SetupPosition(pool.GetId(), secondPositionAddr, secondPositionAssets, -68720000, -68710000, true) + + // Execute a swap that brings the price close enough to the edge of a tick to trigger bankers rounding + swapAddr := testAccs[2] + desiredTokenOut := sdk.NewCoin(USDC, sdk.NewInt(10000)) + s.FundAcc(swapAddr, sdk.NewCoins(sdk.NewCoin(ETH, sdk.NewInt(1000000000000000000)))) + _, _, _, _, _, err := s.clk.SwapInAmtGivenOut(s.Ctx, swapAddr, pool, desiredTokenOut, ETH, sdk.ZeroDec(), sdk.ZeroDec()) + s.Require().NoError(err) + + // Both positions should be able to withdraw successfully + _, _, err = s.clk.WithdrawPosition(s.Ctx, firstPositionAddr, firstPosId, firstPosLiq) + s.Require().NoError(err) + _, _, err = s.clk.WithdrawPosition(s.Ctx, secondPositionAddr, secondPosId, secondPosLiq) + s.Require().NoError(err) +} + func (s *KeeperTestSuite) TestMultipleRanges() { tests := map[string]struct { tickRanges [][]int64 rangeTestParams RangeTestParams }{ - // The following two tests will fail until the tick rounding bug is fixed (and should pass after): - // https://github.com/osmosis-labs/osmosis/issues/5516 - // - // "one min width range": { - // tickRanges: [][]int64{ - // {0, 100}, - // }, - // rangeTestParams: DefaultRangeTestParams, - // }, - // "two adjacent ranges": { - // tickRanges: [][]int64{ - // {-10000, 10000}, - // {10000, 20000}, - // }, - // rangeTestParams: DefaultRangeTestParams, - // }, - - // Both of these lead to underclaiming of fees greater than additive - // error tolerance of 1 per token per position. Increasing ticks increases - // error disproportionally, while increasing tick range decreases error proportionally. - // - // "one range on large tick": { - // tickRanges: [][]int64{ - // {207000000, 207000000 + 100}, - // }, - // }, - // "one range on small tick": { - // tickRanges: [][]int64{ - // {-107000000, -107000000 + 100}, - // }, - // }, + "one min width range": { + tickRanges: [][]int64{ + {0, 100}, + }, + rangeTestParams: DefaultRangeTestParams, + }, + "two adjacent ranges": { + tickRanges: [][]int64{ + {-10000, 10000}, + {10000, 20000}, + }, + rangeTestParams: DefaultRangeTestParams, + }, + "one range on large tick": { + tickRanges: [][]int64{ + {207000000, 207000000 + 100}, + }, + }, + "one range on small tick": { + tickRanges: [][]int64{ + {-107000000, -107000000 + 100}, + }, + }, } for name, tc := range tests { diff --git a/x/concentrated-liquidity/range_test.go b/x/concentrated-liquidity/range_test.go index e733c6265ce..eb0e7837a82 100644 --- a/x/concentrated-liquidity/range_test.go +++ b/x/concentrated-liquidity/range_test.go @@ -150,7 +150,8 @@ func (s *KeeperTestSuite) setupRangesAndAssertInvariants(pool types.Concentrated // Ensure the pool balance is exactly equal to the assets added + amount swapped in - amount swapped out poolAssets := s.App.BankKeeper.GetAllBalances(s.Ctx, pool.GetAddress()) poolSpreadRewards := s.App.BankKeeper.GetAllBalances(s.Ctx, pool.GetSpreadRewardsAddress()) - s.Require().Equal(totalAssets, poolAssets.Add(poolSpreadRewards...)) + // We rebuild coins to handle nil cases cleanly + s.Require().Equal(sdk.NewCoins(totalAssets...), sdk.NewCoins(poolAssets.Add(poolSpreadRewards...)...)) } // numPositionSlice prepares a slice tracking the number of positions to create on each range, fuzzing the number at each step if applicable. diff --git a/x/concentrated-liquidity/spread_rewards_test.go b/x/concentrated-liquidity/spread_rewards_test.go index 4c9a057539a..e8258fc8526 100644 --- a/x/concentrated-liquidity/spread_rewards_test.go +++ b/x/concentrated-liquidity/spread_rewards_test.go @@ -10,7 +10,6 @@ import ( "github.com/osmosis-labs/osmosis/osmoutils/accum" "github.com/osmosis-labs/osmosis/v16/app/apptesting" cl "github.com/osmosis-labs/osmosis/v16/x/concentrated-liquidity" - "github.com/osmosis-labs/osmosis/v16/x/concentrated-liquidity/math" clmodel "github.com/osmosis-labs/osmosis/v16/x/concentrated-liquidity/model" "github.com/osmosis-labs/osmosis/v16/x/concentrated-liquidity/types" ) @@ -489,7 +488,7 @@ func (s *KeeperTestSuite) TestGetInitialSpreadRewardGrowthOppositeDirectionOfLas validPoolId = 1 ) - initialPoolTick, err := math.PriceToTickRoundDown(DefaultAmt1.ToDec().Quo(DefaultAmt0.ToDec()), DefaultTickSpacing) + initialPoolTick, err := s.PriceToTickRoundDownSpacing(DefaultAmt1.ToDec().Quo(DefaultAmt0.ToDec()), DefaultTickSpacing) s.Require().NoError(err) tests := map[string]struct { diff --git a/x/concentrated-liquidity/swaps.go b/x/concentrated-liquidity/swaps.go index 46ddebada4a..6f9537ec260 100644 --- a/x/concentrated-liquidity/swaps.go +++ b/x/concentrated-liquidity/swaps.go @@ -365,8 +365,7 @@ func (k Keeper) computeOutAmtGivenIn( } 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(...) - price := computedSqrtPrice.Mul(computedSqrtPrice) - swapState.tick, err = math.PriceToTickRoundDown(price, p.GetTickSpacing()) + swapState.tick, err = math.SqrtPriceToTickRoundDownSpacing(computedSqrtPrice, p.GetTickSpacing()) if err != nil { return sdk.Coin{}, sdk.Coin{}, 0, sdk.Dec{}, sdk.Dec{}, sdk.Dec{}, err } @@ -489,8 +488,7 @@ func (k Keeper) computeInAmtGivenOut( } else if !sqrtPriceStart.Equal(computedSqrtPrice) { // Otherwise, if the computedSqrtPrice calculated from ComputeSwapWithinBucketInGivenOut(...) 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 ComputeSwapWithinBucketInGivenOut(...) - price := computedSqrtPrice.Mul(computedSqrtPrice) - swapState.tick, err = math.PriceToTickRoundDown(price, p.GetTickSpacing()) + swapState.tick, err = math.SqrtPriceToTickRoundDownSpacing(computedSqrtPrice, p.GetTickSpacing()) if err != nil { return sdk.Coin{}, sdk.Coin{}, 0, sdk.Dec{}, sdk.Dec{}, sdk.Dec{}, err } diff --git a/x/concentrated-liquidity/swaps_test.go b/x/concentrated-liquidity/swaps_test.go index d223a78f33a..7fcf1d4f37c 100644 --- a/x/concentrated-liquidity/swaps_test.go +++ b/x/concentrated-liquidity/swaps_test.go @@ -455,7 +455,7 @@ var ( expectedTokenIn: sdk.NewCoin("eth", sdk.NewInt(12892)), expectedTokenOut: sdk.NewCoin("usdc", sdk.NewInt(64417624)), expectedTick: func() int64 { - tick, _ := math.PriceToTickRoundDown(sdk.NewDec(4994), DefaultTickSpacing) + tick, _ := math.SqrtPriceToTickRoundDownSpacing(sqrt4994, DefaultTickSpacing) return tick }(), expectedSqrtPrice: sdk.MustNewDecFromStr("70.668238976219012613"), // https://www.wolframalpha.com/input?i=%28%281517882343.751510418088349649%29%29+%2F+%28%28%281517882343.751510418088349649%29+%2F+%2870.710678118654752440%29%29+%2B+%2812891.26207649936510%29%29 @@ -620,7 +620,7 @@ var ( expectedTokenOut: sdk.NewCoin("usdc", sdk.NewInt(64417624)), expectedSpreadRewardGrowthAccumulatorValue: sdk.MustNewDecFromStr("0.000000085792039652"), expectedTick: func() int64 { - tick, _ := math.PriceToTickRoundDown(sdk.NewDec(4994), DefaultTickSpacing) + tick, _ := math.SqrtPriceToTickRoundDownSpacing(sqrt4994, DefaultTickSpacing) return tick }(), expectedSqrtPrice: sdk.MustNewDecFromStr("70.668238976219012614"), // https://www.wolframalpha.com/input?i=%28%281517882343.751510418088349649%29%29+%2F+%28%28%281517882343.751510418088349649%29+%2F+%2870.710678118654752440%29%29+%2B+%2813020+*+%281+-+0.01%29%29%29 @@ -1566,9 +1566,9 @@ func (s *KeeperTestSuite) getExpectedLiquidity(test SwapTest, pool types.Concent test.newUpperPrice = DefaultUpperPrice } - newLowerTick, err := math.PriceToTickRoundDown(test.newLowerPrice, pool.GetTickSpacing()) + newLowerTick, err := s.PriceToTickRoundDownSpacing(test.newLowerPrice, pool.GetTickSpacing()) s.Require().NoError(err) - newUpperTick, err := math.PriceToTickRoundDown(test.newUpperPrice, pool.GetTickSpacing()) + newUpperTick, err := s.PriceToTickRoundDownSpacing(test.newUpperPrice, pool.GetTickSpacing()) s.Require().NoError(err) _, lowerSqrtPrice, err := math.TickToSqrtPrice(newLowerTick) @@ -2265,9 +2265,9 @@ func (s *KeeperTestSuite) TestCalcOutAmtGivenIn_NonMutative() { func (s *KeeperTestSuite) setupSecondPosition(test SwapTest, pool types.ConcentratedPoolExtension) { if !test.secondPositionLowerPrice.IsNil() { - newLowerTick, err := math.PriceToTickRoundDown(test.secondPositionLowerPrice, pool.GetTickSpacing()) + newLowerTick, err := s.PriceToTickRoundDownSpacing(test.secondPositionLowerPrice, pool.GetTickSpacing()) s.Require().NoError(err) - newUpperTick, err := math.PriceToTickRoundDown(test.secondPositionUpperPrice, pool.GetTickSpacing()) + newUpperTick, err := s.PriceToTickRoundDownSpacing(test.secondPositionUpperPrice, pool.GetTickSpacing()) s.Require().NoError(err) _, _, _, _, _, _, err = s.App.ConcentratedLiquidityKeeper.CreatePosition(s.Ctx, pool.GetId(), s.TestAccs[1], DefaultCoins, sdk.ZeroInt(), sdk.ZeroInt(), newLowerTick, newUpperTick) diff --git a/x/concentrated-liquidity/tick.go b/x/concentrated-liquidity/tick.go index 806f81fd7cb..cbb186469f3 100644 --- a/x/concentrated-liquidity/tick.go +++ b/x/concentrated-liquidity/tick.go @@ -195,12 +195,12 @@ func validateTickRangeIsValid(tickSpacing uint64, lowerTick int64, upperTick int // the first tick (given our precision) that is able to represent this price is -161000000, so we use this tick instead. // // This really only applies to very small tick values, as the increment of a single tick continues to get smaller as the tick value gets smaller. -func roundTickToCanonicalPriceTick(lowerTick, upperTick int64, priceTickLower, priceTickUpper sdk.Dec, tickSpacing uint64) (int64, int64, error) { - newLowerTick, err := math.PriceToTickRoundDown(priceTickLower, tickSpacing) +func roundTickToCanonicalPriceTick(lowerTick, upperTick int64, sqrtPriceTickLower, sqrtPriceTickUpper sdk.Dec, tickSpacing uint64) (int64, int64, error) { + newLowerTick, err := math.SqrtPriceToTickRoundDownSpacing(sqrtPriceTickLower, tickSpacing) if err != nil { return 0, 0, err } - newUpperTick, err := math.PriceToTickRoundDown(priceTickUpper, tickSpacing) + newUpperTick, err := math.SqrtPriceToTickRoundDownSpacing(sqrtPriceTickUpper, tickSpacing) if err != nil { return 0, 0, err } diff --git a/x/concentrated-liquidity/tick_test.go b/x/concentrated-liquidity/tick_test.go index 831816eaf8a..b764e146c51 100644 --- a/x/concentrated-liquidity/tick_test.go +++ b/x/concentrated-liquidity/tick_test.go @@ -1251,7 +1251,7 @@ func (s *KeeperTestSuite) TestGetTickLiquidityNetInDirection() { curPrice := sdk.OneDec() // TODO: consider adding tests for GetTickLiquidityNetInDirection // with tick spacing > 1, requiring price to tick conversion with rounding. - curTick, err := math.PriceToTick(curPrice) + curTick, err := s.PriceToTick(curPrice) s.Require().NoError(err) if test.currentPoolTick > 0 { _, sqrtPrice, err := math.TickToSqrtPrice(test.currentPoolTick) @@ -1462,13 +1462,13 @@ func (s *KeeperTestSuite) TestRoundTickToCanonicalPriceTick() { s.Run(test.name, func() { s.SetupTest() - priceTickLower, _, err := math.TickToSqrtPrice(test.lowerTick) + _, sqrtPriceTickLower, err := math.TickToSqrtPrice(test.lowerTick) s.Require().NoError(err) - priceTickUpper, _, err := math.TickToSqrtPrice(test.upperTick) + _, sqrtPriceTickUpper, err := math.TickToSqrtPrice(test.upperTick) s.Require().NoError(err) // System Under Test - newLowerTick, newUpperTick, err := cl.RoundTickToCanonicalPriceTick(test.lowerTick, test.upperTick, priceTickLower, priceTickUpper, DefaultTickSpacing) + newLowerTick, newUpperTick, err := cl.RoundTickToCanonicalPriceTick(test.lowerTick, test.upperTick, sqrtPriceTickLower, sqrtPriceTickUpper, DefaultTickSpacing) if test.expectedError != nil { s.Require().Error(err)