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.

Sign up for GitHub

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

Jump to bottom

feat: optional validate basic #15735

Merged
merged 5 commits into from
Apr 7, 2023
Merged

feat: optional validate basic #15735

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
64a355c
feat: optional validate basic
julienrbrt Apr 7, 2023
665fde2
updates
julienrbrt Apr 7, 2023
9325a4a
Merge branch 'main' into julien/optional-validate-basic
julienrbrt Apr 7, 2023
b689164
word
julienrbrt Apr 7, 2023
5427a00
Merge branch 'main' into julien/optional-validate-basic
julienrbrt Apr 7, 2023
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
View file
Open in desktop
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
1 change: 0 additions & 1 deletion UPGRADING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@ app.ConsensusParamsKeeper = consensusparamkeeper.NewKeeper(
)
```


### Packages

#### Store
Expand Down
8 changes: 6 additions & 2 deletions baseapp/baseapp.go
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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 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
Expand Down
7 changes: 6 additions & 1 deletion x/authz/msgs.go
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion x/feegrant/go.mod
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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