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

feat: Add max metadata length as a group keeper parameter #11034

Merged
merged 23 commits into from
Feb 3, 2022
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
6d59be9
wip
likhita-809 Jan 20, 2022
a2111f7
wip
likhita-809 Jan 20, 2022
e344c24
Merge branch 'master' of https://github.com/cosmos/cosmos-sdk into li…
likhita-809 Jan 24, 2022
a91d02c
wip
likhita-809 Jan 24, 2022
d1f4808
set maxMetadataLength in keeper
likhita-809 Jan 25, 2022
dac22da
Merge branch 'master' of https://github.com/cosmos/cosmos-sdk into li…
likhita-809 Jan 27, 2022
1266944
wip
likhita-809 Jan 27, 2022
4892ac2
Merge branch 'master' of https://github.com/cosmos/cosmos-sdk into li…
likhita-809 Jan 27, 2022
f3884bb
Merge branch 'master' of https://github.com/cosmos/cosmos-sdk into li…
likhita-809 Jan 31, 2022
886042c
address review comments
likhita-809 Jan 31, 2022
df396d2
Merge branch 'master' into likhita/add-MaxMetadataLength-to-groupKeeper
likhita-809 Jan 31, 2022
8428805
address review comments
likhita-809 Feb 2, 2022
873ddf1
change MaxMetadataLength to MaxMetadataLen in group specs
likhita-809 Feb 2, 2022
13816c7
use group config and add changelog
likhita-809 Feb 2, 2022
10caed0
Merge branch 'master' of https://github.com/cosmos/cosmos-sdk into li…
likhita-809 Feb 2, 2022
d244bcd
remove unecessary changes
likhita-809 Feb 2, 2022
9ef457b
nit
likhita-809 Feb 2, 2022
2bea0f3
add spec for metadata
likhita-809 Feb 2, 2022
4ce26d6
Merge branch 'master' of https://github.com/cosmos/cosmos-sdk into li…
likhita-809 Feb 3, 2022
40ab321
Merge branch 'master' into likhita/add-MaxMetadataLength-to-groupKeeper
likhita-809 Feb 3, 2022
033341f
address review comment
likhita-809 Feb 3, 2022
2f1bbb0
Merge branch 'likhita/add-MaxMetadataLength-to-groupKeeper' of https:…
likhita-809 Feb 3, 2022
58d4aa5
Merge branch 'master' into likhita/add-MaxMetadataLength-to-groupKeeper
likhita-809 Feb 3, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,14 @@ 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)
groupParams := &groupkeeper.KeeperParams{
likhita-809 marked this conversation as resolved.
Show resolved Hide resolved
StoreKey: keys[group.StoreKey],
Cdc: appCodec,
Router: app.msgSvcRouter,
AccKeeper: app.AccountKeeper,
MaxMetadataLength: int(255),
likhita-809 marked this conversation as resolved.
Show resolved Hide resolved
}
app.GroupKeeper = groupkeeper.NewKeeper(*groupParams)

// register the proposal types
govRouter := govv1beta1.NewRouter()
Expand Down
37 changes: 28 additions & 9 deletions x/group/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,16 +73,26 @@ type Keeper struct {
voteByVoterIndex orm.Index

router *authmiddleware.MsgServiceRouter

maxMetadataLength int // MaxMetadataLength defines the max length of the metadata bytes field for various entities within the group module
likhita-809 marked this conversation as resolved.
Show resolved Hide resolved
}

type KeeperParams struct {
blushi marked this conversation as resolved.
Show resolved Hide resolved
StoreKey storetypes.StoreKey
Cdc codec.Codec
Router *authmiddleware.MsgServiceRouter
AccKeeper group.AccountKeeper
MaxMetadataLength int
likhita-809 marked this conversation as resolved.
Show resolved Hide resolved
}

