Skip to content

feat: add view functions for verify proof of validator and deposit me… #1045

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

Open
wants to merge 2 commits into
base: feat/vaults
Choose a base branch
from

Conversation

DiRaiks
Copy link
Contributor

@DiRaiks DiRaiks commented Apr 22, 2025

A short summary of the changes.

Context

New Public Functions:

  • Added validatePubKeyWCProof to validate a validator's proof against the Beacon block root using withdrawal credentials and a public key. This function wraps the private _validatePubKeyWCProof method for external use.
  • Added verifyDepositMessage to verify the deposit message signature using BLS12-381 pairing checks. This function wraps the BLS12_381.verifyDepositMessage method for external use.

Refactoring of Existing Calls:

  • Replaced calls to _validatePubKeyWCProof with validatePubKeyWCProof for proof validation in multiple locations, ensuring consistency and reusability. [1] [2] [3]
  • Replaced calls to BLS12_381.verifyDepositMessage with verifyDepositMessage for deposit verification, improving code readability and modularity.

Problem

What problem this PR solves, link relevant issue if it exists

Solution

Your proposed solution

@DiRaiks DiRaiks requested a review from a team as a code owner April 22, 2025 17:51
Copy link

github-actions bot commented Apr 22, 2025

badge

Hardhat Unit Tests Coverage Summary

