Address comments

.changelog/unreleased/bug-fixes/provider/1925-apply-audit-suggestions.md

@@ -1,3 +1,2 @@
 - Apply audit suggestions that include a bug fix in the way we compute the
-  maximum capped power. ([\#1925](https://github.com/cosmos/interchain-
-  security/pull/1925))
+  maximum capped power. ([\#1925](https://github.com/cosmos/interchain-security/pull/1925))

.changelog/unreleased/features/provider/2035-min-stake-max-rank.md

@@ -1,3 +1,2 @@
 - Add minimum stake and maximum validator rank requirements to let consumer chains
-  determine requirements for validators that validate their chain. ([\#2035](https://github.com/cosmos/interchain-
-  security/pull/2035))
+  determine requirements for validators that validate their chain. ([\#2035](https://github.com/cosmos/interchain-security/pull/2035))

.changelog/unreleased/state-breaking/provider/1925-apply-audit-suggestions.md

@@ -1,3 +1,2 @@
 - Apply audit suggestions that include a bug fix in the way we compute the
-  maximum capped power. ([\#1925](https://github.com/cosmos/interchain-
-  security/pull/1925))
+  maximum capped power. ([\#1925](https://github.com/cosmos/interchain-security/pull/1925))

.changelog/unreleased/state-breaking/provider/2035-min-stake-max-rank.md

@@ -1,3 +1,2 @@
 - Add minimum stake and maximum validator rank requirements to let consumer chains
-  determine requirements for validators that validate their chain. ([\#2035](https://github.com/cosmos/interchain-
-  security/pull/2035))
+  determine requirements for validators that validate their chain. ([\#2035](https://github.com/cosmos/interchain-security/pull/2035))

proto/interchain_security/ccv/provider/v1/provider.proto

@@ -107,9 +107,9 @@ message ConsumerAdditionProposal {
   // Corresponds to a list of provider consensus addresses of validators that CANNOT validate the consumer chain.
   repeated string denylist = 19;
   // Corresponds to the minimal amount of (provider chain) stake required to validate on the consumer chain.
-  int64 min_stake = 20;
+  uint64 min_stake = 20;
   // Corresponds to the maximal rank in the provider chain validator set that a validator can have to validate on the consumer chain.
-  int32 max_rank = 21;
+  uint32 max_rank = 21;
 }
 
 // ConsumerRemovalProposal is a governance proposal on the provider chain to
@@ -161,9 +161,9 @@ message ConsumerModificationProposal {
   // Corresponds to a list of provider consensus addresses of validators that CANNOT validate the consumer chain.
   repeated string denylist = 8;
   // Corresponds to the minimal amount of (provider chain) stake required to validate on the consumer chain.
-  int64 min_stake = 9;
+  uint64 min_stake = 9;
   // Corresponds to the maximal rank in the provider chain validator set that a validator can have to validate on the consumer chain.
-  int32 max_rank = 10;
+  uint32 max_rank = 10;
 }
 
 

proto/interchain_security/ccv/provider/v1/tx.proto

@@ -184,9 +184,9 @@ message MsgConsumerAddition {
   // signer address
   string authority = 18 [(cosmos_proto.scalar) = "cosmos.AddressString"];
   // Corresponds to the minimal amount of (provider chain) stake required to validate on the consumer chain.
-  int64 min_stake = 19;
+  uint64 min_stake = 19;
   // Corresponds to the maximal rank in the provider chain validator set that a validator can have to validate on the consumer chain.
-  int32 max_rank = 20;
+  uint32 max_rank = 20;
 }
 
 // MsgConsumerAdditionResponse defines response type for MsgConsumerAddition messages
@@ -325,9 +325,9 @@ message MsgConsumerModification {
   // signer address
   string authority = 9 [(cosmos_proto.scalar) = "cosmos.AddressString"];
   // Corresponds to the minimal amount of (provider chain) stake required to validate on the consumer chain.
-  int64 min_stake = 10;
+  uint64 min_stake = 10;
   // Corresponds to the maximal rank in the provider chain validator set that a validator can have to validate on the consumer chain.
-  int32 max_rank = 11;
+  uint32 max_rank = 11;
 }
 
 message MsgConsumerModificationResponse {}

tests/integration/partial_set_security_test.go

@@ -29,8 +29,8 @@ func TestMinStakeMaxRank(t *testing.T) {
 	testCases := []struct {
 		name                string
 		stakedTokens        []int64
-		minStake            int64
-		maxRank             int32
+		minStake            uint64
+		maxRank             uint32
 		expectedConsuValSet []int64
 	}{
 		{

x/ccv/provider/client/legacy_proposals.go

@@ -41,8 +41,8 @@ type ConsumerAdditionProposalJSON struct {
 	ValidatorSetCap    uint32   `json:"validator_set_cap"`
 	Allowlist          []string `json:"allowlist"`
 	Denylist           []string `json:"denylist"`
-	MinStake           int64    `json:"min_stake"`
-	MaxValidatorRank   int32    `json:"max_validator_rank"`
+	MinStake           uint64   `json:"min_stake"`
+	MaxValidatorRank   uint32   `json:"max_validator_rank"`
 }
 
 type ConsumerAdditionProposalReq struct {
@@ -174,8 +174,8 @@ type ConsumerModificationProposalJSON struct {
 	ValidatorSetCap    uint32   `json:"validator_set_cap"`
 	Allowlist          []string `json:"allowlist"`
 	Denylist           []string `json:"denylist"`
-	MinStake           int64    `json:"min_stake"`
-	MaxValidatorRank   int32    `json:"max_validator_rank"`
+	MinStake           uint64   `json:"min_stake"`
+	MaxValidatorRank   uint32   `json:"max_validator_rank"`
 
 	Deposit string `json:"deposit"`
 }

x/ccv/provider/keeper/keeper.go

@@ -1588,12 +1588,12 @@ func (k Keeper) DeleteMinimumPowerInTopN(
 func (k Keeper) SetMinStake(
 	ctx sdk.Context,
 	chainID string,
-	minStake int64,
+	minStake uint64,
 ) {
 	store := ctx.KVStore(k.storeKey)
 
 	buf := make([]byte, 8)
-	binary.BigEndian.PutUint64(buf, uint64(minStake))
+	binary.BigEndian.PutUint64(buf, minStake)
 
 	store.Set(types.MinStakeKey(chainID), buf)
 }
@@ -1603,13 +1603,13 @@ func (k Keeper) SetMinStake(
 func (k Keeper) GetMinStake(
 	ctx sdk.Context,
 	chainID string,
-) (int64, bool) {
+) (uint64, bool) {
 	store := ctx.KVStore(k.storeKey)
 	buf := store.Get(types.MinStakeKey(chainID))
 	if buf == nil {
 		return 0, false
 	}
-	return int64(binary.BigEndian.Uint64(buf)), true
+	return binary.BigEndian.Uint64(buf), true
 }
 
 // DeleteMinStake removes the minimum stake required for a validator to validate
@@ -1627,12 +1627,12 @@ func (k Keeper) DeleteMinStake(
 func (k Keeper) SetMaxValidatorRank(
 	ctx sdk.Context,
 	chainID string,
-	maxRank int32,
+	maxRank uint32,
 ) {
 	store := ctx.KVStore(k.storeKey)
 
 	buf := make([]byte, 4)
-	binary.BigEndian.PutUint32(buf, uint32(maxRank))
+	binary.BigEndian.PutUint32(buf, maxRank)
 
 	store.Set(types.MaxValidatorRankKey(chainID), buf)
 }
@@ -1642,13 +1642,13 @@ func (k Keeper) SetMaxValidatorRank(
 func (k Keeper) GetMaxValidatorRank(
 	ctx sdk.Context,
 	chainID string,
-) (int32, bool) {
+) (uint32, bool) {
 	store := ctx.KVStore(k.storeKey)
 	buf := store.Get(types.MaxValidatorRankKey(chainID))
 	if buf == nil {
 		return 0, false
 	}
-	return int32(binary.BigEndian.Uint32(buf)), true
+	return binary.BigEndian.Uint32(buf), true
 }
 
 // DeleteMaxValidatorRank removes the maximum position in the validator set a validator can have to validate

x/ccv/provider/keeper/keeper_test.go

@@ -853,12 +853,15 @@ func TestKeeperIntConsumerParams(t *testing.T) {
 		},
 		{
 			name:        "Minimum Stake",
-			settingFunc: func(ctx sdk.Context, id string, val int64) { k.SetMinStake(ctx, id, val) },
-			getFunc:     func(ctx sdk.Context, id string) (int64, bool) { return k.GetMinStake(ctx, id) },
+			settingFunc: func(ctx sdk.Context, id string, val int64) { k.SetMinStake(ctx, id, uint64(val)) },
+			getFunc: func(ctx sdk.Context, id string) (int64, bool) {
+				val, found := k.GetMinStake(ctx, id)
+				return int64(val), found
+			},
 		},
 		{
 			name:        "Maximum Validator Rank",
-			settingFunc: func(ctx sdk.Context, id string, val int64) { k.SetMaxValidatorRank(ctx, id, int32(val)) },
+			settingFunc: func(ctx sdk.Context, id string, val int64) { k.SetMaxValidatorRank(ctx, id, uint32(val)) },
 			getFunc: func(ctx sdk.Context, id string) (int64, bool) {
 				val, found := k.GetMaxValidatorRank(ctx, id)
 				return int64(val), found

x/ccv/provider/keeper/legacy_proposal_test.go

@@ -294,16 +294,16 @@ func TestHandleConsumerModificationProposal(t *testing.T) {
 	expectedValidatorSetCap := uint32(20)
 	expectedAllowlistedValidator := "cosmosvalcons1wpex7anfv3jhystyv3eq20r35a"
 	expectedDenylistedValidator := "cosmosvalcons1nx7n5uh0ztxsynn4sje6eyq2ud6rc6klc96w39"
-	expectedMinStake := int64(0)
-	expectedMaxValidatorRank := int32(20)
+	expectedMinStake := uint64(0)
+	expectedMaxValidatorRank := uint32(20)
 	proposal := providertypes.NewConsumerModificationProposal("title", "description", chainID,
 		expectedTopN,
 		expectedValidatorsPowerCap,
 		expectedValidatorSetCap,
 		[]string{expectedAllowlistedValidator},
 		[]string{expectedDenylistedValidator},
-		int64(expectedMinStake),
-		int32(expectedMaxValidatorRank),
+		expectedMinStake,
+		expectedMaxValidatorRank,
 	).(*providertypes.ConsumerModificationProposal)
 
 	err := providerKeeper.HandleLegacyConsumerModificationProposal(ctx, proposal)

x/ccv/provider/keeper/partial_set_security.go

@@ -322,17 +322,14 @@ func (k Keeper) FulfillsMinStake(ctx sdk.Context, chainID string, providerAddr t
 	}
 
 	// validator has enough stake to validate the chain
-	return validator.GetTokens().GTE(math.NewInt(minStake))
+	return validator.GetTokens().GTE(math.NewIntFromUint64(minStake))
 }
 
 // ComputeNextValidators computes the validators for the upcoming epoch based on the currently `bondedValidators`.
 func (k Keeper) ComputeNextValidators(ctx sdk.Context, chainID string, bondedValidators []stakingtypes.Validator) []types.ConsensusValidator {
-	// sort the bonded validators by power in descending order
+	// sort the bonded validators by number of staked tokens in descending order
 	sort.Slice(bondedValidators, func(i, j int) bool {
-		iTokens := bondedValidators[i].GetTokens()
-		jTokens := bondedValidators[j].GetTokens()
-		result := iTokens.GT(jTokens)
-		return result
+		return bondedValidators[i].GetTokens().GT(bondedValidators[j].GetTokens())
 	})
 
 	// take only the first `MaxValidatorRank` many validators; others are not allowed to validate
@@ -341,7 +338,7 @@ func (k Keeper) ComputeNextValidators(ctx sdk.Context, chainID string, bondedVal
 		tmpValidators := bondedValidators[:maxRank]
 
 		// also include other validators that have the same number of tokens as the last validator in the list
-		for i := maxRank; int(i) < len(bondedValidators); i++ {
+		for i := int(maxRank); i < len(bondedValidators); i++ {
 			// if the validator has the same number of tokens as the last validator in the list, include it
 			if bondedValidators[i].GetTokens().Equal(bondedValidators[maxRank-1].GetTokens()) {
 				tmpValidators = append(tmpValidators, bondedValidators[i])

x/ccv/provider/keeper/partial_set_security_test.go

@@ -718,7 +718,7 @@ func TestMinStake(t *testing.T) {
 
 	testCases := []struct {
 		name           string
-		minStake       int64
+		minStake       uint64
 		expectedFulfil []bool
 	}{
 		{
@@ -763,7 +763,7 @@ func TestMaxValidatorRank(t *testing.T) {
 
 	testCases := []struct {
 		name                      string
-		maxRank                   int32
+		maxRank                   uint32
 		expectedProviderConsAddrs []types.ProviderConsAddress
 	}{
 		{

x/ccv/provider/types/legacy_proposal.go

@@ -58,8 +58,8 @@ func NewConsumerAdditionProposal(title, description, chainID string,
 	validatorSetCap uint32,
 	allowlist []string,
 	denylist []string,
-	minStake int64,
-	maxValidatorRank int32,
+	minStake uint64,
+	maxValidatorRank uint32,
 ) govv1beta1.Content {
 	return &ConsumerAdditionProposal{
 		Title:                             title,
@@ -247,8 +247,8 @@ func NewConsumerModificationProposal(title, description, chainID string,
 	validatorSetCap uint32,
 	allowlist []string,
 	denylist []string,
-	minStake int64,
-	maxValidatorRank int32,
+	minStake uint64,
+	maxValidatorRank uint32,
 ) govv1beta1.Content {
 	return &ConsumerModificationProposal{
 		Title:              title,