Skip to content

Commit

Permalink
refactor/test(CL): Stricter rounding behavior in CL math methods; uni…
Browse files Browse the repository at this point in the history 
…t tests at low price level (#6369)

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

* comment updates

* clarifying comment

* remove comment
  • Loading branch information
@p0mvn
p0mvn authored Sep 14, 2023
1 parent 3f15d3d commit 9362621
Show file tree
Hide file tree
Showing 4 changed files with 117 additions and 7 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,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

Expand Down
20 changes: 16 additions & 4 deletions x/concentrated-liquidity/math/math.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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()
}

Expand All @@ -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()
}

Expand Down Expand Up @@ -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. This only matters in cases where at least one of the sqrt prices is below 1.
// 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
Expand Down Expand Up @@ -124,11 +134,13 @@ func GetNextSqrtPriceFromAmount0InRoundingUp(sqrtPriceCurrent, liquidity, amount
return sqrtPriceCurrent
}

product := amountZeroRemainingIn.Mul(sqrtPriceCurrent)
// Truncate at precision end to make denominator smaller so that the final result is larger.
product := amountZeroRemainingIn.MulTruncate(sqrtPriceCurrent)
// denominator = product + liquidity
denominator := product
denominator.AddMut(liquidity)
return liquidity.Mul(sqrtPriceCurrent).QuoRoundUp(denominator)
// MulRoundUp and QuoRoundUp to make the final result larger by rounding up at precision end.
return liquidity.MulRoundUp(sqrtPriceCurrent).QuoRoundUp(denominator)
}

// GetNextSqrtPriceFromAmount0OutRoundingUp utilizes sqrtPriceCurrent, liquidity, and amount of denom0 that still needs
Expand Down
97 changes: 97 additions & 0 deletions x/concentrated-liquidity/math/math_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -277,6 +319,26 @@ 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
amount1Expected: osmomath.MustNewBigDecFromStr("0.000000000000000000000000000103787163").Ceil(),
},
}

for name, tc := range testCases {
Expand Down Expand Up @@ -449,6 +511,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)
}
Expand All @@ -470,6 +540,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)
}
Expand Down Expand Up @@ -499,6 +577,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)
}
Expand All @@ -519,6 +605,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)
}
6 changes: 3 additions & 3 deletions x/concentrated-liquidity/python/clmath.py
View file
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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:
Expand Down

0 comments on commit 9362621

Please sign in to comment.