From ebb6366837b4ce5c9234a84bb61e4fbde286f624 Mon Sep 17 00:00:00 2001 From: rhuairahrighairigh Date: Sat, 25 Apr 2020 00:22:56 +0100 Subject: [PATCH] address various pr comments --- x/committee/abci_test.go | 12 ++--- x/committee/client/rest/rest.go | 1 - x/committee/keeper/integration_test.go | 8 ++-- x/committee/keeper/keeper_test.go | 12 ++--- x/committee/keeper/proposal.go | 2 +- x/committee/keeper/proposal_test.go | 64 +++++++++++++------------- x/committee/keeper/querier_test.go | 22 ++++----- x/committee/proposal_handler_test.go | 50 ++++++++++---------- x/committee/types/genesis_test.go | 24 +++++----- x/committee/types/types.go | 35 ++++++++------ 10 files changed, 120 insertions(+), 110 deletions(-) diff --git a/x/committee/abci_test.go b/x/committee/abci_test.go index f990dd43..21e2af68 100644 --- a/x/committee/abci_test.go +++ b/x/committee/abci_test.go @@ -37,11 +37,11 @@ func (suite *ModuleTestSuite) TestBeginBlock() { suite.app.InitializeFromGenesisStates() normalCom := committee.Committee{ - ID: 12, - Members: suite.addresses[:2], - Permissions: []committee.Permission{committee.GodPermission{}}, - VoteThreshold: d("0.8"), - MaxProposalDuration: time.Hour * 24 * 7, + ID: 12, + Members: suite.addresses[:2], + Permissions: []committee.Permission{committee.GodPermission{}}, + VoteThreshold: d("0.8"), + ProposalDuration: time.Hour * 24 * 7, } suite.keeper.SetCommittee(suite.ctx, normalCom) @@ -55,7 +55,7 @@ func (suite *ModuleTestSuite) TestBeginBlock() { suite.NoError(err) // Run BeginBlocker - proposalDurationLaterCtx := suite.ctx.WithBlockTime(suite.ctx.BlockTime().Add(normalCom.MaxProposalDuration)) + proposalDurationLaterCtx := suite.ctx.WithBlockTime(suite.ctx.BlockTime().Add(normalCom.ProposalDuration)) suite.NotPanics(func() { committee.BeginBlocker(proposalDurationLaterCtx, abci.RequestBeginBlock{}, suite.keeper) }) diff --git a/x/committee/client/rest/rest.go b/x/committee/client/rest/rest.go index a172adba..96849d09 100644 --- a/x/committee/client/rest/rest.go +++ b/x/committee/client/rest/rest.go @@ -10,7 +10,6 @@ import ( const ( RestProposalID = "proposal-id" RestCommitteeID = "committee-id" - RestVoter = "voter" ) // RegisterRoutes - Central function to define routes that get registered by the main application diff --git a/x/committee/keeper/integration_test.go b/x/committee/keeper/integration_test.go index 47185ea0..e681c6c8 100644 --- a/x/committee/keeper/integration_test.go +++ b/x/committee/keeper/integration_test.go @@ -2,16 +2,18 @@ package keeper_test import ( sdk "github.com/cosmos/cosmos-sdk/types" + + "github.com/kava-labs/kava/x/committee/keeper" "github.com/kava-labs/kava/x/committee/types" ) // proposalVoteMap collects up votes into a map indexed by proposalID func getProposalVoteMap(k keeper.Keeper, ctx sdk.Context) map[uint64]([]types.Vote) { - proposalVoteMap = map[uint64]([]types.Vote){} + proposalVoteMap := map[uint64]([]types.Vote){} - keeper.IterateProposals(suite.ctx, func(p types.Proposal) bool { - keeper.IterateVotes(suite.ctx, p.ID, func(v types.Vote) bool { + k.IterateProposals(ctx, func(p types.Proposal) bool { + k.IterateVotes(ctx, p.ID, func(v types.Vote) bool { proposalVoteMap[p.ID] = append(proposalVoteMap[p.ID], v) return false }) diff --git a/x/committee/keeper/keeper_test.go b/x/committee/keeper/keeper_test.go index c317d8d7..9144f449 100644 --- a/x/committee/keeper/keeper_test.go +++ b/x/committee/keeper/keeper_test.go @@ -37,12 +37,12 @@ func (suite *KeeperTestSuite) SetupTest() { func (suite *KeeperTestSuite) TestGetSetDeleteCommittee() { // setup test com := types.Committee{ - ID: 12, - Description: "This committee is for testing.", - Members: suite.addresses, - Permissions: []types.Permission{types.GodPermission{}}, - VoteThreshold: d("0.667"), - MaxProposalDuration: time.Hour * 24 * 7, + ID: 12, + Description: "This committee is for testing.", + Members: suite.addresses, + Permissions: []types.Permission{types.GodPermission{}}, + VoteThreshold: d("0.667"), + ProposalDuration: time.Hour * 24 * 7, } // write and read from store diff --git a/x/committee/keeper/proposal.go b/x/committee/keeper/proposal.go index f93fa78f..66cfddb4 100644 --- a/x/committee/keeper/proposal.go +++ b/x/committee/keeper/proposal.go @@ -30,7 +30,7 @@ func (k Keeper) SubmitProposal(ctx sdk.Context, proposer sdk.AccAddress, committ } // Get a new ID and store the proposal - deadline := ctx.BlockTime().Add(com.MaxProposalDuration) + deadline := ctx.BlockTime().Add(com.ProposalDuration) proposalID, err := k.StoreNewProposal(ctx, pubProposal, committeeID, deadline) if err != nil { return 0, err diff --git a/x/committee/keeper/proposal_test.go b/x/committee/keeper/proposal_test.go index 91754d5e..7f77c847 100644 --- a/x/committee/keeper/proposal_test.go +++ b/x/committee/keeper/proposal_test.go @@ -17,12 +17,12 @@ import ( func (suite *KeeperTestSuite) TestSubmitProposal() { normalCom := types.Committee{ - ID: 12, - Description: "This committee is for testing.", - Members: suite.addresses[:2], - Permissions: []types.Permission{types.GodPermission{}}, - VoteThreshold: d("0.667"), - MaxProposalDuration: time.Hour * 24 * 7, + ID: 12, + Description: "This committee is for testing.", + Members: suite.addresses[:2], + Permissions: []types.Permission{types.GodPermission{}}, + VoteThreshold: d("0.667"), + ProposalDuration: time.Hour * 24 * 7, } noPermissionsCom := normalCom noPermissionsCom.Permissions = []types.Permission{} @@ -96,7 +96,7 @@ func (suite *KeeperTestSuite) TestSubmitProposal() { pr, found := keeper.GetProposal(ctx, id) suite.True(found) suite.Equal(tc.committeeID, pr.CommitteeID) - suite.Equal(ctx.BlockTime().Add(tc.committee.MaxProposalDuration), pr.Deadline) + suite.Equal(ctx.BlockTime().Add(tc.committee.ProposalDuration), pr.Deadline) } else { suite.NotNil(err) } @@ -141,7 +141,7 @@ func (suite *KeeperTestSuite) TestAddVote() { name: "proposal expired", proposalID: types.DefaultNextProposalID, voter: normalCom.Members[0], - voteTime: firstBlockTime.Add(normalCom.MaxProposalDuration), + voteTime: firstBlockTime.Add(normalCom.ProposalDuration), expectPass: false, }, } @@ -175,12 +175,12 @@ func (suite *KeeperTestSuite) TestAddVote() { func (suite *KeeperTestSuite) TestGetProposalResult() { normalCom := types.Committee{ - ID: 12, - Description: "This committee is for testing.", - Members: suite.addresses[:5], - Permissions: []types.Permission{types.GodPermission{}}, - VoteThreshold: d("0.667"), - MaxProposalDuration: time.Hour * 24 * 7, + ID: 12, + Description: "This committee is for testing.", + Members: suite.addresses[:5], + Permissions: []types.Permission{types.GodPermission{}}, + VoteThreshold: d("0.667"), + ProposalDuration: time.Hour * 24 * 7, } var defaultID uint64 = 1 firstBlockTime := time.Date(1998, time.January, 1, 1, 0, 0, 0, time.UTC) @@ -327,23 +327,23 @@ func (suite *KeeperTestSuite) TestCloseExpiredProposals() { // Setup test state firstBlockTime := time.Date(1998, time.January, 1, 1, 0, 0, 0, time.UTC) - testGenesis = types.NewGenesisState( + testGenesis := types.NewGenesisState( 3, []types.Committee{ { - ID: 1, - Description: "This committee is for testing.", - Members: suite.addresses[:3], - Permissions: []types.Permission{types.GodPermission{}}, - VoteThreshold: d("0.667"), - MaxProposalDuration: time.Hour * 24 * 7, + ID: 1, + Description: "This committee is for testing.", + Members: suite.addresses[:3], + Permissions: []types.Permission{types.GodPermission{}}, + VoteThreshold: d("0.667"), + ProposalDuration: time.Hour * 24 * 7, }, { - ID: 2, - Members: suite.addresses[2:], - Permissions: nil, - VoteThreshold: d("0.667"), - MaxProposalDuration: time.Hour * 24 * 7, + ID: 2, + Members: suite.addresses[2:], + Permissions: nil, + VoteThreshold: d("0.667"), + ProposalDuration: time.Hour * 24 * 7, }, }, []types.Proposal{ @@ -367,16 +367,16 @@ func (suite *KeeperTestSuite) TestCloseExpiredProposals() { }, ) suite.app.InitializeFromGenesisStates( - NewCommitteeGenesisState(suite.cdc, testGenesis), + NewCommitteeGenesisState(suite.app.Codec(), testGenesis), ) // close proposals - ctx := tApp.NewContext(true, abci.Header{Height: 1, Time: firstBlockTime}) + ctx := suite.app.NewContext(true, abci.Header{Height: 1, Time: firstBlockTime}) suite.keeper.CloseExpiredProposals(ctx) // check for _, p := range testGenesis.Proposals { - _, found := k.GetProposal(ctx, p.ID) + _, found := suite.keeper.GetProposal(ctx, p.ID) votes := getProposalVoteMap(suite.keeper, ctx) if ctx.BlockTime().After(p.Deadline) { @@ -389,15 +389,15 @@ func (suite *KeeperTestSuite) TestCloseExpiredProposals() { } // close (later time) - ctx := tApp.NewContext(true, abci.Header{Height: 1, Time: firstBlockTime.Add(7 * 24 * time.Hour)}) + ctx = suite.app.NewContext(true, abci.Header{Height: 1, Time: firstBlockTime.Add(7 * 24 * time.Hour)}) suite.keeper.CloseExpiredProposals(ctx) // check for _, p := range testGenesis.Proposals { - _, found := k.GetProposal(ctx, p.ID) + _, found := suite.keeper.GetProposal(ctx, p.ID) votes := getProposalVoteMap(suite.keeper, ctx) - if ctx.BlockTime().After(p.Deadline) { + if ctx.BlockTime().Equal(p.Deadline) || ctx.BlockTime().After(p.Deadline) { suite.False(found) suite.Empty(votes[p.ID]) } else { diff --git a/x/committee/keeper/querier_test.go b/x/committee/keeper/querier_test.go index 383fb4d1..0514151a 100644 --- a/x/committee/keeper/querier_test.go +++ b/x/committee/keeper/querier_test.go @@ -55,19 +55,19 @@ func (suite *QuerierTestSuite) SetupTest() { 3, []types.Committee{ { - ID: 1, - Description: "This committee is for testing.", - Members: suite.addresses[:3], - Permissions: []types.Permission{types.GodPermission{}}, - VoteThreshold: d("0.667"), - MaxProposalDuration: time.Hour * 24 * 7, + ID: 1, + Description: "This committee is for testing.", + Members: suite.addresses[:3], + Permissions: []types.Permission{types.GodPermission{}}, + VoteThreshold: d("0.667"), + ProposalDuration: time.Hour * 24 * 7, }, { - ID: 2, - Members: suite.addresses[2:], - Permissions: nil, - VoteThreshold: d("0.667"), - MaxProposalDuration: time.Hour * 24 * 7, + ID: 2, + Members: suite.addresses[2:], + Permissions: nil, + VoteThreshold: d("0.667"), + ProposalDuration: time.Hour * 24 * 7, }, }, []types.Proposal{ diff --git a/x/committee/proposal_handler_test.go b/x/committee/proposal_handler_test.go index f9c58263..43da7eb2 100644 --- a/x/committee/proposal_handler_test.go +++ b/x/committee/proposal_handler_test.go @@ -38,19 +38,19 @@ func (suite *ProposalHandlerTestSuite) SetupTest() { 2, []committee.Committee{ { - ID: 1, - Description: "This committee is for testing.", - Members: suite.addresses[:3], - Permissions: []types.Permission{types.GodPermission{}}, - VoteThreshold: d("0.667"), - MaxProposalDuration: time.Hour * 24 * 7, + ID: 1, + Description: "This committee is for testing.", + Members: suite.addresses[:3], + Permissions: []types.Permission{types.GodPermission{}}, + VoteThreshold: d("0.667"), + ProposalDuration: time.Hour * 24 * 7, }, { - ID: 2, - Members: suite.addresses[2:], - Permissions: nil, - VoteThreshold: d("0.667"), - MaxProposalDuration: time.Hour * 24 * 7, + ID: 2, + Members: suite.addresses[2:], + Permissions: nil, + VoteThreshold: d("0.667"), + ProposalDuration: time.Hour * 24 * 7, }, }, []committee.Proposal{ @@ -74,10 +74,10 @@ func (suite *ProposalHandlerTestSuite) TestProposalHandler_ChangeCommittee() { "A Title", "A proposal description.", committee.Committee{ - ID: 34, - Members: suite.addresses[:1], - VoteThreshold: d("1"), - MaxProposalDuration: time.Hour * 24, + ID: 34, + Members: suite.addresses[:1], + VoteThreshold: d("1"), + ProposalDuration: time.Hour * 24, }, ), expectPass: true, @@ -88,11 +88,11 @@ func (suite *ProposalHandlerTestSuite) TestProposalHandler_ChangeCommittee() { "A Title", "A proposal description.", committee.Committee{ - ID: suite.testGenesis.Committees[0].ID, - Members: suite.addresses, // add new members - Permissions: suite.testGenesis.Committees[0].Permissions, - VoteThreshold: suite.testGenesis.Committees[0].VoteThreshold, - MaxProposalDuration: suite.testGenesis.Committees[0].MaxProposalDuration, + ID: suite.testGenesis.Committees[0].ID, + Members: suite.addresses, // add new members + Permissions: suite.testGenesis.Committees[0].Permissions, + VoteThreshold: suite.testGenesis.Committees[0].VoteThreshold, + ProposalDuration: suite.testGenesis.Committees[0].ProposalDuration, }, ), expectPass: true, @@ -112,11 +112,11 @@ func (suite *ProposalHandlerTestSuite) TestProposalHandler_ChangeCommittee() { "A Title", "A proposal description.", committee.Committee{ - ID: suite.testGenesis.Committees[0].ID, - Members: append(suite.addresses, suite.addresses[0]), // duplicate address - Permissions: suite.testGenesis.Committees[0].Permissions, - VoteThreshold: suite.testGenesis.Committees[0].VoteThreshold, - MaxProposalDuration: suite.testGenesis.Committees[0].MaxProposalDuration, + ID: suite.testGenesis.Committees[0].ID, + Members: append(suite.addresses, suite.addresses[0]), // duplicate address + Permissions: suite.testGenesis.Committees[0].Permissions, + VoteThreshold: suite.testGenesis.Committees[0].VoteThreshold, + ProposalDuration: suite.testGenesis.Committees[0].ProposalDuration, }, ), expectPass: false, diff --git a/x/committee/types/genesis_test.go b/x/committee/types/genesis_test.go index 009b5371..7fde2202 100644 --- a/x/committee/types/genesis_test.go +++ b/x/committee/types/genesis_test.go @@ -25,20 +25,20 @@ func TestGenesisState_Validate(t *testing.T) { NextProposalID: 2, Committees: []Committee{ { - ID: 1, - Description: "This committee is for testing.", - Members: addresses[:3], - Permissions: []Permission{GodPermission{}}, - VoteThreshold: d("0.667"), - MaxProposalDuration: time.Hour * 24 * 7, + ID: 1, + Description: "This committee is for testing.", + Members: addresses[:3], + Permissions: []Permission{GodPermission{}}, + VoteThreshold: d("0.667"), + ProposalDuration: time.Hour * 24 * 7, }, { - ID: 2, - Description: "This committee is also for testing.", - Members: addresses[2:], - Permissions: nil, - VoteThreshold: d("0.8"), - MaxProposalDuration: time.Hour * 24 * 21, + ID: 2, + Description: "This committee is also for testing.", + Members: addresses[2:], + Permissions: nil, + VoteThreshold: d("0.8"), + ProposalDuration: time.Hour * 24 * 21, }, }, Proposals: []Proposal{ diff --git a/x/committee/types/types.go b/x/committee/types/types.go index b5e46211..4f705a22 100644 --- a/x/committee/types/types.go +++ b/x/committee/types/types.go @@ -17,22 +17,22 @@ const MaxCommitteeDescriptionLength int = 5000 // A Committee is a collection of addresses that are allowed to vote and enact any governance proposal that passes their permissions. type Committee struct { - ID uint64 `json:"id" yaml:"id"` - Description string `json:"description" yaml:"description"` - Members []sdk.AccAddress `json:"members" yaml:"members"` - Permissions []Permission `json:"permissions" yaml:"permissions"` - VoteThreshold sdk.Dec `json:"vote_threshold" yaml:"vote_threshold"` - MaxProposalDuration time.Duration `json:"max_proposal_duration" yaml:"max_proposal_duration"` + ID uint64 `json:"id" yaml:"id"` + Description string `json:"description" yaml:"description"` + Members []sdk.AccAddress `json:"members" yaml:"members"` + Permissions []Permission `json:"permissions" yaml:"permissions"` + VoteThreshold sdk.Dec `json:"vote_threshold" yaml:"vote_threshold"` + ProposalDuration time.Duration `json:"proposal_duration" yaml:"proposal_duration"` } func NewCommittee(id uint64, description string, members []sdk.AccAddress, permissions []Permission, threshold sdk.Dec, duration time.Duration) Committee { return Committee{ - ID: id, - Description: description, - Members: members, - Permissions: permissions, - VoteThreshold: threshold, - MaxProposalDuration: duration, + ID: id, + Description: description, + Members: members, + Permissions: permissions, + VoteThreshold: threshold, + ProposalDuration: duration, } } @@ -84,7 +84,7 @@ func (c Committee) Validate() error { return fmt.Errorf("invalid threshold") } - if c.MaxProposalDuration < 0 { + if c.ProposalDuration < 0 { return fmt.Errorf("invalid time") } @@ -110,6 +110,15 @@ type Proposal struct { Deadline time.Time `json:"deadline" yaml:"deadline"` } +func NewProposal(pubProposal PubProposal, id uint64, committeeID uint64, deadline time.Time) Proposal { + return Proposal{ + PubProposal: pubProposal, + ID: id, + CommitteeID: committeeID, + Deadline: deadline, + } +} + // 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 {