Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Start removing HybridCodec (init + auth, bank, distribution) #6838

Merged
merged 6 commits into from
Jul 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions codec/any.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (

// MarshalAny is a convenience function for packing the provided value in an
// Any and then proto marshaling it to bytes
func MarshalAny(m Marshaler, x interface{}) ([]byte, error) {
func MarshalAny(m BinaryMarshaler, x interface{}) ([]byte, error) {
msg, ok := x.(proto.Message)
if !ok {
return nil, fmt.Errorf("can't proto marshal %T", x)
Expand All @@ -32,7 +32,7 @@ func MarshalAny(m Marshaler, x interface{}) ([]byte, error) {
// Ex:
// var x MyInterface
// err := UnmarshalAny(unpacker, &x, bz)
func UnmarshalAny(m Marshaler, iface interface{}, bz []byte) error {
func UnmarshalAny(m BinaryMarshaler, iface interface{}, bz []byte) error {
any := &types.Any{}

err := m.UnmarshalBinaryBare(bz, any)
Expand Down
8 changes: 5 additions & 3 deletions codec/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,12 @@ type (
//
// 1. AminoCodec: Provides full Amino serialization compatibility.
// 2. ProtoCodec: Provides full Protobuf serialization compatibility.
// 3. HybridCodec: Provides Protobuf serialization for binary encoding and Amino
// for JSON encoding.
Marshaler interface {
BinaryMarshaler
JSONMarshaler
}

BinaryMarshaler interface {
MarshalBinaryBare(o ProtoMarshaler) ([]byte, error)
MustMarshalBinaryBare(o ProtoMarshaler) []byte

Expand All @@ -32,7 +35,6 @@ type (
UnmarshalBinaryLengthPrefixed(bz []byte, ptr ProtoMarshaler) error
MustUnmarshalBinaryLengthPrefixed(bz []byte, ptr ProtoMarshaler)

JSONMarshaler
types.AnyUnpacker
}

Expand Down
2 changes: 1 addition & 1 deletion simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ func NewSimApp(
)

app.mm.RegisterInvariants(&app.CrisisKeeper)
app.mm.RegisterRoutes(app.Router(), app.QueryRouter())
app.mm.RegisterRoutes(app.Router(), app.QueryRouter(), codec.NewAminoCodec(encodingConfig.Amino))
app.mm.RegisterQueryServices(app.GRPCQueryRouter())

// add test gRPC service for testing gRPC queries in isolation
Expand Down
13 changes: 7 additions & 6 deletions tests/mocks/types_module_module.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 5 additions & 5 deletions types/module/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ type AppModule interface {
// Deprecated: use RegisterQueryService
QuerierRoute() string
// Deprecated: use RegisterQueryService
NewQuerierHandler() sdk.Querier
LegacyQuerierHandler(codec.JSONMarshaler) sdk.Querier
// RegisterQueryService allows a module to register a gRPC query service
RegisterQueryService(grpc.Server)

Expand Down Expand Up @@ -192,8 +192,8 @@ func (GenesisOnlyAppModule) Route() sdk.Route { return sdk.Route{} }
// QuerierRoute returns an empty module querier route
func (GenesisOnlyAppModule) QuerierRoute() string { return "" }

// NewQuerierHandler returns an empty module querier
func (gam GenesisOnlyAppModule) NewQuerierHandler() sdk.Querier { return nil }
// LegacyQuerierHandler returns an empty module querier
func (gam GenesisOnlyAppModule) LegacyQuerierHandler(codec.JSONMarshaler) sdk.Querier { return nil }

func (gam GenesisOnlyAppModule) RegisterQueryService(grpc.Server) {}

Expand Down Expand Up @@ -264,13 +264,13 @@ func (m *Manager) RegisterInvariants(ir sdk.InvariantRegistry) {
}

// RegisterRoutes registers all module routes and module querier routes
func (m *Manager) RegisterRoutes(router sdk.Router, queryRouter sdk.QueryRouter) {
func (m *Manager) RegisterRoutes(router sdk.Router, queryRouter sdk.QueryRouter, legacyQuerierCdc codec.JSONMarshaler) {
for _, module := range m.Modules {
if !module.Route().Empty() {
router.AddRoute(module.Route())
}
if module.QuerierRoute() != "" {
queryRouter.AddRoute(module.QuerierRoute(), module.NewQuerierHandler())
queryRouter.AddRoute(module.QuerierRoute(), module.LegacyQuerierHandler(legacyQuerierCdc))
}
}
}
Expand Down
6 changes: 4 additions & 2 deletions types/module/module_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func TestGenesisOnlyAppModule(t *testing.T) {

require.True(t, goam.Route().Empty())
require.Empty(t, goam.QuerierRoute())
require.Nil(t, goam.NewQuerierHandler())
require.Nil(t, goam.LegacyQuerierHandler(nil))

// no-op
goam.RegisterInvariants(mockInvariantRegistry)
Expand Down Expand Up @@ -163,7 +163,9 @@ func TestManager_RegisterRoutes(t *testing.T) {
mockAppModule1.EXPECT().NewQuerierHandler().Times(1).Return(handler3)
queryRouter.EXPECT().AddRoute(gomock.Eq("querierRoute1"), gomock.Eq(handler3)).Times(1)

mm.RegisterRoutes(router, queryRouter)
amino := codec.New()
jsonCdc := codec.NewAminoCodec(amino)
mm.RegisterRoutes(router, queryRouter, jsonCdc)
}

func TestManager_RegisterQueryServices(t *testing.T) {
Expand Down
22 changes: 3 additions & 19 deletions x/auth/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"github.com/tendermint/tendermint/libs/log"

"github.com/cosmos/cosmos-sdk/codec"
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
"github.com/cosmos/cosmos-sdk/x/auth/types"
Expand All @@ -19,7 +18,7 @@ import (
// encoding/decoding library.
type AccountKeeper struct {
key sdk.StoreKey
cdc codec.Marshaler
cdc codec.BinaryMarshaler
paramSubspace paramtypes.Subspace
permAddrs map[string]types.PermissionsForAddress

Expand All @@ -30,7 +29,7 @@ type AccountKeeper struct {
// NewAccountKeeper returns a new sdk.AccountKeeper that uses go-amino to
// (binary) encode and decode concrete sdk.Accounts.
func NewAccountKeeper(
cdc codec.Marshaler, key sdk.StoreKey, paramstore paramtypes.Subspace, proto func() types.AccountI,
cdc codec.BinaryMarshaler, key sdk.StoreKey, paramstore paramtypes.Subspace, proto func() types.AccountI,
maccPerms map[string][]string,
) AccountKeeper {

Expand Down Expand Up @@ -203,19 +202,4 @@ func (ak AccountKeeper) UnmarshalAccount(bz []byte) (types.AccountI, error) {
return acc, nil
}

// UnmarshalAccountJSON returns an AccountI from JSON encoded bytes
func (ak AccountKeeper) UnmarshalAccountJSON(bz []byte) (types.AccountI, error) {
var any codectypes.Any
if err := ak.cdc.UnmarshalJSON(bz, &any); err != nil {
return nil, err
}

var acc types.AccountI
if err := ak.cdc.UnpackAny(&any, &acc); err != nil {
return nil, err
}

return acc, nil
}

func (ak AccountKeeper) GetCodec() codec.Marshaler { return ak.cdc }
func (ak AccountKeeper) GetCodec() codec.BinaryMarshaler { return ak.cdc }
16 changes: 8 additions & 8 deletions x/auth/keeper/querier.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,24 +10,24 @@ import (
)

// NewQuerier creates a querier for auth REST endpoints
func NewQuerier(k AccountKeeper) sdk.Querier {
func NewQuerier(k AccountKeeper, cdc codec.JSONMarshaler) sdk.Querier {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we be consistent with the parameter naming?

Suggested change
func NewQuerier(k AccountKeeper, cdc codec.JSONMarshaler) sdk.Querier {
func NewQuerier(k AccountKeeper, legacyQuerierCdc codec.JSONMarshaler) sdk.Querier {

return func(ctx sdk.Context, path []string, req abci.RequestQuery) ([]byte, error) {
switch path[0] {
case types.QueryAccount:
return queryAccount(ctx, req, k)
return queryAccount(ctx, req, k, cdc)

case types.QueryParams:
return queryParams(ctx, k)
return queryParams(ctx, k, cdc)

default:
return nil, sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "unknown query path: %s", path[0])
}
}
}

func queryAccount(ctx sdk.Context, req abci.RequestQuery, k AccountKeeper) ([]byte, error) {
func queryAccount(ctx sdk.Context, req abci.RequestQuery, k AccountKeeper, cdc codec.JSONMarshaler) ([]byte, error) {
var params types.QueryAccountRequest
if err := k.cdc.UnmarshalJSON(req.Data, &params); err != nil {
if err := cdc.UnmarshalJSON(req.Data, &params); err != nil {
return nil, sdkerrors.Wrap(sdkerrors.ErrJSONUnmarshal, err.Error())
}

Expand All @@ -36,18 +36,18 @@ func queryAccount(ctx sdk.Context, req abci.RequestQuery, k AccountKeeper) ([]by
return nil, sdkerrors.Wrapf(sdkerrors.ErrUnknownAddress, "account %s does not exist", params.Address)
}

bz, err := codec.MarshalJSONIndent(k.cdc, account)
bz, err := codec.MarshalJSONIndent(cdc, account)
if err != nil {
return nil, sdkerrors.Wrap(sdkerrors.ErrJSONMarshal, err.Error())
}

return bz, nil
}

func queryParams(ctx sdk.Context, k AccountKeeper) ([]byte, error) {
func queryParams(ctx sdk.Context, k AccountKeeper, cdc codec.JSONMarshaler) ([]byte, error) {
params := k.GetParams(ctx)

res, err := codec.MarshalJSONIndent(k.cdc, params)
res, err := codec.MarshalJSONIndent(cdc, params)
if err != nil {
return nil, sdkerrors.Wrap(sdkerrors.ErrJSONMarshal, err.Error())
}
Expand Down
12 changes: 7 additions & 5 deletions x/auth/keeper/querier_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"fmt"
"testing"

"github.com/cosmos/cosmos-sdk/codec"

"github.com/stretchr/testify/require"

abci "github.com/tendermint/tendermint/abci/types"
Expand All @@ -15,15 +17,15 @@ import (

func TestQueryAccount(t *testing.T) {
app, ctx := createTestApp(true)
cdc := app.Codec()
jsonCdc := codec.NewAminoCodec(app.Codec())

req := abci.RequestQuery{
Path: "",
Data: []byte{},
}

path := []string{types.QueryAccount}
querier := keep.NewQuerier(app.AccountKeeper)
querier := keep.NewQuerier(app.AccountKeeper, jsonCdc)

bz, err := querier(ctx, []string{"other"}, req)
require.Error(t, err)
Expand All @@ -37,13 +39,13 @@ func TestQueryAccount(t *testing.T) {
require.Error(t, err)
require.Nil(t, res)

req.Data = cdc.MustMarshalJSON(types.QueryAccountRequest{Address: []byte("")})
req.Data = jsonCdc.MustMarshalJSON(types.QueryAccountRequest{Address: []byte("")})
res, err = querier(ctx, path, req)
require.Error(t, err)
require.Nil(t, res)

_, _, addr := testdata.KeyTestPubAddr()
req.Data = cdc.MustMarshalJSON(types.QueryAccountRequest{Address: addr})
req.Data = jsonCdc.MustMarshalJSON(types.QueryAccountRequest{Address: addr})
res, err = querier(ctx, path, req)
require.Error(t, err)
require.Nil(t, res)
Expand All @@ -58,6 +60,6 @@ func TestQueryAccount(t *testing.T) {
require.NotNil(t, res)

var account types.AccountI
err2 := cdc.UnmarshalJSON(res, &account)
err2 := jsonCdc.UnmarshalJSON(res, &account)
require.Nil(t, err2)
}
6 changes: 3 additions & 3 deletions x/auth/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,9 @@ func (AppModule) QuerierRoute() string {
return types.QuerierRoute
}

// NewQuerierHandler returns the auth module sdk.Querier.
func (am AppModule) NewQuerierHandler() sdk.Querier {
return keeper.NewQuerier(am.accountKeeper)
// LegacyQuerierHandler returns the auth module sdk.Querier.
func (am AppModule) LegacyQuerierHandler(jsonCdc codec.JSONMarshaler) sdk.Querier {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
func (am AppModule) LegacyQuerierHandler(jsonCdc codec.JSONMarshaler) sdk.Querier {
func (am AppModule) LegacyQuerierHandler(legacyQuerierCdc codec.JSONMarshaler) sdk.Querier {

return keeper.NewQuerier(am.accountKeeper, jsonCdc)
}

// RegisterQueryService registers a GRPC query service to respond to the
Expand Down
2 changes: 1 addition & 1 deletion x/auth/simulation/decoder.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (

type AuthUnmarshaler interface {
UnmarshalAccount([]byte) (types.AccountI, error)
GetCodec() codec.Marshaler
GetCodec() codec.BinaryMarshaler
}

// NewDecodeStore returns a decoder function closure that unmarshals the KVPair's
Expand Down
40 changes: 2 additions & 38 deletions x/bank/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,7 @@ import (
"fmt"
"time"

"github.com/gogo/protobuf/proto"

"github.com/cosmos/cosmos-sdk/codec"
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
Expand Down Expand Up @@ -42,8 +39,6 @@ type Keeper interface {
UndelegateCoins(ctx sdk.Context, moduleAccAddr, delegatorAddr sdk.AccAddress, amt sdk.Coins) error
MarshalSupply(supplyI exported.SupplyI) ([]byte, error)
UnmarshalSupply(bz []byte) (exported.SupplyI, error)
MarshalSupplyJSON(supply exported.SupplyI) ([]byte, error)
UnmarshalSupplyJSON(bz []byte) (exported.SupplyI, error)

types.QueryServer
}
Expand All @@ -53,13 +48,13 @@ type BaseKeeper struct {
BaseSendKeeper

ak types.AccountKeeper
cdc codec.Marshaler
cdc codec.BinaryMarshaler
storeKey sdk.StoreKey
paramSpace paramtypes.Subspace
}

func NewBaseKeeper(
cdc codec.Marshaler, storeKey sdk.StoreKey, ak types.AccountKeeper, paramSpace paramtypes.Subspace,
cdc codec.BinaryMarshaler, storeKey sdk.StoreKey, ak types.AccountKeeper, paramSpace paramtypes.Subspace,
blockedAddrs map[string]bool,
) BaseKeeper {

Expand Down Expand Up @@ -370,34 +365,3 @@ func (k BaseKeeper) UnmarshalSupply(bz []byte) (exported.SupplyI, error) {

return evi, nil
}

// MarshalSupplyJSON JSON encodes a supply object implementing the Supply
// interface.
func (k BaseKeeper) MarshalSupplyJSON(supply exported.SupplyI) ([]byte, error) {
msg, ok := supply.(proto.Message)
if !ok {
return nil, fmt.Errorf("cannot proto marshal %T", supply)
}

any, err := codectypes.NewAnyWithValue(msg)
if err != nil {
return nil, err
}

return k.cdc.MarshalJSON(any)
}

// UnmarshalSupplyJSON returns a Supply from JSON encoded bytes
func (k BaseKeeper) UnmarshalSupplyJSON(bz []byte) (exported.SupplyI, error) {
var any codectypes.Any
if err := k.cdc.UnmarshalJSON(bz, &any); err != nil {
return nil, err
}

var supply exported.SupplyI
if err := k.cdc.UnpackAny(&any, &supply); err != nil {
return nil, err
}

return supply, nil
}
Loading