From e48cd0ba288d9a1627a01f9558dadf7a0dee83ed Mon Sep 17 00:00:00 2001 From: kruspy Date: Tue, 9 Apr 2024 11:26:40 +0200 Subject: [PATCH] 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 eb9f9d32..81c53260 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 2b27eeb4..3e7a6f99 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 0ec98d85..2a1a9dae 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