diff --git a/contracts/SafeMath.sol b/contracts/SafeMath.sol new file mode 100644 index 0000000..d4df1f3 --- /dev/null +++ b/contracts/SafeMath.sol @@ -0,0 +1,186 @@ +pragma solidity ^0.5.16; + +// From https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/math/Math.sol +// Subject to the MIT license. + +/** + * @dev Wrappers over Solidity's arithmetic operations with added overflow + * checks. + * + * Arithmetic operations in Solidity wrap on overflow. This can easily result + * in bugs, because programmers usually assume that an overflow raises an + * error, which is the standard behavior in high level programming languages. + * `SafeMath` restores this intuition by reverting the transaction when an + * operation overflows. + * + * Using this library instead of the unchecked operations eliminates an entire + * class of bugs, so it's recommended to use it always. + */ +library SafeMath { + /** + * @dev Returns the addition of two unsigned integers, reverting on overflow. + * + * Counterpart to Solidity's `+` operator. + * + * Requirements: + * - Addition cannot overflow. + */ + function add(uint256 a, uint256 b) internal pure returns (uint256) { + uint256 c = a + b; + require(c >= a, "SafeMath: addition overflow"); + + return c; + } + + /** + * @dev Returns the addition of two unsigned integers, reverting with custom message on overflow. + * + * Counterpart to Solidity's `+` operator. + * + * Requirements: + * - Addition cannot overflow. + */ + function add(uint256 a, uint256 b, string memory errorMessage) internal pure returns (uint256) { + uint256 c = a + b; + require(c >= a, errorMessage); + + return c; + } + + /** + * @dev Returns the subtraction of two unsigned integers, reverting on underflow (when the result is negative). + * + * Counterpart to Solidity's `-` operator. + * + * Requirements: + * - Subtraction cannot underflow. + */ + function sub(uint256 a, uint256 b) internal pure returns (uint256) { + return sub(a, b, "SafeMath: subtraction underflow"); + } + + /** + * @dev Returns the subtraction of two unsigned integers, reverting with custom message on underflow (when the result is negative). + * + * Counterpart to Solidity's `-` operator. + * + * Requirements: + * - Subtraction cannot underflow. + */ + function sub(uint256 a, uint256 b, string memory errorMessage) internal pure returns (uint256) { + require(b <= a, errorMessage); + uint256 c = a - b; + + return c; + } + + /** + * @dev Returns the multiplication of two unsigned integers, reverting on overflow. + * + * Counterpart to Solidity's `*` operator. + * + * Requirements: + * - Multiplication cannot overflow. + */ + function mul(uint256 a, uint256 b) internal pure returns (uint256) { + // Gas optimization: this is cheaper than requiring 'a' not being zero, but the + // benefit is lost if 'b' is also tested. + // See: https://github.com/OpenZeppelin/openzeppelin-contracts/pull/522 + if (a == 0) { + return 0; + } + + uint256 c = a * b; + require(c / a == b, "SafeMath: multiplication overflow"); + + return c; + } + + /** + * @dev Returns the multiplication of two unsigned integers, reverting on overflow. + * + * Counterpart to Solidity's `*` operator. + * + * Requirements: + * - Multiplication cannot overflow. + */ + function mul(uint256 a, uint256 b, string memory errorMessage) internal pure returns (uint256) { + // Gas optimization: this is cheaper than requiring 'a' not being zero, but the + // benefit is lost if 'b' is also tested. + // See: https://github.com/OpenZeppelin/openzeppelin-contracts/pull/522 + if (a == 0) { + return 0; + } + + uint256 c = a * b; + require(c / a == b, errorMessage); + + return c; + } + + /** + * @dev Returns the integer division of two unsigned integers. + * Reverts on division by zero. The result is rounded towards zero. + * + * Counterpart to Solidity's `/` operator. Note: this function uses a + * `revert` opcode (which leaves remaining gas untouched) while Solidity + * uses an invalid opcode to revert (consuming all remaining gas). + * + * Requirements: + * - The divisor cannot be zero. + */ + function div(uint256 a, uint256 b) internal pure returns (uint256) { + return div(a, b, "SafeMath: division by zero"); + } + + /** + * @dev Returns the integer division of two unsigned integers. + * Reverts with custom message on division by zero. The result is rounded towards zero. + * + * Counterpart to Solidity's `/` operator. Note: this function uses a + * `revert` opcode (which leaves remaining gas untouched) while Solidity + * uses an invalid opcode to revert (consuming all remaining gas). + * + * Requirements: + * - The divisor cannot be zero. + */ + function div(uint256 a, uint256 b, string memory errorMessage) internal pure returns (uint256) { + // Solidity only automatically asserts when dividing by 0 + require(b > 0, errorMessage); + uint256 c = a / b; + // assert(a == b * c + a % b); // There is no case in which this doesn't hold + + return c; + } + + /** + * @dev Returns the remainder of dividing two unsigned integers. (unsigned integer modulo), + * Reverts when dividing by zero. + * + * Counterpart to Solidity's `%` operator. This function uses a `revert` + * opcode (which leaves remaining gas untouched) while Solidity uses an + * invalid opcode to revert (consuming all remaining gas). + * + * Requirements: + * - The divisor cannot be zero. + */ + function mod(uint256 a, uint256 b) internal pure returns (uint256) { + return mod(a, b, "SafeMath: modulo by zero"); + } + + /** + * @dev Returns the remainder of dividing two unsigned integers. (unsigned integer modulo), + * Reverts with custom message when dividing by zero. + * + * Counterpart to Solidity's `%` operator. This function uses a `revert` + * opcode (which leaves remaining gas untouched) while Solidity uses an + * invalid opcode to revert (consuming all remaining gas). + * + * Requirements: + * - The divisor cannot be zero. + */ + function mod(uint256 a, uint256 b, string memory errorMessage) internal pure returns (uint256) { + require(b != 0, errorMessage); + return a % b; + } +} \ No newline at end of file diff --git a/contracts/Timelock.sol b/contracts/Timelock.sol new file mode 100644 index 0000000..b085804 --- /dev/null +++ b/contracts/Timelock.sol @@ -0,0 +1,112 @@ +pragma solidity ^0.5.16; + +import "./SafeMath.sol"; + +contract Timelock { + using SafeMath for uint; + + event NewAdmin(address indexed newAdmin); + event NewPendingAdmin(address indexed newPendingAdmin); + event NewDelay(uint indexed newDelay); + event CancelTransaction(bytes32 indexed txHash, address indexed target, uint value, string signature, bytes data, uint eta); + event ExecuteTransaction(bytes32 indexed txHash, address indexed target, uint value, string signature, bytes data, uint eta); + event QueueTransaction(bytes32 indexed txHash, address indexed target, uint value, string signature, bytes data, uint eta); + + uint public constant GRACE_PERIOD = 14 days; + uint public constant MINIMUM_DELAY = 2 days; + uint public constant MAXIMUM_DELAY = 30 days; + + address public admin; + address public pendingAdmin; + uint public delay; + + mapping (bytes32 => bool) public queuedTransactions; + + + constructor(address admin_, uint delay_) public { + + require(delay_ >= MINIMUM_DELAY, "Timelock::constructor: Delay must exceed minimum delay."); + require(delay_ <= MAXIMUM_DELAY, "Timelock::setDelay: Delay must not exceed maximum delay."); + + admin = admin_; + delay = delay_; + } + + function() external payable { } + + function setDelay(uint delay_) public { + require(msg.sender == address(this), "Timelock::setDelay: Call must come from Timelock."); + require(delay_ >= MINIMUM_DELAY, "Timelock::setDelay: Delay must exceed minimum delay."); + require(delay_ <= MAXIMUM_DELAY, "Timelock::setDelay: Delay must not exceed maximum delay."); + delay = delay_; + + emit NewDelay(delay); + } + + function acceptAdmin() public { + require(msg.sender == pendingAdmin, "Timelock::acceptAdmin: Call must come from pendingAdmin."); + admin = msg.sender; + pendingAdmin = address(0); + + emit NewAdmin(admin); + } + + function setPendingAdmin(address pendingAdmin_) public { + require(msg.sender == address(this), "Timelock::setPendingAdmin: Call must come from Timelock."); + pendingAdmin = pendingAdmin_; + + emit NewPendingAdmin(pendingAdmin); + } + + function queueTransaction(address target, uint value, string memory signature, bytes memory data, uint eta) public returns (bytes32) { + require(msg.sender == admin, "Timelock::queueTransaction: Call must come from admin."); + require(eta >= getBlockTimestamp().add(delay), "Timelock::queueTransaction: Estimated execution block must satisfy delay."); + + bytes32 txHash = keccak256(abi.encode(target, value, signature, data, eta)); + queuedTransactions[txHash] = true; + + emit QueueTransaction(txHash, target, value, signature, data, eta); + return txHash; + } + + function cancelTransaction(address target, uint value, string memory signature, bytes memory data, uint eta) public { + require(msg.sender == admin, "Timelock::cancelTransaction: Call must come from admin."); + + bytes32 txHash = keccak256(abi.encode(target, value, signature, data, eta)); + queuedTransactions[txHash] = false; + + emit CancelTransaction(txHash, target, value, signature, data, eta); + } + + function executeTransaction(address target, uint value, string memory signature, bytes memory data, uint eta) public payable returns (bytes memory) { + require(msg.sender == admin, "Timelock::executeTransaction: Call must come from admin."); + + bytes32 txHash = keccak256(abi.encode(target, value, signature, data, eta)); + require(queuedTransactions[txHash], "Timelock::executeTransaction: Transaction hasn't been queued."); + require(getBlockTimestamp() >= eta, "Timelock::executeTransaction: Transaction hasn't surpassed time lock."); + require(getBlockTimestamp() <= eta.add(GRACE_PERIOD), "Timelock::executeTransaction: Transaction is stale."); + + queuedTransactions[txHash] = false; + + bytes memory callData; + + if (bytes(signature).length == 0) { + callData = data; + } else { + callData = abi.encodePacked(bytes4(keccak256(bytes(signature))), data); + } + + // solium-disable-next-line security/no-call-value + (bool success, bytes memory returnData) = target.call.value(value)(callData); + require(success, "Timelock::executeTransaction: Transaction execution reverted."); + + emit ExecuteTransaction(txHash, target, value, signature, data, eta); + + return returnData; + } + + function getBlockTimestamp() internal view returns (uint) { + // solium-disable-next-line security/no-block-members + return block.timestamp; + } +} \ No newline at end of file diff --git a/test/Timelock.test.js b/test/Timelock.test.js new file mode 100644 index 0000000..bb8045a --- /dev/null +++ b/test/Timelock.test.js @@ -0,0 +1,375 @@ +Timelock = artifacts.require("Timelock") +const { + encodeParameters, + etherUnsigned, + freezeTime, + keccak256 + } = require('./Utils/Ethereum'); +require("chai") + .use(require("chai-as-promised")) + .expect(); + + const oneWeekInSeconds = etherUnsigned(7 * 24 * 60 * 60); + const zero = etherUnsigned(0); + const gracePeriod = oneWeekInSeconds.multipliedBy(2); + + describe('Timelock', () => { + let root, notAdmin, newAdmin; + let blockTimestamp; + let timelock; + let delay = oneWeekInSeconds; + let newDelay = delay.multipliedBy(2); + let target; + let value = zero; + let signature = 'setDelay(uint256)'; + let data = encodeParameters(['uint256'], [newDelay.toFixed()]); + let revertData = encodeParameters(['uint256'], [etherUnsigned(60 * 60).toFixed()]); + let eta; + let queuedTxHash; + + beforeEach(async () => { + [root, notAdmin, newAdmin] = await web3.eth.getAccounts(); + timelock = await Timelock.new(root, delay); + + blockTimestamp = etherUnsigned(100); + await freezeTime(blockTimestamp.toNumber()) + target = timelock.address; + eta = blockTimestamp.plus(delay); + + queuedTxHash = keccak256( + encodeParameters( + ['address', 'uint256', 'string', 'bytes', 'uint256'], + [target, value.toString(), signature, data, eta.toString()] + ) + ); + }); + + describe('constructor', () => { + it('sets address of admin', async () => { + let configuredAdmin = await timelock.admin(); + expect(configuredAdmin).to.equal(root); + }); + + it('sets delay', async () => { + let configuredDelay = await timelock.delay(); + expect(configuredDelay.toString()).to.equal(delay.toString()); + }); + }); + + describe('setDelay', () => { + it('requires msg.sender to be Timelock', async () => { + expect(timelock.setDelay(delay, { from: root })).to.eventually.be.rejectedWith('revert Timelock::setDelay: Call must come from Timelock.'); + }); + }); + + describe('setPendingAdmin', () => { + it('requires msg.sender to be Timelock', async () => { + expect( + timelock.setPendingAdmin(newAdmin, { from: root }) + ).to.eventually.be.rejectedWith('revert Timelock::setPendingAdmin: Call must come from Timelock.'); + }); + }); + + describe.skip('acceptAdmin', () => { + afterEach(async () => { + await send(timelock, 'harnessSetAdmin', [root], { from: root }); + }); + + it('requires msg.sender to be pendingAdmin', async () => { + await expect( + send(timelock, 'acceptAdmin', { from: notAdmin }) + ).rejects.toRevert('revert Timelock::acceptAdmin: Call must come from pendingAdmin.'); + }); + + it('sets pendingAdmin to address 0 and changes admin', async () => { + await send(timelock, 'harnessSetPendingAdmin', [newAdmin], { from: root }); + const pendingAdminBefore = await call(timelock, 'pendingAdmin'); + expect(pendingAdminBefore).toEqual(newAdmin); + + const result = await send(timelock, 'acceptAdmin', { from: newAdmin }); + const pendingAdminAfter = await call(timelock, 'pendingAdmin'); + expect(pendingAdminAfter).toEqual('0x0000000000000000000000000000000000000000'); + + const timelockAdmin = await call(timelock, 'admin'); + expect(timelockAdmin).toEqual(newAdmin); + + expect(result).toHaveLog('NewAdmin', { + newAdmin + }); + }); + }); + + describe('queueTransaction', () => { + it('requires admin to be msg.sender', async () => { + expect( + timelock.queueTransaction(target, value, signature, data, eta, { from: notAdmin }) + ).to.eventually.be.rejectedWith('revert Timelock::queueTransaction: Call must come from admin.'); + }); + + it('requires eta to exceed delay', async () => { + const etaLessThanDelay = blockTimestamp.plus(delay).minus(1); + + expect( + timelock.queueTransaction(target, value, signature, data, etaLessThanDelay, { from: root}) + ).to.eventually.be.rejectedWith('revert Timelock::queueTransaction: Estimated execution block must satisfy delay.'); + }); + + it.skip('sets hash as true in queuedTransactions mapping', async () => { + const queueTransactionsHashValueBefore = await timelock.queuedTransactions(queuedTxHash); + expect(queueTransactionsHashValueBefore).to.be.false; + + await timelock.queueTransaction(target, value, signature, data, eta, { from: root }); + + const queueTransactionsHashValueAfter = await timelock.queuedTransactions(queuedTxHash); + expect(queueTransactionsHashValueAfter).to.be.true; + }); + + it.skip('should emit QueueTransaction event', async () => { + const result = await send(timelock, 'queueTransaction', [target, value, signature, data, eta], { + from: root + }); + + expect(result).toHaveLog('QueueTransaction', { + data, + signature, + target, + eta: eta.toString(), + txHash: queuedTxHash, + value: value.toString() + }); + }); + }); + + describe.skip('cancelTransaction', () => { + beforeEach(async () => { + await send(timelock, 'queueTransaction', [target, value, signature, data, eta], { from: root }); + }); + + it('requires admin to be msg.sender', async () => { + await expect( + send(timelock, 'cancelTransaction', [target, value, signature, data, eta], { from: notAdmin }) + ).rejects.toRevert('revert Timelock::cancelTransaction: Call must come from admin.'); + }); + + it('sets hash from true to false in queuedTransactions mapping', async () => { + const queueTransactionsHashValueBefore = await call(timelock, 'queuedTransactions', [queuedTxHash]); + expect(queueTransactionsHashValueBefore).toEqual(true); + + await send(timelock, 'cancelTransaction', [target, value, signature, data, eta], { from: root }); + + const queueTransactionsHashValueAfter = await call(timelock, 'queuedTransactions', [queuedTxHash]); + expect(queueTransactionsHashValueAfter).toEqual(false); + }); + + it('should emit CancelTransaction event', async () => { + const result = await send(timelock, 'cancelTransaction', [target, value, signature, data, eta], { + from: root + }); + + expect(result).toHaveLog('CancelTransaction', { + data, + signature, + target, + eta: eta.toString(), + txHash: queuedTxHash, + value: value.toString() + }); + }); + }); + + describe.skip('queue and cancel empty', () => { + it('can queue and cancel an empty signature and data', async () => { + const txHash = keccak256( + encodeParameters( + ['address', 'uint256', 'string', 'bytes', 'uint256'], + [target, value.toString(), '', '0x', eta.toString()] + ) + ); + expect(await call(timelock, 'queuedTransactions', [txHash])).toBeFalsy(); + await send(timelock, 'queueTransaction', [target, value, '', '0x', eta], { from: root }); + expect(await call(timelock, 'queuedTransactions', [txHash])).toBeTruthy(); + await send(timelock, 'cancelTransaction', [target, value, '', '0x', eta], { from: root }); + expect(await call(timelock, 'queuedTransactions', [txHash])).toBeFalsy(); + }); + }); + + describe.skip('executeTransaction (setDelay)', () => { + beforeEach(async () => { + // Queue transaction that will succeed + await send(timelock, 'queueTransaction', [target, value, signature, data, eta], { + from: root + }); + + // Queue transaction that will revert when executed + await send(timelock, 'queueTransaction', [target, value, signature, revertData, eta], { + from: root + }); + }); + + it('requires admin to be msg.sender', async () => { + await expect( + send(timelock, 'executeTransaction', [target, value, signature, data, eta], { from: notAdmin }) + ).rejects.toRevert('revert Timelock::executeTransaction: Call must come from admin.'); + }); + + it('requires transaction to be queued', async () => { + const differentEta = eta.plus(1); + await expect( + send(timelock, 'executeTransaction', [target, value, signature, data, differentEta], { from: root }) + ).rejects.toRevert("revert Timelock::executeTransaction: Transaction hasn't been queued."); + }); + + it('requires timestamp to be greater than or equal to eta', async () => { + await expect( + send(timelock, 'executeTransaction', [target, value, signature, data, eta], { + from: root + }) + ).rejects.toRevert( + "revert Timelock::executeTransaction: Transaction hasn't surpassed time lock." + ); + }); + + it('requires timestamp to be less than eta plus gracePeriod', async () => { + await freezeTime(blockTimestamp.plus(delay).plus(gracePeriod).plus(1).toNumber()); + + await expect( + send(timelock, 'executeTransaction', [target, value, signature, data, eta], { + from: root + }) + ).rejects.toRevert('revert Timelock::executeTransaction: Transaction is stale.'); + }); + + it('requires target.call transaction to succeed', async () => { + await freezeTime(eta.toNumber()); + + await expect( + send(timelock, 'executeTransaction', [target, value, signature, revertData, eta], { + from: root + }) + ).rejects.toRevert('revert Timelock::executeTransaction: Transaction execution reverted.'); + }); + + it('sets hash from true to false in queuedTransactions mapping, updates delay, and emits ExecuteTransaction event', async () => { + const configuredDelayBefore = await call(timelock, 'delay'); + expect(configuredDelayBefore).toEqual(delay.toString()); + + const queueTransactionsHashValueBefore = await call(timelock, 'queuedTransactions', [queuedTxHash]); + expect(queueTransactionsHashValueBefore).toEqual(true); + + const newBlockTimestamp = blockTimestamp.plus(delay).plus(1); + await freezeTime(newBlockTimestamp.toNumber()); + + const result = await send(timelock, 'executeTransaction', [target, value, signature, data, eta], { + from: root + }); + + const queueTransactionsHashValueAfter = await call(timelock, 'queuedTransactions', [queuedTxHash]); + expect(queueTransactionsHashValueAfter).toEqual(false); + + const configuredDelayAfter = await call(timelock, 'delay'); + expect(configuredDelayAfter).toEqual(newDelay.toString()); + + expect(result).toHaveLog('ExecuteTransaction', { + data, + signature, + target, + eta: eta.toString(), + txHash: queuedTxHash, + value: value.toString() + }); + + expect(result).toHaveLog('NewDelay', { + newDelay: newDelay.toString() + }); + }); + }); + + describe.skip('executeTransaction (setPendingAdmin)', () => { + beforeEach(async () => { + const configuredDelay = await call(timelock, 'delay'); + + delay = etherUnsigned(configuredDelay); + signature = 'setPendingAdmin(address)'; + data = encodeParameters(['address'], [newAdmin]); + eta = blockTimestamp.plus(delay); + + queuedTxHash = keccak256( + encodeParameters( + ['address', 'uint256', 'string', 'bytes', 'uint256'], + [target, value.toString(), signature, data, eta.toString()] + ) + ); + + await send(timelock, 'queueTransaction', [target, value, signature, data, eta], { + from: root + }); + }); + + it('requires admin to be msg.sender', async () => { + await expect( + send(timelock, 'executeTransaction', [target, value, signature, data, eta], { from: notAdmin }) + ).rejects.toRevert('revert Timelock::executeTransaction: Call must come from admin.'); + }); + + it('requires transaction to be queued', async () => { + const differentEta = eta.plus(1); + await expect( + send(timelock, 'executeTransaction', [target, value, signature, data, differentEta], { from: root }) + ).rejects.toRevert("revert Timelock::executeTransaction: Transaction hasn't been queued."); + }); + + it('requires timestamp to be greater than or equal to eta', async () => { + await expect( + send(timelock, 'executeTransaction', [target, value, signature, data, eta], { + from: root + }) + ).rejects.toRevert( + "revert Timelock::executeTransaction: Transaction hasn't surpassed time lock." + ); + }); + + it('requires timestamp to be less than eta plus gracePeriod', async () => { + await freezeTime(blockTimestamp.plus(delay).plus(gracePeriod).plus(1).toNumber()); + + await expect( + send(timelock, 'executeTransaction', [target, value, signature, data, eta], { + from: root + }) + ).rejects.toRevert('revert Timelock::executeTransaction: Transaction is stale.'); + }); + + it('sets hash from true to false in queuedTransactions mapping, updates admin, and emits ExecuteTransaction event', async () => { + const configuredPendingAdminBefore = await call(timelock, 'pendingAdmin'); + expect(configuredPendingAdminBefore).toEqual('0x0000000000000000000000000000000000000000'); + + const queueTransactionsHashValueBefore = await call(timelock, 'queuedTransactions', [queuedTxHash]); + expect(queueTransactionsHashValueBefore).toEqual(true); + + const newBlockTimestamp = blockTimestamp.plus(delay).plus(1); + await freezeTime(newBlockTimestamp.toNumber()) + + const result = await send(timelock, 'executeTransaction', [target, value, signature, data, eta], { + from: root + }); + + const queueTransactionsHashValueAfter = await call(timelock, 'queuedTransactions', [queuedTxHash]); + expect(queueTransactionsHashValueAfter).toEqual(false); + + const configuredPendingAdminAfter = await call(timelock, 'pendingAdmin'); + expect(configuredPendingAdminAfter).toEqual(newAdmin); + + expect(result).toHaveLog('ExecuteTransaction', { + data, + signature, + target, + eta: eta.toString(), + txHash: queuedTxHash, + value: value.toString() + }); + + expect(result).toHaveLog('NewPendingAdmin', { + newPendingAdmin: newAdmin + }); + }); + }); + }); \ No newline at end of file