R4R: Fix tally result

PENDING.md

@@ -82,6 +82,7 @@ BUG FIXES
 
   - \#2733 [x/gov, x/mock/simulation] Fix governance simulation, update x/gov import/export
   - \#2854 [x/bank] Remove unused bank.MsgIssue, prevent possible panic
+  - \#2859 [x/gov] Fixed error that prevented the proposal tally from updating the results after voting
 
 * Tendermint
   * [\#2797](https://github.com/tendermint/tendermint/pull/2797) AddressBook requires addresses to have IDs; Do not crap out immediately after sending pex addrs in seed mode

client/lcd/lcd_test.go

@@ -696,6 +696,8 @@ func TestVote(t *testing.T) {
 	proposal := getProposal(t, port, proposalID)
 	require.Equal(t, "Test", proposal.GetTitle())
 
+	tallyOptionBefore := proposal.GetTallyResult().Yes
+
 	// create SubmitProposal TX
 	resultTx = doDeposit(t, port, seed, name, password, addr, proposalID, 5)
 	tests.WaitForHeight(resultTx.Height+1, port)
@@ -704,13 +706,18 @@ func TestVote(t *testing.T) {
 	proposal = getProposal(t, port, proposalID)
 	require.Equal(t, gov.StatusVotingPeriod, proposal.GetStatus())
 
+	option := gov.OptionYes
+
 	// create SubmitProposal TX
-	resultTx = doVote(t, port, seed, name, password, addr, proposalID)
+	resultTx = doVote(t, port, seed, name, password, addr, proposalID, option.String())
 	tests.WaitForHeight(resultTx.Height+1, port)
 
 	vote := getVote(t, port, proposalID, addr)
 	require.Equal(t, proposalID, vote.ProposalID)
 	require.Equal(t, gov.OptionYes, vote.Option)
+
+	proposal = getProposal(t, port, proposalID)
+	require.Equal(t, tallyOptionBefore.Add(sdk.NewDec(1)), proposal.GetTallyResult().Yes)
 }
 
 func TestUnjail(t *testing.T) {
@@ -800,13 +807,13 @@ func TestProposalsQuery(t *testing.T) {
 	require.Equal(t, proposalID3, proposals[1].GetProposalID())
 
 	// Addr1 votes on proposals #2 & #3
-	resultTx = doVote(t, port, seeds[0], names[0], passwords[0], addrs[0], proposalID2)
+	resultTx = doVote(t, port, seeds[0], names[0], passwords[0], addrs[0], proposalID2, gov.OptionYes.String())
 	tests.WaitForHeight(resultTx.Height+1, port)
-	resultTx = doVote(t, port, seeds[0], names[0], passwords[0], addrs[0], proposalID3)
+	resultTx = doVote(t, port, seeds[0], names[0], passwords[0], addrs[0], proposalID3, gov.OptionYes.String())
 	tests.WaitForHeight(resultTx.Height+1, port)
 
 	// Addr2 votes on proposal #3
-	resultTx = doVote(t, port, seeds[1], names[1], passwords[1], addrs[1], proposalID3)
+	resultTx = doVote(t, port, seeds[1], names[1], passwords[1], addrs[1], proposalID3, gov.OptionYes.String())
 	tests.WaitForHeight(resultTx.Height+1, port)
 
 	// Test query all proposals
@@ -1441,7 +1448,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 uint64) (resultTx ctypes.ResultBroadcastTxCommit) {
+func doVote(t *testing.T, port, seed, name, password string, proposerAddr sdk.AccAddress, proposalID uint64, option string) (resultTx ctypes.ResultBroadcastTxCommit) {
 	// get the account to get the sequence
 	acc := getAccount(t, port, proposerAddr)
 	accnum := acc.GetAccountNumber()
@@ -1452,15 +1459,15 @@ func doVote(t *testing.T, port, seed, name, password string, proposerAddr sdk.Ac
 	// vote on proposal
 	jsonStr := []byte(fmt.Sprintf(`{
 		"voter": "%s",
-		"option": "Yes",
+		"option": "%s",
 		"base_req": {
 			"name": "%s",
 			"password": "%s",
 			"chain_id": "%s",
 			"account_number": "%d",
 			"sequence": "%d"
 		}
-	}`, proposerAddr, name, password, chainID, accnum, sequence))
+	}`, proposerAddr, option, name, password, chainID, accnum, sequence))
 	res, body := Request(t, port, "POST", fmt.Sprintf("/gov/proposals/%d/votes", proposalID), jsonStr)
 	require.Equal(t, http.StatusOK, res.StatusCode, body)
 

x/gov/keeper.go

@@ -239,6 +239,37 @@ func (keeper Keeper) activateVotingPeriod(ctx sdk.Context, proposal Proposal) {
 	keeper.InsertActiveProposalQueue(ctx, proposal.GetVotingEndTime(), proposal.GetProposalID())
 }
 
+// checks if two proposals are equal
+func (keeper Keeper) decreaseProposalTally(ctx sdk.Context, proposal Proposal, option VoteOption) {
+	tally := proposal.GetTallyResult()
+	switch {
+	case option == OptionYes:
+		tally.Yes = tally.Yes.Sub(sdk.NewDec(1))
+	case option == OptionNo:
+		tally.No = tally.No.Sub(sdk.NewDec(1))
+	case option == OptionNoWithVeto:
+		tally.NoWithVeto = tally.NoWithVeto.Sub(sdk.NewDec(1))
+	case option == OptionAbstain:
+		tally.Abstain = tally.Abstain.Sub(sdk.NewDec(1))
+	}
+	proposal.SetTallyResult(tally)
+}
+
+func (keeper Keeper) increaseProposalTally(ctx sdk.Context, proposal Proposal, option VoteOption) {
+	tally := proposal.GetTallyResult()
+	switch {
+	case option == OptionYes:
+		tally.Yes = tally.Yes.Add(sdk.NewDec(1))
+	case option == OptionNo:
+		tally.No = tally.No.Add(sdk.NewDec(1))
+	case option == OptionNoWithVeto:
+		tally.NoWithVeto = tally.NoWithVeto.Add(sdk.NewDec(1))
+	case option == OptionAbstain:
+		tally.Abstain = tally.Abstain.Add(sdk.NewDec(1))
+	}
+	proposal.SetTallyResult(tally)
+}
+
 // =====================================================
 // Params
 
@@ -298,12 +329,35 @@ func (keeper Keeper) AddVote(ctx sdk.Context, proposalID uint64, voterAddr sdk.A
 		return ErrInvalidVote(keeper.codespace, option)
 	}
 
-	vote := Vote{
-		ProposalID: proposalID,
-		Voter:      voterAddr,
-		Option:     option,
+	oldVote, found := keeper.GetVote(ctx, proposalID, voterAddr)
+	if found {
+		// Case 1: address already has a vote
+		if oldVote.Option != option {
+			// Case 1.1: new option is different than the previous one
+			keeper.decreaseProposalTally(ctx, proposal, oldVote.Option)
+			keeper.SetProposal(ctx, proposal)
+			vote := Vote{
+				ProposalID: proposalID,
+				Voter:      voterAddr,
+				Option:     option,
+			}
+			keeper.increaseProposalTally(ctx, proposal, option)
+			keeper.setVote(ctx, proposalID, voterAddr, vote)
+			keeper.SetProposal(ctx, proposal)
+		}
+		// Case 1.2: new option is the same as the one before ==> no updates
+	} else {
+		// Case 2: address doesn't have a vote
+		vote := Vote{
+			ProposalID: proposalID,
+			Voter:      voterAddr,
+			Option:     option,
+		}
+
+		keeper.increaseProposalTally(ctx, proposal, option)
+		keeper.setVote(ctx, proposalID, voterAddr, vote)
+		keeper.SetProposal(ctx, proposal)
 	}
-	keeper.setVote(ctx, proposalID, voterAddr, vote)
 
 	return nil
 }

x/gov/keeper_test.go

@@ -156,6 +156,7 @@ func TestVotes(t *testing.T) {
 
 	proposal := keeper.NewTextProposal(ctx, "Test", "description", ProposalTypeText)
 	proposalID := proposal.GetProposalID()
+	initialTally := proposal.GetTallyResult()
 
 	proposal.SetStatus(StatusVotingPeriod)
 	keeper.SetProposal(ctx, proposal)
@@ -202,6 +203,13 @@ func TestVotes(t *testing.T) {
 	votesIterator.Next()
 	require.False(t, votesIterator.Valid())
 	votesIterator.Close()
+
+	// Test Proposal Tally increased
+	proposal = keeper.GetProposal(ctx, proposalID)
+	finalTally := proposal.GetTallyResult()
+	require.Equal(t, initialTally.Yes.Add(sdk.NewDec(1)), finalTally.Yes)
+	require.Equal(t, initialTally.NoWithVeto.Add(sdk.NewDec(1)), finalTally.NoWithVeto)
+	require.Equal(t, initialTally.Abstain, finalTally.Abstain)
 }
 
 func TestProposalQueues(t *testing.T) {

x/gov/proposals.go

@@ -191,7 +191,7 @@ func (pt *ProposalKind) UnmarshalJSON(data []byte) error {
 	return nil
 }
 
-// Turns VoteOption byte to String
+// Turns ProposalKind byte to String
 func (pt ProposalKind) String() string {
 	switch pt {
 	case ProposalTypeText: