Skip to content

Commit

Permalink
feat: replace all ModuleCdc instances with legacy.Cdc (#11240)
Browse files Browse the repository at this point in the history
## Description
Replaces all the various `ModuleCdc` instances with the global `legacy.Cdc` to properly support Amino serialization for the `x/authz` module. This is needed in order to correctly support Ledger signing for `MsgExec` and messages.

Closes: #11232



---

### Author Checklist

*All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.*

I have...

- [x] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] added `!` to the type prefix if API or client breaking change
- [x] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting))
- [x] provided a link to the relevant issue or specification
- [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules)
- [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing)
- [ ] added a changelog entry to `CHANGELOG.md`
- [ ] included comments for [documenting Go code](https://blog.golang.org/godoc)
- [ ] updated the relevant documentation or specification
- [x] reviewed "Files changed" and left comments if necessary
- [ ] confirmed all CI checks have passed

### Reviewers Checklist

*All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.*

I have...

- [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] confirmed `!` in the type prefix if API or client breaking change
- [ ] confirmed all author checklist items have been addressed 
- [ ] reviewed state machine logic
- [ ] reviewed API design and naming
- [ ] reviewed documentation is accurate
- [ ] reviewed tests and test coverage
- [ ] manually tested (if applicable)
  • Loading branch information
RiccardoM committed Feb 23, 2022
1 parent 4334200 commit ff31490
Show file tree
Hide file tree
Showing 36 changed files with 142 additions and 308 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* [\#11006](https://github.com/cosmos/cosmos-sdk/pull/11006) Add `debug pubkey-raw` command to allow inspecting of pubkeys in legacy bech32 format
* (x/authz) [\#10714](https://github.com/cosmos/cosmos-sdk/pull/10714) Add support for pruning expired authorizations
* [\#10015](https://github.com/cosmos/cosmos-sdk/pull/10015) ADR-040: ICS-23 proofs for SMT store
* [\#11240](https://github.com/cosmos/cosmos-sdk/pull/11240) Replace various modules `ModuleCdc` with the global `legacy.Cdc`

### API Breaking Changes

Expand Down
2 changes: 2 additions & 0 deletions codec/legacy/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"github.com/cosmos/cosmos-sdk/codec"
cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec"
cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
sdk "github.com/cosmos/cosmos-sdk/types"
)

// Cdc defines a global generic sealed Amino codec to be used throughout sdk. It
Expand All @@ -15,6 +16,7 @@ var Cdc = codec.NewLegacyAmino()
func init() {
cryptocodec.RegisterCrypto(Cdc)
codec.RegisterEvidences(Cdc)
sdk.RegisterLegacyAminoCodec(Cdc)
}

// PrivKeyFromBytes unmarshals private key bytes and returns a PrivKey
Expand Down
1 change: 0 additions & 1 deletion x/auth/middleware/sigverify_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,6 @@ func (s *MWTestSuite) TestSigVerification_ExplicitAmino() {

// Set up TxConfig.
aminoCdc := legacy.Cdc
aminoCdc.RegisterInterface((*sdk.Msg)(nil), nil)
aminoCdc.RegisterConcrete(&testdata.TestMsg{}, "testdata.TestMsg", nil)

// We're using TestMsg amino encoding in some tests, so register it here.
Expand Down
11 changes: 0 additions & 11 deletions x/auth/types/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ 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"
"github.com/cosmos/cosmos-sdk/x/auth/migrations/legacytx"
)

Expand Down Expand Up @@ -38,16 +37,6 @@ func RegisterInterfaces(registry types.InterfaceRegistry) {
)
}

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

func init() {
RegisterLegacyAminoCodec(amino)
cryptocodec.RegisterCrypto(amino)

// 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)
}
6 changes: 2 additions & 4 deletions x/auth/vesting/types/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package types

import (
"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/codec/legacy"
"github.com/cosmos/cosmos-sdk/codec/types"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/msgservice"
Expand Down Expand Up @@ -61,9 +62,6 @@ func RegisterInterfaces(registry types.InterfaceRegistry) {
msgservice.RegisterMsgServiceDesc(registry, &_Msg_serviceDesc)
}

var amino = codec.NewLegacyAmino()

func init() {
RegisterLegacyAminoCodec(amino)
amino.Seal()
RegisterLegacyAminoCodec(legacy.Cdc)
}
97 changes: 49 additions & 48 deletions x/auth/vesting/types/msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ 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 @@ -67,7 +68,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(amino.MustMarshalJSON(&msg))
return sdk.MustSortJSON(legacy.Cdc.MustMarshalJSON(&msg))
}

// GetSigners returns the expected signers for a MsgCreateVestingAccount.
Expand All @@ -77,52 +78,52 @@ func (msg MsgCreateVestingAccount) GetSigners() []sdk.AccAddress {
}

// NewMsgCreatePermanentLockedAccount returns a reference to a new MsgCreatePermanentLockedAccount.
//nolint:interfacer
func NewMsgCreatePermanentLockedAccount(fromAddr, toAddr sdk.AccAddress, amount sdk.Coins) *MsgCreatePermanentLockedAccount {
return &MsgCreatePermanentLockedAccount{
FromAddress: fromAddr.String(),
ToAddress: toAddr.String(),
Amount: 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 {
return sdkerrors.ErrInvalidAddress.Wrapf("invalid sender address: %s", err)
}
if _, err := sdk.AccAddressFromBech32(msg.ToAddress); err != nil {
return sdkerrors.ErrInvalidAddress.Wrapf("invalid recipient address: %s", err)
}

if !msg.Amount.IsValid() {
return sdkerrors.ErrInvalidCoins.Wrap(msg.Amount.String())
}

if !msg.Amount.IsAllPositive() {
return sdkerrors.ErrInvalidCoins.Wrap(msg.Amount.String())
}

return nil
}

// GetSignBytes returns the bytes all expected signers must sign over for a
// MsgCreatePermanentLockedAccount.
func (msg MsgCreatePermanentLockedAccount) GetSignBytes() []byte {
return sdk.MustSortJSON(amino.MustMarshalJSON(&msg))
}

// GetSigners returns the expected signers for a MsgCreatePermanentLockedAccount.
func (msg MsgCreatePermanentLockedAccount) GetSigners() []sdk.AccAddress {
from, _ := sdk.AccAddressFromBech32(msg.FromAddress)
return []sdk.AccAddress{from}
}
//nolint:interfacer
func NewMsgCreatePermanentLockedAccount(fromAddr, toAddr sdk.AccAddress, amount sdk.Coins) *MsgCreatePermanentLockedAccount {
return &MsgCreatePermanentLockedAccount{
FromAddress: fromAddr.String(),
ToAddress: toAddr.String(),
Amount: 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 {
return sdkerrors.ErrInvalidAddress.Wrapf("invalid sender address: %s", err)
}
if _, err := sdk.AccAddressFromBech32(msg.ToAddress); err != nil {
return sdkerrors.ErrInvalidAddress.Wrapf("invalid recipient address: %s", err)
}

if !msg.Amount.IsValid() {
return sdkerrors.ErrInvalidCoins.Wrap(msg.Amount.String())
}

if !msg.Amount.IsAllPositive() {
return sdkerrors.ErrInvalidCoins.Wrap(msg.Amount.String())
}

return nil
}

// 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))
}

// GetSigners returns the expected signers for a MsgCreatePermanentLockedAccount.
func (msg MsgCreatePermanentLockedAccount) GetSigners() []sdk.AccAddress {
from, _ := sdk.AccAddressFromBech32(msg.FromAddress)
return []sdk.AccAddress{from}
}

// NewMsgCreatePeriodicVestingAccount returns a reference to a new MsgCreatePeriodicVestingAccount.
//nolint:interfacer
Expand Down Expand Up @@ -153,7 +154,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(amino.MustMarshalJSON(&msg))
return sdk.MustSortJSON(legacy.Cdc.MustMarshalJSON(&msg))
}

// ValidateBasic Implements Msg.
Expand Down
9 changes: 5 additions & 4 deletions x/bank/simulation/operations_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package simulation_test

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

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

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

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

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

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

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

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

var msg types.MsgMultiSend
types.ModuleCdc.UnmarshalJSON(operationMsg.Msg, &msg)
legacy.Cdc.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
19 changes: 0 additions & 19 deletions x/bank/types/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ 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"
Expand All @@ -31,24 +30,6 @@ func RegisterInterfaces(registry types.InterfaceRegistry) {
msgservice.RegisterMsgServiceDesc(registry, &_Msg_serviceDesc)
}

var (
amino = codec.NewLegacyAmino()

// ModuleCdc references the global x/bank module codec. Note, the codec should
// ONLY be used in certain instances of tests and for JSON encoding as Amino is
// still used for that purpose.
//
// The actual codec used for serialization should be provided to x/staking and
// defined at the application level.
ModuleCdc = codec.NewAminoCodec(amino)
)

func init() {
RegisterLegacyAminoCodec(amino)
cryptocodec.RegisterCrypto(amino)
amino.Seal()

// 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)
}
5 changes: 3 additions & 2 deletions x/bank/types/msgs.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
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 @@ -48,7 +49,7 @@ func (msg MsgSend) ValidateBasic() error {

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

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

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

// GetSigners Implements Msg.
Expand Down
19 changes: 0 additions & 19 deletions x/crisis/types/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ 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"
)
Expand All @@ -23,24 +22,6 @@ func RegisterInterfaces(registry codectypes.InterfaceRegistry) {
msgservice.RegisterMsgServiceDesc(registry, &_Msg_serviceDesc)
}

var (
amino = codec.NewLegacyAmino()

// ModuleCdc references the global x/crisis module codec. Note, the codec should
// ONLY be used in certain instances of tests and for JSON encoding as Amino is
// still used for that purpose.
//
// The actual codec used for serialization should be provided to x/crisis and
// defined at the application level.
ModuleCdc = codec.NewAminoCodec(amino)
)

func init() {
RegisterLegacyAminoCodec(amino)
cryptocodec.RegisterCrypto(amino)
amino.Seal()

// 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)
}
3 changes: 2 additions & 1 deletion x/crisis/types/msgs.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
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 @@ -29,7 +30,7 @@ func (msg MsgVerifyInvariant) GetSigners() []sdk.AccAddress {

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

Expand Down
4 changes: 2 additions & 2 deletions x/distribution/client/common/common_test.go
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
package common

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

"github.com/stretchr/testify/require"

"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/x/distribution/types"
)

func TestQueryDelegationRewardsAddrValidation(t *testing.T) {
clientCtx := client.Context{}.WithLegacyAmino(types.ModuleCdc.LegacyAmino)
clientCtx := client.Context{}.WithLegacyAmino(legacy.Cdc)

type args struct {
delAddr string
Expand Down
Loading

0 comments on commit ff31490

Please sign in to comment.