Skip to content

Commit

Permalink
feat: remove all use of the delayed withdrawal router (#524)
Browse files Browse the repository at this point in the history
* modify activateRestaking flow to use checkpointing
* remove withdrawNonBeaconChainETHBalanceWei in favor of checkpointing
  • Loading branch information
wadealexc authored Apr 25, 2024
1 parent 7aa93fb commit ebb1201
Show file tree
Hide file tree
Showing 6 changed files with 197 additions and 217 deletions.
1 change: 0 additions & 1 deletion script/deploy/mainnet/M2Deploy.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,6 @@ contract M2Deploy is Script, Test {
// Check updated storage values
require(eigenPod.hasRestaked(), "eigenPod.hasRestaked not set to true");
require(address(eigenPod).balance == 0, "eigenPod balance not 0 after activating restaking");
require(eigenPod.nonBeaconChainETHBalanceWei() == 0, "non beacon chain eth balance not 0");
require(
eigenPod.mostRecentWithdrawalTimestamp() == block.timestamp,
"eigenPod.mostRecentWithdrawalTimestamp not updated"
Expand Down
9 changes: 0 additions & 9 deletions src/contracts/interfaces/IEigenPod.sol
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,6 @@ interface IEigenPod {
/// @notice the amount of execution layer ETH in this contract that is staked in EigenLayer (i.e. withdrawn from beaconchain but not EigenLayer),
function withdrawableRestakedExecutionLayerGwei() external view returns (uint64);

/// @notice any ETH deposited into the EigenPod contract via the `receive` fallback function
function nonBeaconChainETHBalanceWei() external view returns (uint256);

/// @notice Used to initialize the pointers to contracts crucial to the pod's functionality, in beacon proxy construction from EigenPodManager
function initialize(address owner) external;

Expand Down Expand Up @@ -152,12 +149,6 @@ interface IEigenPod {
*/
function activateRestaking() external;

/// @notice Called by the pod owner to withdraw the balance of the pod when `hasRestaked` is set to false
function withdrawBeforeRestaking() external;

/// @notice Called by the pod owner to withdraw the nonBeaconChainETHBalanceWei
function withdrawNonBeaconChainETHBalanceWei(address recipient, uint256 amountToWithdraw) external;

/// @notice called by owner of a pod to remove any ERC20s deposited in the pod
function recoverTokens(IERC20[] memory tokenList, uint256[] memory amountsToWithdraw, address recipient) external;
}
74 changes: 32 additions & 42 deletions src/contracts/pods/EigenPod.sol
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,6 @@ contract EigenPod is

/// @notice payable fallback function that receives ether deposited to the eigenpods contract
receive() external payable {
nonBeaconChainETHBalanceWei += msg.value;
emit NonBeaconChainETHReceived(msg.value);
}

Expand All @@ -163,7 +162,7 @@ contract EigenPod is
* change in ACTIVE validator balance is tracked, and any validators with 0 balance are marked `WITHDRAWN`.
* @dev Once finalized, the pod owner is awarded shares corresponding to:
* - the total change in their ACTIVE validator balances
* - any ETH in the pod not already awarded shares.
* - any ETH in the pod not already awarded shares
* @dev A checkpoint cannot be created if the pod already has an outstanding checkpoint. If
* this is the case, the pod owner MUST complete the existing checkpoint before starting a new one.
*/
Expand Down Expand Up @@ -362,20 +361,6 @@ contract EigenPod is
_startCheckpoint();
}

/// @notice Called by the pod owner to withdraw the nonBeaconChainETHBalanceWei
function withdrawNonBeaconChainETHBalanceWei(
address recipient,
uint256 amountToWithdraw
) external onlyEigenPodOwner onlyWhenNotPaused(PAUSED_NON_PROOF_WITHDRAWALS) {
require(
amountToWithdraw <= nonBeaconChainETHBalanceWei,
"EigenPod.withdrawnonBeaconChainETHBalanceWei: amountToWithdraw is greater than nonBeaconChainETHBalanceWei"
);
nonBeaconChainETHBalanceWei -= amountToWithdraw;
emit NonBeaconChainETHWithdrawn(recipient, amountToWithdraw);
_sendETH_AsDelayedWithdrawal(recipient, amountToWithdraw);
}

/// @notice called by owner of a pod to remove any ERC20s deposited in the pod
function recoverTokens(
IERC20[] memory tokenList,
Expand Down Expand Up @@ -403,14 +388,9 @@ contract EigenPod is
hasNeverRestaked
{
hasRestaked = true;
_processWithdrawalBeforeRestaking(podOwner);

emit RestakingActivated(podOwner);
}

/// @notice Called by the pod owner to withdraw the balance of the pod when `hasRestaked` is set to false
function withdrawBeforeRestaking() external onlyEigenPodOwner hasNeverRestaked {
_processWithdrawalBeforeRestaking(podOwner);
_startCheckpoint();
}

/// @notice Called by EigenPodManager when the owner wants to create another ETH validator.
Expand Down Expand Up @@ -565,27 +545,47 @@ contract EigenPod is
return balanceDeltaGwei;
}

/**
* @dev Initiate a checkpoint proof by snapshotting both the pod's ETH balance and the
* current block's parent block root. After providing a checkpoint proof for each of the
* pod's ACTIVE validators, the pod's ETH balance is awarded shares and can be withdrawn.
* @dev ACTIVE validators are validators with verified withdrawal credentials (See
* `verifyWithdrawalCredentials` for details)
* @dev If the pod does not have any ACTIVE validators, the checkpoint is automatically
* finalized.
* @dev Once started, a checkpoint MUST be completed! It is not possible to start a
* checkpoint if the existing one is incomplete.
*/
function _startCheckpoint() internal {
require(
currentCheckpointTimestamp == 0,
"EigenPod._startCheckpoint: must finish previous checkpoint before starting another"
);

// Snapshot pod balance at the start of the checkpoint. Once the checkpoint is finalized,
// this amount will be added to the total validator balance delta and credited as shares.
uint256 podBalanceWei =
address(this).balance
- nonBeaconChainETHBalanceWei
- (withdrawableRestakedExecutionLayerGwei * GWEI_TO_WEI);

// Snapshot pod balance at the start of the checkpoint, subtracting pod balance that has
// previously been credited with shares. Once the checkpoint is finalized, `podBalanceGwei`
// will be added to the total validator balance delta and credited as shares.
//
// Note: On finalization, `podBalanceGwei` is added to `withdrawableRestakedExecutionLayerGwei`
// to denote that it has been credited with shares. Because this value is denominated in gwei,
// `podBalanceGwei` is also converted to a gwei amount here. This means that any sub-gwei amounts
// sent to the pod are not credited with shares and are therefore not withdrawable.
// This can be addressed by topping up a pod's balance to a value divisible by 1 gwei.
uint256 podBalanceGwei =
(address(this).balance / GWEI_TO_WEI) - withdrawableRestakedExecutionLayerGwei;

// Create checkpoint using the previous block's root for proofs, and the current
// `activeValidatorCount` as the number of checkpoint proofs needed to finalize
// the checkpoint.
Checkpoint memory checkpoint = Checkpoint({
beaconBlockRoot: _getParentBlockRoot(uint64(block.timestamp)),
podBalanceGwei: podBalanceWei / GWEI_TO_WEI,
podBalanceGwei: podBalanceGwei,
balanceDeltasGwei: 0,
proofsRemaining: activeValidatorCount
});

// Place checkpoint in storage
// Place checkpoint in storage. If `proofsRemaining` is 0, the checkpoint
// is automatically finalized.
currentCheckpointTimestamp = uint64(block.timestamp);
_updateCheckpoint(checkpoint);

Expand All @@ -596,7 +596,7 @@ contract EigenPod is
* @dev Finish progress on a checkpoint and store it in state.
* @dev If the checkpoint has no proofs remaining, it is finalized:
* - a share delta is calculated and sent to the `EigenPodManager`
* - the checkpointed `podBalance` is added to `withdrawableRestakedExecutionLayerGwei`
* - the checkpointed `podBalanceGwei` is added to `withdrawableRestakedExecutionLayerGwei`
* - `lastCheckpointTimestamp` is updated
* - `currentCheckpoint` and `currentCheckpointTimestamp` are deleted
*/
Expand All @@ -621,20 +621,10 @@ contract EigenPod is
}
}

function _processWithdrawalBeforeRestaking(address _podOwner) internal {
mostRecentWithdrawalTimestamp = uint32(block.timestamp);
nonBeaconChainETHBalanceWei = 0;
_sendETH_AsDelayedWithdrawal(_podOwner, address(this).balance);
}

function _sendETH(address recipient, uint256 amountWei) internal {
Address.sendValue(payable(recipient), amountWei);
}

function _sendETH_AsDelayedWithdrawal(address recipient, uint256 amountWei) internal {
delayedWithdrawalRouter.createDelayedWithdrawal{value: amountWei}(podOwner, recipient);
}

/// @notice Query the 4788 oracle to get the parent block root of the slot with the given `timestamp`
/// @param timestamp of the block for which the parent block root will be returned. MUST correspond
/// to an existing slot within the last 24 hours. If the slot at `timestamp` was skipped, this method
Expand Down
2 changes: 1 addition & 1 deletion src/contracts/pods/EigenPodStorage.sol
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ abstract contract EigenPodStorage is IEigenPod {
mapping(bytes32 => ValidatorInfo) internal _validatorPubkeyHashToInfo;

/// @notice This variable tracks any ETH deposited into this contract via the `receive` fallback function
uint256 public nonBeaconChainETHBalanceWei;
uint256 internal __deprecated_nonBeaconChainETHBalanceWei;

/// @notice This variable tracks the total amount of partial withdrawals claimed via merkle proofs prior to a switch to ZK proofs for claiming partial withdrawals
uint64 __deprecated_sumOfPartialWithdrawalsClaimedGwei;
Expand Down
174 changes: 87 additions & 87 deletions src/test/EigenPod.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -273,39 +273,39 @@ contract EigenPodTests is ProofParsing, EigenPodPausingConstants {
cheats.stopPrank();
}

function testWithdrawBeforeRestaking() public {
testStaking();
IEigenPod pod = eigenPodManager.getPod(podOwner);
// function testWithdrawBeforeRestaking() public {
// testStaking();
// IEigenPod pod = eigenPodManager.getPod(podOwner);

//simulate that hasRestaked is set to false, so that we can test withdrawBeforeRestaking for pods deployed before M2 activation
cheats.store(address(pod), bytes32(uint256(52)), bytes32(uint256(1)));
require(pod.hasRestaked() == false, "Pod should not be restaked");
// //simulate that hasRestaked is set to false, so that we can test withdrawBeforeRestaking for pods deployed before M2 activation
// cheats.store(address(pod), bytes32(uint256(52)), bytes32(uint256(1)));
// require(pod.hasRestaked() == false, "Pod should not be restaked");

// simulate a withdrawal
cheats.deal(address(pod), stakeAmount);
cheats.startPrank(podOwner);
cheats.expectEmit(true, true, true, true, address(delayedWithdrawalRouter));
emit DelayedWithdrawalCreated(
podOwner,
podOwner,
stakeAmount,
delayedWithdrawalRouter.userWithdrawalsLength(podOwner)
);
// // simulate a withdrawal
// cheats.deal(address(pod), stakeAmount);
// cheats.startPrank(podOwner);
// cheats.expectEmit(true, true, true, true, address(delayedWithdrawalRouter));
// emit DelayedWithdrawalCreated(
// podOwner,
// podOwner,
// stakeAmount,
// delayedWithdrawalRouter.userWithdrawalsLength(podOwner)
// );

uint timestampBeforeTx = pod.mostRecentWithdrawalTimestamp();
// uint timestampBeforeTx = pod.mostRecentWithdrawalTimestamp();

pod.withdrawBeforeRestaking();
// pod.withdrawBeforeRestaking();

require(_getLatestDelayedWithdrawalAmount(podOwner) == stakeAmount, "Payment amount should be stake amount");
require(
pod.mostRecentWithdrawalTimestamp() == uint64(block.timestamp),
"Most recent withdrawal block number not updated"
);
require(
pod.mostRecentWithdrawalTimestamp() > timestampBeforeTx,
"Most recent withdrawal block number not updated"
);
}
// require(_getLatestDelayedWithdrawalAmount(podOwner) == stakeAmount, "Payment amount should be stake amount");
// require(
// pod.mostRecentWithdrawalTimestamp() == uint64(block.timestamp),
// "Most recent withdrawal block number not updated"
// );
// require(
// pod.mostRecentWithdrawalTimestamp() > timestampBeforeTx,
// "Most recent withdrawal block number not updated"
// );
// }

function testDeployEigenPodWithoutActivateRestaking() public {
// ./solidityProofGen -newBalance=32000115173 "ValidatorFieldsProof" 302913 true "data/withdrawal_proof_goerli/goerli_block_header_6399998.json" "data/withdrawal_proof_goerli/goerli_slot_6399998.json" "withdrawal_credential_proof_302913.json"
Expand Down Expand Up @@ -344,69 +344,69 @@ contract EigenPodTests is ProofParsing, EigenPodPausingConstants {
cheats.stopPrank();
}

function testWithdrawNonBeaconChainETHBalanceWei() public {
IEigenPod pod = testDeployAndVerifyNewEigenPod();
// function testWithdrawNonBeaconChainETHBalanceWei() public {
// IEigenPod pod = testDeployAndVerifyNewEigenPod();

cheats.deal(address(podOwner), 10 ether);
emit log_named_address("Pod:", address(pod));
// cheats.deal(address(podOwner), 10 ether);
// emit log_named_address("Pod:", address(pod));

uint256 balanceBeforeDeposit = pod.nonBeaconChainETHBalanceWei();
// uint256 balanceBeforeDeposit = pod.nonBeaconChainETHBalanceWei();

(bool sent, ) = payable(address(pod)).call{value: 1 ether}("");
// (bool sent, ) = payable(address(pod)).call{value: 1 ether}("");

require(sent == true, "not sent");
// require(sent == true, "not sent");

uint256 balanceAfterDeposit = pod.nonBeaconChainETHBalanceWei();
// uint256 balanceAfterDeposit = pod.nonBeaconChainETHBalanceWei();

require(
balanceBeforeDeposit < balanceAfterDeposit
&& (balanceAfterDeposit - balanceBeforeDeposit) == 1 ether,
"increment checks"
);
// require(
// balanceBeforeDeposit < balanceAfterDeposit
// && (balanceAfterDeposit - balanceBeforeDeposit) == 1 ether,
// "increment checks"
// );

cheats.startPrank(podOwner, podOwner);
cheats.expectEmit(true, true, true, true, address(pod));
emit NonBeaconChainETHWithdrawn(podOwner, 1 ether);
pod.withdrawNonBeaconChainETHBalanceWei(
podOwner,
1 ether
);
// cheats.startPrank(podOwner, podOwner);
// cheats.expectEmit(true, true, true, true, address(pod));
// emit NonBeaconChainETHWithdrawn(podOwner, 1 ether);
// pod.withdrawNonBeaconChainETHBalanceWei(
// podOwner,
// 1 ether
// );

uint256 balanceAfterWithdrawal = pod.nonBeaconChainETHBalanceWei();
// uint256 balanceAfterWithdrawal = pod.nonBeaconChainETHBalanceWei();

require(
balanceAfterWithdrawal < balanceAfterDeposit
&& balanceAfterWithdrawal == balanceBeforeDeposit,
"decrement checks"
);
// require(
// balanceAfterWithdrawal < balanceAfterDeposit
// && balanceAfterWithdrawal == balanceBeforeDeposit,
// "decrement checks"
// );

cheats.stopPrank();
}
// cheats.stopPrank();
// }

function testWithdrawFromPod() public {
IEigenPod pod = eigenPodManager.getPod(podOwner);
cheats.startPrank(podOwner);
// function testWithdrawFromPod() public {
// IEigenPod pod = eigenPodManager.getPod(podOwner);
// cheats.startPrank(podOwner);

cheats.expectEmit(true, true, true, true, address(pod));
emit EigenPodStaked(pubkey);
// cheats.expectEmit(true, true, true, true, address(pod));
// emit EigenPodStaked(pubkey);

eigenPodManager.stake{value: stakeAmount}(pubkey, signature, depositDataRoot);
cheats.stopPrank();
// eigenPodManager.stake{value: stakeAmount}(pubkey, signature, depositDataRoot);
// cheats.stopPrank();

cheats.deal(address(pod), stakeAmount);
// cheats.deal(address(pod), stakeAmount);

// this is testing if pods deployed before M2 that do not have hasRestaked initialized to true, will revert
cheats.store(address(pod), bytes32(uint256(52)), bytes32(uint256(1)));
// // this is testing if pods deployed before M2 that do not have hasRestaked initialized to true, will revert
// cheats.store(address(pod), bytes32(uint256(52)), bytes32(uint256(1)));

cheats.startPrank(podOwner);
uint256 userWithdrawalsLength = delayedWithdrawalRouter.userWithdrawalsLength(podOwner);
// cheats.expectEmit(true, true, true, true, address(delayedWithdrawalRouter));
//cheats.expectEmit(true, true, true, true);
emit DelayedWithdrawalCreated(podOwner, podOwner, stakeAmount, userWithdrawalsLength);
pod.withdrawBeforeRestaking();
cheats.stopPrank();
require(address(pod).balance == 0, "Pod balance should be 0");
}
// cheats.startPrank(podOwner);
// uint256 userWithdrawalsLength = delayedWithdrawalRouter.userWithdrawalsLength(podOwner);
// // cheats.expectEmit(true, true, true, true, address(delayedWithdrawalRouter));
// //cheats.expectEmit(true, true, true, true);
// emit DelayedWithdrawalCreated(podOwner, podOwner, stakeAmount, userWithdrawalsLength);
// pod.withdrawBeforeRestaking();
// cheats.stopPrank();
// require(address(pod).balance == 0, "Pod balance should be 0");
// }

// function testFullWithdrawalProof() public {
// setJSON("./src/test/test-data/fullWithdrawalProof_Latest.json");
Expand Down Expand Up @@ -1059,20 +1059,20 @@ contract EigenPodTests is ProofParsing, EigenPodPausingConstants {
cheats.stopPrank();
}

function testCallWithdrawBeforeRestakingFromNonOwner(address nonPodOwner) external fuzzedAddress(nonPodOwner) {
cheats.assume(nonPodOwner != podOwner);
testStaking();
IEigenPod pod = eigenPodManager.getPod(podOwner);
// function testCallWithdrawBeforeRestakingFromNonOwner(address nonPodOwner) external fuzzedAddress(nonPodOwner) {
// cheats.assume(nonPodOwner != podOwner);
// testStaking();
// IEigenPod pod = eigenPodManager.getPod(podOwner);

// this is testing if pods deployed before M2 that do not have hasRestaked initialized to true, will revert
cheats.store(address(pod), bytes32(uint256(52)), bytes32(0));
require(pod.hasRestaked() == false, "Pod should not be restaked");
// // this is testing if pods deployed before M2 that do not have hasRestaked initialized to true, will revert
// cheats.store(address(pod), bytes32(uint256(52)), bytes32(0));
// require(pod.hasRestaked() == false, "Pod should not be restaked");

//simulate a withdrawal
cheats.startPrank(nonPodOwner);
cheats.expectRevert(bytes("EigenPod.onlyEigenPodOwner: not podOwner"));
pod.withdrawBeforeRestaking();
}
// //simulate a withdrawal
// cheats.startPrank(nonPodOwner);
// cheats.expectRevert(bytes("EigenPod.onlyEigenPodOwner: not podOwner"));
// pod.withdrawBeforeRestaking();
// }

/* test deprecated since this is checked on the EigenPodManager level, rather than the EigenPod level
TODO: @Sidu28 - check whether we have adequate coverage of the correct function
Expand Down
Loading

0 comments on commit ebb1201

Please sign in to comment.