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

Fractional Governance Voting #2929

Open
apbendi opened this issue Oct 28, 2021 · 11 comments
Open

Fractional Governance Voting #2929

apbendi opened this issue Oct 28, 2021 · 11 comments

Comments

@apbendi
Copy link
Contributor

apbendi commented Oct 28, 2021

🧐 Motivation

We'd like to implement a version/extension of Governor that enables delegates to distribute their total voting weight fractionally across For/Against/Abstain. There are many possible uses for this, many of them related to allowing owners of governance tokens which are pooled to participate in Governance votes. Take, for example, cUNI community voting.

📝 Details

In current Governor implementations, a delegate assigns all their voting weight to a single choice: For/Against/Abstain. Specifically, this is implemented concretely in the _countVote implementation in GovernorCountingSimple.

We'd like to implement a version that enables the delegate to split their voting weight fractionally. For example, a delegate with a weight of 1,000 might assign 700 to For, 100 to Against, and 200 to Abstain.

One challenge with this idea is that it can't trivially be implemented as an extension to the existing Governor implementation or interface in the same way as GovernorCountingSimple.

Specifically, GovernorCountingSimple implements the virtual _countVotes method, which is itself called by the internals of the concrete Governor.

A hypothetical GovernorCountingFractional faces a challenge here, because there is no where in the signature of _countVotes method to include data about how to fractionalize the votes. The only available parameter is the support parameter, but as a uint8, it's not big enough to hold data about how to split votes between 3 discreet options.

Given this, we'd like some feedback on how we might go about implementing this feature in a way you'd be likely to accept. There are any number of options, but here are a few to consider:

  1. Change the signature of _countVote, and the castVote family of methods, to make support a uint256. With the extra bits, we could pack the vote counts/proportions for each options into the support parameter. This is an easy approach, with minimal surface are for the implementation, but the obvious downside is backwards compatibility as it changes the signatures.
  2. Extend Governor and add a new castWeightedVote family of methods. This works, and contains the fractional voting changes to a single new file, but it feels a bit hacky. The castWeightedVote methods added would exist on the inheriting class, but effectively "redo" much of the functionality from the base Governor. It eschews the benefits of using the inheritance pattern in the first place.
  3. Create a new IGovernorFractional interface and GovernorFractional implementation. In this version, the IGovernorFractional would extend and add fractional voting methods to the IGovernor interface. The GovernorFractional interface would implement the castWeightedVote methods. Finally, a GovernorFractionalCountingSimple implementation would provide a concrete implementation of a new, virtual _countFractionalVote method. Internally, the _countVote implementation could curry to the new _countFractionalVote method for backwards compatibility with the inherited, non-fractional voting methods.

We're totally open to feedback on this feature proposal, along with the best way to go about implement it. We believe this feature could be beneficial to many projects opting for OZ implementations of Governance. We're eager and willing to tackle the implementation ourselves and open a PR, but want to do it in a way that will fit with the rest of the library. Thanks in advance for your consideration!

@frangio
Copy link
Contributor

frangio commented Nov 1, 2021

Thank you for this proposal!

One challenge with this idea is that it can't trivially be implemented as an extension to the existing Governor implementation or interface in the same way as GovernorCountingSimple.

I want to push back on this a little. My intuition is that something equivalent to the approach you described can be implemented with the existing system.

Instead of thinking of a vote as being fractionally distributed across three discrete options (for, abstain, and against), we can think of a vote as a number between 0 and N expressing a level of support for the proposal, with 0 being fully against the proposal, and N being fully in favor of the proposal. The numbers in between can express a more nuanced position, and the number exactly in the middle expresses indiference as to the result (i.e. abstention).

This voting system can be implemented using the existing support parameter of castVote. After delegates have voted expressing their level of support for a proposal, the contract can obtain the average level of support, weighted by the voting power of each delegate. If this weighted average is in the upper half of the range, the proposal passes, because most people expressed support for it.

Note that this generalizes the system implemented in CountingSimple: if you take N = 2, you get almost exactly that system. (Except for the fact that the enum of support options is ordered differently! Abstain is encoded as "2" for historical reasons, where according to the system described above we would get Against = 0, Abstain = 1, For = 2.)

If we take N = 255, we are able to express a much wider range of levels of support. The scenario that you described is one where this wider range becomes very useful. If a delegate represents a large number of votes, and those votes do not all agree on a proposal, the support number can represent that quite precisely.

First let's imagine out of 1000 votes only 700 are in favor of a proposal, and the other 300 are against it, i.e. 70% of those votes are in favor. 70% of 255 (N = 255) is ~194, so the delegate should cast a vote with support = 194. The weighted average in the end will be affected in the same way as if the 70% and 30% of voting power had voted separately for and against respectively.

Abstain is a little harder to see, but we can think about it as being half-against and half-in-favor. If out of 1000 votes there are 700 in favor, 200 abstain, and 100 against, we represent it the same as if it was 800 in favor and 200 against. 80% of 255 is 204, so the delegate casts a vote with support = 204. I don't have a simple way to transmit the intuition that this results in the same outcome, but I do think it does. We should think about that more and try to explain it well.

