From 447e7579a8c4ca869468ed2fd18f1a137d231156 Mon Sep 17 00:00:00 2001 From: rhuairahrighairigh Date: Tue, 28 Apr 2020 01:26:00 +0100 Subject: [PATCH] tidy up codec type registrations --- x/committee/alias.go | 38 +++++++++--------- x/committee/module.go | 2 +- x/committee/types/codec.go | 51 ++++++++++++------------ x/committee/types/committee.go | 9 +++-- x/committee/types/permissions.go | 25 ++++++------ x/committee/types/proposal.go | 29 ++++++-------- x/committee/types/proposal_test.go | 57 ++++++++++++++++++++++++++ x/committee/types/test_test.go | 64 ++++++++++++++++++++++++++++++ 8 files changed, 198 insertions(+), 77 deletions(-) create mode 100644 x/committee/types/proposal_test.go create mode 100644 x/committee/types/test_test.go diff --git a/x/committee/alias.go b/x/committee/alias.go index 17138e4b..5daadb23 100644 --- a/x/committee/alias.go +++ b/x/committee/alias.go @@ -41,25 +41,25 @@ const ( var ( // function aliases - NewKeeper = keeper.NewKeeper - NewQuerier = keeper.NewQuerier - DefaultGenesisState = types.DefaultGenesisState - GetKeyFromID = types.GetKeyFromID - GetVoteKey = types.GetVoteKey - NewCommittee = types.NewCommittee - NewCommitteeChangeProposal = types.NewCommitteeChangeProposal - NewCommitteeDeleteProposal = types.NewCommitteeDeleteProposal - NewGenesisState = types.NewGenesisState - NewMsgSubmitProposal = types.NewMsgSubmitProposal - NewMsgVote = types.NewMsgVote - NewProposal = types.NewProposal - NewQueryCommitteeParams = types.NewQueryCommitteeParams - NewQueryProposalParams = types.NewQueryProposalParams - NewQueryVoteParams = types.NewQueryVoteParams - RegisterAppCodec = types.RegisterAppCodec - RegisterModuleCodec = types.RegisterModuleCodec - RegisterProposalTypeCodec = types.RegisterProposalTypeCodec - Uint64FromBytes = types.Uint64FromBytes + NewKeeper = keeper.NewKeeper + NewQuerier = keeper.NewQuerier + DefaultGenesisState = types.DefaultGenesisState + GetKeyFromID = types.GetKeyFromID + GetVoteKey = types.GetVoteKey + NewCommittee = types.NewCommittee + NewCommitteeChangeProposal = types.NewCommitteeChangeProposal + NewCommitteeDeleteProposal = types.NewCommitteeDeleteProposal + NewGenesisState = types.NewGenesisState + NewMsgSubmitProposal = types.NewMsgSubmitProposal + NewMsgVote = types.NewMsgVote + NewProposal = types.NewProposal + NewQueryCommitteeParams = types.NewQueryCommitteeParams + NewQueryProposalParams = types.NewQueryProposalParams + NewQueryVoteParams = types.NewQueryVoteParams + RegisterCodec = types.RegisterCodec + RegisterPermissionTypeCodec = types.RegisterPermissionTypeCodec + RegisterProposalTypeCodec = types.RegisterProposalTypeCodec + Uint64FromBytes = types.Uint64FromBytes // variable aliases ProposalHandler = client.ProposalHandler diff --git a/x/committee/module.go b/x/committee/module.go index 1423eff7..84f06265 100644 --- a/x/committee/module.go +++ b/x/committee/module.go @@ -36,7 +36,7 @@ func (AppModuleBasic) Name() string { // RegisterCodec register module codec func (AppModuleBasic) RegisterCodec(cdc *codec.Codec) { - RegisterAppCodec(cdc) + RegisterCodec(cdc) } // DefaultGenesis default genesis state diff --git a/x/committee/types/codec.go b/x/committee/types/codec.go index 5c246fc0..dcbaddb6 100644 --- a/x/committee/types/codec.go +++ b/x/committee/types/codec.go @@ -2,39 +2,32 @@ package types import ( "github.com/cosmos/cosmos-sdk/codec" - "github.com/cosmos/cosmos-sdk/x/distribution" - "github.com/cosmos/cosmos-sdk/x/gov" - "github.com/cosmos/cosmos-sdk/x/params" + distrtypes "github.com/cosmos/cosmos-sdk/x/distribution/types" + govtypes "github.com/cosmos/cosmos-sdk/x/gov/types" + paramstypes "github.com/cosmos/cosmos-sdk/x/params/types" ) -// ModuleCdc generic sealed codec to be used throughout module +// ModuleCdc is a generic codec to be used throughout module var ModuleCdc *codec.Codec func init() { cdc := codec.New() - RegisterModuleCodec(cdc) - ModuleCdc = cdc.Seal() + RegisterCodec(cdc) + ModuleCdc = cdc + // ModuleCdc is not sealed so that other modules can register their own pubproposal and/or permission types. + + // Register external module pubproposal types. Ideally these would be registered within the modules' types pkg init function. + // However registration happens here as a work-around. + RegisterProposalTypeCodec(distrtypes.CommunityPoolSpendProposal{}, "cosmos-sdk/CommunityPoolSpendProposal") + RegisterProposalTypeCodec(paramstypes.ParameterChangeProposal{}, "cosmos-sdk/ParameterChangeProposal") + RegisterProposalTypeCodec(govtypes.TextProposal{}, "cosmos-sdk/TextProposal") } -// TODO decide if not using gov's Content type would be better +// RegisterCodec registers the necessary types for the module +func RegisterCodec(cdc *codec.Codec) { -func RegisterModuleCodec(cdc *codec.Codec) { - cdc.RegisterInterface((*gov.Content)(nil), nil) // registering the Content interface on the ModuleCdc will not conflict with gov. - // Ideally dist and params would register their proposals on here at their init. However don't want to fork them so: - cdc.RegisterConcrete(distribution.CommunityPoolSpendProposal{}, "cosmos-sdk/CommunityPoolSpendProposal", nil) - cdc.RegisterConcrete(params.ParameterChangeProposal{}, "cosmos-sdk/ParameterChangeProposal", nil) - cdc.RegisterConcrete(gov.TextProposal{}, "cosmos-sdk/TextProposal", nil) - - RegisterAppCodec(cdc) -} - -// RegisterAppCodec registers the necessary types for the module -func RegisterAppCodec(cdc *codec.Codec) { // Proposals - // The app codec needs the gov.Content type registered. This is done by the gov module. - // Ideally it would registered here as well in case these modules are ever used separately. - // However amino panics if you register the same interface a second time. So leaving it out for now. - // cdc.RegisterInterface((*gov.Content)(nil), nil) + cdc.RegisterInterface((*PubProposal)(nil), nil) cdc.RegisterConcrete(CommitteeChangeProposal{}, "kava/CommitteeChangeProposal", nil) cdc.RegisterConcrete(CommitteeDeleteProposal{}, "kava/CommitteeDeleteProposal", nil) @@ -49,9 +42,15 @@ func RegisterAppCodec(cdc *codec.Codec) { cdc.RegisterConcrete(MsgVote{}, "kava/MsgVote", nil) } -// RegisterProposalTypeCodec registers an external proposal content type defined -// in another module for the internal ModuleCdc. This allows the MsgSubmitProposal -// to be correctly Amino encoded and decoded. +// RegisterPermissionTypeCodec allows external modules to register their own permission types on +// the internal ModuleCdc. This allows the MsgSubmitProposal to be correctly Amino encoded and +// decoded (when the msg contains a CommitteeChangeProposal). +func RegisterPermissionTypeCodec(o interface{}, name string) { + ModuleCdc.RegisterConcrete(o, name, nil) +} + +// RegisterProposalTypeCodec allows external modules to register their own pubproposal types on the +// internal ModuleCdc. This allows the MsgSubmitProposal to be correctly Amino encoded and decoded. func RegisterProposalTypeCodec(o interface{}, name string) { ModuleCdc.RegisterConcrete(o, name, nil) } diff --git a/x/committee/types/committee.go b/x/committee/types/committee.go index fb22a196..c516f2bb 100644 --- a/x/committee/types/committee.go +++ b/x/committee/types/committee.go @@ -5,7 +5,7 @@ import ( "time" sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/x/gov" + govtypes "github.com/cosmos/cosmos-sdk/x/gov/types" "gopkg.in/yaml.v2" ) @@ -96,9 +96,12 @@ func (c Committee) Validate() error { // Proposals // ------------------------------------------ -// PubProposal is an interface that all gov proposals defined in other modules must satisfy. -type PubProposal = gov.Content +// PubProposal is the interface that all proposals must fulfill to be submitted to a committee. +// Proposal types can be created external to this module. For example a ParamChangeProposal, or CommunityPoolSpendProposal. +// It is pinned to the equivalent type in the gov module to create compatability between proposal types. +type PubProposal govtypes.Content +// Proposal is an internal record of a governance proposal submitted to a committee. type Proposal struct { PubProposal `json:"pub_proposal" yaml:"pub_proposal"` ID uint64 `json:"id" yaml:"id"` diff --git a/x/committee/types/permissions.go b/x/committee/types/permissions.go index 1b33a46e..3cfbd5f9 100644 --- a/x/committee/types/permissions.go +++ b/x/committee/types/permissions.go @@ -1,16 +1,17 @@ package types import ( - "github.com/cosmos/cosmos-sdk/x/gov" - "github.com/cosmos/cosmos-sdk/x/params" + govtypes "github.com/cosmos/cosmos-sdk/x/gov/types" + paramstypes "github.com/cosmos/cosmos-sdk/x/params/types" ) func init() { - // CommitteeChange/Delete proposals need to be registered on gov's ModuleCdc. + // CommitteeChange/Delete proposals are registered on gov's ModuleCdc (see proposal.go). // But since these proposals contain Permissions, these types also need registering: - gov.ModuleCdc.RegisterInterface((*Permission)(nil), nil) - gov.RegisterProposalTypeCodec(GodPermission{}, "kava/GodPermission") - gov.RegisterProposalTypeCodec(ParamChangePermission{}, "kava/ParamChangePermission") + govtypes.ModuleCdc.RegisterInterface((*Permission)(nil), nil) + govtypes.RegisterProposalTypeCodec(GodPermission{}, "kava/GodPermission") + govtypes.RegisterProposalTypeCodec(ParamChangePermission{}, "kava/ParamChangePermission") + govtypes.RegisterProposalTypeCodec(TextPermission{}, "kava/TextPermission") } // Permission is anything with a method that validates whether a proposal is allowed by it or not. @@ -27,7 +28,7 @@ type GodPermission struct{} var _ Permission = GodPermission{} -func (GodPermission) Allows(gov.Content) bool { return true } +func (GodPermission) Allows(PubProposal) bool { return true } func (GodPermission) MarshalYAML() (interface{}, error) { valueToMarshal := struct { @@ -49,8 +50,8 @@ type ParamChangePermission struct { var _ Permission = ParamChangePermission{} -func (perm ParamChangePermission) Allows(p gov.Content) bool { - proposal, ok := p.(params.ParameterChangeProposal) +func (perm ParamChangePermission) Allows(p PubProposal) bool { + proposal, ok := p.(paramstypes.ParameterChangeProposal) if !ok { return false } @@ -79,7 +80,7 @@ type AllowedParam struct { } type AllowedParams []AllowedParam -func (allowed AllowedParams) Contains(paramChange params.ParamChange) bool { +func (allowed AllowedParams) Contains(paramChange paramstypes.ParamChange) bool { for _, p := range allowed { if paramChange.Subspace == p.Subspace && paramChange.Key == p.Key { return true @@ -97,8 +98,8 @@ type TextPermission struct{} var _ Permission = TextPermission{} -func (TextPermission) Allows(p gov.Content) bool { - _, ok := p.(gov.TextProposal) +func (TextPermission) Allows(p PubProposal) bool { + _, ok := p.(govtypes.TextProposal) return ok } diff --git a/x/committee/types/proposal.go b/x/committee/types/proposal.go index 95f073fa..426df42d 100644 --- a/x/committee/types/proposal.go +++ b/x/committee/types/proposal.go @@ -12,19 +12,24 @@ const ( ProposalTypeCommitteeDelete = "CommitteeDelete" ) -// CommitteeChangeProposal is a gov proposal for creating a new committee or modifying an existing one. -type CommitteeChangeProposal struct { - Title string `json:"title" yaml:"title"` - Description string `json:"description" yaml:"description"` - NewCommittee Committee `json:"new_committee" yaml:"new_committee"` -} - -var _ govtypes.Content = CommitteeChangeProposal{} +// ensure proposal types fulfill the PubProposal interface and the gov Content interface. +var _, _ govtypes.Content = CommitteeChangeProposal{}, CommitteeDeleteProposal{} +var _, _ PubProposal = CommitteeChangeProposal{}, CommitteeDeleteProposal{} func init() { // Gov proposals need to be registered on gov's ModuleCdc so MsgSubmitProposal can be encoded. govtypes.RegisterProposalType(ProposalTypeCommitteeChange) govtypes.RegisterProposalTypeCodec(CommitteeChangeProposal{}, "kava/CommitteeChangeProposal") + + govtypes.RegisterProposalType(ProposalTypeCommitteeDelete) + govtypes.RegisterProposalTypeCodec(CommitteeDeleteProposal{}, "kava/CommitteeDeleteProposal") +} + +// CommitteeChangeProposal is a gov proposal for creating a new committee or modifying an existing one. +type CommitteeChangeProposal struct { + Title string `json:"title" yaml:"title"` + Description string `json:"description" yaml:"description"` + NewCommittee Committee `json:"new_committee" yaml:"new_committee"` } func NewCommitteeChangeProposal(title string, description string, newCommittee Committee) CommitteeChangeProposal { @@ -71,14 +76,6 @@ type CommitteeDeleteProposal struct { CommitteeID uint64 `json:"committee_id" yaml:"committee_id"` } -var _ govtypes.Content = CommitteeDeleteProposal{} - -func init() { - // Gov proposals need to be registered on gov's ModuleCdc so MsgSubmitProposal can be encoded. - govtypes.RegisterProposalType(ProposalTypeCommitteeDelete) - govtypes.RegisterProposalTypeCodec(CommitteeDeleteProposal{}, "kava/CommitteeDeleteProposal") -} - func NewCommitteeDeleteProposal(title string, description string, committeeID uint64) CommitteeDeleteProposal { return CommitteeDeleteProposal{ Title: title, diff --git a/x/committee/types/proposal_test.go b/x/committee/types/proposal_test.go new file mode 100644 index 00000000..0b6ab0e4 --- /dev/null +++ b/x/committee/types/proposal_test.go @@ -0,0 +1,57 @@ +package types + +import ( + "time" + + "github.com/cosmos/cosmos-sdk/codec" + sdk "github.com/cosmos/cosmos-sdk/types" + govtypes "github.com/cosmos/cosmos-sdk/x/gov/types" +) + +func (suite *TypesTestSuite) TestCommitteeChangeProposalMarshals() { + + ccp := CommitteeChangeProposal{ + Title: "A Title", + Description: "A description for this committee.", + NewCommittee: Committee{ + ID: 12, + Description: "This committee is for testing.", + Members: nil, + Permissions: []Permission{ParamChangePermission{}}, + VoteThreshold: d("0.667"), + ProposalDuration: time.Hour * 24 * 7, + }, + } + + appCdc := codec.New() + // register sdk types in case their needed + sdk.RegisterCodec(appCdc) + codec.RegisterCrypto(appCdc) + codec.RegisterEvidences(appCdc) + // register committee types + RegisterCodec(appCdc) + + var ppModuleCdc PubProposal + suite.NotPanics(func() { + ModuleCdc.MustUnmarshalBinaryBare( + ModuleCdc.MustMarshalBinaryBare(PubProposal(ccp)), + &ppModuleCdc, + ) + }) + + var ppAppCdc PubProposal + suite.NotPanics(func() { + appCdc.MustUnmarshalBinaryBare( + appCdc.MustMarshalBinaryBare(PubProposal(ccp)), + &ppAppCdc, + ) + }) + + var ppGovCdc govtypes.Content + suite.NotPanics(func() { + govtypes.ModuleCdc.MustUnmarshalBinaryBare( + govtypes.ModuleCdc.MustMarshalBinaryBare(govtypes.Content(ccp)), + &ppGovCdc, + ) + }) +} diff --git a/x/committee/types/test_test.go b/x/committee/types/test_test.go new file mode 100644 index 00000000..3234a830 --- /dev/null +++ b/x/committee/types/test_test.go @@ -0,0 +1,64 @@ +package types + +import ( + "fmt" + "testing" + + "github.com/cosmos/cosmos-sdk/codec" +) + +type InterA interface { + GetTitle() string +} + +type InterB InterA + +// interface { +// GetDescription() string +// } + +type Prop1 struct{} + +func (p Prop1) GetTitle() string { return "prop1 title" } +func (p Prop1) GetDescription() string { return "prop1 description" } + +type Prop2 struct{} + +func (p Prop2) GetTitle() string { return "prop2 title" } +func (p Prop2) GetDescription() string { return "prop2 description" } + +func TestTest(t *testing.T) { + /* + register content, register new pubproposal + register concrete types (should satisfy both of them) + + try marshalling and unmarshalling all 4 combinations + */ + cdc := codec.New() + + cdc.RegisterInterface((*InterA)(nil), nil) + cdc.RegisterConcrete(Prop1{}, "test/prop1", nil) + cdc.RegisterInterface((*InterB)(nil), nil) + cdc.RegisterConcrete(Prop2{}, "test/prop2", nil) + + p1ia := InterA(Prop1{}) + p2ia := InterA(Prop2{}) + p1ib := InterB(Prop1{}) + p2ib := InterB(Prop2{}) + + var iap1 InterA + cdc.MustUnmarshalBinaryBare(cdc.MustMarshalBinaryBare(p1ia), &iap1) + fmt.Printf("%T, %T\n", p1ia, iap1) + + var iap2 InterA + cdc.MustUnmarshalBinaryBare(cdc.MustMarshalBinaryBare(p2ia), &iap2) + fmt.Printf("%T, %T\n", p2ia, iap2) + + var ibp1 InterB + cdc.MustUnmarshalBinaryBare(cdc.MustMarshalBinaryBare(p1ib), &ibp1) + fmt.Printf("%T, %T\n", p1ib, ibp1) + + var ibp2 InterB + cdc.MustUnmarshalBinaryBare(cdc.MustMarshalBinaryBare(p2ib), &ibp2) + fmt.Printf("%T, %T\n", p2ib, ibp2) +}