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.

Sign up for GitHub

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

Jump to bottom

Staking router #463

Merged
merged 38 commits into from
Jan 23, 2023
Merged

Staking router #463

merged 38 commits into from
Jan 23, 2023

Conversation

loga4
Copy link
Contributor

@loga4 loga4 commented Dec 6, 2022

The Lido protocol upgrade introduces the StakingRouter contract, that move towards a modular node operators registry architecture.

github-advanced-security[bot]
github-advanced-security bot found potential problems Dec 6, 2022
View reviewed changes
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Slither found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.

ujenjt
ujenjt requested changes Dec 21, 2022
View reviewed changes
contracts/0.4.24/Lido.sol Outdated Show resolved Hide resolved
contracts/0.4.24/Lido.sol Outdated Show resolved Hide resolved
contracts/0.4.24/Lido.sol Outdated Show resolved Hide resolved
contracts/0.4.24/Lido.sol Outdated Show resolved Hide resolved
contracts/0.4.24/Lido.sol Outdated Show resolved Hide resolved
contracts/0.8.9/StakingRouter.sol Outdated Show resolved Hide resolved
contracts/0.8.9/StakingRouter.sol Outdated Show resolved Hide resolved
contracts/0.8.9/StakingRouter.sol Outdated Show resolved Hide resolved
contracts/0.8.9/StakingRouter.sol Outdated Show resolved Hide resolved
contracts/0.8.9/StakingRouter.sol Outdated Show resolved Hide resolved
avsetsin
avsetsin reviewed Dec 22, 2022
View reviewed changes
contracts/0.4.24/Lido.sol Outdated Show resolved Hide resolved
avsetsin
avsetsin reviewed Dec 22, 2022
View reviewed changes
contracts/0.4.24/interfaces/IStakingModule.sol Outdated Show resolved Hide resolved
contracts/0.4.24/interfaces/INodeOperatorsRegistry.sol Outdated Show resolved Hide resolved
contracts/0.4.24/interfaces/INodeOperatorsRegistry.sol Outdated Show resolved Hide resolved
contracts/0.4.24/Lido.sol Outdated Show resolved Hide resolved
contracts/0.4.24/Lido.sol Outdated Show resolved Hide resolved
contracts/0.4.24/Lido.sol Outdated Show resolved Hide resolved
contracts/0.8.9/DepositSecurityModule.sol Outdated Show resolved Hide resolved
contracts/0.8.9/StakingRouter.sol Outdated Show resolved Hide resolved
contracts/0.8.9/StakingRouter.sol Outdated Show resolved Hide resolved
contracts/0.8.9/StakingRouter.sol Outdated Show resolved Hide resolved
avsetsin
avsetsin reviewed Dec 23, 2022
View reviewed changes
contracts/0.8.9/DepositSecurityModule.sol Outdated Show resolved Hide resolved
contracts/0.4.24/Lido.sol Outdated Show resolved Hide resolved
contracts/0.4.24/interfaces/ILido.sol Outdated Show resolved Hide resolved
contracts/0.4.24/Lido.sol Outdated Show resolved Hide resolved
contracts/0.4.24/Lido.sol Outdated Show resolved Hide resolved
contracts/0.4.24/Lido.sol Outdated Show resolved Hide resolved
contracts/0.4.24/nos/NodeOperatorsRegistry.sol Outdated Show resolved Hide resolved
contracts/0.8.9/StakingRouter.sol Outdated Show resolved Hide resolved
contracts/0.8.9/StakingRouter.sol Outdated Show resolved Hide resolved
contracts/0.8.9/StakingRouter.sol Outdated Show resolved Hide resolved
@TheDZhon TheDZhon self-requested a review December 23, 2022 14:03
avsetsin
avsetsin reviewed Dec 24, 2022
View reviewed changes
contracts/0.4.24/Lido.sol Outdated Show resolved Hide resolved
contracts/0.4.24/Lido.sol Outdated Show resolved Hide resolved
contracts/0.4.24/Lido.sol Outdated Show resolved Hide resolved
contracts/0.4.24/nos/NodeOperatorsRegistry.sol Outdated Show resolved Hide resolved
contracts/0.4.24/nos/NodeOperatorsRegistry.sol Outdated Show resolved Hide resolved
contracts/0.4.24/nos/NodeOperatorsRegistry.sol Outdated Show resolved Hide resolved
contracts/0.4.24/nos/NodeOperatorsRegistry.sol Outdated Show resolved Hide resolved
contracts/0.4.24/nos/NodeOperatorsRegistry.sol Outdated Show resolved Hide resolved
contracts/0.4.24/nos/NodeOperatorsRegistry.sol Outdated Show resolved Hide resolved
avsetsin
avsetsin reviewed Dec 25, 2022
View reviewed changes
contracts/0.8.9/StakingRouter.sol Outdated Show resolved Hide resolved
contracts/0.8.9/StakingRouter.sol Outdated Show resolved Hide resolved
contracts/0.8.9/StakingRouter.sol Show resolved Hide resolved
contracts/0.8.9/StakingRouter.sol Outdated Show resolved Hide resolved
contracts/0.8.9/StakingRouter.sol Outdated Show resolved Hide resolved
ujenjt
ujenjt requested changes Dec 25, 2022
View reviewed changes
contracts/0.4.24/Lido.sol Outdated Show resolved Hide resolved
contracts/0.4.24/Lido.sol Outdated Show resolved Hide resolved
contracts/0.4.24/Lido.sol Outdated Show resolved Hide resolved
TheDZhon
TheDZhon requested changes Dec 25, 2022
View reviewed changes
Copy link
Contributor

