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

feat: optional validate basic #15735

Merged
merged 5 commits into from
Apr 7, 2023
Merged
Changes from 1 commit
Commits
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
Next Next commit
feat: optional validate basic
julienrbrt committed Apr 7, 2023
commit 64a355c277366ddf26f3699889d8260813b2db24
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -40,6 +40,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

### Features

* (types) [#]() Make `ValidateBasic() error` method of `Msg` interface optional. Modules should validate messages directly in their message handlers ([RFC 001](https://docs.cosmos.network/main/rfc/rfc-001-tx-validation)).
* (x/genutil) [#15679](https://github.com/cosmos/cosmos-sdk/pull/15679) Allow applications to specify a custom genesis migration function for the `genesis migrate` command.
* (client) [#15458](https://github.com/cosmos/cosmos-sdk/pull/15458) Add a `CmdContext` field to client.Context initialized to cobra command's context.
* (core) [#15133](https://github.com/cosmos/cosmos-sdk/pull/15133) Implement RegisterServices in the module manager.
8 changes: 6 additions & 2 deletions baseapp/baseapp.go
Original file line number Diff line number Diff line change
@@ -522,8 +522,12 @@ func validateBasicTxMsgs(msgs []sdk.Msg) error {
}

for _, msg := range msgs {
err := msg.ValidateBasic()
if err != nil {
m, ok := msg.(sdk.HasValidateBasic)
if !ok {
continue
}

if err := m.ValidateBasic(); err != nil {
Comment on lines 522 to +530
Copy link
Contributor

Choose a reason for hiding this comment

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

Change potentially affects state.

Call sequence:

github.com/cosmos/cosmos-sdk/baseapp.validateBasicTxMsgs (baseapp/baseapp.go:519)
(*github.com/cosmos/cosmos-sdk/baseapp.BaseApp).runTx (baseapp/baseapp.go:610)
(*github.com/cosmos/cosmos-sdk/baseapp.BaseApp).DeliverTx (baseapp/baseapp.go:410)

return err
}
}
11 changes: 7 additions & 4 deletions baseapp/msg_service_router.go
Original file line number Diff line number Diff line change
@@ -115,16 +115,19 @@ func (msr *MsgServiceRouter) RegisterService(sd *grpc.ServiceDesc, handler inter
)
}

msr.routes[requestTypeName] = func(ctx sdk.Context, req sdk.Msg) (*sdk.Result, error) {
msr.routes[requestTypeName] = func(ctx sdk.Context, msg sdk.Msg) (*sdk.Result, error) {
ctx = ctx.WithEventManager(sdk.NewEventManager())
interceptor := func(goCtx context.Context, _ interface{}, _ *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (interface{}, error) {
goCtx = context.WithValue(goCtx, sdk.SdkContextKey, ctx)
return handler(goCtx, req)
return handler(goCtx, msg)
}

if err := req.ValidateBasic(); err != nil {
return nil, err
if m, ok := msg.(sdk.HasValidateBasic); ok {
if err := m.ValidateBasic(); err != nil {
return nil, err
}
}

// Call the method handler from the service description with the handler object.
// We don't do any decoding here because the decoding was already done.
res, err := methodHandler(handler, ctx, noopDecoder, interceptor)
7 changes: 6 additions & 1 deletion client/tx/tx.go
Original file line number Diff line number Diff line change
@@ -40,7 +40,12 @@ func GenerateOrBroadcastTxWithFactory(clientCtx client.Context, txf Factory, msg
// Right now, we're factorizing that call inside this function.
// ref: https://github.com/cosmos/cosmos-sdk/pull/9236#discussion_r623803504
for _, msg := range msgs {
if err := msg.ValidateBasic(); err != nil {
m, ok := msg.(sdk.HasValidateBasic)
if !ok {
continue
}

if err := m.ValidateBasic(); err != nil {
return err
}
}
19 changes: 11 additions & 8 deletions types/tx_msg.go
Original file line number Diff line number Diff line change
@@ -16,10 +16,6 @@ type (
Msg interface {
proto.Message

// ValidateBasic does a simple validation check that
// doesn't require access to any other information.
ValidateBasic() error

// GetSigners returns the addrs of signers that must sign.
// CONTRACT: All signatures must be present to be valid.
// CONTRACT: Returns addrs in some deterministic order.
@@ -42,12 +38,10 @@ type (

// Tx defines the interface a transaction must fulfill.
Tx interface {
HasValidateBasic

// GetMsgs gets the all the transaction's messages.
GetMsgs() []Msg

// ValidateBasic does a simple and lightweight validation check that doesn't
// require access to any other information.
ValidateBasic() error
}

// FeeTx defines the interface to be implemented by Tx to use the FeeDecorators
@@ -72,6 +66,15 @@ type (

GetTimeoutHeight() uint64
}

// HasValidateBasic defines a type that has a ValidateBasic method.
// ValidateBasic is deprecated and now faculatative.
// Prefer validating messages directly in the msg server.
HasValidateBasic interface {
// ValidateBasic does a simple validation check that
// doesn't require access to any other information.
ValidateBasic() error
}
)

// TxDecoder unmarshals transaction bytes
7 changes: 6 additions & 1 deletion x/authz/msgs.go
Original file line number Diff line number Diff line change
@@ -201,7 +201,12 @@ func (msg MsgExec) ValidateBasic() error {
return err
}
for _, msg := range msgs {
if err = msg.ValidateBasic(); err != nil {
m, ok := msg.(sdk.HasValidateBasic)
if !ok {
continue
}

if err = m.ValidateBasic(); err != nil {
return err
}
}
6 changes: 4 additions & 2 deletions x/genutil/client/cli/gentx.go
Original file line number Diff line number Diff line change
@@ -166,8 +166,10 @@ $ %s gentx my-key-name 1000000stake --home=/path/to/home/dir --keyring-backend=o
w := bytes.NewBuffer([]byte{})
clientCtx = clientCtx.WithOutput(w)

if err = msg.ValidateBasic(); err != nil {
return err
if m, ok := msg.(sdk.HasValidateBasic); ok {
if err := m.ValidateBasic(); err != nil {
return err
}
}

if err = txBldr.PrintUnsignedTx(clientCtx, msg); err != nil {
7 changes: 5 additions & 2 deletions x/genutil/types/genesis_state.go
Original file line number Diff line number Diff line change
@@ -109,8 +109,11 @@ func DefaultMessageValidator(msgs []sdk.Msg) error {
if _, ok := msgs[0].(*stakingtypes.MsgCreateValidator); !ok {
return fmt.Errorf("unexpected GenTx message type; expected: MsgCreateValidator, got: %T", msgs[0])
}
if err := msgs[0].ValidateBasic(); err != nil {
return fmt.Errorf("invalid GenTx '%s': %w", msgs[0], err)

if m, ok := msgs[0].(sdk.HasValidateBasic); ok {
if err := m.ValidateBasic(); err != nil {
return fmt.Errorf("invalid GenTx '%s': %w", msgs[0], err)
}
}

return nil
6 changes: 4 additions & 2 deletions x/gov/keeper/proposal.go
Original file line number Diff line number Diff line change
@@ -42,8 +42,10 @@ func (keeper Keeper) SubmitProposal(ctx sdk.Context, messages []sdk.Msg, metadat
msgsStr += fmt.Sprintf(",%s", sdk.MsgTypeURL(msg))

// perform a basic validation of the message
if err := msg.ValidateBasic(); err != nil {
return v1.Proposal{}, errorsmod.Wrap(types.ErrInvalidProposalMsg, err.Error())
if m, ok := msg.(sdk.HasValidateBasic); ok {
if err := m.ValidateBasic(); err != nil {
return v1.Proposal{}, errorsmod.Wrap(types.ErrInvalidProposalMsg, err.Error())
}
}

signers := msg.GetSigners()
7 changes: 6 additions & 1 deletion x/gov/types/v1/msgs.go
Original file line number Diff line number Diff line change
@@ -98,7 +98,12 @@ func (m MsgSubmitProposal) ValidateBasic() error {
}

for idx, msg := range msgs {
if err := msg.ValidateBasic(); err != nil {
m, ok := msg.(sdk.HasValidateBasic)
if !ok {
continue
}

if err := m.ValidateBasic(); err != nil {
return errorsmod.Wrap(types.ErrInvalidProposalMsg,
fmt.Sprintf("msg: %d, err: %s", idx, err.Error()))
}
7 changes: 6 additions & 1 deletion x/group/msgs.go
Original file line number Diff line number Diff line change
@@ -588,7 +588,12 @@ func (m MsgSubmitProposal) ValidateBasic() error {
}

for i, msg := range msgs {
if err := msg.ValidateBasic(); err != nil {
m, ok := msg.(sdk.HasValidateBasic)
if !ok {
continue
}

if err := m.ValidateBasic(); err != nil {
return errorsmod.Wrapf(err, "msg %d", i)
}
}