Skip to content
This repository has been archived by the owner on Aug 3, 2023. It is now read-only.

Commit

Permalink
Complete proposal execution workflow
Browse files Browse the repository at this point in the history
  • Loading branch information
alpe committed Mar 30, 2020
1 parent 09801ab commit 2da8b98
Show file tree
Hide file tree
Showing 8 changed files with 571 additions and 84 deletions.
30 changes: 6 additions & 24 deletions incubator/group/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,7 @@ func (k Keeper) ExecProposal(ctx sdk.Context, id ProposalID) error {
if base.Status == ProposalBase_Closed && base.Result == ProposalBase_Accepted && base.ExecutorResult != ProposalBase_Success {
logger := ctx.Logger().With("module", fmt.Sprintf("x/%s", ModuleName))
ctx, flush := ctx.CacheContext()
_, err := executeMsgs(ctx, k.router, accountMetadata, base, proposal.GetMsgs())
_, err := doExecuteMsgs(ctx, k.router, accountMetadata.Base.GroupAccount, proposal.GetMsgs())
if err != nil {
base.ExecutorResult = ProposalBase_Failure
proposalType := reflect.TypeOf(proposal).String()
Expand All @@ -469,29 +469,6 @@ func (k Keeper) ExecProposal(ctx sdk.Context, id ProposalID) error {
return storeUpdates()
}

func executeMsgs(ctx sdk.Context, router sdk.Router, accountMetadata StdGroupAccountMetadata, base ProposalBase, msgs []sdk.Msg) ([]sdk.Result, error) {
results := make([]sdk.Result, len(msgs))
for i, msg := range msgs {
for _, acct := range msg.GetSigners() {
if !accountMetadata.Base.GroupAccount.Equals(acct) {
base.ExecutorResult = ProposalBase_Failure
return nil, errors.Wrap(errors.ErrUnauthorized, "proposal msg does not have permission")
}
}

handler := router.Route(ctx, msg.Route())
if handler == nil {
return nil, errors.Wrapf(ErrInvalid, "no message handler found for %q", msg.Route())
}
r, err := handler(ctx, msg)
if err != nil {
return nil, errors.Wrapf(err, "message %q at position %d", msg.Type(), i)
}
results[i] = *r
}
return results, nil
}

func (k Keeper) GetProposal(ctx sdk.Context, id ProposalID) (ProposalI, error) {
loaded := reflect.New(k.proposalModelType).Interface().(ProposalI)
if _, err := k.proposalTable.GetOne(ctx, id.Uint64(), loaded); err != nil {
Expand All @@ -516,12 +493,17 @@ func (k Keeper) CreateProposal(ctx sdk.Context, accountAddress sdk.AccAddress, c
return 0, errors.Wrap(err, "get group by account")
}

// only members can propose
for i := range proposers {
if !k.groupMemberTable.Has(ctx, GroupMember{Group: g.Group, Member: proposers[i]}.NaturalKey()) {
return 0, errors.Wrapf(ErrUnauthorized, "not in group: %s", proposers[i])
}
}

if err := ensureMsgAuthZ(msgs, account.Base.GroupAccount); err != nil {
return 0, err
}

blockTime, err := types.TimestampProto(ctx.BlockTime())
if err != nil {
return 0, errors.Wrap(err, "block time conversion")
Expand Down
104 changes: 78 additions & 26 deletions incubator/group/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,13 @@ func TestCreateProposal(t *testing.T) {
srcProposers: []sdk.AccAddress{[]byte("valid--admin-address")},
expErr: true,
},
"reject msgs that are not authz by group account": {
srcAccount: accountAddr,
srcComment: "test",
srcMsgs: []sdk.Msg{&testdata.MsgAuthenticate{Signers: []sdk.AccAddress{[]byte("not-group-acct-addrs")}}},
srcProposers: []sdk.AccAddress{[]byte("valid-member-address")},
expErr: true,
},
}
for msg, spec := range specs {
t.Run(msg, func(t *testing.T) {
Expand Down Expand Up @@ -377,7 +384,7 @@ func TestVote(t *testing.T) {
srcChoice group.Choice
srcComment string
srcCtx sdk.Context
doBefore func(ctx sdk.Context)
doBefore func(t *testing.T, ctx sdk.Context)
expErr bool
expVoteState group.Tally
expProposalStatus group.ProposalBase_Status
Expand Down Expand Up @@ -448,6 +455,15 @@ func TestVote(t *testing.T) {
expProposalStatus: group.ProposalBase_Closed,
expResult: group.ProposalBase_Accepted,
},
"reject new votes when final decision is made already": {
srcProposalID: myProposalID,
srcVoters: []sdk.AccAddress{[]byte("valid-member-address")},
srcChoice: group.Choice_YES,
doBefore: func(t *testing.T, ctx sdk.Context) {
require.NoError(t, k.Vote(ctx, myProposalID, []sdk.AccAddress{[]byte("power-member-address")}, group.Choice_VETO, ""))
},
expErr: true,
},
"comment too long": {
srcProposalID: myProposalID,
srcComment: strings.Repeat("a", 256),
Expand Down Expand Up @@ -512,7 +528,7 @@ func TestVote(t *testing.T) {
srcProposalID: myProposalID,
srcVoters: []sdk.AccAddress{[]byte("valid-member-address")},
srcChoice: group.Choice_NO,
doBefore: func(ctx sdk.Context) {
doBefore: func(t *testing.T, ctx sdk.Context) {
err := k.Vote(ctx, myProposalID, []sdk.AccAddress{[]byte("power-member-address")}, group.Choice_YES, "")
require.NoError(t, err)
},
Expand All @@ -522,7 +538,7 @@ func TestVote(t *testing.T) {
srcProposalID: myProposalID,
srcVoters: []sdk.AccAddress{[]byte("valid-member-address")},
srcChoice: group.Choice_NO,
doBefore: func(ctx sdk.Context) {
doBefore: func(t *testing.T, ctx sdk.Context) {
err := k.Vote(ctx, myProposalID, []sdk.AccAddress{[]byte("valid-member-address")}, group.Choice_YES, "")
require.NoError(t, err)
},
Expand All @@ -532,7 +548,7 @@ func TestVote(t *testing.T) {
srcProposalID: myProposalID,
srcVoters: []sdk.AccAddress{[]byte("valid-member-address")},
srcChoice: group.Choice_NO,
doBefore: func(ctx sdk.Context) {
doBefore: func(t *testing.T, ctx sdk.Context) {
g, err := k.GetGroup(ctx, myGroupID)
require.NoError(t, err)
g.Comment = "modified"
Expand All @@ -544,7 +560,7 @@ func TestVote(t *testing.T) {
srcProposalID: myProposalID,
srcVoters: []sdk.AccAddress{[]byte("valid-member-address")},
srcChoice: group.Choice_NO,
doBefore: func(ctx sdk.Context) {
doBefore: func(t *testing.T, ctx sdk.Context) {
a, err := k.GetGroupAccount(ctx, accountAddr)
require.NoError(t, err)
a.Base.Comment = "modified"
Expand All @@ -562,7 +578,7 @@ func TestVote(t *testing.T) {
ctx, _ = ctx.CacheContext()

if spec.doBefore != nil {
spec.doBefore(ctx)
spec.doBefore(t, ctx)
}
err := k.Vote(ctx, spec.srcProposalID, spec.srcVoters, spec.srcChoice, spec.srcComment)
if spec.expErr {
Expand Down Expand Up @@ -627,15 +643,15 @@ func TestExecProposal(t *testing.T) {

specs := map[string]struct {
srcBlockTime time.Time
setupProposal func(ctx sdk.Context) group.ProposalID
setupProposal func(t *testing.T, ctx sdk.Context) group.ProposalID
expErr bool
expProposalStatus group.ProposalBase_Status
expProposalResult group.ProposalBase_Result
expExecutorResult group.ProposalBase_ExecutorResult
expPayloadCounter uint64
}{
"proposal executed when accepted": {
setupProposal: func(ctx sdk.Context) group.ProposalID {
setupProposal: func(t *testing.T, ctx sdk.Context) group.ProposalID {
member := []sdk.AccAddress{[]byte("valid-member-address")}
myProposalID, err := k.CreateProposal(ctx, accountAddr, "test", member, []sdk.Msg{
&testdata.MsgIncCounter{},
Expand All @@ -650,7 +666,7 @@ func TestExecProposal(t *testing.T) {
expPayloadCounter: 1,
},
"proposal with multiple messages executed when accepted": {
setupProposal: func(ctx sdk.Context) group.ProposalID {
setupProposal: func(t *testing.T, ctx sdk.Context) group.ProposalID {
member := []sdk.AccAddress{[]byte("valid-member-address")}
myProposalID, err := k.CreateProposal(ctx, accountAddr, "test", member, []sdk.Msg{
&testdata.MsgIncCounter{}, &testdata.MsgIncCounter{},
Expand All @@ -665,7 +681,7 @@ func TestExecProposal(t *testing.T) {
expPayloadCounter: 2,
},
"proposal not executed when rejected": {
setupProposal: func(ctx sdk.Context) group.ProposalID {
setupProposal: func(t *testing.T, ctx sdk.Context) group.ProposalID {
member := []sdk.AccAddress{[]byte("valid-member-address")}
myProposalID, err := k.CreateProposal(ctx, accountAddr, "test", member, []sdk.Msg{
&testdata.MsgAlwaysFail{},
Expand All @@ -679,7 +695,7 @@ func TestExecProposal(t *testing.T) {
expExecutorResult: group.ProposalBase_NotRun,
},
"open proposal must not fail": {
setupProposal: func(ctx sdk.Context) group.ProposalID {
setupProposal: func(t *testing.T, ctx sdk.Context) group.ProposalID {
member := []sdk.AccAddress{[]byte("valid-member-address")}
myProposalID, err := k.CreateProposal(ctx, accountAddr, "test", member, []sdk.Msg{
&testdata.MsgAlwaysFail{},
Expand All @@ -692,13 +708,13 @@ func TestExecProposal(t *testing.T) {
expExecutorResult: group.ProposalBase_NotRun,
},
"existing proposal required": {
setupProposal: func(ctx sdk.Context) group.ProposalID {
setupProposal: func(t *testing.T, ctx sdk.Context) group.ProposalID {
return 9999
},
expErr: true,
},
"Decision policy also applied on timeout": {
setupProposal: func(ctx sdk.Context) group.ProposalID {
setupProposal: func(t *testing.T, ctx sdk.Context) group.ProposalID {
member := []sdk.AccAddress{[]byte("valid-member-address")}
myProposalID, err := k.CreateProposal(ctx, accountAddr, "test", member, []sdk.Msg{
&testdata.MsgAlwaysFail{},
Expand All @@ -713,7 +729,7 @@ func TestExecProposal(t *testing.T) {
expExecutorResult: group.ProposalBase_NotRun,
},
"Decision policy also applied after timeout": {
setupProposal: func(ctx sdk.Context) group.ProposalID {
setupProposal: func(t *testing.T, ctx sdk.Context) group.ProposalID {
member := []sdk.AccAddress{[]byte("valid-member-address")}
myProposalID, err := k.CreateProposal(ctx, accountAddr, "test", member, []sdk.Msg{
&testdata.MsgAlwaysFail{},
Expand All @@ -727,8 +743,8 @@ func TestExecProposal(t *testing.T) {
expProposalResult: group.ProposalBase_Rejected,
expExecutorResult: group.ProposalBase_NotRun,
},
"with group modified": {
setupProposal: func(ctx sdk.Context) group.ProposalID {
"with group modified before tally": {
setupProposal: func(t *testing.T, ctx sdk.Context) group.ProposalID {
member := []sdk.AccAddress{[]byte("valid-member-address")}
myProposalID, err := k.CreateProposal(ctx, accountAddr, "test", member, []sdk.Msg{
&testdata.MsgAlwaysFail{},
Expand All @@ -745,8 +761,8 @@ func TestExecProposal(t *testing.T) {
expProposalResult: group.ProposalBase_Undefined,
expExecutorResult: group.ProposalBase_NotRun,
},
"with group account modified": {
setupProposal: func(ctx sdk.Context) group.ProposalID {
"with group account modified before tally": {
setupProposal: func(t *testing.T, ctx sdk.Context) group.ProposalID {
member := []sdk.AccAddress{[]byte("valid-member-address")}
myProposalID, err := k.CreateProposal(ctx, accountAddr, "test", member, []sdk.Msg{
&testdata.MsgAlwaysFail{},
Expand All @@ -763,8 +779,45 @@ func TestExecProposal(t *testing.T) {
expProposalResult: group.ProposalBase_Undefined,
expExecutorResult: group.ProposalBase_NotRun,
},
"not executed when was successfully already": {
setupProposal: func(ctx sdk.Context) group.ProposalID {
"with group modified after tally": {
setupProposal: func(t *testing.T, ctx sdk.Context) group.ProposalID {
member := []sdk.AccAddress{[]byte("valid-member-address")}
myProposalID, err := k.CreateProposal(ctx, accountAddr, "test", member, []sdk.Msg{
&testdata.MsgAlwaysFail{},
})
require.NoError(t, err)
require.NoError(t, k.Vote(ctx, myProposalID, member, group.Choice_YES, ""))
// then modify group after tally on vote
g, err := k.GetGroup(ctx, myGroupID)
require.NoError(t, err)
g.Comment = "modified"
require.NoError(t, k.UpdateGroup(ctx, &g))
return myProposalID
},
expProposalStatus: group.ProposalBase_Closed,
expProposalResult: group.ProposalBase_Accepted,
expExecutorResult: group.ProposalBase_Failure,
},
"with group account modified after tally": {
setupProposal: func(t *testing.T, ctx sdk.Context) group.ProposalID {
member := []sdk.AccAddress{[]byte("valid-member-address")}
myProposalID, err := k.CreateProposal(ctx, accountAddr, "test", member, []sdk.Msg{
&testdata.MsgAlwaysFail{},
})
require.NoError(t, err)
// then modify group account
a, err := k.GetGroupAccount(ctx, accountAddr)
require.NoError(t, err)
a.Base.Comment = "modified"
require.NoError(t, k.UpdateGroupAccount(ctx, &a))
return myProposalID
},
expProposalStatus: group.ProposalBase_Aborted,
expProposalResult: group.ProposalBase_Undefined,
expExecutorResult: group.ProposalBase_NotRun,
},
"prevent double execution when successful": {
setupProposal: func(t *testing.T, ctx sdk.Context) group.ProposalID {
member := []sdk.AccAddress{[]byte("valid-member-address")}
myProposalID, err := k.CreateProposal(ctx, accountAddr, "test", member, []sdk.Msg{
&testdata.MsgIncCounter{},
Expand All @@ -779,11 +832,11 @@ func TestExecProposal(t *testing.T) {
expProposalResult: group.ProposalBase_Accepted,
expExecutorResult: group.ProposalBase_Success,
},
"set executor status on failure": {
setupProposal: func(ctx sdk.Context) group.ProposalID {
"rollback all msg updates on failure": {
setupProposal: func(t *testing.T, ctx sdk.Context) group.ProposalID {
member := []sdk.AccAddress{[]byte("valid-member-address")}
myProposalID, err := k.CreateProposal(ctx, accountAddr, "test", member, []sdk.Msg{
&testdata.MsgAlwaysFail{},
&testdata.MsgIncCounter{}, &testdata.MsgAlwaysFail{},
})
require.NoError(t, err)
require.NoError(t, k.Vote(ctx, myProposalID, member, group.Choice_YES, ""))
Expand All @@ -794,7 +847,7 @@ func TestExecProposal(t *testing.T) {
expExecutorResult: group.ProposalBase_Failure,
},
"executable when failed before": {
setupProposal: func(ctx sdk.Context) group.ProposalID {
setupProposal: func(t *testing.T, ctx sdk.Context) group.ProposalID {
member := []sdk.AccAddress{[]byte("valid-member-address")}
myProposalID, err := k.CreateProposal(ctx, accountAddr, "test", member, []sdk.Msg{
&testdata.MsgConditional{ExpectedCounter: 1}, &testdata.MsgIncCounter{},
Expand All @@ -810,12 +863,11 @@ func TestExecProposal(t *testing.T) {
expProposalResult: group.ProposalBase_Accepted,
expExecutorResult: group.ProposalBase_Success,
},
// execute with different signer (access different account
}
for msg, spec := range specs {
t.Run(msg, func(t *testing.T) {
ctx, _ := parentCtx.CacheContext()
proposalID := spec.setupProposal(ctx)
proposalID := spec.setupProposal(t, ctx)

if !spec.srcBlockTime.IsZero() {
ctx = ctx.WithBlockTime(spec.srcBlockTime)
Expand Down
41 changes: 41 additions & 0 deletions incubator/group/proposal_executor.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package group

import (
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/errors"
)

// ensureMsgAuthZ checks that if a message requires signers that all of them are equal to the given group account.
func ensureMsgAuthZ(msgs []sdk.Msg, groupAccount sdk.AccAddress) error {
for i := range msgs {
for _, acct := range msgs[i].GetSigners() {
if !groupAccount.Equals(acct) {
return errors.Wrap(errors.ErrUnauthorized, "msg does not have group account authorization")
}
}
}
return nil
}

// doExecuteMsgs routes the messages to the registered handlers. Messages are limited to those that require no authZ or
// by the group account only. Otherwise this gives access to other peoples accounts as the sdk ant handler is bypassed
func doExecuteMsgs(ctx sdk.Context, router sdk.Router, groupAccount sdk.AccAddress, msgs []sdk.Msg) ([]sdk.Result, error) {
results := make([]sdk.Result, len(msgs))
if err := ensureMsgAuthZ(msgs, groupAccount); err != nil {
return nil, err
}
for i, msg := range msgs {
handler := router.Route(ctx, msg.Route())
if handler == nil {
return nil, errors.Wrapf(ErrInvalid, "no message handler found for %q", msg.Route())
}
r, err := handler(ctx, msg)
if err != nil {
return nil, errors.Wrapf(err, "message %q at position %d", msg.Type(), i)
}
if r != nil {
results[i] = *r
}
}
return results, nil
}
Loading

0 comments on commit 2da8b98

Please sign in to comment.