Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

R4R: Switch gov proposal-queues to use iterators #2638

Merged
merged 17 commits into from
Nov 7, 2018
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions PENDING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
38 changes: 19 additions & 19 deletions client/lcd/lcd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down
14 changes: 14 additions & 0 deletions client/utils/rest.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion cmd/gaia/app/sim_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func appStateFn(r *rand.Rand, accs []simulation.Account) json.RawMessage {

// Random genesis states
govGenesis := gov.GenesisState{
StartingProposalID: int64(r.Intn(100)),
StartingProposalID: uint64(r.Intn(100)),
DepositProcedure: gov.DepositProcedure{
MinDeposit: sdk.Coins{sdk.NewInt64Coin("steak", int64(r.Intn(1e3)))},
MaxDepositPeriod: time.Duration(r.Intn(2*172800)) * time.Second,
Expand Down
7 changes: 4 additions & 3 deletions cmd/gaia/cli_test/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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), "")
Expand Down Expand Up @@ -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)
Expand Down
45 changes: 14 additions & 31 deletions docs/spec/governance/state.md
Original file line number Diff line number Diff line change
Expand Up @@ -96,11 +96,13 @@ 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.Address // Address of the submitter
sunnya97 marked this conversation as resolved.
Show resolved Hide resolved

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
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
NoVotes sdk.Dec
Expand Down Expand Up @@ -134,47 +136,29 @@ 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')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't used.


for finishedProposalID in GetAllFinishedProposalIDs(block.Time)
proposal = load(Governance, <proposalID|'proposal'>) // proposal is a const key

if (CurrentTime == proposal.VotingStartTime + votingProcedure.VotingPeriod && proposal.CurrentStatus == ProposalStatusActive)

// End of voting period, tally

ProposalProcessingQueue.pop()
validators =


Keeper.getAllValidators()
validators = Keeper.getAllValidators()
tmpValMap := map(sdk.Address)ValidatorGovInfo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/sdk.Address/sdk.ValAddress/g


// Initiate mapping at 0. Validators that remain at 0 at the end of tally will be punished
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment (as well as the comment in the code) don't make sense... This comment makes it sound like .Minus being 0 means the validator will be punished, but that's not it.... right?

It seems that Minus should be called DelegatorOverrideShares

for each validator in validators
tmpValMap(validator).Minus = 0



// Tally
voterIterator = rangeQuery(Governance, <proposalID|'addresses'>) //return all the addresses that voted on the proposal
for each (voterAddress, vote) in voterIterator
Expand Down Expand Up @@ -212,5 +196,4 @@ And the pseudocode for the `ProposalProcessingQueue`:
proposal.CurrentStatus = ProposalStatusRejected

