[CL Message Audit]: AddToPositions

proto/osmosis/concentrated-liquidity/tx.proto

@@ -12,6 +12,11 @@ service Msg {
   rpc CreatePosition(MsgCreatePosition) returns (MsgCreatePositionResponse);
   rpc WithdrawPosition(MsgWithdrawPosition)
       returns (MsgWithdrawPositionResponse);
+  // AddToPosition attempts to add amount0Added and amount1Added to a position
+  // with the given position id.
+  // Due backwards-compatibility with future implementations of charging, this
+  // function deletes the old position and creates a new one with the resulting
+  // amount after addition.
   rpc AddToPosition(MsgAddToPosition) returns (MsgAddToPositionResponse);
   rpc CollectFees(MsgCollectFees) returns (MsgCollectFeesResponse);
   rpc CollectIncentives(MsgCollectIncentives)
@@ -75,12 +80,34 @@ message MsgCreatePositionResponse {
 message MsgAddToPosition {
   uint64 position_id = 1 [ (gogoproto.moretags) = "yaml:\"position_id\"" ];
   string sender = 2 [ (gogoproto.moretags) = "yaml:\"sender\"" ];
-  cosmos.base.v1beta1.Coin token_desired0 = 3 [
-    (gogoproto.moretags) = "yaml:\"token_desired0\"",
+  // amount0 represents the amount of token0 willing to put in.
+  string amount0 = 3 [
+    (gogoproto.customtype) = "github.com/cosmos/cosmos-sdk/types.Int",
+    (gogoproto.moretags) = "yaml:\"token_min_amount0\"",
+    (gogoproto.nullable) = false
+  ];
+  // amount1 represents the amount of token1 willing to put in.
+  string amount1 = 4 [
+    (gogoproto.customtype) = "github.com/cosmos/cosmos-sdk/types.Int",
+    (gogoproto.moretags) = "yaml:\"token_min_amount1\"",
     (gogoproto.nullable) = false
   ];
-  cosmos.base.v1beta1.Coin token_desired1 = 4 [
-    (gogoproto.moretags) = "yaml:\"token_desired1\"",
+  // token_min_amount0 represents the minimum amount of token0 desired from the
+  // new position being created. Note that this field indicates the min amount0
+  // corresponding to the total liquidity of the position, not just the
+  // liquidity that is being added.
+  string token_min_amount0 = 5 [
+    (gogoproto.customtype) = "github.com/cosmos/cosmos-sdk/types.Int",
+    (gogoproto.moretags) = "yaml:\"token_min_amount0\"",
+    (gogoproto.nullable) = false
+  ];
+  // token_min_amount0 represents the minimum amount of token0 desired from the
+  // new position being created. Note that this field indicates the min amount0
+  // corresponding to the total liquidity of the position, not just the
+  // liquidity that is being added.
+  string token_min_amount1 = 6 [
+    (gogoproto.customtype) = "github.com/cosmos/cosmos-sdk/types.Int",
+    (gogoproto.moretags) = "yaml:\"token_min_amount1\"",
     (gogoproto.nullable) = false
   ];
 }

x/concentrated-liquidity/export_test.go

@@ -177,8 +177,8 @@ func (k Keeper) CreatePosition(ctx sdk.Context, poolId uint64, owner sdk.AccAddr
 	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 sdk.Int) (uint64, sdk.Int, sdk.Int, error) {
-	return k.addToPosition(ctx, owner, positionId, amount0Added, amount1Added)
+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)
 }
 
 func (ss *SwapState) UpdateFeeGrowthGlobal(feeChargeTotal sdk.Dec) {

x/concentrated-liquidity/lp.go

@@ -244,13 +244,17 @@ func (k Keeper) WithdrawPosition(ctx sdk.Context, owner sdk.AccAddress, position
 // For the sake of backwards-compatibility with future implementations of charging, this function deletes the old position and creates
 // a new one with the resulting amount after addition. Note that due to truncation after `withdrawPosition`, there is some rounding error
 // that is upper bounded by 1 unit of the more valuable token.
+// Uses the amount0MinGiven,amount1MinGiven as the minimum token out for creating the new position.
+// Note that these field indicates the min amount corresponding to the total liquidity of the position,
+// not only for the liquidity amount that is being added.
+// Uses amounts withdrawn from the original position if provided min amount is zero.
 // Returns error if
 // - Withdrawing full position fails
 // - Creating new position with added liquidity fails
 // - Position with `positionId` is the last position in the pool
 // - Position is superfluid staked
 // TODO: handle adding to SFS positions
-func (k Keeper) addToPosition(ctx sdk.Context, owner sdk.AccAddress, positionId uint64, amount0Added, amount1Added sdk.Int) (uint64, sdk.Int, sdk.Int, error) {
+func (k Keeper) addToPosition(ctx sdk.Context, owner sdk.AccAddress, positionId uint64, amount0Added, amount1Added, amount0MinGiven, amount1MinGiven sdk.Int) (uint64, sdk.Int, sdk.Int, error) {
 	position, err := k.GetPosition(ctx, positionId)
 	if err != nil {
 		return 0, sdk.Int{}, sdk.Int{}, err
@@ -261,7 +265,7 @@ func (k Keeper) addToPosition(ctx sdk.Context, owner sdk.AccAddress, positionId
 		return 0, sdk.Int{}, sdk.Int{}, types.NotPositionOwnerError{PositionId: positionId, Address: owner.String()}
 	}
 
-	if amount0Added.IsNegative() || amount1Added.IsNegative() {
+	if amount0Added.IsNegative() || amount1Added.IsNegative() || (amount0Added.IsZero() && amount1Added.IsZero()) {
 		return 0, sdk.Int{}, sdk.Int{}, types.NegativeAmountAddedError{PositionId: position.PositionId, Asset0Amount: amount0Added, Asset1Amount: amount1Added}
 	}
 
@@ -298,7 +302,16 @@ func (k Keeper) addToPosition(ctx sdk.Context, owner sdk.AccAddress, positionId
 		return 0, sdk.Int{}, sdk.Int{}, err
 	}
 	tokensProvided := sdk.NewCoins(sdk.NewCoin(pool.GetToken0(), amount0Desired), sdk.NewCoin(pool.GetToken1(), amount1Desired))
-	newPositionId, actualAmount0, actualAmount1, _, _, err := k.createPosition(ctx, position.PoolId, owner, tokensProvided, amount0Withdrawn, amount1Withdrawn, position.LowerTick, position.UpperTick)
+	minimumAmount0 := amount0Withdrawn
+	minimumAmount1 := amount1Withdrawn
+
+	if !amount0MinGiven.IsZero() {
+		minimumAmount0 = amount0MinGiven
+	}
+	if !amount1MinGiven.IsZero() {
+		minimumAmount1 = amount1MinGiven
+	}
+	newPositionId, actualAmount0, actualAmount1, _, _, err := k.createPosition(ctx, position.PoolId, owner, tokensProvided, minimumAmount0, minimumAmount1, position.LowerTick, position.UpperTick)
 	if err != nil {
 		return 0, sdk.Int{}, sdk.Int{}, err
 	}

x/concentrated-liquidity/lp_test.go

@@ -651,7 +651,6 @@ func (s *KeeperTestSuite) TestWithdrawPosition() {
 
 func (s *KeeperTestSuite) TestAddToPosition() {
 	defaultTimeElapsed := time.Hour * 24
-	roundingError := sdk.OneInt()
 	invalidSender := s.TestAccs[2]
 
 	// These amounts are set based on the actual amounts passed in as inputs
@@ -744,21 +743,16 @@ func (s *KeeperTestSuite) TestAddToPosition() {
 			amount1ToAdd: amount1PerfectRatio.QuoRaw(2),
 		},
 
-		// Error catching
-
-		"error: attempt to add to a position with underlying lock that has finished unlocking": {
+		"Add to a position with underlying lock that has finished unlocking": {
 			// setup parameters for creating a pool and position.
 			setupConfig: baseCase,
 
 			// system under test parameters
 			sutConfigOverwrite: &lpTest{
-				amount0Expected: amount0PerfectRatio.Add(amount0PerfectRatio).Sub(roundingError),
-				// Since we round on the other the asset when we withdraw, asset0 turns into the bottleneck and
-				// thus we cannot use the full amount of asset1. We calculate the below using the following formula and rounding up:
-				// amount1 = L * (sqrtPriceUpper - sqrtPriceLower)
-				// https://www.wolframalpha.com/input?i=3035764327.860030912175533748+*+%2870.710678118654752440+-+67.416615162732695594%29
-				amount1Expected: sdk.NewInt(9999998816),
-				expectedError:   types.PositionSuperfluidStakedError{PositionId: uint64(1)},
+				// 1998976eth (amount withdrawn with rounded down amounts) + 998977(token amount in)
+				amount0Expected: sdk.NewInt(2997953),
+				// tokens Provided for token1 is 9999999999 (amount withdrawn) + 5000000000 = 14999999999usdc, then liquidity needed is calculated internally
+				amount1Expected: sdk.NewInt(14997436189),
 			},
 			timeElapsed:  defaultTimeElapsed,
 			amount0ToAdd: amount0PerfectRatio,
@@ -813,12 +807,7 @@ func (s *KeeperTestSuite) TestAddToPosition() {
 
 			// system under test parameters
 			sutConfigOverwrite: &lpTest{
-				amount0Expected: amount0PerfectRatio.Sub(roundingError),
-				// Since we round on the other the asset when we withdraw, asset0 turns into the bottleneck and
-				// thus we cannot use the full amount of asset1. We calculate the below using the following formula and rounding up:
-				// amount1 = L * (sqrtPriceUpper - sqrtPriceLower)
-				// https://www.wolframalpha.com/input?i=3035764327.860030912175533748+*+%2870.710678118654752440+-+67.416615162732695594%29
-				expectedError: types.InsufficientLiquidityCreatedError{Actual: sdk.NewInt(4999996906), Minimum: baseCase.tokensProvided.AmountOf(DefaultCoin1.Denom).Sub(roundingError)},
+				expectedError: types.NegativeAmountAddedError{PositionId: 1, Asset0Amount: sdk.ZeroInt(), Asset1Amount: sdk.ZeroInt()},
 			},
 			timeElapsed:  defaultTimeElapsed,
 			amount0ToAdd: sdk.ZeroInt(),
@@ -851,53 +840,48 @@ func (s *KeeperTestSuite) TestAddToPosition() {
 			amount0ToAdd:       amount0PerfectRatio,
 			amount1ToAdd:       amount1PerfectRatio,
 		},
-		"error: attempt to add negative asset0 to position": {
+		"error: not position owner": {
 			// setup parameters for creating a pool and position.
-			setupConfig: baseCase,
+			setupConfig:    baseCase,
+			senderNotOwner: true,
 
 			// system under test parameters
 			sutConfigOverwrite: &lpTest{
-				expectedError: types.NegativeAmountAddedError{PositionId: 1, Asset0Amount: amount0PerfectRatio.Neg(), Asset1Amount: amount1PerfectRatio},
+				expectedError: types.NotPositionOwnerError{PositionId: 1, Address: invalidSender.String()},
 			},
-			lastPositionInPool: true,
-			timeElapsed:        defaultTimeElapsed,
-			amount0ToAdd:       amount0PerfectRatio.Neg(),
-			amount1ToAdd:       amount1PerfectRatio,
+			timeElapsed:  defaultTimeElapsed,
+			amount0ToAdd: amount0PerfectRatio,
+			amount1ToAdd: amount1PerfectRatio,
 		},
-		"error: attempt to add negative asset1 to position": {
+		"error: minimum amount 0 is less than actual amount": {
 			// setup parameters for creating a pool and position.
 			setupConfig: baseCase,
 
 			// system under test parameters
 			sutConfigOverwrite: &lpTest{
-				expectedError: types.NegativeAmountAddedError{PositionId: 1, Asset0Amount: amount0PerfectRatio, Asset1Amount: amount1PerfectRatio.Neg()},
+				amount0Minimum: sdk.NewInt(1997960),
+				expectedError: types.InsufficientLiquidityCreatedError{
+					Actual:      sdk.NewInt(1997954),
+					Minimum:     sdk.NewInt(1997960),
+					IsTokenZero: true,
+				},
 			},
-			lastPositionInPool: true,
-			timeElapsed:        defaultTimeElapsed,
-			amount0ToAdd:       amount0PerfectRatio,
-			amount1ToAdd:       amount1PerfectRatio.Neg(),
+			timeElapsed:  defaultTimeElapsed,
+			amount0ToAdd: amount0PerfectRatio,
+			amount1ToAdd: amount1PerfectRatio,
 		},
-		"error: attempt to add negative amounts for both assets to position": {
+		"error: minimum amount 1 is less than actual amount": {
 			// setup parameters for creating a pool and position.
 			setupConfig: baseCase,
 
 			// system under test parameters
 			sutConfigOverwrite: &lpTest{
-				expectedError: types.NegativeAmountAddedError{PositionId: 1, Asset0Amount: amount0PerfectRatio.Neg(), Asset1Amount: amount1PerfectRatio.Neg()},
-			},
-			lastPositionInPool: true,
-			timeElapsed:        defaultTimeElapsed,
-			amount0ToAdd:       amount0PerfectRatio.Neg(),
-			amount1ToAdd:       amount1PerfectRatio.Neg(),
-		},
-		"error: not position owner": {
-			// setup parameters for creating a pool and position.
-			setupConfig:    baseCase,
-			senderNotOwner: true,
-
-			// system under test parameters
-			sutConfigOverwrite: &lpTest{
-				expectedError: types.NotPositionOwnerError{PositionId: 1, Address: invalidSender.String()},
+				amount1Minimum: sdk.NewInt(9999998916),
+				expectedError: types.InsufficientLiquidityCreatedError{
+					Actual:      sdk.NewInt(9999998816),
+					Minimum:     sdk.NewInt(9999998916),
+					IsTokenZero: false,
+				},
 			},
 			timeElapsed:  defaultTimeElapsed,
 			amount0ToAdd: amount0PerfectRatio,
@@ -927,31 +911,32 @@ func (s *KeeperTestSuite) TestAddToPosition() {
 			// If a setupConfig is provided, use it to create a pool and position.
 			pool := s.PrepareConcentratedPool()
 			fundCoins := config.tokensProvided
+			// Fund tokens that is used to create initial position
 			if tc.amount0ToAdd.IsPositive() && tc.amount1ToAdd.IsPositive() {
 				fundCoins = fundCoins.Add(sdk.NewCoins(sdk.NewCoin(ETH, tc.amount0ToAdd), sdk.NewCoin(USDC, tc.amount1ToAdd))...)
 			}
 			s.FundAcc(owner, fundCoins)
 
 			// Create a position from the parameters in the test case.
-			var amount0Initial, amount1Initial sdk.Int
 			if tc.createLockLocked {
-				_, amount0Initial, amount1Initial, _, _, _, err = concentratedLiquidityKeeper.CreateFullRangePositionLocked(s.Ctx, pool.GetId(), owner, fundCoins, tc.timeElapsed)
+				_, _, _, _, _, _, err = concentratedLiquidityKeeper.CreateFullRangePositionLocked(s.Ctx, pool.GetId(), owner, fundCoins, tc.timeElapsed)
 				s.Require().NoError(err)
 			} else if tc.createLockUnlocking {
-				_, amount0Initial, amount1Initial, _, _, _, err = concentratedLiquidityKeeper.CreateFullRangePositionUnlocking(s.Ctx, pool.GetId(), owner, fundCoins, tc.timeElapsed+time.Hour)
+				_, _, _, _, _, _, err = concentratedLiquidityKeeper.CreateFullRangePositionUnlocking(s.Ctx, pool.GetId(), owner, fundCoins, tc.timeElapsed+time.Hour)
 				s.Require().NoError(err)
 			} else if tc.createLockUnlocked {
-				_, amount0Initial, amount1Initial, _, _, _, err = concentratedLiquidityKeeper.CreateFullRangePositionUnlocking(s.Ctx, pool.GetId(), owner, fundCoins, tc.timeElapsed-time.Hour)
+				_, _, _, _, _, _, err = concentratedLiquidityKeeper.CreateFullRangePositionUnlocking(s.Ctx, pool.GetId(), owner, fundCoins, tc.timeElapsed-time.Hour)
 				s.Require().NoError(err)
 			} else {
-				_, amount0Initial, amount1Initial, _, _, err = concentratedLiquidityKeeper.CreatePosition(s.Ctx, pool.GetId(), owner, config.tokensProvided, sdk.ZeroInt(), sdk.ZeroInt(), DefaultLowerTick, DefaultUpperTick)
+				_, _, _, _, _, err = concentratedLiquidityKeeper.CreatePosition(s.Ctx, pool.GetId(), owner, config.tokensProvided, sdk.ZeroInt(), sdk.ZeroInt(), DefaultLowerTick, DefaultUpperTick)
 				s.Require().NoError(err)
 			}
-			preSendBalanceSender := s.App.BankKeeper.GetAllBalances(s.Ctx, owner)
+			s.Ctx = s.Ctx.WithBlockTime(s.Ctx.BlockTime().Add(tc.timeElapsed))
+			preBalanceToken0 := s.App.BankKeeper.GetBalance(s.Ctx, owner, pool.GetToken0())
 
 			if !tc.lastPositionInPool {
 				s.FundAcc(s.TestAccs[1], fundCoins)
-				_, _, _, _, _, err = concentratedLiquidityKeeper.CreatePosition(s.Ctx, pool.GetId(), s.TestAccs[1], config.tokensProvided, sdk.ZeroInt(), sdk.ZeroInt(), DefaultLowerTick, DefaultUpperTick)
+				_, _, _, _, _, err := concentratedLiquidityKeeper.CreatePosition(s.Ctx, pool.GetId(), s.TestAccs[1], config.tokensProvided, sdk.ZeroInt(), sdk.ZeroInt(), DefaultLowerTick, DefaultUpperTick)
 				s.Require().NoError(err)
 			}
 
@@ -960,8 +945,12 @@ func (s *KeeperTestSuite) TestAddToPosition() {
 				sender = invalidSender
 			}
 
+			// now we fund the sender account again for the amount0ToAdd and amount1ToAdd coins.
+			s.FundAcc(sender, sdk.NewCoins(sdk.NewCoin(ETH, tc.amount0ToAdd), sdk.NewCoin(USDC, tc.amount1ToAdd)))
+
 			// --- System under test ---
-			newPosId, newAmt0, newAmt1, err := concentratedLiquidityKeeper.AddToPosition(s.Ctx, sender, config.positionId, tc.amount0ToAdd, tc.amount1ToAdd)
+			newPosId, newAmt0, newAmt1, err := concentratedLiquidityKeeper.AddToPosition(s.Ctx, sender, config.positionId, tc.amount0ToAdd, tc.amount1ToAdd, config.amount0Minimum, config.amount1Minimum)
+			// config.amount0Minimum
 			if config.expectedError != nil {
 				s.Require().Error(err)
 				s.Require().Equal(sdk.Int{}, newAmt0)
@@ -978,15 +967,23 @@ func (s *KeeperTestSuite) TestAddToPosition() {
 			// We expect the position ID to be 3 since we have two setup positions
 			s.Require().Equal(uint64(3), newPosId)
 
-			// Ensure balances were deducted by the correct amounts
-			// Note that we subtract rounding error from the initial amount of
-			// both assets since both are truncated upon withdrawal (so there is at least one
-			// unit of each left in the pool).
-			postSendBalanceSender := s.App.BankKeeper.GetAllBalances(s.Ctx, sender)
-			s.Require().Equal(
-				sdk.NewCoins(sdk.NewCoin(pool.GetToken0(), config.amount0Expected.Sub(amount0Initial.Sub(roundingError))), sdk.NewCoin(pool.GetToken1(), config.amount1Expected.Sub(amount1Initial.Sub(roundingError)))),
-				preSendBalanceSender.Sub(postSendBalanceSender),
-			)
+			expectedAmount1Delta := sdk.ZeroInt()
+
+			// delta amount1 only exists if the actual amount from addToPosition is not equivilent to tokens provided.
+			// delta amount1 is calculated via (amount1 to create initial position) + (amount1 added to position) - (actual amount 1)
+			if fundCoins.AmountOf(pool.GetToken1()).Add(tc.amount1ToAdd).Sub(newAmt1).GT(sdk.ZeroInt()) {
+				expectedAmount1Delta = config.tokensProvided.AmountOf(pool.GetToken1()).Add(tc.amount1ToAdd).Sub(newAmt1)
+			}
+
+			postBalanceToken0 := s.App.BankKeeper.GetBalance(s.Ctx, sender, pool.GetToken0())
+			postBalanceToken1 := s.App.BankKeeper.GetBalance(s.Ctx, sender, pool.GetToken1())
+
+			var errTolerance osmomath.ErrTolerance
+			errTolerance.AdditiveTolerance = sdk.OneDec()
+			errTolerance.RoundingDir = osmomath.RoundDown
+
+			s.Require().Equal(0, errTolerance.Compare(preBalanceToken0.Amount, postBalanceToken0.Amount))
+			s.Require().Equal(0, errTolerance.Compare(expectedAmount1Delta, postBalanceToken1.Amount.Sub(tc.amount1ToAdd)))
 		})
 	}
 }

x/concentrated-liquidity/msg_server.go

@@ -81,7 +81,14 @@ func (server msgServer) AddToPosition(goCtx context.Context, msg *types.MsgAddTo
 		return nil, err
 	}
 
-	positionId, actualAmount0, actualAmount1, err := server.keeper.addToPosition(ctx, sender, msg.PositionId, msg.TokenDesired0.Amount, msg.TokenDesired1.Amount)
+	if msg.TokenMinAmount0.IsNil() {
+		msg.TokenMinAmount0 = sdk.ZeroInt()
+	}
+	if msg.TokenMinAmount1.IsNil() {
+		msg.TokenMinAmount1 = sdk.ZeroInt()
+	}
+
+	positionId, actualAmount0, actualAmount1, err := server.keeper.addToPosition(ctx, sender, msg.PositionId, msg.Amount0, msg.Amount1, msg.TokenMinAmount0, msg.TokenMinAmount1)
 	if err != nil {
 		return nil, err
 	}

x/concentrated-liquidity/msg_server_test.go

@@ -112,7 +112,6 @@ func (suite *KeeperTestSuite) TestCreatePositionMsg() {
 			ctx := suite.Ctx
 
 			baseConfigCopy := *baseCase
-			fmt.Println(baseConfigCopy.tokensProvided)
 			mergeConfigs(&baseConfigCopy, &tc)
 			tc = baseConfigCopy
 
@@ -191,10 +190,10 @@ func (suite *KeeperTestSuite) TestAddToPosition_Events() {
 
 			suite.FundAcc(suite.TestAccs[0], sdk.NewCoins(DefaultCoin0, DefaultCoin1))
 			msg := &types.MsgAddToPosition{
-				PositionId:    posId,
-				Sender:        suite.TestAccs[0].String(),
-				TokenDesired0: DefaultCoin0,
-				TokenDesired1: DefaultCoin1,
+				PositionId: posId,
+				Sender:     suite.TestAccs[0].String(),
+				Amount0:    DefaultCoin0.Amount,
+				Amount1:    DefaultCoin1.Amount,
 			}
 
 			response, err := msgServer.AddToPosition(sdk.WrapSDKContext(suite.Ctx), msg)