refactor/test(CL): Stricter rounding behavior in CL math methods; unit tests at low price level

CHANGELOG.md

@@ -46,6 +46,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 * [#6334](https://github.com/osmosis-labs/osmosis/pull/6334) fix: enable taker fee cli
 * [#6352](https://github.com/osmosis-labs/osmosis/pull/6352) Reduce error blow-up in CalcAmount0Delta by changing the order of math operations.
+* [#6368](https://github.com/osmosis-labs/osmosis/pull/6368) Stricter rounding behavior in CL math's CalcAmount0Delta and GetNextSqrtPriceFromAmount0InRoundingUp
 
 ### API Breaks
 

x/concentrated-liquidity/math/math.go

@@ -18,6 +18,7 @@ func Liquidity0(amount osmomath.Int, sqrtPriceA, sqrtPriceB osmomath.BigDec) osm
 
 	// We convert to BigDec to avoid precision loss when calculating liquidity. Without doing this,
 	// our liquidity calculations will be off from our theoretical calculations within our tests.
+	// TODO (perf): consider better conversion helpers to minimize reallocations.
 	amountBigDec := osmomath.BigDecFromDec(amount.ToLegacyDec())
 
 	product := sqrtPriceA.Mul(sqrtPriceB)
@@ -26,6 +27,7 @@ func Liquidity0(amount osmomath.Int, sqrtPriceA, sqrtPriceB osmomath.BigDec) osm
 		panic(fmt.Sprintf("liquidity0 diff is zero: sqrtPriceA %s sqrtPriceB %s", sqrtPriceA, sqrtPriceB))
 	}
 
+	// TODO (perf): consider Dec() function that does not reallocate
 	return amountBigDec.MulMut(product).QuoMut(diff).Dec()
 }
 
@@ -40,12 +42,14 @@ func Liquidity1(amount osmomath.Int, sqrtPriceA, sqrtPriceB osmomath.BigDec) osm
 
 	// We convert to BigDec to avoid precision loss when calculating liquidity. Without doing this,
 	// our liquidity calculations will be off from our theoretical calculations within our tests.
+	// TODO (perf): consider better conversion helpers to minimize reallocations.
 	amountBigDec := osmomath.BigDecFromDec(amount.ToLegacyDec())
 	diff := sqrtPriceB.Sub(sqrtPriceA)
 	if diff.IsZero() {
 		panic(fmt.Sprintf("liquidity1 diff is zero: sqrtPriceA %s sqrtPriceB %s", sqrtPriceA, sqrtPriceB))
 	}
 
+	// TODO (perf): consider Dec() function that does not reallocate
 	return amountBigDec.QuoMut(diff).Dec()
 }
 
@@ -73,13 +77,19 @@ func CalcAmount0Delta(liq, sqrtPriceA, sqrtPriceB osmomath.BigDec, roundUp bool)
 		// - calculating amountIn during swap
 		// - adding liquidity (request user to provide more tokens in in favor of the pool)
 		// The denominator is truncated to get a higher final amount.
-		return liq.MulRoundUp(diff).QuoRoundUp(sqrtPriceA).QuoRoundUp(sqrtPriceB).Ceil()
+		// Note that the order of divisions is important here. First, we divide by a larger number (sqrtPriceB) and then by a smaller number (sqrtPriceA).
+		// This leads to a smaller error amplification.
+		// TODO (perf): QuoRoundUpMut with no reallocation.
+		return liq.MulRoundUp(diff).QuoRoundUp(sqrtPriceB).QuoRoundUp(sqrtPriceA).Ceil()
 	}
 	// These are truncated at precision end to round in favor of the pool when:
 	// - calculating amount out during swap
 	// - withdrawing liquidity
 	// Each intermediary step is truncated at precision end to get a smaller final amount.
