26Q2 - Security Upgrades by pankajjagtapp · Pull Request #385 · etherfi-protocol/smart-contracts

pankajjagtapp · 2026-04-29T20:57:29Z

Note

High Risk
High risk because it modifies access control and pause/upgrade permissions across core contracts (AuctionManager, EETH, EtherFiAdmin, BucketRateLimiter, etc.) and changes oracle report/withdrawal finalization logic, which can directly impact protocol safety and fund flows.

Overview
Security-focused refactor across core contracts. Replaces legacy owner/admin mappings and per-contract role constants with consolidated RoleRegistry gates (timelock/multisig/guardian/upgrader), adds IBlacklister enforcement on user-facing paths, and introduces pause-until controls via PausableUntil (including new guardian/ops pause flows).

Protocol behavior tightened. EtherFiAdmin now validates oracle reports via a staged _validateReport pipeline (APR/fees/withdrawals/validator-approval rate caps) and adds a permissionless stale-oracle escape hatch to finalize withdrawals when reports stop; EETH adds rate-limited mint/burn/transfer paths and pause controls; AuctionManager and BucketRateLimiter switch to role-based operations and add clearer custom errors.

Ops tooling aligned. Numerous scripts update role references to tier roles, rename timelock delay constants, adjust constructor args/bytecode verification for upgraded implementations, add an Optimism RPC secret to CI, and ignore foundry.lock.

^{Reviewed by Cursor Bugbot for commit 97cfd1f. Bugbot is set up for automated code reviews on this repo. Configure here.}

… paused

github-actions · 2026-04-29T21:01:09Z

📊 Forge Coverage Report

Coverage report could not be generated. Check workflow logs for details.

_{Generated by workflow run #731}

…idityPool is paused; add tests for permissionless claims

…guard feat: Reentrancy Guard and permissonless claim

…until

yash/feat/pausable-until

yash/fix-tests

…rors fix/misc-changes

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 4 total unresolved issues (including 3 from previous reviews).

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit bdac379. Configure here.}

cursor · 2026-05-21T00:29:24Z

-        if (_etherFiNodesManager && IEtherFiPausable(address(etherFiNodesManager)).paused()) {
-            etherFiNodesManager.unPauseContract();
-        }
-


Missing constructor validation for validator approval ceiling

Medium Severity

The constructor validates all immutable ceiling parameters for non-zero (_maxAcceptableRebaseAprInBps, _maxValidatorTaskBatchSize, _staleOracleReportBlockWindow, _maxAcceptableFinalizedWithdrawalAmountPerDay) but omits validation for _maxAcceptableNumValidatorsToApprovePerDay. If deployed with 0, updateMaxNumValidatorsToApprovePerDay can never set maxNumValidatorsToApprovePerDay above 0 (since _maxNumValidatorsToApprovePerDay > 0 always reverts), permanently blocking all validator approvals through _validateValidatorApprovals.

^{Reviewed by Cursor Bugbot for commit bdac379. Configure here.}

…ate-limits Signed-off-by: Pankaj Jagtap <jagtap.pankaj2000@gmail.com>

… contracts

…Liquifier

…limits feat: integrate EtherFi rate limiter into EETH and WeETH contracts

pankajjagtapp · 2026-05-21T17:39:19Z

