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

revert: replace all ModuleCdc instances with legacy.Cdc #11680

Merged
merged 7 commits into from
Apr 20, 2022
Merged

revert: replace all ModuleCdc instances with legacy.Cdc #11680

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6364ac7
revert: replace all ModuleCdc instances with legacy.Cdc
RiccardoM Apr 19, 2022
d6b31a2
updated signing docs
RiccardoM Apr 19, 2022
56b90c0
added gov/v1beta1 LegacyCodec to gov/v1
RiccardoM Apr 19, 2022
421cb75
Merge branch 'master' into riccardom/revert-global-cdc
alexanderbez Apr 19, 2022
5bd449a
Merge branch 'master' of github.com:cosmos/cosmos-sdk into riccardom/…
RiccardoM Apr 20, 2022
cf0d175
Added codec package docs
RiccardoM Apr 20, 2022
7e9dce8
Merge remote-tracking branch 'origin/riccardom/revert-global-cdc' int…
RiccardoM Apr 20, 2022
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
8 changes: 4 additions & 4 deletions docs/core/encoding.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,12 @@ Since the `MsgExec` message type can contain different messages instances, it is
add the following code inside the `init` method of their module's `codec.go` file:

```go
import "github.com/cosmos/cosmos-sdk/codec/legacy"
import authzcodec "github.com/cosmos/cosmos-sdk/x/authz/codec"

init() {
// Register all Amino interfaces and concrete types on the global Amino codec so that this can later be
// used to properly serialize x/authz MsgExec instances
RegisterLegacyAminoCodec(legacy.Cdc)
// Register all Amino interfaces and concrete types on the authz Amino codec so that this can later be
// used to properly serialize MsgGrant and MsgExec instances
RegisterLegacyAminoCodec(authzcodec.Amino)
}
```

Expand Down
17 changes: 15 additions & 2 deletions x/auth/types/codec.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@ package types

import (
"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/codec/legacy"
"github.com/cosmos/cosmos-sdk/codec/types"
cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/x/auth/migrations/legacytx"
authzcodec "github.com/cosmos/cosmos-sdk/x/authz/codec"
)

// RegisterLegacyAminoCodec registers the account interfaces and concrete types on the
Expand Down Expand Up @@ -37,6 +39,17 @@ func RegisterInterfaces(registry types.InterfaceRegistry) {
)
}

var (
amino = codec.NewLegacyAmino()
ModuleCdc = codec.NewAminoCodec(amino)
)

func init() {
RegisterLegacyAminoCodec(legacy.Cdc)
RegisterLegacyAminoCodec(amino)
cryptocodec.RegisterCrypto(amino)
sdk.RegisterLegacyAminoCodec(amino)

// Register all Amino interfaces and concrete types on the authz Amino codec so that this can later be
// used to properly serialize MsgGrant and MsgExec instances
RegisterLegacyAminoCodec(authzcodec.Amino)
}
15 changes: 14 additions & 1 deletion x/auth/vesting/types/codec.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@ import (
"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/codec/legacy"
"github.com/cosmos/cosmos-sdk/codec/types"
cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/msgservice"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
"github.com/cosmos/cosmos-sdk/x/auth/vesting/exported"
authzcodec "github.com/cosmos/cosmos-sdk/x/authz/codec"
)

// RegisterLegacyAminoCodec registers the vesting interfaces and concrete types on the
Expand Down Expand Up @@ -62,6 +64,17 @@ func RegisterInterfaces(registry types.InterfaceRegistry) {
msgservice.RegisterMsgServiceDesc(registry, &_Msg_serviceDesc)
}

var (
amino = codec.NewLegacyAmino()
ModuleCdc = codec.NewAminoCodec(amino)
)

func init() {
RegisterLegacyAminoCodec(legacy.Cdc)
RegisterLegacyAminoCodec(amino)
cryptocodec.RegisterCrypto(amino)
sdk.RegisterLegacyAminoCodec(amino)

// Register all Amino interfaces and concrete types on the authz Amino codec so that this can later be
// used to properly serialize MsgGrant and MsgExec instances
RegisterLegacyAminoCodec(authzcodec.Amino)
}
8 changes: 3 additions & 5 deletions x/auth/vesting/types/msgs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ package types

