Help with Governor 5.x contracts

Support Contracts
1

Trying to set-up a Governor, using a TimelockController, with voting delay, voting period, etc.

The current contracts to inherit are:

contract HaiGovernor is
  Governor,
  GovernorSettings,
  GovernorCountingSimple,
  GovernorVotes,
  GovernorVotesQuorumFraction,
  GovernorTimelockControl {...}

On the documentation: https://docs.openzeppelin.com/contracts/5.x/governance
The proposed governor imports all these contracts, but, for example in _executeOperations internal method, the documentation overrides Governor over GovernorTimelockControl.

    function _executeOperations(
        uint256 proposalId,
        address[] memory targets,
        uint256[] memory values,
        bytes[] memory calldatas,
        bytes32 descriptionHash
    ) internal override(Governor, GovernorTimelockControl) {
        super._executeOperations(proposalId, targets, values, calldatas, descriptionHash);
    }

Shouldn't the override order be inverse if we want to keep GovernorTimelockControl behaviour? The docs doesn't explain this setup.

Is there any other consideration to take into account when inheriting these contracts? Or is the default configuration good enough to be used as is.

Also, given that this Governor is gonna be set on Optimism, what's the best way to address voting delay and periods, since the 12s per block is not ensured in this chain?

Thanks a lot!
:carrot:

1 Like
2

Hey @wei3erHase, sorry for the late response, most of the team is right now at DevConnect Istanbul.

The execution order depends on the linearization order and not on the override(...) order specified (i know, weird). So in this case, because the GovernorTimelockControl is the last contract defined in the is inheritance list, it's considered the most derived contract.

You can test it out easily in Foundry:

// src/Inheritance.sol
// SPDX-License-Identifier: MIT

pragma solidity ^0.8.20;

import {console2} from "forge-std/console2.sol";

contract Governor {
    function foo() public virtual {
        console2.log("Governor");
    }
}

contract GovernorTimelockControl {
    function foo() public virtual {
        console2.log("GovernorTimelockControl");
    }
}

contract Inheritance1 is GovernorTimelockControl, Governor {
    function foo() public override(GovernorTimelockControl, Governor) {
        super.foo();
    }
}

contract Inheritance2 is GovernorTimelockControl, Governor {
    function foo() public override(Governor, GovernorTimelockControl) {
        super.foo();
    }
}

contract Inheritance3 is Governor, GovernorTimelockControl {
    function foo() public override(GovernorTimelockControl, Governor) {
        super.foo();
    }
}

contract Inheritance4 is Governor, GovernorTimelockControl {
    function foo() public override(Governor, GovernorTimelockControl) {
        super.foo();
    }
}

test/Inheritance.t.sol
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import {Test, console2} from "forge-std/Test.sol";
import {Inheritance1, Inheritance2, Inheritance3, Inheritance4} from "../src/Inheritance.sol";

interface IInheritance {
    function foo() external;
}

contract InheritanceTest is Test {
    IInheritance public i_1;
    IInheritance public i_2;
    IInheritance public i_3;
    IInheritance public i_4;

    function setUp() public {
        i_1 = IInheritance(address(new Inheritance1()));
        i_2 = IInheritance(address(new Inheritance2()));
        i_3 = IInheritance(address(new Inheritance3()));
        i_4 = IInheritance(address(new Inheritance4()));
    }

    function test_inheritance() public {
        console2.log("=== Case 1 ===");
        i_1.foo();
        console2.log("=== Case 2 ===");
        i_2.foo();
        console2.log("=== Case 3 ===");
        i_3.foo();
        console2.log("=== Case 4 ===");
        i_4.foo();
    }
}

The result of running forge test -vv will be the following:

[PASS] test_inheritance() (gas: 28639)
Logs:
  === Case 1 ===
  Governor
  === Case 2 ===
  Governor
  === Case 3 ===
  GovernorTimelockControl
  === Case 4 ===
  GovernorTimelockControl

Shouldn't the override order be inverse if we want to keep GovernorTimelockControl behaviour? The docs doesn't explain this setup.

This is actually a good prompt for the documentation, we can consider adding further explanation about this.

Also, given that this Governor is gonna be set on Optimism, what's the best way to address voting delay and periods, since the 12s per block is not ensured in this chain?

I'll recommend checking out EIP-6372 for customizing a contract clock. It's built-in in Governor and you can override it using something like this:

function clock() public view override returns (uint48) {
    return uint48(block.timestamp);
}

// solhint-disable-next-line func-name-mixedcase
function CLOCK_MODE() public view virtual override returns (string memory) {
    return "mode=timestamp";
}

Hope it helps!

1 Like