Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Staking Router post-audit fixes #497

Merged
merged 37 commits into from
Jan 23, 2023

Conversation

Psirex
Copy link
Contributor

@Psirex Psirex commented Jan 18, 2023

Simple summary

The common changes in this commit are tests. But there are also a couple of changes in contracts: in the distribution of rewards bug has been fixed, a limit on the number of modules has been added, NodeOperatorRegistry contract has been modified to reduce the bytecode size, and external methods use uint256 data instead uint24 if applicable.

Contracts Changes

  • Lido.sol

    • added Lido.getVersion() method
  • StakingRouter.sol

    • added stakingModules limit
    • changed uint24 _stakingModuleId -> uint256_stakingModuleId, add lenght check + cast for external functions
    • fix getStakingRewardsDistribution() rewarded modules collection
    • stricter naming convention module -> stakingModule across the code
    • added getVersion() method
    • changed visibility of FEE_PRECISION_POINTS constant
    • unified events params and their sequence
  • NodeOperatorsRegistry.sol

    • fix wrong vetted keys calculation on finalizeUpgrade_v2() for deactivated node operators
    • remove extra check that _exitedValidatorsKeysCount >= 0 from unsafeUpdateExitedValidatorsKeysCount()
    • remove NodeOperatorUnusedValidatorsKeysTrimmed emitting from the invalidateReadyToDepositKeys() and fix the wrong totalKeysTrimmed argument value in the NodeOperatorTotalKeysTrimmed event
    • rework removeSigningKey() to used _removeUnusedSigningKeys() under the hood
    • rework removeSigningKeyOperatorBH() to used _removeUnusedSigningKeys() under the hood
    • move onlyExistedNodeOperator(_nodeOperatorId) check to the _removeUnusedSigningKeys() method
    • get rid of repeated checks and events emitting in the _removeUnusedSigningKey() by moving them into the _removeUnusedSigningKeys()
    • add getStETH() method
    • The next modifications were applied to the NodeOperatorsRegistry contract to reduce the bytecode size from 27.511 Kb to 23.507 Kb:
      • Replace modifiers with internal methods
      • Refactor updateExitedValidatorsKeysCount() & unsafeUpdateExitedValidatorsKeysCount() to use the same internal method _updateExitedValidatorsKeysCount()
      • Refactor addSigningKeys(), addSigningKeysOperatorBH() to use the same internal method _addSigningKeys()
      • Refactor removeSigningKey(), removeSigningKeys(), removeSigningKeyOperatorBH(), and removeSigningKeysOperatorBH() to use the same internal method _removeUnusedSigningKeys()

Tests

  • Added Lido.sol tests and coverage
    • 100% Statements 179/179
    • 98.57% Branches 69/70
    • 100% Functions 52/52
    • 100% Lines 179/179
  • 100% coverage by unit tests for the NodeOperatorsRegistry contract.
  • MinFirstAllocationStrategy.sol tests for solidity version 0.4.24
  • Added a 3000+ test cases for StakingRouter.getKeysAllocation()

Psirex and others added 30 commits January 12, 2023 04:29
unit test + coverage + fixes for Lido.sol
…b-tests

Staking router/allocation lib tests
…s_registry_tests

NodeOperatorsRegistry full unit tests coverage & fixes
…s_contract_size

Reduce NodeOperatorsRegistry contract size
…posits-tests

test: allocating between modules with different params
loga4 and others added 7 commits January 23, 2023 18:29
… into staking_router_external_params_to_uint256

# Conflicts:
#	contracts/0.8.9/StakingRouter.sol
#	contracts/0.8.9/interfaces/IStakingRouter.sol
#	contracts/0.8.9/test_helpers/StakingRouterMockForDepositSecurityModule.sol
…ms_to_uint256

Staking router external params to uint256
@TheDZhon TheDZhon merged commit 6011611 into staking_router Jan 23, 2023
@loga4 loga4 deleted the staking_router_post_audit_fixes branch January 29, 2023 05:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants