refactor: reduce the number of returns in tick conversions and update…

… position (#6071)

* refactor: reduce the number of returns in tick conversions and update position

* changelog

CHANGELOG.md

@@ -40,6 +40,12 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## Unreleased
+
+### API breaks
+
+* [#6071](https://github.com/osmosis-labs/osmosis/pull/6071) reduce number of returns for UpdatePosition and TicksToSqrtPrice functions
+
 ## v17.0.0
 
 ### API breaks

simulation/executor/legacyconfig.go

@@ -115,6 +115,7 @@ func NewExecutionDbConfigFromFlags() ExecutionDbConfig {
 // SetupSimulation creates the config, db (levelDB), temporary directory and logger for
 // the simulation tests. If `FlagEnabledValue` is false it skips the current test.
 // Returns error on an invalid db intantiation or temp dir creation.
+// nolint: revive
 func SetupSimulation(dirPrefix, dbName string) (cfg Config, db dbm.DB, logger log.Logger, cleanup func(), err error) {
 	if !FlagEnabledValue {
 		return Config{}, nil, nil, func() {}, nil

x/concentrated-liquidity/bench_test.go

@@ -149,7 +149,10 @@ func runBenchmark(b *testing.B, testFunc func(b *testing.B, s *BenchTestSuite, p
 				// Normalize upperTick to be a multiple of tickSpacing
 				upperTick = upperTick - upperTick%tickSpacing
 
-				priceLowerTick, priceUpperTick, _, _, err := clmath.TicksToSqrtPrice(lowerTick, upperTick)
+				priceLowerTick, err := clmath.TickToPrice(lowerTick)
+				noError(b, err)
+
+				priceUpperTick, err := clmath.TickToPrice(upperTick)
 				noError(b, err)
 
 				lowerTick, upperTick, err = cl.RoundTickToCanonicalPriceTick(

x/concentrated-liquidity/lp.go

@@ -77,7 +77,7 @@ func (k Keeper) CreatePosition(ctx sdk.Context, poolId uint64, owner sdk.AccAddr
 	}
 
 	// Transform the provided ticks into their corresponding sqrtPrices.
-	_, _, sqrtPriceLowerTick, sqrtPriceUpperTick, err := math.TicksToSqrtPrice(lowerTick, upperTick)
+	sqrtPriceLowerTick, sqrtPriceUpperTick, err := math.TicksToSqrtPrice(lowerTick, upperTick)
 	if err != nil {
 		return CreatePositionData{}, err
 	}
@@ -111,21 +111,21 @@ func (k Keeper) CreatePosition(ctx sdk.Context, poolId uint64, owner sdk.AccAddr
 	}
 
 	// Initialize / update the position in the pool based on the provided tick range and liquidity delta.
-	actualAmount0, actualAmount1, _, _, err := k.UpdatePosition(ctx, poolId, owner, lowerTick, upperTick, liquidityDelta, joinTime, positionId)
+	updateData, err := k.UpdatePosition(ctx, poolId, owner, lowerTick, upperTick, liquidityDelta, joinTime, positionId)
 	if err != nil {
 		return CreatePositionData{}, err
 	}
 
 	// Check if the actual amounts of tokens 0 and 1 are greater than or equal to the given minimum amounts.
-	if actualAmount0.LT(amount0Min) {
-		return CreatePositionData{}, types.InsufficientLiquidityCreatedError{Actual: actualAmount0, Minimum: amount0Min, IsTokenZero: true}
+	if updateData.Amount0.LT(amount0Min) {
+		return CreatePositionData{}, types.InsufficientLiquidityCreatedError{Actual: updateData.Amount0, Minimum: amount0Min, IsTokenZero: true}
 	}
-	if actualAmount1.LT(amount1Min) {
-		return CreatePositionData{}, types.InsufficientLiquidityCreatedError{Actual: actualAmount1, Minimum: amount1Min}
+	if updateData.Amount1.LT(amount1Min) {
+		return CreatePositionData{}, types.InsufficientLiquidityCreatedError{Actual: updateData.Amount1, Minimum: amount1Min}
 	}
 
 	// Transfer the actual amounts of tokens 0 and 1 from the position owner to the pool.
-	err = k.sendCoinsBetweenPoolAndUser(ctx, pool.GetToken0(), pool.GetToken1(), actualAmount0, actualAmount1, owner, pool.GetAddress())
+	err = k.sendCoinsBetweenPoolAndUser(ctx, pool.GetToken0(), pool.GetToken1(), updateData.Amount0, updateData.Amount1, owner, pool.GetAddress())
 	if err != nil {
 		return CreatePositionData{}, err
 	}
@@ -139,8 +139,8 @@ func (k Keeper) CreatePosition(ctx sdk.Context, poolId uint64, owner sdk.AccAddr
 		upperTick:      upperTick,
 		joinTime:       joinTime,
 		liquidityDelta: liquidityDelta,
-		actualAmount0:  actualAmount0,
-		actualAmount1:  actualAmount1,
+		actualAmount0:  updateData.Amount0,
+		actualAmount1:  updateData.Amount1,
 	}
 	event.emit(ctx)
 
@@ -153,18 +153,18 @@ func (k Keeper) CreatePosition(ctx sdk.Context, poolId uint64, owner sdk.AccAddr
 	}
 
 	tokensAdded := sdk.Coins{}
-	if actualAmount0.IsPositive() {
-		tokensAdded = tokensAdded.Add(sdk.NewCoin(pool.GetToken0(), actualAmount0))
+	if updateData.Amount0.IsPositive() {
+		tokensAdded = tokensAdded.Add(sdk.NewCoin(pool.GetToken0(), updateData.Amount0))
 	}
-	if actualAmount1.IsPositive() {
-		tokensAdded = tokensAdded.Add(sdk.NewCoin(pool.GetToken1(), actualAmount1))
+	if updateData.Amount1.IsPositive() {
+		tokensAdded = tokensAdded.Add(sdk.NewCoin(pool.GetToken1(), updateData.Amount1))
 	}
 	k.RecordTotalLiquidityIncrease(ctx, tokensAdded)
 
 	return CreatePositionData{
 		ID:        positionId,
-		Amount0:   actualAmount0,
-		Amount1:   actualAmount1,
+		Amount0:   updateData.Amount0,
+		Amount1:   updateData.Amount1,
 		Liquidity: liquidityDelta,
 		LowerTick: lowerTick,
 		UpperTick: upperTick,
@@ -236,13 +236,13 @@ func (k Keeper) WithdrawPosition(ctx sdk.Context, owner sdk.AccAddress, position
 	liquidityDelta := requestedLiquidityAmountToWithdraw.Neg()
 
 	// Update the position in the pool based on the provided tick range and liquidity delta.
-	actualAmount0, actualAmount1, lowerTickIsEmpty, upperTickIsEmpty, err := k.UpdatePosition(ctx, position.PoolId, owner, position.LowerTick, position.UpperTick, liquidityDelta, position.JoinTime, positionId)
+	updateData, err := k.UpdatePosition(ctx, position.PoolId, owner, position.LowerTick, position.UpperTick, liquidityDelta, position.JoinTime, positionId)
 	if err != nil {
 		return sdk.Int{}, sdk.Int{}, err
 	}
 
 	// Transfer the actual amounts of tokens 0 and 1 from the pool to the position owner.
-	err = k.sendCoinsBetweenPoolAndUser(ctx, pool.GetToken0(), pool.GetToken1(), actualAmount0.Abs(), actualAmount1.Abs(), pool.GetAddress(), owner)
+	err = k.sendCoinsBetweenPoolAndUser(ctx, pool.GetToken0(), pool.GetToken1(), updateData.Amount0.Abs(), updateData.Amount1.Abs(), pool.GetAddress(), owner)
 	if err != nil {
 		return sdk.Int{}, sdk.Int{}, err
 	}
@@ -284,19 +284,19 @@ func (k Keeper) WithdrawPosition(ctx sdk.Context, owner sdk.AccAddress, position
 	}
 
 	// If lowertick/uppertick has no liquidity in it, delete it from state.
-	if lowerTickIsEmpty {
+	if updateData.LowerTickIsEmpty {
 		k.RemoveTickInfo(ctx, position.PoolId, position.LowerTick)
 	}
-	if upperTickIsEmpty {
+	if updateData.UpperTickIsEmpty {
 		k.RemoveTickInfo(ctx, position.PoolId, position.UpperTick)
 	}
 
 	tokensRemoved := sdk.Coins{}
-	if actualAmount0.IsPositive() {
-		tokensRemoved = tokensRemoved.Add(sdk.NewCoin(pool.GetToken0(), actualAmount0))
+	if updateData.Amount0.IsPositive() {
+		tokensRemoved = tokensRemoved.Add(sdk.NewCoin(pool.GetToken0(), updateData.Amount0))
 	}
-	if actualAmount1.IsPositive() {
-		tokensRemoved = tokensRemoved.Add(sdk.NewCoin(pool.GetToken1(), actualAmount1))
+	if updateData.Amount1.IsPositive() {
+		tokensRemoved = tokensRemoved.Add(sdk.NewCoin(pool.GetToken1(), updateData.Amount1))
 	}
 	k.RecordTotalLiquidityDecrease(ctx, tokensRemoved)
 
@@ -309,12 +309,12 @@ func (k Keeper) WithdrawPosition(ctx sdk.Context, owner sdk.AccAddress, position
 		upperTick:      position.UpperTick,
 		joinTime:       position.JoinTime,
 		liquidityDelta: liquidityDelta,
-		actualAmount0:  actualAmount0,
-		actualAmount1:  actualAmount1,
+		actualAmount0:  updateData.Amount0,
+		actualAmount1:  updateData.Amount1,
 	}
 	event.emit(ctx)
 
-	return actualAmount0.Neg(), actualAmount1.Neg(), nil
+	return updateData.Amount0.Neg(), updateData.Amount1.Neg(), nil
 }
 
 // addToPosition attempts to add amount0Added and amount1Added to a position with the given position id.
@@ -421,62 +421,67 @@ func (k Keeper) addToPosition(ctx sdk.Context, owner sdk.AccAddress, positionId
 // Positive returned amounts imply that tokens are added to the pool.
 // If the lower and/or upper ticks are being updated to have zero liquidity, a boolean is returned to flag the tick as empty to be deleted at the end of the withdrawPosition method.
 // WARNING: this method may mutate the pool, make sure to refetch the pool after calling this method.
-func (k Keeper) UpdatePosition(ctx sdk.Context, poolId uint64, owner sdk.AccAddress, lowerTick, upperTick int64, liquidityDelta sdk.Dec, joinTime time.Time, positionId uint64) (sdk.Int, sdk.Int, bool, bool, error) {
+func (k Keeper) UpdatePosition(ctx sdk.Context, poolId uint64, owner sdk.AccAddress, lowerTick, upperTick int64, liquidityDelta sdk.Dec, joinTime time.Time, positionId uint64) (types.UpdatePositionData, error) {
 	if err := k.validatePositionUpdateById(ctx, positionId, owner, lowerTick, upperTick, liquidityDelta, joinTime, poolId); err != nil {
-		return sdk.Int{}, sdk.Int{}, false, false, err
+		return types.UpdatePositionData{}, err
 	}
 
 	pool, err := k.getPoolById(ctx, poolId)
 	if err != nil {
-		return sdk.Int{}, sdk.Int{}, false, false, err
+		return types.UpdatePositionData{}, err
 	}
 
 	currentTick := pool.GetCurrentTick()
 
 	// update lower tickInfo state
 	lowerTickIsEmpty, err := k.initOrUpdateTick(ctx, poolId, currentTick, lowerTick, liquidityDelta, false)
 	if err != nil {
-		return sdk.Int{}, sdk.Int{}, false, false, err
+		return types.UpdatePositionData{}, err
 	}
 
 	// update upper tickInfo state
 	upperTickIsEmpty, err := k.initOrUpdateTick(ctx, poolId, currentTick, upperTick, liquidityDelta, true)
 	if err != nil {
-		return sdk.Int{}, sdk.Int{}, false, false, err
+		return types.UpdatePositionData{}, err
 	}
 
 	// update position state
 	err = k.initOrUpdatePosition(ctx, poolId, owner, lowerTick, upperTick, liquidityDelta, joinTime, positionId)
 	if err != nil {
-		return sdk.Int{}, sdk.Int{}, false, false, err
+		return types.UpdatePositionData{}, err
 	}
 
 	// Refetch pool to get the updated pool.
 	// Note that updateUptimeAccumulatorsToNow may modify the pool state and rewrite it to the store.
 	pool, err = k.getPoolById(ctx, poolId)
 	if err != nil {
-		return sdk.Int{}, sdk.Int{}, false, false, err
+		return types.UpdatePositionData{}, err
 	}
 
 	// calculate the actual amounts of tokens 0 and 1 that were added or removed from the pool.
 	actualAmount0, actualAmount1, err := pool.CalcActualAmounts(ctx, lowerTick, upperTick, liquidityDelta)
 	if err != nil {
-		return sdk.Int{}, sdk.Int{}, false, false, err
+		return types.UpdatePositionData{}, err
 	}
 
 	// the pool's liquidity value is only updated if this position is active
 	pool.UpdateLiquidityIfActivePosition(ctx, lowerTick, upperTick, liquidityDelta)
 
 	if err := k.setPool(ctx, pool); err != nil {
-		return sdk.Int{}, sdk.Int{}, false, false, err
+		return types.UpdatePositionData{}, err
 	}
 
 	if err := k.initOrUpdatePositionSpreadRewardAccumulator(ctx, poolId, lowerTick, upperTick, positionId, liquidityDelta); err != nil {
-		return sdk.Int{}, sdk.Int{}, false, false, err
+		return types.UpdatePositionData{}, err
 	}
 
 	// The returned amounts are rounded down to avoid returning more to clients than they actually deposited.
-	return actualAmount0.TruncateInt(), actualAmount1.TruncateInt(), lowerTickIsEmpty, upperTickIsEmpty, nil
+	return types.UpdatePositionData{
+		Amount0:          actualAmount0.TruncateInt(),
+		Amount1:          actualAmount1.TruncateInt(),
+		LowerTickIsEmpty: lowerTickIsEmpty,
+		UpperTickIsEmpty: upperTickIsEmpty,
+	}, nil
 }
 
 // sendCoinsBetweenPoolAndUser takes the amounts calculated from a join/exit position and executes the send between pool and user

x/concentrated-liquidity/lp_test.go

@@ -1637,7 +1637,7 @@ func (s *KeeperTestSuite) TestUpdatePosition() {
 			s.Ctx = s.Ctx.WithBlockTime(expectedUpdateTime)
 
 			// system under test
-			actualAmount0, actualAmount1, lowerTickIsEmpty, upperTickIsEmpty, err := s.App.ConcentratedLiquidityKeeper.UpdatePosition(
+			updateData, err := s.App.ConcentratedLiquidityKeeper.UpdatePosition(
 				s.Ctx,
 				tc.poolId,
 				s.TestAccs[tc.ownerIndex],
@@ -1650,17 +1650,17 @@ func (s *KeeperTestSuite) TestUpdatePosition() {
 
 			if tc.expectedError {
 				s.Require().Error(err)
-				s.Require().Equal(sdk.Int{}, actualAmount0)
-				s.Require().Equal(sdk.Int{}, actualAmount1)
+				s.Require().Equal(sdk.Int{}, updateData.Amount0)
+				s.Require().Equal(sdk.Int{}, updateData.Amount1)
 			} else {
 				s.Require().NoError(err)
 
 				if tc.liquidityDelta.Equal(DefaultLiquidityAmt.Neg()) {
-					s.Require().True(lowerTickIsEmpty)
-					s.Require().True(upperTickIsEmpty)
+					s.Require().True(updateData.LowerTickIsEmpty)
+					s.Require().True(updateData.UpperTickIsEmpty)
 				} else {
-					s.Require().False(lowerTickIsEmpty)
-					s.Require().False(upperTickIsEmpty)
+					s.Require().False(updateData.LowerTickIsEmpty)
+					s.Require().False(updateData.UpperTickIsEmpty)
 				}
 
 				var (
@@ -1681,8 +1681,8 @@ func (s *KeeperTestSuite) TestUpdatePosition() {
 					expectedAmount1 = tc.amount1Expected.ToDec()
 				}
 
-				s.Require().Equal(expectedAmount0.TruncateInt().String(), actualAmount0.String())
-				s.Require().Equal(expectedAmount1.TruncateInt().String(), actualAmount1.String())
+				s.Require().Equal(expectedAmount0.TruncateInt().String(), updateData.Amount0.String())
+				s.Require().Equal(expectedAmount1.TruncateInt().String(), updateData.Amount1.String())
 
 				// validate if position has been properly updated
 				s.validatePositionUpdate(s.Ctx, tc.positionId, tc.expectedPositionLiquidity)

x/concentrated-liquidity/math/tick.go

@@ -13,19 +13,19 @@ import (
 // TicksToSqrtPrice returns the sqrtPrice for the lower and upper ticks by
 // individually calling `TickToSqrtPrice` method.
 // Returns error if fails to calculate price.
-func TicksToSqrtPrice(lowerTick, upperTick int64) (sdk.Dec, sdk.Dec, sdk.Dec, sdk.Dec, error) {
+func TicksToSqrtPrice(lowerTick, upperTick int64) (sdk.Dec, sdk.Dec, error) {
 	if lowerTick >= upperTick {
-		return sdk.Dec{}, sdk.Dec{}, sdk.Dec{}, sdk.Dec{}, types.InvalidLowerUpperTickError{LowerTick: lowerTick, UpperTick: upperTick}
+		return sdk.Dec{}, sdk.Dec{}, types.InvalidLowerUpperTickError{LowerTick: lowerTick, UpperTick: upperTick}
 	}
-	priceUpperTick, sqrtPriceUpperTick, err := TickToSqrtPrice(upperTick)
+	_, sqrtPriceUpperTick, err := TickToSqrtPrice(upperTick)
 	if err != nil {
-		return sdk.Dec{}, sdk.Dec{}, sdk.Dec{}, sdk.Dec{}, err
+		return sdk.Dec{}, sdk.Dec{}, err
 	}
-	priceLowerTick, sqrtPriceLowerTick, err := TickToSqrtPrice(lowerTick)
+	_, sqrtPriceLowerTick, err := TickToSqrtPrice(lowerTick)
 	if err != nil {
-		return sdk.Dec{}, sdk.Dec{}, sdk.Dec{}, sdk.Dec{}, err
+		return sdk.Dec{}, sdk.Dec{}, err
 	}
-	return priceLowerTick, priceUpperTick, sqrtPriceLowerTick, sqrtPriceUpperTick, nil
+	return sqrtPriceLowerTick, sqrtPriceUpperTick, nil
 }
 
 // TickToSqrtPrice returns the sqrtPrice given a tickIndex

x/concentrated-liquidity/math/tick_test.go

@@ -283,7 +283,7 @@ func TestTicksToSqrtPrice(t *testing.T) {
 	for name, tc := range testCases {
 		tc := tc
 		t.Run(name, func(t *testing.T) {
-			priceLower, priceUpper, lowerSqrtPrice, upperSqrtPrice, err := math.TicksToSqrtPrice(tc.lowerTickIndex.Int64(), tc.upperTickIndex.Int64())
+			lowerSqrtPrice, upperSqrtPrice, err := math.TicksToSqrtPrice(tc.lowerTickIndex.Int64(), tc.upperTickIndex.Int64())
 			if tc.expectedError != nil {
 				require.Error(t, err)
 				require.Equal(t, tc.expectedError.Error(), err.Error())
@@ -297,8 +297,6 @@ func TestTicksToSqrtPrice(t *testing.T) {
 			expectedUpperSqrtPrice, err := osmomath.MonotonicSqrt(tc.expectedUpperPrice)
 			require.NoError(t, err)
 
-			require.Equal(t, tc.expectedLowerPrice.String(), priceLower.String())
-			require.Equal(t, tc.expectedUpperPrice.String(), priceUpper.String())
 			require.Equal(t, expectedLowerSqrtPrice.String(), lowerSqrtPrice.String())
 			require.Equal(t, expectedUpperSqrtPrice.String(), upperSqrtPrice.String())
 		})

x/concentrated-liquidity/model/pool.go

@@ -228,7 +228,7 @@ func (p Pool) CalcActualAmounts(ctx sdk.Context, lowerTick, upperTick int64, liq
 	}
 
 	// Transform the provided ticks into their corresponding sqrtPrices.
-	_, _, sqrtPriceLowerTick, sqrtPriceUpperTick, err := math.TicksToSqrtPrice(lowerTick, upperTick)
+	sqrtPriceLowerTick, sqrtPriceUpperTick, err := math.TicksToSqrtPrice(lowerTick, upperTick)
 	if err != nil {
 		return sdk.Dec{}, sdk.Dec{}, err
 	}

x/concentrated-liquidity/position.go

@@ -572,7 +572,7 @@ func (k Keeper) fungifyChargedPosition(ctx sdk.Context, owner sdk.AccAddress, po
 
 	// Create the new position in the pool based on the provided tick range and liquidity delta.
 	// This also initializes the spread reward accumulator and the uptime accumulators for the new position.
-	_, _, _, _, err = k.UpdatePosition(ctx, poolId, owner, lowerTick, upperTick, combinedLiquidityOfAllPositions, joinTime, newPositionId)
+	_, err = k.UpdatePosition(ctx, poolId, owner, lowerTick, upperTick, combinedLiquidityOfAllPositions, joinTime, newPositionId)
 	if err != nil {
 		return 0, err
 	}

x/concentrated-liquidity/types/cl.go

@@ -20,3 +20,13 @@ type CreateFullRangePositionData struct {
 	Amount1   sdk.Int
 	Liquidity sdk.Dec
 }
+
+// UpdatePositionData represents the return data from updating a position.
+// Tick flags are used to signal if tick is not referenced by any liquidity after the update
+// for removal purposes.
+type UpdatePositionData struct {
+	Amount0          sdk.Int
+	Amount1          sdk.Int
+	LowerTickIsEmpty bool
+	UpperTickIsEmpty bool
+}

x/superfluid/keeper/slash.go

@@ -135,7 +135,7 @@ func (k Keeper) prepareConcentratedLockForSlash(ctx sdk.Context, lock *lockuptyp
 	coinsToSlash := sdk.NewCoins(asset0, asset1)
 
 	// Update the cl positions liquidity to the new amount
-	_, _, _, _, err = k.clk.UpdatePosition(ctx, position.PoolId, sdk.MustAccAddressFromBech32(position.Address), position.LowerTick, position.UpperTick, slashAmtNeg, position.JoinTime, position.PositionId)
+	_, err = k.clk.UpdatePosition(ctx, position.PoolId, sdk.MustAccAddressFromBech32(position.Address), position.LowerTick, position.UpperTick, slashAmtNeg, position.JoinTime, position.PositionId)
 	if err != nil {
 		return sdk.AccAddress{}, sdk.Coins{}, err
 	}

x/superfluid/types/expected_keepers.go

@@ -109,7 +109,7 @@ type EpochKeeper interface {
 type ConcentratedKeeper interface {
 	GetPosition(ctx sdk.Context, positionId uint64) (model.Position, error)
 	SetPosition(ctx sdk.Context, poolId uint64, owner sdk.AccAddress, lowerTick, upperTick int64, joinTime time.Time, liquidity sdk.Dec, positionId uint64, underlyingLockId uint64) error
-	UpdatePosition(ctx sdk.Context, poolId uint64, owner sdk.AccAddress, lowerTick, upperTick int64, liquidityDelta sdk.Dec, joinTime time.Time, positionId uint64) (sdk.Int, sdk.Int, bool, bool, error)
+	UpdatePosition(ctx sdk.Context, poolId uint64, owner sdk.AccAddress, lowerTick, upperTick int64, liquidityDelta sdk.Dec, joinTime time.Time, positionId uint64) (cltypes.UpdatePositionData, error)
 	GetConcentratedPoolById(ctx sdk.Context, poolId uint64) (cltypes.ConcentratedPoolExtension, error)
 	CreateFullRangePositionLocked(ctx sdk.Context, clPoolId uint64, owner sdk.AccAddress, coins sdk.Coins, remainingLockDuration time.Duration) (positionData cltypes.CreateFullRangePositionData, concentratedLockID uint64, err error)
 	CreateFullRangePositionUnlocking(ctx sdk.Context, clPoolId uint64, owner sdk.AccAddress, coins sdk.Coins, remainingLockDuration time.Duration) (positionData cltypes.CreateFullRangePositionData, concentratedLockID uint64, err error)