Skip to content

Commit

Permalink
refactor: Use config in gov keeper (#11093)
Browse files Browse the repository at this point in the history
## Description

ref: #11034 (comment)



---

### Author Checklist

*All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.*

I have...

- [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] added `!` to the type prefix if API or client breaking change
- [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting))
- [ ] provided a link to the relevant issue or specification
- [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules)
- [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing)
- [ ] added a changelog entry to `CHANGELOG.md`
- [ ] included comments for [documenting Go code](https://blog.golang.org/godoc)
- [ ] updated the relevant documentation or specification
- [ ] reviewed "Files changed" and left comments if necessary
- [ ] confirmed all CI checks have passed

### Reviewers Checklist

*All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.*

I have...

- [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] confirmed `!` in the type prefix if API or client breaking change
- [ ] confirmed all author checklist items have been addressed 
- [ ] reviewed state machine logic
- [ ] reviewed API design and naming
- [ ] reviewed documentation is accurate
- [ ] reviewed tests and test coverage
- [ ] manually tested (if applicable)
  • Loading branch information
amaury1093 authored Feb 3, 2022
1 parent dbc19b3 commit ea51126
Show file tree
Hide file tree
Showing 7 changed files with 52 additions and 24 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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,
}
}

Expand Down
2 changes: 1 addition & 1 deletion x/gov/keeper/proposal.go
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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,
}
}

0 comments on commit ea51126

Please sign in to comment.