Skip to content

Commit

Permalink
Merge PR cosmos#3779: Split Proposal Interface
Browse files Browse the repository at this point in the history
  • Loading branch information
mossid authored and cwgoes committed Mar 15, 2019
1 parent 8d6d8ad commit 465bb02
Show file tree
Hide file tree
Showing 19 changed files with 438 additions and 330 deletions.
30 changes: 15 additions & 15 deletions client/lcd/lcd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -680,7 +680,7 @@ func TestDeposit(t *testing.T) {
// query proposal
totalCoins := sdk.Coins{sdk.NewCoin(sdk.DefaultBondDenom, sdk.TokensFromTendermintPower(10))}
proposal = getProposal(t, port, proposalID)
require.True(t, proposal.GetTotalDeposit().IsEqual(totalCoins))
require.True(t, proposal.TotalDeposit.IsEqual(totalCoins))

// query deposit
deposit := getDeposit(t, port, proposalID, addr)
Expand Down Expand Up @@ -718,7 +718,7 @@ func TestVote(t *testing.T) {
// query proposal
proposal := getProposal(t, port, proposalID)
require.Equal(t, "Test", proposal.GetTitle())
require.Equal(t, gov.StatusVotingPeriod, proposal.GetStatus())
require.Equal(t, gov.StatusVotingPeriod, proposal.Status)

// vote
resultTx = doVote(t, port, seed, name1, pw, addr, proposalID, "Yes", fees)
Expand Down Expand Up @@ -855,13 +855,13 @@ func TestProposalsQuery(t *testing.T) {
// Only proposals #1 should be in Deposit Period
proposals := getProposalsFilterStatus(t, port, gov.StatusDepositPeriod)
require.Len(t, proposals, 1)
require.Equal(t, proposalID1, proposals[0].GetProposalID())
require.Equal(t, proposalID1, proposals[0].ProposalID)

// Only proposals #2 and #3 should be in Voting Period
proposals = getProposalsFilterStatus(t, port, gov.StatusVotingPeriod)
require.Len(t, proposals, 2)
require.Equal(t, proposalID2, proposals[0].GetProposalID())
require.Equal(t, proposalID3, proposals[1].GetProposalID())
require.Equal(t, proposalID2, proposals[0].ProposalID)
require.Equal(t, proposalID3, proposals[1].ProposalID)

// Addr1 votes on proposals #2 & #3
resultTx = doVote(t, port, seeds[0], names[0], passwords[0], addrs[0], proposalID2, "Yes", fees)
Expand All @@ -875,31 +875,31 @@ func TestProposalsQuery(t *testing.T) {

// Test query all proposals
proposals = getProposalsAll(t, port)
require.Equal(t, proposalID1, (proposals[0]).GetProposalID())
require.Equal(t, proposalID2, (proposals[1]).GetProposalID())
require.Equal(t, proposalID3, (proposals[2]).GetProposalID())
require.Equal(t, proposalID1, (proposals[0]).ProposalID)
require.Equal(t, proposalID2, (proposals[1]).ProposalID)
require.Equal(t, proposalID3, (proposals[2]).ProposalID)

// Test query deposited by addr1
proposals = getProposalsFilterDepositor(t, port, addrs[0])
require.Equal(t, proposalID1, (proposals[0]).GetProposalID())
require.Equal(t, proposalID1, (proposals[0]).ProposalID)

// Test query deposited by addr2
proposals = getProposalsFilterDepositor(t, port, addrs[1])
require.Equal(t, proposalID2, (proposals[0]).GetProposalID())
require.Equal(t, proposalID3, (proposals[1]).GetProposalID())
require.Equal(t, proposalID2, (proposals[0]).ProposalID)
require.Equal(t, proposalID3, (proposals[1]).ProposalID)

// Test query voted by addr1
proposals = getProposalsFilterVoter(t, port, addrs[0])
require.Equal(t, proposalID2, (proposals[0]).GetProposalID())
require.Equal(t, proposalID3, (proposals[1]).GetProposalID())
require.Equal(t, proposalID2, (proposals[0]).ProposalID)
require.Equal(t, proposalID3, (proposals[1]).ProposalID)

// Test query voted by addr2
proposals = getProposalsFilterVoter(t, port, addrs[1])
require.Equal(t, proposalID3, (proposals[0]).GetProposalID())
require.Equal(t, proposalID3, (proposals[0]).ProposalID)

// Test query voted and deposited by addr1
proposals = getProposalsFilterVoterDepositor(t, port, addrs[0], addrs[0])
require.Equal(t, proposalID2, (proposals[0]).GetProposalID())
require.Equal(t, proposalID2, (proposals[0]).ProposalID)

// Test query votes on Proposal 2
votes := getVotes(t, port, proposalID2)
Expand Down
14 changes: 7 additions & 7 deletions cmd/gaia/cli_test/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -463,12 +463,12 @@ func TestGaiaCLISubmitProposal(t *testing.T) {

// Ensure propsal is directly queryable
proposal1 := f.QueryGovProposal(1)
require.Equal(t, uint64(1), proposal1.GetProposalID())
require.Equal(t, gov.StatusDepositPeriod, proposal1.GetStatus())
require.Equal(t, uint64(1), proposal1.ProposalID)
require.Equal(t, gov.StatusDepositPeriod, proposal1.Status)

// Ensure query proposals returns properly
proposalsQuery = f.QueryGovProposals()
require.Equal(t, uint64(1), proposalsQuery[0].GetProposalID())
require.Equal(t, uint64(1), proposalsQuery[0].ProposalID)

// Query the deposits on the proposal
deposit := f.QueryGovDeposit(1, fooAddr)
Expand Down Expand Up @@ -507,8 +507,8 @@ func TestGaiaCLISubmitProposal(t *testing.T) {

// Fetch the proposal and ensure it is now in the voting period
proposal1 = f.QueryGovProposal(1)
require.Equal(t, uint64(1), proposal1.GetProposalID())
require.Equal(t, gov.StatusVotingPeriod, proposal1.GetStatus())
require.Equal(t, uint64(1), proposal1.ProposalID)
require.Equal(t, gov.StatusVotingPeriod, proposal1.Status)

// Test vote generate only
success, stdout, stderr = f.TxGovVote(1, gov.OptionYes, keyFoo, "--generate-only")
Expand Down Expand Up @@ -544,15 +544,15 @@ func TestGaiaCLISubmitProposal(t *testing.T) {

// Ensure the proposal returns as in the voting period
proposalsQuery = f.QueryGovProposals("--status=VotingPeriod")
require.Equal(t, uint64(1), proposalsQuery[0].GetProposalID())
require.Equal(t, uint64(1), proposalsQuery[0].ProposalID)

// submit a second test proposal
f.TxGovSubmitProposal(keyFoo, "Text", "Apples", "test", sdk.NewCoin(denom, proposalTokens), "-y")
tests.WaitForNextNBlocksTM(1, f.Port)

// Test limit on proposals query
proposalsQuery = f.QueryGovProposals("--limit=1")
require.Equal(t, uint64(2), proposalsQuery[0].GetProposalID())
require.Equal(t, uint64(2), proposalsQuery[0].ProposalID)

f.Cleanup()
}
Expand Down
17 changes: 13 additions & 4 deletions docs/spec/governance/02_state.md
Original file line number Diff line number Diff line change
Expand Up @@ -84,13 +84,11 @@ This type is used in a temp map when tallying

## Proposals

`Proposals` are an item to be voted on.
`Proposals` are an item to be voted on. It contains the `ProposalContent` which denotes what this proposal is about, and the other fields, which are the mutable state of the governance process.

```go
type Proposal struct {
Title string // Title of the proposal
Description string // Description of the proposal
Type ProposalType // Type of proposal. Initial set {PlainTextProposal, SoftwareUpgradeProposal}
ProposalContent // Proposal content interface
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
Expand All @@ -108,6 +106,17 @@ type Proposal struct {
}
```

`ProposalContent`s are an interface which contains the information about the `Proposal` where it is provided from an external source, including the proposer. Governance process itself does not evaluate about the internal content.

```go
type ProposalContent interface {
GetTitle() string
GetDescription() string
ProposalType() ProposalKind
}

```

We also mention a method to update the tally for a given proposal:

```go
Expand Down
4 changes: 2 additions & 2 deletions x/gov/client/cli/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ $ gaiacli query gov votes 1
var proposal gov.Proposal
cdc.MustUnmarshalJSON(res, &proposal)

propStatus := proposal.GetStatus()
propStatus := proposal.Status
if !(propStatus == gov.StatusVotingPeriod || propStatus == gov.StatusDepositPeriod) {
res, err = gcutils.QueryVotesByTxQuery(cdc, cliCtx, params)
} else {
Expand Down Expand Up @@ -339,7 +339,7 @@ $ gaiacli query gov deposits 1
var proposal gov.Proposal
cdc.MustUnmarshalJSON(res, &proposal)

propStatus := proposal.GetStatus()
propStatus := proposal.Status
if !(propStatus == gov.StatusVotingPeriod || propStatus == gov.StatusDepositPeriod) {
res, err = gcutils.QueryDepositsByTxQuery(cdc, cliCtx, params)
} else {
Expand Down
4 changes: 2 additions & 2 deletions x/gov/client/rest/rest.go
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ func queryDepositsHandlerFn(cdc *codec.Codec, cliCtx context.CLIContext) http.Ha

// For inactive proposals we must query the txs directly to get the deposits
// as they're no longer in state.
propStatus := proposal.GetStatus()
propStatus := proposal.Status
if !(propStatus == gov.StatusVotingPeriod || propStatus == gov.StatusDepositPeriod) {
res, err = gcutils.QueryDepositsByTxQuery(cdc, cliCtx, params)
} else {
Expand Down Expand Up @@ -489,7 +489,7 @@ func queryVotesOnProposalHandlerFn(cdc *codec.Codec, cliCtx context.CLIContext)

// For inactive proposals we must query the txs directly to get the votes
// as they're no longer in state.
propStatus := proposal.GetStatus()
propStatus := proposal.Status
if !(propStatus == gov.StatusVotingPeriod || propStatus == gov.StatusDepositPeriod) {
res, err = gcutils.QueryVotesByTxQuery(cdc, cliCtx, params)
} else {
Expand Down
5 changes: 3 additions & 2 deletions x/gov/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@ func RegisterCodec(cdc *codec.Codec) {
cdc.RegisterConcrete(MsgDeposit{}, "cosmos-sdk/MsgDeposit", nil)
cdc.RegisterConcrete(MsgVote{}, "cosmos-sdk/MsgVote", nil)

cdc.RegisterInterface((*Proposal)(nil), nil)
cdc.RegisterConcrete(&TextProposal{}, "gov/TextProposal", nil)
cdc.RegisterInterface((*ProposalContent)(nil), nil)
cdc.RegisterConcrete(TextProposal{}, "gov/TextProposal", nil)
cdc.RegisterConcrete(SoftwareUpgradeProposal{}, "gov/SoftwareUpgradeProposal", nil)
}

func init() {
Expand Down
28 changes: 17 additions & 11 deletions x/gov/endblocker.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@ func EndBlocker(ctx sdk.Context, keeper Keeper) sdk.Tags {
var proposalID uint64

keeper.cdc.MustUnmarshalBinaryLengthPrefixed(inactiveIterator.Value(), &proposalID)
inactiveProposal := keeper.GetProposal(ctx, proposalID)
inactiveProposal, ok := keeper.GetProposal(ctx, proposalID)
if !ok {
panic(fmt.Sprintf("proposal %d does not exist", proposalID))
}

keeper.DeleteProposal(ctx, proposalID)
keeper.DeleteDeposits(ctx, proposalID) // delete any associated deposits (burned)
Expand All @@ -28,10 +31,10 @@ func EndBlocker(ctx sdk.Context, keeper Keeper) sdk.Tags {

logger.Info(
fmt.Sprintf("proposal %d (%s) didn't meet minimum deposit of %s (had only %s); deleted",
inactiveProposal.GetProposalID(),
inactiveProposal.ProposalID,
inactiveProposal.GetTitle(),
keeper.GetDepositParams(ctx).MinDeposit,
inactiveProposal.GetTotalDeposit(),
inactiveProposal.TotalDeposit,
),
)
}
Expand All @@ -43,28 +46,31 @@ func EndBlocker(ctx sdk.Context, keeper Keeper) sdk.Tags {
var proposalID uint64

keeper.cdc.MustUnmarshalBinaryLengthPrefixed(activeIterator.Value(), &proposalID)
activeProposal := keeper.GetProposal(ctx, proposalID)
activeProposal, ok := keeper.GetProposal(ctx, proposalID)
if !ok {
panic(fmt.Sprintf("proposal %d does not exist", proposalID))
}
passes, tallyResults := tally(ctx, keeper, activeProposal)

var tagValue string
if passes {
keeper.RefundDeposits(ctx, activeProposal.GetProposalID())
activeProposal.SetStatus(StatusPassed)
keeper.RefundDeposits(ctx, activeProposal.ProposalID)
activeProposal.Status = StatusPassed
tagValue = tags.ActionProposalPassed
} else {
keeper.DeleteDeposits(ctx, activeProposal.GetProposalID())
activeProposal.SetStatus(StatusRejected)
keeper.DeleteDeposits(ctx, activeProposal.ProposalID)
activeProposal.Status = StatusRejected
tagValue = tags.ActionProposalRejected
}

activeProposal.SetFinalTallyResult(tallyResults)
activeProposal.FinalTallyResult = tallyResults
keeper.SetProposal(ctx, activeProposal)
keeper.RemoveFromActiveProposalQueue(ctx, activeProposal.GetVotingEndTime(), activeProposal.GetProposalID())
keeper.RemoveFromActiveProposalQueue(ctx, activeProposal.VotingEndTime, activeProposal.ProposalID)

logger.Info(
fmt.Sprintf(
"proposal %d (%s) tallied; passed: %v",
activeProposal.GetProposalID(), activeProposal.GetTitle(), passes,
activeProposal.ProposalID, activeProposal.GetTitle(), passes,
),
)

Expand Down
4 changes: 3 additions & 1 deletion x/gov/endblocker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,9 @@ func TestTickPassedVotingPeriod(t *testing.T) {
require.True(t, activeQueue.Valid())
var activeProposalID uint64
keeper.cdc.UnmarshalBinaryLengthPrefixed(activeQueue.Value(), &activeProposalID)
require.Equal(t, StatusVotingPeriod, keeper.GetProposal(ctx, activeProposalID).GetStatus())
proposal, ok := keeper.GetProposal(ctx, activeProposalID)
require.True(t, ok)
require.Equal(t, StatusVotingPeriod, proposal.Status)
depositsIterator := keeper.GetDeposits(ctx, proposalID)
require.True(t, depositsIterator.Valid())
depositsIterator.Close()
Expand Down
8 changes: 4 additions & 4 deletions x/gov/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,11 +122,11 @@ func InitGenesis(ctx sdk.Context, k Keeper, data GenesisState) {
k.setVote(ctx, vote.ProposalID, vote.Vote.Voter, vote.Vote)
}
for _, proposal := range data.Proposals {
switch proposal.GetStatus() {
switch proposal.Status {
case StatusDepositPeriod:
k.InsertInactiveProposalQueue(ctx, proposal.GetDepositEndTime(), proposal.GetProposalID())
k.InsertInactiveProposalQueue(ctx, proposal.DepositEndTime, proposal.ProposalID)
case StatusVotingPeriod:
k.InsertActiveProposalQueue(ctx, proposal.GetVotingEndTime(), proposal.GetProposalID())
k.InsertActiveProposalQueue(ctx, proposal.VotingEndTime, proposal.ProposalID)
}
k.SetProposal(ctx, proposal)
}
Expand All @@ -142,7 +142,7 @@ func ExportGenesis(ctx sdk.Context, k Keeper) GenesisState {
var votes []VoteWithMetadata
proposals := k.GetProposalsFiltered(ctx, nil, nil, StatusNil, 0)
for _, proposal := range proposals {
proposalID := proposal.GetProposalID()
proposalID := proposal.ProposalID
depositsIterator := k.GetDeposits(ctx, proposalID)
defer depositsIterator.Close()
for ; depositsIterator.Valid(); depositsIterator.Next() {
Expand Down
51 changes: 36 additions & 15 deletions x/gov/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,12 @@ func TestEqualProposals(t *testing.T) {

ctx := mapp.BaseApp.NewContext(false, abci.Header{})

// Create two proposals
proposal1 := keeper.NewTextProposal(ctx, "Test", "description", ProposalTypeText)
proposal2 := keeper.NewTextProposal(ctx, "Test", "description", ProposalTypeText)
// Submit two proposals
proposal := testProposal()
proposal1, err := keeper.SubmitProposal(ctx, proposal)
require.NoError(t, err)
proposal2, err := keeper.SubmitProposal(ctx, proposal)
require.NoError(t, err)

// They are similar but their IDs should be different
require.NotEqual(t, proposal1, proposal2)
Expand All @@ -48,11 +51,15 @@ func TestEqualProposals(t *testing.T) {
require.False(t, state1.Equal(state2))

// Now make proposals identical by setting both IDs to 55
proposal1.SetProposalID(55)
proposal2.SetProposalID(55)
proposal1.ProposalID = 55
proposal2.ProposalID = 55
require.Equal(t, proposal1, proposal1)
require.True(t, ProposalEqual(proposal1, proposal2))

// Reassign proposals into state
state1.Proposals[0] = proposal1
state2.Proposals[0] = proposal2

// State should be identical now..
require.Equal(t, state1, state2)
require.True(t, state1.Equal(state2))
Expand All @@ -69,17 +76,24 @@ func TestImportExportQueues(t *testing.T) {
ctx := mapp.BaseApp.NewContext(false, abci.Header{})

// Create two proposals, put the second into the voting period
proposal1 := keeper.NewTextProposal(ctx, "Test", "description", ProposalTypeText)
proposalID1 := proposal1.GetProposalID()
proposal := testProposal()
proposal1, err := keeper.SubmitProposal(ctx, proposal)
require.NoError(t, err)
proposalID1 := proposal1.ProposalID

proposal2 := keeper.NewTextProposal(ctx, "Test", "description", ProposalTypeText)
proposalID2 := proposal2.GetProposalID()
proposal2, err := keeper.SubmitProposal(ctx, proposal)
require.NoError(t, err)
proposalID2 := proposal2.ProposalID

_, votingStarted := keeper.AddDeposit(ctx, proposalID2, addrs[0], keeper.GetDepositParams(ctx).MinDeposit)
require.True(t, votingStarted)

require.True(t, keeper.GetProposal(ctx, proposalID1).GetStatus() == StatusDepositPeriod)
require.True(t, keeper.GetProposal(ctx, proposalID2).GetStatus() == StatusVotingPeriod)
proposal1, ok := keeper.GetProposal(ctx, proposalID1)
require.True(t, ok)
proposal2, ok = keeper.GetProposal(ctx, proposalID2)
require.True(t, ok)
require.True(t, proposal1.Status == StatusDepositPeriod)
require.True(t, proposal2.Status == StatusVotingPeriod)

genAccs := mapp.AccountKeeper.GetAllAccounts(ctx)

Expand All @@ -96,12 +110,19 @@ func TestImportExportQueues(t *testing.T) {
ctx2 = ctx2.WithBlockTime(ctx2.BlockHeader().Time.Add(keeper2.GetDepositParams(ctx2).MaxDepositPeriod).Add(keeper2.GetVotingParams(ctx2).VotingPeriod))

// Make sure that they are still in the DepositPeriod and VotingPeriod respectively
require.True(t, keeper2.GetProposal(ctx2, proposalID1).GetStatus() == StatusDepositPeriod)
require.True(t, keeper2.GetProposal(ctx2, proposalID2).GetStatus() == StatusVotingPeriod)
proposal1, ok = keeper2.GetProposal(ctx2, proposalID1)
require.True(t, ok)
proposal2, ok = keeper2.GetProposal(ctx2, proposalID2)
require.True(t, ok)
require.True(t, proposal1.Status == StatusDepositPeriod)
require.True(t, proposal2.Status == StatusVotingPeriod)

// Run the endblocker. Check to make sure that proposal1 is removed from state, and proposal2 is finished VotingPeriod.
EndBlocker(ctx2, keeper2)

require.Nil(t, keeper2.GetProposal(ctx2, proposalID1))
require.True(t, keeper2.GetProposal(ctx2, proposalID2).GetStatus() == StatusRejected)
proposal1, ok = keeper2.GetProposal(ctx2, proposalID1)
require.False(t, ok)
proposal2, ok = keeper2.GetProposal(ctx2, proposalID2)
require.True(t, ok)
require.True(t, proposal2.Status == StatusRejected)
}
Loading

0 comments on commit 465bb02

Please sign in to comment.