Merge PR cosmos#4374: Don't Burn Deposits for Rejected Governance Pro…

…posals Unless Vetoed

.pending/features/gaia/4373-Don-t-Burn-Depo

@@ -0,0 +1 @@
+#4373 Don't burn deposits for rejected governance proposals unless vetoed.

x/gov/endblocker.go

@@ -50,13 +50,17 @@ func EndBlocker(ctx sdk.Context, keeper Keeper) sdk.Tags {
 		if !ok {
 			panic(fmt.Sprintf("proposal %d does not exist", proposalID))
 		}
-		passes, tallyResults := tally(ctx, keeper, activeProposal)
+		passes, burnDeposits, tallyResults := tally(ctx, keeper, activeProposal)
 
 		var tagValue, logMsg string
 
-		if passes {
+		if burnDeposits {
+			keeper.DeleteDeposits(ctx, activeProposal.ProposalID)
+		} else {
 			keeper.RefundDeposits(ctx, activeProposal.ProposalID)
+		}
 
+		if passes {
 			handler := keeper.router.GetRoute(activeProposal.ProposalRoute())
 			cacheCtx, writeCache := ctx.CacheContext()
 
@@ -77,8 +81,6 @@ func EndBlocker(ctx sdk.Context, keeper Keeper) sdk.Tags {
 				logMsg = fmt.Sprintf("passed, but failed on execution: %s", err.ABCILog())
 			}
 		} else {
-			keeper.DeleteDeposits(ctx, activeProposal.ProposalID)
-
 			activeProposal.Status = StatusRejected
 			tagValue = tags.ActionProposalRejected
 			logMsg = "rejected"

x/gov/querier.go

@@ -218,7 +218,7 @@ func queryTally(ctx sdk.Context, path []string, req abci.RequestQuery, keeper Ke
 		tallyResult = proposal.FinalTallyResult
 	} else {
 		// proposal is in voting period
-		_, tallyResult = tally(ctx, keeper, proposal)
+		_, _, tallyResult = tally(ctx, keeper, proposal)
 	}
 
 	bz, err := codec.MarshalJSONIndent(keeper.cdc, tallyResult)

x/gov/tally.go

@@ -26,7 +26,7 @@ func newValidatorGovInfo(address sdk.ValAddress, bondedTokens sdk.Int, delegator
 }
 
 // TODO: Break into several smaller functions for clarity
-func tally(ctx sdk.Context, keeper Keeper, proposal Proposal) (passes bool, tallyResults TallyResult) {
+func tally(ctx sdk.Context, keeper Keeper, proposal Proposal) (passes bool, burnDeposits bool, tallyResults TallyResult) {
 	results := make(map[VoteOption]sdk.Dec)
 	results[OptionYes] = sdk.ZeroDec()
 	results[OptionAbstain] = sdk.ZeroDec()
@@ -105,30 +105,30 @@ func tally(ctx sdk.Context, keeper Keeper, proposal Proposal) (passes bool, tall
 	// TODO: Upgrade the spec to cover all of these cases & remove pseudocode.
 	// If there is no staked coins, the proposal fails
 	if keeper.vs.TotalBondedTokens(ctx).IsZero() {
-		return false, tallyResults
+		return false, false, tallyResults
 	}
 
 	// If there is not enough quorum of votes, the proposal fails
 	percentVoting := totalVotingPower.Quo(keeper.vs.TotalBondedTokens(ctx).ToDec())
 	if percentVoting.LT(tallyParams.Quorum) {
-		return false, tallyResults
+		return false, true, tallyResults
 	}
 
 	// If no one votes (everyone abstains), proposal fails
 	if totalVotingPower.Sub(results[OptionAbstain]).Equal(sdk.ZeroDec()) {
-		return false, tallyResults
+		return false, false, tallyResults
 	}
 
 	// If more than 1/3 of voters veto, proposal fails
 	if results[OptionNoWithVeto].Quo(totalVotingPower).GT(tallyParams.Veto) {
-		return false, tallyResults
+		return false, true, tallyResults
 	}
 
 	// If more than 1/2 of non-abstaining voters vote Yes, proposal passes
 	if results[OptionYes].Quo(totalVotingPower.Sub(results[OptionAbstain])).GT(tallyParams.Threshold) {
-		return true, tallyResults
+		return true, false, tallyResults
 	}
 
 	// If more than 1/2 of non-abstaining voters vote No, proposal fails
-	return false, tallyResults
+	return false, false, tallyResults
 }

x/gov/tally_test.go

@@ -38,9 +38,10 @@ func TestTallyNoOneVotes(t *testing.T) {
 
 	proposal, ok := input.keeper.GetProposal(ctx, proposalID)
 	require.True(t, ok)
-	passes, tallyResults := tally(ctx, input.keeper, proposal)
+	passes, burnDeposits, tallyResults := tally(ctx, input.keeper, proposal)
 
 	require.False(t, passes)
+	require.True(t, burnDeposits)
 	require.True(t, tallyResults.Equals(EmptyTallyResult()))
 }
 
@@ -73,8 +74,9 @@ func TestTallyNoQuorum(t *testing.T) {
 
 	proposal, ok := input.keeper.GetProposal(ctx, proposalID)
 	require.True(t, ok)
-	passes, _ := tally(ctx, input.keeper, proposal)
+	passes, burnDeposits, _ := tally(ctx, input.keeper, proposal)
 	require.False(t, passes)
+	require.True(t, burnDeposits)
 }
 
 func TestTallyOnlyValidatorsAllYes(t *testing.T) {
@@ -108,9 +110,10 @@ func TestTallyOnlyValidatorsAllYes(t *testing.T) {
 
 	proposal, ok := input.keeper.GetProposal(ctx, proposalID)
 	require.True(t, ok)
-	passes, tallyResults := tally(ctx, input.keeper, proposal)
+	passes, burnDeposits, tallyResults := tally(ctx, input.keeper, proposal)
 
 	require.True(t, passes)
+	require.False(t, burnDeposits)
 	require.False(t, tallyResults.Equals(EmptyTallyResult()))
 }
 
@@ -145,9 +148,10 @@ func TestTallyOnlyValidators51No(t *testing.T) {
 
 	proposal, ok := input.keeper.GetProposal(ctx, proposalID)
 	require.True(t, ok)
-	passes, _ := tally(ctx, input.keeper, proposal)
+	passes, burnDeposits, _ := tally(ctx, input.keeper, proposal)
 
 	require.False(t, passes)
+	require.False(t, burnDeposits)
 }
 
 func TestTallyOnlyValidators51Yes(t *testing.T) {
@@ -183,9 +187,10 @@ func TestTallyOnlyValidators51Yes(t *testing.T) {
 
 	proposal, ok := input.keeper.GetProposal(ctx, proposalID)
 	require.True(t, ok)
-	passes, tallyResults := tally(ctx, input.keeper, proposal)
+	passes, burnDeposits, tallyResults := tally(ctx, input.keeper, proposal)
 
 	require.True(t, passes)
+	require.False(t, burnDeposits)
 	require.False(t, tallyResults.Equals(EmptyTallyResult()))
 }
 
@@ -222,10 +227,12 @@ func TestTallyOnlyValidatorsVetoed(t *testing.T) {
 
 	proposal, ok := input.keeper.GetProposal(ctx, proposalID)
 	require.True(t, ok)
-	passes, tallyResults := tally(ctx, input.keeper, proposal)
+	passes, burnDeposits, tallyResults := tally(ctx, input.keeper, proposal)
 
 	require.False(t, passes)
+	require.True(t, burnDeposits)
 	require.False(t, tallyResults.Equals(EmptyTallyResult()))
+
 }
 
 func TestTallyOnlyValidatorsAbstainPasses(t *testing.T) {
@@ -261,9 +268,10 @@ func TestTallyOnlyValidatorsAbstainPasses(t *testing.T) {
 
 	proposal, ok := input.keeper.GetProposal(ctx, proposalID)
 	require.True(t, ok)
-	passes, tallyResults := tally(ctx, input.keeper, proposal)
+	passes, burnDeposits, tallyResults := tally(ctx, input.keeper, proposal)
 
 	require.True(t, passes)
+	require.False(t, burnDeposits)
 	require.False(t, tallyResults.Equals(EmptyTallyResult()))
 }
 
@@ -300,9 +308,10 @@ func TestTallyOnlyValidatorsAbstainFails(t *testing.T) {
 
 	proposal, ok := input.keeper.GetProposal(ctx, proposalID)
 	require.True(t, ok)
-	passes, tallyResults := tally(ctx, input.keeper, proposal)
+	passes, burnDeposits, tallyResults := tally(ctx, input.keeper, proposal)
 
 	require.False(t, passes)
+	require.False(t, burnDeposits)
 	require.False(t, tallyResults.Equals(EmptyTallyResult()))
 }
 
@@ -337,9 +346,10 @@ func TestTallyOnlyValidatorsNonVoter(t *testing.T) {
 
 	proposal, ok := input.keeper.GetProposal(ctx, proposalID)
 	require.True(t, ok)
-	passes, tallyResults := tally(ctx, input.keeper, proposal)
+	passes, burnDeposits, tallyResults := tally(ctx, input.keeper, proposal)
 
 	require.False(t, passes)
+	require.False(t, burnDeposits)
 	require.False(t, tallyResults.Equals(EmptyTallyResult()))
 }
 
@@ -382,9 +392,10 @@ func TestTallyDelgatorOverride(t *testing.T) {
 
 	proposal, ok := input.keeper.GetProposal(ctx, proposalID)
 	require.True(t, ok)
-	passes, tallyResults := tally(ctx, input.keeper, proposal)
+	passes, burnDeposits, tallyResults := tally(ctx, input.keeper, proposal)
 
 	require.False(t, passes)
+	require.False(t, burnDeposits)
 	require.False(t, tallyResults.Equals(EmptyTallyResult()))
 }
 
@@ -425,9 +436,10 @@ func TestTallyDelgatorInherit(t *testing.T) {
 
 	proposal, ok := input.keeper.GetProposal(ctx, proposalID)
 	require.True(t, ok)
-	passes, tallyResults := tally(ctx, input.keeper, proposal)
+	passes, burnDeposits, tallyResults := tally(ctx, input.keeper, proposal)
 
 	require.True(t, passes)
+	require.False(t, burnDeposits)
 	require.False(t, tallyResults.Equals(EmptyTallyResult()))
 }
 
@@ -472,9 +484,10 @@ func TestTallyDelgatorMultipleOverride(t *testing.T) {
 
 	proposal, ok := input.keeper.GetProposal(ctx, proposalID)
 	require.True(t, ok)
-	passes, tallyResults := tally(ctx, input.keeper, proposal)
+	passes, burnDeposits, tallyResults := tally(ctx, input.keeper, proposal)
 
 	require.False(t, passes)
+	require.False(t, burnDeposits)
 	require.False(t, tallyResults.Equals(EmptyTallyResult()))
 }
 
@@ -530,9 +543,10 @@ func TestTallyDelgatorMultipleInherit(t *testing.T) {
 
 	proposal, ok := input.keeper.GetProposal(ctx, proposalID)
 	require.True(t, ok)
-	passes, tallyResults := tally(ctx, input.keeper, proposal)
+	passes, burnDeposits, tallyResults := tally(ctx, input.keeper, proposal)
 
 	require.False(t, passes)
+	require.False(t, burnDeposits)
 	require.False(t, tallyResults.Equals(EmptyTallyResult()))
 }
 
@@ -582,8 +596,9 @@ func TestTallyJailedValidator(t *testing.T) {
 
 	proposal, ok := input.keeper.GetProposal(ctx, proposalID)
 	require.True(t, ok)
-	passes, tallyResults := tally(ctx, input.keeper, proposal)
+	passes, burnDeposits, tallyResults := tally(ctx, input.keeper, proposal)
 
 	require.True(t, passes)
+	require.False(t, burnDeposits)
 	require.False(t, tallyResults.Equals(EmptyTallyResult()))
 }