From dba4b88d538f060629cb92177748b2af6b053b32 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Wed, 6 Mar 2024 04:13:52 +0100 Subject: [PATCH] Reduce Sqrt calls in TickToSqrtPrice (#7599) (#7667) * Reduce Sqrt calls in TickToSqrtPrice * Move error to types (cherry picked from commit 5a763c11f92ed24fb55663287969a82d8352b261) Co-authored-by: Dev Ojha --- x/concentrated-liquidity/math/tick.go | 71 +++++++++++++++--------- x/concentrated-liquidity/types/errors.go | 1 + 2 files changed, 46 insertions(+), 26 deletions(-) diff --git a/x/concentrated-liquidity/math/tick.go b/x/concentrated-liquidity/math/tick.go index 5acde23de19..86fd72a08d4 100644 --- a/x/concentrated-liquidity/math/tick.go +++ b/x/concentrated-liquidity/math/tick.go @@ -1,7 +1,6 @@ package math import ( - "errors" "fmt" "github.com/osmosis-labs/osmosis/osmomath" @@ -254,10 +253,9 @@ func CalculateSqrtPriceToTick(sqrtPrice osmomath.BigDec) (tickIndex int64, err e } // We have a candidate bucket index `t`. We discern here if: - // * sqrtPrice in [TickToSqrtPrice(t - 1), TickToSqrtPrice(t)) - // * sqrtPrice in [TickToSqrtPrice(t), TickToSqrtPrice(t + 1)) - // * sqrtPrice in [TickToSqrtPrice(t+1), TickToSqrtPrice(t + 2)) - // * sqrtPrice not in either. + // * sqrtPrice in [ TickToSqrtPrice(t - 1), TickToSqrtPrice(t) ) => bucket t - 1 + // * sqrtPrice in [ TickToSqrtPrice(t), TickToSqrtPrice(t + 1) ) => bucket t + // * sqrtPrice in [ TickToSqrtPrice(t + 1), TickToSqrtPrice(t + 2) ) => bucket t + 1 // We handle boundary checks, by saying that if our candidate is the min tick, // set the candidate to min tick + 1. // If our candidate is at or above max tick - 1, set the candidate to max tick - 2. @@ -274,34 +272,55 @@ func CalculateSqrtPriceToTick(sqrtPrice osmomath.BigDec) (tickIndex int64, err e outOfBounds = true } - sqrtPriceTmin1, errM1 := TickToSqrtPrice(tick - 1) - sqrtPriceT, errT := TickToSqrtPrice(tick) - sqrtPriceTplus1, errP1 := TickToSqrtPrice(tick + 1) - sqrtPriceTplus2, errP2 := TickToSqrtPrice(tick + 2) - if errM1 != nil || errT != nil || errP1 != nil || errP2 != nil { - return 0, errors.New("internal error in computing square roots within CalculateSqrtPriceToTick") - } - - // We error if sqrtPriceT is above sqrtPriceTplus2 or below sqrtPriceTmin1. - // For cases where calculated tick does not fall on a limit (min/max tick), the upper end is exclusive. - // For cases where calculated tick falls on a limit, the upper end is inclusive, since the actual tick is - // already shifted and making it exclusive would make min/max tick impossible to reach by construction. - // We do this primary for code simplicity, as alternatives would require more branching and special cases. - if (!outOfBounds && sqrtPrice.GTE(sqrtPriceTplus2)) || (outOfBounds && sqrtPrice.GT(sqrtPriceTplus2)) || sqrtPrice.LT(sqrtPriceTmin1) { - return 0, types.SqrtPriceToTickError{OutOfBounds: outOfBounds} + sqrtPriceTplus1, err := TickToSqrtPrice(tick + 1) + if err != nil { + return 0, types.ErrCalculateSqrtPriceToTick } + // code path where sqrtPrice is either in tick t + 1, or out of bounds. + if sqrtPrice.GTE(sqrtPriceTplus1) { + // out of bounds check + sqrtPriceTplus2, err := TickToSqrtPrice(tick + 2) + if err != nil { + return 0, types.ErrCalculateSqrtPriceToTick + } + // We error if sqrtPriceT is above sqrtPriceTplus2 + // For cases where calculated tick does not fall on a limit (min/max tick), the upper end is exclusive. + // For cases where calculated tick falls on a limit, the upper end is inclusive, since the actual tick is + // already shifted and making it exclusive would make min/max tick impossible to reach by construction. + // We do this primary for code simplicity, as alternatives would require more branching and special cases. + if (!outOfBounds && sqrtPrice.GTE(sqrtPriceTplus2)) || (outOfBounds && sqrtPrice.GT(sqrtPriceTplus2)) { + return 0, types.SqrtPriceToTickError{OutOfBounds: outOfBounds} + } - // We expect this case to only be hit when the original provided sqrt price is exactly equal to the max sqrt price. - if sqrtPrice.Equal(sqrtPriceTplus2) { - return tick + 2, nil + // We expect this case to only be hit when the original provided sqrt price is exactly equal to the max sqrt price. + if sqrtPrice.Equal(sqrtPriceTplus2) { + return tick + 2, nil + } + // we are not out of bounds, therefore its tick t+1! + return tick + 1, nil } - // The remaining cases handle shifting tick index by +/- 1. - if sqrtPrice.GTE(sqrtPriceTplus1) { - return tick + 1, nil + // code path where sqrtPrice is either in tick t - 1, t, or out of bounds. + // The out of bounds case here should never be possible, but we need to more rigorously check this + // to delete that sqrt call. + sqrtPriceT, err := TickToSqrtPrice(tick) + if err != nil { + return 0, types.ErrCalculateSqrtPriceToTick } + // sqrtPriceT <= sqrtPrice < sqrtPriceTplus1, this were in bucket t if sqrtPrice.GTE(sqrtPriceT) { return tick, nil } + + // check we are not out of bounds from below. + // TODO: Validate this case is impossible, and delete it + sqrtPriceTmin1, err := TickToSqrtPrice(tick - 1) + if err != nil { + return 0, types.ErrCalculateSqrtPriceToTick + } + if sqrtPrice.LT(sqrtPriceTmin1) { + return 0, types.SqrtPriceToTickError{OutOfBounds: outOfBounds} + } + return tick - 1, nil } diff --git a/x/concentrated-liquidity/types/errors.go b/x/concentrated-liquidity/types/errors.go index ec9725552d8..c61a268551d 100644 --- a/x/concentrated-liquidity/types/errors.go +++ b/x/concentrated-liquidity/types/errors.go @@ -19,6 +19,7 @@ var ( ErrZeroLiquidity = errors.New("liquidity cannot be 0") ErrNextTickInfoNil = errors.New("next tick info cannot be nil") ErrPoolNil = errors.New("pool cannot be nil") + ErrCalculateSqrtPriceToTick = errors.New("internal error in computing square roots within CalculateSqrtPriceToTick") ) // x/concentrated-liquidity module sentinel errors.