From cca3c3cbf270deb3682d3ee66d535ca63fa48762 Mon Sep 17 00:00:00 2001 From: Sunny Aggarwal Date: Mon, 20 May 2019 10:13:50 -0400 Subject: [PATCH] Merge PR #4374: Don't Burn Deposits for Rejected Governance Proposals Unless Vetoed --- .pending/features/gaia/4373-Don-t-Burn-Depo | 1 + devtools-stamp | 0 x/gov/endblocker.go | 10 +++-- x/gov/querier.go | 2 +- x/gov/tally.go | 14 +++---- x/gov/tally_test.go | 43 ++++++++++++++------- 6 files changed, 44 insertions(+), 26 deletions(-) create mode 100644 .pending/features/gaia/4373-Don-t-Burn-Depo create mode 100644 devtools-stamp diff --git a/.pending/features/gaia/4373-Don-t-Burn-Depo b/.pending/features/gaia/4373-Don-t-Burn-Depo new file mode 100644 index 0000000000..516dcb7cd8 --- /dev/null +++ b/.pending/features/gaia/4373-Don-t-Burn-Depo @@ -0,0 +1 @@ +#4373 Don't burn deposits for rejected governance proposals unless vetoed. diff --git a/devtools-stamp b/devtools-stamp new file mode 100644 index 0000000000..e69de29bb2 diff --git a/x/gov/endblocker.go b/x/gov/endblocker.go index fcfe9e08f4..a8fd67f24a 100644 --- a/x/gov/endblocker.go +++ b/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" diff --git a/x/gov/querier.go b/x/gov/querier.go index 881e2d53f3..87202800da 100644 --- a/x/gov/querier.go +++ b/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) diff --git a/x/gov/tally.go b/x/gov/tally.go index 19bde1478d..09b5cc71e5 100644 --- a/x/gov/tally.go +++ b/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 } diff --git a/x/gov/tally_test.go b/x/gov/tally_test.go index 9626c79cb7..1628394d52 100644 --- a/x/gov/tally_test.go +++ b/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())) }