SSV.network

Liquidations ensure that clusters have an adequate amount of SSV deposited to cover unclaimed network and operator fees. This is critical as operators continuously receive SSV rewards regardless of the cluster's SSV balance. In the case where a cluster's SSV balance falls below the liquidation threshold and is not liquidated, the operators risk not being able to withdraw their earnings as the SSV balance of `SSVNetwork` is insufficient. Additionally, the cluster may remain insolvent upon reactivation due to mismatch between the operators' balances and the cluster's balance.

When an operator calls `registerOperator()`, the operator's `publicKey` and `fee` amounts are passed as arguments. Once `registerOperator()` is called, the operator is registered with its `publicKey`, and the operator owner is set as the caller. Since the operator's `publicKey` refers to the operator's SSV node and differs from the caller's public key, there is no way of verifying that the caller is the actual owner of the operator's `publicKey`. Therefore, a malicious user could see a `registerOperator()` call in the pending transaction pool and front-run the transaction with the operator's `publicKey`. The same vulnerability is present with validators when calling `registerValidator()`. While the caller of `registerOperator()` or `registerValidator()` needs prior authorization from the `owner` via the `SSVNetwork.setRegisterAuth()` function, any registered operator or validator can front-run calls to `registerOperator()` or `registerValidator()` that are from other authorized users.

In order to validate the proper deployment and initialization of the contracts, it is a good practice to emit events. Also, any important state transition can be logged, which is beneficial for monitoring the contract, and also tracking eventual bugs, or hacks. Below we present a non-exhaustive list of events that could be emitted to improve application management: 1. `OperatorLib.updateSnapshotSt()`: `operator.snapshot.*`. 1. `OperatorLib.updateOperators()`: `operator.validatorCount`. 1. `ProtocolLib.updateNetworkFee()`: `StorageProtocol.networkFeeIndex`, `StorageProtocol.networkFeeIndexBlockNumber` and `StorageProtocol.networkFee`. 1. `ProtocolLib.updateDAOEarnings()`: `StorageProtocol.daoBalance` and `StorageProtocol.daoIndexBlockNumber`. 1. `ProtocolLib.updateDAO()`: `StorageProtocol.daoValidatorCount`. 1. `ClusterLib.updateBalance()`: `cluster.balance`. 1. `ClusterLib.updateClusterData()`: `cluster.index` and `cluster.networkFeeIndex`.

At `SSVNetwork.sol#770`, comparisons are made against `block.timestamp`. Projects may rely on block timestamps for various purposes. Given that this project may, in principle, be deployed on any EVM chain, it is important to note that different networks may impose different requirements for their validators when it comes to setting the timestamp of a block, and attackers may be able to manipulate timestamps for their own purposes. If a smart contract relies on a timestamp, it must take this into account.

If there are enough operators removed for a validator, the consensus between operators may be affected for the particular validator. For example, if there are four operators, and two operators are removed, consensus cannot be reached among the operators. The validator will become idle and lose out on block proposal and attestation rewards. Based on the current implementation, the validator still owes payments to any active operators even though the setup is invalid.

The `SSVNetwork.removeOperator()` function allows operators to remove themselves from the clusters they were previously operating for. Although this action causes the operators to stop receiving SSV rewards, the clusters are not updated to reflect the operator's removal. As a result, if sufficiently many operators are removed, consensus may not be reached. In addition, if an operator stops running an operator node without calling `removeOperator()`, they will no longer contribute to the consensus threshold, but they will still receive SSV rewards from the cluster owner. Based on the current implementation, the validator still owes payments to any active operators even though the setup is invalid. Therefore, validators must track the performance of the operators they use to ensure the success of their validator and maximize their rewards.

The contracts are not designed to accommodate deflationary tokens when accepting token transfers. If a user transfers funds that incur a fee on transfer during `deposit()` or `reactivate()`, the contract will report that the user has deposited more funds than they actually sent. This can cause issues during withdrawal. This issue is provided as informational if payments are updated to accept other tokens.

The current implementation of the `updateNetworkFee()` function in the code snippet is emitting the entire network `fee` in the `NetworkFeeUpdated` event, despite the shrunken `fee` being saved in the `network` struct. This could lead to confusion and misinterpretation of the actual network fee. ``` Network memory network_ = network; DAO memory dao_ = dao; dao_.updateDAOEarnings(network.networkFee); dao = dao_; network_.networkFeeIndex = NetworkLib.currentNetworkFeeIndex(network_); network_.networkFeeIndexBlockNumber = uint64(block.number); emit NetworkFeeUpdated(network_.networkFee.expand(), fee); network_.networkFee = fee.shrink(); network = network_; ``` Inconsistent uses of shrunken fees were found in the following functions: 1. `fee` in `SSVNetwork.declareOperatorFee()` 2. `fee` in `SSVNetwork.reduceOperatorFee()` 3. `fee` in `SSVNetwork.registerOperator()` 4. `fee` in `SSVNetwork.updateNetworkFee()` 5. `amount` in `SSVNetwork.updateMinimumLiquidationCollateral()` 6. `fee` in `SSVNetwork.declareOperatorFee()` 7. `fee` in `SSVNetwork.reduceOperatorFee()`

The project uses Shamir Secret Sharing to split validator private keys into encrypted key shares for distribution to operators. However, suppose an operator is removed from a cluster and later added back in. The case where the private key would be split into the same key shares as in the initial configuration and a different encrypted key share would be sent to an operator should be avoided, as this may result in an operator potentially accumulating the quorum number of key shares, compromising the validator private key.

It is crucial to validate inputs, even if the inputs come from trusted addresses, to avoid human error. A lack of robust input validation can only increase the likelihood and impact in the event of mistakes. The following is the list of places that can potentially benefit from a stricter input validation: 1. `SSVNetwork.sol#L110`: the `minimumBlocksBeforeLiquidation_` of the `__SSVNetwork_init_unchained()` can be less than `MINIMAL_LIQUIDATION_THRESHOLD`. 1. `SSVNetwork.sol#L173`: the `recipientAddress` of the `setFeeRecipientAddress()` should not be zero address. 1. `SSVNetwork.sol#L180`: the `keccak256` of `publicKey` of the `registerValidator()` should not be zero bytes. 1. `SSVNetwork.sol#L180`: the length of `shares` of the `registerValidator()` should match the length of `operatorIds`. 1. `SSVNetwork.sol#L604`: the `amount` of `withdrawNetworkEarnings()` should be greater than zero. 1. **(Fix Review)** `ProtocolLib.sol#18`: the `fee` of the `updateNetworkFee()` function not checked to be non-zero or otherwise reasonably bound. 1. **(Fix Review)** `ProtocolLib.sol#L38`: the `deltaValidatorCount` of the `updateDAO()` function is not checked to be non-zero or otherwise reasonably bound. 1. **(Fix Review)** `SSVNetworkViews.sol#L58:` the `operatorId` of the `getOperatorById()` function is not checked to be an existing operator. 1. **(Fix Review)** `SSVDAO.sol#L45`: the `percentage` of the `updateOperatorFeeIncreaseLimit()` function is not checked to be non-zero or smaller than `PRECISION_FACTOR`.