-	return liq.MulTruncate(diff).QuoTruncate(sqrtPriceA).QuoTruncate(sqrtPriceB)
+	// Note that the order of divisions is important here. First, we divide by a larger number (sqrtPriceB) and then by a smaller number (sqrtPriceA).
+	// This leads to a smaller error amplification.
+	// TODO (perf): QuoTruncate with no reallocation.
+	return liq.MulTruncate(diff).QuoTruncate(sqrtPriceB).QuoTruncate(sqrtPriceA)
 }
 
 // CalcAmount1Delta takes the asset with the smaller liquidity in the pool as well as the sqrtpCur and the nextPrice and calculates the amount of asset 1
@@ -124,11 +134,11 @@ func GetNextSqrtPriceFromAmount0InRoundingUp(sqrtPriceCurrent, liquidity, amount
 		return sqrtPriceCurrent
 	}
 
-	product := amountZeroRemainingIn.Mul(sqrtPriceCurrent)
+	product := amountZeroRemainingIn.MulTruncate(sqrtPriceCurrent)
 	// denominator = product + liquidity
 	denominator := product
 	denominator.AddMut(liquidity)
-	return liquidity.Mul(sqrtPriceCurrent).QuoRoundUp(denominator)
+	return liquidity.MulRoundUp(sqrtPriceCurrent).QuoRoundUp(denominator)
 }
 
 // GetNextSqrtPriceFromAmount0OutRoundingUp utilizes sqrtPriceCurrent, liquidity, and amount of denom0 that still needs

x/concentrated-liquidity/math/math_test.go

@@ -19,6 +19,17 @@ var (
 	sqrt4545BigDec = osmomath.BigDecFromDec(sqrt4545)
 	sqrt5000BigDec = osmomath.BigDecFromDec(sqrt5000)
 	sqrt5500BigDec = osmomath.BigDecFromDec(sqrt5500)
+
+	// sqrt(10 ^-36 * 567) 36 decimals
+	// chosen arbitrarily for testing the extended price range
+	sqrtANearMin = osmomath.MustNewBigDecFromStr("0.000000000000000023811761799581315315")
+	// sqrt(10 ^-36 * 123567) 36 decimals
+	// chosen arbitrarily for testing the extended price range
+	sqrtBNearMin = osmomath.MustNewBigDecFromStr("0.000000000000000351520980881653776714")
+	// This value is estimated using liquidity1 function in clmath.py between sqrtANearMin and sqrtBNearMin.
+	smallLiquidity = osmomath.MustNewBigDecFromStr("0.000000000000316705045072312223884779")
+	// Arbitrary small value, exists to test small movement over the low price range.
+	smallValue = osmomath.MustNewBigDecFromStr("10.12345678912345671234567891234567")
 )
 
 // liquidity1 takes an amount of asset1 in the pool as well as the sqrtpCur and the nextPrice
@@ -39,6 +50,15 @@ func TestLiquidity1(t *testing.T) {
 			expectedLiquidity: "1517882343.751510418088349649",
 			// https://www.wolframalpha.com/input?i=5000000000+%2F+%2870.710678118654752440+-+67.416615162732695594%29
 		},
