diff --git a/app/app.go b/app/app.go index 41a26c22..bfc2d75e 100644 --- a/app/app.go +++ b/app/app.go @@ -282,7 +282,7 @@ func NewApp(logger log.Logger, db dbm.DB, traceStore io.Writer, loadLatest bool, // there is nothing left over in the validator fee pool, so as to keep the // CanWithdrawInvariant invariant. // Auction.BeginBlocker will close out expired auctions and pay debt back to cdp. So it should be run before cdp.BeginBlocker which cancels out debt with stable and starts more auctions. - app.mm.SetOrderBeginBlockers(mint.ModuleName, distr.ModuleName, slashing.ModuleName, validatorvesting.ModuleName, auction.ModuleName, cdp.ModuleName) + app.mm.SetOrderBeginBlockers(mint.ModuleName, distr.ModuleName, slashing.ModuleName, validatorvesting.ModuleName, auction.ModuleName, cdp.ModuleName, committee.ModuleName) app.mm.SetOrderEndBlockers(crisis.ModuleName, gov.ModuleName, staking.ModuleName, pricefeed.ModuleName) diff --git a/x/committee/abci.go b/x/committee/abci.go index 54e706d8..5ab25598 100644 --- a/x/committee/abci.go +++ b/x/committee/abci.go @@ -1,12 +1,23 @@ package committee -// func BeginBlocker() { -// // TODO much the same as the current gov endblocker does +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + abci "github.com/tendermint/tendermint/abci/types" -// // Get all active proposals -// // If voting periods are over, tally up the results -// // If a proposal passes run it through the correct handler -// // Handler need to be registered in app.go as they are for the current gov module -// handler := keeper.Router().GetRoute(proposal.ProposalRoute()) -// err := handler(ctx, proposal.Content) -// } + "github.com/kava-labs/kava/x/committee/types" +) + +// BeginBlocker runs at the start of every block. +func BeginBlocker(ctx sdk.Context, _ abci.RequestBeginBlock, k Keeper) { + + // Close all expired proposals + // TODO optimize by using an index to avoid iterating over non expired proposals + k.IterateProposals(ctx, func(proposal types.Proposal) bool { + if proposal.HasExpiredBy(ctx.BlockTime()) { + if err := k.CloseOutProposal(ctx, proposal.ID); err != nil { + panic(err) // if an expired proposal does not close then something has gone very wrong + } + } + return false + }) +} diff --git a/x/committee/abci_test.go b/x/committee/abci_test.go new file mode 100644 index 00000000..7c64b766 --- /dev/null +++ b/x/committee/abci_test.go @@ -0,0 +1,68 @@ +package committee_test + +import ( + "testing" + "time" + + "github.com/stretchr/testify/suite" + + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/gov" + abci "github.com/tendermint/tendermint/abci/types" + + "github.com/kava-labs/kava/app" + "github.com/kava-labs/kava/x/committee" +) + +type ModuleTestSuite struct { + suite.Suite + + keeper committee.Keeper + app app.TestApp + ctx sdk.Context + + addresses []sdk.AccAddress +} + +func (suite *ModuleTestSuite) SetupTest() { + suite.app = app.NewTestApp() + suite.keeper = suite.app.GetCommitteeKeeper() + suite.ctx = suite.app.NewContext(true, abci.Header{}) + _, suite.addresses = app.GeneratePrivKeyAddressPairs(5) +} + +func (suite *ModuleTestSuite) TestBeginBlock() { + suite.app.InitializeFromGenesisStates() + // TODO replace below with genesis state + normalCom := committee.Committee{ + ID: 12, + Members: suite.addresses[:2], + Permissions: []committee.Permission{committee.GodPermission{}}, + } + suite.keeper.SetCommittee(suite.ctx, normalCom) + + pprop1 := gov.NewTextProposal("1A Title", "A description of this proposal.") + id1, err := suite.keeper.SubmitProposal(suite.ctx, normalCom.Members[0], normalCom.ID, pprop1) + suite.NoError(err) + + oneHrLaterCtx := suite.ctx.WithBlockTime(suite.ctx.BlockTime().Add(time.Hour)) + pprop2 := gov.NewTextProposal("2A Title", "A description of this proposal.") + id2, err := suite.keeper.SubmitProposal(oneHrLaterCtx, normalCom.Members[0], normalCom.ID, pprop2) + suite.NoError(err) + + // Run BeginBlocker + proposalDurationLaterCtx := suite.ctx.WithBlockTime(suite.ctx.BlockTime().Add(committee.MaxProposalDuration)) + suite.NotPanics(func() { + committee.BeginBlocker(proposalDurationLaterCtx, abci.RequestBeginBlock{}, suite.keeper) + }) + + // Check expired proposals are gone + _, found := suite.keeper.GetProposal(suite.ctx, id1) + suite.False(found, "expected expired proposal to be closed") + _, found = suite.keeper.GetProposal(suite.ctx, id2) + suite.True(found, "expected non expired proposal to be not closed") +} + +func TestModuleTestSuite(t *testing.T) { + suite.Run(t, new(ModuleTestSuite)) +} diff --git a/x/committee/alias.go b/x/committee/alias.go index 48abeee3..cd23dfbf 100644 --- a/x/committee/alias.go +++ b/x/committee/alias.go @@ -9,27 +9,35 @@ import ( const ( DefaultNextProposalID = types.DefaultNextProposalID + DefaultParamspace = types.DefaultParamspace ModuleName = types.ModuleName + QuerierRoute = types.QuerierRoute + RouterKey = types.RouterKey StoreKey = types.StoreKey + TypeMsgSubmitProposal = types.TypeMsgSubmitProposal + TypeMsgVote = types.TypeMsgVote ) var ( // function aliases - NewKeeper = keeper.NewKeeper - DefaultGenesisState = types.DefaultGenesisState - GetKeyFromID = types.GetKeyFromID - GetVoteKey = types.GetVoteKey - NewGenesisState = types.NewGenesisState - RegisterCodec = types.RegisterCodec - Uint64FromBytes = types.Uint64FromBytes + NewKeeper = keeper.NewKeeper + DefaultGenesisState = types.DefaultGenesisState + GetKeyFromID = types.GetKeyFromID + GetVoteKey = types.GetVoteKey + NewGenesisState = types.NewGenesisState + NewMsgSubmitProposal = types.NewMsgSubmitProposal + NewMsgVote = types.NewMsgVote + RegisterCodec = types.RegisterCodec + Uint64FromBytes = types.Uint64FromBytes // variable aliases - CommitteeKeyPrefix = types.CommitteeKeyPrefix - ModuleCdc = types.ModuleCdc - NextProposalIDKey = types.NextProposalIDKey - ProposalKeyPrefix = types.ProposalKeyPrefix - VoteKeyPrefix = types.VoteKeyPrefix - VoteThreshold = types.VoteThreshold + CommitteeKeyPrefix = types.CommitteeKeyPrefix + MaxProposalDuration = types.MaxProposalDuration + ModuleCdc = types.ModuleCdc + NextProposalIDKey = types.NextProposalIDKey + ProposalKeyPrefix = types.ProposalKeyPrefix + VoteKeyPrefix = types.VoteKeyPrefix + VoteThreshold = types.VoteThreshold ) type ( diff --git a/x/committee/keeper/keeper.go b/x/committee/keeper/keeper.go index 59337b39..f6ba1fee 100644 --- a/x/committee/keeper/keeper.go +++ b/x/committee/keeper/keeper.go @@ -1,10 +1,11 @@ package keeper import ( + "time" + "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/store/prefix" sdk "github.com/cosmos/cosmos-sdk/types" - govtypes "github.com/cosmos/cosmos-sdk/x/gov/types" "github.com/kava-labs/kava/x/committee/types" @@ -94,7 +95,7 @@ func (k Keeper) IncrementNextProposalID(ctx sdk.Context) sdk.Error { } // StoreNewProposal stores a proposal, adding a new ID -func (k Keeper) StoreNewProposal(ctx sdk.Context, committeeID uint64, pubProposal types.PubProposal) (uint64, sdk.Error) { +func (k Keeper) StoreNewProposal(ctx sdk.Context, pubProposal types.PubProposal, committeeID uint64, deadline time.Time) (uint64, sdk.Error) { newProposalID, err := k.GetNextProposalID(ctx) if err != nil { return 0, err @@ -103,6 +104,7 @@ func (k Keeper) StoreNewProposal(ctx sdk.Context, committeeID uint64, pubProposa PubProposal: pubProposal, ID: newProposalID, CommitteeID: committeeID, + Deadline: deadline, } k.SetProposal(ctx, proposal) @@ -139,6 +141,22 @@ func (k Keeper) DeleteProposal(ctx sdk.Context, proposalID uint64) { store.Delete(types.GetKeyFromID(proposalID)) } +// IterateProposals provides an iterator over all stored proposals. +// For each proposal, cb will be called. If cb returns true, the iterator will close and stop. +func (k Keeper) IterateProposals(ctx sdk.Context, cb func(proposal types.Proposal) (stop bool)) { + iterator := sdk.KVStorePrefixIterator(ctx.KVStore(k.storeKey), types.ProposalKeyPrefix) + + defer iterator.Close() + for ; iterator.Valid(); iterator.Next() { + var proposal types.Proposal + k.cdc.MustUnmarshalBinaryLengthPrefixed(iterator.Value(), &proposal) + + if cb(proposal) { + break + } + } +} + // ---------- Votes ---------- // GetVote gets a vote from the store. diff --git a/x/committee/keeper/proposal.go b/x/committee/keeper/proposal.go index 4aeeac82..0bd65072 100644 --- a/x/committee/keeper/proposal.go +++ b/x/committee/keeper/proposal.go @@ -27,7 +27,8 @@ func (k Keeper) SubmitProposal(ctx sdk.Context, proposer sdk.AccAddress, committ } // Get a new ID and store the proposal - return k.StoreNewProposal(ctx, committeeID, pubProposal) + deadline := ctx.BlockTime().Add(types.MaxProposalDuration) + return k.StoreNewProposal(ctx, pubProposal, committeeID, deadline) } func (k Keeper) AddVote(ctx sdk.Context, proposalID uint64, voter sdk.AccAddress) sdk.Error { @@ -36,6 +37,9 @@ func (k Keeper) AddVote(ctx sdk.Context, proposalID uint64, voter sdk.AccAddress if !found { return sdk.ErrInternal("proposal not found") } + if pr.HasExpiredBy(ctx.BlockTime()) { + return sdk.ErrInternal("proposal expired") + } com, found := k.GetCommittee(ctx, pr.CommitteeID) if !found { return sdk.ErrInternal("committee disbanded") @@ -65,7 +69,9 @@ func (k Keeper) CloseOutProposal(ctx sdk.Context, proposalID uint64) sdk.Error { votes = append(votes, vote) return false }) - if sdk.NewDec(int64(len(votes))).GTE(types.VoteThreshold.MulInt64(int64(len(com.Members)))) { // TODO move vote counting stuff to committee methods // TODO add timeout check here - close if expired regardless of votes + proposalPasses := sdk.NewDec(int64(len(votes))).GTE(types.VoteThreshold.MulInt64(int64(len(com.Members)))) + + if proposalPasses { // eneact vote // The proposal handler may execute state mutating logic depending // on the proposal content. If the handler fails, no state mutation @@ -77,16 +83,17 @@ func (k Keeper) CloseOutProposal(ctx sdk.Context, proposalID uint64) sdk.Error { // write state to the underlying multi-store writeCache() } // if handler returns error, then still delete the proposal - it's still over, but send an event + } + if proposalPasses || pr.HasExpiredBy(ctx.BlockTime()) { // delete proposal and votes k.DeleteProposal(ctx, proposalID) for _, v := range votes { k.DeleteVote(ctx, v.ProposalID, v.Voter) } - } else { - return sdk.ErrInternal("note enough votes to close proposal") + return nil } - return nil + return sdk.ErrInternal("note enough votes to close proposal") } func (k Keeper) ValidatePubProposal(ctx sdk.Context, pubProposal types.PubProposal) sdk.Error { diff --git a/x/committee/keeper/proposal_test.go b/x/committee/keeper/proposal_test.go index fe5ca6c7..0d9edcc2 100644 --- a/x/committee/keeper/proposal_test.go +++ b/x/committee/keeper/proposal_test.go @@ -2,6 +2,7 @@ package keeper_test import ( "reflect" + "time" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/gov" @@ -89,8 +90,10 @@ func (suite *KeeperTestSuite) TestSubmitProposal() { if tc.expectPass { suite.NoError(err) - _, found := keeper.GetProposal(ctx, id) + pr, found := keeper.GetProposal(ctx, id) suite.True(found) + suite.Equal(tc.committeeID, pr.CommitteeID) + suite.Equal(ctx.BlockTime().Add(types.MaxProposalDuration), pr.Deadline) } else { suite.NotNil(err) } @@ -104,11 +107,13 @@ func (suite *KeeperTestSuite) TestAddVote() { Members: suite.addresses[:2], Permissions: []types.Permission{types.GodPermission{}}, } + firstBlockTime := time.Date(1998, time.January, 1, 1, 0, 0, 0, time.UTC) testcases := []struct { name string proposalID uint64 voter sdk.AccAddress + voteTime time.Time expectPass bool }{ { @@ -129,6 +134,13 @@ func (suite *KeeperTestSuite) TestAddVote() { voter: suite.addresses[4], expectPass: false, }, + { + name: "proposal expired", + proposalID: types.DefaultNextProposalID, + voter: normalCom.Members[0], + voteTime: firstBlockTime.Add(types.MaxProposalDuration), + expectPass: false, + }, } for _, tc := range testcases { @@ -136,7 +148,7 @@ func (suite *KeeperTestSuite) TestAddVote() { // Create local testApp because suite doesn't run the SetupTest function for subtests, which would mean the app state is not be reset between subtests. tApp := app.NewTestApp() keeper := tApp.GetCommitteeKeeper() - ctx := tApp.NewContext(true, abci.Header{}) + ctx := tApp.NewContext(true, abci.Header{Height: 1, Time: firstBlockTime}) tApp.InitializeFromGenesisStates() // setup the committee and proposal @@ -144,6 +156,7 @@ func (suite *KeeperTestSuite) TestAddVote() { _, err := keeper.SubmitProposal(ctx, normalCom.Members[0], normalCom.ID, gov.NewTextProposal("A Title", "A description of this proposal.")) suite.NoError(err) + ctx = ctx.WithBlockTime(tc.voteTime) err = keeper.AddVote(ctx, tc.proposalID, tc.voter) if tc.expectPass { diff --git a/x/committee/module.go b/x/committee/module.go index 6372da75..97752b05 100644 --- a/x/committee/module.go +++ b/x/committee/module.go @@ -150,7 +150,7 @@ func (am AppModule) ExportGenesis(ctx sdk.Context) json.RawMessage { // BeginBlock module begin-block func (am AppModule) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) { - // TODO BeginBlocker(ctx, req, am.keeper) + BeginBlocker(ctx, req, am.keeper) } // EndBlock module end-block diff --git a/x/committee/types/types.go b/x/committee/types/types.go index 2930bd37..deec6e68 100644 --- a/x/committee/types/types.go +++ b/x/committee/types/types.go @@ -1,13 +1,19 @@ package types import ( + "time" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/gov" ) -// -------- Committees -------- +// TODO move these into params +var ( + VoteThreshold sdk.Dec = sdk.MustNewDecFromStr("0.75") + MaxProposalDuration time.Duration = time.Hour * 24 * 7 +) -var VoteThreshold sdk.Dec = sdk.MustNewDecFromStr("0.75") +// -------- Committees -------- // A Committee is a collection of addresses that are allowed to vote and enact any governance proposal that passes their permissions. type Committee struct { @@ -50,6 +56,13 @@ type Proposal struct { PubProposal ID uint64 CommitteeID uint64 + Deadline time.Time +} + +// HasExpiredBy calculates if the proposal will have expired by a certain time. +// All votes must be cast before deadline, those cast at time == deadline are not valid +func (p Proposal) HasExpiredBy(time time.Time) bool { + return !time.Before(p.Deadline) } type Vote struct {