import (
"fmt"
"github.com/cosmos/cosmos-sdk/codec/legacy"

sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
)
Expand Down Expand Up @@ -68,7 +66,7 @@ func (msg MsgCreateVestingAccount) ValidateBasic() error {
// GetSignBytes returns the bytes all expected signers must sign over for a
// MsgCreateVestingAccount.
func (msg MsgCreateVestingAccount) GetSignBytes() []byte {
return sdk.MustSortJSON(legacy.Cdc.MustMarshalJSON(&msg))
return sdk.MustSortJSON(ModuleCdc.MustMarshalJSON(&msg))
}

// GetSigners returns the expected signers for a MsgCreateVestingAccount.
Expand Down Expand Up @@ -116,7 +114,7 @@ func (msg MsgCreatePermanentLockedAccount) ValidateBasic() error {
// GetSignBytes returns the bytes all expected signers must sign over for a
// MsgCreatePermanentLockedAccount.
func (msg MsgCreatePermanentLockedAccount) GetSignBytes() []byte {
return sdk.MustSortJSON(legacy.Cdc.MustMarshalJSON(&msg))
return sdk.MustSortJSON(ModuleCdc.MustMarshalJSON(&msg))
}

// GetSigners returns the expected signers for a MsgCreatePermanentLockedAccount.
Expand Down Expand Up @@ -154,7 +152,7 @@ func (msg MsgCreatePeriodicVestingAccount) GetSigners() []sdk.AccAddress {
// GetSignBytes returns the bytes all expected signers must sign over for a
// MsgCreatePeriodicVestingAccount.
func (msg MsgCreatePeriodicVestingAccount) GetSignBytes() []byte {
return sdk.MustSortJSON(legacy.Cdc.MustMarshalJSON(&msg))
return sdk.MustSortJSON(ModuleCdc.MustMarshalJSON(&msg))
}

// ValidateBasic Implements Msg.
Expand Down
5 changes: 3 additions & 2 deletions x/authz/codec.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
types "github.com/cosmos/cosmos-sdk/codec/types"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/msgservice"
authzcodec "github.com/cosmos/cosmos-sdk/x/authz/codec"
)

// RegisterLegacyAminoCodec registers the necessary x/authz interfaces and concrete types
Expand Down Expand Up @@ -36,7 +37,7 @@ func RegisterInterfaces(registry types.InterfaceRegistry) {
msgservice.RegisterMsgServiceDesc(registry, MsgServiceDesc())
}
func init() {
// Register all Amino interfaces and concrete types on the global Amino codec so that this can later be
// Register all Amino interfaces and concrete types on the authz Amino codec so that this can later be
// used to properly serialize MsgGrant and MsgExec instances
RegisterLegacyAminoCodec(legacy.Cdc)
RegisterLegacyAminoCodec(authzcodec.Amino)
}
18 changes: 18 additions & 0 deletions x/authz/codec/cdc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package codec
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've created this new package in order to avoid any possible dependency cycle that might arise if we leave the ModuleCdc definition inside the x/authz/codec.go file. If you think we can keep it inside x/authz/codec.go then I can move it there without any issue

Copy link
Contributor

Choose a reason for hiding this comment

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

to avoid any possible dependency cycle that might arise if we leave the ModuleCdc definition inside the x/authz/codec.go

Which dep cycle do you see happening?

By default, I would say let's keep like all other modules, unless there's a strong reason to do a separate package.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok I've just tried moving it back to authz/codec.go and the following popped up:

package github.com/cosmos/cosmos-sdk/server
        imports github.com/cosmos/cosmos-sdk/server/rosetta
        imports github.com/cosmos/cosmos-sdk/x/auth/tx
        imports github.com/cosmos/cosmos-sdk/x/auth/middleware
        imports github.com/cosmos/cosmos-sdk/x/auth/types
        imports github.com/cosmos/cosmos-sdk/x/authz
        imports github.com/cosmos/cosmos-sdk/x/auth/types: import cycle not allowed
package github.com/cosmos/cosmos-sdk/server
        imports github.com/cosmos/cosmos-sdk/server/rosetta
        imports github.com/cosmos/cosmos-sdk/x/auth/tx
        imports github.com/cosmos/cosmos-sdk/x/auth/middleware
        imports github.com/cosmos/cosmos-sdk/x/auth/types
        imports github.com/cosmos/cosmos-sdk/x/authz
        imports github.com/cosmos/cosmos-sdk/x/auth/types: import cycle not allowed

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice! Can we add a comment somewhere saying that authz's codec is in a separate package to avoid dep cycle? Maybe a package godoc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@AmauryM @alexanderbez documentation has been added


import (
"github.com/cosmos/cosmos-sdk/codec"
cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec"
sdk "github.com/cosmos/cosmos-sdk/types"
)

var (
Amino = codec.NewLegacyAmino()
ModuleCdc = codec.NewAminoCodec(Amino)
)

func init() {
cryptocodec.RegisterCrypto(Amino)
codec.RegisterEvidences(Amino)
sdk.RegisterLegacyAminoCodec(Amino)
}
18 changes: 18 additions & 0 deletions x/authz/codec/doc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/*
Package codec provides a singleton instance of Amino codec that should be used to register
any concrete type that can later be referenced inside a MsgGrant or MsgExec instance so that they
can be (de)serialized properly.

Amino types should be ideally registered inside this codec within the init function of each module's
codec.go file as follows:

func init() {
// ...

RegisterLegacyAminoCodec(authzcodec.Amino)
}

The codec instance is put inside this package and not the x/authz package in order to avoid any dependency cycle.

*/
package codec
9 changes: 4 additions & 5 deletions x/authz/msgs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
package authz

import (
authzcodec "github.com/cosmos/cosmos-sdk/x/authz/codec"
"time"

"github.com/cosmos/cosmos-sdk/codec/legacy"

"github.com/gogo/protobuf/proto"

cdctypes "github.com/cosmos/cosmos-sdk/codec/types"
Expand Down Expand Up @@ -77,7 +76,7 @@ func (msg MsgGrant) Route() string {

// GetSignBytes implements the LegacyMsg.GetSignBytes method.
func (msg MsgGrant) GetSignBytes() []byte {
return sdk.MustSortJSON(legacy.Cdc.MustMarshalJSON(&msg))
return sdk.MustSortJSON(authzcodec.ModuleCdc.MustMarshalJSON(&msg))
}

// GetAuthorization returns the cache value from the MsgGrant.Authorization if present.
Expand Down Expand Up @@ -167,7 +166,7 @@ func (msg MsgRevoke) Route() string {

// GetSignBytes implements the LegacyMsg.GetSignBytes method.
func (msg MsgRevoke) GetSignBytes() []byte {
return sdk.MustSortJSON(legacy.Cdc.MustMarshalJSON(&msg))
return sdk.MustSortJSON(authzcodec.ModuleCdc.MustMarshalJSON(&msg))
}

// NewMsgExec creates a new MsgExecAuthorized
Expand Down Expand Up @@ -234,5 +233,5 @@ func (msg MsgExec) Route() string {

// GetSignBytes implements the LegacyMsg.GetSignBytes method.
func (msg MsgExec) GetSignBytes() []byte {
return sdk.MustSortJSON(legacy.Cdc.MustMarshalJSON(&msg))
return sdk.MustSortJSON(authzcodec.ModuleCdc.MustMarshalJSON(&msg))
}
9 changes: 4 additions & 5 deletions x/bank/simulation/operations_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package simulation_test

import (
"github.com/cosmos/cosmos-sdk/codec/legacy"
"math/rand"
"testing"

Expand Down Expand Up @@ -81,7 +80,7 @@ func (suite *SimTestSuite) TestSimulateMsgSend() {
suite.Require().NoError(err)

var msg types.MsgSend
legacy.Cdc.UnmarshalJSON(operationMsg.Msg, &msg)
types.ModuleCdc.UnmarshalJSON(operationMsg.Msg, &msg)

suite.Require().True(operationMsg.OK)
suite.Require().Equal("65337742stake", msg.Amount.String())
Expand Down Expand Up @@ -110,7 +109,7 @@ func (suite *SimTestSuite) TestSimulateMsgMultiSend() {
require.NoError(err)

var msg types.MsgMultiSend
legacy.Cdc.UnmarshalJSON(operationMsg.Msg, &msg)
types.ModuleCdc.UnmarshalJSON(operationMsg.Msg, &msg)

require.True(operationMsg.OK)
require.Len(msg.Inputs, 3)
Expand Down Expand Up @@ -147,7 +146,7 @@ func (suite *SimTestSuite) TestSimulateModuleAccountMsgSend() {
suite.Require().Error(err)

var msg types.MsgSend
legacy.Cdc.UnmarshalJSON(operationMsg.Msg, &msg)
types.ModuleCdc.UnmarshalJSON(operationMsg.Msg, &msg)

suite.Require().False(operationMsg.OK)
suite.Require().Equal(operationMsg.Comment, "invalid transfers")
Expand Down Expand Up @@ -176,7 +175,7 @@ func (suite *SimTestSuite) TestSimulateMsgMultiSendToModuleAccount() {
suite.Require().Error(err)

var msg types.MsgMultiSend
legacy.Cdc.UnmarshalJSON(operationMsg.Msg, &msg)
types.ModuleCdc.UnmarshalJSON(operationMsg.Msg, &msg)

suite.Require().False(operationMsg.OK) // sending tokens to a module account should fail
suite.Require().Equal(operationMsg.Comment, "invalid transfers")
Expand Down
15 changes: 14 additions & 1 deletion x/bank/types/codec.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@ import (
"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/codec/legacy"
"github.com/cosmos/cosmos-sdk/codec/types"
cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/msgservice"
"github.com/cosmos/cosmos-sdk/x/authz"
authzcodec "github.com/cosmos/cosmos-sdk/x/authz/codec"
)

// RegisterLegacyAminoCodec registers the necessary x/bank interfaces and concrete types
Expand All @@ -30,6 +32,17 @@ func RegisterInterfaces(registry types.InterfaceRegistry) {
msgservice.RegisterMsgServiceDesc(registry, &_Msg_serviceDesc)
}

var (
amino = codec.NewLegacyAmino()
ModuleCdc = codec.NewAminoCodec(amino)
)

func init() {
RegisterLegacyAminoCodec(legacy.Cdc)
RegisterLegacyAminoCodec(amino)
cryptocodec.RegisterCrypto(amino)
sdk.RegisterLegacyAminoCodec(amino)

// Register all Amino interfaces and concrete types on the authz Amino codec so that this can later be
// used to properly serialize MsgGrant and MsgExec instances
RegisterLegacyAminoCodec(authzcodec.Amino)
}
5 changes: 2 additions & 3 deletions x/bank/types/msgs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package types

import (
"github.com/cosmos/cosmos-sdk/codec/legacy"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
)
Expand Down Expand Up @@ -49,7 +48,7 @@ func (msg MsgSend) ValidateBasic() error {

// GetSignBytes Implements Msg.
func (msg MsgSend) GetSignBytes() []byte {
return sdk.MustSortJSON(legacy.Cdc.MustMarshalJSON(&msg))
return sdk.MustSortJSON(ModuleCdc.MustMarshalJSON(&msg))
}

// GetSigners Implements Msg.
Expand Down Expand Up @@ -88,7 +87,7 @@ func (msg MsgMultiSend) ValidateBasic() error {

// GetSignBytes Implements Msg.
func (msg MsgMultiSend) GetSignBytes() []byte {
return sdk.MustSortJSON(legacy.Cdc.MustMarshalJSON(&msg))
return sdk.MustSortJSON(ModuleCdc.MustMarshalJSON(&msg))
}

// GetSigners Implements Msg.
Expand Down
15 changes: 14 additions & 1 deletion x/crisis/types/codec.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@ import (
"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/codec/legacy"
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/msgservice"
authzcodec "github.com/cosmos/cosmos-sdk/x/authz/codec"
)

// RegisterLegacyAminoCodec registers the necessary x/crisis interfaces and concrete types
Expand All @@ -22,6 +24,17 @@ func RegisterInterfaces(registry codectypes.InterfaceRegistry) {
msgservice.RegisterMsgServiceDesc(registry, &_Msg_serviceDesc)
}

var (
amino = codec.NewLegacyAmino()
ModuleCdc = codec.NewAminoCodec(amino)
)

func init() {
RegisterLegacyAminoCodec(legacy.Cdc)
RegisterLegacyAminoCodec(amino)
cryptocodec.RegisterCrypto(amino)
sdk.RegisterLegacyAminoCodec(amino)

// Register all Amino interfaces and concrete types on the authz Amino codec so that this can later be
// used to properly serialize MsgGrant and MsgExec instances
RegisterLegacyAminoCodec(authzcodec.Amino)
}
3 changes: 1 addition & 2 deletions x/crisis/types/msgs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package types

import (
"github.com/cosmos/cosmos-sdk/codec/legacy"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
)
Expand Down Expand Up @@ -30,7 +29,7 @@ func (msg MsgVerifyInvariant) GetSigners() []sdk.AccAddress {

// GetSignBytes gets the sign bytes for the msg MsgVerifyInvariant
func (msg MsgVerifyInvariant) GetSignBytes() []byte {
bz := legacy.Cdc.MustMarshalJSON(&msg)
bz := ModuleCdc.MustMarshalJSON(&msg)
return sdk.MustSortJSON(bz)
}

Expand Down
Loading