fix: resolve inheritance order and storage layout for upgraded rc

foundry.toml

@@ -26,15 +26,15 @@
     # Specifies the exact version of Solidity to use, overriding auto-detection.
     solc_version = '0.8.27'
     # If enabled, treats Solidity compiler warnings as errors, preventing artifact generation if warnings are present.
-    deny_warnings = false 
+    deny_warnings = false
     # If set to true, changes compilation pipeline to go through the new IR optimizer.
     via_ir = false
     # Whether or not to enable the Solidity optimizer.
     optimizer = true
-    # The number of runs specifies roughly how often each opcode of the deployed code will be executed 
-    # across the life-time of the contract. This means it is a trade-off parameter between code size (deploy cost) 
+    # The number of runs specifies roughly how often each opcode of the deployed code will be executed
+    # across the life-time of the contract. This means it is a trade-off parameter between code size (deploy cost)
     # and code execution cost (cost after deployment).
-    optimizer_runs = 200
+    optimizer_runs = 100
 
     # Test Configuration
 
@@ -44,7 +44,7 @@
     # - 4 (-vvvv): Stack traces for all tests and setup traces for failing tests are displayed.
     # - 5 (-vvvvv): Stack and setup traces are always displayed.
     verbosity = 0
-    # Enables the Foreign Function Interface (FFI) cheatcode. 
+    # Enables the Foreign Function Interface (FFI) cheatcode.
     # WARNING: This allows arbitrary programs to run on your computer, which poses security risks.
     ffi = true
     # Contracts to include in gas reports. By default, all contracts are included.
@@ -53,7 +53,7 @@
     show_progress = true
     # Sparse mode only compiles files that match certain criteria.
     sparse_mode = true
-    
+
     gas_limit = 5000000000
     no-match-contract = "FFI"
     fs_permissions = [{ access = "read-write", path = "./" }]
@@ -64,21 +64,21 @@
     # Formatting style for long function headers
     multiline_func_header = "params_first"  # Options: "attributes_first", "params_first", "all"
     # Sort import statements alphabetically
-    sort_imports = false  
+    sort_imports = false
     # Maximum line length where formatter will wrap the line
     line_length = 100  # Default: 120
     # Number of spaces per indentation level
     tab_width = 4  # Default: 4
     # Whether to print spaces between brackets
-    bracket_spacing = false  
+    bracket_spacing = false
     # Style of uint/int256 types
     int_types = "long"  # Options: "long", "short", "preserve"
     # Quotation mark style
     quote_style = "double"  # Options: "double", "single", "preserve"
     # Style of underscores in number literals
     number_underscore = "remove"  # Options: "preserve", "thousands", "remove"
     # Whether or not to wrap comments at line_length
-    wrap_comments = false  
+    wrap_comments = false
     # List of files to ignore during formatting (can use glob patterns)
     # ignore = [
     #     "./script/**/*",
@@ -94,11 +94,13 @@
 #     ]
 
 [profile.ci.fuzz]
-    optimizer = false
+    optimizer=true
+    optimizer_runs = 100
     runs = 32
 
 [profile.intense.fuzz]
-    optimizer = false
+    optimizer=true
+    optimizer_runs = 100
     runs = 5000
 
 [profile.forktest.fuzz]

src/BLSApkRegistry.sol

@@ -82,7 +82,7 @@ contract BLSApkRegistry is BLSApkRegistryStorage {
         address operator,
         PubkeyRegistrationParams calldata params,
         BN254.G1Point calldata pubkeyRegistrationMessageHash
-    ) external onlyRegistryCoordinator returns (bytes32 operatorId) {
+    ) public onlyRegistryCoordinator returns (bytes32 operatorId) {
         bytes32 pubkeyHash = BN254.hashG1Point(params.pubkeyG1);
         require(pubkeyHash != ZERO_PK_HASH, ZeroPubKey());
         require(getOperatorId(operator) == bytes32(0), OperatorAlreadyRegistered());
@@ -124,6 +124,19 @@ contract BLSApkRegistry is BLSApkRegistryStorage {
         return pubkeyHash;
     }
 
+    /// @inheritdoc IBLSApkRegistry
+    function getOrRegisterOperatorId(
+        address operator,
+        PubkeyRegistrationParams calldata params,
+        BN254.G1Point calldata pubkeyRegistrationMessageHash
+    ) external onlyRegistryCoordinator returns (bytes32 operatorId) {
+        operatorId = getOperatorId(operator);
+        if (operatorId == 0) {
+            operatorId = registerBLSPublicKey(operator, params, pubkeyRegistrationMessageHash);
+        }
+        return operatorId;
+    }
+
     /// @notice Verifies and registers a G2 public key for an operator that already has a G1 key
     /// @dev This is meant to be used as a one-time way to add G2 public keys for operators that have G1 keys but no G2 key on chain
     /// @param operator The address of the operator to register the G2 key for

src/RegistryCoordinator.sol

@@ -28,26 +28,20 @@ import {RegistryCoordinatorStorage} from "./RegistryCoordinatorStorage.sol";
  *
  * @author Layr Labs, Inc.
  */
-contract RegistryCoordinator is RegistryCoordinatorStorage {
+contract RegistryCoordinator is RegistryCoordinatorStorage, SlashingRegistryCoordinator {
     using BitmapUtils for *;
 
     constructor(
-        IServiceManager _serviceManager,
-        IStakeRegistry _stakeRegistry,
-        IBLSApkRegistry _blsApkRegistry,
-        IIndexRegistry _indexRegistry,
-        ISocketRegistry _socketRegistry,
-        IAllocationManager _allocationManager,
-        IPauserRegistry _pauserRegistry
+        RegistryCoordinatorStorage.RegistryCoordinatorParams memory params
     )
-        RegistryCoordinatorStorage(
-            _serviceManager,
-            _stakeRegistry,
-            _blsApkRegistry,
-            _indexRegistry,
-            _socketRegistry,
-            _allocationManager,
-            _pauserRegistry
+        RegistryCoordinatorStorage(params)
+        SlashingRegistryCoordinator(
+            params.slashingParams.stakeRegistry,
+            params.slashingParams.blsApkRegistry,
+            params.slashingParams.indexRegistry,
+            params.slashingParams.socketRegistry,
+            params.slashingParams.allocationManager,
+            params.slashingParams.pauserRegistry
         )
     {}
 
@@ -167,7 +161,7 @@ contract RegistryCoordinator is RegistryCoordinatorStorage {
             for (uint256 i = 0; i < quorumNumbers.length; i++) {
                 singleQuorumNumber[0] = quorumNumbers[i];
 
-                if (_isM2Quorum(uint8(quorumNumbers[i]))) {
+                if (isM2Quorum(uint8(quorumNumbers[i]))) {
                     // For M2 quorums, use _deregisterOperator
                     _deregisterOperator({operator: operator, quorumNumbers: singleQuorumNumber});
                 } else {
@@ -271,9 +265,9 @@ contract RegistryCoordinator is RegistryCoordinatorStorage {
 
     /// @notice Returns true if the quorum number is an M2 quorum
     /// @dev We use bitwise and to check if the quorum number is an M2 quorum
-    function _isM2Quorum(
+    function isM2Quorum(
         uint8 quorumNumber
-    ) internal view returns (bool) {
+    ) public view returns (bool) {
         return m2QuorumBitmap().isSet(quorumNumber);
     }
 
@@ -293,11 +287,4 @@ contract RegistryCoordinator is RegistryCoordinatorStorage {
 
         return _getTotalQuorumBitmap();
     }
-
-    /// @notice Returns true if the quorum number is an M2 quorum
-    function isM2Quorum(
-        uint8 quorumNumber
-    ) external view returns (bool) {
-        return _isM2Quorum(quorumNumber);
-    }
 }

src/RegistryCoordinatorStorage.sol

@@ -11,12 +11,35 @@ import {IServiceManager} from "./interfaces/IServiceManager.sol";
 import {IRegistryCoordinator} from "./interfaces/IRegistryCoordinator.sol";
 import {ISocketRegistry} from "./interfaces/ISocketRegistry.sol";
 
-import {SlashingRegistryCoordinator} from "./SlashingRegistryCoordinator.sol";
+abstract contract RegistryCoordinatorStorage is IRegistryCoordinator {
+    /**
+     * @notice Parameters for initializing SlashingRegistryCoordinator
+     * @param stakeRegistry The StakeRegistry contract that keeps track of operators' stakes
+     * @param blsApkRegistry The BLSApkRegistry contract that keeps track of operators' BLS public keys
+     * @param indexRegistry The IndexRegistry contract that keeps track of ordered operator lists
+     * @param socketRegistry The SocketRegistry contract that keeps track of operators' sockets
+     * @param allocationManager The AllocationManager contract for operator set management
+     * @param pauserRegistry The PauserRegistry contract for pausing functionality
+     */
+    struct SlashingRegistryParams {
+        IStakeRegistry stakeRegistry;
+        IBLSApkRegistry blsApkRegistry;
+        IIndexRegistry indexRegistry;
+        ISocketRegistry socketRegistry;
+        IAllocationManager allocationManager;
+        IPauserRegistry pauserRegistry;
+    }
+
+    /**
+     * @notice Parameters for initializing RegistryCoordinator
+     * @param serviceManager The ServiceManager contract for this AVS
+     * @param slashingParams Parameters for initializing SlashingRegistryCoordinator
+     */
+    struct RegistryCoordinatorParams {
+        IServiceManager serviceManager;
+        SlashingRegistryParams slashingParams;
+    }
 
-abstract contract RegistryCoordinatorStorage is
-    IRegistryCoordinator,
-    SlashingRegistryCoordinator
-{
     /**
      *
      *                            CONSTANTS AND IMMUTABLES
@@ -45,24 +68,9 @@ abstract contract RegistryCoordinatorStorage is
     uint256 internal _m2QuorumBitmap;
 
     constructor(
-        IServiceManager _serviceManager,
-        IStakeRegistry _stakeRegistry,
-        IBLSApkRegistry _blsApkRegistry,
-        IIndexRegistry _indexRegistry,
-        ISocketRegistry _socketRegistry,
-        IAllocationManager _allocationManager,
-        IPauserRegistry _pauserRegistry
-    )
-        SlashingRegistryCoordinator(
-            _stakeRegistry,
-            _blsApkRegistry,
-            _indexRegistry,
-            _socketRegistry,
-            _allocationManager,
-            _pauserRegistry
-        )
-    {
-        serviceManager = _serviceManager;
+        RegistryCoordinatorParams memory params
+    ) {
+        serviceManager = params.serviceManager;
     }
 
     uint256[48] private __GAP;

src/SlashingRegistryCoordinator.sol

@@ -25,7 +25,8 @@ import {QuorumBitmapHistoryLib} from "./libraries/QuorumBitmapHistoryLib.sol";
 
 import {OwnableUpgradeable} from "@openzeppelin-upgrades/contracts/access/OwnableUpgradeable.sol";
 import {Initializable} from "@openzeppelin-upgrades/contracts/proxy/utils/Initializable.sol";
-import {EIP712} from "@openzeppelin/contracts/utils/cryptography/draft-EIP712.sol";
+import {EIP712Upgradeable} from
+    "@openzeppelin-upgrades/contracts/utils/cryptography/EIP712Upgradeable.sol";
 
 import {Pausable} from "eigenlayer-contracts/src/contracts/permissions/Pausable.sol";
 import {SlashingRegistryCoordinatorStorage} from "./SlashingRegistryCoordinatorStorage.sol";
@@ -40,12 +41,12 @@ import {SlashingRegistryCoordinatorStorage} from "./SlashingRegistryCoordinatorS
  * @author Layr Labs, Inc.
  */
 contract SlashingRegistryCoordinator is
-    EIP712,
     Initializable,
     Pausable,
     OwnableUpgradeable,
     SlashingRegistryCoordinatorStorage,
-    ISignatureUtils
+    ISignatureUtils,
+    EIP712Upgradeable
 {
     using BitmapUtils for *;
     using BN254 for BN254.G1Point;
@@ -84,7 +85,6 @@ contract SlashingRegistryCoordinator is
             _socketRegistry,
             _allocationManager
         )
-        EIP712("AVSRegistryCoordinator", "v0.0.1")
         Pausable(_pauserRegistry)
     {
         _disableInitializers();
@@ -96,17 +96,18 @@ contract SlashingRegistryCoordinator is
      *
      */
     function initialize(
-        address _initialOwner,
-        address _churnApprover,
-        address _ejector,
-        uint256 _initialPausedStatus,
-        address _avs
+        address initialOwner,
+        address churnApprover,
+        address ejector,
+        uint256 initialPausedStatus,
+        address avs
     ) external initializer {
-        _transferOwnership(_initialOwner);
-        _setChurnApprover(_churnApprover);
-        _setPausedStatus(_initialPausedStatus);
-        _setEjector(_ejector);
-        _setAVS(_avs);
+        __EIP712_init("AVSRegistryCoordinator", "v0.0.1");
+        _transferOwnership(initialOwner);
+        _setChurnApprover(churnApprover);
+        _setPausedStatus(initialPausedStatus);
+        _setEjector(ejector);
+        _setAVS(avs);
     }
 
     /// @inheritdoc ISlashingRegistryCoordinator
@@ -662,13 +663,9 @@ contract SlashingRegistryCoordinator is
         address operator,
         IBLSApkRegistryTypes.PubkeyRegistrationParams memory params
     ) internal returns (bytes32 operatorId) {
-        operatorId = blsApkRegistry.getOperatorId(operator);
-        if (operatorId == 0) {
-            operatorId = blsApkRegistry.registerBLSPublicKey(
-                operator, params, pubkeyRegistrationMessageHash(operator)
-            );
-        }
-        return operatorId;
+        return blsApkRegistry.getOrRegisterOperatorId(
+            operator, params, pubkeyRegistrationMessageHash(operator)
+        );
     }
 
     /**
@@ -1104,9 +1101,7 @@ contract SlashingRegistryCoordinator is
     function pubkeyRegistrationMessageHash(
         address operator
     ) public view returns (BN254.G1Point memory) {
-        return BN254.hashToG1(
-            _hashTypedDataV4(keccak256(abi.encode(PUBKEY_REGISTRATION_TYPEHASH, operator)))
-        );
+        return BN254.hashToG1(calculatePubkeyRegistrationMessageHash(operator));
     }
 
     /**

src/interfaces/IBLSApkRegistry.sol

@@ -163,7 +163,7 @@ interface IBLSApkRegistry is IBLSApkRegistryErrors, IBLSApkRegistryEvents {
      * @param quorumNumbers The quorum numbers to register for, where each byte is an 8-bit integer.
      * @dev Access restricted to the RegistryCoordinator.
      * @dev Preconditions (assumed, not validated):
-     *      1. `quorumNumbers` has no duplicatesd
+     *      1. `quorumNumbers` has no duplicates
      *      2. `quorumNumbers.length` != 0
      *      3. `quorumNumbers` is ordered ascending
      *      4. The operator is not already registered
@@ -291,4 +291,17 @@ interface IBLSApkRegistry is IBLSApkRegistryErrors, IBLSApkRegistryEvents {
     function getOperatorFromPubkeyHash(
         bytes32 pubkeyHash
     ) external view returns (address operator);
+
+    /**
+     * @notice Gets an operator's ID if it exists, or registers a new BLS public key and returns the new ID
+     * @param operator The address of the operator
+     * @param params The parameters for registering a new BLS public key
+     * @param pubkeyRegistrationMessageHash The hash of the message to sign for registration
+     * @return operatorId The operator's ID (pubkey hash)
+     */
+    function getOrRegisterOperatorId(
+        address operator,
+        PubkeyRegistrationParams calldata params,
+        BN254.G1Point calldata pubkeyRegistrationMessageHash
+    ) external returns (bytes32 operatorId);
 }

test/harnesses/RegistryCoordinatorHarness.t.sol

@@ -19,13 +19,17 @@ contract RegistryCoordinatorHarness is RegistryCoordinator, Test {
         IPauserRegistry _pauserRegistry
     )
         RegistryCoordinator(
-            _serviceManager,
-            _stakeRegistry,
-            _blsApkRegistry,
-            _indexRegistry,
-            _socketRegistry,
-            _allocationManager,
-            _pauserRegistry
+            RegistryCoordinatorStorage.RegistryCoordinatorParams(
+                _serviceManager,
+                RegistryCoordinatorStorage.SlashingRegistryParams(
+                    _stakeRegistry,
+                    _blsApkRegistry,
+                    _indexRegistry,
+                    _socketRegistry,
+                    _allocationManager,
+                    _pauserRegistry
+                )
+            )
         )
     {
         _transferOwnership(msg.sender);