[InternalReview][MsgFungifyChargedPositions]

.vscode/launch.json

@@ -6,25 +6,14 @@
             "type": "go",
             "request": "launch",
             "mode": "test",
-            "program": "${workspaceFolder}/tests/e2e",
+            "program": "${workspaceFolder}/x/concentrated-liquidity",
             "args": [
                 "-test.timeout",
                 "30m",
                 "-test.run",
-                "IntegrationTestSuite",
+                "TestKeeperTestSuite/TestValidateAndFungifyChargedPositions/22",
                 "-test.v"
-            ],
-            "buildFlags": "-tags e2e",
-            "env": {
-                "OSMOSIS_E2E": "True",
-                "OSMOSIS_E2E_SKIP_IBC": "true",
-                "OSMOSIS_E2E_SKIP_UPGRADE": "true",
-                "OSMOSIS_E2E_SKIP_CLEANUP": "true",
-                "OSMOSIS_E2E_SKIP_STATE_SYNC": "true",
-                "OSMOSIS_E2E_UPGRADE_VERSION": "v16",
-                "OSMOSIS_E2E_DEBUG_LOG": "false",
-            },
-            "preLaunchTask": "e2e-setup"
+            ]
         }
     ]
 }

osmoutils/accum/accum.go

@@ -347,8 +347,9 @@ func (accum AccumulatorObject) deletePosition(positionName string) {
 	accum.store.Delete(FormatPositionPrefixKey(accum.name, positionName))
 }
 
