Remove GovernorCompatibilyBravo and add simpler GovernorStorage by Amxx · Pull Request #4360 · OpenZeppelin/openzeppelin-contracts (original) (raw)

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)