Skip to content

Commit

Permalink
apply review suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
@mpoke
mpoke committed Aug 30, 2024
1 parent 0a73edc commit 27c6167
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 34 deletions.
24 changes: 9 additions & 15 deletions x/ccv/provider/types/msg.go
View file
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 {
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
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",
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)
}
}
}

0 comments on commit 27c6167

Please sign in to comment.