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

[erc20-votes-with-override] Adding ERC-20 votes with override strategy. #448

Conversation

serenae-fansubs
Copy link
Contributor

@serenae-fansubs serenae-fansubs commented Mar 8, 2022

Addresses #449.

This strategy is similar to ERC-20 Votes, except that it also allows individual delegators to override their vote on a particular proposal if they wish. This is most useful for social (off-chain only) proposals.

If an account has any delegated voting power returned from getVotes, adds that value, minus the balances from any delegators that have also individually voted.

If an account is delegating to itself, then its own token balance will already be included in the getVotes return value.

If an account is delegating to a different valid address, adds the local token balance. The account must be delegated to another valid address, otherwise the local token balance will not be added.

@ChaituVR

This comment was marked as resolved.

@ChaituVR ChaituVR changed the title Adding ERC-20 votes with override strategy. [erc20-votes-with-override] Adding ERC-20 votes with override strategy. Mar 9, 2022
@serenae-fansubs
Copy link
Contributor Author

@ChaituVR Thank you! Addressed those comments about Object/Record, all checks are passing now :)

Comment on lines 21 to 24
If getVotes returns a nonzero value, then that is used. Then any delegators
that have individually voted will have their balances subtracted from the
result.

Copy link
Member

Choose a reason for hiding this comment

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

How do the getVotes know that users have already voted? Sorry @serenae-fansubs Trying to get my brain around it :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's the OpenZeppelin getVotes method, and really maybe it would have made more sense if they called it "getVotingPower" or something, I can see the confusion. That method is just retrieving the total sum of that account's delegated voting power.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also updated the comments there to make it more clear that it's talking about voting power, and refactored to make it a bit easier to follow.

This also catches an edge case where an account is delegating to someone else, has a balance, and also has votes because someone else is delegating to them. The votes returned from getVotes are always counted no matter what, just like in the erc20-votes strategy. The local token balance is only added when an account is delegating to different valid address.
@ChaituVR ChaituVR merged commit 7b8d7ea into snapshot-labs:master Mar 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants