refactor: remove query by events for x/gov deposits

[atheeshp]

Description

Closes: #9419

---

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 in the PR title
• added ! to the type prefix if API or client breaking change
• targeted the correct branch (see PR Targeting)
• provided a link to the relevant issue or specification
• followed the guidelines for building modules
• included the necessary unit and integration tests
• added a changelog entry to CHANGELOG.md
• included comments for documenting Go code
• 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...

• confirmed the correct type prefix in the PR title
• confirmed ! in the type prefix if API or client breaking change
• confirmed all author checklist items have been addressed
• reviewed state machine logic
• reviewed API design and naming
• reviewed documentation is accurate
• reviewed tests and test coverage
• manually tested (if applicable)

[codecov]

Codecov Report

Merging #9519 (3b0079f) into master (36ab23a) will increase coverage by 0.05%.
The diff coverage is 92.39%.

❗ Current head 3b0079f differs from pull request most recent head e26d3c4. Consider uploading reports for the commit e26d3c4 to get more accurate results

@@            Coverage Diff             @@
##           master    #9519      +/-   ##
==========================================
+ Coverage   63.47%   63.52%   +0.05%     
==========================================
  Files         566      566              
  Lines       53107    53077      -30     
==========================================
+ Hits        33711    33719       +8     
+ Misses      17488    17451      -37     
+ Partials     1908     1907       -1     

Impacted Files	Coverage Δ
x/gov/client/cli/query.go	0.00% <0.00%> (ø)
x/gov/keeper/vote.go	90.32% <33.33%> (ø)
x/gov/abci.go	91.57% <75.00%> (ø)
x/gov/client/testutil/deposits.go	94.11% <97.53%> (-0.73%)	⬇️
x/gov/keeper/deposit.go	81.14% <100.00%> (ø)
x/genutil/client/cli/init.go	67.39% <0.00%> (+6.48%)	⬆️

[robert-zaremba]

afaik, we are not deleting the proposal data for which the voting period crossed, we are only deleting the deposits information for those voting period crossed before to this PR.

This is what I meant with today: the current logic in master.
You are right, we are removing it from the "active queue" and moving it to a proposal proposals set.

[alexanderbez]

I do not think we should be keeping any deposits in state, even for proposals that have passed the voting period -- both failed and successful proposals. In any case, I do not think we should be keeping deposits in state.

Just want to confirm, the proposal in this PR is to keep the deposits in certain circumstances?

[robert-zaremba]

@alexanderbez see the tree solutions in my post above. If we want to keep some proposals and be able to query for not refunded deposits (burned), then for integrity, imho, we should not delete that deposit records (but we can and should delete refunded deposits).
If we don't need to query that deposits, then we can use solution 2.

[atheeshp]

I do not think we should be keeping any deposits in state, even for proposals that have passed the voting period -- both failed and successful proposals. In any case, I do not think we should be keeping deposits in state.

Just want to confirm, the proposal in this PR is to keep the deposits in certain circumstances?

Yeah, I agree with you. But our question is when a voting period completed proposal's deposits queried what should be returned?
If returning "zero or null" is fine for those proposals, then I can make the changes accordingly.

[atheeshp]

Just want to confirm, the proposal in this PR is to keep the deposits in certain circumstances?

No, for now we are keeping them only for fulfilling the deposits or deposit queries.

[alexanderbez]

Yeah, I agree with you. But our question is when a voting period completed proposal's deposits queried what should be returned?
If returning "zero or null" is fine for those proposals, then I can make the changes accordingly.

I suppose nothing should be returned, i.e. [].

[atheeshp]

Yeah, I agree with you. But our question is when a voting period completed proposal's deposits queried what should be returned?
If returning "zero or null" is fine for those proposals, then I can make the changes accordingly.

I suppose nothing should be returned, i.e. [].

@robert-zaremba, I udpated this PR according to the @alexanderbez 's suggeston, can you review again.

[robert-zaremba]

Yeah, I agree with you. But our question is when a voting period completed proposal's deposits queried what should be returned?
If returning "zero or null" is fine for those proposals, then I can make the changes accordingly.

Yes, this is basically solution 2.

[atheeshp]

Yeah, I agree with you. But our question is when a voting period completed proposal's deposits queried what should be returned?
If returning "zero or null" is fine for those proposals, then I can make the changes accordingly.

Yes, this is basically solution 2.

Yes

[robert-zaremba]

utACK. Left one suggestion in the docs.

[blushi]

@atheeshp @robert-zaremba should we put automerge on?

[atheeshp]

@atheeshp @robert-zaremba should we put automerge on?

I think we should.
cc: @robert-zaremba