diff --git a/src/accounts/ERC7821.sol b/src/accounts/ERC7821.sol index a18a63459b..3ecd68e876 100644 --- a/src/accounts/ERC7821.sol +++ b/src/accounts/ERC7821.sol @@ -107,6 +107,7 @@ contract ERC7821 is Receiver { /// @dev Executes the `calls` and returns the results. /// Reverts and bubbles up error if any call fails. + /// The `mode` and `executionData` are passed along in case there's a need to use them. function _execute( bytes32 mode, bytes calldata executionData, @@ -116,16 +117,6 @@ contract ERC7821 is Receiver { // Silence compiler warning on unused variables. mode = mode; executionData = executionData; - return _execute(calls, opData); - } - - /// @dev Executes the `calls` and returns the results. - /// Reverts and bubbles up error if any call fails. - function _execute(Call[] calldata calls, bytes calldata opData) - internal - virtual - returns (bytes[] memory) - { // Very basic auth to only allow this contract to be called by itself. // Override this function to perform more complex auth with `opData`. if (opData.length == uint256(0)) { diff --git a/src/accounts/Timelock.sol b/src/accounts/Timelock.sol index 7e81cae2d3..8ced0440fe 100644 --- a/src/accounts/Timelock.sol +++ b/src/accounts/Timelock.sol @@ -93,9 +93,9 @@ contract Timelock is ERC7821, EnumerableRoles { /// @dev The minimum delay has been set to `newMinDelay`. event MinDelaySet(uint256 newMinDelay); - /// @dev `keccak256(bytes("Proposed(bytes32.bytes.uint256)"))`. + /// @dev `keccak256(bytes("Proposed(bytes32,bytes,uint256)"))`. uint256 private constant _PROPOSED_EVENT_SIGNATURE = - 0x669fc92dce88fa911cd7ae8854344e95be1b397d6f4fde0369c400fb11e18c91; + 0xbebd4e757b91b0f4e9671796f940352b8ce72b4878d18387feb19859ae5c9e25; /// @dev `keccak256(bytes("Executed(bytes32,bytes)"))`. uint256 private constant _EXECUTED_EVENT_SIGNATURE = @@ -253,18 +253,18 @@ contract Timelock is ERC7821, EnumerableRoles { internal virtual { - uint256 i; + uint256 j; uint256 n = addresses.length << 5; if (n != uint256(0)) { do { address a; /// @solidity memory-safe-assembly assembly { - a := calldataload(add(addresses.offset, shl(5, i))) - i := add(i, 1) + a := calldataload(add(addresses.offset, j)) + j := add(j, 0x20) } _setRole(a, role, active); - } while (i != n); + } while (j != n); } } @@ -297,7 +297,7 @@ contract Timelock is ERC7821, EnumerableRoles { bytes calldata executionData, Call[] calldata calls, bytes calldata opData - ) internal virtual override(ERC7821) returns (bytes[] memory results) { + ) internal virtual override(ERC7821) returns (bytes[] memory) { /// @solidity memory-safe-assembly assembly { let m := mload(0x40) @@ -329,7 +329,7 @@ contract Timelock is ERC7821, EnumerableRoles { } } } - return _execute(calls, opData); + return _execute(calls, bytes32(0)); } /// @dev This guards the public `setRole` function, diff --git a/test/Timelock.t.sol b/test/Timelock.t.sol index caa400cd4c..207d251d62 100644 --- a/test/Timelock.t.sol +++ b/test/Timelock.t.sol @@ -5,6 +5,52 @@ import "./utils/SoladyTest.sol"; import {Timelock} from "../src/accounts/Timelock.sol"; contract TimelockTest is SoladyTest { + struct Call { + address target; + int256 value; + bytes data; + } + + event Proposed(bytes32 indexed id, bytes executionData, uint256 readyTimestamp); + event Executed(bytes32 indexed id, bytes executionData); + event Cancelled(bytes32 indexed id); + event MinDelaySet(uint256 newMinDelay); + + Timelock timelock; + + uint256 internal constant _DEFAULT_MIN_DELAY = 1000; + address internal constant _ALICE = address(111); + bytes32 internal constant _SUPPORTED_MODE = bytes10(0x01000000000078210001); + + function setUp() public { + timelock = new Timelock(); + } + + function _initializeTimelock() internal { + address[] memory a = new address[](2); + a[0] = address(this); + a[1] = _ALICE; + timelock.initialize(_DEFAULT_MIN_DELAY, a, a, a); + } + + function testSetAndGetMinDelay(uint256 newMinDelay) public { + newMinDelay = _bound(newMinDelay, 0, 2 ** 243 - 1); + Call[] memory calls = new Call[](1); + calls[0].target = address(timelock); + calls[0].data = abi.encodeWithSignature("setMinDelay(uint256)", newMinDelay); + _initializeTimelock(); + bytes memory executionData = abi.encode(calls); + vm.expectEmit(true, true, true, true); + emit Proposed(keccak256(executionData), executionData, block.timestamp + _DEFAULT_MIN_DELAY); + bytes32 id = timelock.propose(executionData, _DEFAULT_MIN_DELAY); + vm.warp(block.timestamp + _DEFAULT_MIN_DELAY); + vm.expectEmit(true, true, true, true); + emit Executed(id, executionData); + vm.expectEmit(true, true, true, true); + emit MinDelaySet(newMinDelay); + timelock.execute(_SUPPORTED_MODE, executionData); + } + function testOperationStateDifferentialTrick(uint256 packed, uint256 blockTimestamp) public pure diff --git a/test/utils/mocks/MockERC7821.sol b/test/utils/mocks/MockERC7821.sol index e477fbf807..4d85fc1711 100644 --- a/test/utils/mocks/MockERC7821.sol +++ b/test/utils/mocks/MockERC7821.sol @@ -9,7 +9,7 @@ import {Brutalizer} from "../Brutalizer.sol"; contract MockERC7821 is ERC7821, Brutalizer { bytes public lastOpData; - function _execute(Call[] calldata calls, bytes calldata opData) + function _execute(bytes32, bytes calldata, Call[] calldata calls, bytes calldata opData) internal virtual override