feat: optional validate basic (#15735)

cosmos · Apr 7, 2023 · df161c2 · df161c2
1 parent ee70c14
commit df161c2
Show file tree

Hide file tree

Showing 18 changed files with 76 additions and 46 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -40,6 +40,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### Features
 
+* (types) [#15735](https://github.com/cosmos/cosmos-sdk/pull/15735) 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.

diff --git a/UPGRADING.md b/UPGRADING.md
@@ -82,7 +82,6 @@ app.ConsensusParamsKeeper = consensusparamkeeper.NewKeeper(
 )
 ```
 
-
 ### Packages
 
 #### Store

diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go
@@ -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 {
  return err
  }
  }

diff --git a/baseapp/msg_service_router.go b/baseapp/msg_service_router.go
@@ -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)

diff --git a/client/tx/tx.go b/client/tx/tx.go
@@ -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
  }
  }

diff --git a/tools/rosetta/converter.go b/tools/rosetta/converter.go
@@ -158,12 +158,15 @@ func (c converter) UnsignedTx(ops []*rosettatypes.Operation) (tx authsigning.Tx,
  }
 
  // verify message correctness
- if err = msg.ValidateBasic(); err != nil {
- return nil, crgerrs.WrapError(
- crgerrs.ErrBadArgument,
- fmt.Sprintf("validation of operation at index %d failed: %s", op.OperationIdentifier.Index, err),
- )
+ if m, ok := msg.(sdk.HasValidateBasic); ok {
+ if err = m.ValidateBasic(); err != nil {
+ return nil, crgerrs.WrapError(
+ crgerrs.ErrBadArgument,
+ fmt.Sprintf("validation of operation at index %d failed: %s", op.OperationIdentifier.Index, err),
+ )
+ }
  }
+
  signers := msg.GetSigners()
  // check if there are enough signers
  if len(signers) == 0 {

diff --git a/tools/rosetta/go.mod b/tools/rosetta/go.mod
@@ -7,7 +7,7 @@ require (
  cosmossdk.io/math v1.0.0
  github.com/coinbase/rosetta-sdk-go/types v1.0.0
  github.com/cometbft/cometbft v0.37.0
- github.com/cosmos/cosmos-sdk v0.46.0-beta2.0.20230330094838-d21f58c638d5
+ github.com/cosmos/cosmos-sdk v0.46.0-beta2.0.20230407105549-64a355c27736
  github.com/cosmos/rosetta-sdk-go v0.10.0
  github.com/decred/dcrd/dcrec/secp256k1/v4 v4.1.0
  github.com/spf13/cobra v1.7.0

diff --git a/tools/rosetta/go.sum b/tools/rosetta/go.sum
@@ -172,8 +172,8 @@ github.com/cosmos/cosmos-db v1.0.0-rc.1 h1:SjnT8B6WKMW9WEIX32qMhnEEKcI7ZP0+G1Sa9
 github.com/cosmos/cosmos-db v1.0.0-rc.1/go.mod h1:Dnmk3flSf5lkwCqvvjNpoxjpXzhxnCAFzKHlbaForso=
 github.com/cosmos/cosmos-proto v1.0.0-beta.3 h1:VitvZ1lPORTVxkmF2fAp3IiA61xVwArQYKXTdEcpW6o=
 github.com/cosmos/cosmos-proto v1.0.0-beta.3/go.mod h1:t8IASdLaAq+bbHbjq4p960BvcTqtwuAxid3b/2rOD6I=
-github.com/cosmos/cosmos-sdk v0.46.0-beta2.0.20230330094838-d21f58c638d5 h1:zO3mov9MaHWNnYZyQ8Wz/CZhrjfizMKvvLH41Ro/FYk=
-github.com/cosmos/cosmos-sdk v0.46.0-beta2.0.20230330094838-d21f58c638d5/go.mod h1:aKJRE3RjbwJUFGKG+kTDLhrST9vzFr8FlsTlv4eD+80=
+github.com/cosmos/cosmos-sdk v0.46.0-beta2.0.20230407105549-64a355c27736 h1:s5qqDSbWPBetLak/tiFUdz58atedY5hzEKu2XAZV2oA=
+github.com/cosmos/cosmos-sdk v0.46.0-beta2.0.20230407105549-64a355c27736/go.mod h1:nUeoGQBaRyGM1OQpQpj3AcdObVkmC6xS6AhyZ8dBZhY=
 github.com/cosmos/go-bip39 v0.0.0-20180819234021-555e2067c45d/go.mod h1:tSxLoYXyBmiFeKpvmq4dzayMdCjCnu8uqmCysIGBT2Y=
 github.com/cosmos/go-bip39 v1.0.0 h1:pcomnQdrdH22njcAatO0yWojsUnCO3y2tNoV1cb6hHY=
 github.com/cosmos/go-bip39 v1.0.0/go.mod h1:RNJv0H/pOIVgxw6KS7QeX2a0Uo0aKUlfhZ4xuwvCdJw=

diff --git a/types/tx_msg.go b/types/tx_msg.go
@@ -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 facultative.
+ // 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

diff --git a/x/authz/msgs.go b/x/authz/msgs.go
@@ -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
  }
  }

diff --git a/x/feegrant/go.mod b/x/feegrant/go.mod
@@ -12,7 +12,7 @@ require (
  cosmossdk.io/store v0.1.0-alpha.1.0.20230328185921-37ba88872dbc
  github.com/cometbft/cometbft v0.37.0
  github.com/cosmos/cosmos-proto v1.0.0-beta.3
- github.com/cosmos/cosmos-sdk v0.46.0-beta2.0.20230330132045-4b148aad6c22
+ github.com/cosmos/cosmos-sdk v0.46.0-beta2.0.20230407105549-64a355c27736
  github.com/cosmos/gogoproto v1.4.7
  github.com/golang/mock v1.6.0
  github.com/golang/protobuf v1.5.3

diff --git a/x/feegrant/go.sum b/x/feegrant/go.sum
@@ -186,8 +186,8 @@ github.com/cosmos/cosmos-db v1.0.0-rc.1 h1:SjnT8B6WKMW9WEIX32qMhnEEKcI7ZP0+G1Sa9
 github.com/cosmos/cosmos-db v1.0.0-rc.1/go.mod h1:Dnmk3flSf5lkwCqvvjNpoxjpXzhxnCAFzKHlbaForso=
 github.com/cosmos/cosmos-proto v1.0.0-beta.3 h1:VitvZ1lPORTVxkmF2fAp3IiA61xVwArQYKXTdEcpW6o=
 github.com/cosmos/cosmos-proto v1.0.0-beta.3/go.mod h1:t8IASdLaAq+bbHbjq4p960BvcTqtwuAxid3b/2rOD6I=
-github.com/cosmos/cosmos-sdk v0.46.0-beta2.0.20230330132045-4b148aad6c22 h1:3bslElsuLl+GXtUvIdf80zXKo2OehIS7P0beGRozfI4=
-github.com/cosmos/cosmos-sdk v0.46.0-beta2.0.20230330132045-4b148aad6c22/go.mod h1:elh/LpgsDux3TLyHshvqIvqAxbK1rYpBONS5TVzpFno=
+github.com/cosmos/cosmos-sdk v0.46.0-beta2.0.20230407105549-64a355c27736 h1:s5qqDSbWPBetLak/tiFUdz58atedY5hzEKu2XAZV2oA=
+github.com/cosmos/cosmos-sdk v0.46.0-beta2.0.20230407105549-64a355c27736/go.mod h1:nUeoGQBaRyGM1OQpQpj3AcdObVkmC6xS6AhyZ8dBZhY=
 github.com/cosmos/go-bip39 v0.0.0-20180819234021-555e2067c45d/go.mod h1:tSxLoYXyBmiFeKpvmq4dzayMdCjCnu8uqmCysIGBT2Y=
 github.com/cosmos/go-bip39 v1.0.0 h1:pcomnQdrdH22njcAatO0yWojsUnCO3y2tNoV1cb6hHY=
 github.com/cosmos/go-bip39 v1.0.0/go.mod h1:RNJv0H/pOIVgxw6KS7QeX2a0Uo0aKUlfhZ4xuwvCdJw=

diff --git a/x/feegrant/msgs.go b/x/feegrant/msgs.go
@@ -37,11 +37,6 @@ func NewMsgGrantAllowance(feeAllowance FeeAllowanceI, granter, grantee sdk.AccAd
  }, nil
 }
 
-// ValidateBasic implements the sdk.Msg interface.
-func (msg MsgGrantAllowance) ValidateBasic() error {
- return nil
-}
-
 // GetSigners gets the granter account associated with an allowance
 func (msg MsgGrantAllowance) GetSigners() []sdk.AccAddress {
  granter, _ := sdk.AccAddressFromBech32(msg.Granter)
@@ -77,11 +72,6 @@ func NewMsgRevokeAllowance(granter, grantee sdk.AccAddress) MsgRevokeAllowance {
  return MsgRevokeAllowance{Granter: granter.String(), Grantee: grantee.String()}
 }
 
-// ValidateBasic implements the sdk.Msg interface.
-func (msg MsgRevokeAllowance) ValidateBasic() error {
- return nil
-}
-
 // GetSigners gets the granter address associated with an Allowance
 // to revoke.
 func (msg MsgRevokeAllowance) GetSigners() []sdk.AccAddress {

diff --git a/x/genutil/client/cli/gentx.go b/x/genutil/client/cli/gentx.go
@@ -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 {

diff --git a/x/genutil/types/genesis_state.go b/x/genutil/types/genesis_state.go
@@ -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

diff --git a/x/gov/keeper/proposal.go b/x/gov/keeper/proposal.go
@@ -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()

diff --git a/x/gov/types/v1/msgs.go b/x/gov/types/v1/msgs.go
@@ -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()))
  }

diff --git a/x/group/msgs.go b/x/group/msgs.go
@@ -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)
  }
  }