Skip to content

Commit

Permalink
feat: Add percentage decision policy to x/group (#11072)
Browse files Browse the repository at this point in the history
## Description

Closes: #10946 

Add a percentage decision policy to group module.

---

### 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 11, 2022
1 parent 89739a5 commit 2c184d2
Show file tree
Hide file tree
Showing 13 changed files with 1,642 additions and 199 deletions.
697 changes: 633 additions & 64 deletions api/cosmos/group/v1beta1/types.pulsar.go

Large diffs are not rendered by default.

12 changes: 12 additions & 0 deletions proto/cosmos/group/v1beta1/types.proto
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,18 @@ message ThresholdDecisionPolicy {
google.protobuf.Duration timeout = 2 [(gogoproto.stdduration) = true, (gogoproto.nullable) = false];
}

// PercentageDecisionPolicy implements the DecisionPolicy interface
message PercentageDecisionPolicy {
option (cosmos_proto.implements_interface) = "DecisionPolicy";

// percentage is the minimum percentage the weighted sum of yes votes must meet for a proposal to succeed.
string percentage = 1;

// timeout is the duration from submission of a proposal to the end of voting period
// Within these times votes and exec messages can be submitted.
google.protobuf.Duration timeout = 2 [(gogoproto.stdduration) = true, (gogoproto.nullable) = false];
}

// Choice defines available types of choices for voting.
enum Choice {

Expand Down
3 changes: 3 additions & 0 deletions x/group/client/cli/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,9 @@ Note, the '--from' flag is ignored as it is implied from [admin].
Example:
$ %s tx group create-group-policy [admin] [group-id] [metadata] \
'{"@type":"/cosmos.group.v1beta1.ThresholdDecisionPolicy", "threshold":"1", "timeout":"1s"}'
Here, we can use percentage decision policy when needed, where 0 < percentage <= 1.
Ex: '{"@type":"/cosmos.group.v1beta1.PercentageDecisionPolicy", "percentage":"0.5", "timeout":"1s"}'
`,
version.AppName,
),
Expand Down
2 changes: 2 additions & 0 deletions x/group/client/testutil/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,7 @@ func (s *IntegrationTestSuite) TestQueryGroupPoliciesByGroup() {
s.groupPolicies[2],
s.groupPolicies[3],
s.groupPolicies[4],
s.groupPolicies[5],
},
},
}
Expand Down Expand Up @@ -443,6 +444,7 @@ func (s *IntegrationTestSuite) TestQueryGroupPoliciesByAdmin() {
s.groupPolicies[2],
s.groupPolicies[3],
s.groupPolicies[4],
s.groupPolicies[5],
},
},
}
Expand Down
114 changes: 113 additions & 1 deletion x/group/client/testutil/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,10 +129,29 @@ func (s *IntegrationTestSuite) SetupSuite() {
out, err = cli.ExecTestCLICmd(val.ClientCtx, client.QueryGroupPoliciesByGroupCmd(), []string{"1", fmt.Sprintf("--%s=json", tmcli.OutputFlag)})
s.Require().NoError(err, out.String())
}
percentage := 0.5
// create group policy with percentage decision policy
out, err = cli.ExecTestCLICmd(val.ClientCtx, client.MsgCreateGroupPolicyCmd(),
append(
[]string{
val.Address.String(),
"1",
validMetadata,
fmt.Sprintf("{\"@type\":\"/cosmos.group.v1beta1.PercentageDecisionPolicy\", \"percentage\":\"%f\", \"timeout\":\"30000s\"}", percentage),
},
commonFlags...,
),
)
s.Require().NoError(err, out.String())
s.Require().NoError(val.ClientCtx.Codec.UnmarshalJSON(out.Bytes(), &txResp), out.String())
s.Require().Equal(uint32(0), txResp.Code, out.String())

out, err = cli.ExecTestCLICmd(val.ClientCtx, client.QueryGroupPoliciesByGroupCmd(), []string{"1", fmt.Sprintf("--%s=json", tmcli.OutputFlag)})
s.Require().NoError(err, out.String())

var res group.QueryGroupPoliciesByGroupResponse
s.Require().NoError(val.ClientCtx.Codec.UnmarshalJSON(out.Bytes(), &res))
s.Require().Equal(len(res.GroupPolicies), 5)
s.Require().Equal(len(res.GroupPolicies), 6)
s.groupPolicies = res.GroupPolicies

// create a proposal
Expand Down Expand Up @@ -711,6 +730,22 @@ func (s *IntegrationTestSuite) TestTxCreateGroupPolicy() {
&sdk.TxResponse{},
0,
},
{
"correct data with percentage decision policy",
append(
[]string{
val.Address.String(),
fmt.Sprintf("%v", groupID),
validMetadata,
"{\"@type\":\"/cosmos.group.v1beta1.PercentageDecisionPolicy\", \"percentage\":\"0.5\", \"timeout\":\"1s\"}",
},
commonFlags...,
),
false,
"",
&sdk.TxResponse{},
0,
},
{
"with amino-json",
append(
Expand Down Expand Up @@ -776,6 +811,38 @@ func (s *IntegrationTestSuite) TestTxCreateGroupPolicy() {
&sdk.TxResponse{},
0,
},
{
"invalid percentage decision policy with negative value",
append(
[]string{
val.Address.String(),
fmt.Sprintf("%v", groupID),
validMetadata,
"{\"@type\":\"/cosmos.group.v1beta1.PercentageDecisionPolicy\", \"percentage\":\"-0.5\", \"timeout\":\"1s\"}",
},
commonFlags...,
),
true,
"expected a positive decimal",
&sdk.TxResponse{},
0,
},
{
"invalid percentage decision policy with value greater than 1",
append(
[]string{
val.Address.String(),
fmt.Sprintf("%v", groupID),
validMetadata,
"{\"@type\":\"/cosmos.group.v1beta1.PercentageDecisionPolicy\", \"percentage\":\"2\", \"timeout\":\"1s\"}",
},
commonFlags...,
),
true,
"percentage must be > 0 and <= 1",
&sdk.TxResponse{},
0,
},
}

for _, tc := range testCases {
Expand Down Expand Up @@ -936,6 +1003,21 @@ func (s *IntegrationTestSuite) TestTxUpdateGroupPolicyDecisionPolicy() {
&sdk.TxResponse{},
0,
},
{
"correct data with percentage decision policy",
append(
[]string{
groupPolicy.Admin,
groupPolicy.Address,
"{\"@type\":\"/cosmos.group.v1beta1.PercentageDecisionPolicy\", \"percentage\":\"0.5\", \"timeout\":\"40000s\"}",
},
commonFlags...,
),
false,
"",
&sdk.TxResponse{},
0,
},
{
"with amino-json",
append(
Expand Down Expand Up @@ -982,6 +1064,36 @@ func (s *IntegrationTestSuite) TestTxUpdateGroupPolicyDecisionPolicy() {
&sdk.TxResponse{},
0,
},
{
"invalid percentage decision policy with negative value",
append(
[]string{
groupPolicy.Admin,
groupPolicy.Address,
"{\"@type\":\"/cosmos.group.v1beta1.PercentageDecisionPolicy\", \"percentage\":\"-0.5\", \"timeout\":\"1s\"}",
},
commonFlags...,
),
true,
"expected a positive decimal",
&sdk.TxResponse{},
0,
},
{
"invalid percentage decision policy with value greater than 1",
append(
[]string{
groupPolicy.Admin,
groupPolicy.Address,
"{\"@type\":\"/cosmos.group.v1beta1.PercentageDecisionPolicy\", \"percentage\":\"2\", \"timeout\":\"40000s\"}",
},
commonFlags...,
),
true,
"percentage must be > 0 and <= 1",
&sdk.TxResponse{},
0,
},
}

for _, tc := range testCases {
Expand Down
2 changes: 2 additions & 0 deletions x/group/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
func RegisterLegacyAminoCodec(cdc *codec.LegacyAmino) {
cdc.RegisterInterface((*DecisionPolicy)(nil), nil)
cdc.RegisterConcrete(&ThresholdDecisionPolicy{}, "cosmos-sdk/ThresholdDecisionPolicy", nil)
cdc.RegisterConcrete(&PercentageDecisionPolicy{}, "cosmos-sdk/PercentageDecisionPolicy", nil)
cdc.RegisterConcrete(&MsgCreateGroup{}, "cosmos-sdk/MsgCreateGroup", nil)
cdc.RegisterConcrete(&MsgUpdateGroupMembers{}, "cosmos-sdk/MsgUpdateGroupMembers", nil)
cdc.RegisterConcrete(&MsgUpdateGroupAdmin{}, "cosmos-sdk/MsgUpdateGroupAdmin", nil)
Expand Down Expand Up @@ -49,6 +50,7 @@ func RegisterInterfaces(registry cdctypes.InterfaceRegistry) {
"cosmos.group.v1beta1.DecisionPolicy",
(*DecisionPolicy)(nil),
&ThresholdDecisionPolicy{},
&PercentageDecisionPolicy{},
)
}

Expand Down
15 changes: 15 additions & 0 deletions x/group/internal/math/dec.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,21 @@ func Add(x Dec, y Dec) (Dec, error) {
return x.Add(y)
}

var dec128Context = apd.Context{
Precision: 34,
MaxExponent: apd.MaxExponent,
MinExponent: apd.MinExponent,
Traps: apd.DefaultTraps,
}

// Quo returns a new Dec with value `x/y` (formatted as decimal128, 34 digit precision) without mutating any
// argument and error if there is an overflow.
func (x Dec) Quo(y Dec) (Dec, error) {
var z Dec
_, err := dec128Context.Quo(&z.dec, &x.dec, &y.dec)
return z, sdkerrors.Wrap(err, "decimal quotient error")
}

func (x Dec) IsZero() bool {
return x.dec.IsZero()
}
Expand Down
4 changes: 4 additions & 0 deletions x/group/internal/math/dec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,10 @@ func TestDec(t *testing.T) {
require.NoError(t, err)
require.True(t, res.IsEqual(minusFivePointZero))

res, err = four.Quo(two)
require.NoError(t, err)
require.True(t, res.IsEqual(two))

require.False(t, zero.IsNegative())
require.False(t, one.IsNegative())
require.True(t, minusOne.IsNegative())
Expand Down
87 changes: 79 additions & 8 deletions x/group/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -719,9 +719,10 @@ func (s *TestSuite) TestCreateGroupPolicy() {
myGroupID := groupRes.GroupId

specs := map[string]struct {
req *group.MsgCreateGroupPolicy
policy group.DecisionPolicy
expErr bool
req *group.MsgCreateGroupPolicy
policy group.DecisionPolicy
expErr bool
expErrMsg string
}{
"all good": {
req: &group.MsgCreateGroupPolicy{
Expand All @@ -734,6 +735,17 @@ func (s *TestSuite) TestCreateGroupPolicy() {
time.Second,
),
},
"all good with percentage decision policy": {
req: &group.MsgCreateGroupPolicy{
Admin: addr1.String(),
Metadata: nil,
GroupId: myGroupID,
},
policy: group.NewPercentageDecisionPolicy(
"0.5",
time.Second,
),
},
"decision policy threshold > total group weight": {
req: &group.MsgCreateGroupPolicy{
Admin: addr1.String(),
Expand All @@ -755,7 +767,8 @@ func (s *TestSuite) TestCreateGroupPolicy() {
"1",
time.Second,
),
expErr: true,
expErr: true,
expErrMsg: "not found",
},
"admin not group admin": {
req: &group.MsgCreateGroupPolicy{
Expand All @@ -767,7 +780,8 @@ func (s *TestSuite) TestCreateGroupPolicy() {
"1",
time.Second,
),
expErr: true,
expErr: true,
expErrMsg: "not group admin",
},
"metadata too long": {
req: &group.MsgCreateGroupPolicy{
Expand All @@ -779,7 +793,34 @@ func (s *TestSuite) TestCreateGroupPolicy() {
"1",
time.Second,
),
expErr: true,
expErr: true,
expErrMsg: "limit exceeded",
},
"percentage decision policy with negative value": {
req: &group.MsgCreateGroupPolicy{
Admin: addr1.String(),
Metadata: nil,
GroupId: myGroupID,
},
policy: group.NewPercentageDecisionPolicy(
"-0.5",
time.Second,
),
expErr: true,
expErrMsg: "expected a positive decimal",
},
"percentage decision policy with value greater than 1": {
req: &group.MsgCreateGroupPolicy{
Admin: addr1.String(),
Metadata: nil,
GroupId: myGroupID,
},
policy: group.NewPercentageDecisionPolicy(
"2",
time.Second,
),
expErr: true,
expErrMsg: "percentage must be > 0 and <= 1",
},
}
for msg, spec := range specs {
Expand All @@ -791,6 +832,7 @@ func (s *TestSuite) TestCreateGroupPolicy() {
res, err := s.keeper.CreateGroupPolicy(s.ctx, spec.req)
if spec.expErr {
s.Require().Error(err)
s.Require().Contains(err.Error(), spec.expErrMsg)
return
}
s.Require().NoError(err)
Expand All @@ -806,7 +848,12 @@ func (s *TestSuite) TestCreateGroupPolicy() {
s.Assert().Equal(spec.req.Admin, groupPolicy.Admin)
s.Assert().Equal(spec.req.Metadata, groupPolicy.Metadata)
s.Assert().Equal(uint64(1), groupPolicy.Version)
s.Assert().Equal(spec.policy.(*group.ThresholdDecisionPolicy), groupPolicy.GetDecisionPolicy())
percentageDecisionPolicy, ok := spec.policy.(*group.PercentageDecisionPolicy)
if ok {
s.Assert().Equal(percentageDecisionPolicy, groupPolicy.GetDecisionPolicy())
} else {
s.Assert().Equal(spec.policy.(*group.ThresholdDecisionPolicy), groupPolicy.GetDecisionPolicy())
}
})
}
}
Expand Down Expand Up @@ -1029,6 +1076,26 @@ func (s *TestSuite) TestUpdateGroupPolicyDecisionPolicy() {
},
expErr: false,
},
"correct data with percentage decision policy": {
req: &group.MsgUpdateGroupPolicyDecisionPolicy{
Admin: admin.String(),
Address: groupPolicyAddr,
},
policy: group.NewPercentageDecisionPolicy(
"0.5",
time.Duration(2)*time.Second,
),
expGroupPolicy: &group.GroupPolicyInfo{
Admin: admin.String(),
Address: groupPolicyAddr,
GroupId: myGroupID,
Metadata: nil,
Version: 3,
DecisionPolicy: nil,
CreatedAt: s.blockTime,
},
expErr: false,
},
}
for msg, spec := range specs {
spec := spec
Expand Down Expand Up @@ -1076,9 +1143,13 @@ func (s *TestSuite) TestGroupPoliciesByAdminOrGroup() {
"10",
time.Second,
),
group.NewPercentageDecisionPolicy(
"0.5",
time.Second,
),
}

count := 2
count := 3
expectAccs := make([]*group.GroupPolicyInfo, count)
for i := range expectAccs {
req := &group.MsgCreateGroupPolicy{
Expand Down
Loading

0 comments on commit 2c184d2

Please sign in to comment.