Skip to content

Commit

Permalink
go/governance: Add support for proposal metadata
Browse files Browse the repository at this point in the history
  • Loading branch information
kostko committed Apr 21, 2024
1 parent cd3f82b commit 7560aa3
Show file tree
Hide file tree
Showing 8 changed files with 115 additions and 17 deletions.
1 change: 1 addition & 0 deletions .changelog/5650.breaking.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
go/governance: Add support for proposal metadata
12 changes: 6 additions & 6 deletions go/consensus/cometbft/apps/governance/transactions.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,13 @@ func (app *governanceApplication) submitProposal(
"proposal_content", proposalContent,
)

params, err := state.ConsensusParameters(ctx)
if err != nil {
return nil, fmt.Errorf("failed to fetch consensus parameters: %w", err)
}

// Validate proposal content basics.
if err := proposalContent.ValidateBasic(); err != nil {
if err := proposalContent.ValidateBasic(params); err != nil {
ctx.Logger().Debug("governance: malformed proposal content",
"content", proposalContent,
"err", err,
Expand All @@ -39,11 +44,6 @@ func (app *governanceApplication) submitProposal(
return nil, nil
}

params, err := state.ConsensusParameters(ctx)
if err != nil {
return nil, fmt.Errorf("failed to fetch consensus parameters: %w", err)
}

// To not violate the consensus, change parameters proposals should be ignored when disabled.
if proposalContent.ChangeParameters != nil && !params.EnableChangeParametersProposal {
return nil, governance.ErrInvalidArgument
Expand Down
33 changes: 33 additions & 0 deletions go/consensus/cometbft/apps/governance/transactions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ func TestSubmitProposal(t *testing.T) {
VotingPeriod: beacon.EpochTime(50),
}

allowPropMetaParams := *baseConsParams
allowPropMetaParams.AllowProposalMetadata = true

for _, tc := range []struct {
msg string
params *governance.ConsensusParameters
Expand Down Expand Up @@ -157,6 +160,36 @@ func TestSubmitProposal(t *testing.T) {
func() {},
nil,
},
{
"should fail with metadata when metadata is not allowed",
baseConsParams,
pk1,
&governance.ProposalContent{
Metadata: &governance.ProposalMetadata{
Title: "Proposal that should fail",
},
Upgrade: &governance.UpgradeProposal{
Descriptor: baseAtEpoch(200),
},
},
func() {},
governance.ErrInvalidArgument,
},
{
"should work with metadata when metadata is allowed",
&allowPropMetaParams,
pk1,
&governance.ProposalContent{
Metadata: &governance.ProposalMetadata{
Title: "Proposal that should work",
},
Upgrade: &governance.UpgradeProposal{
Descriptor: baseAtEpoch(200),
},
},
func() {},
nil,
},
{
"should work with valid cancel upgrade proposal",
baseConsParams,
Expand Down
2 changes: 1 addition & 1 deletion go/consensus/cometbft/apps/supplementarysanity/checks.go
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ func checkGovernance(ctx *abciAPI.Context, epoch beacon.EpochTime) error {
if err != nil {
return fmt.Errorf("Proposals(): %w", err)
}
err = governance.SanityCheckProposals(proposals, epoch, govDeposits)
err = governance.SanityCheckProposals(proposals, epoch, govDeposits, params)
if err != nil {
return fmt.Errorf("SanityCheck Proposals: %w", err)
}
Expand Down
71 changes: 64 additions & 7 deletions go/governance/api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"encoding/base64"
"fmt"
"io"
"reflect"

beacon "github.com/oasisprotocol/oasis-core/go/beacon/api"
"github.com/oasisprotocol/oasis-core/go/common/cbor"
Expand Down Expand Up @@ -60,21 +59,38 @@ var (

// ProposalContent is a consensus layer governance proposal content.
type ProposalContent struct {
// Metadata contains optional proposal metadata which is ignored during proposal execution.
Metadata *ProposalMetadata `json:"metadata,omitempty"`

Upgrade *UpgradeProposal `json:"upgrade,omitempty"`
CancelUpgrade *CancelUpgradeProposal `json:"cancel_upgrade,omitempty"`
ChangeParameters *ChangeParametersProposal `json:"change_parameters,omitempty"`
}

// ValidateBasic performs basic proposal content validity checks.
func (p *ProposalContent) ValidateBasic() error {
numProposals := 0
values := reflect.ValueOf(*p)
for i := 0; i < values.NumField(); i++ {
if !values.Field(i).IsNil() {
numProposals++
func (p *ProposalContent) ValidateBasic(params *ConsensusParameters) error {
// Validate metadata if present.
if p.Metadata != nil {
if !params.AllowProposalMetadata {
return ErrInvalidArgument // Must be consistent with error during unmarshal.
}
if err := p.Metadata.ValidateBasic(); err != nil {
return err
}
}

// Validate content.
var numProposals int
if p.Upgrade != nil {
numProposals++
}
if p.CancelUpgrade != nil {
numProposals++
}
if p.ChangeParameters != nil {
numProposals++
}

switch {
case numProposals > 1:
return fmt.Errorf("proposal content has multiple fields set")
Expand Down Expand Up @@ -121,6 +137,9 @@ func (p *ProposalContent) Equals(other *ProposalContent) bool {
// PrettyPrint writes a pretty-printed representation of ProposalContent to the
// given writer.
func (p ProposalContent) PrettyPrint(ctx context.Context, prefix string, w io.Writer) {
if p.Metadata != nil {
p.Metadata.PrettyPrint(ctx, prefix, w)
}
if p.Upgrade != nil {
fmt.Fprintf(w, "%sUpgrade:\n", prefix)
p.Upgrade.PrettyPrint(ctx, prefix+" ", w)
Expand All @@ -141,6 +160,41 @@ func (p ProposalContent) PrettyType() (interface{}, error) {
return p, nil
}

// MaxProposalTitleLength is the maximum length of a proposal's title.
const MaxProposalTitleLength = 100

// ProposalMetadata contains metadata about a proposal.
type ProposalMetadata struct {
// Title is the human-readable proposal title.
Title string `json:"title"`
// Description is the human-readable description.
Description string `json:"description,omitempty"`
}

// ValidateBasic performs basic proposal metadata validity checks.
func (p *ProposalMetadata) ValidateBasic() error {
if len(p.Title) > MaxProposalTitleLength {
return fmt.Errorf("%w: proposal title too long", ErrInvalidArgument)
}
return nil
}

// PrettyPrint writes a pretty-printed representation of ProposalMetadata to the given writer.
func (p ProposalMetadata) PrettyPrint(ctx context.Context, prefix string, w io.Writer) {
if len(p.Title) > 0 {
fmt.Fprintf(w, "%sTitle: %s\n", prefix, p.Title)
}
if len(p.Description) > 0 {
fmt.Fprintf(w, "%sDescription:\n", prefix)
fmt.Fprintf(w, "%s%s\n", prefix, p.Description)
}
}

// PrettyType returns a representation of ProposalMetadata that can be used for pretty printing.
func (p ProposalMetadata) PrettyType() (interface{}, error) {
return p, nil
}

// UpgradeProposal is an upgrade proposal.
type UpgradeProposal struct {
upgrade.Descriptor
Expand Down Expand Up @@ -379,6 +433,9 @@ type ConsensusParameters struct {

// AllowVoteWithoutEntity is true iff casting votes without a registered entity is allowed.
AllowVoteWithoutEntity bool `json:"allow_vote_without_entity,omitempty"`

// AllowProposalMetadata is true iff proposals are allowed to contain metadata.
AllowProposalMetadata bool `json:"allow_proposal_metadata,omitempty"`
}

// ConsensusParameterChanges are allowed governance consensus parameter changes.
Expand Down
6 changes: 3 additions & 3 deletions go/governance/api/sanity_check.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func (c *ConsensusParameterChanges) SanityCheck() error {
}

// SanityCheckProposals sanity checks proposals.
func SanityCheckProposals(proposals []*Proposal, epoch beacon.EpochTime, governanceDeposit *quantity.Quantity) error {
func SanityCheckProposals(proposals []*Proposal, epoch beacon.EpochTime, governanceDeposit *quantity.Quantity, params *ConsensusParameters) error {
activeProposalDeposits := quantity.NewFromUint64(0)
for _, p := range proposals {
if p.CreatedAt > epoch {
Expand All @@ -57,7 +57,7 @@ func SanityCheckProposals(proposals []*Proposal, epoch beacon.EpochTime, governa
if !p.Submitter.IsValid() {
return fmt.Errorf("proposal %v: invalid proposal submitter", p.ID)
}
if err := p.Content.ValidateBasic(); err != nil {
if err := p.Content.ValidateBasic(params); err != nil {
return fmt.Errorf("proposal %v: basic validation failure: %w", p.ID, err)
}

Expand Down Expand Up @@ -146,7 +146,7 @@ func (g *Genesis) SanityCheck(now beacon.EpochTime, governanceDeposits *quantity
if err := g.Parameters.SanityCheck(); err != nil {
return fmt.Errorf("governance: parameters sanity check failed: %w", err)
}
if err := SanityCheckProposals(g.Proposals, now, governanceDeposits); err != nil {
if err := SanityCheckProposals(g.Proposals, now, governanceDeposits, &g.Parameters); err != nil {
return fmt.Errorf("governance: proposals sanity check failed: %w", err)
}
for _, p := range g.Proposals {
Expand Down
6 changes: 6 additions & 0 deletions go/oasis-test-runner/scenario/e2e/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,9 @@ func (c *upgrade240Checker) PreUpgradeFn(ctx context.Context, ctrl *oasis.Contro
if govParams.AllowVoteWithoutEntity {
return fmt.Errorf("voting without entity is allowed")
}
if govParams.AllowProposalMetadata {
return fmt.Errorf("proposal metadata is allowed")
}

return nil
}
Expand Down Expand Up @@ -157,6 +160,9 @@ func (c *upgrade240Checker) PostUpgradeFn(ctx context.Context, ctrl *oasis.Contr
if !govParams.AllowVoteWithoutEntity {
return fmt.Errorf("voting without entity is not allowed")
}
if !govParams.AllowProposalMetadata {
return fmt.Errorf("proposal metadata is not allowed")
}

return nil
}
Expand Down
1 change: 1 addition & 0 deletions go/upgrade/migrations/consensus_240.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ func (h *Handler240) ConsensusUpgrade(privateCtx interface{}) error {
return fmt.Errorf("failed to load governance consensus parameters: %w", err)
}
govParams.AllowVoteWithoutEntity = true
govParams.AllowProposalMetadata = true

if err = govState.SetConsensusParameters(abciCtx, govParams); err != nil {
return fmt.Errorf("failed to update governance consensus parameters: %w", err)
Expand Down

0 comments on commit 7560aa3

Please sign in to comment.