Skip to content

Commit

Permalink
refactor: remove .Type() and .Route() from msgs (#14751)
Browse files Browse the repository at this point in the history
  • Loading branch information
julienrbrt authored Jan 24, 2023
1 parent bc0386e commit 8dbdfea
Show file tree
Hide file tree
Showing 48 changed files with 409 additions and 769 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

### Improvements

* (x/auth/tx) [#14751](https://github.com/cosmos/cosmos-sdk/pull/14751) Remove `.Type()` and `Route()` methods from all msgs and `legacytx.LegacyMsg` interface.
* [#14691](https://github.com/cosmos/cosmos-sdk/pull/14691) Change behavior of `sdk.StringifyEvents` to not flatten events attributes by events type.
* This change only affects ABCI message logs, and not the actual events.
* [#14692](https://github.com/cosmos/cosmos-sdk/pull/14692) Improve RPC queries error message when app is at height 0.
Expand Down Expand Up @@ -165,6 +166,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

### API Breaking Changes

* (x/simulation) [#14751](https://github.com/cosmos/cosmos-sdk/pull/14751) Remove the `MsgType` field from `simulation.OperationInput` struct.
* (crypto/keyring) [#13734](https://github.com/cosmos/cosmos-sdk/pull/13834) The keyring's `Sign` method now takes a new `signMode` argument. It is only used if the signing key is a Ledger hardware device. You can set it to 0 in all other cases.
* (x/evidence) [14724](https://github.com/cosmos/cosmos-sdk/pull/14724) Extract Evidence in its own go.mod and rename the package to `cosmossdk.io/x/evidence`.
* (x/nft) [#14725](https://github.com/cosmos/cosmos-sdk/pull/14725) Extract NFT in its own go.mod and rename the package to `cosmossdk.io/x/nft`.
Expand Down
7 changes: 2 additions & 5 deletions baseapp/baseapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -835,11 +835,8 @@ func createEvents(events sdk.Events, msg sdk.Msg) sdk.Events {

// verify that events have no module attribute set
if _, found := events.GetAttributes(sdk.AttributeKeyModule); !found {
// here we assume that routes module name is the second element of the route
// e.g. "cosmos.bank.v1beta1.MsgSend" => "bank"
moduleName := strings.Split(eventMsgName, ".")
if len(moduleName) > 1 {
msgEvent = msgEvent.AppendAttributes(sdk.NewAttribute(sdk.AttributeKeyModule, moduleName[1]))
if moduleName := sdk.GetModuleNameFromTypeURL(eventMsgName); moduleName != "" {
msgEvent = msgEvent.AppendAttributes(sdk.NewAttribute(sdk.AttributeKeyModule, moduleName))
}
}

Expand Down
2 changes: 0 additions & 2 deletions docs/docs/building-modules/02-messages-and-queries.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,6 @@ https://github.com/cosmos/cosmos-sdk/blob/v0.47.0-rc1/types/tx_msg.go#L14-L26

It extends `proto.Message` and contains the following methods:

* `Route() string`: Name of the route for this message. Typically all `message`s in a module have the same route, which is most often the module's name.
* `Type() string`: Type of the message, used primarily in [events](../core/08-events.md). This should return a message-specific `string`, typically the denomination of the message itself.
* [`ValidateBasic() error`](../basics/01-tx-lifecycle.md#ValidateBasic).
* `GetSignBytes() []byte`: Return the canonical byte representation of the message. Used to generate a signature.
* `GetSigners() []AccAddress`: Return the list of signers. The Cosmos SDK will make sure that each `message` contained in a transaction is signed by all the signers listed in the list returned by this method.
Expand Down
1 change: 0 additions & 1 deletion docs/docs/core/08-events.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ The following examples show how to query Events using the Cosmos SDK.
| ------------------------------------------------ | -------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `tx.height=23` | Query all transactions at height 23 |
| `message.action='/cosmos.bank.v1beta1.Msg/Send'` | Query all transactions containing a x/bank `Send` [Service `Msg`](../building-modules/03-msg-services.md). Note the `'`s around the value. |
| `message.action='send'` | Query all transactions containing a x/bank `Send` [legacy `Msg`](../building-modules/03-msg-services.md#legacy-amino-msgs). Note the `'`s around the value. |
| `message.module='bank'` | Query all transactions containing messages from the x/bank module. Note the `'`s around the value. |
| `create_validator.validator='cosmosval1...'` | x/staking-specific Event, see [x/staking SPEC](../modules/staking/README.md). |

Expand Down
2 changes: 0 additions & 2 deletions testutil/testdata/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,6 @@ func NewTestMsg(addrs ...sdk.AccAddress) *TestMsg {

var _ sdk.Msg = (*TestMsg)(nil)

func (msg *TestMsg) Route() string { return "TestMsg" }
func (msg *TestMsg) Type() string { return "Test message" }
func (msg *TestMsg) GetSignBytes() []byte {
bz, err := json.Marshal(msg.Signers)
if err != nil {
Expand Down
22 changes: 13 additions & 9 deletions types/simulation/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,10 @@ type OperationMsg struct {
}

// NewOperationMsgBasic creates a new operation message from raw input.
func NewOperationMsgBasic(route, name, comment string, ok bool, msg []byte) OperationMsg {
func NewOperationMsgBasic(moduleName, msgType, comment string, ok bool, msg []byte) OperationMsg {
return OperationMsg{
Route: route,
Name: name,
Route: moduleName,
Name: msgType,
Comment: comment,
OK: ok,
Msg: msg,
Expand All @@ -78,18 +78,22 @@ func NewOperationMsgBasic(route, name, comment string, ok bool, msg []byte) Oper

// NewOperationMsg - create a new operation message from sdk.Msg
func NewOperationMsg(msg sdk.Msg, ok bool, comment string, cdc *codec.ProtoCodec) OperationMsg {
if legacyMsg, okType := msg.(legacytx.LegacyMsg); okType {
return NewOperationMsgBasic(legacyMsg.Route(), legacyMsg.Type(), comment, ok, legacyMsg.GetSignBytes())
msgType := sdk.MsgTypeURL(msg)
moduleName := sdk.GetModuleNameFromTypeURL(msgType)
if moduleName == "" {
moduleName = msgType
}

bz := cdc.MustMarshalJSON(msg)
if legacyMsg, okType := msg.(legacytx.LegacyMsg); okType {
return NewOperationMsgBasic(moduleName, msgType, comment, ok, legacyMsg.GetSignBytes())
}

return NewOperationMsgBasic(sdk.MsgTypeURL(msg), sdk.MsgTypeURL(msg), comment, ok, bz)
return NewOperationMsgBasic(moduleName, msgType, comment, ok, cdc.MustMarshalJSON(msg))
}

// NoOpMsg - create a no-operation message
func NoOpMsg(route, msgType, comment string) OperationMsg {
return NewOperationMsgBasic(route, msgType, comment, false, nil)
func NoOpMsg(moduleName, msgType, comment string) OperationMsg {
return NewOperationMsgBasic(moduleName, msgType, comment, false, nil)
}

// log entry text for this operation msg
Expand Down
13 changes: 13 additions & 0 deletions types/tx_msg.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package types
import (
"encoding/json"
fmt "fmt"
strings "strings"

"github.com/cosmos/gogoproto/proto"

Expand Down Expand Up @@ -102,3 +103,15 @@ func GetMsgFromTypeURL(cdc codec.Codec, input string) (Msg, error) {

return msg, nil
}

// GetModuleNameFromTypeURL assumes that module name is the second element of the msg type URL
// e.g. "cosmos.bank.v1beta1.MsgSend" => "bank"
// It returns an empty string if the input is not a valid type URL
func GetModuleNameFromTypeURL(input string) string {
moduleName := strings.Split(input, ".")
if len(moduleName) > 1 {
return moduleName[1]
}

return ""
}
2 changes: 0 additions & 2 deletions types/tx_msg_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@ func (s *testMsgSuite) TestMsg() {
msg := testdata.NewTestMsg(accAddr)
s.Require().NotNil(msg)
s.Require().True(accAddr.Equals(msg.GetSigners()[0]))
s.Require().Equal("TestMsg", msg.Route())
s.Require().Equal("Test message", msg.Type())
s.Require().Nil(msg.ValidateBasic())
s.Require().NotPanics(func() { msg.GetSignBytes() })
}
Expand Down
12 changes: 2 additions & 10 deletions x/auth/migrations/legacytx/stdsign.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,22 +17,14 @@ import (
"github.com/cosmos/cosmos-sdk/types/tx/signing"
)

// LegacyMsg defines the old interface a message must fulfill, containing
// Amino signing method and legacy router info.
// LegacyMsg defines the old interface a message must fulfill,
// containing Amino signing method.
// Deprecated: Please use `Msg` instead.
type LegacyMsg interface {
sdk.Msg

// Get the canonical byte representation of the Msg.
GetSignBytes() []byte

// Return the message type.
// Must be alphanumeric or empty.
Route() string

// Returns a human-readable string for the message, intended for utilization
// within tags
Type() string
}

// StdSignDoc is replay-prevention structure.
Expand Down
10 changes: 7 additions & 3 deletions x/auth/types/msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,27 @@ package types
import (
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
"github.com/cosmos/cosmos-sdk/x/auth/migrations/legacytx"
)

var _ sdk.Msg = &MsgUpdateParams{}
var (
_ sdk.Msg = &MsgUpdateParams{}
_ legacytx.LegacyMsg = &MsgUpdateParams{}
)

// GetSignBytes implements the LegacyMsg interface.
func (msg MsgUpdateParams) GetSignBytes() []byte {
return sdk.MustSortJSON(ModuleCdc.MustMarshalJSON(&msg))
}

// GetSigners returns the expected signers for a MsgUpdateParams message.
func (msg *MsgUpdateParams) GetSigners() []sdk.AccAddress {
func (msg MsgUpdateParams) GetSigners() []sdk.AccAddress {
addr, _ := sdk.AccAddressFromBech32(msg.Authority)
return []sdk.AccAddress{addr}
}

// ValidateBasic does a sanity check on the provided data.
func (msg *MsgUpdateParams) ValidateBasic() error {
func (msg MsgUpdateParams) ValidateBasic() error {
if _, err := sdk.AccAddressFromBech32(msg.Authority); err != nil {
return sdkerrors.Wrap(err, "invalid authority address")
}
Expand Down
40 changes: 9 additions & 31 deletions x/auth/vesting/types/msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,18 @@ import (

sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
"github.com/cosmos/cosmos-sdk/x/auth/migrations/legacytx"
)

// TypeMsgCreateVestingAccount defines the type value for a MsgCreateVestingAccount.
const TypeMsgCreateVestingAccount = "msg_create_vesting_account"
var (
_ sdk.Msg = &MsgCreateVestingAccount{}
_ sdk.Msg = &MsgCreatePermanentLockedAccount{}
_ sdk.Msg = &MsgCreatePeriodicVestingAccount{}

// TypeMsgCreatePermanentLockedAccount defines the type value for a MsgCreatePermanentLockedAccount.
const TypeMsgCreatePermanentLockedAccount = "msg_create_permanent_locked_account"

// TypeMsgCreatePeriodicVestingAccount defines the type value for a MsgCreateVestingAccount.
const TypeMsgCreatePeriodicVestingAccount = "msg_create_periodic_vesting_account"

var _ sdk.Msg = &MsgCreateVestingAccount{}

var _ sdk.Msg = &MsgCreatePermanentLockedAccount{}

var _ sdk.Msg = &MsgCreatePeriodicVestingAccount{}
_ legacytx.LegacyMsg = &MsgCreateVestingAccount{}
_ legacytx.LegacyMsg = &MsgCreatePermanentLockedAccount{}
_ legacytx.LegacyMsg = &MsgCreatePeriodicVestingAccount{}
)

// NewMsgCreateVestingAccount returns a reference to a new MsgCreateVestingAccount.
//
Expand All @@ -35,12 +31,6 @@ func NewMsgCreateVestingAccount(fromAddr, toAddr sdk.AccAddress, amount sdk.Coin
}
}

// Route returns the message route for a MsgCreateVestingAccount.
func (msg MsgCreateVestingAccount) Route() string { return RouterKey }

// Type returns the message type for a MsgCreateVestingAccount.
func (msg MsgCreateVestingAccount) Type() string { return TypeMsgCreateVestingAccount }

// ValidateBasic Implements Msg.
func (msg MsgCreateVestingAccount) ValidateBasic() error {
if _, err := sdk.AccAddressFromBech32(msg.FromAddress); err != nil {
Expand Down Expand Up @@ -88,12 +78,6 @@ func NewMsgCreatePermanentLockedAccount(fromAddr, toAddr sdk.AccAddress, amount
}
}

// Route returns the message route for a MsgCreatePermanentLockedAccount.
func (msg MsgCreatePermanentLockedAccount) Route() string { return RouterKey }

// Type returns the message type for a MsgCreatePermanentLockedAccount.
func (msg MsgCreatePermanentLockedAccount) Type() string { return TypeMsgCreatePermanentLockedAccount }

// ValidateBasic Implements Msg.
func (msg MsgCreatePermanentLockedAccount) ValidateBasic() error {
if _, err := sdk.AccAddressFromBech32(msg.FromAddress); err != nil {
Expand Down Expand Up @@ -138,12 +122,6 @@ func NewMsgCreatePeriodicVestingAccount(fromAddr, toAddr sdk.AccAddress, startTi
}
}

// Route returns the message route for a MsgCreatePeriodicVestingAccount.
func (msg MsgCreatePeriodicVestingAccount) Route() string { return RouterKey }

// Type returns the message type for a MsgCreatePeriodicVestingAccount.
func (msg MsgCreatePeriodicVestingAccount) Type() string { return TypeMsgCreatePeriodicVestingAccount }

// GetSigners returns the expected signers for a MsgCreatePeriodicVestingAccount.
func (msg MsgCreatePeriodicVestingAccount) GetSigners() []sdk.AccAddress {
from, err := sdk.AccAddressFromBech32(msg.FromAddress)
Expand Down
30 changes: 0 additions & 30 deletions x/authz/msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,16 +66,6 @@ func (msg MsgGrant) ValidateBasic() error {
return msg.Grant.ValidateBasic()
}

// Type implements the LegacyMsg.Type method.
func (msg MsgGrant) Type() string {
return sdk.MsgTypeURL(&msg)
}

// Route implements the LegacyMsg.Route method.
func (msg MsgGrant) Route() string {
return sdk.MsgTypeURL(&msg)
}

// GetSignBytes implements the LegacyMsg.GetSignBytes method.
func (msg MsgGrant) GetSignBytes() []byte {
return sdk.MustSortJSON(authzcodec.ModuleCdc.MustMarshalJSON(&msg))
Expand Down Expand Up @@ -157,16 +147,6 @@ func (msg MsgRevoke) ValidateBasic() error {
return nil
}

// Type implements the LegacyMsg.Type method.
func (msg MsgRevoke) Type() string {
return sdk.MsgTypeURL(&msg)
}

// Route implements the LegacyMsg.Route method.
func (msg MsgRevoke) Route() string {
return sdk.MsgTypeURL(&msg)
}

// GetSignBytes implements the LegacyMsg.GetSignBytes method.
func (msg MsgRevoke) GetSignBytes() []byte {
return sdk.MustSortJSON(authzcodec.ModuleCdc.MustMarshalJSON(&msg))
Expand Down Expand Up @@ -235,16 +215,6 @@ func (msg MsgExec) ValidateBasic() error {
return nil
}

// Type implements the LegacyMsg.Type method.
func (msg MsgExec) Type() string {
return sdk.MsgTypeURL(&msg)
}

// Route implements the LegacyMsg.Route method.
func (msg MsgExec) Route() string {
return sdk.MsgTypeURL(&msg)
}

// GetSignBytes implements the LegacyMsg.GetSignBytes method.
func (msg MsgExec) GetSignBytes() []byte {
return sdk.MustSortJSON(authzcodec.ModuleCdc.MustMarshalJSON(&msg))
Expand Down
16 changes: 7 additions & 9 deletions x/authz/simulation/operations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,11 @@ func (suite *SimTestSuite) TestWeightedOperations() {
expected := []struct {
weight int
opMsgRoute string
opMsgName string
}{
{simulation.WeightGrant, simulation.TypeMsgGrant},
{simulation.WeightExec, simulation.TypeMsgExec},
{simulation.WeightRevoke, simulation.TypeMsgRevoke},
{simulation.WeightGrant, authz.ModuleName, simulation.TypeMsgGrant},
{simulation.WeightExec, authz.ModuleName, simulation.TypeMsgExec},
{simulation.WeightRevoke, authz.ModuleName, simulation.TypeMsgRevoke},
}

require := suite.Require()
Expand All @@ -83,12 +84,9 @@ func (suite *SimTestSuite) TestWeightedOperations() {
// the following checks are very much dependent from the ordering of the output given
// by WeightedOperations. if the ordering in WeightedOperations changes some tests
// will fail
require.Equal(expected[i].weight, w.Weight(),
"weight should be the same. %v", op.Comment)
require.Equal(expected[i].opMsgRoute, op.Route,
"route should be the same. %v", op.Comment)
require.Equal(expected[i].opMsgRoute, op.Name,
"operation Msg name should be the same %v", op.Comment)
require.Equal(expected[i].weight, w.Weight(), "weight should be the same. %v", op.Comment)
require.Equal(expected[i].opMsgRoute, op.Route, "route should be the same. %v", op.Comment)
require.Equal(expected[i].opMsgName, op.Name, "operation Msg name should be the same %v", op.Comment)
}
}

Expand Down
Loading

0 comments on commit 8dbdfea

Please sign in to comment.