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

[VAULTS] PredepositGuarantee #932

Open
wants to merge 46 commits into
base: feat/vaults
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
c19a543
feat: verify deposit guardian upon deposit
failingtwice Jan 27, 2025
b710b58
feat: add comment
failingtwice Jan 27, 2025
86f80bf
fix: use bytes for sig and XOR for aggregate root
failingtwice Jan 28, 2025
a0de885
fix: remove unused imports
failingtwice Jan 28, 2025
295ba6e
fix: elaborate comment
failingtwice Jan 28, 2025
938a519
feat: predeposit guardian concept WIP, full of bugs and errors
failingtwice Jan 28, 2025
34d203d
refactor: cleanup logic
failingtwice Jan 29, 2025
21f475e
feat: remove unnecessary mapping
failingtwice Jan 29, 2025
331a6e6
feat: remove node operator check
failingtwice Jan 30, 2025
2c84783
fix: rename
failingtwice Jan 30, 2025
ccde1b2
fix: comment on naming
failingtwice Jan 30, 2025
d0954f7
fix: comment on naming
failingtwice Jan 30, 2025
c5312c0
feat: add accounting&delegation to predeposit guardian
Jeday Jan 30, 2025
9d2b349
feat: proof validation
Jeday Jan 31, 2025
e4d3ebc
fix: clean up errors
Jeday Jan 31, 2025
74917ee
fix: mitigate mal staking vault
Jeday Jan 31, 2025
a17c375
fix: prove using validator container
Jeday Feb 3, 2025
8674bba
fix: withdraw
Jeday Feb 3, 2025
87c7e01
fix: use uint256 for collateral
Jeday Feb 3, 2025
ad9e476
fix: merge prove flows
Jeday Feb 3, 2025
5212738
fix: move predeposit to vaults
Jeday Feb 3, 2025
0d5f663
test: pdg cl verifier test
Jeday Feb 4, 2025
ae6d487
fix: use correct merkle implementation
Jeday Feb 4, 2025
2fc25b6
feat: add predeposit guarantee to vault factory
Jeday Feb 5, 2025
563d852
docs: update libs with refs
Jeday Feb 5, 2025
d2f5f24
fix: remove GIndex usage
Jeday Feb 5, 2025
4ef7550
fix: rework no voucher
Jeday Feb 6, 2025
6d6242b
fix: rewrite hashTreeRoot for calldata
Jeday Feb 6, 2025
ecc06c6
test: local testing merkle tree
Jeday Feb 6, 2025
ef47aed
feat: enhance PredepositGuarantee with events and improved error hand…
DiRaiks Feb 6, 2025
3bb45e8
feat: rework for wc proof
Jeday Feb 7, 2025
500e8be
feat: clean up SSZ lib
Jeday Feb 7, 2025
56b7752
docs: clProofVerifier
Jeday Feb 7, 2025
364e1e7
test: update to childBlockTimestamp
Jeday Feb 8, 2025
04a70a9
Merge branch 'predeposit-guardian' of github.com:lidofinance/core int…
DiRaiks Feb 10, 2025
3576bb9
refactor: simplify node operator bond top-up logic
DiRaiks Feb 10, 2025
16014b0
test: export PG test helpers
Jeday Feb 10, 2025
7b61c6a
Merge pull request #936 from lidofinance/predeposit-guardian-fixes
Jeday Feb 10, 2025
6e3b647
feat: integrate predeposit guarantee into locator
Jeday Feb 10, 2025
8c45174
fix: add whenResumed
Jeday Feb 10, 2025
60b3157
fix: move up from internal function
Jeday Feb 11, 2025
3223ab5
test: pdg happy path
Jeday Feb 11, 2025
917c685
fix: enforce balance multiple of ether
Jeday Feb 11, 2025
966ab9c
Merge branch 'feat/vaults' of github.com:lidofinance/core into predep…
Jeday Feb 11, 2025
e79014d
feat: add PDG integration to dashboard
Jeday Feb 12, 2025
d077613
feat: allow GIndex change via slot proof
Jeday Feb 13, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
157 changes: 157 additions & 0 deletions contracts/0.8.25/vaults/PredepositGuardian.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
// SPDX-FileCopyrightText: 2025 Lido <[email protected]>
// SPDX-License-Identifier: GPL-3.0

