Removed duplicate check for min/max validators in rebalance (#630)

Co-authored-by: shellvish <104537253+shellvish@users.noreply.github.com>
Stride-Labs · Mar 8, 2023 · 85e76ec · 85e76ec
1 parent 9b17edf
commit 85e76ec
Show file tree

Hide file tree

Showing 5 changed files with 70 additions and 88 deletions.
diff --git a/app/apptesting/test_helpers.go b/app/apptesting/test_helpers.go
@@ -24,6 +24,7 @@ import (
 	tmtypes "github.com/tendermint/tendermint/proto/tendermint/types"
 
 	"github.com/Stride-Labs/stride/v6/app"
+	"github.com/Stride-Labs/stride/v6/utils"
 )
 
 var (
@@ -359,6 +360,14 @@ func GenerateTestAddrs() (string, string) {
 	return validAddr, invalidAddr
 }
 
+// Grabs an admin address to test validate basic on admin txs
+func GetAdminAddress() (address string, ok bool) {
+	for address := range utils.Admins {
+		return address, true
+	}
+	return "", false
+}
+
 // Modifies sdk config to have stride address prefixes (used for non-keeper tests)
 func SetupConfig() {
 	app.SetupConfig()

diff --git a/x/stakeibc/keeper/msg_server_rebalance_validators.go b/x/stakeibc/keeper/msg_server_rebalance_validators.go
@@ -11,7 +11,6 @@ import (
 	errorsmod "cosmossdk.io/errors"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	stakingTypes "github.com/cosmos/cosmos-sdk/x/staking/types"
-	"github.com/spf13/cast"
 
 	"github.com/Stride-Labs/stride/v6/utils"
 	"github.com/Stride-Labs/stride/v6/x/stakeibc/types"
@@ -26,15 +25,7 @@ func (k msgServer) RebalanceValidators(goCtx context.Context, msg *types.MsgReba
 		k.Logger(ctx).Error(fmt.Sprintf("Host Zone not found %s", msg.HostZone))
 		return nil, types.ErrInvalidHostZone
 	}
-	maxNumRebalance := cast.ToInt(msg.NumRebalance)
-	if maxNumRebalance < 1 {
-		k.Logger(ctx).Error(fmt.Sprintf("Invalid number of validators to rebalance %d", maxNumRebalance))
-		return nil, types.ErrInvalidNumValidator
-	}
-	if maxNumRebalance > 4 {
-		k.Logger(ctx).Error(fmt.Sprintf("Invalid number of validators to rebalance %d", maxNumRebalance))
-		return nil, types.ErrInvalidNumValidator
-	}
+
 	validatorDeltas, err := k.GetValidatorDelegationAmtDifferences(ctx, hostZone)
 	if err != nil {
 		k.Logger(ctx).Error(fmt.Sprintf("Error getting validator deltas for Host Zone %s: %s", hostZone.ChainId, err))
@@ -68,14 +59,7 @@ func (k msgServer) RebalanceValidators(goCtx context.Context, msg *types.MsgReba
 		return nil, errorsmod.Wrapf(sdkerrors.ErrInvalidRequest, "no validator delegations found for Host Zone %s, cannot rebalance 0 delegations!", hostZone.ChainId)
 	}
 
-	overweight_delta := sdk.NewDecFromInt(valDeltaList[overWeightIndex].deltaAmt).Quo(sdk.NewDecFromInt(total_delegation))
-	underweight_delta := sdk.NewDecFromInt(valDeltaList[underWeightIndex].deltaAmt).Quo(sdk.NewDecFromInt(total_delegation))
-	max_delta := sdk.MaxDec(overweight_delta, underweight_delta)
-	rebalanceThreshold := sdk.NewDec(int64(k.GetParam(ctx, types.KeyValidatorRebalancingThreshold))).Quo(sdk.NewDec(10000))
-	if max_delta.LT(rebalanceThreshold) {
-		k.Logger(ctx).Error("Not enough validator disruption to rebalance")
-		return nil, types.ErrWeightsNotDifferent
-	}
+	// TODO Remove ValidatorRebalancingThreshold from our params
 
 	var msgs []sdk.Msg
 	delegationIca := hostZone.GetDelegationAccount()
@@ -92,7 +76,7 @@ func (k msgServer) RebalanceValidators(goCtx context.Context, msg *types.MsgReba
 		Rebalancings: []*types.Rebalancing{},
 	}
 
-	for i := 1; i <= maxNumRebalance; i++ {
+	for i := uint64(1); i <= msg.NumRebalance; i++ {
 		underWeightElem := valDeltaList[underWeightIndex]
 		overWeightElem := valDeltaList[overWeightIndex]
 		if underWeightElem.deltaAmt.LT(sdkmath.ZeroInt()) {

diff --git a/x/stakeibc/keeper/msg_server_rebalance_validators_test.go b/x/stakeibc/keeper/msg_server_rebalance_validators_test.go
@@ -162,43 +162,6 @@ func (s *KeeperTestSuite) TestRebalanceValidators_Successful() {
 	s.Require().Equal("stride_VAL4", secondRebal.SrcValidator, "second rebalance takes from val4")
 }
 
-func (s *KeeperTestSuite) TestRebalanceValidators_InvalidNumValidators() {
-	s.SetupRebalanceValidators()
-
-	// Rebalance with 0 validators should fail
-	badMsg_tooFew := stakeibctypes.MsgRebalanceValidators{
-		Creator:      "stride_ADDRESS",
-		HostZone:     "GAIA",
-		NumRebalance: 0,
-	}
-	_, err := s.GetMsgServer().RebalanceValidators(sdk.WrapSDKContext(s.Ctx), &badMsg_tooFew)
-	expectedErrMsg := "invalid number of validators"
-	s.Require().EqualError(err, expectedErrMsg, "rebalancing 0 validators should fail")
-
-	// Rebalance with 5 validators should fail
-	badMsg_tooMany := stakeibctypes.MsgRebalanceValidators{
-		Creator:      "stride_ADDRESS",
-		HostZone:     "GAIA",
-		NumRebalance: 5,
-	}
-	_, err = s.GetMsgServer().RebalanceValidators(sdk.WrapSDKContext(s.Ctx), &badMsg_tooMany)
-	s.Require().EqualError(err, expectedErrMsg, "rebalancing 5 validators should fail")
-}
-
-func (s *KeeperTestSuite) TestRebalanceValidators_InvalidNoChange() {
-	s.SetupRebalanceValidators()
-
-	// Rebalance with all weights properly set should fail
-	badMsg_rightWeights := stakeibctypes.MsgRebalanceValidators{
-		Creator:      "stride_ADDRESS",
-		HostZone:     "GAIA",
-		NumRebalance: 1,
-	}
-	_, err := s.GetMsgServer().RebalanceValidators(sdk.WrapSDKContext(s.Ctx), &badMsg_rightWeights)
-	expectedErrMsg := "validator weights haven't changed"
-	s.Require().EqualError(err, expectedErrMsg, "rebalancing with weights set properly should fail")
-}
-
 func (s *KeeperTestSuite) TestRebalanceValidators_InvalidNoValidators() {
 	s.SetupRebalanceValidators()
 
@@ -239,25 +202,3 @@ func (s *KeeperTestSuite) TestRebalanceValidators_InvalidAllValidatorsNoWeight()
 	expectedErrMsg := "no non-zero validator weights"
 	s.Require().EqualError(err, expectedErrMsg, "rebalancing with no validators should fail")
 }
-
-func (s *KeeperTestSuite) TestRebalanceValidators_InvalidNotEnoughDiff() {
-	s.SetupRebalanceValidators()
-
-	hz, found := s.App.StakeibcKeeper.GetHostZone(s.Ctx, "GAIA")
-	s.Require().True(found, "host zone should exist")
-	validators := hz.GetValidators()
-	s.Require().Equal(5, len(validators), "host zone should have 5 validators")
-	// modify weight to 25
-	validators[0].Weight = 101
-	s.App.StakeibcKeeper.SetHostZone(s.Ctx, hz)
-
-	// Rebalance without enough difference should fail
-	badMsg_noValidators := stakeibctypes.MsgRebalanceValidators{
-		Creator:      "stride_ADDRESS",
-		HostZone:     "GAIA",
-		NumRebalance: 2,
-	}
-	_, err := s.GetMsgServer().RebalanceValidators(sdk.WrapSDKContext(s.Ctx), &badMsg_noValidators)
-	expectedErrMsg := "validator weights haven't changed"
-	s.Require().EqualError(err, expectedErrMsg, "rebalancing without sufficient change should fail")
-}
diff --git a/x/stakeibc/types/message_rebalance_validators.go b/x/stakeibc/types/message_rebalance_validators.go
@@ -10,6 +10,11 @@ import (
 	"github.com/Stride-Labs/stride/v6/utils"
 )
 
+const (
+	MinNumRebalanceValidators = 1
+	MaxNumRebalanceValidators = 1
+)
+
 const TypeMsgRebalanceValidators = "rebalance_validators"
 
 var _ sdk.Msg = &MsgRebalanceValidators{}
@@ -51,7 +56,7 @@ func (msg *MsgRebalanceValidators) ValidateBasic() error {
 	if err := utils.ValidateAdminAddress(msg.Creator); err != nil {
 		return err
 	}
-	if (msg.NumRebalance < 1) || (msg.NumRebalance > 10) {
+	if (msg.NumRebalance < MinNumRebalanceValidators) || (msg.NumRebalance > MaxNumRebalanceValidators) {
 		return errorsmod.Wrapf(sdkerrors.ErrInvalidRequest, fmt.Sprintf("invalid number of validators to rebalance (%d)", msg.NumRebalance))
 	}
 	return nil

diff --git a/x/stakeibc/types/message_rebalance_validators_test.go b/x/stakeibc/types/message_rebalance_validators_test.go
@@ -1,30 +1,73 @@
-package types
+package types_test
 
 import (
 	"testing"
 
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	"github.com/stretchr/testify/require"
 
-	"github.com/Stride-Labs/stride/v6/testutil/sample"
+	"github.com/Stride-Labs/stride/v6/app/apptesting"
+	"github.com/Stride-Labs/stride/v6/x/stakeibc/types"
 )
 
 func TestMsgRebalanceValidators_ValidateBasic(t *testing.T) {
+	validNotAdminAddress, invalidAddress := apptesting.GenerateTestAddrs()
+	validAdminAddress, ok := apptesting.GetAdminAddress()
+	require.True(t, ok)
+
 	tests := []struct {
 		name string
-		msg  MsgRebalanceValidators
+		msg  types.MsgRebalanceValidators
 		err  error
 	}{
+		{
+			name: "successful message min vals",
+			msg: types.MsgRebalanceValidators{
+				Creator:      validAdminAddress,
+				NumRebalance: 1,
+			},
+		},
+		{
+			name: "successful message mid vals",
+			msg: types.MsgRebalanceValidators{
+				Creator:      validAdminAddress,
+				NumRebalance: 1,
+			},
+		},
+		{
+			name: "successful message max vals",
+			msg: types.MsgRebalanceValidators{
+				Creator:      validAdminAddress,
+				NumRebalance: 1,
+			},
+		},
+		{
+			name: "too few validators",
+			msg: types.MsgRebalanceValidators{
+				Creator:      validAdminAddress,
+				NumRebalance: 0,
+			},
+			err: sdkerrors.ErrInvalidRequest,
+		},
+		{
+			name: "too many validators",
+			msg: types.MsgRebalanceValidators{
+				Creator:      validAdminAddress,
+				NumRebalance: 2,
+			},
+			err: sdkerrors.ErrInvalidRequest,
+		},
 		{
 			name: "invalid address",
-			msg: MsgRebalanceValidators{
-				Creator: "invalid_address",
+			msg: types.MsgRebalanceValidators{
+				Creator: invalidAddress,
 			},
 			err: sdkerrors.ErrInvalidAddress,
-		}, {
-			name: "valid address but not whitelisted",
-			msg: MsgRebalanceValidators{
-				Creator: sample.AccAddress(),
+		},
+		{
+			name: "invalid admin address",
+			msg: types.MsgRebalanceValidators{
+				Creator: validNotAdminAddress,
 			},
 			err: sdkerrors.ErrInvalidAddress,
 		},