Enable proto JSON generally and remove HybridCodec (#6859)

* Remove HybridCodec * WIP on fixing proto JSON issues * WIP on fixing proto JSON issues * WIP on fixing proto JSON issues * WIP on fixing proto JSON issues * WIP on fixing proto JSON issues * Test fixes * Delete hybrid_codec.go * Fixes * Fixes * Fixes * Test fixes * Test fixes * Test fixes * Lint * Sim fixes * Sim fixes * Revert * Remove vesting account JSON tests * Update CHANGELOG.md * Lint * Sim fixes * Sim fixes * Docs * Migrate more amino usages * Remove custom VoteOption String() and json marshaling * Fix tests * Add comments, update CHANGELOG.md Co-authored-by: Jonathan Gimeno <jgimeno@gmail.com> Co-authored-by: Alexander Bezobchuk <alexanderbez@users.noreply.github.com>
cosmos · Aug 13, 2020 · 816c5a3 · 816c5a3
1 parent 134e1dc
commit 816c5a3
Show file tree

Hide file tree

Showing 77 changed files with 325 additions and 845 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -62,7 +62,8 @@ older clients.
 * (client/keys) [\#5889](https://github.com/cosmos/cosmos-sdk/pull/5889) Remove `keys update` command.
 * (x/evidence) [\#5952](https://github.com/cosmos/cosmos-sdk/pull/5952) Remove CLI and REST handlers for querying `x/evidence` parameters.
 * (server) [\#5982](https://github.com/cosmos/cosmos-sdk/pull/5982) `--pruning` now must be set to `custom` if you want to customise the granular options.
-* (x/gov) [\#7000](https://github.com/cosmos/cosmos-sdk/pull/7000) `ProposalStatus` is now JSON serialized using its protobuf name, so expect names like `PROPOSAL_STATUS_DEPOSIT_PERIOD` as opposed to `DepositPeriod`.
+* (x/gov) [\#7000](https://github.com/cosmos/cosmos-sdk/pull/7000) [\#6859](https://github.com/cosmos/cosmos-sdk/pull/6859) `ProposalStatus` and `VoteOption` are now JSON serialized using its protobuf name, so expect names like `PROPOSAL_STATUS_DEPOSIT_PERIOD` as opposed to `DepositPeriod`.
+* (x/auth/vesting) [\#6859](https://github.com/cosmos/cosmos-sdk/pull/6859) Custom JSON marshaling of vesting accounts was removed. Vesting accounts are now marshaled using their default proto or amino JSON representation.
 
 ### API Breaking Changes
 

diff --git a/codec/hybrid_codec.go b/codec/hybrid_codec.go
diff --git a/codec/hybrid_codec_test.go b/codec/hybrid_codec_test.go
diff --git a/codec/json.go b/codec/json.go
@@ -12,7 +12,9 @@ import (
 // ProtoMarshalJSON provides an auxiliary function to return Proto3 JSON encoded
 // bytes of a message.
 func ProtoMarshalJSON(msg proto.Message) ([]byte, error) {
-	jm := &jsonpb.Marshaler{OrigName: true}
+	// We use the OrigName because camel casing fields just doesn't make sense.
+	// EmitDefaults is also often the more expected behavior for CLI users
+	jm := &jsonpb.Marshaler{OrigName: true, EmitDefaults: true}
 	err := types.UnpackInterfaces(msg, types.ProtoJSONPacker{JSONPBMarshaler: jm})
 	if err != nil {
 		return nil, err

diff --git a/proto/cosmos/gov/v1beta1/gov.proto b/proto/cosmos/gov/v1beta1/gov.proto
@@ -15,8 +15,6 @@ option (gogoproto.goproto_getters_all)  = false;
 
 // VoteOption defines a vote option
 enum VoteOption {
-  option (gogoproto.enum_stringer)         = false;
-  option (gogoproto.goproto_enum_stringer) = false;
   option (gogoproto.goproto_enum_prefix)   = false;
 
   // VOTE_OPTION_UNSPECIFIED defines a no-op vote option.

diff --git a/server/start.go b/server/start.go
@@ -106,7 +106,8 @@ which accepts a path for the resulting pprof file.
 
 			serverCtx.Logger.Info("starting ABCI with Tendermint")
 
-			err := startInProcess(serverCtx, clientCtx.JSONMarshaler, appCreator)
+			// amino is needed here for backwards compatibility of REST routes
+			err := startInProcess(serverCtx, clientCtx.LegacyAmino, appCreator)
 			return err
 		},
 	}
@@ -177,7 +178,8 @@ func startStandAlone(ctx *Context, appCreator types.AppCreator) error {
 	select {}
 }
 
-func startInProcess(ctx *Context, cdc codec.JSONMarshaler, appCreator types.AppCreator) error {
+// legacyAminoCdc is used for the legacy REST API
+func startInProcess(ctx *Context, legacyAminoCdc *codec.LegacyAmino, appCreator types.AppCreator) error {
 	cfg := ctx.Config
 	home := cfg.RootDir
 
@@ -230,7 +232,9 @@ func startInProcess(ctx *Context, cdc codec.JSONMarshaler, appCreator types.AppC
 		clientCtx := client.Context{}.
 			WithHomeDir(home).
 			WithChainID(genDoc.ChainID).
-			WithJSONMarshaler(cdc).
+			WithJSONMarshaler(legacyAminoCdc).
+			// amino is needed here for backwards compatibility of REST routes
+			WithLegacyAmino(legacyAminoCdc).
 			WithClient(local.New(tmNode))
 
 		apiSrv = api.New(clientCtx, ctx.Logger.With("module", "api-server"))

diff --git a/simapp/app.go b/simapp/app.go
@@ -202,7 +202,7 @@ func NewSimApp(
 		memKeys:           memKeys,
 	}
 
-	app.ParamsKeeper = initParamsKeeper(appCodec, keys[paramstypes.StoreKey], tkeys[paramstypes.TStoreKey])
+	app.ParamsKeeper = initParamsKeeper(appCodec, cdc, keys[paramstypes.StoreKey], tkeys[paramstypes.TStoreKey])
 
 	// set the BaseApp's parameter store
 	bApp.SetParamStore(app.ParamsKeeper.Subspace(baseapp.Paramspace).WithKeyTable(std.ConsensusParamsKeyTable()))
@@ -503,9 +503,12 @@ func (app *SimApp) SimulationManager() *module.SimulationManager {
 // RegisterAPIRoutes registers all application module routes with the provided
 // API server.
 func (app *SimApp) RegisterAPIRoutes(apiSvr *api.Server) {
-	rpc.RegisterRoutes(apiSvr.ClientCtx, apiSvr.Router)
-	authrest.RegisterTxRoutes(apiSvr.ClientCtx, apiSvr.Router)
-	ModuleBasics.RegisterRESTRoutes(apiSvr.ClientCtx, apiSvr.Router)
+	clientCtx := apiSvr.ClientCtx
+	// amino is needed here for backwards compatibility of REST routes
+	clientCtx = clientCtx.WithJSONMarshaler(clientCtx.LegacyAmino)
+	rpc.RegisterRoutes(clientCtx, apiSvr.Router)
+	authrest.RegisterTxRoutes(clientCtx, apiSvr.Router)
+	ModuleBasics.RegisterRESTRoutes(clientCtx, apiSvr.Router)
 }
 
 // GetMaccPerms returns a copy of the module account permissions
@@ -518,8 +521,8 @@ func GetMaccPerms() map[string][]string {
 }
 
 // initParamsKeeper init params keeper and its subspaces
-func initParamsKeeper(appCodec codec.Marshaler, key, tkey sdk.StoreKey) paramskeeper.Keeper {
-	paramsKeeper := paramskeeper.NewKeeper(appCodec, key, tkey)
+func initParamsKeeper(appCodec codec.BinaryMarshaler, legacyAmino *codec.LegacyAmino, key, tkey sdk.StoreKey) paramskeeper.Keeper {
+	paramsKeeper := paramskeeper.NewKeeper(appCodec, legacyAmino, key, tkey)
 
 	paramsKeeper.Subspace(authtypes.ModuleName)
 	paramsKeeper.Subspace(banktypes.ModuleName)

diff --git a/simapp/app_test.go b/simapp/app_test.go
@@ -1,15 +1,14 @@
 package simapp
 
 import (
+	"encoding/json"
 	"os"
 	"testing"
 
 	"github.com/stretchr/testify/require"
 	"github.com/tendermint/tendermint/libs/log"
 	dbm "github.com/tendermint/tm-db"
 
-	"github.com/cosmos/cosmos-sdk/codec"
-
 	abci "github.com/tendermint/tendermint/abci/types"
 )
 
@@ -18,7 +17,7 @@ func TestSimAppExport(t *testing.T) {
 	app := NewSimApp(log.NewTMLogger(log.NewSyncWriter(os.Stdout)), db, nil, true, map[int64]bool{}, DefaultNodeHome, 0, MakeEncodingConfig())
 
 	genesisState := NewDefaultGenesisState()
-	stateBytes, err := codec.MarshalJSONIndent(app.LegacyAmino(), genesisState)
+	stateBytes, err := json.MarshalIndent(genesisState, "", "  ")
 	require.NoError(t, err)
 
 	// Initialize the chain

diff --git a/simapp/params/amino.go b/simapp/params/amino.go
@@ -12,8 +12,7 @@ import (
 func MakeEncodingConfig() EncodingConfig {
 	cdc := codec.New()
 	interfaceRegistry := types.NewInterfaceRegistry()
-	// TODO: switch to using AminoCodec here once amino compatibility is fixed
-	marshaler := codec.NewHybridCodec(cdc, interfaceRegistry)
+	marshaler := codec.NewAminoCodec(cdc)
 
 	return EncodingConfig{
 		InterfaceRegistry: interfaceRegistry,

diff --git a/simapp/params/proto.go b/simapp/params/proto.go
@@ -13,8 +13,8 @@ import (
 func MakeEncodingConfig() EncodingConfig {
 	amino := codec.New()
 	interfaceRegistry := types.NewInterfaceRegistry()
-	marshaler := codec.NewHybridCodec(amino, interfaceRegistry)
-	txCfg := tx.NewTxConfig(codec.NewProtoCodec(interfaceRegistry), std.DefaultPublicKeyCodec{}, tx.DefaultSignModes)
+	marshaler := codec.NewProtoCodec(interfaceRegistry)
+	txCfg := tx.NewTxConfig(marshaler, std.DefaultPublicKeyCodec{}, tx.DefaultSignModes)
 
 	return EncodingConfig{
 		InterfaceRegistry: interfaceRegistry,

diff --git a/simapp/sim_bench_test.go b/simapp/sim_bench_test.go
@@ -30,8 +30,8 @@ func BenchmarkFullAppSimulation(b *testing.B) {
 
 	// run randomized simulation
 	_, simParams, simErr := simulation.SimulateFromSeed(
-		b, os.Stdout, app.BaseApp, AppStateFn(app.LegacyAmino(), app.SimulationManager()),
-		SimulationOperations(app, app.LegacyAmino(), config),
+		b, os.Stdout, app.BaseApp, AppStateFn(app.AppCodec(), app.SimulationManager()),
+		SimulationOperations(app, app.AppCodec(), config),
 		app.ModuleAccountAddrs(), config,
 	)
 
@@ -69,8 +69,8 @@ func BenchmarkInvariants(b *testing.B) {
 
 	// run randomized simulation
 	_, simParams, simErr := simulation.SimulateFromSeed(
-		b, os.Stdout, app.BaseApp, AppStateFn(app.LegacyAmino(), app.SimulationManager()),
-		SimulationOperations(app, app.LegacyAmino(), config),
+		b, os.Stdout, app.BaseApp, AppStateFn(app.AppCodec(), app.SimulationManager()),
+		SimulationOperations(app, app.AppCodec(), config),
 		app.ModuleAccountAddrs(), config,
 	)