From e9622d147f9c90f1bbfb00c8fc05b86cd1f7e2a0 Mon Sep 17 00:00:00 2001 From: Dima Lekhovitsky Date: Mon, 23 Oct 2023 13:03:16 +0300 Subject: [PATCH] feat: `forbidBoundsUpdate` in controller timelock --- contracts/governance/ControllerTimelockV3.sol | 19 +++++ .../interfaces/IControllerTimelockV3.sol | 2 + .../unit/governance/ControllerTimelock.t.sol | 72 ++++++++++++++++++- 3 files changed, 92 insertions(+), 1 deletion(-) diff --git a/contracts/governance/ControllerTimelockV3.sol b/contracts/governance/ControllerTimelockV3.sol index 3b788b45..ea27e1c8 100644 --- a/contracts/governance/ControllerTimelockV3.sol +++ b/contracts/governance/ControllerTimelockV3.sol @@ -541,6 +541,25 @@ contract ControllerTimelockV3 is PolicyManagerV3, IControllerTimelockV3 { }); // U:[CT-16] } + /// @notice Queues a transaction to forbid permissionless bounds update in an LP price feed + /// @dev Requires the policy for keccak(group(priceFeed), "UPDATE_BOUNDS_ALLOWED") to be enabled, + /// otherwise auto-fails the check + /// @param priceFeed The price feed to forbid bounds update for + function forbidBoundsUpdate(address priceFeed) external override { + if (!_checkPolicy(priceFeed, "UPDATE_BOUNDS_ALLOWED", 0, 0)) { + revert ParameterChecksFailedException(); // U:[CT-17] + } + + _queueTransaction({ + target: priceFeed, + signature: "forbidBoundsUpdate()", + data: "", + delay: _getPolicyDelay(priceFeed, "UPDATE_BOUNDS_ALLOWED"), + sanityCheckValue: 0, + sanityCheckCallData: "" + }); // U:[CT-17] + } + /// @dev Internal function that stores the transaction in the queued tx map /// @param target The contract to call /// @param signature The signature of the called function diff --git a/contracts/interfaces/IControllerTimelockV3.sol b/contracts/interfaces/IControllerTimelockV3.sol index 1342c71a..1771345c 100644 --- a/contracts/interfaces/IControllerTimelockV3.sol +++ b/contracts/interfaces/IControllerTimelockV3.sol @@ -74,6 +74,8 @@ interface IControllerTimelockV3 is IControllerTimelockV3Events, IVersion { function setReservePriceFeedStatus(address priceOracle, address token, bool active) external; + function forbidBoundsUpdate(address priceFeed) external; + // --------- // // EXECUTION // // --------- // diff --git a/contracts/test/unit/governance/ControllerTimelock.t.sol b/contracts/test/unit/governance/ControllerTimelock.t.sol index 7ee16ac0..96828733 100644 --- a/contracts/test/unit/governance/ControllerTimelock.t.sol +++ b/contracts/test/unit/governance/ControllerTimelock.t.sol @@ -540,7 +540,7 @@ contract ControllerTimelockTest is Test, IControllerTimelockV3Events { /// @dev U:[CT-5]: setCreditManagerDebtLimit works correctly function test_U_CT_05_setCreditManagerDebtLimit_works_correctly() public { - (address creditManager, address creditFacade,, address pool,) = _makeMocks(); + (address creditManager, /* address creditFacade */,, address pool,) = _makeMocks(); // TODO: double check // vm.mockCall(creditFacade, abi.encodeCall(ICreditFacadeV3.trackTotalDebt, ()), abi.encode(false)); @@ -1676,4 +1676,74 @@ contract ControllerTimelockTest is Test, IControllerTimelockV3Events { assertTrue(!queued, "Transaction is still queued after execution"); } + + /// @dev U:[CT-17]: forbidBoundsUpdate works correctly + function test_U_CT_17_forbidBoundsUpdate_works_correctly() public { + address priceFeed = makeAddr("PRICE_FEED"); + vm.mockCall(priceFeed, abi.encodeWithSignature("forbidBoundsUpdate()"), ""); + + vm.prank(CONFIGURATOR); + controllerTimelock.setGroup(priceFeed, "PRICE_FEED"); + + bytes32 POLICY_CODE = keccak256(abi.encode("PRICE_FEED", "UPDATE_BOUNDS_ALLOWED")); + + Policy memory policy = Policy({ + enabled: false, + admin: admin, + delay: 1 days, + flags: 0, + exactValue: 0, + minValue: 0, + maxValue: 0, + referencePoint: 0, + referencePointUpdatePeriod: 0, + referencePointTimestampLU: 0, + minPctChangeDown: 0, + minPctChangeUp: 0, + maxPctChangeDown: 0, + maxPctChangeUp: 0, + minChange: 0, + maxChange: 0 + }); + + // VERIFY THAT THE FUNCTION CANNOT BE CALLED WITHOUT RESPECTIVE POLICY + vm.expectRevert(ParameterChecksFailedException.selector); + vm.prank(admin); + controllerTimelock.forbidBoundsUpdate(priceFeed); + + vm.prank(CONFIGURATOR); + controllerTimelock.setPolicy(POLICY_CODE, policy); + + // VERIFY THAT THE FUNCTION IS ONLY CALLABLE BY ADMIN + vm.expectRevert(ParameterChecksFailedException.selector); + vm.prank(USER); + controllerTimelock.forbidBoundsUpdate(priceFeed); + + // VERIFY THAT THE FUNCTION IS QUEUED AND EXECUTED CORRECTLY + bytes32 txHash = keccak256(abi.encode(admin, priceFeed, "forbidBoundsUpdate()", "", block.timestamp + 1 days)); + + vm.expectEmit(true, false, false, true); + emit QueueTransaction(txHash, admin, priceFeed, "forbidBoundsUpdate()", "", uint40(block.timestamp + 1 days)); + + vm.prank(admin); + controllerTimelock.forbidBoundsUpdate(priceFeed); + + (,,,,,, uint256 sanityCheckValue, bytes memory sanityCheckCallData) = + controllerTimelock.queuedTransactions(txHash); + + assertEq(sanityCheckValue, 0, "Sanity check value written incorrectly"); + + assertEq(sanityCheckCallData, ""); + + vm.expectCall(priceFeed, abi.encodeWithSignature("forbidBoundsUpdate()")); + + vm.warp(block.timestamp + 1 days); + + vm.prank(admin); + controllerTimelock.executeTransaction(txHash); + + (bool queued,,,,,,,) = controllerTimelock.queuedTransactions(txHash); + + assertTrue(!queued, "Transaction is still queued after execution"); + } }