Skip to content

yash/feat/pausable-until#382

Merged
0xpanicError merged 13 commits intopankaj/feat/security-upgradesfrom
yash/feat/pausable-until
Apr 29, 2026
Merged

yash/feat/pausable-until#382
0xpanicError merged 13 commits intopankaj/feat/security-upgradesfrom
yash/feat/pausable-until

Conversation

@0xpanicError
Copy link
Copy Markdown

@0xpanicError 0xpanicError commented Apr 24, 2026

Note

High Risk
Touches pausing and role-gated control paths across many core contracts (deposits/withdrawals/redemptions/rate limiting), so miswiring _requireNotPaused or role assignments can halt protocol operations or weaken emergency controls.

Overview
Adds a new PausableUntil utility (namespaced storage, 1-day max duration + per-pauser cooldown) and integrates it into multiple contracts so whenNotPaused checks also block during a timed pause.

Exposes pauseContractUntil() / unpauseContractUntil() across LiquidityPool, Liquifier, EtherFiNodesManager, EtherFiRateLimiter, EtherFiRedemptionManager, PriorityWithdrawalQueue, WithdrawRequestNFT, WeETHWithdrawAdapter, and CumulativeMerkleRewardsDistributor, with new RoleRegistry roles PAUSE_UNTIL_ROLE and UNPAUSE_UNTIL_ROLE and interface updates.

Refactors Liquifier access control away from local admins/pausers mappings to RoleRegistry roles (LIQUIFIER_ADMIN_ROLE, LIQUIFIER_SENDER_ROLE, PROTOCOL_PAUSER/UNPAUSER), and removes the scan-completion gate from WithdrawRequestNFT.unPauseContract(); adds extensive tests for timed pause behavior and role requirements (including a new PausableUntil.t.sol and adapter tests).

Reviewed by Cursor Bugbot for commit de8e044. Bugbot is set up for automated code reviews on this repo. Configure here.

Comment thread src/EtherFiNodesManager.sol
Comment thread src/EtherFiNodesManager.sol Outdated
Comment thread src/utils/PausableUntil.sol
Comment thread src/Liquifier.sol
Comment thread src/EtherFiRateLimiter.sol
Comment thread src/Liquifier.sol Outdated
Comment thread src/Liquifier.sol Outdated
Comment thread src/Liquifier.sol Outdated
Comment thread src/WithdrawRequestNFT.sol Outdated
Comment thread src/Liquifier.sol Outdated
Copy link
Copy Markdown
Contributor

@pankajjagtapp pankajjagtapp left a comment

Choose a reason for hiding this comment

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

lgtm

@0xpanicError 0xpanicError merged commit e8df02c into pankaj/feat/security-upgrades Apr 29, 2026
1 check passed
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

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

Fix All in Cursor

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

Reviewed by Cursor Bugbot for commit de8e044. Configure here.

function _requireNotPaused() internal view override {
_requireNotPausedUntil();
super._requireNotPaused();
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Override blocks full-pause escalation during timed pause

Medium Severity

The _requireNotPaused override routes through _requireNotPausedUntil(), which also gates OZ's internal _pause() (since it carries a whenNotPaused modifier). This means pauseContract() reverts while pause-until is active, preventing the PROTOCOL_PAUSER from escalating a time-limited pause to a full indefinite pause. Contracts with custom pause implementations (e.g., LiquidityPool, PriorityWithdrawalQueue) don't have this limitation because their pauseContract() sets paused = true directly. This creates inconsistent emergency-pause behavior across the protocol.

Additional Locations (2)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit de8e044. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants