From 2c702e656313bb00ab861f856edfee87231ff5d0 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Mon, 22 May 2023 13:54:42 +0200 Subject: [PATCH] feat(gov,group): improve metadata checks and add docs (#16235) --- CHANGELOG.md | 19 +++++++++--------- api/cosmos/gov/v1/gov.pulsar.go | 4 +++- api/cosmos/group/v1/types.pulsar.go | 6 +++++- proto/cosmos/gov/v1/gov.proto | 4 +++- proto/cosmos/group/v1/types.proto | 6 +++++- x/gov/keeper/msg_server.go | 20 ++++++++++++++++++- x/gov/keeper/msg_server_test.go | 30 +++++++++++++++++++++++++++++ x/gov/types/v1/gov.pb.go | 4 +++- x/group/keeper/msg_server.go | 20 +++++++++++++++++++ x/group/keeper/msg_server_test.go | 24 +++++++++++++++++++++++ x/group/types.pb.go | 6 +++++- 11 files changed, 127 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9570864198..d8ae717b83 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -122,6 +122,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### State Machine Breaking +* (x/group,x/gov) [#16235](https://github.com/cosmos/cosmos-sdk/pull/16235) A group and gov proposal is rejected if the proposal metadata title and summary do not match the proposal title and summary. * (x/staking) [#15701](https://github.com/cosmos/cosmos-sdk/pull/15701) The `HistoricalInfoKey` has been updated to use a binary format. * (x/slashing) [#15580](https://github.com/cosmos/cosmos-sdk/pull/15580) The validator slashing window now stores "chunked" bitmap entries for each validator's signing window instead of a single boolean entry per signing window index. * (x/feegrant) [#14294](https://github.com/cosmos/cosmos-sdk/pull/14294) Moved the logic of rejecting duplicate grant from `msg_server` to `keeper` method. @@ -202,19 +203,19 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (x/gov) [#16106](https://github.com/cosmos/cosmos-sdk/pull/16106) Remove gRPC query methods from Keeper * (x/gov) [#16118](https://github.com/cosmos/cosmos-sdk/pull/16118/) Use collections for constituion and params state management. * (x/gov) [#16127](https://github.com/cosmos/cosmos-sdk/pull/16127) Use collections for deposit state management: - - The following methods are removed from the gov keeper: `GetDeposit`, `GetAllDeposits`, `IterateAllDeposits`. - - The following functions are removed from the gov types: `DepositKey`, `DepositsKey`. + * The following methods are removed from the gov keeper: `GetDeposit`, `GetAllDeposits`, `IterateAllDeposits`. + * The following functions are removed from the gov types: `DepositKey`, `DepositsKey`. * (x/gov) [#16164](https://github.com/cosmos/cosmos-sdk/pull/16164) Use collections for vote state management: - - Removed: types `VoteKey`, `VoteKeys` - - Removed: keeper `IterateVotes`, `IterateAllVotes`, `GetVotes`, `GetVote`, `SetVote` + * Removed: types `VoteKey`, `VoteKeys` + * Removed: keeper `IterateVotes`, `IterateAllVotes`, `GetVotes`, `GetVote`, `SetVote` * (x/gov) [#16171](https://github.com/cosmos/cosmos-sdk/pull/16171) Use collections for proposal state management (part 1): - - Removed: keeper: `GetProposal`, `UnmarshalProposal`, `MarshalProposal`, `IterateProposal`, `GetProposal`, `GetProposalFiltered`, `GetProposals`, `GetProposalID`, `SetProposalID` - - Remove: errors unused errors + * Removed: keeper: `GetProposal`, `UnmarshalProposal`, `MarshalProposal`, `IterateProposal`, `GetProposal`, `GetProposalFiltered`, `GetProposals`, `GetProposalID`, `SetProposalID` + * Remove: errors unused errors * (sims) [#16155](https://github.com/cosmos/cosmos-sdk/pull/16155) - * `simulation.NewOperationMsg` now marshals the operation msg as proto bytes instead of legacy amino JSON bytes. - * `simulation.NewOperationMsg` is now 2-arity instead of 3-arity with the obsolete argument `codec.ProtoCodec` removed. - * The field `OperationMsg.Msg` is now of type `[]byte` instead of `json.RawMessage`. + * `simulation.NewOperationMsg` now marshals the operation msg as proto bytes instead of legacy amino JSON bytes. + * `simulation.NewOperationMsg` is now 2-arity instead of 3-arity with the obsolete argument `codec.ProtoCodec` removed. + * The field `OperationMsg.Msg` is now of type `[]byte` instead of `json.RawMessage`. * (cli) [#16209](https://github.com/cosmos/cosmos-sdk/pull/16209) Add API `StartCmdWithOptions` to create customized start command. diff --git a/api/cosmos/gov/v1/gov.pulsar.go b/api/cosmos/gov/v1/gov.pulsar.go index aa7610a2ee..b50d87f0e7 100644 --- a/api/cosmos/gov/v1/gov.pulsar.go +++ b/api/cosmos/gov/v1/gov.pulsar.go @@ -7118,6 +7118,7 @@ type Proposal struct { // voting_end_time is the end time of voting on a proposal. VotingEndTime *timestamppb.Timestamp `protobuf:"bytes,9,opt,name=voting_end_time,json=votingEndTime,proto3" json:"voting_end_time,omitempty"` // metadata is any arbitrary metadata attached to the proposal. + // the recommended format of the metadata is to be found here: https://docs.cosmos.network/v0.47/modules/gov#proposal-3 Metadata string `protobuf:"bytes,10,opt,name=metadata,proto3" json:"metadata,omitempty"` // title is the title of the proposal // @@ -7332,7 +7333,8 @@ type Vote struct { Voter string `protobuf:"bytes,2,opt,name=voter,proto3" json:"voter,omitempty"` // options is the weighted vote options. Options []*WeightedVoteOption `protobuf:"bytes,4,rep,name=options,proto3" json:"options,omitempty"` - // metadata is any arbitrary metadata to attached to the vote. + // metadata is any arbitrary metadata to attached to the vote. + // the recommended format of the metadata is to be found here: https://docs.cosmos.network/v0.47/modules/gov#vote-5 Metadata string `protobuf:"bytes,5,opt,name=metadata,proto3" json:"metadata,omitempty"` } diff --git a/api/cosmos/group/v1/types.pulsar.go b/api/cosmos/group/v1/types.pulsar.go index bc96cac970..3b8ad57793 100644 --- a/api/cosmos/group/v1/types.pulsar.go +++ b/api/cosmos/group/v1/types.pulsar.go @@ -7832,6 +7832,7 @@ type GroupInfo struct { // admin is the account address of the group's admin. Admin string `protobuf:"bytes,2,opt,name=admin,proto3" json:"admin,omitempty"` // metadata is any arbitrary metadata to attached to the group. + // the recommended format of the metadata is to be found here: https://docs.cosmos.network/v0.47/modules/group#group-1 Metadata string `protobuf:"bytes,3,opt,name=metadata,proto3" json:"metadata,omitempty"` // version is used to track changes to a group's membership structure that // would break existing proposals. Whenever any members weight is changed, @@ -7965,6 +7966,7 @@ type GroupPolicyInfo struct { // admin is the account address of the group admin. Admin string `protobuf:"bytes,3,opt,name=admin,proto3" json:"admin,omitempty"` // metadata is any arbitrary metadata attached to the group policy. + // the recommended format of the metadata is to be found here: https://docs.cosmos.network/v0.47/modules/group#decision-policy-1 Metadata string `protobuf:"bytes,4,opt,name=metadata,proto3" json:"metadata,omitempty"` // version is used to track changes to a group's GroupPolicyInfo structure that // would create a different result on a running proposal. @@ -8058,6 +8060,7 @@ type Proposal struct { // group_policy_address is the account address of group policy. GroupPolicyAddress string `protobuf:"bytes,2,opt,name=group_policy_address,json=groupPolicyAddress,proto3" json:"group_policy_address,omitempty"` // metadata is any arbitrary metadata attached to the proposal. + // the recommended format of the metadata is to be found here: https://docs.cosmos.network/v0.47/modules/group#proposal-4 Metadata string `protobuf:"bytes,3,opt,name=metadata,proto3" json:"metadata,omitempty"` // proposers are the account addresses of the proposers. Proposers []string `protobuf:"bytes,4,rep,name=proposers,proto3" json:"proposers,omitempty"` @@ -8280,7 +8283,7 @@ func (x *TallyResult) GetNoWithVetoCount() string { return "" } -// Vote represents a vote for a proposal. +// Vote represents a vote for a proposal.string metadata type Vote struct { state protoimpl.MessageState sizeCache protoimpl.SizeCache @@ -8293,6 +8296,7 @@ type Vote struct { // option is the voter's choice on the proposal. Option VoteOption `protobuf:"varint,3,opt,name=option,proto3,enum=cosmos.group.v1.VoteOption" json:"option,omitempty"` // metadata is any arbitrary metadata attached to the vote. + // the recommended format of the metadata is to be found here: https://docs.cosmos.network/v0.47/modules/group#vote-2 Metadata string `protobuf:"bytes,4,opt,name=metadata,proto3" json:"metadata,omitempty"` // submit_time is the timestamp when the vote was submitted. SubmitTime *timestamppb.Timestamp `protobuf:"bytes,5,opt,name=submit_time,json=submitTime,proto3" json:"submit_time,omitempty"` diff --git a/proto/cosmos/gov/v1/gov.proto b/proto/cosmos/gov/v1/gov.proto index 38a645f40b..6f9c5465c9 100644 --- a/proto/cosmos/gov/v1/gov.proto +++ b/proto/cosmos/gov/v1/gov.proto @@ -80,6 +80,7 @@ message Proposal { google.protobuf.Timestamp voting_end_time = 9 [(gogoproto.stdtime) = true]; // metadata is any arbitrary metadata attached to the proposal. + // the recommended format of the metadata is to be found here: https://docs.cosmos.network/v0.47/modules/gov#proposal-3 string metadata = 10; // title is the title of the proposal @@ -150,7 +151,8 @@ message Vote { // options is the weighted vote options. repeated WeightedVoteOption options = 4; - // metadata is any arbitrary metadata to attached to the vote. + // metadata is any arbitrary metadata to attached to the vote. + // the recommended format of the metadata is to be found here: https://docs.cosmos.network/v0.47/modules/gov#vote-5 string metadata = 5; } diff --git a/proto/cosmos/group/v1/types.proto b/proto/cosmos/group/v1/types.proto index 99838401ea..2126b23345 100644 --- a/proto/cosmos/group/v1/types.proto +++ b/proto/cosmos/group/v1/types.proto @@ -131,6 +131,7 @@ message GroupInfo { string admin = 2 [(cosmos_proto.scalar) = "cosmos.AddressString"]; // metadata is any arbitrary metadata to attached to the group. + // the recommended format of the metadata is to be found here: https://docs.cosmos.network/v0.47/modules/group#group-1 string metadata = 3; // version is used to track changes to a group's membership structure that @@ -171,6 +172,7 @@ message GroupPolicyInfo { string admin = 3 [(cosmos_proto.scalar) = "cosmos.AddressString"]; // metadata is any arbitrary metadata attached to the group policy. + // the recommended format of the metadata is to be found here: https://docs.cosmos.network/v0.47/modules/group#decision-policy-1 string metadata = 4; // version is used to track changes to a group's GroupPolicyInfo structure that @@ -199,6 +201,7 @@ message Proposal { string group_policy_address = 2 [(cosmos_proto.scalar) = "cosmos.AddressString"]; // metadata is any arbitrary metadata attached to the proposal. + // the recommended format of the metadata is to be found here: https://docs.cosmos.network/v0.47/modules/group#proposal-4 string metadata = 3; // proposers are the account addresses of the proposers. @@ -313,7 +316,7 @@ message TallyResult { string no_with_veto_count = 4; } -// Vote represents a vote for a proposal. +// Vote represents a vote for a proposal.string metadata message Vote { // proposal is the unique ID of the proposal. uint64 proposal_id = 1; @@ -325,6 +328,7 @@ message Vote { VoteOption option = 3; // metadata is any arbitrary metadata attached to the vote. + // the recommended format of the metadata is to be found here: https://docs.cosmos.network/v0.47/modules/group#vote-2 string metadata = 4; // submit_time is the timestamp when the vote was submitted. diff --git a/x/gov/keeper/msg_server.go b/x/gov/keeper/msg_server.go index 764efede5d..5ebee5b0e7 100644 --- a/x/gov/keeper/msg_server.go +++ b/x/gov/keeper/msg_server.go @@ -2,6 +2,7 @@ package keeper import ( "context" + "encoding/json" "fmt" "strconv" @@ -47,11 +48,28 @@ func (k msgServer) SubmitProposal(goCtx context.Context, msg *v1.MsgSubmitPropos return nil, err } - // Check that either metadata or Msgs length is non nil. + // check that either metadata or Msgs length is non nil. if len(msg.Messages) == 0 && len(msg.Metadata) == 0 { return nil, errors.Wrap(govtypes.ErrNoProposalMsgs, "either metadata or Msgs length must be non-nil") } + // verify that if present, the metadata title and summary equals the proposal title and summary + if len(msg.Metadata) != 0 { + proposalMetadata := govtypes.ProposalMetadata{} + if err := json.Unmarshal([]byte(msg.Metadata), &proposalMetadata); err == nil { + if proposalMetadata.Title != msg.Title { + return nil, errors.Wrapf(govtypes.ErrInvalidProposalContent, "metadata title '%s' must equal proposal title '%s'", proposalMetadata.Title, msg.Title) + } + + if proposalMetadata.Summary != msg.Summary { + return nil, errors.Wrapf(govtypes.ErrInvalidProposalContent, "metadata summary '%s' must equal proposal summary '%s'", proposalMetadata.Summary, msg.Summary) + } + } + + // if we can't unmarshal the metadata, this means the client didn't use the recommended metadata format + // nothing can be done here, and this is still a valid case, so we ignore the error + } + proposalMsgs, err := msg.GetMsgs() if err != nil { return nil, err diff --git a/x/gov/keeper/msg_server_test.go b/x/gov/keeper/msg_server_test.go index f8e2496673..cb4181c823 100644 --- a/x/gov/keeper/msg_server_test.go +++ b/x/gov/keeper/msg_server_test.go @@ -102,6 +102,36 @@ func (suite *KeeperTestSuite) TestSubmitProposalReq() { expErr: true, expErrMsg: "proposal summary cannot be empty", }, + "title != metadata.title": { + preRun: func() (*v1.MsgSubmitProposal, error) { + return v1.NewMsgSubmitProposal( + []sdk.Msg{bankMsg}, + initialDeposit, + proposer.String(), + "{\"title\":\"Proposal\", \"description\":\"description of proposal\"}", + "Proposal2", + "description of proposal", + false, + ) + }, + expErr: true, + expErrMsg: "metadata title 'Proposal' must equal proposal title 'Proposal2", + }, + "summary != metadata.summary": { + preRun: func() (*v1.MsgSubmitProposal, error) { + return v1.NewMsgSubmitProposal( + []sdk.Msg{bankMsg}, + initialDeposit, + proposer.String(), + "{\"title\":\"Proposal\", \"description\":\"description of proposal\"}", + "Proposal", + "description", + false, + ) + }, + expErr: true, + expErrMsg: "metadata summary '' must equal proposal summary 'description'", + }, "metadata too long": { preRun: func() (*v1.MsgSubmitProposal, error) { return v1.NewMsgSubmitProposal( diff --git a/x/gov/types/v1/gov.pb.go b/x/gov/types/v1/gov.pb.go index 8384b8f1df..9cc1b39bdd 100644 --- a/x/gov/types/v1/gov.pb.go +++ b/x/gov/types/v1/gov.pb.go @@ -264,6 +264,7 @@ type Proposal struct { // voting_end_time is the end time of voting on a proposal. VotingEndTime *time.Time `protobuf:"bytes,9,opt,name=voting_end_time,json=votingEndTime,proto3,stdtime" json:"voting_end_time,omitempty"` // metadata is any arbitrary metadata attached to the proposal. + // the recommended format of the metadata is to be found here: https://docs.cosmos.network/v0.47/modules/gov#proposal-3 Metadata string `protobuf:"bytes,10,opt,name=metadata,proto3" json:"metadata,omitempty"` // title is the title of the proposal // @@ -496,7 +497,8 @@ type Vote struct { Voter string `protobuf:"bytes,2,opt,name=voter,proto3" json:"voter,omitempty"` // options is the weighted vote options. Options []*WeightedVoteOption `protobuf:"bytes,4,rep,name=options,proto3" json:"options,omitempty"` - // metadata is any arbitrary metadata to attached to the vote. + // metadata is any arbitrary metadata to attached to the vote. + // the recommended format of the metadata is to be found here: https://docs.cosmos.network/v0.47/modules/gov#vote-5 Metadata string `protobuf:"bytes,5,opt,name=metadata,proto3" json:"metadata,omitempty"` } diff --git a/x/group/keeper/msg_server.go b/x/group/keeper/msg_server.go index 4deb2f0076..d6980be4f2 100644 --- a/x/group/keeper/msg_server.go +++ b/x/group/keeper/msg_server.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "encoding/binary" + "encoding/json" "fmt" "strings" @@ -16,6 +17,8 @@ import ( "github.com/cosmos/cosmos-sdk/x/group/errors" "github.com/cosmos/cosmos-sdk/x/group/internal/math" "github.com/cosmos/cosmos-sdk/x/group/internal/orm" + + govtypes "github.com/cosmos/cosmos-sdk/x/gov/types" ) var _ group.MsgServer = Keeper{} @@ -527,6 +530,23 @@ func (k Keeper) SubmitProposal(goCtx context.Context, msg *group.MsgSubmitPropos return nil, err } + // verify that if present, the metadata title and summary equals the proposal title and summary + if len(msg.Metadata) != 0 { + proposalMetadata := govtypes.ProposalMetadata{} + if err := json.Unmarshal([]byte(msg.Metadata), &proposalMetadata); err == nil { + if proposalMetadata.Title != msg.Title { + return nil, fmt.Errorf("metadata title '%s' must equal proposal title '%s'", proposalMetadata.Title, msg.Title) + } + + if proposalMetadata.Summary != msg.Summary { + return nil, fmt.Errorf("metadata summary '%s' must equal proposal summary '%s'", proposalMetadata.Summary, msg.Summary) + } + } + + // if we can't unmarshal the metadata, this means the client didn't use the recommended metadata format + // nothing can be done here, and this is still a valid case, so we ignore the error + } + msgs, err := msg.GetMsgs() if err != nil { return nil, errorsmod.Wrap(err, "request msgs") diff --git a/x/group/keeper/msg_server_test.go b/x/group/keeper/msg_server_test.go index 5d44acd5d6..0ec85d13c1 100644 --- a/x/group/keeper/msg_server_test.go +++ b/x/group/keeper/msg_server_test.go @@ -1695,6 +1695,30 @@ func (s *TestSuite) TestSubmitProposal() { expProposal: defaultProposal, postRun: func(sdkCtx sdk.Context) {}, }, + "title != metadata.title": { + req: &group.MsgSubmitProposal{ + GroupPolicyAddress: accountAddr.String(), + Proposers: []string{addr2.String()}, + Metadata: "{\"title\":\"title\",\"summary\":\"description\"}", + Title: "title2", + Summary: "description", + }, + expErr: true, + expErrMsg: "metadata title 'title' must equal proposal title 'title2'", + postRun: func(sdkCtx sdk.Context) {}, + }, + "summary != metadata.summary": { + req: &group.MsgSubmitProposal{ + GroupPolicyAddress: accountAddr.String(), + Proposers: []string{addr2.String()}, + Metadata: "{\"title\":\"title\",\"summary\":\"description of proposal\"}", + Title: "title", + Summary: "description", + }, + expErr: true, + expErrMsg: "metadata summary 'description of proposal' must equal proposal summary 'description'", + postRun: func(sdkCtx sdk.Context) {}, + }, "metadata too long": { req: &group.MsgSubmitProposal{ GroupPolicyAddress: accountAddr.String(), diff --git a/x/group/types.pb.go b/x/group/types.pb.go index 882d8e4289..dfa7b37e38 100644 --- a/x/group/types.pb.go +++ b/x/group/types.pb.go @@ -491,6 +491,7 @@ type GroupInfo struct { // admin is the account address of the group's admin. Admin string `protobuf:"bytes,2,opt,name=admin,proto3" json:"admin,omitempty"` // metadata is any arbitrary metadata to attached to the group. + // the recommended format of the metadata is to be found here: https://docs.cosmos.network/v0.47/modules/group#group-1 Metadata string `protobuf:"bytes,3,opt,name=metadata,proto3" json:"metadata,omitempty"` // version is used to track changes to a group's membership structure that // would break existing proposals. Whenever any members weight is changed, @@ -642,6 +643,7 @@ type GroupPolicyInfo struct { // admin is the account address of the group admin. Admin string `protobuf:"bytes,3,opt,name=admin,proto3" json:"admin,omitempty"` // metadata is any arbitrary metadata attached to the group policy. + // the recommended format of the metadata is to be found here: https://docs.cosmos.network/v0.47/modules/group#decision-policy-1 Metadata string `protobuf:"bytes,4,opt,name=metadata,proto3" json:"metadata,omitempty"` // version is used to track changes to a group's GroupPolicyInfo structure that // would create a different result on a running proposal. @@ -695,6 +697,7 @@ type Proposal struct { // group_policy_address is the account address of group policy. GroupPolicyAddress string `protobuf:"bytes,2,opt,name=group_policy_address,json=groupPolicyAddress,proto3" json:"group_policy_address,omitempty"` // metadata is any arbitrary metadata attached to the proposal. + // the recommended format of the metadata is to be found here: https://docs.cosmos.network/v0.47/modules/group#proposal-4 Metadata string `protobuf:"bytes,3,opt,name=metadata,proto3" json:"metadata,omitempty"` // proposers are the account addresses of the proposers. Proposers []string `protobuf:"bytes,4,rep,name=proposers,proto3" json:"proposers,omitempty"` @@ -813,7 +816,7 @@ func (m *TallyResult) XXX_DiscardUnknown() { var xxx_messageInfo_TallyResult proto.InternalMessageInfo -// Vote represents a vote for a proposal. +// Vote represents a vote for a proposal.string metadata type Vote struct { // proposal is the unique ID of the proposal. ProposalId uint64 `protobuf:"varint,1,opt,name=proposal_id,json=proposalId,proto3" json:"proposal_id,omitempty"` @@ -822,6 +825,7 @@ type Vote struct { // option is the voter's choice on the proposal. Option VoteOption `protobuf:"varint,3,opt,name=option,proto3,enum=cosmos.group.v1.VoteOption" json:"option,omitempty"` // metadata is any arbitrary metadata attached to the vote. + // the recommended format of the metadata is to be found here: https://docs.cosmos.network/v0.47/modules/group#vote-2 Metadata string `protobuf:"bytes,4,opt,name=metadata,proto3" json:"metadata,omitempty"` // submit_time is the timestamp when the vote was submitted. SubmitTime time.Time `protobuf:"bytes,5,opt,name=submit_time,json=submitTime,proto3,stdtime" json:"submit_time"`