From 11c8e7a8eb3403004f6ee9a349cebba667c0eac0 Mon Sep 17 00:00:00 2001 From: Amaury M <1293565+amaurym@users.noreply.github.com> Date: Tue, 15 Nov 2022 17:09:51 +0100 Subject: [PATCH 01/11] fix: don't check MinExecutionPeriod in `Allow` --- x/group/types.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/x/group/types.go b/x/group/types.go index ce01ff7b267d..e839c2bc905f 100644 --- a/x/group/types.go +++ b/x/group/types.go @@ -66,10 +66,6 @@ func (p ThresholdDecisionPolicy) ValidateBasic() error { // Allow allows a proposal to pass when the tally of yes votes equals or exceeds the threshold before the timeout. func (p ThresholdDecisionPolicy) Allow(tallyResult TallyResult, totalPower string, sinceSubmission time.Duration) (DecisionPolicyResult, error) { - if sinceSubmission < p.Windows.MinExecutionPeriod { - return DecisionPolicyResult{}, errors.ErrUnauthorized.Wrapf("must wait %s after submission before execution, currently at %s", p.Windows.MinExecutionPeriod, sinceSubmission) - } - threshold, err := math.NewPositiveDecFromString(p.Threshold) if err != nil { return DecisionPolicyResult{}, sdkerrors.Wrap(err, "threshold") @@ -179,10 +175,6 @@ func (p *PercentageDecisionPolicy) Validate(g GroupInfo, config Config) error { // Allow allows a proposal to pass when the tally of yes votes equals or exceeds the percentage threshold before the timeout. func (p PercentageDecisionPolicy) Allow(tally TallyResult, totalPower string, sinceSubmission time.Duration) (DecisionPolicyResult, error) { - if sinceSubmission < p.Windows.MinExecutionPeriod { - return DecisionPolicyResult{}, errors.ErrUnauthorized.Wrapf("must wait %s after submission before execution, currently at %s", p.Windows.MinExecutionPeriod, sinceSubmission) - } - percentage, err := math.NewPositiveDecFromString(p.Percentage) if err != nil { return DecisionPolicyResult{}, sdkerrors.Wrap(err, "percentage") From e747e846dc95920d557cdd656dd490de86486db0 Mon Sep 17 00:00:00 2001 From: Amaury M <1293565+amaurym@users.noreply.github.com> Date: Tue, 15 Nov 2022 17:35:35 +0100 Subject: [PATCH 02/11] Check MinExecutionPeriod on doExecuteMsgs --- x/group/keeper/msg_server.go | 6 +++--- x/group/keeper/proposal_executor.go | 8 +++++++- x/group/types.go | 17 ++++++++++++++--- 3 files changed, 24 insertions(+), 7 deletions(-) diff --git a/x/group/keeper/msg_server.go b/x/group/keeper/msg_server.go index fd144525cc34..e760fa3807de 100644 --- a/x/group/keeper/msg_server.go +++ b/x/group/keeper/msg_server.go @@ -685,8 +685,7 @@ func (k Keeper) doTallyAndUpdate(ctx sdk.Context, p *group.Proposal, electorate return err } - sinceSubmission := ctx.BlockTime().Sub(p.SubmitTime) // duration passed since proposal submission. - result, err := policy.Allow(tallyResult, electorate.TotalWeight, sinceSubmission) + result, err := policy.Allow(tallyResult, electorate.TotalWeight) // If the result was final (i.e. enough votes to pass) or if the voting // period ended, then we consider the proposal as final. isFinal := result.Final || ctx.BlockTime().After(p.VotingPeriodEnd) @@ -754,7 +753,8 @@ func (k Keeper) Exec(goCtx context.Context, req *group.MsgExec) (*group.MsgExecR return nil, err } - if results, err := k.doExecuteMsgs(cacheCtx, k.router, proposal, addr); err != nil { + decisionPolicy := policyInfo.DecisionPolicy.GetCachedValue().(group.DecisionPolicy) + if results, err := k.doExecuteMsgs(cacheCtx, k.router, proposal, addr, decisionPolicy); err != nil { proposal.ExecutorResult = group.PROPOSAL_EXECUTOR_RESULT_FAILURE logs = fmt.Sprintf("proposal execution failed on proposal %d, because of error %s", id, err.Error()) k.Logger(ctx).Info("proposal execution failed", "cause", err, "proposalID", id) diff --git a/x/group/keeper/proposal_executor.go b/x/group/keeper/proposal_executor.go index bc1317c629ac..3bf6f9102340 100644 --- a/x/group/keeper/proposal_executor.go +++ b/x/group/keeper/proposal_executor.go @@ -12,7 +12,13 @@ import ( // doExecuteMsgs routes the messages to the registered handlers. Messages are limited to those that require no authZ or // by the account of group policy only. Otherwise this gives access to other peoples accounts as the sdk middlewares are bypassed -func (s Keeper) doExecuteMsgs(ctx sdk.Context, router *baseapp.MsgServiceRouter, proposal group.Proposal, groupPolicyAcc sdk.AccAddress) ([]sdk.Result, error) { +func (s Keeper) doExecuteMsgs(ctx sdk.Context, router *baseapp.MsgServiceRouter, proposal group.Proposal, groupPolicyAcc sdk.AccAddress, decisionPolicy group.DecisionPolicy) ([]sdk.Result, error) { + // Ensure it's not too early to execute the messages. + minExecutionDate := proposal.SubmitTime.Add(decisionPolicy.GetMinExecutionPeriod()) + if ctx.BlockTime().Before(minExecutionDate) { + return nil, errors.ErrInvalid.Wrapf("must wait until %s to execute proposal", minExecutionDate) + } + // Ensure it's not too late to execute the messages. // After https://github.com/cosmos/cosmos-sdk/issues/11245, proposals should // be pruned automatically, so this function should not even be called, as diff --git a/x/group/types.go b/x/group/types.go index e839c2bc905f..c1ae21fe907f 100644 --- a/x/group/types.go +++ b/x/group/types.go @@ -31,10 +31,13 @@ type DecisionPolicy interface { // GetVotingPeriod returns the duration after proposal submission where // votes are accepted. GetVotingPeriod() time.Duration + // GetMinExecutionPeriod returns the minimum duration after submission + // where we can execution a proposal. + GetMinExecutionPeriod() time.Duration // Allow defines policy-specific logic to allow a proposal to pass or not, // based on its tally result, the group's total power and the time since // the proposal was submitted. - Allow(tallyResult TallyResult, totalPower string, sinceSubmission time.Duration) (DecisionPolicyResult, error) + Allow(tallyResult TallyResult, totalPower string) (DecisionPolicyResult, error) ValidateBasic() error Validate(g GroupInfo, config Config) error @@ -52,6 +55,10 @@ func (p ThresholdDecisionPolicy) GetVotingPeriod() time.Duration { return p.Windows.VotingPeriod } +func (p ThresholdDecisionPolicy) GetMinExecutionPeriod() time.Duration { + return p.Windows.MinExecutionPeriod +} + func (p ThresholdDecisionPolicy) ValidateBasic() error { if _, err := math.NewPositiveDecFromString(p.Threshold); err != nil { return sdkerrors.Wrap(err, "threshold") @@ -65,7 +72,7 @@ func (p ThresholdDecisionPolicy) ValidateBasic() error { } // Allow allows a proposal to pass when the tally of yes votes equals or exceeds the threshold before the timeout. -func (p ThresholdDecisionPolicy) Allow(tallyResult TallyResult, totalPower string, sinceSubmission time.Duration) (DecisionPolicyResult, error) { +func (p ThresholdDecisionPolicy) Allow(tallyResult TallyResult, totalPower string) (DecisionPolicyResult, error) { threshold, err := math.NewPositiveDecFromString(p.Threshold) if err != nil { return DecisionPolicyResult{}, sdkerrors.Wrap(err, "threshold") @@ -150,6 +157,10 @@ func (p PercentageDecisionPolicy) GetVotingPeriod() time.Duration { return p.Windows.VotingPeriod } +func (p PercentageDecisionPolicy) GetMinExecutionPeriod() time.Duration { + return p.Windows.MinExecutionPeriod +} + func (p PercentageDecisionPolicy) ValidateBasic() error { percentage, err := math.NewPositiveDecFromString(p.Percentage) if err != nil { @@ -174,7 +185,7 @@ func (p *PercentageDecisionPolicy) Validate(g GroupInfo, config Config) error { } // Allow allows a proposal to pass when the tally of yes votes equals or exceeds the percentage threshold before the timeout. -func (p PercentageDecisionPolicy) Allow(tally TallyResult, totalPower string, sinceSubmission time.Duration) (DecisionPolicyResult, error) { +func (p PercentageDecisionPolicy) Allow(tally TallyResult, totalPower string) (DecisionPolicyResult, error) { percentage, err := math.NewPositiveDecFromString(p.Percentage) if err != nil { return DecisionPolicyResult{}, sdkerrors.Wrap(err, "percentage") From c9c0b014b95817067ee41d9aa55b59c09db81aad Mon Sep 17 00:00:00 2001 From: Amaury M <1293565+amaurym@users.noreply.github.com> Date: Tue, 15 Nov 2022 19:21:54 +0100 Subject: [PATCH 03/11] Fix TestExec --- x/group/keeper/proposal_executor.go | 2 +- x/group/types_test.go | 47 ++--------------------------- 2 files changed, 3 insertions(+), 46 deletions(-) diff --git a/x/group/keeper/proposal_executor.go b/x/group/keeper/proposal_executor.go index 3bf6f9102340..4ae0b305179f 100644 --- a/x/group/keeper/proposal_executor.go +++ b/x/group/keeper/proposal_executor.go @@ -16,7 +16,7 @@ func (s Keeper) doExecuteMsgs(ctx sdk.Context, router *baseapp.MsgServiceRouter, // Ensure it's not too early to execute the messages. minExecutionDate := proposal.SubmitTime.Add(decisionPolicy.GetMinExecutionPeriod()) if ctx.BlockTime().Before(minExecutionDate) { - return nil, errors.ErrInvalid.Wrapf("must wait until %s to execute proposal", minExecutionDate) + return nil, errors.ErrInvalid.Wrapf("must wait until %s to execute proposal %d", minExecutionDate, proposal.Id) } // Ensure it's not too late to execute the messages. diff --git a/x/group/types_test.go b/x/group/types_test.go index db40b238433f..0bf8f2e70f06 100644 --- a/x/group/types_test.go +++ b/x/group/types_test.go @@ -239,30 +239,10 @@ func TestPercentageDecisionPolicyAllow(t *testing.T) { }, false, }, - { - "time since submission < min execution period", - &group.PercentageDecisionPolicy{ - Percentage: "0.5", - Windows: &group.DecisionPolicyWindows{ - VotingPeriod: time.Second * 10, - MinExecutionPeriod: time.Minute, - }, - }, - &group.TallyResult{ - YesCount: "2", - NoCount: "0", - AbstainCount: "0", - NoWithVetoCount: "0", - }, - "3", - time.Duration(time.Second * 50), - group.DecisionPolicyResult{}, - true, - }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - policyResult, err := tc.policy.Allow(*tc.tally, tc.totalPower, tc.votingDuration) + policyResult, err := tc.policy.Allow(*tc.tally, tc.totalPower) if tc.expErr { require.Error(t, err) } else { @@ -393,33 +373,10 @@ func TestThresholdDecisionPolicyAllow(t *testing.T) { }, false, }, - { - "time since submission < min execution period", - &group.ThresholdDecisionPolicy{ - Threshold: "3", - Windows: &group.DecisionPolicyWindows{ - VotingPeriod: time.Second * 10, - MinExecutionPeriod: time.Minute, - }, - }, - &group.TallyResult{ - YesCount: "3", - NoCount: "0", - AbstainCount: "0", - NoWithVetoCount: "0", - }, - "3", - time.Duration(time.Second * 50), - group.DecisionPolicyResult{ - Allow: false, - Final: true, - }, - true, - }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - policyResult, err := tc.policy.Allow(*tc.tally, tc.totalPower, tc.votingDuration) + policyResult, err := tc.policy.Allow(*tc.tally, tc.totalPower) if tc.expErr { require.Error(t, err) } else { From 6e0e7e8f5669f3bdc206037e1adddde3f9a1b876 Mon Sep 17 00:00:00 2001 From: Amaury M <1293565+amaurym@users.noreply.github.com> Date: Tue, 15 Nov 2022 19:22:30 +0100 Subject: [PATCH 04/11] Fix TestExec --- x/group/keeper/keeper_test.go | 103 ++++++++++++++++++++++++++++++++-- 1 file changed, 97 insertions(+), 6 deletions(-) diff --git a/x/group/keeper/keeper_test.go b/x/group/keeper/keeper_test.go index a67390083793..794441bd6254 100644 --- a/x/group/keeper/keeper_test.go +++ b/x/group/keeper/keeper_test.go @@ -98,7 +98,7 @@ func (s *TestSuite) SetupTest() { policy := group.NewThresholdDecisionPolicy( "2", time.Second, - 0, + 5*time.Second, // Must wait 5 seconds before executing proposal ) policyReq := &group.MsgCreateGroupPolicy{ Admin: s.addrs[0].String(), @@ -2379,6 +2379,7 @@ func (s *TestSuite) TestExecProposal() { msgs := []sdk.Msg{msgSend1} return submitProposalAndVote(ctx, s, msgs, proposers, group.VOTE_OPTION_YES) }, + srcBlockTime: s.blockTime.Add(6 * time.Second), // After min execution period end expProposalStatus: group.PROPOSAL_STATUS_ACCEPTED, expExecutorResult: group.PROPOSAL_EXECUTOR_RESULT_SUCCESS, expBalance: true, @@ -2392,6 +2393,7 @@ func (s *TestSuite) TestExecProposal() { return submitProposalAndVote(ctx, s, msgs, proposers, group.VOTE_OPTION_YES) }, + srcBlockTime: s.blockTime.Add(6 * time.Second), // After min execution period end expProposalStatus: group.PROPOSAL_STATUS_ACCEPTED, expExecutorResult: group.PROPOSAL_EXECUTOR_RESULT_SUCCESS, expBalance: true, @@ -2403,6 +2405,7 @@ func (s *TestSuite) TestExecProposal() { msgs := []sdk.Msg{msgSend1} return submitProposalAndVote(ctx, s, msgs, proposers, group.VOTE_OPTION_NO) }, + srcBlockTime: s.blockTime.Add(6 * time.Second), // After min execution period end expProposalStatus: group.PROPOSAL_STATUS_REJECTED, expExecutorResult: group.PROPOSAL_EXECUTOR_RESULT_NOT_RUN, }, @@ -2419,34 +2422,59 @@ func (s *TestSuite) TestExecProposal() { }, expErr: true, }, - "Decision policy also applied on timeout": { + "Decision policy also applied on exactly voting period end": { setupProposal: func(ctx context.Context) uint64 { msgs := []sdk.Msg{msgSend1} return submitProposalAndVote(ctx, s, msgs, proposers, group.VOTE_OPTION_NO) }, - srcBlockTime: s.blockTime.Add(time.Second), + srcBlockTime: s.blockTime.Add(time.Second), // Voting period is 1s expProposalStatus: group.PROPOSAL_STATUS_REJECTED, expExecutorResult: group.PROPOSAL_EXECUTOR_RESULT_NOT_RUN, }, - "Decision policy also applied after timeout": { + "Decision policy also applied after voting period end": { setupProposal: func(ctx context.Context) uint64 { msgs := []sdk.Msg{msgSend1} return submitProposalAndVote(ctx, s, msgs, proposers, group.VOTE_OPTION_NO) }, - srcBlockTime: s.blockTime.Add(time.Second).Add(time.Millisecond), + srcBlockTime: s.blockTime.Add(time.Second).Add(time.Millisecond), // Voting period is 1s expProposalStatus: group.PROPOSAL_STATUS_REJECTED, expExecutorResult: group.PROPOSAL_EXECUTOR_RESULT_NOT_RUN, }, + "exec proposal before MinExecutionPeriod should fail": { + setupProposal: func(ctx context.Context) uint64 { + msgs := []sdk.Msg{msgSend1} + return submitProposalAndVote(ctx, s, msgs, proposers, group.VOTE_OPTION_YES) + }, + srcBlockTime: s.blockTime.Add(4 * time.Second), // min execution date is 5s later after s.blockTime + expProposalStatus: group.PROPOSAL_STATUS_ACCEPTED, + expExecutorResult: group.PROPOSAL_EXECUTOR_RESULT_FAILURE, // Because MinExecutionPeriod has not passed + }, + "exec proposal at exactly MinExecutionPeriod should pass": { + setupProposal: func(ctx context.Context) uint64 { + msgs := []sdk.Msg{msgSend1} + s.bankKeeper.EXPECT().Send(gomock.Any(), msgSend1).Return(nil, nil) + return submitProposalAndVote(ctx, s, msgs, proposers, group.VOTE_OPTION_YES) + }, + srcBlockTime: s.blockTime.Add(5 * time.Second), // min execution date is 5s later after s.blockTime + expProposalStatus: group.PROPOSAL_STATUS_ACCEPTED, + expExecutorResult: group.PROPOSAL_EXECUTOR_RESULT_SUCCESS, + }, "prevent double execution when successful": { setupProposal: func(ctx context.Context) uint64 { myProposalID := submitProposalAndVote(ctx, s, []sdk.Msg{msgSend1}, proposers, group.VOTE_OPTION_YES) s.bankKeeper.EXPECT().Send(gomock.Any(), msgSend1).Return(nil, nil) + // Wait after min execution period end before Exec + sdkCtx := sdk.UnwrapSDKContext(ctx) + sdkCtx = sdkCtx.WithBlockTime(sdkCtx.BlockTime().Add(6 * time.Second)) // MinExecutionPeriod is 5s + ctx = sdk.WrapSDKContext(sdkCtx) + _, err := s.groupKeeper.Exec(ctx, &group.MsgExec{Executor: addr1.String(), ProposalId: myProposalID}) s.Require().NoError(err) return myProposalID }, - expErr: true, // since proposal is pruned after a successful MsgExec + srcBlockTime: s.blockTime.Add(6 * time.Second), // After min execution period end + expErr: true, // since proposal is pruned after a successful MsgExec expProposalStatus: group.PROPOSAL_STATUS_ACCEPTED, expExecutorResult: group.PROPOSAL_EXECUTOR_RESULT_SUCCESS, expBalance: true, @@ -2461,6 +2489,7 @@ func (s *TestSuite) TestExecProposal() { return submitProposalAndVote(ctx, s, msgs, proposers, group.VOTE_OPTION_YES) }, + srcBlockTime: s.blockTime.Add(6 * time.Second), // After min execution period end expProposalStatus: group.PROPOSAL_STATUS_ACCEPTED, expExecutorResult: group.PROPOSAL_EXECUTOR_RESULT_FAILURE, }, @@ -2469,6 +2498,11 @@ func (s *TestSuite) TestExecProposal() { msgs := []sdk.Msg{msgSend2} myProposalID := submitProposalAndVote(ctx, s, msgs, proposers, group.VOTE_OPTION_YES) + // Wait after min execution period end before Exec + sdkCtx := sdk.UnwrapSDKContext(ctx) + sdkCtx = sdkCtx.WithBlockTime(sdkCtx.BlockTime().Add(6 * time.Second)) // MinExecutionPeriod is 5s + ctx = sdk.WrapSDKContext(sdkCtx) + s.bankKeeper.EXPECT().Send(gomock.Any(), msgSend2).Return(nil, fmt.Errorf("error")) _, err := s.groupKeeper.Exec(ctx, &group.MsgExec{Executor: addr1.String(), ProposalId: myProposalID}) s.bankKeeper.EXPECT().Send(gomock.Any(), msgSend2).Return(nil, nil) @@ -2478,6 +2512,7 @@ func (s *TestSuite) TestExecProposal() { return myProposalID }, + srcBlockTime: s.blockTime.Add(6 * time.Second), // After min execution period end expProposalStatus: group.PROPOSAL_STATUS_ACCEPTED, expExecutorResult: group.PROPOSAL_EXECUTOR_RESULT_SUCCESS, }, @@ -3153,3 +3188,59 @@ func (s *TestSuite) createGroupAndGroupPolicy( return policyAddr, groupID } + +func (s *TestSuite) TestTallyProposalsAtVPEnd() { + addrs := s.addrs + addr1 := addrs[0] + addr2 := addrs[1] + votingPeriod := time.Duration(4 * time.Minute) + minExecutionPeriod := votingPeriod + group.DefaultConfig().MaxExecutionPeriod + + groupMsg := &group.MsgCreateGroupWithPolicy{ + Admin: addr1.String(), + Members: []group.MemberRequest{ + {Address: addr1.String(), Weight: "1"}, + {Address: addr2.String(), Weight: "1"}, + }, + } + policy := group.NewThresholdDecisionPolicy( + "1", + votingPeriod, + minExecutionPeriod, + ) + s.Require().NoError(groupMsg.SetDecisionPolicy(policy)) + + s.setNextAccount() + groupRes, err := s.groupKeeper.CreateGroupWithPolicy(s.ctx, groupMsg) + s.Require().NoError(err) + accountAddr := groupRes.GetGroupPolicyAddress() + groupPolicy, err := sdk.AccAddressFromBech32(accountAddr) + s.Require().NoError(err) + s.Require().NotNil(groupPolicy) + + proposalRes, err := s.groupKeeper.SubmitProposal(s.ctx, &group.MsgSubmitProposal{ + GroupPolicyAddress: accountAddr, + Proposers: []string{addr1.String()}, + Messages: nil, + }) + s.Require().NoError(err) + + _, err = s.groupKeeper.Vote(s.ctx, &group.MsgVote{ + ProposalId: proposalRes.ProposalId, + Voter: addr1.String(), + Option: group.VOTE_OPTION_YES, + }) + s.Require().NoError(err) + + // move forward in time + ctx := s.sdkCtx.WithBlockTime(s.sdkCtx.BlockTime().Add(votingPeriod + 1)) + + result, err := s.groupKeeper.TallyResult(ctx, &group.QueryTallyResultRequest{ + ProposalId: proposalRes.ProposalId, + }) + s.Require().Equal("1", result.Tally.YesCount) + s.Require().NoError(err) + + s.Require().NoError(s.groupKeeper.TallyProposalsAtVPEnd(ctx)) + s.NotPanics(func() { module.EndBlocker(ctx, s.groupKeeper) }) +} From 36f381ca82885ef8a131c3952e9f8bc3c324ac9e Mon Sep 17 00:00:00 2001 From: Amaury M <1293565+amaurym@users.noreply.github.com> Date: Tue, 15 Nov 2022 19:28:54 +0100 Subject: [PATCH 05/11] test exec pruned --- x/group/keeper/keeper_test.go | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/x/group/keeper/keeper_test.go b/x/group/keeper/keeper_test.go index 794441bd6254..18a35f1ef995 100644 --- a/x/group/keeper/keeper_test.go +++ b/x/group/keeper/keeper_test.go @@ -33,6 +33,8 @@ import ( minttypes "github.com/cosmos/cosmos-sdk/x/mint/types" ) +const minExecutionPeriod = 5 * time.Second + type TestSuite struct { suite.Suite @@ -98,7 +100,7 @@ func (s *TestSuite) SetupTest() { policy := group.NewThresholdDecisionPolicy( "2", time.Second, - 5*time.Second, // Must wait 5 seconds before executing proposal + minExecutionPeriod, // Must wait 5 seconds before executing proposal ) policyReq := &group.MsgCreateGroupPolicy{ Admin: s.addrs[0].String(), @@ -2379,7 +2381,7 @@ func (s *TestSuite) TestExecProposal() { msgs := []sdk.Msg{msgSend1} return submitProposalAndVote(ctx, s, msgs, proposers, group.VOTE_OPTION_YES) }, - srcBlockTime: s.blockTime.Add(6 * time.Second), // After min execution period end + srcBlockTime: s.blockTime.Add(minExecutionPeriod), // After min execution period end expProposalStatus: group.PROPOSAL_STATUS_ACCEPTED, expExecutorResult: group.PROPOSAL_EXECUTOR_RESULT_SUCCESS, expBalance: true, @@ -2393,7 +2395,7 @@ func (s *TestSuite) TestExecProposal() { return submitProposalAndVote(ctx, s, msgs, proposers, group.VOTE_OPTION_YES) }, - srcBlockTime: s.blockTime.Add(6 * time.Second), // After min execution period end + srcBlockTime: s.blockTime.Add(minExecutionPeriod), // After min execution period end expProposalStatus: group.PROPOSAL_STATUS_ACCEPTED, expExecutorResult: group.PROPOSAL_EXECUTOR_RESULT_SUCCESS, expBalance: true, @@ -2405,7 +2407,7 @@ func (s *TestSuite) TestExecProposal() { msgs := []sdk.Msg{msgSend1} return submitProposalAndVote(ctx, s, msgs, proposers, group.VOTE_OPTION_NO) }, - srcBlockTime: s.blockTime.Add(6 * time.Second), // After min execution period end + srcBlockTime: s.blockTime.Add(minExecutionPeriod), // After min execution period end expProposalStatus: group.PROPOSAL_STATUS_REJECTED, expExecutorResult: group.PROPOSAL_EXECUTOR_RESULT_NOT_RUN, }, @@ -2466,15 +2468,15 @@ func (s *TestSuite) TestExecProposal() { // Wait after min execution period end before Exec sdkCtx := sdk.UnwrapSDKContext(ctx) - sdkCtx = sdkCtx.WithBlockTime(sdkCtx.BlockTime().Add(6 * time.Second)) // MinExecutionPeriod is 5s + sdkCtx = sdkCtx.WithBlockTime(sdkCtx.BlockTime().Add(minExecutionPeriod)) // MinExecutionPeriod is 5s ctx = sdk.WrapSDKContext(sdkCtx) _, err := s.groupKeeper.Exec(ctx, &group.MsgExec{Executor: addr1.String(), ProposalId: myProposalID}) s.Require().NoError(err) return myProposalID }, - srcBlockTime: s.blockTime.Add(6 * time.Second), // After min execution period end - expErr: true, // since proposal is pruned after a successful MsgExec + srcBlockTime: s.blockTime.Add(minExecutionPeriod), // After min execution period end + expErr: true, // since proposal is pruned after a successful MsgExec expProposalStatus: group.PROPOSAL_STATUS_ACCEPTED, expExecutorResult: group.PROPOSAL_EXECUTOR_RESULT_SUCCESS, expBalance: true, @@ -2489,7 +2491,7 @@ func (s *TestSuite) TestExecProposal() { return submitProposalAndVote(ctx, s, msgs, proposers, group.VOTE_OPTION_YES) }, - srcBlockTime: s.blockTime.Add(6 * time.Second), // After min execution period end + srcBlockTime: s.blockTime.Add(minExecutionPeriod), // After min execution period end expProposalStatus: group.PROPOSAL_STATUS_ACCEPTED, expExecutorResult: group.PROPOSAL_EXECUTOR_RESULT_FAILURE, }, @@ -2500,7 +2502,7 @@ func (s *TestSuite) TestExecProposal() { // Wait after min execution period end before Exec sdkCtx := sdk.UnwrapSDKContext(ctx) - sdkCtx = sdkCtx.WithBlockTime(sdkCtx.BlockTime().Add(6 * time.Second)) // MinExecutionPeriod is 5s + sdkCtx = sdkCtx.WithBlockTime(sdkCtx.BlockTime().Add(minExecutionPeriod)) // MinExecutionPeriod is 5s ctx = sdk.WrapSDKContext(sdkCtx) s.bankKeeper.EXPECT().Send(gomock.Any(), msgSend2).Return(nil, fmt.Errorf("error")) @@ -2512,7 +2514,7 @@ func (s *TestSuite) TestExecProposal() { return myProposalID }, - srcBlockTime: s.blockTime.Add(6 * time.Second), // After min execution period end + srcBlockTime: s.blockTime.Add(minExecutionPeriod), // After min execution period end expProposalStatus: group.PROPOSAL_STATUS_ACCEPTED, expExecutorResult: group.PROPOSAL_EXECUTOR_RESULT_SUCCESS, }, @@ -2704,6 +2706,11 @@ func (s *TestSuite) TestExecPrunedProposalsAndVotes() { s.bankKeeper.EXPECT().Send(gomock.Any(), msgSend2).Return(nil, fmt.Errorf("error")) + // Wait for min execution period end + sdkCtx := sdk.UnwrapSDKContext(ctx) + sdkCtx = sdkCtx.WithBlockTime(sdkCtx.BlockTime().Add(minExecutionPeriod)) + ctx = sdk.WrapSDKContext(sdkCtx) + _, err := s.groupKeeper.Exec(ctx, &group.MsgExec{Executor: addr1.String(), ProposalId: myProposalID}) s.bankKeeper.EXPECT().Send(gomock.Any(), msgSend2).Return(nil, nil) @@ -2725,6 +2732,8 @@ func (s *TestSuite) TestExecPrunedProposalsAndVotes() { sdkCtx = sdkCtx.WithBlockTime(spec.srcBlockTime) } + // Wait for min execution period end + sdkCtx = sdkCtx.WithBlockTime(sdkCtx.BlockTime().Add(minExecutionPeriod)) ctx = sdk.WrapSDKContext(sdkCtx) _, err := s.groupKeeper.Exec(ctx, &group.MsgExec{Executor: addr1.String(), ProposalId: proposalID}) if spec.expErr { From e6ed07dab713f786d885e75e0e3ff9226ceadee7 Mon Sep 17 00:00:00 2001 From: Amaury M <1293565+amaurym@users.noreply.github.com> Date: Tue, 15 Nov 2022 20:35:36 +0100 Subject: [PATCH 06/11] Fix submitproposal --- x/group/keeper/keeper_test.go | 63 ++++++++++++++++++++--------------- 1 file changed, 37 insertions(+), 26 deletions(-) diff --git a/x/group/keeper/keeper_test.go b/x/group/keeper/keeper_test.go index 18a35f1ef995..3b5b2559531e 100644 --- a/x/group/keeper/keeper_test.go +++ b/x/group/keeper/keeper_test.go @@ -1533,38 +1533,49 @@ func (s *TestSuite) TestGroupPoliciesByAdminOrGroup() { func (s *TestSuite) TestSubmitProposal() { addrs := s.addrs addr1 := addrs[0] - addr2 := addrs[1] + addr2 := addrs[1] // Has weight 2 addr4 := addrs[3] - addr5 := addrs[4] + addr5 := addrs[4] // Has weight 1 myGroupID := s.groupID - accountAddr := s.groupPolicyAddr - - msgSend := &banktypes.MsgSend{ - FromAddress: s.groupPolicyAddr.String(), - ToAddress: addr2.String(), - Amount: sdk.Coins{sdk.NewInt64Coin("test", 100)}, - } + // Create a new group policy to test TRY_EXEC policyReq := &group.MsgCreateGroupPolicy{ Admin: addr1.String(), GroupId: myGroupID, } policy := group.NewThresholdDecisionPolicy( - "100", + "2", time.Second, - 0, + 0, // no MinExecutionPeriod to test TRY_EXEC ) err := policyReq.SetDecisionPolicy(policy) s.Require().NoError(err) + s.setNextAccount() + res, err := s.groupKeeper.CreateGroupPolicy(s.ctx, policyReq) + s.Require().NoError(err) + groupPolicyAddr := sdk.MustAccAddressFromBech32(res.Address) + // Create a new group policy with super high threshold + bigThresholdPolicy := group.NewThresholdDecisionPolicy( + "100", + time.Second, + 0, + ) s.setNextAccount() + err = policyReq.SetDecisionPolicy(bigThresholdPolicy) + s.Require().NoError(err) bigThresholdRes, err := s.groupKeeper.CreateGroupPolicy(s.ctx, policyReq) s.Require().NoError(err) bigThresholdAddr := bigThresholdRes.Address + msgSend := &banktypes.MsgSend{ + FromAddress: groupPolicyAddr.String(), + ToAddress: addr2.String(), + Amount: sdk.Coins{sdk.NewInt64Coin("test", 100)}, + } defaultProposal := group.Proposal{ - GroupPolicyAddress: accountAddr.String(), + GroupPolicyAddress: groupPolicyAddr.String(), Status: group.PROPOSAL_STATUS_SUBMITTED, FinalTallyResult: group.TallyResult{ YesCount: "0", @@ -1584,7 +1595,7 @@ func (s *TestSuite) TestSubmitProposal() { }{ "all good with minimal fields set": { req: &group.MsgSubmitProposal{ - GroupPolicyAddress: accountAddr.String(), + GroupPolicyAddress: groupPolicyAddr.String(), Proposers: []string{addr2.String()}, }, expProposal: defaultProposal, @@ -1592,11 +1603,11 @@ func (s *TestSuite) TestSubmitProposal() { }, "all good with good msg payload": { req: &group.MsgSubmitProposal{ - GroupPolicyAddress: accountAddr.String(), + GroupPolicyAddress: groupPolicyAddr.String(), Proposers: []string{addr2.String()}, }, msgs: []sdk.Msg{&banktypes.MsgSend{ - FromAddress: accountAddr.String(), + FromAddress: groupPolicyAddr.String(), ToAddress: addr2.String(), Amount: sdk.Coins{sdk.NewInt64Coin("token", 100)}, }}, @@ -1605,7 +1616,7 @@ func (s *TestSuite) TestSubmitProposal() { }, "metadata too long": { req: &group.MsgSubmitProposal{ - GroupPolicyAddress: accountAddr.String(), + GroupPolicyAddress: groupPolicyAddr.String(), Proposers: []string{addr2.String()}, Metadata: strings.Repeat("a", 256), }, @@ -1643,7 +1654,7 @@ func (s *TestSuite) TestSubmitProposal() { }, "only group members can create a proposal": { req: &group.MsgSubmitProposal{ - GroupPolicyAddress: accountAddr.String(), + GroupPolicyAddress: groupPolicyAddr.String(), Proposers: []string{addr4.String()}, }, expErr: true, @@ -1651,7 +1662,7 @@ func (s *TestSuite) TestSubmitProposal() { }, "all proposers must be in group": { req: &group.MsgSubmitProposal{ - GroupPolicyAddress: accountAddr.String(), + GroupPolicyAddress: groupPolicyAddr.String(), Proposers: []string{addr2.String(), addr4.String()}, }, expErr: true, @@ -1659,7 +1670,7 @@ func (s *TestSuite) TestSubmitProposal() { }, "admin that is not a group member can not create proposal": { req: &group.MsgSubmitProposal{ - GroupPolicyAddress: accountAddr.String(), + GroupPolicyAddress: groupPolicyAddr.String(), Proposers: []string{addr1.String()}, }, expErr: true, @@ -1667,7 +1678,7 @@ func (s *TestSuite) TestSubmitProposal() { }, "reject msgs that are not authz by group policy": { req: &group.MsgSubmitProposal{ - GroupPolicyAddress: accountAddr.String(), + GroupPolicyAddress: groupPolicyAddr.String(), Proposers: []string{addr2.String()}, }, msgs: []sdk.Msg{&testdata.TestMsg{Signers: []string{addr1.String()}}}, @@ -1681,13 +1692,13 @@ func (s *TestSuite) TestSubmitProposal() { } }, req: &group.MsgSubmitProposal{ - GroupPolicyAddress: accountAddr.String(), + GroupPolicyAddress: groupPolicyAddr.String(), Proposers: []string{addr2.String()}, Exec: group.Exec_EXEC_TRY, }, msgs: []sdk.Msg{msgSend}, expProposal: group.Proposal{ - GroupPolicyAddress: accountAddr.String(), + GroupPolicyAddress: groupPolicyAddr.String(), Status: group.PROPOSAL_STATUS_ACCEPTED, FinalTallyResult: group.TallyResult{ YesCount: "2", @@ -1698,10 +1709,10 @@ func (s *TestSuite) TestSubmitProposal() { ExecutorResult: group.PROPOSAL_EXECUTOR_RESULT_SUCCESS, }, postRun: func(sdkCtx sdk.Context) { - s.bankKeeper.EXPECT().GetAllBalances(sdkCtx, accountAddr).Return(sdk.NewCoins(sdk.NewInt64Coin("test", 9900))) + s.bankKeeper.EXPECT().GetAllBalances(sdkCtx, groupPolicyAddr).Return(sdk.NewCoins(sdk.NewInt64Coin("test", 9900))) s.bankKeeper.EXPECT().GetAllBalances(sdkCtx, addr2).Return(sdk.NewCoins(sdk.NewInt64Coin("test", 100))) - fromBalances := s.bankKeeper.GetAllBalances(sdkCtx, accountAddr) + fromBalances := s.bankKeeper.GetAllBalances(sdkCtx, groupPolicyAddr) s.Require().Contains(fromBalances, sdk.NewInt64Coin("test", 9900)) toBalances := s.bankKeeper.GetAllBalances(sdkCtx, addr2) s.Require().Contains(toBalances, sdk.NewInt64Coin("test", 100)) @@ -1709,13 +1720,13 @@ func (s *TestSuite) TestSubmitProposal() { }, "with try exec, not enough yes votes for proposal to pass": { req: &group.MsgSubmitProposal{ - GroupPolicyAddress: accountAddr.String(), + GroupPolicyAddress: groupPolicyAddr.String(), Proposers: []string{addr5.String()}, Exec: group.Exec_EXEC_TRY, }, msgs: []sdk.Msg{msgSend}, expProposal: group.Proposal{ - GroupPolicyAddress: accountAddr.String(), + GroupPolicyAddress: groupPolicyAddr.String(), Status: group.PROPOSAL_STATUS_SUBMITTED, FinalTallyResult: group.TallyResult{ YesCount: "0", // Since tally doesn't pass Allow(), we consider the proposal not final From 130a0e477bdd92eaa724a6bec37f404adc9158de Mon Sep 17 00:00:00 2001 From: Amaury M <1293565+amaurym@users.noreply.github.com> Date: Tue, 15 Nov 2022 21:30:19 +0100 Subject: [PATCH 07/11] Add changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 817a82908812..262dd0e0c347 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -114,6 +114,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (x/gov) [#12771](https://github.com/cosmos/cosmos-sdk/pull/12771) Initial deposit requirement for proposals at submission time. * (x/staking) [#12967](https://github.com/cosmos/cosmos-sdk/pull/12967) `unbond` now creates only one unbonding delegation entry when multiple unbondings exist at a single height (e.g. through multiple messages in a transaction). * (x/auth/vesting) [#13502](https://github.com/cosmos/cosmos-sdk/pull/13502) Add Amino Msg registration for `MsgCreatePeriodicVestingAccount`. +* (x/group) [#13876](https://github.com/cosmos/cosmos-sdk/pull/13876) Fix group MinExecutionPeriod that is checked on execution now, instead on voting period end. ### API Breaking Changes @@ -168,6 +169,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * [#13794](https://github.com/cosmos/cosmos-sdk/pull/13794) Most methods on `types/module.AppModule` have been moved to extension interfaces. `module.Manager.Modules` is now of type `map[string]interface{}` to support in parallel the new `cosmossdk.io/core/appmodule.AppModule` API. +* (x/group) [#13876](https://github.com/cosmos/cosmos-sdk/pull/13876) Add `GetMinExecutionPeriod` method on DecisionPolicy interface. ### CLI Breaking Changes From 83a91f846e3dc080fe7b77916d4c2c29201e48d9 Mon Sep 17 00:00:00 2001 From: Amaury M <1293565+amaurym@users.noreply.github.com> Date: Tue, 15 Nov 2022 21:35:32 +0100 Subject: [PATCH 08/11] typo --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 262dd0e0c347..3dac739557a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -114,7 +114,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (x/gov) [#12771](https://github.com/cosmos/cosmos-sdk/pull/12771) Initial deposit requirement for proposals at submission time. * (x/staking) [#12967](https://github.com/cosmos/cosmos-sdk/pull/12967) `unbond` now creates only one unbonding delegation entry when multiple unbondings exist at a single height (e.g. through multiple messages in a transaction). * (x/auth/vesting) [#13502](https://github.com/cosmos/cosmos-sdk/pull/13502) Add Amino Msg registration for `MsgCreatePeriodicVestingAccount`. -* (x/group) [#13876](https://github.com/cosmos/cosmos-sdk/pull/13876) Fix group MinExecutionPeriod that is checked on execution now, instead on voting period end. +* (x/group) [#13876](https://github.com/cosmos/cosmos-sdk/pull/13876) Fix group MinExecutionPeriod that is checked on execution now, instead of voting period end. ### API Breaking Changes From b747bf553da161124dec387347eee887f003ecb3 Mon Sep 17 00:00:00 2001 From: Amaury M <1293565+amaurym@users.noreply.github.com> Date: Tue, 15 Nov 2022 21:42:37 +0100 Subject: [PATCH 09/11] revert some changes --- x/group/keeper/keeper_test.go | 39 ++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/x/group/keeper/keeper_test.go b/x/group/keeper/keeper_test.go index 3b5b2559531e..73d49e75c059 100644 --- a/x/group/keeper/keeper_test.go +++ b/x/group/keeper/keeper_test.go @@ -1538,23 +1538,24 @@ func (s *TestSuite) TestSubmitProposal() { addr5 := addrs[4] // Has weight 1 myGroupID := s.groupID + accountAddr := s.groupPolicyAddr // Create a new group policy to test TRY_EXEC policyReq := &group.MsgCreateGroupPolicy{ Admin: addr1.String(), GroupId: myGroupID, } - policy := group.NewThresholdDecisionPolicy( + noMinExecPeriodPolicy := group.NewThresholdDecisionPolicy( "2", time.Second, 0, // no MinExecutionPeriod to test TRY_EXEC ) - err := policyReq.SetDecisionPolicy(policy) + err := policyReq.SetDecisionPolicy(noMinExecPeriodPolicy) s.Require().NoError(err) s.setNextAccount() res, err := s.groupKeeper.CreateGroupPolicy(s.ctx, policyReq) s.Require().NoError(err) - groupPolicyAddr := sdk.MustAccAddressFromBech32(res.Address) + noMinExecPeriodPolicyAddr := sdk.MustAccAddressFromBech32(res.Address) // Create a new group policy with super high threshold bigThresholdPolicy := group.NewThresholdDecisionPolicy( @@ -1570,12 +1571,12 @@ func (s *TestSuite) TestSubmitProposal() { bigThresholdAddr := bigThresholdRes.Address msgSend := &banktypes.MsgSend{ - FromAddress: groupPolicyAddr.String(), + FromAddress: noMinExecPeriodPolicyAddr.String(), ToAddress: addr2.String(), Amount: sdk.Coins{sdk.NewInt64Coin("test", 100)}, } defaultProposal := group.Proposal{ - GroupPolicyAddress: groupPolicyAddr.String(), + GroupPolicyAddress: accountAddr.String(), Status: group.PROPOSAL_STATUS_SUBMITTED, FinalTallyResult: group.TallyResult{ YesCount: "0", @@ -1595,7 +1596,7 @@ func (s *TestSuite) TestSubmitProposal() { }{ "all good with minimal fields set": { req: &group.MsgSubmitProposal{ - GroupPolicyAddress: groupPolicyAddr.String(), + GroupPolicyAddress: accountAddr.String(), Proposers: []string{addr2.String()}, }, expProposal: defaultProposal, @@ -1603,11 +1604,11 @@ func (s *TestSuite) TestSubmitProposal() { }, "all good with good msg payload": { req: &group.MsgSubmitProposal{ - GroupPolicyAddress: groupPolicyAddr.String(), + GroupPolicyAddress: accountAddr.String(), Proposers: []string{addr2.String()}, }, msgs: []sdk.Msg{&banktypes.MsgSend{ - FromAddress: groupPolicyAddr.String(), + FromAddress: accountAddr.String(), ToAddress: addr2.String(), Amount: sdk.Coins{sdk.NewInt64Coin("token", 100)}, }}, @@ -1616,7 +1617,7 @@ func (s *TestSuite) TestSubmitProposal() { }, "metadata too long": { req: &group.MsgSubmitProposal{ - GroupPolicyAddress: groupPolicyAddr.String(), + GroupPolicyAddress: accountAddr.String(), Proposers: []string{addr2.String()}, Metadata: strings.Repeat("a", 256), }, @@ -1654,7 +1655,7 @@ func (s *TestSuite) TestSubmitProposal() { }, "only group members can create a proposal": { req: &group.MsgSubmitProposal{ - GroupPolicyAddress: groupPolicyAddr.String(), + GroupPolicyAddress: accountAddr.String(), Proposers: []string{addr4.String()}, }, expErr: true, @@ -1662,7 +1663,7 @@ func (s *TestSuite) TestSubmitProposal() { }, "all proposers must be in group": { req: &group.MsgSubmitProposal{ - GroupPolicyAddress: groupPolicyAddr.String(), + GroupPolicyAddress: accountAddr.String(), Proposers: []string{addr2.String(), addr4.String()}, }, expErr: true, @@ -1670,7 +1671,7 @@ func (s *TestSuite) TestSubmitProposal() { }, "admin that is not a group member can not create proposal": { req: &group.MsgSubmitProposal{ - GroupPolicyAddress: groupPolicyAddr.String(), + GroupPolicyAddress: accountAddr.String(), Proposers: []string{addr1.String()}, }, expErr: true, @@ -1678,7 +1679,7 @@ func (s *TestSuite) TestSubmitProposal() { }, "reject msgs that are not authz by group policy": { req: &group.MsgSubmitProposal{ - GroupPolicyAddress: groupPolicyAddr.String(), + GroupPolicyAddress: accountAddr.String(), Proposers: []string{addr2.String()}, }, msgs: []sdk.Msg{&testdata.TestMsg{Signers: []string{addr1.String()}}}, @@ -1692,13 +1693,13 @@ func (s *TestSuite) TestSubmitProposal() { } }, req: &group.MsgSubmitProposal{ - GroupPolicyAddress: groupPolicyAddr.String(), + GroupPolicyAddress: noMinExecPeriodPolicyAddr.String(), Proposers: []string{addr2.String()}, Exec: group.Exec_EXEC_TRY, }, msgs: []sdk.Msg{msgSend}, expProposal: group.Proposal{ - GroupPolicyAddress: groupPolicyAddr.String(), + GroupPolicyAddress: noMinExecPeriodPolicyAddr.String(), Status: group.PROPOSAL_STATUS_ACCEPTED, FinalTallyResult: group.TallyResult{ YesCount: "2", @@ -1709,10 +1710,10 @@ func (s *TestSuite) TestSubmitProposal() { ExecutorResult: group.PROPOSAL_EXECUTOR_RESULT_SUCCESS, }, postRun: func(sdkCtx sdk.Context) { - s.bankKeeper.EXPECT().GetAllBalances(sdkCtx, groupPolicyAddr).Return(sdk.NewCoins(sdk.NewInt64Coin("test", 9900))) + s.bankKeeper.EXPECT().GetAllBalances(sdkCtx, noMinExecPeriodPolicyAddr).Return(sdk.NewCoins(sdk.NewInt64Coin("test", 9900))) s.bankKeeper.EXPECT().GetAllBalances(sdkCtx, addr2).Return(sdk.NewCoins(sdk.NewInt64Coin("test", 100))) - fromBalances := s.bankKeeper.GetAllBalances(sdkCtx, groupPolicyAddr) + fromBalances := s.bankKeeper.GetAllBalances(sdkCtx, noMinExecPeriodPolicyAddr) s.Require().Contains(fromBalances, sdk.NewInt64Coin("test", 9900)) toBalances := s.bankKeeper.GetAllBalances(sdkCtx, addr2) s.Require().Contains(toBalances, sdk.NewInt64Coin("test", 100)) @@ -1720,13 +1721,13 @@ func (s *TestSuite) TestSubmitProposal() { }, "with try exec, not enough yes votes for proposal to pass": { req: &group.MsgSubmitProposal{ - GroupPolicyAddress: groupPolicyAddr.String(), + GroupPolicyAddress: noMinExecPeriodPolicyAddr.String(), Proposers: []string{addr5.String()}, Exec: group.Exec_EXEC_TRY, }, msgs: []sdk.Msg{msgSend}, expProposal: group.Proposal{ - GroupPolicyAddress: groupPolicyAddr.String(), + GroupPolicyAddress: noMinExecPeriodPolicyAddr.String(), Status: group.PROPOSAL_STATUS_SUBMITTED, FinalTallyResult: group.TallyResult{ YesCount: "0", // Since tally doesn't pass Allow(), we consider the proposal not final From df0843d789aeb2ef9a1feb78660ee2f21988d424 Mon Sep 17 00:00:00 2001 From: Amaury M <1293565+amaurym@users.noreply.github.com> Date: Tue, 15 Nov 2022 21:55:59 +0100 Subject: [PATCH 10/11] add minExecutionPeriod --- x/group/keeper/keeper_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/group/keeper/keeper_test.go b/x/group/keeper/keeper_test.go index 73d49e75c059..f4b04b7cdc80 100644 --- a/x/group/keeper/keeper_test.go +++ b/x/group/keeper/keeper_test.go @@ -1561,7 +1561,7 @@ func (s *TestSuite) TestSubmitProposal() { bigThresholdPolicy := group.NewThresholdDecisionPolicy( "100", time.Second, - 0, + minExecutionPeriod, ) s.setNextAccount() err = policyReq.SetDecisionPolicy(bigThresholdPolicy) From fcaaee5365408cfabee2fcd3858489b274a87e25 Mon Sep 17 00:00:00 2001 From: Amaury M <1293565+amaurym@users.noreply.github.com> Date: Wed, 16 Nov 2022 11:24:51 +0100 Subject: [PATCH 11/11] Add docs and specs --- x/group/README.md | 13 +++++++++++++ x/group/types.go | 3 ++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/x/group/README.md b/x/group/README.md index 802f438872c7..edbbc3486775 100644 --- a/x/group/README.md +++ b/x/group/README.md @@ -109,6 +109,16 @@ A threshold decision policy defines a threshold of yes votes (based on a tally of voter weights) that must be achieved in order for a proposal to pass. For this decision policy, abstain and veto are simply treated as no's. +This decision policy also has a VotingPeriod window and a MinExecutionPeriod +window. The former defines the duration after proposal submission where members +are allowed to vote, after which tallying is performed. The latter specifies +the minimum duration after proposal submission where the proposal can be +executed. If set to 0, then the proposal is allowed to be executed immediately +on submission (using the `TRY_EXEC` option). Obviously, MinExecutionPeriod +cannot be greater than VotingPeriod+MaxExecutionPeriod (where MaxExecution is +the app-defined duration that specifies the window after voting ended where a +proposal can be executed). + #### Percentage decision policy A percentage decision policy is similar to a threshold decision policy, except @@ -117,6 +127,9 @@ It's more suited for groups where the group members' weights can be updated, as the percentage threshold stays the same, and doesn't depend on how those member weights get updated. +Same as the Threshold decision policy, the percentage decision policy has the +two VotingPeriod and MinExecutionPeriod parameters. + ### Proposal Any member(s) of a group can submit a proposal for a group policy account to decide upon. diff --git a/x/group/types.go b/x/group/types.go index c1ae21fe907f..49798968af46 100644 --- a/x/group/types.go +++ b/x/group/types.go @@ -32,7 +32,8 @@ type DecisionPolicy interface { // votes are accepted. GetVotingPeriod() time.Duration // GetMinExecutionPeriod returns the minimum duration after submission - // where we can execution a proposal. + // where we can execution a proposal. It can be set to 0 or to a value + // lesser than VotingPeriod to allow TRY_EXEC. GetMinExecutionPeriod() time.Duration // Allow defines policy-specific logic to allow a proposal to pass or not, // based on its tally result, the group's total power and the time since