Remove module-level ModuleCdc, use the global legacy.Cdc

[amaury1093]

Summary

Replace all modules' individual ModuleCdc with the global legacy.Cdc

Problem Definition

Each module currently define it's own amino ModuleCdc, which is used to serialize Msgs for amino signing. Each ModuleCdc registers the module's own Msg types, via the RegisterLegacyAminoCodec method.

We have a problem when using nested Msgs. An example is authz's MsgExec. Authz's RegisterLegacyAminoCodec registers MsgExec, but it doesn't register e.g. a nested bank MsgSend. So authz's ModuleCdc is not aware of the bank msg types, leading to weird Amino JSON encoding: cosmos/cosmjs#1026.

We'll have the same problem for group, gov, and any other external module that uses nested Msgs.

Proposal

@RiccardoM proposed an elegant solution in #11224. Basically, instead of each module defining its own ModuleCdc, they all rely on the global legacy.Cdc from "github.com/cosmos/cosmos-sdk/codec/legacy".

Each module actually then call RegisterLegacyAminoCodec(legacy.Cdc) on that single global legacy.Cdc.

All module developers ideally change their code too

We've been using authz in production since v0.43, and people were able to sign with Ledger. So this proposal is actually optional security-wise (I think) or functioanlity-wise.

It's just that without this proposal, it's a PITA for clients like CosmJS to decode. So if we decide to do this change, it's highly recommended for all module developers to follow. It's consensus-breaking though.

All modue developers need to do the following 2 changes.

1. Remove ModuleCdc, register using legacy.Cdc

// x/mymodule/codec.go

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

func RegisterLegacyAminoCodec(cdc *codec.LegacyAmino) { ... }

- 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)
+	RegisterLegacyAminoCodec(legacy.Cdc)
-	cryptocodec.RegisterCrypto(amino)
}

2. For each Msg in the module, change GetSignBytes to use the global legacy.Cdc

// x/mymodule/codec.go

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

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

Proposal

Perform the 2 changes described above for all SDK modules. Note that this has already been done in authz in #11224

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

[tac0turtle]

should this live within the framework eg or are we fine with it on its own.

[robert-zaremba]

I think the only long term solution is to use top-down approach as we do with proto codec.

[RiccardoM]

I think the only long term solution is to use top-down approach as we do with proto codec.

I agree with you, but at the same time I think we can use this approach to properly support Ledger signing for the x/authz module until a better fix is found since this solution has no particular drawback