From a33f23c263236309c08130978cc39077ca9dc809 Mon Sep 17 00:00:00 2001 From: Dev Ojha Date: Wed, 14 Jun 2023 15:09:33 +0200 Subject: [PATCH 01/16] Add sqrt price to tick --- x/concentrated-liquidity/math/TickHandling.md | 0 x/concentrated-liquidity/math/tick.go | 80 ++++++++++++++++--- 2 files changed, 70 insertions(+), 10 deletions(-) create mode 100644 x/concentrated-liquidity/math/TickHandling.md diff --git a/x/concentrated-liquidity/math/TickHandling.md b/x/concentrated-liquidity/math/TickHandling.md new file mode 100644 index 00000000000..e69de29bb2d diff --git a/x/concentrated-liquidity/math/tick.go b/x/concentrated-liquidity/math/tick.go index 20ac2d8e32b..2fd3331a576 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" @@ -166,13 +167,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 +201,72 @@ 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 } + +// CalculatePriceToTick takes in a price and returns the corresponding tick index. +// This function does not take into consideration tick spacing. +// NOTE: This should not be called in the state machine. +// 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 CalculatePriceToTick(price sdk.Dec) (tickIndex int64) { + // TODO: Make truncate, since this defines buckets as + // [TickToPrice(b - .5), TickToPrice(b+.5)) + return calculatePriceToTickDec(price).RoundInt64() +} + +// CalculatePriceToTick takes in a square root 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 sqrt prices in range [TickToSqrtPrice(b), TickToSqrtPrice(b+1)). +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 the max tick, set the candidate to max tick - 2. + outOfBounds := false + if truncatedTick <= types.MinTick { + truncatedTick = types.MinTick + 1 + outOfBounds = true + } else if truncatedTick >= types.MaxTick { + 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") + } + if sqrtPrice.GTE(sqrtPriceTplus2) || sqrtPrice.LT(sqrtPriceTmin1) { + return 0, fmt.Errorf("sqrt price to tick could not find a satisfying tick index. Hit bounds: %v", outOfBounds) + } + if sqrtPrice.GTE(sqrtPriceTplus1) { + return truncatedTick + 1, nil + } + if sqrtPrice.GTE(sqrtPriceT) { + return truncatedTick, nil + } + return truncatedTick - 1, nil +} From f7447b7f263c996581140f7c43b2b0257c885363 Mon Sep 17 00:00:00 2001 From: alpo Date: Thu, 15 Jun 2023 11:14:59 -0700 Subject: [PATCH 02/16] implement and test sqrt price to tick --- x/concentrated-liquidity/keeper_test.go | 8 +-- x/concentrated-liquidity/lp.go | 6 +- x/concentrated-liquidity/math/tick.go | 62 +++++++++++----- x/concentrated-liquidity/math/tick_test.go | 70 +++++++++++++++++++ .../spread_rewards_test.go | 2 +- x/concentrated-liquidity/swaps.go | 6 +- x/concentrated-liquidity/swaps_test.go | 12 ++-- x/concentrated-liquidity/tick.go | 6 +- 8 files changed, 133 insertions(+), 39 deletions(-) diff --git a/x/concentrated-liquidity/keeper_test.go b/x/concentrated-liquidity/keeper_test.go index 8bad6cb7cc8..0c061ba67f8 100644 --- a/x/concentrated-liquidity/keeper_test.go +++ b/x/concentrated-liquidity/keeper_test.go @@ -49,10 +49,10 @@ 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) + DefaultExponentConsecutivePositionLowerTick, _ = math.PriceToTickRoundDownSpacing(sdk.NewDec(5500), DefaultTickSpacing) + DefaultExponentConsecutivePositionUpperTick, _ = math.PriceToTickRoundDownSpacing(sdk.NewDec(6250), DefaultTickSpacing) + DefaultExponentOverlappingPositionLowerTick, _ = math.PriceToTickRoundDownSpacing(sdk.NewDec(4000), DefaultTickSpacing) + DefaultExponentOverlappingPositionUpperTick, _ = math.PriceToTickRoundDownSpacing(sdk.NewDec(4999), 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 3e69bab2f3f..f71975cf908 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 } @@ -475,7 +475,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/math/tick.go b/x/concentrated-liquidity/math/tick.go index 2fd3331a576..c0d8b317ee7 100644 --- a/x/concentrated-liquidity/math/tick.go +++ b/x/concentrated-liquidity/math/tick.go @@ -114,38 +114,64 @@ func PriceToTick(price sdk.Dec) (int64, error) { return tickIndex, 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 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} + } + + 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. +// NOTE: This should not be called in the state machine. // 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) { +func PriceToTickRoundDownSpacing(price sdk.Dec, tickSpacing uint64) (int64, error) { tickIndex, err := PriceToTick(price) 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) + tickIndex, err = RoundDownTickToSpacing(tickIndex, int64(tickSpacing)) + if err != nil { + return 0, err } - if tickIndexModulus != 0 { - tickIndex = tickIndex - tickIndexModulus + return tickIndex, nil +} + +// 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 } - // 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 diff --git a/x/concentrated-liquidity/math/tick_test.go b/x/concentrated-liquidity/math/tick_test.go index 578bbbebd8e..459385e1d7f 100644 --- a/x/concentrated-liquidity/math/tick_test.go +++ b/x/concentrated-liquidity/math/tick_test.go @@ -714,3 +714,73 @@ func (suite *ConcentratedMathTestSuite) TestCalculatePriceToTick() { }) } } + +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, + }, + } + 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 + _, inverseSqrtPrice, err := math.TickToSqrtPrice(tickIndex) + s.Require().NoError(err) + _, inverseSqrtPriceTickAbove, err := math.TickToSqrtPrice(tickIndex + int64(tc.tickSpacing)) + s.Require().NoError(err) + + s.Require().True(inverseSqrtPrice.LTE(tc.sqrtPrice)) + s.Require().True(inverseSqrtPriceTickAbove.GT(tc.sqrtPrice)) + }) + } +} diff --git a/x/concentrated-liquidity/spread_rewards_test.go b/x/concentrated-liquidity/spread_rewards_test.go index 4c9a057539a..abc8f354628 100644 --- a/x/concentrated-liquidity/spread_rewards_test.go +++ b/x/concentrated-liquidity/spread_rewards_test.go @@ -489,7 +489,7 @@ func (s *KeeperTestSuite) TestGetInitialSpreadRewardGrowthOppositeDirectionOfLas validPoolId = 1 ) - initialPoolTick, err := math.PriceToTickRoundDown(DefaultAmt1.ToDec().Quo(DefaultAmt0.ToDec()), DefaultTickSpacing) + initialPoolTick, err := math.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 e4fc0bf5885..91531bb9bcb 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(sqrtPrice) { // Otherwise if the sqrtPrice calculated from computeSwapStep does not equal the sqrtPrice we started with at the // beginning of this iteration, we set the swapState tick to the corresponding tick of the sqrtPrice calculated from computeSwapStep - price := sqrtPrice.Mul(sqrtPrice) - swapState.tick, err = math.PriceToTickRoundDown(price, p.GetTickSpacing()) + swapState.tick, err = math.SqrtPriceToTickRoundDownSpacing(sqrtPrice, p.GetTickSpacing()) if err != nil { return sdk.Coin{}, sdk.Coin{}, 0, sdk.Dec{}, sdk.Dec{}, sdk.Dec{}, err } @@ -495,8 +494,7 @@ func (k Keeper) computeInAmtGivenOut( } else if !sqrtPriceStart.Equal(sqrtPrice) { // otherwise if the sqrtPrice calculated from computeSwapStep does not equal the sqrtPrice we started with at the // beginning of this iteration, we set the swapState tick to the corresponding tick of the sqrtPrice calculated from computeSwapStep - price := sqrtPrice.Mul(sqrtPrice) - swapState.tick, err = math.PriceToTickRoundDown(price, p.GetTickSpacing()) + swapState.tick, err = math.SqrtPriceToTickRoundDownSpacing(sqrtPrice, 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 75a707f75a6..43d0e7bce10 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.PriceToTickRoundDownSpacing(sdk.NewDec(4994), 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.PriceToTickRoundDownSpacing(sdk.NewDec(4994), 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 := math.PriceToTickRoundDownSpacing(test.newLowerPrice, pool.GetTickSpacing()) s.Require().NoError(err) - newUpperTick, err := math.PriceToTickRoundDown(test.newUpperPrice, pool.GetTickSpacing()) + newUpperTick, err := math.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 := math.PriceToTickRoundDownSpacing(test.secondPositionLowerPrice, pool.GetTickSpacing()) s.Require().NoError(err) - newUpperTick, err := math.PriceToTickRoundDown(test.secondPositionUpperPrice, pool.GetTickSpacing()) + newUpperTick, err := math.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 fa110b47025..a362b40766f 100644 --- a/x/concentrated-liquidity/tick.go +++ b/x/concentrated-liquidity/tick.go @@ -200,12 +200,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 } From a1baa14df3acc965a6a4c065acd8d6d75910697c Mon Sep 17 00:00:00 2001 From: alpo Date: Thu, 15 Jun 2023 11:37:04 -0700 Subject: [PATCH 03/16] add original attack vector as test case --- x/concentrated-liquidity/position_test.go | 29 +++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/x/concentrated-liquidity/position_test.go b/x/concentrated-liquidity/position_test.go index 25e84379660..76b66156478 100644 --- a/x/concentrated-liquidity/position_test.go +++ b/x/concentrated-liquidity/position_test.go @@ -2509,3 +2509,32 @@ 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) +} From f6583ea15d7bf8cbd76db7f5e81a4440163f7ed5 Mon Sep 17 00:00:00 2001 From: alpo Date: Thu, 15 Jun 2023 11:38:33 -0700 Subject: [PATCH 04/16] fix tick tests broken from renaming --- x/concentrated-liquidity/math/tick_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/x/concentrated-liquidity/math/tick_test.go b/x/concentrated-liquidity/math/tick_test.go index 459385e1d7f..5313fdc3e80 100644 --- a/x/concentrated-liquidity/math/tick_test.go +++ b/x/concentrated-liquidity/math/tick_test.go @@ -379,7 +379,7 @@ func (suite *ConcentratedMathTestSuite) TestPriceToTick() { tick, _ := math.PriceToTick(tc.price) // With tick spacing of one, no rounding should occur. - tickRoundDown, err := math.PriceToTickRoundDown(tc.price, one) + tickRoundDown, err := math.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 := math.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 := math.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 := math.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 := math.PriceToTickRoundDownSpacing(priceFromSqrtPrice, tickSpacing) suite.Require().NoError(err) suite.Require().Equal(tickFromPrice, inverseTickFromSqrtPrice, "expected: %s, actual: %s", tickFromPrice, inverseTickFromSqrtPrice) From 7baf57506d81099bc1abe72c6824c2eae777f405 Mon Sep 17 00:00:00 2001 From: alpo Date: Thu, 15 Jun 2023 18:10:05 -0700 Subject: [PATCH 05/16] fix max tick handling and add tests at limits --- x/concentrated-liquidity/lp_test.go | 5 ++- x/concentrated-liquidity/math/tick.go | 19 ++++++++- x/concentrated-liquidity/math/tick_test.go | 46 +++++++++++++++++++--- x/concentrated-liquidity/tick_test.go | 6 +-- 4 files changed, 65 insertions(+), 11 deletions(-) diff --git a/x/concentrated-liquidity/lp_test.go b/x/concentrated-liquidity/lp_test.go index 279ed0cf0cf..874cb23e67d 100644 --- a/x/concentrated-liquidity/lp_test.go +++ b/x/concentrated-liquidity/lp_test.go @@ -1568,7 +1568,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 c0d8b317ee7..fead77e946a 100644 --- a/x/concentrated-liquidity/math/tick.go +++ b/x/concentrated-liquidity/math/tick.go @@ -273,7 +273,7 @@ func CalculateSqrtPriceToTick(sqrtPrice sdk.Dec) (tickIndex int64, err error) { if truncatedTick <= types.MinTick { truncatedTick = types.MinTick + 1 outOfBounds = true - } else if truncatedTick >= types.MaxTick { + } else if truncatedTick >= types.MaxTick-1 { truncatedTick = types.MaxTick - 2 outOfBounds = true } @@ -285,9 +285,24 @@ func CalculateSqrtPriceToTick(sqrtPrice sdk.Dec) (tickIndex int64, err error) { if errM1 != nil || errT != nil || errP1 != nil || errP2 != nil { return 0, errors.New("internal error in computing square roots within CalculateSqrtPriceToTick") } - if sqrtPrice.GTE(sqrtPriceTplus2) || sqrtPrice.LT(sqrtPriceTmin1) { + fmt.Println("sqrtPriceTplus1: ", sqrtPriceTplus1) + fmt.Println("sqrtPriceTplus2: ", sqrtPriceTplus2) + + // 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)) || sqrtPrice.LT(sqrtPriceTmin1) || sqrtPrice.GT(sqrtPriceTplus2) { 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 } diff --git a/x/concentrated-liquidity/math/tick_test.go b/x/concentrated-liquidity/math/tick_test.go index 5313fdc3e80..d88b7333015 100644 --- a/x/concentrated-liquidity/math/tick_test.go +++ b/x/concentrated-liquidity/math/tick_test.go @@ -766,6 +766,39 @@ func (s *ConcentratedMathTestSuite) TestSqrtPriceToTickRoundDownSpacing() { 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() { @@ -773,14 +806,17 @@ func (s *ConcentratedMathTestSuite) TestSqrtPriceToTickRoundDownSpacing() { s.Require().NoError(err) s.Require().Equal(tc.tickExpected, tickIndex) - // Ensure returned bucket properly encapsulates given sqrt price + // 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) - _, inverseSqrtPriceTickAbove, err := math.TickToSqrtPrice(tickIndex + int64(tc.tickSpacing)) - s.Require().NoError(err) - s.Require().True(inverseSqrtPrice.LTE(tc.sqrtPrice)) - s.Require().True(inverseSqrtPriceTickAbove.GT(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/tick_test.go b/x/concentrated-liquidity/tick_test.go index d7374523249..826840df302 100644 --- a/x/concentrated-liquidity/tick_test.go +++ b/x/concentrated-liquidity/tick_test.go @@ -1550,13 +1550,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) From ac92ddb7a9d7c233f6ddb22b23aba2045f8b3fb7 Mon Sep 17 00:00:00 2001 From: alpo Date: Thu, 15 Jun 2023 18:21:16 -0700 Subject: [PATCH 06/16] clean up prints --- x/concentrated-liquidity/math/tick.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/x/concentrated-liquidity/math/tick.go b/x/concentrated-liquidity/math/tick.go index fead77e946a..7a92b62c1fb 100644 --- a/x/concentrated-liquidity/math/tick.go +++ b/x/concentrated-liquidity/math/tick.go @@ -285,8 +285,6 @@ func CalculateSqrtPriceToTick(sqrtPrice sdk.Dec) (tickIndex int64, err error) { if errM1 != nil || errT != nil || errP1 != nil || errP2 != nil { return 0, errors.New("internal error in computing square roots within CalculateSqrtPriceToTick") } - fmt.Println("sqrtPriceTplus1: ", sqrtPriceTplus1) - fmt.Println("sqrtPriceTplus2: ", sqrtPriceTplus2) // 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. From d5b382183c6268c93fd91ecb5cbd33a9cbfc5b07 Mon Sep 17 00:00:00 2001 From: alpo Date: Thu, 15 Jun 2023 19:44:17 -0700 Subject: [PATCH 07/16] enable relevant randomized test cases --- x/concentrated-liquidity/invariant_test.go | 6 ++--- x/concentrated-liquidity/position_test.go | 29 ++++++++++------------ x/concentrated-liquidity/range_test.go | 12 ++++----- 3 files changed, 22 insertions(+), 25 deletions(-) diff --git a/x/concentrated-liquidity/invariant_test.go b/x/concentrated-liquidity/invariant_test.go index 6453401d0ec..e2dcfd03e55 100644 --- a/x/concentrated-liquidity/invariant_test.go +++ b/x/concentrated-liquidity/invariant_test.go @@ -144,10 +144,10 @@ func (s *KeeperTestSuite) assertWithdrawAllInvariant() { totalWithdrawn = totalWithdrawn.Add(withdrawn...) } - // We allow for an additive tolerance of 1 per position in the pool, since this is the maximum that can be truncated - // when collecting spread rewards. + // We allow for an additive tolerance of 2 per position in the pool, since we expect up to 1 to be lost to rounding on + // join and withdrawal each. errTolerance := osmomath.ErrTolerance{ - AdditiveTolerance: sdk.NewDec(int64(len(allPositions))), + AdditiveTolerance: sdk.NewDec(2 * int64(len(allPositions))), RoundingDir: osmomath.RoundDown, } diff --git a/x/concentrated-liquidity/position_test.go b/x/concentrated-liquidity/position_test.go index 4242c89e826..ac008dd1c7f 100644 --- a/x/concentrated-liquidity/position_test.go +++ b/x/concentrated-liquidity/position_test.go @@ -2540,22 +2540,19 @@ func (s *KeeperTestSuite) TestMultipleRanges() { 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, - // }, + "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 diff --git a/x/concentrated-liquidity/range_test.go b/x/concentrated-liquidity/range_test.go index 4983760d907..7f874c0b61e 100644 --- a/x/concentrated-liquidity/range_test.go +++ b/x/concentrated-liquidity/range_test.go @@ -47,7 +47,7 @@ type RangeTestParams struct { var ( DefaultRangeTestParams = RangeTestParams{ // Base amounts - baseNumPositions: 1000, + baseNumPositions: 100, baseAssets: sdk.NewCoins(sdk.NewCoin(ETH, sdk.NewInt(5000000000)), sdk.NewCoin(USDC, sdk.NewInt(5000000000))), baseTimeBetweenJoins: time.Hour, baseSwapAmount: sdk.NewInt(10000000), @@ -109,7 +109,8 @@ func (s *KeeperTestSuite) setupRangesAndAssertInvariants(pool types.Concentrated // Set up assets for new position curAssets := getRandomizedAssets(testParams.baseAssets, testParams.fuzzAssets) - s.FundAcc(curAddr, curAssets) + roundingError := sdk.NewCoins(sdk.NewCoin(pool.GetToken0(), sdk.OneInt()), sdk.NewCoin(pool.GetToken1(), sdk.OneInt())) + s.FundAcc(curAddr, curAssets.Add(roundingError...)) // Set up position curPositionId, actualAmt0, actualAmt1, curLiquidity, actualLowerTick, actualUpperTick, err := s.clk.CreatePosition(s.Ctx, pool.GetId(), curAddr, curAssets, sdk.ZeroInt(), sdk.ZeroInt(), ranges[curRange][0], ranges[curRange][1]) @@ -122,7 +123,6 @@ func (s *KeeperTestSuite) setupRangesAndAssertInvariants(pool types.Concentrated // Let time elapse after join if applicable timeElapsed := s.addRandomizedBlockTime(testParams.baseTimeBetweenJoins, testParams.fuzzTimeBetweenJoins) - s.assertGlobalInvariants() // Execute swap against pool if applicable swappedIn, swappedOut := s.executeRandomizedSwap(pool, swapAddresses, testParams.baseSwapAmount, testParams.fuzzSwapAmounts) @@ -149,7 +149,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()) - s.Require().Equal(totalAssets, poolAssets) + poolSpreadRewards := s.App.BankKeeper.GetAllBalances(s.Ctx, pool.GetSpreadRewardsAddress()) + s.Require().Equal(totalAssets, 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. @@ -187,7 +188,7 @@ func (s *KeeperTestSuite) prepareNumPositionSlice(ranges [][]int64, baseNumPosit // TODO: Make swaps that target getting to a tick boundary exactly func (s *KeeperTestSuite) executeRandomizedSwap(pool types.ConcentratedPoolExtension, swapAddresses []sdk.AccAddress, baseSwapAmount sdk.Int, fuzzSwap bool) (sdk.Coin, sdk.Coin) { // Quietly skip if no swap assets or swap addresses provided - if baseSwapAmount.Equal(sdk.Int{}) || len(swapAddresses) == 0 { + if (baseSwapAmount == sdk.Int{}) || len(swapAddresses) == 0 { return sdk.Coin{}, sdk.Coin{} } @@ -250,7 +251,6 @@ func (s *KeeperTestSuite) addRandomizedBlockTime(baseTimeToAdd time.Duration, fu } s.AddBlockTime(timeToAdd) - } return baseTimeToAdd From 1fa59a807adb1d5836846279f6fb163dd7eac9c0 Mon Sep 17 00:00:00 2001 From: alpo Date: Fri, 16 Jun 2023 19:09:38 -0700 Subject: [PATCH 08/16] relax exact rounding error checks in global invariants to allow for overrounding in pool's favor --- x/concentrated-liquidity/invariant_test.go | 20 +++++++++++--------- x/concentrated-liquidity/range_test.go | 2 +- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/x/concentrated-liquidity/invariant_test.go b/x/concentrated-liquidity/invariant_test.go index e2dcfd03e55..b1c750ae806 100644 --- a/x/concentrated-liquidity/invariant_test.go +++ b/x/concentrated-liquidity/invariant_test.go @@ -87,11 +87,12 @@ func (s *KeeperTestSuite) assertTotalRewardsInvariant() { totalCollectedIncentives = totalCollectedIncentives.Add(collectedIncentives...) } - // We allow for an additive tolerance of 1 per position in the pool, since this is the maximum that can be truncated - // when collecting spread rewards. + // For global invariant checks, we simply ensure that any rounding error was in the pool's favor. + // This is to allow for cases where we slightly overround, which would otherwise fail here. + // TODO: create ErrTolerance type that allows for additive OR multiplicative tolerance to allow for + // tightening this check further. errTolerance := osmomath.ErrTolerance{ - AdditiveTolerance: sdk.NewDec(int64(len(allPositions))), - RoundingDir: osmomath.RoundDown, + RoundingDir: osmomath.RoundDown, } // Assert total collected spread rewards and incentives equal to expected @@ -144,15 +145,16 @@ func (s *KeeperTestSuite) assertWithdrawAllInvariant() { totalWithdrawn = totalWithdrawn.Add(withdrawn...) } - // We allow for an additive tolerance of 2 per position in the pool, since we expect up to 1 to be lost to rounding on - // join and withdrawal each. + // For global invariant checks, we simply ensure that any rounding error was in the pool's favor. + // This is to allow for cases where we slightly overround, which would otherwise fail here. + // TODO: create ErrTolerance type that allows for additive OR multiplicative tolerance to allow for + // tightening this check further. errTolerance := osmomath.ErrTolerance{ - AdditiveTolerance: sdk.NewDec(2 * int64(len(allPositions))), - RoundingDir: osmomath.RoundDown, + RoundingDir: osmomath.RoundDown, } // Assert total withdrawn assets equal to expected - s.Require().True(errTolerance.EqualCoins(expectedTotalWithdrawn, totalWithdrawn)) + s.Require().True(errTolerance.EqualCoins(expectedTotalWithdrawn, totalWithdrawn), "expected withdrawn vs. actual: %s vs. %s", expectedTotalWithdrawn, totalWithdrawn) // Refetch total pool balances across all pools remainingPositions, finalTotalPoolAssets, remainingTotalSpreadRewards, remainingTotalIncentives := s.getAllPositionsAndPoolBalances(cachedCtx) diff --git a/x/concentrated-liquidity/range_test.go b/x/concentrated-liquidity/range_test.go index 7f874c0b61e..e733c6265ce 100644 --- a/x/concentrated-liquidity/range_test.go +++ b/x/concentrated-liquidity/range_test.go @@ -47,7 +47,7 @@ type RangeTestParams struct { var ( DefaultRangeTestParams = RangeTestParams{ // Base amounts - baseNumPositions: 100, + baseNumPositions: 10, baseAssets: sdk.NewCoins(sdk.NewCoin(ETH, sdk.NewInt(5000000000)), sdk.NewCoin(USDC, sdk.NewInt(5000000000))), baseTimeBetweenJoins: time.Hour, baseSwapAmount: sdk.NewInt(10000000), From 2a4f1c4cd6e3c4b14da70dceeb0fbf92eaa45faa Mon Sep 17 00:00:00 2001 From: alpo Date: Sun, 18 Jun 2023 10:37:57 -0700 Subject: [PATCH 09/16] clean up comments --- x/concentrated-liquidity/math/tick.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/x/concentrated-liquidity/math/tick.go b/x/concentrated-liquidity/math/tick.go index 7a92b62c1fb..0f9b78ad6d3 100644 --- a/x/concentrated-liquidity/math/tick.go +++ b/x/concentrated-liquidity/math/tick.go @@ -250,10 +250,8 @@ func CalculatePriceToTick(price sdk.Dec) (tickIndex int64) { return calculatePriceToTickDec(price).RoundInt64() } -// CalculatePriceToTick takes in a square root and returns the corresponding tick index. +// CalculateSqrtPriceToTick takes in a square root 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 sqrt prices in range [TickToSqrtPrice(b), TickToSqrtPrice(b+1)). 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. @@ -268,7 +266,11 @@ func CalculateSqrtPriceToTick(sqrtPrice sdk.Dec) (tickIndex int64, err error) { // * 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 the max tick, set the candidate to max tick - 2. + // 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 From 08bd83f286f1ab2ecd2b493717b95569eb73acb1 Mon Sep 17 00:00:00 2001 From: alpo Date: Sun, 18 Jun 2023 14:42:16 -0700 Subject: [PATCH 10/16] move unused functions outside the state machine --- app/apptesting/concentrated_liquidity.go | 56 +++++++++++++++++ tests/cl-genesis-positions/convert.go | 12 +++- x/concentrated-liquidity/keeper_test.go | 13 ++-- x/concentrated-liquidity/math/math_test.go | 8 ++- x/concentrated-liquidity/math/tick.go | 60 +------------------ x/concentrated-liquidity/math/tick_test.go | 16 ++--- .../spread_rewards_test.go | 3 +- x/concentrated-liquidity/swaps_test.go | 12 ++-- x/concentrated-liquidity/tick_test.go | 2 +- 9 files changed, 98 insertions(+), 84 deletions(-) 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 6d1c02a1268..96e1fc047ea 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 cfa3cbc22d6..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.PriceToTickRoundDownSpacing(sdk.NewDec(5500), DefaultTickSpacing) - DefaultExponentConsecutivePositionUpperTick, _ = math.PriceToTickRoundDownSpacing(sdk.NewDec(6250), DefaultTickSpacing) - DefaultExponentOverlappingPositionLowerTick, _ = math.PriceToTickRoundDownSpacing(sdk.NewDec(4000), DefaultTickSpacing) - DefaultExponentOverlappingPositionUpperTick, _ = math.PriceToTickRoundDownSpacing(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/math/math_test.go b/x/concentrated-liquidity/math/math_test.go index 30da7a6dbfc..057dca9bbfb 100644 --- a/x/concentrated-liquidity/math/math_test.go +++ b/x/concentrated-liquidity/math/math_test.go @@ -20,9 +20,11 @@ func TestConcentratedTestSuite(t *testing.T) { suite.Run(t, new(ConcentratedMathTestSuite)) } -var sqrt4545 = sdk.MustNewDecFromStr("67.416615162732695594") -var sqrt5000 = sdk.MustNewDecFromStr("70.710678118654752440") -var sqrt5500 = sdk.MustNewDecFromStr("74.161984870956629487") +var ( + Sqrt4545 = sdk.MustNewDecFromStr("67.416615162732695594") + Sqrt5000 = sdk.MustNewDecFromStr("70.710678118654752440") + Sqrt5500 = sdk.MustNewDecFromStr("74.161984870956629487") +) // liquidity1 takes an amount of asset1 in the pool as well as the sqrtpCur and the nextPrice // sqrtPriceA is the smaller of sqrtpCur and the nextPrice diff --git a/x/concentrated-liquidity/math/tick.go b/x/concentrated-liquidity/math/tick.go index 0f9b78ad6d3..1e74ba16174 100644 --- a/x/concentrated-liquidity/math/tick.go +++ b/x/concentrated-liquidity/math/tick.go @@ -92,28 +92,6 @@ 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 - } - - 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 := CalculatePriceToTick(price) - - return tickIndex, 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 @@ -141,26 +119,6 @@ func RoundDownTickToSpacing(tickIndex int64, tickSpacing int64) (int64, error) { 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. -// NOTE: This should not be called in the state machine. -// 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 PriceToTickRoundDownSpacing(price sdk.Dec, tickSpacing uint64) (int64, error) { - tickIndex, err := PriceToTick(price) - if err != nil { - return 0, err - } - - tickIndex, err = RoundDownTickToSpacing(tickIndex, int64(tickSpacing)) - if err != nil { - return 0, err - } - - return tickIndex, nil -} - // 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) { @@ -193,7 +151,7 @@ func powTenBigDec(exponent int64) osmomath.BigDec { return bigNegPowersOfTen[-exponent] } -func calculatePriceToTickDec(price sdk.Dec) (tickIndex sdk.Dec) { +func CalculatePriceToTickDec(price sdk.Dec) (tickIndex sdk.Dec) { if price.Equal(sdkOneDec) { return sdk.ZeroDec() } @@ -236,27 +194,13 @@ func calculatePriceToTickDec(price sdk.Dec) (tickIndex sdk.Dec) { return tickIndex } -// CalculatePriceToTick takes in a price and returns the corresponding tick index. -// This function does not take into consideration tick spacing. -// NOTE: This should not be called in the state machine. -// 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 CalculatePriceToTick(price sdk.Dec) (tickIndex int64) { - // TODO: Make truncate, since this defines buckets as - // [TickToPrice(b - .5), TickToPrice(b+.5)) - return calculatePriceToTickDec(price).RoundInt64() -} - // 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) + tick := CalculatePriceToTickDec(price) truncatedTick := tick.TruncateInt64() // We have a candidate bucket index `t`. We discern here if: diff --git a/x/concentrated-liquidity/math/tick_test.go b/x/concentrated-liquidity/math/tick_test.go index d88b7333015..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.PriceToTickRoundDownSpacing(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.PriceToTickRoundDownSpacing(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.PriceToTickRoundDownSpacing(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.PriceToTickRoundDownSpacing(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.PriceToTickRoundDownSpacing(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,7 +709,7 @@ 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) }) } diff --git a/x/concentrated-liquidity/spread_rewards_test.go b/x/concentrated-liquidity/spread_rewards_test.go index abc8f354628..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.PriceToTickRoundDownSpacing(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_test.go b/x/concentrated-liquidity/swaps_test.go index 43d0e7bce10..1c5f0163b2f 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.PriceToTickRoundDownSpacing(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.PriceToTickRoundDownSpacing(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.PriceToTickRoundDownSpacing(test.newLowerPrice, pool.GetTickSpacing()) + newLowerTick, err := s.PriceToTickRoundDownSpacing(test.newLowerPrice, pool.GetTickSpacing()) s.Require().NoError(err) - newUpperTick, err := math.PriceToTickRoundDownSpacing(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.PriceToTickRoundDownSpacing(test.secondPositionLowerPrice, pool.GetTickSpacing()) + newLowerTick, err := s.PriceToTickRoundDownSpacing(test.secondPositionLowerPrice, pool.GetTickSpacing()) s.Require().NoError(err) - newUpperTick, err := math.PriceToTickRoundDownSpacing(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_test.go b/x/concentrated-liquidity/tick_test.go index 826840df302..dfc06bfd340 100644 --- a/x/concentrated-liquidity/tick_test.go +++ b/x/concentrated-liquidity/tick_test.go @@ -1240,7 +1240,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) From 83fcbcfbff707f132fca6476b6862d0156d2bc42 Mon Sep 17 00:00:00 2001 From: alpo Date: Sun, 18 Jun 2023 15:34:27 -0700 Subject: [PATCH 11/16] fix conflicts --- x/concentrated-liquidity/swaps.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/x/concentrated-liquidity/swaps.go b/x/concentrated-liquidity/swaps.go index a2f6bab6384..a0e20017084 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 } From 13b213baf1ae738a5de44305de53f0d05e6679dc Mon Sep 17 00:00:00 2001 From: alpo Date: Sun, 18 Jun 2023 16:13:02 -0700 Subject: [PATCH 12/16] clean up tests --- x/concentrated-liquidity/math/math_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/x/concentrated-liquidity/math/math_test.go b/x/concentrated-liquidity/math/math_test.go index 057dca9bbfb..d3044b359eb 100644 --- a/x/concentrated-liquidity/math/math_test.go +++ b/x/concentrated-liquidity/math/math_test.go @@ -21,9 +21,9 @@ func TestConcentratedTestSuite(t *testing.T) { } var ( - Sqrt4545 = sdk.MustNewDecFromStr("67.416615162732695594") - Sqrt5000 = sdk.MustNewDecFromStr("70.710678118654752440") - Sqrt5500 = sdk.MustNewDecFromStr("74.161984870956629487") + sqrt4545 = sdk.MustNewDecFromStr("67.416615162732695594") + sqrt5000 = sdk.MustNewDecFromStr("70.710678118654752440") + sqrt5500 = sdk.MustNewDecFromStr("74.161984870956629487") ) // liquidity1 takes an amount of asset1 in the pool as well as the sqrtpCur and the nextPrice From 7e0c38c17c0a8a151e3db658230cb48ebf7a2442 Mon Sep 17 00:00:00 2001 From: alpo Date: Sun, 18 Jun 2023 16:18:31 -0700 Subject: [PATCH 13/16] remove empty tick handling file in favor of future readme section on ticks --- x/concentrated-liquidity/math/TickHandling.md | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 x/concentrated-liquidity/math/TickHandling.md diff --git a/x/concentrated-liquidity/math/TickHandling.md b/x/concentrated-liquidity/math/TickHandling.md deleted file mode 100644 index e69de29bb2d..00000000000 From 5e3b29121dc0ccbeac05a692f74da20cbfdc7fc9 Mon Sep 17 00:00:00 2001 From: alpo Date: Sun, 18 Jun 2023 16:35:06 -0700 Subject: [PATCH 14/16] clean up diff --- x/concentrated-liquidity/math/math_test.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/x/concentrated-liquidity/math/math_test.go b/x/concentrated-liquidity/math/math_test.go index d3044b359eb..30da7a6dbfc 100644 --- a/x/concentrated-liquidity/math/math_test.go +++ b/x/concentrated-liquidity/math/math_test.go @@ -20,11 +20,9 @@ func TestConcentratedTestSuite(t *testing.T) { suite.Run(t, new(ConcentratedMathTestSuite)) } -var ( - sqrt4545 = sdk.MustNewDecFromStr("67.416615162732695594") - sqrt5000 = sdk.MustNewDecFromStr("70.710678118654752440") - sqrt5500 = sdk.MustNewDecFromStr("74.161984870956629487") -) +var sqrt4545 = sdk.MustNewDecFromStr("67.416615162732695594") +var sqrt5000 = sdk.MustNewDecFromStr("70.710678118654752440") +var sqrt5500 = sdk.MustNewDecFromStr("74.161984870956629487") // liquidity1 takes an amount of asset1 in the pool as well as the sqrtpCur and the nextPrice // sqrtPriceA is the smaller of sqrtpCur and the nextPrice From 7c6f2562d6895e5ba3618512d28333aaa0cb14eb Mon Sep 17 00:00:00 2001 From: alpo Date: Mon, 19 Jun 2023 08:12:31 -0700 Subject: [PATCH 15/16] clean up boundary check --- x/concentrated-liquidity/math/tick.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/concentrated-liquidity/math/tick.go b/x/concentrated-liquidity/math/tick.go index 1e74ba16174..4023929d5b0 100644 --- a/x/concentrated-liquidity/math/tick.go +++ b/x/concentrated-liquidity/math/tick.go @@ -237,7 +237,7 @@ func CalculateSqrtPriceToTick(sqrtPrice sdk.Dec) (tickIndex int64, err error) { // 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)) || sqrtPrice.LT(sqrtPriceTmin1) || sqrtPrice.GT(sqrtPriceTplus2) { + 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) } From d621b423f070502a0529c8a92e69f986ff0e19ae Mon Sep 17 00:00:00 2001 From: alpo Date: Mon, 19 Jun 2023 08:20:50 -0700 Subject: [PATCH 16/16] enable remaining fuzz tests --- x/concentrated-liquidity/position_test.go | 25 +++++++++-------------- x/concentrated-liquidity/range_test.go | 3 ++- 2 files changed, 12 insertions(+), 16 deletions(-) diff --git a/x/concentrated-liquidity/position_test.go b/x/concentrated-liquidity/position_test.go index ac008dd1c7f..a76330af396 100644 --- a/x/concentrated-liquidity/position_test.go +++ b/x/concentrated-liquidity/position_test.go @@ -2553,21 +2553,16 @@ func (s *KeeperTestSuite) TestMultipleRanges() { }, 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 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.