Renzo

Core

6.1 Incorrect queuedShares scaling might lead to DoS and incorrect TVL calculation

To correctly track the staker's share accounting in case of a slash, EigenLayer distinguishes between two share types: deposit and withdrawable shares. Deposit shares track the staker's original deposit in the protocol. In contrast, withdrawable shares specify what can be withdrawn from the protocol, factoring in the slashing that could have happened post-deposit. The conversion between share types is performed with the help of the deposit scaling factor (DSF) and slashing factor. Provided deposit shares D, the following equation can be used to calculate withdrawable shares W. $W = D * DSF1e18 * slashing\ factor1e18$ In EigenLayer's contracts, whenever the staker's delegation is increased, the deposit scaling factor for that staker is updated as well. Examples of such operations include: depositing tokens into a strategy, completing the queued withdrawal in the form of shares or delegating shares to an operator.

function _increaseDelegation(...) internal {
    // ...
    DepositScalingFactor storage dsf =
        _depositScalingFactor[staker][strategy];
    dsf.update(prevDepositShares, addedShares, slashingFactor);
    // ... 
}

The slashing factor is a number that starts at the value of 1e18 and can only decrease down to the lower boundary value of 0 in case of a full slash. The DSF also starts at 1e18 for a fresh deposit but can exceed 1e18 in certain scenarios. A notable example includes the first deposit / delegation to an operator that was previously slashed. Since stakers shouldn't be affected by the slashes that happened prior to their initial delegation, their DSF will be incremented to offset the current slashing factor for that operator.

function update(
    DepositScalingFactor storage dsf,
    uint256 prevDepositShares,
    uint256 addedShares,
    uint256 slashingFactor
) internal {
    // @audit If this is the staker's first deposit or they are delegating to
    // an operator, the prevDepositShares will be zero.
    if (prevDepositShares == 0) {
        // @audit Stakers shouldn't be punished for any slashing that happened
        // prior to their first deposit. The inverse scaling factor is calculated
        // to offset prior slashing.
        dsf._scalingFactor = dsf.scalingFactor().divWad(slashingFactor);
        return;
    }
    // ...
}

For example if the current slashing factor for an operator is 0.9, the staker that deposited 100 shares would be able to withdraw only 90 shares right after the deposit. To mitigate the effect of previous slashing, the inversed DSF is calculated for the staker: 1 * (1 / 0.9) = 1.11. Using the formula, withdrawable shares would be computed as: 100 * 1.11 * 0.9 = 100, which matches the initial deposit of 100 shares.

The problem in Renzo's OperatorDelegator contract is that in the withdrawal flow, the deposit share amounts are used interchangeably with deposit amounts scaled by the DSF, which is incorrect. Whenever an admin queues the withdrawal on the OperatorDelegator contract, the queueWithdrawal(...) function from the OperatorDelegatorAdminLib is executed to prepare the withdrawal params with deposit share amounts to withdraw. These deposit amounts are added to the queuedShares mapping for each token that is being withdrawn. The queuedShares mapping is used in the TVL calculation to track the 'in-flight' funds.

function queueWithdrawal(
    IERC20[] calldata tokens,
    uint256[] calldata tokenAmounts,
    // ...
) external returns (/* ... */) {
    // ... 
    for (uint256 i; i < tokens.length;) {
        if (address(tokens[i]) == IS_NATIVE) {
            // ... 
            // @audit For BeaconChainETH strategy token amount is 1:1 with deposit shares.
            queuedWithdrawalParams[0].depositShares[i] = tokenAmounts[i];
        } else {
            // ...
            // @audit For ERC20 strategies get the exchange ratio from the strategy.
            queuedWithdrawalParams[0].depositShares[i] =
            tokenStrategyMapping[tokens[i]].underlyingToSharesView(
                tokenAmounts[i]
            );
        }
        // ...
        queuedShares[address(tokens[i])] +=
            queuedWithdrawalParams[0].depositShares[i];
    // ...
    } 
    // ... 
    // @audit Create the withdrawal struct, queue it, and return its hash.
    bytes32 withdrawalRoot =
        delegationManager.queueWithdrawals(queuedWithdrawalParams)[0];
    // ...
}

The queueWithdrawals(...) function from the DelegationManager contract will forward the queuedWithdrawalParams to the _removeSharesAndQueueWithdrawal(...) function to construct the Withdrawal struct, queue it and return its hash as the withdrawalRoot. The thing to note is that the _removeSharesAndQueueWithdrawal(...) function will first scale the provided deposit share amounts by multiplying them with the staker's current DSF and then storing them in the Withdrawal struct.

function _removeSharesAndQueueWithdrawal(
    address staker,
    address operator,
    IStrategy[] memory strategies,
    uint256[] memory depositSharesToWithdraw, // @audit The provided deposit share amount.
    // ...
) internal returns (bytes32) {
    // ...
    for (uint256 i = 0; i < strategies.length; ++i) {
        // ...
        DepositScalingFactor memory dsf =
            _depositScalingFactor[staker][strategies[i]];
        // ...
        // @audit The shares queued for withdrawal are scaled by DSF. This value can be bigger than the provided deposit shares amount.
        scaledShares[i] =
            dsf.scaleForQueueWithdrawal(depositSharesToWithdraw[i]);
        // ...
    } // ...
    Withdrawal memory withdrawal = Withdrawal({
        staker: staker,
        delegatedTo: operator,
        withdrawer: staker,
        nonce: nonce,
        startBlock: uint32(block.number),
        strategies: strategies,
        // @audit-issue Withdrawal struct holds SCALED deposit shares,
        // while queuedShares mapping holds regular deposit shares.
        scaledShares: scaledShares
    });
    bytes32 withdrawalRoot = calculateWithdrawalRoot(withdrawal);
    // ...
    return withdrawalRoot;
}

The fact that the Withdrawal struct holds scaled deposit shares becomes problematic once the completeQueuedWithdrawal(...) function is called to finalize the withdrawal. The function will first call the DelegationManager to complete the withdrawal and withdraw the shares as tokens. Since the shares are no longer queued, the _reduceQueuedShares(...) will be called to decrement the queuedShares mapping initially incremented in the queueWithdrawal(...) function.

function completeQueuedWithdrawal(
    IDelegationManager.Withdrawal calldata withdrawal,
    IERC20[] calldata tokens
) external nonReentrant onlyNativeEthRestakeAdmin {
    // ... 
    // @audit Withdraw shares as tokens.
    try delegationManager.completeQueuedWithdrawal(
        withdrawal, tokens, true
    ) {} catch {
        delegationManager.completeQueuedWithdrawal(
            withdrawal, tokens, 0, true
        );
    }
    // @audit Since the shares are no longer queued, decrease
    // the queuedShares mapping.
    _reduceQueuedShares(withdrawal, tokens);
    // ...
}

If there was no slash, the queuedShares mapping should be decremented by the amount it was incremented with. This, however, might not be the case since scaled shares can be larger than the deposit amount if the staker had the DSF greater than 1e18 at the time of queuing the withdrawal.

function _reduceQueuedShares(
    IDelegationManager.Withdrawal calldata withdrawal,
    IERC20[] memory tokens
) internal {
    for (uint256 i; i < tokens.length;) {
        // ... 
        // @audit-issue If 100 deposit shares were queued for withdrawal, the queuedShares
        // mapping holds the value of 100. The scaled shares, however can be >100 if the staker's
        // DSF is greater than 1e18. The following line will either revert leading to DoS
        // or will lead to incorrect TVL calculation if too many shares are subtracted.
        queuedShares[address(tokens[i])] -= withdrawal.scaledShares[i];
        // ... 
    }
}

If there are no more withdrawals queued, the subtraction of too many shares will cause the _reduceQueuedShares(...) function to revert, leading to a DoS of the withdrawal completion flow until the queuedShares mapping is filled again with new withdrawals to make the subtraction succeed.

Even if there are enough shares in the queuedShares mapping, subtracting the scaled shares will influence the TVL calculation, making the TVL lower than it should be.

The problem is also present in the emergencyTrackQueuedWithdrawal(...) function.

Recommendation

The main purpose of the queuedShares mapping is to track the 'in-flight' tokens in the TVL calculation. Once the withdrawal is completed via the completeQueuedWithdrawal(...) function, the value in the mapping should be decremented accordingly. Consider subtracting the unscaled deposit amount of shares from the queuedShares.

Resolution

Updated the code to properly reduce initialWithdrawableShares (tracked in queuedWithdrawalTokenInfo mapping) from the queuedShares while completing the withdrawal through completeQueuedWithdrawals. Code Ref.

6.2 The slashed Beacon Chain ETH strategy shares are accounted for twice in TVL calculations

Whenever an AVS slashes an operator, the DelegationManager contract will call the increaseBurnableShares(...) function on an appropriate share manager contract. The StrategyManager contract will be called for ERC20 strategies and EigenPodManager contract for Beacon Chain ETH strategy. Both contracts track the amount of shares to burn. The StrategyManager contract allows anyone to call the burnShares(...) function to transfer the slashed ERC20 tokens to the burn address. This however, is not the case for the EigenPodManager contract. The slashed native tokens stay in the EigenPod contract. The burnableETHShares state variable is not utilized by EigenLayer contracts in their current version. It might change post Pectra when beacon chain withdrawals are allowed to be triggered from the execution layer.

function increaseBurnableShares(
    IStrategy,
    uint256 addedSharesToBurn
) external onlyDelegationManager nonReentrant {
    // @audit The burnableETHShares variable is not utilized by the 
    // current version of EigenLayer contracts, except for the 'write'
    // and event emission below.
    burnableETHShares += addedSharesToBurn;
    emit BurnableETHSharesIncreased(addedSharesToBurn);
}

The fact that native tokens are not transferred out of the contract when burned, makes it so that the tokens slashed by an AVS, are included in the podBalanceGwei when starting the checkpoint.

function _startCheckpoint(...) internal {
    // ...
    // @audit The slashed native tokens are not "burned", they are locked
    // in the EigenPod contract. The address(this).balance includes these funds.
    uint64 podBalanceGwei = uint64(address(this).balance / GWEI_TO_WEI)
        - restakedExecutionLayerGwei;
    // ...
    Checkpoint memory checkpoint = Checkpoint({
        // ...
        podBalanceGwei: podBalanceGwei,
        // ...
    });
    // ...
}

Subsequently the slashed native tokens are included in the restakedExecutionLayerGwei when the checkpoint is finalized.

// @audit This function is called when finalizing the checkpoint.
function _updateCheckpoint(
    Checkpoint memory checkpoint
) internal {
    // ...
    // @audit Increment restakedExecutionLayerGwei by pod balance gwei that was
    // set in the _startCheckpoint(...) function.
    restakedExecutionLayerGwei += checkpoint.podBalanceGwei;
    // ...
}

In Renzo's TVL calculation the restakedExecutionLayerGwei is used to exclude any unclaimed rewards sitting in the EigenPod contract from the TVL calculation until they are claimed and processed by the OperatorDelegator contract. The value is queried by calling the withdrawableRestakedExecutionLayerGwei(...) function from the EigenPod contract.

function withdrawableRestakedExecutionLayerGwei() external view returns (uint64)
{
    // @audit Renzo's TVL calculation utilizes this function. It does not account
    // for the `burnableETHShares` locked in the EigenPod.
    return restakedExecutionLayerGwei;
}

The problem is that the _getPartialWithdrawalsPodDelta(...) function that calls the withdrawableRestakedExecutionLayerGwei(...) does not accommodate for the fact that the value returned from the call includes the AVS slashed tokens that are locked in the EigenPod contract.

function _getPartialWithdrawalsPodDelta() internal view
    returns (uint256 podDelta)
{
    podDelta = (
        // @audit Since the restaked execution layer gwei can be inflated by locked
        // native Ether, the condition will more likely evaluate to TRUE.
        uint256(eigenPod.withdrawableRestakedExecutionLayerGwei())
            * GWEI_TO_WEI > totalBeaconChainExitBalance
    )
        // @audit If condition is TRUE, the returned pod delta is non-zero.
        ? uint256(eigenPod.withdrawableRestakedExecutionLayerGwei())
            * GWEI_TO_WEI - totalBeaconChainExitBalance
        : 0;
}

As a result the non-zero pod delta will be returned even in situations when there are no rewards and no full withdrawal proceeds in the EigenPod contract. The pod delta is subtracted from the withdrawable shares of a particular operator. It is incorrect since withdrawable shares were already adjusted by any slashing imposed by an AVS.

// @audit This function is called for every operator delegator during TVL calculation.
function getStakedETHBalance() external view returns (uint256) {
    // ...
    // @audit The withdrawable shares are already adjusted by slashing.
    try delegationManager.getWithdrawableShares(address(this), strategies)
    returns (uint256[] memory withdrawableShares, uint256[] memory) {
        uint256 collateralBalance = withdrawableShares[0];
        // ...
        // @audit-issue The shares are lowered by the pod delta which includes slashing.
        // Slashing is included twice now. TVL will be lower OR subtraction will underflow
        // and revert the TVL calculation.
        collateralBalance -= _getPartialWithdrawalsPodDelta();
        return collateralBalance;
    } catch {
        // ...
    }
}

Over time the TVL discrepancy will only increase if the validator incurs slashes on the Beacon Chain ETH strategy. If the withdrawable shares are sufficient to cover the inflated pod delta, the TVL will be lower than it should be. However, if there are not enough withdrawable shares, the TVL calculation will revert due to underflow in the getStakedETHBalance(...) function causing a DoS in core flows such as deposits and withdrawals.

Recommendation

Since the EigenPod contract cannot directly differentiate between rewards and slashed tokens, and AVS slashing is rare, consider adding an admin-controlled mechanism to adjust the TVL if a slash occurs in the Beacon Chain ETH strategy. The adjustment value can be derived from the OperatorSharesSlashed events emitted by the slashOperatorShares(...) function from the DelegationManager contract whenever an Operator utilized by Renzo is slashed.

This solution could serve as a temporary measure until EigenLayer introduces native token burns.

Resolution

Added an admin function emergencyTrackAVSEthSlashedAmount to track beaconChainEthAvsSlashingAmount in OperatorDelegator along with updated calculation in _getPartialWithdrawalsPodDelta. Code Ref.

