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 4 commits
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
2 changes: 2 additions & 0 deletions x/ccv/provider/types/errors.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,6 @@ var (
ErrCannotCreateTopNChain = errorsmod.Register(ModuleName, 41, "cannot create Top N chain outside permissionlessly")
ErrCannotPrepareForLaunch = errorsmod.Register(ModuleName, 42, "cannot prepare chain for launch")
ErrInvalidStopTime = errorsmod.Register(ModuleName, 43, "invalid stop time")
ErrInvalidMsgCreateConsumer = errorsmod.Register(ModuleName, 44, "invalid create consumer message")
ErrInvalidMsgUpdateConsumer = errorsmod.Register(ModuleName, 45, "invalid update consumer message")
)
156 changes: 97 additions & 59 deletions x/ccv/provider/types/msg.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,17 @@ const (
TypeMsgOptIn = "opt_in"
TypeMsgOptOut = "opt_out"
TypeMsgSetConsumerCommissionRate = "set_consumer_commission_rate"

// MaxNameLength defines the maximum consumer name length
MaxNameLength = 50
// MaxDescriptionLength
mpoke marked this conversation as resolved.
Show resolved Hide resolved
MaxDescriptionLength = 10000
// MaxMetadataLength defines the maximum consumer metadata length
MaxMetadataLength = 255
// MaxHashLength defines the maximum length of a hash
MaxHashLength = 64
// MaxValidatorCount defines the maximum number of validators
MaxValidatorCount = 1000
)

var (
Expand Down Expand Up @@ -257,17 +268,17 @@ func (msg MsgCreateConsumer) Route() string { return RouterKey }

// ValidateBasic implements the sdk.Msg interface.
func (msg MsgCreateConsumer) ValidateBasic() error {
if err := ValidateField("chain id", msg.ChainId, cmttypes.MaxChainIDLen); err != nil {
return err
if err := ValidateStringField("ChainId", msg.ChainId, cmttypes.MaxChainIDLen); err != nil {
return errorsmod.Wrapf(ErrInvalidMsgCreateConsumer, "ChainId: %s", err.Error())
}

if err := ValidateConsumerMetadata(msg.Metadata); err != nil {
return err
return errorsmod.Wrapf(ErrInvalidMsgCreateConsumer, "Metadata: %s", err.Error())
}

if msg.InitializationParameters != nil {
if err := ValidateInitializationParameters(*msg.InitializationParameters); err != nil {
return err
return errorsmod.Wrapf(ErrInvalidMsgCreateConsumer, "InitializationParameters: %s", err.Error())
}
}

Expand All @@ -277,7 +288,7 @@ func (msg MsgCreateConsumer) ValidateBasic() error {
"first create the chain and then use `MsgUpdateConsumer` to make the chain Top N")
}
if err := ValidatePowerShapingParameters(*msg.PowerShapingParameters); err != nil {
return err
return errorsmod.Wrapf(ErrInvalidMsgCreateConsumer, "PowerShapingParameters: %s", err.Error())
}
}

Expand Down Expand Up @@ -325,24 +336,28 @@ func (msg MsgUpdateConsumer) Route() string { return RouterKey }
// ValidateBasic implements the sdk.Msg interface.
func (msg MsgUpdateConsumer) ValidateBasic() error {
if err := ValidateConsumerId(msg.ConsumerId); err != nil {
return err
return errorsmod.Wrapf(ErrInvalidMsgUpdateConsumer, "ConsumerId: %s", err.Error())
}

if err := ccvtypes.ValidateAccAddress(msg.NewOwnerAddress); err != nil {
mpoke marked this conversation as resolved.
Show resolved Hide resolved
return errorsmod.Wrapf(ErrInvalidMsgUpdateConsumer, "NewOwnerAddress: %s", err.Error())
}

if msg.Metadata != nil {
if err := ValidateConsumerMetadata(*msg.Metadata); err != nil {
return err
return errorsmod.Wrapf(ErrInvalidMsgUpdateConsumer, "Metadata: %s", err.Error())
}
}

if msg.InitializationParameters != nil {
if err := ValidateInitializationParameters(*msg.InitializationParameters); err != nil {
return err
return errorsmod.Wrapf(ErrInvalidMsgUpdateConsumer, "InitializationParameters: %s", err.Error())
}
}

if msg.PowerShapingParameters != nil {
if err := ValidatePowerShapingParameters(*msg.PowerShapingParameters); err != nil {
return err
return errorsmod.Wrapf(ErrInvalidMsgUpdateConsumer, "PowerShapingParameters: %s", err.Error())
}
}

Expand Down Expand Up @@ -736,8 +751,10 @@ func ValidateConsumerId(consumerId string) error {
return nil
}

// ValidateField validates that `field` is not empty and has at most `maxLength` characters
func ValidateField(nameOfTheField string, field string, maxLength int) error {
// ValidateStringField validates that a string `field` satisfies the following properties:
// - is not empty
// - has at most `maxLength` characters
func ValidateStringField(nameOfTheField string, field string, maxLength int) error {
mpoke marked this conversation as resolved.
Show resolved Hide resolved
Comment on lines +752 to +755
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggest formatting with gofumpt.

The function is correctly implemented. However, it should be formatted using gofumpt with the -extra flag to adhere to stricter formatting standards.

Apply this command to format the file:

gofumpt -w -extra msg.go

The logic of the function is approved.

Tools
golangci-lint

755-755: File is not gofumpt-ed with -extra

(gofumpt)

if strings.TrimSpace(field) == "" {
return fmt.Errorf("%s cannot be empty", nameOfTheField)
} else if len(field) > maxLength {
Expand All @@ -746,112 +763,133 @@ func ValidateField(nameOfTheField string, field string, maxLength int) error {
return nil
}

// TruncateString truncates a string to maximum length characters
func TruncateString(str string, length int) string {
Copy link
Contributor

@insumity insumity Aug 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

s/length/maxLength

Why not getting the minimum m of len(str) and maxLength and then return str[:m] instead?

Copy link
Contributor

@MSalopek MSalopek Aug 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Tend to agree, this is a one-liner that can be inlined in funcs. (taking Karolo's approach)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Used the solution here in case we want to have Chains with Japanese description :)

mpoke marked this conversation as resolved.
Show resolved Hide resolved
if length <= 0 {
return ""
}

truncated := ""
count := 0
for _, char := range str {
truncated += string(char)
count++
if count >= length {
break
}
}
return truncated
}

// ValidateConsumerMetadata validates that all the provided metadata are in the expected range
func ValidateConsumerMetadata(metadata ConsumerMetadata) error {
const maxNameLength = 100
const maxDescriptionLength = 50000
const maxMetadataLength = 1000

if err := ValidateField("name", metadata.Name, maxNameLength); err != nil {
return err
if err := ValidateStringField("name", metadata.Name, MaxNameLength); err != nil {
return errorsmod.Wrapf(ErrInvalidConsumerMetadata, "Name: %s", err.Error())
}

if err := ValidateField("description", metadata.Description, maxDescriptionLength); err != nil {
return err
if err := ValidateStringField("description", metadata.Description, MaxDescriptionLength); err != nil {
return errorsmod.Wrapf(ErrInvalidConsumerMetadata, "Description: %s", err.Error())
}

if err := ValidateField("metadata", metadata.Metadata, maxMetadataLength); err != nil {
return err
if err := ValidateStringField("metadata", metadata.Metadata, MaxMetadataLength); err != nil {
return errorsmod.Wrapf(ErrInvalidConsumerMetadata, "Metadata: %s", err.Error())
}

return nil
}

// ValidateConsAddressList validates a list of consensus addresses
func ValidateConsAddressList(list []string, maxLength int) error {
if len(list) > maxLength {
return fmt.Errorf("consensus address list too long; got: %d, max: %d", len(list), maxLength)
}
for _, address := range list {
_, err := sdk.ConsAddressFromBech32(address)
if err != nil {
return fmt.Errorf("invalid address %s: %s", address, err.Error())
}
}
return nil
}

// ValidatePowerShapingParameters validates that all the provided power-shaping parameters are in the expected range
func ValidatePowerShapingParameters(powerShapingParameters PowerShapingParameters) error {
const maxAllowlistLength = 500
const maxDenylistLength = 500

// Top N corresponds to the top N% of validators that have to validate the consumer chain and can only be 0 (for an
// Opt In chain) or in the range [50, 100] (for a Top N chain).
if powerShapingParameters.Top_N != 0 && (powerShapingParameters.Top_N < 50 || powerShapingParameters.Top_N > 100) {
return fmt.Errorf("parameter Top N can either be 0 or in the range [50, 100]")
return errorsmod.Wrap(ErrInvalidPowerShapingParameters, "Top N can either be 0 or in the range [50, 100]")
}

if powerShapingParameters.ValidatorsPowerCap != 0 && powerShapingParameters.ValidatorsPowerCap > 100 {
return fmt.Errorf("validators' power cap has to be in the range [1, 100]")
if powerShapingParameters.ValidatorsPowerCap > 100 {
return errorsmod.Wrap(ErrInvalidPowerShapingParameters, "ValidatorsPowerCap has to be in the range [0, 100]")
}

if len(powerShapingParameters.Allowlist) > maxAllowlistLength {
return fmt.Errorf("allowlist cannot exceed length: %d", maxAllowlistLength)
if err := ValidateConsAddressList(powerShapingParameters.Allowlist, MaxValidatorCount); err != nil {
return errorsmod.Wrapf(ErrInvalidPowerShapingParameters, "Allowlist: %s", err.Error())
}

if len(powerShapingParameters.Denylist) > maxDenylistLength {
return fmt.Errorf("denylist cannot exceed length: %d", maxDenylistLength)
if err := ValidateConsAddressList(powerShapingParameters.Denylist, MaxValidatorCount); err != nil {
return errorsmod.Wrapf(ErrInvalidPowerShapingParameters, "Denylist: %s", err.Error())
}

return nil
}

// ValidateInitializationParameters validates that all the provided parameters are in the expected range
func ValidateInitializationParameters(initializationParameters ConsumerInitializationParameters) error {
const maxGenesisHashLength = 1000
const maxBinaryHashLength = 1000
const maxConsumerRedistributionFractionLength = 50
const maxDistributionTransmissionChannelLength = 1000

if initializationParameters.InitialHeight.IsZero() {
return errorsmod.Wrap(ErrInvalidConsumerInitializationParameters, "initial height cannot be zero")
return errorsmod.Wrap(ErrInvalidConsumerInitializationParameters, "InitialHeight cannot be zero")
}

if len(initializationParameters.GenesisHash) == 0 {
return errorsmod.Wrap(ErrInvalidConsumerInitializationParameters, "genesis hash cannot be empty")
} else if len(initializationParameters.GenesisHash) > maxGenesisHashLength {
return errorsmod.Wrapf(ErrInvalidConsumerInitializationParameters, "genesis hash cannot exceed %d bytes", maxGenesisHashLength)
if err := validateHash(initializationParameters.GenesisHash); err != nil {
return errorsmod.Wrapf(ErrInvalidConsumerInitializationParameters, "GenesisHash: %s", err.Error())
}

if len(initializationParameters.BinaryHash) == 0 {
return errorsmod.Wrap(ErrInvalidConsumerInitializationParameters, "binary hash cannot be empty")
} else if len(initializationParameters.BinaryHash) > maxBinaryHashLength {
return errorsmod.Wrapf(ErrInvalidConsumerInitializationParameters, "binary hash cannot exceed %d bytes", maxBinaryHashLength)
if err := validateHash(initializationParameters.BinaryHash); err != nil {
return errorsmod.Wrapf(ErrInvalidConsumerInitializationParameters, "BinaryHash: %s", err.Error())
}

if initializationParameters.SpawnTime.IsZero() {
return errorsmod.Wrap(ErrInvalidConsumerInitializationParameters, "spawn time cannot be zero")
return errorsmod.Wrap(ErrInvalidConsumerInitializationParameters, "SpawnTime cannot be zero")
}

if err := ccvtypes.ValidateStringFraction(initializationParameters.ConsumerRedistributionFraction); err != nil {
return errorsmod.Wrapf(ErrInvalidConsumerInitializationParameters, "consumer redistribution fraction is invalid: %s", err.Error())
} else if err := ValidateField("consumer redistribution fraction", initializationParameters.ConsumerRedistributionFraction, maxConsumerRedistributionFractionLength); err != nil {
mpoke marked this conversation as resolved.
Show resolved Hide resolved
return errorsmod.Wrapf(ErrInvalidConsumerInitializationParameters, "consumer redistribution fraction is invalid: %s", err.Error())
return errorsmod.Wrapf(ErrInvalidConsumerInitializationParameters, "ConsumerRedistributionFraction: %s", err.Error())
}

if err := ccvtypes.ValidatePositiveInt64(initializationParameters.BlocksPerDistributionTransmission); err != nil {
return errorsmod.Wrap(ErrInvalidConsumerInitializationParameters, "blocks per distribution transmission has to be positive")
return errorsmod.Wrapf(ErrInvalidConsumerInitializationParameters, "BlocksPerDistributionTransmission: %s", err.Error())
}

if err := ccvtypes.ValidateDistributionTransmissionChannel(initializationParameters.DistributionTransmissionChannel); err != nil {
return errorsmod.Wrapf(ErrInvalidConsumerInitializationParameters, "distribution transmission channel is invalid: %s", err.Error())
} else if len(initializationParameters.DistributionTransmissionChannel) > maxDistributionTransmissionChannelLength {
// note that the distribution transmission channel can be the empty string (i.e., "") and hence we only check its max length here
return errorsmod.Wrapf(ErrInvalidConsumerInitializationParameters, "distribution transmission channel exceeds %d length", maxDistributionTransmissionChannelLength)
return errorsmod.Wrapf(ErrInvalidConsumerInitializationParameters, "DistributionTransmissionChannel: %s", err.Error())
}

if err := ccvtypes.ValidatePositiveInt64(initializationParameters.HistoricalEntries); err != nil {
return errorsmod.Wrap(ErrInvalidConsumerInitializationParameters, "historical entries has to be positive")
return errorsmod.Wrapf(ErrInvalidConsumerInitializationParameters, "HistoricalEntries: %s", err.Error())
}

if err := ccvtypes.ValidateDuration(initializationParameters.CcvTimeoutPeriod); err != nil {
return errorsmod.Wrap(ErrInvalidConsumerInitializationParameters, "ccv timeout period cannot be zero")
return errorsmod.Wrapf(ErrInvalidConsumerInitializationParameters, "CcvTimeoutPeriod: %s", err.Error())
}

if err := ccvtypes.ValidateDuration(initializationParameters.TransferTimeoutPeriod); err != nil {
return errorsmod.Wrap(ErrInvalidConsumerInitializationParameters, "transfer timeout period cannot be zero")
return errorsmod.Wrapf(ErrInvalidConsumerInitializationParameters, "TransferTimeoutPeriod: %s", err.Error())
}

if err := ccvtypes.ValidateDuration(initializationParameters.UnbondingPeriod); err != nil {
return errorsmod.Wrapf(ErrInvalidConsumerInitializationParameters, "invalid unbonding period: %s", err.Error())
return errorsmod.Wrapf(ErrInvalidConsumerInitializationParameters, "UnbondingPeriod: %s", err.Error())
}

return nil
}

// validateHash validates a hash
mpoke marked this conversation as resolved.
Show resolved Hide resolved
func validateHash(hash []byte) error {
if len(hash) == 0 {
return fmt.Errorf("hash cannot be empty")
}
if len(hash) > MaxHashLength {
return fmt.Errorf("hash is too long; got: %d, max: %d", len(hash), MaxHashLength)
}
return nil
}
Loading
Loading