@TheDZhon TheDZhon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tremendous changes 🙏

Leaving the first batch of comments to start with something.

contracts/0.4.24/Lido.sol Outdated Show resolved Hide resolved
contracts/0.4.24/Lido.sol Outdated Show resolved Hide resolved
contracts/0.4.24/Lido.sol Outdated Show resolved Hide resolved
contracts/0.4.24/Lido.sol Outdated Show resolved Hide resolved
contracts/0.4.24/Lido.sol Outdated Show resolved Hide resolved
contracts/0.4.24/Lido.sol Outdated Show resolved Hide resolved
contracts/0.4.24/interfaces/ILido.sol Outdated Show resolved Hide resolved
contracts/0.4.24/interfaces/ILido.sol Outdated Show resolved Hide resolved
contracts/0.4.24/interfaces/ILido.sol Outdated Show resolved Hide resolved
contracts/0.4.24/interfaces/ILido.sol Outdated Show resolved Hide resolved
TheDZhon
TheDZhon requested changes Dec 26, 2022
View reviewed changes
Copy link
Contributor

@TheDZhon TheDZhon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is the second batch 🙏

contracts/0.4.24/nos/NodeOperatorsRegistry.sol Outdated Show resolved Hide resolved
contracts/0.4.24/nos/NodeOperatorsRegistry.sol Outdated Show resolved Hide resolved
contracts/0.4.24/nos/NodeOperatorsRegistry.sol Outdated Show resolved Hide resolved
contracts/0.4.24/nos/NodeOperatorsRegistry.sol Outdated Show resolved Hide resolved
contracts/0.4.24/nos/NodeOperatorsRegistry.sol Outdated Show resolved Hide resolved
contracts/0.4.24/nos/NodeOperatorsRegistry.sol Outdated Show resolved Hide resolved
contracts/0.4.24/nos/NodeOperatorsRegistry.sol Outdated Show resolved Hide resolved
contracts/0.4.24/nos/NodeOperatorsRegistry.sol Outdated Show resolved Hide resolved
contracts/0.4.24/nos/NodeOperatorsRegistry.sol Outdated Show resolved Hide resolved
contracts/0.4.24/nos/NodeOperatorsRegistry.sol Outdated Show resolved Hide resolved
TheDZhon
TheDZhon reviewed Dec 26, 2022
View reviewed changes
contracts/0.8.9/ModuleSolo.sol Outdated Show resolved Hide resolved
contracts/0.8.9/ModuleSolo.sol Outdated Show resolved Hide resolved
ujenjt
ujenjt reviewed Dec 26, 2022
View reviewed changes
contracts/0.4.24/Lido.sol Outdated Show resolved Hide resolved
ujenjt
ujenjt reviewed Dec 26, 2022
View reviewed changes
contracts/0.4.24/Lido.sol Outdated Show resolved Hide resolved
ujenjt
ujenjt reviewed Dec 26, 2022
View reviewed changes
contracts/0.4.24/Lido.sol Outdated Show resolved Hide resolved
ujenjt
ujenjt reviewed Dec 26, 2022
View reviewed changes
contracts/0.4.24/Lido.sol Outdated Show resolved Hide resolved
ujenjt
ujenjt reviewed Dec 26, 2022
View reviewed changes
contracts/0.4.24/Lido.sol Outdated Show resolved Hide resolved
ujenjt
ujenjt reviewed Dec 26, 2022
View reviewed changes
contracts/0.4.24/Lido.sol Outdated Show resolved Hide resolved
ujenjt
ujenjt reviewed Dec 26, 2022
View reviewed changes
contracts/0.4.24/Lido.sol Outdated Show resolved Hide resolved
ujenjt
ujenjt reviewed Dec 26, 2022
View reviewed changes
contracts/0.4.24/Lido.sol Outdated Show resolved Hide resolved
ujenjt
ujenjt reviewed Dec 26, 2022
View reviewed changes
contracts/0.4.24/Lido.sol Outdated Show resolved Hide resolved
ujenjt
ujenjt reviewed Dec 26, 2022
View reviewed changes
contracts/0.4.24/Lido.sol Outdated Show resolved Hide resolved
Psirex and others added 28 commits January 18, 2023 19:45
@Psirex
Merge pull request #489 from lidofinance/staking-router/allocation-li…
a3371e3 
…b-tests

Staking router/allocation lib tests
@Psirex
Merge branch 'staking_router_post_audit_fixes' into staking_router_no…
cac9e0c 
…de_operators_registry_tests
@Psirex
Merge pull request #488 from lidofinance/staking_router_node_operator…
1184a78 
…s_registry_tests

NodeOperatorsRegistry full unit tests coverage & fixes
@Psirex
Merge branch 'staking_router_post_audit_fixes' into staking-router-fix
e4564e6
@Psirex
Merge pull request #494 from lidofinance/staking-router-fix
c91f786 
fix: reward distribution array
@Psirex
NodeOperatorsRegistry contract size optimization
c374dc4
@Psirex
Fix argument naming
0106c5c
@mymphe
test: allocating between modules with different params
d8a8b72
@mymphe
feat: skip by default
e372df5
@mymphe
fix: remove comments
b7235cc
@Psirex
Merge pull request #498 from lidofinance/staking_router_node_operator…
a09b4d5 
…s_contract_size

Reduce NodeOperatorsRegistry contract size
@loga4
Merge pull request #501 from lidofinance/staking-router/allocating-de…
f47e80a 
…posits-tests

test: allocating between modules with different params
@bulbozaur
staking router modules limit
527e80e
@bulbozaur
tests: fix deposit test
ff307e9
@bulbozaur
fix: keys allocation fix
a68ed09
@bulbozaur
rm _estimateStakingModuleMaxDepositableKeysByIndex
9916c72
@bulbozaur
Merge pull request #502 from lidofinance/staking_router_keys_allocati…
1fbe233 
…on_fix

fix: keys allocation fix
@loga4
fix uint24 -> uint256 moduleId represent
44800c5
@loga4
remove console
aeda9dd
@loga4
update reverts errors
719b331
@loga4
small fixes
72b0db7
@loga4
small fixes
954b823
@loga4
add comments for interfaces
5c4301c
@loga4
Merge remote-tracking branch 'origin/staking_router_post_audit_fixes'…
273bb87 
… 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
@loga4
fix conflicts
9bcaf0e
@loga4
use uint256 stakingModuleId for sig
5c21650
@TheDZhon
Merge pull request #505 from lidofinance/staking_router_external_para…
e79c57f 
…ms_to_uint256

Staking router external params to uint256
@TheDZhon
Merge pull request #497 from lidofinance/staking_router_post_audit_fixes
6011611 
Staking Router post-audit fixes
@TheDZhon TheDZhon changed the base branch from develop to feature/shapella-upgrade January 23, 2023 15:16
@arwer13 arwer13 merged commit ad08f38 into feature/shapella-upgrade Jan 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Psirex Psirex Psirex left review comments

@krogla krogla krogla left review comments

@avsetsin avsetsin avsetsin left review comments

@ujenjt ujenjt ujenjt requested changes

@TheDZhon TheDZhon TheDZhon requested changes

@arwer13 arwer13 arwer13 requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@loga4 @TheDZhon @ujenjt @avsetsin @arwer13 @krogla @Psirex @mymphe @bulbozaur