Merge pull request #732 from lidofinance/fix/shapella-upgrade-from-rc1-to-rc2

Fix: shapella upgrade from rc1 to rc2

Author: TheDZhon

contracts/0.4.24/Lido.sol

@@ -122,7 +122,7 @@ interface IWithdrawalQueue {
         view
         returns (uint256 ethToLock, uint256 sharesToBurn);
 
-    function finalize(uint256[] _batches, uint256 _maxShareRate) external payable;
+    function finalize(uint256 _lastIdToFinalize, uint256 _maxShareRate) external payable;
 
     function isPaused() external view returns (bool);
 
@@ -282,12 +282,12 @@ contract Lido is Versioned, StETHPermit, AragonApp {
         LIDO_LOCATOR_POSITION.setStorageAddress(_lidoLocator);
         _initializeEIP712StETH(_eip712StETH);
 
-        // set unlimited allowance for burner from withdrawal queue
+        // set infinite allowance for burner from withdrawal queue
         // to burn finalized requests' shares
         _approve(
             ILidoLocator(_lidoLocator).withdrawalQueue(),
             ILidoLocator(_lidoLocator).burner(),
-           ~uint256(0)
+            INFINITE_ALLOWANCE
         );
 
         emit LidoLocatorSet(_lidoLocator);
@@ -851,7 +851,7 @@ contract Lido is Versioned, StETHPermit, AragonApp {
         if (_etherToLockOnWithdrawalQueue > 0) {
             IWithdrawalQueue withdrawalQueue = IWithdrawalQueue(_contracts.withdrawalQueue);
             withdrawalQueue.finalize.value(_etherToLockOnWithdrawalQueue)(
-                _withdrawalFinalizationBatches,
+                _withdrawalFinalizationBatches[_withdrawalFinalizationBatches.length - 1],
                 _simulatedShareRate
             );
         }

contracts/0.4.24/StETH.sol

@@ -51,6 +51,7 @@ contract StETH is IERC20, Pausable {
     using UnstructuredStorage for bytes32;
 
     address constant internal INITIAL_TOKEN_HOLDER = 0xdead;
+    uint256 constant internal INFINITE_ALLOWANCE = ~uint256(0);
 
     /**
      * @dev StETH balances are dynamic and are calculated based on the accounts' shares
@@ -234,11 +235,8 @@ contract StETH is IERC20, Pausable {
      * @dev The `_amount` argument is the amount of tokens, not shares.
      */
     function transferFrom(address _sender, address _recipient, uint256 _amount) external returns (bool) {
-        uint256 currentAllowance = allowances[_sender][msg.sender];
-        require(currentAllowance >= _amount, "ALLOWANCE_EXCEEDED");
-
+        _spendAllowance(_sender, msg.sender, _amount);
         _transfer(_sender, _recipient, _amount);
-        _approve(_sender, msg.sender, currentAllowance.sub(_amount));
         return true;
     }
 
@@ -247,7 +245,7 @@ contract StETH is IERC20, Pausable {
      *
      * This is an alternative to `approve` that can be used as a mitigation for
      * problems described in:
-     * https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC20/IERC20.sol#L42
+     * https://github.com/OpenZeppelin/openzeppelin-contracts/blob/b709eae01d1da91902d06ace340df6b324e6f049/contracts/token/ERC20/IERC20.sol#L57
      * Emits an `Approval` event indicating the updated allowance.
      *
      * Requirements:
@@ -264,7 +262,7 @@ contract StETH is IERC20, Pausable {
      *
      * This is an alternative to `approve` that can be used as a mitigation for
      * problems described in:
-     * https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC20/IERC20.sol#L42
+     * https://github.com/OpenZeppelin/openzeppelin-contracts/blob/b709eae01d1da91902d06ace340df6b324e6f049/contracts/token/ERC20/IERC20.sol#L57
      * Emits an `Approval` event indicating the updated allowance.
      *
      * Requirements:
@@ -355,12 +353,9 @@ contract StETH is IERC20, Pausable {
     function transferSharesFrom(
         address _sender, address _recipient, uint256 _sharesAmount
     ) external returns (uint256) {
-        uint256 currentAllowance = allowances[_sender][msg.sender];
         uint256 tokensAmount = getPooledEthByShares(_sharesAmount);
-        require(currentAllowance >= tokensAmount, "ALLOWANCE_EXCEEDED");
-
+        _spendAllowance(_sender, msg.sender, tokensAmount);
         _transferShares(_sender, _recipient, _sharesAmount);
-        _approve(_sender, msg.sender, currentAllowance.sub(tokensAmount));
         _emitTransferEvents(_sender, _recipient, tokensAmount, _sharesAmount);
         return tokensAmount;
     }
@@ -403,6 +398,22 @@ contract StETH is IERC20, Pausable {
         emit Approval(_owner, _spender, _amount);
     }
 
+    /**
+     * @dev Updates `owner` s allowance for `spender` based on spent `amount`.
+     *
+     * Does not update the allowance amount in case of infinite allowance.
+     * Revert if not enough allowance is available.
+     *
+     * Might emit an {Approval} event.
+     */
+    function _spendAllowance(address _owner, address _spender, uint256 _amount) internal {
+        uint256 currentAllowance = allowances[_owner][_spender];
+        if (currentAllowance != INFINITE_ALLOWANCE) {
+            require(currentAllowance >= _amount, "ALLOWANCE_EXCEEDED");
+            _approve(_owner, _spender, currentAllowance - _amount);
+        }
+    }
+
     /**
      * @return the total amount of shares in existence.
      */

contracts/0.4.24/nos/NodeOperatorsRegistry.sol

@@ -85,7 +85,7 @@ contract NodeOperatorsRegistry is AragonApp, Versioned {
     // SigningKeysStats
     /// @dev Operator's max validator keys count approved for deposit by the DAO
     uint8 internal constant TOTAL_VETTED_KEYS_COUNT_OFFSET = 0;
-    /// @dev Number of keys in the EXITED state for this operator for all time
+    /// @dev Number of keys in the EXITED state of this operator for all time
     uint8 internal constant TOTAL_EXITED_KEYS_COUNT_OFFSET = 1;
     /// @dev Total number of keys of this operator for all time
     uint8 internal constant TOTAL_KEYS_COUNT_OFFSET = 2;
@@ -116,11 +116,11 @@ contract NodeOperatorsRegistry is AragonApp, Versioned {
 
     // Summary SigningKeysStats
     uint8 internal constant SUMMARY_MAX_VALIDATORS_COUNT_OFFSET = 0;
-    /// @dev Number of keys in the EXITED state for this operator for all time
+    /// @dev Number of keys of all operators which were in the EXITED state for all time
     uint8 internal constant SUMMARY_EXITED_KEYS_COUNT_OFFSET = 1;
-    /// @dev Total number of keys of this operator for all time
+    /// @dev Total number of keys of all operators for all time
     uint8 internal constant SUMMARY_TOTAL_KEYS_COUNT_OFFSET = 2;
-    /// @dev Number of keys of this operator which were in DEPOSITED state for all time
+    /// @dev Number of keys of all operators which were in the DEPOSITED state for all time
     uint8 internal constant SUMMARY_DEPOSITED_KEYS_COUNT_OFFSET = 3;
 
     //
@@ -150,7 +150,7 @@ contract NodeOperatorsRegistry is AragonApp, Versioned {
     // bytes32 internal constant TYPE_POSITION = keccak256("lido.NodeOperatorsRegistry.type");
     bytes32 internal constant TYPE_POSITION = 0xbacf4236659a602d72c631ba0b0d67ec320aaf523f3ae3590d7faee4f42351d0;
 
-    // bytes32 internal constant TYPE_POSITION = keccak256("lido.NodeOperatorsRegistry.stuckPenaltyDelay");
+    // bytes32 internal constant STUCK_PENALTY_DELAY_POSITION = keccak256("lido.NodeOperatorsRegistry.stuckPenaltyDelay");
     bytes32 internal constant STUCK_PENALTY_DELAY_POSITION = 0x8e3a1f3826a82c1116044b334cae49f3c3d12c3866a1c4b18af461e12e58a18e;
 
     //
@@ -286,7 +286,7 @@ contract NodeOperatorsRegistry is AragonApp, Versioned {
     /// @return id a unique key of the added operator
     function addNodeOperator(string _name, address _rewardAddress) external returns (uint256 id) {
         _onlyValidNodeOperatorName(_name);
-        _onlyNonZeroAddress(_rewardAddress);
+        _onlyValidRewardAddress(_rewardAddress);
         _auth(MANAGE_NODE_OPERATOR_ROLE);
 
         id = getNodeOperatorsCount();
@@ -370,7 +370,7 @@ contract NodeOperatorsRegistry is AragonApp, Versioned {
     /// @param _nodeOperatorId Node operator id to set reward address for
     /// @param _rewardAddress Execution layer Ethereum address to set as reward address
     function setNodeOperatorRewardAddress(uint256 _nodeOperatorId, address _rewardAddress) external {
-        _onlyNonZeroAddress(_rewardAddress);
+        _onlyValidRewardAddress(_rewardAddress);
         _onlyExistedNodeOperator(_nodeOperatorId);
         _auth(MANAGE_NODE_OPERATOR_ROLE);
 
@@ -383,7 +383,7 @@ contract NodeOperatorsRegistry is AragonApp, Versioned {
     /// @dev Current implementation preserves invariant: depositedSigningKeysCount <= vettedSigningKeysCount <= totalSigningKeysCount.
     ///     If _vettedSigningKeysCount out of range [depositedSigningKeysCount, totalSigningKeysCount], the new vettedSigningKeysCount
     ///     value will be set to the nearest range border.
-    /// @param _nodeOperatorId Node operator id to set reward address for
+    /// @param _nodeOperatorId Node operator id to set staking limit for
     /// @param _vettedSigningKeysCount New staking limit of the node operator
     function setNodeOperatorStakingLimit(uint256 _nodeOperatorId, uint64 _vettedSigningKeysCount) external {
         _onlyExistedNodeOperator(_nodeOperatorId);
@@ -1439,6 +1439,13 @@ contract NodeOperatorsRegistry is AragonApp, Versioned {
         require(bytes(_name).length > 0 && bytes(_name).length <= MAX_NODE_OPERATOR_NAME_LENGTH, "WRONG_NAME_LENGTH");
     }
 
+    function _onlyValidRewardAddress(address _rewardAddress) internal view {
+        _onlyNonZeroAddress(_rewardAddress);
+        // The Lido address is forbidden explicitly because stETH transfers on this contract will revert
+        // See onExitedAndStuckValidatorsCountsUpdated() and StETH._transferShares() for details
+        require(_rewardAddress != getLocator().lido(), "LIDO_REWARD_ADDRESS");
+    }
+
     function _onlyNonZeroAddress(address _a) internal pure {
         require(_a != address(0), "ZERO_ADDRESS");
     }

contracts/0.8.9/Burner.sol

@@ -61,7 +61,6 @@ contract Burner is IBurner, AccessControlEnumerable {
     error ZeroAddress(string field);
 
     bytes32 public constant REQUEST_BURN_MY_STETH_ROLE = keccak256("REQUEST_BURN_MY_STETH_ROLE");
-    bytes32 public constant RECOVER_ASSETS_ROLE = keccak256("RECOVER_ASSETS_ROLE");
     bytes32 public constant REQUEST_BURN_SHARES_ROLE = keccak256("REQUEST_BURN_SHARES_ROLE");
 
     uint256 private coverSharesBurnRequested;
@@ -164,7 +163,7 @@ contract Burner is IBurner, AccessControlEnumerable {
       *
       */
     function requestBurnMyStETHForCover(uint256 _stETHAmountToBurn) external onlyRole(REQUEST_BURN_MY_STETH_ROLE) {
-        require(IStETH(STETH).transferFrom(msg.sender, address(this), _stETHAmountToBurn));
+        IStETH(STETH).transferFrom(msg.sender, address(this), _stETHAmountToBurn);
         uint256 sharesAmount = IStETH(STETH).getSharesByPooledEth(_stETHAmountToBurn);
         _requestBurn(sharesAmount, _stETHAmountToBurn, true /* _isCover */);
     }
@@ -197,7 +196,7 @@ contract Burner is IBurner, AccessControlEnumerable {
       *
       */
     function requestBurnMyStETH(uint256 _stETHAmountToBurn) external onlyRole(REQUEST_BURN_MY_STETH_ROLE) {
-        require(IStETH(STETH).transferFrom(msg.sender, address(this), _stETHAmountToBurn));
+        IStETH(STETH).transferFrom(msg.sender, address(this), _stETHAmountToBurn);
         uint256 sharesAmount = IStETH(STETH).getSharesByPooledEth(_stETHAmountToBurn);
         _requestBurn(sharesAmount, _stETHAmountToBurn, false /* _isCover */);
     }
@@ -223,15 +222,15 @@ contract Burner is IBurner, AccessControlEnumerable {
       * but not marked for burning) to the Lido treasury address set upon the
       * contract construction.
       */
-    function recoverExcessStETH() external onlyRole(RECOVER_ASSETS_ROLE) {
+    function recoverExcessStETH() external {
         uint256 excessStETH = getExcessStETH();
 
         if (excessStETH > 0) {
             uint256 excessSharesAmount = IStETH(STETH).getSharesByPooledEth(excessStETH);
 
             emit ExcessStETHRecovered(msg.sender, excessStETH, excessSharesAmount);
 
-            require(IStETH(STETH).transfer(TREASURY, excessStETH));
+            IStETH(STETH).transfer(TREASURY, excessStETH);
         }
     }
 
@@ -249,7 +248,7 @@ contract Burner is IBurner, AccessControlEnumerable {
       * @param _token an ERC20-compatible token
       * @param _amount token amount
       */
-    function recoverERC20(address _token, uint256 _amount) external onlyRole(RECOVER_ASSETS_ROLE) {
+    function recoverERC20(address _token, uint256 _amount) external {
         if (_amount == 0) revert ZeroRecoveryAmount();
         if (_token == STETH) revert StETHRecoveryWrongFunc();
 
@@ -265,7 +264,7 @@ contract Burner is IBurner, AccessControlEnumerable {
       * @param _token an ERC721-compatible token
       * @param _tokenId minted token id
       */
-    function recoverERC721(address _token, uint256 _tokenId) external onlyRole(RECOVER_ASSETS_ROLE) {
+    function recoverERC721(address _token, uint256 _tokenId) external {
         if (_token == STETH) revert StETHRecoveryWrongFunc();
 
         emit ERC721Recovered(msg.sender, _token, _tokenId);

contracts/0.8.9/DepositSecurityModule.sol

@@ -72,6 +72,11 @@ contract DepositSecurityModule {
     IStakingRouter public immutable STAKING_ROUTER;
     IDepositContract public immutable DEPOSIT_CONTRACT;
 
+    /**
+     * NB: both `maxDepositsPerBlock` and `minDepositBlockDistance` values
+     * must be harmonized with `OracleReportSanityChecker.churnValidatorsPerDayLimit`
+     * (see docs for the `OracleReportSanityChecker.setChurnValidatorsPerDayLimit` function)
+     */
     uint256 internal maxDepositsPerBlock;
     uint256 internal minDepositBlockDistance;
     uint256 internal pauseIntentValidityPeriodBlocks;
@@ -176,6 +181,9 @@ contract DepositSecurityModule {
 
     /**
      * Sets `maxDepositsPerBlock`. Only callable by the owner.
+     *
+     * NB: the value must be harmonized with `OracleReportSanityChecker.churnValidatorsPerDayLimit`
+     * (see docs for the `OracleReportSanityChecker.setChurnValidatorsPerDayLimit` function)
      */
     function setMaxDeposits(uint256 newValue) external onlyOwner {
         _setMaxDeposits(newValue);
@@ -195,6 +203,9 @@ contract DepositSecurityModule {
 
     /**
      * Sets `minDepositBlockDistance`. Only callable by the owner.
+     *
+     * NB: the value must be harmonized with `OracleReportSanityChecker.churnValidatorsPerDayLimit`
+     * (see docs for the `OracleReportSanityChecker.setChurnValidatorsPerDayLimit` function)
      */
     function setMinDepositBlockDistance(uint256 newValue) external onlyOwner {
         _setMinDepositBlockDistance(newValue);

contracts/0.8.9/OracleDaemonConfig.sol

@@ -38,6 +38,7 @@ contract OracleDaemonConfig is AccessControlEnumerable {
     function update(string calldata _key, bytes calldata _value) external onlyRole(CONFIG_MANAGER_ROLE) {
         if (values[_key].length == 0) revert ValueDoesntExist(_key);
         if (_value.length == 0) revert EmptyValue(_key);
+        if (keccak256(values[_key]) == keccak256(_value)) revert ValueIsSame(_key, _value);
         values[_key] = _value;
 
         emit ConfigValueUpdated(_key, _value);
@@ -76,6 +77,7 @@ contract OracleDaemonConfig is AccessControlEnumerable {
     error EmptyValue(string key);
     error ValueDoesntExist(string key);
     error ZeroAddress();
+    error ValueIsSame(string key, bytes value);
 
     event ConfigValueSet(string key, bytes value);
     event ConfigValueUpdated(string key, bytes value);

contracts/0.8.9/StakingRouter.sol

@@ -497,7 +497,7 @@ contract StakingRouter is AccessControlEnumerable, BeaconChainDepositor, Version
             uint256 totalExitedValidators,
             /* uint256 totalDepositedValidators */,
             /* uint256 depositableValidatorsCount */
-        ) = IStakingModule(stakingModule.stakingModuleAddress).getNodeOperatorSummary(_nodeOperatorId);
+        ) = IStakingModule(moduleAddr).getNodeOperatorSummary(_nodeOperatorId);
 
         if (_correction.currentModuleExitedValidatorsCount != stakingModule.exitedValidatorsCount ||
             _correction.currentNodeOperatorExitedValidatorsCount != totalExitedValidators ||
@@ -1038,8 +1038,8 @@ contract StakingRouter is AccessControlEnumerable, BeaconChainDepositor, Version
             }
         }
 
-        // sanity check
-        if (totalFee >= precisionPoints) revert ValueOver100Percent("totalFee");
+        // Total fee never exceeds 100%
+        assert(totalFee <= precisionPoints);
 
         /// @dev shrink arrays
         if (rewardedStakingModulesCount < stakingModulesCount) {