[CL]: Fix bound check bug for liquidity amounts

x/concentrated-liquidity/incentives.go

@@ -202,7 +202,10 @@ func (k Keeper) prepareBalancerPoolAsFullRange(ctx sdk.Context, clPoolId uint64,
 	// Calculate the amount of liquidity the Balancer amounts qualify in the CL pool. Note that since we use the CL spot price, this is
 	// safe against prices drifting apart between the two pools (we take the lower bound on the qualifying liquidity in this case).
 	// The `sqrtPriceLowerTick` and `sqrtPriceUpperTick` fields are set to the appropriate values for a full range position.
-	qualifyingFullRangeSharesPreDiscount := math.GetLiquidityFromAmounts(clPool.GetCurrentSqrtPrice(), types.MinSqrtPrice, types.MaxSqrtPrice, asset0Amount, asset1Amount)
+	qualifyingFullRangeSharesPreDiscount, err := math.GetLiquidityFromAmounts(clPool.GetCurrentSqrtPrice(), types.MinSqrtPrice, types.MaxSqrtPrice, asset0Amount, asset1Amount, clPool.GetTickSpacing())
+	if err != nil {
+		return 0, sdk.ZeroDec(), err
+	}
 
 	// Get discount ratio from governance-set discount rate. Note that the case we check for is technically impossible, but we include
 	// the check as a guardrail anyway. Specifically, we error if the discount ratio is not [0, 1].

x/concentrated-liquidity/incentives_test.go

@@ -1046,7 +1046,8 @@ func (s *KeeperTestSuite) TestUpdateUptimeAccumulatorsToNow() {
 					clPool, err = clKeeper.GetPoolById(s.Ctx, clPool.GetId())
 					s.Require().NoError(err)
 					if tc.canonicalBalancerPoolAssets != nil {
-						qualifyingBalancerLiquidityPreDiscount := math.GetLiquidityFromAmounts(clPool.GetCurrentSqrtPrice(), types.MinSqrtPrice, types.MaxSqrtPrice, tc.canonicalBalancerPoolAssets[0].Token.Amount, tc.canonicalBalancerPoolAssets[1].Token.Amount)
+						qualifyingBalancerLiquidityPreDiscount, err := math.GetLiquidityFromAmounts(clPool.GetCurrentSqrtPrice(), types.MinSqrtPrice, types.MaxSqrtPrice, tc.canonicalBalancerPoolAssets[0].Token.Amount, tc.canonicalBalancerPoolAssets[1].Token.Amount, clPool.GetTickSpacing())
+						s.Require().NoError(err)
 						qualifyingBalancerLiquidity = (sdk.OneDec().Sub(types.DefaultBalancerSharesDiscount)).Mul(qualifyingBalancerLiquidityPreDiscount)
 						qualifyingLiquidity = qualifyingLiquidity.Add(qualifyingBalancerLiquidity)
 
@@ -3365,7 +3366,8 @@ func (s *KeeperTestSuite) TestPrepareBalancerPoolAsFullRange() {
 				s.Require().NoError(err)
 				asset0BalancerAmount := tc.balancerPoolAssets[0].Token.Amount.ToDec().Mul(tc.portionOfSharesBonded).TruncateInt()
 				asset1BalancerAmount := tc.balancerPoolAssets[1].Token.Amount.ToDec().Mul(tc.portionOfSharesBonded).TruncateInt()
-				qualifyingSharesPreDiscount := math.GetLiquidityFromAmounts(updatedClPool.GetCurrentSqrtPrice(), types.MinSqrtPrice, types.MaxSqrtPrice, asset1BalancerAmount, asset0BalancerAmount)
+				qualifyingSharesPreDiscount, err := math.GetLiquidityFromAmounts(updatedClPool.GetCurrentSqrtPrice(), types.MinSqrtPrice, types.MaxSqrtPrice, asset1BalancerAmount, asset0BalancerAmount, updatedClPool.GetTickSpacing())
+				s.Require().NoError(err)
 				qualifyingShares := (sdk.OneDec().Sub(types.DefaultBalancerSharesDiscount)).Mul(qualifyingSharesPreDiscount)
 
 				// TODO: clean this check up (will likely require refactoring the whole test)

x/concentrated-liquidity/lp.go

@@ -94,7 +94,10 @@ func (k Keeper) createPosition(ctx sdk.Context, poolId uint64, owner sdk.AccAddr
 	}
 
 	// Calculate the amount of liquidity that will be added to the pool when this position is created.
-	liquidityDelta = math.GetLiquidityFromAmounts(pool.GetCurrentSqrtPrice(), sqrtPriceLowerTick, sqrtPriceUpperTick, amount0Desired, amount1Desired)
+	liquidityDelta, err = math.GetLiquidityFromAmounts(pool.GetCurrentSqrtPrice(), sqrtPriceLowerTick, sqrtPriceUpperTick, amount0Desired, amount1Desired, pool.GetTickSpacing())
+	if err != nil {
+		return 0, sdk.Int{}, sdk.Int{}, sdk.Dec{}, time.Time{}, 0, 0, err
+	}
 	if liquidityDelta.IsZero() {
 		return 0, sdk.Int{}, sdk.Int{}, sdk.Dec{}, time.Time{}, 0, 0, errors.New("liquidityDelta calculated equals zero")
 	}

x/concentrated-liquidity/math/math.go

@@ -171,16 +171,53 @@ func GetNextSqrtPriceFromAmount1OutRoundingDown(sqrtPriceCurrent, liquidity, amo
 
 // GetLiquidityFromAmounts takes the current sqrtPrice and the sqrtPrice for the upper and lower ticks as well as the amounts of asset0 and asset1
 // and returns the resulting liquidity from these inputs.
-func GetLiquidityFromAmounts(sqrtPrice, sqrtPriceA, sqrtPriceB sdk.Dec, amount0, amount1 sdk.Int) (liquidity sdk.Dec) {
+func GetLiquidityFromAmounts(sqrtPrice, sqrtPriceA, sqrtPriceB sdk.Dec, amount0, amount1 sdk.Int, tickSpacing uint64) (liquidity sdk.Dec, err error) {
 	// Reorder the prices so that sqrtPriceA is the smaller of the two.
 	if sqrtPriceA.GT(sqrtPriceB) {
 		sqrtPriceA, sqrtPriceB = sqrtPriceB, sqrtPriceA
 	}
 
-	if sqrtPrice.LTE(sqrtPriceA) {
+	// Retrieve rounded ticks to do appropriate bound checks
+	lowerTick, err := PriceToTickRoundDown(sqrtPriceA.Power(2), tickSpacing)
+	if err != nil {
+		return sdk.ZeroDec(), err
+	}
+	upperTick, err := PriceToTickRoundDown(sqrtPriceB.Power(2), tickSpacing)
+	if err != nil {
+		return sdk.ZeroDec(), err
+	}
+	currentTick, err := PriceToTickRoundDown(sqrtPrice.Power(2), tickSpacing)
+	if err != nil {
+		return sdk.ZeroDec(), err
+	}
+
+	/* TODO: decide on keeping or removing this based on whether sqrt prices should align with rounded tick boundaries
+	//
+	// Validate that the original sqrt price values fell on tick spacing bounds.
+	// While this is an expensive check, it is critical to ensure that invalid state
+	// is never reached.
+	_, inverseCurSqrtPrice, err := TickToSqrtPrice(currentTick)
+	if err != nil {
+		return sdk.ZeroDec(), err
+	}
+	_, inverseLowerSqrtPrice, err := TickToSqrtPrice(lowerTick)
+	if err != nil {
+		return sdk.ZeroDec(), err
+	}
+	_, inverseUpperSqrtPrice, err := TickToSqrtPrice(upperTick)
+	if err != nil {
+		return sdk.ZeroDec(), err
+	}
+
+	if !sqrtPriceA.Equal(inverseLowerSqrtPrice) || !sqrtPrice.Equal(inverseCurSqrtPrice) || !sqrtPriceB.Equal(inverseUpperSqrtPrice) {
+		return sdk.ZeroDec(), types.InvalidSqrtPriceBoundError{LowerSqrtPrice: sqrtPriceA, CurrentSqrtPrice: sqrtPrice, UpperSqrtPrice: sqrtPriceB}
+	}
+	*/
+
+	if currentTick < lowerTick {
 		// If the current price is less than or equal to the lower tick, then we use the liquidity0 formula.
 		liquidity = Liquidity0(amount0, sqrtPriceA, sqrtPriceB)
-	} else if sqrtPrice.LTE(sqrtPriceB) {
+	} else if currentTick < upperTick {
 		// If the current price is between the lower and upper ticks (inclusive of the lower tick but not the upper tick),
 		// then we use the minimum of the liquidity0 and liquidity1 formulas.
 		liquidity0 := Liquidity0(amount0, sqrtPrice, sqrtPriceB)
@@ -191,7 +228,7 @@ func GetLiquidityFromAmounts(sqrtPrice, sqrtPriceA, sqrtPriceB sdk.Dec, amount0,
 		liquidity = Liquidity1(amount1, sqrtPriceB, sqrtPriceA)
 	}
 
-	return liquidity
+	return liquidity, nil
 }
 
 // SquareRoundUp squares and rounds up at precision end.

x/concentrated-liquidity/math/math_test.go

@@ -20,7 +20,12 @@ func TestConcentratedTestSuite(t *testing.T) {
 	suite.Run(t, new(ConcentratedMathTestSuite))
 }
 
-var sqrt5000 = sdk.MustNewDecFromStr("70.710678118654752440")
+var (
+	sqrt4545 = sdk.MustNewDecFromStr("67.416615162732695594")
+	sqrt5000 = sdk.MustNewDecFromStr("70.710678118654752440")
+	sqrt5001 = sdk.MustNewDecFromStr("70.717748832948578242")
+	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
@@ -330,27 +335,29 @@ func (suite *ConcentratedMathTestSuite) TestGetLiquidityFromAmounts() {
 		// price of x in terms of y
 		expectedLiquidity0 sdk.Dec
 		expectedLiquidity1 sdk.Dec
+
+		expectedError error
 	}{
 		"happy path (case A)": {
-			currentSqrtP:      sdk.MustNewDecFromStr("67"),                    // 4489
-			sqrtPHigh:         sdk.MustNewDecFromStr("74.161984870956629487"), // 5500
-			sqrtPLow:          sdk.MustNewDecFromStr("67.416615162732695594"), // 4545
+			currentSqrtP:      sdk.MustNewDecFromStr("67"), // sqrt(4489)
+			sqrtPHigh:         sqrt5500,
+			sqrtPLow:          sqrt4545,
 			amount0Desired:    sdk.NewInt(1000000),
 			amount1Desired:    sdk.ZeroInt(),
 			expectedLiquidity: "741212151.448720111852782017",
 		},
 		"happy path (case B)": {
-			currentSqrtP:      sqrt5000,                                       // 5000
-			sqrtPHigh:         sdk.MustNewDecFromStr("74.161984870956629487"), // 5500
-			sqrtPLow:          sdk.MustNewDecFromStr("67.416615162732695594"), // 4545
+			currentSqrtP:      sqrt5000,
+			sqrtPHigh:         sqrt5500,
+			sqrtPLow:          sqrt4545,
 			amount0Desired:    sdk.NewInt(1000000),
 			amount1Desired:    sdk.NewInt(5000000000),
 			expectedLiquidity: "1517882343.751510418088349649",
 		},
 		"happy path (case C)": {
-			currentSqrtP:      sdk.MustNewDecFromStr("75"),                    // 5625
-			sqrtPHigh:         sdk.MustNewDecFromStr("74.161984870956629487"), // 5500
-			sqrtPLow:          sdk.MustNewDecFromStr("67.416615162732695594"), // 4545
+			currentSqrtP:      sdk.MustNewDecFromStr("75"), // sqrt(5625)
+			sqrtPHigh:         sqrt5500,
+			sqrtPLow:          sqrt4545,
 			amount0Desired:    sdk.ZeroInt(),
 			amount1Desired:    sdk.NewInt(5000000000),
 			expectedLiquidity: "741249214.836069764856625637",
@@ -388,6 +395,49 @@ func (suite *ConcentratedMathTestSuite) TestGetLiquidityFromAmounts() {
 			expectedLiquidity0: sdk.MustNewDecFromStr("7.706742302257039729"),
 			expectedLiquidity1: sdk.MustNewDecFromStr("4.828427124746190095"),
 		},
+		"bound check: current price equal to upper price": {
+			currentSqrtP:   sqrt5500,
+			sqrtPHigh:      sqrt5500,
+			sqrtPLow:       sqrt4545,
+			amount0Desired: sdk.NewInt(1000000),
+			amount1Desired: sdk.NewInt(5000000000),
+			// We expect Liquidity1 to be returned since having current sqrt price equal to upper sqrt price
+			// should be considered above range.
+			// The specific calculation is:
+			//     expectedLiquidity = amount1Desired / (sqrtPHigh - sqrtPLow)
+			// where we use raw values for sqrt5500 & sqrt4545 that are truncated to 18 decimal places:
+			// https://www.wolframalpha.com/input?i=5000000000%2F%2874.161984870956629487-67.416615162732695594%29
+			expectedLiquidity: "741249214.836069764856625637",
+		},
+
+		/* TODO: determine whether this panicking is expected behavior or if curTick <= lowTick should default to the Liquidity0 branch
+		"bound check: current price on lower tick": {
+			currentSqrtP:   sqrt4545,
+			sqrtPHigh:      sqrt5500,
+			sqrtPLow:       sqrt4545,
+			amount0Desired: sdk.NewInt(1000000),
+			amount1Desired: sdk.NewInt(5000000000),
+			// We expect Liquidity0 to be returned since having current sqrt price equal to lower sqrt price
+			// should be considered within range.
+			// The specific calculation is:
+			//     expectedLiquidity = amount0Desired * (sqrtPHigh * sqrtPLow) / (sqrtPHigh - sqrtPLow)
+			// where we use raw values for sqrt5500 & sqrt4545 that are truncated to 18 decimal places:
+			expectedLiquidity: "741212151.448720111852782017",
+		},
+		*/
+
+		/* TODO: convert this to non error case if the rounded sqrt price check is removed
+		"error: current price does not comply with tick spacing requirements": {
+			currentSqrtP:      sqrt5000.Add(sdk.MustNewDecFromStr("0.0001")),
+			sqrtPHigh:         sqrt5000,
+			sqrtPLow:          sqrt4545,
+			amount0Desired:    sdk.NewInt(1000000),
+			amount1Desired:    sdk.NewInt(5000000000),
+			expectedLiquidity: "0.000000000000000000",
+
+			expectedError: cltypes.InvalidSqrtPriceBoundError{LowerSqrtPrice: sqrt4545, CurrentSqrtPrice: sqrt5000.Add(sdk.MustNewDecFromStr("0.0001")), UpperSqrtPrice: sqrt5000},
+		},
+		*/
 	}
 
 	for name, tc := range testCases {
@@ -398,7 +448,15 @@ func (suite *ConcentratedMathTestSuite) TestGetLiquidityFromAmounts() {
 			// CASE B: if the currentSqrtP is less than the sqrtPHigh but greater than sqrtPLow, the liquidity is split between asset0 and asset1,
 			// so GetLiquidityFromAmounts returns the smaller liquidity of asset0 and asset1
 			// CASE C: if the currentSqrtP is greater than the sqrtPHigh, all the liquidity is in asset1, so GetLiquidityFromAmounts returns the liquidity of asset1
-			liquidity := math.GetLiquidityFromAmounts(tc.currentSqrtP, tc.sqrtPLow, tc.sqrtPHigh, tc.amount0Desired, tc.amount1Desired)
+			// TODO: make default tick spacing into a test field and test multiple authorized tick spacings
+			liquidity, err := math.GetLiquidityFromAmounts(tc.currentSqrtP, tc.sqrtPLow, tc.sqrtPHigh, tc.amount0Desired, tc.amount1Desired, defaultTickSpacing)
+
+			if tc.expectedError != nil {
+				suite.Require().ErrorContains(err, tc.expectedError.Error())
+			} else {
+				suite.Require().NoError(err)
+			}
+
 			suite.Require().Equal(tc.expectedLiquidity, liquidity.String())
 		})
 	}

x/concentrated-liquidity/swaps_test.go

@@ -1544,7 +1544,8 @@ func (s *KeeperTestSuite) getExpectedLiquidity(test SwapTest, pool types.Concent
 		test.poolLiqAmount1 = DefaultAmt1
 	}
 
-	expectedLiquidity := math.GetLiquidityFromAmounts(DefaultCurrSqrtPrice, lowerSqrtPrice, upperSqrtPrice, test.poolLiqAmount0, test.poolLiqAmount1)
+	expectedLiquidity, err := math.GetLiquidityFromAmounts(DefaultCurrSqrtPrice, lowerSqrtPrice, upperSqrtPrice, test.poolLiqAmount0, test.poolLiqAmount1, pool.GetTickSpacing())
+	s.Require().NoError(err)
 	return expectedLiquidity
 }
 

x/concentrated-liquidity/swapstrategy/one_for_zero_test.go

@@ -61,6 +61,9 @@ func (suite *StrategyTestSuite) TestComputeSwapStepOutGivenIn_OneForZero() {
 		}
 	)
 
+	customPriceLiquidity, err := math.GetLiquidityFromAmounts(sqrt(1), sqrt(100_000_000), sqrt(100_000_100), defaultAmountZero.TruncateInt(), defaultAmountOne.TruncateInt(), defaultTickSpacing)
+	suite.Require().NoError(err)
+
 	tests := map[string]struct {
 		sqrtPriceCurrent     sdk.Dec
 		sqrtPriceTarget      sdk.Dec
@@ -134,7 +137,7 @@ func (suite *StrategyTestSuite) TestComputeSwapStepOutGivenIn_OneForZero() {
 		"5: custom amounts at high price levels - reach target": {
 			sqrtPriceCurrent: sqrt(100_000_000),
 			sqrtPriceTarget:  sqrt(100_000_100),
-			liquidity:        math.GetLiquidityFromAmounts(sqrt(1), sqrt(100_000_000), sqrt(100_000_100), defaultAmountZero.TruncateInt(), defaultAmountOne.TruncateInt()),
+			liquidity:        customPriceLiquidity,
 
 			// this value is exactly enough to reach the target
 			amountOneInRemaining: sdk.NewDec(1336900668450),

x/concentrated-liquidity/types/errors.go

@@ -833,3 +833,13 @@ type TickToSqrtPriceConversionError struct {
 func (e TickToSqrtPriceConversionError) Error() string {
 	return fmt.Sprintf("could not convert next tick  to nextSqrtPrice (%v)", e.NextTick)
 }
+
+type InvalidSqrtPriceBoundError struct {
+	LowerSqrtPrice   sdk.Dec
+	CurrentSqrtPrice sdk.Dec
+	UpperSqrtPrice   sdk.Dec
+}
+
+func (e InvalidSqrtPriceBoundError) Error() string {
+	return fmt.Sprintf("attempted to run bound checks on invalid sqrt prices. Lower (%s), current (%s), upper (%s)", e.LowerSqrtPrice, e.CurrentSqrtPrice, e.UpperSqrtPrice)
+}