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
Show file tree
Hide file tree
Changes from 3 commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
8 changes: 6 additions & 2 deletions baseapp/baseapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down
11 changes: 7 additions & 4 deletions baseapp/msg_service_router.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
7 changes: 6 additions & 1 deletion client/tx/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down
13 changes: 8 additions & 5 deletions tools/rosetta/converter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion tools/rosetta/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions tools/rosetta/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
19 changes: 11 additions & 8 deletions types/tx_msg.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand All @@ -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
Expand Down
7 changes: 6 additions & 1 deletion x/authz/msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down
6 changes: 3 additions & 3 deletions x/circuit/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@ require (
cosmossdk.io/store v0.1.0-alpha.1.0.20230328185921-37ba88872dbc
github.com/cosmos/cosmos-sdk v0.46.0-beta2.0.20230330094838-d21f58c638d5
github.com/cosmos/gogoproto v1.4.7
github.com/golang/protobuf v1.5.3
github.com/grpc-ecosystem/grpc-gateway v1.16.0
github.com/regen-network/gocuke v0.6.2
google.golang.org/genproto v0.0.0-20230320184635-7606e756e683
google.golang.org/grpc v1.54.0
gotest.tools/v3 v3.4.0
)
Expand Down Expand Up @@ -46,11 +49,9 @@ require (
github.com/go-logfmt/logfmt v0.6.0 // indirect
github.com/gofrs/uuid v4.3.0+incompatible // indirect
github.com/gogo/protobuf v1.3.2 // indirect
github.com/golang/protobuf v1.5.3 // indirect
github.com/golang/snappy v0.0.4 // indirect
github.com/google/btree v1.1.2 // indirect
github.com/google/go-cmp v0.5.9 // indirect
github.com/grpc-ecosystem/grpc-gateway v1.16.0 // indirect
github.com/gtank/merlin v0.1.1 // indirect
github.com/hashicorp/go-immutable-radix v1.3.1 // indirect
github.com/hashicorp/golang-lru v0.5.5-0.20210104140557-80c98217689d // indirect
Expand Down Expand Up @@ -91,7 +92,6 @@ require (
golang.org/x/net v0.8.0 // indirect
golang.org/x/sys v0.6.0 // indirect
golang.org/x/text v0.8.0 // indirect
google.golang.org/genproto v0.0.0-20230320184635-7606e756e683 // indirect
google.golang.org/protobuf v1.30.0 // indirect
gopkg.in/ini.v1 v1.67.0 // indirect
gopkg.in/yaml.v2 v2.4.0 // indirect
Expand Down
2 changes: 1 addition & 1 deletion x/feegrant/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions x/feegrant/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
10 changes: 0 additions & 10 deletions x/feegrant/msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down
6 changes: 4 additions & 2 deletions x/genutil/client/cli/gentx.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
7 changes: 5 additions & 2 deletions x/genutil/types/genesis_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 4 additions & 2 deletions x/gov/keeper/proposal.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
7 changes: 6 additions & 1 deletion x/gov/types/v1/msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()))
}
Expand Down
7 changes: 6 additions & 1 deletion x/group/msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down
2 changes: 1 addition & 1 deletion x/upgrade/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ require (
cosmossdk.io/errors v1.0.0-beta.7
cosmossdk.io/log v1.0.0
cosmossdk.io/store v0.1.0-alpha.1.0.20230328185921-37ba88872dbc
github.com/armon/go-metrics v0.4.1
github.com/cometbft/cometbft v0.37.0
github.com/cosmos/cosmos-db v1.0.0-rc.1
github.com/cosmos/cosmos-proto v1.0.0-beta.3
Expand Down Expand Up @@ -40,7 +41,6 @@ require (
github.com/99designs/keyring v1.2.1 // indirect
github.com/ChainSafe/go-schnorrkel v0.0.0-20200405005733-88cbf1b4c40d // indirect
github.com/DataDog/zstd v1.5.2 // indirect
github.com/armon/go-metrics v0.4.1 // indirect
github.com/aws/aws-sdk-go v1.44.224 // indirect
github.com/beorn7/perks v1.0.1 // indirect
github.com/bgentry/go-netrc v0.0.0-20140422174119-9fd32a8b3d3d // indirect
Expand Down