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

Proposals cannot be executed based on quorum after voting period over #14

Closed
uint opened this issue Jan 11, 2022 · 5 comments · Fixed by #18
Closed

Proposals cannot be executed based on quorum after voting period over #14

uint opened this issue Jan 11, 2022 · 5 comments · Fixed by #18
Assignees
Labels
bug Something isn't working

Comments

@uint
Copy link
Contributor

uint commented Jan 11, 2022

Proposals can pass early or after the voting period is over. If a proposal doesn't pass early, but after the voting period is over, it will still be Status::Open internally and execute_execute will error.

This affects tgrade-community-pool, tgrade-validator-voting, oc-proposals and cw3-flex-multisig.

The simplest solution is to do something like Ethan's suggestion in #12 - use .current_status() rather than directly comparing the .status field.

More importantly, we need tests for this situation.

@ethanfrey
Copy link
Contributor

yes, tests first and a failing CI run before the fix. 💯
Thanks for looking into this and finding the bug

@uint
Copy link
Contributor Author

uint commented Jan 11, 2022

Here's a test case to better illustrate this:

threshold: 50%
quorum: 20%
allow_end_early: doesn't matter

alice: 4 voting power
bob: 6 voting power

1. Alice votes YES, Bob doesn't vote.
2. status = open, because Alice's vote couldn't pass proposal early (4/10 votes are YES, Bob can still reject proposal).
3. Wait for the voting period to end.
4. status = passed (in query, not internally)
5. Anyone should now be able to execute the proposal - 20% quorum was met, 100% cast votes are YES.

@uint
Copy link
Contributor Author

uint commented Jan 11, 2022

Bonus points: move the logic around checking/updating statuses in execute_execute to voting-contract somehow, and test it there.

@uint uint added the bug Something isn't working label Jan 11, 2022
@ethanfrey
Copy link
Contributor

Some of the execute_execute logic is generic, the rest is dispatching the actual proposal.

Maybe we could have a generic execute_execute<P> that checks the status and updates it and then dispatches to some Fn(DepsMut, Env, Info, P) -> Result<Response, E> which is passed as an arg?

This whole issue of extensibility is a bit tricky for me. Whether subclassing, or decorators, or other meta-programming tricks. I find it a bit tricky to extend other code in many places.

@ethanfrey
Copy link
Contributor

Closed by #18

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants