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

Check and fix usages of error type ErrInvalidMsg #1310

Closed
webmaster128 opened this issue Apr 1, 2023 · 3 comments
Closed

Check and fix usages of error type ErrInvalidMsg #1310

webmaster128 opened this issue Apr 1, 2023 · 3 comments
Assignees
Milestone

Comments

@webmaster128
Copy link
Member

In an error ack I got error code 10, which is documented as

	// ErrInvalidMsg error when we cannot process the error returned from the contract
	ErrInvalidMsg = errorsmod.Register(DefaultCodespace, 10, "invalid CosmosMsg from the contract")

However, it seems like the error ErrInvalidMsg is used in places where no contract created CosmosMsg is in case, such as

func (msg MsgUpdateAdmin) ValidateBasic() error {
	if _, err := sdk.AccAddressFromBech32(msg.Sender); err != nil {
		return errorsmod.Wrap(err, "sender")
	}
	if _, err := sdk.AccAddressFromBech32(msg.Contract); err != nil {
		return errorsmod.Wrap(err, "contract")
	}
	if _, err := sdk.AccAddressFromBech32(msg.NewAdmin); err != nil {
		return errorsmod.Wrap(err, "new admin")
	}
	if strings.EqualFold(msg.Sender, msg.NewAdmin) {
		return errorsmod.Wrap(ErrInvalidMsg, "new admin is the same as the old") // <---- should not be ErrInvalidMsg
	}
	return nil
}

For other usages of ErrInvalidMsg it would also be good to double check if "invalid CosmosMsg from the contract" is the correct definition.

@alpe alpe added this to the v0.32.0 milestone Apr 17, 2023
@pinosu pinosu self-assigned this Apr 18, 2023
@pinosu
Copy link
Contributor

pinosu commented Apr 18, 2023

I just double checked and to me it seems that the only case where usage was not proper was solved in #1311.

@alpe
Copy link
Contributor

alpe commented Apr 21, 2023

Fixed in #1317 for v0.32

@alpe alpe closed this as completed Apr 21, 2023
@webmaster128
Copy link
Member Author

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants