modern rebalance timelock and threshold (#221)

* modern timelock and threshold * del duplicity in LibraryWrapper * update timelock properly * update strategy-controller test, passes * update library in other tests, not sure if passing * update live-upgrades test, passes * some tests updated, not all pass * update live-estimates test, passes * update storage gap * smooth out aave-adapter test for rebase * fix rebase, compiles * prettier test * fix errors.json after rebase * update strategy-controller test, passes * timelocked updateRebalanceParameters * keep library functions disjoint * update live-estimates test, passing
EnsoFinance · Jul 27, 2022 · 06e2230 · 06e2230
1 parent e72adda
commit 06e2230
Show file tree

Hide file tree

Showing 41 changed files with 742 additions and 455 deletions.
diff --git a/contracts/Strategy.sol b/contracts/Strategy.sol
@@ -170,20 +170,25 @@ contract Strategy is IStrategy, IStrategyManagement, StrategyTokenFees, Initiali
         _tempRouter = router;
     }
 
-    function updateTimelock(bytes4 functionSelector, uint256 delay) external override {
+    function updateTimelock(bytes4 selector, uint256 delay) external {
         _onlyManager();
-        _startTimelock(this.updateTimelock.selector, abi.encode(functionSelector, delay));
+        _startTimelock(
+          keccak256(abi.encode(this.updateTimelock.selector)), // identifier
+          abi.encode(keccak256(abi.encode(selector)), delay)); // payload
         emit UpdateTimelock(delay, false);
     }
 
-    function finalizeTimelock() external override {
-        if (!_timelockIsReady(this.updateTimelock.selector)) {
-            TimelockData memory td = _timelockData(this.updateTimelock.selector);
+    function finalizeTimelock() external {
+        bytes32 key = keccak256(abi.encode(this.updateTimelock.selector));
+        if (!_timelockIsReady(key)) {
+            TimelockData memory td = _timelockData(key);
             _require(td.delay == 0, uint256(0xb3e5dea2190e00) /* error_macro_for("finalizeTimelock: timelock is not ready.") */);
         }
-        (bytes4 selector, uint256 delay) = abi.decode(_getTimelockValue(this.updateTimelock.selector), (bytes4, uint256));
-        _setTimelock(selector, delay);
-        _resetTimelock(this.updateTimelock.selector);
+        bytes memory value = _getTimelockValue(key);
+        require(value.length != 0, "timelock never started.");
+        (bytes32 identifier, uint256 delay) = abi.decode(value, (bytes32, uint256));
+        _setTimelock(identifier, delay);
+        _resetTimelock(key);
         emit UpdateTimelock(delay, true);
     }
 
@@ -358,15 +363,18 @@ contract Strategy is IStrategy, IStrategyManagement, StrategyTokenFees, Initiali
      */
     function updateTradeData(address item, TradeData memory data) external override {
         _onlyManager();
-        _startTimelock(this.updateTradeData.selector, abi.encode(item, data));
+        _startTimelock(
+          keccak256(abi.encode(this.updateTradeData.selector)), // identifier
+          abi.encode(item, data)); // payload
         emit UpdateTradeData(item, false);
     }
 
     function finalizeUpdateTradeData() external {
-        _require(_timelockIsReady(this.updateTradeData.selector), uint256(0xb3e5dea2190e06) /* error_macro_for("finalizeUpdateTradeData: timelock not ready.") */);
-        (address item, TradeData memory data) = abi.decode(_getTimelockValue(this.updateTradeData.selector), (address, TradeData));
+        bytes32 key = keccak256(abi.encode(this.updateTradeData.selector));
+        _require(_timelockIsReady(key), uint256(0xb3e5dea2190e06) /* error_macro_for("finalizeUpdateTradeData: timelock not ready.") */);
+        (address item, TradeData memory data) = abi.decode(_getTimelockValue(key), (address, TradeData));
         _tradeData[item] = data;
-        _resetTimelock(this.updateTradeData.selector);
+        _resetTimelock(key);
         emit UpdateTradeData(item, true);
     }
 
@@ -564,7 +572,7 @@ contract Strategy is IStrategy, IStrategyManagement, StrategyTokenFees, Initiali
         return exists.doesExist(bytes32(uint256(token)));
     }
 
-    function _timelockData(bytes4 functionSelector) internal override returns(TimelockData storage) {
-        return __timelockData[functionSelector];
+    function _timelockData(bytes32 identifier) internal override returns(TimelockData storage) {
+        return __timelockData[identifier];
     }
 }
diff --git a/contracts/StrategyController.sol b/contracts/StrategyController.sol
diff --git a/contracts/StrategyControllerStorage.sol b/contracts/StrategyControllerStorage.sol
@@ -13,7 +13,11 @@ contract StrategyControllerStorage  is StrategyTypes {
     mapping(address => StrategyState) internal _strategyStates;
     mapping(address => Timelock) internal _timelocks;
     address internal _pool;
+    mapping(bytes32 => TimelockData) internal __timelockData;
+
+    uint256 internal _rebalanceTimelockPeriod; 
+    uint256 internal _rebalanceThresholdScalar; 
 
     // Gap for future storage changes
-    uint256[49] private __gap;
+    uint256[46] private __gap;
 }
diff --git a/contracts/StrategyProxyFactory.sol b/contracts/StrategyProxyFactory.sol
@@ -197,6 +197,10 @@ contract StrategyProxyFactory is IStrategyProxyFactory, StrategyProxyFactoryStor
         emit NewStreamingFee(uint256(fee));
     }
 
+    function updateRebalanceParameters(uint256 rebalanceTimelockPeriod, uint256 rebalanceThresholdScalar) external onlyOwner {
+        IStrategyController(controller).updateRebalanceParameters(rebalanceTimelockPeriod, rebalanceThresholdScalar);
+    }
+
     /*
      * @dev This function is called by StrategyProxyAdmin
      */

diff --git a/contracts/StrategyTokenStorage.sol b/contracts/StrategyTokenStorage.sol
@@ -34,7 +34,7 @@ contract StrategyTokenStorage is StrategyTypes {
     address[] internal _debt;
     mapping(address => int256) internal _percentage;
     mapping(address => TradeData) internal _tradeData;
-    mapping(bytes4 => TimelockData) internal __timelockData;
+    mapping(bytes32 => TimelockData) internal __timelockData;
 
     uint256 internal _managementFee;
     uint256 internal _managementFeeRate;

diff --git a/contracts/helpers/Timelocks.sol b/contracts/helpers/Timelocks.sol
@@ -5,48 +5,50 @@ import "./StrategyTypes.sol";
 
 abstract contract Timelocks is StrategyTypes {
 
-    event TimelockSet(bytes4 selector, uint256 value);
+    event TimelockSet(bytes32 identifier, uint256 value);
     event UpdateTimelock(uint256 delay, bool finalized);
 
 
     bytes constant public UNSET_VALUE = abi.encode(keccak256("Timelocks: unset value."));
 
     // updgradable implementations would benefit from the ability to set new timelocks.
-    function updateTimelock(bytes4 selector, uint256 delay) external virtual;
-    function finalizeTimelock() external virtual;
+    // not mandatory so suppressing "virtual". See EmergencyEstimator and StrategyController
+    // for an example and non-example
+    //function updateTimelock(bytes32 identifier, uint256 delay) external virtual;
+    //function finalizeTimelock() external virtual;
 
 
     // delay value is not validated but is assumed to be sensible 
     // since this function is internal, this way `_timelockIsReady` will not overflow
-    function _setTimelock(bytes4 selector, uint256 delay) internal {
-        TimelockData storage td = _timelockData(selector); 
+    function _setTimelock(bytes32 identifier, uint256 delay) internal {
+        TimelockData storage td = _timelockData(identifier); 
         require(delay <= uint128(-1), "_setTimelock: delay out of range.");
         td.delay = uint128(delay);
         td.value = UNSET_VALUE;
-        emit TimelockSet(selector, delay);
+        emit TimelockSet(identifier, delay);
     }
 
-    function _timelockData(bytes4 functionSelector) internal virtual returns(TimelockData storage);
+    function _timelockData(bytes32 identifier) internal virtual returns(TimelockData storage);
 
-    function _startTimelock(bytes4 selector, bytes memory value) internal {
-        TimelockData storage td = _timelockData(selector); 
+    function _startTimelock(bytes32 identifier, bytes memory value) internal {
+        TimelockData storage td = _timelockData(identifier); 
         td.timestamp = uint128(block.timestamp);
         td.value = value;
     }
 
-    function _timelockIsReady(bytes4 selector) internal returns(bool) {
-        TimelockData memory td = _timelockData(selector); 
+    function _timelockIsReady(bytes32 identifier) internal returns(bool) {
+        TimelockData memory td = _timelockData(identifier); 
         if (td.timestamp == 0) return false;
         if (uint128(block.timestamp) >= td.timestamp + td.delay) return true;
     }
 
     // unchecked, assumes caller has checked `isReady`
-    function _getTimelockValue(bytes4 selector) internal returns(bytes memory) {
-        return _timelockData(selector).value; 
+    function _getTimelockValue(bytes32 identifier) internal returns(bytes memory) {
+        return _timelockData(identifier).value; 
     }
 
-    function _resetTimelock(bytes4 selector) internal {
-        TimelockData storage td = _timelockData(selector); 
+    function _resetTimelock(bytes32 identifier) internal {
+        TimelockData storage td = _timelockData(identifier); 
         td.timestamp = 0;
         td.value = UNSET_VALUE;
     }

diff --git a/contracts/implementations/recovery/StrategyControllerPaused.sol b/contracts/implementations/recovery/StrategyControllerPaused.sol
@@ -272,6 +272,10 @@ contract StrategyControllerPaused is IStrategyController, StrategyControllerStor
         }
     }
 
+    function updateRebalanceParameters(uint256 rebalanceTimelockPeriod, uint256 rebalanceThresholdScalar) external override {
+        revert("StrategyControllerPaused.");
+    }
+
     function oracle() public view override returns (IOracle) {
         return IOracle(_oracle);
     }
@@ -288,6 +292,10 @@ contract StrategyControllerPaused is IStrategyController, StrategyControllerStor
         return _pool;
     }
 
+    function rebalanceThresholdScalar() external view override returns(uint256) {
+        return _rebalanceThresholdScalar;
+    }
+
     // Internal Strategy Functions
     /**
      * @notice Deposit eth or weth into strategy

diff --git a/contracts/interfaces/IStrategyController.sol b/contracts/interfaces/IStrategyController.sol
@@ -68,6 +68,8 @@ interface IStrategyController is StrategyTypes {
         uint256 newValue
     ) external;
 
+    function updateRebalanceParameters(uint256 rebalanceTimelockPeriod, uint256 rebalanceThresholdScalar) external;
+
     function finalizeValue(IStrategy strategy) external;
 
     function openStrategy(IStrategy strategy) external;
@@ -90,4 +92,6 @@ interface IStrategyController is StrategyTypes {
     function weth() external view returns (address);
 
     function pool() external view returns (address);
+
+    function rebalanceThresholdScalar() external view returns(uint256);
 }
diff --git a/contracts/libraries/ControllerLibrary.sol b/contracts/libraries/ControllerLibrary.sol
@@ -153,18 +153,19 @@ library ControllerLibrary {
         IOracle oracle,
         address weth,
         uint256 rebalanceSlippage,
+        uint256 rebalanceThresholdScalar,
         bytes memory data
     ) public {
         _onlyApproved(address(router));
         strategy.settleSynths();
-        (bool balancedBefore, uint256 totalBefore, int256[] memory estimates) = verifyBalance(strategy, oracle);
+        (bool balancedBefore, uint256 totalBefore, int256[] memory estimates) = verifyBalance(strategy, oracle, rebalanceThresholdScalar);
         require(!balancedBefore, "Balanced");
         if (router.category() != IStrategyRouter.RouterCategory.GENERIC)
             data = abi.encode(totalBefore, estimates);
         // Rebalance
         _useRouter(strategy, router, router.rebalance, weth, data);
         // Recheck total
-        (bool balancedAfter, uint256 totalAfter, ) = verifyBalance(strategy, oracle);
+        (bool balancedAfter, uint256 totalAfter, ) = verifyBalance(strategy, oracle, 0);
         require(balancedAfter, "Not balanced");
         _checkSlippage(totalAfter, totalBefore, rebalanceSlippage);
         strategy.updateTokenValue(totalAfter, strategy.totalSupply());
@@ -364,10 +365,17 @@ library ControllerLibrary {
      *         whether the strategy is balanced. Necessary to confirm the balance
      *         before and after a rebalance to ensure nothing fishy happened
      */
-    function verifyBalance(IStrategy strategy, IOracle oracle) public view returns (bool, uint256, int256[] memory) {
-        (uint256 total, int256[] memory estimates) =
-            oracle.estimateStrategy(IStrategy(strategy));
+    function verifyBalance(IStrategy strategy, IOracle oracle, uint256 rebalanceThresholdScalar) public view returns (bool, uint256, int256[] memory) {
         uint256 threshold = strategy.rebalanceThreshold();
+        if (rebalanceThresholdScalar > 0) { // wider threshold
+            threshold = threshold.mul(rebalanceThresholdScalar) / uint256(DIVISOR);
+        }
+        return _verifyBalance(strategy, oracle, threshold);
+    }
+
+    function _verifyBalance(IStrategy strategy, IOracle oracle, uint256 threshold) private view returns (bool, uint256, int256[] memory) {
+        (uint256 total, int256[] memory estimates) =
+            oracle.estimateStrategy(strategy);
 
         bool balanced = true;
         address[] memory strategyItems = strategy.items();

diff --git a/contracts/oracles/estimators/EmergencyEstimator.sol b/contracts/oracles/estimators/EmergencyEstimator.sol
@@ -12,27 +12,32 @@ contract EmergencyEstimator is IEstimator, Ownable, Timelocks {
     using SignedSafeMath for int256;
 
     mapping(address => int256) public estimates;
-    mapping(bytes4 => TimelockData) private __timelockData;
+    mapping(bytes32 => TimelockData) private __timelockData;
 
     event EstimateSet(address token, int256 amount, bool finalized);
 
     constructor() public {
-        _setTimelock(this.updateEstimate.selector, 5 minutes);
+        _setTimelock(
+          keccak256(abi.encode(this.updateEstimate.selector)), // identifier
+          5 minutes);
     }
 
-    function updateTimelock(bytes4 functionSelector, uint256 delay) external override onlyOwner {
-        _startTimelock(this.updateTimelock.selector, abi.encode(functionSelector, delay));
+    function updateTimelock(bytes32 identifier, uint256 delay) external onlyOwner {
+        _startTimelock(
+          keccak256(abi.encode(this.updateTimelock.selector)), // identifier
+          abi.encode(identifier, delay)); // payload
         emit UpdateTimelock(delay, false);
     }
 
-    function finalizeTimelock() external override {
-        if (!_timelockIsReady(this.updateTimelock.selector)) {
-            TimelockData memory td = _timelockData(this.updateTimelock.selector);
+    function finalizeTimelock() external {
+        bytes32 key = keccak256(abi.encode(this.updateTimelock.selector));
+        if (!_timelockIsReady(key)) {
+            TimelockData memory td = _timelockData(key);
             require(td.delay == 0, "finalizeTimelock: timelock is not ready.");
         }
-        (bytes4 selector, uint256 delay) = abi.decode(_getTimelockValue(this.updateTimelock.selector), (bytes4, uint256));
-        _setTimelock(selector, delay);
-        _resetTimelock(this.updateTimelock.selector);
+        (bytes32 identifier, uint256 delay) = abi.decode(_getTimelockValue(key), (bytes4, uint256));
+        _setTimelock(identifier, delay);
+        _resetTimelock(key);
         emit UpdateTimelock(delay, true);
     }
 
@@ -41,14 +46,16 @@ contract EmergencyEstimator is IEstimator, Ownable, Timelocks {
     }
 
     function updateEstimate(address token, int256 amount) external onlyOwner {
-        _startTimelock(this.updateEstimate.selector, abi.encode(token, amount));
+        _startTimelock(
+          keccak256(abi.encode(this.updateEstimate.selector)), // identifier
+          abi.encode(token, amount)); // payload
         emit EstimateSet(token, amount, false);
     }
 
     function finalizeSetEstimate() external {
-        require(_timelockIsReady(this.updateEstimate.selector), "finalizeSetEstimate: timelock not ready.");
-        (address token, int256 amount) = abi.decode(_getTimelockValue(this.updateEstimate.selector), (address, int256));
-        _resetTimelock(this.updateEstimate.selector);
+        require(_timelockIsReady(keccak256(abi.encode(this.updateEstimate.selector))), "finalizeSetEstimate: timelock not ready.");
+        (address token, int256 amount) = abi.decode(_getTimelockValue(keccak256(abi.encode(this.updateEstimate.selector))), (address, int256));
+        _resetTimelock(keccak256(abi.encode(this.updateEstimate.selector)));
         estimates[token] = amount;
         emit EstimateSet(token, amount, true);
     }
@@ -62,7 +69,7 @@ contract EmergencyEstimator is IEstimator, Ownable, Timelocks {
         return int256(balance).mul(estimates[token]).div(int256(10**uint256(IERC20NonStandard(token).decimals())));
     }
 
-    function _timelockData(bytes4 functionSelector) internal override returns(TimelockData storage) {
-        return __timelockData[functionSelector];
+    function _timelockData(bytes32 identifier) internal override returns(TimelockData storage) {
+        return __timelockData[identifier];
     }
 }