From e0e599263b70735b28f3fb97ec212bd0e98c6ed3 Mon Sep 17 00:00:00 2001 From: Amaury M <1293565+amaurym@users.noreply.github.com> Date: Wed, 2 Feb 2022 11:32:02 +0100 Subject: [PATCH 1/4] refactor: Use config in gov keeper --- simapp/app.go | 8 ++++++-- x/gov/keeper/keeper.go | 23 +++++++++++------------ x/gov/keeper/proposal.go | 2 +- x/gov/types/config.go | 14 ++++++++++++++ 4 files changed, 32 insertions(+), 15 deletions(-) create mode 100644 x/gov/types/config.go diff --git a/simapp/app.go b/simapp/app.go index dd7127c098f..7899b160168 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -302,10 +302,14 @@ func NewSimApp( AddRoute(paramproposal.RouterKey, params.NewParamChangeProposalHandler(app.ParamsKeeper)). AddRoute(distrtypes.RouterKey, distr.NewCommunityPoolSpendProposalHandler(app.DistrKeeper)). AddRoute(upgradetypes.RouterKey, upgrade.NewSoftwareUpgradeProposalHandler(app.UpgradeKeeper)) - govMaxMetadataLen := uint64(10000) + govConfig := govtypes.DefaultConfig() + /* + Example of setting gov params: + govConfig.MaxMetadataLen = 10000 + */ govKeeper := govkeeper.NewKeeper( appCodec, keys[govtypes.StoreKey], app.GetSubspace(govtypes.ModuleName), app.AccountKeeper, app.BankKeeper, - &stakingKeeper, govRouter, app.msgSvcRouter, govMaxMetadataLen, + &stakingKeeper, govRouter, app.msgSvcRouter, govConfig, ) app.GovKeeper = *govKeeper.SetHooks( diff --git a/x/gov/keeper/keeper.go b/x/gov/keeper/keeper.go index 1d0c20cfd44..60fa8ede34e 100644 --- a/x/gov/keeper/keeper.go +++ b/x/gov/keeper/keeper.go @@ -42,8 +42,7 @@ type Keeper struct { // Msg server router router *middleware.MsgServiceRouter - // maxMetadataLen defines the maximum proposal metadata length. - maxMetadataLen uint64 + config types.Config } // NewKeeper returns a governance keeper. It handles: @@ -57,7 +56,7 @@ func NewKeeper( cdc codec.BinaryCodec, key storetypes.StoreKey, paramSpace types.ParamSubspace, authKeeper types.AccountKeeper, bankKeeper types.BankKeeper, sk types.StakingKeeper, legacyRouter v1beta1.Router, router *middleware.MsgServiceRouter, - maxMetadataLen uint64, + config types.Config, ) Keeper { // ensure governance module account is set @@ -71,15 +70,15 @@ func NewKeeper( legacyRouter.Seal() return Keeper{ - storeKey: key, - paramSpace: paramSpace, - authKeeper: authKeeper, - bankKeeper: bankKeeper, - sk: sk, - cdc: cdc, - legacyRouter: legacyRouter, - router: router, - maxMetadataLen: maxMetadataLen, + storeKey: key, + paramSpace: paramSpace, + authKeeper: authKeeper, + bankKeeper: bankKeeper, + sk: sk, + cdc: cdc, + legacyRouter: legacyRouter, + router: router, + config: config, } } diff --git a/x/gov/keeper/proposal.go b/x/gov/keeper/proposal.go index 4c630736c55..df4be21dc9b 100644 --- a/x/gov/keeper/proposal.go +++ b/x/gov/keeper/proposal.go @@ -12,7 +12,7 @@ import ( // SubmitProposal create new proposal given an array of messages func (keeper Keeper) SubmitProposal(ctx sdk.Context, messages []sdk.Msg, metadata []byte) (v1beta2.Proposal, error) { - if metadata != nil && len(metadata) > int(keeper.maxMetadataLen) { + if metadata != nil && uint64(len(metadata)) > keeper.config.MaxMetadataLen { return v1beta2.Proposal{}, types.ErrMetadataTooLong.Wrapf("got metadata with length %d", len(metadata)) } diff --git a/x/gov/types/config.go b/x/gov/types/config.go new file mode 100644 index 00000000000..d19767f7d15 --- /dev/null +++ b/x/gov/types/config.go @@ -0,0 +1,14 @@ +package types + +// Config is a config struct used for intialising the gov module to avoid using globals. +type Config struct { + // MaxMetadataLen defines the maximum proposal metadata length. + MaxMetadataLen uint64 +} + +// DefaultConfig returns the default config for gov. +func DefaultConfig() Config { + return Config{ + MaxMetadataLen: 255, + } +} From 33cc66f8223d0dfda0d5964b76903657456f6897 Mon Sep 17 00:00:00 2001 From: Amaury M <1293565+amaurym@users.noreply.github.com> Date: Wed, 2 Feb 2022 11:47:55 +0100 Subject: [PATCH 2/4] Update spec regarding metadata --- CHANGELOG.md | 2 +- x/gov/spec/02_state.md | 3 +++ x/gov/spec/03_messages.md | 18 ++++++++++-------- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 754a308be2b..6d1144b5b71 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -118,7 +118,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * [\#10748](https://github.com/cosmos/cosmos-sdk/pull/10748) Move legacy `x/gov` api to `v1beta1` directory. * [\#10816](https://github.com/cosmos/cosmos-sdk/pull/10816) Reuse blocked addresses from the bank module. No need to pass them to distribution. * [\#10852](https://github.com/cosmos/cosmos-sdk/pull/10852) Move `x/gov/types` to `x/gov/types/v1beta2`. -* [\#10868](https://github.com/cosmos/cosmos-sdk/pull/10868), [\#10989](https://github.com/cosmos/cosmos-sdk/pull/10989) The Gov keeper accepts now 2 more mandatory arguments, the ServiceMsgRouter and a maximum proposal metadata length. +* [\#10868](https://github.com/cosmos/cosmos-sdk/pull/10868), [\#10989](https://github.com/cosmos/cosmos-sdk/pull/10989), [\#11093](https://github.com/cosmos/cosmos-sdk/pull/11093) The Gov keeper accepts now 2 more mandatory arguments, the ServiceMsgRouter and a gov Config including the max metadata length. ### Client Breaking Changes diff --git a/x/gov/spec/02_state.md b/x/gov/spec/02_state.md index ed9fcdee7b6..311676bb15f 100644 --- a/x/gov/spec/02_state.md +++ b/x/gov/spec/02_state.md @@ -36,6 +36,9 @@ the following `JSON` template: This makes it far easier for clients to support multiple networks. +The metadata has a maximum length that is chosen by the app developer, and +passed into the gov keeper as a config. + ### Writing a module that uses governance There are many aspects of a chain, or of the individual modules that you may want to diff --git a/x/gov/spec/03_messages.md b/x/gov/spec/03_messages.md index 541793fb788..c9edebc97a5 100644 --- a/x/gov/spec/03_messages.md +++ b/x/gov/spec/03_messages.md @@ -9,10 +9,12 @@ order: 3 Proposals can be submitted by any account via a `MsgSubmitProposal` transaction. -+++ https://github.com/cosmos/cosmos-sdk/blob/v0.40.0/proto/cosmos/gov/v1beta1/tx.proto#L24-L39 ++++ https://github.com/cosmos/cosmos-sdk/blob/ab9545527d630fe38761aa61cc5c95eabd68e0e6/proto/cosmos/gov/v1beta2/tx.proto#L34-L44 -The `Content` of a `MsgSubmitProposal` message must have an appropriate router -set in the governance module. +All `sdk.Msgs` passed into the `messages` field of a `MsgSubmitProposal` message +must be registered in the app's `MsgServiceRouter`. Each of these messages must +have one signer, namely the gov module account. And finally, the metadata length +must not be larger than the `maxMetadataLen` config passed into the gov keeper. **State modifications:** @@ -21,7 +23,7 @@ set in the governance module. - Initialise `Proposal`'s attributes - Decrease balance of sender by `InitialDeposit` - If `MinDeposit` is reached: - - Push `proposalID` in `ProposalProcessingQueue` + - Push `proposalID` in `ProposalProcessingQueue` - Transfer `InitialDeposit` from the `Proposer` to the governance `ModuleAccount` A `MsgSubmitProposal` transaction can be handled according to the following @@ -35,6 +37,8 @@ upon receiving txGovSubmitProposal from sender do if !correctlyFormatted(txGovSubmitProposal) // check if proposal is correctly formatted and the messages have routes to other modules. Includes fee payment. + // check if all messages' unique Signer is the gov acct. + // check if the metadata is not too long. throw initialDeposit = txGovSubmitProposal.InitialDeposit @@ -51,9 +55,7 @@ upon receiving txGovSubmitProposal from sender do proposalID = generate new proposalID proposal = NewProposal() - proposal.Title = txGovSubmitProposal.Title - proposal.Description = txGovSubmitProposal.Description - proposal.Type = txGovSubmitProposal.Type + proposal.Messages = txGovSubmitProposal.Messages proposal.TotalDeposit = initialDeposit proposal.SubmitTime = proposal.DepositEndTime = .Add(depositParam.MaxDepositPeriod) @@ -83,7 +85,7 @@ Once a proposal is submitted, if - Add `deposit` of sender in `proposal.Deposits` - Increase `proposal.TotalDeposit` by sender's `deposit` - If `MinDeposit` is reached: - - Push `proposalID` in `ProposalProcessingQueueEnd` + - Push `proposalID` in `ProposalProcessingQueueEnd` - Transfer `Deposit` from the `proposer` to the governance `ModuleAccount` A `MsgDeposit` transaction has to go through a number of checks to be valid. From 829be9e0a613ff477d12e83819286284c1cb39c1 Mon Sep 17 00:00:00 2001 From: Amaury M <1293565+amaurym@users.noreply.github.com> Date: Wed, 2 Feb 2022 11:50:20 +0100 Subject: [PATCH 3/4] Add metadata in docs --- x/gov/spec/03_messages.md | 1 + 1 file changed, 1 insertion(+) diff --git a/x/gov/spec/03_messages.md b/x/gov/spec/03_messages.md index c9edebc97a5..714edd54424 100644 --- a/x/gov/spec/03_messages.md +++ b/x/gov/spec/03_messages.md @@ -56,6 +56,7 @@ upon receiving txGovSubmitProposal from sender do proposal = NewProposal() proposal.Messages = txGovSubmitProposal.Messages + proposal.Metadata = txGovSubmitProposal.Metadata proposal.TotalDeposit = initialDeposit proposal.SubmitTime = proposal.DepositEndTime = .Add(depositParam.MaxDepositPeriod) From e5242c13b7eeb903fc950fc1078b2682b0a19c19 Mon Sep 17 00:00:00 2001 From: Amaury M <1293565+amaurym@users.noreply.github.com> Date: Wed, 2 Feb 2022 15:59:47 +0100 Subject: [PATCH 4/4] Set default value --- x/gov/keeper/keeper.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/x/gov/keeper/keeper.go b/x/gov/keeper/keeper.go index 60fa8ede34e..a05c953f538 100644 --- a/x/gov/keeper/keeper.go +++ b/x/gov/keeper/keeper.go @@ -69,6 +69,11 @@ func NewKeeper( // could create invalid or non-deterministic behavior. legacyRouter.Seal() + // If MaxMetadataLen not set by app developer, set to default value. + if config.MaxMetadataLen == 0 { + config.MaxMetadataLen = types.DefaultConfig().MaxMetadataLen + } + return Keeper{ storeKey: key, paramSpace: paramSpace,