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(CL): change tick API from sdk.Dec to osmomath.BigDec #6032

Closed
Tracked by #5726
p0mvn opened this issue Aug 13, 2023 · 0 comments · Fixed by #6033
Closed
Tracked by #5726

refactor(CL): change tick API from sdk.Dec to osmomath.BigDec #6032

p0mvn opened this issue Aug 13, 2023 · 0 comments · Fixed by #6033
Assignees
Labels
F: concentrated-liquidity Tracking the development of concentrated liquidity feature to improve filtering on the project board

Comments

@p0mvn
Copy link
Member

p0mvn commented Aug 13, 2023

Background

Prior to launch, we made a change to the precision of low-level swap calculations from sdk.Dec to osmomath.BigDec

This was made due to the lack of precision in intermediary calculations, leading to undesired behavior.

As part of the earlier precision increase refactor in #5612, we changed the core math but not the APIs. For example,

func (s oneForZeroStrategy) ComputeSwapWithinBucketOutGivenIn(sqrtPriceCurrent osmomath.BigDec, sqrtPriceTarget, liquidity, amountOneInRemaining sdk.Dec) (osmomath.BigDec, sdk.Dec, sdk.Dec, sdk.Dec) {
sqrtPriceTargetBigDec := osmomath.BigDecFromSDKDec(sqrtPriceTarget)
liquidityBigDec := osmomath.BigDecFromSDKDec(liquidity)
amountOneInRemainingBigDec := osmomath.BigDecFromSDKDec(amountOneInRemaining)

Note that we take an 18 decimal sdk.Dec input and immediately convert it to 36 decimal osmomath.BigDec output.

Currently, we are dealing with a production problem where our minimum supported spot price is too high. As a result, it prevents us from supporting certain base and quote pairs. Especially, where the base is precision 18, the quote is 6 and the real spot price is small, taking us under the minimum supported of 10^-12.

One of the solutions is to lower our minimum spot price simply. However, our lower bound is constrained by the non-monotonicity in tick <> sqrt price conversions. Ref: #6019

It has been shown in #6020 that increasing precision to 36 in our tick <> sqrt price conversions, makes the non-monotonicity issues to be resolved.

Now, we run into a problem in that there are several CL pools in production with a minimum spot price bound of 10^-12 and we are to add start adding pools with a new one of 10^-30. As a result, we have to decide how to maintain both minimum spot prices.

One approach is to perform a migration. However, it would be too risky and controversial since user positions would have to be moved. The reason is that the notion of full range changes. Therefore, some superfluid positions with 10^-12 min spot price would not have the same minimum anymore.

Suggested Design

Instead, we propose to maintain 2 minimum spot prices.

  • v1 - 10^-12. No pool can be created with this min spot price after the next major upgrade
  • v2 - 10^-30. Every pool is created with this spot price after the next major upgrade

As part of this decision, we will have to snapshot the latest CL pool id during the upgrade to let us decide which minimum spot price to use for each pool.

Additionally, some internal tick <> sqrt price conversion functions will either use the sdk.Dec (for v1) implementation or osmomath.BigDec (for v2).

To minimize code smells from having to continuously check whether a pool is v1 or v2, we propose implementing a shared osmomath.BigDec API. By implementing a strategy and factory design patterns, we will load the appropriate math library that will use the v1 or v2 implementations under the hood.

As a result, this will allow us to relax the min supported spot price in the least intrusive way while also not requiring migrations.

Acceptance Criteria

  • API is changed from sdk.Dec to osmomath.BigDec in a non-state-breaking way
  • Tests refactored where applicable
@p0mvn p0mvn added the F: concentrated-liquidity Tracking the development of concentrated liquidity feature to improve filtering on the project board label Aug 13, 2023
@p0mvn p0mvn self-assigned this Aug 13, 2023
@osmosis-labs osmosis-labs deleted a comment from p0mvn Aug 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F: concentrated-liquidity Tracking the development of concentrated liquidity feature to improve filtering on the project board
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant