Skip to content

Commit

Permalink
Fix bug in CheckExceedsValidatorLiquidStakingCap formula
Browse files Browse the repository at this point in the history
  • Loading branch information
@sainoe
sainoe committed Jan 4, 2024
1 parent 773c61c commit 5ff3cce
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 22 deletions.
22 changes: 15 additions & 7 deletions x/staking/keeper/liquid_stake.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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, false) {
return validator, types.ErrValidatorLiquidStakingCapExceeded
}

Expand Down
83 changes: 72 additions & 11 deletions x/staking/keeper/liquid_stake_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
{
Expand All @@ -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,
},
{
Expand All @@ -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,
},
}
Expand All @@ -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)
})
}
Expand Down Expand Up @@ -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, true)
require.ErrorIs(err, types.ErrInsufficientValidatorBondShares)
checkValidatorLiquidShares(initialLiquidShares, "shares after low bond factor")

Expand All @@ -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, true)
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, true)
require.ErrorIs(err, types.ErrValidatorLiquidStakingCapExceeded)
checkValidatorLiquidShares(expectedLiquidSharesAfterFirstStake, "shares after liquid staking cap hit")

Expand All @@ -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, true)
require.NoError(err, "no error expected after increasing liquid staking cap")
checkValidatorLiquidShares(expectedLiquidSharesAfterSecondStake, "shares after loose liquid stake cap")
}
Expand Down
8 changes: 4 additions & 4 deletions x/staking/keeper/msg_server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down

0 comments on commit 5ff3cce

Please sign in to comment.