refactor: reduce number of returns for creating full range position (#…

…6004) * refactor: reduce number of returns for creating full range position * update changelog --------- Co-authored-by: devbot-wizard <[email protected]>
osmosis-labs · Aug 9, 2023 · 07c8556 · 07c8556
1 parent c3129e0
commit 07c8556
Show file tree

Hide file tree

Showing 23 changed files with 248 additions and 232 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -45,6 +45,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### API breaks
 
 * [#5983](https://github.com/osmosis-labs/osmosis/pull/5983) refactor(CL): 6 return values in CL CreatePosition with a struct
+* [#6004](https://github.com/osmosis-labs/osmosis/pull/6004) reduce number of returns for creating full range position
 
 ### Features
 

diff --git a/app/apptesting/concentrated_liquidity.go b/app/apptesting/concentrated_liquidity.go
@@ -72,11 +72,11 @@ func (s *KeeperTestHelper) PrepareConcentratedPoolWithCoinsAndLockedFullRangePos
 	clPool := s.PrepareCustomConcentratedPool(s.TestAccs[0], denom1, denom2, DefaultTickSpacing, sdk.ZeroDec())
 	fundCoins := sdk.NewCoins(sdk.NewCoin(denom1, DefaultCoinAmount), sdk.NewCoin(denom2, DefaultCoinAmount))
 	s.FundAcc(s.TestAccs[0], fundCoins)
-	positionId, _, _, _, concentratedLockId, err := s.App.ConcentratedLiquidityKeeper.CreateFullRangePositionLocked(s.Ctx, clPool.GetId(), s.TestAccs[0], fundCoins, time.Hour*24*14)
+	positionData, concentratedLockId, err := s.App.ConcentratedLiquidityKeeper.CreateFullRangePositionLocked(s.Ctx, clPool.GetId(), s.TestAccs[0], fundCoins, time.Hour*24*14)
 	s.Require().NoError(err)
 	clPool, err = s.App.ConcentratedLiquidityKeeper.GetConcentratedPoolById(s.Ctx, clPool.GetId())
 	s.Require().NoError(err)
-	return clPool, concentratedLockId, positionId
+	return clPool, concentratedLockId, positionData.ID
 }
 
 // PrepareCustomConcentratedPool sets up a concentrated liquidity pool with the custom parameters.
@@ -113,9 +113,9 @@ func (s *KeeperTestHelper) PrepareMultipleConcentratedPools(poolsToCreate uint16
 // CreateFullRangePosition creates a full range position and returns position id and the liquidity created.
 func (s *KeeperTestHelper) CreateFullRangePosition(pool types.ConcentratedPoolExtension, coins sdk.Coins) (uint64, sdk.Dec) {
 	s.FundAcc(s.TestAccs[0], coins)
-	positionId, _, _, liquidity, err := s.App.ConcentratedLiquidityKeeper.CreateFullRangePosition(s.Ctx, pool.GetId(), s.TestAccs[0], coins)
+	positionData, err := s.App.ConcentratedLiquidityKeeper.CreateFullRangePosition(s.Ctx, pool.GetId(), s.TestAccs[0], coins)
 	s.Require().NoError(err)
-	return positionId, liquidity
+	return positionData.ID, positionData.Liquidity
 }
 
 // WithdrawFullRangePosition withdraws given liquidity from a position specified by id.

diff --git a/app/upgrades/v16/upgrades.go b/app/upgrades/v16/upgrades.go
@@ -149,7 +149,7 @@ func CreateUpgradeHandler(
 
 		// Create a full range position via the community pool with the funds that were swapped.
 		fullRangeOsmoDaiCoins := sdk.NewCoins(respectiveOsmo, oneDai)
-		_, actualOsmoAmtUsed, actualDaiAmtUsed, _, err := keepers.ConcentratedLiquidityKeeper.CreateFullRangePosition(ctx, clPoolId, communityPoolAddress, fullRangeOsmoDaiCoins)
+		positionData, err := keepers.ConcentratedLiquidityKeeper.CreateFullRangePosition(ctx, clPoolId, communityPoolAddress, fullRangeOsmoDaiCoins)
 		if err != nil {
 			return nil, err
 		}
@@ -158,7 +158,7 @@ func CreateUpgradeHandler(
 
 		// Remove coins we used from the community pool to make the CL position
 		feePool := keepers.DistrKeeper.GetFeePool(ctx)
-		fulllRangeOsmoDaiCoinsUsed := sdk.NewCoins(sdk.NewCoin(DesiredDenom0, actualOsmoAmtUsed), sdk.NewCoin(DAIIBCDenom, actualDaiAmtUsed))
+		fulllRangeOsmoDaiCoinsUsed := sdk.NewCoins(sdk.NewCoin(DesiredDenom0, positionData.Amount0), sdk.NewCoin(DAIIBCDenom, positionData.Amount1))
 		newPool, negative := feePool.CommunityPool.SafeSub(sdk.NewDecCoinsFromCoins(fulllRangeOsmoDaiCoinsUsed...))
 		if negative {
 			return nil, fmt.Errorf("community pool cannot be negative: %s", newPool)

diff --git a/app/upgrades/v17/upgrades.go b/app/upgrades/v17/upgrades.go
@@ -85,13 +85,13 @@ func CreateUpgradeHandler(
 
 			// Create a full range position via the community pool with the funds we calculated above.
 			fullRangeCoins := sdk.NewCoins(respectiveBaseAsset, oneOsmo)
-			_, actualBaseAmtUsed, actualQuoteAmtUsed, _, err := keepers.ConcentratedLiquidityKeeper.CreateFullRangePosition(ctx, clPoolId, communityPoolAddress, fullRangeCoins)
+			positionData, err := keepers.ConcentratedLiquidityKeeper.CreateFullRangePosition(ctx, clPoolId, communityPoolAddress, fullRangeCoins)
 			if err != nil {
 				return nil, err
 			}
 
 			// Track the coins used to create the full range position (we manually update the fee pool later all at once).
-			fullRangeCoinsUsed = fullRangeCoinsUsed.Add(sdk.NewCoins(sdk.NewCoin(QuoteAsset, actualQuoteAmtUsed), sdk.NewCoin(assetPair.BaseAsset, actualBaseAmtUsed))...)
+			fullRangeCoinsUsed = fullRangeCoinsUsed.Add(sdk.NewCoins(sdk.NewCoin(QuoteAsset, positionData.Amount1), sdk.NewCoin(assetPair.BaseAsset, positionData.Amount0))...)
 
 			// If pair was previously superfluid enabled, add the cl pool's full range denom as an authorized superfluid asset.
 			if assetPair.Superfluid {

diff --git a/x/concentrated-liquidity/lp_test.go b/x/concentrated-liquidity/lp_test.go
@@ -12,6 +12,7 @@ import (
 	cl "github.com/osmosis-labs/osmosis/v17/x/concentrated-liquidity"
 	"github.com/osmosis-labs/osmosis/v17/x/concentrated-liquidity/model"
 	clmodel "github.com/osmosis-labs/osmosis/v17/x/concentrated-liquidity/model"
+	cltypes "github.com/osmosis-labs/osmosis/v17/x/concentrated-liquidity/types"
 	types "github.com/osmosis-labs/osmosis/v17/x/concentrated-liquidity/types"
 )
 
@@ -391,25 +392,25 @@ const (
 
 func (s *KeeperTestSuite) createPositionWithLockState(ls lockState, poolId uint64, owner sdk.AccAddress, providedCoins sdk.Coins, dur time.Duration) (uint64, sdk.Dec) {
 	var (
-		positionData     cl.CreatePositionData
-		liquidityCreated sdk.Dec
-		positionId       uint64
-		err              error
+		positionData          cl.CreatePositionData
+		fullRangePositionData cltypes.CreateFullRangePositionData
+		err                   error
 	)
 
 	if ls == locked {
-		positionId, _, _, liquidityCreated, _, err = s.clk.CreateFullRangePositionLocked(s.Ctx, poolId, owner, providedCoins, dur)
+		fullRangePositionData, _, err = s.clk.CreateFullRangePositionLocked(s.Ctx, poolId, owner, providedCoins, dur)
 	} else if ls == unlocking {
-		positionId, _, _, liquidityCreated, _, err = s.clk.CreateFullRangePositionUnlocking(s.Ctx, poolId, owner, providedCoins, dur+time.Hour)
+		fullRangePositionData, _, err = s.clk.CreateFullRangePositionUnlocking(s.Ctx, poolId, owner, providedCoins, dur+time.Hour)
 	} else if ls == unlocked {
-		positionId, _, _, liquidityCreated, _, err = s.clk.CreateFullRangePositionUnlocking(s.Ctx, poolId, owner, providedCoins, dur-time.Hour)
+		fullRangePositionData, _, err = s.clk.CreateFullRangePositionUnlocking(s.Ctx, poolId, owner, providedCoins, dur-time.Hour)
 	} else {
 		positionData, err = s.clk.CreatePosition(s.Ctx, poolId, owner, providedCoins, sdk.ZeroInt(), sdk.ZeroInt(), DefaultLowerTick, DefaultUpperTick)
-		positionId = positionData.ID
-		liquidityCreated = positionData.Liquidity
+		s.Require().NoError(err)
+		return positionData.ID, positionData.Liquidity
 	}
+	// full range case
 	s.Require().NoError(err)
-	return positionId, liquidityCreated
+	return fullRangePositionData.ID, fullRangePositionData.Liquidity
 }
 
 func (s *KeeperTestSuite) TestWithdrawPosition() {
@@ -1999,8 +2000,8 @@ func (s *KeeperTestSuite) TestIsLockMature() {
 		s.Run(name, func() {
 			tc := tc
 			var (
-				positionId         uint64
 				concentratedLockId uint64
+				positionData       types.CreateFullRangePositionData
 				err                error
 			)
 			s.SetupTest()
@@ -2012,17 +2013,15 @@ func (s *KeeperTestSuite) TestIsLockMature() {
 			s.FundAcc(s.TestAccs[0], coinsToFund)
 
 			if tc.unlockingPosition {
-				positionId, _, _, _, concentratedLockId, err = s.App.ConcentratedLiquidityKeeper.CreateFullRangePositionUnlocking(s.Ctx, pool.GetId(), s.TestAccs[0], coinsToFund, tc.remainingLockDuration)
-				s.Require().NoError(err)
+				positionData, concentratedLockId, err = s.App.ConcentratedLiquidityKeeper.CreateFullRangePositionUnlocking(s.Ctx, pool.GetId(), s.TestAccs[0], coinsToFund, tc.remainingLockDuration)
 			} else if tc.lockedPosition {
-				positionId, _, _, _, concentratedLockId, err = s.App.ConcentratedLiquidityKeeper.CreateFullRangePositionLocked(s.Ctx, pool.GetId(), s.TestAccs[0], coinsToFund, tc.remainingLockDuration)
-				s.Require().NoError(err)
+				positionData, concentratedLockId, err = s.App.ConcentratedLiquidityKeeper.CreateFullRangePositionLocked(s.Ctx, pool.GetId(), s.TestAccs[0], coinsToFund, tc.remainingLockDuration)
 			} else {
-				positionId, _, _, _, err = s.App.ConcentratedLiquidityKeeper.CreateFullRangePosition(s.Ctx, pool.GetId(), s.TestAccs[0], coinsToFund)
-				s.Require().NoError(err)
+				positionData, err = s.App.ConcentratedLiquidityKeeper.CreateFullRangePosition(s.Ctx, pool.GetId(), s.TestAccs[0], coinsToFund)
 			}
+			s.Require().NoError(err)
 
-			_, err = s.App.ConcentratedLiquidityKeeper.GetPosition(s.Ctx, positionId)
+			_, err = s.App.ConcentratedLiquidityKeeper.GetPosition(s.Ctx, positionData.ID)
 			s.Require().NoError(err)
 
 			// Increment block time by a second to ensure test cases with zero lock duration are in the past
@@ -2031,11 +2030,7 @@ func (s *KeeperTestSuite) TestIsLockMature() {
 			// System under test
 			lockIsMature, _ := s.App.ConcentratedLiquidityKeeper.IsLockMature(s.Ctx, concentratedLockId)
 
-			if tc.expectedLockIsMature {
-				s.Require().True(lockIsMature)
-			} else {
-				s.Require().False(lockIsMature)
-			}
+			s.Require().Equal(tc.expectedLockIsMature, lockIsMature)
 		})
 	}
 }

diff --git a/x/concentrated-liquidity/pool_test.go b/x/concentrated-liquidity/pool_test.go
@@ -683,7 +683,7 @@ func (s *KeeperTestSuite) TestGetUserUnbondingPositions() {
 
 			// Create 3 locked positions
 			for i := 0; i < 3; i++ {
-				_, _, _, _, _, err := s.App.ConcentratedLiquidityKeeper.CreateFullRangePositionLocked(s.Ctx, clPool.GetId(), defaultAddress, defaultFunds, time.Hour)
+				_, _, err := s.App.ConcentratedLiquidityKeeper.CreateFullRangePositionLocked(s.Ctx, clPool.GetId(), defaultAddress, defaultFunds, time.Hour)
 				s.Require().NoError(err)
 			}
 

diff --git a/x/concentrated-liquidity/position.go b/x/concentrated-liquidity/position.go
@@ -393,78 +393,78 @@ func (k Keeper) deletePosition(ctx sdk.Context,
 
 // CreateFullRangePosition creates a full range (min to max tick) concentrated liquidity position for the given pool ID, owner, and coins.
 // The function returns the amounts of token 0 and token 1, and the liquidity created from the position.
-func (k Keeper) CreateFullRangePosition(ctx sdk.Context, poolId uint64, owner sdk.AccAddress, coins sdk.Coins) (positionId uint64, amount0, amount1 sdk.Int, liquidity sdk.Dec, err error) {
+func (k Keeper) CreateFullRangePosition(ctx sdk.Context, poolId uint64, owner sdk.AccAddress, coins sdk.Coins) (types.CreateFullRangePositionData, error) {
 	// Check that exactly two coins are provided.
 	if len(coins) != 2 {
-		return 0, sdk.Int{}, sdk.Int{}, sdk.Dec{}, types.NumCoinsError{NumCoins: len(coins)}
+		return types.CreateFullRangePositionData{}, types.NumCoinsError{NumCoins: len(coins)}
 	}
 
 	concentratedPool, err := k.GetConcentratedPoolById(ctx, poolId)
 	if err != nil {
-		return 0, sdk.Int{}, sdk.Int{}, sdk.Dec{}, err
+		return types.CreateFullRangePositionData{}, err
 	}
 
 	// Defense in depth, ensure coins provided match the pool's token denominations.
 	if coins.AmountOf(concentratedPool.GetToken0()).LTE(sdk.ZeroInt()) {
-		return 0, sdk.Int{}, sdk.Int{}, sdk.Dec{}, types.Amount0IsNegativeError{Amount0: coins.AmountOf(concentratedPool.GetToken0())}
+		return types.CreateFullRangePositionData{}, types.Amount0IsNegativeError{Amount0: coins.AmountOf(concentratedPool.GetToken0())}
 	}
 	if coins.AmountOf(concentratedPool.GetToken1()).LTE(sdk.ZeroInt()) {
-		return 0, sdk.Int{}, sdk.Int{}, sdk.Dec{}, types.Amount1IsNegativeError{Amount1: coins.AmountOf(concentratedPool.GetToken1())}
+		return types.CreateFullRangePositionData{}, types.Amount1IsNegativeError{Amount1: coins.AmountOf(concentratedPool.GetToken1())}
 	}
 
 	// Create a full range (min to max tick) concentrated liquidity position.
 	positionData, err := k.CreatePosition(ctx, concentratedPool.GetId(), owner, coins, sdk.ZeroInt(), sdk.ZeroInt(), types.MinInitializedTick, types.MaxTick)
 	if err != nil {
-		return 0, sdk.Int{}, sdk.Int{}, sdk.Dec{}, err
+		return types.CreateFullRangePositionData{}, err
 	}
 
-	return positionData.ID, positionData.Amount0, positionData.Amount1, positionData.Liquidity, nil
+	return types.CreateFullRangePositionData{ID: positionData.ID, Amount0: positionData.Amount0, Amount1: positionData.Amount1, Liquidity: positionData.Liquidity}, nil
 }
 
 // CreateFullRangePositionLocked creates a full range (min to max tick) concentrated liquidity position for the given pool ID, owner, and coins.
 // CL shares are minted which represent the underlying liquidity and are locked for the given duration.
 // State entries are also created to map the position ID to the underlying lock ID.
-func (k Keeper) CreateFullRangePositionLocked(ctx sdk.Context, clPoolId uint64, owner sdk.AccAddress, coins sdk.Coins, remainingLockDuration time.Duration) (positionId uint64, amount0, amount1 sdk.Int, liquidity sdk.Dec, concentratedLockID uint64, err error) {
+func (k Keeper) CreateFullRangePositionLocked(ctx sdk.Context, clPoolId uint64, owner sdk.AccAddress, coins sdk.Coins, remainingLockDuration time.Duration) (positionData types.CreateFullRangePositionData, concentratedLockID uint64, err error) {
 	// Create a full range (min to max tick) concentrated liquidity position.
-	positionId, amount0, amount1, liquidity, err = k.CreateFullRangePosition(ctx, clPoolId, owner, coins)
+	positionData, err = k.CreateFullRangePosition(ctx, clPoolId, owner, coins)
 	if err != nil {
-		return 0, sdk.Int{}, sdk.Int{}, sdk.Dec{}, 0, err
+		return types.CreateFullRangePositionData{}, 0, err
 	}
 
 	// Mint CL shares (similar to GAMM shares) for the position and lock them for the remaining lock duration.
 	// Also sets the position ID to underlying lock ID mapping.
-	concentratedLockId, _, err := k.mintSharesAndLock(ctx, clPoolId, positionId, owner, remainingLockDuration)
+	concentratedLockId, _, err := k.mintSharesAndLock(ctx, clPoolId, positionData.ID, owner, remainingLockDuration)
 	if err != nil {
-		return 0, sdk.Int{}, sdk.Int{}, sdk.Dec{}, 0, err
+		return types.CreateFullRangePositionData{}, 0, err
 	}
 
-	return positionId, amount0, amount1, liquidity, concentratedLockId, nil
+	return positionData, concentratedLockId, nil
 }
 
 // CreateFullRangePositionUnlocking creates a full range (min to max tick) concentrated liquidity position for the given pool ID, owner, and coins.
 // This function is strictly used when migrating a balancer position to CL, where the balancer position is currently unlocking.
 // We lock the cl position for whatever the remaining time is from the balancer position and immediately begin unlocking from where it left off.
-func (k Keeper) CreateFullRangePositionUnlocking(ctx sdk.Context, clPoolId uint64, owner sdk.AccAddress, coins sdk.Coins, remainingLockDuration time.Duration) (positionId uint64, amount0, amount1 sdk.Int, liquidity sdk.Dec, concentratedLockID uint64, err error) {
+func (k Keeper) CreateFullRangePositionUnlocking(ctx sdk.Context, clPoolId uint64, owner sdk.AccAddress, coins sdk.Coins, remainingLockDuration time.Duration) (positionData types.CreateFullRangePositionData, concentratedLockID uint64, err error) {
 	// Create a full range (min to max tick) concentrated liquidity position.
-	positionId, amount0, amount1, liquidity, err = k.CreateFullRangePosition(ctx, clPoolId, owner, coins)
+	positionData, err = k.CreateFullRangePosition(ctx, clPoolId, owner, coins)
 	if err != nil {
-		return 0, sdk.Int{}, sdk.Int{}, sdk.Dec{}, 0, err
+		return types.CreateFullRangePositionData{}, 0, err
 	}
 
 	// Mint cl shares for the position and lock them for the remaining lock duration.
 	// Also sets the position ID to underlying lock ID mapping.
-	concentratedLockId, underlyingLiquidityTokenized, err := k.mintSharesAndLock(ctx, clPoolId, positionId, owner, remainingLockDuration)
+	concentratedLockId, underlyingLiquidityTokenized, err := k.mintSharesAndLock(ctx, clPoolId, positionData.ID, owner, remainingLockDuration)
 	if err != nil {
-		return 0, sdk.Int{}, sdk.Int{}, sdk.Dec{}, 0, err
+		return types.CreateFullRangePositionData{}, 0, err
 	}
 
 	// Begin unlocking the newly created concentrated lock.
 	concentratedLockID, err = k.lockupKeeper.BeginForceUnlock(ctx, concentratedLockId, underlyingLiquidityTokenized)
 	if err != nil {
-		return 0, sdk.Int{}, sdk.Int{}, sdk.Dec{}, 0, err
+		return types.CreateFullRangePositionData{}, 0, err
 	}
 
-	return positionId, amount0, amount1, liquidity, concentratedLockID, nil
+	return positionData, concentratedLockID, nil
 }
 
 // mintSharesAndLock mints the shares for the full range concentrated liquidity position and locks them for the given duration. It also updates the position ID to underlying lock ID mapping.