From ee6bedc74225f04cdaf6564d3ef3c68515b3912b Mon Sep 17 00:00:00 2001 From: likhita-809 <78951027+likhita-809@users.noreply.github.com> Date: Thu, 3 Feb 2022 18:10:45 +0530 Subject: [PATCH] feat: Add max metadata length as a group keeper parameter (#11034) ## 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) --- simapp/app.go | 7 ++++++- x/group/keeper/config.go | 14 ++++++++++++++ x/group/keeper/keeper.go | 12 +++++++++++- x/group/keeper/msg_server.go | 24 ++++++++++++------------ x/group/spec/03_messages.md | 15 +++++++++------ x/group/types.go | 5 ----- 6 files changed, 52 insertions(+), 25 deletions(-) create mode 100644 x/group/keeper/config.go diff --git a/simapp/app.go b/simapp/app.go index 7899b16016..692623fc4b 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -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() diff --git a/x/group/keeper/config.go b/x/group/keeper/config.go new file mode 100644 index 0000000000..f162b6aaf0 --- /dev/null +++ b/x/group/keeper/config.go @@ -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, + } +} diff --git a/x/group/keeper/keeper.go b/x/group/keeper/keeper.go index 255dd7790b..48640f1c1d 100644 --- a/x/group/keeper/keeper.go +++ b/x/group/keeper/keeper.go @@ -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, @@ -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 } @@ -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)) diff --git a/x/group/keeper/msg_server.go b/x/group/keeper/msg_server.go index aedb88e7aa..fdc669ee81 100644 --- a/x/group/keeper/msg_server.go +++ b/x/group/keeper/msg_server.go @@ -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 } @@ -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, @@ -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 } @@ -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 } @@ -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 } @@ -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 } @@ -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 } @@ -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 } diff --git a/x/group/spec/03_messages.md b/x/group/spec/03_messages.md index 2408973789..e4f6744b47 100644 --- a/x/group/spec/03_messages.md +++ b/x/group/spec/03_messages.md @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 diff --git a/x/group/types.go b/x/group/types.go index b5802ec52d..ad2ad99e76 100644 --- a/x/group/types.go +++ b/x/group/types.go @@ -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