Skip to content

Commit

Permalink
revert: replace all ModuleCdc instances with legacy.Cdc (cosmos#11680)
Browse files Browse the repository at this point in the history
Reverts the usage of a singleton `legacy.Cdc` codec while (de)serializing `x/authz` messages.

Closes: cosmos#11643

---

*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...

- [ ] 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
- [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting))
- [ ] 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
- [ ] reviewed "Files changed" and left comments if necessary
- [ ] confirmed all CI checks have passed

*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 authored and p0mvn committed May 5, 2022
1 parent 1778bc2 commit f6bb1ea
Show file tree
Hide file tree
Showing 19 changed files with 198 additions and 78 deletions.
18 changes: 18 additions & 0 deletions docs/core/encoding.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,24 @@ Note, there are length-prefixed variants of the above functionality and this is
typically used for when the data needs to be streamed or grouped together
(e.g. `ResponseDeliverTx.Data`)

#### Authz authorizations

Since the `MsgExec` message type can contain different messages instances, it is important that developers
add the following code inside the `init` method of their module's `codec.go` file:

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

init() {
// 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)
}
```

This will allow the `x/authz` module to properly serialize and de-serializes `MsgExec` instances using Amino,
which is required when signing this kind of messages using a Ledger.

### Gogoproto

Modules are encouraged to utilize Protobuf encoding for their respective types. In the SDK, we use the [Gogoproto](https://github.com/gogo/protobuf) specific implementation of the Protobuf spec that offers speed and DX improvements compared to the official [Google protobuf implementation](https://github.com/protocolbuffers/protobuf).
Expand Down
9 changes: 8 additions & 1 deletion x/auth/types/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ import (
"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/codec/types"
cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec"
"github.com/cosmos/cosmos-sdk/x/auth/legacy/legacytx"
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 @@ -45,4 +47,9 @@ var (
func init() {
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)
}
14 changes: 12 additions & 2 deletions x/auth/vesting/types/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@ package types
import (
"github.com/cosmos/cosmos-sdk/codec"
"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 @@ -58,9 +60,17 @@ func RegisterInterfaces(registry types.InterfaceRegistry) {
msgservice.RegisterMsgServiceDesc(registry, &_Msg_serviceDesc)
}

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

func init() {
RegisterLegacyAminoCodec(amino)
amino.Seal()
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)
}
2 changes: 1 addition & 1 deletion x/auth/vesting/types/msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,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(ModuleCdc.MustMarshalJSON(&msg))
}

// GetSigners returns the expected signers for a MsgCreateVestingAccount.
Expand Down
6 changes: 6 additions & 0 deletions x/authz/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,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"
)

// RegisterInterfaces registers the interfaces types with the interface registry
Expand All @@ -22,3 +23,8 @@ func RegisterInterfaces(registry types.InterfaceRegistry) {

msgservice.RegisterMsgServiceDesc(registry, MsgServiceDesc())
}
func init() {
// 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
sdk.RegisterLegacyAminoCodec(authzcodec.Amino)
}
18 changes: 18 additions & 0 deletions x/authz/codec/cdc.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package codec

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
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
8 changes: 4 additions & 4 deletions x/authz/msgs.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
package authz

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

"github.com/gogo/protobuf/proto"

"github.com/cosmos/cosmos-sdk/codec/legacy"
cdctypes "github.com/cosmos/cosmos-sdk/codec/types"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
Expand Down Expand Up @@ -79,7 +79,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 @@ -172,7 +172,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 @@ -243,5 +243,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))
}
16 changes: 7 additions & 9 deletions x/bank/types/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
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,19 +31,16 @@ func RegisterInterfaces(registry types.InterfaceRegistry) {
}

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.
amino = codec.NewLegacyAmino()
ModuleCdc = codec.NewAminoCodec(amino)
)

func init() {
RegisterLegacyAminoCodec(amino)
cryptocodec.RegisterCrypto(amino)
amino.Seal()
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)
}
16 changes: 7 additions & 9 deletions x/crisis/types/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
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 @@ -23,19 +24,16 @@ func RegisterInterfaces(registry codectypes.InterfaceRegistry) {
}

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.
amino = codec.NewLegacyAmino()
ModuleCdc = codec.NewAminoCodec(amino)
)

func init() {
RegisterLegacyAminoCodec(amino)
cryptocodec.RegisterCrypto(amino)
amino.Seal()
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)
}
24 changes: 14 additions & 10 deletions x/distribution/simulation/operations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,16 +166,20 @@ func (suite *SimTestSuite) testSimulateMsgWithdrawValidatorCommission(tokenName
// execute operation
op := simulation.SimulateMsgWithdrawValidatorCommission(suite.app.AccountKeeper, suite.app.BankKeeper, suite.app.DistrKeeper, suite.app.StakingKeeper)
operationMsg, futureOperations, err := op(r, suite.app.BaseApp, suite.ctx, accounts, "")
suite.Require().NoError(err)

var msg types.MsgWithdrawValidatorCommission
types.ModuleCdc.UnmarshalJSON(operationMsg.Msg, &msg)

suite.Require().True(operationMsg.OK)
suite.Require().Equal("cosmosvaloper1tnh2q55v8wyygtt9srz5safamzdengsn9dsd7z", msg.ValidatorAddress)
suite.Require().Equal(types.TypeMsgWithdrawValidatorCommission, msg.Type())
suite.Require().Equal(types.ModuleName, msg.Route())
suite.Require().Len(futureOperations, 0)
if !operationMsg.OK {
suite.Require().Equal("could not find account", operationMsg.Comment)
} else {
suite.Require().NoError(err)

var msg types.MsgWithdrawValidatorCommission
types.ModuleCdc.UnmarshalJSON(operationMsg.Msg, &msg)

suite.Require().True(operationMsg.OK)
suite.Require().Equal("cosmosvaloper1tnh2q55v8wyygtt9srz5safamzdengsn9dsd7z", msg.ValidatorAddress)
suite.Require().Equal(types.TypeMsgWithdrawValidatorCommission, msg.Type())
suite.Require().Equal(types.ModuleName, msg.Route())
suite.Require().Len(futureOperations, 0)
}
}

// TestSimulateMsgFundCommunityPool tests the normal scenario of a valid message of type TypeMsgFundCommunityPool.
Expand Down
18 changes: 8 additions & 10 deletions x/distribution/types/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ import (
cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/msgservice"
govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
authzcodec "github.com/cosmos/cosmos-sdk/x/authz/codec"
govtypes "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1"
)

// RegisterLegacyAminoCodec registers the necessary x/distribution interfaces and concrete types
Expand Down Expand Up @@ -36,19 +37,16 @@ func RegisterInterfaces(registry types.InterfaceRegistry) {
}

var (
amino = codec.NewLegacyAmino()

// ModuleCdc references the global x/distribution 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/distribution and
// defined at the application level.
amino = codec.NewLegacyAmino()
ModuleCdc = codec.NewAminoCodec(amino)
)

func init() {
RegisterLegacyAminoCodec(amino)
cryptocodec.RegisterCrypto(amino)
amino.Seal()
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)
}
16 changes: 7 additions & 9 deletions x/evidence/types/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
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"
"github.com/cosmos/cosmos-sdk/x/evidence/exported"
)

Expand All @@ -29,19 +30,16 @@ func RegisterInterfaces(registry types.InterfaceRegistry) {
}

var (
amino = codec.NewLegacyAmino()

// ModuleCdc references the global x/evidence 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/evidence and
// defined at the application level.
amino = codec.NewLegacyAmino()
ModuleCdc = codec.NewAminoCodec(amino)
)

func init() {
RegisterLegacyAminoCodec(amino)
cryptocodec.RegisterCrypto(amino)
amino.Seal()
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)
}
1 change: 0 additions & 1 deletion x/evidence/types/msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package types

import (
"fmt"

"github.com/gogo/protobuf/proto"

"github.com/cosmos/cosmos-sdk/codec/types"
Expand Down
Loading

0 comments on commit f6bb1ea

Please sign in to comment.