6.3 The emergencyTrackSlashedQueuedWithdrawalDelta(...) function will revert for beaconChainETHStrategy due to missing underlyingToken(...) function

With the upcoming update to EigenLayer, shares in the withdrawal queue remain slashable for MIN_WITHDRAWAL_DELAY_BLOCKS (approximately 17.5 days in blocks). During this delay period, any slashing will decrease the amount of withdrawable shares in EigenLayer. Slashing of the shares in the withdrawal queue takes effect immediately in EigenLayer and is reflected in the withdrawable share balance.

On Renzo's side, the emergencyTrackSlashedQueuedWithdrawalDelta(...) is used to track the potential slashes to the shares that are currently in the EigenLayer withdrawal queue.

// @audit Permisionless function to track queued shares that were slashed.
function emergencyTrackSlashedQueuedWithdrawalDelta(
    bytes32[] calldata withdrawalRoots
) external {
    OperatorDelegatorAdminLib.trackSlashedQueuedWithdrawalDelta(
        withdrawalRoots,
        queuedWithdrawal,
        queuedWithdrawalTokensSlashedDelta,
        totalTokenQueuedSharesSlashedDelta,
        delegationManager
    );
}

This function is permissionless and can be called by anyone to calculate the slashing delta between the deposit and withdrawable shares. Any difference between these share types is considered a slashing delta and will be deducted from the TVL.

To calculate the slashing delta, the trackSlashedQueuedWithdrawalDelta function in the OperatorDelegatorAdminLib library will iterate through the withdrawal roots, retrieving the queued withdrawal struct from EigenLayer:

function trackSlashedQueuedWithdrawalDelta(
    bytes32[] calldata withdrawalRoots,
    // ...
) external {
    for (uint256 i = 0; i < withdrawalRoots.length;) {
        // ...
        for (uint256 j = 0; j < withdrawal.scaledShares.length;) {
            // @audit-issue The BeaconChainETH strategy does not implement the 
            // underlyingToken(...) function. The call will revert.
            address underlyingToken =
                address(withdrawal.strategies[j].underlyingToken());
           // ... 
        }
        // ...
    }
}

The issue is that this function assumes each EigenLayer strategy implements the underlyingToken() function. However, it doesn't account for the fact that beaconChainETHStrategy doesn't implement this function. In EigenLayer, the beaconChainETHStrategy is a special virtual strategy that points to an empty address ( 0xbeaC0eeEeeeeEEeEeEEEEeeEEeEeeeEeeEEBEaC0) as defined in the EigenPodManagerStorage contract.

When the emergencyTrackSlashedQueuedWithdrawalDelta function is called for a withdrawal that includes the beaconChainETHStrategy, the transaction will revert when trying to call underlyingToken(...) on the virtual strategy address, making it impossible to track slashing for withdrawals that include beaconChainETHStrategy.

Recommendation

Consider modifying the logic inside the trackSlashedQueuedWithdrawalDelta function to handle the special case of beaconChainETHStrategy.

Resolution

Implemented _getUnderlyingFromStrategy . Fixed in Code Ref.

6.4 Checkpoint system can be DoSed when currentCheckpointTimestamp loses synchronization

EigenLayer's checkpoint system allows EigenPod owners to prove the current balances of their validators on the Beacon Chain and receive corresponding EigenLayer shares or tokens from withdrawal and reward proceeds. The process starts with a call to startCheckpoint(...) and ends when all checkpoint proofs are verified by calling verifyCheckpointProofs(...). EigenPod also includes an emergency checkpointing mechanism via verifyStaleBalance(...) that allows anyone to start a checkpoint if a validator has been slashed on the Beacon Chain.

Renzo protocol manages the checkpointing process for EigenLayer through two primary functions on the OperatorDelegator contract: startCheckpoint(...) and verifyCheckpointProofs(...). This process is crucial for tracking validator balances, claiming rewards, and handling validator exits. The protocol's admin calls startCheckpoint(...) to initiate a checkpoint, which sets the currentCheckpointTimestamp to track the active checkpoint:

function startCheckpoint(...) external onlyNativeEthRestakeAdmin {
    if (currentCheckpointTimestamp != 0) revert CheckpointAlreadyActive();
    // ... 
    eigenPod.startCheckpoint(true);
    // @audit The current checkpoint time is current block.timestamp
    currentCheckpointTimestamp = eigenPod.currentCheckpointTimestamp();
    // ...
}

Once the checkpoint proofs are verified, verifyCheckpointProofs(...) function updates the lastCheckpointTimestamp and clears the currentCheckpointTimestamp:

function verifyCheckpointProofs(...) external onlyNativeEthRestakeAdmin {
    // ...
    if (eigenPod.lastCheckpointTimestamp() == currentCheckpointTimestamp) {
        // ...
    // @audit Set the current as last finalized.
        lastCheckpointTimestamp = currentCheckpointTimestamp;
    // @audit Clear the current so that checkpoint can be started again.
        delete currentCheckpointTimestamp;
    }
}

The code above checks if the checkpoint has been completed by comparing the eigenPod.lastCheckpointTimestamp() with the currentCheckpointTimestamp. Since in the EigenPod contract the lastCheckpointTimestamp is updated only after checkpoint proofs are verified, this check works correctly. The checkpoint state stays synchronized between the protocol and the EigenPod.

However, in a specific scenario the eigenPod.lastCheckpointTimestamp() can go out of sync with the currentCheckpointTimestamp. In such a scenario, verifying the checkpoint via Renzo contracts won't be possible. Consider the following sequence of events:

1. The protocol admin starts a checkpoint via startCheckpoint(), setting currentCheckpointTimestamp to the current block timestamp. For the sake of example, lets assume the current block timestamp is 1. After a startCheckpoint(...) function is called in Renzo, this will be the protocol state:
   

   currentCheckpointTimestamp = 1 lastCheckpointTimestamp = 0 eigenPod.currentCheckpointTimestamp() = 1 eigenPod.lastCheckpointTimestamp() = 0

2. Instead of using the protocol's verifyCheckpointProofs(...) function, a user directly calls verifyCheckpointProofs(...) on the EigenPod contract, completing the checkpoint. This updates the EigenPod's state but not the protocol's state:
   

   currentCheckpointTimestamp = 1 lastCheckpointTimestamp = 0 eigenPod.currentCheckpointTimestamp() = 0 eigenPod.lastCheckpointTimestamp() = 1

3. A validator gets slashed on the Beacon Chain;
4. Anyone can then call verifyStaleBalance(...) directly on the EigenPod, which automatically starts a new checkpoint in the EigenPod. Lets assume the new timestamp is 3:
   

   currentCheckpointTimestamp = 1 lastCheckpointTimestamp = 0 eigenPod.currentCheckpointTimestamp() = 3 eigenPod.lastCheckpointTimestamp() = 1

5. The user completes this new checkpoint by calling verifyCheckpointProofs(...) directly on the EigenPod:
   

   currentCheckpointTimestamp = 1 lastCheckpointTimestamp = 0 eigenPod.currentCheckpointTimestamp() = 0 eigenPod.lastCheckpointTimestamp() = 3

Now, the protocol and EigenPod states are completely out of sync. When the protocol admin calls verifyCheckpointProofs(...), the condition eigenPod.lastCheckpointTimestamp() == currentCheckpointTimestamp will be false ( 3 != 1), so the protocol's currentCheckpointTimestamp remains stuck at 1.

Since currentCheckpointTimestamp is non-zero, the admin cannot start a new checkpoint due to the check in startCheckpoint(...):

function startCheckpoint() external onlyNativeEthRestakeAdmin {
    if (currentCheckpointTimestamp != 0) revert CheckpointAlreadyActive();
    // ... 
}

The protocol does have an emergency function emergencyTrackMissedCheckpoint(...) that can update the lastCheckpointTimestamp, but it doesn't reset the currentCheckpointTimestamp.

This creates a permanent denial-of-service condition where the protocol cannot start new checkpoints or verify existing ones, preventing it from:

• Minting new shares for accrued validator rewards;
• Withdrawing ETH from validators that have exited the Beacon Chain;

If the checkpoints are tracked off-chain in an automatic manner by Renzo's backend, the described scenario is less likely to happen during regular protocol operations. If the attacker, however, turned out to be one of the Validator Operators utilized by Renzo, the timing of calls to verifyCheckpointProofs(...) and verifyStaleBalance(...) is less strict since the Operator can commit the slashable offence at any point in time and call verifyStaleBalance(...) right after.

Note that if the checkpoint is started on the Renzo side, it is marked as recorded. If, after that, the checkpoint is verified outside of Renzo and an additional checkpoint is missed, the totalBeaconChainExitBalance will not be updated correctly. If Renzo's admin attempts to call the emergencyTrackMissedCheckpoint(...) function with both missed checkpoints, the call will revert since the first one is marked as recorded. The totalBeaconChainExitBalance can only be updated for the second one.

Recommendation

Consider adding an additional emergency setter function for the currentCheckpointTimestamp to ensure that the protocol is still operational in a rare scenario where more than one checkpoint is missed.

Resolution

For now considering the rarity of the issue. The offchain watcher will prioritize to sync the checkpoint through OperatorDelegator::verifyCheckpointProofs as step 4 and 5 will not be executed atomically which provides a time window during which admin can prioritize and sync the checkpoints on OperatorDelegator.

6.5 The emergencyTrackSlashedQueuedWithdrawalDelta(...) function can be used to artificially decrease the TVL to get better exchange rate on ezETH

