Native staking changes and unit tests (#2029)

* Fixed native staking deployment since the strategist is got from the vault

* Refactor of some Native Staking events
Refactor of Native Staking unit tests

* Renamed AccountingBeaconChainRewards to AccountingConsensusRewards
Accounting updated to handle zero ETH from the beacon chain

* fixed bug not accounting for previous consensus rewards
Blow fuse if ETH balance < previous consensus rewards

* Pause collectRewardTokens and doAccounting on accounting failure.

Validated asset on deposit to Native Staking Strategy.

Moved depositSSV from NativeStakingSSVStrategy to ValidatorRegistrator

moved onlyStrategist modified and VAULT_ADDRESS immutable from ValidatorAccountant to ValidatorRegistrator

manuallyFixAccounting changed to use whenPaused modifier

made fuseIntervalEnd inclusive

Natspec updates

refactoring of native staking unit tests

contracts/contracts/strategies/NativeStaking/NativeStakingSSVStrategy.sol

@@ -82,7 +82,8 @@ contract NativeStakingSSVStrategy is
     }
 
     /// @dev Convert accumulated ETH to WETH and send to the Harvester.
-    function _collectRewardTokens() internal override {
+    /// Will revert if the strategy is paused for accounting.
+    function _collectRewardTokens() internal override whenNotPaused {
         // collect ETH from execution rewards from the fee accumulator
         uint256 executionRewards = FeeAccumulator(FEE_ACCUMULATOR_ADDRESS)
             .collect();
@@ -114,19 +115,23 @@ contract NativeStakingSSVStrategy is
         }
     }
 
-    /// @notice Deposit asset into the underlying platform
-    /// @param _asset Address of asset to deposit
-    /// @param _amount Amount of assets to deposit
+    /// @notice Unlike other strategies, this does not deposit assets into the underlying platform.
+    /// It just checks the asset is WETH and emits the Deposit event.
+    /// To deposit WETH into validators `registerSsvValidator` and `stakeEth` must be used.
+    /// Will NOT revert if the strategy is paused from an accounting failure.
+    /// @param _asset Address of asset to deposit. Has to be WETH.
+    /// @param _amount Amount of assets that were transferred to the strategy by the vault.
     function deposit(address _asset, uint256 _amount)
         external
         override
         onlyVault
         nonReentrant
     {
+        require(_asset == WETH_TOKEN_ADDRESS, "Unsupported asset");
         _deposit(_asset, _amount);
     }
 
-    /// @dev Deposit WETH to this contract to enable automated action to stake it
+    /// @dev Deposit WETH to this strategy so it can later be staked into a validator.
     /// @param _asset Address of WETH
     /// @param _amount Amount of WETH to deposit
     function _deposit(address _asset, uint256 _amount) internal {
@@ -143,7 +148,10 @@ contract NativeStakingSSVStrategy is
         emit Deposit(_asset, address(0), _amount);
     }
 
