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
3

Hey @ernestognw I'm checking out the code here and I have read all the contracts related to governor,

can't find the implementation of clock and CLOCK_MODE and it still answers back the default answer, how ?

 /**
     * @inheritdoc IERC6372
     */
    function clock() public view virtual returns (uint48);

    /**
     * @inheritdoc IERC6372
     */
    // solhint-disable-next-line func-name-mixedcase
    function CLOCK_MODE() public view virtual returns (string memory);

how can it sill answer the default values if I have no implementation while importing the code from the libraries ?

also, if I want clock and CLOCK_MODE do I have to override both in governor and token ?

4

Also,

image
image2312×1714 323 KB

aren't we missing the FROM ? in the example you shared above ?

These are the tests I created, I wish I could easily find where the functions which returns those answers ( both test are ok )

image
image1200×582 73.7 KB

I've only found in OpenZeppelin repos these implementation in VOTEs.sol in utils, but that is only imported in token, then how come the governor can access the same implementation in it's interface.

5

I found it on GovernorVotes so when implementing it we have to override on both ( token and Governor ) right ?