The emergencyTrackSlashedQueuedWithdrawalDelta(...) function from the OperatorDelegator contract can be called by anyone to signal that the shares queued for withdrawal have been slashed while waiting in the queue. The function utilizes the OperatorDelegatorAdminLib, where the trackSlashedQueuedWithdrawalDelta(...) function implements all the functionality.

The function first queries the currently withdrawable shares for a particular withdrawal root (the root must be present in the withdrawal queue). It then compares the currently withdrawable value with the number of shares queued for deposit when the queueWithdrawal(...) function was called. If there is a difference between the two, it means that there was a slash, and the slashing delta must be subtracted from the TVL calculation.

The problem with the code handling this logic is that the value of shares queued for deposit and stored in the Withdrawal struct is scaled by the deposit scaling factor. In a situation where the staker delegated to an operator that was previously slashed, the scaled shares will be higher than the deposit amount provided by the staker to offset the prior slash. In such scenario even when there was no slash in between the deposit and withdrawal, anyone can call the emergencyTrackSlashedQueuedWithdrawalDelta(...) function to signal a slash since the slashingDelta will be positive.

function trackSlashedQueuedWithdrawalDelta(
    bytes32[] calldata withdrawalRoots,
    // ...
) external {
    for (uint256 i = 0; i < withdrawalRoots.length;) {
        // ... 
        (
            IDelegationManager.Withdrawal memory withdrawal,
            // @audit currentShares are WITHDRAWABLE shares
            uint256[] memory currentShares
        ) = delegationManager.getQueuedWithdrawal(
            withdrawalRoots[i]
        );

        for (uint256 j = 0; j < withdrawal.scaledShares.length;) {
            // ...
            // @audit Since scaled shares are not affected by the slashing factor,
            // They will be higher than currently withdrawable shares even if there was 
            // no new slash.
            uint256 slashingDelta = withdrawal.scaledShares[j]
                > currentShares[j]
                ? withdrawal.scaledShares[j] - currentShares[j]
                : 0;
            // @audit Adjust the slashed amount. This will lower the TVL calculation.
            totalTokenQueuedSharesSlashedDelta[underlyingToken] +=
                slashingDelta;
            // ... 
        }
        // ...
    }
}

The totalTokenQueuedSharesSlashedDelta mapping is updated with the latest slashing delta. Since this value is used to decrease the value of 'in-flight' shares during the TVL calculation, the TVL will be lower than it should be. It allows the user to call this function before depositing to get a more favorable exchange rate when minting ezETH tokens.

Recommendation

Since the slashing delta informs about the difference between withdrawable shares at two points in time, consider adjusting the scaled shares amount by the slashing factor at the time of the deposit to make the computation correct.

Resolution

Updated the code to calculate slashing delta in the denomination of withdrawableShares instead of scaledShares. Code Ref.

6.6 Users might get a worse exchange rate on ezETH due to the TVL spike after AVS are the withdrawal

The queueWithdrawals(...) function in the OperatorDelegator contract allows the protocol admin to start a withdrawal from the specified token strategies in EigenLayer. In the function arguments, the Renzo admin will provide the strategies to withdraw from and the amount of tokens to be withdrawn from EigenLayer.

The EigenLayer will decrease the OperatorDelegator's withdrawable shares as soon as the withdrawal is queued. Since Renzo's TVL calculation before queuing the withdrawal was based on the amount of withdrawable shares, this difference must be accounted for.

Renzo tracks the shares in EigenLayer's withdrawal queue to include the in-flight tokens in the TVL calculation. This is done via the queuedShares storage variable incremented in the queueWithdrawals(...) function in the OperatorDelegator contract by the depositShares amount. Thanks to this mechanism, whenever the TVL is decreased by the amount of the withdrawable shares when the withdrawal is queued, it is incremented right after by the deposit share amount expected to be received. This ensures that the TVL remains constant.

function queueWithdrawal(
    IERC20[] calldata tokens,
    uint256[] calldata tokenAmounts, 
    // ...
) external returns (/*...*/) {
    // ... 
    for (uint256 i; i < tokens.length;) {
        if (address(tokens[i]) == IS_NATIVE) {
            // ...
            queuedWithdrawalParams[0].depositShares[i] = tokenAmounts[i];
        } else {
            // ...
            queuedWithdrawalParams[0].depositShares[i] =
            tokenStrategyMapping[tokens[i]].underlyingToSharesView(
                tokenAmounts[i]
            );
        }
        // ...
        // @audit-issue In case of a 50% slash on 100 deposit shares, only 50 
        // shares are withdrawable and accounted in the TVL before the withdrawal. 
        // The queuedShares mapping gets incremented by 100, which inflates the TVL by 50 shares.
        queuedShares[address(tokens[i])] +=
            queuedWithdrawalParams[0].depositShares[i];
        // ...
    } 
    // ...
}

