fix!: update formula in CheckValidatorLiquidStakingCap (#11)

* Fix bug in `CheckExceedsValidatorLiquidStakingCap` formula * Update x/staking/keeper/liquid_stake.go Co-authored-by: Marius Poke <[email protected]> * fix unit test bug --------- Co-authored-by: Marius Poke <[email protected]>
informalsystems · Jan 5, 2024 · dc3df2e · dc3df2e
1 parent 773c61c
commit dc3df2e
Show file tree

Hide file tree

Showing 3 changed files with 91 additions and 22 deletions.
diff --git a/x/staking/keeper/liquid_stake.go b/x/staking/keeper/liquid_stake.go
@@ -96,12 +96,20 @@ func (k Keeper) CheckExceedsValidatorBondCap(ctx sdk.Context, validator types.Va
 }
 
 // CheckExceedsValidatorLiquidStakingCap checks if a liquid delegation could cause the
-// total liuquid shares to exceed the liquid staking cap
+// total liquid shares to exceed the liquid staking cap
 // A liquid delegation is defined as either tokenized shares, or a delegation from an ICA Account
+// If the liquid delegation's shares are already bonded (e.g. in the event of a tokenized share)
+// the tokens are already included in the validator's delegator shares
+// If the liquid delegation's shares are not bonded (e.g. normal delegation),
+// we need to add the shares to the current validator's delegator shares to get the total shares
 // Returns true if the cap is exceeded
-func (k Keeper) CheckExceedsValidatorLiquidStakingCap(ctx sdk.Context, validator types.Validator, shares sdk.Dec) bool {
+func (k Keeper) CheckExceedsValidatorLiquidStakingCap(ctx sdk.Context, validator types.Validator, shares sdk.Dec, sharesAlreadyBonded bool) bool {
 	updatedLiquidShares := validator.LiquidShares.Add(shares)
-	updatedTotalShares := validator.DelegatorShares.Add(shares)
+
+	updatedTotalShares := validator.DelegatorShares
+	if !sharesAlreadyBonded {
+		updatedTotalShares = updatedTotalShares.Add(shares)
+	}
 
 	liquidStakePercent := updatedLiquidShares.Quo(updatedTotalShares)
 	liquidStakingCap := k.ValidatorLiquidStakingCap(ctx)
@@ -138,9 +146,9 @@ func (k Keeper) DecreaseTotalLiquidStakedTokens(ctx sdk.Context, amount math.Int
 //
 // The percentage of validator liquid shares must be less than the ValidatorLiquidStakingCap,
 // and the total liquid staked shares cannot exceed the validator bond cap
-// 1) (TotalLiquidStakedTokens / TotalStakedTokens) <= ValidatorLiquidStakingCap
-// 2) LiquidShares <= (ValidatorBondShares * ValidatorBondFactor)
-func (k Keeper) SafelyIncreaseValidatorLiquidShares(ctx sdk.Context, valAddress sdk.ValAddress, shares sdk.Dec) (types.Validator, error) {
+//  1. (TotalLiquidStakedTokens / TotalStakedTokens) <= ValidatorLiquidStakingCap
+//  2. LiquidShares <= (ValidatorBondShares * ValidatorBondFactor)
+func (k Keeper) SafelyIncreaseValidatorLiquidShares(ctx sdk.Context, valAddress sdk.ValAddress, shares sdk.Dec, sharesAlreadyBonded bool) (types.Validator, error) {
 	validator, found := k.GetValidator(ctx, valAddress)
 	if !found {
 		return validator, types.ErrNoValidatorFound
@@ -150,7 +158,7 @@ func (k Keeper) SafelyIncreaseValidatorLiquidShares(ctx sdk.Context, valAddress
 	if k.CheckExceedsValidatorBondCap(ctx, validator, shares) {
 		return validator, types.ErrInsufficientValidatorBondShares
 	}
-	if k.CheckExceedsValidatorLiquidStakingCap(ctx, validator, shares) {
+	if k.CheckExceedsValidatorLiquidStakingCap(ctx, validator, shares, sharesAlreadyBonded) {
 		return validator, types.ErrValidatorLiquidStakingCapExceeded
 	}
 

diff --git a/x/staking/keeper/liquid_stake_test.go b/x/staking/keeper/liquid_stake_test.go
@@ -218,72 +218,131 @@ func (s *KeeperTestSuite) TestCheckExceedsValidatorLiquidStakingCap() {
 		validatorLiquidShares sdk.Dec
 		validatorTotalShares  sdk.Dec
 		newLiquidShares       sdk.Dec
+		tokenizingShares      bool
 		expectedExceeds       bool
 	}{
 		{
 			// Cap: 10% - Delegation Below Threshold
 			// Liquid Shares: 5, Total Shares: 95, New Liquid Shares: 1
 			// => Liquid Shares: 5+1=6, Total Shares: 95+1=96 => 6/96 = 6% < 10% cap
-			name:                  "10 percent cap _ delegation below cap",
+			name:                  "10 percent cap _ native delegation _ below cap",
 			validatorLiquidCap:    sdk.MustNewDecFromStr("0.1"),
 			validatorLiquidShares: sdk.NewDec(5),
 			validatorTotalShares:  sdk.NewDec(95),
 			newLiquidShares:       sdk.NewDec(1),
+			tokenizingShares:      false,
 			expectedExceeds:       false,
 		},
 		{
 			// Cap: 10% - Delegation At Threshold
 			// Liquid Shares: 5, Total Shares: 95, New Liquid Shares: 5
 			// => Liquid Shares: 5+5=10, Total Shares: 95+5=100 => 10/100 = 10% == 10% cap
-			name:                  "10 percent cap _ delegation equals cap",
+			name:                  "10 percent cap _ native delegation _ equals cap",
 			validatorLiquidCap:    sdk.MustNewDecFromStr("0.1"),
 			validatorLiquidShares: sdk.NewDec(5),
 			validatorTotalShares:  sdk.NewDec(95),
 			newLiquidShares:       sdk.NewDec(4),
+			tokenizingShares:      false,
 			expectedExceeds:       false,
 		},
 		{
 			// Cap: 10% - Delegation Exceeds Threshold
 			// Liquid Shares: 5, Total Shares: 95, New Liquid Shares: 6
 			// => Liquid Shares: 5+6=11, Total Shares: 95+6=101 => 11/101 = 11% > 10% cap
-			name:                  "10 percent cap _ delegation exceeds cap",
+			name:                  "10 percent cap _ native delegation _ exceeds cap",
 			validatorLiquidCap:    sdk.MustNewDecFromStr("0.1"),
 			validatorLiquidShares: sdk.NewDec(5),
 			validatorTotalShares:  sdk.NewDec(95),
 			newLiquidShares:       sdk.NewDec(6),
+			tokenizingShares:      false,
 			expectedExceeds:       true,
 		},
 		{
 			// Cap: 20% - Delegation Below Threshold
 			// Liquid Shares: 20, Total Shares: 220, New Liquid Shares: 29
 			// => Liquid Shares: 20+29=49, Total Shares: 220+29=249 => 49/249 = 19% < 20% cap
-			name:                  "20 percent cap _ delegation below cap",
+			name:                  "20 percent cap _ native delegation _ below cap",
 			validatorLiquidCap:    sdk.MustNewDecFromStr("0.2"),
 			validatorLiquidShares: sdk.NewDec(20),
 			validatorTotalShares:  sdk.NewDec(220),
 			newLiquidShares:       sdk.NewDec(29),
+			tokenizingShares:      false,
 			expectedExceeds:       false,
 		},
 		{
 			// Cap: 20% - Delegation At Threshold
 			// Liquid Shares: 20, Total Shares: 220, New Liquid Shares: 30
 			// => Liquid Shares: 20+30=50, Total Shares: 220+30=250 => 50/250 = 20% == 20% cap
-			name:                  "20 percent cap _ delegation equals cap",
+			name:                  "20 percent cap _ native delegation _ equals cap",
 			validatorLiquidCap:    sdk.MustNewDecFromStr("0.2"),
 			validatorLiquidShares: sdk.NewDec(20),
 			validatorTotalShares:  sdk.NewDec(220),
 			newLiquidShares:       sdk.NewDec(30),
+			tokenizingShares:      false,
 			expectedExceeds:       false,
 		},
 		{
 			// Cap: 20% - Delegation Exceeds Threshold
 			// Liquid Shares: 20, Total Shares: 220, New Liquid Shares: 31
 			// => Liquid Shares: 20+31=51, Total Shares: 220+31=251 => 51/251 = 21% > 20% cap
-			name:                  "20 percent cap _ delegation exceeds cap",
+			name:                  "20 percent cap _ native delegation _ exceeds cap",
 			validatorLiquidCap:    sdk.MustNewDecFromStr("0.2"),
 			validatorLiquidShares: sdk.NewDec(20),
 			validatorTotalShares:  sdk.NewDec(220),
 			newLiquidShares:       sdk.NewDec(31),
+			tokenizingShares:      false,
+			expectedExceeds:       true,
+		},
+		{
+			// Cap: 50% - Native Delegation - Delegation At Threshold
+			// Liquid shares: 0, Total Shares: 100, New Liquid Shares: 50
+			// Total Liquid Shares: 0+50=50, Total Shares: 100+50=150
+			// => 50/150 = 33% < 50% cap
+			name:                  "50 percent cap _ native delegation _ delegation equals cap",
+			validatorLiquidCap:    sdk.MustNewDecFromStr("0.5"),
+			validatorLiquidShares: sdk.NewDec(0),
+			validatorTotalShares:  sdk.NewDec(100),
+			newLiquidShares:       sdk.NewDec(50),
+			tokenizingShares:      false,
+			expectedExceeds:       false,
+		},
+		{
+			// Cap: 50% - Tokenized Delegation - Delegation At Threshold
+			// Liquid shares: 0, Total Shares: 100, New Liquid Shares: 50
+			// Total Liquid Shares => 0+50=50, Total Shares: 100,  New Liquid Shares: 50
+			// => 50 / 100 = 50% == 50% cap
+			name:                  "50 percent cap _ tokenized delegation _ delegation equals cap",
+			validatorLiquidCap:    sdk.MustNewDecFromStr("0.5"),
+			validatorLiquidShares: sdk.NewDec(0),
+			validatorTotalShares:  sdk.NewDec(100),
+			newLiquidShares:       sdk.NewDec(50),
+			tokenizingShares:      true,
+			expectedExceeds:       false,
+		},
+		{
+			// Cap: 50% - Native Delegation - Delegation At Threshold
+			// Liquid shares: 0, Total Shares: 100, New Liquid Shares: 51
+			// Total Liquid Shares: 0+51=51, Total Shares: 100+51=151
+			// => 51/150 = 33% < 50% cap
+			name:                  "50 percent cap _ native delegation _ delegation equals cap",
+			validatorLiquidCap:    sdk.MustNewDecFromStr("0.5"),
+			validatorLiquidShares: sdk.NewDec(0),
+			validatorTotalShares:  sdk.NewDec(100),
+			newLiquidShares:       sdk.NewDec(51),
+			tokenizingShares:      false,
+			expectedExceeds:       false,
+		},
+		{
+			// Cap: 50% - Tokenized Delegation - Delegation At Threshold
+			// Liquid shares: 0, Total Shares: 100, New Liquid Shares: 50
+			// Total Liquid Shares => 0+51=51, Total Shares: 100,  New Liquid Shares: 51
+			// => 51 / 100 = 51% > 50% cap
+			name:                  "50 percent cap _ tokenized delegation _ delegation equals cap",
+			validatorLiquidCap:    sdk.MustNewDecFromStr("0.5"),
+			validatorLiquidShares: sdk.NewDec(0),
+			validatorTotalShares:  sdk.NewDec(100),
+			newLiquidShares:       sdk.NewDec(51),
+			tokenizingShares:      true,
 			expectedExceeds:       true,
 		},
 		{
@@ -293,6 +352,7 @@ func (s *KeeperTestSuite) TestCheckExceedsValidatorLiquidStakingCap() {
 			validatorLiquidShares: sdk.NewDec(0),
 			validatorTotalShares:  sdk.NewDec(1_000_000),
 			newLiquidShares:       sdk.NewDec(1),
+			tokenizingShares:      false,
 			expectedExceeds:       true,
 		},
 		{
@@ -302,6 +362,7 @@ func (s *KeeperTestSuite) TestCheckExceedsValidatorLiquidStakingCap() {
 			validatorLiquidShares: sdk.NewDec(1),
 			validatorTotalShares:  sdk.NewDec(1_000_000),
 			newLiquidShares:       sdk.NewDec(1),
+			tokenizingShares:      false,
 			expectedExceeds:       false,
 		},
 	}
@@ -320,7 +381,7 @@ func (s *KeeperTestSuite) TestCheckExceedsValidatorLiquidStakingCap() {
 			}
 
 			// Check whether the cap is exceeded
-			actualExceeds := keeper.CheckExceedsValidatorLiquidStakingCap(ctx, validator, tc.newLiquidShares)
+			actualExceeds := keeper.CheckExceedsValidatorLiquidStakingCap(ctx, validator, tc.newLiquidShares, tc.tokenizingShares)
 			require.Equal(tc.expectedExceeds, actualExceeds, tc.name)
 		})
 	}
@@ -386,7 +447,7 @@ func (s *KeeperTestSuite) TestSafelyIncreaseValidatorLiquidShares() {
 
 	// Attempt to increase the validator liquid shares, it should throw an
 	// error that the validator bond cap was exceeded
-	_, err := keeper.SafelyIncreaseValidatorLiquidShares(ctx, valAddress, firstIncreaseAmount)
+	_, err := keeper.SafelyIncreaseValidatorLiquidShares(ctx, valAddress, firstIncreaseAmount, false)
 	require.ErrorIs(err, types.ErrInsufficientValidatorBondShares)
 	checkValidatorLiquidShares(initialLiquidShares, "shares after low bond factor")
 
@@ -396,12 +457,12 @@ func (s *KeeperTestSuite) TestSafelyIncreaseValidatorLiquidShares() {
 
 	// Try the increase again and check that it succeeded
 	expectedLiquidSharesAfterFirstStake := initialLiquidShares.Add(firstIncreaseAmount)
-	_, err = keeper.SafelyIncreaseValidatorLiquidShares(ctx, valAddress, firstIncreaseAmount)
+	_, err = keeper.SafelyIncreaseValidatorLiquidShares(ctx, valAddress, firstIncreaseAmount, false)
 	require.NoError(err)
 	checkValidatorLiquidShares(expectedLiquidSharesAfterFirstStake, "shares with cap loose bond cap")
 
 	// Attempt another increase, it should fail from the liquid staking cap
-	_, err = keeper.SafelyIncreaseValidatorLiquidShares(ctx, valAddress, secondIncreaseAmount)
+	_, err = keeper.SafelyIncreaseValidatorLiquidShares(ctx, valAddress, secondIncreaseAmount, false)
 	require.ErrorIs(err, types.ErrValidatorLiquidStakingCapExceeded)
 	checkValidatorLiquidShares(expectedLiquidSharesAfterFirstStake, "shares after liquid staking cap hit")
 
@@ -411,7 +472,7 @@ func (s *KeeperTestSuite) TestSafelyIncreaseValidatorLiquidShares() {
 
 	// Finally confirm that the increase succeeded this time
 	expectedLiquidSharesAfterSecondStake := expectedLiquidSharesAfterFirstStake.Add(secondIncreaseAmount)
-	_, err = keeper.SafelyIncreaseValidatorLiquidShares(ctx, valAddress, secondIncreaseAmount)
+	_, err = keeper.SafelyIncreaseValidatorLiquidShares(ctx, valAddress, secondIncreaseAmount, false)
 	require.NoError(err, "no error expected after increasing liquid staking cap")
 	checkValidatorLiquidShares(expectedLiquidSharesAfterSecondStake, "shares after loose liquid stake cap")
 }

diff --git a/x/staking/keeper/msg_server.go b/x/staking/keeper/msg_server.go
@@ -222,7 +222,7 @@ func (k msgServer) Delegate(goCtx context.Context, msg *types.MsgDelegate) (*typ
 		if err := k.SafelyIncreaseTotalLiquidStakedTokens(ctx, tokens, false); err != nil {
 			return nil, err
 		}
-		validator, err = k.SafelyIncreaseValidatorLiquidShares(ctx, valAddr, shares)
+		validator, err = k.SafelyIncreaseValidatorLiquidShares(ctx, valAddr, shares, false)
 		if err != nil {
 			return nil, err
 		}
@@ -328,7 +328,7 @@ func (k msgServer) BeginRedelegate(goCtx context.Context, msg *types.MsgBeginRed
 		if err != nil {
 			return nil, err
 		}
-		if _, err := k.SafelyIncreaseValidatorLiquidShares(ctx, valDstAddr, dstShares); err != nil {
+		if _, err := k.SafelyIncreaseValidatorLiquidShares(ctx, valDstAddr, dstShares, false); err != nil {
 			return nil, err
 		}
 		if _, err := k.DecreaseValidatorLiquidShares(ctx, valSrcAddr, srcShares); err != nil {
@@ -540,7 +540,7 @@ func (k msgServer) CancelUnbondingDelegation(goCtx context.Context, msg *types.M
 		if err := k.SafelyIncreaseTotalLiquidStakedTokens(ctx, tokens, false); err != nil {
 			return nil, err
 		}
-		validator, err = k.SafelyIncreaseValidatorLiquidShares(ctx, valAddr, shares)
+		validator, err = k.SafelyIncreaseValidatorLiquidShares(ctx, valAddr, shares, false)
 		if err != nil {
 			return nil, err
 		}
@@ -724,7 +724,7 @@ func (k msgServer) TokenizeShares(goCtx context.Context, msg *types.MsgTokenizeS
 		if err := k.SafelyIncreaseTotalLiquidStakedTokens(ctx, msg.Amount.Amount, true); err != nil {
 			return nil, err
 		}
-		validator, err = k.SafelyIncreaseValidatorLiquidShares(ctx, valAddr, shares)
+		validator, err = k.SafelyIncreaseValidatorLiquidShares(ctx, valAddr, shares, true)
 		if err != nil {
 			return nil, err
 		}