Fix validator weight check (#1176)

x/stakeibc/keeper/gov.go

@@ -13,6 +13,11 @@ func (k Keeper) AddValidatorsProposal(ctx sdk.Context, msg *types.AddValidatorsP
 		}
 	}
 
+	// Confirm none of the validator's exceed the weight cap
+	if err := k.CheckValidatorWeightsBelowCap(ctx, msg.HostZone); err != nil {
+		return err
+	}
+
 	return nil
 }
 

x/stakeibc/keeper/host_zone_test.go

@@ -488,7 +488,13 @@ func (s *KeeperTestSuite) TestCheckValidatorWeightsBelowCap() {
 			params.ValidatorWeightCap = tc.weightCap
 			s.App.StakeibcKeeper.SetParams(s.Ctx, params)
 
-			err := s.App.StakeibcKeeper.CheckValidatorWeightsBelowCap(s.Ctx, tc.validators)
+			hostZone := types.HostZone{
+				ChainId:    HostChainId,
+				Validators: tc.validators,
+			}
+			s.App.StakeibcKeeper.SetHostZone(s.Ctx, hostZone)
+
+			err := s.App.StakeibcKeeper.CheckValidatorWeightsBelowCap(s.Ctx, HostChainId)
 			if !tc.exceedsCap {
 				s.Require().NoError(err, "set should not have exceeded cap")
 			} else {

x/stakeibc/keeper/msg_server.go

@@ -291,6 +291,11 @@ func (k msgServer) AddValidators(goCtx context.Context, msg *types.MsgAddValidat
 		}
 	}
 
+	// Confirm none of the validator's exceed the weight cap
+	if err := k.CheckValidatorWeightsBelowCap(ctx, msg.HostZone); err != nil {
+		return nil, err
+	}
+
 	return &types.MsgAddValidatorsResponse{}, nil
 }
 
@@ -334,7 +339,7 @@ func (k msgServer) ChangeValidatorWeight(goCtx context.Context, msg *types.MsgCh
 	}
 
 	// Confirm the new weights wouldn't cause any validator to exceed the weight cap
-	if err := k.CheckValidatorWeightsBelowCap(ctx, hostZone.Validators); err != nil {
+	if err := k.CheckValidatorWeightsBelowCap(ctx, msg.HostZone); err != nil {
 		return nil, errorsmod.Wrapf(err, "unable to change validator weight")
 	}
 

x/stakeibc/keeper/msg_server_test.go

@@ -13,6 +13,7 @@ import (
 	channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types"
 	ibctesting "github.com/cosmos/ibc-go/v7/testing"
 
+	"github.com/Stride-Labs/stride/v21/app/apptesting"
 	"github.com/Stride-Labs/stride/v21/utils"
 	epochtypes "github.com/Stride-Labs/stride/v21/x/epochs/types"
 	icqtypes "github.com/Stride-Labs/stride/v21/x/interchainquery/types"
@@ -626,6 +627,91 @@ func (s *KeeperTestSuite) TestAddValidators_NameAlreadyExists() {
 	s.Require().ErrorContains(err, expectedError)
 }
 
+func (s *KeeperTestSuite) TestAddValidators_SuccessfulManyValidators() {
+	s.App.StakeibcKeeper.SetHostZone(s.Ctx, types.HostZone{
+		ChainId:      HostChainId,
+		ConnectionId: ibctesting.FirstConnectionID,
+	})
+	s.MockClientLatestHeight(1)
+
+	// Setup validators in a top-heavy order so that *if* the weight cap
+	// was checked after each validator, it would fail midway
+	// However, the addition of last validator causes the highest weight
+	// validator to be below 10%
+	validators := []*types.Validator{
+		{Name: "val1", Weight: 10},
+		{Name: "val2", Weight: 10},
+		{Name: "val3", Weight: 9},
+		{Name: "val4", Weight: 9},
+		{Name: "val5", Weight: 8},
+		{Name: "val6", Weight: 8},
+		{Name: "val7", Weight: 7},
+		{Name: "val8", Weight: 7},
+		{Name: "val9", Weight: 6},
+		{Name: "val10", Weight: 6},
+		{Name: "val11", Weight: 5},
+		{Name: "val12", Weight: 5},
+		{Name: "val13", Weight: 4},
+		{Name: "val14", Weight: 4},
+		{Name: "val15", Weight: 3},
+	}
+
+	// Assign an address for each
+	addresses := apptesting.CreateRandomAccounts(len(validators))
+	for i, validator := range validators {
+		validator.Address = addresses[i].String()
+	}
+
+	// Submit the add validator message - it should succeed
+	addValidatorMsg := types.MsgAddValidators{
+		HostZone:   HostChainId,
+		Validators: validators,
+	}
+	_, err := s.GetMsgServer().AddValidators(sdk.WrapSDKContext(s.Ctx), &addValidatorMsg)
+	s.Require().NoError(err, "no error expected when adding validators")
+}
+
+func (s *KeeperTestSuite) TestAddValidators_ValidatorWeightCapExceeded() {
+	s.App.StakeibcKeeper.SetHostZone(s.Ctx, types.HostZone{
+		ChainId:      HostChainId,
+		ConnectionId: ibctesting.FirstConnectionID,
+	})
+	s.MockClientLatestHeight(1)
+
+	// The distribution below will lead to the first two validators owning more
+	// than a 10% share
+	validators := []*types.Validator{
+		{Name: "val1", Weight: 10},
+		{Name: "val2", Weight: 10},
+		{Name: "val3", Weight: 9},
+		{Name: "val4", Weight: 9},
+		{Name: "val5", Weight: 8},
+		{Name: "val6", Weight: 8},
+		{Name: "val7", Weight: 7},
+		{Name: "val8", Weight: 7},
+		{Name: "val9", Weight: 6},
+		{Name: "val10", Weight: 6},
+		{Name: "val11", Weight: 5},
+		{Name: "val12", Weight: 5},
+		{Name: "val13", Weight: 4},
+		{Name: "val14", Weight: 4},
+	}
+
+	// Assign an address for each
+	addresses := apptesting.CreateRandomAccounts(len(validators))
+	for i, validator := range validators {
+		validator.Address = addresses[i].String()
+	}
+
+	// Submit the add validator message - it should error
+	addValidatorMsg := types.MsgAddValidators{
+		HostZone:   HostChainId,
+		Validators: validators,
+	}
+	_, err := s.GetMsgServer().AddValidators(sdk.WrapSDKContext(s.Ctx), &addValidatorMsg)
+	s.Require().ErrorContains(err, "validator exceeds weight cap")
+}
+
 // ----------------------------------------------------
 //	               DeleteValidator
 // ----------------------------------------------------

x/stakeibc/keeper/validator.go

@@ -68,11 +68,6 @@ func (k Keeper) AddValidatorToHostZone(ctx sdk.Context, chainId string, validato
 		SlashQueryCheckpoint:      checkpoint,
 	})
 
-	// Finally, confirm none of the validator's exceed the weight cap
-	if err := k.CheckValidatorWeightsBelowCap(ctx, hostZone.Validators); err != nil {
-		return err
-	}
-
 	k.SetHostZone(ctx, hostZone)
 
 	return nil
@@ -238,7 +233,13 @@ func (k Keeper) DecrementValidatorDelegationChangesInProgress(hostZone *types.Ho
 }
 
 // Checks if any validator's portion of the weight is greater than the cap
-func (k Keeper) CheckValidatorWeightsBelowCap(ctx sdk.Context, validators []*types.Validator) error {
+func (k Keeper) CheckValidatorWeightsBelowCap(ctx sdk.Context, chainId string) error {
+	hostZone, found := k.GetHostZone(ctx, chainId)
+	if !found {
+		return types.ErrHostZoneNotFound.Wrapf("host zone %s not found", chainId)
+	}
+	validators := hostZone.Validators
+
 	// If there's only a few validators, don't enforce this yet
 	if len(validators) < MinValidatorsBeforeWeightCapCheck {
 		return nil