LaconicNetwork/cosmos-sdk
7
0
Fork 0
Code Issues Pull Requests Packages Projects Releases 2 Wiki Activity

refactor(group): Distinguish Voting period and Execution period for group policies (#11198)

Browse Source 
## Description

Closes: #11092

## TODOs

I'm thinking to do the 2 todos in a separate PR, or else this PR is too big. WDYT?

- [ ] #11246 This involves adding a new index ProposalsByVotingPeriodEnd, so might be better to do in another PR
- [ ] #11245  Also should be done in a separate PR (as it needs the above index)

### Main change 1: Group policy proto defs have `voting_period` and `min_execution_period`

For group policies:

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

+  // voting_period is the duration from submission of a proposal to the end of voting period
+  // Within this times votes can be submitted with MsgVote.
+  google.protobuf.Duration voting_period = 2 [(gogoproto.stdduration) = true, (gogoproto.nullable) = false];

+  // min_execution_period is the minimum duration after the proposal submission
+  // where members can start sending MsgExec. This means that the window for
+  // sending a MsgExec transaction is:
+  // `[ submission + min_execution_period ; submission + voting_period + max_execution_period]`
+  // where max_execution_period is a app-specific config, defined in the keeper.
+  // If not set, min_execution_period will default to 0.
+  google.protobuf.Duration min_execution_period = 3 [(gogoproto.stdduration) = true, (gogoproto.nullable) = false];
```

### Main Change 2: We don't update proposal's FinalTallyResult result on MsgVote/MsgSubmitProposal

Unless the msg has TryExec set to true, in which case the FinalTallyResult is updated ONLY if the tally is final.

### Main Change 3: Add a keeper-level `MaxExecutionPeriod`

MsgExecs will be rejected if they are sent after `voting_period_end + MaxExecutionPeriod`



---

### 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)
This commit is contained in:
Amaury
2022-03-02 12:00:59 +00:00
committed by GitHub
parent 26c9a2d06d
commit da36c46f3a
27 changed files with 1937 additions and 743 deletions
Download Patch File Download Diff File Expand all files Collapse all files
x/group/keeper/config.go
-14
View File
@@ -1,14 +0,0 @@
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,
}
}
x/group/keeper/genesis_test.go
+6 -4
View File
@@ -73,7 +73,9 @@ func (s *GenesisTestSuite) TestInitExportGenesis() {
}
err := groupPolicy.SetDecisionPolicy(&group.ThresholdDecisionPolicy{
Threshold: "1",
Timeout: time.Second,
Windows: &group.DecisionPolicyWindows{
VotingPeriod: time.Second,
},
})
s.Require().NoError(err)
@@ -95,8 +97,8 @@ func (s *GenesisTestSuite) TestInitExportGenesis() {
AbstainCount: "0",
NoWithVetoCount: "0",
},
Timeout: timeout,
ExecutorResult: group.PROPOSAL_EXECUTOR_RESULT_SUCCESS,
VotingPeriodEnd: timeout,
ExecutorResult: group.PROPOSAL_EXECUTOR_RESULT_SUCCESS,
}
err = proposal.SetMsgs([]sdk.Msg{&banktypes.MsgSend{
FromAddress: accAddr.String(),
@@ -216,7 +218,7 @@ func (s *GenesisTestSuite) assertProposalsEqual(g *group.Proposal, other *group.
require.Equal(g.Status, other.Status)
require.Equal(g.Result, other.Result)
require.Equal(g.FinalTallyResult, other.FinalTallyResult)
require.Equal(g.Timeout, other.Timeout)
require.Equal(g.VotingPeriodEnd, other.VotingPeriodEnd)
require.Equal(g.ExecutorResult, other.ExecutorResult)
require.Equal(g.GetMsgs(), other.GetMsgs())
}
x/group/keeper/grpc_query.go
+53
View File
@@ -10,6 +10,7 @@ import (
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
"github.com/cosmos/cosmos-sdk/types/query"
"github.com/cosmos/cosmos-sdk/x/group"
"github.com/cosmos/cosmos-sdk/x/group/errors"
"github.com/cosmos/cosmos-sdk/x/group/internal/orm"
)
@@ -303,3 +304,55 @@ func (q Keeper) getVotesByProposal(ctx sdk.Context, proposalID uint64, pageReque
func (q Keeper) getVotesByVoter(ctx sdk.Context, voter sdk.AccAddress, pageRequest *query.PageRequest) (orm.Iterator, error) {
return q.voteByVoterIndex.GetPaginated(ctx.KVStore(q.key), voter.Bytes(), pageRequest)
}
// Tally is a function that tallies a proposal by iterating through its votes,
// and returns the tally result without modifying the proposal or any state.
// TODO Merge with https://github.com/cosmos/cosmos-sdk/issues/11151
func (q Keeper) Tally(ctx sdk.Context, p group.Proposal, groupId uint64) (group.TallyResult, error) {
// If proposal has already been tallied and updated, then its status is
// closed, in which case we just return the previously stored result.
if p.Status == group.PROPOSAL_STATUS_CLOSED {
return p.FinalTallyResult, nil
}
it, err := q.voteByProposalIndex.Get(ctx.KVStore(q.key), p.Id)
if err != nil {
return group.TallyResult{}, err
}
defer it.Close()
tallyResult := group.DefaultTallyResult()
var vote group.Vote
for {
_, err = it.LoadNext(&vote)
if errors.ErrORMIteratorDone.Is(err) {
break
}
if err != nil {
return group.TallyResult{}, err
}
var member group.GroupMember
err := q.groupMemberTable.GetOne(ctx.KVStore(q.key), orm.PrimaryKey(&group.GroupMember{
GroupId: groupId,
Member: &group.Member{Address: vote.Voter},
}), &member)
switch {
case sdkerrors.ErrNotFound.Is(err):
// If the member left the group after voting, then we simply skip the
// vote.
continue
case err != nil:
// For any other errors, we stop and return the error.
return group.TallyResult{}, err
}
if err := tallyResult.Add(vote, member.Member.Weight); err != nil {
return group.TallyResult{}, sdkerrors.Wrap(err, "add new vote")
}
}
return tallyResult, nil
}
x/group/keeper/invariants_test.go
+17 -18
View File
@@ -25,10 +25,9 @@ import (
type invariantTestSuite struct {
suite.Suite
ctx sdk.Context
cdc *codec.ProtoCodec
key *storetypes.KVStoreKey
blockTime time.Time
ctx sdk.Context
cdc *codec.ProtoCodec
key *storetypes.KVStoreKey
}
func TestInvariantTestSuite(t *testing.T) {
@@ -81,7 +80,7 @@ func (s *invariantTestSuite) TestTallyVotesInvariant() {
Status: group.PROPOSAL_STATUS_SUBMITTED,
Result: group.PROPOSAL_RESULT_UNFINALIZED,
FinalTallyResult: group.TallyResult{YesCount: "1", NoCount: "0", AbstainCount: "0", NoWithVetoCount: "0"},
Timeout: prevCtx.BlockTime().Add(time.Second * 600),
VotingPeriodEnd: prevCtx.BlockTime().Add(time.Second * 600),
ExecutorResult: group.PROPOSAL_EXECUTOR_RESULT_NOT_RUN,
},
@@ -95,7 +94,7 @@ func (s *invariantTestSuite) TestTallyVotesInvariant() {
Status: group.PROPOSAL_STATUS_SUBMITTED,
Result: group.PROPOSAL_RESULT_UNFINALIZED,
FinalTallyResult: group.TallyResult{YesCount: "2", NoCount: "0", AbstainCount: "0", NoWithVetoCount: "0"},
Timeout: curCtx.BlockTime().Add(time.Second * 600),
VotingPeriodEnd: curCtx.BlockTime().Add(time.Second * 600),
ExecutorResult: group.PROPOSAL_EXECUTOR_RESULT_NOT_RUN,
},
},
@@ -110,7 +109,7 @@ func (s *invariantTestSuite) TestTallyVotesInvariant() {
Status: group.PROPOSAL_STATUS_SUBMITTED,
Result: group.PROPOSAL_RESULT_UNFINALIZED,
FinalTallyResult: group.TallyResult{YesCount: "2", NoCount: "0", AbstainCount: "0", NoWithVetoCount: "0"},
Timeout: prevCtx.BlockTime().Add(time.Second * 600),
VotingPeriodEnd: prevCtx.BlockTime().Add(time.Second * 600),
ExecutorResult: group.PROPOSAL_EXECUTOR_RESULT_NOT_RUN,
},
curProposal: &group.Proposal{
@@ -123,7 +122,7 @@ func (s *invariantTestSuite) TestTallyVotesInvariant() {
Status: group.PROPOSAL_STATUS_SUBMITTED,
Result: group.PROPOSAL_RESULT_UNFINALIZED,
FinalTallyResult: group.TallyResult{YesCount: "1", NoCount: "0", AbstainCount: "0", NoWithVetoCount: "0"},
Timeout: curCtx.BlockTime().Add(time.Second * 600),
VotingPeriodEnd: curCtx.BlockTime().Add(time.Second * 600),
ExecutorResult: group.PROPOSAL_EXECUTOR_RESULT_NOT_RUN,
},
expBroken: true,
@@ -139,7 +138,7 @@ func (s *invariantTestSuite) TestTallyVotesInvariant() {
Status: group.PROPOSAL_STATUS_SUBMITTED,
Result: group.PROPOSAL_RESULT_UNFINALIZED,
FinalTallyResult: group.TallyResult{YesCount: "0", NoCount: "2", AbstainCount: "0", NoWithVetoCount: "0"},
Timeout: prevCtx.BlockTime().Add(time.Second * 600),
VotingPeriodEnd: prevCtx.BlockTime().Add(time.Second * 600),
ExecutorResult: group.PROPOSAL_EXECUTOR_RESULT_NOT_RUN,
},
curProposal: &group.Proposal{
@@ -152,7 +151,7 @@ func (s *invariantTestSuite) TestTallyVotesInvariant() {
Status: group.PROPOSAL_STATUS_SUBMITTED,
Result: group.PROPOSAL_RESULT_UNFINALIZED,
FinalTallyResult: group.TallyResult{YesCount: "0", NoCount: "1", AbstainCount: "0", NoWithVetoCount: "0"},
Timeout: curCtx.BlockTime().Add(time.Second * 600),
VotingPeriodEnd: curCtx.BlockTime().Add(time.Second * 600),
ExecutorResult: group.PROPOSAL_EXECUTOR_RESULT_NOT_RUN,
},
expBroken: true,
@@ -168,7 +167,7 @@ func (s *invariantTestSuite) TestTallyVotesInvariant() {
Status: group.PROPOSAL_STATUS_SUBMITTED,
Result: group.PROPOSAL_RESULT_UNFINALIZED,
FinalTallyResult: group.TallyResult{YesCount: "0", NoCount: "0", AbstainCount: "2", NoWithVetoCount: "0"},
Timeout: prevCtx.BlockTime().Add(time.Second * 600),
VotingPeriodEnd: prevCtx.BlockTime().Add(time.Second * 600),
ExecutorResult: group.PROPOSAL_EXECUTOR_RESULT_NOT_RUN,
},
curProposal: &group.Proposal{
@@ -181,7 +180,7 @@ func (s *invariantTestSuite) TestTallyVotesInvariant() {
Status: group.PROPOSAL_STATUS_SUBMITTED,
Result: group.PROPOSAL_RESULT_UNFINALIZED,
FinalTallyResult: group.TallyResult{YesCount: "0", NoCount: "0", AbstainCount: "1", NoWithVetoCount: "0"},
Timeout: curCtx.BlockTime().Add(time.Second * 600),
VotingPeriodEnd: curCtx.BlockTime().Add(time.Second * 600),
ExecutorResult: group.PROPOSAL_EXECUTOR_RESULT_NOT_RUN,
},
expBroken: true,
@@ -197,7 +196,7 @@ func (s *invariantTestSuite) TestTallyVotesInvariant() {
Status: group.PROPOSAL_STATUS_SUBMITTED,
Result: group.PROPOSAL_RESULT_UNFINALIZED,
FinalTallyResult: group.TallyResult{YesCount: "0", NoCount: "0", AbstainCount: "0", NoWithVetoCount: "2"},
Timeout: prevCtx.BlockTime().Add(time.Second * 600),
VotingPeriodEnd: prevCtx.BlockTime().Add(time.Second * 600),
ExecutorResult: group.PROPOSAL_EXECUTOR_RESULT_NOT_RUN,
},
curProposal: &group.Proposal{
@@ -210,7 +209,7 @@ func (s *invariantTestSuite) TestTallyVotesInvariant() {
Status: group.PROPOSAL_STATUS_SUBMITTED,
Result: group.PROPOSAL_RESULT_UNFINALIZED,
FinalTallyResult: group.TallyResult{YesCount: "0", NoCount: "0", AbstainCount: "0", NoWithVetoCount: "1"},
Timeout: curCtx.BlockTime().Add(time.Second * 600),
VotingPeriodEnd: curCtx.BlockTime().Add(time.Second * 600),
ExecutorResult: group.PROPOSAL_EXECUTOR_RESULT_NOT_RUN,
},
expBroken: true,
@@ -413,7 +412,7 @@ func (s *invariantTestSuite) TestTallyVotesSumInvariant() {
Status: group.PROPOSAL_STATUS_SUBMITTED,
Result: group.PROPOSAL_RESULT_UNFINALIZED,
FinalTallyResult: group.TallyResult{YesCount: "4", NoCount: "3", AbstainCount: "0", NoWithVetoCount: "0"},
Timeout: curCtx.BlockTime().Add(time.Second * 600),
VotingPeriodEnd: curCtx.BlockTime().Add(time.Second * 600),
ExecutorResult: group.PROPOSAL_EXECUTOR_RESULT_NOT_RUN,
},
votes: []*group.Vote{
@@ -471,7 +470,7 @@ func (s *invariantTestSuite) TestTallyVotesSumInvariant() {
Status: group.PROPOSAL_STATUS_SUBMITTED,
Result: group.PROPOSAL_RESULT_UNFINALIZED,
FinalTallyResult: group.TallyResult{YesCount: "6", NoCount: "0", AbstainCount: "0", NoWithVetoCount: "0"},
Timeout: curCtx.BlockTime().Add(time.Second * 600),
VotingPeriodEnd: curCtx.BlockTime().Add(time.Second * 600),
ExecutorResult: group.PROPOSAL_EXECUTOR_RESULT_NOT_RUN,
},
votes: []*group.Vote{
@@ -529,7 +528,7 @@ func (s *invariantTestSuite) TestTallyVotesSumInvariant() {
Status: group.PROPOSAL_STATUS_SUBMITTED,
Result: group.PROPOSAL_RESULT_UNFINALIZED,
FinalTallyResult: group.TallyResult{YesCount: "4", NoCount: "3", AbstainCount: "0", NoWithVetoCount: "0"},
Timeout: curCtx.BlockTime().Add(time.Second * 600),
VotingPeriodEnd: curCtx.BlockTime().Add(time.Second * 600),
ExecutorResult: group.PROPOSAL_EXECUTOR_RESULT_NOT_RUN,
},
votes: []*group.Vote{
@@ -561,7 +560,7 @@ func (s *invariantTestSuite) TestTallyVotesSumInvariant() {
_, err := groupTable.Create(cacheCurCtx.KVStore(key), groupsInfo)
s.Require().NoError(err)
err = groupPolicy.SetDecisionPolicy(group.NewThresholdDecisionPolicy("1", time.Second))
err = groupPolicy.SetDecisionPolicy(group.NewThresholdDecisionPolicy("1", time.Second, 0))
s.Require().NoError(err)
err = groupPolicyTable.Create(cacheCurCtx.KVStore(key), groupPolicy)
s.Require().NoError(err)
x/group/keeper/keeper.go
+6 -3
View File
@@ -74,10 +74,10 @@ type Keeper struct {
router *authmiddleware.MsgServiceRouter
config Config
config group.Config
}
func NewKeeper(storeKey storetypes.StoreKey, cdc codec.Codec, router *authmiddleware.MsgServiceRouter, accKeeper group.AccountKeeper, config Config) Keeper {
func NewKeeper(storeKey storetypes.StoreKey, cdc codec.Codec, router *authmiddleware.MsgServiceRouter, accKeeper group.AccountKeeper, config group.Config) Keeper {
k := Keeper{
key: storeKey,
router: router,
@@ -207,7 +207,10 @@ func NewKeeper(storeKey storetypes.StoreKey, cdc codec.Codec, router *authmiddle
k.voteTable = *voteTable
if config.MaxMetadataLen == 0 {
config.MaxMetadataLen = DefaultConfig().MaxMetadataLen
config.MaxMetadataLen = group.DefaultConfig().MaxMetadataLen
}
if config.MaxExecutionPeriod == 0 {
config.MaxExecutionPeriod = group.DefaultConfig().MaxExecutionPeriod
}
k.config = config
x/group/keeper/keeper_test.go
+66 -31
View File
@@ -61,6 +61,7 @@ func (s *TestSuite) SetupTest() {
policy := group.NewThresholdDecisionPolicy(
"2",
time.Second,
0,
)
policyReq := &group.MsgCreateGroupPolicy{
Admin: s.addrs[0].String(),
@@ -693,6 +694,7 @@ func (s *TestSuite) TestCreateGroupWithPolicy() {
policy: group.NewThresholdDecisionPolicy(
"1",
time.Second,
0,
),
},
"group policy as admin is true": {
@@ -704,6 +706,7 @@ func (s *TestSuite) TestCreateGroupWithPolicy() {
policy: group.NewThresholdDecisionPolicy(
"1",
time.Second,
0,
),
},
"group metadata too long": {
@@ -716,6 +719,7 @@ func (s *TestSuite) TestCreateGroupWithPolicy() {
policy: group.NewThresholdDecisionPolicy(
"1",
time.Second,
0,
),
expErr: true,
expErrMsg: "limit exceeded",
@@ -730,6 +734,7 @@ func (s *TestSuite) TestCreateGroupWithPolicy() {
policy: group.NewThresholdDecisionPolicy(
"1",
time.Second,
0,
),
expErr: true,
expErrMsg: "limit exceeded",
@@ -747,6 +752,7 @@ func (s *TestSuite) TestCreateGroupWithPolicy() {
policy: group.NewThresholdDecisionPolicy(
"1",
time.Second,
0,
),
expErr: true,
expErrMsg: "limit exceeded",
@@ -763,6 +769,7 @@ func (s *TestSuite) TestCreateGroupWithPolicy() {
policy: group.NewThresholdDecisionPolicy(
"1",
time.Second,
0,
),
expErr: true,
expErrMsg: "expected a positive decimal",
@@ -776,6 +783,7 @@ func (s *TestSuite) TestCreateGroupWithPolicy() {
policy: group.NewThresholdDecisionPolicy(
"10",
time.Second,
0,
),
expErr: false,
},
@@ -876,6 +884,7 @@ func (s *TestSuite) TestCreateGroupPolicy() {
policy: group.NewThresholdDecisionPolicy(
"1",
time.Second,
0,
),
},
"all good with percentage decision policy": {
@@ -886,6 +895,7 @@ func (s *TestSuite) TestCreateGroupPolicy() {
policy: group.NewPercentageDecisionPolicy(
"0.5",
time.Second,
0,
),
},
"decision policy threshold > total group weight": {
@@ -896,6 +906,7 @@ func (s *TestSuite) TestCreateGroupPolicy() {
policy: group.NewThresholdDecisionPolicy(
"10",
time.Second,
0,
),
},
"group id does not exists": {
@@ -906,6 +917,7 @@ func (s *TestSuite) TestCreateGroupPolicy() {
policy: group.NewThresholdDecisionPolicy(
"1",
time.Second,
0,
),
expErr: true,
expErrMsg: "not found",
@@ -918,6 +930,7 @@ func (s *TestSuite) TestCreateGroupPolicy() {
policy: group.NewThresholdDecisionPolicy(
"1",
time.Second,
0,
),
expErr: true,
expErrMsg: "not group admin",
@@ -931,6 +944,7 @@ func (s *TestSuite) TestCreateGroupPolicy() {
policy: group.NewThresholdDecisionPolicy(
"1",
time.Second,
0,
),
expErr: true,
expErrMsg: "limit exceeded",
@@ -943,6 +957,7 @@ func (s *TestSuite) TestCreateGroupPolicy() {
policy: group.NewPercentageDecisionPolicy(
"-0.5",
time.Second,
0,
),
expErr: true,
expErrMsg: "expected a positive decimal",
@@ -955,6 +970,7 @@ func (s *TestSuite) TestCreateGroupPolicy() {
policy: group.NewPercentageDecisionPolicy(
"2",
time.Second,
0,
),
expErr: true,
expErrMsg: "percentage must be > 0 and <= 1",
@@ -1194,6 +1210,7 @@ func (s *TestSuite) TestUpdateGroupPolicyDecisionPolicy() {
policy: group.NewThresholdDecisionPolicy(
"2",
time.Duration(2)*time.Second,
0,
),
expGroupPolicy: &group.GroupPolicyInfo{
Admin: admin.String(),
@@ -1216,6 +1233,7 @@ func (s *TestSuite) TestUpdateGroupPolicyDecisionPolicy() {
policy: group.NewPercentageDecisionPolicy(
"0.5",
time.Duration(2)*time.Second,
0,
),
expGroupPolicy: &group.GroupPolicyInfo{
Admin: admin.String(),
@@ -1278,14 +1296,17 @@ func (s *TestSuite) TestGroupPoliciesByAdminOrGroup() {
group.NewThresholdDecisionPolicy(
"1",
time.Second,
0,
),
group.NewThresholdDecisionPolicy(
"10",
time.Second,
0,
),
group.NewPercentageDecisionPolicy(
"0.5",
time.Second,
0,
),
}
@@ -1376,6 +1397,7 @@ func (s *TestSuite) TestSubmitProposal() {
policy := group.NewThresholdDecisionPolicy(
"100",
time.Second,
0,
)
err := policyReq.SetDecisionPolicy(policy)
s.Require().NoError(err)
@@ -1523,7 +1545,7 @@ func (s *TestSuite) TestSubmitProposal() {
Status: group.PROPOSAL_STATUS_SUBMITTED,
Result: group.PROPOSAL_RESULT_UNFINALIZED,
FinalTallyResult: group.TallyResult{
YesCount: "1",
YesCount: "0", // Since tally doesn't pass Allow(), we consider the proposal not final
NoCount: "0",
AbstainCount: "0",
NoWithVetoCount: "0",
@@ -1562,7 +1584,7 @@ func (s *TestSuite) TestSubmitProposal() {
s.Assert().Equal(spec.expProposal.Result, proposal.Result)
s.Assert().Equal(spec.expProposal.FinalTallyResult, proposal.FinalTallyResult)
s.Assert().Equal(spec.expProposal.ExecutorResult, proposal.ExecutorResult)
s.Assert().Equal(s.blockTime.Add(time.Second), proposal.Timeout)
s.Assert().Equal(s.blockTime.Add(time.Second), proposal.VotingPeriodEnd)
if spec.msgs == nil { // then empty list is ok
s.Assert().Len(proposal.GetMsgs(), 0)
@@ -1684,6 +1706,7 @@ func (s *TestSuite) TestVote() {
policy := group.NewThresholdDecisionPolicy(
"2",
time.Duration(2),
0,
)
policyReq := &group.MsgCreateGroupPolicy{
Admin: addr1.String(),
@@ -1732,23 +1755,19 @@ func (s *TestSuite) TestVote() {
s.Assert().Equal(uint64(1), proposals[0].GroupPolicyVersion)
s.Assert().Equal(group.PROPOSAL_STATUS_SUBMITTED, proposals[0].Status)
s.Assert().Equal(group.PROPOSAL_RESULT_UNFINALIZED, proposals[0].Result)
s.Assert().Equal(group.TallyResult{
YesCount: "0",
NoCount: "0",
AbstainCount: "0",
NoWithVetoCount: "0",
}, proposals[0].FinalTallyResult)
s.Assert().Equal(group.DefaultTallyResult(), proposals[0].FinalTallyResult)
specs := map[string]struct {
srcCtx sdk.Context
expFinalTallyResult group.TallyResult
req *group.MsgVote
doBefore func(ctx context.Context)
postRun func(sdkCtx sdk.Context)
expProposalStatus group.ProposalStatus
expResult group.ProposalResult
expExecutorResult group.ProposalExecutorResult
expErr bool
srcCtx sdk.Context
expTallyResult group.TallyResult // expected after tallying
isFinal bool // is the tally result final?
req *group.MsgVote
doBefore func(ctx context.Context)
postRun func(sdkCtx sdk.Context)
expProposalStatus group.ProposalStatus // expected after tallying
expResult group.ProposalResult // expected after tallying
expExecutorResult group.ProposalExecutorResult // expected after tallying
expErr bool
}{
"vote yes": {
req: &group.MsgVote{
@@ -1756,7 +1775,7 @@ func (s *TestSuite) TestVote() {
Voter: addr4.String(),
Option: group.VOTE_OPTION_YES,
},
expFinalTallyResult: group.TallyResult{
expTallyResult: group.TallyResult{
YesCount: "1",
NoCount: "0",
AbstainCount: "0",
@@ -1774,12 +1793,13 @@ func (s *TestSuite) TestVote() {
Option: group.VOTE_OPTION_YES,
Exec: group.Exec_EXEC_TRY,
},
expFinalTallyResult: group.TallyResult{
expTallyResult: group.TallyResult{
YesCount: "2",
NoCount: "0",
AbstainCount: "0",
NoWithVetoCount: "0",
},
isFinal: true,
expProposalStatus: group.PROPOSAL_STATUS_CLOSED,
expResult: group.PROPOSAL_RESULT_ACCEPTED,
expExecutorResult: group.PROPOSAL_EXECUTOR_RESULT_SUCCESS,
@@ -1797,7 +1817,7 @@ func (s *TestSuite) TestVote() {
Option: group.VOTE_OPTION_YES,
Exec: group.Exec_EXEC_TRY,
},
expFinalTallyResult: group.TallyResult{
expTallyResult: group.TallyResult{
YesCount: "1",
NoCount: "0",
AbstainCount: "0",
@@ -1814,7 +1834,7 @@ func (s *TestSuite) TestVote() {
Voter: addr4.String(),
Option: group.VOTE_OPTION_NO,
},
expFinalTallyResult: group.TallyResult{
expTallyResult: group.TallyResult{
YesCount: "0",
NoCount: "1",
AbstainCount: "0",
@@ -1831,7 +1851,7 @@ func (s *TestSuite) TestVote() {
Voter: addr4.String(),
Option: group.VOTE_OPTION_ABSTAIN,
},
expFinalTallyResult: group.TallyResult{
expTallyResult: group.TallyResult{
YesCount: "0",
NoCount: "0",
AbstainCount: "1",
@@ -1848,7 +1868,7 @@ func (s *TestSuite) TestVote() {
Voter: addr4.String(),
Option: group.VOTE_OPTION_NO_WITH_VETO,
},
expFinalTallyResult: group.TallyResult{
expTallyResult: group.TallyResult{
YesCount: "0",
NoCount: "0",
AbstainCount: "0",
@@ -1865,7 +1885,7 @@ func (s *TestSuite) TestVote() {
Voter: addr3.String(),
Option: group.VOTE_OPTION_YES,
},
expFinalTallyResult: group.TallyResult{
expTallyResult: group.TallyResult{
YesCount: "2",
NoCount: "0",
AbstainCount: "0",
@@ -1887,6 +1907,7 @@ func (s *TestSuite) TestVote() {
ProposalId: myProposalID,
Voter: addr3.String(),
Option: group.VOTE_OPTION_NO_WITH_VETO,
Exec: 1, // Execute the proposal so that its status is final
})
s.Require().NoError(err)
},
@@ -1947,7 +1968,7 @@ func (s *TestSuite) TestVote() {
expErr: true,
postRun: func(sdkCtx sdk.Context) {},
},
"on timeout": {
"on voting period end": {
req: &group.MsgVote{
ProposalId: myProposalID,
Voter: addr4.String(),
@@ -1968,6 +1989,7 @@ func (s *TestSuite) TestVote() {
ProposalId: myProposalID,
Voter: addr3.String(),
Option: group.VOTE_OPTION_YES,
Exec: 1, // Execute to close the proposal.
})
s.Require().NoError(err)
},
@@ -2019,7 +2041,9 @@ func (s *TestSuite) TestVote() {
groupPolicy,
&group.ThresholdDecisionPolicy{
Threshold: "1",
Timeout: time.Second,
Windows: &group.DecisionPolicyWindows{
VotingPeriod: time.Second,
},
},
)
s.Require().NoError(err)
@@ -2093,16 +2117,26 @@ func (s *TestSuite) TestVote() {
s.Assert().Equal(spec.req.Metadata, votesByVoter[0].Metadata)
s.Assert().Equal(s.blockTime, votesByVoter[0].SubmitTime)
// and proposal is updated
proposalRes, err := s.keeper.Proposal(ctx, &group.QueryProposalRequest{
ProposalId: spec.req.ProposalId,
})
s.Require().NoError(err)
proposal := proposalRes.Proposal
s.Assert().Equal(spec.expFinalTallyResult, proposal.FinalTallyResult)
s.Assert().Equal(spec.expResult, proposal.Result)
s.Assert().Equal(spec.expProposalStatus, proposal.Status)
s.Assert().Equal(spec.expExecutorResult, proposal.ExecutorResult)
if spec.isFinal {
s.Assert().Equal(spec.expTallyResult, proposal.FinalTallyResult)
s.Assert().Equal(spec.expResult, proposal.Result)
s.Assert().Equal(spec.expProposalStatus, proposal.Status)
s.Assert().Equal(spec.expExecutorResult, proposal.ExecutorResult)
} else {
s.Assert().Equal(group.DefaultTallyResult(), proposal.FinalTallyResult) // Make sure proposal isn't mutated.
// do a round of tallying
tallyResult, err := s.keeper.Tally(sdkCtx, *proposal, myGroupID)
s.Require().NoError(err)
s.Assert().Equal(spec.expTallyResult, tallyResult)
}
spec.postRun(sdkCtx)
})
@@ -2370,6 +2404,7 @@ func createGroupAndGroupPolicy(
policy := group.NewThresholdDecisionPolicy(
"1",
time.Second,
0,
)
err = groupPolicy.SetDecisionPolicy(policy)
s.Require().NoError(err)
x/group/keeper/msg_server.go
+21 -38
View File
@@ -467,16 +467,11 @@ func (k Keeper) SubmitProposal(goCtx context.Context, req *group.MsgSubmitPropos
}
// Prevent proposal that can not succeed.
err = policy.Validate(g)
err = policy.Validate(g, k.config)
if err != nil {
return nil, err
}
// Define proposal timout.
// The voting window begins as soon as the proposal is submitted.
timeout := policy.GetTimeout()
window := timeout
m := &group.Proposal{
Id: k.proposalTable.Sequence().PeekNextVal(ctx.KVStore(k.key)),
Address: req.Address,
@@ -488,14 +483,10 @@ func (k Keeper) SubmitProposal(goCtx context.Context, req *group.MsgSubmitPropos
Result: group.PROPOSAL_RESULT_UNFINALIZED,
Status: group.PROPOSAL_STATUS_SUBMITTED,
ExecutorResult: group.PROPOSAL_EXECUTOR_RESULT_NOT_RUN,
Timeout: ctx.BlockTime().Add(window),
FinalTallyResult: group.TallyResult{
YesCount: "0",
NoCount: "0",
AbstainCount: "0",
NoWithVetoCount: "0",
},
VotingPeriodEnd: ctx.BlockTime().Add(policy.GetVotingPeriod()), // The voting window begins as soon as the proposal is submitted.
FinalTallyResult: group.DefaultTallyResult(),
}
if err := m.SetMsgs(msgs); err != nil {
return nil, sdkerrors.Wrap(err, "create proposal")
}
@@ -524,6 +515,7 @@ func (k Keeper) SubmitProposal(goCtx context.Context, req *group.MsgSubmitPropos
return &group.MsgSubmitProposalResponse{ProposalId: id}, sdkerrors.Wrap(err, "The proposal was created but failed on vote")
}
}
// Then try to execute the proposal
_, err = k.Exec(sdk.WrapSDKContext(ctx), &group.MsgExec{
ProposalId: id,
@@ -619,15 +611,7 @@ func (k Keeper) Vote(goCtx context.Context, req *group.MsgVote) (*group.MsgVoteR
if proposal.Status != group.PROPOSAL_STATUS_SUBMITTED {
return nil, sdkerrors.Wrap(errors.ErrInvalid, "proposal not open for voting")
}
proposalTimeout, err := gogotypes.TimestampProto(proposal.Timeout)
if err != nil {
return nil, err
}
votingPeriodEnd, err := gogotypes.TimestampFromProto(proposalTimeout)
if err != nil {
return nil, err
}
if votingPeriodEnd.Before(ctx.BlockTime()) || votingPeriodEnd.Equal(ctx.BlockTime()) {
if ctx.BlockTime().After(proposal.VotingPeriodEnd) {
return nil, sdkerrors.Wrap(errors.ErrExpired, "voting period has ended already")
}
@@ -663,9 +647,6 @@ func (k Keeper) Vote(goCtx context.Context, req *group.MsgVote) (*group.MsgVoteR
Metadata: metadata,
SubmitTime: ctx.BlockTime(),
}
if err := proposal.FinalTallyResult.Add(newVote, voter.Member.Weight); err != nil {
return nil, sdkerrors.Wrap(err, "add new vote")
}
// The ORM will return an error if the vote already exists,
// making sure than a voter hasn't already voted.
@@ -673,15 +654,6 @@ func (k Keeper) Vote(goCtx context.Context, req *group.MsgVote) (*group.MsgVoteR
return nil, sdkerrors.Wrap(err, "store vote")
}
// Run tally with new votes to close early.
if err := doTally(ctx, &proposal, electorate, policyInfo); err != nil {
return nil, err
}
if err = k.proposalTable.Update(ctx.KVStore(k.key), id, &proposal); err != nil {
return nil, err
}
err = ctx.EventManager().EmitTypedEvent(&group.EventVote{ProposalId: id})
if err != nil {
return nil, err
@@ -701,8 +673,9 @@ func (k Keeper) Vote(goCtx context.Context, req *group.MsgVote) (*group.MsgVoteR
return &group.MsgVoteResponse{}, nil
}
// doTally updates the proposal status and tally if necessary based on the group policy's decision policy.
func doTally(ctx sdk.Context, p *group.Proposal, electorate group.GroupInfo, policyInfo group.GroupPolicyInfo) error {
// doTallyAndUpdate performs a tally, and updates the proposal's
// `FinalTallyResult` field only if the tally is final.
func (k Keeper) doTallyAndUpdate(ctx sdk.Context, p *group.Proposal, electorate group.GroupInfo, policyInfo group.GroupPolicyInfo) error {
policy := policyInfo.GetDecisionPolicy()
pSubmittedAt, err := gogotypes.TimestampProto(p.SubmitTime)
if err != nil {
@@ -712,16 +685,25 @@ func doTally(ctx sdk.Context, p *group.Proposal, electorate group.GroupInfo, pol
if err != nil {
return err
}
switch result, err := policy.Allow(p.FinalTallyResult, electorate.TotalWeight, ctx.BlockTime().Sub(submittedAt)); {
tallyResult, err := k.Tally(ctx, *p, policyInfo.GroupId)
if err != nil {
return err
}
switch result, err := policy.Allow(tallyResult, electorate.TotalWeight, ctx.BlockTime().Sub(submittedAt)); {
case err != nil:
return sdkerrors.Wrap(err, "policy execution")
case result.Allow && result.Final:
p.FinalTallyResult = tallyResult
p.Result = group.PROPOSAL_RESULT_ACCEPTED
p.Status = group.PROPOSAL_STATUS_CLOSED
case !result.Allow && result.Final:
p.FinalTallyResult = tallyResult
p.Result = group.PROPOSAL_RESULT_REJECTED
p.Status = group.PROPOSAL_STATUS_CLOSED
}
return nil
}
@@ -770,7 +752,8 @@ func (k Keeper) Exec(goCtx context.Context, req *group.MsgExec) (*group.MsgExecR
proposal.Status = group.PROPOSAL_STATUS_ABORTED
return storeUpdates()
}
if err := doTally(ctx, &proposal, electorate, policyInfo); err != nil {
if err := k.doTallyAndUpdate(ctx, &proposal, electorate, policyInfo); err != nil {
return nil, err
}
}
x/group/keeper/proposal_executor.go
+10
View File
@@ -11,6 +11,16 @@ 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 ant handler is bypassed
func (s Keeper) doExecuteMsgs(ctx sdk.Context, router *authmiddleware.MsgServiceRouter, proposal group.Proposal, groupPolicyAcc sdk.AccAddress) ([]sdk.Result, error) {
// 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
// the proposal doesn't exist in state. For sanity check, we can still keep
// this simple and cheap check.
expiryDate := proposal.VotingPeriodEnd.Add(s.config.MaxExecutionPeriod)
if expiryDate.Before(ctx.BlockTime()) {
return nil, grouperrors.ErrExpired.Wrapf("proposal expired on %s", expiryDate)
}
msgs := proposal.GetMsgs()
results := make([]sdk.Result, len(msgs))