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

refactor: Use config in gov keeper #11093

Merged
merged 5 commits into from Feb 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Expand Up @@ -119,7 +119,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

Expand Down
8 changes: 6 additions & 2 deletions simapp/app.go
Expand Up @@ -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(
Expand Down
28 changes: 16 additions & 12 deletions x/gov/keeper/keeper.go
Expand Up @@ -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:
Expand All @@ -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
Expand All @@ -70,16 +69,21 @@ 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,
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,
amaury1093 marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down
2 changes: 1 addition & 1 deletion x/gov/keeper/proposal.go
Expand Up @@ -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))
}

Expand Down
3 changes: 3 additions & 0 deletions x/gov/spec/02_state.md
Expand Up @@ -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
Expand Down
19 changes: 11 additions & 8 deletions x/gov/spec/03_messages.md
Expand Up @@ -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:**

Expand All @@ -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
Expand All @@ -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
Expand All @@ -51,9 +55,8 @@ 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.Metadata = txGovSubmitProposal.Metadata
proposal.TotalDeposit = initialDeposit
proposal.SubmitTime = <CurrentTime>
proposal.DepositEndTime = <CurrentTime>.Add(depositParam.MaxDepositPeriod)
Expand Down Expand Up @@ -83,7 +86,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.
Expand Down
14 changes: 14 additions & 0 deletions 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,
}
}