certusone
/
cosmos-sdk
mirror of https://github.com/certusone/cosmos-sdk.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

refactor: remove query by events for x/gov deposits (#9519) 

<!--
The default pull request template is for types feat, fix, or refactor.
For other templates, add one of the following parameters to the url:
- template=docs.md
- template=other.md
-->

## Description

Closes: #9419 

<!-- Add a description of the changes that this PR introduces and the files that
are the most critical to review. -->

---

### Author Checklist

*All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.*

I have...

- [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] added `!` to the type prefix if API or client breaking change
- [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting))
- [ ] provided a link to the relevant issue or specification
- [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules)
- [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing)
- [ ] added a changelog entry to `CHANGELOG.md`
- [ ] included comments for [documenting Go code](https://blog.golang.org/godoc)
- [ ] updated the relevant documentation or specification
- [ ] reviewed "Files changed" and left comments if necessary
- [ ] confirmed all CI checks have passed

### Reviewers Checklist

*All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.*

I have...

- [x] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [x] confirmed `!` in the type prefix if API or client breaking change
- [ ] confirmed all author checklist items have been addressed 
- [ ] reviewed state machine logic
- [x] reviewed API design and naming
- [x] reviewed documentation is accurate
- [ ] reviewed tests and test coverage
- [ ] manually tested (if applicable)
This commit is contained in:
atheeshp 2021-07-30 23:07:38 +05:30 committed by GitHub
parent 36ab23a31d
commit 19aeb763a3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 120 additions and 128 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@ -54,6 +54,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (client/keys) [\#9407](https://github.com/cosmos/cosmos-sdk/pull/9601) Added `keys rename` CLI command and `Keyring.Rename` interface method to rename a key in the keyring.
* (x/slashing) [\#9458](https://github.com/cosmos/cosmos-sdk/pull/9458) Coins burned from slashing is now returned from Slash function and included in Slash event.
* [\#9246](https://github.com/cosmos/cosmos-sdk/pull/9246) The `New` method for the network package now returns an error.
* [\#9519](https://github.com/cosmos/cosmos-sdk/pull/9519) `DeleteDeposits` renamed to `DeleteAndBurnDeposits`, `RefundDeposits` renamed to `RefundAndDeleteDeposits`
* (codec) [\#9521](https://github.com/cosmos/cosmos-sdk/pull/9521) Removed deprecated `clientCtx.JSONCodec` from `client.Context`.
* (codec) [\#9521](https://github.com/cosmos/cosmos-sdk/pull/9521) Rename `EncodingConfig.Marshaler` to `Codec`.
* [\#9418](https://github.com/cosmos/cosmos-sdk/pull/9418) `sdk.Msg`'s `GetSigners()` method updated to return `[]string`.

8
x/gov/abci.go
View File

@ -16,10 +16,10 @@ func EndBlocker(ctx sdk.Context, keeper keeper.Keeper) {
logger := keeper.Logger(ctx)
// delete inactive proposal from store and its deposits
// delete dead proposals from store and burn theirs deposits. A proposal is dead when it's inactive and didn't get enough deposit on time to get into voting phase.
keeper.IterateInactiveProposalsQueue(ctx, ctx.BlockHeader().Time, func(proposal types.Proposal) bool {
keeper.DeleteProposal(ctx, proposal.ProposalId)
keeper.DeleteDeposits(ctx, proposal.ProposalId)
keeper.DeleteAndBurnDeposits(ctx, proposal.ProposalId)
// called when proposal become inactive
keeper.AfterProposalFailedMinDeposit(ctx, proposal.ProposalId)
@ -50,9 +50,9 @@ func EndBlocker(ctx sdk.Context, keeper keeper.Keeper) {
passes, burnDeposits, tallyResults := keeper.Tally(ctx, proposal)
if burnDeposits {
keeper.DeleteDeposits(ctx, proposal.ProposalId)
keeper.DeleteAndBurnDeposits(ctx, proposal.ProposalId)
} else {
keeper.RefundDeposits(ctx, proposal.ProposalId)
keeper.RefundAndDeleteDeposits(ctx, proposal.ProposalId)
}
if passes {

37
x/gov/client/cli/query.go
View File

@ -366,7 +366,7 @@ $ %s query gov deposit 1 cosmos1skjwj5whet0lpe65qaq4rpq03hjxlwd9nf39lk
// check to see if the proposal is in the store
ctx := cmd.Context()
proposalRes, err := queryClient.Proposal(
_, err = queryClient.Proposal(
ctx,
&types.QueryProposalRequest{ProposalId: proposalID},
)
@ -374,23 +374,6 @@ $ %s query gov deposit 1 cosmos1skjwj5whet0lpe65qaq4rpq03hjxlwd9nf39lk
return fmt.Errorf("failed to fetch proposal-id %d: %s", proposalID, err)
}
depositorAddr, err := sdk.AccAddressFromBech32(args[1])
if err != nil {
return err
}
var deposit types.Deposit
propStatus := proposalRes.Proposal.Status
if !(propStatus == types.StatusVotingPeriod || propStatus == types.StatusDepositPeriod) {
params := types.NewQueryDepositParams(proposalID, depositorAddr)
resByTxQuery, err := gcutils.QueryDepositByTxQuery(clientCtx, params)
if err != nil {
return err
}
clientCtx.Codec.MustUnmarshalJSON(resByTxQuery, &deposit)
return clientCtx.PrintProto(&deposit)
}
res, err := queryClient.Deposit(
ctx,
&types.QueryDepositRequest{ProposalId: proposalID, Depositor: args[1]},
@ -439,7 +422,7 @@ $ %s query gov deposits 1
// check to see if the proposal is in the store
ctx := cmd.Context()
proposalRes, err := queryClient.Proposal(
_, err = queryClient.Proposal(
ctx,
&types.QueryProposalRequest{ProposalId: proposalID},
)
@ -447,22 +430,6 @@ $ %s query gov deposits 1
return fmt.Errorf("failed to fetch proposal-id %d: %s", proposalID, err)
}
propStatus := proposalRes.GetProposal().Status
if !(propStatus == types.StatusVotingPeriod || propStatus == types.StatusDepositPeriod) {
params := types.NewQueryProposalParams(proposalID)
resByTxQuery, err := gcutils.QueryDepositsByTxQuery(clientCtx, params)
if err != nil {
return err
}
var dep types.Deposits
// TODO migrate to use JSONCodec (implement MarshalJSONArray
// or wrap lists of proto.Message in some other message)
clientCtx.LegacyAmino.MustUnmarshalJSON(resByTxQuery, &dep)
return clientCtx.PrintObjectLegacy(dep)
}
pageReq, err := client.ReadPageRequest(cmd.Flags())
if err != nil {
return err

2
x/gov/client/testutil/cli_test.go
View File

@ -1,5 +1,3 @@
// +build norace
package testutil
import (

168
x/gov/client/testutil/deposits.go
View File

@ -16,9 +16,10 @@ import (
type DepositTestSuite struct {
suite.Suite
cfg network.Config
network *network.Network
fees string
cfg network.Config
network *network.Network
deposits sdk.Coins
proposalIDs []string
}
func NewDepositTestSuite(cfg network.Config) *DepositTestSuite {
@ -34,8 +35,55 @@ func (s *DepositTestSuite) SetupSuite() {
_, err = s.network.WaitForHeight(1)
s.Require().NoError(err)
s.fees = sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(20))).String()
val := s.network.Validators[0]
deposits := sdk.Coins{
sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(0)),
sdk.NewCoin(s.cfg.BondDenom, types.DefaultMinDepositTokens.Sub(sdk.NewInt(50))),
}
s.deposits = deposits
// create 2 proposals for testing
for i := 0; i < len(deposits); i++ {
id := i + 1
deposit := deposits[i]
s.createProposal(val, deposit, id)
s.proposalIDs = append(s.proposalIDs, fmt.Sprintf("%d", id))
}
}
func (s *DepositTestSuite) SetupNewSuite() {
s.T().Log("setting up new test suite")
var err error
s.network, err = network.New(s.T(), s.T().TempDir(), s.cfg)
s.Require().NoError(err)
_, err = s.network.WaitForHeight(1)
s.Require().NoError(err)
}
func (s *DepositTestSuite) createProposal(val *network.Validator, initialDeposit sdk.Coin, id int) {
var exactArgs []string
if !initialDeposit.IsZero() {
exactArgs = append(exactArgs, fmt.Sprintf("--%s=%s", cli.FlagDeposit, initialDeposit.String()))
}
_, err := MsgSubmitProposal(
val.ClientCtx,
val.Address.String(),
fmt.Sprintf("Text Proposal %d", id),
"Where is the title!?",
types.ProposalTypeText,
exactArgs...,
)
s.Require().NoError(err)
_, err = s.network.WaitForHeight(1)
s.Require().NoError(err)
}
func (s *DepositTestSuite) TearDownSuite() {
@ -43,77 +91,42 @@ func (s *DepositTestSuite) TearDownSuite() {
s.network.Cleanup()
}
func (s *DepositTestSuite) TestQueryDepositsInitialDeposit() {
val := s.network.Validators[0]
clientCtx := val.ClientCtx
initialDeposit := sdk.NewCoin(s.cfg.BondDenom, types.DefaultMinDepositTokens.Sub(sdk.NewInt(20))).String()
// create a proposal with deposit
_, err := MsgSubmitProposal(val.ClientCtx, val.Address.String(),
"Text Proposal 1", "Where is the title!?", types.ProposalTypeText,
fmt.Sprintf("--%s=%s", cli.FlagDeposit, initialDeposit))
s.Require().NoError(err)
// deposit more amount
_, err = MsgDeposit(clientCtx, val.Address.String(), "1", sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(50)).String())
s.Require().NoError(err)
// waiting for voting period to end
time.Sleep(20 * time.Second)
// query deposit & verify initial deposit
deposit := s.queryDeposit(val, "1", false)
s.Require().Equal(deposit.Amount.String(), initialDeposit)
// query deposits
deposits := s.queryDeposits(val, "1", false)
s.Require().Equal(len(deposits), 2)
// verify initial deposit
s.Require().Equal(deposits[0].Amount.String(), initialDeposit)
}
func (s *DepositTestSuite) TestQueryDepositsWithoutInitialDeposit() {
val := s.network.Validators[0]
clientCtx := val.ClientCtx
// create a proposal without deposit
_, err := MsgSubmitProposal(val.ClientCtx, val.Address.String(),
"Text Proposal 2", "Where is the title!?", types.ProposalTypeText)
s.Require().NoError(err)
proposalID := s.proposalIDs[0]
// deposit amount
_, err = MsgDeposit(clientCtx, val.Address.String(), "2", sdk.NewCoin(s.cfg.BondDenom, types.DefaultMinDepositTokens.Add(sdk.NewInt(50))).String())
depositAmount := sdk.NewCoin(s.cfg.BondDenom, types.DefaultMinDepositTokens.Add(sdk.NewInt(50))).String()
_, err := MsgDeposit(clientCtx, val.Address.String(), proposalID, depositAmount)
s.Require().NoError(err)
// waiting for voting period to end
time.Sleep(20 * time.Second)
_, err = s.network.WaitForHeight(2)
s.Require().NoError(err)
// query deposit
deposit := s.queryDeposit(val, "2", false)
s.Require().Equal(deposit.Amount.String(), sdk.NewCoin(s.cfg.BondDenom, types.DefaultMinDepositTokens.Add(sdk.NewInt(50))).String())
deposit := s.queryDeposit(val, proposalID, false, "")
s.Require().NotNil(deposit)
s.Require().Equal(deposit.Amount.String(), depositAmount)
// query deposits
deposits := s.queryDeposits(val, "2", false)
s.Require().Equal(len(deposits), 1)
deposits := s.queryDeposits(val, proposalID, false, "")
s.Require().NotNil(deposits)
s.Require().Len(deposits.Deposits, 1)
// verify initial deposit
s.Require().Equal(deposits[0].Amount.String(), sdk.NewCoin(s.cfg.BondDenom, types.DefaultMinDepositTokens.Add(sdk.NewInt(50))).String())
s.Require().Equal(deposits.Deposits[0].Amount.String(), depositAmount)
}
func (s *DepositTestSuite) TestQueryProposalNotEnoughDeposits() {
val := s.network.Validators[0]
clientCtx := val.ClientCtx
initialDeposit := sdk.NewCoin(s.cfg.BondDenom, types.DefaultMinDepositTokens.Sub(sdk.NewInt(2000))).String()
// create a proposal with deposit
_, err := MsgSubmitProposal(val.ClientCtx, val.Address.String(),
"Text Proposal 3", "Where is the title!?", types.ProposalTypeText,
fmt.Sprintf("--%s=%s", cli.FlagDeposit, initialDeposit))
s.Require().NoError(err)
proposalID := s.proposalIDs[1]
// query proposal
args := []string{"3", fmt.Sprintf("--%s=json", tmcli.OutputFlag)}
args := []string{proposalID, fmt.Sprintf("--%s=json", tmcli.OutputFlag)}
cmd := cli.GetCmdQueryProposal()
_, err = clitestutil.ExecTestCLICmd(clientCtx, cmd, args)
_, err := clitestutil.ExecTestCLICmd(clientCtx, cmd, args)
s.Require().NoError(err)
// waiting for deposit period to end
@ -122,74 +135,81 @@ func (s *DepositTestSuite) TestQueryProposalNotEnoughDeposits() {
// query proposal
_, err = clitestutil.ExecTestCLICmd(clientCtx, cmd, args)
s.Require().Error(err)
s.Require().Contains(err.Error(), "proposal 3 doesn't exist")
s.Require().Contains(err.Error(), fmt.Sprintf("proposal %s doesn't exist", proposalID))
}
func (s *DepositTestSuite) TestRejectedProposalDeposits() {
// resetting state required (proposal is getting removed from state and proposalID is not in sequence)
s.TearDownSuite()
s.SetupNewSuite()
val := s.network.Validators[0]
clientCtx := val.ClientCtx
initialDeposit := sdk.NewCoin(s.cfg.BondDenom, types.DefaultMinDepositTokens)
id := 1
proposalID := fmt.Sprintf("%d", id)
// create a proposal with deposit
_, err := MsgSubmitProposal(clientCtx, val.Address.String(),
"Text Proposal 4", "Where is the title!?", types.ProposalTypeText,
fmt.Sprintf("--%s=%s", cli.FlagDeposit, initialDeposit))
s.Require().NoError(err)
s.createProposal(val, initialDeposit, id)
// query deposits
var deposits types.QueryDepositsResponse
args := []string{"4", fmt.Sprintf("--%s=json", tmcli.OutputFlag)}
args := []string{proposalID, fmt.Sprintf("--%s=json", tmcli.OutputFlag)}
cmd := cli.GetCmdQueryDeposits()
out, err := clitestutil.ExecTestCLICmd(val.ClientCtx, cmd, args)
s.Require().NoError(err)
s.Require().NoError(val.ClientCtx.LegacyAmino.UnmarshalJSON(out.Bytes(), &deposits))
s.Require().Equal(len(deposits.Deposits), 1)
// verify initial deposit
s.Require().Equal(deposits.Deposits[0].Amount.String(), sdk.NewCoin(s.cfg.BondDenom, types.DefaultMinDepositTokens).String())
s.Require().Equal(deposits.Deposits[0].Amount.String(), initialDeposit.String())
// vote
_, err = MsgVote(clientCtx, val.Address.String(), "4", "no")
_, err = MsgVote(clientCtx, val.Address.String(), proposalID, "no")
s.Require().NoError(err)
time.Sleep(20 * time.Second)
_, err = s.network.WaitForHeight(3)
s.Require().NoError(err)
args = []string{"4", fmt.Sprintf("--%s=json", tmcli.OutputFlag)}
args = []string{proposalID, fmt.Sprintf("--%s=json", tmcli.OutputFlag)}
cmd = cli.GetCmdQueryProposal()
_, err = clitestutil.ExecTestCLICmd(clientCtx, cmd, args)
s.Require().NoError(err)
// query deposits
depositsRes := s.queryDeposits(val, "4", false)
s.Require().Equal(len(depositsRes), 1)
depositsRes := s.queryDeposits(val, proposalID, false, "")
s.Require().NotNil(depositsRes)
s.Require().Len(depositsRes.Deposits, 1)
// verify initial deposit
s.Require().Equal(depositsRes[0].Amount.String(), initialDeposit.String())
s.Require().Equal(depositsRes.Deposits[0].Amount.String(), initialDeposit.String())
}
func (s *DepositTestSuite) queryDeposits(val *network.Validator, proposalID string, exceptErr bool) types.Deposits {
func (s *DepositTestSuite) queryDeposits(val *network.Validator, proposalID string, exceptErr bool, message string) *types.QueryDepositsResponse {
args := []string{proposalID, fmt.Sprintf("--%s=json", tmcli.OutputFlag)}
var depositsRes types.Deposits
var depositsRes *types.QueryDepositsResponse
cmd := cli.GetCmdQueryDeposits()
out, err := clitestutil.ExecTestCLICmd(val.ClientCtx, cmd, args)
if exceptErr {
s.Require().Error(err)
s.Require().Contains(err.Error(), message)
return nil
}
s.Require().NoError(err)
s.Require().NoError(val.ClientCtx.LegacyAmino.UnmarshalJSON(out.Bytes(), &depositsRes))
return depositsRes
}
func (s *DepositTestSuite) queryDeposit(val *network.Validator, proposalID string, exceptErr bool) *types.Deposit {
func (s *DepositTestSuite) queryDeposit(val *network.Validator, proposalID string, exceptErr bool, message string) *types.Deposit {
args := []string{proposalID, val.Address.String(), fmt.Sprintf("--%s=json", tmcli.OutputFlag)}
var depositRes types.Deposit
var depositRes *types.Deposit
cmd := cli.GetCmdQueryDeposit()
out, err := clitestutil.ExecTestCLICmd(val.ClientCtx, cmd, args)
if exceptErr {
s.Require().Error(err)
s.Require().Contains(err.Error(), message)
return nil
}
s.Require().NoError(err)
s.Require().NoError(val.ClientCtx.LegacyAmino.UnmarshalJSON(out.Bytes(), &depositRes))
return &depositRes
return depositRes
}

8
x/gov/keeper/deposit.go
View File

@ -53,8 +53,8 @@ func (keeper Keeper) GetDeposits(ctx sdk.Context, proposalID uint64) (deposits t
return
}
// DeleteDeposits deletes all the deposits on a specific proposal without refunding them
func (keeper Keeper) DeleteDeposits(ctx sdk.Context, proposalID uint64) {
// DeleteAndBurnDeposits deletes and burn all the deposits on a specific proposal.
func (keeper Keeper) DeleteAndBurnDeposits(ctx sdk.Context, proposalID uint64) {
store := ctx.KVStore(keeper.storeKey)
keeper.IterateDeposits(ctx, proposalID, func(deposit types.Deposit) bool {
@ -166,8 +166,8 @@ func (keeper Keeper) AddDeposit(ctx sdk.Context, proposalID uint64, depositorAdd
return activatedVotingPeriod, nil
}
// RefundDeposits refunds and deletes all the deposits on a specific proposal
func (keeper Keeper) RefundDeposits(ctx sdk.Context, proposalID uint64) {
// RefundAndDeleteDeposits refunds and deletes all the deposits on a specific proposal.
func (keeper Keeper) RefundAndDeleteDeposits(ctx sdk.Context, proposalID uint64) {
store := ctx.KVStore(keeper.storeKey)
keeper.IterateDeposits(ctx, proposalID, func(deposit types.Deposit) bool {

10
x/gov/keeper/deposit_test.go
View File

@ -95,16 +95,20 @@ func TestDeposits(t *testing.T) {
deposit, found = app.GovKeeper.GetDeposit(ctx, proposalID, TestAddrs[1])
require.True(t, found)
require.Equal(t, fourStake, deposit.Amount)
app.GovKeeper.RefundDeposits(ctx, proposalID)
app.GovKeeper.RefundAndDeleteDeposits(ctx, proposalID)
deposit, found = app.GovKeeper.GetDeposit(ctx, proposalID, TestAddrs[1])
require.False(t, found)
require.Equal(t, addr0Initial, app.BankKeeper.GetAllBalances(ctx, TestAddrs[0]))
require.Equal(t, addr1Initial, app.BankKeeper.GetAllBalances(ctx, TestAddrs[1]))
// Test delete deposits
// Test delete and burn deposits
proposal, err = app.GovKeeper.SubmitProposal(ctx, tp)
require.NoError(t, err)
proposalID = proposal.ProposalId
_, err = app.GovKeeper.AddDeposit(ctx, proposalID, TestAddrs[0], fourStake)
require.NoError(t, err)
app.GovKeeper.DeleteDeposits(ctx, proposalID)
app.GovKeeper.DeleteAndBurnDeposits(ctx, proposalID)
deposits = app.GovKeeper.GetDeposits(ctx, proposalID)
require.Len(t, deposits, 0)
require.Equal(t, addr0Initial.Sub(fourStake), app.BankKeeper.GetAllBalances(ctx, TestAddrs[0]))
}

6
x/gov/keeper/vote.go
View File

@ -78,8 +78,8 @@ func (keeper Keeper) GetVote(ctx sdk.Context, proposalID uint64, voterAddr sdk.A
// SetVote sets a Vote to the gov store
func (keeper Keeper) SetVote(ctx sdk.Context, vote types.Vote) {
// vote.Option is a deprecated field, we don't set it in state
if vote.Option != types.OptionEmpty { //nolint
vote.Option = types.OptionEmpty //nolint
if vote.Option != types.OptionEmpty { // nolint
vote.Option = types.OptionEmpty // nolint
}
store := ctx.KVStore(keeper.storeKey)
@ -135,6 +135,6 @@ func (keeper Keeper) deleteVote(ctx sdk.Context, proposalID uint64, voterAddr sd
// there's only 1 VoteOption.
func populateLegacyOption(vote *types.Vote) {
if len(vote.Options) == 1 && vote.Options[0].Weight.Equal(sdk.MustNewDecFromStr("1.0")) {
vote.Option = vote.Options[0].Option //nolint
vote.Option = vote.Options[0].Option // nolint
}
}

8
x/gov/spec/01_concepts.md
View File

@ -67,10 +67,12 @@ The deposit is kept in escrow and held by the governance `ModuleAccount` until t
### Deposit refund and burn
When a the a proposal finalized, the coins from the deposit are either refunded or burned, according to the final tally of the proposal:
When a proposal is finalized, the coins from the deposit are either refunded or burned, according to the final tally of the proposal:
- If the proposal is approved or if it's rejected but _not_ vetoed, deposits will automatically be refunded to their respective depositor (transferred from the governance `ModuleAccount`).
- When the proposal is vetoed with a supermajority, deposits be burned from the governance `ModuleAccount`.
- If the proposal is approved or rejected but _not_ vetoed, each deposit will be automatically refunded to its respective depositor (transferred from the governance `ModuleAccount`).
- When the proposal is vetoed with a supermajority, deposits will be burned from the governance `ModuleAccount` and the proposal information along with its deposit information will be removed from state.
- All refunded or burned deposits are removed from the state. Events are issued when burning or refunding a deposit.
- NOTE: The proposals which completed the voting period, cannot return the deposits when queried.
## Vote