feat: geometric tick spacing with additive ranges

[czarcas7ic]

Geometric Tick Spacing with Additive Ranges ADR

Context

In Uniswap V3, discrete points (called ticks) are used when providing liquidity in a concentrated liquidity pool. The price [p] corresponding to a tick [t] is defined by the equation:

$$ p(i) = 1.0001^t $$

This results in a .01% difference between adjacent tick prices. However, this does not allow for control over the specific prices that the ticks correspond to. For example, if a user wants to make a limit order at the $17,100.50 price point, they would have to interact with either tick 97473 (corresponding to price $17,099.60) or tick 97474 (price $17101.30).

Since we know what range a pair will generally trade in, how do we go about providing more granularity at that range and provide a more optimal price range between ticks instead of the "one-size-fits-all" approach explained above?

Decision

In Osmosis' implementation of concentrated liquidity, we will instead make use of geometric tick spacing with additive ranges.

We start by defining a precision factor at a spot price of one ($k_{p1}$).

For instance, if $k_{p1} = -4$ , then each tick starting at 1 and ending at the first factor of 10 will represents a spot price increase of 0.0001. At this precision factor:

• $tick_0 = 1$ (tick 0 is always equal to 1 regardless of precision factor)
• $tick_1 = 1.0001$
• $tick_2 = 1.0002$
• $tick_3 = 1.0003$

This continues on until we reach a spot price of 10. At this point, since we have increased by a factor of 10, our $k_{current}$ increases from -4 to -3, and the ticks will increase as follows:

• $tick_{90000} = 10$
• $tick_{90001} = 10.001$
• $tick_{90002} = 10.002$

For spot prices less than a dollar, the precision factor decreases at every factor of 10. For example, with a $k_{p1}$ of -4:

• $tick_{-1} = 0.9999$
• $tick_{-2} = 0.9998$
• $tick_{-5001} = 0.4999$
• $tick_{-5002} = 0.4998$

With a $k_{p1}$ of -6:

• $tick_{-1} = 0.999999$
• $tick_{-2} = 0.999998$
• $tick_{-5001} = 0.994999$
• $tick_{-5002} = 0.994998$

Formulas

After we define $k_{p1}$ (this is chosen by the user based on what precision they desire the asset pair to trade at), we can then calculate how many ticks must be crossed in order for k to be incremented ($kIncrementDistance$):

$$ kIncrementDistance = 9 * 10^{(-k_{p1})} $$

Now that we know how many ticks must be crossed in order for our k to be incremented, we can then figure out what our change in k will be based on what tick we are trading at:

$$ kΔ = ⌊ tick / kIncrementDistance ⌋ $$

With $kΔ$ and $k_{p1}$, we can figure out what the k value we will be at when we reach the provided tick:

$$ k_{current} = k_{p1} + kΔ $$

Knowing what our $k_{current}$ is, we must then figure out what power of 10 this k corresponds to:

$$ currentIncrement = 10^{(k_{current})} $$

Lastly, we must determine how many ticks above the current increment we are at:

$$ numAdditiveTicks = tick - (kΔ * kIncrementDistance) $$

With this, we can determine the price:

$$ price = (10^{kΔ}) + (numAdditiveTicks * currentIncrement) $$

Example

Bob sets a limit order on the USD<>BTC pool at tick 36650010. This pool's $k_{p1}$ is -6. What price did Bob set his limit order at?

$$ kIncrementDistance = 9 * 10^{(6)} = 9000000$$

$$ kΔ = ⌊ 36650010 / 9000000 ⌋ = 4$$

$$ k_{current} = -6 + 4 = -2$$

$$ currentIncrement = 10^{(-2)} = 0.01$$

$$ numAdditiveTicks = 36650010 - (4 * 9000000) = 650010$$

$$ price = (10^{4}) + (650010 * 0.01) = $16,500.10$$

Bob set his limit order at price $16,500.10

Consequences

This decision allows us to define ticks at spot prices that users actually desire to trade on, rather than arbitrarily defining ticks at .01% distance between each other. This will also make integration with UX seamless, instead of either

a) Preventing trade at a desirable spot price or
b) Having the front end round the tick's actual price to the nearest human readable/desirable spot price

One draw back of this implementation is the requirement to create many ticks that will likely never be used. For example, in order to create ticks at 10 cent increments for spot prices greater than $10000, a $k_{p1}$ value of -5 must be set, requiring us to traverse ticks 1-3600000 before reaching $10,000. This should simply be an inconvenience and should not present any valid DOS vector for the chain.

[p0mvn]

Why doesn't the wolfram result below match this?

[czarcas7ic]

We are off from multiplying and then dividing the sdk.Dec. I can fix this if we want to transform the sqrtPriceA and sqrtPriceB into BigDecs when calculating 0 delta. What do you think?

[p0mvn]

Nice work. Reviewed logic so far but not tests.

Could you please also commit the ADR into the architecture.md?

[p0mvn]

What is the recason for having TickSpacing by the user? Isn't it determined by PrecisionFactorAtPriceOne dynamically?

[czarcas7ic]

No, I think we need to find some different verbiage to explain this since its really easy to get confused. This tick spacing is referring to the ticks that are authorized to be initialized, i.e if this number is 5, then only ticks 0, 5, 10 etc can be initialized. The tick spacing you are referring to is the prices that each and every tick reference, which changes based on k value

[p0mvn]

With our new model for the prices that each and every tick reference that are based on k value, what is the use case for restricting people to provide liquidity at any tick possible given this new model?

In other words, why do we still want to restrict authorized ticks?

[p0mvn]

TickSpacing does not seem to be sued anywhere other than validation in withdrawPosition and createPosition

[czarcas7ic]

I think it actually serves an important use case! Let me give an example:

Imagine we have a BTC<>USD pool trading in the $XXXX.XX range, and we want to trade at 10 cent ticks. Instead of launching the pool with with a kAtPriceOne value of -5, we can instead launch a BTC<>USD pool with tick spacing of 10 and a kAtPriceOne value of -6. This way, it gives us maneuver space if BTC were to go to $100,000. Instead of having to create a whole new pool, we can just change the tick spacing value to 1!

The problem with this is we cant easily bump this back up to 10 once we go to 1, so if we went from $100,000 to say $90,000 we would have to make a new pool.

The way I forsee it, if an asset were to have some crazy pump this is just a "fail-safe" for a lack of a better term that we can trigger while we decide if we want to make a new pool.

Please lmk your thoughts!

[AlpinYukseloglu]

In your example, who would have the right to update tick spacing? Is the goal to make it a governance-changeable parameter?

[czarcas7ic]

Yeah feels like gov param would make the most sense

[AlpinYukseloglu]

Hmm normally protocol governance shouldn't be voting on individual pool parameters, but I can see the fallback scenario argument

As long as we limit to only being able to reduce tick spacing (to prevent a pool from being gov attacked) I don't see any security concerns, but it's still not 100% clear to me that this setup is a clean design

[czarcas7ic]

Yeah I am not convinced either way yet, but its better to have the logic there and take it away instead of having to reimplement it later imo

[p0mvn]

In this round, I focused on reviewing tick.go and tick_test.go`.

TickToPrice LGTM.

Wondering if we can simplify and speed up PriceToTick but don't have a specific suggestion yet, only some ideas

[p0mvn]

q: what is the runtime of this loop? Is it a concern?

[p0mvn]

I have no solid understanding of the time complexity here. This function seems to be called every swap step. Each swap step, we start searching from the price of 1 until the actual price. This seems like O(k * n) time complexity where k is the number of swap steps and n is the price difference from 1.

I think this could be optimized. For every new swap step we are unlikely to drift too far away from the previous price. As a result, we can start searching from the old price, not from 1.

I'm not sure how worthwhile it is of an improvement. But the potential O(k * n) seems concerning. Would like to know what other reviewers think

[p0mvn]

another idea is to binary search this

[p0mvn]

Another idea is to take a log base 10 of price via osmomath and truncate it. This will give us a geometric part of the ticks or kDelta. Then, we can binary search the additive range (maybe there is even an easier way to compute it)

Again, not sure if I'm overthinking the performance implications here

[czarcas7ic]

We should only loop through this a maximum of the number of k values there are. So for instance, with a maximum of a k value of -12 and min of -1, we should only loop through this 11 times.

Agreed there is probably a better way to do this though, down to try these suggestions. Do you have any good recommendation on benchmarking?

[p0mvn]

Do you have any good recommendation on benchmarking?

I think we can try creating a benchmark around calcOutAmtGivenIn, initialize a pool close to max spot price, initialize many positions and try executing a large swap where kAtPriceOne is the smallest one possible. I think this would help us benchmark the worst-case scenario. Such a benchmark would be useful even outside of the context of PriceToTick

I don't think it's worth blocking this PR on this at this moment. If you don't a relatively quick and trivial way to simplify this, we might want to capture this thread in an issue and punt it until later

[p0mvn]

Do you mind committing the ADR / the doc for this into the architecture.md in this PR?

I think it would be useful to review it directly with the changes.

[p0mvn]

• TickToPrice
• TestTickToPrice
• PriceToTick
• TestPriceToTick

All LGTM after comments from this round are addressed.

Going to focus on swaps and swap tests in the next review cycle

[p0mvn]

Let's define these decs as file-global variables to avoid reallocations.

Also, are you sure we want to use banker's rounding here and not truncate for maxTick and round up for min tick?

[czarcas7ic]

These will always be whole numbers, but yes I can do that anyway

[czarcas7ic]

I thought that file-global vars only make sense if a number is used more than once?

[p0mvn]

I'm suggesting the file-global vars here to reduce these allocations:
https://github.com/osmosis-labs/cosmos-sdk/blob/48fe175f983925ff6202d492a363d2b2e3a62497/types/decimal.go#L94

If we use NewDec directly in the function, every time the function is called, Go will re-allocate the underlying memory which takes away resources. While this overhead is small, it is still good to address if the function is low-level and called frequently

[p0mvn]

Let's add tests with MaxSpotPrice + 1, MinSpotPrice and MinSpotPrice - 1 to test the edge cases please

[czarcas7ic]

Added MaxSpotPrice+1 and MinSpotPrice 00c430b

Couldn't add MinSpotPrice - 1 since that is already checked by "price must be positive"

osmosis/x/concentrated-liquidity/internal/math/tick_test.go

Lines 213 to 217 in 00c430b

	"error: price must be positive": {
	price: sdk.NewDec(-1),
	exponentAtPriceOne: sdk.NewInt(-6),
	expectedError: fmt.Errorf("price must be greater than zero"),
	},

This is because min spot price is the minimum a sdk.Dec could possibly be, and any smaller it would be zero

[p0mvn]

Please also add "error: resulting tickIndex too small"

[czarcas7ic]

I just added a check to check price is within the allowed spot price bounds before this check. I do not know how to trigger either tickIndex too large or tickIndex too small error, and it feels like it is just a defense in depth at this point

[p0mvn]

What do you mean by traversal here?

[czarcas7ic]

updated the comment

[p0mvn]

Awesome work!

LGTM once comments are addressed. Let's not block on suggestions as long as we track them in issues

[p0mvn]

nit: unsure what the value of this getter is when the field is exported anyways

[czarcas7ic]

Without this getter, how would you retrieve it from a ConcentratedPoolExtension pool object? Here is an example:

osmosis/x/concentrated-liquidity/swaps.go

Lines 287 to 290 in b021858

	nextPrice, err := math.TickToPrice(nextTick, p.GetPrecisionFactorAtPriceOne())
	if err != nil {
	return sdk.Coin{}, sdk.Coin{}, sdk.Int{}, sdk.Dec{}, sdk.Dec{}, fmt.Errorf("could not convert next tick (%v) to nextPrice", nextTick)
	}

[p0mvn]

I'm confused why we are changing these. This test does not seem to be touched by the tick logic.

What is the reason? If unrelated, can this be submitted separately?

[czarcas7ic]

The DefaultLiquidityAmt changed when we changed tick logic

osmosis/x/concentrated-liquidity/keeper_test.go

Lines 32 to 33 in e826b66

	DefaultLiquidityAmt = sdk.MustNewDecFromStr("1517882343.751510418088349649")
	DefaultTickSpacing = uint64(1)

[p0mvn]

Agreed with earlier comment about bringing back TickToSqrtPrice function

[czarcas7ic]

#3988

[p0mvn]

I think this value is off but can be addressed here: #3962

[p0mvn]

Let's make an issue for this please

[czarcas7ic]

#3989

[ValarDragon]

LGTM, all comments I have can be done in later revisions

[czarcas7ic]

Thanks everyone for the super thorough reviews! Will merge post chain dev meeting tomorrow if there is consensus on doing so!

[p0mvn]

Discussed offline - going to merge to allow us to continue working on fees off of main. All comments are addressed/tracked in issues. Great work @czarcas7ic