store(Governance, <proposalID|'proposal'>, proposal)
checkProposal()
```
8 changes: 8 additions & 0 deletions types/utils.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package types

import (
"encoding/binary"
"encoding/json"
"time"

Expand Down Expand Up @@ -36,6 +37,13 @@ func MustSortJSON(toSortJSON []byte) []byte {
return js
}

// Uint64ToBigEndian - marshals uint64 to a bigendian byte slice so it can be sorted
func Uint64ToBigEndian(i uint64) []byte {
b := make([]byte, 8)
binary.BigEndian.PutUint64(b, i)
return b
}

// Slight modification of the RFC3339Nano but it right pads all zeros and drops the time zone info
const SortableTimeFormat = "2006-01-02T15:04:05.000000000"

Expand Down
18 changes: 9 additions & 9 deletions x/gov/client/cli/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ func GetCmdDeposit(cdc *codec.Codec) *cobra.Command {
return err
}

proposalID := viper.GetInt64(flagProposalID)
proposalID := uint64(viper.GetInt64(flagProposalID))

amount, err := sdk.ParseCoins(viper.GetString(flagDeposit))
if err != nil {
Expand Down Expand Up @@ -215,7 +215,7 @@ func GetCmdVote(cdc *codec.Codec) *cobra.Command {
return err
}

proposalID := viper.GetInt64(flagProposalID)
proposalID := uint64(viper.GetInt64(flagProposalID))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't viper support Uints? Why don't we just change it from a string to that?

option := viper.GetString(flagOption)

byteVoteOption, err := gov.VoteOptionFromString(client.NormalizeVoteOption(option))
Expand Down Expand Up @@ -256,7 +256,7 @@ func GetCmdQueryProposal(queryRoute string, cdc *codec.Codec) *cobra.Command {
Short: "Query details of a single proposal",
RunE: func(cmd *cobra.Command, args []string) error {
cliCtx := context.NewCLIContext().WithCodec(cdc)
proposalID := viper.GetInt64(flagProposalID)
proposalID := uint64(viper.GetInt64(flagProposalID))

params := gov.QueryProposalParams{
ProposalID: proposalID,
Expand Down Expand Up @@ -291,7 +291,7 @@ func GetCmdQueryProposals(queryRoute string, cdc *codec.Codec) *cobra.Command {
bechDepositerAddr := viper.GetString(flagDepositer)
bechVoterAddr := viper.GetString(flagVoter)
strProposalStatus := viper.GetString(flagStatus)
latestProposalsIDs := viper.GetInt64(flagLatestProposalIDs)
latestProposalsIDs := uint64(viper.GetInt64(flagLatestProposalIDs))

params := gov.QueryProposalsParams{
NumLatestProposals: latestProposalsIDs,
Expand Down Expand Up @@ -368,7 +368,7 @@ func GetCmdQueryVote(queryRoute string, cdc *codec.Codec) *cobra.Command {
Short: "Query details of a single vote",
RunE: func(cmd *cobra.Command, args []string) error {
cliCtx := context.NewCLIContext().WithCodec(cdc)
proposalID := viper.GetInt64(flagProposalID)
proposalID := uint64(viper.GetInt64(flagProposalID))

voterAddr, err := sdk.AccAddressFromBech32(viper.GetString(flagVoter))
if err != nil {
Expand Down Expand Up @@ -407,7 +407,7 @@ func GetCmdQueryVotes(queryRoute string, cdc *codec.Codec) *cobra.Command {
Short: "Query votes on a proposal",
RunE: func(cmd *cobra.Command, args []string) error {
cliCtx := context.NewCLIContext().WithCodec(cdc)
proposalID := viper.GetInt64(flagProposalID)
proposalID := uint64(viper.GetInt64(flagProposalID))

params := gov.QueryVotesParams{
ProposalID: proposalID,
Expand Down Expand Up @@ -440,7 +440,7 @@ func GetCmdQueryDeposit(queryRoute string, cdc *codec.Codec) *cobra.Command {
Short: "Query details of a deposit",
RunE: func(cmd *cobra.Command, args []string) error {
cliCtx := context.NewCLIContext().WithCodec(cdc)
proposalID := viper.GetInt64(flagProposalID)
proposalID := uint64(viper.GetInt64(flagProposalID))

depositerAddr, err := sdk.AccAddressFromBech32(viper.GetString(flagDepositer))
if err != nil {
Expand Down Expand Up @@ -479,7 +479,7 @@ func GetCmdQueryDeposits(queryRoute string, cdc *codec.Codec) *cobra.Command {
Short: "Query deposits on a proposal",
RunE: func(cmd *cobra.Command, args []string) error {
cliCtx := context.NewCLIContext().WithCodec(cdc)
proposalID := viper.GetInt64(flagProposalID)
proposalID := uint64(viper.GetInt64(flagProposalID))

params := gov.QueryDepositsParams{
ProposalID: proposalID,
Expand Down Expand Up @@ -511,7 +511,7 @@ func GetCmdQueryTally(queryRoute string, cdc *codec.Codec) *cobra.Command {
Short: "Get the tally of a proposal vote",
RunE: func(cmd *cobra.Command, args []string) error {
cliCtx := context.NewCLIContext().WithCodec(cdc)
proposalID := viper.GetInt64(flagProposalID)
proposalID := uint64(viper.GetInt64(flagProposalID))

params := gov.QueryTallyParams{
ProposalID: proposalID,
Expand Down
Loading