Native staking updates (#2023)

* Update Natspec * Generated docs for native eth strategy * Prettier and linter Fixed spelling of ValidatorAccountant events Implemented depositSSV * Updated Natspec Moved MAX_STAKE on ValidatorAccountant to a constant * Removed strategist from strategy as its already maintained in the Vault * Fix compilation error * Fix unit tests * fix linter
OriginProtocol · Apr 22, 2024 · 121c7f7 · 121c7f7
1 parent 5d5e9cb
commit 121c7f7
Show file tree

Hide file tree

Showing 18 changed files with 741 additions and 156 deletions.
diff --git a/contracts/contracts/interfaces/IDepositContract.sol b/contracts/contracts/interfaces/IDepositContract.sol
@@ -31,4 +31,4 @@ interface IDepositContract {
     /// @notice Query the current deposit count.
     /// @return The deposit count encoded as a little endian 64-bit number.
     function get_deposit_count() external view returns (bytes memory);
-}
+}
diff --git a/contracts/contracts/mocks/BeaconChainDepositContractMock.sol b/contracts/contracts/mocks/BeaconChainDepositContractMock.sol
@@ -25,13 +25,29 @@ contract BeaconChainDepositContractMock {
     ) external payable {
         // Extended ABI length checks since dynamic types are used.
         require(pubkey.length == 48, "DepositContract: invalid pubkey length");
-        require(withdrawal_credentials.length == 32, "DepositContract: invalid withdrawal_credentials length");
-        require(signature.length == 96, "DepositContract: invalid signature length");
+        require(
+            withdrawal_credentials.length == 32,
+            "DepositContract: invalid withdrawal_credentials length"
+        );
+        require(
+            signature.length == 96,
+            "DepositContract: invalid signature length"
+        );
 
         // Check deposit amount
         require(msg.value >= 1 ether, "DepositContract: deposit value too low");
-        require(msg.value % 1 gwei == 0, "DepositContract: deposit value not multiple of gwei");
-        uint deposit_amount = msg.value / 1 gwei;
-        require(deposit_amount <= type(uint64).max, "DepositContract: deposit value too high");
+        require(
+            msg.value % 1 gwei == 0,
+            "DepositContract: deposit value not multiple of gwei"
+        );
+        uint256 deposit_amount = msg.value / 1 gwei;
+        require(
+            deposit_amount <= type(uint64).max,
+            "DepositContract: deposit value too high"
+        );
+        require(
+            deposit_data_root != 0,
+            "DepositContract: invalid deposit_data_root"
+        );
     }
-}
+}
diff --git a/contracts/contracts/strategies/NativeStaking/FeeAccumulator.sol b/contracts/contracts/strategies/NativeStaking/FeeAccumulator.sol
@@ -7,13 +7,14 @@ import { IWETH9 } from "../../interfaces/IWETH9.sol";
 /**
  * @title Fee Accumulator for Native Staking SSV Strategy
  * @notice This contract is setup to receive fees from processing transactions on the beacon chain
- *         which includes priority fees and any MEV rewards
+ *         which includes priority fees and any MEV rewards.
+ * It does NOT include swept ETH from consensus rewards or withdrawals.
  * @author Origin Protocol Inc
  */
 contract FeeAccumulator is Governable {
-    /// @dev ETH is sent to the collector address
+    /// @notice The address the WETH is sent to on `collect` which is the Native Staking Strategy
     address public immutable COLLECTOR;
-    /// @notice WETH token address
+    /// @notice The address of the Wrapped ETH (WETH) token contract
     address public immutable WETH_TOKEN_ADDRESS;
 
     error CallerNotCollector(address caller, address expectedCaller);
@@ -23,30 +24,32 @@ contract FeeAccumulator is Governable {
 
     /**
      * @param _collector Address of the contract that collects the fees
+     * @param _weth Address of the Wrapped ETH (WETH) token contract
      */
     constructor(address _collector, address _weth) {
         COLLECTOR = _collector;
         WETH_TOKEN_ADDRESS = _weth;
     }
 
-    /*
-     * @notice Asserts that the caller is the collector
+    /**
+     * @dev Asserts that the caller is the collector
      */
     function _assertIsCollector() internal view {
         if (msg.sender != COLLECTOR) {
             revert CallerNotCollector(msg.sender, COLLECTOR);
         }
     }
 
-    /*
-     * @notice Send all the ETH to the collector
+    /**
+     * @notice Converts ETH to WETH and sends the WETH to the collector
+     * @return weth The amount of WETH sent to the collector
      */
-    function collect() external returns (uint256 wethReturned) {
+    function collect() external returns (uint256 weth) {
         _assertIsCollector();
-        wethReturned = address(this).balance;
-        if (wethReturned > 0) {
-            IWETH9(WETH_TOKEN_ADDRESS).deposit{ value: wethReturned }();
-            IWETH9(WETH_TOKEN_ADDRESS).transfer(COLLECTOR, wethReturned);
+        weth = address(this).balance;
+        if (weth > 0) {
+            IWETH9(WETH_TOKEN_ADDRESS).deposit{ value: weth }();
+            IWETH9(WETH_TOKEN_ADDRESS).transfer(COLLECTOR, weth);
         }
     }
 }
diff --git a/contracts/contracts/strategies/NativeStaking/NativeStakingSSVStrategy.sol b/contracts/contracts/strategies/NativeStaking/NativeStakingSSVStrategy.sol
@@ -5,6 +5,7 @@ import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
 import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
 
 import { InitializableAbstractStrategy } from "../../utils/InitializableAbstractStrategy.sol";
+import { ISSVNetwork, Cluster } from "../../interfaces/ISSVNetwork.sol";
 import { IWETH9 } from "../../interfaces/IWETH9.sol";
 import { FeeAccumulator } from "./FeeAccumulator.sol";
 import { ValidatorAccountant } from "./ValidatorAccountant.sol";
@@ -39,7 +40,7 @@ contract NativeStakingSSVStrategy is
 
     error EmptyRecipient();
     error NotWeth();
-    error InsuffiscientWethBalance(
+    error InsufficientWethBalance(
         uint256 requiredBalance,
         uint256 availableBalance
     );
@@ -60,7 +61,12 @@ contract NativeStakingSSVStrategy is
         address _beaconChainDepositContract
     )
         InitializableAbstractStrategy(_baseConfig)
-        ValidatorAccountant(_wethAddress, _baseConfig.vaultAddress, _beaconChainDepositContract, _ssvNetwork)
+        ValidatorAccountant(
+            _wethAddress,
+            _baseConfig.vaultAddress,
+            _beaconChainDepositContract,
+            _ssvNetwork
+        )
     {
         SSV_TOKEN_ADDRESS = _ssvToken;
         FEE_ACCUMULATOR_ADDRESS = _feeAccumulator;
@@ -98,7 +104,8 @@ contract NativeStakingSSVStrategy is
             beaconChainRewardWETH;
     }
 
-    /// @notice Collect accumulated WETH & SSV tokens and send to the Harvester.
+    /// @notice Convert accumulated ETH to WETH and send to the Harvester.
+    /// Only callable by the Harvester.
     function collectRewardTokens()
         external
         virtual
@@ -129,7 +136,7 @@ contract NativeStakingSSVStrategy is
             if (balance > 0) {
                 if (address(rewardToken) == WETH_TOKEN_ADDRESS) {
                     if (beaconChainRewardWETH > balance) {
-                        revert InsuffiscientWethBalance(
+                        revert InsufficientWethBalance(
                             beaconChainRewardWETH,
                             balance
                         );
@@ -232,9 +239,9 @@ contract NativeStakingSSVStrategy is
     function _abstractSetPToken(address _asset, address) internal override {}
 
     /// @notice Returns the total value of (W)ETH that is staked to the validators
-    /// and also present on the native staking and fee accumulator contracts
+    /// and also present on the native staking and fee accumulator contracts.
     /// @param _asset      Address of weth asset
-    /// @return balance    Total value of (W)ETH 
+    /// @return balance    Total value of (W)ETH
     function checkBalance(address _asset)
         external
         view
@@ -254,14 +261,13 @@ contract NativeStakingSSVStrategy is
         _pause();
     }
 
-    /// @dev Retuns bool indicating whether asset is supported by strategy
-    /// @param _asset Address of the asset
+    /// @notice Returns bool indicating whether asset is supported by strategy.
+    /// @param _asset The address of the asset token.
     function supportsAsset(address _asset) public view override returns (bool) {
         return _asset == WETH_TOKEN_ADDRESS;
     }
 
-    /// @notice Approve the spending of all assets
-    /// @dev Approves the SSV Network contract to transfer SSV tokens for deposits
+    /// @notice Approves the SSV Network contract to transfer SSV tokens for deposits
     function safeApproveAllTokens() external override {
         /// @dev Approves the SSV Network contract to transfer SSV tokens for deposits
         IERC20(SSV_TOKEN_ADDRESS).approve(
@@ -271,17 +277,20 @@ 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 fron-running can't DOS our maintenance service
-    ///      that tries to top us SSV tokens. 
+    /// @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
-    ) 
-    onlyStrategist
-    external {
-        // address SSV_NETWORK_ADDRESS = lrtConfig.getContract(LRTConstants.SSV_NETWORK);
-        // ISSVNetwork(SSV_NETWORK_ADDRESS).deposit(address(this), operatorIds, amount, cluster);
+    ) external onlyStrategist {
+        ISSVNetwork(SSV_NETWORK_ADDRESS).deposit(
+            address(this),
+            operatorIds,
+            amount,
+            cluster
+        );
     }
 }
diff --git a/contracts/contracts/strategies/NativeStaking/README.md b/contracts/contracts/strategies/NativeStaking/README.md
@@ -0,0 +1,21 @@
+# Diagrams
+
+## Native Staking SSV Strategy
+
+### Hierarchy
+
+![Native Staking SSV Strategy Hierarchy](../../../docs/NativeStakingSSVStrategyHierarchy.svg)
+
+### Squashed
+
+![Native Staking SSV Strategy Squashed](../../../docs/NativeStakingSSVStrategySquashed.svg)
+
+### Storage
+
+![Native Staking SSV Strategy Storage](../../../docs/NativeStakingSSVStrategyStorage.svg)
+
+## Fee Accumulator
+
+### Squashed
+
+![Fee Accumulator Squashed](../../../docs/FeeAccumulatorSquashed.svg)
diff --git a/contracts/contracts/strategies/NativeStaking/ValidatorAccountant.sol b/contracts/contracts/strategies/NativeStaking/ValidatorAccountant.sol
@@ -3,13 +3,18 @@ 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 Accountant of the rewards Beacon Chain ETH
 /// @notice This contract contains the logic to attribute the Beacon Chain swept ETH either to full
 /// or partial withdrawals
 /// @author Origin Protocol Inc
 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;
 
     /// @dev The WETH present on this contract will come from 2 sources:
@@ -23,14 +28,12 @@ abstract contract ValidatorAccountant is ValidatorRegistrator {
     /// present as a result of a deposit.
     uint256 public beaconChainRewardWETH = 0;
 
-    /// @dev start of fuse interval
+    /// @notice start of fuse interval
     uint256 public fuseIntervalStart = 0;
-    /// @dev end of fuse interval
+    /// @notice end of fuse interval
     uint256 public fuseIntervalEnd = 0;
-    /// @dev Governor that can manually correct the accounting
+    /// @notice Governor that can manually correct the accounting
     address public accountingGovernor;
-    /// @dev Strategist that can pause the accounting
-    address public strategist;
 
     uint256[50] private __gap;
 
@@ -40,12 +43,12 @@ abstract contract ValidatorAccountant is ValidatorRegistrator {
         uint256 start,
         uint256 end
     );
-    event AccuntingFullyWithdrawnValidator(
+    event AccountingFullyWithdrawnValidator(
         uint256 noOfValidators,
         uint256 remainingValidators,
         uint256 wethSentToVault
     );
-    event AccuntingValidatorSlashed(
+    event AccountingValidatorSlashed(
         uint256 remainingValidators,
         uint256 wethSentToVault
     );
@@ -54,10 +57,6 @@ abstract contract ValidatorAccountant is ValidatorRegistrator {
         address newAddress
     );
     event AccountingBeaconChainRewards(uint256 amount);
-    event StrategistAddressChanged(
-        address oldStrategist,
-        address newStrategist
-    );
 
     event AccountingManuallyFixed(
         uint256 oldActiveDepositedValidators,
@@ -85,16 +84,29 @@ abstract contract ValidatorAccountant is ValidatorRegistrator {
 
     /// @dev Throws if called by any account other than the Strategist
     modifier onlyStrategist() {
-        require(msg.sender == strategist, "Caller is not the Strategist");
+        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)
-    ValidatorRegistrator(_wethAddress, _beaconChainDepositContract, _ssvNetwork) {
+    constructor(
+        address _wethAddress,
+        address _vaultAddress,
+        address _beaconChainDepositContract,
+        address _ssvNetwork
+    )
+        ValidatorRegistrator(
+            _wethAddress,
+            _beaconChainDepositContract,
+            _ssvNetwork
+        )
+    {
         VAULT_ADDRESS = _vaultAddress;
     }
 
@@ -103,11 +115,6 @@ abstract contract ValidatorAccountant is ValidatorRegistrator {
         accountingGovernor = _address;
     }
 
-    function setStrategist(address _address) external onlyGovernor {
-        emit StrategistAddressChanged(strategist, _address);
-        strategist = _address;
-    }
-
     /// @notice set fuse interval values
     function setFuseInterval(
         uint256 _fuseIntervalStart,
@@ -133,19 +140,24 @@ abstract contract ValidatorAccountant is ValidatorRegistrator {
         fuseIntervalEnd = _fuseIntervalEnd;
     }
 
+    /* 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
+    /// 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.
     /// @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
+    /// 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
-    /// for now
-    function doAccounting() external onlyRegistrator returns (bool accountingValid) {
+    /// for now.
+    /* solhint-enable max-line-length */
+    function doAccounting()
+        external
+        onlyRegistrator
+        returns (bool accountingValid)
+    {
         uint256 ethBalance = address(this).balance;
-        uint256 MAX_STAKE = 32 ether;
         accountingValid = true;
 
         // send the WETH that is from fully withdrawn validators to the Vault
@@ -157,7 +169,7 @@ abstract contract ValidatorAccountant is ValidatorRegistrator {
             IWETH9(WETH_TOKEN_ADDRESS).deposit{ value: wethToVault }();
             IWETH9(WETH_TOKEN_ADDRESS).transfer(VAULT_ADDRESS, wethToVault);
 
-            emit AccuntingFullyWithdrawnValidator(
+            emit AccountingFullyWithdrawnValidator(
                 fullyWithdrawnValidators,
                 activeDepositedValidators,
                 wethToVault
@@ -173,6 +185,7 @@ abstract contract ValidatorAccountant is ValidatorRegistrator {
         // Beacon chain rewards swept (partial validator withdrawals)
         if (ethRemaining <= fuseIntervalStart) {
             IWETH9(WETH_TOKEN_ADDRESS).deposit{ value: ethRemaining }();
+            // solhint-disable-next-line reentrancy
             beaconChainRewardWETH += ethRemaining;
             emit AccountingBeaconChainRewards(ethRemaining);
         }
@@ -182,7 +195,7 @@ abstract contract ValidatorAccountant is ValidatorRegistrator {
             IWETH9(WETH_TOKEN_ADDRESS).transfer(VAULT_ADDRESS, ethRemaining);
             activeDepositedValidators -= 1;
 
-            emit AccuntingValidatorSlashed(
+            emit AccountingValidatorSlashed(
                 activeDepositedValidators,
                 ethRemaining
             );