Skip to content

Commit

Permalink
refactor(CL): replace 6 return values in CreatePosition with a struct…
Browse files Browse the repository at this point in the history
… (backport #5983) (#5988)

Co-authored-by: Roman <[email protected]>
  • Loading branch information
mergify[bot] and p0mvn authored Aug 9, 2023
1 parent 01fa841 commit b38ffb5
Show file tree
Hide file tree
Showing 18 changed files with 151 additions and 107 deletions.
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## Unreleased

### State Breaking
### API breaks

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

### Bug Fixes

Expand Down
1 change: 0 additions & 1 deletion go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -950,7 +950,6 @@ github.com/osmosis-labs/go-mutesting v0.0.0-20221208041716-b43bcd97b3b3 h1:Ylmch
github.com/osmosis-labs/go-mutesting v0.0.0-20221208041716-b43bcd97b3b3/go.mod h1:lV6KnqXYD/ayTe7310MHtM3I2q8Z6bBfMAi+bhwPYtI=
github.com/osmosis-labs/osmosis/osmomath v0.0.5 h1:veOu4VvnqMbJyjx2MEEFW53I9h3m8Kd3WN4hVvUyeIU=
github.com/osmosis-labs/osmosis/osmomath v0.0.5/go.mod h1:UlftwozB+QObT3o0YfkuuyL9fsVdgoWt0dm6J7MLYnU=
github.com/osmosis-labs/osmosis/osmoutils v0.0.6-0.20230709040235-cbf530ed88cc h1:NhjYKK0ADJCq5nmDfLMGRIAq8/QIhrStlt78+5xOZP4=
github.com/osmosis-labs/osmosis/osmoutils v0.0.6-0.20230718110737-18371921cd51 h1:z7X7+p3CIxXoxo6264Pw3NBqAaeqEZyZ4euto/Si2x4=
github.com/osmosis-labs/osmosis/osmoutils v0.0.6-0.20230718110737-18371921cd51/go.mod h1:Pl8Nzx6O6ow/+aqfMoMSz4hX+zz6RrnDYsooptECGxM=
github.com/osmosis-labs/osmosis/x/epochs v0.0.1 h1:RCL3+W2s712dgtaKGgUHVt/6iJKfuPH63tkd3eiJeRo=
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
4 changes: 0 additions & 4 deletions x/concentrated-liquidity/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,10 +207,6 @@ func UpdatePositionToInitValuePlusGrowthOutside(accumulator *accum.AccumulatorOb
return updatePositionToInitValuePlusGrowthOutside(accumulator, positionKey, growthOutside)
}

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) {
return k.createPosition(ctx, poolId, owner, tokensProvided, amount0Min, amount1Min, lowerTick, upperTick)
}

func (k Keeper) AddToPosition(ctx sdk.Context, owner sdk.AccAddress, positionId uint64, amount0Added, amount1Added, amount0Min, amount1Min sdk.Int) (uint64, sdk.Int, sdk.Int, error) {
return k.addToPosition(ctx, owner, positionId, amount0Added, amount1Added, amount0Min, amount1Min)
}
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 @@ -3132,7 +3133,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 @@ -3165,7 +3166,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 @@ -3183,7 +3184,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 @@ -3268,35 +3269,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
Loading

0 comments on commit b38ffb5

Please sign in to comment.