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

refactor(CL): replace 6 return values in CreatePosition with a struct #5983

Merged
merged 6 commits into from
Aug 8, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## Unreleased

### API breaks

* [#5983](https://github.com/osmosis-labs/osmosis/pull/5983) refactor(CL): 6 return values in CL CreatePosition with a struct

### Features

* [#5072](https://github.com/osmosis-labs/osmosis/pull/5072) IBC-hooks: Add support for async acks when processing onRecvPacket
Expand Down
4 changes: 2 additions & 2 deletions x/concentrated-liquidity/bench_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ type BenchTestSuite struct {
func (s *BenchTestSuite) createPosition(accountIndex int, poolId uint64, coin0, coin1 sdk.Coin, lowerTick, upperTick int64) {
tokensDesired := sdk.NewCoins(coin0, coin1)

_, _, _, _, _, _, err := s.App.ConcentratedLiquidityKeeper.CreatePosition(s.Ctx, poolId, s.TestAccs[accountIndex], tokensDesired, sdk.ZeroInt(), sdk.ZeroInt(), lowerTick, upperTick)
_, err := s.App.ConcentratedLiquidityKeeper.CreatePosition(s.Ctx, poolId, s.TestAccs[accountIndex], tokensDesired, sdk.ZeroInt(), sdk.ZeroInt(), lowerTick, upperTick)
if err != nil {
// This can happen for ticks that map to the very small prices
// e.g 2 * 10^(-18) ends up mapping to the same sqrt price
Expand Down Expand Up @@ -113,7 +113,7 @@ func runBenchmark(b *testing.B, testFunc func(b *testing.B, s *BenchTestSuite, p
tokenDesired0 := sdk.NewCoin(denom0, sdk.NewInt(100))
tokenDesired1 := sdk.NewCoin(denom1, sdk.NewInt(100))
tokensDesired := sdk.NewCoins(tokenDesired0, tokenDesired1)
_, _, _, _, _, _, err = clKeeper.CreatePosition(s.Ctx, clPoolId, s.TestAccs[0], tokensDesired, sdk.ZeroInt(), sdk.ZeroInt(), types.MinInitializedTick, types.MaxTick)
_, err = clKeeper.CreatePosition(s.Ctx, clPoolId, s.TestAccs[0], tokensDesired, sdk.ZeroInt(), sdk.ZeroInt(), types.MinInitializedTick, types.MaxTick)
noError(b, err)

pool, err := clKeeper.GetPoolById(s.Ctx, clPoolId)
Expand Down
8 changes: 4 additions & 4 deletions x/concentrated-liquidity/fuzz_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -490,13 +490,13 @@ func (s *KeeperTestSuite) addRandomPositon(r *rand.Rand, poolId uint64, minTick,

fmt.Println("creating position: ", "accountName", "lowerTick", lowerTick, "upperTick", upperTick, "token0Desired", tokenDesired0, "tokenDesired1", tokenDesired1)

positionId, amt0, amt1, liq, _, _, err := s.App.ConcentratedLiquidityKeeper.CreatePosition(s.Ctx, poolId, s.TestAccs[accountIndex], tokensDesired, sdk.ZeroInt(), sdk.ZeroInt(), types.MinInitializedTick, types.MaxTick)
positionData, err := s.App.ConcentratedLiquidityKeeper.CreatePosition(s.Ctx, poolId, s.TestAccs[accountIndex], tokensDesired, sdk.ZeroInt(), sdk.ZeroInt(), types.MinInitializedTick, types.MaxTick)
s.Require().NoError(err)
fmt.Printf("actually created: %s%s %s%s \n", amt0, ETH, amt1, USDC)
fmt.Printf("actually created: %s%s %s%s \n", positionData.Amount0, ETH, positionData.Amount1, USDC)

s.positionData = append(s.positionData, positionAndLiquidity{
positionId: positionId,
liquidity: liq,
positionId: positionData.ID,
liquidity: positionData.Liquidity,
accountIndex: accountIndex,
})
}
Expand Down
19 changes: 10 additions & 9 deletions x/concentrated-liquidity/incentives_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1048,8 +1048,9 @@ func (s *KeeperTestSuite) TestUpdateUptimeAccumulatorsToNow() {
if !tc.isInvalidBalancerPool {
depositedCoins := sdk.NewCoins(sdk.NewCoin(clPool.GetToken0(), testQualifyingDepositsOne), sdk.NewCoin(clPool.GetToken1(), testQualifyingDepositsOne))
s.FundAcc(testAddressOne, depositedCoins)
_, _, _, qualifyingLiquidity, _, _, err = clKeeper.CreatePosition(s.Ctx, clPool.GetId(), testAddressOne, depositedCoins, sdk.ZeroInt(), sdk.ZeroInt(), clPool.GetCurrentTick()-100, clPool.GetCurrentTick()+100)
positionData, err := clKeeper.CreatePosition(s.Ctx, clPool.GetId(), testAddressOne, depositedCoins, sdk.ZeroInt(), sdk.ZeroInt(), clPool.GetCurrentTick()-100, clPool.GetCurrentTick()+100)
s.Require().NoError(err)
qualifyingLiquidity = positionData.Liquidity

// If a canonical balancer pool exists, we add its respective shares to the qualifying amount as well.
clPool, err = clKeeper.GetPoolById(s.Ctx, clPool.GetId())
Expand Down Expand Up @@ -3127,7 +3128,7 @@ func (s *KeeperTestSuite) TestPrepareClaimAllIncentivesForPosition() {
pool := s.PrepareConcentratedPool()

// Set up position
positionIdOne, _, _, _, _, _, err := s.clk.CreatePosition(s.Ctx, pool.GetId(), s.TestAccs[0], requiredBalances, sdk.ZeroInt(), sdk.ZeroInt(), DefaultLowerTick, DefaultUpperTick)
positionOneData, err := s.clk.CreatePosition(s.Ctx, pool.GetId(), s.TestAccs[0], requiredBalances, sdk.ZeroInt(), sdk.ZeroInt(), DefaultLowerTick, DefaultUpperTick)
s.Require().NoError(err)

// Set incentives for pool to ensure accumulators work correctly
Expand Down Expand Up @@ -3160,7 +3161,7 @@ func (s *KeeperTestSuite) TestPrepareClaimAllIncentivesForPosition() {
// Determine what the expected forfeited incentives per share is, including the dust since we reinvest the dust when we forfeit.
if tc.blockTimeElapsed < tc.minUptimeIncentiveRecord {
for _, uptimeAccum := range uptimeAccumulatorsPreClaim {
newPositionName := string(types.KeyPositionId(positionIdOne))
newPositionName := string(types.KeyPositionId(positionOneData.ID))
// Check if the accumulator contains the position.
hasPosition := uptimeAccum.HasPosition(newPositionName)
if hasPosition {
Expand All @@ -3178,7 +3179,7 @@ func (s *KeeperTestSuite) TestPrepareClaimAllIncentivesForPosition() {
}

// System under test
collectedInc, forfeitedIncentives, err := s.clk.PrepareClaimAllIncentivesForPosition(s.Ctx, positionIdOne)
collectedInc, forfeitedIncentives, err := s.clk.PrepareClaimAllIncentivesForPosition(s.Ctx, positionOneData.ID)
s.Require().NoError(err)
s.Require().Equal(tc.expectedCoins.String(), collectedInc.String())
s.Require().Equal(expectedForfeitedIncentives.String(), forfeitedIncentives.String())
Expand Down Expand Up @@ -3262,35 +3263,35 @@ func (s *KeeperTestSuite) TestFunctional_ClaimIncentives_LiquidityChange_Varying
s.Require().NoError(err)

// Set up position
positionIdOne, _, _, _, _, _, err := s.App.ConcentratedLiquidityKeeper.CreatePosition(s.Ctx, defaultPoolId, defaultAddress, DefaultCoins, sdk.ZeroInt(), sdk.ZeroInt(), DefaultLowerTick, DefaultUpperTick)
positionOneData, err := s.App.ConcentratedLiquidityKeeper.CreatePosition(s.Ctx, defaultPoolId, defaultAddress, DefaultCoins, sdk.ZeroInt(), sdk.ZeroInt(), DefaultLowerTick, DefaultUpperTick)
s.Require().NoError(err)

// Increase block time by the fully charged duration (first time)
s.Ctx = s.Ctx.WithBlockTime(s.Ctx.BlockTime().Add(testFullChargeDuration))

// Claim incentives.
collected, _, err := s.App.ConcentratedLiquidityKeeper.CollectIncentives(s.Ctx, defaultAddress, positionIdOne)
collected, _, err := s.App.ConcentratedLiquidityKeeper.CollectIncentives(s.Ctx, defaultAddress, positionOneData.ID)
s.Require().NoError(err)
s.Require().Equal(expectedCoinsPerFullCharge.String(), collected.String())

// Increase block time by the fully charged duration (second time)
s.Ctx = s.Ctx.WithBlockTime(s.Ctx.BlockTime().Add(testFullChargeDuration))

// Create another position
positionIdTwo, _, _, _, _, _, err := s.App.ConcentratedLiquidityKeeper.CreatePosition(s.Ctx, defaultPoolId, defaultAddress, DefaultCoins, sdk.ZeroInt(), sdk.ZeroInt(), DefaultLowerTick, DefaultUpperTick)
positionTwoData, err := s.App.ConcentratedLiquidityKeeper.CreatePosition(s.Ctx, defaultPoolId, defaultAddress, DefaultCoins, sdk.ZeroInt(), sdk.ZeroInt(), DefaultLowerTick, DefaultUpperTick)
s.Require().NoError(err)

// Increase block time by the fully charged duration (third time)
s.Ctx = s.Ctx.WithBlockTime(s.Ctx.BlockTime().Add(testFullChargeDuration))

// Claim for second position. Must only claim half of the original expected amount since now there are 2 positions.
collected, _, err = s.App.ConcentratedLiquidityKeeper.CollectIncentives(s.Ctx, defaultAddress, positionIdTwo)
collected, _, err = s.App.ConcentratedLiquidityKeeper.CollectIncentives(s.Ctx, defaultAddress, positionTwoData.ID)
s.Require().NoError(err)
s.Require().Equal(expectedHalfOfExpectedCoinsPerFullCharge.String(), collected.String())

// Claim for first position and observe that claims full expected charge for the period between 1st claim and 2nd position creation
// and half of the full charge amount since the 2nd position was created.
collected, _, err = s.App.ConcentratedLiquidityKeeper.CollectIncentives(s.Ctx, defaultAddress, positionIdOne)
collected, _, err = s.App.ConcentratedLiquidityKeeper.CollectIncentives(s.Ctx, defaultAddress, positionOneData.ID)
s.Require().NoError(err)
// Note, adding one since both expected amounts already subtract one (-2 in total)
s.Require().Equal(expectedCoinsPerFullCharge.Add(expectedHalfOfExpectedCoinsPerFullCharge.Add(oneUUSDCCoin)...).String(), collected.String())
Expand Down
10 changes: 5 additions & 5 deletions x/concentrated-liquidity/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,11 +130,11 @@ func (s *KeeperTestSuite) SetupPosition(poolId uint64, owner sdk.AccAddress, pro
}

s.FundAcc(owner, providedCoins.Add(roundingErrorCoins...))
positionId, _, _, _, _, _, err := s.App.ConcentratedLiquidityKeeper.CreatePosition(s.Ctx, poolId, owner, providedCoins, sdk.ZeroInt(), sdk.ZeroInt(), lowerTick, upperTick)
positionData, err := s.App.ConcentratedLiquidityKeeper.CreatePosition(s.Ctx, poolId, owner, providedCoins, sdk.ZeroInt(), sdk.ZeroInt(), lowerTick, upperTick)
s.Require().NoError(err)
liquidity, err := s.App.ConcentratedLiquidityKeeper.GetPositionLiquidity(s.Ctx, positionId)
liquidity, err := s.App.ConcentratedLiquidityKeeper.GetPositionLiquidity(s.Ctx, positionData.ID)
s.Require().NoError(err)
return liquidity, positionId
return liquidity, positionData.ID
}

// SetupDefaultPositions sets up four different positions to the given pool with different accounts for each position./
Expand Down Expand Up @@ -467,9 +467,9 @@ func (s *KeeperTestSuite) runFungifySetup(address sdk.AccAddress, numPositions i
// Set up fully charged positions
totalLiquidity := sdk.ZeroDec()
for i := 0; i < numPositions; i++ {
_, _, _, liquidityCreated, _, _, err := s.clk.CreatePosition(s.Ctx, defaultPoolId, address, DefaultCoins, sdk.ZeroInt(), sdk.ZeroInt(), DefaultLowerTick, DefaultUpperTick)
positionData, err := s.clk.CreatePosition(s.Ctx, defaultPoolId, address, DefaultCoins, sdk.ZeroInt(), sdk.ZeroInt(), DefaultLowerTick, DefaultUpperTick)
s.Require().NoError(err)
totalLiquidity = totalLiquidity.Add(liquidityCreated)
totalLiquidity = totalLiquidity.Add(positionData.Liquidity)
}

return pool, expectedPositionIds, totalLiquidity
Expand Down
67 changes: 42 additions & 25 deletions x/concentrated-liquidity/lp.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,16 @@ import (

const noUnderlyingLockId = uint64(0)

// CreatePositionData reprsents the return data from CreatePosition.
ValarDragon marked this conversation as resolved.
Show resolved Hide resolved
type CreatePositionData struct {
ID uint64
Amount0 sdk.Int
Amount1 sdk.Int
Liquidity sdk.Dec
LowerTick int64
UpperTick int64
}

// createPosition creates a concentrated liquidity position in range between lowerTick and upperTick
// in a given poolId with the desired amount of each token. Since LPs are only allowed to provide
// liquidity proportional to the existing reserves, the actual amount of tokens used might differ from requested.
Expand All @@ -32,92 +42,92 @@ const noUnderlyingLockId = uint64(0)
// - the liquidity delta is zero
// - the amount0 or amount1 returned from the position update is less than the given minimums
// - the pool or user does not have enough tokens to satisfy the requested amount
func (k Keeper) CreatePosition(ctx sdk.Context, poolId uint64, owner sdk.AccAddress, tokensProvided sdk.Coins, amount0Min, amount1Min sdk.Int, lowerTick, upperTick int64) (positionId uint64, actualAmount0 sdk.Int, actualAmount1 sdk.Int, liquidityDelta sdk.Dec, lowerTickResult int64, upperTickResult int64, err error) {
func (k Keeper) CreatePosition(ctx sdk.Context, poolId uint64, owner sdk.AccAddress, tokensProvided sdk.Coins, amount0Min, amount1Min sdk.Int, lowerTick, upperTick int64) (positionDate CreatePositionData, err error) {
// Use the current blockTime as the position's join time.
joinTime := ctx.BlockTime()

// Retrieve the pool associated with the given pool ID.
pool, err := k.getPoolById(ctx, poolId)
if err != nil {
return 0, sdk.Int{}, sdk.Int{}, sdk.Dec{}, 0, 0, err
return CreatePositionData{}, err
}

for _, token := range tokensProvided {
if token.Denom != pool.GetToken0() && token.Denom != pool.GetToken1() {
return 0, sdk.Int{}, sdk.Int{}, sdk.Dec{}, 0, 0, errors.New("token provided is not one of the pool tokens")
return CreatePositionData{}, errors.New("token provided is not one of the pool tokens")
}
}

// Check if the provided tick range is valid according to the pool's tick spacing and module parameters.
if err := validateTickRangeIsValid(pool.GetTickSpacing(), lowerTick, upperTick); err != nil {
return 0, sdk.Int{}, sdk.Int{}, sdk.Dec{}, 0, 0, err
return CreatePositionData{}, err
}
amount0Desired := tokensProvided.AmountOf(pool.GetToken0())
amount1Desired := tokensProvided.AmountOf(pool.GetToken1())
if amount0Desired.IsZero() && amount1Desired.IsZero() {
return 0, sdk.Int{}, sdk.Int{}, sdk.Dec{}, 0, 0, errors.New("cannot create a position with zero amounts of both pool tokens")
return CreatePositionData{}, errors.New("cannot create a position with zero amounts of both pool tokens")
}

// sanity check that both given minimum accounts are not negative amounts.
if amount0Min.IsNegative() {
return 0, sdk.Int{}, sdk.Int{}, sdk.Dec{}, 0, 0, types.NotPositiveRequireAmountError{Amount: amount0Min.String()}
return CreatePositionData{}, types.NotPositiveRequireAmountError{Amount: amount0Min.String()}
}
if amount1Min.IsNegative() {
return 0, sdk.Int{}, sdk.Int{}, sdk.Dec{}, 0, 0, types.NotPositiveRequireAmountError{Amount: amount1Min.String()}
return CreatePositionData{}, types.NotPositiveRequireAmountError{Amount: amount1Min.String()}
}

// Transform the provided ticks into their corresponding sqrtPrices.
_, _, sqrtPriceLowerTick, sqrtPriceUpperTick, err := math.TicksToSqrtPrice(lowerTick, upperTick)
if err != nil {
return 0, sdk.Int{}, sdk.Int{}, sdk.Dec{}, 0, 0, err
return CreatePositionData{}, err
}

// If multiple ticks can represent the same spot price, ensure we are using the largest of those ticks.
lowerTick, upperTick, err = roundTickToCanonicalPriceTick(lowerTick, upperTick, sqrtPriceLowerTick, sqrtPriceUpperTick, pool.GetTickSpacing())
if err != nil {
return 0, sdk.Int{}, sdk.Int{}, sdk.Dec{}, 0, 0, err
return CreatePositionData{}, err
}

positionId = k.getNextPositionIdAndIncrement(ctx)
positionId := k.getNextPositionIdAndIncrement(ctx)

// If this is the first position created in this pool, ensure that the position includes both asset0 and asset1
// in order to assign an initial spot price.
hasPositions, err := k.HasAnyPositionForPool(ctx, poolId)
if err != nil {
return 0, sdk.Int{}, sdk.Int{}, sdk.Dec{}, 0, 0, err
return CreatePositionData{}, err
}

if !hasPositions {
err := k.initializeInitialPositionForPool(ctx, pool, amount0Desired, amount1Desired)
if err != nil {
return 0, sdk.Int{}, sdk.Int{}, sdk.Dec{}, 0, 0, err
return CreatePositionData{}, err
}
}

// Calculate the amount of liquidity that will be added to the pool when this position is created.
liquidityDelta = math.GetLiquidityFromAmounts(pool.GetCurrentSqrtPrice(), sqrtPriceLowerTick, sqrtPriceUpperTick, amount0Desired, amount1Desired)
liquidityDelta := math.GetLiquidityFromAmounts(pool.GetCurrentSqrtPrice(), sqrtPriceLowerTick, sqrtPriceUpperTick, amount0Desired, amount1Desired)
if liquidityDelta.IsZero() {
return 0, sdk.Int{}, sdk.Int{}, sdk.Dec{}, 0, 0, errors.New("liquidityDelta calculated equals zero")
return CreatePositionData{}, errors.New("liquidityDelta calculated equals zero")
}

// 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)
actualAmount0, actualAmount1, _, _, err := k.UpdatePosition(ctx, poolId, owner, lowerTick, upperTick, liquidityDelta, joinTime, positionId)
if err != nil {
return 0, sdk.Int{}, sdk.Int{}, sdk.Dec{}, 0, 0, err
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 0, sdk.Int{}, sdk.Int{}, sdk.Dec{}, 0, 0, types.InsufficientLiquidityCreatedError{Actual: actualAmount0, Minimum: amount0Min, IsTokenZero: true}
return CreatePositionData{}, types.InsufficientLiquidityCreatedError{Actual: actualAmount0, Minimum: amount0Min, IsTokenZero: true}
}
if actualAmount1.LT(amount1Min) {
return 0, sdk.Int{}, sdk.Int{}, sdk.Dec{}, 0, 0, types.InsufficientLiquidityCreatedError{Actual: actualAmount1, Minimum: amount1Min}
return CreatePositionData{}, types.InsufficientLiquidityCreatedError{Actual: actualAmount1, 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())
if err != nil {
return 0, sdk.Int{}, sdk.Int{}, sdk.Dec{}, 0, 0, err
return CreatePositionData{}, err
}

event := &liquidityChangeEvent{
Expand Down Expand Up @@ -151,7 +161,14 @@ func (k Keeper) CreatePosition(ctx sdk.Context, poolId uint64, owner sdk.AccAddr
}
k.RecordTotalLiquidityIncrease(ctx, tokensAdded)

return positionId, actualAmount0, actualAmount1, liquidityDelta, lowerTick, upperTick, nil
return CreatePositionData{
ID: positionId,
Amount0: actualAmount0,
Amount1: actualAmount1,
Liquidity: liquidityDelta,
LowerTick: lowerTick,
UpperTick: upperTick,
}, nil
}

// WithdrawPosition attempts to withdraw liquidityAmount from a position with the given pool id in the given tick range.
Expand Down Expand Up @@ -375,7 +392,7 @@ func (k Keeper) addToPosition(ctx sdk.Context, owner sdk.AccAddress, positionId
if !amount1MinGiven.IsZero() {
minimumAmount1 = amount1Withdrawn.Add(amount1MinGiven)
}
newPositionId, actualAmount0, actualAmount1, _, _, _, err := k.CreatePosition(ctx, position.PoolId, owner, tokensProvided, minimumAmount0, minimumAmount1, position.LowerTick, position.UpperTick)
newPositionData, err := k.CreatePosition(ctx, position.PoolId, owner, tokensProvided, minimumAmount0, minimumAmount1, position.LowerTick, position.UpperTick)
if err != nil {
return 0, sdk.Int{}, sdk.Int{}, err
}
Expand All @@ -387,13 +404,13 @@ func (k Keeper) addToPosition(ctx sdk.Context, owner sdk.AccAddress, positionId
sdk.NewAttribute(sdk.AttributeKeyModule, types.AttributeValueCategory),
sdk.NewAttribute(sdk.AttributeKeySender, owner.String()),
sdk.NewAttribute(types.AttributeKeyPositionId, strconv.FormatUint(positionId, 10)),
sdk.NewAttribute(types.AttributeKeyNewPositionId, strconv.FormatUint(newPositionId, 10)),
sdk.NewAttribute(types.AttributeAmount0, actualAmount0.String()),
sdk.NewAttribute(types.AttributeAmount1, actualAmount1.String()),
sdk.NewAttribute(types.AttributeKeyNewPositionId, strconv.FormatUint(newPositionData.ID, 10)),
sdk.NewAttribute(types.AttributeAmount0, newPositionData.Amount0.String()),
sdk.NewAttribute(types.AttributeAmount1, newPositionData.Amount1.String()),
),
})

return newPositionId, actualAmount0, actualAmount1, nil
return newPositionData.ID, newPositionData.Amount0, newPositionData.Amount1, nil
}

// UpdatePosition updates the position in the given pool id and in the given tick range and liquidityAmount.
Expand Down
Loading