Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add TimelockController #2354

Merged
merged 74 commits into from
Sep 17, 2020
Merged
Show file tree
Hide file tree
Changes from 54 commits
Commits
Show all changes
74 commits
Select commit Hold shift + click to select a range
f3e90fb
timelock
Amxx Aug 28, 2020
8f41c6d
new commit/reveal version of timelock
Amxx Aug 28, 2020
8eb329c
reverting on failled operation + minDelay
Amxx Aug 29, 2020
be3a111
timelock: testing double commitment
Amxx Aug 29, 2020
3c34cb7
timelock using access control
Amxx Aug 30, 2020
760c23c
minor simplification
Amxx Aug 30, 2020
818a2fe
Timelock access control refactor
Amxx Aug 30, 2020
0e3c228
lint
Amxx Aug 30, 2020
dbcadac
timelock documentation
Amxx Aug 30, 2020
639cde8
doc
Amxx Aug 31, 2020
5c438d3
timelock: open role to everyone with address(0)
Amxx Sep 1, 2020
05bc209
split timelock logic
Amxx Sep 1, 2020
032cc7b
timelock doc
Amxx Sep 1, 2020
8d01d66
timelock: naming
Amxx Sep 1, 2020
9f39d26
timelock controller docs
Amxx Sep 1, 2020
7da0098
timelock with public schedule
Amxx Sep 1, 2020
7176a2e
timelock testing
Amxx Sep 1, 2020
3cfad30
Merge branch 'master' into timelock-public
Amxx Sep 2, 2020
e4f11de
timelock doc
Amxx Sep 2, 2020
1c05edc
adding predecessor
Amxx Sep 2, 2020
948e510
timelock testing
Amxx Sep 3, 2020
6320cd7
timelock testing lint
Amxx Sep 3, 2020
f631e80
timelock gas optimisation
Amxx Sep 3, 2020
fe9a85f
timelock gas tweek
Amxx Sep 3, 2020
a352260
moving timelock testing to test/utils
Amxx Sep 3, 2020
c42c05b
all-in-one timelock
Amxx Sep 4, 2020
14ceb84
minor testing update
Amxx Sep 4, 2020
bd3d36c
minor testing update
Amxx Sep 4, 2020
610bd29
linting
Amxx Sep 4, 2020
32cdcf7
linting
Amxx Sep 4, 2020
f8b5aa0
linting
Amxx Sep 4, 2020
6d86153
minor dependency fix
Amxx Sep 4, 2020
1fdeba8
TimelockController refactor & cleanup
Amxx Sep 4, 2020
79eaa94
hardcoded values in tests
Amxx Sep 5, 2020
74ddc18
timelock docs and minor refactor
Amxx Sep 5, 2020
94fe13f
adding constructor event + remove payable from schedule
Amxx Sep 7, 2020
900ed50
doc
Amxx Sep 7, 2020
ae4bcde
tests fix
Amxx Sep 7, 2020
f01745b
typo + guide start
Amxx Sep 7, 2020
ff458da
timelock guide
Amxx Sep 7, 2020
3d88c92
timelock doc
Amxx Sep 9, 2020
3b4f074
Merge branch 'master' into timelock-public
Amxx Sep 9, 2020
bb62b65
TimelockController doc typo
Amxx Sep 9, 2020
8bc0ae3
documentation refactor
Amxx Sep 10, 2020
2c28b82
documentation refactor
Amxx Sep 10, 2020
bfa2fa2
timelock: addressing review comments
Amxx Sep 11, 2020
06eeb19
addressing more review comments
Amxx Sep 12, 2020
fae6d5a
doc update
Amxx Sep 12, 2020
79dfd00
Solving reentrancy issue introduced in 06eeb1
Amxx Sep 13, 2020
0815268
compile fix
Amxx Sep 13, 2020
96d700a
Merge branch 'master' into timelock-public
Amxx Sep 13, 2020
8da3b96
execution checks refactor
Amxx Sep 13, 2020
4adc4e4
cleanup
Amxx Sep 13, 2020
eb1599f
doc typos
Amxx Sep 13, 2020
9954a52
Update test/access/TimelockController.test.js
Amxx Sep 15, 2020
b962f01
Update test/access/TimelockController.test.js
Amxx Sep 15, 2020
287310b
Update test/access/TimelockController.test.js
Amxx Sep 15, 2020
be14be9
Update contracts/access/README.adoc
Amxx Sep 15, 2020
b1de782
Update contracts/access/README.adoc
Amxx Sep 15, 2020
84285d8
Update contracts/access/README.adoc
Amxx Sep 15, 2020
106152c
Update contracts/access/README.adoc
Amxx Sep 15, 2020
96e0122
removing redundant events checks
Amxx Sep 15, 2020
cd23b05
fix lint
Amxx Sep 15, 2020
38e8a1d
Merge branch 'master' into timelock-public
Amxx Sep 15, 2020
79b07f9
Update test/access/TimelockController.test.js
Amxx Sep 15, 2020
52042e0
Apply suggestions from code review
Amxx Sep 16, 2020
cd0b97c
addressing review
Amxx Sep 16, 2020
fed7a3a
fix tests
Amxx Sep 16, 2020
20a6299
Merge branch 'master' into timelock-public
Amxx Sep 16, 2020
a798e76
lint fix
Amxx Sep 16, 2020
45476a1
skip OutOfGas test when running coverage
Amxx Sep 16, 2020
13ee9cb
re-fixing lint (don't use vim in the future)
Amxx Sep 16, 2020
60fc3bf
Addressing comments about TimelockController.test.js structure
Amxx Sep 17, 2020
64500e4
fix typo and increase test timeout
frangio Sep 17, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 88 additions & 2 deletions contracts/access/README.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,94 @@ NOTE: This document is better viewed at https://docs.openzeppelin.com/contracts/

Contract modules for authorization and access control mechanisms.

== Contracts
== Authorization

{{Ownable}}

{{AccessControl}}

== Timelock

{{TimelockController}}

=== Terminology
Amxx marked this conversation as resolved.
Show resolved Hide resolved

* *Operation:* A transaction (or a set of transactions) that is the subject of the timelock. It has to be scheduled by a proposer and executed by an executor. The timelock enforces a minimum delay between the proposition and the execution (see xref:access-control.adoc#operation_lifecycle[operation lifecycle]). If the operation contains multiple transaction (batch mode), they are executed atomically. Operations are identified by the hash of their content.
Amxx marked this conversation as resolved.
Show resolved Hide resolved
* *Operation status:*
** *Unset:* An operation that is not part of the timelock mechanism.
** *Pending:* An operation that has been scheduled, before the timer expires.
** *Ready:* An operation that has been scheduled, after the timer expires.
** *Done:* An operation that has been executed.
* *Predecessor*: An (optional) dependency between operations. An operation can depend on another operation (its predecessor), forcing the execution order of these two operations.
* *Role*:
** *Proposer:* An address (smart contract or EOA) that is in charge of scheduling (and cancelling) operations.
** *executor:* An address (smart contract or EOA) that is in charge of executing operations.
Amxx marked this conversation as resolved.
Show resolved Hide resolved

=== Operation structure

Operation executed by the xref:api:access.adoc#TimelockController[`TimelockControler`] can contain one or multiple subsequent calls. Depending on whether you need to multiple calls to be executed atomically, you can either use simple or batched operations.

==== Simple operations contain:
frangio marked this conversation as resolved.
Show resolved Hide resolved

* A *target*, the address of the smart contract that the timelock should operate on.
* A *value*, in wei, that should be sent with the transaction. Most of the time this will be 0. Ether can be deposited before-end or passed along when executing the transaction.
* A *data* field, containing the encoded function selector and parameters of the call. This can be produced using a number of tools. For example, a maintenance operation granting role `ROLE` to `ACCOUNT` can be encode using web3js as follows:

```javascript
const data = timelock.contract.methods.grantRole(ROLE, ACCOUNT).encodeABI()
```

* A *predecessor*, that specifies a dependency between operations. This dependency is optional. Use `bytes32(0)` if the operation does not have any dependency.
* A *salt*, used to disambiguate two otherwise identical operations. This can be any random value.

==== Batched operations contain:

* An array (*targets*) of targets.
* An array (*values*) of values.
* An array (*datas*) of data fields.
* A *predecessor*, that specifies a dependency between operations. This dependency is optional. Use `bytes32(0)` if the operation does not have any dependency.
* A *salt*, used to disambiguate two otherwise identical operations. This can be any random value.

TIP: The three arrays must have the same length. They contain the details of the calls, just like in a simple operation.

=== Operation lifecycle

Timelocked operations are identified by a unique id (their hash) and follow a specific lifecycle:

`Unset` -> `Pending` -> `Pending` + `Ready` -> `Done`

* By calling xref:api:access.adoc#TimelockController-schedule-address-uint256-bytes-bytes32-bytes32-uint256-[`schedule`] (or xref:api:access.adoc#TimelockController-scheduleBatch-address---uint256---bytes---bytes32-bytes32-uint256-[`scheduleBatch`]), a proposer moves the operation from the `Unset` to the `Pending` state. This starts a timer that must be longer than the minimum delay. The timer expires at a timestamp accessible through the xref:api:access.adoc#TimelockController-getTimestamp-bytes32-[`getTimestamp`] methods.
Amxx marked this conversation as resolved.
Show resolved Hide resolved
* Once the timer expires, the operation gets the `Ready` state. At this point, it can be executed.
Amxx marked this conversation as resolved.
Show resolved Hide resolved
* By calling xref:api:access.adoc#TimelockController-TimelockController-execute-address-uint256-bytes-bytes32-bytes32-[`execute`] (or xref:api:access.adoc#TimelockController-executeBatch-address---uint256---bytes---bytes32-bytes32-[`executeBatch`]), an executor triggers the operation's underlying transactions and moves it to the `Done` state. If the operation has a predecessor, it has to be in the `Done` state for this transition to succeed.
* xref:api:access.adoc#TimelockController-TimelockController-cancel-bytes32-[`cancel`] allows proposers to cancel any `Pending` operation. This resets the operation to the `Unset` state. It is thus possible for a proposer to re-schedule an operation that has been cancelled. In this case, the timer restarts when the operation is re-scheduled.

Operations status can be queried using the functions:

* xref:api:access.adoc#TimelockController-isOperationPending-bytes32-[`isOperationPending(bytes32)`]
* xref:api:access.adoc#TimelockController-isOperationReady-bytes32-[`isOperationReady(bytes32)`]
* xref:api:access.adoc#TimelockController-isOperationDone-bytes32-[`isOperationDone(bytes32)`]

=== Roles

==== Admin

The admins are in charge of managing proposers and executors. For the timelock to be self-governed, this role should only be given to the timelock itself. Upon deployment, both the timelock and the deployer have this role. After further configuration and testing, the deployer should renounce this role such that all further maintenance operations have to go through the timelock process.
Amxx marked this conversation as resolved.
Show resolved Hide resolved

This role is identified by the *ADMIN_ROLE* value: `0xa49807205ce4d355092ef5a8a18f56e8913cf4a201fbe287825b095693c21775`

==== Proposer

The proposers are in charge of scheduling (and cancelling) operations. This is a critical role, that should be given to governing entities. This could be an EOA, a multisig, or a DAO.

WARNING: *Proposer fight:* Having multiple proposers, while providing redundancy in case one becomes unavailable, can be dangerous. As proposer have their say on all operations, they could cancel operations they disagree with, including operations to remove them for the proposers.

This role is identified by the *PROPOSER_ROLE* value: `0xb09aa5aeb3702cfd50b6b62bc4532604938f21248a27a1d5ca736082b6819cc1`

==== Executor

The executors are in charge of executing the operations scheduled by the proposers once the timelock expires. Logic dictates that multisig or DAO that are proposers should also be executors in order to guarantee operations that have been scheduled will eventually be executed. However, having additional executor can reduce the cost (the executing transaction does not require validation by the multisig or DAO that proposed it), while ensuring whoever is in charge of execution cannot trigger actions that have not been scheduled by the proposers.

This role is identified by the *EXECUTOR_ROLE* value: `0xd8aa0f3194971a2a116679f7c2090f6939c8d4e01a2a8d7e41d55e5351469e63`

==== Warning
Amxx marked this conversation as resolved.
Show resolved Hide resolved

WARNING: A live contract without at least one proposer and one executor is locked. Make sure these roles are filled by reliable entities before the deployer renounces its administrative rights in favour of the timelock contract itself. See the {AccessControl} documentation to learn more about role management.
frangio marked this conversation as resolved.
Show resolved Hide resolved
281 changes: 281 additions & 0 deletions contracts/access/TimelockController.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,281 @@
// SPDX-License-Identifier: MIT

pragma solidity ^0.6.0;
pragma experimental ABIEncoderV2;

import "./../math/SafeMath.sol";
import "./AccessControl.sol";

/**
* @dev Contract module which acts as a timelocked controller. When set as the
* owner of an `Ownable` smart contract, it enforces a timelock on all
* `onlyOwner` maintenance operations. This gives time for users of the
* controlled contract to exit before a potentially dangerous maintenance
* operation is applied.
*
* By default, this contract is self administered, meaning administration tasks
* have to go through the timelock process. The proposer (resp executor) role
* is in charge of proposing (resp executing) operations. A common use case is
* to position this {TimelockController} as the owner of a smart contract, with
* a multisig or a DAO as the sole proposer.
*/
contract TimelockController is AccessControl {
Amxx marked this conversation as resolved.
Show resolved Hide resolved

bytes32 public constant ADMIN_ROLE = keccak256("ADMIN_ROLE");
frangio marked this conversation as resolved.
Show resolved Hide resolved
bytes32 public constant PROPOSER_ROLE = keccak256("PROPOSER_ROLE");
bytes32 public constant EXECUTOR_ROLE = keccak256("EXECUTOR_ROLE");
uint256 internal constant _DONE_TIMESTAMP = uint256(1);

mapping(bytes32 => uint256) private _timestamps;
uint256 private _minDelay;

/**
* @dev Emitted when a call is scheduled as part of operation `id`.
*/
event CallScheduled(bytes32 indexed id, uint256 indexed index, address target, uint256 value, bytes data, bytes32 predecessor, uint256 delay);

/**
* @dev Emitted when a call is performed as part of operation `id`.
*/
event CallExecuted(bytes32 indexed id, uint256 indexed index, address target, uint256 value, bytes data);

/**
* @dev Emitted when operation `id` is cancelled.
*/
event Cancelled(bytes32 indexed id);

/**
* @dev Emitted when the minimum delay for future operations is modified.
*/
event MinDelayChange(uint256 oldDuration, uint256 newDuration);

/**
* @dev Initializes the contract with a given `minDelay`.
*/
constructor(uint256 minDelay, address[] memory proposers, address[] memory executors) public {
_setRoleAdmin(ADMIN_ROLE, ADMIN_ROLE);
_setRoleAdmin(PROPOSER_ROLE, ADMIN_ROLE);
_setRoleAdmin(EXECUTOR_ROLE, ADMIN_ROLE);

// deployer + self administration
_setupRole(ADMIN_ROLE, _msgSender());
_setupRole(ADMIN_ROLE, address(this));

// register proposers
for (uint256 i = 0; i < proposers.length; ++i) {
_setupRole(PROPOSER_ROLE, proposers[i]);
}

// register executors
for (uint256 i = 0; i < executors.length; ++i) {
_setupRole(EXECUTOR_ROLE, executors[i]);
}

_minDelay = minDelay;
emit MinDelayChange(0, minDelay);
}

/**
* @dev Modifier to make a function callable only by a certain role. In
* addition to checking the sender's role, `address(0)` 's role is also
* considered. Granting a role to `address(0)` is equivalent to enabling
* this role for everyone.
*/
modifier onlyRole(bytes32 role) {
require(hasRole(role, _msgSender()) || hasRole(role, address(0)), "TimelockController: sender requiers permission");
frangio marked this conversation as resolved.
Show resolved Hide resolved
Amxx marked this conversation as resolved.
Show resolved Hide resolved
_;
}

/*
* @dev Contract might receive/hold ETH as part of the maintenance process.
*/
receive() external payable {}

/**
* @dev Returns whether an operation is pending or not.
*/
function isOperationPending(bytes32 id) public view returns (bool pending) {
return _timestamps[id] > _DONE_TIMESTAMP;
}

/**
* @dev Returns whether an operation is ready or not.
*/
function isOperationReady(bytes32 id) public view returns (bool ready) {
// solhint-disable-next-line not-rely-on-time
return _timestamps[id] > _DONE_TIMESTAMP && _timestamps[id] <= block.timestamp;
}

/**
* @dev Returns whether an operation is done or not.
*/
function isOperationDone(bytes32 id) public view returns (bool done) {
return _timestamps[id] == _DONE_TIMESTAMP;
}

/**
* @dev Returns the timestamp at with an operation becomes ready (0 for
* unset operations, 1 for done operations).
*/
function getTimestamp(bytes32 id) public view returns (uint256 timestamp) {
return _timestamps[id];
}

/**
* @dev Returns the minimum delay for an operation to become valid.
*/
function getMinDelay() public view returns (uint256 duration) {
return _minDelay;
}

/**
* @dev Returns the identifier of an operation containing a single
* transaction.
*/
function hashOperation(address target, uint256 value, bytes calldata data, bytes32 predecessor, bytes32 salt) public pure returns (bytes32 hash) {
return keccak256(abi.encode(target, value, data, predecessor, salt));
}

/**
* @dev Returns the identifier of an operation containing a batch of
* transactions.
*/
function hashOperationBatch(address[] calldata targets, uint256[] calldata values, bytes[] calldata datas, bytes32 predecessor, bytes32 salt) public pure returns (bytes32 hash) {
return keccak256(abi.encode(targets, values, datas, predecessor, salt));
Amxx marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* @dev Schedule an operation containing a single transaction.
*
* Emits a {CallScheduled} event.
*
* Requirements:
*
* - the caller must have the 'proposer' role.
*/
function schedule(address target, uint256 value, bytes calldata data, bytes32 predecessor, bytes32 salt, uint256 delay) public virtual onlyRole(PROPOSER_ROLE) {
bytes32 id = hashOperation(target, value, data, predecessor, salt);
_schedule(id, delay);
emit CallScheduled(id, 0, target, value, data, predecessor, delay);
}

/**
* @dev Schedule an operation containing a batch of transactions.
*
* Emits one {CallScheduled} event per transaction in the batch.
*
* Requirements:
*
* - the caller must have the 'proposer' role.
*/
function scheduleBatch(address[] calldata targets, uint256[] calldata values, bytes[] calldata datas, bytes32 predecessor, bytes32 salt, uint256 delay) public virtual onlyRole(PROPOSER_ROLE) {
require(targets.length == values.length, "TimelockController: length missmatch");
require(targets.length == datas.length, "TimelockController: length missmatch");
Amxx marked this conversation as resolved.
Show resolved Hide resolved

bytes32 id = hashOperationBatch(targets, values, datas, predecessor, salt);
_schedule(id, delay);
for (uint256 i = 0; i < targets.length; ++i) {
emit CallScheduled(id, i, targets[i], values[i], datas[i], predecessor, delay);
}
}

/**
* @dev Schedule an operation that is to becomes valid after a given delay.
*/
function _schedule(bytes32 id, uint256 delay) private {
require(_timestamps[id] == 0, "TimelockController: operation already scheduled");
require(delay >= _minDelay, "TimelockController: insufficient delay");
// solhint-disable-next-line not-rely-on-time
_timestamps[id] = SafeMath.add(block.timestamp, delay);
}

/**
* @dev Cancel an operation.
*
* Requirements:
*
* - the caller must have the 'proposer' role.
*/
function cancel(bytes32 id) public virtual onlyRole(PROPOSER_ROLE) {
require(isOperationPending(id), "TimelockController: operation cannot be cancelled");
delete _timestamps[id];

emit Cancelled(id);
}

/**
* @dev Execute an (ready) operation containing a single transaction.
*
* Emits a {CallExecuted} event.
*
* Requirements:
*
* - the caller must have the 'executor' role.
*/
function execute(address target, uint256 value, bytes calldata data, bytes32 predecessor, bytes32 salt) public payable virtual onlyRole(EXECUTOR_ROLE) {
bytes32 id = hashOperation(target, value, data, predecessor, salt);
_beforeCall(predecessor);
_call(id, 0, target, value, data);
_afterCall(id);
}

/**
* @dev Execute an (ready) operation containing a batch of transactions.
*
* Emits one {CallExecuted} event per transaction in the batch.
*
* Requirements:
*
* - the caller must have the 'executor' role.
*/
function executeBatch(address[] calldata targets, uint256[] calldata values, bytes[] calldata datas, bytes32 predecessor, bytes32 salt) public payable virtual onlyRole(EXECUTOR_ROLE) {
require(targets.length == values.length, "TimelockController: length missmatch");
require(targets.length == datas.length, "TimelockController: length missmatch");

Amxx marked this conversation as resolved.
Show resolved Hide resolved
bytes32 id = hashOperationBatch(targets, values, datas, predecessor, salt);
_beforeCall(predecessor);
for (uint256 i = 0; i < targets.length; ++i) {
_call(id, i, targets[i], values[i], datas[i]);
}
_afterCall(id);
}

/**
* @dev Checks before execution of an operation's calls.
*/
function _beforeCall(bytes32 predecessor) private view {
require(predecessor == bytes32(0) || isOperationDone(predecessor), "TimelockController: missing dependency");
}

/**
* @dev Checks after execution of an operation's calls.
*/
function _afterCall(bytes32 id) private {
require(isOperationReady(id), "TimelockController: operation is not ready");
_timestamps[id] = _DONE_TIMESTAMP;
}

/**
* @dev Execute an operation's call.
*
* Emits a {CallExecuted} event.
*/
function _call(bytes32 id, uint256 index, address target, uint256 value, bytes calldata data) private {
// solhint-disable-next-line avoid-low-level-calls
(bool success,) = target.call{value: value}(data);
require(success, "TimelockController: underlying transaction reverted");
Amxx marked this conversation as resolved.
Show resolved Hide resolved

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

/**
* @dev Changes the timelock duration for future operations.
*
* Emits a {MinDelayChange} event.
*/
function updateDelay(uint256 newDelay) external virtual {
require(msg.sender == address(this), "TimelockController: restricted maintenance access");
Amxx marked this conversation as resolved.
Show resolved Hide resolved
emit MinDelayChange(_minDelay, newDelay);
_minDelay = newDelay;
}
}
Loading