-    function unpublishReport(bytes32 _hash) external isAdmin {
-        require(consensusStates[_hash].consensusReached, "Consensus is not reached yet");
+    function unpublishReport(bytes32 _hash) external onlyOperations {
+        if (!consensusStates[_hash].consensusReached) revert ConsensusNotReached();


Check that the task has not been executed before unpublishing

pankajjagtapp · 2026-05-21T18:15:56Z


-    function setClaimDelay(uint256 _claimDelay) external {
-        if(!roleRegistry.hasRole(CUMULATIVE_MERKLE_REWARDS_DISTRIBUTOR_CLAIM_DELAY_SETTER_ROLE, msg.sender)) revert IncorrectRole();
+    function setClaimDelay(uint256 _claimDelay) external onlyOperations {


Should put this behind onlyAdmin, operating timelock

pankajjagtapp · 2026-05-21T18:16:28Z


-    function updateWhitelistedRecipient(address user, bool isWhitelisted) external {
-        if(!roleRegistry.hasRole(CUMULATIVE_MERKLE_REWARDS_DISTRIBUTOR_ADMIN_ROLE, msg.sender)) revert IncorrectRole();
+    function updateWhitelistedRecipient(address user, bool isWhitelisted) external onlyOperations {


Should put this behind onlyAdmin, operating timelock

pankajjagtapp · 2026-05-21T19:49:54Z

+    function revokeEOA1Role(address account) external onlyOperations {
+        roleRegistry.revokeFast(roleRegistry.ORACLE_OPERATIONS_ROLE(), account);
+    }
+
+    function revokeEOA2Role(address account) external onlyOperations {
+        roleRegistry.revokeFast(roleRegistry.HOUSEKEEPING_OPERATIONS_ROLE(), account);
+    }
+
+    function revokeEOA3Role(address account) external onlyOperations {
+        roleRegistry.revokeFast(roleRegistry.EXECUTOR_OPERATIONS_ROLE(), account);
+    }
+
+    function revokeEOA4Role(address account) external onlyOperations {
+        roleRegistry.revokeFast(roleRegistry.EIGENPOD_OPERATIONS_ROLE(), account);
+    }


Rename EOA naming from here

pankajjagtapp · 2026-05-21T20:06:17Z

+    IBlacklister public immutable blacklister;
+    INodeOperatorManager public immutable nodeOperatorManager;
+    address public immutable stakingManagerContractAddress;
+    address public immutable membershipManagerContractAddress;


Import our treasury address here and update the transferAccumulatedRevenue to send the ETH to the Treasury

pankajjagtapp · 2026-05-21T20:56:36Z

+    IRoleRegistry public immutable roleRegistry;
+    IBlacklister public immutable blacklister;
+
+    bytes32 public constant MEMBERSHIP_MANAGER_OPERATIONS_ROLE = keccak256("MEMBERSHIP_MANAGER_OPERATIONS_ROLE");


Should remove this

pankajjagtapp · 2026-05-21T21:05:02Z

 /// @title Router token swapping functionality
 /// @notice Functions for swapping tokens via PancakeSwap V3
 interface IPancackeV3SwapRouter {
    function WETH9() external returns (address);

    struct ExactInputSingleParams {
        address tokenIn;
        address tokenOut;
        uint24 fee;
        address recipient;
        uint256 deadline;
        uint256 amountIn;
        uint256 amountOutMinimum;
        uint160 sqrtPriceLimitX96;
    }

    /// @notice Swaps `amountIn` of one token for as much as possible of another token
    /// @dev Setting `amountIn` to 0 will cause the contract to look up its own balance,
    /// and swap the entire amount, enabling contracts to send tokens before calling this function.
    /// @param params The parameters necessary for the swap, encoded as `ExactInputSingleParams` in calldata
    /// @return amountOut The amount of the received token
    function exactInputSingle(ExactInputSingleParams calldata params) external payable returns (uint256 amountOut);


Let's move this to ILiquifier or remove all together with address private pancakeswapRouter

pankajjagtapp · 2026-05-21T21:05:18Z

 interface IERC20Burnable is IERC20 {
    function burn(uint256 amount) external;
 }


Same, should more to ILiquiifier. Let's take this easy wins

pankajjagtapp · 2026-05-21T21:08:58Z

    }

-    function updateTimeBoundCapRefreshInterval(uint32 _timeBoundCapRefreshInterval) external onlyOwner {
+    function updateTimeBoundCapRefreshInterval(uint32 _timeBoundCapRefreshInterval) external onlyOperations {


Let's put all such setter functions behind onlyAdmin on this contract and remove inconsistency with RedemptionManager and within this contract

pankajjagtapp · 2026-05-21T21:26:46Z


    /// Set the claimer of the restaking rewards of this contract
-    function setRewardsClaimer(address _claimer) external onlyAdmin {
+    function setRewardsClaimer(address _claimer) external onlyOperations {


Lets keep this onlyAdmin. We dont want to change the receipient address. We had request to have similar receipient address setter behind Timelock.

pankajjagtapp · 2026-05-21T21:27:18Z


    // Send the ETH back to the liquidity pool
-    function withdrawEther() public onlyAdmin {
+    function withdrawEther() public onlyOperations {


This should be houseKeeping role

pankajjagtapp · 2026-05-21T21:28:27Z

    }

-    function transferAccumulatedRevenue() external onlyAdmin {
+    function transferAccumulatedRevenue() external onlyOperations {


Housekeeping Role after the change for transfer to Treasury is implemented

pankajjagtapp · 2026-05-21T21:30:42Z

+    function pauseContract() external onlyOperations {
        _pause();
    }
-    function unPauseContract() external {
-        if (!roleRegistry.hasRole(roleRegistry.PROTOCOL_UNPAUSER(), msg.sender)) revert IncorrectRole();
+    function unPauseContract() external onlyOperations {
        _unpause();
    }


We intended to remove this because no pausing functionality in this one

pankajjagtapp · 2026-05-21T21:34:32Z

+    function completeQueuedWithdrawals(
+        IDelegationManager.Withdrawal[] memory _queuedWithdrawals,
+        IERC20[][] memory _tokens
+    ) external {
+        if (!roleRegistry.hasRole(roleRegistry.EXECUTOR_OPERATIONS_ROLE(), msg.sender)) revert IncorrectRole();


HouseKeeping Role

pankajjagtapp · 2026-05-21T21:35:25Z

+    function stEthClaimWithdrawals(uint256[] calldata _requestIds, uint256[] calldata _hints) external {
+        if (!roleRegistry.hasRole(roleRegistry.EXECUTOR_OPERATIONS_ROLE(), msg.sender)) revert IncorrectRole();
        lidoWithdrawalQueue.claimWithdrawals(_requestIds, _hints);


HouseKeeping Role

pankajjagtapp · 2026-05-21T21:37:00Z

    function setProofSubmitter(address node, address proofSubmitter) public onlyEigenlayerAdmin whenNotPaused {
        _validateNode(node);
        IEtherFiNode(node).setProofSubmitter(proofSubmitter);
    }



onlyOperations Role

pankajjagtapp · 2026-05-21T21:37:16Z

    function setProofSubmitter(uint256 id, address proofSubmitter) external onlyEigenlayerAdmin whenNotPaused {
        setProofSubmitter(etherfiNodeAddress(id), proofSubmitter);
    }


onlyOperations

pankajjagtapp · 2026-05-21T21:41:58Z

    function queueETHWithdrawal(address node, uint256 amount) public onlyEigenlayerAdmin whenNotPaused returns (bytes32 withdrawalRoot) {
        _validateNode(node);
        rateLimiter.consume(UNRESTAKING_LIMIT_ID, SafeCast.toUint64(amount / 1 gwei));
        return IEtherFiNode(node).queueETHWithdrawal(amount);
    }

    function queueETHWithdrawal(uint256 id, uint256 amount) external onlyEigenlayerAdmin whenNotPaused returns (bytes32 withdrawalRoot) {
        return queueETHWithdrawal(etherfiNodeAddress(id), amount);
    }


Exectuor Operations role

pankajjagtapp · 2026-05-21T21:42:05Z

    function queueWithdrawals(address node, IDelegationManager.QueuedWithdrawalParams[] calldata params) public onlyEigenlayerAdmin whenNotPaused {
        _validateNode(node);
        // need to rate limit any beacon eth being withdrawn
        rateLimiter.consume(UNRESTAKING_LIMIT_ID, SafeCast.toUint64(sumRestakingETHWithdrawals(params) / 1 gwei));
        IEtherFiNode(node).queueWithdrawals(params);
    }

    function queueWithdrawals(uint256 id, IDelegationManager.QueuedWithdrawalParams[] calldata params) external onlyEigenlayerAdmin whenNotPaused {
        queueWithdrawals(etherfiNodeAddress(id), params);
    }


Exectuor Operations role

pankajjagtapp · 2026-05-21T21:43:41Z

    /// @param _bidId the ID of the validator
    function processAuctionFeeTransfer(
        uint256 _bidId
    ) external onlyStakingManagerContract {
        uint256 amount = bids[_bidId].amount;
        uint256 newAccumulatedRevenue = accumulatedRevenue + amount;
        if (newAccumulatedRevenue >= accumulatedRevenueThreshold) {
            accumulatedRevenue = 0;
            (bool sent, ) = membershipManagerContractAddress.call{value: newAccumulatedRevenue}("");
-            require(sent, "Failed to send Ether");
+            if (!sent) revert EtherTransferFailed();
        } else {
            accumulatedRevenue = uint128(newAccumulatedRevenue);
        }
    }


Not used anywhere. Can just remove it

pankajjagtapp · 2026-05-22T11:32:07Z

    uint32 public nextRequestId;
    uint32 public lastFinalizedRequestId;
    uint16 public shareRemainderSplitToTreasuryInBps;
    uint16 public _unused_gap;


Let's make this variable private too ?

pankajjagtapp · 2026-05-22T12:21:38Z

+    function setBlacklistUntil(address user, uint256 until) external onlyOperations {
+        blacklistedUntil[user] = block.timestamp + until;
+        emit UserBlacklistedUntil(user, block.timestamp + until);
+    }


Thinking what if we need to blacklist some malicious users across the industry every now and then. It would not be possible to keep doing Multisig transactions with our process. What to do in that case? @0xpanicError @seongyun-ko

pankajjagtapp · 2026-05-22T12:22:51Z

Recommend to abstract modifiers and pausable functions in the Pausable contract. changes in every contract otherwise

0xpanicError and others added 8 commits April 29, 2026 14:14

fix: validate finalized invalidated request

d084040

fix: cursor comments

ceef22a

fix: loop logic

28ae102

fix: address comments

2dab0fc

fix: withdraw ether role

fee01a3

test: Add tests for claimWithdraw functionality when LiquidityPool is…

eff1be6

… paused

fix: address comments

602bd36

refactor: deprecate pausers

c97ec94

pankajjagtapp assigned pankajjagtapp and 0xpanicError Apr 29, 2026

pankajjagtapp added the enhancement New feature or request label Apr 29, 2026

cursor Bot reviewed Apr 29, 2026

View reviewed changes

Comment thread src/EtherFiAdmin.sol

pankajjagtapp and others added 6 commits April 29, 2026 16:12

feat: Allow claimWithdraw and batchClaimWithdraw to succeed when Liqu…

1493373

…idityPool is paused; add tests for permissionless claims

test: add fork test fro min amount for share

ca46d04

test: fix tests

fb904b5

Merge pull request #379 from etherfi-protocol/pankaj/feat/reentrancy-…

de9e377

…guard feat: Reentrancy Guard and permissonless claim

Merge 'origin/pankaj/feat/security-upgrades' into yash/feat/pausable-…

de8e044

…until

Merge pull request #382 from etherfi-protocol/yash/feat/pausable-until

e8df02c

yash/feat/pausable-until

cursor Bot reviewed Apr 29, 2026

View reviewed changes

Comment thread src/LiquidityPool.sol

fix: pause until with permissionless claims

6f9f673

cursor Bot reviewed Apr 29, 2026

View reviewed changes

Comment thread src/PriorityWithdrawalQueue.sol

0xpanicError and others added 4 commits April 30, 2026 10:29

test: fix failing tests

74c3b00

test: create helper to fix validator fork tests

0998c14

test: fix liquid scroll vault tests

fa8df1f

Merge pull request #387 from etherfi-protocol/yash/fix-tests

6db9c1a

yash/fix-tests

cursor Bot reviewed Apr 30, 2026

View reviewed changes

Comment thread src/Liquifier.sol

pankajjagtapp added 3 commits April 30, 2026 13:11

fix: build errors from prior merges (PausableUntil typo, Liquifier ctor)

b124469

feat(EtherFiOracle): migrate admin ACL to RoleRegistry

532e2b4

fix(EtherFiOracle): tighten role-migration tests per code review

69ba812

0xpanicError and others added 3 commits May 20, 2026 17:15

refactor: remove deprecated code

45ee2b9

fix: superguardian for token pause until

e7ff1f9

Merge pull request #416 from etherfi-protocol/yash/refactor/custom-er…

bdac379

…rors fix/misc-changes

cursor Bot reviewed May 21, 2026

View reviewed changes

pankajjagtapp and others added 4 commits May 21, 2026 06:07

Merge branch 'pankaj/feat/security-upgrades' into pankaj/feat/token-r…

d9bf6b8

…ate-limits Signed-off-by: Pankaj Jagtap <jagtap.pankaj2000@gmail.com>

refactor: integrate RateLimitMath for rate limiting in EETH and WeETH…

df37f3c

… contracts

feat: add ReportUnpublished event and validation for deposit caps in …

226b139

…Liquifier

Merge pull request #417 from etherfi-protocol/pankaj/feat/token-rate-…

97cfd1f

…limits feat: integrate EtherFi rate limiter into EETH and WeETH contracts

pankajjagtapp commented May 21, 2026

View reviewed changes

cursor Bot mentioned this pull request May 22, 2026

Pankaj/feat/security upgrades pr scripts #420

Open

pankajjagtapp commented May 22, 2026

View reviewed changes

Conversation

pankajjagtapp commented Apr 29, 2026 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📊 Forge Coverage Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor Bot May 21, 2026

Choose a reason for hiding this comment

Missing constructor validation for validator approval ceiling

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pankajjagtapp commented May 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

pankajjagtapp commented Apr 29, 2026 •

edited by cursor Bot

Loading

github-actions Bot commented Apr 29, 2026 •

edited

Loading