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

perf: iterator improvements for swap in given out and liquidity for full range query #5248

Merged
merged 2 commits into from
May 20, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
24 changes: 21 additions & 3 deletions x/concentrated-liquidity/swaps.go
Original file line number Diff line number Diff line change
Expand Up @@ -535,19 +535,30 @@ func (k Keeper) computeInAmtGivenOut(

totalFees = sdk.ZeroDec()

nextTickIter := swapStrategy.InitializeNextTickIterator(ctx, poolId, swapState.tick)
defer nextTickIter.Close()
if !nextTickIter.Valid() {
return sdk.Coin{}, sdk.Coin{}, 0, sdk.Dec{}, sdk.Dec{}, sdk.Dec{}, types.RanOutOfTicksForPoolError{PoolId: poolId}
}

// TODO: This should be GT 0 but some instances have very small remainder
// need to look into fixing this
for swapState.amountSpecifiedRemaining.GT(smallestDec) && !swapState.sqrtPrice.Equal(sqrtPriceLimit) {
// log the sqrtPrice we start the iteration with
sqrtPriceStart := swapState.sqrtPrice

// Iterator must be valid to be able to retrieve the next tick from it below.
if !nextTickIter.Valid() {
return sdk.Coin{}, sdk.Coin{}, 0, sdk.Dec{}, sdk.Dec{}, sdk.Dec{}, types.RanOutOfTicksForPoolError{PoolId: poolId}
}

// we first check to see what the position of the nearest initialized tick is
// if zeroForOne is false, we look to the left of the tick the current sqrt price is at
// if zeroForOne is true, we look to the right of the tick the current sqrt price is at
// if no ticks are initialized (no users have created liquidity positions) then we return an error
nextTick, ok := swapStrategy.NextInitializedTick(ctx, poolId, swapState.tick)
if !ok {
return sdk.Coin{}, sdk.Coin{}, 0, sdk.Dec{}, sdk.Dec{}, sdk.Dec{}, types.InvalidTickError{}
nextTick, err := types.TickIndexFromBytes(nextTickIter.Key())
if err != nil {
return sdk.Coin{}, sdk.Coin{}, 0, sdk.Dec{}, sdk.Dec{}, sdk.Dec{}, err
}

// utilizing the next initialized tick, we find the corresponding nextPrice (the target price)
Expand Down Expand Up @@ -591,6 +602,13 @@ func (k Keeper) computeInAmtGivenOut(
if err != nil {
return sdk.Coin{}, sdk.Coin{}, 0, sdk.Dec{}, sdk.Dec{}, sdk.Dec{}, err
}

// Move next tick iterator to the next tick as the tick is crossed.
if !nextTickIter.Valid() {
return sdk.Coin{}, sdk.Coin{}, 0, sdk.Dec{}, sdk.Dec{}, sdk.Dec{}, types.RanOutOfTicksForPoolError{PoolId: poolId}
}
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need to check this again here right?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think you're correct. Updated: 74bd8ac

nextTickIter.Next()

liquidityNet = swapStrategy.SetLiquidityDeltaSign(liquidityNet)
// update the swapState's liquidity with the new tick's liquidity
swapState.liquidity = swapState.liquidity.AddMut(liquidityNet)
Expand Down
32 changes: 0 additions & 32 deletions x/concentrated-liquidity/swapstrategy/one_for_zero.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,38 +194,6 @@ func (s oneForZeroStrategy) InitializeTickValue(currentTick int64) int64 {
return currentTick + 1
}

// NextInitializedTick returns the next initialized tick index based on the
// provided tickindex. If no initialized tick exists, <0, false>
// will be returned.
//
// oneForZerostrategy searches for the next tick to the right of the current tickIndex.
func (s oneForZeroStrategy) NextInitializedTick(ctx sdk.Context, poolId uint64, tickIndex int64) (next int64, initialized bool) {
store := ctx.KVStore(s.storeKey)

// Construct a prefix store with a prefix of <TickPrefix | poolID>, allowing
// us to retrieve the next initialized tick without having to scan all ticks.
prefixBz := types.KeyTickPrefixByPoolId(poolId)
prefixStore := prefix.NewStore(store, prefixBz)

startKey := types.TickIndexToBytes(tickIndex)

iter := prefixStore.Iterator(startKey, nil)
defer iter.Close()
for ; iter.Valid(); iter.Next() {
// Since, we constructed our prefix store with <TickPrefix | poolID>, the
// key is the encoding of a tick index.
tick, err := types.TickIndexFromBytes(iter.Key())
if err != nil {
panic(fmt.Errorf("invalid tick index (%s): %v", string(iter.Key()), err))
}

if tick > tickIndex {
return tick, true
}
}
return 0, false
}

// SetLiquidityDeltaSign sets the liquidity delta sign for the given liquidity delta.
// This is called when consuming all liquidity.
// When a position is created, we add liquidity to lower tick
Expand Down
5 changes: 0 additions & 5 deletions x/concentrated-liquidity/swapstrategy/swap_strategy.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,6 @@ type swapStrategy interface {
// on the actual current tick.
// See oneForZeroStrategy or zeroForOneStrategy for implementation details.
InitializeTickValue(currentTick int64) int64
// NextInitializedTick returns the next initialized tick index based on the
// provided tickindex. If no initialized tick exists, <0, false>
// will be returned.
// See oneForZeroStrategy or zeroForOneStrategy for implementation details.
NextInitializedTick(ctx sdk.Context, poolId uint64, tickIndex int64) (next int64, initialized bool)
// SetLiquidityDeltaSign sets the liquidity delta sign for the given liquidity delta.
// This is called when consuming all liquidity.
// When a position is created, we add liquidity to lower tick
Expand Down
88 changes: 0 additions & 88 deletions x/concentrated-liquidity/swapstrategy/swap_strategy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (

"github.com/osmosis-labs/osmosis/osmomath"
"github.com/osmosis-labs/osmosis/v15/app/apptesting"
"github.com/osmosis-labs/osmosis/v15/x/concentrated-liquidity/model"
"github.com/osmosis-labs/osmosis/v15/x/concentrated-liquidity/swapstrategy"
"github.com/osmosis-labs/osmosis/v15/x/concentrated-liquidity/types"
)
Expand Down Expand Up @@ -54,93 +53,6 @@ func (suite *StrategyTestSuite) SetupTest() {
suite.Setup()
}

// TODO: split up this test case to be separate for each strategy.
func (suite *StrategyTestSuite) TestNextInitializedTick() {
suite.SetupTest()
ctx := suite.Ctx

liquidityTicks := []int64{-200, -55, -4, 70, 78, 84, 139, 240, 535}
for _, t := range liquidityTicks {
suite.App.ConcentratedLiquidityKeeper.SetTickInfo(ctx, 1, t, model.TickInfo{})
}

_, err := suite.App.ConcentratedLiquidityKeeper.GetAllInitializedTicksForPool(ctx, 1)
suite.Require().NoError(err)

clStoreKey := suite.App.GetKey(types.ModuleName)

suite.Run("lte=true", func() {
suite.Run("returns tick to right if at initialized tick", func() {
swapStrategy := swapstrategy.New(false, sdk.ZeroDec(), clStoreKey, sdk.ZeroDec(), defaultTickSpacing)

n, initd := swapStrategy.NextInitializedTick(ctx, 1, 78)
suite.Require().Equal(int64(84), n)
suite.Require().True(initd)
})
suite.Run("returns tick to right if at initialized tick", func() {
swapStrategy := swapstrategy.New(false, sdk.ZeroDec(), clStoreKey, sdk.ZeroDec(), defaultTickSpacing)

n, initd := swapStrategy.NextInitializedTick(suite.Ctx, 1, -55)
suite.Require().Equal(int64(-4), n)
suite.Require().True(initd)
})
suite.Run("returns the tick directly to the right", func() {
swapStrategy := swapstrategy.New(false, sdk.ZeroDec(), clStoreKey, sdk.ZeroDec(), defaultTickSpacing)

n, initd := swapStrategy.NextInitializedTick(suite.Ctx, 1, 77)
suite.Require().Equal(int64(78), n)
suite.Require().True(initd)
})
suite.Run("returns the tick directly to the right", func() {
swapStrategy := swapstrategy.New(false, sdk.ZeroDec(), clStoreKey, sdk.ZeroDec(), defaultTickSpacing)

n, initd := swapStrategy.NextInitializedTick(suite.Ctx, 1, -56)
suite.Require().Equal(int64(-55), n)
suite.Require().True(initd)
})
suite.Run("returns the next words initialized tick if on the right boundary", func() {
swapStrategy := swapstrategy.New(false, sdk.ZeroDec(), clStoreKey, sdk.ZeroDec(), defaultTickSpacing)

n, initd := swapStrategy.NextInitializedTick(suite.Ctx, 1, -257)
suite.Require().Equal(int64(-200), n)
suite.Require().True(initd)
})
suite.Run("returns the next initialized tick from the next word", func() {
swapStrategy := swapstrategy.New(false, sdk.ZeroDec(), clStoreKey, sdk.ZeroDec(), defaultTickSpacing)

suite.App.ConcentratedLiquidityKeeper.SetTickInfo(suite.Ctx, 1, 340, model.TickInfo{})

n, initd := swapStrategy.NextInitializedTick(suite.Ctx, 1, 328)
suite.Require().Equal(int64(340), n)
suite.Require().True(initd)
})
})

suite.Run("lte=false", func() {
suite.Run("returns tick directly to the left of input tick if not initialized", func() {
swapStrategy := swapstrategy.New(true, sdk.ZeroDec(), clStoreKey, sdk.ZeroDec(), defaultTickSpacing)

n, initd := swapStrategy.NextInitializedTick(suite.Ctx, 1, 79)
suite.Require().Equal(int64(78), n)
suite.Require().True(initd)
})
suite.Run("returns previous tick even though given is initialized", func() {
swapStrategy := swapstrategy.New(true, sdk.ZeroDec(), clStoreKey, sdk.ZeroDec(), defaultTickSpacing)

n, initd := swapStrategy.NextInitializedTick(suite.Ctx, 1, 78)
suite.Require().Equal(int64(70), n)
suite.Require().True(initd)
})
suite.Run("returns next initialized tick far away", func() {
swapStrategy := swapstrategy.New(true, sdk.ZeroDec(), clStoreKey, sdk.ZeroDec(), defaultTickSpacing)

n, initd := swapStrategy.NextInitializedTick(suite.Ctx, 1, 100)
suite.Require().Equal(int64(84), n)
suite.Require().True(initd)
})
})
}

// TestComputeSwapState_Inverse validates that out given in and in given out compute swap steps
// for one strategy produce the same results as the other given inverse inputs.
// That is, if we swap in A of token0 and expect to get out B of token 1,
Expand Down
32 changes: 0 additions & 32 deletions x/concentrated-liquidity/swapstrategy/zero_for_one.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,38 +189,6 @@ func (s zeroForOneStrategy) InitializeTickValue(currentTick int64) int64 {
return currentTick + 1
}

// NextInitializedTick returns the next initialized tick index based on the
// provided tickindex. If no initialized tick exists, <0, false>
// will be returned.
//
// zeroForOneStrategy searches for the next tick to the left of the current tickIndex.
func (s zeroForOneStrategy) NextInitializedTick(ctx sdk.Context, poolId uint64, tickIndex int64) (next int64, initialized bool) {
store := ctx.KVStore(s.storeKey)

// Construct a prefix store with a prefix of <TickPrefix | poolID>, allowing
// us to retrieve the next initialized tick without having to scan all ticks.
prefixBz := types.KeyTickPrefixByPoolId(poolId)
prefixStore := prefix.NewStore(store, prefixBz)

startKey := types.TickIndexToBytes(tickIndex)

iter := prefixStore.ReverseIterator(nil, startKey)
defer iter.Close()

for ; iter.Valid(); iter.Next() {
// Since, we constructed our prefix store with <TickPrefix | poolID>, the
// key is the encoding of a tick index.
tick, err := types.TickIndexFromBytes(iter.Key())
if err != nil {
panic(fmt.Errorf("invalid tick index (%s): %v", string(iter.Key()), err))
}
if tick <= tickIndex {
return tick, true
}
}
return 0, false
}

// SetLiquidityDeltaSign sets the liquidity delta sign for the given liquidity delta.
// This is called when consuming all liquidity.
// When a position is created, we add liquidity to lower tick
Expand Down
28 changes: 13 additions & 15 deletions x/concentrated-liquidity/tick.go
Original file line number Diff line number Diff line change
Expand Up @@ -235,9 +235,15 @@ func (k Keeper) GetTickLiquidityForFullRange(ctx sdk.Context, poolId uint64) ([]
// we do -1 to make min tick inclusive.
currentTick := types.MinTick - 1

nextTick, ok := swapStrategy.NextInitializedTick(ctx, poolId, currentTick)
if !ok {
return []queryproto.LiquidityDepthWithRange{}, types.InvalidTickError{Tick: currentTick, IsLower: false, MinTick: types.MinTick, MaxTick: types.MaxTick}
nextTickIter := swapStrategy.InitializeNextTickIterator(ctx, poolId, currentTick)
defer nextTickIter.Close()
if !nextTickIter.Valid() {
return []queryproto.LiquidityDepthWithRange{}, types.RanOutOfTicksForPoolError{PoolId: poolId}
}

nextTick, err := types.TickIndexFromBytes(nextTickIter.Key())
if err != nil {
return []queryproto.LiquidityDepthWithRange{}, err
}

tick, err := k.getTickByTickIndex(ctx, poolId, nextTick)
Expand All @@ -252,26 +258,18 @@ func (k Keeper) GetTickLiquidityForFullRange(ctx sdk.Context, poolId uint64) ([]
currentTick = nextTick
totalLiquidityWithinRange := currentLiquidity

// iterator assignments
store := ctx.KVStore(k.storeKey)
prefixBz := types.KeyTickPrefixByPoolId(poolId)
prefixStore := prefix.NewStore(store, prefixBz)
currentTickKey := types.TickIndexToBytes(currentTick)
maxTickKey := types.TickIndexToBytes(types.MaxTick)
iterator := prefixStore.Iterator(currentTickKey, storetypes.InclusiveEndBytes(maxTickKey))
defer iterator.Close()
previousTickIndex := currentTick

// start from the next index so that the current tick can become lower tick.
iterator.Next()
for ; iterator.Valid(); iterator.Next() {
tickIndex, err := types.TickIndexFromBytes(iterator.Key())
nextTickIter.Next()
for ; nextTickIter.Valid(); nextTickIter.Next() {
tickIndex, err := types.TickIndexFromBytes(nextTickIter.Key())
if err != nil {
return []queryproto.LiquidityDepthWithRange{}, err
}

tickStruct := model.TickInfo{}
err = proto.Unmarshal(iterator.Value(), &tickStruct)
err = proto.Unmarshal(nextTickIter.Value(), &tickStruct)
if err != nil {
return []queryproto.LiquidityDepthWithRange{}, err
}
Expand Down