-
Notifications
You must be signed in to change notification settings - Fork 195
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
[VAULTS][POC] a very raw bug-ridden proof-of-concept for PredepositGuardian #932
Draft
failingtwice
wants to merge
11
commits into
vault-guardian
Choose a base branch
from
predeposit-guardian
base: vault-guardian
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
938a519
feat: predeposit guardian concept WIP, full of bugs and errors
failingtwice 34d203d
refactor: cleanup logic
failingtwice 21f475e
feat: remove unnecessary mapping
failingtwice 331a6e6
feat: remove node operator check
failingtwice 2c84783
fix: rename
failingtwice ccde1b2
fix: comment on naming
failingtwice d0954f7
fix: comment on naming
failingtwice c5312c0
feat: add accounting&delegation to predeposit guardian
Jeday 9d2b349
feat: proof validation
Jeday e4d3ebc
fix: clean up errors
Jeday 74917ee
fix: mitigate mal staking vault
Jeday File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,302 @@ | ||
// SPDX-FileCopyrightText: 2025 Lido <[email protected]> | ||
// SPDX-License-Identifier: GPL-3.0 | ||
|
||
// See contracts/COMPILERS.md | ||
pragma solidity 0.8.25; | ||
|
||
import {MerkleProof} from "@openzeppelin/contracts-v5.2/utils/cryptography/MerkleProof.sol"; | ||
|
||
import {StakingVault} from "./StakingVault.sol"; | ||
import {IDepositContract} from "../interfaces/IDepositContract.sol"; | ||
|
||
contract PredepositGuardian { | ||
uint256 public constant PREDEPOSIT_AMOUNT = 1 ether; | ||
|
||
enum ValidatorStatus { | ||
NO_RECORD, | ||
AWAITING_PROOF, | ||
PROVED, | ||
PROVED_INVALID, | ||
WITHDRAWN | ||
} | ||
|
||
// See `BEACON_ROOTS_ADDRESS` constant in the EIP-4788. | ||
address public constant BEACON_ROOTS = 0x000F3df6D732807Ef1319fB7B8bB8522d0Beac02; | ||
|
||
mapping(address nodeOperator => uint256) public nodeOperatorCollateral; | ||
mapping(address nodeOperator => uint256) public nodeOperatorCollateralLocked; | ||
mapping(address nodeOperator => address delegate) public nodeOperatorDelegate; | ||
|
||
mapping(bytes32 validatorPubkeyHash => ValidatorStatus validatorStatus) public validatorStatuses; | ||
mapping(bytes32 validatorPubkeyHash => StakingVault) public validatorStakingVault; | ||
// node operator can be taken from vault,but this prevents malicious vault from changing node operator midflight | ||
mapping(bytes32 validatorPubkeyHash => address nodeOperator) public validatorToNodeOperator; | ||
|
||
/// views | ||
|
||
function nodeOperatorBalance(address nodeOperator) external view returns (uint256, uint256) { | ||
return (nodeOperatorCollateral[nodeOperator], nodeOperatorCollateralLocked[nodeOperator]); | ||
} | ||
|
||
/// NO Balance operations | ||
|
||
function topUpNodeOperatorCollateral(address _nodeOperator) external payable { | ||
if (msg.value == 0) revert ZeroArgument("msg.value"); | ||
_topUpNodeOperatorCollateral(_nodeOperator); | ||
} | ||
|
||
function withdrawNodeOperatorCollateral(address _nodeOperator, uint256 _amount, address _recipient) external { | ||
if (_amount == 0) revert ZeroArgument("amount"); | ||
if (_nodeOperator == address(0)) revert ZeroArgument("_nodeOperator"); | ||
|
||
_isValidNodeOperatorCaller(_nodeOperator); | ||
|
||
if (nodeOperatorCollateral[_nodeOperator] - nodeOperatorCollateralLocked[_nodeOperator] >= _amount) | ||
revert NotEnoughUnlockedCollateralToWithdraw(); | ||
|
||
nodeOperatorCollateral[_nodeOperator] -= _amount; | ||
(bool success, ) = _recipient.call{value: _amount}(""); | ||
|
||
if (!success) revert WithdrawalFailed(); | ||
|
||
// TODO: event | ||
} | ||
|
||
// delegation | ||
|
||
function delegateNodeOperator(address _delegate) external { | ||
nodeOperatorDelegate[msg.sender] = _delegate; | ||
// TODO: event | ||
} | ||
|
||
/// Deposit operations | ||
|
||
function predeposit(StakingVault _stakingVault, StakingVault.Deposit[] calldata _deposits) external payable { | ||
if (_deposits.length == 0) revert PredepositNoDeposits(); | ||
|
||
address _nodeOperator = _stakingVault.nodeOperator(); | ||
_isValidNodeOperatorCaller(_nodeOperator); | ||
|
||
// optional top up | ||
if (msg.value != 0) { | ||
_topUpNodeOperatorCollateral(_nodeOperator); | ||
} | ||
|
||
uint256 unlockedCollateral = nodeOperatorCollateral[_nodeOperator] - | ||
nodeOperatorCollateralLocked[_nodeOperator]; | ||
|
||
uint256 totalDepositAmount = PREDEPOSIT_AMOUNT * _deposits.length; | ||
|
||
if (unlockedCollateral < totalDepositAmount) revert NotEnoughUnlockedCollateralToPredeposit(); | ||
|
||
for (uint256 i = 0; i < _deposits.length; i++) { | ||
StakingVault.Deposit calldata _deposit = _deposits[i]; | ||
|
||
bytes32 validatorId = keccak256(_deposit.pubkey); | ||
|
||
if (validatorStatuses[validatorId] != ValidatorStatus.NO_RECORD) { | ||
revert MustBeNewValidatorPubkey(); | ||
} | ||
|
||
// cannot predeposit a validator with a deposit amount that is not 1 ether | ||
if (_deposit.amount != PREDEPOSIT_AMOUNT) revert PredepositDepositAmountInvalid(); | ||
|
||
validatorStatuses[validatorId] = ValidatorStatus.AWAITING_PROOF; | ||
validatorStakingVault[validatorId] = _stakingVault; | ||
validatorToNodeOperator[validatorId] = _nodeOperator; | ||
} | ||
|
||
nodeOperatorCollateralLocked[_nodeOperator] += totalDepositAmount; | ||
_stakingVault.depositToBeaconChain(_deposits); | ||
// TODO: event | ||
} | ||
|
||
function proveValidatorPreDeposit( | ||
StakingVault.Deposit calldata _deposit, | ||
bytes32[] calldata proof, | ||
uint64 beaconBlockTimestamp | ||
) external { | ||
bytes32 validatorId = keccak256(_deposit.pubkey); | ||
// check that the validator is predeposited | ||
if (validatorStatuses[validatorId] != ValidatorStatus.AWAITING_PROOF) { | ||
revert ValidatorNotPreDeposited(); | ||
} | ||
|
||
// NB! this is potential attack vector, what if the staking vault is malicious | ||
// it can change WC to block node operator from bringing proof | ||
// we could check if staking vault must always have wc to it's own address is invariant | ||
_validateDepositDataRoot(_deposit, validatorStakingVault[validatorId].withdrawalCredentials()); | ||
|
||
// check that predeposit was made to the staking vault in proof | ||
_validateProof(proof, _deposit.depositDataRoot, beaconBlockTimestamp); | ||
|
||
nodeOperatorCollateralLocked[validatorToNodeOperator[validatorId]] -= PREDEPOSIT_AMOUNT; | ||
validatorStatuses[validatorId] = ValidatorStatus.PROVED; | ||
|
||
// TODO: event | ||
} | ||
|
||
function proveInvalidValidatorPreDeposit( | ||
StakingVault.Deposit calldata _deposit, | ||
bytes32 _invalidWC, | ||
bytes32[] calldata proof, | ||
uint64 beaconBlockTimestamp | ||
) external { | ||
bytes32 _validatorId = keccak256(_deposit.pubkey); | ||
|
||
// check that the validator is predeposited | ||
if (validatorStatuses[_validatorId] != ValidatorStatus.AWAITING_PROOF) { | ||
revert ValidatorNotPreDeposited(); | ||
} | ||
|
||
_validateDepositDataRoot(_deposit, _invalidWC); | ||
|
||
// NB! this is potential attack vector, if the staking vault is malicious | ||
// it can change WC to steal from the node operator | ||
// alt check if staking vault must always have wc to it's own address is invariant | ||
//if (address(validatorStakingVault[_validatorId]) == _wcToAddress(_invalidWC)) { | ||
if (validatorStakingVault[_validatorId].withdrawalCredentials() == _invalidWC) { | ||
revert WithdrawalCredentialsAreValid(); | ||
} | ||
|
||
_validateProof(proof, _deposit.depositDataRoot, beaconBlockTimestamp); | ||
|
||
validatorStatuses[_validatorId] = ValidatorStatus.PROVED_INVALID; | ||
|
||
// TODO: event | ||
} | ||
|
||
function depositToProvenValidators( | ||
StakingVault _stakingVault, | ||
StakingVault.Deposit[] calldata _deposits | ||
) external payable { | ||
_isValidNodeOperatorCaller(_stakingVault.nodeOperator()); | ||
|
||
for (uint256 i = 0; i < _deposits.length; i++) { | ||
StakingVault.Deposit calldata _deposit = _deposits[i]; | ||
bytes32 _validatorId = keccak256(_deposit.pubkey); | ||
|
||
if (validatorStatuses[_validatorId] != ValidatorStatus.PROVED) { | ||
revert DepositToUnprovenValidator(); | ||
} | ||
|
||
if (validatorStakingVault[_validatorId] != _stakingVault) { | ||
revert DepositToWrongVault(); | ||
} | ||
} | ||
|
||
_stakingVault.depositToBeaconChain(_deposits); | ||
} | ||
|
||
// called by the staking vault owner if the predeposited validator has a different withdrawal credentials than the vault's withdrawal credentials, | ||
// i.e. node operator was malicious | ||
function withdrawDisprovenCollateral(bytes32 _validatorId, address _recipient) external { | ||
if (_recipient == address(0)) revert ZeroArgument("_recipient"); | ||
|
||
address _nodeOperator = validatorToNodeOperator[_validatorId]; | ||
if (validatorStatuses[_validatorId] != ValidatorStatus.PROVED_INVALID) revert ValidatorNotProvenInvalid(); | ||
|
||
if (msg.sender != validatorStakingVault[_validatorId].owner()) revert WithdrawSenderNotStakingVaultOwner(); | ||
|
||
nodeOperatorCollateralLocked[_nodeOperator] -= PREDEPOSIT_AMOUNT; | ||
nodeOperatorCollateral[_nodeOperator] -= PREDEPOSIT_AMOUNT; | ||
validatorStatuses[_validatorId] = ValidatorStatus.WITHDRAWN; | ||
|
||
(bool success, ) = _recipient.call{value: PREDEPOSIT_AMOUNT}(""); | ||
if (!success) revert WithdrawalFailed(); | ||
|
||
//TODO: events | ||
} | ||
|
||
/// Internal functions | ||
|
||
function _validateProof( | ||
bytes32[] calldata _proof, | ||
bytes32 _depositDataRoot, | ||
uint64 beaconBlockTimestamp | ||
) internal view { | ||
if (!MerkleProof.verifyCalldata(_proof, _getParentBlockRoot(beaconBlockTimestamp), _depositDataRoot)) | ||
revert InvalidProof(); | ||
} | ||
|
||
function _topUpNodeOperatorCollateral(address _nodeOperator) internal { | ||
if (_nodeOperator == address(0)) revert ZeroArgument("_nodeOperator"); | ||
nodeOperatorCollateral[_nodeOperator] += msg.value; | ||
// TODO: event | ||
} | ||
|
||
function _isValidNodeOperatorCaller(address _nodeOperator) internal view { | ||
if (msg.sender != _nodeOperator && nodeOperatorDelegate[_nodeOperator] != msg.sender) | ||
revert MustBeNodeOperatorOrDelegate(); | ||
} | ||
|
||
function _getParentBlockRoot(uint64 blockTimestamp) internal view returns (bytes32) { | ||
(bool success, bytes memory data) = BEACON_ROOTS.staticcall(abi.encode(blockTimestamp)); | ||
|
||
if (!success || data.length == 0) { | ||
revert RootNotFound(); | ||
} | ||
|
||
return abi.decode(data, (bytes32)); | ||
} | ||
|
||
function _validateDepositDataRoot(StakingVault.Deposit calldata _deposit, bytes32 _invalidWC) internal pure { | ||
bytes32 pubkey_root = sha256(abi.encodePacked(_deposit.pubkey, bytes16(0))); | ||
bytes32 signature_root = sha256( | ||
abi.encodePacked( | ||
sha256(abi.encodePacked(_deposit.signature[:64])), | ||
sha256(abi.encodePacked(_deposit.signature[64:], bytes32(0))) | ||
) | ||
); | ||
bytes32 node = sha256( | ||
abi.encodePacked( | ||
sha256(abi.encodePacked(pubkey_root, _invalidWC)), | ||
sha256(abi.encodePacked(_deposit.amount, bytes24(0), signature_root)) | ||
) | ||
); | ||
|
||
if (_deposit.depositDataRoot != node) { | ||
revert InvalidDepositRoot(); | ||
} | ||
} | ||
|
||
function _wcToAddress(bytes32 _withdrawalCredentials) internal pure returns (address) { | ||
return address(uint160(uint256(_withdrawalCredentials))); | ||
} | ||
|
||
// predeposit errors | ||
error PredepositNoDeposits(); | ||
error PredepositValueNotMultipleOfPrediposit(); | ||
error PredepositDepositAmountInvalid(); | ||
error MustBeNewValidatorPubkey(); | ||
error NotEnoughUnlockedCollateralToPredeposit(); | ||
|
||
// proving errors | ||
error ValidatorNotPreDeposited(); | ||
error RootNotFound(); | ||
error InvalidProof(); | ||
error InvalidDepositRoot(); | ||
|
||
// depositing errors | ||
error DepositToUnprovenValidator(); | ||
error DepositToWrongVault(); | ||
|
||
// withdrawal proven | ||
error NotEnoughUnlockedCollateralToWithdraw(); | ||
|
||
// withdrawal disproven | ||
error ValidatorNotProvenInvalid(); | ||
error WithdrawSenderNotStakingVaultOwner(); | ||
error WithdrawSenderNotNodeOperator(); | ||
error WithdrawValidatorDoesNotBelongToNodeOperator(); | ||
error WithdrawalCollateralOfWrongVault(); | ||
error WithdrawalCredentialsAreValid(); | ||
/// withdrawal genereic | ||
error WithdrawalFailed(); | ||
|
||
// auth | ||
error MustBeNodeOperatorOrDelegate(); | ||
|
||
// general | ||
error ZeroArgument(string argument); | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,7 +5,6 @@ | |
pragma solidity 0.8.25; | ||
|
||
import {OwnableUpgradeable} from "contracts/openzeppelin/5.2/upgradeable/access/OwnableUpgradeable.sol"; | ||
import {SignatureChecker} from "@openzeppelin/contracts-v5.2/utils/cryptography/SignatureChecker.sol"; | ||
|
||
import {VaultHub} from "./VaultHub.sol"; | ||
|
||
|
@@ -68,6 +67,7 @@ contract StakingVault is IStakingVault, OwnableUpgradeable { | |
uint128 locked; | ||
int128 inOutDelta; | ||
address nodeOperator; | ||
// depositGuardian becomes the depositor, instead of just guardian, perhaps a renaming is needed 🌚 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe make |
||
address depositGuardian; | ||
bool beaconChainDepositsPaused; | ||
} | ||
|
@@ -318,30 +318,23 @@ contract StakingVault is IStakingVault, OwnableUpgradeable { | |
* @param _deposits Array of deposit structs | ||
* @dev Includes a check to ensure StakingVault is balanced before making deposits | ||
*/ | ||
function depositToBeaconChain( | ||
Deposit[] calldata _deposits, | ||
bytes32 _expectedGlobalDepositRoot, | ||
bytes calldata _signature | ||
) external { | ||
function depositToBeaconChain(Deposit[] calldata _deposits) external { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. strawman sanity checks would be needed for the input data |
||
if (_deposits.length == 0) revert ZeroArgument("_deposits"); | ||
|
||
bytes32 currentGlobalDepositRoot = BEACON_CHAIN_DEPOSIT_CONTRACT.get_deposit_root(); | ||
if (_expectedGlobalDepositRoot != currentGlobalDepositRoot) | ||
revert GlobalDepositRootMismatch(_expectedGlobalDepositRoot, currentGlobalDepositRoot); | ||
if (!isBalanced()) revert Unbalanced(); | ||
|
||
ERC7201Storage storage $ = _getStorage(); | ||
if (msg.sender != $.nodeOperator) revert NotAuthorized("depositToBeaconChain", msg.sender); | ||
if ($.beaconChainDepositsPaused) revert BeaconChainDepositsArePaused(); | ||
if (!isBalanced()) revert Unbalanced(); | ||
if (msg.sender != $.depositGuardian) revert NotAuthorized("depositToBeaconChain", msg.sender); | ||
|
||
uint256 totalAmount = 0; | ||
uint256 numberOfDeposits = _deposits.length; | ||
// XOR is a commutative operation, so the aggregate root will be the same regardless of the order of deposits | ||
bytes32 depositDataBatchXorRoot; | ||
|
||
uint256 totalAmount = 0; | ||
|
||
for (uint256 i = 0; i < numberOfDeposits; i++) { | ||
Deposit calldata deposit = _deposits[i]; | ||
|
||
//TODO: check BLS signature | ||
// check deposit data root | ||
BEACON_CHAIN_DEPOSIT_CONTRACT.deposit{value: deposit.amount}( | ||
deposit.pubkey, | ||
bytes.concat(withdrawalCredentials()), | ||
|
@@ -350,23 +343,8 @@ contract StakingVault is IStakingVault, OwnableUpgradeable { | |
); | ||
|
||
totalAmount += deposit.amount; | ||
depositDataBatchXorRoot ^= keccak256(abi.encodePacked(deposit.depositDataRoot)); | ||
} | ||
|
||
if ( | ||
!SignatureChecker.isValidSignatureNow( | ||
$.depositGuardian, | ||
keccak256( | ||
abi.encodePacked( | ||
DEPOSIT_GUARDIAN_MESSAGE_PREFIX, | ||
_expectedGlobalDepositRoot, | ||
depositDataBatchXorRoot | ||
) | ||
), | ||
_signature | ||
) | ||
) revert DepositGuardianSignatureInvalid(); | ||
|
||
emit DepositedToBeaconChain(msg.sender, numberOfDeposits, totalAmount); | ||
} | ||
|
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.