Revert "Revert "Merge PR #2217: Governance BFT Time""

This reverts commit 72e9664.

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](https://github.com/cosmos/cosmos-sdk/issues/2195) Made governance use BFT Time instead of Block Heights for deposit and voting periods.
 
 * SDK
  * [core] [\#1807](https://github.com/cosmos/cosmos-sdk/issues/1807) Switch from use of rational to decimal
@@ -138,4 +139,4 @@ BUG FIXES
  * [\#2158](https://github.com/cosmos/cosmos-sdk/issues/2158) Fix non-deterministic ordering of validator iteration when slashing in `gov EndBlocker`
  * [simulation] \#1924 Make simulation stop on SIGTERM
 
-* Tendermint
+* Tendermint

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.
 }