Merge PR #5619: proto: migrate params to hybrid codec

tac0turtle · web-flow · commit 48d39e344e84 · 2020-02-12T11:34:09.000-05:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -44,6 +44,8 @@ balances or a single balance by denom when the `denom` query parameter is presen
 
 ### API Breaking Changes
 
+* (x/params) [\#5619](https://github.com/cosmos/cosmos-sdk/pull/5619) The `x/params` keeper now accepts a `codec.Marshaller` instead of
+a reference to an amino codec. Amino is still used for JSON serialization.
 * (types) [\#5579](https://github.com/cosmos/cosmos-sdk/pull/5579) The `keepRecent` field has been removed from the `PruningOptions` type.
 The `PruningOptions` type now only includes fields `KeepEvery` and `SnapshotEvery`, where `KeepEvery`
 determines which committed heights are flushed to disk and `SnapshotEvery` determines which of these
diff --git a/simapp/app.go b/simapp/app.go
@@ -150,7 +150,7 @@ func NewSimApp(
 	}
 
 	// init params keeper and subspaces
-	app.ParamsKeeper = params.NewKeeper(app.cdc, keys[params.StoreKey], tkeys[params.TStoreKey])
+	app.ParamsKeeper = params.NewKeeper(appCodec.Params, keys[params.StoreKey], tkeys[params.TStoreKey])
 	app.subspaces[auth.ModuleName] = app.ParamsKeeper.Subspace(auth.DefaultParamspace)
 	app.subspaces[bank.ModuleName] = app.ParamsKeeper.Subspace(bank.DefaultParamspace)
 	app.subspaces[staking.ModuleName] = app.ParamsKeeper.Subspace(staking.DefaultParamspace)
diff --git a/simapp/codec.go b/simapp/codec.go
@@ -5,6 +5,7 @@ import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/x/auth/vesting"
 	distr "github.com/cosmos/cosmos-sdk/x/distribution"
+	"github.com/cosmos/cosmos-sdk/x/params"
 	"github.com/cosmos/cosmos-sdk/x/staking"
 )
 
@@ -13,6 +14,7 @@ import (
 type AppCodec struct {
 	amino *codec.Codec
 
+	Params       *params.Codec
 	Staking      *staking.Codec
 	Distribution *distr.Codec
 }
@@ -22,6 +24,7 @@ func NewAppCodec() *AppCodec {
 
 	return &AppCodec{
 		amino:        amino,
+		Params:       params.NewCodec(amino),
 		Staking:      staking.NewCodec(amino),
 		Distribution: distr.NewCodec(amino),
 	}
diff --git a/x/distribution/keeper/test_common.go b/x/distribution/keeper/test_common.go
@@ -126,7 +126,7 @@ func CreateTestInputAdvanced(
 	blacklistedAddrs[distrAcc.GetAddress().String()] = true
 
 	cdc := MakeTestCodec()
-	pk := params.NewKeeper(cdc, keyParams, tkeyParams)
+	pk := params.NewKeeper(params.ModuleCdc, keyParams, tkeyParams)
 
 	ctx := sdk.NewContext(ms, abci.Header{ChainID: "foochainid"}, isCheckTx, log.NewNopLogger())
 	accountKeeper := auth.NewAccountKeeper(cdc, keyAcc, pk.Subspace(auth.DefaultParamspace), auth.ProtoBaseAccount)
diff --git a/x/gov/keeper/test_common.go b/x/gov/keeper/test_common.go
@@ -146,7 +146,7 @@ func createTestInput(
 	blacklistedAddrs[notBondedPool.GetAddress().String()] = true
 	blacklistedAddrs[bondPool.GetAddress().String()] = true
 
-	pk := params.NewKeeper(cdc, keyParams, tkeyParams)
+	pk := params.NewKeeper(params.ModuleCdc, keyParams, tkeyParams)
 	accountKeeper := auth.NewAccountKeeper(cdc, keyAcc, pk.Subspace(auth.DefaultParamspace), auth.ProtoBaseAccount)
 	bankKeeper := bank.NewBaseKeeper(cdc, keyBank, accountKeeper, pk.Subspace(bank.DefaultParamspace), blacklistedAddrs)
 	supplyKeeper := supply.NewKeeper(cdc, keySupply, accountKeeper, bankKeeper, maccPerms)
diff --git a/x/mock/app.go b/x/mock/app.go
@@ -69,7 +69,7 @@ func NewApp() *App {
 		TotalCoinsSupply: sdk.NewCoins(),
 	}
 
-	app.ParamsKeeper = params.NewKeeper(app.Cdc, app.KeyParams, app.TKeyParams)
+	app.ParamsKeeper = params.NewKeeper(params.ModuleCdc, app.KeyParams, app.TKeyParams)
 	app.AccountKeeper = auth.NewAccountKeeper(
 		app.Cdc,
 		app.KeyAccount,
diff --git a/x/params/alias.go b/x/params/alias.go
@@ -30,12 +30,14 @@ var (
 	NewParameterChangeProposal = types.NewParameterChangeProposal
 	NewParamChange             = types.NewParamChange
 	ValidateChanges            = types.ValidateChanges
+	NewCodec                   = types.NewCodec
 
 	// variable aliases
 	ModuleCdc = types.ModuleCdc
 )
 
 type (
+	Codec                   = types.Codec
 	ParamSetPair            = subspace.ParamSetPair
 	ParamSetPairs           = subspace.ParamSetPairs
 	ParamSet                = subspace.ParamSet
diff --git a/x/params/commmon_test.go b/x/params/commmon_test.go
@@ -17,12 +17,12 @@ type s struct {
 	I int
 }
 
-func createTestCodec() *codec.Codec {
+func createTestCodec() codec.Marshaler {
 	cdc := codec.New()
 	sdk.RegisterCodec(cdc)
 	cdc.RegisterConcrete(s{}, "test/s", nil)
 	cdc.RegisterConcrete(invalid{}, "test/invalid", nil)
-	return cdc
+	return NewCodec(cdc)
 }
 
 func defaultContext(key sdk.StoreKey, tkey sdk.StoreKey) sdk.Context {
@@ -38,7 +38,7 @@ func defaultContext(key sdk.StoreKey, tkey sdk.StoreKey) sdk.Context {
 	return ctx
 }
 
-func testComponents() (*codec.Codec, sdk.Context, sdk.StoreKey, sdk.StoreKey, Keeper) {
+func testComponents() (codec.Marshaler, sdk.Context, sdk.StoreKey, sdk.StoreKey, Keeper) {
 	cdc := createTestCodec()
 	mkey := sdk.NewKVStoreKey("test")
 	tkey := sdk.NewTransientStoreKey("transient_test")
diff --git a/x/params/keeper.go b/x/params/keeper.go
@@ -13,14 +13,14 @@ import (
 
 // Keeper of the global paramstore
 type Keeper struct {
-	cdc    *codec.Codec
+	cdc    codec.Marshaler
 	key    sdk.StoreKey
 	tkey   sdk.StoreKey
 	spaces map[string]*Subspace
 }
 
 // NewKeeper constructs a params keeper
-func NewKeeper(cdc *codec.Codec, key, tkey sdk.StoreKey) Keeper {
+func NewKeeper(cdc codec.Marshaler, key, tkey sdk.StoreKey) Keeper {
 	return Keeper{
 		cdc:    cdc,
 		key:    key,
diff --git a/x/params/proposal_handler_test.go b/x/params/proposal_handler_test.go
@@ -74,7 +74,7 @@ func newTestInput(t *testing.T) testInput {
 	err := cms.LoadLatestVersion()
 	require.Nil(t, err)
 
-	keeper := params.NewKeeper(cdc, keyParams, tKeyParams)
+	keeper := params.NewKeeper(types.ModuleCdc, keyParams, tKeyParams)
 	ctx := sdk.NewContext(cms, abci.Header{}, false, log.NewNopLogger())
 
 	return testInput{ctx, cdc, keeper}
diff --git a/x/params/subspace/subspace.go b/x/params/subspace/subspace.go
@@ -22,15 +22,15 @@ const (
 // Transient store persists for a block, so we use it for
 // recording whether the parameter has been changed or not
 type Subspace struct {
-	cdc   *codec.Codec
+	cdc   codec.Marshaler
 	key   sdk.StoreKey // []byte -> []byte, stores parameter
 	tkey  sdk.StoreKey // []byte -> bool, stores parameter change
 	name  []byte
 	table KeyTable
 }
 
 // NewSubspace constructs a store with namestore
-func NewSubspace(cdc *codec.Codec, key sdk.StoreKey, tkey sdk.StoreKey, name string) Subspace {
+func NewSubspace(cdc codec.Marshaler, key sdk.StoreKey, tkey sdk.StoreKey, name string) Subspace {
 	return Subspace{
 		cdc:   cdc,
 		key:   key,
@@ -99,6 +99,8 @@ func (s Subspace) Validate(ctx sdk.Context, key []byte, value interface{}) error
 // Get queries for a parameter by key from the Subspace's KVStore and sets the
 // value to the provided pointer. If the value does not exist, it will panic.
 func (s Subspace) Get(ctx sdk.Context, key []byte, ptr interface{}) {
+	s.checkType(key, ptr)
+
 	store := s.kvStore(ctx)
 	bz := store.Get(key)
 
diff --git a/x/params/subspace/subspace_test.go b/x/params/subspace/subspace_test.go
@@ -14,18 +14,19 @@ import (
 	"github.com/cosmos/cosmos-sdk/store"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/x/params/subspace"
+	"github.com/cosmos/cosmos-sdk/x/params/types"
 )
 
 type SubspaceTestSuite struct {
 	suite.Suite
 
-	cdc *codec.Codec
+	cdc codec.Marshaler
 	ctx sdk.Context
 	ss  subspace.Subspace
 }
 
 func (suite *SubspaceTestSuite) SetupTest() {
-	cdc := codec.New()
+	cdc := types.ModuleCdc
 	db := dbm.NewMemDB()
 
 	ms := store.NewCommitMultiStore(db)
@@ -46,7 +47,7 @@ func (suite *SubspaceTestSuite) TestKeyTable() {
 		suite.ss.WithKeyTable(paramKeyTable())
 	})
 	suite.Require().NotPanics(func() {
-		ss := subspace.NewSubspace(codec.New(), key, tkey, "testsubspace2")
+		ss := subspace.NewSubspace(types.ModuleCdc, key, tkey, "testsubspace2")
 		ss = ss.WithKeyTable(paramKeyTable())
 	})
 }
diff --git a/x/params/types/codec.go b/x/params/types/codec.go
@@ -4,13 +4,26 @@ import (
 	"github.com/cosmos/cosmos-sdk/codec"
 )
 
+type Codec struct {
+	codec.Marshaler
+
+	// Keep reference to the amino codec to allow backwards compatibility along
+	// with type, and interface registration.
+	amino *codec.Codec
+}
+
+func NewCodec(amino *codec.Codec) *Codec {
+	return &Codec{Marshaler: codec.NewHybridCodec(amino), amino: amino}
+}
+
 // module codec
-var ModuleCdc *codec.Codec
+var ModuleCdc *Codec
 
 func init() {
-	ModuleCdc = codec.New()
-	RegisterCodec(ModuleCdc)
-	ModuleCdc.Seal()
+	ModuleCdc = NewCodec(codec.New())
+
+	RegisterCodec(ModuleCdc.amino)
+	ModuleCdc.amino.Seal()
 }
 
 // RegisterCodec registers all necessary param module types with a given codec.
diff --git a/x/params/types/proposal.go b/x/params/types/proposal.go
@@ -4,6 +4,8 @@ import (
 	"fmt"
 	"strings"
 
+	"gopkg.in/yaml.v2"
+
 	govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
 )
 
@@ -20,14 +22,6 @@ func init() {
 	govtypes.RegisterProposalTypeCodec(ParameterChangeProposal{}, "cosmos-sdk/ParameterChangeProposal")
 }
 
-// ParameterChangeProposal defines a proposal which contains multiple parameter
-// changes.
-type ParameterChangeProposal struct {
-	Title       string        `json:"title" yaml:"title"`
-	Description string        `json:"description" yaml:"description"`
-	Changes     []ParamChange `json:"changes" yaml:"changes"`
-}
-
 func NewParameterChangeProposal(title, description string, changes []ParamChange) ParameterChangeProposal {
 	return ParameterChangeProposal{title, description, changes}
 }
@@ -75,24 +69,14 @@ func (pcp ParameterChangeProposal) String() string {
 	return b.String()
 }
 
-// ParamChange defines a parameter change.
-type ParamChange struct {
-	Subspace string `json:"subspace" yaml:"subspace"`
-	Key      string `json:"key" yaml:"key"`
-	Value    string `json:"value" yaml:"value"`
-}
-
 func NewParamChange(subspace, key, value string) ParamChange {
 	return ParamChange{subspace, key, value}
 }
 
 // String implements the Stringer interface.
 func (pc ParamChange) String() string {
-	return fmt.Sprintf(`Param Change:
-  Subspace: %s
-  Key:      %s
-  Value:    %X
-`, pc.Subspace, pc.Key, pc.Value)
+	out, _ := yaml.Marshal(pc)
+	return string(out)
 }
 
 // ValidateChanges performs basic validation checks over a set of ParamChange. It
diff --git a/x/params/types/types.pb.go b/x/params/types/types.pb.go
diff --git a/x/params/types/types.proto b/x/params/types/types.proto
diff --git a/x/slashing/internal/keeper/test_common.go b/x/slashing/internal/keeper/test_common.go
diff --git a/x/staking/keeper/test_common.go b/x/staking/keeper/test_common.go

Original file line number	Diff line number	Diff line change
`@@ -150,7 +150,7 @@ func NewSimApp(`
`150`	`150`	`}`
`151`	`151`
`152`	`152`	`// init params keeper and subspaces`
`153`		`- app.ParamsKeeper = params.NewKeeper(app.cdc, keys[params.StoreKey], tkeys[params.TStoreKey])`
	`153`	`+ app.ParamsKeeper = params.NewKeeper(appCodec.Params, keys[params.StoreKey], tkeys[params.TStoreKey])`
`154`	`154`	`app.subspaces[auth.ModuleName] = app.ParamsKeeper.Subspace(auth.DefaultParamspace)`
`155`	`155`	`app.subspaces[bank.ModuleName] = app.ParamsKeeper.Subspace(bank.DefaultParamspace)`
`156`	`156`	`app.subspaces[staking.ModuleName] = app.ParamsKeeper.Subspace(staking.DefaultParamspace)`
Original file line number	Diff line number	Diff line change
`@@ -5,6 +5,7 @@ import (`
`5`	`5`	`sdk "github.com/cosmos/cosmos-sdk/types"`
`6`	`6`	`"github.com/cosmos/cosmos-sdk/x/auth/vesting"`
`7`	`7`	`distr "github.com/cosmos/cosmos-sdk/x/distribution"`
	`8`	`+ "github.com/cosmos/cosmos-sdk/x/params"`
`8`	`9`	`"github.com/cosmos/cosmos-sdk/x/staking"`
`9`	`10`	`)`
`10`	`11`
`@@ -13,6 +14,7 @@ import (`
`13`	`14`	`type AppCodec struct {`
`14`	`15`	`amino *codec.Codec`
`15`	`16`
	`17`	`+ Params *params.Codec`
`16`	`18`	`Staking *staking.Codec`
`17`	`19`	`Distribution *distr.Codec`
`18`	`20`	`}`
`@@ -22,6 +24,7 @@ func NewAppCodec() *AppCodec {`
`22`	`24`
`23`	`25`	`return &AppCodec{`
`24`	`26`	`amino: amino,`
	`27`	`+ Params: params.NewCodec(amino),`
`25`	`28`	`Staking: staking.NewCodec(amino),`
`26`	`29`	`Distribution: distr.NewCodec(amino),`
`27`	`30`	`}`
Original file line number	Diff line number	Diff line change
`@@ -69,7 +69,7 @@ func NewApp() *App {`
`69`	`69`	`TotalCoinsSupply: sdk.NewCoins(),`
`70`	`70`	`}`
`71`	`71`
`72`		`- app.ParamsKeeper = params.NewKeeper(app.Cdc, app.KeyParams, app.TKeyParams)`
	`72`	`+ app.ParamsKeeper = params.NewKeeper(params.ModuleCdc, app.KeyParams, app.TKeyParams)`
`73`	`73`	`app.AccountKeeper = auth.NewAccountKeeper(`
`74`	`74`	`app.Cdc,`
`75`	`75`	`app.KeyAccount,`