Merge pull request #496 from lidofinance/staking_router_tests_and_fixes

staking router tests and fixes

contracts/0.8.9/BeaconChainDepositor.sol

@@ -1,4 +1,4 @@
-// SPDX-FileCopyrightText: 2022 Lido <[email protected]>2
+// SPDX-FileCopyrightText: 2022 Lido <[email protected]>
 // SPDX-License-Identifier: GPL-3.0
 
 // See contracts/COMPILERS.md

contracts/0.8.9/interfaces/IStakingRouter.sol

@@ -9,7 +9,7 @@ interface IStakingRouter {
     function getStakingRewardsDistribution()
         external
         view
-        returns (address[] memory recipients, uint96[] memory moduleFees, uint96 totalFeee, uint256 precisionPoints);
+        returns (address[] memory recipients, uint96[] memory stakingModuleFees, uint96 totalFeee, uint256 precisionPoints);
 
     function deposit(uint256 maxDepositsCount, uint24 stakingModuleId, bytes calldata depositCalldata) external payable returns (uint256);
 
@@ -32,37 +32,37 @@ interface IStakingRouter {
     }
 
     struct StakingModule {
-        /// @notice unique id of the module
+        /// @notice unique id of the staking module
         uint24 id;
-        /// @notice address of module
+        /// @notice address of staking module
         address stakingModuleAddress;
-        /// @notice rewarf fee of the module
-        uint16 moduleFee;
+        /// @notice rewarf fee of the staking module
+        uint16 stakingModuleFee;
         /// @notice treasury fee
         uint16 treasuryFee;
         /// @notice target percent of total keys in protocol, in BP
         uint16 targetShare;
-        /// @notice module status if module can not accept the deposits or can participate in further reward distribution
+        /// @notice staking module status if staking module can not accept the deposits or can participate in further reward distribution
         uint8 status;
-        /// @notice name of module
+        /// @notice name of staking module
         string name;
-        /// @notice block.timestamp of the last deposit of the module
+        /// @notice block.timestamp of the last deposit of the staking module
         uint64 lastDepositAt;
-        /// @notice block.number of the last deposit of the module
+        /// @notice block.number of the last deposit of the staking module
         uint256 lastDepositBlock;
     }
 
     function getStakingModules() external view returns (StakingModule[] memory res);
 
-    function addModule(
+    function addStakingModule(
         string memory _name,
         address _stakingModuleAddress,
         uint16 _targetShare,
-        uint16 _moduleFee,
+        uint16 _stakingModuleFee,
         uint16 _treasuryFee
     ) external;
 
-    function updateStakingModule(uint24 _stakingModuleId, uint16 _targetShare, uint16 _moduleFee, uint16 _treasuryFee) external;
+    function updateStakingModule(uint24 _stakingModuleId, uint16 _targetShare, uint16 _stakingModuleFee, uint16 _treasuryFee) external;
 
     function getStakingModule(uint24 _stakingModuleId) external view returns (StakingModule memory);
 

contracts/0.8.9/test_helpers/StakingModuleMock.sol

@@ -10,6 +10,7 @@ import {IStakingModule} from "../interfaces/IStakingModule.sol";
 contract StakingModuleMock is IStakingModule {
     uint256 private _activeKeysCount;
     uint256 private _availableKeysCount;
+    uint256 private _keysNonce;
 
     function getActiveKeysCount() public view returns (uint256) {
         return _activeKeysCount;
@@ -42,7 +43,13 @@ contract StakingModuleMock is IStakingModule {
         readyToDepositValidatorsKeysCount = _availableKeysCount;
     }
 
-    function getValidatorsKeysNonce() external view returns (uint256) {}
+    function getValidatorsKeysNonce() external view returns (uint256) {
+        return _keysNonce;
+    }
+
+    function setValidatorsKeysNonce(uint256 _newKeysNonce) external {
+        _keysNonce = _newKeysNonce;
+    }
 
     function getNodeOperatorsCount() external view returns (uint256) {}
 
@@ -62,7 +69,9 @@ contract StakingModuleMock is IStakingModule {
 
     function updateExitedValidatorsKeysCount(uint256 _nodeOperatorId, uint256 _exitedValidatorsCount) external {}
 
-    function invalidateReadyToDepositKeys() external {}
+    function invalidateReadyToDepositKeys() external {
+        _availableKeysCount = _activeKeysCount;
+    }
 
     function requestValidatorsKeysForDeposits(uint256 _keysCount, bytes calldata _calldata)
         external

contracts/0.8.9/test_helpers/StakingRouterMock.sol

@@ -12,5 +12,9 @@ contract StakingRouterMock is StakingRouter {
         // unlock impl
         _setContractVersion(0);
     }
+
+    function getStakingModuleIndexById(uint24 _stakingModuleId) external view returns (uint256) {
+        return _getStakingModuleIndexById(_stakingModuleId);
+    }
 }
 

contracts/0.8.9/test_helpers/StakingRouterMockForDepositSecurityModule.sol

@@ -17,7 +17,7 @@ contract StakingRouterMockForDepositSecurityModule is IStakingRouter {
 
     function getStakingModules() external view returns (StakingModule[] memory res) {}
 
-    function addModule(
+    function addStakingModule(
         string memory _name,
         address _stakingModuleAddress,
         uint16 _targetShare,

test/0.4.24/lido.rewards-distribution.test.js

@@ -100,34 +100,48 @@ contract('Lido', ([appManager, voting, user2]) => {
     await stakingRouter.initialize(appManager, app.address, wc)
 
     // Set up the staking router permissions.
-    const MODULE_MANAGE_ROLE = await stakingRouter.MODULE_MANAGE_ROLE()
+    const STAKING_MODULE_MANAGE_ROLE = await stakingRouter.STAKING_MODULE_MANAGE_ROLE()
 
-    await stakingRouter.grantRole(MODULE_MANAGE_ROLE, voting, { from: appManager })
+    await stakingRouter.grantRole(STAKING_MODULE_MANAGE_ROLE, voting, { from: appManager })
 
     await app.setStakingRouter(stakingRouter.address, { from: voting })
 
     soloModule = await ModuleSolo.new(app.address, { from: appManager })
 
-    await stakingRouter.addModule('Curated', curatedModule.address, cfgCurated.targetShare, cfgCurated.moduleFee, cfgCurated.treasuryFee, {
-      from: voting
-    })
+    await stakingRouter.addStakingModule(
+      'Curated',
+      curatedModule.address,
+      cfgCurated.targetShare,
+      cfgCurated.moduleFee,
+      cfgCurated.treasuryFee,
+      {
+        from: voting
+      }
+    )
 
     await curatedModule.increaseTotalSigningKeysCount(500_000, { from: appManager })
     await curatedModule.increaseDepositedSigningKeysCount(499_950, { from: appManager })
     await curatedModule.increaseVettedSigningKeysCount(499_950, { from: appManager })
 
-    await stakingRouter.addModule('Solo', soloModule.address, cfgCommunity.targetShare, cfgCommunity.moduleFee, cfgCommunity.treasuryFee, {
-      from: voting
-    })
+    await stakingRouter.addStakingModule(
+      'Solo',
+      soloModule.address,
+      cfgCommunity.targetShare,
+      cfgCommunity.moduleFee,
+      cfgCommunity.treasuryFee,
+      {
+        from: voting
+      }
+    )
     await soloModule.setTotalKeys(100, { from: appManager })
     await soloModule.setTotalUsedKeys(10, { from: appManager })
     await soloModule.setTotalStoppedKeys(0, { from: appManager })
   })
 
   it('Rewards distribution fills treasury', async () => {
     const beaconBalance = ETH(1)
-    const { moduleFees, totalFee, precisionPoints } = await stakingRouter.getStakingRewardsDistribution()
-    const treasuryShare = moduleFees.reduce((total, share) => total.sub(share), totalFee)
+    const { stakingModuleFees, totalFee, precisionPoints } = await stakingRouter.getStakingRewardsDistribution()
+    const treasuryShare = stakingModuleFees.reduce((total, share) => total.sub(share), totalFee)
     const treasuryRewards = bn(beaconBalance).mul(treasuryShare).div(precisionPoints)
     await app.submit(ZERO_ADDRESS, { from: user2, value: ETH(32) })
 
@@ -141,7 +155,7 @@ contract('Lido', ([appManager, voting, user2]) => {
 
   it('Rewards distribution fills modules', async () => {
     const beaconBalance = ETH(1)
-    const { recipients, moduleFees, precisionPoints } = await stakingRouter.getStakingRewardsDistribution()
+    const { recipients, stakingModuleFees, precisionPoints } = await stakingRouter.getStakingRewardsDistribution()
 
     await app.submit(ZERO_ADDRESS, { from: user2, value: ETH(32) })
 
@@ -154,7 +168,7 @@ contract('Lido', ([appManager, voting, user2]) => {
 
     for (let i = 0; i < recipients.length; i++) {
       const moduleBalanceAfter = await app.balanceOf(recipients[i])
-      const moduleRewards = bn(beaconBalance).mul(moduleFees[i]).div(precisionPoints)
+      const moduleRewards = bn(beaconBalance).mul(stakingModuleFees[i]).div(precisionPoints)
       assert(moduleBalanceAfter.gt(moduleBalanceBefore[i]))
       assertBn(fixRound(moduleBalanceBefore[i].add(moduleRewards)), fixRound(moduleBalanceAfter))
     }

test/0.4.24/lido.test.js

@@ -1,12 +1,12 @@
 const { hash } = require('eth-ens-namehash')
-const { assert } = require('chai')
 const { newDao, newApp } = require('./helpers/dao')
 const { getInstalledApp } = require('@aragon/contract-helpers-test/src/aragon-os')
 const { assertBn, assertRevert, assertEvent } = require('@aragon/contract-helpers-test/src/asserts')
 const { ZERO_ADDRESS, bn, getEventAt } = require('@aragon/contract-helpers-test')
 const { BN } = require('bn.js')
 const { formatEther } = require('ethers/lib/utils')
 const { getEthBalance, formatStEth, formatBN, hexConcat, pad, ETH, tokens } = require('../helpers/utils')
+const { assert } = require('../helpers/assert')
 const nodeOperators = require('../helpers/node-operators')
 
 const NodeOperatorsRegistry = artifacts.require('NodeOperatorsRegistry')
@@ -79,7 +79,7 @@ contract('Lido', ([appManager, voting, user1, user2, user3, nobody, depositor, t
     stakingRouter = await StakingRouter.new(depositContract.address)
     await stakingRouter.initialize(appManager, app.address, ZERO_ADDRESS)
     await stakingRouter.grantRole(await stakingRouter.MANAGE_WITHDRAWAL_CREDENTIALS_ROLE(), voting, { from: appManager })
-    await stakingRouter.grantRole(await stakingRouter.MODULE_MANAGE_ROLE(), voting, { from: appManager })
+    await stakingRouter.grantRole(await stakingRouter.STAKING_MODULE_MANAGE_ROLE(), voting, { from: appManager })
 
     // BeaconChainDepositor
     beaconChainDepositor = await BeaconChainDepositorMock.new(depositContract.address)
@@ -130,7 +130,7 @@ contract('Lido', ([appManager, voting, user1, user2, user3, nobody, depositor, t
     // Initialize the app's proxy.
     await app.initialize(oracle.address, treasury, stakingRouter.address, depositor)
 
-    await stakingRouter.addModule(
+    await stakingRouter.addStakingModule(
       'Curated',
       operators.address,
       10_000, // 100 % _targetShare
@@ -348,37 +348,37 @@ contract('Lido', ([appManager, voting, user1, user2, user3, nobody, depositor, t
 
     let module1 = await stakingRouter.getStakingModule(curated.id)
     assertBn(module1.targetShare, 10000)
-    assertBn(module1.moduleFee, 500)
+    assertBn(module1.stakingModuleFee, 500)
     assertBn(module1.treasuryFee, 500)
 
-    // stakingModuleId, targetShare, moduleFee, treasuryFee
+    // stakingModuleId, targetShare, stakingModuleFee, treasuryFee
     await stakingRouter.updateStakingModule(module1.id, module1.targetShare, 300, 700, { from: voting })
 
     module1 = await stakingRouter.getStakingModule(curated.id)
 
     await assertRevert(
       stakingRouter.updateStakingModule(module1.id, module1.targetShare, 300, 700, { from: user1 }),
-      `AccessControl: account ${user1.toLowerCase()} is missing role ${await stakingRouter.MODULE_MANAGE_ROLE()}`
+      `AccessControl: account ${user1.toLowerCase()} is missing role ${await stakingRouter.STAKING_MODULE_MANAGE_ROLE()}`
     )
 
     await assertRevert(
       stakingRouter.updateStakingModule(module1.id, module1.targetShare, 300, 700, { from: nobody }),
-      `AccessControl: account ${nobody.toLowerCase()} is missing role ${await stakingRouter.MODULE_MANAGE_ROLE()}`
+      `AccessControl: account ${nobody.toLowerCase()} is missing role ${await stakingRouter.STAKING_MODULE_MANAGE_ROLE()}`
     )
 
-    await assertRevert(
+    await assert.revertsWithCustomError(
       stakingRouter.updateStakingModule(module1.id, 10001, 300, 700, { from: voting }),
-      `ed with custom error 'ErrorValueOver100Percent("_targetShare")`
+      'ErrorValueOver100Percent("_targetShare")'
     )
 
-    await assertRevert(
+    await assert.revertsWithCustomError(
       stakingRouter.updateStakingModule(module1.id, 10000, 10001, 700, { from: voting }),
-      `ed with custom error 'ErrorValueOver100Percent("_moduleFee + _treasuryFee")`
+      'ErrorValueOver100Percent("_stakingModuleFee + _treasuryFee")'
     )
 
-    await assertRevert(
+    await assert.revertsWithCustomError(
       stakingRouter.updateStakingModule(module1.id, 10000, 300, 10001, { from: voting }),
-      `ed with custom error 'ErrorValueOver100Percent("_moduleFee + _treasuryFee")`
+      'ErrorValueOver100Percent("_stakingModuleFee + _treasuryFee")'
     )
 
     // distribution fee calculates on active keys in modules
@@ -516,9 +516,9 @@ contract('Lido', ([appManager, voting, user1, user2, user3, nobody, depositor, t
     await operators.addSigningKeys(0, 1, pad('0x010203', 48), pad('0x01', 96), { from: voting })
 
     // can not deposit with unset withdrawalCredentials
-    await assertRevert(
+    await assert.revertsWithCustomError(
       app.methods['deposit(uint256,uint24,bytes)'](MAX_DEPOSITS, CURATED_MODULE_ID, CALLDATA, { from: depositor }),
-      `ed with custom error 'ErrorEmptyWithdrawalsCredentials()`
+      'ErrorEmptyWithdrawalsCredentials()'
     )
     // set withdrawalCredentials with keys, because they were trimmed
     await stakingRouter.setWithdrawalCredentials(pad('0x0202', 32), { from: voting })

test/0.8.9/staking-router-deposits-allocation.test.js

@@ -8,13 +8,13 @@ const DepositContractMock = artifacts.require('DepositContractMock.sol')
 contract('StakingRouter', (accounts) => {
   let evmSnapshotId
   let depositContract, stakingRouter
-  let curatedStakingModuleMock, soloStakingModuleMock, dvtStakingModuleMock
+  let curatedStakingModuleMock, soloStakingModuleMock
   const [deployer, lido, admin] = accounts
 
   before(async () => {
     depositContract = await DepositContractMock.new({ from: deployer })
     stakingRouter = await StakingRouter.new(depositContract.address, { from: deployer })
-    ;[curatedStakingModuleMock, soloStakingModuleMock, dvtStakingModuleMock] = await Promise.all([
+    ;[curatedStakingModuleMock, soloStakingModuleMock] = await Promise.all([
       StakingModuleMock.new({ from: deployer }),
       StakingModuleMock.new({ from: deployer }),
       StakingModuleMock.new({ from: deployer })
@@ -24,15 +24,15 @@ contract('StakingRouter', (accounts) => {
     await stakingRouter.initialize(admin, lido, wc, { from: deployer })
 
     // Set up the staking router permissions.
-    const [MANAGE_WITHDRAWAL_CREDENTIALS_ROLE, MODULE_PAUSE_ROLE, MODULE_MANAGE_ROLE] = await Promise.all([
+    const [MANAGE_WITHDRAWAL_CREDENTIALS_ROLE, STAKING_MODULE_PAUSE_ROLE, STAKING_MODULE_MANAGE_ROLE] = await Promise.all([
       stakingRouter.MANAGE_WITHDRAWAL_CREDENTIALS_ROLE(),
-      stakingRouter.MODULE_PAUSE_ROLE(),
-      stakingRouter.MODULE_MANAGE_ROLE()
+      stakingRouter.STAKING_MODULE_PAUSE_ROLE(),
+      stakingRouter.STAKING_MODULE_MANAGE_ROLE()
     ])
 
     await stakingRouter.grantRole(MANAGE_WITHDRAWAL_CREDENTIALS_ROLE, admin, { from: admin })
-    await stakingRouter.grantRole(MODULE_PAUSE_ROLE, admin, { from: admin })
-    await stakingRouter.grantRole(MODULE_MANAGE_ROLE, admin, { from: admin })
+    await stakingRouter.grantRole(STAKING_MODULE_PAUSE_ROLE, admin, { from: admin })
+    await stakingRouter.grantRole(STAKING_MODULE_MANAGE_ROLE, admin, { from: admin })
 
     evmSnapshotId = await hre.ethers.provider.send('evm_snapshot', [])
   })
@@ -44,7 +44,7 @@ contract('StakingRouter', (accounts) => {
 
   describe('One staking module', () => {
     beforeEach(async () => {
-      await stakingRouter.addModule(
+      await stakingRouter.addStakingModule(
         'Curated',
         curatedStakingModuleMock.address,
         10_000, // target share 100 %
@@ -90,15 +90,15 @@ contract('StakingRouter', (accounts) => {
 
   describe('Two staking modules', () => {
     beforeEach(async () => {
-      await stakingRouter.addModule(
+      await stakingRouter.addStakingModule(
         'Curated',
         curatedStakingModuleMock.address,
         10_000, // 100 % _targetShare
         1_000, // 10 % _moduleFee
         5_000, // 50 % _treasuryFee
         { from: admin }
       )
-      await stakingRouter.addModule(
+      await stakingRouter.addStakingModule(
         'Solo',
         soloStakingModuleMock.address,
         200, // 2 % _targetShare
@@ -134,15 +134,15 @@ contract('StakingRouter', (accounts) => {
 
   describe('Make deposit', () => {
     beforeEach(async () => {
-      await stakingRouter.addModule(
+      await stakingRouter.addStakingModule(
         'Curated',
         curatedStakingModuleMock.address,
         10_000, // 100 % _targetShare
         1_000, // 10 % _moduleFee
         5_000, // 50 % _treasuryFee
         { from: admin }
       )
-      await stakingRouter.addModule(
+      await stakingRouter.addStakingModule(
         'Solo',
         soloStakingModuleMock.address,
         200, // 2 % _targetShare