fix(CL): error blow up in CalcAmount0Delta causes OverChargeSwapOutGivenInError when swapping zero for one

CHANGELOG.md

@@ -45,6 +45,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Bug Fixes
 
 * [#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.
 
 ### API Breaks
 

osmoutils/osmoassert/assertions.go

@@ -60,9 +60,13 @@ func messageFromMsgAndArgs(msgAndArgs ...interface{}) string {
 	return ""
 }
 
+type Stringer interface {
+	String() string
+}
+
 // Equal compares A with B and asserts that they are equal within tolerance error tolerance
-func Equal[T any](t *testing.T, tolerance osmomath.ErrTolerance, A, B T) {
-	errMsg := fmt.Sprintf("expected %T, actual %T", A, B)
+func Equal[T Stringer](t *testing.T, tolerance osmomath.ErrTolerance, A, B T) {
+	errMsg := fmt.Sprintf("expected %s, actual %s", A.String(), B.String())
 	switch a := any(A).(type) {
 	case osmomath.Int:
 		b, ok := any(B).(osmomath.Int)
@@ -81,5 +85,8 @@ func Equal[T any](t *testing.T, tolerance osmomath.ErrTolerance, A, B T) {
 		failNowIfNot(t, ok)
 
 		require.True(t, tolerance.CompareDec(a, b) == 0, errMsg)
+	default:
+		require.FailNow(t, "unsupported types")
 	}
+
 }

x/concentrated-liquidity/math/math.go

@@ -66,22 +66,20 @@ func CalcAmount0Delta(liq, sqrtPriceA, sqrtPriceB osmomath.BigDec, roundUp bool)
 	// additionally, without rounding, there exists cases where the swapState.amountSpecifiedRemaining.IsPositive() for loop within
 	// the CalcOut/In functions never actually reach zero due to dust that would have never gotten counted towards the amount (numbers after the 10^6 place)
 	if roundUp {
-		// Note that we do MulTruncate so that the denominator is smaller as this is
+		// Note that we do MulRoundUp so that the numerator is larger as this is
 		// the case where we want to round up to favor the pool.
+		// For the same reasons, QuoRoundUp to round up at precision end after division.
 		// Examples include:
 		// - 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.
-		denom := sqrtPriceA.MulTruncate(sqrtPriceB)
-		return liq.Mul(diff).QuoMut(denom).Ceil()
+		return liq.MulRoundUp(diff).QuoRoundUp(sqrtPriceA).QuoRoundUp(sqrtPriceB).Ceil()
 	}
 	// These are truncated at precision end to round in favor of the pool when:
 	// - calculating amount out during swap
 	// - withdrawing liquidity
-	// The denominator is rounded up to get a smaller final amount.
-	denom := sqrtPriceA.MulRoundUp(sqrtPriceB)
-
-	return liq.MulTruncate(diff).QuoTruncate(denom)
+	// Each intermediary step is truncated at precision end to get a smaller final amount.
+	return liq.MulTruncate(diff).QuoTruncate(sqrtPriceA).QuoTruncate(sqrtPriceB)
 }
 
 // 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

x/concentrated-liquidity/math/math_test.go

@@ -158,6 +158,39 @@ func TestCalcAmount0Delta(t *testing.T) {
 			amount0Expected: osmomath.MustNewBigDecFromStr("6098022989717817431593106314408.88812810159039320984467945943").Ceil(), // rounded up at precision end.
 			isWithTolerance: true,
 		},
+		// See: https://github.com/osmosis-labs/osmosis/issues/6351 for details.
+		// The values were taken from the failing swap on the development branch that reproduced the issue.
+		"edge case: low sqrt prices may cause error amplification if incorrect order of operations (round up)": {
+			// from decimal import *
+			// from math import *
+			// getcontext().prec = 100
+			// max_sqrt_p = Decimal("0.00000099994999874993749609347654199")
+			// min_sqrt_p = Decimal("0.000000000000001409841835100661211756")
+			// liq = Decimal("5000252259822539816806336.971796256914465071095518135400579243")
+			// liq * (max_sqrt_p - min_sqrt_p) / (max_sqrt_p * min_sqrt_p)
+			liquidity:       osmomath.MustNewBigDecFromStr("5000252259822539816806336.971796256914465071095518135400579243"),
+			sqrtPA:          osmomath.MustNewBigDecFromStr("0.00000099994999874993749609347654199"),
+			sqrtPB:          osmomath.MustNewBigDecFromStr("0.000000000000001409841835100661211756"),
+			roundUp:         true,
+			amount0Expected: osmomath.MustNewBigDecFromStr("3546676037185128488234786333758360815266.999539026068480181194797910898392880").Ceil(),
+		},
+		// See: https://github.com/osmosis-labs/osmosis/issues/6351 for details.
+		// The values were taken from the failing swap on the development branch that reproduced the issue.
+		"edge case: low sqrt prices may cause error amplification if incorrect order of operations (round down)": {
+			// from decimal import *
+			// from math import *
+			// getcontext().prec = 100
+			// max_sqrt_p = Decimal("0.00000099994999874993749609347654199")
+			// min_sqrt_p = Decimal("0.000000000000001409841835100661211756")
+			// liq = Decimal("5000252259822539816806336.971796256914465071095518135400579243")
+			// liq * (max_sqrt_p - min_sqrt_p) / (max_sqrt_p * min_sqrt_p)
+			liquidity:       osmomath.MustNewBigDecFromStr("5000252259822539816806336.971796256914465071095518135400579243"),
+			sqrtPA:          osmomath.MustNewBigDecFromStr("0.00000099994999874993749609347654199"),
+			sqrtPB:          osmomath.MustNewBigDecFromStr("0.000000000000001409841835100661211756"),
+			roundUp:         false,
+			isWithTolerance: true,
+			amount0Expected: osmomath.MustNewBigDecFromStr("3546676037185128488234786333758360815266.999539026068480181194797910898392880"),
+		},
 	}
 
 	for name, tc := range testCases {

x/concentrated-liquidity/swapstrategy/one_for_zero_test.go

@@ -159,7 +159,9 @@ func (suite *StrategyTestSuite) TestComputeSwapStepOutGivenIn_OneForZero() {
 			expectedAmountOut:               osmomath.ZeroDec(),
 			expectedSpreadRewardChargeTotal: osmomath.ZeroDec(),
 		},
-		"7: invalid zero difference between sqrt price current and sqrt price next due to precision loss, full amount remaining in is charged and amount out calculated from sqrt price": {
+		// This edge case does not occur anymore. The fix observed in PR: https://github.com/osmosis-labs/osmosis/pull/6352
+		// See linked issue for detals of the change.
+		"7: (fixed) invalid zero difference between sqrt price current and sqrt price next due to precision loss, full amount remaining in is charged and amount out calculated from sqrt price": {
 			// Note the numbers are hand-picked to reproduce this specific case.
 			sqrtPriceCurrent: osmomath.BigDecFromDec(osmomath.MustNewDecFromStr("0.000001000049998750")),
 			sqrtPriceTarget:  osmomath.MustNewDecFromStr("0.000001000049998751"),
@@ -175,12 +177,21 @@ func (suite *StrategyTestSuite) TestComputeSwapStepOutGivenIn_OneForZero() {
 			expectedAmountInConsumed: osmomath.NewDec(99),
 			// liquidity * (sqrtPriceNext - sqrtPriceCurrent) / (sqrtPriceNext * sqrtPriceCurrent)
 			// calculated with x/concentrated-liquidity/python/clmath.py
+			// # Calculate amount in until sqrtPriceTarget
+			// amountIn = calc_amount_one_delta(liquidity, sqrtPriceCurrent, sqrtPriceTarget, True)
+			// Decimal('100.002498062401598791937822606808718081')
+			// # Greater than amountOneInRemaining => calculate sqrtPriceNext
+			//
+			// amountOneInRemaining = Decimal('99')
+			// sqrtPriceNext = get_next_sqrt_price_from_amount1_in_round_down(liquidity, sqrtPriceCurrent, amountOneInRemaining)
+			// Decimal("0.000001000049998750989975269800000000")
+			//
 			// diff = (sqrtPriceNext - sqrtPriceCurrent)
-			// diff = round_decimal(diff, 36, ROUND_FLOOR) (0.000000000000000000989975269800000000)
-			// mul = (sqrtPriceNext * sqrtPriceCurrent)
-			// mul = round_decimal(mul, 36, ROUND_CEILING) (0.000000000001000100000000865026329827)
-			//  round_decimal(liquidity * diff / mul, 36, ROUND_FLOOR)
-			expectedAmountOut:               osmomath.MustNewBigDecFromStr("98990100989815.389417309844929293132374729779331247").Dec(),
+			// diff = round_decimal(diff, 36, ROUND_FLOOR)
+			// mul = round_decimal(liquidity * diff, 36, ROUND_FLOOR)
+			// div1= round_decimal(mul / sqrtPriceNext, 36, ROUND_FLOOR)
+			// round_decimal(div1 / sqrtPriceCurrent, 36, ROUND_FLOOR)
+			expectedAmountOut:               osmomath.MustNewBigDecFromStr("98990100989815.389417309941839547862158319016747061").Dec(),
 			expectedSpreadRewardChargeTotal: osmomath.ZeroDec(),
 		},
 		"8: invalid zero difference between sqrt price current and sqrt price next due to precision loss. Returns 0 for amounts out. Note that the caller should detect this and fail.": {