Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix!: msgs validation #2195

Merged
merged 5 commits into from
Aug 30, 2024
Merged

fix!: msgs validation #2195

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5791be6
add testing for ValidateStringField
mpoke Aug 30, 2024
ae6e830
ValidateInitializationParameters
mpoke Aug 30, 2024
9dbdb0a
ValidatePowerShapingParameters
mpoke Aug 30, 2024
0a73edc
validate NewOwnerAddress
mpoke Aug 30, 2024
27c6167
apply review suggestions
mpoke Aug 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 9 additions & 15 deletions x/ccv/provider/types/msg.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ const (

// MaxNameLength defines the maximum consumer name length
MaxNameLength = 50
// MaxDescriptionLength
// MaxDescriptionLength defines the maximum consumer description length
MaxDescriptionLength = 10000
// MaxMetadataLength defines the maximum consumer metadata length
MaxMetadataLength = 255
Expand Down Expand Up @@ -339,9 +339,7 @@ func (msg MsgUpdateConsumer) ValidateBasic() error {
return errorsmod.Wrapf(ErrInvalidMsgUpdateConsumer, "ConsumerId: %s", err.Error())
}

if err := ccvtypes.ValidateAccAddress(msg.NewOwnerAddress); err != nil {
return errorsmod.Wrapf(ErrInvalidMsgUpdateConsumer, "NewOwnerAddress: %s", err.Error())
}
// Note that NewOwnerAddress is validated when handling the message in UpdateConsumer

if msg.Metadata != nil {
if err := ValidateConsumerMetadata(*msg.Metadata); err != nil {
Expand Down Expand Up @@ -764,8 +762,8 @@ func ValidateStringField(nameOfTheField string, field string, maxLength int) err
}

// TruncateString truncates a string to maximum length characters
func TruncateString(str string, length int) string {
if length <= 0 {
func TruncateString(str string, maxLength int) string {
if maxLength <= 0 {
return ""
}

Expand All @@ -774,7 +772,7 @@ func TruncateString(str string, length int) string {
for _, char := range str {
truncated += string(char)
count++
if count >= length {
if count >= maxLength {
break
}
}
Expand Down Expand Up @@ -840,11 +838,11 @@ func ValidateInitializationParameters(initializationParameters ConsumerInitializ
return errorsmod.Wrap(ErrInvalidConsumerInitializationParameters, "InitialHeight cannot be zero")
}

if err := validateHash(initializationParameters.GenesisHash); err != nil {
if err := ValidateByteSlice(initializationParameters.GenesisHash, MaxHashLength); err != nil {
return errorsmod.Wrapf(ErrInvalidConsumerInitializationParameters, "GenesisHash: %s", err.Error())
}

if err := validateHash(initializationParameters.BinaryHash); err != nil {
if err := ValidateByteSlice(initializationParameters.BinaryHash, MaxHashLength); err != nil {
return errorsmod.Wrapf(ErrInvalidConsumerInitializationParameters, "BinaryHash: %s", err.Error())
}

Expand Down Expand Up @@ -883,12 +881,8 @@ func ValidateInitializationParameters(initializationParameters ConsumerInitializ
return nil
}

// validateHash validates a hash
func validateHash(hash []byte) error {
if len(hash) == 0 {
return fmt.Errorf("hash cannot be empty")
}
if len(hash) > MaxHashLength {
func ValidateByteSlice(hash []byte, maxLength int) error {
if len(hash) > maxLength {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't we care if it's 0 anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To allow for the binary hash and the genesis hash to be empty as they are not checked anywhere. If we start doing something with them, then we add them.

return fmt.Errorf("hash is too long; got: %d, max: %d", len(hash), MaxHashLength)
}
return nil
Expand Down
75 changes: 56 additions & 19 deletions x/ccv/provider/types/msg_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,8 @@ func TestValidateInitializationParameters(t *testing.T) {
name: "valid",
params: types.ConsumerInitializationParameters{
InitialHeight: clienttypes.NewHeight(3, 4),
GenesisHash: []byte{0x01}, // cannot be empty
BinaryHash: []byte{0x01}, // cannot be empty
GenesisHash: []byte{0x01},
BinaryHash: []byte{0x01},
SpawnTime: now,
UnbondingPeriod: time.Duration(100000000000),
CcvTimeoutPeriod: time.Duration(100000000000),
Expand All @@ -112,8 +112,8 @@ func TestValidateInitializationParameters(t *testing.T) {
name: "invalid - zero height",
params: types.ConsumerInitializationParameters{
InitialHeight: clienttypes.ZeroHeight(),
GenesisHash: []byte{0x01}, // cannot be empty
BinaryHash: []byte{0x01}, // cannot be empty
GenesisHash: []byte{0x01},
BinaryHash: []byte{0x01},
SpawnTime: now,
UnbondingPeriod: time.Duration(100000000000),
CcvTimeoutPeriod: time.Duration(100000000000),
Expand All @@ -130,7 +130,7 @@ func TestValidateInitializationParameters(t *testing.T) {
params: types.ConsumerInitializationParameters{
InitialHeight: clienttypes.NewHeight(3, 4),
GenesisHash: tooLongHash,
BinaryHash: []byte{0x01}, // cannot be empty
BinaryHash: []byte{0x01},
SpawnTime: now,
UnbondingPeriod: time.Duration(100000000000),
CcvTimeoutPeriod: time.Duration(100000000000),
Expand All @@ -146,8 +146,8 @@ func TestValidateInitializationParameters(t *testing.T) {
name: "invalid - zero spawn time",
params: types.ConsumerInitializationParameters{
InitialHeight: clienttypes.NewHeight(3, 4),
GenesisHash: []byte{0x01}, // cannot be empty
BinaryHash: []byte{0x01}, // cannot be empty
GenesisHash: []byte{0x01},
BinaryHash: []byte{0x01},
SpawnTime: time.Time{},
UnbondingPeriod: time.Duration(100000000000),
CcvTimeoutPeriod: time.Duration(100000000000),
Expand All @@ -163,8 +163,8 @@ func TestValidateInitializationParameters(t *testing.T) {
name: "invalid - zero duration",
mpoke marked this conversation as resolved.
Show resolved Hide resolved
params: types.ConsumerInitializationParameters{
InitialHeight: clienttypes.NewHeight(3, 4),
GenesisHash: []byte{0x01}, // cannot be empty
BinaryHash: []byte{0x01}, // cannot be empty
GenesisHash: []byte{0x01},
BinaryHash: []byte{0x01},
SpawnTime: now,
UnbondingPeriod: 0,
CcvTimeoutPeriod: time.Duration(100000000000),
Expand All @@ -180,8 +180,8 @@ func TestValidateInitializationParameters(t *testing.T) {
name: "invalid -- ConsumerRedistributionFraction > 1",
params: types.ConsumerInitializationParameters{
InitialHeight: clienttypes.NewHeight(3, 4),
GenesisHash: []byte{0x01}, // cannot be empty
BinaryHash: []byte{0x01}, // cannot be empty
GenesisHash: []byte{0x01},
BinaryHash: []byte{0x01},
SpawnTime: now,
UnbondingPeriod: time.Duration(100000000000),
CcvTimeoutPeriod: time.Duration(100000000000),
Expand All @@ -197,8 +197,8 @@ func TestValidateInitializationParameters(t *testing.T) {
name: "invalid -- ConsumerRedistributionFraction wrong format",
params: types.ConsumerInitializationParameters{
InitialHeight: clienttypes.NewHeight(3, 4),
GenesisHash: []byte{0x01}, // cannot be empty
BinaryHash: []byte{0x01}, // cannot be empty
GenesisHash: []byte{0x01},
BinaryHash: []byte{0x01},
SpawnTime: now,
UnbondingPeriod: time.Duration(100000000000),
CcvTimeoutPeriod: time.Duration(100000000000),
Expand All @@ -214,8 +214,8 @@ func TestValidateInitializationParameters(t *testing.T) {
name: "invalid - BlocksPerDistributionTransmission zero",
params: types.ConsumerInitializationParameters{
InitialHeight: clienttypes.NewHeight(3, 4),
GenesisHash: []byte{0x01}, // cannot be empty
BinaryHash: []byte{0x01}, // cannot be empty
GenesisHash: []byte{0x01},
BinaryHash: []byte{0x01},
SpawnTime: now,
UnbondingPeriod: time.Duration(100000000000),
CcvTimeoutPeriod: time.Duration(100000000000),
Expand All @@ -231,8 +231,8 @@ func TestValidateInitializationParameters(t *testing.T) {
name: "invalid - HistoricalEntries zero",
params: types.ConsumerInitializationParameters{
InitialHeight: clienttypes.NewHeight(3, 4),
GenesisHash: []byte{0x01}, // cannot be empty
BinaryHash: []byte{0x01}, // cannot be empty
GenesisHash: []byte{0x01},
BinaryHash: []byte{0x01},
SpawnTime: now,
UnbondingPeriod: time.Duration(100000000000),
CcvTimeoutPeriod: time.Duration(100000000000),
Expand All @@ -248,8 +248,8 @@ func TestValidateInitializationParameters(t *testing.T) {
name: "invalid - DistributionTransmissionChannel too long",
params: types.ConsumerInitializationParameters{
InitialHeight: clienttypes.NewHeight(3, 4),
GenesisHash: []byte{0x01}, // cannot be empty
BinaryHash: []byte{0x01}, // cannot be empty
GenesisHash: []byte{0x01},
BinaryHash: []byte{0x01},
SpawnTime: now,
UnbondingPeriod: time.Duration(100000000000),
CcvTimeoutPeriod: time.Duration(100000000000),
Expand Down Expand Up @@ -325,3 +325,40 @@ func TestValidateConsAddressList(t *testing.T) {
}
}
}

func TestValidateByteSlice(t *testing.T) {
testCases := []struct {
name string
slice []byte
maxLength int
valid bool
}{
{
name: "valid: empty",
slice: []byte{},
maxLength: 5,
valid: true,
},
{
name: "invalid: too long",
slice: []byte{0x01, 0x02},
maxLength: 1,
valid: false,
},
{
name: "valid",
slice: []byte{0x01, 0x02},
maxLength: 5,
valid: true,
},
}

for _, tc := range testCases {
err := types.ValidateByteSlice(tc.slice, tc.maxLength)
if tc.valid {
require.NoError(t, err, tc.name)
} else {
require.Error(t, err, tc.name)
}
}
}
Loading