func NewKeeper(storeKey storetypes.StoreKey, cdc codec.Codec, router *authmiddleware.MsgServiceRouter, accKeeper group.AccountKeeper) Keeper {
func NewKeeper(params KeeperParams) Keeper {
k := Keeper{
key: storeKey,
router: router,
accKeeper: accKeeper,
key: params.StoreKey,
router: params.Router,
accKeeper: params.AccKeeper,
}

groupTable, err := orm.NewAutoUInt64Table([2]byte{GroupTablePrefix}, GroupTableSeqPrefix, &group.GroupInfo{}, cdc)
groupTable, err := orm.NewAutoUInt64Table([2]byte{GroupTablePrefix}, GroupTableSeqPrefix, &group.GroupInfo{}, params.Cdc)
if err != nil {
panic(err.Error())
}
Expand All @@ -99,7 +109,7 @@ func NewKeeper(storeKey storetypes.StoreKey, cdc codec.Codec, router *authmiddle
k.groupTable = *groupTable

// Group Member Table
groupMemberTable, err := orm.NewPrimaryKeyTable([2]byte{GroupMemberTablePrefix}, &group.GroupMember{}, cdc)
groupMemberTable, err := orm.NewPrimaryKeyTable([2]byte{GroupMemberTablePrefix}, &group.GroupMember{}, params.Cdc)
if err != nil {
panic(err.Error())
}
Expand All @@ -125,7 +135,7 @@ func NewKeeper(storeKey storetypes.StoreKey, cdc codec.Codec, router *authmiddle

// Group Policy Table
k.groupPolicySeq = orm.NewSequence(GroupPolicyTableSeqPrefix)
groupPolicyTable, err := orm.NewPrimaryKeyTable([2]byte{GroupPolicyTablePrefix}, &group.GroupPolicyInfo{}, cdc)
groupPolicyTable, err := orm.NewPrimaryKeyTable([2]byte{GroupPolicyTablePrefix}, &group.GroupPolicyInfo{}, params.Cdc)
if err != nil {
panic(err.Error())
}
Expand All @@ -149,7 +159,7 @@ func NewKeeper(storeKey storetypes.StoreKey, cdc codec.Codec, router *authmiddle
k.groupPolicyTable = *groupPolicyTable

// Proposal Table
proposalTable, err := orm.NewAutoUInt64Table([2]byte{ProposalTablePrefix}, ProposalTableSeqPrefix, &group.Proposal{}, cdc)
proposalTable, err := orm.NewAutoUInt64Table([2]byte{ProposalTablePrefix}, ProposalTableSeqPrefix, &group.Proposal{}, params.Cdc)
if err != nil {
panic(err.Error())
}
Expand Down Expand Up @@ -182,7 +192,7 @@ func NewKeeper(storeKey storetypes.StoreKey, cdc codec.Codec, router *authmiddle
k.proposalTable = *proposalTable

// Vote Table
voteTable, err := orm.NewPrimaryKeyTable([2]byte{VoteTablePrefix}, &group.Vote{}, cdc)
voteTable, err := orm.NewPrimaryKeyTable([2]byte{VoteTablePrefix}, &group.Vote{}, params.Cdc)
if err != nil {
panic(err.Error())
}
Expand All @@ -204,6 +214,12 @@ func NewKeeper(storeKey storetypes.StoreKey, cdc codec.Codec, router *authmiddle
}
k.voteTable = *voteTable

if params.MaxMetadataLength != 0 {
k.maxMetadataLength = params.MaxMetadataLength
} else {
k.maxMetadataLength = defaultMaxMetadataLength
}

return k

}
Expand All @@ -213,6 +229,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() int { return k.maxMetadataLength }

// 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
33 changes: 21 additions & 12 deletions x/group/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,10 @@ var _ group.MsgServer = Keeper{}

// TODO: Revisit this once we have propoer gas fee framework.
// Tracking issues https://github.com/cosmos/cosmos-sdk/issues/9054, https://github.com/cosmos/cosmos-sdk/discussions/9072
likhita-809 marked this conversation as resolved.
Show resolved Hide resolved
const gasCostPerIteration = uint64(20)
const (
gasCostPerIteration = uint64(20)
defaultMaxMetadataLength = 255
)

func (k Keeper) CreateGroup(goCtx context.Context, req *group.MsgCreateGroup) (*group.MsgCreateGroupResponse, error) {
ctx := sdk.UnwrapSDKContext(goCtx)
Expand All @@ -34,14 +37,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 +108,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 +224,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 +246,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 +362,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 +384,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 +494,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 @@ -774,9 +777,15 @@ 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)
func (k Keeper) assertMetadataLength(metadata []byte, description string) error {
if metadata != nil && k.maxMetadataLength != 0 {
if len(metadata) > k.maxMetadataLength {
return sdkerrors.Wrapf(errors.ErrMaxLimit, description)
}
} else if metadata != nil {
if len(metadata) > defaultMaxMetadataLength {
return sdkerrors.Wrapf(errors.ErrMaxLimit, description)
}
likhita-809 marked this conversation as resolved.
Show resolved Hide resolved
}
return nil
}
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