Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gov: Only burn on vetoed proposals #11010

Closed
2 of 4 tasks
tac0turtle opened this issue Jan 24, 2022 · 10 comments · Fixed by #11011
Closed
2 of 4 tasks

gov: Only burn on vetoed proposals #11010

tac0turtle opened this issue Jan 24, 2022 · 10 comments · Fixed by #11011
Labels
C:x/gov S:needs architecture review To discuss on the next architecture review call to come to alignment T: UX

Comments

@tac0turtle
Copy link
Member

Summary

Currently we burn assets in governance in three places.

  • vetoed proposal
  • unable to reach quorum
  • unable to reach deposit

Proposal

reduce burning of funds to only vetoed proposals.


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@amaury1093
Copy link
Contributor

Can you explain the rationale behind this? Is there a risk of seeing an increase of non-serious proposals?

@tac0turtle
Copy link
Member Author

this came from https://twitter.com/unitylchaos_/status/1485370764421308417?s=20. I tend to agree that this should not be the design. I know there is a risk of more proposals coming in, but the spam is still present since anyone could open a proposal with a minuscule amount of tokens.

@alexanderbez
Copy link
Contributor

I think this needs more discussion. I personally believe deposits should be burned on props that fail to meet quorum or min deposits or that are vetoed.

@tac0turtle
Copy link
Member Author

I think deposits don't really matter as I could make a proposal with .001 of a token essentially making the cost minimal to open things.

Failure to meet quorum is tricky without dynamic quorums. There have been a few cases on other chains that people are for the proposal but the marketing wasn't there to get people to vote.

@alexanderbez
Copy link
Contributor

Yes, you can spam. But there are many ways you can spam a network. That doesn't mean we shouldn't necessarily burn deposits IMO.

@amaury1093 amaury1093 added the S:needs architecture review To discuss on the next architecture review call to come to alignment label Jan 24, 2022
@hxrts
Copy link
Contributor

hxrts commented Jan 26, 2022

Had a discussion about this on the most recent gov/groups call. Conclusion was that long-term we should separate mechanisms for preventing transaction spam (not unique to gov proposals) + governance spam (more about people's attention). We have some ideas to do that for future iterations of gov/groups. In the short-term, I support this change as proposals still need to reach the deposit threshold to get validator attention.

@mergify mergify bot closed this as completed in #11011 Jan 26, 2022
mergify bot pushed a commit that referenced this issue Jan 26, 2022
## Description

Closes: #11010 

only burn deposits on veto'd proposals/ 

---

### 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...

- [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) 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)
@robert-zaremba
Copy link
Collaborator

It seams #11011 got merged without reaching a consensus here. Reopening to getting a consensus.

Personally, I also think that proposal deposits should be be burned in the last case (not reaching the minimum deposit level), and agree that for missing quorum we should not burn the deposits.

@hxrts
Copy link
Contributor

hxrts commented Jan 27, 2022

most validators aren't tracking anything that doesn't hit the min deposit level so it's not spamming their attention. any block space spam mechanism should be in line with what we're doing for groups/authz.

@alexanderbez
Copy link
Contributor

most validators aren't tracking anything that doesn't hit the min deposit level so it's not spamming their attention. any block space spam mechanism should be in line with what we're doing for groups/authz.

this++

@robert-zaremba
Copy link
Collaborator

We are going to make the burning configurable (opt-in): #11057

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:x/gov S:needs architecture review To discuss on the next architecture review call to come to alignment T: UX
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

5 participants