Skip to content

Commit

Permalink
refactor: reduce number of returns for creating full range position (…
Browse files Browse the repository at this point in the history 
…backport #6004) (#6009)

* refactor: reduce number of returns for creating full range position (backport #6004)

* changelog

---------

Co-authored-by: Roman <roman@osmosis.team>
  • Loading branch information
@mergify @p0mvn
mergify[bot] and p0mvn committed Aug 10, 2023
1 parent b38ffb5 commit 4509279
Show file tree
Hide file tree
Showing 22 changed files with 246 additions and 233 deletions.
4 changes: 2 additions & 2 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ 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

### Bug Fixes
* [#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

### Misc Improvements
* [#5976](https://github.com/osmosis-labs/osmosis/pull/5976) chore[e2e]: faster epoch
Expand Down
8 changes: 4 additions & 4 deletions app/apptesting/concentrated_liquidity.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions app/upgrades/v16/upgrades.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,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
}
Expand All @@ -157,7 +157,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)
Expand Down
40 changes: 17 additions & 23 deletions x/concentrated-liquidity/lp_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -391,25 +391,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 types.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() {
Expand Down Expand Up @@ -1976,8 +1976,8 @@ func (s *KeeperTestSuite) TestIsLockMature() {
s.Run(name, func() {
tc := tc
var (
positionId uint64
concentratedLockId uint64
positionData types.CreateFullRangePositionData
err error
)
s.SetupTest()
Expand All @@ -1989,17 +1989,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
Expand All @@ -2008,11 +2006,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)
})
}
}
Expand Down
2 changes: 1 addition & 1 deletion x/concentrated-liquidity/pool_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -682,7 +682,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)
}

Expand Down
40 changes: 20 additions & 20 deletions x/concentrated-liquidity/position.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Loading

0 comments on commit 4509279

Please sign in to comment.