From 7ce339c9068b47ec56b7e5061e6d2086141f850d Mon Sep 17 00:00:00 2001 From: marcello33 Date: Thu, 18 Apr 2024 16:55:09 +0200 Subject: [PATCH 1/7] x/gov: add: MaxVoteOptionsLen --- api/cosmos/gov/module/v1/module.pulsar.go | 105 ++++++++++++++---- x/gov/keeper/config.go | 3 + x/gov/keeper/keeper.go | 13 +++ x/gov/keeper/vote.go | 5 + x/gov/proto/cosmos/gov/module/v1/module.proto | 4 + x/gov/types/errors.go | 1 + 6 files changed, 109 insertions(+), 22 deletions(-) diff --git a/api/cosmos/gov/module/v1/module.pulsar.go b/api/cosmos/gov/module/v1/module.pulsar.go index 2df55ac73881..5355b463d7d8 100644 --- a/api/cosmos/gov/module/v1/module.pulsar.go +++ b/api/cosmos/gov/module/v1/module.pulsar.go @@ -14,11 +14,12 @@ import ( ) var ( - md_Module protoreflect.MessageDescriptor - fd_Module_max_metadata_len protoreflect.FieldDescriptor - fd_Module_authority protoreflect.FieldDescriptor - fd_Module_max_title_len protoreflect.FieldDescriptor - fd_Module_max_summary_len protoreflect.FieldDescriptor + md_Module protoreflect.MessageDescriptor + fd_Module_max_metadata_len protoreflect.FieldDescriptor + fd_Module_authority protoreflect.FieldDescriptor + fd_Module_max_title_len protoreflect.FieldDescriptor + fd_Module_max_summary_len protoreflect.FieldDescriptor + fd_Module_max_vote_options_len protoreflect.FieldDescriptor ) func init() { @@ -28,6 +29,7 @@ func init() { fd_Module_authority = md_Module.Fields().ByName("authority") fd_Module_max_title_len = md_Module.Fields().ByName("max_title_len") fd_Module_max_summary_len = md_Module.Fields().ByName("max_summary_len") + fd_Module_max_vote_options_len = md_Module.Fields().ByName("max_vote_options_len") } var _ protoreflect.Message = (*fastReflection_Module)(nil) @@ -119,6 +121,12 @@ func (x *fastReflection_Module) Range(f func(protoreflect.FieldDescriptor, proto return } } + if x.MaxVoteOptionsLen != uint64(0) { + value := protoreflect.ValueOfUint64(x.MaxVoteOptionsLen) + if !f(fd_Module_max_vote_options_len, value) { + return + } + } } // Has reports whether a field is populated. @@ -142,6 +150,8 @@ func (x *fastReflection_Module) Has(fd protoreflect.FieldDescriptor) bool { return x.MaxTitleLen != uint64(0) case "cosmos.gov.module.v1.Module.max_summary_len": return x.MaxSummaryLen != uint64(0) + case "cosmos.gov.module.v1.Module.max_vote_options_len": + return x.MaxVoteOptionsLen != uint64(0) default: if fd.IsExtension() { panic(fmt.Errorf("proto3 declared messages do not support extensions: cosmos.gov.module.v1.Module")) @@ -166,6 +176,8 @@ func (x *fastReflection_Module) Clear(fd protoreflect.FieldDescriptor) { x.MaxTitleLen = uint64(0) case "cosmos.gov.module.v1.Module.max_summary_len": x.MaxSummaryLen = uint64(0) + case "cosmos.gov.module.v1.Module.max_vote_options_len": + x.MaxVoteOptionsLen = uint64(0) default: if fd.IsExtension() { panic(fmt.Errorf("proto3 declared messages do not support extensions: cosmos.gov.module.v1.Module")) @@ -194,6 +206,9 @@ func (x *fastReflection_Module) Get(descriptor protoreflect.FieldDescriptor) pro case "cosmos.gov.module.v1.Module.max_summary_len": value := x.MaxSummaryLen return protoreflect.ValueOfUint64(value) + case "cosmos.gov.module.v1.Module.max_vote_options_len": + value := x.MaxVoteOptionsLen + return protoreflect.ValueOfUint64(value) default: if descriptor.IsExtension() { panic(fmt.Errorf("proto3 declared messages do not support extensions: cosmos.gov.module.v1.Module")) @@ -222,6 +237,8 @@ func (x *fastReflection_Module) Set(fd protoreflect.FieldDescriptor, value proto x.MaxTitleLen = value.Uint() case "cosmos.gov.module.v1.Module.max_summary_len": x.MaxSummaryLen = value.Uint() + case "cosmos.gov.module.v1.Module.max_vote_options_len": + x.MaxVoteOptionsLen = value.Uint() default: if fd.IsExtension() { panic(fmt.Errorf("proto3 declared messages do not support extensions: cosmos.gov.module.v1.Module")) @@ -250,6 +267,8 @@ func (x *fastReflection_Module) Mutable(fd protoreflect.FieldDescriptor) protore panic(fmt.Errorf("field max_title_len of message cosmos.gov.module.v1.Module is not mutable")) case "cosmos.gov.module.v1.Module.max_summary_len": panic(fmt.Errorf("field max_summary_len of message cosmos.gov.module.v1.Module is not mutable")) + case "cosmos.gov.module.v1.Module.max_vote_options_len": + panic(fmt.Errorf("field max_vote_options_len of message cosmos.gov.module.v1.Module is not mutable")) default: if fd.IsExtension() { panic(fmt.Errorf("proto3 declared messages do not support extensions: cosmos.gov.module.v1.Module")) @@ -271,6 +290,8 @@ func (x *fastReflection_Module) NewField(fd protoreflect.FieldDescriptor) protor return protoreflect.ValueOfUint64(uint64(0)) case "cosmos.gov.module.v1.Module.max_summary_len": return protoreflect.ValueOfUint64(uint64(0)) + case "cosmos.gov.module.v1.Module.max_vote_options_len": + return protoreflect.ValueOfUint64(uint64(0)) default: if fd.IsExtension() { panic(fmt.Errorf("proto3 declared messages do not support extensions: cosmos.gov.module.v1.Module")) @@ -353,6 +374,9 @@ func (x *fastReflection_Module) ProtoMethods() *protoiface.Methods { if x.MaxSummaryLen != 0 { n += 1 + runtime.Sov(uint64(x.MaxSummaryLen)) } + if x.MaxVoteOptionsLen != 0 { + n += 1 + runtime.Sov(uint64(x.MaxVoteOptionsLen)) + } if x.unknownFields != nil { n += len(x.unknownFields) } @@ -382,6 +406,11 @@ func (x *fastReflection_Module) ProtoMethods() *protoiface.Methods { i -= len(x.unknownFields) copy(dAtA[i:], x.unknownFields) } + if x.MaxVoteOptionsLen != 0 { + i = runtime.EncodeVarint(dAtA, i, uint64(x.MaxVoteOptionsLen)) + i-- + dAtA[i] = 0x28 + } if x.MaxSummaryLen != 0 { i = runtime.EncodeVarint(dAtA, i, uint64(x.MaxSummaryLen)) i-- @@ -542,6 +571,25 @@ func (x *fastReflection_Module) ProtoMethods() *protoiface.Methods { break } } + case 5: + if wireType != 0 { + return protoiface.UnmarshalOutput{NoUnkeyedLiterals: input.NoUnkeyedLiterals, Flags: input.Flags}, fmt.Errorf("proto: wrong wireType = %d for field MaxVoteOptionsLen", wireType) + } + x.MaxVoteOptionsLen = 0 + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return protoiface.UnmarshalOutput{NoUnkeyedLiterals: input.NoUnkeyedLiterals, Flags: input.Flags}, runtime.ErrIntOverflow + } + if iNdEx >= l { + return protoiface.UnmarshalOutput{NoUnkeyedLiterals: input.NoUnkeyedLiterals, Flags: input.Flags}, io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + x.MaxVoteOptionsLen |= uint64(b&0x7F) << shift + if b < 0x80 { + break + } + } default: iNdEx = preIndex skippy, err := runtime.Skip(dAtA[iNdEx:]) @@ -607,6 +655,9 @@ type Module struct { // max_summary_len defines the maximum proposal summary length. // Defaults to 10200 if not explicitly set. MaxSummaryLen uint64 `protobuf:"varint,4,opt,name=max_summary_len,json=maxSummaryLen,proto3" json:"max_summary_len,omitempty"` + // max_vote_options_len defines the maximum number of vote options a proposal can have. + // Defaults to 0 if not explicitly set. + MaxVoteOptionsLen uint64 `protobuf:"varint,5,opt,name=max_vote_options_len,json=maxVoteOptionsLen,proto3" json:"max_vote_options_len,omitempty"` } func (x *Module) Reset() { @@ -657,6 +708,13 @@ func (x *Module) GetMaxSummaryLen() uint64 { return 0 } +func (x *Module) GetMaxVoteOptionsLen() uint64 { + if x != nil { + return x.MaxVoteOptionsLen + } + return 0 +} + var File_cosmos_gov_module_v1_module_proto protoreflect.FileDescriptor var file_cosmos_gov_module_v1_module_proto_rawDesc = []byte{ @@ -665,7 +723,7 @@ var file_cosmos_gov_module_v1_module_proto_rawDesc = []byte{ 0x6f, 0x74, 0x6f, 0x12, 0x14, 0x63, 0x6f, 0x73, 0x6d, 0x6f, 0x73, 0x2e, 0x67, 0x6f, 0x76, 0x2e, 0x6d, 0x6f, 0x64, 0x75, 0x6c, 0x65, 0x2e, 0x76, 0x31, 0x1a, 0x20, 0x63, 0x6f, 0x73, 0x6d, 0x6f, 0x73, 0x2f, 0x61, 0x70, 0x70, 0x2f, 0x76, 0x31, 0x61, 0x6c, 0x70, 0x68, 0x61, 0x31, 0x2f, 0x6d, - 0x6f, 0x64, 0x75, 0x6c, 0x65, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x22, 0xb8, 0x01, 0x0a, 0x06, + 0x6f, 0x64, 0x75, 0x6c, 0x65, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x22, 0xe9, 0x01, 0x0a, 0x06, 0x4d, 0x6f, 0x64, 0x75, 0x6c, 0x65, 0x12, 0x28, 0x0a, 0x10, 0x6d, 0x61, 0x78, 0x5f, 0x6d, 0x65, 0x74, 0x61, 0x64, 0x61, 0x74, 0x61, 0x5f, 0x6c, 0x65, 0x6e, 0x18, 0x01, 0x20, 0x01, 0x28, 0x04, 0x52, 0x0e, 0x6d, 0x61, 0x78, 0x4d, 0x65, 0x74, 0x61, 0x64, 0x61, 0x74, 0x61, 0x4c, 0x65, 0x6e, @@ -675,22 +733,25 @@ var file_cosmos_gov_module_v1_module_proto_rawDesc = []byte{ 0x03, 0x20, 0x01, 0x28, 0x04, 0x52, 0x0b, 0x6d, 0x61, 0x78, 0x54, 0x69, 0x74, 0x6c, 0x65, 0x4c, 0x65, 0x6e, 0x12, 0x26, 0x0a, 0x0f, 0x6d, 0x61, 0x78, 0x5f, 0x73, 0x75, 0x6d, 0x6d, 0x61, 0x72, 0x79, 0x5f, 0x6c, 0x65, 0x6e, 0x18, 0x04, 0x20, 0x01, 0x28, 0x04, 0x52, 0x0d, 0x6d, 0x61, 0x78, - 0x53, 0x75, 0x6d, 0x6d, 0x61, 0x72, 0x79, 0x4c, 0x65, 0x6e, 0x3a, 0x1a, 0xba, 0xc0, 0x96, 0xda, - 0x01, 0x14, 0x0a, 0x12, 0x63, 0x6f, 0x73, 0x6d, 0x6f, 0x73, 0x73, 0x64, 0x6b, 0x2e, 0x69, 0x6f, - 0x2f, 0x78, 0x2f, 0x67, 0x6f, 0x76, 0x42, 0xca, 0x01, 0x0a, 0x18, 0x63, 0x6f, 0x6d, 0x2e, 0x63, - 0x6f, 0x73, 0x6d, 0x6f, 0x73, 0x2e, 0x67, 0x6f, 0x76, 0x2e, 0x6d, 0x6f, 0x64, 0x75, 0x6c, 0x65, - 0x2e, 0x76, 0x31, 0x42, 0x0b, 0x4d, 0x6f, 0x64, 0x75, 0x6c, 0x65, 0x50, 0x72, 0x6f, 0x74, 0x6f, - 0x50, 0x01, 0x5a, 0x2e, 0x63, 0x6f, 0x73, 0x6d, 0x6f, 0x73, 0x73, 0x64, 0x6b, 0x2e, 0x69, 0x6f, - 0x2f, 0x61, 0x70, 0x69, 0x2f, 0x63, 0x6f, 0x73, 0x6d, 0x6f, 0x73, 0x2f, 0x67, 0x6f, 0x76, 0x2f, - 0x6d, 0x6f, 0x64, 0x75, 0x6c, 0x65, 0x2f, 0x76, 0x31, 0x3b, 0x6d, 0x6f, 0x64, 0x75, 0x6c, 0x65, - 0x76, 0x31, 0xa2, 0x02, 0x03, 0x43, 0x47, 0x4d, 0xaa, 0x02, 0x14, 0x43, 0x6f, 0x73, 0x6d, 0x6f, - 0x73, 0x2e, 0x47, 0x6f, 0x76, 0x2e, 0x4d, 0x6f, 0x64, 0x75, 0x6c, 0x65, 0x2e, 0x56, 0x31, 0xca, - 0x02, 0x14, 0x43, 0x6f, 0x73, 0x6d, 0x6f, 0x73, 0x5c, 0x47, 0x6f, 0x76, 0x5c, 0x4d, 0x6f, 0x64, - 0x75, 0x6c, 0x65, 0x5c, 0x56, 0x31, 0xe2, 0x02, 0x20, 0x43, 0x6f, 0x73, 0x6d, 0x6f, 0x73, 0x5c, - 0x47, 0x6f, 0x76, 0x5c, 0x4d, 0x6f, 0x64, 0x75, 0x6c, 0x65, 0x5c, 0x56, 0x31, 0x5c, 0x47, 0x50, - 0x42, 0x4d, 0x65, 0x74, 0x61, 0x64, 0x61, 0x74, 0x61, 0xea, 0x02, 0x17, 0x43, 0x6f, 0x73, 0x6d, - 0x6f, 0x73, 0x3a, 0x3a, 0x47, 0x6f, 0x76, 0x3a, 0x3a, 0x4d, 0x6f, 0x64, 0x75, 0x6c, 0x65, 0x3a, - 0x3a, 0x56, 0x31, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, + 0x53, 0x75, 0x6d, 0x6d, 0x61, 0x72, 0x79, 0x4c, 0x65, 0x6e, 0x12, 0x2f, 0x0a, 0x14, 0x6d, 0x61, + 0x78, 0x5f, 0x76, 0x6f, 0x74, 0x65, 0x5f, 0x6f, 0x70, 0x74, 0x69, 0x6f, 0x6e, 0x73, 0x5f, 0x6c, + 0x65, 0x6e, 0x18, 0x05, 0x20, 0x01, 0x28, 0x04, 0x52, 0x11, 0x6d, 0x61, 0x78, 0x56, 0x6f, 0x74, + 0x65, 0x4f, 0x70, 0x74, 0x69, 0x6f, 0x6e, 0x73, 0x4c, 0x65, 0x6e, 0x3a, 0x1a, 0xba, 0xc0, 0x96, + 0xda, 0x01, 0x14, 0x0a, 0x12, 0x63, 0x6f, 0x73, 0x6d, 0x6f, 0x73, 0x73, 0x64, 0x6b, 0x2e, 0x69, + 0x6f, 0x2f, 0x78, 0x2f, 0x67, 0x6f, 0x76, 0x42, 0xca, 0x01, 0x0a, 0x18, 0x63, 0x6f, 0x6d, 0x2e, + 0x63, 0x6f, 0x73, 0x6d, 0x6f, 0x73, 0x2e, 0x67, 0x6f, 0x76, 0x2e, 0x6d, 0x6f, 0x64, 0x75, 0x6c, + 0x65, 0x2e, 0x76, 0x31, 0x42, 0x0b, 0x4d, 0x6f, 0x64, 0x75, 0x6c, 0x65, 0x50, 0x72, 0x6f, 0x74, + 0x6f, 0x50, 0x01, 0x5a, 0x2e, 0x63, 0x6f, 0x73, 0x6d, 0x6f, 0x73, 0x73, 0x64, 0x6b, 0x2e, 0x69, + 0x6f, 0x2f, 0x61, 0x70, 0x69, 0x2f, 0x63, 0x6f, 0x73, 0x6d, 0x6f, 0x73, 0x2f, 0x67, 0x6f, 0x76, + 0x2f, 0x6d, 0x6f, 0x64, 0x75, 0x6c, 0x65, 0x2f, 0x76, 0x31, 0x3b, 0x6d, 0x6f, 0x64, 0x75, 0x6c, + 0x65, 0x76, 0x31, 0xa2, 0x02, 0x03, 0x43, 0x47, 0x4d, 0xaa, 0x02, 0x14, 0x43, 0x6f, 0x73, 0x6d, + 0x6f, 0x73, 0x2e, 0x47, 0x6f, 0x76, 0x2e, 0x4d, 0x6f, 0x64, 0x75, 0x6c, 0x65, 0x2e, 0x56, 0x31, + 0xca, 0x02, 0x14, 0x43, 0x6f, 0x73, 0x6d, 0x6f, 0x73, 0x5c, 0x47, 0x6f, 0x76, 0x5c, 0x4d, 0x6f, + 0x64, 0x75, 0x6c, 0x65, 0x5c, 0x56, 0x31, 0xe2, 0x02, 0x20, 0x43, 0x6f, 0x73, 0x6d, 0x6f, 0x73, + 0x5c, 0x47, 0x6f, 0x76, 0x5c, 0x4d, 0x6f, 0x64, 0x75, 0x6c, 0x65, 0x5c, 0x56, 0x31, 0x5c, 0x47, + 0x50, 0x42, 0x4d, 0x65, 0x74, 0x61, 0x64, 0x61, 0x74, 0x61, 0xea, 0x02, 0x17, 0x43, 0x6f, 0x73, + 0x6d, 0x6f, 0x73, 0x3a, 0x3a, 0x47, 0x6f, 0x76, 0x3a, 0x3a, 0x4d, 0x6f, 0x64, 0x75, 0x6c, 0x65, + 0x3a, 0x3a, 0x56, 0x31, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, } var ( diff --git a/x/gov/keeper/config.go b/x/gov/keeper/config.go index 5e8f3a7d0c6a..5b66c35669c4 100644 --- a/x/gov/keeper/config.go +++ b/x/gov/keeper/config.go @@ -26,6 +26,8 @@ type Config struct { MaxMetadataLen uint64 // MaxSummaryLen defines the amount of characters that can be used for proposal summary MaxSummaryLen uint64 + // MaxVoteOptionsLen defines the maximum number of vote options a proposal can have + MaxVoteOptionsLen uint64 // CalculateVoteResultsAndVotingPowerFn is a function signature for calculating vote results and voting power // Keeping it nil will use the default implementation CalculateVoteResultsAndVotingPowerFn CalculateVoteResultsAndVotingPowerFn @@ -37,6 +39,7 @@ func DefaultConfig() Config { MaxTitleLen: 255, MaxMetadataLen: 255, MaxSummaryLen: 10200, + MaxVoteOptionsLen: 0, // means all supported options CalculateVoteResultsAndVotingPowerFn: nil, } } diff --git a/x/gov/keeper/keeper.go b/x/gov/keeper/keeper.go index 57464eb01181..51e11fd29fc3 100644 --- a/x/gov/keeper/keeper.go +++ b/x/gov/keeper/keeper.go @@ -106,6 +106,10 @@ func NewKeeper( if config.MaxSummaryLen == 0 { config.MaxSummaryLen = defaultConfig.MaxSummaryLen } + // If MaxVoteOptionsLen not set by app developer, set to default value. + if config.MaxVoteOptionsLen == 0 { + config.MaxVoteOptionsLen = defaultConfig.MaxVoteOptionsLen + } sb := collections.NewSchemaBuilder(env.KVStoreService) k := &Keeper{ @@ -230,3 +234,12 @@ func (k Keeper) assertSummaryLength(summary string) error { } return nil } + +// assertVoteOptionsLen returns an error if given vote options length +// is greater than a pre-defined MaxVoteOptionsLen. +func (k Keeper) assertVoteOptionsLen(options v1.WeightedVoteOptions) error { + if uint64(len(options)) > k.config.MaxVoteOptionsLen { + return types.ErrTooManyVoteOptions.Wrapf("got %d options, maximum allowed is %d", len(options), k.config.MaxVoteOptionsLen) + } + return nil +} diff --git a/x/gov/keeper/vote.go b/x/gov/keeper/vote.go index f1f13d7d71c8..25d11e514722 100644 --- a/x/gov/keeper/vote.go +++ b/x/gov/keeper/vote.go @@ -35,6 +35,11 @@ func (k Keeper) AddVote(ctx context.Context, proposalID uint64, voterAddr sdk.Ac return err } + err = k.assertVoteOptionsLen(options) + if err != nil { + return err + } + for _, option := range options { switch proposal.ProposalType { case v1.ProposalType_PROPOSAL_TYPE_OPTIMISTIC: diff --git a/x/gov/proto/cosmos/gov/module/v1/module.proto b/x/gov/proto/cosmos/gov/module/v1/module.proto index a6803c67c4b9..7eb865841348 100644 --- a/x/gov/proto/cosmos/gov/module/v1/module.proto +++ b/x/gov/proto/cosmos/gov/module/v1/module.proto @@ -24,4 +24,8 @@ message Module { // max_summary_len defines the maximum proposal summary length. // Defaults to 10200 if not explicitly set. uint64 max_summary_len = 4; + + // max_vote_options_len defines the maximum number of vote options a proposal can have. + // Defaults to 0 if not explicitly set. + uint64 max_vote_options_len = 5; } diff --git a/x/gov/types/errors.go b/x/gov/types/errors.go index 95e06916092c..d4b9fdba8725 100644 --- a/x/gov/types/errors.go +++ b/x/gov/types/errors.go @@ -27,4 +27,5 @@ var ( ErrInvalidDepositDenom = errors.Register(ModuleName, 23, "invalid deposit denom") ErrTitleTooLong = errors.Register(ModuleName, 24, "title too long") ErrTooLateToCancel = errors.Register(ModuleName, 25, "too late to cancel proposal") + ErrTooManyVoteOptions = errors.Register(ModuleName, 26, "too many vote options") ) From e269668bace456c68fdf4a2dc5098a152680fa36 Mon Sep 17 00:00:00 2001 From: marcello33 Date: Thu, 18 Apr 2024 17:23:15 +0200 Subject: [PATCH 2/7] x/gov: chg: update README and CHANGELOG --- x/gov/CHANGELOG.md | 1 + x/gov/README.md | 2 ++ 2 files changed, 3 insertions(+) diff --git a/x/gov/CHANGELOG.md b/x/gov/CHANGELOG.md index 722c5f5f780b..7135149cd2be 100644 --- a/x/gov/CHANGELOG.md +++ b/x/gov/CHANGELOG.md @@ -35,6 +35,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * [#18620](https://github.com/cosmos/cosmos-sdk/pull/18620) Add optimistic proposals. * [#18762](https://github.com/cosmos/cosmos-sdk/pull/18762) Add multiple choice proposals. * [#19853](https://github.com/cosmos/cosmos-sdk/pull/19853) Emit `depositor` in `EventTypeProposalDeposit` and `proposal_type` in `EventTypeSubmitProposal` +* [#20087](https://github.com/cosmos/cosmos-sdk/pull/20087) add `MaxVoteOptionsLen` ### Improvements diff --git a/x/gov/README.md b/x/gov/README.md index 919afe01ec72..1ff484eefd7f 100644 --- a/x/gov/README.md +++ b/x/gov/README.md @@ -183,6 +183,8 @@ https://github.com/cosmos/cosmos-sdk/blob/v0.47.0-rc1/proto/cosmos/gov/v1beta1/g For a weighted vote to be valid, the `options` field must not contain duplicate vote options, and the sum of weights of all options must be equal to 1. +The maximum number of vote options can be limited by the developer via a config parameter, named `MaxVoteOptionsLen`, which gets passed into the gov keeper. + ### Quorum Quorum is defined as the minimum percentage of voting power that needs to be From 7054d908a8d660d9985bf0ee3f33b6863b095340 Mon Sep 17 00:00:00 2001 From: marcello33 Date: Thu, 18 Apr 2024 17:25:05 +0200 Subject: [PATCH 3/7] x/gov: chg: update CHANGELOG improvements ordering --- x/gov/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/gov/CHANGELOG.md b/x/gov/CHANGELOG.md index 7135149cd2be..28b3668741b3 100644 --- a/x/gov/CHANGELOG.md +++ b/x/gov/CHANGELOG.md @@ -27,6 +27,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Features +* [#20087](https://github.com/cosmos/cosmos-sdk/pull/20087) add `MaxVoteOptionsLen` * [#19592](https://github.com/cosmos/cosmos-sdk/pull/19592) Add custom tally function. * [#19304](https://github.com/cosmos/cosmos-sdk/pull/19304) Add `MsgSudoExec` for allowing executing any message as a sudo. * [#19101](https://github.com/cosmos/cosmos-sdk/pull/19101) Add message based params configuration. @@ -35,7 +36,6 @@ Ref: https://keepachangelog.com/en/1.0.0/ * [#18620](https://github.com/cosmos/cosmos-sdk/pull/18620) Add optimistic proposals. * [#18762](https://github.com/cosmos/cosmos-sdk/pull/18762) Add multiple choice proposals. * [#19853](https://github.com/cosmos/cosmos-sdk/pull/19853) Emit `depositor` in `EventTypeProposalDeposit` and `proposal_type` in `EventTypeSubmitProposal` -* [#20087](https://github.com/cosmos/cosmos-sdk/pull/20087) add `MaxVoteOptionsLen` ### Improvements From f4fda02e9dcc6d0f39d985c99ca5e69121e98888 Mon Sep 17 00:00:00 2001 From: marcello33 Date: Fri, 19 Apr 2024 11:04:01 +0200 Subject: [PATCH 4/7] x/gov: add: unit tests, improve assertVoteOptionsLen and comments --- x/gov/README.md | 2 +- x/gov/keeper/common_test.go | 69 +++++++++++++++++++++++++++++++++++++ x/gov/keeper/config.go | 6 ++-- x/gov/keeper/keeper.go | 8 +++-- x/gov/keeper/vote_test.go | 61 ++++++++++++++++++++++++++++++++ x/gov/types/errors.go | 2 +- 6 files changed, 141 insertions(+), 7 deletions(-) diff --git a/x/gov/README.md b/x/gov/README.md index 1ff484eefd7f..fdfdc4632623 100644 --- a/x/gov/README.md +++ b/x/gov/README.md @@ -183,7 +183,7 @@ https://github.com/cosmos/cosmos-sdk/blob/v0.47.0-rc1/proto/cosmos/gov/v1beta1/g For a weighted vote to be valid, the `options` field must not contain duplicate vote options, and the sum of weights of all options must be equal to 1. -The maximum number of vote options can be limited by the developer via a config parameter, named `MaxVoteOptionsLen`, which gets passed into the gov keeper. +The maximum number of weighted vote options can be limited by the developer via a config parameter, named `MaxVoteOptionsLen`, which gets passed into the gov keeper. ### Quorum diff --git a/x/gov/keeper/common_test.go b/x/gov/keeper/common_test.go index 3e1be306d15f..a6f687a066e5 100644 --- a/x/gov/keeper/common_test.go +++ b/x/gov/keeper/common_test.go @@ -166,6 +166,75 @@ func setupGovKeeper(t *testing.T, expectations ...func(sdk.Context, mocks)) ( return govKeeper, m, encCfg, ctx } +// setupGovKeeperWithMaxVoteOptionsLen creates a govKeeper with a defeine maxVoteOptionsLen, as well as all its dependencies. +func setupGovKeeperWithMaxVoteOptionsLen(t *testing.T, maxVoteOptionsLen uint64, expectations ...func(sdk.Context, mocks)) ( + *keeper.Keeper, + mocks, + moduletestutil.TestEncodingConfig, + sdk.Context, +) { + t.Helper() + key := storetypes.NewKVStoreKey(types.StoreKey) + storeService := runtime.NewKVStoreService(key) + testCtx := testutil.DefaultContextWithDB(t, key, storetypes.NewTransientStoreKey("transient_test")) + ctx := testCtx.Ctx.WithHeaderInfo(header.Info{Time: time.Now()}) + encCfg := moduletestutil.MakeTestEncodingConfig(codectestutil.CodecOptions{}) + v1.RegisterInterfaces(encCfg.InterfaceRegistry) + v1beta1.RegisterInterfaces(encCfg.InterfaceRegistry) + banktypes.RegisterInterfaces(encCfg.InterfaceRegistry) + + baseApp := baseapp.NewBaseApp( + "authz", + log.NewNopLogger(), + testCtx.DB, + encCfg.TxConfig.TxDecoder(), + ) + baseApp.SetCMS(testCtx.CMS) + baseApp.SetInterfaceRegistry(encCfg.InterfaceRegistry) + + environment := runtime.NewEnvironment(storeService, log.NewNopLogger(), runtime.EnvWithRouterService(baseApp.GRPCQueryRouter(), baseApp.MsgServiceRouter())) + + // gomock initializations + ctrl := gomock.NewController(t) + m := mocks{ + acctKeeper: govtestutil.NewMockAccountKeeper(ctrl), + bankKeeper: govtestutil.NewMockBankKeeper(ctrl), + stakingKeeper: govtestutil.NewMockStakingKeeper(ctrl), + poolKeeper: govtestutil.NewMockPoolKeeper(ctrl), + } + if len(expectations) == 0 { + err := mockDefaultExpectations(ctx, m) + require.NoError(t, err) + } else { + for _, exp := range expectations { + exp(ctx, m) + } + } + + govAddr, err := m.acctKeeper.AddressCodec().BytesToString(govAcct) + require.NoError(t, err) + + config := keeper.DefaultConfig() + config.MaxVoteOptionsLen = maxVoteOptionsLen + + // Gov keeper initializations + govKeeper := keeper.NewKeeper(encCfg.Codec, environment, m.acctKeeper, m.bankKeeper, m.stakingKeeper, m.poolKeeper, config, govAddr) + require.NoError(t, govKeeper.ProposalID.Set(ctx, 1)) + govRouter := v1beta1.NewRouter() // Also register legacy gov handlers to test them too. + govRouter.AddRoute(types.RouterKey, v1beta1.ProposalHandler) + govKeeper.SetLegacyRouter(govRouter) + err = govKeeper.Params.Set(ctx, v1.DefaultParams()) + require.NoError(t, err) + err = govKeeper.Constitution.Set(ctx, "constitution") + require.NoError(t, err) + + // Register all handlers for the MegServiceRouter. + v1.RegisterMsgServer(baseApp.MsgServiceRouter(), keeper.NewMsgServerImpl(govKeeper)) + banktypes.RegisterMsgServer(baseApp.MsgServiceRouter(), nil) // Nil is fine here as long as we never execute the proposal's Msgs. + + return govKeeper, m, encCfg, ctx +} + // trackMockBalances sets up expected calls on the Mock BankKeeper, and also // locally tracks accounts balances (not modules balances). func trackMockBalances(bankKeeper *govtestutil.MockBankKeeper) error { diff --git a/x/gov/keeper/config.go b/x/gov/keeper/config.go index 5b66c35669c4..ea01891974a5 100644 --- a/x/gov/keeper/config.go +++ b/x/gov/keeper/config.go @@ -26,7 +26,9 @@ type Config struct { MaxMetadataLen uint64 // MaxSummaryLen defines the amount of characters that can be used for proposal summary MaxSummaryLen uint64 - // MaxVoteOptionsLen defines the maximum number of vote options a proposal can have + // MaxVoteOptionsLen defines the maximum number of vote options a proposal can have. + // This only applies to WeightedVoteOption messages and not to the VoteOption messages + // 0 means this param is disabled, hence all supported options are allowed MaxVoteOptionsLen uint64 // CalculateVoteResultsAndVotingPowerFn is a function signature for calculating vote results and voting power // Keeping it nil will use the default implementation @@ -39,7 +41,7 @@ func DefaultConfig() Config { MaxTitleLen: 255, MaxMetadataLen: 255, MaxSummaryLen: 10200, - MaxVoteOptionsLen: 0, // means all supported options + MaxVoteOptionsLen: 0, // 0 means this param is disabled, hence all supported options are allowed CalculateVoteResultsAndVotingPowerFn: nil, } } diff --git a/x/gov/keeper/keeper.go b/x/gov/keeper/keeper.go index 51e11fd29fc3..264f9fc0722a 100644 --- a/x/gov/keeper/keeper.go +++ b/x/gov/keeper/keeper.go @@ -106,7 +106,7 @@ func NewKeeper( if config.MaxSummaryLen == 0 { config.MaxSummaryLen = defaultConfig.MaxSummaryLen } - // If MaxVoteOptionsLen not set by app developer, set to default value. + // If MaxVoteOptionsLen not set by app developer, set to default value, meaning all supported options are allowed if config.MaxVoteOptionsLen == 0 { config.MaxVoteOptionsLen = defaultConfig.MaxVoteOptionsLen } @@ -237,9 +237,11 @@ func (k Keeper) assertSummaryLength(summary string) error { // assertVoteOptionsLen returns an error if given vote options length // is greater than a pre-defined MaxVoteOptionsLen. +// It's only being checked when config.MaxVoteOptionsLen > 0 (param enabled) func (k Keeper) assertVoteOptionsLen(options v1.WeightedVoteOptions) error { - if uint64(len(options)) > k.config.MaxVoteOptionsLen { - return types.ErrTooManyVoteOptions.Wrapf("got %d options, maximum allowed is %d", len(options), k.config.MaxVoteOptionsLen) + maxVoteOptionsLen := k.config.MaxVoteOptionsLen + if maxVoteOptionsLen > 0 && uint64(len(options)) > maxVoteOptionsLen { + return types.ErrTooManyVoteOptions.Wrapf("got %d weighted vote options, maximum allowed is %d", len(options), k.config.MaxVoteOptionsLen) } return nil } diff --git a/x/gov/keeper/vote_test.go b/x/gov/keeper/vote_test.go index b008f804f7e3..2187c2468056 100644 --- a/x/gov/keeper/vote_test.go +++ b/x/gov/keeper/vote_test.go @@ -166,3 +166,64 @@ func TestVotes_MultipleChoiceProposal(t *testing.T) { require.NoError(t, govKeeper.AddVote(ctx, proposalID, addrs[1], v1.NewNonSplitVoteOption(v1.OptionTwo), "")) require.NoError(t, govKeeper.AddVote(ctx, proposalID, addrs[0], v1.NewNonSplitVoteOption(v1.OptionThree), "")) } + +func TestVotes_MaxVoteOptionsLen(t *testing.T) { + maxVoteOptionsLen := 3 + govKeeper, mocks, _, ctx := setupGovKeeperWithMaxVoteOptionsLen(t, uint64(maxVoteOptionsLen)) + authKeeper, bankKeeper, stakingKeeper := mocks.acctKeeper, mocks.bankKeeper, mocks.stakingKeeper + addrs := simtestutil.AddTestAddrsIncremental(bankKeeper, stakingKeeper, ctx, 2, sdkmath.NewInt(10000000)) + authKeeper.EXPECT().AddressCodec().Return(address.NewBech32Codec("cosmos")).AnyTimes() + + addrs1Str, err := authKeeper.AddressCodec().BytesToString(addrs[1]) + require.NoError(t, err) + + tp := TestProposal + proposal, err := govKeeper.SubmitProposal(ctx, tp, "", "title", "description", sdk.AccAddress("cosmos1ghekyjucln7y67ntx7cf27m9dpuxxemn4c8g4r"), v1.ProposalType_PROPOSAL_TYPE_STANDARD) + require.NoError(t, err) + proposalID := proposal.Id + metadata := "metadata" + + require.Error(t, govKeeper.AddVote(ctx, proposalID, addrs[0], v1.NewNonSplitVoteOption(v1.OptionYes), metadata), "proposal not on voting period") + require.Error(t, govKeeper.AddVote(ctx, 10, addrs[0], v1.NewNonSplitVoteOption(v1.OptionYes), ""), "invalid proposal ID") + + proposal.Status = v1.StatusVotingPeriod + err = govKeeper.Proposals.Set(ctx, proposal.Id, proposal) + require.NoError(t, err) + + // not exceeding MaxVoteOptionsLen, no errors should be thrown + require.NoError(t, govKeeper.AddVote(ctx, proposalID, addrs[1], v1.WeightedVoteOptions{ + v1.NewWeightedVoteOption(v1.OptionYes, sdkmath.LegacyNewDecWithPrec(60, 2)), + v1.NewWeightedVoteOption(v1.OptionNo, sdkmath.LegacyNewDecWithPrec(30, 2)), + v1.NewWeightedVoteOption(v1.OptionAbstain, sdkmath.LegacyNewDecWithPrec(10, 2)), + }, "")) + vote, err := govKeeper.Votes.Get(ctx, collections.Join(proposalID, addrs[1])) + require.Nil(t, err) + require.Equal(t, addrs1Str, vote.Voter) + require.Equal(t, proposalID, vote.ProposalId) + require.True(t, len(vote.Options) == 3) + require.Equal(t, v1.OptionYes, vote.Options[0].Option) + require.Equal(t, v1.OptionNo, vote.Options[1].Option) + require.Equal(t, v1.OptionAbstain, vote.Options[2].Option) + require.Equal(t, vote.Options[0].Weight, sdkmath.LegacyNewDecWithPrec(60, 2).String()) + require.Equal(t, vote.Options[1].Weight, sdkmath.LegacyNewDecWithPrec(30, 2).String()) + require.Equal(t, vote.Options[2].Weight, sdkmath.LegacyNewDecWithPrec(10, 2).String()) + + // exceeding MaxVoteOptionsLen, an error should be thrown + err = govKeeper.AddVote(ctx, proposalID, addrs[1], v1.WeightedVoteOptions{ + v1.NewWeightedVoteOption(v1.OptionYes, sdkmath.LegacyNewDecWithPrec(60, 2)), + v1.NewWeightedVoteOption(v1.OptionNo, sdkmath.LegacyNewDecWithPrec(30, 2)), + v1.NewWeightedVoteOption(v1.OptionAbstain, sdkmath.LegacyNewDecWithPrec(5, 2)), + v1.NewWeightedVoteOption(v1.OptionNoWithVeto, sdkmath.LegacyNewDecWithPrec(5, 2))}, + "", + ) + require.Error(t, err) + require.Contains(t, err.Error(), "too many weighted vote options") + + // only one vote should have gone through + var votes v1.Votes + require.NoError(t, govKeeper.Votes.Walk(ctx, nil, func(_ collections.Pair[uint64, sdk.AccAddress], value v1.Vote) (stop bool, err error) { + votes = append(votes, &value) + return false, nil + })) + require.Len(t, votes, 1) +} diff --git a/x/gov/types/errors.go b/x/gov/types/errors.go index d4b9fdba8725..ba6b81b68934 100644 --- a/x/gov/types/errors.go +++ b/x/gov/types/errors.go @@ -27,5 +27,5 @@ var ( ErrInvalidDepositDenom = errors.Register(ModuleName, 23, "invalid deposit denom") ErrTitleTooLong = errors.Register(ModuleName, 24, "title too long") ErrTooLateToCancel = errors.Register(ModuleName, 25, "too late to cancel proposal") - ErrTooManyVoteOptions = errors.Register(ModuleName, 26, "too many vote options") + ErrTooManyVoteOptions = errors.Register(ModuleName, 26, "too many weighted vote options") ) From 5d143353f20b360229a1300b7272d9f2a97846b6 Mon Sep 17 00:00:00 2001 From: marcello33 Date: Fri, 19 Apr 2024 11:05:32 +0200 Subject: [PATCH 5/7] x/gov: chg: better naming and fixed typo --- x/gov/keeper/common_test.go | 2 +- x/gov/keeper/vote_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/x/gov/keeper/common_test.go b/x/gov/keeper/common_test.go index a6f687a066e5..c8eb206c0d7d 100644 --- a/x/gov/keeper/common_test.go +++ b/x/gov/keeper/common_test.go @@ -166,7 +166,7 @@ func setupGovKeeper(t *testing.T, expectations ...func(sdk.Context, mocks)) ( return govKeeper, m, encCfg, ctx } -// setupGovKeeperWithMaxVoteOptionsLen creates a govKeeper with a defeine maxVoteOptionsLen, as well as all its dependencies. +// setupGovKeeperWithMaxVoteOptionsLen creates a govKeeper with a defined maxVoteOptionsLen, as well as all its dependencies. func setupGovKeeperWithMaxVoteOptionsLen(t *testing.T, maxVoteOptionsLen uint64, expectations ...func(sdk.Context, mocks)) ( *keeper.Keeper, mocks, diff --git a/x/gov/keeper/vote_test.go b/x/gov/keeper/vote_test.go index 2187c2468056..382e9387403b 100644 --- a/x/gov/keeper/vote_test.go +++ b/x/gov/keeper/vote_test.go @@ -167,7 +167,7 @@ func TestVotes_MultipleChoiceProposal(t *testing.T) { require.NoError(t, govKeeper.AddVote(ctx, proposalID, addrs[0], v1.NewNonSplitVoteOption(v1.OptionThree), "")) } -func TestVotes_MaxVoteOptionsLen(t *testing.T) { +func TestVotes_CustomMaxVoteOptionsLen(t *testing.T) { maxVoteOptionsLen := 3 govKeeper, mocks, _, ctx := setupGovKeeperWithMaxVoteOptionsLen(t, uint64(maxVoteOptionsLen)) authKeeper, bankKeeper, stakingKeeper := mocks.acctKeeper, mocks.bankKeeper, mocks.stakingKeeper From 0670fbc875f8797745744f5172620fc2463cae3e Mon Sep 17 00:00:00 2001 From: marcello33 Date: Sun, 21 Apr 2024 10:57:02 +0200 Subject: [PATCH 6/7] chg: go mod tidy on x/gov --- x/gov/go.mod | 6 ++++-- x/gov/go.sum | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/x/gov/go.mod b/x/gov/go.mod index ed19896944ce..c86c042a39ac 100644 --- a/x/gov/go.mod +++ b/x/gov/go.mod @@ -1,6 +1,8 @@ module cosmossdk.io/x/gov -go 1.21 +go 1.21.0 + +toolchain go1.22.2 require ( buf.build/gen/go/cosmos/gogo-proto/protocolbuffers/go v1.32.0-20230509103710-5e5b9fdd0180.1 // indirect @@ -20,7 +22,7 @@ require ( github.com/chzyer/readline v1.5.1 github.com/cockroachdb/tokenbucket v0.0.0-20230807174530-cc333fc44b06 // indirect github.com/cometbft/cometbft v0.38.7-0.20240412124004-1f67e396cf45 - github.com/cosmos/cosmos-proto v1.0.0-beta.4 + github.com/cosmos/cosmos-proto v1.0.0-beta.5 github.com/cosmos/cosmos-sdk v0.51.0 github.com/cosmos/gogoproto v1.4.12 github.com/golang/mock v1.6.0 diff --git a/x/gov/go.sum b/x/gov/go.sum index 413c9cabc950..e2a70bbefc78 100644 --- a/x/gov/go.sum +++ b/x/gov/go.sum @@ -142,8 +142,8 @@ github.com/cosmos/btcutil v1.0.5 h1:t+ZFcX77LpKtDBhjucvnOH8C2l2ioGsBNEQ3jef8xFk= github.com/cosmos/btcutil v1.0.5/go.mod h1:IyB7iuqZMJlthe2tkIFL33xPyzbFYP0XVdS8P5lUPis= github.com/cosmos/cosmos-db v1.0.2 h1:hwMjozuY1OlJs/uh6vddqnk9j7VamLv+0DBlbEXbAKs= github.com/cosmos/cosmos-db v1.0.2/go.mod h1:Z8IXcFJ9PqKK6BIsVOB3QXtkKoqUOp1vRvPT39kOXEA= -github.com/cosmos/cosmos-proto v1.0.0-beta.4 h1:aEL7tU/rLOmxZQ9z4i7mzxcLbSCY48OdY7lIWTLG7oU= -github.com/cosmos/cosmos-proto v1.0.0-beta.4/go.mod h1:oeB+FyVzG3XrQJbJng0EnV8Vljfk9XvTIpGILNU/9Co= +github.com/cosmos/cosmos-proto v1.0.0-beta.5 h1:eNcayDLpip+zVLRLYafhzLvQlSmyab+RC5W7ZfmxJLA= +github.com/cosmos/cosmos-proto v1.0.0-beta.5/go.mod h1:hQGLpiIUloJBMdQMMWb/4wRApmI9hjHH05nefC0Ojec= github.com/cosmos/go-bip39 v1.0.0 h1:pcomnQdrdH22njcAatO0yWojsUnCO3y2tNoV1cb6hHY= github.com/cosmos/go-bip39 v1.0.0/go.mod h1:RNJv0H/pOIVgxw6KS7QeX2a0Uo0aKUlfhZ4xuwvCdJw= github.com/cosmos/gogogateway v1.2.0 h1:Ae/OivNhp8DqBi/sh2A8a1D0y638GpL3tkmLQAiKxTE= From ee6470615c0c1999f69d474d2492a414f7886bf9 Mon Sep 17 00:00:00 2001 From: marcello33 Date: Mon, 22 Apr 2024 07:04:23 +0200 Subject: [PATCH 7/7] chg: remove toolchain / fix lint --- x/gov/go.mod | 2 -- x/gov/keeper/vote_test.go | 5 ++--- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/x/gov/go.mod b/x/gov/go.mod index c86c042a39ac..7d43557957b8 100644 --- a/x/gov/go.mod +++ b/x/gov/go.mod @@ -2,8 +2,6 @@ module cosmossdk.io/x/gov go 1.21.0 -toolchain go1.22.2 - require ( buf.build/gen/go/cosmos/gogo-proto/protocolbuffers/go v1.32.0-20230509103710-5e5b9fdd0180.1 // indirect buf.build/gen/go/tendermint/tendermint/protocolbuffers/go v1.32.0-20231117195010-33ed361a9051.1 // indirect diff --git a/x/gov/keeper/vote_test.go b/x/gov/keeper/vote_test.go index 382e9387403b..502cb8569ad4 100644 --- a/x/gov/keeper/vote_test.go +++ b/x/gov/keeper/vote_test.go @@ -213,9 +213,8 @@ func TestVotes_CustomMaxVoteOptionsLen(t *testing.T) { v1.NewWeightedVoteOption(v1.OptionYes, sdkmath.LegacyNewDecWithPrec(60, 2)), v1.NewWeightedVoteOption(v1.OptionNo, sdkmath.LegacyNewDecWithPrec(30, 2)), v1.NewWeightedVoteOption(v1.OptionAbstain, sdkmath.LegacyNewDecWithPrec(5, 2)), - v1.NewWeightedVoteOption(v1.OptionNoWithVeto, sdkmath.LegacyNewDecWithPrec(5, 2))}, - "", - ) + v1.NewWeightedVoteOption(v1.OptionNoWithVeto, sdkmath.LegacyNewDecWithPrec(5, 2)), + }, "") require.Error(t, err) require.Contains(t, err.Error(), "too many weighted vote options")