fix: duplicate price for multiple ticks

[czarcas7ic]

Closes: #XXX

What is the purpose of the change

Our CL tick logic works off of a precision of 10^-18. This, combined with the fact that our per-tick precision increases at every factor of 10, means that we can have multiple ticks that correlate to the same spot price. For instance. tick -161795100 (along with many other ticks around it) correlates to a spot price of 0.000000000000000002. We need a method to determine what the largest tick (rounding up) is that corresponds to a given spot price and only use that tick (kind of like setting a canonical tick) when referencing this price.

Changes:

• When creating a position, the provided ticks are checked to see if there is a larger tick (rounding up) that represents that price with the same level of precision. If so, that tick is changed to that value
  • Example: the lower tick -161795100 correlates to price 0.000000000000000002. The tick is checked, and a larger tick (-161000000) represents that same price. The lower tick is changed to -161000000.
• TickToSqrtPrice now also returns the price as well
  • This is to prevent losing precision when squaring the square root price
• CreatePosition now returns the lowerTick and upperTick in the response
• Update README

Testing and Verifying

• Added TestRoundTick
• Expand TestCreatePositionand TestInverseRelation_CreatePosition_WithdrawPosition

Documentation and Release Note

• Does this pull request introduce a new feature or user-facing behavior changes?
• Changelog entry added to Unreleased section of CHANGELOG.md?

Where is the change documented?

• Specification (x/{module}/README.md)
• Osmosis documentation site
• Code comments?
• N/A

[mattverse]

good catch, leaving some nit comments, I'm also wondering if there could be any possible edge cases stemming from this.. 🤔

[mattverse]

asking to learn: why do we use the term of "changed" here?
Is my understanding correct that the provided priceTickLower || provided priceTickUpper indicates the new price that is being used and thus if this is a tick that has not been used previously, (lowerTick and upperTick being provided), we define them as new ticks?

[czarcas7ic]

We do an initial tick check at the very begining to fail early. This tick check does not do any tick rounding, however (We need to utilize sqrtPrices of the ticks to check this). Once we get the sqrtPrice of the lower and upper tick, we can then check if the provided tick is the canonical tick that is being used for that sqrtPrice. If one of the two ticks change from this check, we ensure the final tick range is valid once again (just for safety).

[mattverse]

should it be roundDownTick? (not sure)

[czarcas7ic]

No, its a little confusing though. The "roundDown" in PriceToTickRoundDown is referring to tick spacing rounding. The PriceToTickRoundDown is rounding us "up" to the largest tick that represents this sqrtPrice, but rounds it down when taking into consideration tick spacing. So the word "rounding" and its direction depends on what lens you are viewing it from. From the scale of ticks, its rounding up since if we give it a smaller tick that has other ticks larger than it that represent the same sqrtPrice, it will return the large tick. But from the tick spacing perspective, it will round down to ensure that tick being returned is a multiple of the tick spacing of the pool. Please lmk if that makes sense

CC @p0mvn

[p0mvn]

This makes sense to me, thanks for the breakdown. Would be valuable to update docs with this context

[mattverse]

nit: maybe roundTickToClosestPriceTick?

[czarcas7ic]

if we want to change this, I am thinking roundTickToCanonicalPriceTick, because closeness has nothing to do with it

[czarcas7ic]

changed it to roundTickToCanonicalPriceTick a74fbd6

[p0mvn]

Do we also need to handle this for the initial tick?

osmosis/x/concentrated-liquidity/lp.go

Lines 421 to 424 in c673324

	initialTick, err := math.PriceToTickRoundDown(initialSpotPrice, pool.GetTickSpacing())
	if err != nil {
	return err
	}

[czarcas7ic]

No, only because the PriceToTickRoundDown function always returns the canonical tick. The only thing that needs to be changed is user provided ticks

[p0mvn]

LGTM, nice work

[p0mvn]

Can we get this in ASAP please so that I can merge #5170?

[czarcas7ic]

@p0mvn yeah ill address these rn