Tracking the in-flight tokens via the queuedShares mapping works whenever the deposit shares are equivalent to the withdrawable shares. This, however, might not be the case if the operator was previously slashed. Consider the following scenario:

1. Renzo's TVL is 100 at the beginning (100 deposit shares delegated to a single operator);
2. The operator gets slashed 50% by an AVS (or Beacon Chain; it does not matter for the example);
3. Renzo's TVL drops to 50 since the getWithdrawableShares(...) from EigenLayer represents the updated slashed amount. The deposit shares are still 100 since they represent the nominal deposit value;
4. Since EigenLayer calculates the withdrawable shares on-demand with the latest slashing factor, to make the full withdrawal from EigenLayer, Renzo's admin must provide 100 deposit shares as an input amount. This amount will be adjusted and Renzo will be credited with 50 shares once withdrawal is completed;
5. Once the admin queues the full withdrawal specifying 100 deposit shares as input amount, all available shares in EigenLayer (50 shares) are decreased to 0;
6. Renzo increases the queuedShares mapping by 100 deposit shares to account for the in-flight tokens. The TVL rises to 100;
7. After the withdrawal period, once the completeQueuedWithdrawal(...) function is called and withdrawal is complete, the TVL drops to 50 since only 50 shares were withdrawn from EigenLayer;

Since the TVL is inflated right after queueing the withdrawal, any user that deposits funds into the protocol at that time will get a worse exchange rate on ezETH tokens. The problem is also present in the emergencyTrackQueuedWithdrawal(...) function.

Recommendation

Starting from the slashing update, EigenLayer distinguishes between the deposit and withdrawable shares. The same distinction should be implemented in Renzo's smart contracts. Consider separating the deposit amount of shares used as input to the function from the amount of withdrawable shares utilized during the TVL calculation. The fix for this issue could pair with the fix for [High] Incorrect queuedShares scaling might lead to DoS and incorrect TVL calculation issue.

Resolution

Updated the code to increment queuedShares with the withdrawableShares amount instead of depositShares. While queueWithdrawal with depositShares. Code Ref.

6.7 The emergencyTrackQueuedWithdrawals(...) function doesn't verify the queued withdrawal ownership, potentially leading to inflated TVL

The emergencyTrackQueuedWithdrawals(...) function is a permissioned function and can only be called by the EmergencyWithdrawTrackingAdmin. This function is designed to track pending queued withdrawal shares if an Operator forcefully undelegates the OperatorDelegator.

The function utilizes the trackQueuedWithdrawals(...) function from the OperatorDelegatorAdminLib, which updates the queuedShares mapping used in TVL calculations:

// @audit OperatorDelegatorAdminLib.sol
function trackQueuedWithdrawals(
    IDelegationManager.Withdrawal[] calldata withdrawals,
    // ...
) external {
    // ...
    for (uint256 i = 0; i < withdrawals.length; ) {
        // ...
        for (uint256 j = 0; j < withdrawals[i].scaledShares.length; ) {
            // @audit-issue Increases queuedShares mapping without 
        // verifying if withdrawal.staker is this contract.
            queuedShares[address(tokens[j])] += withdrawals[i].scaledShares[j];
            // ...
        }
        // ...
    }
}

However, the function lacks validation to ensure that the tracked withdrawal belongs to the OperatorDelegator contract. While the function verifies that the withdrawal is pending in EigenLayer, it doesn't check the staker field of each withdrawal to confirm the ownership.

Suppose an admin makes an error and calls this function with a queued withdrawal belonging to another EigenLayer user. In that case, it'll incorrectly increase the queuedShares mapping and consequently inflate the reported TVL despite the fact that this withdrawal does not belong to the OperatorDelegator contract.

Recommendation

Consider implementing a check that verifies each withdrawal's staker field matches the address of the OperatorDelegator contract before adding the corresponding shares to the queuedShares mapping.

Resolution

Added a check to verify the staker should be OperatorDelegator at - Code Ref.

6.8 The fillERC20withdrawBuffer(...) function doesn't refund excess tokens sent

The fillERC20withdrawBuffer(...) function from the DepositQueue contract is used to refill the ERC20 withdrawal buffer. It is utilized when new deposits enter the protocol via RestakeManager's deposit(...) function or during the completion of queued withdrawals in the OperatorDelegator contract.

In each case, if the token amount about to be transferred to the buffer is greater than the buffer's token deficit, the amount will be reduced before the transfer so that no extra tokens are sent to refill the buffer. However, the fillERC20withdrawBuffer(...) function in the DepositQueue contract is permissionless and can be called directly. If the admin (or a user) attempts to fill the buffer outside of the usual flow and sends an amount that is greater than the withdrawal queue deficit, the extra tokens will remain in the WithdrawQueue contract. They will be treated as a donation to be used for user withdrawals.

