Accounting oracle: remove an erroneous and excess check

contracts/0.8.9/StakingRouter.sol

@@ -299,27 +299,72 @@ contract StakingRouter is AccessControlEnumerable, BeaconChainDepositor, Version
         }
     }
 
+    /// @notice Updates total numbers of exited validators for staking modules with the specified
+    /// module ids.
+    ///
+    /// @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.
+    ///
+    /// 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:
+    ///
+    /// 1. In the first data submission phase, the oracle calls `updateExitedValidatorsCountByStakingModule` on the
+    ///    staking router, passing the totals by module. The staking router stores these totals and uses them to
+    ///    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
+    ///    `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
+    ///    `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
+    ///    (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
+    ///    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.
+    ///
     function updateExitedValidatorsCountByStakingModule(
         uint256[] calldata _stakingModuleIds,
         uint256[] calldata _exitedValidatorsCounts
     )
         external
         onlyRole(REPORT_EXITED_VALIDATORS_ROLE)
+        returns (uint256)
     {
         if (_stakingModuleIds.length != _exitedValidatorsCounts.length) {
             revert ArraysLengthMismatch(_stakingModuleIds.length, _exitedValidatorsCounts.length);
         }
 
-        uint256 stakingModuleId;
+        uint256 newlyExitedValidatorsCount;
+
         for (uint256 i = 0; i < _stakingModuleIds.length; ) {
-            stakingModuleId = _stakingModuleIds[i];
+            uint256 stakingModuleId = _stakingModuleIds[i];
             StakingModule storage stakingModule = _getStakingModuleById(stakingModuleId);
 
             uint256 prevReportedExitedValidatorsCount = stakingModule.exitedValidatorsCount;
             if (_exitedValidatorsCounts[i] < prevReportedExitedValidatorsCount) {
                 revert ExitedValidatorsCountCannotDecrease();
             }
 
+            newlyExitedValidatorsCount += _exitedValidatorsCounts[i] - prevReportedExitedValidatorsCount;
+
             (
                 uint256 totalExitedValidatorsCount,
                 /* uint256 totalDepositedValidators */,
@@ -337,8 +382,20 @@ contract StakingRouter is AccessControlEnumerable, BeaconChainDepositor, Version
             stakingModule.exitedValidatorsCount = _exitedValidatorsCounts[i];
             unchecked { ++i; }
         }
+
+        return newlyExitedValidatorsCount;
     }
 
+    /// @notice Updates exited validators counts per node operator for the staking module with
+    /// the specified id.
+    ///
+    /// See the docs for `updateExitedValidatorsCountByStakingModule` for the description of the
+    /// overall update process.
+    ///
+    /// @param _stakingModuleId The id of the staking modules to be updated.
+    /// @param _nodeOperatorIds Ids of the node operators to be updated.
+    /// @param _exitedValidatorsCounts New counts of exited validators for the specified node operators.
+    ///
     function reportStakingModuleExitedValidatorsCountByNodeOperator(
         uint256 _stakingModuleId,
         bytes calldata _nodeOperatorIds,
@@ -440,6 +497,16 @@ contract StakingRouter is AccessControlEnumerable, BeaconChainDepositor, Version
         }
     }
 
+    /// @notice Updates stuck validators counts per node operator for the staking module with
+    /// the specified id.
+    ///
+    /// See the docs for `updateExitedValidatorsCountByStakingModule` for the description of the
+    /// overall update process.
+    ///
+    /// @param _stakingModuleId The id of the staking modules to be updated.
+    /// @param _nodeOperatorIds Ids of the node operators to be updated.
+    /// @param _stuckValidatorsCounts New counts of stuck validators for the specified node operators.
+    ///
     function reportStakingModuleStuckValidatorsCountByNodeOperator(
         uint256 _stakingModuleId,
         bytes calldata _nodeOperatorIds,
@@ -453,6 +520,13 @@ contract StakingRouter is AccessControlEnumerable, BeaconChainDepositor, Version
         IStakingModule(moduleAddr).updateStuckValidatorsCount(_nodeOperatorIds, _stuckValidatorsCounts);
     }
 
+    /// @notice Called by the oracle when the second phase of data reporting finishes, i.e. when the
+    /// oracle submitted the complete data on the stuck and exited validator counts per node operator
+    /// for the current reporting frame.
+    ///
+    /// See the docs for `updateExitedValidatorsCountByStakingModule` for the description of the
+    /// overall update process.
+    ///
     function onValidatorsCountsByNodeOperatorReportingFinished()
         external
         onlyRole(REPORT_EXITED_VALIDATORS_ROLE)
@@ -479,16 +553,6 @@ contract StakingRouter is AccessControlEnumerable, BeaconChainDepositor, Version
         }
     }
 
-    function getExitedValidatorsCountAcrossAllModules() external view returns (uint256) {
-        uint256 stakingModulesCount = getStakingModulesCount();
-        uint256 exitedValidatorsCount = 0;
-        for (uint256 i; i < stakingModulesCount; ) {
-            exitedValidatorsCount += _getStakingModuleByIndex(i).exitedValidatorsCount;
-            unchecked { ++i; }
-        }
-        return exitedValidatorsCount;
-    }
-
     /**
      * @notice Returns all registered staking modules
      */

contracts/0.8.9/oracle/AccountingOracle.sol

@@ -57,12 +57,10 @@ interface IOracleReportSanityChecker {
 }
 
 interface IStakingRouter {
-    function getExitedValidatorsCountAcrossAllModules() external view returns (uint256);
-
     function updateExitedValidatorsCountByStakingModule(
         uint256[] calldata moduleIds,
         uint256[] calldata exitedValidatorsCounts
-    ) external;
+    ) external returns (uint256);
 
     function reportStakingModuleExitedValidatorsCountByNodeOperator(
         uint256 stakingModuleId,
@@ -96,7 +94,6 @@ contract AccountingOracle is BaseOracle {
     error IncorrectOracleMigration(uint256 code);
     error SenderNotAllowed();
     error InvalidExitedValidatorsData();
-    error NumExitedValidatorsCannotDecrease();
     error UnsupportedExtraDataFormat(uint256 format);
     error UnsupportedExtraDataType(uint256 itemIndex, uint256 dataType);
     error CannotSubmitExtraDataBeforeMainData();
@@ -657,32 +654,24 @@ contract AccountingOracle is BaseOracle {
             unchecked { ++i; }
         }
 
-        uint256 exitedValidators = 0;
         for (uint256 i = 0; i < stakingModuleIds.length;) {
             if (numExitedValidatorsByStakingModule[i] == 0) {
                 revert InvalidExitedValidatorsData();
-            } else {
-                exitedValidators += numExitedValidatorsByStakingModule[i];
             }
             unchecked { ++i; }
         }
 
-        uint256 prevExitedValidators = stakingRouter.getExitedValidatorsCountAcrossAllModules();
-        if (exitedValidators < prevExitedValidators) {
-            revert NumExitedValidatorsCannotDecrease();
-        }
+        uint256 newlyExitedValidatorsCount = stakingRouter.updateExitedValidatorsCountByStakingModule(
+            stakingModuleIds,
+            numExitedValidatorsByStakingModule
+        );
 
-        uint256 exitedValidatorsPerDay =
-            (exitedValidators - prevExitedValidators) * (1 days) /
+        uint256 exitedValidatorsRatePerDay =
+            newlyExitedValidatorsCount * (1 days) /
             (SECONDS_PER_SLOT * slotsElapsed);
 
         IOracleReportSanityChecker(LOCATOR.oracleReportSanityChecker())
-            .checkExitedValidatorsRatePerDay(exitedValidatorsPerDay);
-
-        stakingRouter.updateExitedValidatorsCountByStakingModule(
-            stakingModuleIds,
-            numExitedValidatorsByStakingModule
-        );
+            .checkExitedValidatorsRatePerDay(exitedValidatorsRatePerDay);
     }
 
     function _submitReportExtraDataEmpty() internal {

contracts/0.8.9/test_helpers/oracle/MockStakingRouterForAccountingOracle.sol

@@ -19,7 +19,8 @@ contract MockStakingRouterForAccountingOracle is IStakingRouter {
         bytes keysCounts;
     }
 
-    uint256 internal _exitedKeysCountAcrossAllModules;
+    mapping(uint256 => uint256) internal _exitedKeysCountsByModuleId;
+
     UpdateExitedKeysByModuleCallData internal _lastCall_updateExitedKeysByModule;
 
     ReportKeysByNodeOperatorCallData[] public calls_reportExitedKeysByNodeOperator;
@@ -28,10 +29,6 @@ contract MockStakingRouterForAccountingOracle is IStakingRouter {
     uint256 public totalCalls_onValidatorsCountsByNodeOperatorReportingFinished;
 
 
-    function setExitedKeysCountAcrossAllModules(uint256 count) external {
-        _exitedKeysCountAcrossAllModules = count;
-    }
-
     function lastCall_updateExitedKeysByModule()
         external view returns (UpdateExitedKeysByModuleCallData memory)
     {
@@ -50,17 +47,23 @@ contract MockStakingRouterForAccountingOracle is IStakingRouter {
     /// IStakingRouter
     ///
 
-    function getExitedValidatorsCountAcrossAllModules() external view returns (uint256) {
-        return _exitedKeysCountAcrossAllModules;
-    }
-
     function updateExitedValidatorsCountByStakingModule(
         uint256[] calldata moduleIds,
         uint256[] calldata exitedKeysCounts
-    ) external {
+    ) external returns (uint256) {
         _lastCall_updateExitedKeysByModule.moduleIds = moduleIds;
         _lastCall_updateExitedKeysByModule.exitedKeysCounts = exitedKeysCounts;
         ++_lastCall_updateExitedKeysByModule.callCount;
+
+        uint256 newlyExitedValidatorsCount;
+
+        for (uint256 i = 0; i < moduleIds.length; ++i) {
+            uint256 moduleId = moduleIds[i];
+            newlyExitedValidatorsCount += exitedKeysCounts[i] - _exitedKeysCountsByModuleId[moduleId];
+            _exitedKeysCountsByModuleId[moduleId] = exitedKeysCounts[i];
+        }
+
+        return newlyExitedValidatorsCount;
     }
 
     function reportStakingModuleExitedValidatorsCountByNodeOperator(

lib/abi/IStakingRouter.json

@@ -1 +1 @@
-[{"inputs":[],"name":"getExitedValidatorsCountAcrossAllModules","outputs":[{"internalType":"uint256","name":"","type":"uint256"}],"stateMutability":"view","type":"function"},{"inputs":[],"name":"onValidatorsCountsByNodeOperatorReportingFinished","outputs":[],"stateMutability":"nonpayable","type":"function"},{"inputs":[{"internalType":"uint256","name":"stakingModuleId","type":"uint256"},{"internalType":"bytes","name":"nodeOperatorIds","type":"bytes"},{"internalType":"bytes","name":"exitedValidatorsCounts","type":"bytes"}],"name":"reportStakingModuleExitedValidatorsCountByNodeOperator","outputs":[],"stateMutability":"nonpayable","type":"function"},{"inputs":[{"internalType":"uint256","name":"stakingModuleId","type":"uint256"},{"internalType":"bytes","name":"nodeOperatorIds","type":"bytes"},{"internalType":"bytes","name":"stuckValidatorsCounts","type":"bytes"}],"name":"reportStakingModuleStuckValidatorsCountByNodeOperator","outputs":[],"stateMutability":"nonpayable","type":"function"},{"inputs":[{"internalType":"uint256[]","name":"moduleIds","type":"uint256[]"},{"internalType":"uint256[]","name":"exitedValidatorsCounts","type":"uint256[]"}],"name":"updateExitedValidatorsCountByStakingModule","outputs":[],"stateMutability":"nonpayable","type":"function"}]
+[{"inputs":[],"name":"onValidatorsCountsByNodeOperatorReportingFinished","outputs":[],"stateMutability":"nonpayable","type":"function"},{"inputs":[{"internalType":"uint256","name":"stakingModuleId","type":"uint256"},{"internalType":"bytes","name":"nodeOperatorIds","type":"bytes"},{"internalType":"bytes","name":"exitedValidatorsCounts","type":"bytes"}],"name":"reportStakingModuleExitedValidatorsCountByNodeOperator","outputs":[],"stateMutability":"nonpayable","type":"function"},{"inputs":[{"internalType":"uint256","name":"stakingModuleId","type":"uint256"},{"internalType":"bytes","name":"nodeOperatorIds","type":"bytes"},{"internalType":"bytes","name":"stuckValidatorsCounts","type":"bytes"}],"name":"reportStakingModuleStuckValidatorsCountByNodeOperator","outputs":[],"stateMutability":"nonpayable","type":"function"},{"inputs":[{"internalType":"uint256[]","name":"moduleIds","type":"uint256[]"},{"internalType":"uint256[]","name":"exitedValidatorsCounts","type":"uint256[]"}],"name":"updateExitedValidatorsCountByStakingModule","outputs":[{"internalType":"uint256","name":"","type":"uint256"}],"stateMutability":"nonpayable","type":"function"}]

test/0.8.9/oracle/accounting-oracle-submit-report-data.test.js

@@ -379,27 +379,6 @@ contract('AccountingOracle', ([admin, member1]) => {
         )
       })
 
-      it('reverts with NumExitedValidatorsCannotDecrease if total count of exited validators less then previous exited number', async () => {
-        const totalExitedValidators = reportFields.numExitedValidatorsByStakingModule.reduce(
-          (sum, curr) => sum + curr,
-          0
-        )
-        await mockStakingRouter.setExitedKeysCountAcrossAllModules(totalExitedValidators + 1)
-        await assert.reverts(
-          oracle.submitReportData(reportItems, oracleVersion, { from: member1 }),
-          'NumExitedValidatorsCannotDecrease()'
-        )
-      })
-
-      it('does not reverts with NumExitedValidatorsCannotDecrease if total count of exited validators equals to previous exited number', async () => {
-        const totalExitedValidators = reportFields.numExitedValidatorsByStakingModule.reduce(
-          (sum, curr) => sum + curr,
-          0
-        )
-        await mockStakingRouter.setExitedKeysCountAcrossAllModules(totalExitedValidators)
-        await oracle.submitReportData(reportItems, oracleVersion, { from: member1 })
-      })
-
       it('reverts with ExitedValidatorsLimitExceeded if exited validators rate limit will be reached', async () => {
         // Really simple test here for now
         // TODO: Come up with more tests for better coverage of edge-case scenarios that can be accrued

test/0.8.9/staking-router/report-exited-keys.test.js

@@ -1,7 +1,7 @@
 const { contract, ethers } = require('hardhat')
 const { assert } = require('../../helpers/assert')
 const { EvmSnapshot } = require('../../helpers/blockchain')
-const { hexConcat, hex, ETH } = require('../../helpers/utils')
+const { hexConcat, hex, ETH, addSendWithResult } = require('../../helpers/utils')
 const { deployProtocol } = require('../../helpers/protocol')
 const { setupNodeOperatorsRegistry } = require('../../helpers/staking-modules')
 
@@ -26,6 +26,7 @@ contract('StakingRouter', ([admin, depositor]) => {
     })
 
     router = deployed.stakingRouter
+    addSendWithResult(router.updateExitedValidatorsCountByStakingModule)
     voting = deployed.voting.address
     operators = await setupNodeOperatorsRegistry(deployed, true)
     module2 = await setupNodeOperatorsRegistry(deployed, true)
@@ -154,9 +155,16 @@ contract('StakingRouter', ([admin, depositor]) => {
       assert.equal(+distribution.shares[0], op1shareBefore * sharesDistribute)
       assert.equal(+distribution.shares[1], op2shareBefore * sharesDistribute)
 
-      // //update exited validators
+      // update exited validators
       const exitValidatorsCount = 20
-      await router.updateExitedValidatorsCountByStakingModule([module1Id], [exitValidatorsCount], { from: admin })
+      const newlyExitedValidatorsCount = await router.updateExitedValidatorsCountByStakingModule.sendWithResult(
+        [module1Id],
+        [exitValidatorsCount],
+        {
+          from: admin,
+        }
+      )
+      assert.equals(newlyExitedValidatorsCount, exitValidatorsCount)
 
       const nodeOpIds = [0]
       const exitedValidatorsCounts = [exitValidatorsCount]
@@ -219,7 +227,12 @@ contract('StakingRouter', ([admin, depositor]) => {
 
       // //update exited validators
       const exitValidatorsCount = 20
-      await router.updateExitedValidatorsCountByStakingModule([module1Id], [exitValidatorsCount], { from: admin })
+      const newlyExitedCount = await router.updateExitedValidatorsCountByStakingModule.sendWithResult(
+        [module1Id],
+        [exitValidatorsCount],
+        { from: admin }
+      )
+      assert.equals(newlyExitedCount, exitValidatorsCount)
 
       const nodeOpIds = [0]
       const exitedValidatorsCounts = [exitValidatorsCount]
@@ -289,9 +302,13 @@ contract('StakingRouter', ([admin, depositor]) => {
       assert.deepEqual([15, 5], await maxDepositsPerModule())
 
       // update exited validators
-      let exitValidatorsCount = 1
-      await router.updateExitedValidatorsCountByStakingModule([module1Id], [exitValidatorsCount], { from: admin })
-
+      const exitValidatorsCount = 1
+      const exitedCount = await router.updateExitedValidatorsCountByStakingModule.sendWithResult(
+        [module1Id],
+        [exitValidatorsCount],
+        { from: admin }
+      )
+      assert.equals(exitValidatorsCount, exitedCount)
       const nodeOpIds = [0]
       let exitedValidatorsCounts = [exitValidatorsCount]
 
@@ -314,11 +331,16 @@ contract('StakingRouter', ([admin, depositor]) => {
       assert.deepEqual([16, 5], maxDepositsPerModuleAfterAlloc)
 
       // update next exited validators
-      exitValidatorsCount = 30
-      exitedValidatorsCounts = [exitValidatorsCount]
+      const nextExitValidatorsCount = 30
+      exitedValidatorsCounts = [nextExitValidatorsCount]
       keysData = hexConcat(...exitedValidatorsCounts.map((c) => hex(c, 16)))
 
-      await router.updateExitedValidatorsCountByStakingModule([module1Id], [exitValidatorsCount], { from: admin })
+      const newlyExitedCount = await router.updateExitedValidatorsCountByStakingModule.sendWithResult(
+        [module1Id],
+        [nextExitValidatorsCount],
+        { from: admin }
+      )
+      assert.equals(newlyExitedCount, nextExitValidatorsCount - exitValidatorsCount)
       // report exited by module and node operator
       await router.reportStakingModuleExitedValidatorsCountByNodeOperator(module1Id, nodeOpIdsData, keysData, {
         from: admin,