Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 6 commits into from
Sep 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
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
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())
Comment on lines +21 to 22
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to reviewer:

tracking these here: #6370

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this pr can resolve this TODO #6409


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()
Comment on lines +30 to 31
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: this should be addressed by #6261, CC: @pysel

}

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()
Comment on lines +52 to 53
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: this should be addressed by #6261, CC: @pysel

}

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()
p0mvn marked this conversation as resolved.
Show resolved Hide resolved
}
// 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
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
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