Merge pull request #717 from lidofinance/fix/lido-fee-distribution

Fix/lido fee distribution

Author: TheDZhon

contracts/0.4.24/Lido.sol

@@ -112,6 +112,8 @@ interface IStakingRouter {
         external
         view
         returns (uint256);
+
+    function TOTAL_BASIS_POINTS() external view returns (uint256);
 }
 
 interface IWithdrawalQueue {
@@ -673,7 +675,7 @@ contract Lido is Versioned, StETHPermit, AragonApp {
      * Depends on the bunker state and protocol's pause state
      */
     function canDeposit() public view returns (bool) {
-        return !IWithdrawalQueue(getLidoLocator().withdrawalQueue()).isBunkerModeActive() && !isStopped();
+        return !_withdrawalQueue().isBunkerModeActive() && !isStopped();
     }
 
     /**
@@ -682,7 +684,7 @@ contract Lido is Versioned, StETHPermit, AragonApp {
      */
     function getDepositableEther() public view returns (uint256) {
         uint256 bufferedEther = _getBufferedEther();
-        uint256 withdrawalReserve = IWithdrawalQueue(getLidoLocator().withdrawalQueue()).unfinalizedStETH();
+        uint256 withdrawalReserve = _withdrawalQueue().unfinalizedStETH();
         return bufferedEther > withdrawalReserve ? bufferedEther - withdrawalReserve : 0;
     }
 
@@ -698,7 +700,7 @@ contract Lido is Versioned, StETHPermit, AragonApp {
         require(msg.sender == locator.depositSecurityModule(), "APP_AUTH_DSM_FAILED");
         require(canDeposit(), "CAN_NOT_DEPOSIT");
 
-        IStakingRouter stakingRouter = IStakingRouter(locator.stakingRouter());
+        IStakingRouter stakingRouter = _stakingRouter();
         uint256 depositsCount = Math256.min(
             _maxDepositsCount,
             stakingRouter.getStakingModuleMaxDepositsCount(_stakingModuleId, getDepositableEther())
@@ -730,7 +732,7 @@ contract Lido is Versioned, StETHPermit, AragonApp {
      * @dev DEPRECATED: use StakingRouter.getWithdrawalCredentials() instead
      */
     function getWithdrawalCredentials() external view returns (bytes32) {
-        return IStakingRouter(getLidoLocator().stakingRouter()).getWithdrawalCredentials();
+        return _stakingRouter().getWithdrawalCredentials();
     }
 
     /**
@@ -746,7 +748,7 @@ contract Lido is Versioned, StETHPermit, AragonApp {
      * @dev DEPRECATED: use LidoLocator.treasury()
      */
     function getTreasury() external view returns (address) {
-        return getLidoLocator().treasury();
+        return _treasury();
     }
 
     /**
@@ -757,11 +759,11 @@ contract Lido is Versioned, StETHPermit, AragonApp {
      * inaccurate because the actual value is truncated here to 1e4 precision.
      */
     function getFee() external view returns (uint16 totalFee) {
-        totalFee = IStakingRouter(getLidoLocator().stakingRouter()).getTotalFeeE4Precision();
+        totalFee = _stakingRouter().getTotalFeeE4Precision();
     }
 
     /**
-     * @notice Returns current fee distribution
+     * @notice Returns current fee distribution, values relative to the total fee (getFee())
      * @dev DEPRECATED: Now fees information is stored in StakingRouter and
      * with higher precision. Use StakingRouter.getStakingFeeAggregateDistribution() instead.
      * @return treasuryFeeBasisPoints return treasury fee in TOTAL_BASIS_POINTS (10000 is 100% fee) precision
@@ -780,9 +782,15 @@ contract Lido is Versioned, StETHPermit, AragonApp {
             uint16 operatorsFeeBasisPoints
         )
     {
-        insuranceFeeBasisPoints = 0;  // explicitly set to zero
-        (treasuryFeeBasisPoints, operatorsFeeBasisPoints) = IStakingRouter(getLidoLocator().stakingRouter())
+        IStakingRouter stakingRouter = _stakingRouter();
+        uint256 totalBasisPoints = stakingRouter.TOTAL_BASIS_POINTS();
+        uint256 totalFee = stakingRouter.getTotalFeeE4Precision();
+        (uint256 treasuryFeeBasisPointsAbs, uint256 operatorsFeeBasisPointsAbs) = stakingRouter
             .getStakingFeeAggregateDistributionE4Precision();
+
+        insuranceFeeBasisPoints = 0;  // explicitly set to zero
+        treasuryFeeBasisPoints = uint16((treasuryFeeBasisPointsAbs * totalBasisPoints) / totalFee);
+        operatorsFeeBasisPoints = uint16((operatorsFeeBasisPointsAbs * totalBasisPoints) / totalFee);
     }
 
     /*
@@ -970,7 +978,7 @@ contract Lido is Versioned, StETHPermit, AragonApp {
         StakingRewardsDistribution memory ret,
         IStakingRouter router
     ) {
-        router = IStakingRouter(getLidoLocator().stakingRouter());
+        router = _stakingRouter();
 
         (
             ret.recipients,
@@ -1075,7 +1083,7 @@ contract Lido is Versioned, StETHPermit, AragonApp {
     }
 
     function _transferTreasuryRewards(uint256 treasuryReward) internal {
-        address treasury = getLidoLocator().treasury();
+        address treasury = _treasury();
         _transferShares(address(this), treasury, treasuryReward);
         _emitTransferAfterMintingShares(treasury, treasuryReward);
     }
@@ -1371,4 +1379,16 @@ contract Lido is Versioned, StETHPermit, AragonApp {
             ret.postTokenRebaseReceiver
         ) = getLidoLocator().oracleReportComponentsForLido();
     }
+
+    function _stakingRouter() internal view returns (IStakingRouter) {
+        return IStakingRouter(getLidoLocator().stakingRouter());
+    }
+
+    function _withdrawalQueue() internal view returns (IWithdrawalQueue) {
+        return IWithdrawalQueue(getLidoLocator().withdrawalQueue());
+    }
+
+    function _treasury() internal view returns (address) {
+        return getLidoLocator().treasury();
+    }
 }

contracts/0.8.9/StakingRouter.sol

@@ -319,7 +319,7 @@ contract StakingRouter is AccessControlEnumerable, BeaconChainDepositor, Version
     /// @param _stakingModuleIds Ids of the staking modules to be updated.
     /// @param _exitedValidatorsCounts New counts of exited validators for the specified staking modules.
     ///
-    /// @return The total increase in the aggregate number of exited validators accross all updated modules.
+    /// @return The total increase in the aggregate number of exited validators across all updated modules.
     ///
     /// The total numbers are stored in the staking router and can differ from the totals obtained by calling
     /// `IStakingModule.getStakingModuleSummary()`. The overall process of updating validator counts is the following:
@@ -329,27 +329,27 @@ contract StakingRouter is AccessControlEnumerable, BeaconChainDepositor, Version
     ///    distribute new stake and staking fees between the modules. There can only be single call of this function
     ///    per oracle reporting frame.
     ///
-    /// 2. In the first part of the second data submittion phase, the oracle calls
+    /// 2. In the first part of the second data submission phase, the oracle calls
     ///    `StakingRouter.reportStakingModuleStuckValidatorsCountByNodeOperator` on the staking router which passes the
     ///    counts by node operator to the staking module by calling `IStakingModule.updateStuckValidatorsCount`.
     ///    This can be done multiple times for the same module, passing data for different subsets of node operators.
     ///
-    /// 3. In the second part of the second data submittion phase, the oracle calls
+    /// 3. In the second part of the second data submission phase, the oracle calls
     ///    `StakingRouter.reportStakingModuleExitedValidatorsCountByNodeOperator` on the staking router which passes
     ///    the counts by node operator to the staking module by calling `IStakingModule.updateExitedValidatorsCount`.
     ///    This can be done multiple times for the same module, passing data for different subsets of node
     ///    operators.
     ///
-    /// 4. At the end of the second data submission phase, it's expected for the aggragate exited validators count
-    ///    accross all module's node operators (stored in the module) to match the total count for this module
+    /// 4. At the end of the second data submission phase, it's expected for the aggregate exited validators count
+    ///    across all module's node operators (stored in the module) to match the total count for this module
     ///    (stored in the staking router). However, it might happen that the second phase of data submission doesn't
     ///    finish until the new oracle reporting frame is started, in which case staking router will emit a warning
     ///    event `StakingModuleExitedValidatorsIncompleteReporting` when the first data submission phase is performed
     ///    for a new reporting frame. This condition will result in the staking module having an incomplete data about
     ///    the exited and maybe stuck validator counts during the whole reporting frame. Handling this condition is
     ///    the responsibility of each staking module.
     ///
-    /// 5. When the second reporting phase is finshed, i.e. when the oracle submitted the complete data on the stuck
+    /// 5. When the second reporting phase is finished, i.e. when the oracle submitted the complete data on the stuck
     ///    and exited validator counts per node operator for the current reporting frame, the oracle calls
     ///    `StakingRouter.onValidatorsCountsByNodeOperatorReportingFinished` which, in turn, calls
     ///    `IStakingModule.onExitedAndStuckValidatorsCountsUpdated` on all modules.

test/scenario/lido_happy_path.test.js

@@ -14,6 +14,13 @@ const { INITIAL_HOLDER } = require('../helpers/constants')
 
 const NodeOperatorsRegistry = artifacts.require('NodeOperatorsRegistry')
 const CURATED_MODULE_ID = 1
+const TOTAL_BASIS_POINTS = 10000
+const CURATED_MODULE_MODULE_FEE = 500
+const CURATED_MODULE_TREASURY_FEE = 500
+
+// Fee and its distribution are in basis points, 10000 corresponding to 100%
+// Total fee is 10%
+const totalFeePoints = 0.1 * TOTAL_BASIS_POINTS
 
 contract('Lido: happy path', (addresses) => {
   const [
@@ -41,14 +48,14 @@ contract('Lido: happy path', (addresses) => {
     async () => {
       const deployed = await deployProtocol({
         stakingModulesFactory: async (protocol) => {
-          const curatedModule = await setupNodeOperatorsRegistry(protocol)
+          const curatedModule = await setupNodeOperatorsRegistry(protocol, true)
           return [
             {
               module: curatedModule,
               name: 'Curated',
               targetShares: 10000,
-              moduleFee: 500,
-              treasuryFee: 500,
+              moduleFee: CURATED_MODULE_MODULE_FEE,
+              treasuryFee: CURATED_MODULE_TREASURY_FEE,
             },
           ]
         },
@@ -84,22 +91,6 @@ contract('Lido: happy path', (addresses) => {
     }
   )
 
-  // Fee and its distribution are in basis points, 10000 corresponding to 100%
-
-  // Total fee is 10%
-  const totalFeePoints = 0.1 * 10000
-
-  it.skip('voting sets fee and its distribution', async () => {
-    // Fee and distribution were set
-    assert.equals(await pool.getFee({ from: nobody }), totalFeePoints, 'total fee')
-    const distribution = await pool.getFeeDistribution({ from: nobody })
-    console.log('distribution', distribution)
-    const treasuryFeePoints = 0 // TODO
-    const nodeOperatorsFeePoints = 0 // TODO
-    assert.equals(distribution.treasuryFeeBasisPoints, treasuryFeePoints, 'treasury fee')
-    assert.equals(distribution.operatorsFeeBasisPoints, nodeOperatorsFeePoints, 'node operators fee')
-  })
-
   it('voting sets withdrawal credentials', async () => {
     const wc = '0x'.padEnd(66, '1234')
     assert.equal(await pool.getWithdrawalCredentials({ from: nobody }), wc, 'withdrawal credentials')
@@ -546,4 +537,30 @@ contract('Lido: happy path', (addresses) => {
     nodeOperatorInfo = await nodeOperatorsRegistry.getNodeOperator(nodeOperator2.id, false)
     assert.equals(nodeOperatorInfo.totalSigningKeys, nodeOperatorInfo.usedSigningKeys)
   })
+
+  it('getFee and getFeeDistribution works as expected', async () => {
+    // Need to have at least single deposited key, otherwise StakingRouter.getStakingRewardsDistribution
+    // will return zero fees because no modules with non-zero total active validators
+    // This is done in the changes in the tests above, but assuming there are no such changes
+    // one could use the following:
+    // await nodeOperatorsRegistry.increaseNodeOperatorDepositedSigningKeysCount(0, 1)
+
+    function getFeeRelativeToTotalFee(absoluteFee) {
+      return (absoluteFee * TOTAL_BASIS_POINTS) / totalFeePoints
+    }
+
+    assert.equals(await pool.getFee({ from: nobody }), totalFeePoints, 'total fee')
+    const distribution = await pool.getFeeDistribution({ from: nobody })
+    assert.equals(distribution.insuranceFeeBasisPoints, 0, 'insurance fee')
+    assert.equals(
+      distribution.treasuryFeeBasisPoints,
+      getFeeRelativeToTotalFee(CURATED_MODULE_TREASURY_FEE),
+      'treasury fee'
+    )
+    assert.equals(
+      distribution.operatorsFeeBasisPoints,
+      getFeeRelativeToTotalFee(CURATED_MODULE_MODULE_FEE),
+      'node operators fee'
+    )
+  })
 })