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.

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

chore: improve the validations of parameters #312

Merged
merged 1 commit into from
Sep 15, 2023

Conversation

forcodedancing
Copy link
Collaborator

@forcodedancing forcodedancing commented Sep 15, 2023

Description

Improve the validation of parameters.

Rationale

Chore

Example

NA

Changes

Notable changes:

  • parameter validations

@@ -36,6 +38,11 @@ func (m *CrossChainParamsChange) ValidateBasic() error {
if err != nil {
return types.ErrAddressNotValid
}
} else {
_, err := hex.DecodeString(value)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is a hardfork change?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

func (k Keeper) SyncParams(ctx sdk.Context, destChainId sdk.ChainID, cpc govv1.CrossChainParamsChange) error {
	if err := cpc.ValidateBasic(); err != nil {
		return err
	}
	values := make([]byte, 0)
	addresses := make([]byte, 0)

	for i, v := range cpc.Values {
		var value []byte
		var err error
		if cpc.Key == types.KeyUpgrade {
			value = sdk.MustAccAddressFromHex(v)
		} else {
			value, err = hex.DecodeString(v)
			if err != nil {
				return sdkerrors.Wrapf(types.ErrInvalidValue, "value is not valid %s", v)
			}
		}
		values = append(values, value...)
		addr := sdk.MustAccAddressFromHex(cpc.Targets[i])
		addresses = append(addresses, addr.Bytes()...)
	}
...

This is also checked in the keeper.

@unclezoro unclezoro added this pull request to the merge queue Sep 15, 2023
Merged via the queue into bnb-chain:develop with commit bf87a12 Sep 15, 2023
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants