From 41238dca0b87d481157a0aecd1e685f5904b51ef Mon Sep 17 00:00:00 2001 From: BkChoy Date: Fri, 12 Apr 2024 14:09:17 -0400 Subject: [PATCH] metis staking fixes --- contracts/core/StakingPool.sol | 6 +- contracts/core/ccip/WrappedTokenBridge.sol | 16 ++++ contracts/core/ccip/base/CCIPReceiver.sol | 2 +- .../core/ccip/base/SDLPoolCCIPController.sol | 16 ++++ contracts/linkStaking/OperatorVCS.sol | 14 --- .../base/VaultControllerStrategy.sol | 63 +++++++------ contracts/metisStaking/SequencerVCS.sol | 90 ++++++++++++------- contracts/metisStaking/SequencerVault.sol | 32 +++---- .../ccip/SequencerRewardsCCIPSender.sol | 8 +- .../interfaces/IMetisLockingPool.sol | 2 +- .../metisStaking/interfaces/ISequencerVCS.sol | 2 + .../interfaces/ISequencerVault.sol | 3 +- 12 files changed, 151 insertions(+), 103 deletions(-) diff --git a/contracts/core/StakingPool.sol b/contracts/core/StakingPool.sol index bd5273ca..cf2ea4ea 100644 --- a/contracts/core/StakingPool.sol +++ b/contracts/core/StakingPool.sol @@ -48,7 +48,7 @@ contract StakingPool is StakingRewardsPool { for (uint256 i = 0; i < _fees.length; i++) { fees.push(_fees[i]); } - require(_totalFeesBasisPoints() <= 5000, "Total fees must be <= 50%"); + require(_totalFeesBasisPoints() <= 4000, "Total fees must be <= 40%"); } modifier onlyPriorityPool() { @@ -282,7 +282,7 @@ contract StakingPool is StakingRewardsPool { **/ function addFee(address _receiver, uint256 _feeBasisPoints) external onlyOwner { fees.push(Fee(_receiver, _feeBasisPoints)); - require(_totalFeesBasisPoints() <= 5000, "Total fees must be <= 50%"); + require(_totalFeesBasisPoints() <= 4000, "Total fees must be <= 40%"); } /** @@ -306,7 +306,7 @@ contract StakingPool is StakingRewardsPool { fees[_index].basisPoints = _feeBasisPoints; } - require(_totalFeesBasisPoints() <= 5000, "Total fees must be <= 50%"); + require(_totalFeesBasisPoints() <= 4000, "Total fees must be <= 40%"); } /** diff --git a/contracts/core/ccip/WrappedTokenBridge.sol b/contracts/core/ccip/WrappedTokenBridge.sol index a150b2e2..8ef0fc73 100644 --- a/contracts/core/ccip/WrappedTokenBridge.sol +++ b/contracts/core/ccip/WrappedTokenBridge.sol @@ -154,6 +154,22 @@ contract WrappedTokenBridge is CCIPReceiver { } } + /** + * @notice Sets the CCIP router + * @param _router router address + **/ + function setRouter(address _router) external override onlyOwner { + if (_router == address(0)) revert InvalidRouter(address(0)); + + address curRouter = getRouter(); + linkToken.approve(curRouter, 0); + wrappedToken.approve(curRouter, 0); + + linkToken.approve(_router, type(uint256).max); + wrappedToken.approve(_router, type(uint256).max); + i_router = _router; + } + /** * @notice Wraps and transfers tokens to a destination chain * @param _destinationChainSelector id of destination chain diff --git a/contracts/core/ccip/base/CCIPReceiver.sol b/contracts/core/ccip/base/CCIPReceiver.sol index f84e3482..2b3722fb 100644 --- a/contracts/core/ccip/base/CCIPReceiver.sol +++ b/contracts/core/ccip/base/CCIPReceiver.sol @@ -51,7 +51,7 @@ abstract contract CCIPReceiver is IAny2EVMMessageReceiver, IERC165, Ownable { /// @notice Sets the router /// @param _router router address - function setRouter(address _router) external onlyOwner { + function setRouter(address _router) external virtual onlyOwner { if (_router == address(0)) revert InvalidRouter(address(0)); i_router = _router; } diff --git a/contracts/core/ccip/base/SDLPoolCCIPController.sol b/contracts/core/ccip/base/SDLPoolCCIPController.sol index 2b71767f..f7e8ec83 100644 --- a/contracts/core/ccip/base/SDLPoolCCIPController.sol +++ b/contracts/core/ccip/base/SDLPoolCCIPController.sol @@ -155,6 +155,22 @@ abstract contract SDLPoolCCIPController is CCIPReceiver { reSDLTokenBridge = _reSDLTokenBridge; } + /** + * @notice Sets the CCIP router + * @param _router router address + **/ + function setRouter(address _router) external override onlyOwner { + if (_router == address(0)) revert InvalidRouter(address(0)); + + address curRouter = getRouter(); + linkToken.approve(curRouter, 0); + sdlToken.approve(curRouter, 0); + + linkToken.approve(_router, type(uint256).max); + sdlToken.approve(_router, type(uint256).max); + i_router = _router; + } + /** * @notice Verifies the sender of a CCIP message is whitelisted * @param _message CCIP message diff --git a/contracts/linkStaking/OperatorVCS.sol b/contracts/linkStaking/OperatorVCS.sol index e30b4f95..731f7066 100644 --- a/contracts/linkStaking/OperatorVCS.sol +++ b/contracts/linkStaking/OperatorVCS.sol @@ -287,18 +287,4 @@ contract OperatorVCS is VaultControllerStrategy { function togglePreRelease() external onlyOwner { preRelease = !preRelease; } - - /** - * @notice updates rewards for all strategies controlled by the staking pool - * @dev called before operatorRewardPercentage is changed to - * credit any past rewards at the old rate - */ - function _updateStrategyRewards() private { - address[] memory strategies = stakingPool.getStrategies(); - uint256[] memory strategyIdxs = new uint256[](strategies.length); - for (uint256 i = 0; i < strategies.length; ++i) { - strategyIdxs[i] = i; - } - stakingPool.updateStrategyRewards(strategyIdxs, ""); - } } diff --git a/contracts/linkStaking/base/VaultControllerStrategy.sol b/contracts/linkStaking/base/VaultControllerStrategy.sol index c2284038..4bf213bc 100644 --- a/contracts/linkStaking/base/VaultControllerStrategy.sol +++ b/contracts/linkStaking/base/VaultControllerStrategy.sol @@ -37,10 +37,11 @@ abstract contract VaultControllerStrategy is Strategy { uint256[9] private __gap; - event UpgradedVaults(uint256 startIndex, uint256 numVaults, bytes data); + event UpgradedVaults(uint256[] vaults); event SetMaxDepositSizeBP(uint256 maxDepositSizeBP); event SetVaultImplementation(address vaultImplementation); + error FeesTooLarge(); error InvalidBasisPoints(); /** @@ -71,7 +72,7 @@ abstract contract VaultControllerStrategy is Strategy { for (uint256 i = 0; i < _fees.length; ++i) { fees.push(_fees[i]); } - require(_totalFeesBasisPoints() <= 5000, "Total fees must be <= 50%"); + if (_totalFeesBasisPoints() > 3000) revert FeesTooLarge(); if (_maxDepositSizeBP > 10000) revert InvalidBasisPoints(); maxDepositSizeBP = _maxDepositSizeBP; @@ -234,21 +235,19 @@ abstract contract VaultControllerStrategy is Strategy { } /** - * @notice upgrades vaults to a new implementation contract - * @dev reverts if sender is not owner - * @param _startIndex index of first vault to upgrade - * @param _numVaults number of vaults to upgrade starting at _startIndex - * @param _data optional encoded function call to be executed after upgrade + * @notice Upgrades vaults to a new implementation contract + * @param _vaults list of vault indexes to upgrade + * @param _data list of encoded function calls to be executed for each vault after upgrade */ - function upgradeVaults( - uint256 _startIndex, - uint256 _numVaults, - bytes memory _data - ) external onlyOwner { - for (uint256 i = _startIndex; i < _startIndex + _numVaults; ++i) { - _upgradeVault(i, _data); + function upgradeVaults(uint256[] calldata _vaults, bytes[] memory _data) external onlyOwner { + for (uint256 i = 0; i < _vaults.length; ++i) { + if (_data[i].length == 0) { + vaults[_vaults[i]].upgradeTo(vaultImplementation); + } else { + vaults[_vaults[i]].upgradeToAndCall(vaultImplementation, _data[i]); + } } - emit UpgradedVaults(_startIndex, _numVaults, _data); + emit UpgradedVaults(_vaults); } /** @@ -261,22 +260,21 @@ abstract contract VaultControllerStrategy is Strategy { /** * @notice adds a new fee - * @dev - * - reverts if sender is not owner - * - reverts if total fees exceed 50% + * @dev stakingPool.updateStrategyRewards is called to credit all past fees at + * the old rate before the percentage changes * @param _receiver receiver of fee * @param _feeBasisPoints fee in basis points **/ function addFee(address _receiver, uint256 _feeBasisPoints) external onlyOwner { + _updateStrategyRewards(); fees.push(Fee(_receiver, _feeBasisPoints)); - require(_totalFeesBasisPoints() <= 5000, "Total fees must be <= 50%"); + if (_totalFeesBasisPoints() > 3000) revert FeesTooLarge(); } /** * @notice updates an existing fee - * @dev - * - reverts if sender is not owner - * - reverts if total fees exceed 50% + * @dev stakingPool.updateStrategyRewards is called to credit all past fees at + * the old rate before the percentage changes * @param _index index of fee * @param _receiver receiver of fee * @param _feeBasisPoints fee in basis points @@ -286,7 +284,7 @@ abstract contract VaultControllerStrategy is Strategy { address _receiver, uint256 _feeBasisPoints ) external onlyOwner { - require(_index < fees.length, "Fee does not exist"); + _updateStrategyRewards(); if (_feeBasisPoints == 0) { fees[_index] = fees[fees.length - 1]; @@ -296,7 +294,7 @@ abstract contract VaultControllerStrategy is Strategy { fees[_index].basisPoints = _feeBasisPoints; } - require(_totalFeesBasisPoints() <= 5000, "Total fees must be <= 50%"); + if (_totalFeesBasisPoints() > 3000) revert FeesTooLarge(); } /** @@ -372,17 +370,16 @@ abstract contract VaultControllerStrategy is Strategy { } /** - * @notice upgrades a vault controlled by this strategy - * @param _vaultIdx index of vault to upgrade - * @param _data optional encoded function call to be executed after upgrade + * @notice Updates rewards for all strategies controlled by the staking pool + * @dev called before fees are changed to credit any past rewards at the old rate */ - function _upgradeVault(uint256 _vaultIdx, bytes memory _data) internal { - IVault vault = vaults[_vaultIdx]; - if (_data.length == 0) { - vault.upgradeTo(vaultImplementation); - } else { - vault.upgradeToAndCall(vaultImplementation, _data); + function _updateStrategyRewards() internal { + address[] memory strategies = stakingPool.getStrategies(); + uint256[] memory strategyIdxs = new uint256[](strategies.length); + for (uint256 i = 0; i < strategies.length; ++i) { + strategyIdxs[i] = i; } + stakingPool.updateStrategyRewards(strategyIdxs, ""); } /** diff --git a/contracts/metisStaking/SequencerVCS.sol b/contracts/metisStaking/SequencerVCS.sol index a15dc5af..85c3dd29 100644 --- a/contracts/metisStaking/SequencerVCS.sol +++ b/contracts/metisStaking/SequencerVCS.sol @@ -41,11 +41,10 @@ contract SequencerVCS is Strategy { event VaultAdded(address signer); event SetOperatorRewardPercentage(uint256 operatorRewardPercentage); event SetVaultImplementation(address vaultImplementation); - event UpgradedVaults(uint256 startIndex, uint256 numVaults, bytes data); + event UpgradedVaults(uint256[] vaults); error FeesTooLarge(); error AddressNotContract(); - error InvalidPercentage(); error SenderNotAuthorized(); error ZeroAddress(); @@ -89,9 +88,9 @@ contract SequencerVCS is Strategy { for (uint256 i = 0; i < _fees.length; ++i) { fees.push(_fees[i]); } - if (_totalFeesBasisPoints() > 5000) revert FeesTooLarge(); + if (_totalFeesBasisPoints() > 3000) revert FeesTooLarge(); - if (_operatorRewardPercentage > 10000) revert InvalidPercentage(); + if (_operatorRewardPercentage > 3000) revert FeesTooLarge(); operatorRewardPercentage = _operatorRewardPercentage; } @@ -231,8 +230,8 @@ contract SequencerVCS is Strategy { /** * @notice Updates deposit accounting and calculates fees on newly earned rewards - * @param _data encoded minRewards (uint256) - min amount of rewards required to claim (set 0 to skip reward claiming) - * and l2Gas (uint32) - gas limit for reward bridging + * @param _data encoded minRewards (uint256) - min amount of rewards required to relock/claim (set 0 to skip reward claiming), + * l2Gas (uint32) - per vault gas limit for reward bridging, and l2Fee (uint256) - per vault fee to pay for reward bridging * @return depositChange change in deposits since last update * @return receivers list of fee receivers * @return amounts list of fee amounts @@ -246,7 +245,9 @@ contract SequencerVCS is Strategy { uint256[] memory amounts ) { - (uint256 minRewards, uint32 l2Gas) = _data.length == 0 ? (0, 0) : abi.decode(_data, (uint256, uint32)); + (uint256 minRewards, uint32 l2Gas, uint256 l2Fee) = _data.length == 0 + ? (0, 0, 0) + : abi.decode(_data, (uint256, uint32, uint256)); uint256 vaultDeposits; uint256 operatorRewards; @@ -254,7 +255,10 @@ contract SequencerVCS is Strategy { uint256 vaultCount = vaults.length; for (uint256 i = 0; i < vaultCount; ++i) { - (uint256 deposits, uint256 opRewards, uint256 claimed) = vaults[i].updateDeposits(minRewards, l2Gas); + (uint256 deposits, uint256 opRewards, uint256 claimed) = vaults[i].updateDeposits{value: l2Fee}( + minRewards, + l2Gas + ); vaultDeposits += deposits; operatorRewards += opRewards; claimedRewards += claimed; @@ -317,7 +321,7 @@ contract SequencerVCS is Strategy { * @return maximum deposits */ function getMaxDeposits() public view override returns (uint256) { - return vaults.length * lockingInfo.maxLock(); + return vaults.length * getVaultDepositMax(); } /** @@ -329,13 +333,32 @@ contract SequencerVCS is Strategy { } /** - * @notice Relocks sequencer rewards for a list of vaults - * @param _vaults list of vaults + * @notice Returns the minimum that can be deposited into a vault + * @return minimum vault deposit */ - function relockRewards(uint256[] calldata _vaults) external { - for (uint256 i = 0; i < _vaults.length; ++i) { - vaults[_vaults[i]].relockRewards(); - } + function getVaultDepositMin() public view returns (uint256) { + return lockingInfo.minLock(); + } + + /** + * @notice Returns the maximum that can be deposited into a vault + * @return maximum vault deposit + */ + function getVaultDepositMax() public view returns (uint256) { + return lockingInfo.maxLock(); + } + + /** + * @notice Receives ETH transfers + */ + receive() external payable {} + + /** + * @notice Withdraws ETH sitting in the contract + * @param _amount amount to withdraw + */ + function withdrawETH(uint256 _amount) external onlyOwner { + payable(msg.sender).transfer(_amount); } /** @@ -370,23 +393,18 @@ contract SequencerVCS is Strategy { /** * @notice Upgrades vaults to a new implementation contract - * @param _startIndex index of first vault to upgrade - * @param _numVaults number of vaults to upgrade starting at _startIndex - * @param _data optional encoded function call to be executed after upgrade + * @param _vaults list of vault indexes to upgrade + * @param _data list of encoded function calls to be executed for each vault after upgrade */ - function upgradeVaults( - uint256 _startIndex, - uint256 _numVaults, - bytes memory _data - ) external onlyOwner { - for (uint256 i = _startIndex; i < _startIndex + _numVaults; ++i) { - if (_data.length == 0) { - vaults[i].upgradeTo(vaultImplementation); + function upgradeVaults(uint256[] calldata _vaults, bytes[] memory _data) external onlyOwner { + for (uint256 i = 0; i < _vaults.length; ++i) { + if (_data[i].length == 0) { + vaults[_vaults[i]].upgradeTo(vaultImplementation); } else { - vaults[i].upgradeToAndCall(vaultImplementation, _data); + vaults[_vaults[i]].upgradeToAndCall(vaultImplementation, _data[i]); } } - emit UpgradedVaults(_startIndex, _numVaults, _data); + emit UpgradedVaults(_vaults); } /** @@ -399,16 +417,21 @@ contract SequencerVCS is Strategy { /** * @notice Adds a new fee + * @dev stakingPool.updateStrategyRewards is called to credit all past fees at + * the old rate before the percentage changes * @param _receiver receiver of fee * @param _feeBasisPoints fee in basis points **/ function addFee(address _receiver, uint256 _feeBasisPoints) external onlyOwner { + _updateStrategyRewards(); fees.push(Fee(_receiver, _feeBasisPoints)); - if (_totalFeesBasisPoints() > 5000) revert FeesTooLarge(); + if (_totalFeesBasisPoints() > 3000) revert FeesTooLarge(); } /** * @notice Updates an existing fee + * @dev stakingPool.updateStrategyRewards is called to credit all past fees at + * the old rate before the percentage changes * @param _index index of fee * @param _receiver receiver of fee * @param _feeBasisPoints fee in basis points @@ -418,6 +441,8 @@ contract SequencerVCS is Strategy { address _receiver, uint256 _feeBasisPoints ) external onlyOwner { + _updateStrategyRewards(); + if (_feeBasisPoints == 0) { fees[_index] = fees[fees.length - 1]; fees.pop(); @@ -426,7 +451,7 @@ contract SequencerVCS is Strategy { fees[_index].basisPoints = _feeBasisPoints; } - if (_totalFeesBasisPoints() > 5000) revert FeesTooLarge(); + if (_totalFeesBasisPoints() > 3000) revert FeesTooLarge(); } /** @@ -436,7 +461,7 @@ contract SequencerVCS is Strategy { * @param _operatorRewardPercentage basis point amount */ function setOperatorRewardPercentage(uint256 _operatorRewardPercentage) public onlyOwner { - if (_operatorRewardPercentage > 10000) revert InvalidPercentage(); + if (_operatorRewardPercentage > 3000) revert FeesTooLarge(); _updateStrategyRewards(); @@ -473,8 +498,7 @@ contract SequencerVCS is Strategy { /** * @notice Updates rewards for all strategies controlled by the staking pool - * @dev called before operatorRewardPercentage is changed to - * credit any past rewards at the old rate + * @dev called before fees are changed to credit any past rewards at the old rate */ function _updateStrategyRewards() private { address[] memory strategies = stakingPool.getStrategies(); diff --git a/contracts/metisStaking/SequencerVault.sol b/contracts/metisStaking/SequencerVault.sol index 878c28c3..04ebf007 100644 --- a/contracts/metisStaking/SequencerVault.sol +++ b/contracts/metisStaking/SequencerVault.sol @@ -35,7 +35,7 @@ contract SequencerVault is Initializable, UUPSUpgradeable, OwnableUpgradeable { error SenderNotAuthorized(); error ZeroAddress(); - error NoRewards(); + error SequencerNotInitialized(); /// @custom:oz-upgrades-unsafe-allow constructor constructor() { @@ -158,14 +158,15 @@ contract SequencerVault is Initializable, UUPSUpgradeable, OwnableUpgradeable { /** * @notice Updates the deposit and reward accounting for this vault * @dev will only pay out rewards if the vault is net positive when accounting for lost deposits - * @param _minRewards min amount of rewards to claim (set 0 to skip reward claiming) - * @param _l2Gas bridge reward to L2 gasLimit + * @param _minRewards min amount of rewards to relock/claim (set 0 to skip reward claiming) + * @param _l2Gas L2 gasLimit for bridging rewards * @return the current total deposits in this vault * @return the operator rewards earned by this vault since the last update * @return the rewards that were claimed in this update */ function updateDeposits(uint256 _minRewards, uint32 _l2Gas) external + payable onlyVaultController returns ( uint256, @@ -187,22 +188,21 @@ contract SequencerVault is Initializable, UUPSUpgradeable, OwnableUpgradeable { uint256 claimedRewards; if (_minRewards != 0 && rewards >= _minRewards) { - lockingPool.withdrawRewards(seqId, _l2Gas); - trackedTotalDeposits -= SafeCastUpgradeable.toUint128(rewards); - totalDeposits -= rewards; - claimedRewards = rewards; + if (principal + rewards <= vaultController.getVaultDepositMax()) { + lockingPool.relock(seqId, 0, true); + } else { + lockingPool.withdrawRewards{value: msg.value}(seqId, _l2Gas); + trackedTotalDeposits -= SafeCastUpgradeable.toUint128(rewards); + totalDeposits -= rewards; + claimedRewards = rewards; + } } - return (totalDeposits, opRewards, claimedRewards); - } + if (address(this).balance != 0) { + payable(msg.sender).transfer(address(this).balance); + } - /** - * @notice Relocks sequencer rewards - * @dev will revert if there is insufficient space in sequencer to lock rewards - */ - function relockRewards() external { - if (seqId == 0 || getRewards() == 0) revert NoRewards(); - lockingPool.relock(seqId, 0, true); + return (totalDeposits, opRewards, claimedRewards); } /** diff --git a/contracts/metisStaking/ccip/SequencerRewardsCCIPSender.sol b/contracts/metisStaking/ccip/SequencerRewardsCCIPSender.sol index f48b6513..9346a427 100644 --- a/contracts/metisStaking/ccip/SequencerRewardsCCIPSender.sol +++ b/contracts/metisStaking/ccip/SequencerRewardsCCIPSender.sol @@ -131,10 +131,16 @@ contract SequencerRewardsCCIPSender is UUPSUpgradeable, OwnableUpgradeable { /** * @notice Sets the CCIP router - * @param _router addres of router + * @param _router router address **/ function setRouter(address _router) external onlyOwner { if (_router == address(0)) revert ZeroAddress(); + + linkToken.approve(address(router), 0); + metisToken.approve(address(router), 0); + + linkToken.approve(_router, type(uint256).max); + metisToken.approve(_router, type(uint256).max); router = IRouterClient(_router); } diff --git a/contracts/metisStaking/interfaces/IMetisLockingPool.sol b/contracts/metisStaking/interfaces/IMetisLockingPool.sol index 4355d037..3336fd9a 100644 --- a/contracts/metisStaking/interfaces/IMetisLockingPool.sol +++ b/contracts/metisStaking/interfaces/IMetisLockingPool.sol @@ -43,5 +43,5 @@ interface IMetisLockingPool { bool _lockReward ) external; - function withdrawRewards(uint256 _seqId, uint32 _l2Gas) external; + function withdrawRewards(uint256 _seqId, uint32 _l2Gas) external payable; } diff --git a/contracts/metisStaking/interfaces/ISequencerVCS.sol b/contracts/metisStaking/interfaces/ISequencerVCS.sol index 27884d7a..d47274a5 100644 --- a/contracts/metisStaking/interfaces/ISequencerVCS.sol +++ b/contracts/metisStaking/interfaces/ISequencerVCS.sol @@ -9,4 +9,6 @@ interface ISequencerVCS { function withdrawOperatorRewards(address _rewardsReceiver, uint256 _amount) external returns (uint256); function handleIncomingL2Rewards(uint256 _amount) external; + + function getVaultDepositMax() external view returns (uint256); } diff --git a/contracts/metisStaking/interfaces/ISequencerVault.sol b/contracts/metisStaking/interfaces/ISequencerVault.sol index acb2d50b..79ed4767 100644 --- a/contracts/metisStaking/interfaces/ISequencerVault.sol +++ b/contracts/metisStaking/interfaces/ISequencerVault.sol @@ -8,6 +8,7 @@ interface ISequencerVault { function updateDeposits(uint256 _minRewards, uint32 _l2Gas) external + payable returns ( uint256, uint256, @@ -16,7 +17,7 @@ interface ISequencerVault { function deposit(uint256 _amount) external; - function relockRewards() external; + function relockRewards() external returns (uint256); function upgradeToAndCall(address _newImplementation, bytes memory _data) external;