From 2f995925687f10d73b674975162a080e2c7a546d Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Mon, 24 Jul 2023 14:19:10 +0200 Subject: [PATCH 1/4] chore: cherry-pick some inj changes --- client/flags/flags.go | 1 + client/keys/add.go | 2 +- simapp/simd/cmd/testnet.go | 2 +- x/gov/keeper/keeper.go | 9 +++++++++ x/gov/keeper/proposal.go | 2 +- x/staking/keeper/msg_server.go | 9 +++++++++ 6 files changed, 22 insertions(+), 3 deletions(-) diff --git a/client/flags/flags.go b/client/flags/flags.go index a2d918edde9a..c397b89be82e 100644 --- a/client/flags/flags.go +++ b/client/flags/flags.go @@ -76,6 +76,7 @@ const ( FlagOffset = "offset" FlagCountTotal = "count-total" FlagTimeoutHeight = "timeout-height" + FlagKeyAlgorithm = "algo" FlagKeyType = "key-type" FlagFeePayer = "fee-payer" FlagFeeGranter = "fee-granter" diff --git a/client/keys/add.go b/client/keys/add.go index 58bc313c7019..94c2b92df038 100644 --- a/client/keys/add.go +++ b/client/keys/add.go @@ -82,7 +82,7 @@ Example: // support old flags name for backwards compatibility f.SetNormalizeFunc(func(f *pflag.FlagSet, name string) pflag.NormalizedName { - if name == "algo" { + if name == flags.FlagKeyAlgorithm { name = flags.FlagKeyType } diff --git a/simapp/simd/cmd/testnet.go b/simapp/simd/cmd/testnet.go index cd539255b189..fdb0474a062a 100644 --- a/simapp/simd/cmd/testnet.go +++ b/simapp/simd/cmd/testnet.go @@ -83,7 +83,7 @@ func addTestnetFlagsToCmd(cmd *cobra.Command) { // support old flags name for backwards compatibility cmd.Flags().SetNormalizeFunc(func(f *pflag.FlagSet, name string) pflag.NormalizedName { - if name == "algo" { + if name == flags.FlagKeyAlgorithm { name = flags.FlagKeyType } diff --git a/x/gov/keeper/keeper.go b/x/gov/keeper/keeper.go index 3396997124ce..af5dc81d2341 100644 --- a/x/gov/keeper/keeper.go +++ b/x/gov/keeper/keeper.go @@ -183,3 +183,12 @@ func (k Keeper) assertMetadataLength(metadata string) error { } return nil } + +// assertSummaryLength returns an error if given summary length +// is greater than a pre-defined 40*MaxMetadataLen. +func (keeper Keeper) assertSummaryLength(summary string) error { + if summary != "" && uint64(len(summary)) > 40*keeper.config.MaxMetadataLen { + return types.ErrMetadataTooLong.Wrapf("got summary with length %d", len(summary)) + } + return nil +} diff --git a/x/gov/keeper/proposal.go b/x/gov/keeper/proposal.go index 094b792a1943..35080aaea019 100644 --- a/x/gov/keeper/proposal.go +++ b/x/gov/keeper/proposal.go @@ -24,7 +24,7 @@ func (keeper Keeper) SubmitProposal(ctx context.Context, messages []sdk.Msg, met } // assert summary is no longer than predefined max length of metadata - err = keeper.assertMetadataLength(summary) + err = keeper.assertSummaryLength(summary) if err != nil { return v1.Proposal{}, err } diff --git a/x/staking/keeper/msg_server.go b/x/staking/keeper/msg_server.go index 283216993679..d7fb9744a72b 100644 --- a/x/staking/keeper/msg_server.go +++ b/x/staking/keeper/msg_server.go @@ -177,6 +177,15 @@ func (k msgServer) EditValidator(ctx context.Context, msg *types.MsgEditValidato if msg.CommissionRate.GT(math.LegacyOneDec()) || msg.CommissionRate.IsNegative() { return nil, errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "commission rate must be between 0 and 1 (inclusive)") } + + minCommissionRate, err := k.MinCommissionRate(ctx) + if err != nil { + return nil, errorsmod.Wrap(sdkerrors.ErrLogic, err.Error()) + } + + if msg.CommissionRate.LT(minCommissionRate) { + return nil, errorsmod.Wrapf(sdkerrors.ErrInvalidRequest, "commission rate cannot be less than the min commission rate %s", minCommissionRate.String()) + } } // validator must already be registered From 4f28bbbca67492f860e3c7345b61529b922f2b4e Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Mon, 24 Jul 2023 14:24:35 +0200 Subject: [PATCH 2/4] sync with group --- x/gov/keeper/keeper.go | 2 +- x/gov/types/errors.go | 1 + x/group/keeper/msg_server.go | 11 ++++++++++- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/x/gov/keeper/keeper.go b/x/gov/keeper/keeper.go index af5dc81d2341..9b2cab294276 100644 --- a/x/gov/keeper/keeper.go +++ b/x/gov/keeper/keeper.go @@ -188,7 +188,7 @@ func (k Keeper) assertMetadataLength(metadata string) error { // is greater than a pre-defined 40*MaxMetadataLen. func (keeper Keeper) assertSummaryLength(summary string) error { if summary != "" && uint64(len(summary)) > 40*keeper.config.MaxMetadataLen { - return types.ErrMetadataTooLong.Wrapf("got summary with length %d", len(summary)) + return types.ErrSummaryTooLong.Wrapf("got summary with length %d", len(summary)) } return nil } diff --git a/x/gov/types/errors.go b/x/gov/types/errors.go index 43bd7481b2ae..a5d8675b82f9 100644 --- a/x/gov/types/errors.go +++ b/x/gov/types/errors.go @@ -23,4 +23,5 @@ var ( ErrInvalidProposer = errors.Register(ModuleName, 18, "invalid proposer") ErrVotingPeriodEnded = errors.Register(ModuleName, 20, "voting period already ended") ErrInvalidProposal = errors.Register(ModuleName, 21, "invalid proposal") + ErrSummaryTooLong = errors.Register(ModuleName, 22, "summary too long") ) diff --git a/x/group/keeper/msg_server.go b/x/group/keeper/msg_server.go index e27f78be0295..2fd3972a0a74 100644 --- a/x/group/keeper/msg_server.go +++ b/x/group/keeper/msg_server.go @@ -521,7 +521,7 @@ func (k Keeper) SubmitProposal(goCtx context.Context, msg *group.MsgSubmitPropos return nil, err } - if err := k.assertMetadataLength(msg.Summary, "proposal summary"); err != nil { + if err := k.assertSummaryLength(msg.Summary); err != nil { return nil, err } @@ -1062,6 +1062,15 @@ func (k Keeper) assertMetadataLength(metadata, description string) error { return nil } +// assertSummaryLength returns an error if given summary length +// is greater than a pre-defined 40*MaxMetadataLen. +func (keeper Keeper) assertSummaryLength(summary string) error { + if summary != "" && uint64(len(summary)) > 40*keeper.config.MaxMetadataLen { + return errorsmod.Wrapf(errors.ErrMaxLimit, "proposal summary is too long") + } + return nil +} + // validateDecisionPolicies loops through all decision policies from the group, // and calls each of their Validate() method. func (k Keeper) validateDecisionPolicies(ctx sdk.Context, g group.GroupInfo) error { From 8d73d92e4e675c9d382e9638dec8acb7a1248333 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Mon, 24 Jul 2023 14:30:43 +0200 Subject: [PATCH 3/4] changelog + another cherry-pick --- CHANGELOG.md | 1 + math/CHANGELOG.md | 3 ++- math/int.go | 5 +++++ 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ce8834c9be9..33a06587d22f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Improvements +* (x/group, x/gov) [#17109](https://github.com/cosmos/cosmos-sdk/pull/17109) Let proposal summary be 40x longer than metadata limit. * (all) [#16537](https://github.com/cosmos/cosmos-sdk/pull/16537) Properly propagated `fmt.Errorf` errors and using `errors.New` where appropriate. ### Bug Fixes diff --git a/math/CHANGELOG.md b/math/CHANGELOG.md index 987642bf71a0..6f4d90e3e271 100644 --- a/math/CHANGELOG.md +++ b/math/CHANGELOG.md @@ -38,7 +38,8 @@ Ref: https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.j ## Improvements -* [#16263](https://github.com/cosmos/cosmos-sdk/pull/16263) Improved math/Int.Size by computing the decimal digits count instead of firstly invoking .Marshal() then checking the length +* [#17109](https://github.com/cosmos/cosmos-sdk/pull/17109) Add `.ToLegacyDec()` method on `math.Int` type for converting to `math.LegacyDec`. +* [#16263](https://github.com/cosmos/cosmos-sdk/pull/16263) Improved `math/Int.Size` by computing the decimal digits count instead of firstly invoking .Marshal() then checking the length ### Bug Fixes diff --git a/math/int.go b/math/int.go index 13495ba75a5b..b94a38f8352f 100644 --- a/math/int.go +++ b/math/int.go @@ -154,6 +154,11 @@ func ZeroInt() Int { return Int{big.NewInt(0)} } // OneInt returns Int value with one func OneInt() Int { return Int{big.NewInt(1)} } +// ToLegacyDec converts Int to LegacyDec +func (i Int) ToLegacyDec() LegacyDec { + return LegacyNewDecFromInt(i) +} + // Int64 converts Int to int64 // Panics if the value is out of range func (i Int) Int64() int64 { From b12938f6c0582bf66d478e80ab39b4ee9cffc3bd Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Mon, 24 Jul 2023 14:51:38 +0200 Subject: [PATCH 4/4] docs, lint and test --- x/gov/README.md | 4 ++++ x/gov/keeper/msg_server_test.go | 15 +++++++++++++++ x/group/keeper/msg_server.go | 4 ++-- x/group/keeper/msg_server_test.go | 11 +++++++++++ 4 files changed, 32 insertions(+), 2 deletions(-) diff --git a/x/gov/README.md b/x/gov/README.md index 683e8bbbac71..a5083274973a 100644 --- a/x/gov/README.md +++ b/x/gov/README.md @@ -1183,6 +1183,10 @@ where `proposal.json` contains: By default the metadata, summary and title are both limited by 255 characters, this can be overridden by the application developer. ::: +:::tip +When metadata is not specified, the title is limited to 255 characters and the summary 40x the title length. +::: + ##### submit-legacy-proposal The `submit-legacy-proposal` command allows users to submit a governance legacy proposal along with an initial deposit. diff --git a/x/gov/keeper/msg_server_test.go b/x/gov/keeper/msg_server_test.go index 3d8620a6bf42..74cdf00b4745 100644 --- a/x/gov/keeper/msg_server_test.go +++ b/x/gov/keeper/msg_server_test.go @@ -147,6 +147,21 @@ func (suite *KeeperTestSuite) TestSubmitProposalReq() { expErr: true, expErrMsg: "metadata too long", }, + "summary too long": { + preRun: func() (*v1.MsgSubmitProposal, error) { + return v1.NewMsgSubmitProposal( + []sdk.Msg{bankMsg}, + initialDeposit, + proposer.String(), + "", + "Proposal", + strings.Repeat("1", 300*40), + false, + ) + }, + expErr: true, + expErrMsg: "summary too long", + }, "many signers": { preRun: func() (*v1.MsgSubmitProposal, error) { return v1.NewMsgSubmitProposal( diff --git a/x/group/keeper/msg_server.go b/x/group/keeper/msg_server.go index 2fd3972a0a74..048b82a3566b 100644 --- a/x/group/keeper/msg_server.go +++ b/x/group/keeper/msg_server.go @@ -1064,8 +1064,8 @@ func (k Keeper) assertMetadataLength(metadata, description string) error { // assertSummaryLength returns an error if given summary length // is greater than a pre-defined 40*MaxMetadataLen. -func (keeper Keeper) assertSummaryLength(summary string) error { - if summary != "" && uint64(len(summary)) > 40*keeper.config.MaxMetadataLen { +func (k Keeper) assertSummaryLength(summary string) error { + if summary != "" && uint64(len(summary)) > 40*k.config.MaxMetadataLen { return errorsmod.Wrapf(errors.ErrMaxLimit, "proposal summary is too long") } return nil diff --git a/x/group/keeper/msg_server_test.go b/x/group/keeper/msg_server_test.go index 6d6d24c3b099..7cbe0a9f39c2 100644 --- a/x/group/keeper/msg_server_test.go +++ b/x/group/keeper/msg_server_test.go @@ -1742,6 +1742,17 @@ func (s *TestSuite) TestSubmitProposal() { expErrMsg: "limit exceeded", postRun: func(sdkCtx sdk.Context) {}, }, + "summary too long": { + req: &group.MsgSubmitProposal{ + GroupPolicyAddress: accountAddr.String(), + Proposers: []string{addr2.String()}, + Metadata: "{\"title\":\"title\",\"summary\":\"description\"}", + Summary: strings.Repeat("a", 256*40), + }, + expErr: true, + expErrMsg: "limit exceeded", + postRun: func(sdkCtx sdk.Context) {}, + }, "group policy required": { req: &group.MsgSubmitProposal{ Proposers: []string{addr2.String()},