Use a customizable _execute function in TimelockController

[Joeysantoro]

Fixes #N/A

Set the _call method of the TimelockController to internal virtual so it can be overridden.

For context, I am looking to write a TimelockController which exclusively executes through a Gnosis Safe as a module https://github.com/gnosis/safe-contracts/blob/main/contracts/base/ModuleManager.sol#L61-L73

So the "call" would get overridden to:

    function _call(
        bytes32 id,
        uint256 index,
        address target,
        uint256 value,
        bytes calldata data
    ) internal override {
        safe.executeTransactionFromModule(target, value, data, Operation.CALL);

        emit CallExecuted(id, index, target, value, data);

PR Checklist

• Tests
• Documentation
• Changelog entry

[Amxx]

Hello @Joeysantoro

Overriding the _execute function is possible in the governor, so your change would not only make sense, but improve consistency.

The way you propose we change it is safe in your case, but I'm worried some override might forget the event. I'm going to refactor this PR a bit.

[Amxx]

If these changes follow your idea, I think we can go ahead and add a changelog before we can merge

[frangio]

This makes sense to me.

The tests are failing but I'm not sure why. @Amxx Can you check?

[Joeysantoro]

This makes sense to me.

The tests are failing but I'm not sure why. @Amxx Can you check?

Tests are failing because Addresses.verifyCallResult will bubble up errors by default which is a different behavior to always returning the TimelockController error.

Changing it back to require(success, "TimelockController: underlying transaction reverted"); would fix this but the newer implementation is slightly better so maybe its best to just fix the test

[Amxx]

I already fixed part of the tests. I'll have a second pass

[Joeysantoro]

I edited this to use the old require logic instead so tests won't be broken. @frangio assuming the tests pass I think this can merge?

The Addresses.verifyCallResult could be fixed in another PR

[frangio]

Agree to merge.