feat: deprecate legacy admin flows; migrate 5 contracts to RoleRegistry#390
feat: deprecate legacy admin flows; migrate 5 contracts to RoleRegistry#390pankajjagtapp wants to merge 14 commits intopankaj/feat/security-upgradesfrom
Conversation
…ckets - Replace admins/pausers mappings with DEPRECATED_ prefixed storage slots - Add 4 function-granular roles: ADMIN, REQUEST_WITHDRAWALS, CLAIM_WITHDRAWALS, DEPOSIT_INTO_STRATEGY - Add rate-limiter consume() calls on stEthRequestWithdrawal, queueWithdrawals, depositIntoStrategy - Expand constructor to accept roleRegistry and rateLimiter immutables - Remove updateAdmin, updatePauser, _requireAdmin, _requirePauser, onlyPauser - Add 12-test migration suite; fix all compile callers across test and script files Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Verifies that upgrading EtherFiOracle, NodeOperatorManager, AuctionManager, and EtherFiRestaker to their RoleRegistry-aware implementations leaves all 400 sequential storage slots byte-identical. Also asserts the new roleRegistry and rateLimiter immutables read non-zero post-upgrade on a mainnet fork. BucketRateLimiter is excluded: it has no standalone mainnet proxy (only deployed fresh in unit tests via TestSetup). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
💡 Codex Reviewsmart-contracts/src/LiquidityPool.sol Lines 225 to 227 in 200eaaa
smart-contracts/src/WithdrawRequestNFT.sol Lines 140 to 142 in 200eaaa
ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
📊 Forge Coverage ReportGenerated by workflow run #698 |
…o gwei - Adjusted the conversion logic to ensure any non-zero wei amount consumes at least 1 gwei from the bucket, preventing sub-gwei dust from bypassing the rate limiter.
… function - Replaced direct calls to withdrawEther with a new internal function _withdrawEther to streamline the withdrawal process and enhance code maintainability.
| @@ -794,19 +794,21 @@ contract EtherFiOracleTest is TestSetup { | |||
|
|
|||
| function test_updateAdmin() public { | |||
… use onlyAdmin modifier - Changed the access control for token registration and capacity settings in BucketRateLimiter from onlyOwner to onlyAdmin. - Simplified the initializeOnUpgrade function in AuctionManager by removing the etherFiAdminContractAddress parameter, aligning with the new role-based access control structure.
…nto pankaj/deprecate-legacy-admin-flows
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit cdca554. Configure here.

Summary
Migrates
EtherFiOracle,NodeOperatorManager,BucketRateLimiter,AuctionManager, andEtherFiRestakerfrom localmapping(address => bool) public adminsaccess control to the protocol-wideRoleRegistry. Each contract:IRoleRegistry public immutable roleRegistry(constructor-set, no storage slot).<NAME>_ADMIN_ROLEconstant.admins→DEPRECATED_admins(andpausers→DEPRECATED_pauserswhere applicable) — preserves existing storage slots.onlyAdminmodifier body withroleRegistry.hasRole(...) revert IncorrectRole();.pauseContract/unPauseContractthroughPROTOCOL_PAUSER/PROTOCOL_UNPAUSER.updateAdmin(andupdatePauser) setters.EtherFiRestakeradditionally:ADMIN,REQUEST_WITHDRAWALS,CLAIM_WITHDRAWALS,DEPOSIT_INTO_STRATEGY).IEtherFiRateLimiter public immutable rateLimiterplus 3 rate-limit bucket IDs (STETH_REQUEST_WITHDRAWAL_LIMIT_ID,QUEUE_WITHDRAWALS_LIMIT_ID,DEPOSIT_INTO_STRATEGY_LIMIT_ID). Rate-limited functions callrateLimiter.consume(...)after the role gate.Tests
test/fork-tests/RoleMigrationStorageIntegrity.t.sol): forks mainnet, snapshots first 400 storage slots of each proxy pre-upgrade, upgrades in place, asserts byte-identical storage post-upgrade. Covers 4 mainnet-deployed proxies (BucketRateLimiterhas no standalone mainnet proxy and is unit-tested instead).roleRegistry.grantRoleinstead of removedupdateAdmincalls;IncorrectRole.selectorreplaces legacy revert strings.Out of scope (follow-on work)
EtherFiRestaker(capacity / refill rate must be set by operations team viaOPERATING_TIMELOCKpost-upgrade).OPERATING_TIMELOCK(0xcD42...d7a) — operator concern.Test plan
🤖 Generated with Claude Code
Note
High Risk
High risk because it changes authorization for multiple core protocol contracts (admin and pause/unpause paths) and introduces rate-limiter enforcement on
EtherFiRestakerwithdrawal/strategy operations, which could block operations if roles or buckets are misconfigured during/after upgrade.Overview
Migrates privileged access control from per-contract
admins/pausersmappings toRoleRegistryacrossAuctionManager,NodeOperatorManager,EtherFiOracle, andBucketRateLimiter, preserving storage by renaming old mappings toDEPRECATED_*, removingupdateAdmin/updatePausersetters, and switchingpauseContract/unPauseContractto protocol-widePROTOCOL_PAUSER/PROTOCOL_UNPAUSERchecks.Updates
EtherFiRestakerto use role-based, function-granular permissions plus an immutablerateLimiter;stEthRequestWithdrawal,queueWithdrawals, anddepositIntoStrategynow consume rate-limit buckets (wei→gwei rounding-up) and admin/pausing gates are enforced viaRoleRegistryroles instead of legacy mappings.Scripts and tests are updated accordingly: deployment/verification scripts pass new constructor args and grant roles via
RoleRegistry, new role-migration tests assert selector removals and deprecated-storage readability, and a mainnet fork test snapshots storage slots pre/post upgrade to validate no storage drift for upgraded proxies.Reviewed by Cursor Bugbot for commit cdca554. Bugbot is set up for automated code reviews on this repo. Configure here.