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

Update cw3 spec #188

Merged
merged 4 commits into from
Dec 16, 2020
Merged

Update cw3 spec #188

merged 4 commits into from
Dec 16, 2020

Conversation

ethanfrey
Copy link
Member

Closes #182

Also added lots of comments to clarify how the various threshold types work.

@ethanfrey
Copy link
Member Author

@maurolacy please review this first before looking more at #180. I hope this documents everything better. If you approve, you can merge it into that PR, but I wanted to pull out the documentation issues into an easy-to-review subset.

You can skim down to the cw3 package, that is where the real changes are made. The contracts were just updated to follow the changes in the spec.

@ethanfrey ethanfrey mentioned this pull request Dec 15, 2020
5 tasks
Copy link
Contributor

@maurolacy maurolacy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Much better, thanks. I've made some changes, but they are just formal, i. e. they don't substantially change anything you've written.

Comment on lines +42 to +43
/// Veto is generally to be treated as a no vote. Some implementations may allow certain
/// voters to be able to Veto, or them to be counted stronger then nos in some way.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// Veto is generally to be treated as a no vote. Some implementations may allow certain
/// voters to be able to Veto, or them to be counted stronger then nos in some way.
/// Veto is generally to be treated as a No vote. Some implementations may allow certain
/// voters to be able to Veto, or them to be counted stronger than No in some way.

/// The total_weight used for calculating success as well as the weights of each
/// individual voter used in tallying should be snapshotted at the beginning of
/// the block at which the proposal starts (this is likely the responsibility of a
/// correct cw4 implementation).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

/// Example: we set `percentage` to 51%. Proposal 1 starts when there is a `total_weight` of 5.
/// This will require 3 weight of yes votes in order to pass. Later, the Proposal 2 starts but the
/// `total_weight` of the group has increased to 9. That proposal will then automatically
/// require 5 yes of 9 to pass, rather than 3 yes of 9 as would be the case with `AbsoluteCount`.
Copy link
Contributor

@maurolacy maurolacy Dec 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, this was the source of my confusion. In my opinion, there's no need to talk about / mention different proposals.

By mentioning snaoshotting of the group weights above, it's understood that every proposal will work with a snapshot of the group weights, taken at the moment of (each) proposal creation.

I'll see if I can come up with a better statement here, but for the most part I think that this is addressed / clear enough.

Comment on lines +72 to +83
/// Declares a percentage of the total weight that must cast yes votes in order for
/// a proposal to pass. As with `AbsoluteCount`, it only matters the sum of yes votes.
///
/// This is useful for similar circumstances as `AbsoluteCount`, where we have a relatively
/// small set of voters and participation is required. The advantage here is that if the
/// voting set (group) changes between proposals, the number of votes needed is adjusted
/// accordingly.
///
/// Example: we set `percentage` to 51%. Proposal 1 starts when there is a `total_weight` of 5.
/// This will require 3 weight of yes votes in order to pass. Later, the Proposal 2 starts but the
/// `total_weight` of the group has increased to 9. That proposal will then automatically
/// require 5 yes of 9 to pass, rather than 3 yes of 9 as would be the case with `AbsoluteCount`.
Copy link
Contributor

@maurolacy maurolacy Dec 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// Declares a percentage of the total weight that must cast yes votes in order for
/// a proposal to pass. As with `AbsoluteCount`, it only matters the sum of yes votes.
///
/// This is useful for similar circumstances as `AbsoluteCount`, where we have a relatively
/// small set of voters and participation is required. The advantage here is that if the
/// voting set (group) changes between proposals, the number of votes needed is adjusted
/// accordingly.
///
/// Example: we set `percentage` to 51%. Proposal 1 starts when there is a `total_weight` of 5.
/// This will require 3 weight of yes votes in order to pass. Later, the Proposal 2 starts but the
/// `total_weight` of the group has increased to 9. That proposal will then automatically
/// require 5 yes of 9 to pass, rather than 3 yes of 9 as would be the case with `AbsoluteCount`.
/// Declares a percentage of the total weight that must cast Yes votes, in order for
/// a proposal to pass. As with `AbsoluteCount`, Yes votes are the only ones that count.
///
/// This is useful for similar circumstances as `AbsoluteCount`, where we have a relatively
/// small set of voters, and participation is required.
/// It is understood that if the voting set (group) changes between different proposals that refer to the same group, each proposal will work with a
/// different set of voter weights (the ones snapshotted at proposal creation), and the passing weight for each proposal will be computed
/// based on the absolute percentage, times the total weights of the members at the time of each proposal creation.
///
/// Example: we set `percentage` to 51%. Proposal 1 starts when there is a `total_weight` of 5.
/// This will require 3 weight of yes votes in order to pass. Later, the Proposal 2 starts but the
/// `total_weight` of the group has increased to 9. That proposal will then automatically
/// require 5 yes of 9 to pass, rather than 3 yes of 9 as would be the case with `AbsoluteCount`.

Better?

Maybe the example is now redundant.

I'll still consider adding a paragraph about subtracting Abstain votes (if wee agree on that that should be considered here too).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, and more explicit.

We say we only count Yes votes, so no need to mention Abstain explicitly, right?

Comment on lines +91 to +109
/// Declares a `quorum` of the total votes that must participate in the election in order
/// for the vote to be considered at all. Within the votes that were cast, it requires `threshold`
/// in favor. That is calculated by ignoring the abstain votes (they count towards `quorum`
/// but do not influence `threshold`). That is, we calculate `yes / (yes + no + veto)`
/// and compare that with `threshold` to consider if the proposal was passed.
///
/// It is rather difficult for a proposal of this type to pass early. That can only happen if
/// the required quorum has been already met, and in the case if all remaining voters were
/// to vote no, the threshold would still be met.
///
/// 30% yes votes, 10% no votes, and 20% abstain would pass early if quorum <= 60%
/// (who has cast votes) and if the threshold is <= 37.5% (the remaining 40% voting
/// no => 30% yes + 50% no). Once the voting period has passed with no additional votes,
/// that same proposal would be considered successful if quorum <= 60% and threshold <= 75%
/// (percent in favor if we ignore abstain votes).
///
/// This type is more common in general elections where participation is expected to often
/// be low, and `AbsolutePercentage` would either be too restrictive to pass anything,
/// or allow low percentages to pass if there was high participation in one election.
Copy link
Contributor

@maurolacy maurolacy Dec 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// Declares a `quorum` of the total votes that must participate in the election in order
/// for the vote to be considered at all. Within the votes that were cast, it requires `threshold`
/// in favor. That is calculated by ignoring the abstain votes (they count towards `quorum`
/// but do not influence `threshold`). That is, we calculate `yes / (yes + no + veto)`
/// and compare that with `threshold` to consider if the proposal was passed.
///
/// It is rather difficult for a proposal of this type to pass early. That can only happen if
/// the required quorum has been already met, and in the case if all remaining voters were
/// to vote no, the threshold would still be met.
///
/// 30% yes votes, 10% no votes, and 20% abstain would pass early if quorum <= 60%
/// (who has cast votes) and if the threshold is <= 37.5% (the remaining 40% voting
/// no => 30% yes + 50% no). Once the voting period has passed with no additional votes,
/// that same proposal would be considered successful if quorum <= 60% and threshold <= 75%
/// (percent in favor if we ignore abstain votes).
///
/// This type is more common in general elections where participation is expected to often
/// be low, and `AbsolutePercentage` would either be too restrictive to pass anything,
/// or allow low percentages to pass if there was high participation in one election.
/// In addition to a `threshold`, declares a `quorum` of the total votes that must participate in the election in order
/// for the vote to be considered at all. Within the votes that were cast, it requires `threshold` votes
/// in favor. That is calculated by ignoring the Abstain votes (they count towards `quorum`,
/// but do not influence `threshold`). That is, we calculate `Yes / (Yes + No + Veto)`
/// and compare that with `threshold` to consider if the proposal was passed.
///
/// It is rather difficult for a proposal of this type to pass early. That can only happen if
/// the required quorum has been already met, and there are already enough Yes votes for the proposal to pass.
///
/// 30% Yes votes, 10% No votes, and 20% Abstain would pass early if quorum <= 60%
/// (who has cast votes) and if the threshold is <= 37.5% (the remaining 40% voting
/// no => 30% yes + 50% no). Once the voting period has passed with no additional votes,
/// that same proposal would be considered successful if quorum <= 60% and threshold <= 75%
/// (percent in favor if we ignore abstain votes).
///
/// This type is more common in general elections, where participation is often expected to
/// be low, and `AbsolutePercentage` would either be too hight to pass anything,
/// or allow low percentages to pass, independently of if there was high participation in one election or not.

Here, I think the example could be removed too.

We can still iterate on this a couple times IMO, as these are important / useful remarks. And also, in that way we make sure that we agree on them. ;-)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the example is useful to explain why it is hard to pass early.
I could definitely be worded better

pub expires: Expiration,
/// This is the threshold that is applied to this proposal. Both the rules of the voting contract,
/// as well as the total_weight of the voting group may have changed since this time. That means
/// that the generic `Threshold{}` query does not provide valid information for existing proposals.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important point. Thanks for that last remark.

@maurolacy
Copy link
Contributor

OK, I'll merge and we can follow-up / revisit in the original PR.

@maurolacy maurolacy merged commit 0719092 into add-threshold-to-cw3-flex Dec 16, 2020
@maurolacy
Copy link
Contributor

Applying proposed changes locally, for formatting.

@maurolacy maurolacy deleted the update-cw3-spec branch December 16, 2020 12:59
@ethanfrey
Copy link
Member Author

I think I'll merge in your proposed changes, I was just taking care of other business first.
Guess I can do that on the main PR branch

@maurolacy
Copy link
Contributor

maurolacy commented Dec 16, 2020

Beware that I already pushed them. In any case, let's discuss them in the original branch.

@ethanfrey
Copy link
Member Author

Ah, okay

This was referenced Dec 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants