Use a customizable _execute function in TimelockController

CHANGELOG.md

@@ -22,6 +22,7 @@
  * `Governor`: Implement `IERC721Receiver` and `IERC1155Receiver` to improve token custody by governors. ([#3230](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3230))
  * `TimelockController`: Implement `IERC721Receiver` and `IERC1155Receiver` to improve token custody by timelocks. ([#3230](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3230))
  * `TimelockController`: Add a separate canceller role for the ability to cancel. ([#3165](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3165))
+ * `TimelockController`: Migrate `_call` to `_execute` and allow inheritance similar to `Governor`. ([#3317](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3317))
  * `Initializable`: add a reinitializer modifier that enables the initialization of new modules, added to already initialized contracts through upgradeability. ([#3232](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3232))
  * `Initializable`: add an Initialized event that tracks initialized version numbers. ([#3294](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3294))
 

contracts/governance/TimelockController.sol

@@ -6,6 +6,7 @@ pragma solidity ^0.8.0;
 import "../access/AccessControl.sol";
 import "../token/ERC721/IERC721Receiver.sol";
 import "../token/ERC1155/IERC1155Receiver.sol";
+import "../utils/Address.sol";
 
 /**
  * @dev Contract module which acts as a timelocked controller. When set as the
@@ -288,13 +289,15 @@ contract TimelockController is AccessControl, IERC721Receiver, IERC1155Receiver
     function execute(
         address target,
         uint256 value,
-        bytes calldata data,
+        bytes calldata payload,
         bytes32 predecessor,
         bytes32 salt
     ) public payable virtual onlyRoleOrOpenRole(EXECUTOR_ROLE) {
-        bytes32 id = hashOperation(target, value, data, predecessor, salt);
+        bytes32 id = hashOperation(target, value, payload, predecessor, salt);
+
         _beforeCall(id, predecessor);
-        _call(id, 0, target, value, data);
+        _execute(target, value, payload);
+        emit CallExecuted(id, 0, target, value, payload);
         _afterCall(id);
     }
 
@@ -318,13 +321,30 @@ contract TimelockController is AccessControl, IERC721Receiver, IERC1155Receiver
         require(targets.length == payloads.length, "TimelockController: length mismatch");
 
         bytes32 id = hashOperationBatch(targets, values, payloads, predecessor, salt);
+
         _beforeCall(id, predecessor);
         for (uint256 i = 0; i < targets.length; ++i) {
-            _call(id, i, targets[i], values[i], payloads[i]);
+            address target = targets[i];
+            uint256 value = values[i];
+            bytes calldata payload = payloads[i];
+            _execute(target, value, payload);
+            emit CallExecuted(id, i, target, value, payload);
         }
         _afterCall(id);
     }
 
+    /**
+     * @dev Execute an operation's call.
+     */
+    function _execute(
+        address target,
+        uint256 value,
+        bytes calldata data
+    ) internal virtual {
+        (bool success, bytes memory returndata) = target.call{value: value}(data);
+        Address.verifyCallResult(success, returndata, "TimelockController: underlying transaction reverted");
+    }
+
     /**
      * @dev Checks before execution of an operation's calls.
      */
@@ -341,24 +361,6 @@ contract TimelockController is AccessControl, IERC721Receiver, IERC1155Receiver
         _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 {
-        (bool success, ) = target.call{value: value}(data);
-        require(success, "TimelockController: underlying transaction reverted");
-
-        emit CallExecuted(id, index, target, value, data);
-    }
-
     /**
      * @dev Changes the minimum timelock duration for future operations.
      *

test/governance/TimelockController.test.js

@@ -634,7 +634,7 @@ contract('TimelockController', function (accounts) {
             ],
             [
               this.callreceivermock.contract.methods.mockFunction().encodeABI(),
-              this.callreceivermock.contract.methods.mockFunctionThrows().encodeABI(),
+              this.callreceivermock.contract.methods.mockFunctionRevertsNoReason().encodeABI(),
               this.callreceivermock.contract.methods.mockFunction().encodeABI(),
             ],
             ZERO_BYTES32,
@@ -886,6 +886,38 @@ contract('TimelockController', function (accounts) {
       );
     });
 
+    it('call reverting with message', async function () {
+      const operation = genOperation(
+        this.callreceivermock.address,
+        0,
+        this.callreceivermock.contract.methods.mockFunctionRevertsReason().encodeABI(),
+        ZERO_BYTES32,
+        '0xb1b1b276fdf1a28d1e00537ea73b04d56639128b08063c1a2f70a52e38cba693',
+      );
+
+      await this.mock.schedule(
+        operation.target,
+        operation.value,
+        operation.data,
+        operation.predecessor,
+        operation.salt,
+        MINDELAY,
+        { from: proposer },
+      );
+      await time.increase(MINDELAY);
+      await expectRevert(
+        this.mock.execute(
+          operation.target,
+          operation.value,
+          operation.data,
+          operation.predecessor,
+          operation.salt,
+          { from: executor },
+        ),
+        'CallReceiverMock: reverting',
+      );
+    });
+
     it('call throw', async function () {
       const operation = genOperation(
         this.callreceivermock.address,
@@ -905,7 +937,7 @@ contract('TimelockController', function (accounts) {
         { from: proposer },
       );
       await time.increase(MINDELAY);
-      await expectRevert(
+      await expectRevert.unspecified(
         this.mock.execute(
           operation.target,
           operation.value,
@@ -914,7 +946,6 @@ contract('TimelockController', function (accounts) {
           operation.salt,
           { from: executor },
         ),
-        'TimelockController: underlying transaction reverted',
       );
     });
 
@@ -937,7 +968,7 @@ contract('TimelockController', function (accounts) {
         { from: proposer },
       );
       await time.increase(MINDELAY);
-      await expectRevert(
+      await expectRevert.outOfGas(
         this.mock.execute(
           operation.target,
           operation.value,
@@ -946,7 +977,6 @@ contract('TimelockController', function (accounts) {
           operation.salt,
           { from: executor, gas: '70000' },
         ),
-        'TimelockController: underlying transaction reverted',
       );
     });