From 060360a52c46f2474dc6d2bd2f074a18b8b7d742 Mon Sep 17 00:00:00 2001 From: kruspy Date: Mon, 8 Apr 2024 19:28:06 +0200 Subject: [PATCH 1/3] add validator cap and bond checks + tests --- x/liquidstake/keeper/genesis_test.go | 2 +- x/liquidstake/keeper/keeper_test.go | 6 +- x/liquidstake/keeper/liquidstake.go | 53 +++++++- x/liquidstake/keeper/liquidstake_test.go | 151 ++++++++++++++++++++++- x/liquidstake/keeper/rebalancing_test.go | 16 +-- x/liquidstake/types/expected_keepers.go | 2 + x/liquidstake/types/rebalancing.go | 19 --- x/liquidstake/types/rebalancing_test.go | 94 -------------- 8 files changed, 216 insertions(+), 127 deletions(-) diff --git a/x/liquidstake/keeper/genesis_test.go b/x/liquidstake/keeper/genesis_test.go index bb2e90ba5..17d26e7f2 100644 --- a/x/liquidstake/keeper/genesis_test.go +++ b/x/liquidstake/keeper/genesis_test.go @@ -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) diff --git a/x/liquidstake/keeper/keeper_test.go b/x/liquidstake/keeper/keeper_test.go index cfd1da6cd..88745f1be 100644 --- a/x/liquidstake/keeper/keeper_test.go +++ b/x/liquidstake/keeper/keeper_test.go @@ -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{}) diff --git a/x/liquidstake/keeper/liquidstake.go b/x/liquidstake/keeper/liquidstake.go index 9d22da3ff..eb9f9d329 100644 --- a/x/liquidstake/keeper/liquidstake.go +++ b/x/liquidstake/keeper/liquidstake.go @@ -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 } @@ -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( + 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) { + 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) + 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( diff --git a/x/liquidstake/keeper/liquidstake_test.go b/x/liquidstake/keeper/liquidstake_test.go index 0da3ef7d7..2b27eeb49 100644 --- a/x/liquidstake/keeper/liquidstake_test.go +++ b/x/liquidstake/keeper/liquidstake_test.go @@ -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" @@ -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 @@ -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)) @@ -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{ @@ -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)) + } + }) + } +} diff --git a/x/liquidstake/keeper/rebalancing_test.go b/x/liquidstake/keeper/rebalancing_test.go index 20848155b..0ec98d858 100644 --- a/x/liquidstake/keeper/rebalancing_test.go +++ b/x/liquidstake/keeper/rebalancing_test.go @@ -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() @@ -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{ @@ -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{ @@ -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 @@ -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 @@ -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 diff --git a/x/liquidstake/types/expected_keepers.go b/x/liquidstake/types/expected_keepers.go index a15a80414..bab2e961f 100644 --- a/x/liquidstake/types/expected_keepers.go +++ b/x/liquidstake/types/expected_keepers.go @@ -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) diff --git a/x/liquidstake/types/rebalancing.go b/x/liquidstake/types/rebalancing.go index 8e1c214a9..368a5d84d 100644 --- a/x/liquidstake/types/rebalancing.go +++ b/x/liquidstake/types/rebalancing.go @@ -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) { diff --git a/x/liquidstake/types/rebalancing_test.go b/x/liquidstake/types/rebalancing_test.go index da6d085c8..2e2c71af2 100644 --- a/x/liquidstake/types/rebalancing_test.go +++ b/x/liquidstake/types/rebalancing_test.go @@ -25,100 +25,6 @@ var liquidValidators = []types.LiquidValidator{ }, } -func TestDivideByWeight(t *testing.T) { - testCases := []struct { - whitelistedVals []types.WhitelistedValidator - addStakingAmt math.Int - currentDelShares []math.Int - expectedOutputs []math.Int - expectedCrumb math.Int - }{ - { - whitelistedVals: []types.WhitelistedValidator{ - { - ValidatorAddress: liquidValidators[0].OperatorAddress, - TargetWeight: math.NewInt(1), - }, - { - ValidatorAddress: liquidValidators[1].OperatorAddress, - TargetWeight: math.NewInt(1), - }, - { - ValidatorAddress: liquidValidators[2].OperatorAddress, - TargetWeight: math.NewInt(1), - }, - }, - addStakingAmt: math.NewInt(10 * 1000000), - currentDelShares: []math.Int{math.NewInt(2000000), math.NewInt(2000000), math.NewInt(1000000)}, - expectedOutputs: []math.Int{math.NewInt(3333333), math.NewInt(3333333), math.NewInt(3333333)}, - expectedCrumb: math.NewInt(1), - }, - { - whitelistedVals: []types.WhitelistedValidator{ - { - ValidatorAddress: liquidValidators[0].OperatorAddress, - TargetWeight: math.NewInt(2), - }, - { - ValidatorAddress: liquidValidators[1].OperatorAddress, - TargetWeight: math.NewInt(2), - }, - { - ValidatorAddress: liquidValidators[2].OperatorAddress, - TargetWeight: math.NewInt(1), - }, - }, - addStakingAmt: math.NewInt(10 * 1000000), - currentDelShares: []math.Int{math.NewInt(1000000), math.NewInt(1000000), math.NewInt(1000000)}, - expectedOutputs: []math.Int{math.NewInt(4000000), math.NewInt(4000000), math.NewInt(2000000)}, - expectedCrumb: math.NewInt(0), - }, - { - whitelistedVals: []types.WhitelistedValidator{ - { - ValidatorAddress: liquidValidators[0].OperatorAddress, - TargetWeight: math.NewInt(1), - }, - { - ValidatorAddress: liquidValidators[1].OperatorAddress, - TargetWeight: math.NewInt(1), - }, - { - ValidatorAddress: liquidValidators[2].OperatorAddress, - TargetWeight: math.NewInt(1), - }, - }, - addStakingAmt: math.NewInt(10), - currentDelShares: []math.Int{math.NewInt(3), math.NewInt(2), math.NewInt(1)}, - expectedOutputs: []math.Int{math.NewInt(3), math.NewInt(3), math.NewInt(3)}, - expectedCrumb: math.NewInt(1), - }, - } - - for _, tc := range testCases { - 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 := types.DivideByWeight(activeVals, tc.addStakingAmt, valsMap) - for _, v := range outputs { - totalTargetAmt = totalTargetAmt.Add(v) - } - require.EqualValues(t, tc.expectedOutputs, outputs) - require.EqualValues(t, tc.addStakingAmt, totalTargetAmt.Add(crumb)) - require.Equal(t, tc.expectedCrumb.String(), crumb.String()) - } -} - func TestMinMaxGap(t *testing.T) { testCases := []struct { name string From 0328044fcc2b2175fa44fbb73322c9abfd6ab70a Mon Sep 17 00:00:00 2001 From: kruspy Date: Mon, 8 Apr 2024 19:33:30 +0200 Subject: [PATCH 2/3] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 068c4ee1d..a82e800e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 From e48cd0ba288d9a1627a01f9558dadf7a0dee83ed Mon Sep 17 00:00:00 2001 From: kruspy Date: Tue, 9 Apr 2024 11:26:40 +0200 Subject: [PATCH 3/3] divide leftover correctly + adjust tests --- x/liquidstake/keeper/liquidstake.go | 103 ++++++++++++++-------- x/liquidstake/keeper/liquidstake_test.go | 106 ++++++++++++++--------- x/liquidstake/keeper/rebalancing_test.go | 4 +- 3 files changed, 132 insertions(+), 81 deletions(-) diff --git a/x/liquidstake/keeper/liquidstake.go b/x/liquidstake/keeper/liquidstake.go index eb9f9d329..81c53260f 100644 --- a/x/liquidstake/keeper/liquidstake.go +++ b/x/liquidstake/keeper/liquidstake.go @@ -547,24 +547,27 @@ 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 is added to the first active liquid validator - weightedAmt, crumb := k.DivideByWeight(ctx, activeVals, stakingAmt, whitelistedValsMap) - if len(weightedAmt) == 0 { +func (k Keeper) LiquidDelegate( + ctx sdk.Context, + proxyAcc sdk.AccAddress, + activeVals types.ActiveLiquidValidators, + stakingAmt math.Int, + whitelistedValsMap types.WhitelistedValsMap, +) (err error) { + delegations := k.DivideByWeight(ctx, activeVals, stakingAmt, whitelistedValsMap) + if len(delegations) == 0 { return types.ErrInvalidActiveLiquidValidators } - weightedAmt[0] = weightedAmt[0].Add(crumb) - for i, val := range activeVals { - if !weightedAmt[i].IsPositive() { - continue - } - validator, _ := k.stakingKeeper.GetValidator(ctx, val.GetOperator()) - err = k.DelegateWithCap(ctx, proxyAcc, validator, weightedAmt[i]) + + for valStrAddr, delegationAmt := range delegations { + valAddr, _ := sdk.ValAddressFromBech32(valStrAddr) + validator, _ := k.stakingKeeper.GetValidator(ctx, valAddr) + err = k.DelegateWithCap(ctx, proxyAcc, validator, delegationAmt) if err != nil { return err } } + return nil } @@ -720,52 +723,80 @@ 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. +// DivideByWeight divide the input amount to delegate between the active validators, specifically checking if they have +// room to receive more liquid delegations. The leftover that might be left is divided across all validators that can +// receive it. func (k Keeper) DivideByWeight( ctx sdk.Context, avs types.ActiveLiquidValidators, - input math.Int, + totalInput math.Int, whitelistedValsMap types.WhitelistedValsMap, -) (outputs []math.Int, crumb math.Int) { +) map[string]math.Int { totalWeight := avs.TotalWeight(whitelistedValsMap) if !totalWeight.IsPositive() { - return []math.Int{}, sdk.ZeroInt() + return map[string]math.Int{} } - totalOutput := sdk.ZeroInt() - unitInput := math.LegacyNewDecFromInt(input).QuoTruncate(math.LegacyNewDecFromInt(totalWeight)) + totalDelegation := sdk.ZeroInt() + delegations := make(map[string]math.Int) + inputPerValidator := math.LegacyNewDecFromInt(totalInput).QuoTruncate(math.LegacyNewDecFromInt(totalWeight)) + + // loop through all validators and check if the amount of shares they would receive is within the validator specific caps 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()) + delegation := inputPerValidator.MulInt(val.GetWeight(whitelistedValsMap, true)).TruncateInt() + if !delegation.IsPositive() { // continue if the delegation is 0 + continue + } + delegationShares := validator.GetDelegatorShares().MulInt(delegation).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) { - totalOutput = totalOutput.Add(output) - outputs = append(outputs, output) + if !k.stakingKeeper.CheckExceedsValidatorBondCap(ctx, validator, delegationShares) && + !k.stakingKeeper.CheckExceedsValidatorLiquidStakingCap(ctx, validator, delegationShares, false) { + totalDelegation = totalDelegation.Add(delegation) + delegations[val.GetOperator().String()] = delegation } } - if len(outputs) == 0 { - return []math.Int{}, sdk.ZeroInt() + if len(delegations) == 0 { + return map[string]math.Int{} } - // redistribute crumb evenly to the other outputs if there is enough - numOutputs := sdk.NewInt(int64(len(outputs))) - totalCrumb := input.Sub(totalOutput) - crumbPerOutput := totalCrumb.Quo(numOutputs) - if totalCrumb.GTE(numOutputs) { - for i := range outputs { - totalOutput = totalOutput.Add(crumbPerOutput) - outputs[i] = outputs[i].Add(crumbPerOutput) + // the leftover amount that could not be delegated is divided across all validators + leftOver := totalInput.Sub(totalDelegation) + numValidators := math.NewInt(int64(len(delegations))) + for leftOver.IsPositive() { + diffPerValidator := leftOver.Quo(numValidators) + if leftOver.LT(numValidators) { // if the leftover is less than the number of validators to be delegated, just add it to the first one that can receive it + diffPerValidator = leftOver + } + for strAddr := range delegations { + // get the new delegation amount by adding the leftover to the existing delegation + newDelegationAmount := delegations[strAddr].Add(diffPerValidator) + + // calculate the shares the input would receive + valAddr, _ := sdk.ValAddressFromBech32(strAddr) + validator, _ := k.stakingKeeper.GetValidator(ctx, valAddr) + delegationShares := validator.GetDelegatorShares().MulInt(newDelegationAmount).QuoInt(validator.GetTokens()) + + // if the validator can receive the leftover amount, update the map entry and add it to the total delegated amount + if !k.stakingKeeper.CheckExceedsValidatorBondCap(ctx, validator, delegationShares) && + !k.stakingKeeper.CheckExceedsValidatorLiquidStakingCap(ctx, validator, delegationShares, false) { + totalDelegation = totalDelegation.Add(diffPerValidator) + delegations[strAddr] = newDelegationAmount + } + + // recalculate the leftover amount after each pass + leftOver = totalInput.Sub(totalDelegation) + if leftOver.IsZero() { + break + } } } - return outputs, input.Sub(totalOutput) + return delegations } // PrioritiseInactiveLiquidValidators sorts LiquidValidators array to have inactive validators first. Used for the case when diff --git a/x/liquidstake/keeper/liquidstake_test.go b/x/liquidstake/keeper/liquidstake_test.go index 2b27eeb49..3e7a6f993 100644 --- a/x/liquidstake/keeper/liquidstake_test.go +++ b/x/liquidstake/keeper/liquidstake_test.go @@ -100,11 +100,7 @@ func (s *KeeperTestSuite) TestLiquidStake() { s.ctx, types.LiquidStakeProxyAcc, valOpers[2], ) s.Require().True(found) - s.Require().Equal(proxyAccDel1.Shares, math.LegacyNewDec(16668)) - s.Require().Equal(proxyAccDel2.Shares, math.LegacyNewDec(16666)) - s.Require().Equal(proxyAccDel2.Shares, math.LegacyNewDec(16666)) - s.Require().Equal(stakingAmt.ToLegacyDec(), - proxyAccDel1.Shares.Add(proxyAccDel2.Shares).Add(proxyAccDel3.Shares)) + s.Require().Equal(stakingAmt.ToLegacyDec(), proxyAccDel1.Shares.Add(proxyAccDel2.Shares).Add(proxyAccDel3.Shares)) liquidBondDenom := s.keeper.LiquidBondDenom(s.ctx) balanceBeforeUBD := s.app.BankKeeper.GetBalance( @@ -183,9 +179,7 @@ func (s *KeeperTestSuite) TestLiquidStake() { s.ctx, types.LiquidStakeProxyAcc, valOpers[2], ) s.Require().True(found) - s.Require().Equal(math.LegacyNewDec(13335), proxyAccDel1.Shares) - s.Require().Equal(math.LegacyNewDec(13333), proxyAccDel2.Shares) - s.Require().Equal(math.LegacyNewDec(13333), proxyAccDel3.Shares) + s.Require().Equal(stakingAmt.Sub(unbondingAmt).ToLegacyDec(), proxyAccDel1.Shares.Add(proxyAccDel2.Shares).Add(proxyAccDel3.Shares)) res = s.keeper.GetAllLiquidValidatorStates(s.ctx) s.Require().Equal(params.WhitelistedValidators[0].ValidatorAddress, @@ -422,7 +416,7 @@ func (s *KeeperTestSuite) TestShareInflation() { s.keeper.SetParams(s.ctx, params) s.keeper.UpdateLiquidValidatorSet(s.ctx) - initialStakingAmt := math.NewInt(1) // little amount + initialStakingAmt := math.NewInt(10) // little amount initializingStakingAmt := math.NewInt(10000) // normal amount attacker := s.delAddrs[0] user := s.delAddrs[1] @@ -470,14 +464,13 @@ 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 string + whitelistedVals []types.WhitelistedValidator + addStakingAmt math.Int + expectedDelegations map[string]math.Int }{ { - name: "Success with crumbs", + name: "Success with leftover less than delegations length", whitelistedVals: []types.WhitelistedValidator{ { ValidatorAddress: valOpers[0].String(), @@ -492,12 +485,15 @@ func (s *KeeperTestSuite) TestDivideByWeight() { TargetWeight: math.NewInt(1), }, }, - addStakingAmt: math.NewInt(100000), - expectedOutputs: []math.Int{math.NewInt(33333), math.NewInt(33333), math.NewInt(33333)}, - expectedCrumb: math.NewInt(1), + addStakingAmt: math.NewInt(100000), + expectedDelegations: map[string]math.Int{ + valOpers[0].String(): math.NewInt(33334), + valOpers[1].String(): math.NewInt(33333), + valOpers[2].String(): math.NewInt(33333), + }, }, { - name: "Success without crumb", + name: "Success without leftover", whitelistedVals: []types.WhitelistedValidator{ { ValidatorAddress: valOpers[0].String(), @@ -512,12 +508,15 @@ func (s *KeeperTestSuite) TestDivideByWeight() { TargetWeight: math.NewInt(1), }, }, - addStakingAmt: math.NewInt(100000), - expectedOutputs: []math.Int{math.NewInt(40000), math.NewInt(40000), math.NewInt(20000)}, - expectedCrumb: math.NewInt(0), + addStakingAmt: math.NewInt(100000), + expectedDelegations: map[string]math.Int{ + valOpers[0].String(): math.NewInt(40000), + valOpers[1].String(): math.NewInt(40000), + valOpers[2].String(): math.NewInt(20000), + }, }, { - name: "First validator reaches the cap, part of the crumb gets divided among validators", + name: "First validator reaches the cap, the leftover gets divided among validators", whitelistedVals: []types.WhitelistedValidator{ { ValidatorAddress: valOpers[0].String(), @@ -532,12 +531,14 @@ func (s *KeeperTestSuite) TestDivideByWeight() { TargetWeight: math.NewInt(1), }, }, - addStakingAmt: math.NewInt(2500003), - expectedOutputs: []math.Int{math.NewInt(1250001), math.NewInt(1250001)}, - expectedCrumb: math.NewInt(1), + addStakingAmt: math.NewInt(2500003), + expectedDelegations: map[string]math.Int{ + valOpers[1].String(): math.NewInt(1250002), + valOpers[2].String(): math.NewInt(1250001), + }, }, { - name: "First validator reaches the cap, all the crumb gets divided among validators", + name: "First validator reaches the cap, the leftover gets divided among validators evenly", whitelistedVals: []types.WhitelistedValidator{ { ValidatorAddress: valOpers[0].String(), @@ -552,9 +553,11 @@ func (s *KeeperTestSuite) TestDivideByWeight() { TargetWeight: math.NewInt(1), }, }, - addStakingAmt: math.NewInt(2500002), - expectedOutputs: []math.Int{math.NewInt(1250001), math.NewInt(1250001)}, - expectedCrumb: math.NewInt(0), + addStakingAmt: math.NewInt(2500002), + expectedDelegations: map[string]math.Int{ + valOpers[1].String(): math.NewInt(1250001), + valOpers[2].String(): math.NewInt(1250001), + }, }, { name: "All validators reach the cap", @@ -572,9 +575,27 @@ func (s *KeeperTestSuite) TestDivideByWeight() { TargetWeight: math.NewInt(1), }, }, - addStakingAmt: math.NewInt(1000000000), - expectedOutputs: []math.Int{}, - expectedCrumb: math.NewInt(0), + addStakingAmt: math.NewInt(1000000000), + expectedDelegations: map[string]math.Int{}, + }, + { + name: "Amount below minimum", + 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(1), + expectedDelegations: map[string]math.Int{}, }, } @@ -582,10 +603,8 @@ func (s *KeeperTestSuite) TestDivideByWeight() { 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) + require.IsType(t, map[string]math.Int{}, tc.expectedDelegations) - totalTargetAmt := sdk.ZeroInt() valsMap := types.GetWhitelistedValsMap(tc.whitelistedVals) var activeVals types.ActiveLiquidValidators for _, v := range tc.whitelistedVals { @@ -593,14 +612,15 @@ func (s *KeeperTestSuite) TestDivideByWeight() { OperatorAddress: v.ValidatorAddress, }) } - outputs, crumb := s.keeper.DivideByWeight(s.ctx, activeVals, tc.addStakingAmt, valsMap) - for _, v := range outputs { - totalTargetAmt = totalTargetAmt.Add(v) + delegations := s.keeper.DivideByWeight(s.ctx, activeVals, tc.addStakingAmt, valsMap) + + require.EqualValues(t, tc.expectedDelegations, delegations) + totalDelegationAmount := sdk.ZeroInt() + for _, d := range delegations { + totalDelegationAmount = totalDelegationAmount.Add(d) } - 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)) + if !(len(delegations) == 0) { + require.EqualValues(t, tc.addStakingAmt, totalDelegationAmount) } }) } diff --git a/x/liquidstake/keeper/rebalancing_test.go b/x/liquidstake/keeper/rebalancing_test.go index 0ec98d858..2a1a9daeb 100644 --- a/x/liquidstake/keeper/rebalancing_test.go +++ b/x/liquidstake/keeper/rebalancing_test.go @@ -438,10 +438,10 @@ func (s *KeeperTestSuite) TestRebalancingConsecutiveCase() { s.ctx = s.ctx.WithBlockHeight(s.ctx.BlockHeight() + 100).WithBlockTime(s.ctx.BlockTime().Add(time.Hour * 24 * 20).Add(time.Hour)) staking.EndBlocker(s.ctx, s.app.StakingKeeper) reds = s.keeper.UpdateLiquidValidatorSet(s.ctx) - s.Require().Len(reds, 9) + s.Require().LessOrEqual(len(reds), 9) // failed redelegations with small amount (less than rebalancing trigger) - s.Require().Equal(s.redelegationsErrorCount(reds), 6) + s.Require().LessOrEqual(s.redelegationsErrorCount(reds), 6) s.printRedelegationsLiquidTokens() // assert rebalanced