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

Feat/withdrawal credentials #904

Open
wants to merge 12 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
97 changes: 80 additions & 17 deletions contracts/0.8.9/WithdrawalVault.sol
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@
import "@openzeppelin/contracts-v4.4/token/ERC20/utils/SafeERC20.sol";

import {Versioned} from "./utils/Versioned.sol";
import {AccessControlEnumerable} from "./utils/access/AccessControlEnumerable.sol";
import {TriggerableWithdrawals} from "./lib/TriggerableWithdrawals.sol";
import { ILidoLocator } from "../common/interfaces/ILidoLocator.sol";
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
import { ILidoLocator } from "../common/interfaces/ILidoLocator.sol";
import {ILidoLocator} from "../common/interfaces/ILidoLocator.sol";


interface ILido {
/**
Expand All @@ -22,12 +25,14 @@
/**
* @title A vault for temporary storage of withdrawals
*/
contract WithdrawalVault is Versioned {
contract WithdrawalVault is AccessControlEnumerable, Versioned {
using SafeERC20 for IERC20;

ILido public immutable LIDO;
address public immutable TREASURY;

bytes32 public constant ADD_FULL_WITHDRAWAL_REQUEST_ROLE = keccak256("ADD_FULL_WITHDRAWAL_REQUEST_ROLE");

// Events
/**
* Emitted when the ERC20 `token` recovered (i.e. transferred)
Expand All @@ -42,34 +47,44 @@
event ERC721Recovered(address indexed requestedBy, address indexed token, uint256 tokenId);

// Errors
error LidoZeroAddress();
error TreasuryZeroAddress();
error ZeroAddress();
error NotLido();
error NotEnoughEther(uint256 requested, uint256 balance);
error ZeroAmount();
error InsufficientTriggerableWithdrawalFee(uint256 providedTotalFee, uint256 requiredTotalFee, uint256 requestCount);
error TriggerableWithdrawalRefundFailed();

/**
* @param _lido the Lido token (stETH) address
* @param _treasury the Lido treasury address (see ERC20/ERC721-recovery interfaces)
*/
constructor(ILido _lido, address _treasury) {
if (address(_lido) == address(0)) {
revert LidoZeroAddress();
}
if (_treasury == address(0)) {
revert TreasuryZeroAddress();
}
constructor(address _lido, address _treasury) {
_requireNonZero(_lido);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
_requireNonZero(_lido);
_onlyNonZeroAddress(_lido);

_requireNonZero(_treasury);

LIDO = _lido;
LIDO = ILido(_lido);
TREASURY = _treasury;
}

/**
* @notice Initialize the contract explicitly.
* Sets the contract version to '1'.
*/
function initialize() external {
_initializeContractVersionTo(1);
/// @notice Initializes the contract. Can be called only once.
/// @param _admin Lido DAO Aragon agent contract address.
/// @dev Proxy initialization method.
function initialize(address _admin) external {
// Initializations for v0 --> v2
_checkContractVersion(0);

_initialize_v2(_admin);
_initializeContractVersionTo(2);
}

/// @notice Finalizes upgrade to v2 (from v1). Can be called only once.
/// @param _admin Lido DAO Aragon agent contract address.
function finalizeUpgrade_v2(address _admin) external {
// Finalization for v1 --> v2
_checkContractVersion(1);

_initialize_v2(_admin);
_updateContractVersion(2);
}

/**
Expand Down Expand Up @@ -122,4 +137,52 @@

_token.transferFrom(address(this), TREASURY, _tokenId);
}

/**
* @dev Adds full withdrawal requests for the provided public keys.
* The validator will fully withdraw and exit its duties as a validator.
* @param pubkeys An array of public keys for the validators requesting full withdrawals.
*/
function addFullWithdrawalRequests(
bytes calldata pubkeys
) external payable onlyRole(ADD_FULL_WITHDRAWAL_REQUEST_ROLE) {
uint256 prevBalance = address(this).balance - msg.value;

uint256 minFeePerRequest = TriggerableWithdrawals.getWithdrawalRequestFee();
uint256 totalFee = pubkeys.length / TriggerableWithdrawals.PUBLIC_KEY_LENGTH * minFeePerRequest;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
uint256 totalFee = pubkeys.length / TriggerableWithdrawals.PUBLIC_KEY_LENGTH * minFeePerRequest;
uint256 totalFee = pubkeys.length / TriggerableWithdrawals.PUBLIC_KEY_LENGTH * minFeePerRequest;


if(totalFee > msg.value) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if(totalFee > msg.value) {
if (totalFee > msg.value) {

revert InsufficientTriggerableWithdrawalFee(
msg.value,
totalFee,
pubkeys.length / TriggerableWithdrawals.PUBLIC_KEY_LENGTH
);
}

TriggerableWithdrawals.addFullWithdrawalRequests(pubkeys, minFeePerRequest);

uint256 refund = msg.value - totalFee;
if (refund > 0) {
(bool success, ) = msg.sender.call{value: refund}("");

if (!success) {
revert TriggerableWithdrawalRefundFailed();
}
}

assert(address(this).balance == prevBalance);
}
Comment on lines +146 to +174

Check warning

Code scanning / Slither

Divide before multiply Medium

Comment on lines +146 to +174

Check warning

Code scanning / Slither

Dangerous strict equalities Medium


function getWithdrawalRequestFee() external view returns (uint256) {
return TriggerableWithdrawals.getWithdrawalRequestFee();
}

function _requireNonZero(address _address) internal pure {
if (_address == address(0)) revert ZeroAddress();
}

function _initialize_v2(address _admin) internal {
_requireNonZero(_admin);
_setupRole(DEFAULT_ADMIN_ROLE, _admin);
}
}
163 changes: 163 additions & 0 deletions contracts/0.8.9/lib/TriggerableWithdrawals.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
// SPDX-FileCopyrightText: 2023 Lido <[email protected]>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// SPDX-FileCopyrightText: 2023 Lido <[email protected]>
// SPDX-FileCopyrightText: 2025 Lido <[email protected]>

// SPDX-License-Identifier: GPL-3.0

pragma solidity 0.8.9;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
pragma solidity 0.8.9;
// solhint-disable-next-line lido/fixed-compiler-version
pragma solidity >=0.8.9 <0.9.0;

Also, consider moving to common libs?

library TriggerableWithdrawals {
address constant WITHDRAWAL_REQUEST = 0x0c15F14308530b7CDB8460094BbB9cC28b9AaaAA;
Copy link
Member

Choose a reason for hiding this comment

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

Add some comment with "validation" link?

Suggested change
address constant WITHDRAWAL_REQUEST = 0x0c15F14308530b7CDB8460094BbB9cC28b9AaaAA;
/// @dev https://eips.ethereum.org/EIPS/eip-7002#configuration
address constant WITHDRAWAL_REQUEST = 0x0c15F14308530b7CDB8460094BbB9cC28b9AaaAA;

Copy link
Member

@tamtamchik tamtamchik Jan 20, 2025

Choose a reason for hiding this comment

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

May this contract address be changed on testnets to something else (as, for example, with the deposit contract on Holesky)? We'll have to use a separate library for it?

uint256 internal constant WITHDRAWAL_REQUEST_CALLDATA_LENGTH = 56;
uint256 internal constant PUBLIC_KEY_LENGTH = 48;
uint256 internal constant WITHDRAWAL_AMOUNT_LENGTH = 8;

error MismatchedArrayLengths(uint256 keysCount, uint256 amountsCount);
error InsufficientBalance(uint256 balance, uint256 totalWithdrawalFee);
error InsufficientRequestFee(uint256 feePerRequest, uint256 minFeePerRequest);

error WithdrawalRequestFeeReadFailed();
error WithdrawalRequestAdditionFailed(bytes callData);
error NoWithdrawalRequests();
error PartialWithdrawalRequired(uint256 index);
error InvalidPublicKeyLength();

/**
* @dev Adds full withdrawal requests for the provided public keys.
* The validator will fully withdraw and exit its duties as a validator.
* @param pubkeys An array of public keys for the validators requesting full withdrawals.
*/
function addFullWithdrawalRequests(
bytes calldata pubkeys,
uint256 feePerRequest
) internal {
uint256 keysCount = _validateAndCountPubkeys(pubkeys);
feePerRequest = _validateAndAdjustFee(feePerRequest, keysCount);

bytes memory callData = new bytes(56);

for (uint256 i = 0; i < keysCount; i++) {
_copyPubkeyToMemory(pubkeys, callData, i);

(bool success, ) = WITHDRAWAL_REQUEST.call{value: feePerRequest}(callData);

if (!success) {
revert WithdrawalRequestAdditionFailed(callData);
}
}
Copy link
Member

Choose a reason for hiding this comment

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

emit WithdrawalRequestAdded(...) ? Or is this done on the WITHDRAWAL_REQUEST contract side, or should it be done on the side of the contract that uses this library?

}

/**
* @dev Adds partial withdrawal requests for the provided public keys with corresponding amounts.
* A partial withdrawal is any withdrawal where the amount is greater than zero.
* A full withdrawal is any withdrawal where the amount is zero.
* This allows withdrawal of any balance exceeding 32 ETH (e.g., if a validator has 35 ETH, up to 3 ETH can be withdrawn).
* However, the protocol enforces a minimum balance of 32 ETH per validator, even if a higher amount is requested.
* @param pubkeys An array of public keys for the validators requesting withdrawals.
* @param amounts An array of corresponding withdrawal amounts for each public key.
*/
function addPartialWithdrawalRequests(
bytes calldata pubkeys,
uint64[] calldata amounts,
uint256 feePerRequest
) internal {
for (uint256 i = 0; i < amounts.length; i++) {
if (amounts[i] == 0) {
revert PartialWithdrawalRequired(i);
}
}

addWithdrawalRequests(pubkeys, amounts, feePerRequest);
}

/**
* @dev Adds partial or full withdrawal requests for the provided public keys with corresponding amounts.
* A partial withdrawal is any withdrawal where the amount is greater than zero.
* This allows withdrawal of any balance exceeding 32 ETH (e.g., if a validator has 35 ETH, up to 3 ETH can be withdrawn).
* However, the protocol enforces a minimum balance of 32 ETH per validator, even if a higher amount is requested.
* @param pubkeys An array of public keys for the validators requesting withdrawals.
* @param amounts An array of corresponding withdrawal amounts for each public key.
*/
function addWithdrawalRequests(
bytes calldata pubkeys,
uint64[] calldata amounts,
uint256 feePerRequest
) internal {
uint256 keysCount = _validateAndCountPubkeys(pubkeys);

if (keysCount != amounts.length) {
revert MismatchedArrayLengths(keysCount, amounts.length);
}

feePerRequest = _validateAndAdjustFee(feePerRequest, keysCount);

bytes memory callData = new bytes(56);
for (uint256 i = 0; i < keysCount; i++) {
_copyPubkeyToMemory(pubkeys, callData, i);
_copyAmountToMemory(callData, amounts[i]);

(bool success, ) = WITHDRAWAL_REQUEST.call{value: feePerRequest}(callData);

if (!success) {
revert WithdrawalRequestAdditionFailed(callData);
}
}
}

/**
* @dev Retrieves the current withdrawal request fee.
* @return The minimum fee required per withdrawal request.
*/
function getWithdrawalRequestFee() internal view returns (uint256) {
(bool success, bytes memory feeData) = WITHDRAWAL_REQUEST.staticcall("");

if (!success) {
revert WithdrawalRequestFeeReadFailed();
}

return abi.decode(feeData, (uint256));
}

function _copyPubkeyToMemory(bytes calldata pubkeys, bytes memory target, uint256 keyIndex) private pure {
assembly {
calldatacopy(
add(target, 32),
add(pubkeys.offset, mul(keyIndex, PUBLIC_KEY_LENGTH)),
PUBLIC_KEY_LENGTH
)
}
}

function _copyAmountToMemory(bytes memory target, uint64 amount) private pure {
assembly {
mstore(add(target, 80), shl(192, amount))
}
}

function _validateAndCountPubkeys(bytes calldata pubkeys) private pure returns (uint256) {
if(pubkeys.length % PUBLIC_KEY_LENGTH != 0) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if(pubkeys.length % PUBLIC_KEY_LENGTH != 0) {
if (pubkeys.length % PUBLIC_KEY_LENGTH != 0) {

revert InvalidPublicKeyLength();
}

uint256 keysCount = pubkeys.length / PUBLIC_KEY_LENGTH;
if (keysCount == 0) {
revert NoWithdrawalRequests();
}

return keysCount;
}

function _validateAndAdjustFee(uint256 feePerRequest, uint256 keysCount) private view returns (uint256) {
uint256 minFeePerRequest = getWithdrawalRequestFee();

if (feePerRequest == 0) {
feePerRequest = minFeePerRequest;
}

if (feePerRequest < minFeePerRequest) {
revert InsufficientRequestFee(feePerRequest, minFeePerRequest);
}

if(address(this).balance < feePerRequest * keysCount) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if(address(this).balance < feePerRequest * keysCount) {
if (address(this).balance < feePerRequest * keysCount) {

revert InsufficientBalance(address(this).balance, feePerRequest * keysCount);
}

return feePerRequest;
}
}
5 changes: 5 additions & 0 deletions scripts/scratch/steps/0120-initialize-non-aragon-contracts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export async function main() {
const exitBusOracleAdmin = testnetAdmin;
const stakingRouterAdmin = testnetAdmin;
const withdrawalQueueAdmin = testnetAdmin;
const withdrawalVaultAdmin = testnetAdmin;

// Initialize NodeOperatorsRegistry

Expand Down Expand Up @@ -108,6 +109,10 @@ export async function main() {
{ from: deployer },
);

// Initialize WithdrawalVault
const withdrawalVault = await loadContract("WithdrawalVault", withdrawalVaultAddress);
await makeTx(withdrawalVault, "initialize", [withdrawalVaultAdmin], { from: deployer });

// Initialize WithdrawalQueue
const withdrawalQueue = await loadContract("WithdrawalQueueERC721", withdrawalQueueAddress);
await makeTx(withdrawalQueue, "initialize", [withdrawalQueueAdmin], { from: deployer });
Expand Down
38 changes: 38 additions & 0 deletions test/0.8.9/contracts/TriggerableWithdrawals_Harness.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
pragma solidity 0.8.9;

import {TriggerableWithdrawals} from "contracts/0.8.9/lib/TriggerableWithdrawals.sol";

contract TriggerableWithdrawals_Harness {
function addFullWithdrawalRequests(
bytes calldata pubkeys,
uint256 feePerRequest
) external {
TriggerableWithdrawals.addFullWithdrawalRequests(pubkeys, feePerRequest);
}

function addPartialWithdrawalRequests(
bytes calldata pubkeys,
uint64[] calldata amounts,
uint256 feePerRequest
) external {
TriggerableWithdrawals.addPartialWithdrawalRequests(pubkeys, amounts, feePerRequest);
}

function addWithdrawalRequests(
bytes calldata pubkeys,
uint64[] calldata amounts,
uint256 feePerRequest
) external {
TriggerableWithdrawals.addWithdrawalRequests(pubkeys, amounts, feePerRequest);
}

function getWithdrawalRequestFee() external view returns (uint256) {
return TriggerableWithdrawals.getWithdrawalRequestFee();
}

function getWithdrawalsContractAddress() public pure returns (address) {
return TriggerableWithdrawals.WITHDRAWAL_REQUEST;
}

function deposit() external payable {}
}
15 changes: 15 additions & 0 deletions test/0.8.9/contracts/WithdrawalVault__Harness.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// SPDX-License-Identifier: UNLICENSED
// for testing purposes only

pragma solidity 0.8.9;

import {WithdrawalVault} from "contracts/0.8.9/WithdrawalVault.sol";

contract WithdrawalVault__Harness is WithdrawalVault {
constructor(address _lido, address _treasury) WithdrawalVault(_lido, _treasury) {
}

function harness__initializeContractVersionTo(uint256 _version) external {
_initializeContractVersionTo(_version);
}
}
Loading
Loading