Switch gov proposal-queues to use iterators (#2638)

* switched gov proposals queue to use iterators
* update gov spec
* update proposal.Equal
* Amino api change
* switched proposalID to uint64
* renamed Gov Procedures to Params
* s/ActiveProposalQueueProposalKey/KeyActiveProposalQueueProposal/g
* numLatestProposals -> Limit
* fixed staking invariant breakage because of gov deposits
* Send deposits to DepositedCoinsAccAddr or BurnedDepositCoinsAccAddr

Gopkg.toml

@@ -36,7 +36,7 @@
 
 [[override]]
   name = "github.com/tendermint/tendermint"
-  revision = "ebee4377b15f2958b08994485375dd2ee8a649ac" # TODO replace w/ 0.26.1
+  version = "v0.26.1-rc0" # TODO replace w/ 0.26.1
 
 ## deps without releases:
 
@@ -84,4 +84,3 @@
 [prune]
   go-tests = true
   unused-packages = true
-

PENDING.md

@@ -40,6 +40,7 @@ IMPROVEMENTS
 * Gaia CLI  (`gaiacli`)
 
 * Gaia
+ - #2637 [x/gov] Switched inactive and active proposal queues to an iterator based queue
 
 * SDK
  - \#2573 [x/distribution] add accum invariance

client/lcd/lcd_test.go

@@ -628,8 +628,8 @@ func TestSubmitProposal(t *testing.T) {
 	require.Equal(t, uint32(0), resultTx.CheckTx.Code)
 	require.Equal(t, uint32(0), resultTx.DeliverTx.Code)
 
-	var proposalID int64
-	cdc.UnmarshalBinaryBare(resultTx.DeliverTx.GetData(), &proposalID)
+	var proposalID uint64
+	cdc.MustUnmarshalBinaryLengthPrefixed(resultTx.DeliverTx.GetData(), &proposalID)
 
 	// query proposal
 	proposal := getProposal(t, port, proposalID)
@@ -650,8 +650,8 @@ func TestDeposit(t *testing.T) {
 	require.Equal(t, uint32(0), resultTx.CheckTx.Code)
 	require.Equal(t, uint32(0), resultTx.DeliverTx.Code)
 
-	var proposalID int64
-	cdc.UnmarshalBinaryBare(resultTx.DeliverTx.GetData(), &proposalID)
+	var proposalID uint64
+	cdc.MustUnmarshalBinaryLengthPrefixed(resultTx.DeliverTx.GetData(), &proposalID)
 
 	// query proposal
 	proposal := getProposal(t, port, proposalID)
@@ -684,8 +684,8 @@ func TestVote(t *testing.T) {
 	require.Equal(t, uint32(0), resultTx.CheckTx.Code)
 	require.Equal(t, uint32(0), resultTx.DeliverTx.Code)
 
-	var proposalID int64
-	cdc.UnmarshalBinaryBare(resultTx.DeliverTx.GetData(), &proposalID)
+	var proposalID uint64
+	cdc.MustUnmarshalBinaryLengthPrefixed(resultTx.DeliverTx.GetData(), &proposalID)
 
 	// query proposal
 	proposal := getProposal(t, port, proposalID)
@@ -732,18 +732,18 @@ func TestProposalsQuery(t *testing.T) {
 
 	// Addr1 proposes (and deposits) proposals #1 and #2
 	resultTx := doSubmitProposal(t, port, seeds[0], names[0], passwords[0], addrs[0], 5)
-	var proposalID1 int64
-	cdc.UnmarshalBinaryBare(resultTx.DeliverTx.GetData(), &proposalID1)
+	var proposalID1 uint64
+	cdc.MustUnmarshalBinaryLengthPrefixed(resultTx.DeliverTx.GetData(), &proposalID1)
 	tests.WaitForHeight(resultTx.Height+1, port)
 	resultTx = doSubmitProposal(t, port, seeds[0], names[0], passwords[0], addrs[0], 5)
-	var proposalID2 int64
-	cdc.UnmarshalBinaryBare(resultTx.DeliverTx.GetData(), &proposalID2)
+	var proposalID2 uint64
+	cdc.MustUnmarshalBinaryLengthPrefixed(resultTx.DeliverTx.GetData(), &proposalID2)
 	tests.WaitForHeight(resultTx.Height+1, port)
 
 	// Addr2 proposes (and deposits) proposals #3
 	resultTx = doSubmitProposal(t, port, seeds[1], names[1], passwords[1], addrs[1], 5)
-	var proposalID3 int64
-	cdc.UnmarshalBinaryBare(resultTx.DeliverTx.GetData(), &proposalID3)
+	var proposalID3 uint64
+	cdc.MustUnmarshalBinaryLengthPrefixed(resultTx.DeliverTx.GetData(), &proposalID3)
 	tests.WaitForHeight(resultTx.Height+1, port)
 
 	// Addr2 deposits on proposals #2 & #3
@@ -1230,7 +1230,7 @@ func getValidatorRedelegations(t *testing.T, port string, validatorAddr sdk.ValA
 
 // ============= Governance Module ================
 
-func getProposal(t *testing.T, port string, proposalID int64) gov.Proposal {
+func getProposal(t *testing.T, port string, proposalID uint64) gov.Proposal {
 	res, body := Request(t, port, "GET", fmt.Sprintf("/gov/proposals/%d", proposalID), nil)
 	require.Equal(t, http.StatusOK, res.StatusCode, body)
 	var proposal gov.Proposal
@@ -1239,7 +1239,7 @@ func getProposal(t *testing.T, port string, proposalID int64) gov.Proposal {
 	return proposal
 }
 
-func getDeposits(t *testing.T, port string, proposalID int64) []gov.Deposit {
+func getDeposits(t *testing.T, port string, proposalID uint64) []gov.Deposit {
 	res, body := Request(t, port, "GET", fmt.Sprintf("/gov/proposals/%d/deposits", proposalID), nil)
 	require.Equal(t, http.StatusOK, res.StatusCode, body)
 	var deposits []gov.Deposit
@@ -1248,7 +1248,7 @@ func getDeposits(t *testing.T, port string, proposalID int64) []gov.Deposit {
 	return deposits
 }
 
-func getDeposit(t *testing.T, port string, proposalID int64, depositerAddr sdk.AccAddress) gov.Deposit {
+func getDeposit(t *testing.T, port string, proposalID uint64, depositerAddr sdk.AccAddress) gov.Deposit {
 	res, body := Request(t, port, "GET", fmt.Sprintf("/gov/proposals/%d/deposits/%s", proposalID, depositerAddr), nil)
 	require.Equal(t, http.StatusOK, res.StatusCode, body)
 	var deposit gov.Deposit
@@ -1257,7 +1257,7 @@ func getDeposit(t *testing.T, port string, proposalID int64, depositerAddr sdk.A
 	return deposit
 }
 
-func getVote(t *testing.T, port string, proposalID int64, voterAddr sdk.AccAddress) gov.Vote {
+func getVote(t *testing.T, port string, proposalID uint64, voterAddr sdk.AccAddress) gov.Vote {
 	res, body := Request(t, port, "GET", fmt.Sprintf("/gov/proposals/%d/votes/%s", proposalID, voterAddr), nil)
 	require.Equal(t, http.StatusOK, res.StatusCode, body)
 	var vote gov.Vote
@@ -1266,7 +1266,7 @@ func getVote(t *testing.T, port string, proposalID int64, voterAddr sdk.AccAddre
 	return vote
 }
 
-func getVotes(t *testing.T, port string, proposalID int64) []gov.Vote {
+func getVotes(t *testing.T, port string, proposalID uint64) []gov.Vote {
 	res, body := Request(t, port, "GET", fmt.Sprintf("/gov/proposals/%d/votes", proposalID), nil)
 	require.Equal(t, http.StatusOK, res.StatusCode, body)
 	var votes []gov.Vote
@@ -1358,7 +1358,7 @@ func doSubmitProposal(t *testing.T, port, seed, name, password string, proposerA
 	return results
 }
 
-func doDeposit(t *testing.T, port, seed, name, password string, proposerAddr sdk.AccAddress, proposalID int64, amount int64) (resultTx ctypes.ResultBroadcastTxCommit) {
+func doDeposit(t *testing.T, port, seed, name, password string, proposerAddr sdk.AccAddress, proposalID uint64, amount int64) (resultTx ctypes.ResultBroadcastTxCommit) {
 
 	acc := getAccount(t, port, proposerAddr)
 	accnum := acc.GetAccountNumber()
@@ -1388,7 +1388,7 @@ func doDeposit(t *testing.T, port, seed, name, password string, proposerAddr sdk
 	return results
 }
 
-func doVote(t *testing.T, port, seed, name, password string, proposerAddr sdk.AccAddress, proposalID int64) (resultTx ctypes.ResultBroadcastTxCommit) {
+func doVote(t *testing.T, port, seed, name, password string, proposerAddr sdk.AccAddress, proposalID uint64) (resultTx ctypes.ResultBroadcastTxCommit) {
 	// get the account to get the sequence
 	acc := getAccount(t, port, proposerAddr)
 	accnum := acc.GetAccountNumber()

client/utils/rest.go

@@ -65,6 +65,20 @@ func ParseInt64OrReturnBadRequest(w http.ResponseWriter, s string) (n int64, ok
 	return n, true
 }
 
+// ParseUint64OrReturnBadRequest converts s to a uint64 value.
+func ParseUint64OrReturnBadRequest(w http.ResponseWriter, s string) (n uint64, ok bool) {
+	var err error
+
+	n, err = strconv.ParseUint(s, 10, 64)
+	if err != nil {
+		err := fmt.Errorf("'%s' is not a valid uint64", s)
+		WriteErrorResponse(w, http.StatusBadRequest, err.Error())
+		return n, false
+	}
+
+	return n, true
+}
+
 // ParseFloat64OrReturnBadRequest converts s to a float64 value. It returns a
 // default value, defaultIfEmpty, if the string is empty.
 func ParseFloat64OrReturnBadRequest(w http.ResponseWriter, s string, defaultIfEmpty float64) (n float64, ok bool) {

cmd/gaia/app/sim_test.go

@@ -70,15 +70,15 @@ func appStateFn(r *rand.Rand, accs []simulation.Account) json.RawMessage {
 
 	// Random genesis states
 	govGenesis := gov.GenesisState{
-		StartingProposalID: int64(r.Intn(100)),
-		DepositProcedure: gov.DepositProcedure{
+		StartingProposalID: uint64(r.Intn(100)),
+		DepositParams: gov.DepositParams{
 			MinDeposit:       sdk.Coins{sdk.NewInt64Coin("steak", int64(r.Intn(1e3)))},
 			MaxDepositPeriod: time.Duration(r.Intn(2*172800)) * time.Second,
 		},
-		VotingProcedure: gov.VotingProcedure{
+		VotingParams: gov.VotingParams{
 			VotingPeriod: time.Duration(r.Intn(2*172800)) * time.Second,
 		},
-		TallyingProcedure: gov.TallyingProcedure{
+		TallyParams: gov.TallyParams{
 			Threshold:         sdk.NewDecWithPrec(5, 1),
 			Veto:              sdk.NewDecWithPrec(334, 3),
 			GovernancePenalty: sdk.NewDecWithPrec(1, 2),
@@ -166,7 +166,7 @@ func testAndRunTxs(app *GaiaApp) []simulation.WeightedOperation {
 		{50, distrsim.SimulateMsgWithdrawDelegatorReward(app.accountKeeper, app.distrKeeper)},
 		{50, distrsim.SimulateMsgWithdrawValidatorRewardsAll(app.accountKeeper, app.distrKeeper)},
 		{5, govsim.SimulateSubmittingVotingAndSlashingForProposal(app.govKeeper, app.stakeKeeper)},
-		{100, govsim.SimulateMsgDeposit(app.govKeeper, app.stakeKeeper)},
+		{100, govsim.SimulateMsgDeposit(app.govKeeper)},
 		{100, stakesim.SimulateMsgCreateValidator(app.accountKeeper, app.stakeKeeper)},
 		{5, stakesim.SimulateMsgEditValidator(app.stakeKeeper)},
 		{100, stakesim.SimulateMsgDelegate(app.accountKeeper, app.stakeKeeper)},

cmd/gaia/cli_test/cli_test.go

@@ -5,13 +5,14 @@ package clitest
 import (
 	"encoding/json"
 	"fmt"
-	"github.com/tendermint/tendermint/types"
 	"io/ioutil"
 	"os"
 	"path"
 	"path/filepath"
 	"testing"
 
+	"github.com/tendermint/tendermint/types"
+
 	"github.com/stretchr/testify/require"
 
 	abci "github.com/tendermint/tendermint/abci/types"
@@ -348,7 +349,7 @@ func TestGaiaCLISubmitProposal(t *testing.T) {
 	require.Equal(t, int64(45), fooAcc.GetCoins().AmountOf("steak").Int64())
 
 	proposal1 := executeGetProposal(t, fmt.Sprintf("gaiacli query proposal --proposal-id=1 --output=json %v", flags))
-	require.Equal(t, int64(1), proposal1.GetProposalID())
+	require.Equal(t, uint64(1), proposal1.GetProposalID())
 	require.Equal(t, gov.StatusDepositPeriod, proposal1.GetStatus())
 
 	proposalsQuery, _ = tests.ExecuteT(t, fmt.Sprintf("gaiacli query proposals %v", flags), "")
@@ -391,7 +392,7 @@ func TestGaiaCLISubmitProposal(t *testing.T) {
 	fooAcc = executeGetAccount(t, fmt.Sprintf("gaiacli query account %s %v", fooAddr, flags))
 	require.Equal(t, int64(35), fooAcc.GetCoins().AmountOf("steak").Int64())
 	proposal1 = executeGetProposal(t, fmt.Sprintf("gaiacli query proposal --proposal-id=1 --output=json %v", flags))
-	require.Equal(t, int64(1), proposal1.GetProposalID())
+	require.Equal(t, uint64(1), proposal1.GetProposalID())
 	require.Equal(t, gov.StatusVotingPeriod, proposal1.GetStatus())
 
 	voteStr := fmt.Sprintf("gaiacli tx vote %v", flags)
@@ -413,12 +414,12 @@ func TestGaiaCLISubmitProposal(t *testing.T) {
 	tests.WaitForNextNBlocksTM(2, port)
 
 	vote := executeGetVote(t, fmt.Sprintf("gaiacli query vote --proposal-id=1 --voter=%s --output=json %v", fooAddr, flags))
-	require.Equal(t, int64(1), vote.ProposalID)
+	require.Equal(t, uint64(1), vote.ProposalID)
 	require.Equal(t, gov.OptionYes, vote.Option)
 
 	votes := executeGetVotes(t, fmt.Sprintf("gaiacli query votes --proposal-id=1 --output=json %v", flags))
 	require.Len(t, votes, 1)
-	require.Equal(t, int64(1), votes[0].ProposalID)
+	require.Equal(t, uint64(1), votes[0].ProposalID)
 	require.Equal(t, gov.OptionYes, votes[0].Option)
 
 	proposalsQuery, _ = tests.ExecuteT(t, fmt.Sprintf("gaiacli query proposals --status=DepositPeriod %v", flags), "")
@@ -438,7 +439,7 @@ func TestGaiaCLISubmitProposal(t *testing.T) {
 	executeWrite(t, spStr, app.DefaultKeyPass)
 	tests.WaitForNextNBlocksTM(2, port)
 
-	proposalsQuery, _ = tests.ExecuteT(t, fmt.Sprintf("gaiacli query proposals --latest=1 %v", flags), "")
+	proposalsQuery, _ = tests.ExecuteT(t, fmt.Sprintf("gaiacli query proposals --limit=1 %v", flags), "")
 	require.Equal(t, "  2 - Apples", proposalsQuery)
 }
 

docs/spec/governance/state.md

@@ -96,10 +96,12 @@ 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
-  SubmitTime           time.Time               //  Time of the block where TxGovSubmitProposal was included
-  Submitter             sdk.Address      //  Address of the submitter
+  SubmitTime            time.Time           //  Time of the block where TxGovSubmitProposal was included
+  DepositEndTime        time.Time           //  Time that the DepositPeriod of a proposal would expire
+  Submitter             sdk.AccAddress      //  Address of the submitter
 
-  VotingStartTime      time.Time               //  Time of the block where MinDeposit was reached. time.Time{} if MinDeposit is not reached
+  VotingStartTime       time.Time           //  Time of the block where MinDeposit was reached. time.Time{} if MinDeposit is not reached
+  VotingEndTime         time.Time           //  Time of the block that the VotingPeriod for a proposal will end.
   CurrentStatus         ProposalStatus      //  Current status of the proposal
 
   YesVotes              sdk.Dec
@@ -134,46 +136,26 @@ For pseudocode purposes, here are the two function we will use to read or write
 
 **Store:**
 * `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
-  `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. 
+  `ProposalIDs` of proposals that reached `MinDeposit`. Each `EndBlock`, all the proposals
+  that have reached the end of their voting period are processed.
+  To process a finished proposal, the application tallies the votes, compute the votes of
+  each validator and checks if every validator in the valdiator set have voted.
+  If the proposal is accepted, deposits are refunded.
 
 And the pseudocode for the `ProposalProcessingQueue`:
 
 ```go
   in EndBlock do 
 
-    checkProposal()  // First call of the recursive function 
-
-
-  // Recursive function. First call in BeginBlock
-  func checkProposal()  
-    proposalID = ProposalProcessingQueue.Peek()
-    if (proposalID == nil)
-      return
-
-    proposal = load(Governance, <proposalID|'proposal'>) // proposal is a const key
-    votingProcedure = load(GlobalParams, 'VotingProcedure')
-
-    if (CurrentTime == proposal.VotingStartTime + votingProcedure.VotingPeriod && proposal.CurrentStatus == ProposalStatusActive)
-
-    // End of voting period, tally
+    for finishedProposalID in GetAllFinishedProposalIDs(block.Time)
+      proposal = load(Governance, <proposalID|'proposal'>) // proposal is a const key
 
-      ProposalProcessingQueue.pop()
-      validators = 
+      validators = Keeper.getAllValidators()
+      tmpValMap := map(sdk.AccAddress)ValidatorGovInfo
 
-
-      Keeper.getAllValidators()
-      tmpValMap := map(sdk.Address)ValidatorGovInfo
-
-      // Initiate mapping at 0. Validators that remain at 0 at the end of tally will be punished
+      // Initiate mapping at 0. This is the amount of shares of the validator's vote that will be overridden by their delegator's votes
       for each validator in validators
-        tmpValMap(validator).Minus = 0
-
-
+        tmpValMap(validator.OperatorAddr).Minus = 0
 
       // Tally
       voterIterator = rangeQuery(Governance, <proposalID|'addresses'>) //return all the addresses that voted on the proposal
@@ -212,5 +194,4 @@ And the pseudocode for the `ProposalProcessingQueue`:
         proposal.CurrentStatus = ProposalStatusRejected
 
       store(Governance, <proposalID|'proposal'>, proposal)
-      checkProposal()        
 ```