Filename                                                                Stmts    Miss  Cover    Missing
--------------------------------------------------------------------  -------  ------  -------  -------------------------------------------------------
contracts/0.4.24/Lido.sol                                                 204       5  97.55%   802-814, 955-956
contracts/0.4.24/StETH.sol                                                 79       0  100.00%
contracts/0.4.24/StETHPermit.sol                                           15       0  100.00%
contracts/0.4.24/lib/Packed64x4.sol                                         5       0  100.00%
contracts/0.4.24/lib/SigningKeys.sol                                       36       0  100.00%
contracts/0.4.24/lib/StakeLimitUtils.sol                                   37       0  100.00%
contracts/0.4.24/nos/NodeOperatorsRegistry.sol                            512       0  100.00%
contracts/0.4.24/utils/Pausable.sol                                         9       0  100.00%
contracts/0.4.24/utils/Versioned.sol                                        5       0  100.00%
contracts/0.6.12/WstETH.sol                                                17       0  100.00%
contracts/0.8.25/Accounting.sol                                            82       1  98.78%   321
contracts/0.8.25/interfaces/IDepositContract.sol                            0       0  100.00%
contracts/0.8.25/interfaces/ILido.sol                                       0       0  100.00%
contracts/0.8.25/interfaces/IOracleReportSanityChecker.sol                  0       0  100.00%
contracts/0.8.25/interfaces/IPostTokenRebaseReceiver.sol                    0       0  100.00%
contracts/0.8.25/interfaces/IStakingRouter.sol                              0       0  100.00%
contracts/0.8.25/interfaces/IWithdrawalQueue.sol                            0       0  100.00%
contracts/0.8.25/lib/BLS.sol                                               27       3  88.89%   273, 340, 367
contracts/0.8.25/lib/BeaconTypes.sol                                        0       0  100.00%
contracts/0.8.25/lib/GIndex.sol                                            33      18  45.45%   23, 35, 56, 64-71, 80, 87-102
contracts/0.8.25/lib/SSZ.sol                                               19       7  63.16%   55-157, 347
contracts/0.8.25/utils/AccessControlConfirmable.sol                        30       0  100.00%
contracts/0.8.25/utils/PausableUntilWithRoles.sol                           3       0  100.00%
contracts/0.8.25/vaults/OperatorGrid.sol                                  135       0  100.00%
contracts/0.8.25/vaults/PinnedBeaconProxy.sol                               5       0  100.00%
contracts/0.8.25/vaults/StakingVault.sol                                  164       2  98.78%   659-660
contracts/0.8.25/vaults/VaultFactory.sol                                   25       4  84.00%   92-97
contracts/0.8.25/vaults/VaultHub.sol                                      203      23  88.67%   138, 326, 329, 331, 383-401, 464, 471, 507-509, 634-635
contracts/0.8.25/vaults/dashboard/Dashboard.sol                           105      24  77.14%   231, 335-371, 399-400, 518, 568-574
contracts/0.8.25/vaults/dashboard/NodeOperatorFee.sol                      41       0  100.00%
contracts/0.8.25/vaults/dashboard/Permissions.sol                          46       6  86.96%   345-358, 410
contracts/0.8.25/vaults/interfaces/IPredepositGuarantee.sol                 0       0  100.00%
contracts/0.8.25/vaults/interfaces/IStakingVault.sol                        0       0  100.00%
contracts/0.8.25/vaults/lib/PinnedBeaconUtils.sol                           6       0  100.00%
contracts/0.8.25/vaults/predeposit_guarantee/CLProofVerifier.sol           11       0  100.00%
contracts/0.8.25/vaults/predeposit_guarantee/PredepositGuarantee.sol      152       0  100.00%
contracts/0.8.4/WithdrawalsManagerProxy.sol                                61       0  100.00%
contracts/0.8.9/BeaconChainDepositor.sol                                   21       2  90.48%   48, 51
contracts/0.8.9/Burner.sol                                                 72       0  100.00%
contracts/0.8.9/DepositSecurityModule.sol                                 128       0  100.00%
contracts/0.8.9/EIP712StETH.sol                                            16       0  100.00%
contracts/0.8.9/LidoExecutionLayerRewardsVault.sol                         16       0  100.00%
contracts/0.8.9/LidoLocator.sol                                            22       0  100.00%
contracts/0.8.9/OracleDaemonConfig.sol                                     28       0  100.00%
contracts/0.8.9/StakingRouter.sol                                         316       0  100.00%
contracts/0.8.9/WithdrawalQueue.sol                                        88       0  100.00%
contracts/0.8.9/WithdrawalQueueBase.sol                                   146       0  100.00%
contracts/0.8.9/WithdrawalQueueERC721.sol                                  89       0  100.00%
contracts/0.8.9/WithdrawalVault.sol                                        21       0  100.00%
contracts/0.8.9/lib/Math.sol                                                4       0  100.00%
contracts/0.8.9/lib/PositiveTokenRebaseLimiter.sol                         22       0  100.00%
contracts/0.8.9/lib/UnstructuredRefStorage.sol                              2       0  100.00%
contracts/0.8.9/oracle/AccountingOracle.sol                               172       2  98.84%   127-128
contracts/0.8.9/oracle/BaseOracle.sol                                      89       1  98.88%   397
contracts/0.8.9/oracle/HashConsensus.sol                                  263       1  99.62%   1005
contracts/0.8.9/oracle/ValidatorsExitBusOracle.sol                         91       2  97.80%   138, 315
contracts/0.8.9/proxy/OssifiableProxy.sol                                  17       0  100.00%
contracts/0.8.9/sanity_checks/OracleReportSanityChecker.sol               217       1  99.54%   856
contracts/0.8.9/utils/DummyEmptyContract.sol                                0       0  100.00%
contracts/0.8.9/utils/PausableUntil.sol                                    31       0  100.00%
contracts/0.8.9/utils/Versioned.sol                                        11       0  100.00%
contracts/0.8.9/utils/access/AccessControl.sol                             23       0  100.00%
contracts/0.8.9/utils/access/AccessControlEnumerable.sol                    9       0  100.00%
contracts/common/utils/PausableUntil.sol                                   29       0  100.00%
contracts/testnets/sepolia/SepoliaDepositAdapter.sol                       21      21  0.00%    49-100
TOTAL                                                                    4031     123  96.95%

Diff against master