-    /// @notice Deposit the entire balance of WETH asset in the strategy into the underlying platform
+    /// @notice Unlike other strategies, this does not deposit assets into the underlying platform.
+    /// It just emits the Deposit event.
+    /// To deposit WETH into validators `registerSsvValidator` and `stakeEth` must be used.
+    /// Will NOT revert if the strategy is paused from an accounting failure.
     function depositAll() external override onlyVault nonReentrant {
         uint256 wethBalance = IERC20(WETH_TOKEN_ADDRESS).balanceOf(
             address(this)
@@ -157,6 +165,7 @@ contract NativeStakingSSVStrategy is
     /// can happen when:
     ///   - the deposit was not a multiple of 32 WETH
     ///   - someone sent WETH directly to this contract
+    /// Will NOT revert if the strategy is paused from an accounting failure.
     /// @param _recipient Address to receive withdrawn assets
     /// @param _asset WETH to withdraw
     /// @param _amount Amount of WETH to withdraw
@@ -180,7 +189,14 @@ contract NativeStakingSSVStrategy is
         IERC20(_asset).safeTransfer(_recipient, _amount);
     }
 
-    /// @notice Remove all supported assets from the underlying platform and send them to Vault contract.
+    /// @notice transfer all WETH deposits back to the vault.
+    /// This does not withdraw from the validators. That has to be done separately with the
+    /// `exitSsvValidator` and `removeSsvValidator` operations.
+    /// This does not withdraw any execution rewards from the FeeAccumulator or
+    /// consensus rewards in this strategy.
+    /// Any ETH in this strategy that was swept from a full validator withdrawal will not be withdrawn.
+    /// ETH from full validator withdrawals is sent to the Vault using `doAccounting`.
+    /// Will NOT revert if the strategy is paused from an accounting failure.
     function withdrawAll() external override onlyVaultOrGovernor nonReentrant {
         uint256 wethBalance = IERC20(WETH_TOKEN_ADDRESS).balanceOf(
             address(this)
@@ -234,24 +250,6 @@ contract NativeStakingSSVStrategy is
         );
     }
 
-    /// @notice Deposits more SSV Tokens to the SSV Network contract which is used to pay the SSV Operators.
-    /// @dev A SSV cluster is defined by the SSVOwnerAddress and the set of operatorIds.
-    /// uses "onlyStrategist" modifier so continuous front-running can't DOS our maintenance service
-    /// that tries to top up SSV tokens.
-    /// @param cluster The SSV cluster details that must be derived from emitted events from the SSVNetwork contract.
-    function depositSSV(
-        uint64[] memory operatorIds,
-        uint256 amount,
-        Cluster memory cluster
-    ) external onlyStrategist {
-        ISSVNetwork(SSV_NETWORK_ADDRESS).deposit(
-            address(this),
-            operatorIds,
-            amount,
-            cluster
-        );
-    }
-
     /**
      * @notice Only accept ETH from the FeeAccumulator
      * @dev don't want to receive donations from anyone else as this will

contracts/contracts/strategies/NativeStaking/ValidatorAccountant.sol

@@ -3,7 +3,6 @@ pragma solidity ^0.8.0;
 
 import { Pausable } from "@openzeppelin/contracts/security/Pausable.sol";
 import { ValidatorRegistrator } from "./ValidatorRegistrator.sol";
-import { IVault } from "../../interfaces/IVault.sol";
 import { IWETH9 } from "../../interfaces/IWETH9.sol";
 
 /// @title Validator Accountant
@@ -15,8 +14,6 @@ abstract contract ValidatorAccountant is ValidatorRegistrator {
     /// @notice The maximum amount of ETH that can be staked by a validator
     /// @dev this can change in the future with EIP-7251, Increase the MAX_EFFECTIVE_BALANCE
     uint256 public constant MAX_STAKE = 32 ether;
-    /// @notice Address of the OETH Vault proxy contract
-    address public immutable VAULT_ADDRESS;
 
     /// @notice Keeps track of the total consensus rewards swept from the beacon chain
     uint256 public consensusRewards = 0;
@@ -30,12 +27,7 @@ abstract contract ValidatorAccountant is ValidatorRegistrator {
 
     uint256[50] private __gap;
 
-    event FuseIntervalUpdated(
-        uint256 oldStart,
-        uint256 oldEnd,
-        uint256 start,
-        uint256 end
-    );
+    event FuseIntervalUpdated(uint256 start, uint256 end);
     event AccountingFullyWithdrawnValidator(
         uint256 noOfValidators,
         uint256 remainingValidators,
@@ -45,11 +37,8 @@ abstract contract ValidatorAccountant is ValidatorRegistrator {
         uint256 remainingValidators,
         uint256 wethSentToVault
     );
-    event AccountingGovernorAddressChanged(
-        address oldAddress,
-        address newAddress
-    );
-    event AccountingBeaconChainRewards(uint256 amount);
+    event AccountingGovernorChanged(address newAddress);
+    event AccountingConsensusRewards(uint256 amount);
 
     event AccountingManuallyFixed(
         uint256 oldActiveDepositedValidators,
@@ -69,15 +58,6 @@ abstract contract ValidatorAccountant is ValidatorRegistrator {
         _;
     }
 
-    /// @dev Throws if called by any account other than the Strategist
-    modifier onlyStrategist() {
-        require(
-            msg.sender == IVault(VAULT_ADDRESS).strategistAddr(),
-            "Caller is not the Strategist"
-        );
-        _;
-    }
-
     /// @param _wethAddress Address of the Erc20 WETH Token contract
     /// @param _vaultAddress Address of the Vault
     /// @param _beaconChainDepositContract Address of the beacon chain deposit contract
@@ -90,15 +70,14 @@ abstract contract ValidatorAccountant is ValidatorRegistrator {
     )
         ValidatorRegistrator(
             _wethAddress,
+            _vaultAddress,
             _beaconChainDepositContract,
             _ssvNetwork
         )
-    {
-        VAULT_ADDRESS = _vaultAddress;
-    }
+    {}
 
     function setAccountingGovernor(address _address) external onlyGovernor {
-        emit AccountingGovernorAddressChanged(accountingGovernor, _address);
+        emit AccountingGovernorChanged(_address);
         accountingGovernor = _address;
     }
 
@@ -115,12 +94,7 @@ abstract contract ValidatorAccountant is ValidatorRegistrator {
             "incorrect fuse interval"
         );
 
-        emit FuseIntervalUpdated(
-            fuseIntervalStart,
-            fuseIntervalEnd,
-            _fuseIntervalStart,
-            _fuseIntervalEnd
-        );
+        emit FuseIntervalUpdated(_fuseIntervalStart, _fuseIntervalEnd);
 
         fuseIntervalStart = _fuseIntervalStart;
         fuseIntervalEnd = _fuseIntervalEnd;
@@ -129,10 +103,10 @@ abstract contract ValidatorAccountant is ValidatorRegistrator {
     /* solhint-disable max-line-length */
     /// This notion page offers a good explanation of how the accounting functions
     /// https://www.notion.so/originprotocol/Limited-simplified-native-staking-accounting-67a217c8420d40678eb943b9da0ee77d
-    /// In short, after dividing by 32 if the ETH remaining on the contract falls between 0 and fuseIntervalStart the accounting
-    /// function will treat that ETH as a Beacon Chain Reward ETH.
-    /// On the contrary if after dividing by 32 the ETH remaining on the contract falls between fuseIntervalEnd and 32 the
-    /// accounting function will treat that as a validator slashing.
+    /// In short, after dividing by 32, if the ETH remaining on the contract falls between 0 and fuseIntervalStart,
+    /// the accounting function will treat that ETH as Beacon chain consensus rewards.
+    /// On the contrary, if after dividing by 32, the ETH remaining on the contract falls between fuseIntervalEnd and 32,
+    /// the accounting function will treat that as a validator slashing.
     /// @notice Perform the accounting attributing beacon chain ETH to either full or partial withdrawals. Returns true when
     /// accounting is valid and fuse isn't "blown". Returns false when fuse is blown.
     /// @dev This function could in theory be permission-less but lets allow only the Registrator (Defender Action) to call it
@@ -141,8 +115,15 @@ abstract contract ValidatorAccountant is ValidatorRegistrator {
     function doAccounting()
         external
         onlyRegistrator
+        whenNotPaused
         returns (bool accountingValid)
     {
+        if (address(this).balance < consensusRewards) {
+            // pause and fail the accounting
+            _pause();
+            return false;
+        }
+
         // Calculate all the new ETH that has been swept to the contract since the last accounting
         uint256 newSweptETH = address(this).balance - consensusRewards;
         accountingValid = true;
@@ -163,18 +144,23 @@ abstract contract ValidatorAccountant is ValidatorRegistrator {
             );
         }
 
-        uint256 ethRemaining = address(this).balance;
+        uint256 ethRemaining = address(this).balance - consensusRewards;
         // should be less than a whole validator stake
         require(ethRemaining < 32 ether, "unexpected accounting");
 
-        // Beacon chain rewards swept (partial validator withdrawals)
-        if (ethRemaining <= fuseIntervalStart) {
+        // If no Beacon chain consensus rewards swept
+        if (ethRemaining == 0) {
+            // do nothing
+            return accountingValid;
+        }
+        // Beacon chain consensus rewards swept (partial validator withdrawals)
+        else if (ethRemaining < fuseIntervalStart) {
             // solhint-disable-next-line reentrancy
             consensusRewards += ethRemaining;
-            emit AccountingBeaconChainRewards(ethRemaining);
+            emit AccountingConsensusRewards(ethRemaining);
         }
-        // Beacon chain rewards swept but also a slashed validator fully exited
-        else if (ethRemaining >= fuseIntervalEnd) {
+        // Beacon chain consensus rewards swept but also a slashed validator fully exited
+        else if (ethRemaining > fuseIntervalEnd) {
             IWETH9(WETH_TOKEN_ADDRESS).deposit{ value: ethRemaining }();
             IWETH9(WETH_TOKEN_ADDRESS).transfer(VAULT_ADDRESS, ethRemaining);
             activeDepositedValidators -= 1;
@@ -209,9 +195,7 @@ abstract contract ValidatorAccountant is ValidatorRegistrator {
         uint256 _consensusRewards,
         uint256 _ethThresholdCheck,
         uint256 _wethThresholdCheck
-    ) external onlyAccountingGovernor {
-        require(paused(), "not paused");
-
+    ) external onlyAccountingGovernor whenPaused {
         uint256 ethBalance = address(this).balance;
         uint256 wethBalance = IWETH9(WETH_TOKEN_ADDRESS).balanceOf(
             address(this)

contracts/contracts/strategies/NativeStaking/ValidatorRegistrator.sol

@@ -4,6 +4,7 @@ pragma solidity ^0.8.0;
 import { Pausable } from "@openzeppelin/contracts/security/Pausable.sol";
 import { Governable } from "../../governance/Governable.sol";
 import { IDepositContract } from "../../interfaces/IDepositContract.sol";
+import { IVault } from "../../interfaces/IVault.sol";
 import { IWETH9 } from "../../interfaces/IWETH9.sol";
 import { ISSVNetwork, Cluster } from "../../interfaces/ISSVNetwork.sol";
 
@@ -25,6 +26,8 @@ abstract contract ValidatorRegistrator is Governable, Pausable {
     address public immutable BEACON_CHAIN_DEPOSIT_CONTRACT;
     /// @notice The address of the SSV Network contract used to interface with
     address public immutable SSV_NETWORK_ADDRESS;
+    /// @notice Address of the OETH Vault proxy contract
+    address public immutable VAULT_ADDRESS;
 
     /// @notice Address of the registrator - allowed to register, exit and remove validators
     address public validatorRegistrator;
@@ -45,7 +48,7 @@ abstract contract ValidatorRegistrator is Governable, Pausable {
         EXIT_COMPLETE // validator has funds withdrawn to the EigenPod and is removed from the SSV
     }
 
-    event RegistratorAddressChanged(address oldAddress, address newAddress);
+    event RegistratorChanged(address newAddress);
     event ETHStaked(bytes pubkey, uint256 amount, bytes withdrawal_credentials);
     event SSVValidatorRegistered(bytes pubkey, uint64[] operatorIds);
     event SSVValidatorExitInitiated(bytes pubkey, uint64[] operatorIds);
@@ -60,22 +63,34 @@ abstract contract ValidatorRegistrator is Governable, Pausable {
         _;
     }
 
+    /// @dev Throws if called by any account other than the Strategist
+    modifier onlyStrategist() {
+        require(
+            msg.sender == IVault(VAULT_ADDRESS).strategistAddr(),
+            "Caller is not the Strategist"
+        );
+        _;
+    }
+
     /// @param _wethAddress Address of the Erc20 WETH Token contract
+    /// @param _vaultAddress Address of the Vault
     /// @param _beaconChainDepositContract Address of the beacon chain deposit contract
     /// @param _ssvNetwork Address of the SSV Network contract
     constructor(
         address _wethAddress,
+        address _vaultAddress,
         address _beaconChainDepositContract,
         address _ssvNetwork
     ) {
         WETH_TOKEN_ADDRESS = _wethAddress;
         BEACON_CHAIN_DEPOSIT_CONTRACT = _beaconChainDepositContract;
         SSV_NETWORK_ADDRESS = _ssvNetwork;
+        VAULT_ADDRESS = _vaultAddress;
     }
 
-    /// @notice Set the address of the registrator
-    function setRegistratorAddress(address _address) external onlyGovernor {
-        emit RegistratorAddressChanged(validatorRegistrator, _address);
+    /// @notice Set the address of the registrator which can register, exit and remove validators
+    function setRegistrator(address _address) external onlyGovernor {
+        emit RegistratorChanged(_address);
         validatorRegistrator = _address;
     }
 
@@ -201,4 +216,22 @@ abstract contract ValidatorRegistrator is Governable, Pausable {
 
         validatorsStates[keccak256(publicKey)] = VALIDATOR_STATE.EXIT_COMPLETE;
     }
+
+    /// @notice Deposits more SSV Tokens to the SSV Network contract which is used to pay the SSV Operators.
+    /// @dev A SSV cluster is defined by the SSVOwnerAddress and the set of operatorIds.
+    /// uses "onlyStrategist" modifier so continuous front-running can't DOS our maintenance service
+    /// that tries to top up SSV tokens.
+    /// @param cluster The SSV cluster details that must be derived from emitted events from the SSVNetwork contract.
+    function depositSSV(
+        uint64[] memory operatorIds,
+        uint256 amount,
+        Cluster memory cluster
+    ) external onlyStrategist {
+        ISSVNetwork(SSV_NETWORK_ADDRESS).deposit(
+            address(this),
+            operatorIds,
+            amount,
+            cluster
+        );
+    }
 }

contracts/deploy/091_native_ssv_staking.js

@@ -11,7 +11,7 @@ module.exports = deploymentWithGovernanceProposal(
     //   "",
   },
   async ({ deployWithConfirmation, ethers, getTxOpts, withConfirmation }) => {
-    const { deployerAddr, strategistAddr } = await getNamedAccounts();
+    const { deployerAddr } = await getNamedAccounts();
     const sDeployer = await ethers.provider.getSigner(deployerAddr);
 
     // Current contracts
@@ -159,12 +159,6 @@ module.exports = deploymentWithGovernanceProposal(
           signature: "setAccountingGovernor(address)",
           args: [deployerAddr], // TODO: change this to the defender action
         },
-        // 6. configure strategist address
-        {
-          contract: cStrategy,
-          signature: "setStrategist(address)",
-          args: [strategistAddr],
-        },
       ],
     };
   }