function fillERC20WithdrawBuffer(
    address _asset,
    uint256 _amount
) external nonReentrant onlyDepositQueue {
    // ...
    // @audit The provided _amount can be greater than queueFilled.
    queueFilled = _checkAndFillWithdrawQueue(_asset, _amount);
    // @audit The full _amount is sent to the contract regardless of queueFilled.
    IERC20(_asset).safeTransferFrom(msg.sender, address(this), _amount);

    emit ERC20BufferFilled(_asset, _amount - queueFilled);
}

Recommendation

If overfilling the buffer is a desired scenario, consider adding a similar warning comment for the users to the one present in the NatSpec of the forwardFullWithdrawalETH(...) function, stating that this function should not be used to send tokens to the protocol.

Otherwise, consider changing the logic so that only the necessary queueFilled amount is transferred to the WithdrawalQueue contract. The rest of the funds could stay in the DepositQueue and generate a higher yield for the users.

Resolution

Overfilling the buffer is a desired scenario. Added the warning at - Code Ref.

6.9 The totalBeaconChainExitBalance can be inflated if no checkpoint was finalized after the validator got slashed

Under normal circumstances, the totalBeaconChainExitBalance variable in the OperatorDelegator contract is designed to track ETH that has exited the Beacon Chain and is expected to be received by the contract.

Whenever a checkpoint is finalized on the EigenLayer side, the _verifyCheckpointProof(...) function will try to determine whether the effective balance of a particular validator has dropped to zero on the Beacon Chain. If it has, then it means that the validator is fully withdrawn. The function compares the previously recorded validator's balance with the current one (zero balance) and marks the difference as the validator's exit balance. Renzo uses this variable to determine how many native tokens received by the protocol contracts come from Beacon Chain withdrawals and how many from rewards. The funds are routed differently based on the origin.

function _verifyCheckpointProof(...) {
    // ...
    // @audit Previously recorded validator's beacon chain balance.
    prevBalanceGwei = validatorInfo.restakedBalanceGwei;
    
    // @audit New effective balance. It can be 0 if the validator is fully withdrawn.
    uint64 newBalanceGwei = BeaconChainProofs.verifyValidatorBalance({
        balanceContainerRoot: balanceContainerRoot,
        validatorIndex: validatorIndex,
        proof: proof
    });
    
    if (newBalanceGwei != prevBalanceGwei) {
        // @audit The difference between the two means that:
        // - there could be rewards (positive delta)
        // - there could be a slash (negative delta)
        // - there could be a full withdrawal (new balance is 0)
        balanceDeltaGwei = int64(newBalanceGwei) - int64(prevBalanceGwei);
        // ...
    }
    // ...
    if (newBalanceGwei == 0) {
        activeValidatorCount--;
        validatorInfo.status = VALIDATOR_STATUS.WITHDRAWN;
        // @audit The previously recorded balance is used as exit balance
        // for a fully withdrawn validator.
        exitedBalanceGwei = uint64(-balanceDeltaGwei);
        // ...
    }
    // ...
}

While Renzo's approach of depending on the exitedBalanceGwei to track the withdrawals is correct, it is under the assumption that the checkpoints are performed often enough. Since the exit balance is based on the previously stored balance, the resulting exit balance will be incorrect if that value is not up to date. Consider the following scenario of a validator that was slashed:

• The slashed validator is automatically initiated for exit in 36 days;
• The validator can suffer an initial slashing penalty of up to 1 ETH;
• If the checkpoint is started and finalized right after the initial penalty, the validator's effective balance can be recorded as 31 ETH (initial stake of 32 minus 1 ETH penalty);
• Halfway through the withdrawal period (after 18 days), the validator may incur a correlation penalty (e.g., an additional 50% of its effective balance, reducing it to 15.5 ETH) due to coordinated misbehavior.
• If no checkpoint is performed, the stored balance remains 31 ETH. Upon exit, the exitedBalanceGwei is set to 31 ETH despite the balance being 15.5 ETH;
• This inflates the totalBeaconChainExitBalance and misclassifies slashed funds as exit proceeds. Future rewards will decrease the inflated exit balance, reducing protocol fees (which are only earned on rewards);

Additionally, inactivity leaks during this period could further reduce the effective balance, exacerbating the discrepancy if checkpoints are skipped.

Recommendation

Consider monitoring all validators initiated for exit. This includes ejected validators due to inactivity leaks, voluntary exits, and slashed validators. If their effective balance changes before they are marked as withdrawn, consider starting and finalizing the checkpoint to ensure correct rewards accounting.

Resolution

The protocol constantly keeps on running and proving checkpoint periodically. On top of that, watchers will be implemented to monitor for any slashing events and report it to the protocol to price in any slashing event. Also, EigenLayer has permissionless functionality for anyone to report a validator slashing through verifyStaleBalance. Through which anyone can report the validator slashing and start a checkpoint on OperatorDelegator Owned EigenPod which can be finalized in a permissionless manner.