fix(group): add group members weight checks (backport cosmos#13869) (c…

…osmos#13880)

* fix(group): add group members weight checks (cosmos#13869)

(cherry picked from commit 3423442)

# Conflicts:
#	CHANGELOG.md
#	api/cosmos/tx/v1beta1/service.pulsar.go
#	types/tx/service.pb.go
#	x/group/keeper/keeper.go

* fix conflicts

* updates

* updates

Co-authored-by: Julien Robert <julien@rbrt.fr>

CHANGELOG.md

@@ -47,6 +47,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### Bug Fixes
 
+* (x/group) [#13869](https://github.com/cosmos/cosmos-sdk/pull/13869) Group members weight must be positive and a finite number.
 * (x/bank) [#13821](https://github.com/cosmos/cosmos-sdk/pull/13821) Fix bank store migration of coin metadata.
 * (x/group) [#13808](https://github.com/cosmos/cosmos-sdk/pull/13808) Fix propagation of message events to the current context in `EndBlocker`.
 * (x/gov) [#13728](https://github.com/cosmos/cosmos-sdk/pull/13728) Fix propagation of message events to the current context in `EndBlocker`.

x/group/internal/math/dec.go

@@ -46,6 +46,10 @@ func (x Dec) IsPositive() bool {
 	return !x.dec.Negative && !x.dec.IsZero()
 }
 
+func (x Dec) IsFinite() bool {
+	return x.dec.Form != apd.Finite
+}
+
 // NewDecFromString returns a new Dec from a string
 // It only support finite numbers, not NaN, +Inf, -Inf
 func NewDecFromString(s string) (Dec, error) {
@@ -55,7 +59,7 @@ func NewDecFromString(s string) (Dec, error) {
 	}
 
 	if d.Form != apd.Finite {
-		return Dec{}, grouperrors.ErrInvalidDecString.Wrapf("expected a finite decimal, got %s", s)
+		return Dec{}, errors.ErrInvalidDecString.Wrapf("expected a finite decimal, got %s", s)
 	}
 
 	return Dec{*d}, nil

x/group/keeper/keeper.go

@@ -395,6 +395,8 @@ func (k Keeper) TallyProposalsAtVPEnd(ctx sdk.Context) error {
 	if err != nil {
 		return nil
 	}
+
+	//nolint:gosec // implicit memory aliasing in for loop
 	for _, proposal := range proposals {
 		policyInfo, err := k.getGroupPolicyInfo(ctx, proposal.GroupPolicyAddress)
 		if err != nil {
@@ -415,8 +417,7 @@ func (k Keeper) TallyProposalsAtVPEnd(ctx sdk.Context) error {
 				return err
 			}
 		} else {
-			err = k.doTallyAndUpdate(ctx, &proposal, electorate, policyInfo) //nolint:gosec // implicit memory aliasing in for loop
-			if err != nil {
+			if err := k.doTallyAndUpdate(ctx, &proposal, electorate, policyInfo); err != nil {
 				return sdkerrors.Wrap(err, "doTallyAndUpdate")
 			}
 

x/group/keeper/keeper_test.go

@@ -202,6 +202,26 @@ func (s *TestSuite) TestCreateGroup() {
 			},
 			expErr: true,
 		},
+		"invalid member weight - Inf": {
+			req: &group.MsgCreateGroup{
+				Admin: addr1.String(),
+				Members: []group.MemberRequest{{
+					Address: addr3.String(),
+					Weight:  "inf",
+				}},
+			},
+			expErr: true,
+		},
+		"invalid member weight - NaN": {
+			req: &group.MsgCreateGroup{
+				Admin: addr1.String(),
+				Members: []group.MemberRequest{{
+					Address: addr3.String(),
+					Weight:  "NaN",
+				}},
+			},
+			expErr: true,
+		},
 	}
 
 	var seq uint32 = 1

x/group/keeper/msg_server.go

@@ -796,14 +796,15 @@ func (k Keeper) doTallyAndUpdate(ctx context.Context, p *group.Proposal, groupIn
 		return err
 	}
 
-	result, err := policy.Allow(tallyResult, groupInfo.TotalWeight)
+	sinceSubmission := ctx.BlockTime().Sub(p.SubmitTime) // duration passed since proposal submission.
+	result, err := policy.Allow(tallyResult, electorate.TotalWeight, sinceSubmission)
 	if err != nil {
-		return errorsmod.Wrap(err, "policy allow")
+		return sdkerrors.Wrap(err, "policy allow")
 	}
 
 	// If the result was final (i.e. enough votes to pass) or if the voting
 	// period ended, then we consider the proposal as final.
-	if isFinal := result.Final || k.HeaderService.HeaderInfo(ctx).Time.After(p.VotingPeriodEnd); isFinal {
+	if isFinal := result.Final || ctx.BlockTime().After(p.VotingPeriodEnd); isFinal {
 		if err := k.pruneVotes(ctx, p.Id); err != nil {
 			return err
 		}

x/group/msgs.go

@@ -89,6 +89,136 @@ func (m MsgCreateGroupWithPolicy) UnpackInterfaces(unpacker gogoprotoany.AnyUnpa
 	return unpacker.UnpackAny(m.DecisionPolicy, &decisionPolicy)
 }
 
+// Route Implements Msg.
+func (m MsgCreateGroupWithPolicy) Route() string {
+	return sdk.MsgTypeURL(&m)
+}
+
+// Type Implements Msg.
+func (m MsgCreateGroupWithPolicy) Type() string {
+	return sdk.MsgTypeURL(&m)
+}
+
+// GetSignBytes Implements Msg.
+func (m MsgCreateGroupWithPolicy) GetSignBytes() []byte {
+	return sdk.MustSortJSON(ModuleCdc.MustMarshalJSON(&m))
+}
+
+// GetSigners returns the expected signers for a MsgCreateGroupWithPolicy.
+func (m MsgCreateGroupWithPolicy) GetSigners() []sdk.AccAddress {
+	admin := sdk.MustAccAddressFromBech32(m.Admin)
+	return []sdk.AccAddress{admin}
+}
+
+// ValidateBasic does a sanity check on the provided data
+func (m MsgCreateGroupWithPolicy) ValidateBasic() error {
+	_, err := sdk.AccAddressFromBech32(m.Admin)
+	if err != nil {
+		return sdkerrors.Wrap(err, "admin")
+	}
+	policy, err := m.GetDecisionPolicy()
+	if err != nil {
+		return sdkerrors.Wrap(err, "decision policy")
+	}
+	if err := policy.ValidateBasic(); err != nil {
+		return sdkerrors.Wrap(err, "decision policy")
+	}
+
+	return strictValidateMembers(m.Members)
+}
+
+var _ sdk.Msg = &MsgCreateGroupPolicy{}
+
+// Route Implements Msg.
+func (m MsgCreateGroupPolicy) Route() string {
+	return sdk.MsgTypeURL(&m)
+}
+
+// Type Implements Msg.
+func (m MsgCreateGroupPolicy) Type() string { return sdk.MsgTypeURL(&m) }
+
+// GetSignBytes Implements Msg.
+func (m MsgCreateGroupPolicy) GetSignBytes() []byte {
+	return sdk.MustSortJSON(ModuleCdc.MustMarshalJSON(&m))
+}
+
+// GetSigners returns the expected signers for a MsgCreateGroupPolicy.
+func (m MsgCreateGroupPolicy) GetSigners() []sdk.AccAddress {
+	admin := sdk.MustAccAddressFromBech32(m.Admin)
+	return []sdk.AccAddress{admin}
+}
+
+// ValidateBasic does a sanity check on the provided data
+func (m MsgCreateGroupPolicy) ValidateBasic() error {
+	_, err := sdk.AccAddressFromBech32(m.Admin)
+	if err != nil {
+		return sdkerrors.Wrap(err, "admin")
+	}
+	if m.GroupId == 0 {
+		return sdkerrors.Wrap(errors.ErrEmpty, "group id")
+	}
+
+	policy, err := m.GetDecisionPolicy()
+	if err != nil {
+		return sdkerrors.Wrap(err, "decision policy")
+	}
+
+	if err := policy.ValidateBasic(); err != nil {
+		return sdkerrors.Wrap(err, "decision policy")
+	}
+	return nil
+}
+
+var _ sdk.Msg = &MsgUpdateGroupPolicyAdmin{}
+
+// Route Implements Msg.
+func (m MsgUpdateGroupPolicyAdmin) Route() string {
+	return sdk.MsgTypeURL(&m)
+}
+
+// Type Implements Msg.
+func (m MsgUpdateGroupPolicyAdmin) Type() string { return sdk.MsgTypeURL(&m) }
+
+// GetSignBytes Implements Msg.
+func (m MsgUpdateGroupPolicyAdmin) GetSignBytes() []byte {
+	return sdk.MustSortJSON(ModuleCdc.MustMarshalJSON(&m))
+}
+
+// GetSigners returns the expected signers for a MsgUpdateGroupPolicyAdmin.
+func (m MsgUpdateGroupPolicyAdmin) GetSigners() []sdk.AccAddress {
+	admin := sdk.MustAccAddressFromBech32(m.Admin)
+
+	return []sdk.AccAddress{admin}
+}
+
+// ValidateBasic does a sanity check on the provided data
+func (m MsgUpdateGroupPolicyAdmin) ValidateBasic() error {
+	admin, err := sdk.AccAddressFromBech32(m.Admin)
+	if err != nil {
+		return sdkerrors.Wrap(err, "admin")
+	}
+
+	newAdmin, err := sdk.AccAddressFromBech32(m.NewAdmin)
+	if err != nil {
+		return sdkerrors.Wrap(err, "new admin")
+	}
+
+	_, err = sdk.AccAddressFromBech32(m.GroupPolicyAddress)
+	if err != nil {
+		return sdkerrors.Wrap(err, "group policy")
+	}
+
+	if admin.Equals(newAdmin) {
+		return sdkerrors.Wrap(errors.ErrInvalid, "new and old admin are same")
+	}
+	return nil
+}
+
+var (
+	_ sdk.Msg                       = &MsgUpdateGroupPolicyDecisionPolicy{}
+	_ types.UnpackInterfacesMessage = MsgUpdateGroupPolicyDecisionPolicy{}
+)
+
 // NewMsgUpdateGroupPolicyDecisionPolicy creates a new MsgUpdateGroupPolicyDecisionPolicy.
 func NewMsgUpdateGroupPolicyDecisionPolicy(admin, address string, decisionPolicy DecisionPolicy) (*MsgUpdateGroupPolicyDecisionPolicy, error) {
 	m := &MsgUpdateGroupPolicyDecisionPolicy{