+		"low price range": {
+			currentSqrtP:   sqrtANearMin,
+			sqrtPLow:       sqrtBNearMin,
+			amount1Desired: osmomath.NewInt(5000000000),
+			// from math import *
+			// from decimal import *
+			// amount1 / (sqrtPriceB - sqrtPriceA)
+			expectedLiquidity: "15257428564277849269508363.222206252646611708",
+		},
 	}
 
 	for name, tc := range testCases {
@@ -76,6 +96,18 @@ func TestLiquidity0(t *testing.T) {
 			expectedLiquidity: "1519437308.014768571720923239",
 			// https://www.wolframalpha.com/input?i=1000000+*+%2870.710678118654752440*+74.161984870956629487%29+%2F+%2874.161984870956629487+-+70.710678118654752440%29
 		},
+		"low price range": {
+			currentSqrtP:   sqrtANearMin,
+			sqrtPHigh:      sqrtBNearMin,
+			amount0Desired: osmomath.NewInt(123999),
+			// from clmath import *
+			// from math import *
+			// product1 = round_osmo_prec_down(sqrtPriceA * sqrtPriceB)
+			// product2 = round_osmo_prec_down(amount0 * product1)
+			// diff = round_osmo_prec_down(sqrtPriceB - sqrtPriceA)
+			// round_sdk_prec_down(product2 / diff)
+			expectedLiquidity: "0.000000000003167050",
+		},
 	}
 
 	for name, tc := range testCases {
@@ -191,6 +223,16 @@ func TestCalcAmount0Delta(t *testing.T) {
 			isWithTolerance: true,
 			amount0Expected: osmomath.MustNewBigDecFromStr("3546676037185128488234786333758360815266.999539026068480181194797910898392880"),
 		},
+		"low price range": {
+			liquidity: smallLiquidity,
+			sqrtPA:    sqrtANearMin,
+			sqrtPB:    sqrtBNearMin,
+			roundUp:   false,
+			// from clmath decimal import *
+			// from math import *
+			// calc_amount_zero_delta(liq, sqrtPriceA, sqrtPriceB, False)
+			amount0Expected: osmomath.MustNewBigDecFromStr("12399.405290456300691064448232516066947340"),
+		},
 	}
 
 	for name, tc := range testCases {
@@ -277,6 +319,27 @@ func TestCalcAmount1Delta(t *testing.T) {
 			roundUp:         true,
 			amount1Expected: osmomath.MustNewBigDecFromStr("28742157707995443393876876754535992.801567623738751734").Ceil(), // round up at precision end.
 		},
+		"low price range (no round up)": {
+			liquidity: smallLiquidity,
+			sqrtPA:    sqrtANearMin,
+			sqrtPB:    sqrtBNearMin,
+			roundUp:   false,
+			// from clmath decimal import *
+			// from math import *
+			// calc_amount_one_delta(liq, sqrtPriceA, sqrtPriceB, False)
+			amount1Expected: osmomath.MustNewBigDecFromStr("0.000000000000000000000000000103787162"),
+		},
+		"low price range (with round up)": {
+			liquidity: smallLiquidity,
+			sqrtPA:    sqrtANearMin,
+			sqrtPB:    sqrtBNearMin,
+			roundUp:   true,
+			// from clmath decimal import *
+			// calc_amount_one_delta(liq, sqrtPriceA, sqrtPriceB, False)
+			// Actual result: 0.000000000000000000000000000103787163
+			// Gets rounded up to 1. Is this acceptable when the multiplicative difference is so large?
+			amount1Expected: osmomath.MustNewBigDecFromStr("0.000000000000000000000000000103787163").Ceil(),
+		},
 	}
 
 	for name, tc := range testCases {
@@ -449,6 +512,14 @@ func TestGetNextSqrtPriceFromAmount0InRoundingUp(t *testing.T) {
 			// round_osmo_prec_up(liquidity / (round_osmo_prec_down(liquidity / sqrtPriceCurrent) + amountRemaining))
 			expected: osmomath.MustNewBigDecFromStr("70.666663910857144331148691821263626767"),
 		},
+		"low price range": {
+			liquidity:        smallLiquidity,
+			sqrtPriceCurrent: sqrtANearMin,
+			amountRemaining:  smallValue,
+			// from clmath decimal import *
+			// get_next_sqrt_price_from_amount0_in_round_up(liq, sqrtPriceA, amountRemaining)
+			expected: osmomath.MustNewBigDecFromStr("0.000000000000000023793654323441728435"),
+		},
 	}
 	runSqrtRoundingTestCase(t, "TestGetNextSqrtPriceFromAmount0InRoundingUp", math.GetNextSqrtPriceFromAmount0InRoundingUp, tests)
 }
@@ -470,6 +541,14 @@ func TestGetNextSqrtPriceFromAmount0OutRoundingUp(t *testing.T) {
 			// liq * sqrt_cur / (liq + token_out * sqrt_cur) = 2.5
 			expected: osmomath.MustNewBigDecFromStr("2.5"),
 		},
