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

feat(gov,group): improve metadata checks and add docs #16235

Merged
merged 3 commits into from
May 22, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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: 3 additions & 1 deletion api/cosmos/gov/v1/gov.pulsar.go

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

6 changes: 5 additions & 1 deletion api/cosmos/group/v1/types.pulsar.go

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

4 changes: 3 additions & 1 deletion proto/cosmos/gov/v1/gov.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
}

Expand Down
6 changes: 5 additions & 1 deletion proto/cosmos/group/v1/types.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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;
Expand All @@ -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.
Expand Down
20 changes: 19 additions & 1 deletion x/gov/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package keeper

import (
"context"
"encoding/json"
"fmt"
"strconv"

Expand Down Expand Up @@ -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
Expand Down
30 changes: 30 additions & 0 deletions x/gov/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,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(
Expand Down
4 changes: 3 additions & 1 deletion x/gov/types/v1/gov.pb.go

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

20 changes: 20 additions & 0 deletions x/group/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"context"
"encoding/binary"
"encoding/json"
"fmt"
"strings"

Expand All @@ -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{}
Expand Down Expand Up @@ -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")
Expand Down
24 changes: 24 additions & 0 deletions x/group/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1697,6 +1697,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(),
Expand Down
6 changes: 5 additions & 1 deletion x/group/types.pb.go

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