-// GetPositionSize returns the number of shares the position corresponding to `addr`
-// in accumulator `accum` has, or an error if no position exists.
+// GetPositionSize returns the number of shares the position corresponding to
+// the given name has in the accumulator. Returns error if position does not exist
+// or if fails to retrieve position from state.
 func (accum AccumulatorObject) GetPositionSize(name string) (sdk.Dec, error) {
 	position, err := GetPosition(accum, name)
 	if err != nil {
@@ -422,7 +423,7 @@ func (accum AccumulatorObject) GetTotalShares() (sdk.Dec, error) {
 }
 
 // AddToUnclaimedRewards adds the given amount of rewards to the unclaimed rewards
-// for the given position. Returns error if no position exists for the given address.
+// for the given position. Returns error if no position exists for the given position name.
 // Returns error if any database errors occur.
 func (accum AccumulatorObject) AddToUnclaimedRewards(positionName string, rewards sdk.DecCoins) error {
 	position, err := GetPosition(accum, positionName)

proto/osmosis/poolmanager/v1beta1/query.proto

@@ -152,9 +152,7 @@ message PoolResponse {
 }
 
 //=============================== AllPools
-message AllPoolsRequest {
-  uint64 pool_id = 1 [ (gogoproto.moretags) = "yaml:\"pool_id\"" ];
-}
+message AllPoolsRequest {}
 message AllPoolsResponse {
   repeated google.protobuf.Any pools = 1
       [ (cosmos_proto.accepts_interface) = "PoolI" ];

x/concentrated-liquidity/client/cli/tx.go

@@ -30,6 +30,7 @@ func NewTxCmd() *cobra.Command {
 	osmocli.AddTxCmd(txCmd, NewCollectFeesCmd)
 	osmocli.AddTxCmd(txCmd, NewCollectIncentivesCmd)
 	osmocli.AddTxCmd(txCmd, NewCreateIncentiveCmd)
+	osmocli.AddTxCmd(txCmd, NewFungifyChargedPositionsCmd)
 	return txCmd
 }
 
@@ -39,10 +40,10 @@ var poolIdFlagOverride = map[string]string{
 
 func NewCreateConcentratedPoolCmd() (*osmocli.TxCliDesc, *clmodel.MsgCreateConcentratedPool) {
 	return &osmocli.TxCliDesc{
-		Use:     "create-concentrated-pool [denom-0] [denom-1] [tick-spacing] [swap-fee]",
+		Use:     "create-pool [denom-0] [denom-1] [tick-spacing] [swap-fee]",
 		Short:   "create a concentrated liquidity pool with the given denom pair, tick spacing, and swap fee",
 		Long:    "denom-1 (the quote denom), tick spacing, and swap fees must all be authorized by the concentrated liquidity module",
-		Example: "create-concentrated-pool uion uosmo 1 0.01 --from val --chain-id osmosis-1",
+		Example: "create-pool uion uosmo 1 0.01 --from val --chain-id osmosis-1",
 	}, &clmodel.MsgCreateConcentratedPool{}
 }
 
@@ -92,12 +93,20 @@ func NewCreateIncentiveCmd() (*osmocli.TxCliDesc, *types.MsgCreateIncentive) {
 	return &osmocli.TxCliDesc{
 		Use:                 "create-incentive [incentive-denom] [incentive-amount] [emission-rate] [start-time] [min-uptime]",
 		Short:               "create an incentive record to emit incentives (per second) to a given pool",
-		Example:             "create-incentive uosmo 69082 0.02 100 2023-03-03 03:20:35.419543805 24h --pool-id 1 --from val --chain-id osmosis-1",
+		Example:             "create-incentive uosmo 69082 0.02 \"2023-03-03T03:20:35.419543805\" 24h --pool-id 1 --from val --chain-id osmosis-1 --fees 875uosmo",
 		CustomFlagOverrides: poolIdFlagOverride,
 		Flags:               osmocli.FlagDesc{RequiredFlags: []*flag.FlagSet{FlagSetJustPoolId()}},
 	}, &types.MsgCreateIncentive{}
 }
 
+func NewFungifyChargedPositionsCmd() (*osmocli.TxCliDesc, *types.MsgFungifyChargedPositions) {
+	return &osmocli.TxCliDesc{
+		Use:     "fungify-positions [position-ids]",
+		Short:   "Combine fully charged positions within the same range into a new single fully charged position",
+		Example: "fungify-positions 1,5,7 --from val --chain-id osmosis-1",
+	}, &types.MsgFungifyChargedPositions{}
+}
+
 // NewCmdCreateConcentratedLiquidityPoolProposal implements a command handler for create concentrated liquidity pool proposal
 func NewCmdCreateConcentratedLiquidityPoolProposal() *cobra.Command {
 	cmd := &cobra.Command{

x/concentrated-liquidity/export_test.go

@@ -16,11 +16,14 @@ const (
 )
 
 var (
-	EmptyCoins           = emptyCoins
-	HundredFooCoins      = sdk.NewDecCoin("foo", sdk.NewInt(100))
-	HundredBarCoins      = sdk.NewDecCoin("bar", sdk.NewInt(100))
-	TwoHundredFooCoins   = sdk.NewDecCoin("foo", sdk.NewInt(200))
-	TwoHundredBarCoins   = sdk.NewDecCoin("bar", sdk.NewInt(200))
+	EmptyCoins         = emptyCoins
+	HundredFooCoins    = sdk.NewDecCoin("foo", sdk.NewInt(100))
+	HundredBarCoins    = sdk.NewDecCoin("bar", sdk.NewInt(100))
+	TwoHundredFooCoins = sdk.NewDecCoin("foo", sdk.NewInt(200))
+	TwoHundredBarCoins = sdk.NewDecCoin("bar", sdk.NewInt(200))
+	// TODO: this is incorrect. Should be grabbing from
+	// authorized params instead. Must verify that all tests still make sense.
+	// https://github.com/osmosis-labs/osmosis/issues/5039
 	FullyChargedDuration = types.SupportedUptimes[len(types.SupportedUptimes)-1]
 )
 
@@ -133,8 +136,8 @@ func (k Keeper) CreateFeeAccumulator(ctx sdk.Context, poolId uint64) error {
 	return k.createFeeAccumulator(ctx, poolId)
 }
 
-func (k Keeper) InitOrUpdateFeeAccumulatorPosition(ctx sdk.Context, poolId uint64, lowerTick, upperTick int64, positionId uint64, liquidity sdk.Dec) error {
-	return k.initOrUpdateFeeAccumulatorPosition(ctx, poolId, lowerTick, upperTick, positionId, liquidity)
+func (k Keeper) InitOrUpdatePositionFeeAccumulator(ctx sdk.Context, poolId uint64, lowerTick, upperTick int64, positionId uint64, liquidity sdk.Dec) error {
+	return k.initOrUpdatePositionFeeAccumulator(ctx, poolId, lowerTick, upperTick, positionId, liquidity)
 }
 
 func (k Keeper) GetFeeGrowthOutside(ctx sdk.Context, poolId uint64, lowerTick, upperTick int64) (sdk.DecCoins, error) {
@@ -204,7 +207,7 @@ func CalcAccruedIncentivesForAccum(ctx sdk.Context, accumUptime time.Duration, q
 }
 
 func (k Keeper) UpdateUptimeAccumulatorsToNow(ctx sdk.Context, poolId uint64) error {
-	return k.updateUptimeAccumulatorsToNow(ctx, poolId)
+	return k.updatePoolUptimeAccumulatorsToNow(ctx, poolId)
 }
 
 func (k Keeper) SetIncentiveRecord(ctx sdk.Context, incentiveRecord types.IncentiveRecord) {
@@ -219,8 +222,8 @@ func (k Keeper) GetInitialUptimeGrowthOutsidesForTick(ctx sdk.Context, poolId ui
 	return k.getInitialUptimeGrowthOutsidesForTick(ctx, poolId, tick)
 }
 
-func (k Keeper) InitOrUpdatePositionUptime(ctx sdk.Context, poolId uint64, position sdk.Dec, owner sdk.AccAddress, lowerTick, upperTick int64, liquidityDelta sdk.Dec, joinTime time.Time, positionId uint64) error {
-	return k.initOrUpdatePositionUptime(ctx, poolId, position, owner, lowerTick, upperTick, liquidityDelta, joinTime, positionId)
+func (k Keeper) InitOrUpdatePositionUptimeAccumulators(ctx sdk.Context, poolId uint64, position sdk.Dec, owner sdk.AccAddress, lowerTick, upperTick int64, liquidityDelta sdk.Dec, positionId uint64) error {
+	return k.initOrUpdatePositionUptimeAccumulators(ctx, poolId, position, owner, lowerTick, upperTick, liquidityDelta, positionId)
 }
 
 func (k Keeper) CollectIncentives(ctx sdk.Context, owner sdk.AccAddress, positionId uint64) (sdk.Coins, sdk.Coins, error) {

x/concentrated-liquidity/fees.go

@@ -46,7 +46,7 @@ func (k Keeper) chargeFee(ctx sdk.Context, poolId uint64, feeUpdate sdk.DecCoin)
 	return nil
 }
 
-// initOrUpdateFeeAccumulatorPosition mutates the fee accumulator position by either creating or updating it
+// initOrUpdatePositionFeeAccumulator mutates the fee accumulator position by either creating or updating it
 // for the given pool id in the range specified by the given lower and upper ticks, position id and liquidityDelta.
 // If liquidityDelta is positive, it adds liquidity. If liquidityDelta is negative, it removes liquidity.
 // If this is a new position, liqudityDelta must be positive.
@@ -62,7 +62,7 @@ func (k Keeper) chargeFee(ctx sdk.Context, poolId uint64, feeUpdate sdk.DecCoin)
 // - fails to create a new position.
 // - fails to prepare the accumulator for update.
 // - fails to update the position's accumulator.
-func (k Keeper) initOrUpdateFeeAccumulatorPosition(ctx sdk.Context, poolId uint64, lowerTick, upperTick int64, positionId uint64, liquidityDelta sdk.Dec) error {
+func (k Keeper) initOrUpdatePositionFeeAccumulator(ctx sdk.Context, poolId uint64, lowerTick, upperTick int64, positionId uint64, liquidityDelta sdk.Dec) error {
 	// Get the fee accumulator for the position's pool.
 	feeAccumulator, err := k.GetFeeAccumulator(ctx, poolId)
 	if err != nil {
@@ -300,6 +300,7 @@ func calculateFeeGrowth(targetTick int64, feeGrowthOutside sdk.DecCoins, current
 // as we must set the position's accumulator value to the sum of
 // - the fee/uptime growth inside at position creation time (position.InitAccumValue)
 // - fee/uptime growth outside at the current block time (feeGrowthOutside/uptimeGrowthOutside)
+// CONTRACT: position accumulator value prior to this call is equal to the growth inside the position at the time of last update.
 func preparePositionAccumulator(accumulator accum.AccumulatorObject, positionKey string, growthOutside sdk.DecCoins) error {
 	position, err := accum.GetPosition(accumulator, positionKey)
 	if err != nil {

x/concentrated-liquidity/fees_test.go

@@ -84,7 +84,7 @@ func (s *KeeperTestSuite) TestCreateAndGetFeeAccumulator() {
 	}
 }
 
-func (s *KeeperTestSuite) TestInitOrUpdateFeeAccumulatorPosition() {
+func (s *KeeperTestSuite) TestInitOrUpdatePositionFeeAccumulator() {
 	// Setup is done once so that we test
 	// the relationship between test cases.
 	// For example, that positions with non-zero liquidity
@@ -138,42 +138,37 @@ func (s *KeeperTestSuite) TestInitOrUpdateFeeAccumulatorPosition() {
 		positionFields positionFields
 
 		expectedLiquidity sdk.Dec
-		expectedPass      bool
+		expectedError     error
 	}
 	tests := []initFeeAccumTest{
 		{
 			name:           "error: negative liquidity for the first position",
 			positionFields: withLiquidity(defaultPositionFields, DefaultLiquidityAmt.Neg()),
-			expectedPass:   false,
+			expectedError:  types.NonPositiveLiquidityForNewPositionError{LiquidityDelta: DefaultLiquidityAmt.Neg(), PositionId: DefaultPositionId},
 		},
 		{
 			name:              "first position",
 			positionFields:    defaultPositionFields,
 			expectedLiquidity: defaultPositionFields.liquidity,
-			expectedPass:      true,
 		},
 		{
 			name:              "second position",
 			positionFields:    withPositionId(withLowerTick(defaultPositionFields, DefaultLowerTick+1), DefaultPositionId+1),
 			expectedLiquidity: defaultPositionFields.liquidity,
-			expectedPass:      true,
 		},
 		{
 			name:              "adding to first position",
 			positionFields:    defaultPositionFields,
-			expectedPass:      true,
 			expectedLiquidity: defaultPositionFields.liquidity.MulInt64(2),
 		},
 		{
 			name:              "removing from first position",
 			positionFields:    withLiquidity(defaultPositionFields, defaultPositionFields.liquidity.Neg()),
-			expectedPass:      true,
 			expectedLiquidity: defaultPositionFields.liquidity,
 		},
 		{
 			name:              "adding to second position",
 			positionFields:    withPositionId(withLowerTick(defaultPositionFields, DefaultLowerTick+1), DefaultPositionId+1),
-			expectedPass:      true,
 			expectedLiquidity: defaultPositionFields.liquidity.MulInt64(2),
 		},
 		{
@@ -186,25 +181,22 @@ func (s *KeeperTestSuite) TestInitOrUpdateFeeAccumulatorPosition() {
 				DefaultPositionId,
 				DefaultLiquidityAmt,
 			},
-			expectedPass: false,
+			expectedError: accum.AccumDoesNotExistError{AccumName: types.KeyFeePoolAccumulator(2)},
 		},
 		{
 			name:              "existing accumulator, different owner - different position",
 			positionFields:    withPositionId(withOwner(defaultPositionFields, s.TestAccs[1]), DefaultPositionId+2),
 			expectedLiquidity: defaultPositionFields.liquidity,
-			expectedPass:      true,
 		},
 		{
 			name:              "existing accumulator, different upper tick - different position",
 			positionFields:    withPositionId(withUpperTick(defaultPositionFields, DefaultUpperTick+1), DefaultPositionId+3),
 			expectedLiquidity: defaultPositionFields.liquidity,
-			expectedPass:      true,
 		},
 		{
 			name:              "existing accumulator, different lower tick - different position",
 			positionFields:    withPositionId(withLowerTick(defaultPositionFields, DefaultUpperTick+1), DefaultPositionId+4),
 			expectedLiquidity: defaultPositionFields.liquidity,
-			expectedPass:      true,
 		},
 	}
 
@@ -213,8 +205,8 @@ func (s *KeeperTestSuite) TestInitOrUpdateFeeAccumulatorPosition() {
 		s.Run(tc.name, func() {
 
 			// System under test
-			err := clKeeper.InitOrUpdateFeeAccumulatorPosition(s.Ctx, tc.positionFields.poolId, tc.positionFields.lowerTick, tc.positionFields.upperTick, tc.positionFields.positionId, tc.positionFields.liquidity)
-			if tc.expectedPass {
+			err := clKeeper.InitOrUpdatePositionFeeAccumulator(s.Ctx, tc.positionFields.poolId, tc.positionFields.lowerTick, tc.positionFields.upperTick, tc.positionFields.positionId, tc.positionFields.liquidity)
+			if tc.expectedError == nil {
 				s.Require().NoError(err)
 
 				// get fee accum and see if position size has been properly initialized
@@ -245,6 +237,7 @@ func (s *KeeperTestSuite) TestInitOrUpdateFeeAccumulatorPosition() {
 				s.Require().Equal(cl.EmptyCoins, positionRecord.UnclaimedRewards)
 			} else {
 				s.Require().Error(err)
+				s.Require().ErrorContains(err, tc.expectedError.Error())
 			}
 		})
 	}
@@ -1302,7 +1295,7 @@ func (s *KeeperTestSuite) TestInitOrUpdateFeeAccumulatorPosition_UpdatingPositio
 			s.Require().NoError(err)
 
 			// InitOrUpdateFeeAccumulatorPosition #1 lower tick to upper tick
-			err = s.App.ConcentratedLiquidityKeeper.InitOrUpdateFeeAccumulatorPosition(s.Ctx, poolId, DefaultLowerTick, DefaultUpperTick, DefaultPositionId, DefaultLiquidityAmt)
+			err = s.App.ConcentratedLiquidityKeeper.InitOrUpdatePositionFeeAccumulator(s.Ctx, poolId, DefaultLowerTick, DefaultUpperTick, DefaultPositionId, DefaultLiquidityAmt)
 			s.Require().NoError(err)
 
 			// Imaginary fee charge #2.
@@ -1311,7 +1304,7 @@ func (s *KeeperTestSuite) TestInitOrUpdateFeeAccumulatorPosition_UpdatingPositio
 			}
 
 			// InitOrUpdateFeeAccumulatorPosition # 2 lower tick to upper tick with a different position id.
-			err = s.App.ConcentratedLiquidityKeeper.InitOrUpdateFeeAccumulatorPosition(s.Ctx, poolId, DefaultLowerTick, DefaultUpperTick, DefaultPositionId+1, DefaultLiquidityAmt)
+			err = s.App.ConcentratedLiquidityKeeper.InitOrUpdatePositionFeeAccumulator(s.Ctx, poolId, DefaultLowerTick, DefaultUpperTick, DefaultPositionId+1, DefaultLiquidityAmt)
 			s.Require().NoError(err)
 
 			// Imaginary fee charge #3.
@@ -1320,7 +1313,7 @@ func (s *KeeperTestSuite) TestInitOrUpdateFeeAccumulatorPosition_UpdatingPositio
 			}
 
 			// InitOrUpdateFeeAccumulatorPosition # 3 lower tick to upper tick with the original position id.
-			err = s.App.ConcentratedLiquidityKeeper.InitOrUpdateFeeAccumulatorPosition(s.Ctx, poolId, DefaultLowerTick, DefaultUpperTick, DefaultPositionId, DefaultLiquidityAmt)
+			err = s.App.ConcentratedLiquidityKeeper.InitOrUpdatePositionFeeAccumulator(s.Ctx, poolId, DefaultLowerTick, DefaultUpperTick, DefaultPositionId, DefaultLiquidityAmt)
 			s.Require().NoError(err)
 
 			// Imaginary fee charge #4.