Remove GovernorCompatibilyBravo and add simpler GovernorStorage

[Amxx]

As discussed.

Fixes LIB-946
Fixes LIB-944

On of the challenge is that in order to provide the queue(uint256) endpoint, I needed to hook into the queue(address[], uint256[], bytes[], bytes32) function, which is defined in IGovernorTimelock and not in IGovernor.

I did not move the propotype declaration, because that would mess up the interfaceId, and I just implemented a base version of the function (that just reverts for now) in Governor. This requires some new overrides. Moving the interface declaration would possibly reduce the need for override, but it would also either break the interfaceIds or require a lot of "xor tricks".

No tests yet, will do them if we agree on the solidity part.

Also

• refactor public/internal functions for queue/_queue and cancel/_cancel, following the execute/_execute pattern
  • public version computes the proposalId from the details AND check the current status of the proposal
  • internal version get the proposalId as a parameter
    • internal no longer recompute / return the proposalId
• new _validateStateBitmap function to avoid code duplication when checking the current state.

To discuss

Need for overrides would be improved if we moved the public declaration of the queue function from IGovernorTimelock to IGovernor. This was not (yet) implemented because of the ERC-165 consequences. However, this comment makes me think that the ERC165 will likely change anyway, and that we are ok with that. So maybe we should also do that change.

PR Checklist

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

[changeset-bot]

🦋 Changeset detected

Latest commit: 1853702

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	Major

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

[ernestognw]

We still need to remove the bravo module mentions from Governor, IGovernor, GovernorCountingSimple, and governance.adoc

[Amxx]

I did an overall refactor of the public and internal functions

Types of functions

• public
  • compute proposalId
  • check restrictions
  • call internal
• internal
  • check state is consistent
  • write flags
  • setup before operation is done
  • calls "internalCalls"
  • cleanup
  • emit event
• "internalCalls"
  • does the external call

Rational

• you can override the public one if you want to add restrictions for the public invocations.
  • In that case you call super
• you can override the internal one if you want to insert extra logic (like storing more stuff in _propose)
  • In that case you call super
• you can override the internalCalls one to reconfigure the logic (execute trough a timelock).
  • In that case you do not call super

Also:

• you can call the internal version directly if you want to bypass the public checks (for example do a proposal without the threshold check). this will check state consistency, set flags, and emit events. We used to have that for _cancel but not the other functions
• you should never call the "internalCalls" directly (can't enforce that)

WDYT @frangio @ernestognw