Skip to content

Commit 4a4082c

Browse files
authored
chore: MOOCOW audit fixes (#1496)
**Motivation:** Addresses Certora review: * I-01: removed unused event * I-02: corrected typo * I-03: deduplicated fee calc * I-04: removed unneeded checks * I-05: corrected interface docs **Modifications:** * Clarified code comments and deduplicated some checks * Add draft audit report
1 parent b6cfb82 commit 4a4082c

File tree

3 files changed

+12
-19
lines changed

3 files changed

+12
-19
lines changed
Binary file not shown.

src/contracts/interfaces/IEigenPod.sol

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,6 @@ interface IEigenPodErrors {
6363

6464
/// @dev Thrown when a predeploy request is initiated with insufficient msg.value
6565
error InsufficientFunds();
66-
/// @dev Thrown when refunding excess fees from a predeploy fails
67-
error RefundFailed();
6866
/// @dev Thrown when calling the predeploy fails
6967
error PredeployFailed();
7068
/// @dev Thrown when querying a predeploy for its current fee fails
@@ -167,7 +165,7 @@ interface IEigenPodEvents is IEigenPodTypes {
167165
/// @notice Emitted when a validator is proven for a given checkpoint
168166
event ValidatorCheckpointed(uint64 indexed checkpointTimestamp, bytes32 indexed pubkeyHash);
169167

170-
/// @notice Emitted when a validaor is proven to have 0 balance at a given checkpoint
168+
/// @notice Emitted when a validator is proven to have 0 balance at a given checkpoint
171169
event ValidatorWithdrawn(uint64 indexed checkpointTimestamp, bytes32 indexed pubkeyHash);
172170

173171
/// @notice Emitted when a consolidation request is initiated where source == target
@@ -300,8 +298,8 @@ interface IEigenPod is IEigenPodErrors, IEigenPodEvents, ISemVerMixin {
300298
/// consolidate their validators on the beacon chain.
301299
/// @param requests An array of requests consisting of the source and target pubkeys
302300
/// of the validators to be consolidated
303-
/// @dev Both the source and target validator MUST have active withdrawal credentials
304-
/// pointed at the pod
301+
/// @dev The target validator MUST have ACTIVE (proven) withdrawal credentials pointed at
302+
/// the pod. This prevents cross-pod consolidations.
305303
/// @dev The consolidation request predeploy requires a fee is sent with each request;
306304
/// this is pulled from msg.value. After submitting all requests, any remaining fee is
307305
/// refunded to the caller by calling its fallback function.
@@ -332,7 +330,8 @@ interface IEigenPod is IEigenPodErrors, IEigenPodEvents, ISemVerMixin {
332330
/// Some requirements that are NOT checked by the pod:
333331
/// - If request.srcPubkey == request.targetPubkey, the validator MUST have 0x01 credentials
334332
/// - If request.srcPubkey != request.targetPubkey, the target validator MUST have 0x02 credentials
335-
/// - Both the source and target validators MUST be active and MUST NOT have initiated exits
333+
/// - Both the source and target validators MUST be active on the beacon chain and MUST NOT have
334+
/// initiated exits
336335
/// - The source validator MUST NOT have pending partial withdrawal requests (via `requestWithdrawal`)
337336
/// - If the source validator is slashed after requesting consolidation (but before processing),
338337
/// the consolidation will be skipped.

src/contracts/pods/EigenPod.sol

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -311,14 +311,12 @@ contract EigenPod is
311311
ConsolidationRequest[] calldata requests
312312
) external payable onlyWhenNotPaused(PAUSED_CONSOLIDATIONS) onlyOwnerOrProofSubmitter {
313313
uint256 fee = getConsolidationRequestFee();
314-
require(msg.value >= fee * requests.length, InsufficientFunds());
315-
uint256 remainder = msg.value - (fee * requests.length);
314+
uint256 totalFee = fee * requests.length;
315+
require(msg.value >= totalFee, InsufficientFunds());
316+
uint256 remainder = msg.value - totalFee;
316317

317318
for (uint256 i = 0; i < requests.length; i++) {
318319
ConsolidationRequest calldata request = requests[i];
319-
// Validate pubkeys are well-formed
320-
require(request.srcPubkey.length == 48, InvalidPubKeyLength());
321-
require(request.targetPubkey.length == 48, InvalidPubKeyLength());
322320

323321
// Ensure target has verified withdrawal credentials pointed at this pod
324322
bytes32 sourcePubkeyHash = _calcPubkeyHash(request.srcPubkey);
@@ -347,24 +345,20 @@ contract EigenPod is
347345
WithdrawalRequest[] calldata requests
348346
) external payable onlyWhenNotPaused(PAUSED_WITHDRAWAL_REQUESTS) onlyOwnerOrProofSubmitter {
349347
uint256 fee = getWithdrawalRequestFee();
350-
require(msg.value >= fee * requests.length, InsufficientFunds());
351-
uint256 remainder = msg.value - (fee * requests.length);
348+
uint256 totalFee = fee * requests.length;
349+
require(msg.value >= totalFee, InsufficientFunds());
350+
uint256 remainder = msg.value - totalFee;
352351

353352
for (uint256 i = 0; i < requests.length; i++) {
354353
WithdrawalRequest calldata request = requests[i];
355-
// Validate pubkey is well-formed.
356-
//
357-
// It's not necessary to perform any additional validation; the worst-case
358-
// scenario is just that the consensus layer skips an invalid request.
359-
require(request.pubkey.length == 48, InvalidPubKeyLength());
354+
bytes32 pubkeyHash = _calcPubkeyHash(request.pubkey);
360355

361356
// Call the predeploy
362357
bytes memory callData = abi.encodePacked(request.pubkey, request.amountGwei);
363358
(bool ok,) = WITHDRAWAL_REQUEST_ADDRESS.call{value: fee}(callData);
364359
require(ok, PredeployFailed());
365360

366361
// Emit event depending on whether the request is a full exit or a partial withdrawal
367-
bytes32 pubkeyHash = _calcPubkeyHash(request.pubkey);
368362
if (request.amountGwei == 0) emit ExitRequested(pubkeyHash);
369363
else emit WithdrawalRequested(pubkeyHash, request.amountGwei);
370364
}

0 commit comments

Comments
 (0)