Add CANCEL_ROLE to TimelockController

[cygnusv]

🧐 Motivation
In the forthcoming Threshold Network DAO, there's a council with veto power over proposals. The only way to allow this with current implementation of TimelockController would be to grant the council the PROPOSER_ROLE, but this council shouldn't be able to make proposals, according to our governance design. We need something like a CANCEL_ROLE that's different from PROPOSER_ROLE and EXECUTION_ROLE. This feature could also be a solution for the "Proposer fight" problem that's discussed in the documentation.

Discussion
I wanted to do this by creating a contract that inherits from TimelockController, but since its variables and functions are marked as private it's being difficult. In particular, we need to further restrict the cancel operation to a different role than proposers, but the modifier in cancel() forces us to override it and re-implement it, and we encounter the problem of _timestamps being private.

Anyway, if you find this interesting, I'd love to help, since I'm working on that anyway.

[Amxx]

Hello @cygnusv

This is definitely an interesting discussion, but changes like that will take time to make. In the meantime, I believe you could fix that by having an Ownable or AccessControl contract, that would be a proposer for the timelock, and which only function (on top of the Ownable or AccessControl) would be a cancel forwarder.

That way your council could cancel through this relayer, but not propose.

[frangio]

We can add a _cancel function without access control check so you can define the function with a new role.

In the meantime, there is a somewhat hacky workaround you can implement as an override:

function cancel(bytes32 id) public virtual override onlyRole(CANCEL_ROLE) {
    bool isProposer = hasRole(PROPOSER_ROLE, msg.sender);
    if (!isProposer) _grantRole(PROPOSER_ROLE, msg.sender);
    super.cancel(id);
    if (!isProposer) _revokeRole(PROPOSER_ROLE, msg.sender);
}

(Note: This requires the latest version of OZ Contracts for the internal grant and revoke functions.)

[cygnusv]

Thanks @frangio and @Amxx !

After playing around with the idea of a cancel proxy I realized that we actually want the cancel functionality to happen at the Governor level, and since it's already a proposer in the timelock, it can cancel. We then just add an access control check in Governor.cancel()

[frangio]

@cygnusv We've been thinking about adding a cancel role, how did you solve this in your governor? Who has the ability to cancel?

[cygnusv]

@frangio We decided to add a cancel role in the Governor contract and implement cancel() there. Since all proposers for the Timelock can cancel, and in our setting only the Governor contract is a proposer, anyone with the cancel role can cancel in the timelock via the Governor contract.
See threshold-network/solidity-contracts@9002bdb#diff-9762b5e3ad775b50a9ae4df345a244e7a6d73f45103be32b229cc8ae2b032686

[frangio]

I see. And in practice who has veto power?

[cygnusv]

In the Threshold DAO, it was decided that a council multisig has veto power in case there's a dangerous proposal (see https://forum.threshold.network/t/threshold-network-dao-proposal-v2/57)