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

Consider removing ERC20VotesComp #3833

Closed
frangio opened this issue Nov 25, 2022 · 5 comments
Closed

Consider removing ERC20VotesComp #3833

frangio opened this issue Nov 25, 2022 · 5 comments
Labels
breaking change Changes that break backwards compatibility of the public API.
Milestone

Comments

@frangio
Copy link
Contributor

frangio commented Nov 25, 2022

We released ERC20VotesComp for users who needed compatibility with an existing GovernorAlpha/Bravo instance. For 5.0, I think we can remove this contract. Anyone who still needs this compatibility can remain on 4.x, but this seems in fact very rare: I can find 1.6k deployments of ERC20Votes but only 80 deployments of ERC20VotesComp. We should remove it from the "Compatibility" section in the Governance guide.

@frangio frangio added the breaking change Changes that break backwards compatibility of the public API. label Nov 25, 2022
@frangio frangio added this to the 5.0 milestone Nov 25, 2022
@frangio
Copy link
Contributor Author

frangio commented Dec 21, 2022

We should consider removing GovernorCompatibilityBravo and GovernorTimelockCompound as well.

@FriskyHamTitz
Copy link

Im using bravo compatibility but only that.

Instead of removing it maybe just move it to a folder called deprecated. It would be frustrating to have to be tied to separate versions of the packages just to implement contracts.

The existing contract is missing a few things which is maybe why it's use is limited because its not fully compatible with bravo. It maybe a good idea to have a fully compatible version, but if you don't feel like supporting maybe just put it in the deprecated folder like above

Things I noticed that were missing:

  1. It's missing a few properties. Proposal count, a few others
  2. There's no way to enumerate the hashes

Regardless it would be nice if all oz governance supported a way to obtain previous proposal without using the graph or enumerating blocks on the rpc.

@Amxx
Copy link
Collaborator

Amxx commented Jun 8, 2023

Done in #4276

@Amxx Amxx closed this as completed Jun 8, 2023
@frangio
Copy link
Contributor Author

frangio commented Jun 9, 2023

@FriskyHamTitz Can you please elaborate a bit more on your use case. Are you interested in the actual Bravo compatibility or are you only interested in how it stores the proposal details in storage?

@FriskyHamTitz
Copy link

FriskyHamTitz commented Jun 10, 2023 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Changes that break backwards compatibility of the public API.
Projects
None yet
Development

No branches or pull requests

3 participants