// See contracts/COMPILERS.md
pragma solidity 0.8.25;

import {StakingVault} from "./StakingVault.sol";

// TODO: think about naming. It's not a deposit guardian, it's the depositor itself
// TODO: minor UX improvement: perhaps there's way to reuse predeposits for a different validator without withdrawing
contract PredepositGuardian {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
contract PredepositGuardian {
contract PredepositGuarantee {

Copy link
Contributor

Choose a reason for hiding this comment

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

fixed

uint256 public constant PREDEPOSIT_AMOUNT = 1 ether;

mapping(bytes32 validatorId => bool isPreDeposited) public validatorPredeposits;
mapping(bytes32 validatorId => bytes32 withdrawalCredentials) public validatorWithdrawalCredentials;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
mapping(bytes32 validatorId => bool isPreDeposited) public validatorPredeposits;
mapping(bytes32 validatorId => bytes32 withdrawalCredentials) public validatorWithdrawalCredentials;
mapping(address nodeOperator => uint256) public nodeOperatorGuarantee;
mapping(bytes validatorPubkey => bytes32 withdrawalCredentials) public validatorWithdrawalCredentials;

Copy link
Contributor

Choose a reason for hiding this comment

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

fixed


// Question: predeposit is permissionless, i.e. the msg.sender doesn't have to be the node operator,
// however, the deposit will still revert if it wasn't signed with the validator private key
Copy link
Contributor

Choose a reason for hiding this comment

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

didn't get this

Copy link
Contributor

Choose a reason for hiding this comment

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

this function can't be permissionless

Copy link
Contributor

Choose a reason for hiding this comment

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

  • let's include accounting and 'outsourcing' for funding

function predeposit(StakingVault stakingVault, StakingVault.Deposit[] calldata deposits) external payable {
if (deposits.length == 0) revert PredepositNoDeposits();
if (msg.value % PREDEPOSIT_AMOUNT != 0) revert PredepositValueNotMultipleOfOneEther();
if (msg.value / PREDEPOSIT_AMOUNT != deposits.length) revert PredepositValueNotMatchingNumberOfDeposits();

for (uint256 i = 0; i < deposits.length; i++) {
StakingVault.Deposit calldata deposit = deposits[i];

bytes32 validatorId = keccak256(deposit.pubkey);

// cannot predeposit a validator that is already predeposited
if (validatorPredeposits[validatorId]) revert PredepositValidatorAlreadyPredeposited();

// cannot predeposit a validator that has withdrawal credentials already proven
if (validatorWithdrawalCredentials[validatorId] != bytes32(0))
revert PredepositValidatorWithdrawalCredentialsAlreadyProven();

// cannot predeposit a validator with a deposit amount that is not 1 ether
if (deposit.amount != PREDEPOSIT_AMOUNT) revert PredepositDepositAmountInvalid();

validatorPredeposits[validatorId] = true;
}

stakingVault.depositToBeaconChain(deposits);
}

function proveValidatorWithdrawalCredentials(
Copy link
Contributor

Choose a reason for hiding this comment

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

should combine prove + deposit

Copy link
Contributor

Choose a reason for hiding this comment

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

fixed

bytes32[] calldata /* proof */,
bytes calldata _pubkey,
bytes32 _withdrawalCredentials
) external {
// TODO: proof logic
// revert if proof is invalid

validatorWithdrawalCredentials[keccak256(_pubkey)] = _withdrawalCredentials;
}

function depositToProvenValidators(
StakingVault _stakingVault,
StakingVault.Deposit[] calldata _deposits
) external payable {
if (msg.sender != _stakingVault.nodeOperator()) revert DepositSenderNotNodeOperator();

for (uint256 i = 0; i < _deposits.length; i++) {
StakingVault.Deposit calldata deposit = _deposits[i];
bytes32 validatorId = keccak256(deposit.pubkey);

if (validatorWithdrawalCredentials[validatorId] != _stakingVault.withdrawalCredentials()) {
revert DepositToUnprovenValidator();
}
}

_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 withdrawDisprovenPredeposits(
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure what should be done here

Copy link
Contributor

Choose a reason for hiding this comment

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

let's implement optionality in Dashboard (either fund stVault or withdraw to the outer address)

StakingVault _stakingVault,
bytes32[] calldata _validatorIds,
address _recipient
) external {
if (msg.sender != _stakingVault.owner()) revert WithdrawSenderNotStakingVaultOwner();
if (_recipient == address(0)) revert WithdrawRecipientZeroAddress();

uint256 validatorsLength = _validatorIds.length;
for (uint256 i = 0; i < validatorsLength; i++) {
bytes32 validatorId = _validatorIds[i];

// cannot withdraw predeposit for a validator that is not pre-deposited
if (!validatorPredeposits[validatorId]) {
revert WithdrawValidatorNotPreDeposited();
}

// cannot withdraw predeposit for a validator that has withdrawal credentials matching the vault's withdrawal credentials
if (validatorWithdrawalCredentials[validatorId] == _stakingVault.withdrawalCredentials()) {
revert WithdrawValidatorWithdrawalCredentialsMatchStakingVault();
}

// set flag to false to prevent double withdrawal
validatorPredeposits[validatorId] = false;

(bool success, ) = _recipient.call{value: 1 ether}("");
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ maybe prevent direct calls to my stVault here (to not lose funds)

Copy link
Contributor

Choose a reason for hiding this comment

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

fixed

if (!success) revert WithdrawValidatorTransferFailed();
}
}

// called by the node operator if the predeposited validator has the same withdrawal credentials as the vault's withdrawal credentials,
// i.e. node operator was honest
function withdrawProvenPredeposits(
StakingVault _stakingVault,
bytes32[] calldata _validatorIds,
address _recipient
) external {
uint256 validatorsLength = _validatorIds.length;
for (uint256 i = 0; i < validatorsLength; i++) {
bytes32 validatorId = _validatorIds[i];

if (msg.sender != _stakingVault.nodeOperator()) {
Jeday marked this conversation as resolved.
Show resolved Hide resolved
revert WithdrawSenderNotNodeOperator();
}

// cannot withdraw predeposit for a validator that is not pre-deposited
if (!validatorPredeposits[validatorId]) {
revert WithdrawValidatorNotPreDeposited();
}

// cannot withdraw predeposit for a validator that has withdrawal credentials not matching the vault's withdrawal credentials
if (validatorWithdrawalCredentials[validatorId] != _stakingVault.withdrawalCredentials()) {
revert WithdrawValidatorWithdrawalCredentialsNotMatchingStakingVault();
}

// set flag to false to prevent double withdrawal
validatorPredeposits[validatorId] = false;

(bool success, ) = _recipient.call{value: 1 ether}("");
if (!success) revert WithdrawValidatorTransferFailed();
}
}

error PredepositNoDeposits();
error PredepositValueNotMultipleOfOneEther();
error PredepositValueNotMatchingNumberOfDeposits();
error PredepositNodeOperatorNotMatching();
error PredepositValidatorAlreadyPredeposited();
error PredepositValidatorWithdrawalCredentialsAlreadyProven();
error PredepositDepositAmountInvalid();
error ValidatorNotPreDeposited();
error DepositSenderNotNodeOperator();
error DepositToUnprovenValidator();
error WithdrawSenderNotStakingVaultOwner();
error WithdrawRecipientZeroAddress();
error WithdrawValidatorNotPreDeposited();
error WithdrawValidatorWithdrawalCredentialsMatchStakingVault();
error WithdrawValidatorTransferFailed();
error WithdrawValidatorWithdrawalCredentialsNotMatchingStakingVault();
error WithdrawSenderNotNodeOperator();
error WithdrawValidatorDoesNotBelongToNodeOperator();
}
60 changes: 56 additions & 4 deletions contracts/0.8.25/vaults/StakingVault.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
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";

Expand Down Expand Up @@ -67,6 +68,8 @@ contract StakingVault is IStakingVault, OwnableUpgradeable {
uint128 locked;
int128 inOutDelta;
address nodeOperator;
// depositGuardian becomes the depositor, instead of just guardian, perhaps a renaming is needed 🌚
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe make depositGuardian immutable

Copy link
Contributor

Choose a reason for hiding this comment

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

Changable but only for unconnceted vaults

address depositGuardian;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

let's rename to trustedDepositor, or something

Copy link
Contributor

Choose a reason for hiding this comment

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

just depositor is also ok, trusted kind of excessive

bool beaconChainDepositsPaused;
}

Expand All @@ -76,6 +79,8 @@ contract StakingVault is IStakingVault, OwnableUpgradeable {
*/
uint64 private constant _VERSION = 1;

bytes32 public constant DEPOSIT_GUARDIAN_MESSAGE_PREFIX = keccak256("StakingVault.DepositGuardianMessagePrefix");
Jeday marked this conversation as resolved.
Show resolved Hide resolved

/**
* @notice Address of `VaultHub`
* Set immutably in the constructor to avoid storage costs
Expand Down Expand Up @@ -122,6 +127,7 @@ contract StakingVault is IStakingVault, OwnableUpgradeable {
function initialize(address _owner, address _nodeOperator, bytes calldata /* _params */) external initializer {
__Ownable_init(_owner);
_getStorage().nodeOperator = _nodeOperator;
_getStorage().depositGuardian = _owner;
}

/**
Expand Down Expand Up @@ -315,22 +321,26 @@ contract StakingVault is IStakingVault, OwnableUpgradeable {
*/
function depositToBeaconChain(Deposit[] calldata _deposits) external {
Copy link
Contributor

Choose a reason for hiding this comment

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

strawman sanity checks would be needed for the input data

Copy link
Contributor

Choose a reason for hiding this comment

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

Added BLS check todo, needs clarification on best use for precompile

if (_deposits.length == 0) revert ZeroArgument("_deposits");
ERC7201Storage storage $ = _getStorage();
if (!isBalanced()) revert Unbalanced();

if (msg.sender != $.nodeOperator) revert NotAuthorized("depositToBeaconChain", msg.sender);
ERC7201Storage storage $ = _getStorage();
if ($.beaconChainDepositsPaused) revert BeaconChainDepositsArePaused();
if (!isBalanced()) revert Unbalanced();
if (msg.sender != $.depositGuardian) revert NotAuthorized("depositToBeaconChain", msg.sender);

uint256 totalAmount = 0;
uint256 numberOfDeposits = _deposits.length;

uint256 totalAmount = 0;

for (uint256 i = 0; i < numberOfDeposits; i++) {
Deposit calldata deposit = _deposits[i];

BEACON_CHAIN_DEPOSIT_CONTRACT.deposit{value: deposit.amount}(
deposit.pubkey,
bytes.concat(withdrawalCredentials()),
deposit.signature,
deposit.depositDataRoot
);

totalAmount += deposit.amount;
}

Expand Down Expand Up @@ -404,6 +414,26 @@ contract StakingVault is IStakingVault, OwnableUpgradeable {
emit Reported(_valuation, _inOutDelta, _locked);
}

/**
* @notice Sets the deposit guardian
* @param _depositGuardian The address of the deposit guardian
* @dev In case where the deposit guardian is a contract, it must implement EIP-1271.
* The interface check for EIP-1271 is omitted because the only way to check for whether an account is a contract
* is to call `extcodesize` on it. This method is not reliable, as it can be broken, for instance, by CREATE2-deployed contracts.
* So to avoid false positives, the contract shifts this responsibility to the owner.
* If a contract mistakenly assigned as a deposit guardian is not a valid EIP-1271 signer,
* the owner can simply set it to a different address.
*/
function setDepositGuardian(address _depositGuardian) external onlyOwner {
if (_depositGuardian == address(0)) revert ZeroArgument("_depositGuardian");

ERC7201Storage storage $ = _getStorage();
address oldDepositGuardian = $.depositGuardian;
$.depositGuardian = _depositGuardian;

emit DepositGuardianSet(oldDepositGuardian, _depositGuardian);
}

/**
* @notice Computes the deposit data root for a validator deposit
* @param _pubkey Validator public key, 48 bytes
Expand Down Expand Up @@ -554,6 +584,13 @@ contract StakingVault is IStakingVault, OwnableUpgradeable {
*/
event BeaconChainDepositsResumed();

/**
* @notice Emitted when the deposit guardian is set
* @param oldDepositGuardian The address of the old deposit guardian
* @param newDepositGuardian The address of the new deposit guardian
*/
event DepositGuardianSet(address oldDepositGuardian, address newDepositGuardian);

/**
* @notice Thrown when an invalid zero value is passed
* @param name Name of the argument that was zero
Expand Down Expand Up @@ -617,6 +654,21 @@ contract StakingVault is IStakingVault, OwnableUpgradeable {
*/
error UnrecoverableError();

/**
* @notice Thrown when the global deposit root does not match the expected global deposit root
*/
error GlobalDepositRootMismatch(bytes32 expected, bytes32 actual);
Jeday marked this conversation as resolved.
Show resolved Hide resolved

/**
* @notice Thrown when the guardian signature is invalid
*/
error DepositGuardianSignatureInvalid();

/**
* @notice Thrown when the deposit guardian is not an EIP-1271 contract
*/
error DepositGuardianContractDoesNotSupportEIP1271();

/**
* @notice Thrown when trying to pause deposits to beacon chain while deposits are already paused
*/
Expand Down
24 changes: 23 additions & 1 deletion contracts/0.8.25/vaults/interfaces/IStakingVault.sol
Original file line number Diff line number Diff line change
Expand Up @@ -28,26 +28,48 @@ interface IStakingVault {
}

function initialize(address _owner, address _operator, bytes calldata _params) external;
function version() external pure returns(uint64);

function version() external pure returns (uint64);

function getInitializedVersion() external view returns (uint64);

function vaultHub() external view returns (address);

function depositContract() external view returns (address);

function nodeOperator() external view returns (address);

function locked() external view returns (uint256);

function valuation() external view returns (uint256);

function isBalanced() external view returns (bool);

function unlocked() external view returns (uint256);

function inOutDelta() external view returns (int256);

function beaconChainDepositsPaused() external view returns (bool);

function withdrawalCredentials() external view returns (bytes32);

function fund() external payable;

function withdraw(address _recipient, uint256 _ether) external;

function depositToBeaconChain(Deposit[] calldata _deposits) external;

function requestValidatorExit(bytes calldata _pubkeys) external;

function lock(uint256 _locked) external;

function rebalance(uint256 _ether) external;

function pauseBeaconChainDeposits() external;

function resumeBeaconChainDeposits() external;

function latestReport() external view returns (Report memory);

function report(uint256 _valuation, int256 _inOutDelta, uint256 _locked) external;
}
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ contract StakingVault__HarnessForTestUpgrade is IStakingVault, OwnableUpgradeabl
}

event InitializedV2();

function __StakingVault_init_v2() internal onlyInitializing {
emit InitializedV2();
}
Expand Down Expand Up @@ -84,24 +85,33 @@ contract StakingVault__HarnessForTestUpgrade is IStakingVault, OwnableUpgradeabl
}

function depositToBeaconChain(Deposit[] calldata _deposits) external {}

function fund() external payable {}

function inOutDelta() external view returns (int256) {
return -1;
}

function isBalanced() external view returns (bool) {
return true;
}

function nodeOperator() external view returns (address) {
return _getVaultStorage().nodeOperator;
}

function rebalance(uint256 _ether) external {}

function report(uint256 _valuation, int256 _inOutDelta, uint256 _locked) external {}

function requestValidatorExit(bytes calldata _pubkeys) external {}

function lock(uint256 _locked) external {}

function locked() external view returns (uint256) {
return 0;
}

function unlocked() external view returns (uint256) {
return 0;
}
Expand All @@ -125,6 +135,7 @@ contract StakingVault__HarnessForTestUpgrade is IStakingVault, OwnableUpgradeabl
}

function pauseBeaconChainDeposits() external {}

function resumeBeaconChainDeposits() external {}

error ZeroArgument(string name);
Expand Down
Loading