Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

bug: lookup and return validators from liquid stake accounting functions #17436

Merged
merged 10 commits into from
Aug 18, 2023
Merged

bug: lookup and return validators from liquid stake accounting functions #17436

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
5a24189
added lookup before validator bond update
sampocs Aug 17, 2023
5d5f8e8
added validator lookup to increase bond function
sampocs Aug 17, 2023
be6377d
returned validator from increase liquid staked tokens
sampocs Aug 17, 2023
87e9146
returned validator from decrease liquid staked shares
sampocs Aug 17, 2023
ce7b75e
updated liquid staked shares functions with val lookup
sampocs Aug 17, 2023
97f6bc9
fixed unit tests
sampocs Aug 17, 2023
b4e2b60
nit
sampocs Aug 17, 2023
abe94d8
fixed unit test
sampocs Aug 17, 2023
e9b5504
added validator lookup to decrease validator bond
sampocs Aug 17, 2023
90002ee
resolved unit tests for decrease val bond
sampocs Aug 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 33 additions & 9 deletions x/staking/keeper/liquid_stake.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,43 +138,67 @@ func (k Keeper) DecreaseTotalLiquidStakedTokens(ctx sdk.Context, amount sdk.Int)
// 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, validator types.Validator, shares sdk.Dec) error {
func (k Keeper) SafelyIncreaseValidatorLiquidShares(ctx sdk.Context, valAddress sdk.ValAddress, shares sdk.Dec) (types.Validator, error) {
validator, found := k.GetValidator(ctx, valAddress)
if !found {
return validator, types.ErrNoValidatorFound
}

// Confirm the validator bond factor and validator liquid staking cap will not be exceeded
if k.CheckExceedsValidatorBondCap(ctx, validator, shares) {
return types.ErrInsufficientValidatorBondShares
return validator, types.ErrInsufficientValidatorBondShares
}
if k.CheckExceedsValidatorLiquidStakingCap(ctx, validator, shares) {
return types.ErrValidatorLiquidStakingCapExceeded
return validator, types.ErrValidatorLiquidStakingCapExceeded
}

// Increment the validator's liquid shares
validator.LiquidShares = validator.LiquidShares.Add(shares)
k.SetValidator(ctx, validator)

return nil
return validator, nil
}

