Skip to content

Commit

Permalink
feat: Add max metadata length as a group keeper parameter (#11034)
Browse files Browse the repository at this point in the history
## Description

Closes: #10972 

Add MaxMetadataLength as a group keeper parameter.

---

### 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
likhita-809 authored Feb 3, 2022
1 parent ea51126 commit ee6bedc
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 25 deletions.
7 changes: 6 additions & 1 deletion simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,12 @@ func NewSimApp(

app.AuthzKeeper = authzkeeper.NewKeeper(keys[authzkeeper.StoreKey], appCodec, app.msgSvcRouter, app.AccountKeeper)

app.GroupKeeper = groupkeeper.NewKeeper(keys[group.StoreKey], appCodec, app.msgSvcRouter, app.AccountKeeper)
groupConfig := groupkeeper.DefaultConfig()
/*
Example of setting group params:
groupConfig.MaxMetadataLen = 1000
*/
app.GroupKeeper = groupkeeper.NewKeeper(keys[group.StoreKey], appCodec, app.msgSvcRouter, app.AccountKeeper, groupConfig)

// register the proposal types
govRouter := govv1beta1.NewRouter()
Expand Down
14 changes: 14 additions & 0 deletions x/group/keeper/config.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package keeper

// Config is a config struct used for intialising the group module to avoid using globals.
type Config struct {
// MaxMetadataLen defines the max length of the metadata bytes field for various entities within the group module. Defaults to 255 if not explicitly set.
MaxMetadataLen uint64
}

// DefaultConfig returns the default config for group.
func DefaultConfig() Config {
return Config{
MaxMetadataLen: 255,
}
}
12 changes: 11 additions & 1 deletion x/group/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,11 @@ type Keeper struct {
voteByVoterIndex orm.Index

router *authmiddleware.MsgServiceRouter

config Config
}

func NewKeeper(storeKey storetypes.StoreKey, cdc codec.Codec, router *authmiddleware.MsgServiceRouter, accKeeper group.AccountKeeper) Keeper {
func NewKeeper(storeKey storetypes.StoreKey, cdc codec.Codec, router *authmiddleware.MsgServiceRouter, accKeeper group.AccountKeeper, config Config) Keeper {
k := Keeper{
key: storeKey,
router: router,
Expand Down Expand Up @@ -204,6 +206,11 @@ func NewKeeper(storeKey storetypes.StoreKey, cdc codec.Codec, router *authmiddle
}
k.voteTable = *voteTable

if config.MaxMetadataLen == 0 {
config.MaxMetadataLen = DefaultConfig().MaxMetadataLen
}
k.config = config

return k

}
Expand All @@ -213,6 +220,9 @@ func (k Keeper) Logger(ctx sdk.Context) log.Logger {
return ctx.Logger().With("module", fmt.Sprintf("x/%s", group.ModuleName))
}

// MaxMetadataLength returns the max length of the metadata bytes field for various entities within the group module.
func (k Keeper) MaxMetadataLength() uint64 { return k.config.MaxMetadataLen }

// GetGroupSequence returns the current value of the group table sequence
func (k Keeper) GetGroupSequence(ctx sdk.Context) uint64 {
return k.groupTable.Sequence().CurVal(ctx.KVStore(k.key))
Expand Down
24 changes: 12 additions & 12 deletions x/group/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,14 @@ func (k Keeper) CreateGroup(goCtx context.Context, req *group.MsgCreateGroup) (*
return nil, err
}

if err := assertMetadataLength(metadata, "group metadata"); err != nil {
if err := k.assertMetadataLength(metadata, "group metadata"); err != nil {
return nil, err
}

totalWeight := math.NewDecFromInt64(0)
for i := range members.Members {
m := members.Members[i]
if err := assertMetadataLength(m.Metadata, "member metadata"); err != nil {
if err := k.assertMetadataLength(m.Metadata, "member metadata"); err != nil {
return nil, err
}

Expand Down Expand Up @@ -105,7 +105,7 @@ func (k Keeper) UpdateGroupMembers(goCtx context.Context, req *group.MsgUpdateGr
return err
}
for i := range req.MemberUpdates {
if err := assertMetadataLength(req.MemberUpdates[i].Metadata, "group member metadata"); err != nil {
if err := k.assertMetadataLength(req.MemberUpdates[i].Metadata, "group member metadata"); err != nil {
return err
}
groupMember := group.GroupMember{GroupId: req.GroupId,
Expand Down Expand Up @@ -221,7 +221,7 @@ func (k Keeper) UpdateGroupMetadata(goCtx context.Context, req *group.MsgUpdateG
return k.groupTable.Update(ctx.KVStore(k.key), g.GroupId, g)
}

if err := assertMetadataLength(req.Metadata, "group metadata"); err != nil {
if err := k.assertMetadataLength(req.Metadata, "group metadata"); err != nil {
return nil, err
}

Expand All @@ -243,7 +243,7 @@ func (k Keeper) CreateGroupPolicy(goCtx context.Context, req *group.MsgCreateGro
groupID := req.GetGroupID()
metadata := req.GetMetadata()

if err := assertMetadataLength(metadata, "group policy metadata"); err != nil {
if err := k.assertMetadataLength(metadata, "group policy metadata"); err != nil {
return nil, err
}

Expand Down Expand Up @@ -359,7 +359,7 @@ func (k Keeper) UpdateGroupPolicyMetadata(goCtx context.Context, req *group.MsgU
return k.groupPolicyTable.Update(ctx.KVStore(k.key), groupPolicy)
}

if err := assertMetadataLength(metadata, "group policy metadata"); err != nil {
if err := k.assertMetadataLength(metadata, "group policy metadata"); err != nil {
return nil, err
}

Expand All @@ -381,7 +381,7 @@ func (k Keeper) CreateProposal(goCtx context.Context, req *group.MsgCreatePropos
proposers := req.Proposers
msgs := req.GetMsgs()

if err := assertMetadataLength(metadata, "metadata"); err != nil {
if err := k.assertMetadataLength(metadata, "metadata"); err != nil {
return nil, err
}

Expand Down Expand Up @@ -491,7 +491,7 @@ func (k Keeper) Vote(goCtx context.Context, req *group.MsgVote) (*group.MsgVoteR
choice := req.Choice
metadata := req.Metadata

if err := assertMetadataLength(metadata, "metadata"); err != nil {
if err := k.assertMetadataLength(metadata, "metadata"); err != nil {
return nil, err
}

Expand Down Expand Up @@ -773,10 +773,10 @@ func (k Keeper) doAuthenticated(ctx sdk.Context, req authNGroupReq, action actio
}

// assertMetadataLength returns an error if given metadata length
// is greater than a fixed maxMetadataLength.
func assertMetadataLength(metadata []byte, description string) error {
if len(metadata) > group.MaxMetadataLength {
return sdkerrors.Wrap(errors.ErrMaxLimit, description)
// is greater than a pre-defined maxMetadataLen.
func (k Keeper) assertMetadataLength(metadata []byte, description string) error {
if metadata != nil && uint64(len(metadata)) > k.config.MaxMetadataLen {
return sdkerrors.Wrapf(errors.ErrMaxLimit, description)
}
return nil
}
15 changes: 9 additions & 6 deletions x/group/spec/03_messages.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,12 @@ order: 3

A new group can be created with the `MsgCreateGroup`, which has an admin address, a list of members and some optional metadata bytes.

The metadata has a maximum length that is chosen by the app developer, and
passed into the group keeper as a config.

+++ https://github.com/cosmos/cosmos-sdk/blob/6f58963e7f6ce820e9b33f02f06f7b96f6d2e347/proto/cosmos/group/v1beta1/tx.proto#L54-L65

It's expecting to fail if metadata length is greater than some `MaxMetadataLength`.
It's expecting to fail if metadata length is greater than `MaxMetadataLen` config.

## Msg/UpdateGroupMembers

Expand All @@ -37,7 +40,7 @@ The `UpdateGroupMetadata` can be used to update a group metadata.
+++ https://github.com/cosmos/cosmos-sdk/blob/6f58963e7f6ce820e9b33f02f06f7b96f6d2e347/proto/cosmos/group/v1beta1/tx.proto#L107-L118

It's expecting to fail if:
- new metadata length is greater than some `MaxMetadataLength`.
- new metadata length is greater than `MaxMetadataLen` config.
- the signer is not the admin of the group.

## Msg/CreateGroupPolicy
Expand All @@ -46,7 +49,7 @@ A new group policy can be created with the `MsgCreateGroupPolicy`, which has an

+++ https://github.com/cosmos/cosmos-sdk/blob/6f58963e7f6ce820e9b33f02f06f7b96f6d2e347/proto/cosmos/group/v1beta1/tx.proto#L121-L142

It's expecting to fail if metadata length is greater than some `MaxMetadataLength`.
It's expecting to fail if metadata length is greater than `MaxMetadataLen` config.

## Msg/UpdateGroupPolicyAdmin

Expand All @@ -71,7 +74,7 @@ The `UpdateGroupPolicyMetadata` can be used to update a group policy metadata.
+++ https://github.com/cosmos/cosmos-sdk/blob/6f58963e7f6ce820e9b33f02f06f7b96f6d2e347/proto/cosmos/group/v1beta1/tx.proto#L184-L195

It's expecting to fail if:
- new metadata length is greater than some `MaxMetadataLength`.
- new metadata length is greater than `MaxMetadataLen` config.
- the signer is not the admin of the group.

## Msg/CreateProposal
Expand All @@ -81,7 +84,7 @@ An optional `Exec` value can be provided to try to execute the proposal immediat

+++ https://github.com/cosmos/cosmos-sdk/blob/6f58963e7f6ce820e9b33f02f06f7b96f6d2e347/proto/cosmos/group/v1beta1/tx.proto#L218-L239

It's expecting to fail if metadata length is greater than some `MaxMetadataLength`.
It's expecting to fail if metadata length is greater than `MaxMetadataLen` config.

## Msg/Vote

Expand All @@ -90,7 +93,7 @@ An optional `Exec` value can be provided to try to execute the proposal immediat

+++ https://github.com/cosmos/cosmos-sdk/blob/6f58963e7f6ce820e9b33f02f06f7b96f6d2e347/proto/cosmos/group/v1beta1/tx.proto#L248-L265

It's expecting to fail if metadata length is greater than some `MaxMetadataLength`.
It's expecting to fail if metadata length is greater than `MaxMetadataLen` config.

## Msg/Exec

Expand Down
5 changes: 0 additions & 5 deletions x/group/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,6 @@ import (
"github.com/cosmos/cosmos-sdk/x/group/internal/orm"
)

// MaxMetadataLength defines the max length of the metadata bytes field
// for various entities within the group module
// TODO: This could be used as params once x/params is upgraded to use protobuf
const MaxMetadataLength = 255

type DecisionPolicyResult struct {
Allow bool
Final bool
Expand Down

0 comments on commit ee6bedc

Please sign in to comment.