addressing review

contracts/access/README.adoc

@@ -27,34 +27,26 @@ Contract modules for authorization and access control mechanisms.
 ** *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.
 
-=== Operation structure
+==== 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:
+Both operations contain:
 
-* 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:
+* *Target*, the address of the smart contract that the timelock should operate on.
+* *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.
+* *Data*, 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.
+* *Predecessor*, that specifies a dependency between operations. This dependency is optional. Use `bytes32(0)` if the operation does not have any dependency.
+* *Salt*, used to disambiguate two otherwise identical operations. This can be any random value.
 
-==== Batched operations contain:
+In the case of batched operations, `target`, `value` and `data` are specified as arrays, which must be of the same length.
 
-* 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
+==== Operation lifecycle
 
 Timelocked operations are identified by a unique id (their hash) and follow a specific lifecycle:
 
@@ -71,23 +63,23 @@ Operations status can be queried using the functions:
 * xref:api:access.adoc#TimelockController-isOperationReady-bytes32-[`isOperationReady(bytes32)`]
 * xref:api:access.adoc#TimelockController-isOperationDone-bytes32-[`isOperationDone(bytes32)`]
 
-=== Roles
+==== Roles
 
-==== Admin
+===== 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 can renounce this role such that all further maintenance operations have to go through the timelock process.
 
-This role is identified by the *ADMIN_ROLE* value: `0xa49807205ce4d355092ef5a8a18f56e8913cf4a201fbe287825b095693c21775`
+This role is identified by the *TIMELOCK_ADMIN_ROLE* value: `0x5f58e3a2316349923ce3780f8d587db2d72378aed66a8261c916544fa6846ca5`
 
-==== Proposer
+===== 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
+===== 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.
 

contracts/access/TimelockController.sol

@@ -21,7 +21,7 @@ import "./AccessControl.sol";
  */
 contract TimelockController is AccessControl {
 
-    bytes32 public constant ADMIN_ROLE = keccak256("ADMIN_ROLE");
+    bytes32 public constant TIMELOCK_ADMIN_ROLE = keccak256("TIMELOCK_ADMIN_ROLE");
     bytes32 public constant PROPOSER_ROLE = keccak256("PROPOSER_ROLE");
     bytes32 public constant EXECUTOR_ROLE = keccak256("EXECUTOR_ROLE");
     uint256 internal constant _DONE_TIMESTAMP = uint256(1);
@@ -53,13 +53,13 @@ contract TimelockController is AccessControl {
      * @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);
+        _setRoleAdmin(TIMELOCK_ADMIN_ROLE, TIMELOCK_ADMIN_ROLE);
+        _setRoleAdmin(PROPOSER_ROLE, TIMELOCK_ADMIN_ROLE);
+        _setRoleAdmin(EXECUTOR_ROLE, TIMELOCK_ADMIN_ROLE);
 
         // deployer + self administration
-        _setupRole(ADMIN_ROLE, _msgSender());
-        _setupRole(ADMIN_ROLE, address(this));
+        _setupRole(TIMELOCK_ADMIN_ROLE, _msgSender());
+        _setupRole(TIMELOCK_ADMIN_ROLE, address(this));
 
         // register proposers
         for (uint256 i = 0; i < proposers.length; ++i) {

docs/modules/ROOT/pages/access-control.adoc

@@ -204,9 +204,11 @@ Roles are managed using the xref:api:access.adoc#AccessControl[`AccessControl`]
 
 There is an additional feature built on top of `AccessControl`: giving the proposer or executor role to `address(0)` opens access to anyone. This feature, while potentially useful for testing and in some cases for the executor role, is dangerous and should be used with caution.
 
-At this point, with both a proposer and an executor assigned, the timelock can perform operations. The next step is for the administrator to renounce its administrative privileges and leave the proposers and executors in charge of the timelock.
+At this point, with both a proposer and an executor assigned, the timelock can perform operations.
 
-WARNING: With administrative rights renounced in favour of timelock itself, assigning new proposers or executors requires a timelocked operation. This means that if the accounts in charge of any of these two roles become unavailable, then the entire contract gets (and any contract it controls) becomes locked indefinitely.
+An optional next step is for the deployer to renounce its administrative privileges and leave the timelock self-administered. If the deployer decides to do so, all further maintenance, including assigning new proposers/schedulers or changing the timelock duration will have to follow the timelock workflow. This links the governance of the timelock to the governance of contracts attached to the timelock, and enforce a delay on timelock maintenance operations.
+
+WARNING: If the deployer renounces administrative rights in favour of timelock itself, assigning new proposers or executors will require a timelocked operation. This means that if the accounts in charge of any of these two roles become unavailable, then the entire contract (and any contract it controls) becomes locked indefinitely.
 
 With both the proposer and executor roles assigned and the timelock in charge of its own administration, you can now transfer the ownership/control of any contract to the timelock.