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.

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

fix: add validator cap and bond checks when creating the delegation strategy #810

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

### Bug Fixes

- [810](https://github.com/persistenceOne/pstake-native/pull/810) Add validator cap and bond checks when creating the delegation strategy
- [792](https://github.com/persistenceOne/pstake-native/pull/792) Use GetHostChainFromHostDenom in ICA Transfer
unsuccessfulAck instead of GetHostChainFromDelegatorAddress as Rewards account too uses ICA Transfer to autocompound
- [795](https://github.com/persistenceOne/pstake-native/pull/795) Reject zero weight validator LSM shares for
Expand Down
2 changes: 1 addition & 1 deletion x/liquidstake/keeper/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func (s *KeeperTestSuite) TestImportExportGenesis() {
k.SetParams(ctx, params)
k.UpdateLiquidValidatorSet(ctx)

stakingAmt := math.NewInt(100000000)
stakingAmt := math.NewInt(100000)
s.Require().NoError(s.liquidStaking(s.delAddrs[0], stakingAmt))
lvs := k.GetAllLiquidValidators(ctx)
s.Require().Len(lvs, 2)
Expand Down
6 changes: 5 additions & 1 deletion x/liquidstake/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,11 @@ func (s *KeeperTestSuite) CreateValidators(powers []int64) ([]sdk.AccAddress, []
valAddrs := testhelpers.ConvertAddrsToValAddrs(addrs)
pks := testhelpers.CreateTestPubKeys(num)
skParams := s.app.StakingKeeper.GetParams(s.ctx)
skParams.ValidatorLiquidStakingCap = sdk.OneDec()
globalCap, _ := sdk.NewDecFromStr("0.1")
skParams.GlobalLiquidStakingCap = globalCap
validatorCap, _ := sdk.NewDecFromStr("0.5")
skParams.ValidatorLiquidStakingCap = validatorCap
skParams.ValidatorBondFactor = sdk.NewDec(250)
s.app.StakingKeeper.SetParams(s.ctx, skParams)
for i, power := range powers {
val, err := stakingtypes.NewValidator(valAddrs[i], pks[i], stakingtypes.Description{})
Expand Down
53 changes: 51 additions & 2 deletions x/liquidstake/keeper/liquidstake.go
Original file line number Diff line number Diff line change
Expand Up @@ -548,8 +548,9 @@ func (k Keeper) LSMDelegate(

// LiquidDelegate delegates staking amount to active validators by proxy account.
func (k Keeper) LiquidDelegate(ctx sdk.Context, proxyAcc sdk.AccAddress, activeVals types.ActiveLiquidValidators, stakingAmt math.Int, whitelistedValsMap types.WhitelistedValsMap) (err error) {
// crumb may occur due to a decimal point error in dividing the staking amount into the weight of liquid validators, It added on first active liquid validator
weightedAmt, crumb := types.DivideByWeight(activeVals, stakingAmt, whitelistedValsMap)
// crumb may occur due to a decimal point error in dividing the staking amount into the weight of liquid validators
// it is added to the first active liquid validator
weightedAmt, crumb := k.DivideByWeight(ctx, activeVals, stakingAmt, whitelistedValsMap)
if len(weightedAmt) == 0 {
return types.ErrInvalidActiveLiquidValidators
}
Expand Down Expand Up @@ -719,6 +720,54 @@ func (k Keeper) LiquidUnbond(
return completionTime, returnAmount, ubd, nil
}

// DivideByWeight divide the input value by the ratio of the param weight of the liquid validator and return it with crumb
// which is may occur while dividing according to the weight of active liquid validators by decimal error.
func (k Keeper) DivideByWeight(
kruspy marked this conversation as resolved.
Show resolved Hide resolved
ctx sdk.Context,
avs types.ActiveLiquidValidators,
input math.Int,
whitelistedValsMap types.WhitelistedValsMap,
) (outputs []math.Int, crumb math.Int) {
totalWeight := avs.TotalWeight(whitelistedValsMap)
if !totalWeight.IsPositive() {
return []math.Int{}, sdk.ZeroInt()
}

totalOutput := sdk.ZeroInt()
unitInput := math.LegacyNewDecFromInt(input).QuoTruncate(math.LegacyNewDecFromInt(totalWeight))
for _, val := range avs {
validator, _ := k.stakingKeeper.GetValidator(ctx, val.GetOperator())

// calculate the shares the input would receive
output := unitInput.MulInt(val.GetWeight(whitelistedValsMap, true)).TruncateInt()
outputShares := validator.GetDelegatorShares().MulInt(output).QuoInt(validator.GetTokens())

// just delegate if the validator does not exceed any of the validator specific caps
if !k.stakingKeeper.CheckExceedsValidatorBondCap(ctx, validator, outputShares) &&
!k.stakingKeeper.CheckExceedsValidatorLiquidStakingCap(ctx, validator, outputShares, false) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here if it still has some space, we should try to fill everything up

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what do you mean ?

totalOutput = totalOutput.Add(output)
outputs = append(outputs, output)
}
}

if len(outputs) == 0 {
return []math.Int{}, sdk.ZeroInt()
}

// redistribute crumb evenly to the other outputs if there is enough
numOutputs := sdk.NewInt(int64(len(outputs)))
totalCrumb := input.Sub(totalOutput)
kruspy marked this conversation as resolved.
Show resolved Hide resolved
crumbPerOutput := totalCrumb.Quo(numOutputs)
if totalCrumb.GTE(numOutputs) {
for i := range outputs {
totalOutput = totalOutput.Add(crumbPerOutput)
outputs[i] = outputs[i].Add(crumbPerOutput)
}
}

return outputs, input.Sub(totalOutput)
}

// PrioritiseInactiveLiquidValidators sorts LiquidValidators array to have inactive validators first. Used for the case when
// unbonding should begin from the inactive validators first.
func (k Keeper) PrioritiseInactiveLiquidValidators(
Expand Down
151 changes: 149 additions & 2 deletions x/liquidstake/keeper/liquidstake_test.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
package keeper_test

import (
"testing"
"time"

"cosmossdk.io/math"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
"github.com/stretchr/testify/require"

testhelpers "github.com/persistenceOne/pstake-native/v2/app/helpers"
"github.com/persistenceOne/pstake-native/v2/x/liquidstake/types"
Expand Down Expand Up @@ -243,7 +245,7 @@ func (s *KeeperTestSuite) TestLiquidStake() {
}

func (s *KeeperTestSuite) TestLiquidStakeFromVestingAccount() {
_, valOpers, _ := s.CreateValidators([]int64{1000000, 2000000, 3000000})
_, valOpers, _ := s.CreateValidators([]int64{1000000000, 2000000000, 3000000000})
params := s.keeper.GetParams(s.ctx)

// add active validator
Expand Down Expand Up @@ -312,6 +314,11 @@ func (s *KeeperTestSuite) TestLiquidStakeEdgeCases() {
s.Require().ErrorIs(err, types.ErrInvalidBondDenom)

// liquid stake, unstaking with huge amount
stakingParams := s.app.StakingKeeper.GetParams(s.ctx)
stakingParams.GlobalLiquidStakingCap = sdk.OneDec()
stakingParams.ValidatorLiquidStakingCap = sdk.OneDec()
stakingParams.ValidatorBondFactor = sdk.NewDec(10000000000000)
s.app.StakingKeeper.SetParams(s.ctx, stakingParams)
hugeAmt := math.NewInt(1_000_000_000_000_000_000)
s.fundAddr(s.delAddrs[0], sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, hugeAmt.MulRaw(2))))
s.Require().NoError(s.liquidStaking(s.delAddrs[0], hugeAmt))
Expand All @@ -334,7 +341,7 @@ func (s *KeeperTestSuite) TestLiquidUnstakeEdgeCases() {
_, valOpers, _ := s.CreateValidators([]int64{1000000, 2000000, 3000000})
params := s.keeper.GetParams(s.ctx)
s.keeper.UpdateLiquidValidatorSet(s.ctx)
stakingAmt := math.NewInt(5000000)
stakingAmt := math.NewInt(100000)

// add active validator
params.WhitelistedValidators = []types.WhitelistedValidator{
Expand Down Expand Up @@ -458,3 +465,143 @@ func (s *KeeperTestSuite) TestShareInflation() {
attackerProfit := unbondingAmt.Sub(initialStakingAmt).Sub(attackerTransferAmount)
s.Require().LessOrEqual(attackerProfit.Int64(), math.ZeroInt().Int64())
}

func (s *KeeperTestSuite) TestDivideByWeight() {
_, valOpers, _ := s.CreateValidators([]int64{2000000, 2000000, 2000000})

testCases := []struct {
name string
whitelistedVals []types.WhitelistedValidator
addStakingAmt math.Int
expectedOutputs []math.Int
expectedCrumb math.Int
}{
{
name: "Success with crumbs",
whitelistedVals: []types.WhitelistedValidator{
{
ValidatorAddress: valOpers[0].String(),
TargetWeight: math.NewInt(1),
},
{
ValidatorAddress: valOpers[1].String(),
TargetWeight: math.NewInt(1),
},
{
ValidatorAddress: valOpers[2].String(),
TargetWeight: math.NewInt(1),
},
},
addStakingAmt: math.NewInt(100000),
expectedOutputs: []math.Int{math.NewInt(33333), math.NewInt(33333), math.NewInt(33333)},
expectedCrumb: math.NewInt(1),
},
{
name: "Success without crumb",
whitelistedVals: []types.WhitelistedValidator{
{
ValidatorAddress: valOpers[0].String(),
TargetWeight: math.NewInt(2),
},
{
ValidatorAddress: valOpers[1].String(),
TargetWeight: math.NewInt(2),
},
{
ValidatorAddress: valOpers[2].String(),
TargetWeight: math.NewInt(1),
},
},
addStakingAmt: math.NewInt(100000),
expectedOutputs: []math.Int{math.NewInt(40000), math.NewInt(40000), math.NewInt(20000)},
expectedCrumb: math.NewInt(0),
},
{
name: "First validator reaches the cap, part of the crumb gets divided among validators",
whitelistedVals: []types.WhitelistedValidator{
{
ValidatorAddress: valOpers[0].String(),
TargetWeight: math.NewInt(8),
},
{
ValidatorAddress: valOpers[1].String(),
TargetWeight: math.NewInt(1),
},
{
ValidatorAddress: valOpers[2].String(),
TargetWeight: math.NewInt(1),
},
},
addStakingAmt: math.NewInt(2500003),
expectedOutputs: []math.Int{math.NewInt(1250001), math.NewInt(1250001)},
expectedCrumb: math.NewInt(1),
},
{
name: "First validator reaches the cap, all the crumb gets divided among validators",
whitelistedVals: []types.WhitelistedValidator{
{
ValidatorAddress: valOpers[0].String(),
TargetWeight: math.NewInt(8),
},
{
ValidatorAddress: valOpers[1].String(),
TargetWeight: math.NewInt(1),
},
{
ValidatorAddress: valOpers[2].String(),
TargetWeight: math.NewInt(1),
},
},
addStakingAmt: math.NewInt(2500002),
expectedOutputs: []math.Int{math.NewInt(1250001), math.NewInt(1250001)},
expectedCrumb: math.NewInt(0),
},
{
name: "All validators reach the cap",
whitelistedVals: []types.WhitelistedValidator{
{
ValidatorAddress: valOpers[0].String(),
TargetWeight: math.NewInt(1),
},
{
ValidatorAddress: valOpers[1].String(),
TargetWeight: math.NewInt(1),
},
{
ValidatorAddress: valOpers[2].String(),
TargetWeight: math.NewInt(1),
},
},
addStakingAmt: math.NewInt(1000000000),
expectedOutputs: []math.Int{},
expectedCrumb: math.NewInt(0),
},
}

for _, tc := range testCases {
s.T().Run(tc.name, func(t *testing.T) {
require.IsType(t, []types.WhitelistedValidator{}, tc.whitelistedVals)
require.IsType(t, math.Int{}, tc.addStakingAmt)
require.IsType(t, math.Int{}, tc.expectedCrumb)
require.IsType(t, []math.Int{}, tc.expectedOutputs)

totalTargetAmt := sdk.ZeroInt()
valsMap := types.GetWhitelistedValsMap(tc.whitelistedVals)
var activeVals types.ActiveLiquidValidators
for _, v := range tc.whitelistedVals {
activeVals = append(activeVals, types.LiquidValidator{
OperatorAddress: v.ValidatorAddress,
})
}
outputs, crumb := s.keeper.DivideByWeight(s.ctx, activeVals, tc.addStakingAmt, valsMap)
for _, v := range outputs {
totalTargetAmt = totalTargetAmt.Add(v)
}
require.EqualValues(t, tc.expectedOutputs, outputs)
require.Equal(t, tc.expectedCrumb.String(), crumb.String())
if !(len(outputs) == 0) && !crumb.IsZero() {
require.EqualValues(t, tc.addStakingAmt, totalTargetAmt.Add(crumb))
}
})
}
}
16 changes: 8 additions & 8 deletions x/liquidstake/keeper/rebalancing_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ func (s *KeeperTestSuite) TestRebalancingCase1() {
proxyAccDel3, found := s.app.StakingKeeper.GetDelegation(s.ctx, types.LiquidStakeProxyAcc, valOpers[2])
s.Require().True(found)

s.Require().EqualValues(proxyAccDel1.Shares.TruncateInt(), math.NewInt(16668))
s.Require().EqualValues(proxyAccDel2.Shares.TruncateInt(), math.NewInt(16665))
s.Require().EqualValues(proxyAccDel3.Shares.TruncateInt(), math.NewInt(16665))
s.Require().EqualValues(proxyAccDel1.Shares.TruncateInt(), math.NewInt(16666))
s.Require().EqualValues(proxyAccDel2.Shares.TruncateInt(), math.NewInt(16666))
s.Require().EqualValues(proxyAccDel3.Shares.TruncateInt(), math.NewInt(16666))
totalLiquidTokens, _ := s.keeper.GetAllLiquidValidators(s.ctx).TotalLiquidTokens(s.ctx, s.app.StakingKeeper, false)
s.Require().EqualValues(stakingAmt, totalLiquidTokens)
s.printRedelegationsLiquidTokens()
Expand Down Expand Up @@ -269,7 +269,7 @@ func (s *KeeperTestSuite) TestRebalancingConsecutiveCase() {
s.keeper.SetParams(s.ctx, params)
s.keeper.UpdateLiquidValidatorSet(s.ctx)

stakingAmt := math.NewInt(10000000000000)
stakingAmt := math.NewInt(1000000000000)
s.fundAddr(s.delAddrs[0], sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, stakingAmt)))
// add active validator
params.WhitelistedValidators = []types.WhitelistedValidator{
Expand Down Expand Up @@ -452,7 +452,7 @@ func (s *KeeperTestSuite) TestRebalancingConsecutiveCase() {
}

func (s *KeeperTestSuite) TestAutocompoundStakingRewards() {
_, valOpers, _ := s.CreateValidators([]int64{2000000, 2000000, 2000000})
_, valOpers, _ := s.CreateValidators([]int64{1000000000, 1000000000, 1000000000})
params := s.keeper.GetParams(s.ctx)

params.WhitelistedValidators = []types.WhitelistedValidator{
Expand Down Expand Up @@ -507,7 +507,7 @@ func (s *KeeperTestSuite) TestLimitAutocompoundStakingRewards() {
s.keeper.SetParams(s.ctx, params)
s.keeper.UpdateLiquidValidatorSet(s.ctx)

stakingAmt := math.NewInt(100000000)
stakingAmt := math.NewInt(100000)
s.Require().NoError(s.liquidStaking(s.delAddrs[0], stakingAmt))

// allocate rewards
Expand Down Expand Up @@ -541,7 +541,7 @@ func (s *KeeperTestSuite) TestRemoveAllLiquidValidator() {
s.Require().NoError(s.keeper.SetParams(s.ctx, params))
s.keeper.UpdateLiquidValidatorSet(s.ctx)

stakingAmt := math.NewInt(100000000)
stakingAmt := math.NewInt(100000)
s.Require().NoError(s.liquidStaking(s.delAddrs[0], stakingAmt))

// allocate rewards
Expand Down Expand Up @@ -587,7 +587,7 @@ func (s *KeeperTestSuite) TestUndelegatedFundsNotBecomeFees() {
s.keeper.UpdateLiquidValidatorSet(s.ctx)

// stake funds
stakingAmt := math.NewInt(100000000)
stakingAmt := math.NewInt(100000)
s.Require().NoError(s.liquidStaking(s.delAddrs[0], stakingAmt))

// remove one validator
Expand Down
2 changes: 2 additions & 0 deletions x/liquidstake/types/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ type StakingKeeper interface {
SafelyIncreaseTotalLiquidStakedTokens(ctx sdk.Context, amount math.Int, sharesAlreadyBonded bool) error
DecreaseTotalLiquidStakedTokens(ctx sdk.Context, amount math.Int) error
GetBondedPool(ctx sdk.Context) (bondedPool authtypes.ModuleAccountI)
CheckExceedsValidatorBondCap(ctx sdk.Context, validator stakingtypes.Validator, shares sdk.Dec) bool
CheckExceedsValidatorLiquidStakingCap(ctx sdk.Context, validator stakingtypes.Validator, shares sdk.Dec, sharesAlreadyBonded bool) bool
}

// MintKeeper expected minting keeper (noalias)
Expand Down
19 changes: 0 additions & 19 deletions x/liquidstake/types/rebalancing.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,25 +14,6 @@ type Redelegation struct {
Error error
}

// DivideByWeight divide the input value by the ratio of the param weight of the liquid validator and return it with crumb
// which is may occur while dividing according to the weight of active liquid validators by decimal error.
func DivideByWeight(avs ActiveLiquidValidators, input math.Int, whitelistedValsMap WhitelistedValsMap) (outputs []math.Int, crumb math.Int) {
totalWeight := avs.TotalWeight(whitelistedValsMap)
if !totalWeight.IsPositive() {
return []math.Int{}, sdk.ZeroInt()
}

totalOutput := sdk.ZeroInt()
unitInput := math.LegacyNewDecFromInt(input).QuoTruncate(math.LegacyNewDecFromInt(totalWeight))
for _, val := range avs {
output := unitInput.MulInt(val.GetWeight(whitelistedValsMap, true)).TruncateInt()
totalOutput = totalOutput.Add(output)
outputs = append(outputs, output)
}

return outputs, input.Sub(totalOutput)
}

// DivideByCurrentWeight divide the input value by the ratio of the weight of the liquid validator's liquid token and return it with crumb
// which is may occur while dividing according to the weight of liquid validators by decimal error, outputs is truncated decimal.
func DivideByCurrentWeight(lvs LiquidValidators, input math.LegacyDec, totalLiquidTokens math.Int, liquidTokenMap map[string]math.Int) (outputs []math.LegacyDec, crumb math.LegacyDec) {
Expand Down
Loading
Loading