From 0d29da00655f21280010740b71c2b51956478de6 Mon Sep 17 00:00:00 2001 From: Roman Date: Fri, 19 May 2023 23:33:46 -0400 Subject: [PATCH 1/2] perf: iterator improvements for swap in given out and liquidity for full range query --- x/concentrated-liquidity/swaps.go | 24 ++++- .../swapstrategy/one_for_zero.go | 32 ------- .../swapstrategy/swap_strategy.go | 5 -- .../swapstrategy/swap_strategy_test.go | 88 ------------------- .../swapstrategy/zero_for_one.go | 32 ------- x/concentrated-liquidity/tick.go | 28 +++--- 6 files changed, 34 insertions(+), 175 deletions(-) diff --git a/x/concentrated-liquidity/swaps.go b/x/concentrated-liquidity/swaps.go index bb9edea550e..db85ea8271f 100644 --- a/x/concentrated-liquidity/swaps.go +++ b/x/concentrated-liquidity/swaps.go @@ -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) @@ -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} + } + nextTickIter.Next() + liquidityNet = swapStrategy.SetLiquidityDeltaSign(liquidityNet) // update the swapState's liquidity with the new tick's liquidity swapState.liquidity = swapState.liquidity.AddMut(liquidityNet) diff --git a/x/concentrated-liquidity/swapstrategy/one_for_zero.go b/x/concentrated-liquidity/swapstrategy/one_for_zero.go index 2f9c9cb6b59..24f9be0aabd 100644 --- a/x/concentrated-liquidity/swapstrategy/one_for_zero.go +++ b/x/concentrated-liquidity/swapstrategy/one_for_zero.go @@ -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 , 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 , 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 diff --git a/x/concentrated-liquidity/swapstrategy/swap_strategy.go b/x/concentrated-liquidity/swapstrategy/swap_strategy.go index 9f3f93dd974..3df37b6ca3a 100644 --- a/x/concentrated-liquidity/swapstrategy/swap_strategy.go +++ b/x/concentrated-liquidity/swapstrategy/swap_strategy.go @@ -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 diff --git a/x/concentrated-liquidity/swapstrategy/swap_strategy_test.go b/x/concentrated-liquidity/swapstrategy/swap_strategy_test.go index bf809c8c871..bc9833871e7 100644 --- a/x/concentrated-liquidity/swapstrategy/swap_strategy_test.go +++ b/x/concentrated-liquidity/swapstrategy/swap_strategy_test.go @@ -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" ) @@ -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, diff --git a/x/concentrated-liquidity/swapstrategy/zero_for_one.go b/x/concentrated-liquidity/swapstrategy/zero_for_one.go index 309a11b7bab..8b9babed315 100644 --- a/x/concentrated-liquidity/swapstrategy/zero_for_one.go +++ b/x/concentrated-liquidity/swapstrategy/zero_for_one.go @@ -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 , 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 , 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 diff --git a/x/concentrated-liquidity/tick.go b/x/concentrated-liquidity/tick.go index a1aeccfd6ae..d8dff4089ff 100644 --- a/x/concentrated-liquidity/tick.go +++ b/x/concentrated-liquidity/tick.go @@ -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) @@ -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 } From 74bd8acce14f793d5984d1d1bf1e4e58959297d0 Mon Sep 17 00:00:00 2001 From: Roman Date: Sat, 20 May 2023 00:28:56 -0400 Subject: [PATCH 2/2] updates --- x/concentrated-liquidity/swaps.go | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/x/concentrated-liquidity/swaps.go b/x/concentrated-liquidity/swaps.go index db85ea8271f..b85e993f685 100644 --- a/x/concentrated-liquidity/swaps.go +++ b/x/concentrated-liquidity/swaps.go @@ -335,9 +335,6 @@ func (k Keeper) computeOutAmtGivenIn( 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} - } totalFees = sdk.ZeroDec() // Iterate and update swapState until we swap all tokenIn or we reach the specific sqrtPriceLimit @@ -409,9 +406,6 @@ func (k Keeper) computeOutAmtGivenIn( } // 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} - } nextTickIter.Next() liquidityNet = swapStrategy.SetLiquidityDeltaSign(liquidityNet) @@ -537,9 +531,6 @@ func (k Keeper) computeInAmtGivenOut( 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 @@ -604,9 +595,6 @@ func (k Keeper) computeInAmtGivenOut( } // 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} - } nextTickIter.Next() liquidityNet = swapStrategy.SetLiquidityDeltaSign(liquidityNet)