Merge PR #2355: Governance BFT Time 2: Electric Boogaloo

PENDING.md

@@ -34,6 +34,7 @@ BREAKING CHANGES
       * `cosmosaccaddr` / `cosmosaccpub` => `cosmos` / `cosmospub`
       * `cosmosvaladdr` / `cosmosvalpub` => `cosmosvaloper` / `cosmosvaloperpub`
     * [x/stake] [#1013] TendermintUpdates now uses transient store
+    * [x/gov] [#2195] Governance uses BFT Time
 
 * SDK
     * [core] [\#1807](https://github.com/cosmos/cosmos-sdk/issues/1807) Switch from use of rational to decimal

docs/spec/governance/state.md

@@ -13,13 +13,13 @@ has to be created and the previous one rendered inactive.
 ```go
 type DepositProcedure struct {
   MinDeposit        sdk.Coins           //  Minimum deposit for a proposal to enter voting period. 
-  MaxDepositPeriod  int64               //  Maximum period for Atom holders to deposit on a proposal. Initial value: 2 months
+  MaxDepositPeriod  time.Time               //  Maximum period for Atom holders to deposit on a proposal. Initial value: 2 months
 }
 ```
 
 ```go
 type VotingProcedure struct {
-  VotingPeriod      int64               //  Length of the voting period. Initial value: 2 weeks
+  VotingPeriod      time.Time               //  Length of the voting period. Initial value: 2 weeks
 }
 ```
 
@@ -28,7 +28,7 @@ type TallyingProcedure struct {
   Threshold         sdk.Dec   //  Minimum propotion of Yes votes for proposal to pass. Initial value: 0.5
   Veto              sdk.Dec   //  Minimum proportion of Veto votes to Total votes ratio for proposal to be vetoed. Initial value: 1/3
   GovernancePenalty sdk.Dec             //  Penalty if validator does not vote
-  GracePeriod       int64               //  If validator entered validator set in this period of blocks before vote ended, governance penalty does not apply
+  GracePeriod       time.Time               //  If validator entered validator set in this period of blocks before vote ended, governance penalty does not apply
 }
 ```
 
@@ -97,10 +97,10 @@ type Proposal struct {
   Type                  ProposalType        //  Type of proposal. Initial set {PlainTextProposal, SoftwareUpgradeProposal}
   TotalDeposit          sdk.Coins           //  Current deposit on this proposal. Initial value is set at InitialDeposit
   Deposits              []Deposit           //  List of deposits on the proposal
-  SubmitBlock           int64               //  Height of the block where TxGovSubmitProposal was included
+  SubmitTime           time.Time               //  Time of the block where TxGovSubmitProposal was included
   Submitter             sdk.Address      //  Address of the submitter
 
-  VotingStartBlock      int64               //  Height of the block where MinDeposit was reached. -1 if MinDeposit is not reached
+  VotingStartTime      time.Time               //  Time of the block where MinDeposit was reached. time.Time{} if MinDeposit is not reached
   CurrentStatus         ProposalStatus      //  Current status of the proposal
 
   YesVotes              sdk.Dec
@@ -137,7 +137,7 @@ For pseudocode purposes, here are the two function we will use to read or write
 * `ProposalProcessingQueue`: A queue `queue[proposalID]` containing all the 
   `ProposalIDs` of proposals that reached `MinDeposit`. Each round, the oldest 
   element of `ProposalProcessingQueue` is checked during `BeginBlock` to see if
-  `CurrentBlock == VotingStartBlock + activeProcedure.VotingPeriod`. If it is, 
+  `CurrentTime == VotingStartTime + activeProcedure.VotingPeriod`. If it is, 
   then the application tallies the votes, compute the votes of each validator and checks if every validator in the valdiator set have voted
   and, if not, applies `GovernancePenalty`. If the proposal is accepted, deposits are refunded.
   After that proposal is ejected from `ProposalProcessingQueue` and the next element of the queue is evaluated. 
@@ -159,7 +159,7 @@ And the pseudocode for the `ProposalProcessingQueue`:
     proposal = load(Governance, <proposalID|'proposal'>) // proposal is a const key
     votingProcedure = load(GlobalParams, 'VotingProcedure')
 
-    if (CurrentBlock == proposal.VotingStartBlock + votingProcedure.VotingPeriod && proposal.CurrentStatus == ProposalStatusActive)
+    if (CurrentTime == proposal.VotingStartTime + votingProcedure.VotingPeriod && proposal.CurrentStatus == ProposalStatusActive)
 
     // End of voting period, tally
 
@@ -194,7 +194,7 @@ And the pseudocode for the `ProposalProcessingQueue`:
 
       // Slash validators that did not vote, or update tally if they voted
       for each validator in validators
-        if (validator.bondHeight < CurrentBlock - tallyingProcedure.GracePeriod)
+        if (validator.bondTime < CurrentTime - tallyingProcedure.GracePeriod)
         // only slash if validator entered validator set before grace period
           if (!tmpValMap(validator).HasVoted)
             slash validator by tallyingProcedure.GovernancePenalty

x/gov/endblocker_test.go

@@ -2,6 +2,7 @@ package gov
 
 import (
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/require"
 
@@ -28,12 +29,18 @@ func TestTickExpiredDepositPeriod(t *testing.T) {
 	require.NotNil(t, keeper.InactiveProposalQueuePeek(ctx))
 	require.False(t, shouldPopInactiveProposalQueue(ctx, keeper))
 
-	ctx = ctx.WithBlockHeight(10)
+	newHeader := ctx.BlockHeader()
+	newHeader.Time = ctx.BlockHeader().Time.Add(time.Duration(1) * time.Second)
+	ctx = ctx.WithBlockHeader(newHeader)
+
 	EndBlocker(ctx, keeper)
 	require.NotNil(t, keeper.InactiveProposalQueuePeek(ctx))
 	require.False(t, shouldPopInactiveProposalQueue(ctx, keeper))
 
-	ctx = ctx.WithBlockHeight(250)
+	newHeader = ctx.BlockHeader()
+	newHeader.Time = ctx.BlockHeader().Time.Add(keeper.GetDepositProcedure(ctx).MaxDepositPeriod)
+	ctx = ctx.WithBlockHeader(newHeader)
+
 	require.NotNil(t, keeper.InactiveProposalQueuePeek(ctx))
 	require.True(t, shouldPopInactiveProposalQueue(ctx, keeper))
 	EndBlocker(ctx, keeper)
@@ -59,7 +66,10 @@ func TestTickMultipleExpiredDepositPeriod(t *testing.T) {
 	require.NotNil(t, keeper.InactiveProposalQueuePeek(ctx))
 	require.False(t, shouldPopInactiveProposalQueue(ctx, keeper))
 
-	ctx = ctx.WithBlockHeight(10)
+	newHeader := ctx.BlockHeader()
+	newHeader.Time = ctx.BlockHeader().Time.Add(time.Duration(2) * time.Second)
+	ctx = ctx.WithBlockHeader(newHeader)
+
 	EndBlocker(ctx, keeper)
 	require.NotNil(t, keeper.InactiveProposalQueuePeek(ctx))
 	require.False(t, shouldPopInactiveProposalQueue(ctx, keeper))
@@ -68,14 +78,20 @@ func TestTickMultipleExpiredDepositPeriod(t *testing.T) {
 	res = govHandler(ctx, newProposalMsg2)
 	require.True(t, res.IsOK())
 
-	ctx = ctx.WithBlockHeight(205)
+	newHeader = ctx.BlockHeader()
+	newHeader.Time = ctx.BlockHeader().Time.Add(keeper.GetDepositProcedure(ctx).MaxDepositPeriod).Add(time.Duration(-1) * time.Second)
+	ctx = ctx.WithBlockHeader(newHeader)
+
 	require.NotNil(t, keeper.InactiveProposalQueuePeek(ctx))
 	require.True(t, shouldPopInactiveProposalQueue(ctx, keeper))
 	EndBlocker(ctx, keeper)
 	require.NotNil(t, keeper.InactiveProposalQueuePeek(ctx))
 	require.False(t, shouldPopInactiveProposalQueue(ctx, keeper))
 
-	ctx = ctx.WithBlockHeight(215)
+	newHeader = ctx.BlockHeader()
+	newHeader.Time = ctx.BlockHeader().Time.Add(time.Duration(5) * time.Second)
+	ctx = ctx.WithBlockHeader(newHeader)
+
 	require.NotNil(t, keeper.InactiveProposalQueuePeek(ctx))
 	require.True(t, shouldPopInactiveProposalQueue(ctx, keeper))
 	EndBlocker(ctx, keeper)
@@ -105,7 +121,10 @@ func TestTickPassedDepositPeriod(t *testing.T) {
 	require.NotNil(t, keeper.InactiveProposalQueuePeek(ctx))
 	require.False(t, shouldPopInactiveProposalQueue(ctx, keeper))
 
-	ctx = ctx.WithBlockHeight(10)
+	newHeader := ctx.BlockHeader()
+	newHeader.Time = ctx.BlockHeader().Time.Add(time.Duration(1) * time.Second)
+	ctx = ctx.WithBlockHeader(newHeader)
+
 	EndBlocker(ctx, keeper)
 	require.NotNil(t, keeper.InactiveProposalQueuePeek(ctx))
 	require.False(t, shouldPopInactiveProposalQueue(ctx, keeper))
@@ -146,14 +165,20 @@ func TestTickPassedVotingPeriod(t *testing.T) {
 	var proposalID int64
 	keeper.cdc.UnmarshalBinaryBare(res.Data, &proposalID)
 
-	ctx = ctx.WithBlockHeight(10)
+	newHeader := ctx.BlockHeader()
+	newHeader.Time = ctx.BlockHeader().Time.Add(time.Duration(1) * time.Second)
+	ctx = ctx.WithBlockHeader(newHeader)
+
 	newDepositMsg := NewMsgDeposit(addrs[1], proposalID, sdk.Coins{sdk.NewInt64Coin("steak", 5)})
 	res = govHandler(ctx, newDepositMsg)
 	require.True(t, res.IsOK())
 
 	EndBlocker(ctx, keeper)
 
-	ctx = ctx.WithBlockHeight(215)
+	newHeader = ctx.BlockHeader()
+	newHeader.Time = ctx.BlockHeader().Time.Add(keeper.GetDepositProcedure(ctx).MaxDepositPeriod).Add(keeper.GetDepositProcedure(ctx).MaxDepositPeriod)
+	ctx = ctx.WithBlockHeader(newHeader)
+
 	require.True(t, shouldPopActiveProposalQueue(ctx, keeper))
 	depositsIterator := keeper.GetDeposits(ctx, proposalID)
 	require.True(t, depositsIterator.Valid())
@@ -197,7 +222,10 @@ func TestSlashing(t *testing.T) {
 	var proposalID int64
 	keeper.cdc.UnmarshalBinaryBare(res.Data, &proposalID)
 
-	ctx = ctx.WithBlockHeight(10)
+	newHeader := ctx.BlockHeader()
+	newHeader.Time = ctx.BlockHeader().Time.Add(time.Duration(1) * time.Second)
+	ctx = ctx.WithBlockHeader(newHeader)
+
 	require.Equal(t, StatusVotingPeriod, keeper.GetProposal(ctx, proposalID).GetStatus())
 
 	newVoteMsg := NewMsgVote(addrs[0], proposalID, OptionYes)
@@ -206,7 +234,10 @@ func TestSlashing(t *testing.T) {
 
 	EndBlocker(ctx, keeper)
 
-	ctx = ctx.WithBlockHeight(215)
+	newHeader = ctx.BlockHeader()
+	newHeader.Time = ctx.BlockHeader().Time.Add(keeper.GetDepositProcedure(ctx).MaxDepositPeriod).Add(keeper.GetDepositProcedure(ctx).MaxDepositPeriod)
+	ctx = ctx.WithBlockHeader(newHeader)
+
 	require.Equal(t, StatusVotingPeriod, keeper.GetProposal(ctx, proposalID).GetStatus())
 
 	EndBlocker(ctx, keeper)

x/gov/genesis.go

@@ -1,6 +1,8 @@
 package gov
 
 import (
+	"time"
+
 	sdk "github.com/cosmos/cosmos-sdk/types"
 )
 
@@ -27,10 +29,10 @@ func DefaultGenesisState() GenesisState {
 		StartingProposalID: 1,
 		DepositProcedure: DepositProcedure{
 			MinDeposit:       sdk.Coins{sdk.NewInt64Coin("steak", 10)},
-			MaxDepositPeriod: 200,
+			MaxDepositPeriod: time.Duration(172800) * time.Second,
 		},
 		VotingProcedure: VotingProcedure{
-			VotingPeriod: 200,
+			VotingPeriod: time.Duration(172800) * time.Second,
 		},
 		TallyingProcedure: TallyingProcedure{
 			Threshold:         sdk.NewDecWithPrec(5, 1),

x/gov/handler.go

@@ -128,9 +128,9 @@ func EndBlocker(ctx sdk.Context, keeper Keeper) (resTags sdk.Tags) {
 	for shouldPopActiveProposalQueue(ctx, keeper) {
 		activeProposal := keeper.ActiveProposalQueuePop(ctx)
 
-		proposalStartBlock := activeProposal.GetVotingStartBlock()
+		proposalStartTime := activeProposal.GetVotingStartTime()
 		votingPeriod := keeper.GetVotingProcedure(ctx).VotingPeriod
-		if ctx.BlockHeight() < proposalStartBlock+votingPeriod {
+		if ctx.BlockHeader().Time.Before(proposalStartTime.Add(votingPeriod)) {
 			continue
 		}
 
@@ -178,7 +178,7 @@ func shouldPopInactiveProposalQueue(ctx sdk.Context, keeper Keeper) bool {
 		return false
 	} else if peekProposal.GetStatus() != StatusDepositPeriod {
 		return true
-	} else if ctx.BlockHeight() >= peekProposal.GetSubmitBlock()+depositProcedure.MaxDepositPeriod {
+	} else if !ctx.BlockHeader().Time.Before(peekProposal.GetSubmitTime().Add(depositProcedure.MaxDepositPeriod)) {
 		return true
 	}
 	return false
@@ -190,7 +190,7 @@ func shouldPopActiveProposalQueue(ctx sdk.Context, keeper Keeper) bool {
 
 	if peekProposal == nil {
 		return false
-	} else if ctx.BlockHeight() >= peekProposal.GetVotingStartBlock()+votingProcedure.VotingPeriod {
+	} else if !ctx.BlockHeader().Time.Before(peekProposal.GetVotingStartTime().Add(votingProcedure.VotingPeriod)) {
 		return true
 	}
 	return false

x/gov/keeper.go

@@ -66,15 +66,14 @@ func (keeper Keeper) NewTextProposal(ctx sdk.Context, title string, description
 		return nil
 	}
 	var proposal Proposal = &TextProposal{
-		ProposalID:       proposalID,
-		Title:            title,
-		Description:      description,
-		ProposalType:     proposalType,
-		Status:           StatusDepositPeriod,
-		TallyResult:      EmptyTallyResult(),
-		TotalDeposit:     sdk.Coins{},
-		SubmitBlock:      ctx.BlockHeight(),
-		VotingStartBlock: -1, // TODO: Make Time
+		ProposalID:   proposalID,
+		Title:        title,
+		Description:  description,
+		ProposalType: proposalType,
+		Status:       StatusDepositPeriod,
+		TallyResult:  EmptyTallyResult(),
+		TotalDeposit: sdk.Coins{},
+		SubmitTime:   ctx.BlockHeader().Time,
 	}
 	keeper.SetProposal(ctx, proposal)
 	keeper.InactiveProposalQueuePush(ctx, proposal)
@@ -200,7 +199,7 @@ func (keeper Keeper) peekCurrentProposalID(ctx sdk.Context) (proposalID int64, e
 }
 
 func (keeper Keeper) activateVotingPeriod(ctx sdk.Context, proposal Proposal) {
-	proposal.SetVotingStartBlock(ctx.BlockHeight())
+	proposal.SetVotingStartTime(ctx.BlockHeader().Time)
 	proposal.SetStatus(StatusVotingPeriod)
 	keeper.SetProposal(ctx, proposal)
 	keeper.ActiveProposalQueuePush(ctx, proposal)

x/gov/keeper_test.go

@@ -2,6 +2,7 @@ package gov
 
 import (
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/require"
 
@@ -45,12 +46,12 @@ func TestActivateVotingPeriod(t *testing.T) {
 
 	proposal := keeper.NewTextProposal(ctx, "Test", "description", ProposalTypeText)
 
-	require.Equal(t, int64(-1), proposal.GetVotingStartBlock())
+	require.True(t, proposal.GetVotingStartTime().Equal(time.Time{}))
 	require.Nil(t, keeper.ActiveProposalQueuePeek(ctx))
 
 	keeper.activateVotingPeriod(ctx, proposal)
 
-	require.Equal(t, proposal.GetVotingStartBlock(), ctx.BlockHeight())
+	require.True(t, proposal.GetVotingStartTime().Equal(ctx.BlockHeader().Time))
 	require.Equal(t, proposal.GetProposalID(), keeper.ActiveProposalQueuePeek(ctx).GetProposalID())
 }
 
@@ -77,7 +78,7 @@ func TestDeposits(t *testing.T) {
 	// Check no deposits at beginning
 	deposit, found := keeper.GetDeposit(ctx, proposalID, addrs[1])
 	require.False(t, found)
-	require.Equal(t, keeper.GetProposal(ctx, proposalID).GetVotingStartBlock(), int64(-1))
+	require.True(t, keeper.GetProposal(ctx, proposalID).GetVotingStartTime().Equal(time.Time{}))
 	require.Nil(t, keeper.ActiveProposalQueuePeek(ctx))
 
 	// Check first deposit
@@ -114,7 +115,7 @@ func TestDeposits(t *testing.T) {
 	require.Equal(t, addr1Initial.Minus(fourSteak), keeper.ck.GetCoins(ctx, addrs[1]))
 
 	// Check that proposal moved to voting period
-	require.Equal(t, ctx.BlockHeight(), keeper.GetProposal(ctx, proposalID).GetVotingStartBlock())
+	require.True(t, keeper.GetProposal(ctx, proposalID).GetVotingStartTime().Equal(ctx.BlockHeader().Time))
 	require.NotNil(t, keeper.ActiveProposalQueuePeek(ctx))
 	require.Equal(t, proposalID, keeper.ActiveProposalQueuePeek(ctx).GetProposalID())
 

x/gov/procedures.go

@@ -1,13 +1,15 @@
 package gov
 
 import (
+	"time"
+
 	sdk "github.com/cosmos/cosmos-sdk/types"
 )
 
 // Procedure around Deposits for governance
 type DepositProcedure struct {
-	MinDeposit       sdk.Coins `json:"min_deposit"`        //  Minimum deposit for a proposal to enter voting period.
-	MaxDepositPeriod int64     `json:"max_deposit_period"` //  Maximum period for Atom holders to deposit on a proposal. Initial value: 2 months
+	MinDeposit       sdk.Coins     `json:"min_deposit"`        //  Minimum deposit for a proposal to enter voting period.
+	MaxDepositPeriod time.Duration `json:"max_deposit_period"` //  Maximum period for Atom holders to deposit on a proposal. Initial value: 2 months
 }
 
 // Procedure around Tallying votes in governance
@@ -19,5 +21,5 @@ type TallyingProcedure struct {
 
 // Procedure around Voting in governance
 type VotingProcedure struct {
-	VotingPeriod int64 `json:"voting_period"` //  Length of the voting period.
+	VotingPeriod time.Duration `json:"voting_period"` //  Length of the voting period.
 }