Skip to content

Commit

Permalink
feat(group): add group event tally result (backport cosmos#16191) (co…
Browse files Browse the repository at this point in the history 
…smos#16306)

Co-authored-by: Jeancarlo Barrios <JeancarloBarrios@users.noreply.github.com>
Co-authored-by: marbar3778 <marbar3778@yahoo.com>
Co-authored-by: Julien Robert <julien@rbrt.fr>
  • Loading branch information
@mergify @JeancarloBarrios
@tac0turtle @julienrbrt
4 people committed Sep 28, 2024
1 parent 93d83a4 commit b621cde
Show file tree
Hide file tree
Showing 7 changed files with 133 additions and 51 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

* (baseapp) [#16290](https://github.com/cosmos/cosmos-sdk/pull/16290) Add circuit breaker setter in baseapp.
* [#16060](https://github.com/cosmos/cosmos-sdk/pull/16060) Support saving restoring snapshot locally.
* (x/group) [#16191](https://github.com/cosmos/cosmos-sdk/pull/16191) Add EventProposalPruned event to group module whenever a proposal is pruned.

### Improvements

Expand Down
58 changes: 29 additions & 29 deletions x/group/events.pb.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion x/group/keeper/genesis.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func (k Keeper) InitGenesis(ctx context.Context, cdc codec.JSONCodec, data json.
}

// ExportGenesis returns the group module's exported genesis.
func (k Keeper) ExportGenesis(ctx context.Context, _ codec.JSONCodec) (*group.GenesisState, error) {
func (k Keeper) ExportGenesis(ctx types.Context, _ codec.JSONCodec) *group.GenesisState {
genesisState := group.NewGenesisState()

var groups []*group.GroupInfo
Expand Down
17 changes: 17 additions & 0 deletions x/group/keeper/keeper.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,15 @@ func (k Keeper) PruneProposals(ctx sdk.Context) error {
if err != nil {
return err
}
// Emit event for proposal finalized with its result
if err := ctx.EventManager().EmitTypedEvent(
&group.EventProposalPruned{
ProposalId: proposal.Id,
Status: proposal.Status,
TallyResult: &proposal.FinalTallyResult,
}); err != nil {
return err
}
}

return nil
Expand Down Expand Up @@ -416,6 +425,14 @@ func (k Keeper) TallyProposalsAtVPEnd(ctx sdk.Context) error {
if err := k.pruneVotes(ctx, proposalID); err != nil {
return err
}
// Emit event for proposal finalized with its result
if err := ctx.EventManager().EmitTypedEvent(
&group.EventProposalPruned{
ProposalId: proposal.Id,
Status: proposal.Status,
}); err != nil {
return err
}
} else if proposal.Status == group.PROPOSAL_STATUS_SUBMITTED {
if err := k.doTallyAndUpdate(ctx, &proposal, electorate, policyInfo); err != nil {
return sdkerrors.Wrap(err, "doTallyAndUpdate")
Expand Down
98 changes: 81 additions & 17 deletions x/group/keeper/keeper_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

"github.com/golang/mock/gomock"
"github.com/stretchr/testify/suite"
abci "github.com/tendermint/tendermint/abci/types"
tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
tmtime "github.com/tendermint/tendermint/types/time"

Expand All @@ -33,6 +34,8 @@ import (
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
)

var EventProposalPruned = "cosmos.group.v1.EventProposalPruned"

const minExecutionPeriod = 5 * time.Second

type TestSuite struct {
Expand Down Expand Up @@ -1673,6 +1676,8 @@ func (s *TestSuite) TestSubmitProposal() {
s.Require().Contains(fromBalances, sdk.NewInt64Coin("test", 9900))
toBalances := s.app.BankKeeper.GetAllBalances(sdkCtx, addr2)
s.Require().Contains(toBalances, sdk.NewInt64Coin("test", 100))
events := sdkCtx.EventManager().ABCIEvents()
s.Require().True(eventTypeFound(events, EventProposalPruned))
},
},
"with try exec, not enough yes votes for proposal to pass": {
Expand Down Expand Up @@ -1761,27 +1766,31 @@ func (s *TestSuite) TestWithdrawProposal() {
proposalId uint64
admin string
expErrMsg string
postRun func(sdkCtx sdk.Context)
}{
"wrong admin": {
preRun: func(sdkCtx sdk.Context) uint64 {
return submitProposal(s.ctx, s, []sdk.Msg{msgSend}, proposers)
},
admin: addr5.String(),
expErrMsg: "unauthorized",
postRun: func(sdkCtx sdk.Context) {},
},
"wrong proposalId": {
preRun: func(sdkCtx sdk.Context) uint64 {
return 1111
},
admin: proposers[0],
expErrMsg: "not found",
postRun: func(sdkCtx sdk.Context) {},
},
"happy case with proposer": {
preRun: func(sdkCtx sdk.Context) uint64 {
return submitProposal(s.ctx, s, []sdk.Msg{msgSend}, proposers)
},
proposalId: proposalID,
admin: proposers[0],
postRun: func(sdkCtx sdk.Context) {},
},
"already closed proposal": {
preRun: func(sdkCtx sdk.Context) uint64 {
Expand All @@ -1796,13 +1805,25 @@ func (s *TestSuite) TestWithdrawProposal() {
proposalId: proposalID,
admin: proposers[0],
expErrMsg: "cannot withdraw a proposal with the status of PROPOSAL_STATUS_WITHDRAWN",
postRun: func(sdkCtx sdk.Context) {},
},
"happy case with group admin address": {
preRun: func(sdkCtx sdk.Context) uint64 {
return submitProposal(s.ctx, s, []sdk.Msg{msgSend}, proposers)
},
proposalId: proposalID,
admin: proposers[0],
postRun: func(sdkCtx sdk.Context) {
resp, err := s.keeper.Proposal(s.ctx, &group.QueryProposalRequest{ProposalId: proposalID})
s.Require().NoError(err)
vpe := resp.Proposal.VotingPeriodEnd
timeDiff := vpe.Sub(s.sdkCtx.BlockTime())
ctxVPE := sdkCtx.WithBlockTime(s.sdkCtx.BlockTime().Add(timeDiff).Add(time.Second * 1))
s.Require().NoError(s.keeper.TallyProposalsAtVPEnd(ctxVPE))
events := ctxVPE.EventManager().ABCIEvents()

s.Require().True(eventTypeFound(events, EventProposalPruned))
},
},
}
for msg, spec := range specs {
Expand All @@ -1825,6 +1846,7 @@ func (s *TestSuite) TestWithdrawProposal() {
resp, err := s.keeper.Proposal(s.ctx, &group.QueryProposalRequest{ProposalId: pId})
s.Require().NoError(err)
s.Require().Equal(resp.GetProposal().Status, group.PROPOSAL_STATUS_WITHDRAWN)
spec.postRun(s.sdkCtx)
})
}
}
Expand Down Expand Up @@ -2287,6 +2309,7 @@ func (s *TestSuite) SetupTest() {
expBalance bool
expFromBalances sdk.Coin
expToBalances sdk.Coin
postRun func(sdkCtx sdk.Context)
}{
"proposal executed when accepted": {
setupProposal: func(ctx context.Context) uint64 {
Expand All @@ -2299,6 +2322,10 @@ func (s *TestSuite) SetupTest() {
expBalance: true,
expFromBalances: sdk.NewInt64Coin("test", 9900),
expToBalances: sdk.NewInt64Coin("test", 100),
postRun: func(sdkCtx sdk.Context) {
events := sdkCtx.EventManager().ABCIEvents()
s.Require().True(eventTypeFound(events, EventProposalPruned))
},
},
"proposal with multiple messages executed when accepted": {
setupProposal: func(ctx context.Context) uint64 {
Expand All @@ -2311,6 +2338,10 @@ func (s *TestSuite) SetupTest() {
expBalance: true,
expFromBalances: sdk.NewInt64Coin("test", 9800),
expToBalances: sdk.NewInt64Coin("test", 200),
postRun: func(sdkCtx sdk.Context) {
events := sdkCtx.EventManager().ABCIEvents()
s.Require().True(eventTypeFound(events, EventProposalPruned))
},
},
"proposal not executed when rejected": {
setupProposal: func(ctx context.Context) uint64 {
Expand All @@ -2320,19 +2351,22 @@ func (s *TestSuite) SetupTest() {
srcBlockTime: s.blockTime.Add(minExecutionPeriod), // After min execution period end
expProposalStatus: group.PROPOSAL_STATUS_REJECTED,
expExecutorResult: group.PROPOSAL_EXECUTOR_RESULT_NOT_RUN,
postRun: func(sdkCtx sdk.Context) {},
},
"open proposal must not fail": {
setupProposal: func(ctx context.Context) uint64 {
return submitProposal(ctx, s, []sdk.Msg{msgSend1}, proposers)
},
expProposalStatus: group.PROPOSAL_STATUS_SUBMITTED,
expExecutorResult: group.PROPOSAL_EXECUTOR_RESULT_NOT_RUN,
postRun: func(sdkCtx sdk.Context) {},
},
"existing proposal required": {
setupProposal: func(ctx context.Context) uint64 {
return 9999
},
expErr: true,
expErr: true,
postRun: func(sdkCtx sdk.Context) {},
},
"Decision policy also applied on exactly voting period end": {
setupProposal: func(ctx context.Context) uint64 {
Expand All @@ -2342,6 +2376,7 @@ func (s *TestSuite) SetupTest() {
srcBlockTime: s.blockTime.Add(time.Second), // Voting period is 1s
expProposalStatus: group.PROPOSAL_STATUS_REJECTED,
expExecutorResult: group.PROPOSAL_EXECUTOR_RESULT_NOT_RUN,
postRun: func(sdkCtx sdk.Context) {},
},
"Decision policy also applied after voting period end": {
setupProposal: func(ctx context.Context) uint64 {
Expand All @@ -2351,6 +2386,7 @@ func (s *TestSuite) SetupTest() {
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,
postRun: func(sdkCtx sdk.Context) {},
},
"exec proposal before MinExecutionPeriod should fail": {
setupProposal: func(ctx context.Context) uint64 {
Expand All @@ -2360,6 +2396,7 @@ func (s *TestSuite) SetupTest() {
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
postRun: func(sdkCtx sdk.Context) {},
},
"exec proposal at exactly MinExecutionPeriod should pass": {
setupProposal: func(ctx context.Context) uint64 {
Expand All @@ -2369,6 +2406,10 @@ func (s *TestSuite) SetupTest() {
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,
postRun: func(sdkCtx sdk.Context) {
events := sdkCtx.EventManager().ABCIEvents()
s.Require().True(eventTypeFound(events, EventProposalPruned))
},
},
"prevent double execution when successful": {
setupProposal: func(ctx context.Context) uint64 {
Expand All @@ -2390,6 +2431,10 @@ func (s *TestSuite) SetupTest() {
expBalance: true,
expFromBalances: sdk.NewInt64Coin("test", 9900),
expToBalances: sdk.NewInt64Coin("test", 100),
postRun: func(sdkCtx sdk.Context) {
events := sdkCtx.EventManager().ABCIEvents()
s.Require().True(eventTypeFound(events, EventProposalPruned))
},
},
"rollback all msg updates on failure": {
setupProposal: func(ctx context.Context) uint64 {
Expand All @@ -2399,6 +2444,7 @@ func (s *TestSuite) SetupTest() {
srcBlockTime: s.blockTime.Add(minExecutionPeriod), // After min execution period end
expProposalStatus: group.PROPOSAL_STATUS_ACCEPTED,
expExecutorResult: group.PROPOSAL_EXECUTOR_RESULT_FAILURE,
postRun: func(sdkCtx sdk.Context) {},
},
"executable when failed before": {
setupProposal: func(ctx context.Context) uint64 {
Expand All @@ -2415,6 +2461,10 @@ func (s *TestSuite) SetupTest() {
srcBlockTime: s.blockTime.Add(minExecutionPeriod), // After min execution period end
expProposalStatus: group.PROPOSAL_STATUS_ACCEPTED,
expExecutorResult: group.PROPOSAL_EXECUTOR_RESULT_SUCCESS,
postRun: func(sdkCtx sdk.Context) {
events := sdkCtx.EventManager().ABCIEvents()
s.Require().True(eventTypeFound(events, EventProposalPruned))
},
},
}
err = policyReq.SetDecisionPolicy(policy)
Expand All @@ -2427,18 +2477,16 @@ func (s *TestSuite) SetupTest() {
policyRes, err := s.groupKeeper.CreateGroupPolicy(s.ctx, policyReq)
s.Require().NoError(err)

addrbz, err := addressCodec.StringToBytes(policyRes.Address)
s.Require().NoError(err)
s.policy = policy
s.groupPolicyAddr = addrbz
s.groupPolicyStrAddr, err = addressCodec.BytesToString(s.groupPolicyAddr)
s.Require().NoError(err)
s.bankKeeper.EXPECT().MintCoins(s.sdkCtx, minttypes.ModuleName, sdk.Coins{sdk.NewInt64Coin("test", 100000)}).Return(nil).AnyTimes()
err = s.bankKeeper.MintCoins(s.sdkCtx, minttypes.ModuleName, sdk.Coins{sdk.NewInt64Coin("test", 100000)})
s.Require().NoError(err)
s.bankKeeper.EXPECT().SendCoinsFromModuleToAccount(s.sdkCtx, minttypes.ModuleName, s.groupPolicyAddr, sdk.Coins{sdk.NewInt64Coin("test", 10000)}).Return(nil).AnyTimes()
err = s.bankKeeper.SendCoinsFromModuleToAccount(s.sdkCtx, minttypes.ModuleName, s.groupPolicyAddr, sdk.Coins{sdk.NewInt64Coin("test", 10000)})
s.Require().NoError(err)
if spec.expBalance {
fromBalances := s.app.BankKeeper.GetAllBalances(sdkCtx, s.groupPolicyAddr)
s.Require().Contains(fromBalances, spec.expFromBalances)
toBalances := s.app.BankKeeper.GetAllBalances(sdkCtx, addr2)
s.Require().Contains(toBalances, spec.expToBalances)
}

spec.postRun(sdkCtx)
})
}
}

func (s *TestSuite) TestExecPrunedProposalsAndVotes() {
Expand Down Expand Up @@ -2571,10 +2619,15 @@ func (s *TestSuite) TestExecPrunedProposalsAndVotes() {
}
s.Require().NoError(err)

groupPolicyAccBumpAccountNumber, err := authtypes.NewBaseAccountWithPubKey(ac)
s.Require().NoError(err)
err = groupPolicyAccBumpAccountNumber.SetAccountNumber(nextAccVal)
s.Require().NoError(err)
if spec.expExecutorResult == group.PROPOSAL_EXECUTOR_RESULT_SUCCESS {
// Make sure proposal is deleted from state
_, err := s.keeper.Proposal(ctx, &group.QueryProposalRequest{ProposalId: proposalID})
s.Require().Contains(err.Error(), spec.expErrMsg)
res, err := s.keeper.VotesByProposal(ctx, &group.QueryVotesByProposalRequest{ProposalId: proposalID})
s.Require().NoError(err)
s.Require().Empty(res.GetVotes())
events := sdkCtx.EventManager().ABCIEvents()
s.Require().True(eventTypeFound(events, EventProposalPruned))

s.accountKeeper.EXPECT().GetAccount(gomock.Any(), sdk.AccAddress(ac.Address())).Return(nil).AnyTimes()
s.accountKeeper.EXPECT().NewAccount(gomock.Any(), groupPolicyAcc).Return(groupPolicyAccBumpAccountNumber).AnyTimes()
Expand Down Expand Up @@ -3134,3 +3187,14 @@ func (s *TestSuite) TestTallyProposalsAtVPEnd_GroupMemberLeaving() {
s.Require().NoError(s.app.GroupKeeper.TallyProposalsAtVPEnd(ctx))
s.NotPanics(func() { module.EndBlocker(ctx, s.app.GroupKeeper) })
}

func eventTypeFound(events []abci.Event, eventType string) bool {
eventTypeFound := false
for _, e := range events {
if e.Type == eventType {
eventTypeFound = true
break
}
}
return eventTypeFound
}
2 changes: 1 addition & 1 deletion x/group/keeper/msg_server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -888,7 +888,7 @@ func (k Keeper) Exec(ctx context.Context, msg *group.MsgExec) (*group.MsgExecRes
}

// Emit event for proposal finalized with its result
if err := k.EventService.EventManager(ctx).Emit(
if err := ctx.EventManager().EmitTypedEvent(
&group.EventProposalPruned{
ProposalId: proposal.Id,
Status: proposal.Status,
Expand Down
Loading

0 comments on commit b621cde

Please sign in to comment.