[CL][InternalReview] MsgAddToConcentratedLiquiditySuperfluidPosition (#…

…5157)

* [InternalReview] MsgAddToConcentratedLiquiditySuperfluidPosition

* updates

* updates

* updates

* updates

* revert me

* updates

* debug

* docs

* go sum

* go mod

* remove

* Update x/superfluid/README.md

* Update x/concentrated-liquidity/position.go

* comment

* coins

* remove Q

* remove Q

* revert name

app/apptesting/concentrated_liquidity.go

@@ -74,7 +74,7 @@ func (s *KeeperTestHelper) PrepareConcentratedPoolWithCoinsAndLockedFullRangePos
 	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)
 	s.Require().NoError(err)
-	clPool, err = s.App.ConcentratedLiquidityKeeper.GetPoolFromPoolIdAndConvertToConcentrated(s.Ctx, clPool.GetId())
+	clPool, err = s.App.ConcentratedLiquidityKeeper.GetConcentratedPoolById(s.Ctx, clPool.GetId())
 	s.Require().NoError(err)
 	return clPool, concentratedLockId, positionId
 }

go.mod

@@ -20,7 +20,7 @@ require (
 	github.com/ory/dockertest/v3 v3.10.0
 	github.com/osmosis-labs/go-mutesting v0.0.0-20221208041716-b43bcd97b3b3
 	github.com/osmosis-labs/osmosis/osmomath v0.0.3-dev.0.20230503232557-ba905586c111
-	github.com/osmosis-labs/osmosis/osmoutils v0.0.0-20230510161551-8bf252f26bae
+	github.com/osmosis-labs/osmosis/osmoutils v0.0.0-20230511015306-615fa4fcbe56
 	github.com/osmosis-labs/osmosis/x/epochs v0.0.0-20230328024000-175ec88e4304
 	github.com/osmosis-labs/osmosis/x/ibc-hooks v0.0.0-20230331072320-5d6f6cfa2627
 	github.com/pkg/errors v0.9.1

go.sum

@@ -938,8 +938,8 @@ 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.3-dev.0.20230503232557-ba905586c111 h1:1ahWbf9iF9sxDOjxrHWFaBGLE0nWFdpiX1pqObUaJO8=
 github.com/osmosis-labs/osmosis/osmomath v0.0.3-dev.0.20230503232557-ba905586c111/go.mod h1:a7lhiXRpn8QJ21OhFpaEnUNErTSIafaYpp02q6uI/Dk=
-github.com/osmosis-labs/osmosis/osmoutils v0.0.0-20230510161551-8bf252f26bae h1:I1Cy+GpTPWbVi0lBw9+bS1w42YfQjvXNK9bW4YbHhcs=
-github.com/osmosis-labs/osmosis/osmoutils v0.0.0-20230510161551-8bf252f26bae/go.mod h1:hk/o9/kmTSZmZqwXcSrPuwj/gpRMCqbE/d3vj6teL2A=
+github.com/osmosis-labs/osmosis/osmoutils v0.0.0-20230511015306-615fa4fcbe56 h1:27yWLC0uXSatRy8aRn0yinHU+K31bkjBRmNnQUDO0Ks=
+github.com/osmosis-labs/osmosis/osmoutils v0.0.0-20230511015306-615fa4fcbe56/go.mod h1:hk/o9/kmTSZmZqwXcSrPuwj/gpRMCqbE/d3vj6teL2A=
 github.com/osmosis-labs/osmosis/x/epochs v0.0.0-20230328024000-175ec88e4304 h1:RIrWLzIiZN5Xd2JOfSOtGZaf6V3qEQYg6EaDTAkMnCo=
 github.com/osmosis-labs/osmosis/x/epochs v0.0.0-20230328024000-175ec88e4304/go.mod h1:yPWoJTj5RKrXKUChAicp+G/4Ni/uVEpp27mi/FF/L9c=
 github.com/osmosis-labs/osmosis/x/ibc-hooks v0.0.0-20230331072320-5d6f6cfa2627 h1:A0SwZgp4bmJFbivYJc8mmVhMjrr3EdUZluBYFke11+w=

osmoutils/errors.go

@@ -0,0 +1,11 @@
+package osmoutils
+
+import "fmt"
+
+type DecNotFoundError struct {
+	Key string
+}
+
+func (e DecNotFoundError) Error() string {
+	return fmt.Sprintf("no sdk.Dec at key (%s)", e.Key)
+}

osmoutils/store_helper.go

@@ -85,7 +85,7 @@ func GetIterValuesWithStop[T any](
 
 // HasAnyAtPrefix returns true if there is at least one value in the given prefix.
 func HasAnyAtPrefix[T any](storeObj store.KVStore, prefix []byte, parseValue func([]byte) (T, error)) (bool, error) {
-	_, err := GetFirstValueInRange(storeObj, prefix, sdk.PrefixEndBytes(prefix),false,  parseValue)
+	_, err := GetFirstValueInRange(storeObj, prefix, sdk.PrefixEndBytes(prefix), false, parseValue)
 	if err != nil {
 		if err == ErrNoValuesInRange {
 			return false, nil
@@ -180,6 +180,21 @@ func MustGetDec(store store.KVStore, key []byte) sdk.Dec {
 	return result.Dec
 }
 
+// GetDec gets dec value from store at key. Returns error if:
+// - database error occurs.
+// - no value at given key is found.
+func GetDec(store store.KVStore, key []byte) (sdk.Dec, error) {
+	result := &sdk.DecProto{}
+	isFound, err := Get(store, key, result)
+	if err != nil {
+		return sdk.Dec{}, err
+	}
+	if !isFound {
+		return sdk.Dec{}, DecNotFoundError{Key: string(key)}
+	}
+	return result.Dec, nil
+}
+
 // Get returns a value at key by mutating the result parameter. Returns true if the value was found and the
 // result mutated correctly. If the value is not in the store, returns false. Returns error only when database or serialization errors occur. (And when an error occurs, returns false)
 func Get(store store.KVStore, key []byte, result proto.Message) (found bool, err error) {

osmoutils/store_helper_test.go

@@ -1243,3 +1243,69 @@ func (s *TestSuite) TestHasAnyAtPrefix() {
 		})
 	}
 }
+
+func (s *TestSuite) TestGetDec() {
+	tests := map[string]struct {
+		// keys and values to preset
+		preSetKeyValues map[string]sdk.Dec
+
+		// keys and values to attempt to get and validate
+		expectedGetKeyValues map[string]sdk.Dec
+
+		expectError error
+	}{
+		"valid get": {
+			preSetKeyValues: map[string]sdk.Dec{
+				keyA: sdk.OneDec(),
+				keyB: sdk.OneDec().Add(sdk.OneDec()),
+				keyC: sdk.OneDec().Add(sdk.OneDec()).Add(sdk.OneDec()),
+			},
+
+			expectedGetKeyValues: map[string]sdk.Dec{
+				keyA: sdk.OneDec(),
+				keyB: sdk.OneDec().Add(sdk.OneDec()),
+				keyC: sdk.OneDec().Add(sdk.OneDec()).Add(sdk.OneDec()),
+			},
+		},
+		"error: attempt to get non-existent key": {
+			preSetKeyValues: map[string]sdk.Dec{
+				keyA: sdk.OneDec(),
+				keyC: sdk.OneDec().Add(sdk.OneDec()).Add(sdk.OneDec()),
+			},
+
+			expectedGetKeyValues: map[string]sdk.Dec{
+				keyA: sdk.OneDec(),
+				keyB: {}, // this one errors
+			},
+
+			expectError: osmoutils.DecNotFoundError{Key: keyB},
+		},
+	}
+
+	for name, tc := range tests {
+		s.Run(name, func() {
+			s.SetupTest()
+			// Setup
+			for key, value := range tc.preSetKeyValues {
+				osmoutils.MustSetDec(s.store, []byte(key), value)
+			}
+
+			for key, expectedValue := range tc.expectedGetKeyValues {
+				// System under test.
+				actualDec, err := osmoutils.GetDec(s.store, []byte(key))
+
+				// Assertions.
+
+				if tc.expectError != nil {
+					s.Require().Error(err)
+					s.Require().ErrorIs(err, tc.expectError)
+					return
+				}
+
+				s.Require().NoError(err)
+				s.Require().Equal(expectedValue.String(), actualDec.String())
+			}
+		})
+
+	}
+}

proto/osmosis/superfluid/tx.proto

@@ -204,5 +204,13 @@ message MsgAddToConcentratedLiquiditySuperfluidPositionResponse {
     (gogoproto.moretags) = "yaml:\"amount1\"",
     (gogoproto.nullable) = false
   ];
+  // new_liquidity is the final liquidity after the add.
+  // It includes the liquidity that existed before in the position
+  // and the new liquidity that was added to the position.
+  string new_liquidity = 5 [
+    (gogoproto.customtype) = "github.com/cosmos/cosmos-sdk/types.Dec",
+    (gogoproto.moretags) = "yaml:\"new_liquidity\"",
+    (gogoproto.nullable) = false
+  ];
   uint64 lock_id = 4 [ (gogoproto.moretags) = "yaml:\"lock_id\"" ];
 }

tests/cl-genesis-positions/convert.go

@@ -115,7 +115,7 @@ func ConvertSubgraphToOsmosisGenesis(positionCreatorAddresses []sdk.AccAddress,
 
 	fmt.Println("Created pool id of: ", poolId)
 
-	pool, err := osmosis.App.ConcentratedLiquidityKeeper.GetPoolFromPoolIdAndConvertToConcentrated(osmosis.Ctx, poolId)
+	pool, err := osmosis.App.ConcentratedLiquidityKeeper.GetConcentratedPoolById(osmosis.Ctx, poolId)
 	if err != nil {
 		panic(err)
 	}

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

@@ -49,9 +49,9 @@ func NewCreateConcentratedPoolCmd() (*osmocli.TxCliDesc, *clmodel.MsgCreateConce
 
 func NewCreatePositionCmd() (*osmocli.TxCliDesc, *types.MsgCreatePosition) {
 	return &osmocli.TxCliDesc{
-		Use:     "create-position [pool-id] [lower-tick] [upper-tick] [token-0-amount] [token-1-amount] [token-0-min-amount] [token-1-min-amount]",
+		Use:     "create-position [pool-id] [lower-tick] [upper-tick] [coins] [token-0-min-amount] [token-1-min-amount]",
 		Short:   "create or add to existing concentrated liquidity position",
-		Example: "create-position 1 \"[-69082]\" 69082 1000000000 10000000 0 0 --from val --chain-id osmosis-1",
+		Example: "create-position 1 \"[-69082]\" 69082 1000000000uosmo,10000000uion 0 0 --from val --chain-id osmosis-1",
 	}, &types.MsgCreatePosition{}
 }
 

x/concentrated-liquidity/client/query_proto_wrap.go

@@ -37,7 +37,7 @@ func (q Querier) UserPositions(ctx sdk.Context, req clquery.UserPositionsRequest
 
 	for _, position := range userPositions {
 		// get the pool from the position
-		pool, err := q.Keeper.GetPoolFromPoolIdAndConvertToConcentrated(ctx, position.PoolId)
+		pool, err := q.Keeper.GetConcentratedPoolById(ctx, position.PoolId)
 		if err != nil {
 			return nil, status.Error(codes.Internal, err.Error())
 		}
@@ -67,7 +67,7 @@ func (q Querier) PositionById(ctx sdk.Context, req clquery.PositionByIdRequest)
 		return nil, status.Error(codes.Internal, err.Error())
 	}
 
-	positionPool, err := q.Keeper.GetPoolFromPoolIdAndConvertToConcentrated(ctx, position.PoolId)
+	positionPool, err := q.Keeper.GetConcentratedPoolById(ctx, position.PoolId)
 	if err != nil {
 		return nil, status.Error(codes.Internal, err.Error())
 	}
@@ -148,7 +148,7 @@ func (q Querier) LiquidityNetInDirection(ctx sdk.Context, req clquery.LiquidityN
 		return nil, err
 	}
 
-	pool, err := q.Keeper.GetPoolFromPoolIdAndConvertToConcentrated(ctx, req.PoolId)
+	pool, err := q.Keeper.GetConcentratedPoolById(ctx, req.PoolId)
 	if err != nil {
 		return nil, err
 	}

x/concentrated-liquidity/export_test.go

@@ -150,8 +150,8 @@ func (k Keeper) UpdateFullRangeLiquidityInPool(ctx sdk.Context, poolId uint64, l
 	return k.updateFullRangeLiquidityInPool(ctx, poolId, liquidity)
 }
 
-func (k Keeper) MintSharesLockAndUpdate(ctx sdk.Context, concentratedPoolId, positionId uint64, owner sdk.AccAddress, remainingLockDuration time.Duration) (concentratedLockID uint64, underlyingLiquidityTokenized sdk.Coins, err error) {
-	return k.mintSharesLockAndUpdate(ctx, concentratedPoolId, positionId, owner, remainingLockDuration)
+func (k Keeper) MintSharesAndLock(ctx sdk.Context, concentratedPoolId, positionId uint64, owner sdk.AccAddress, remainingLockDuration time.Duration) (concentratedLockID uint64, underlyingLiquidityTokenized sdk.Coins, err error) {
+	return k.mintSharesAndLock(ctx, concentratedPoolId, positionId, owner, remainingLockDuration)
 }
 
 func (k Keeper) SetPositionIdToLock(ctx sdk.Context, positionId, underlyingLockId uint64) {

x/concentrated-liquidity/lp.go

@@ -45,6 +45,7 @@ func (k Keeper) createPosition(ctx sdk.Context, poolId uint64, owner sdk.AccAddr
 	if err := validateTickRangeIsValid(pool.GetTickSpacing(), lowerTick, upperTick); err != nil {
 		return 0, sdk.Int{}, sdk.Int{}, sdk.Dec{}, time.Time{}, 0, 0, err
 	}
+
 	amount0Desired := tokensProvided.AmountOf(pool.GetToken0())
 	amount1Desired := tokensProvided.AmountOf(pool.GetToken1())
 	if amount0Desired.IsZero() && amount1Desired.IsZero() {
@@ -299,7 +300,7 @@ func (k Keeper) addToPosition(ctx sdk.Context, owner sdk.AccAddress, positionId
 	// Create new position with updated liquidity.
 	amount0Desired := amount0Withdrawn.Add(amount0Added)
 	amount1Desired := amount1Withdrawn.Add(amount1Added)
-	pool, err := k.GetPoolFromPoolIdAndConvertToConcentrated(ctx, position.PoolId)
+	pool, err := k.GetConcentratedPoolById(ctx, position.PoolId)
 	if err != nil {
 		return 0, sdk.Int{}, sdk.Int{}, err
 	}

x/concentrated-liquidity/pool.go

@@ -225,7 +225,9 @@ func convertPoolInterfaceToConcentrated(poolI poolmanagertypes.PoolI) (types.Con
 	return concentratedPool, nil
 }
 
-func (k Keeper) GetPoolFromPoolIdAndConvertToConcentrated(ctx sdk.Context, poolId uint64) (types.ConcentratedPoolExtension, error) {
+// GetConcentratedPoolById returns a concentrated pool interface associated with the given id.
+// Returns error if fails to fetch the pool from the store.
+func (k Keeper) GetConcentratedPoolById(ctx sdk.Context, poolId uint64) (types.ConcentratedPoolExtension, error) {
 	poolI, err := k.GetPool(ctx, poolId)
 	if err != nil {
 		return nil, err
@@ -271,7 +273,7 @@ func (k Keeper) GetSerializedPools(ctx sdk.Context, pagination *query.PageReques
 // It returns an error if the tick spacing is not one of the authorized tick spacings or is not less than the current tick spacing of the respective pool.
 func (k Keeper) DecreaseConcentratedPoolTickSpacing(ctx sdk.Context, poolIdToTickSpacingRecord []types.PoolIdToTickSpacingRecord) error {
 	for _, poolIdToTickSpacingRecord := range poolIdToTickSpacingRecord {
-		pool, err := k.GetPoolFromPoolIdAndConvertToConcentrated(ctx, poolIdToTickSpacingRecord.PoolId)
+		pool, err := k.GetConcentratedPoolById(ctx, poolIdToTickSpacingRecord.PoolId)
 		if err != nil {
 			return err
 		}

x/concentrated-liquidity/position.go

@@ -286,13 +286,13 @@ 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, clPoolId uint64, owner sdk.AccAddress, coins sdk.Coins) (positionId uint64, amount0, amount1 sdk.Int, liquidity sdk.Dec, joinTime time.Time, err error) {
+func (k Keeper) CreateFullRangePosition(ctx sdk.Context, poolId uint64, owner sdk.AccAddress, coins sdk.Coins) (positionId uint64, amount0, amount1 sdk.Int, liquidity sdk.Dec, joinTime time.Time, err error) {
 	// Check that exactly two coins are provided.
 	if len(coins) != 2 {
 		return 0, sdk.Int{}, sdk.Int{}, sdk.Dec{}, time.Time{}, types.NumCoinsError{NumCoins: len(coins)}
 	}
 
-	concentratedPool, err := k.GetPoolFromPoolIdAndConvertToConcentrated(ctx, clPoolId)
+	concentratedPool, err := k.GetConcentratedPoolById(ctx, poolId)
 	if err != nil {
 		return 0, sdk.Int{}, sdk.Int{}, sdk.Dec{}, time.Time{}, err
 	}
@@ -329,7 +329,7 @@ func (k Keeper) CreateFullRangePositionLocked(ctx sdk.Context, clPoolId uint64,
 
 	// 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, _, err := k.mintSharesLockAndUpdate(ctx, clPoolId, positionId, owner, remainingLockDuration)
+	concentratedLockId, _, err := k.mintSharesAndLock(ctx, clPoolId, positionId, owner, remainingLockDuration)
 	if err != nil {
 		return 0, sdk.Int{}, sdk.Int{}, sdk.Dec{}, time.Time{}, 0, err
 	}
@@ -349,7 +349,7 @@ func (k Keeper) CreateFullRangePositionUnlocking(ctx sdk.Context, clPoolId uint6
 
 	// 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.mintSharesLockAndUpdate(ctx, clPoolId, positionId, owner, remainingLockDuration)
+	concentratedLockId, underlyingLiquidityTokenized, err := k.mintSharesAndLock(ctx, clPoolId, positionId, owner, remainingLockDuration)
 	if err != nil {
 		return 0, sdk.Int{}, sdk.Int{}, sdk.Dec{}, time.Time{}, 0, err
 	}
@@ -363,11 +363,11 @@ func (k Keeper) CreateFullRangePositionUnlocking(ctx sdk.Context, clPoolId uint6
 	return positionId, amount0, amount1, liquidity, joinTime, concentratedLockID, nil
 }
 
-// mintSharesLockAndUpdate 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.
+// 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.
 // In the context of concentrated liquidity, shares need to be minted in order for a lock in its current form to be utilized (we cannot lock non-coin objects).
 // In turn, the locks are a prerequisite for superfluid to be enabled.
 // Additionally, the cl share gets sent to the lockup module account, which, in order to be sent via bank, must be minted.
-func (k Keeper) mintSharesLockAndUpdate(ctx sdk.Context, concentratedPoolId, positionId uint64, owner sdk.AccAddress, remainingLockDuration time.Duration) (concentratedLockID uint64, underlyingLiquidityTokenized sdk.Coins, err error) {
+func (k Keeper) mintSharesAndLock(ctx sdk.Context, concentratedPoolId, positionId uint64, owner sdk.AccAddress, remainingLockDuration time.Duration) (concentratedLockID uint64, underlyingLiquidityTokenized sdk.Coins, err error) {
 	// Ensure the provided position is full range.
 	position, err := k.GetPosition(ctx, positionId)
 	if err != nil {
@@ -392,7 +392,9 @@ func (k Keeper) mintSharesLockAndUpdate(ctx sdk.Context, concentratedPoolId, pos
 
 	// Lock the position for the specified duration.
 	// Note, the end blocker for the lockup module contains an exception for this CL denom. When a lock with a denom of cl/pool/{poolId} is mature,
-	// it does not send the coins to the owner account and instead burns them. This is strictly to use well tested pre-existing methods rather than potentially introducing bugs with new logic and methods.
+	// it does not send the coins to the owner account but instead burns them.
+	// This is implemented in such a way to use well-tested pre-existing methods rather than
+	// completely re-implementing concentrated liquidity superfluid infrastructure that has a risk of introducing bugs with new logic and methods.
 	concentratedLock, err := k.lockupKeeper.CreateLock(ctx, owner, underlyingLiquidityTokenized, remainingLockDuration)
 	if err != nil {
 		return 0, sdk.Coins{}, err
@@ -722,12 +724,18 @@ func (k Keeper) positionHasActiveUnderlyingLockAndUpdate(ctx sdk.Context, positi
 	return hasActiveUnderlyingLock, lockId, nil
 }
 
-// MustGetFullRangeLiquidityInPool returns the total liquidity that is currently in the full range of the pool.
-func (k Keeper) MustGetFullRangeLiquidityInPool(ctx sdk.Context, poolId uint64) sdk.Dec {
+// GetFullRangeLiquidityInPool returns the total liquidity that is currently in the full range of the pool.
+// Returns error if:
+// - fails to retrieve data from the store.
+// - there is no full range liquidity in the pool.
+func (k Keeper) GetFullRangeLiquidityInPool(ctx sdk.Context, poolId uint64) (sdk.Dec, error) {
 	store := ctx.KVStore(k.storeKey)
 	poolIdLiquidityKey := types.KeyPoolIdForLiquidity(poolId)
-	currentTotalFullRangeLiquidity := osmoutils.MustGetDec(store, poolIdLiquidityKey)
-	return currentTotalFullRangeLiquidity
+	currentTotalFullRangeLiquidity, err := osmoutils.GetDec(store, poolIdLiquidityKey)
+	if err != nil {
+		return sdk.Dec{}, err
+	}
+	return currentTotalFullRangeLiquidity, nil
 }
 
 // updateFullRangeLiquidityInPool updates the total liquidity store that is currently in the full range of the pool.