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

msg.ValidateBasic is not called in ante handler #12607

Closed
4 tasks
yihuang opened this issue Jul 18, 2022 · 4 comments
Closed
4 tasks

msg.ValidateBasic is not called in ante handler #12607

yihuang opened this issue Jul 18, 2022 · 4 comments

Comments

@yihuang
Copy link
Collaborator

yihuang commented Jul 18, 2022

Summary of Bug

Is it expected that msg.ValidateBasic is not called in ante handler, it only calls tx.ValidateBasic which don't call msgs's.

I tried this to verify, patch TestMsg as follows:

diff --git a/testutil/testdata/tx.go b/testutil/testdata/tx.go
index f0b470aade..2d0b7c97be 100644
--- a/testutil/testdata/tx.go
+++ b/testutil/testdata/tx.go
@@ -2,6 +2,7 @@ package testdata

 import (
        "encoding/json"
+       "errors"

        "github.com/stretchr/testify/require"

@@ -76,7 +77,7 @@ func (msg *TestMsg) GetSigners() []sdk.AccAddress {

        return addrs
 }
-func (msg *TestMsg) ValidateBasic() error { return nil }
+func (msg *TestMsg) ValidateBasic() error { return errors.New("test") }

 var _ sdk.Msg = &MsgCreateDog{}

And run go test -v ./x/auth/ante/..., all tests still passes.

Version

main/0.46/0.45

Steps to Reproduce


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@amaury1093
Copy link
Contributor

I'm pretty sure it's called here:

if err := validateBasicTxMsgs(msgs); err != nil {
.

Do you have a branch with a reproducible test that sends a MsgCreateDog?

@yihuang
Copy link
Collaborator Author

yihuang commented Jul 18, 2022

I'm pretty sure it's called here:

if err := validateBasicTxMsgs(msgs); err != nil {

.
Do you have a branch with a reproducible test that sends a MsgCreateDog?

I see, I was working on some ante handler related unit tests, and surprised to find it's not called there. Is there a particular reason that tx and msg's ValidateBasic are called at different places?

@amaury1093
Copy link
Contributor

No reason, other than confusing code design. One proposal was for baseapp to do almost nothing (other than holding a reference to the antehandler stack), and putting all logic into antehandlers.

It was attempted in the middlewares ADR, but reverted. I propose to close this issue, and follow-up in #11955 ?

@yihuang
Copy link
Collaborator Author

yihuang commented Jul 18, 2022

👌

@yihuang yihuang closed this as completed Jul 18, 2022
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

2 participants