diff --git a/contracts/governance/ExecutorWithTimelock.sol b/contracts/governance/ExecutorWithTimelock.sol index 91e2f178e..977a57afd 100644 --- a/contracts/governance/ExecutorWithTimelock.sol +++ b/contracts/governance/ExecutorWithTimelock.sol @@ -3,7 +3,9 @@ pragma solidity 0.8.10; pragma abicoder v2; import {IExecutorWithTimelock} from "../interfaces/IExecutorWithTimelock.sol"; +import {IACLManager} from "../interfaces/IACLManager.sol"; import {SafeMath} from "../dependencies/openzeppelin/contracts/SafeMath.sol"; +import {Errors} from "../protocol/libraries/helpers/Errors.sol"; /** * @title Time Locked Executor Contract @@ -13,90 +15,156 @@ import {SafeMath} from "../dependencies/openzeppelin/contracts/SafeMath.sol"; **/ contract ExecutorWithTimelock is IExecutorWithTimelock { using SafeMath for uint256; + enum TransactionStatus { + Default, + Queued, + Approved, + Cancelled, + Executed + } + IACLManager private immutable aclManager; uint256 public immutable override GRACE_PERIOD; uint256 public immutable override MINIMUM_DELAY; uint256 public immutable override MAXIMUM_DELAY; - address private _admin; - address private _pendingAdmin; uint256 private _delay; - mapping(bytes32 => bool) private _queuedTransactions; + // Map of actionHash to status + mapping(bytes32 => TransactionStatus) public transactionStatus; + + // Map of contract selector need to be approved (contract address => selector => needApprove) + mapping(address => mapping(bytes4 => bool)) public needApprovalFilter; /** * @dev Constructor - * @param admin admin address, that can call the main functions, (Governance) * @param delay minimum time between queueing and execution of proposal * @param gracePeriod time after `delay` while a proposal can be executed * @param minimumDelay lower threshold of `delay`, in seconds * @param maximumDelay upper threshold of `delay`, in seconds **/ constructor( - address admin, + address _aclManager, uint256 delay, uint256 gracePeriod, uint256 minimumDelay, uint256 maximumDelay ) { + aclManager = IACLManager(_aclManager); require(delay >= minimumDelay, "DELAY_SHORTER_THAN_MINIMUM"); require(delay <= maximumDelay, "DELAY_LONGER_THAN_MAXIMUM"); _delay = delay; - _admin = admin; GRACE_PERIOD = gracePeriod; MINIMUM_DELAY = minimumDelay; MAXIMUM_DELAY = maximumDelay; emit NewDelay(delay); - emit NewAdmin(admin); } - modifier onlyAdmin() { - require(msg.sender == _admin, "ONLY_BY_ADMIN"); + /** + * @dev Only propose admin can call functions marked by this modifier. + **/ + modifier onlyProposeAdmin() { + _onlyProposeAdmin(); _; } - modifier onlyTimelock() { - require(msg.sender == address(this), "ONLY_BY_THIS_TIMELOCK"); + /** + * @dev Only approve admin can call functions marked by this modifier. + **/ + modifier onlyApproveAdmin() { + _onlyApproveAdmin(); _; } - modifier onlyPendingAdmin() { - require(msg.sender == _pendingAdmin, "ONLY_BY_PENDING_ADMIN"); + /** + * @dev Only propose admin can call functions marked by this modifier. + **/ + modifier onlyProposeOrApproveAdmin() { + _onlyProposeOrApproveAdmin(); _; } - /** - * @dev Set the delay - * @param delay delay between queue and execution of proposal - **/ - function setDelay(uint256 delay) public onlyTimelock { - _validateDelay(delay); - _delay = delay; + function _onlyProposeAdmin() internal view { + require( + aclManager.isActionProposeAdmin(msg.sender), + Errors.CALLER_NOT_ACTION_PROPOSE_ADMIN + ); + } - emit NewDelay(delay); + function _onlyApproveAdmin() internal view { + require( + aclManager.isActionApproveAdmin(msg.sender), + Errors.CALLER_NOT_ACTION_APPROVE_ADMIN + ); + } + + function _onlyProposeOrApproveAdmin() internal view { + require( + aclManager.isActionProposeAdmin(msg.sender) || + aclManager.isActionApproveAdmin(msg.sender), + Errors.CALLER_NOT_ACTION_PROPOSE_OR_APPROVE_ADMIN + ); + } + + modifier onlyTimelock() { + require(msg.sender == address(this), "ONLY_BY_THIS_TIMELOCK"); + _; } /** - * @dev Function enabling pending admin to become admin + * @dev Set if an action need approval + * @param targets the contract addresses for the action + * @param selectors the function selectors for the action + * @param needApprovals identify if action need approval **/ - function acceptAdmin() public onlyPendingAdmin { - _admin = msg.sender; - _pendingAdmin = address(0); - - emit NewAdmin(msg.sender); + function setActionNeedApproval( + address[] calldata targets, + bytes4[][] calldata selectors, + bool[][] calldata needApprovals + ) external onlyApproveAdmin { + uint256 targetLength = targets.length; + require( + selectors.length == targetLength && + needApprovals.length == targetLength, + "invalid params" + ); + for (uint256 index = 0; index < targetLength; index++) { + address target = targets[index]; + bytes4[] calldata targetSelectors = selectors[index]; + bool[] calldata targetApprovals = needApprovals[index]; + uint256 selectorLength = targetSelectors.length; + require(selectorLength == targetApprovals.length, "invalid params"); + for ( + uint256 selectorIndex = 0; + selectorIndex < selectorLength; + selectorIndex++ + ) { + bytes4 selector = targetSelectors[selectorIndex]; + bool needApproval = targetApprovals[selectorIndex]; + bool currentStatus = needApprovalFilter[target][selector]; + if (currentStatus != needApproval) { + needApprovalFilter[target][selector] = needApproval; + emit UpdateActionApproveFilter( + target, + selector, + needApproval + ); + } + } + } } /** - * @dev Setting a new pending admin (that can then become admin) - * Can only be called by this executor (i.e via proposal) - * @param newPendingAdmin address of the new admin + * @dev Set the delay + * @param delay delay between queue and execution of proposal **/ - function setPendingAdmin(address newPendingAdmin) public onlyTimelock { - _pendingAdmin = newPendingAdmin; + function setDelay(uint256 delay) public onlyTimelock { + _validateDelay(delay); + _delay = delay; - emit NewPendingAdmin(newPendingAdmin); + emit NewDelay(delay); } /** @@ -116,7 +184,7 @@ contract ExecutorWithTimelock is IExecutorWithTimelock { bytes memory data, uint256 executionTime, bool withDelegatecall - ) public override onlyAdmin returns (bytes32) { + ) public override onlyProposeAdmin returns (bytes32) { require( executionTime >= block.timestamp.add(_delay), "EXECUTION_TIME_UNDERESTIMATED" @@ -132,7 +200,11 @@ contract ExecutorWithTimelock is IExecutorWithTimelock { withDelegatecall ) ); - _queuedTransactions[actionHash] = true; + require( + transactionStatus[actionHash] == TransactionStatus.Default, + "WRONG_ACTION_STATUS " + ); + transactionStatus[actionHash] = TransactionStatus.Queued; emit QueuedAction( actionHash, @@ -146,6 +218,66 @@ contract ExecutorWithTimelock is IExecutorWithTimelock { return actionHash; } + /** + * @dev Function, called by Governance, that approve a transaction, returns action hash + * @param target smart contract target + * @param value wei value of the transaction + * @param signature function signature of the transaction + * @param data function arguments of the transaction or callData if signature empty + * @param executionTime time at which to execute the transaction + * @param withDelegatecall boolean, true = transaction delegatecalls the target, else calls the target + * @return the action Hash + **/ + function approveTransaction( + address target, + uint256 value, + string memory signature, + bytes memory data, + uint256 executionTime, + bool withDelegatecall + ) public override onlyApproveAdmin returns (bytes32) { + bytes32 actionHash = keccak256( + abi.encode( + target, + value, + signature, + data, + executionTime, + withDelegatecall + ) + ); + + require( + transactionStatus[actionHash] == TransactionStatus.Queued, + "ACTION_NOT_QUEUED" + ); + bytes4 selector; + if (bytes(signature).length == 0) { + selector = bytes4(data); + } else { + selector = bytes4(keccak256(bytes(signature))); + } + bool isNeedApproval = needApprovalFilter[target][selector]; + require(isNeedApproval, "ACTION_NOT_NEED_APPROVAL"); + require( + block.timestamp <= executionTime.add(GRACE_PERIOD), + "GRACE_PERIOD_FINISHED" + ); + + transactionStatus[actionHash] = TransactionStatus.Approved; + + emit ApprovedAction( + actionHash, + target, + value, + signature, + data, + executionTime, + withDelegatecall + ); + return actionHash; + } + /** * @dev Function, called by Governance, that cancels a transaction, returns action hash * @param target smart contract target @@ -163,7 +295,7 @@ contract ExecutorWithTimelock is IExecutorWithTimelock { bytes memory data, uint256 executionTime, bool withDelegatecall - ) public override onlyAdmin returns (bytes32) { + ) public override onlyProposeOrApproveAdmin returns (bytes32) { bytes32 actionHash = keccak256( abi.encode( target, @@ -174,7 +306,13 @@ contract ExecutorWithTimelock is IExecutorWithTimelock { withDelegatecall ) ); - _queuedTransactions[actionHash] = false; + TransactionStatus status = transactionStatus[actionHash]; + require( + status == TransactionStatus.Queued || + status == TransactionStatus.Approved, + "WRONG_ACTION_STATUS " + ); + transactionStatus[actionHash] = TransactionStatus.Cancelled; emit CancelledAction( actionHash, @@ -205,7 +343,7 @@ contract ExecutorWithTimelock is IExecutorWithTimelock { bytes memory data, uint256 executionTime, bool withDelegatecall - ) public payable override onlyAdmin returns (bytes memory) { + ) public payable override onlyProposeAdmin returns (bytes memory) { bytes32 actionHash = keccak256( abi.encode( target, @@ -216,26 +354,35 @@ contract ExecutorWithTimelock is IExecutorWithTimelock { withDelegatecall ) ); - require(_queuedTransactions[actionHash], "ACTION_NOT_QUEUED"); require(block.timestamp >= executionTime, "TIMELOCK_NOT_FINISHED"); require( block.timestamp <= executionTime.add(GRACE_PERIOD), "GRACE_PERIOD_FINISHED" ); - _queuedTransactions[actionHash] = false; - + bytes4 selector; bytes memory callData; - if (bytes(signature).length == 0) { + selector = bytes4(data); callData = data; } else { - callData = abi.encodePacked( - bytes4(keccak256(bytes(signature))), - data + selector = bytes4(keccak256(bytes(signature))); + callData = abi.encodePacked(selector, data); + } + if (needApprovalFilter[target][selector]) { + require( + transactionStatus[actionHash] == TransactionStatus.Approved, + "ACTION_NOT_APPROVED" + ); + } else { + require( + transactionStatus[actionHash] == TransactionStatus.Queued, + "ACTION_NOT_QUEUED" ); } + transactionStatus[actionHash] = TransactionStatus.Executed; + bool success; bytes memory resultData; if (withDelegatecall) { @@ -263,22 +410,6 @@ contract ExecutorWithTimelock is IExecutorWithTimelock { return resultData; } - /** - * @dev Getter of the current admin address (should be governance) - * @return The address of the current admin - **/ - function getAdmin() external view override returns (address) { - return _admin; - } - - /** - * @dev Getter of the current pending admin address - * @return The address of the pending admin - **/ - function getPendingAdmin() external view override returns (address) { - return _pendingAdmin; - } - /** * @dev Getter of the delay between queuing and execution * @return The delay in seconds @@ -299,7 +430,22 @@ contract ExecutorWithTimelock is IExecutorWithTimelock { override returns (bool) { - return _queuedTransactions[actionHash]; + return transactionStatus[actionHash] == TransactionStatus.Queued; + } + + /** + * @dev Returns whether an action (via actionHash) is approved + * @param actionHash hash of the action to be checked + * keccak256(abi.encode(target, value, signature, data, executionTime, withDelegatecall)) + * @return true if underlying action of actionHash is approved + **/ + function isActionApproved(bytes32 actionHash) + external + view + override + returns (bool) + { + return transactionStatus[actionHash] == TransactionStatus.Approved; } function _validateDelay(uint256 delay) internal view { diff --git a/contracts/interfaces/IACLManager.sol b/contracts/interfaces/IACLManager.sol index c5e5cdfb0..347cea43c 100644 --- a/contracts/interfaces/IACLManager.sol +++ b/contracts/interfaces/IACLManager.sol @@ -30,6 +30,12 @@ interface IACLManager { */ function EMERGENCY_ADMIN_ROLE() external view returns (bytes32); + /** + * @notice Returns the identifier of the EmergencyRecoveryAdmin role + * @return The id of the EmergencyRecoveryAdmin role + */ + function EMERGENCY_RECOVERY_ADMIN_ROLE() external view returns (bytes32); + /** * @notice Returns the identifier of the RiskAdmin role * @return The id of the RiskAdmin role @@ -54,6 +60,18 @@ interface IACLManager { */ function ASSET_LISTING_ADMIN_ROLE() external view returns (bytes32); + /** + * @notice Returns the identifier of the ActionApproveAdmin role + * @return The id of the ActionApproveAdmin role + */ + function ACTION_APPROVE_ADMIN_ROLE() external view returns (bytes32); + + /** + * @notice Returns the identifier of the ActionProposeAdmin role + * @return The id of the ActionProposeAdmin role + */ + function ACTION_PROPOSE_ADMIN_ROLE() external view returns (bytes32); + /** * @notice Set the role as admin of a specific role. * @dev By default the admin role for all roles is `DEFAULT_ADMIN_ROLE`. @@ -100,6 +118,66 @@ interface IACLManager { */ function isEmergencyAdmin(address admin) external view returns (bool); + /** + * @notice Adds a new admin as EmergencyRecoveryAdmin + * @param admin The address of the new admin + */ + function addEmergencyRecoveryAdmin(address admin) external; + + /** + * @notice Removes an admin as EmergencyRecoveryAdmin + * @param admin The address of the admin to remove + */ + function removeEmergencyRecoveryAdmin(address admin) external; + + /** + * @notice Returns true if the address is EmergencyRecoveryAdmin, false otherwise + * @param admin The address to check + * @return True if the given address is EmergencyRecoveryAdmin, false otherwise + */ + function isEmergencyRecoveryAdmin(address admin) + external + view + returns (bool); + + /** + * @notice Adds a new admin as ActionApproveAdmin + * @param admin The address of the new admin + */ + function addActionApproveAdmin(address admin) external; + + /** + * @notice Removes an admin as ActionApproveAdmin + * @param admin The address of the admin to remove + */ + function removeActionApproveAdmin(address admin) external; + + /** + * @notice Returns true if the address is ActionApproveAdmin, false otherwise + * @param admin The address to check + * @return True if the given address is UpgradeApproveAdmin, false otherwise + */ + function isActionApproveAdmin(address admin) external view returns (bool); + + /** + * @notice Adds a new admin as ActionProposeAdmin + * @param admin The address of the new admin + */ + function addActionProposeAdmin(address admin) external; + + /** + * @notice Removes an admin as ActionProposeAdmin + * @param admin The address of the admin to remove + */ + function removeActionProposeAdmin(address admin) external; + + /** + * @notice Returns true if the address is ActionProposeAdmin, false otherwise + * @param admin The address to check + * @return True if the given address is ActionProposeAdmin, false otherwise + */ + function isActionProposeAdmin(address admin) external view returns (bool); + /** * @notice Adds a new admin as RiskAdmin * @param admin The address of the new admin diff --git a/contracts/interfaces/IExecutorWithTimelock.sol b/contracts/interfaces/IExecutorWithTimelock.sol index 46adcc5bf..e2edc1e81 100644 --- a/contracts/interfaces/IExecutorWithTimelock.sol +++ b/contracts/interfaces/IExecutorWithTimelock.sol @@ -4,25 +4,45 @@ pragma abicoder v2; interface IExecutorWithTimelock { /** - * @dev emitted when a new pending admin is set - * @param newPendingAdmin address of the new pending admin + * @dev emitted when a new delay (between queueing and execution) is set + * @param delay new delay **/ - event NewPendingAdmin(address newPendingAdmin); + event NewDelay(uint256 delay); /** - * @dev emitted when a new admin is set - * @param newAdmin address of the new admin + * @dev emitted when update if an action need approve before execution + * @param target target contract address + * @param selector action selector + * @param needApproval new status **/ - event NewAdmin(address newAdmin); + event UpdateActionApproveFilter( + address target, + bytes4 selector, + bool needApproval + ); /** - * @dev emitted when a new delay (between queueing and execution) is set - * @param delay new delay + * @dev emitted when a new (trans)action is Queued. + * @param actionHash hash of the action + * @param target address of the targeted contract + * @param value wei value of the transaction + * @param signature function signature of the transaction + * @param data function arguments of the transaction or callData if signature empty + * @param executionTime time at which to execute the transaction + * @param withDelegatecall boolean, true = transaction delegatecalls the target, else calls the target **/ - event NewDelay(uint256 delay); + event QueuedAction( + bytes32 actionHash, + address indexed target, + uint256 value, + string signature, + bytes data, + uint256 executionTime, + bool withDelegatecall + ); /** - * @dev emitted when a new (trans)action is Queued. + * @dev emitted when an action is Approved * @param actionHash hash of the action * @param target address of the targeted contract * @param value wei value of the transaction @@ -31,7 +51,7 @@ interface IExecutorWithTimelock { * @param executionTime time at which to execute the transaction * @param withDelegatecall boolean, true = transaction delegatecalls the target, else calls the target **/ - event QueuedAction( + event ApprovedAction( bytes32 actionHash, address indexed target, uint256 value, @@ -83,18 +103,6 @@ interface IExecutorWithTimelock { bytes resultData ); - /** - * @dev Getter of the current admin address (should be governance) - * @return The address of the current admin - **/ - function getAdmin() external view returns (address); - - /** - * @dev Getter of the current pending admin address - * @return The address of the pending admin - **/ - function getPendingAdmin() external view returns (address); - /** * @dev Getter of the delay between queuing and execution * @return The delay in seconds @@ -109,6 +117,14 @@ interface IExecutorWithTimelock { **/ function isActionQueued(bytes32 actionHash) external view returns (bool); + /** + * @dev Returns whether an action (via actionHash) is approved + * @param actionHash hash of the action to be checked + * keccak256(abi.encode(target, value, signature, data, executionTime, withDelegatecall)) + * @return true if underlying action of actionHash is approved + **/ + function isActionApproved(bytes32 actionHash) external view returns (bool); + /** * @dev Getter of grace period constant * @return grace period in seconds @@ -145,6 +161,24 @@ interface IExecutorWithTimelock { bool withDelegatecall ) external returns (bytes32); + /** + * @dev Function, called by Governance, that approve a transaction, returns action hash + * @param target smart contract target + * @param value wei value of the transaction + * @param signature function signature of the transaction + * @param data function arguments of the transaction or callData if signature empty + * @param executionTime time at which to execute the transaction + * @param withDelegatecall boolean, true = transaction delegatecalls the target, else calls the target + **/ + function approveTransaction( + address target, + uint256 value, + string memory signature, + bytes memory data, + uint256 executionTime, + bool withDelegatecall + ) external returns (bytes32); + /** * @dev Function, called by Governance, that cancels a transaction, returns the callData executed * @param target smart contract target diff --git a/contracts/protocol/configuration/ACLManager.sol b/contracts/protocol/configuration/ACLManager.sol index dee73c48c..547994b5c 100644 --- a/contracts/protocol/configuration/ACLManager.sol +++ b/contracts/protocol/configuration/ACLManager.sol @@ -15,12 +15,18 @@ contract ACLManager is AccessControl, IACLManager { bytes32 public constant override POOL_ADMIN_ROLE = keccak256("POOL_ADMIN"); bytes32 public constant override EMERGENCY_ADMIN_ROLE = keccak256("EMERGENCY_ADMIN"); + bytes32 public constant override EMERGENCY_RECOVERY_ADMIN_ROLE = + keccak256("EMERGENCY_RECOVERY_ADMIN"); bytes32 public constant override RISK_ADMIN_ROLE = keccak256("RISK_ADMIN"); bytes32 public constant override FLASH_BORROWER_ROLE = keccak256("FLASH_BORROWER"); bytes32 public constant override BRIDGE_ROLE = keccak256("BRIDGE"); bytes32 public constant override ASSET_LISTING_ADMIN_ROLE = keccak256("ASSET_LISTING_ADMIN"); + bytes32 public constant override ACTION_APPROVE_ADMIN_ROLE = + keccak256("ACTION_APPROVE_ADMIN"); + bytes32 public constant override ACTION_PROPOSE_ADMIN_ROLE = + keccak256("ACTION_PROPOSE_ADMIN"); IPoolAddressesProvider public immutable ADDRESSES_PROVIDER; @@ -80,6 +86,66 @@ contract ACLManager is AccessControl, IACLManager { return hasRole(EMERGENCY_ADMIN_ROLE, admin); } + /// @inheritdoc IACLManager + function addEmergencyRecoveryAdmin(address admin) external override { + grantRole(EMERGENCY_RECOVERY_ADMIN_ROLE, admin); + } + + /// @inheritdoc IACLManager + function removeEmergencyRecoveryAdmin(address admin) external override { + revokeRole(EMERGENCY_RECOVERY_ADMIN_ROLE, admin); + } + + /// @inheritdoc IACLManager + function isEmergencyRecoveryAdmin(address admin) + external + view + override + returns (bool) + { + return hasRole(EMERGENCY_RECOVERY_ADMIN_ROLE, admin); + } + + /// @inheritdoc IACLManager + function addActionApproveAdmin(address admin) external override { + grantRole(ACTION_APPROVE_ADMIN_ROLE, admin); + } + + /// @inheritdoc IACLManager + function removeActionApproveAdmin(address admin) external override { + revokeRole(ACTION_APPROVE_ADMIN_ROLE, admin); + } + + /// @inheritdoc IACLManager + function isActionApproveAdmin(address admin) + external + view + override + returns (bool) + { + return hasRole(ACTION_APPROVE_ADMIN_ROLE, admin); + } + + /// @inheritdoc IACLManager + function addActionProposeAdmin(address admin) external override { + grantRole(ACTION_PROPOSE_ADMIN_ROLE, admin); + } + + /// @inheritdoc IACLManager + function removeActionProposeAdmin(address admin) external override { + revokeRole(ACTION_PROPOSE_ADMIN_ROLE, admin); + } + + /// @inheritdoc IACLManager + function isActionProposeAdmin(address admin) + external + view + override + returns (bool) + { + return hasRole(ACTION_PROPOSE_ADMIN_ROLE, admin); + } + /// @inheritdoc IACLManager function addRiskAdmin(address admin) external override { grantRole(RISK_ADMIN_ROLE, admin); diff --git a/contracts/protocol/libraries/helpers/Errors.sol b/contracts/protocol/libraries/helpers/Errors.sol index 35267304c..666cfa768 100644 --- a/contracts/protocol/libraries/helpers/Errors.sol +++ b/contracts/protocol/libraries/helpers/Errors.sol @@ -126,4 +126,7 @@ library Errors { string public constant CALLER_NOT_EOA = "131"; //The caller of the function is not an EOA account string public constant MAKER_SAME_AS_TAKER = "132"; //maker and taker shouldn't be the same address string public constant TOKEN_ALREADY_DELEGATED = "133"; //token is already delegted + string public constant CALLER_NOT_ACTION_PROPOSE_ADMIN = "134"; //The caller of the function is not action propose admin + string public constant CALLER_NOT_ACTION_APPROVE_ADMIN = "135"; //The caller of the function is not action approve admin + string public constant CALLER_NOT_ACTION_PROPOSE_OR_APPROVE_ADMIN = "136"; //The caller of the function is not action propose or approve admin } diff --git a/helpers/types.ts b/helpers/types.ts index 28adbc6d5..c3eda27dc 100644 --- a/helpers/types.ts +++ b/helpers/types.ts @@ -382,6 +382,10 @@ export enum ProtocolErrors { SAPE_NOT_ALLOWED = "128", //operation is not allow for sApe. TOTAL_STAKING_AMOUNT_WRONG = "129", //cash plus borrow amount not equal to total staking amount. NOT_THE_BAKC_OWNER = "130", //user is not the bakc owner. + + CALLER_NOT_ACTION_PROPOSE_ADMIN = "134", //The caller of the function is not action propose admin. + CALLER_NOT_ACTION_APPROVE_ADMIN = "135", //The caller of the function is not action approve admin. + CALLER_NOT_ACTION_PROPOSE_OR_APPROVE_ADMIN = "136", //The caller of the function is not action propose or approve admin // SafeCast SAFECAST_UINT128_OVERFLOW = "SafeCast: value doesn't fit in 128 bits", diff --git a/test/time_lock_executor.spec.ts b/test/time_lock_executor.spec.ts index a8b2ac9b1..db03b9d19 100644 --- a/test/time_lock_executor.spec.ts +++ b/test/time_lock_executor.spec.ts @@ -2,6 +2,8 @@ import {expect} from "chai"; import {TestEnv} from "./helpers/make-suite"; import { advanceTimeAndBlock, + evmRevert, + evmSnapshot, timeLatest, waitForTx, } from "../helpers/misc-utils"; @@ -10,27 +12,29 @@ import {testEnvFixture} from "./helpers/setup-env"; import {getACLManager} from "../helpers/contracts-getters"; import {ZERO_ADDRESS} from "../helpers/constants"; import {deployTimeLockExecutor} from "../helpers/contracts-deployments"; +import {ProtocolErrors} from "../helpers/types"; describe("ExecutorWithTimelock Test", () => { let testEnv: TestEnv; let timeLock; let dropReserveEncodedData; + let dropReserveSelector; let executionTimeStamp; before(async () => { testEnv = await loadFixture(testEnvFixture); const {users, weth, configurator} = testEnv; + const aclManager = await getACLManager(); + timeLock = await deployTimeLockExecutor([ - users[0].address, + aclManager.address, "100", "100", "10", "1000", ]); - const aclManager = await getACLManager(); - expect(await aclManager.isPoolAdmin(timeLock.address)).to.be.eq(false); await waitForTx( @@ -38,15 +42,30 @@ describe("ExecutorWithTimelock Test", () => { ); expect(await aclManager.isPoolAdmin(timeLock.address)).to.be.eq(true); + await waitForTx( + await aclManager + .connect(users[3].signer) + .addActionProposeAdmin(users[4].address) + ); + await waitForTx( + await aclManager + .connect(users[3].signer) + .addActionApproveAdmin(users[5].address) + ); + dropReserveEncodedData = configurator.interface.encodeFunctionData( "dropReserve", [weth.address] ); + dropReserveSelector = configurator.interface.getSighash( + "dropReserve(address)" + ); + executionTimeStamp = (await timeLatest()).add(110).toString(); }); - it("queueTransaction fails when user is not the owner", async () => { + it("queueTransaction fails when user is not action propose admin", async () => { const {users, configurator} = testEnv; await expect( @@ -60,19 +79,16 @@ describe("ExecutorWithTimelock Test", () => { executionTimeStamp, false ) - ).to.be.revertedWith("ONLY_BY_ADMIN"); + ).to.be.revertedWith(ProtocolErrors.CALLER_NOT_ACTION_PROPOSE_ADMIN); }); it("queueTransaction fails when execution time wrong", async () => { - const { - users: [user1], - configurator, - } = testEnv; + const {users, configurator} = testEnv; const executionTimeStamp1 = (await timeLatest()).add(10).toString(); await expect( timeLock - .connect(user1.signer) + .connect(users[4].signer) .queueTransaction( configurator.address, 0, @@ -84,35 +100,83 @@ describe("ExecutorWithTimelock Test", () => { ).to.be.revertedWith("EXECUTION_TIME_UNDERESTIMATED"); }); + it("approveTransaction fails when action is not queued", async () => { + const {users, configurator} = testEnv; + + await expect( + timeLock + .connect(users[5].signer) + .approveTransaction( + configurator.address, + 0, + "", + dropReserveEncodedData, + executionTimeStamp, + false + ) + ).to.be.revertedWith("ACTION_NOT_QUEUED"); + }); + it("executeTransaction fails when action is not queued", async () => { - const { - users: [user1], - configurator, - } = testEnv; + const {users, configurator} = testEnv; + const executionTimeStamp2 = (await timeLatest()).sub(50).toString(); await expect( timeLock - .connect(user1.signer) + .connect(users[4].signer) .executeTransaction( configurator.address, 0, "", dropReserveEncodedData, - executionTimeStamp, + executionTimeStamp2, false ) ).to.be.revertedWith("ACTION_NOT_QUEUED"); }); + it("cancelTransaction fails when caller is not propose or approve admin", async () => { + const {users, configurator} = testEnv; + + await expect( + timeLock + .connect(users[3].signer) + .cancelTransaction( + configurator.address, + 0, + "", + dropReserveEncodedData, + executionTimeStamp, + false + ) + ).to.be.revertedWith( + ProtocolErrors.CALLER_NOT_ACTION_PROPOSE_OR_APPROVE_ADMIN + ); + }); + + it("cancelTransaction fails when action is not queued", async () => { + const {users, configurator} = testEnv; + + await expect( + timeLock + .connect(users[4].signer) + .cancelTransaction( + configurator.address, + 0, + "", + dropReserveEncodedData, + executionTimeStamp, + false + ) + ).to.be.revertedWith("WRONG_ACTION_STATUS"); + }); + it("queueTransaction success", async () => { - const { - users: [user1], - configurator, - } = testEnv; + const {users, configurator} = testEnv; expect( await timeLock - .connect(user1.signer) + .connect(users[4].signer) .queueTransaction( configurator.address, 0, @@ -124,15 +188,52 @@ describe("ExecutorWithTimelock Test", () => { ); }); + it("cancelTransaction success by propose admin", async () => { + const {users, configurator} = testEnv; + const snapId = await evmSnapshot(); + + expect( + await timeLock + .connect(users[4].signer) + .cancelTransaction( + configurator.address, + 0, + "", + dropReserveEncodedData, + executionTimeStamp, + false + ) + ); + + await evmRevert(snapId); + }); + + it("cancelTransaction success by approve admin", async () => { + const {users, configurator} = testEnv; + const snapId = await evmSnapshot(); + + expect( + await timeLock + .connect(users[5].signer) + .cancelTransaction( + configurator.address, + 0, + "", + dropReserveEncodedData, + executionTimeStamp, + false + ) + ); + + await evmRevert(snapId); + }); + it("executeTransaction fails when execution time not reach", async () => { - const { - users: [user1], - configurator, - } = testEnv; + const {users, configurator} = testEnv; await expect( timeLock - .connect(user1.signer) + .connect(users[4].signer) .executeTransaction( configurator.address, 0, @@ -144,21 +245,113 @@ describe("ExecutorWithTimelock Test", () => { ).to.be.revertedWith("TIMELOCK_NOT_FINISHED"); }); + it("executeTransaction fails when grace period finished", async () => { + const {users, configurator} = testEnv; + + const executionTimeStamp3 = (await timeLatest()).sub(120).toString(); + await expect( + timeLock + .connect(users[4].signer) + .executeTransaction( + configurator.address, + 0, + "", + dropReserveEncodedData, + executionTimeStamp3, + false + ) + ).to.be.revertedWith("GRACE_PERIOD_FINISHED"); + }); + + it("setActionNeedApproval fails when user is not action approve admin", async () => { + const {users, configurator} = testEnv; + + await expect( + timeLock + .connect(users[4].signer) + .setActionNeedApproval( + [configurator.address], + [[dropReserveSelector]], + [[true]] + ) + ).to.be.revertedWith(ProtocolErrors.CALLER_NOT_ACTION_APPROVE_ADMIN); + }); + + it("setActionNeedApproval success", async () => { + const {users, configurator} = testEnv; + + expect( + await timeLock + .connect(users[5].signer) + .setActionNeedApproval( + [configurator.address], + [[dropReserveSelector]], + [[true]] + ) + ); + }); + + it("executeTransaction fails when action need approve", async () => { + const {users, configurator} = testEnv; + + await advanceTimeAndBlock(110); + await expect( + timeLock + .connect(users[4].signer) + .executeTransaction( + configurator.address, + 0, + "", + dropReserveEncodedData, + executionTimeStamp, + false + ) + ).to.be.revertedWith("ACTION_NOT_APPROVED"); + }); + + it("approveTransaction fails when user is not action approve admin", async () => { + const {users, configurator} = testEnv; + + await expect( + timeLock + .connect(users[4].signer) + .approveTransaction( + configurator.address, + 0, + "", + dropReserveEncodedData, + executionTimeStamp, + false + ) + ).to.be.revertedWith(ProtocolErrors.CALLER_NOT_ACTION_APPROVE_ADMIN); + }); + + it("approveTransaction success", async () => { + const {users, configurator} = testEnv; + + expect( + await timeLock + .connect(users[5].signer) + .approveTransaction( + configurator.address, + 0, + "", + dropReserveEncodedData, + executionTimeStamp, + false + ) + ); + }); + it("executeTransaction success", async () => { - const { - users: [user1], - configurator, - pool, - weth, - } = testEnv; + const {users, configurator, pool, weth} = testEnv; let config = await pool.getReserveData(weth.address); expect(config.xTokenAddress).to.be.not.eq(ZERO_ADDRESS); - await advanceTimeAndBlock(110); expect( await timeLock - .connect(user1.signer) + .connect(users[4].signer) .executeTransaction( configurator.address, 0,