+		"low price range": {
+			liquidity:        smallLiquidity,
+			sqrtPriceCurrent: sqrtANearMin,
+			amountRemaining:  smallValue,
+			// from clmath decimal import *
+			// get_next_sqrt_price_from_amount0_out_round_up(liq, sqrtPriceA, amountRemaining)
+			expected: osmomath.MustNewBigDecFromStr("0.000000000000000023829902587267894423"),
+		},
 	}
 	runSqrtRoundingTestCase(t, "TestGetNextSqrtPriceFromAmount0OutRoundingUp", math.GetNextSqrtPriceFromAmount0OutRoundingUp, tests)
 }
@@ -499,6 +578,14 @@ func TestGetNextSqrtPriceFromAmount1InRoundingDown(t *testing.T) {
 			// calculated with x/concentrated-liquidity/python/clmath.py  round_decimal(sqrt_next, 36, ROUND_FLOOR)
 			expected: osmomath.MustNewBigDecFromStr("70.738319930382329008049494613660784220"),
 		},
+		"low price range": {
+			liquidity:        smallLiquidity,
+			sqrtPriceCurrent: sqrtANearMin,
+			amountRemaining:  smallValue,
+			// from clmath decimal import *
+			// get_next_sqrt_price_from_amount1_in_round_down(liq, sqrtPriceA, amountRemaining)
+			expected: osmomath.MustNewBigDecFromStr("31964936923603.477920799226065501544948016880497639"),
+		},
 	}
 	runSqrtRoundingTestCase(t, "TestGetNextSqrtPriceFromAmount1InRoundingDown", math.GetNextSqrtPriceFromAmount1InRoundingDown, tests)
 }
@@ -519,6 +606,17 @@ func TestGetNextSqrtPriceFromAmount1OutRoundingDown(t *testing.T) {
 			// round_osmo_prec_down(sqrtPriceCurrent - round_osmo_prec_up(tokenOut / liquidity))
 			expected: osmomath.MustNewBigDecFromStr("2.5"),
 		},
+		"low price range": {
+			liquidity:        smallLiquidity,
+			sqrtPriceCurrent: sqrtANearMin,
+			amountRemaining:  smallValue,
+			// from clmath decimal import *
+			// get_next_sqrt_price_from_amount1_out_round_down(liq, sqrtPriceA, amountRemaining)
+			// While a negative sqrt price value is invalid and should be caught by the caller,
+			// we mostly focus on testing rounding behavior and math correctness at low spot prices.
+			// For the purposes of our test, this result is acceptable.
+			expected: osmomath.MustNewBigDecFromStr("-31964936923603.477920799226065453921424417717867010"),
+		},
 	}
 	runSqrtRoundingTestCase(t, "TestGetNextSqrtPriceFromAmount1OutRoundingDown", math.GetNextSqrtPriceFromAmount1OutRoundingDown, tests)
 }

x/concentrated-liquidity/python/clmath.py

@@ -1,4 +1,5 @@
 from decimal import Decimal, ROUND_FLOOR, ROUND_CEILING, getcontext
+from math import *
 
 class Coin:
     # Define this class based on what fields sdk.Coin has.
@@ -114,12 +115,11 @@ def calc_amount_zero_delta(liquidity, sqrtPriceA, sqrtPriceB, roundUp):
     diff = sqrtPriceA - sqrtPriceB
 
     product_num = liquidity * diff
-    product_denom = sqrtPriceA * sqrtPriceB
 
     if roundUp:
-        return round_osmo_prec_up(round_osmo_prec_up(product_num) / round_osmo_prec_down(product_denom))
+        return round_osmo_prec_up(round_osmo_prec_up(round_osmo_prec_up(product_num) / sqrtPriceA) / sqrtPriceB)
 
-    return round_osmo_prec_down(round_osmo_prec_down(product_num) / round_osmo_prec_up(product_denom))
+    return round_osmo_prec_down(round_osmo_prec_down(round_osmo_prec_down(product_num) / sqrtPriceA)/ sqrtPriceB)
 
 def calc_amount_one_delta(liquidity, sqrtPriceA, sqrtPriceB, roundUp):
     if sqrtPriceB > sqrtPriceA: