Skip to content

Commit

Permalink
Add more validating for staking and gov modules (#289)
Browse files Browse the repository at this point in the history
## Describe your changes and provide context
Per audit details, we're missing coverage for validation logic in
staking and gov

## Testing performed to validate your change

unit tests
  • Loading branch information
BrandonWeng authored Jun 14, 2023
1 parent df2cf22 commit 8e7f327
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 13 deletions.
2 changes: 1 addition & 1 deletion codecov.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ coverage:
project:
default:
threshold: 1% # allow this much decrease on project
target: 70%
target: 50%

comment:
layout: "reach,diff,flags,tree,betaprofiling"
Expand Down
12 changes: 0 additions & 12 deletions types/accesscontrol/access_operation_map.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,18 +30,6 @@ func SendAllSignalsForTx(messageIndexToAccessOpsChannelMapping MessageAccessOpsC
}
}

func GetMessageAccessOps(
messageIndex int,
messageAccessOpsChannelMapping MessageAccessOpsChannelMapping,
) []AccessOperation {
accessOps := []AccessOperation{}

for accessOp := range messageAccessOpsChannelMapping[messageIndex] {
accessOps = append(accessOps, accessOp)
}
return accessOps
}

func (a *AccessOperation) GetString() {
fmt.Printf("ResourceType=%s, AccessType=%s, IdentifierTemplate=%s", a.GetResourceType(), a.GetAccessType(), a.GetIdentifierTemplate())
}
22 changes: 22 additions & 0 deletions x/gov/types/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,14 @@ func (data GenesisState) Empty() bool {

// ValidateGenesis checks if parameters are within valid ranges
func ValidateGenesis(data *GenesisState) error {
if data == nil {
return fmt.Errorf("governance genesis state cannot be nil")
}

if data.Empty() {
return fmt.Errorf("governance genesis state cannot be nil")
}

threshold := data.TallyParams.Threshold
if threshold.IsNegative() || threshold.GT(sdk.OneDec()) {
return fmt.Errorf("governance vote threshold should be positive and less or equal to one, is %s",
Expand All @@ -62,6 +70,20 @@ func ValidateGenesis(data *GenesisState) error {
threshold)
}

if data.GetTallyParams().GetQuorum(false).IsNegative() || data.GetTallyParams().GetQuorum(false).IsZero() {
return fmt.Errorf("governance vote quorum should be positive, is %s", data.GetTallyParams().GetQuorum(false).String())
}

if data.GetTallyParams().GetQuorum(true).IsNegative() || data.GetTallyParams().GetQuorum(true).IsZero() {
return fmt.Errorf("governance vote expedited quorum should be positive, is %s", data.GetTallyParams().GetQuorum(true).String())
}

if data.GetTallyParams().GetQuorum(true).LTE(data.GetTallyParams().GetQuorum(false)) {
return fmt.Errorf("governance vote expedited quorum %s should be greater than regular quorum %s",
data.GetTallyParams().GetQuorum(true),
data.GetTallyParams().GetQuorum(false))
}

veto := data.TallyParams.VetoThreshold
if veto.IsNegative() || veto.GT(sdk.OneDec()) {
return fmt.Errorf("governance vote veto threshold should be positive and less or equal to one, is %s",
Expand Down
6 changes: 6 additions & 0 deletions x/gov/types/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,9 @@ func TestEqualProposalID(t *testing.T) {
require.Equal(t, state1, state2)
require.True(t, state1.Equal(state2))
}

func TestValidateGenesis(t *testing.T) {
require.Nil(t, ValidateGenesis(DefaultGenesisState()))
require.Error(t, ValidateGenesis(&GenesisState{}))
require.Error(t, ValidateGenesis(nil))
}
6 changes: 6 additions & 0 deletions x/staking/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,12 @@ func TestValidateGenesis(t *testing.T) {
data.Validators[0].Jailed = true
data.Validators[0].Status = types.Bonded
}, true},
{"invalid vp threshold", func(data *types.GenesisState) {
data.Params.MaxVotingPowerEnforcementThreshold = sdk.NewInt(-1)
}, true},
{"invalid vp ratio", func(data *types.GenesisState) {
data.Params.MaxVotingPowerRatio = sdk.NewDec(2)
}, true},
}

for _, tt := range tests {
Expand Down
10 changes: 10 additions & 0 deletions x/staking/types/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,15 @@ func (p Params) Validate() error {
if err := validateMinCommissionRate(p.MinCommissionRate); err != nil {
return err
}

if err := validateMaxVotingPowerEnforcementThreshold(p.MaxVotingPowerEnforcementThreshold); err != nil {
return err
}

if err := validateMaxVotingPowerRatio(p.MaxVotingPowerRatio); err != nil {
return err
}

return nil
}

Expand Down Expand Up @@ -203,6 +212,7 @@ func validateMaxVotingPowerEnforcementThreshold(i interface{}) error {
if v.IsNil() {
return fmt.Errorf("max voting power must be not nil")
}

if v.IsNegative() {
return fmt.Errorf("max voting power must be positive: %s", v)
}
Expand Down

0 comments on commit 8e7f327

Please sign in to comment.