// DecreaseValidatorLiquidShares decrements the liquid shares on a validator
func (k Keeper) DecreaseValidatorLiquidShares(ctx sdk.Context, validator types.Validator, shares sdk.Dec) error {
func (k Keeper) DecreaseValidatorLiquidShares(ctx sdk.Context, valAddress sdk.ValAddress, shares sdk.Dec) (types.Validator, error) {
validator, found := k.GetValidator(ctx, valAddress)
if !found {
return validator, types.ErrNoValidatorFound
}

if shares.GT(validator.LiquidShares) {
return types.ErrValidatorLiquidSharesUnderflow
return validator, types.ErrValidatorLiquidSharesUnderflow
}

validator.LiquidShares = validator.LiquidShares.Sub(shares)
k.SetValidator(ctx, validator)
return nil

return validator, nil
}

// Increase validator bond shares increments the validator's self bond
// in the event that the delegation amount on a validator bond delegation is increased
func (k Keeper) IncreaseValidatorBondShares(ctx sdk.Context, validator types.Validator, shares sdk.Dec) {
func (k Keeper) IncreaseValidatorBondShares(ctx sdk.Context, valAddress sdk.ValAddress, shares sdk.Dec) error {
validator, found := k.GetValidator(ctx, valAddress)
if !found {
return types.ErrNoValidatorFound
}

validator.ValidatorBondShares = validator.ValidatorBondShares.Add(shares)
k.SetValidator(ctx, validator)

return nil
}

// SafelyDecreaseValidatorBond decrements the validator's self bond
// so long as it will not cause the current delegations to exceed the threshold
// set by validator bond factor
func (k Keeper) SafelyDecreaseValidatorBond(ctx sdk.Context, validator types.Validator, shares sdk.Dec) error {
func (k Keeper) SafelyDecreaseValidatorBond(ctx sdk.Context, valAddress sdk.ValAddress, shares sdk.Dec) error {
validator, found := k.GetValidator(ctx, valAddress)
if !found {
return types.ErrNoValidatorFound
}

// Check if the decreased self bond will cause the validator bond threshold to be exceeded
validatorBondFactor := k.ValidatorBondFactor(ctx)
validatorBondEnabled := !validatorBondFactor.Equal(types.ValidatorBondCapDisabled)
Expand Down
22 changes: 11 additions & 11 deletions x/staking/keeper/liquid_stake_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -639,7 +639,7 @@ func TestSafelyIncreaseValidatorLiquidShares(t *testing.T) {
validatorTotalShares := sdk.NewDec(75)

firstIncreaseAmount := sdk.NewDec(20)
secondIncreaseAmount := sdk.NewDec(40)
secondIncreaseAmount := sdk.NewDec(10) // total increase of 30

initialBondFactor := sdk.NewDec(1)
finalBondFactor := sdk.NewDec(10)
Expand All @@ -665,7 +665,7 @@ func TestSafelyIncreaseValidatorLiquidShares(t *testing.T) {

// Attempt to increase the validator liquid shares, it should throw an
// error that the validator bond cap was exceeded
err := app.StakingKeeper.SafelyIncreaseValidatorLiquidShares(ctx, initialValidator, firstIncreaseAmount)
_, err := app.StakingKeeper.SafelyIncreaseValidatorLiquidShares(ctx, valAddress, firstIncreaseAmount)
require.ErrorIs(t, err, types.ErrInsufficientValidatorBondShares)
checkValidatorLiquidShares(initialLiquidShares, "shares after low bond factor")

Expand All @@ -675,12 +675,12 @@ func TestSafelyIncreaseValidatorLiquidShares(t *testing.T) {

// Try the increase again and check that it succeeded
expectedLiquidSharesAfterFirstStake := initialLiquidShares.Add(firstIncreaseAmount)
err = app.StakingKeeper.SafelyIncreaseValidatorLiquidShares(ctx, initialValidator, firstIncreaseAmount)
_, err = app.StakingKeeper.SafelyIncreaseValidatorLiquidShares(ctx, valAddress, firstIncreaseAmount)
require.NoError(t, err)
checkValidatorLiquidShares(expectedLiquidSharesAfterFirstStake, "shares with cap loose bond cap")

// Attempt another increase, it should fail from the liquid staking cap
err = app.StakingKeeper.SafelyIncreaseValidatorLiquidShares(ctx, initialValidator, secondIncreaseAmount)
_, err = app.StakingKeeper.SafelyIncreaseValidatorLiquidShares(ctx, valAddress, secondIncreaseAmount)
require.ErrorIs(t, err, types.ErrValidatorLiquidStakingCapExceeded)
checkValidatorLiquidShares(expectedLiquidSharesAfterFirstStake, "shares after liquid staking cap hit")

Expand All @@ -689,8 +689,8 @@ func TestSafelyIncreaseValidatorLiquidShares(t *testing.T) {
app.StakingKeeper.SetParams(ctx, params)

// Finally confirm that the increase succeeded this time
expectedLiquidSharesAfterSecondStake := initialLiquidShares.Add(secondIncreaseAmount)
err = app.StakingKeeper.SafelyIncreaseValidatorLiquidShares(ctx, initialValidator, secondIncreaseAmount)
expectedLiquidSharesAfterSecondStake := expectedLiquidSharesAfterFirstStake.Add(secondIncreaseAmount)
_, err = app.StakingKeeper.SafelyIncreaseValidatorLiquidShares(ctx, valAddress, secondIncreaseAmount)
require.NoError(t, err, "no error expected after increasing liquid staking cap")
checkValidatorLiquidShares(expectedLiquidSharesAfterSecondStake, "shares after loose liquid stake cap")
}
Expand All @@ -714,15 +714,15 @@ func TestDecreaseValidatorLiquidShares(t *testing.T) {
app.StakingKeeper.SetValidator(ctx, initialValidator)

// Decrease the validator liquid shares, and confirm the new share amount has been updated
err := app.StakingKeeper.DecreaseValidatorLiquidShares(ctx, initialValidator, decreaseAmount)
_, err := app.StakingKeeper.DecreaseValidatorLiquidShares(ctx, valAddress, decreaseAmount)
require.NoError(t, err, "no error expected when decreasing validator liquid shares")

actualValidator, found := app.StakingKeeper.GetValidator(ctx, valAddress)
require.True(t, found)
require.Equal(t, initialLiquidShares.Sub(decreaseAmount), actualValidator.LiquidShares, "liquid shares")

// Attempt to decrease by a larger amount than it has, it should fail
err = app.StakingKeeper.DecreaseValidatorLiquidShares(ctx, actualValidator, initialLiquidShares)
_, err = app.StakingKeeper.DecreaseValidatorLiquidShares(ctx, valAddress, initialLiquidShares)
require.ErrorIs(t, err, types.ErrValidatorLiquidSharesUnderflow)
}

Expand Down Expand Up @@ -758,7 +758,7 @@ func TestSafelyDecreaseValidatorBond(t *testing.T) {
// from (100 * 10 = 1000) to (100 * 5 = 500)
// Since this is still above the initial liquid shares of 200, this will succeed
decreaseAmount, expectedBondShares := sdk.NewDec(5), sdk.NewDec(5)
err := app.StakingKeeper.SafelyDecreaseValidatorBond(ctx, initialValidator, decreaseAmount)
err := app.StakingKeeper.SafelyDecreaseValidatorBond(ctx, valAddress, decreaseAmount)
require.NoError(t, err)

actualValidator, found := app.StakingKeeper.GetValidator(ctx, valAddress)
Expand All @@ -769,15 +769,15 @@ func TestSafelyDecreaseValidatorBond(t *testing.T) {
// This time, the cap will be reduced to (factor * shares) = (100 * 1) = 100
// However, the liquid shares are currently 200, so this should fail
decreaseAmount, expectedBondShares = sdk.NewDec(4), sdk.NewDec(1)
err = app.StakingKeeper.SafelyDecreaseValidatorBond(ctx, actualValidator, decreaseAmount)
err = app.StakingKeeper.SafelyDecreaseValidatorBond(ctx, valAddress, decreaseAmount)
require.ErrorIs(t, err, types.ErrInsufficientValidatorBondShares)

// Finally, disable the cap and attempt to decrease again
// This time it should succeed
params.ValidatorBondFactor = types.ValidatorBondCapDisabled
app.StakingKeeper.SetParams(ctx, params)

err = app.StakingKeeper.SafelyDecreaseValidatorBond(ctx, actualValidator, decreaseAmount)
err = app.StakingKeeper.SafelyDecreaseValidatorBond(ctx, valAddress, decreaseAmount)
require.NoError(t, err)

actualValidator, found = app.StakingKeeper.GetValidator(ctx, valAddress)
Expand Down
86 changes: 24 additions & 62 deletions x/staking/keeper/msg_server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,15 +215,10 @@ func (k msgServer) Delegate(goCtx context.Context, msg *types.MsgDelegate) (*typ
if err := k.SafelyIncreaseTotalLiquidStakedTokens(ctx, tokens, false); err != nil {
return nil, err
}
if err := k.SafelyIncreaseValidatorLiquidShares(ctx, validator, shares); err != nil {
validator, err = k.SafelyIncreaseValidatorLiquidShares(ctx, valAddr, shares)
if err != nil {
return nil, err
}
// Note: this is required for downstream uses of the validator variable
// since the validator's liquid shares were updated above
validator, found = k.GetValidator(ctx, valAddr)
if !found {
return nil, types.ErrNoValidatorFound
}
}

// NOTE: source funds are always unbonded
Expand All @@ -238,7 +233,9 @@ func (k msgServer) Delegate(goCtx context.Context, msg *types.MsgDelegate) (*typ
return nil, types.ErrNoDelegation
}
if delegation.ValidatorBond {
k.IncreaseValidatorBondShares(ctx, validator, newShares)
if err := k.IncreaseValidatorBondShares(ctx, valAddr, newShares); err != nil {
return nil, err
}
}

if tokens.IsInt64() {
Expand Down Expand Up @@ -282,7 +279,7 @@ func (k msgServer) BeginRedelegate(goCtx context.Context, msg *types.MsgBeginRed
return nil, err
}

srcValidator, found := k.GetValidator(ctx, valSrcAddr)
_, found := k.GetValidator(ctx, valSrcAddr)
if !found {
return nil, types.ErrNoValidatorFound
}
Expand Down Expand Up @@ -313,15 +310,9 @@ func (k msgServer) BeginRedelegate(goCtx context.Context, msg *types.MsgBeginRed
// If this is a validator self-bond, the new liquid delegation cannot fall below the self-bond * bond factor
// The delegation on the new validator will not a validator bond
if srcDelegation.ValidatorBond {
if err := k.SafelyDecreaseValidatorBond(ctx, srcValidator, srcShares); err != nil {
if err := k.SafelyDecreaseValidatorBond(ctx, valSrcAddr, srcShares); err != nil {
return nil, err
}
// Note: this is required for downstream uses of the srcValidator variable
// since the validator bond shares were updated above
srcValidator, found = k.GetValidator(ctx, valSrcAddr)
if !found {
return nil, types.ErrNoValidatorFound
}
}

// If this delegation from a liquid staker, the delegation on the new validator
Expand All @@ -332,18 +323,12 @@ func (k msgServer) BeginRedelegate(goCtx context.Context, msg *types.MsgBeginRed
if err != nil {
return nil, err
}
if err := k.SafelyIncreaseValidatorLiquidShares(ctx, dstValidator, dstShares); err != nil {
if _, err := k.SafelyIncreaseValidatorLiquidShares(ctx, valDstAddr, dstShares); err != nil {
return nil, err
}
if err := k.DecreaseValidatorLiquidShares(ctx, srcValidator, srcShares); err != nil {
if _, err := k.DecreaseValidatorLiquidShares(ctx, valSrcAddr, srcShares); err != nil {
return nil, err
}
// Note: this is required for downstream uses of each validator variable
// since the liquid shares were updated above
srcValidator, found = k.GetValidator(ctx, valSrcAddr)
if !found {
return nil, types.ErrNoValidatorFound
}
}

bondDenom := k.BondDenom(ctx)
Expand All @@ -370,7 +355,9 @@ func (k msgServer) BeginRedelegate(goCtx context.Context, msg *types.MsgBeginRed
if err != nil {
return nil, err
}
k.IncreaseValidatorBondShares(ctx, dstValidator, dstShares)
if err := k.IncreaseValidatorBondShares(ctx, valDstAddr, dstShares); err != nil {
return nil, err
}
}

if msg.Amount.Amount.IsInt64() {
Expand Down Expand Up @@ -425,7 +412,7 @@ func (k msgServer) Undelegate(goCtx context.Context, msg *types.MsgUndelegate) (
return nil, err
}

validator, found := k.GetValidator(ctx, addr)
_, found := k.GetValidator(ctx, addr)
if !found {
return nil, types.ErrNoValidatorFound
}
Expand All @@ -441,15 +428,9 @@ func (k msgServer) Undelegate(goCtx context.Context, msg *types.MsgUndelegate) (

// if this is a validator self-bond, the new liquid delegation cannot fall below the self-bond * bond factor
if delegation.ValidatorBond {
if err := k.SafelyDecreaseValidatorBond(ctx, validator, shares); err != nil {
if err := k.SafelyDecreaseValidatorBond(ctx, addr, shares); err != nil {
return nil, err
}
// Note: this is required for downstream uses of the validator variable
// since the validator bond shares was updated above
validator, found = k.GetValidator(ctx, addr)
if !found {
return nil, types.ErrNoValidatorFound
}
}

// if this delegation is from a liquid staking provider (identified if the delegator
Expand All @@ -458,15 +439,9 @@ func (k msgServer) Undelegate(goCtx context.Context, msg *types.MsgUndelegate) (
if err := k.DecreaseTotalLiquidStakedTokens(ctx, tokens); err != nil {
return nil, err
}
if err := k.DecreaseValidatorLiquidShares(ctx, validator, shares); err != nil {
if _, err := k.DecreaseValidatorLiquidShares(ctx, addr, shares); err != nil {
return nil, err
}
// Note: this is required for downstream uses of the validator variable
// since the liquid shares were updated above
validator, found = k.GetValidator(ctx, addr)
if !found {
return nil, types.ErrNoValidatorFound
}
}

bondDenom := k.BondDenom(ctx)
Expand Down Expand Up @@ -574,15 +549,10 @@ func (k msgServer) CancelUnbondingDelegation(goCtx context.Context, msg *types.M
if err := k.SafelyIncreaseTotalLiquidStakedTokens(ctx, tokens, false); err != nil {
return nil, err
}
if err := k.SafelyIncreaseValidatorLiquidShares(ctx, validator, shares); err != nil {
validator, err = k.SafelyIncreaseValidatorLiquidShares(ctx, valAddr, shares)
if err != nil {
return nil, err
}
// Note: this is required for downstream uses of the validator variable
// since the validator's liquid shares were updated above
validator, found = k.GetValidator(ctx, valAddr)
if !found {
return nil, types.ErrNoValidatorFound
}
}

var (
Expand Down Expand Up @@ -621,7 +591,9 @@ func (k msgServer) CancelUnbondingDelegation(goCtx context.Context, msg *types.M
return nil, types.ErrNoDelegation
}
if delegation.ValidatorBond {
k.IncreaseValidatorBondShares(ctx, validator, newShares)
if err := k.IncreaseValidatorBondShares(ctx, valAddr, newShares); err != nil {
return nil, err
}
}

amount := unbondEntry.Balance.Sub(msg.Amount.Amount)
Expand Down Expand Up @@ -746,15 +718,10 @@ func (k msgServer) TokenizeShares(goCtx context.Context, msg *types.MsgTokenizeS
if err := k.SafelyIncreaseTotalLiquidStakedTokens(ctx, msg.Amount.Amount, true); err != nil {
return nil, err
}
if err := k.SafelyIncreaseValidatorLiquidShares(ctx, validator, shares); err != nil {
validator, err = k.SafelyIncreaseValidatorLiquidShares(ctx, valAddr, shares)
if err != nil {
return nil, err
}
// Note: this is required for downstream uses of the validator variable
// since the validator's liquid shares were updated above
validator, found = k.GetValidator(ctx, valAddr)
if !found {
return nil, types.ErrNoValidatorFound
}
}

recordID := k.GetLastTokenizeShareRecordID(ctx) + 1
Expand Down Expand Up @@ -894,15 +861,10 @@ func (k msgServer) RedeemTokensForShares(goCtx context.Context, msg *types.MsgRe
if err := k.DecreaseTotalLiquidStakedTokens(ctx, tokens); err != nil {
return nil, err
}
if err := k.DecreaseValidatorLiquidShares(ctx, validator, shares); err != nil {
validator, err = k.DecreaseValidatorLiquidShares(ctx, valAddr, shares)
if err != nil {
return nil, err
}
// Note: this is required for downstream uses of the validator variable
// since the liquid shares were updated above
validator, found = k.GetValidator(ctx, valAddr)
if !found {
return nil, types.ErrNoValidatorFound
}
}

returnAmount, err := k.Unbond(ctx, record.GetModuleAddress(), valAddr, shares)
Expand Down
Loading