Filename                                                                Stmts    Miss  Cover
--------------------------------------------------------------------  -------  ------  --------
contracts/0.4.24/Lido.sol                                                  -8      +5  -2.45%
contracts/0.4.24/StETH.sol                                                 +7       0  +100.00%
contracts/0.8.25/Accounting.sol                                           +82      +1  +98.78%
contracts/0.8.25/interfaces/IDepositContract.sol                            0       0  +100.00%
contracts/0.8.25/interfaces/ILido.sol                                       0       0  +100.00%
contracts/0.8.25/interfaces/IOracleReportSanityChecker.sol                  0       0  +100.00%
contracts/0.8.25/interfaces/IPostTokenRebaseReceiver.sol                    0       0  +100.00%
contracts/0.8.25/interfaces/IStakingRouter.sol                              0       0  +100.00%
contracts/0.8.25/interfaces/IWithdrawalQueue.sol                            0       0  +100.00%
contracts/0.8.25/lib/BLS.sol                                              +27      +3  +88.89%
contracts/0.8.25/lib/BeaconTypes.sol                                        0       0  +100.00%
contracts/0.8.25/lib/GIndex.sol                                           +33     +18  +45.45%
contracts/0.8.25/lib/SSZ.sol                                              +19      +7  +63.16%
contracts/0.8.25/utils/AccessControlConfirmable.sol                       +30       0  +100.00%
contracts/0.8.25/utils/PausableUntilWithRoles.sol                          +3       0  +100.00%
contracts/0.8.25/vaults/OperatorGrid.sol                                 +135       0  +100.00%
contracts/0.8.25/vaults/PinnedBeaconProxy.sol                              +5       0  +100.00%
contracts/0.8.25/vaults/StakingVault.sol                                 +164      +2  +98.78%
contracts/0.8.25/vaults/VaultFactory.sol                                  +25      +4  +84.00%
contracts/0.8.25/vaults/VaultHub.sol                                     +203     +23  +88.67%
contracts/0.8.25/vaults/dashboard/Dashboard.sol                          +105     +24  +77.14%
contracts/0.8.25/vaults/dashboard/NodeOperatorFee.sol                     +41       0  +100.00%
contracts/0.8.25/vaults/dashboard/Permissions.sol                         +46      +6  +86.96%
contracts/0.8.25/vaults/interfaces/IPredepositGuarantee.sol                 0       0  +100.00%
contracts/0.8.25/vaults/interfaces/IStakingVault.sol                        0       0  +100.00%
contracts/0.8.25/vaults/lib/PinnedBeaconUtils.sol                          +6       0  +100.00%
contracts/0.8.25/vaults/predeposit_guarantee/CLProofVerifier.sol          +11       0  +100.00%
contracts/0.8.25/vaults/predeposit_guarantee/PredepositGuarantee.sol     +152       0  +100.00%
contracts/0.8.9/Burner.sol                                                 +1       0  +100.00%
contracts/0.8.9/LidoLocator.sol                                            +4       0  +100.00%
contracts/0.8.9/oracle/AccountingOracle.sol                               -18       0  -0.11%
contracts/0.8.9/sanity_checks/OracleReportSanityChecker.sol               -15      +1  -0.46%
contracts/common/utils/PausableUntil.sol                                  +29       0  +100.00%
TOTAL                                                                   +1087     +94  -2.09%

Results for commit: e6a2d55

Minimum allowed coverage is 90%

♻️ This comment has been updated with latest results

@@ -317,7 +339,7 @@ contract PredepositGuarantee is IPredepositGuarantee, CLProofVerifier, PausableU

// this check isn't needed in `depositToBeaconChain` because
// Beacon Chain doesn't enforce the signature checks for existing validators and just top-ups to their balance
BLS12_381.verifyDepositMessage(_deposit, _depositsY[i], withdrawalCredentials);
verifyDepositMessage(_deposit, _depositsY[i], withdrawalCredentials);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we check that this extra call does not increase gas cost due to args?

Copy link
Contributor

Choose a reason for hiding this comment

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

can quickly compare via tx.gasUsed in some existing tests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants