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

Delegate override vote #5192

Merged
merged 31 commits into from
Oct 18, 2024
Merged

Conversation

Amxx
Copy link
Collaborator

@Amxx Amxx commented Sep 9, 2024

Replaces #5160 (was not able to push commit to arr00's repository.

To discuss

  • Contract naming
    • VotesOverridable: this is an extension of Votes that provides more features. It can be used to implement overrides, but it may also be used for other stuff. Maybe the name should not be so tightly associated with override.
    • GovernorOverrideDelegateVote: this is a counting module. IMO the name should reflect that.
      • GovernorCountingOverridable ?
  • Errors naming
    • GovernorAlreadyCastVoteOverride: I think we can do better.
  • Function naming
    • overrideVote, overrideVoteBySig: should it include "cast"
  • Events
    • events to emit when overrideVotes is executed.
    • Is it possible to differentiate between a normal vote and an override vote in the VoteCast event?
  • Extension Compatibility
    • The PreventLateQuorum extension expects that all votes call the internal _castVote function. We need to ensure these two extensions are compatible. Some options:
      1. Call _castVote from this function. This would necessitate the usage of vote params.
      2. Create a new internal function that is called every time the vote tally is updated--similar to the idea of the _update function on the ERC20 token.

PR Checklist

  • Tests
  • Documentation
  • Changeset entry (run npx changeset add)

@Amxx Amxx added this to the 5.2 milestone Sep 9, 2024
Copy link

changeset-bot bot commented Sep 9, 2024

🦋 Changeset detected

Latest commit: ca6feb3

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
openzeppelin-solidity Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@arr00 arr00 requested a review from ernestognw October 11, 2024 20:12
.changeset/pink-wasps-hammer.md Outdated Show resolved Hide resolved
@arr00 arr00 requested a review from cairoeth October 16, 2024 19:12
@Amxx Amxx merged commit 378914c into OpenZeppelin:master Oct 18, 2024
17 checks passed
@Amxx Amxx deleted the feat/delegate-override-vote branch October 18, 2024 12:17
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