Another point that needs to be discussed about what I'm proposing is when should quorum be considered reached. I think it would make sense to specify a threshold of total support (not the weighted average but the weighted sum), which determines if quorum is reached. This is where it stops being a generalization of CountingSimple, because abstain votes would only count as half a vote as far as quorum is concerned.

@apbendi
Copy link
Contributor Author

apbendi commented Nov 4, 2021

Hey @frangio, thanks so much for the thoughtful response and proposal! I definitely agree in principle that if we could find a way to make this work without changing the interface, that would be ideal. I do see some potential issues with your proposal, though, so let me share some of those to get your thoughts.

First, the precision issue. With 255 steps, the maximum precision we can represent is about 0.4%. This might be an acceptable tradeoff, but it's not insignificant, especially given one of the top imagined use cases for fractional voting is large pools of governance tokens held in contracts. Take, as a representative example, the cUNI pool, which has something on the order of 10 Million UNI. If that pool were to leverage this method to allow cUNI holders to express their preferences, it could result in an error in weighting of 40,000 UNI. That's currently >$1 Million in economic weight, and more than enough to swing the result in the case of a modestly contentious vote.

The second issue is related to representing abstentions. If I'm understanding your proposal correctly you're arguing that splitting "abstain" votes equally between For and Against would have the same effect as Abstain. However, this doesn't seem to be the case, at least not with regards to the way abstentions are implemented by Bravo (and your concrete implementations which follows Bravo).

In short, in the current system:

  • The quorum threshold is determined by summing For + Abstain
  • The success of a proposal is determined by For > Against

This makes Abstain a discreet option, one which has its own impact on the results of the vote. These impacts are different from splitting the same number of votes between For and Against. Changing the definition of Abstain to mean "split between For and Against" might be a reasonable choice, but it would be a meaningful one that a DAO would have to consider carefully. It certainly would impact the game theoretic ways in which this feature could be used.

One additional complication here: "No Vote", i.e. not voting at all is, in a way, a discreet option for voting as well. It's a debatable question whether a fractional voting system should enable a delegate (which, it's important to remember, might be a contract itself) to vote with only a fraction of their weight, or whether it should require all weight be split between For/Against/Abstain. One could imagine a pool with 1000 weight choosing only to vote with 600 of its voting power 300 For/100 Against/200 Abstain, leaving 400 as "No Vote". Whether this "should" be implemented or not, it would be nice if the architecture at least made it possible for a project to do so.


With all the above in mind, if you'll humor me, I want to take a minute to "lobby" for biting the bullet and making the small breaking change to upgrade the support parameter from uint8 to uint256.

Regardless of how we ultimately tackle fractional voting, I think undertaking this exploration has revealed something to me: the current interface is pretty limited in its ability to be extended to implement alternative voting schemes. We can find a way to make fractional voting work, but the underlying limitation may still prove restricting to other experiments in the future.

Let me give a completely contrived example. Imagine some project wanted to enable voting where delegates could express not only their preference For/Against/Abstain, but also an amount. This amount might represent the quantity of governance tokens to be awarded as a grant, and the vote counting implementation might do something like calculate a voting-power-weighted average to determine the final number. So if the proposal was to grant tokens to a community fund, one delegate might vote Against, while another might vote For/10,000, and another might vote For/30,000.

Implementing the above example (which, again, is completely contrived and just meant to be representative of a multitude of possible voting schemes) would be impossible using the current interface. A would-be implementor would have to do something similar to what I'm proposing in Option 2 or Option 3 in the original issue above, except for their specific usecase, instead of for Fractional Voting.

On the other hand, if the support parameter was uint256, they could easily pack both the amount and the preference in the 256 bits available. Their implementation would simply be an extension of Governor with a custom _countVotes method to unpack the bits, and do whatever calculations/storage are necessary.

In other words, the architecture you've chosen here— with the virtual _countVotes method that can be overriden and implemented concretely to count votes in any custom way— is a really really nice abstraction, and makes creating custom implementations really clean and easy to reason about. But, it's currently hamstrung by a lack of flexibility— there's just not enough surface area to include more data.

Bumping the support param to uint256 is a small change that gives the system significantly more flexibility. Given this has not yet been codified as a standard, or deployed too widely by too many projects, it feels worthwhile to seriously consider making the breaking change.

Ok, that's my pitch 😅 . Very curious to hear what you think. Thanks again for your time and thoughtful consideration here!

@frangio
Copy link
Contributor

frangio commented Nov 4, 2021

the current interface is pretty limited in its ability to be extended to implement alternative voting schemes

I 100% agree with this. It came up as an issue recently when it was suggested that a simple way to implement voting with NFTs requires sending in a list of NFTs together with a vote, but the interface doesn't allow that (#2873 (comment)).

Unfortunately we commit to backwards compatibility and care a lot about it, so we're not willing to make breaking changes like extending uint8 to uint256. Even though the Governor is not standardized, there is a lot of infrastructure already in place, even since before we released our implementation.

However, I think we can and should look into backwards compatible ways of extending the contract. What I'd like us to explore is the ability to submit additional arbitrary data with a vote, that an instance of a Governor could make use of in specific ways, and could even declare how it uses that data via the COUNTING_MODE introspection method.

So in my proposal you could add a function castVote(proposal, support, data) where data encodes the fractional distribution of votes or any of the other options you described.

There should be a reasonable default for this additional data.

Do you think this could work?

@frangio
Copy link
Contributor

frangio commented Nov 4, 2021

We could add a separate castVote with uint256 support, in a backwards compatible way, but I worry this is not general enough for other kinds of customizations.

@apbendi
Copy link
Contributor Author

apbendi commented Nov 4, 2021

Thanks again @frangio! More thoughts below.

Unfortunately we commit to backwards compatibility and care a lot about it, so we're not willing to make breaking changes like extending uint8 to uint256. Even though the Governor is not standardized, there is a lot of infrastructure already in place, even since before we released our implementation.

I totally get that. Makes sense.

you could add a function castVote(proposal, support, data) where data encodes the fractional distribution of votes or any of the other options you described.

I like this idea, and I think it makes a lot of sense! Your example of NFT voting is great demonstration of how not even 256 bits would work for many cases that could be imagined.

My initial response is that we'd be happy to help define the new interface, and to implement fractional voting within this framework. I want to dig in a little bit to make sure I understand how the proposal would work. I haven't actually tried spiking this out yet, but, going off the top of my head, does all this sound correct?

  • Create a new interface, named something like IGovernorExtensible (any better ideas on naming?) that extends from the existing IGovernor interface.
  • Add a new family of castVote methods which add a bytes data parameter
  • Add a new virtual _countVotes method which adds the bytes data parameter
  • Create a new GovernorExtensible implementation, and curry old castVote methods (without the data parameter) to the new castVote methods, passing empty data
  • In GovernorExtensible, also concretely implement the old _countVotes method (without the data parameter) and curry to the new, virtual _countVotes method, passing empty data

Now, anyone that wants to implement a custom voting method with extra data extends GovernorExtensible and concretely implements the _countVotes method WITH the data param. This is what we would do for our fractional voting usecase.

Let me know what you think of this approach! If I haven't missed anything, and it makes sense to you, we can start working on this.

@frangio
Copy link
Contributor

frangio commented Nov 5, 2021

What you described could work but it's different from what I had in mind. I was thinking we should actually change the core Governor and add a _castVote with data (we should find another name for this), and have all other entry points end up there. I think it's important for there to be a single source of truth function for how casting a vote should behave.

I don't know which of the two options (this one vs GovernorExtensible) would work best, I'd need to try to write it out and see if they actually work to implement something like fractional votes.

Please do go ahead and build this, I think it's really needed. I'd suggest trying out both options, I have a hunch only one of them is really going to work (not sure which one).

@apbendi
Copy link
Contributor Author

apbendi commented Nov 9, 2021

Thanks @frangio, this sounds great! I'll start spiking out both options and see how they go. Will share diffs to each once I have them. Not going to bother with tests/documentation for now. Considering this exploratory— possibly even throwaway— code for now.

@Amxx
Copy link
Collaborator

Amxx commented Nov 10, 2021

@apbendi
Copy link
Contributor Author

apbendi commented Nov 11, 2021

hi @frangio, after some initial exploration of this approach, I actually have some additional questions here. In particular, I'm now wondering what's the point of having a generic "data" parameter if a would-be integrator still needs to know how to encode/decode the data for each specific implementation?

In this regard, it feels a little like perhaps the generic approach doesn't actually gain much, and would be strictly worse than adding methods with specific parameters for fractional voting. Does this question make sense? Curious to hear what you think.

@frangio
Copy link
Contributor

frangio commented Nov 12, 2021

Yes this is a valid question. My argument in favor of the generic data parameter is that I think we need it supported at the lower level in the core Governor contract, and passed internally to the relevant functions, if we want to build customizations on top of it like the ones we've mentioned in this thread.

The external interface can offer functions with specific parameters for fractional voting or other features, but would be implemented by this generic primitive internally.

This can all be solved perhaps with less work by forking the code and making the ad-hoc changes required, but having this generic primitive internally is a building block that can be reused for some of the other use cases that are currently not covered.

The encoding problem is not different from the one already present for the uint8 support parameter. An integrator needs to know how to encode a "For" vote into a uint8 value, for example. The way that we've tried to solve this is by having the contract self-document the encoding it expects, through the COUNTING_MODE getter:

function COUNTING_MODE() public pure virtual override returns (string memory) {
return "support=bravo&quorum=for,abstain";

Eventually there would be a registry that documents what a particular value for support means. For example, support=bravo means that Against is encoded as 0, For encoded as 1, and Abstain encoded as 2.

If the generic data parameter were directly exposed (as opposed to wrapping the internal mechanism in an ad-hoc external function), what I had in mind was that the contract can similarly document the encoding in COUNTING_MODE, maybe under another key.

@apbendi
Copy link
Contributor Author

apbendi commented Dec 21, 2021

Hey @frangio, I've opened a PR with the interface extension we discussed here: #3043. Eager to hear what you think!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants