refactor: internal functions

8sunyuan · 8sunyuan · commit c462c88593e6 · 2025-01-23T11:17:29.000-05:00
diff --git a/src/RegistryCoordinator.sol b/src/RegistryCoordinator.sol
@@ -74,8 +74,7 @@ contract RegistryCoordinator is SlashingRegistryCoordinator, IRegistryCoordinato
             operator: msg.sender,
             operatorId: operatorId,
             quorumNumbers: quorumNumbers,
-            socket: socket,
-            operatorSignature: operatorSignature
+            socket: socket
         }).numOperatorsPerQuorum;
 
         // For each quorum, validate that the new operator count does not exceed the maximum
@@ -88,6 +87,16 @@ contract RegistryCoordinator is SlashingRegistryCoordinator, IRegistryCoordinato
                 MaxQuorumsReached()
             );
         }
+
+        // If the operator wasn't registered for any quorums, update their status
+        // and register them with this AVS in EigenLayer core (DelegationManager)
+        if (_operatorInfo[msg.sender].status != OperatorStatus.REGISTERED) {
+            _operatorInfo[msg.sender] =
+                OperatorInfo({operatorId: operatorId, status: OperatorStatus.REGISTERED});
+
+            serviceManager.registerOperatorToAVS(msg.sender, operatorSignature);
+            emit OperatorRegistered(msg.sender, operatorId);
+        }
     }
 
     /// @inheritdoc IRegistryCoordinator
@@ -100,10 +109,6 @@ contract RegistryCoordinator is SlashingRegistryCoordinator, IRegistryCoordinato
         SignatureWithSaltAndExpiry memory operatorSignature
     ) external onlyWhenNotPaused(PAUSED_REGISTER_OPERATOR) {
         require(!isOperatorSetAVS, OperatorSetsEnabled());
-        require(
-            operatorKickParams.length == quorumNumbers.length,
-            InputLengthMismatch()
-        );
 
         /**
          * If the operator has NEVER registered a pubkey before, use `params` to register
@@ -114,45 +119,31 @@ contract RegistryCoordinator is SlashingRegistryCoordinator, IRegistryCoordinato
          */
         bytes32 operatorId = _getOrCreateOperatorId(msg.sender, params);
 
-        // Verify the churn approver's signature for the registering operator and kick params
-        _verifyChurnApproverSignature({
-            registeringOperator: msg.sender,
-            registeringOperatorId: operatorId,
-            operatorKickParams: operatorKickParams,
-            churnApproverSignature: churnApproverSignature
-        });
-
-        // Register the operator in each of the registry contracts and update the operator's
-        // quorum bitmap and registration status
-        RegisterResults memory results = _registerOperator({
+        _registerOperatorWithChurn({
             operator: msg.sender,
             operatorId: operatorId,
             quorumNumbers: quorumNumbers,
             socket: socket,
-            operatorSignature: operatorSignature
+            operatorKickParams: operatorKickParams,
+            churnApproverSignature: churnApproverSignature
         });
 
-        // Check that each quorum's operator count is below the configured maximum. If the max
-        // is exceeded, use `operatorKickParams` to deregister an existing operator to make space
-        for (uint256 i = 0; i < quorumNumbers.length; i++) {
-            OperatorSetParam memory operatorSetParams = _quorumParams[uint8(quorumNumbers[i])];
+        // If the operator wasn't registered for any quorums, update their status
+        // and register them with this AVS in EigenLayer core (DelegationManager)
+        if (_operatorInfo[msg.sender].status != OperatorStatus.REGISTERED) {
+            _operatorInfo[msg.sender] =
+                OperatorInfo({operatorId: operatorId, status: OperatorStatus.REGISTERED});
 
-            /**
-             * If the new operator count for any quorum exceeds the maximum, validate
-             * that churn can be performed, then deregister the specified operator
-             */
-            if (results.numOperatorsPerQuorum[i] > operatorSetParams.maxOperatorCount) {
-                _validateChurn({
-                    quorumNumber: uint8(quorumNumbers[i]),
-                    totalQuorumStake: results.totalStakes[i],
-                    newOperator: msg.sender,
-                    newOperatorStake: results.operatorStakes[i],
-                    kickParams: operatorKickParams[i],
-                    setParams: operatorSetParams
-                });
+            serviceManager.registerOperatorToAVS(msg.sender, operatorSignature);
+            emit OperatorRegistered(msg.sender, operatorId);
+        }
 
-                _deregisterOperator(operatorKickParams[i].operator, quorumNumbers[i:i + 1]);
-            }
+        // If the operator kicked is not registered for any quorums, update their status
+        // and deregister them from the AVS via the EigenLayer core contracts
+        if (_operatorInfo[operatorKickParams[0].operator].status != OperatorStatus.REGISTERED) {
+            _operatorInfo[operatorKickParams[0].operator].status = OperatorStatus.DEREGISTERED;
+            serviceManager.deregisterOperatorFromAVS(operatorKickParams[0].operator);
+            emit OperatorDeregistered(operatorKickParams[0].operator, operatorId);
         }
     }
 
@@ -181,70 +172,6 @@ contract RegistryCoordinator is SlashingRegistryCoordinator, IRegistryCoordinato
         isOperatorSetAVS = true;
     }
 
-    /**
-     *
-     *                         INTERNAL FUNCTIONS
-     *
-     */
-
-    /**
-     * @notice Register the operator for one or more quorums. This method updates the
-     * operator's quorum bitmap, socket, and status, then registers them with each registry.
-     */
-    function _registerOperator(
-        address operator,
-        bytes32 operatorId,
-        bytes memory quorumNumbers,
-        string memory socket,
-        SignatureWithSaltAndExpiry memory operatorSignature
-    ) internal virtual returns (RegisterResults memory results) {
-        /**
-         * Get bitmap of quorums to register for and operator's current bitmap. Validate that:
-         * - we're trying to register for at least 1 quorum
-         * - the quorums we're registering for exist (checked against `quorumCount` in orderedBytesArrayToBitmap)
-         * - the operator is not currently registered for any quorums we're registering for
-         * Then, calculate the operator's new bitmap after registration
-         */
-        uint192 quorumsToAdd =
-            uint192(BitmapUtils.orderedBytesArrayToBitmap(quorumNumbers, quorumCount));
-        uint192 currentBitmap = _currentOperatorBitmap(operatorId);
-        require(!quorumsToAdd.isEmpty(), BitmapEmpty());
-        require(quorumsToAdd.noBitsInCommon(currentBitmap), AlreadyRegisteredForQuorums());
-        uint192 newBitmap = uint192(currentBitmap.plus(quorumsToAdd));
-
-        // Check that the operator can reregister if ejected
-        require(
-            lastEjectionTimestamp[operator] + ejectionCooldown < block.timestamp,
-            CannotReregisterYet()
-        );
-
-        /**
-         * Update operator's bitmap, socket, and status. Only update operatorInfo if needed:
-         * if we're `REGISTERED`, the operatorId and status are already correct.
-         */
-        _updateOperatorBitmap({operatorId: operatorId, newBitmap: newBitmap});
-
-        emit OperatorSocketUpdate(operatorId, socket);
-
-        // If the operator wasn't registered for any quorums, update their status
-        // and register them with this AVS in EigenLayer core (DelegationManager)
-        if (_operatorInfo[operator].status != OperatorStatus.REGISTERED) {
-            _operatorInfo[operator] =
-                OperatorInfo({operatorId: operatorId, status: OperatorStatus.REGISTERED});
-
-            serviceManager.registerOperatorToAVS(operator, operatorSignature);
-            emit OperatorRegistered(operator, operatorId);
-        }
-
-        // Register the operator with the BLSApkRegistry, StakeRegistry, and IndexRegistry
-        blsApkRegistry.registerOperator(operator, quorumNumbers);
-        (results.operatorStakes, results.totalStakes) =
-            stakeRegistry.registerOperator(operator, operatorId, quorumNumbers);
-        results.numOperatorsPerQuorum = indexRegistry.registerOperator(operatorId, quorumNumbers);
-
-        return results;
-    }
-
     /// @dev Hook to allow for any post-deregister logic
     function _afterDeregisterOperator(address operator, bytes32 operatorId, bytes memory quorumNumbers, uint192 newBitmap) internal virtual override {
         // If the operator is no longer registered for any quorums, update their status and deregister
diff --git a/src/SlashingRegistryCoordinator.sol b/src/SlashingRegistryCoordinator.sol
@@ -144,26 +144,47 @@ contract SlashingRegistryCoordinator is
         bytes calldata data
     ) external override onlyAllocationManager onlyWhenNotPaused(PAUSED_REGISTER_OPERATOR) {
         require(isOperatorSetAVS, OperatorSetsNotEnabled());
-        for (uint256 i = 0; i < operatorSetIds.length; i++) {
-            require(!isM2Quorum[uint8(operatorSetIds[i])], OperatorSetsNotSupported());
-        }
-        bytes memory quorumNumbers = new bytes(operatorSetIds.length);
-        for (uint256 i = 0; i < operatorSetIds.length; i++) {
-            quorumNumbers[i] = bytes1(uint8(operatorSetIds[i]));
-        }
+        bytes memory quorumNumbers = _getQuorumNumbers(operatorSetIds);
+
+        (
+            RegistrationType registrationType,
+            string memory socket,
+            IBLSApkRegistry.PubkeyRegistrationParams memory params
+        ) = abi.decode(data, (RegistrationType, string, IBLSApkRegistry.PubkeyRegistrationParams));
+
+        /**
+         * If the operator has NEVER registered a pubkey before, use `params` to register
+         * their pubkey in blsApkRegistry
+         *
+         * If the operator HAS registered a pubkey, `params` is ignored and the pubkey hash
+         * (operatorId) is fetched instead
+         */
+        bytes32 operatorId = _getOrCreateOperatorId(operator, params);
 
-        // Handle churn or normal registration based on first byte in `data`
-        RegistrationType registrationType = RegistrationType(uint8(bytes1(data[0:1])));
         if (registrationType == RegistrationType.NORMAL) {
-            (, string memory socket, IBLSApkRegistry.PubkeyRegistrationParams memory params) = abi.decode(data, (RegistrationType, string, IBLSApkRegistry.PubkeyRegistrationParams));
-            bytes32 operatorId = _getOrCreateOperatorId(operator, params);
-            _registerOperatorToOperatorSet(operator, operatorId, quorumNumbers, socket);
+            uint32[] memory numOperatorsPerQuorum = _registerOperator({
+                operator: operator,
+                operatorId: operatorId,
+                quorumNumbers: quorumNumbers,
+                socket: socket
+            }).numOperatorsPerQuorum;
+
+            // For each quorum, validate that the new operator count does not exceed the maximum
+            // (If it does, an operator needs to be replaced -- see `registerOperatorWithChurn`)
+            for (uint256 i = 0; i < quorumNumbers.length; i++) {
+                uint8 quorumNumber = uint8(quorumNumbers[i]);
+
+                require(
+                    numOperatorsPerQuorum[i] <= _quorumParams[quorumNumber].maxOperatorCount,
+                    MaxQuorumsReached()
+                );
+            }
         } else if (registrationType == RegistrationType.CHURN) {
             // Decode registration data from bytes
             (
                 ,
-                string memory socket,
-                IBLSApkRegistry.PubkeyRegistrationParams memory params,
+                ,
+                ,
                 OperatorKickParam[] memory operatorKickParams,
                 SignatureWithSaltAndExpiry memory churnApproverSignature
             ) = abi.decode(
@@ -176,26 +197,31 @@ contract SlashingRegistryCoordinator is
                     SignatureWithSaltAndExpiry
                 )
             );
-
-            _registerOperatorWithChurn(operator, quorumNumbers, socket, params, operatorKickParams, churnApproverSignature);
+            _registerOperatorWithChurn({
+                operator: operator,
+                operatorId: operatorId,
+                quorumNumbers: quorumNumbers,
+                socket: socket,
+                operatorKickParams: operatorKickParams,
+                churnApproverSignature: churnApproverSignature
+            });
         } else {
             revert InvalidRegistrationType();
         }
+
+        // If the operator wasn't registered for any quorums, update their status
+        // and register them with this AVS in EigenLayer core (DelegationManager)
+        if (_operatorInfo[operator].status != OperatorStatus.REGISTERED) {
+            _operatorInfo[operator] = OperatorInfo(operatorId, OperatorStatus.REGISTERED);
+        }
     }
 
     function deregisterOperator(
         address operator,
         uint32[] memory operatorSetIds
-    ) external override onlyAllocationManager onlyWhenNotPaused(PAUSED_REGISTER_OPERATOR) {
+    ) external override onlyAllocationManager onlyWhenNotPaused(PAUSED_DEREGISTER_OPERATOR) {
         require(isOperatorSetAVS, OperatorSetsNotEnabled());
-        for (uint256 i = 0; i < operatorSetIds.length; i++) {
-            require(!isM2Quorum[uint8(operatorSetIds[i])], OperatorSetsNotSupported());
-        }
-        bytes memory quorumNumbers = new bytes(operatorSetIds.length);
-        for (uint256 i = 0; i < operatorSetIds.length; i++) {
-            quorumNumbers[i] = bytes1(uint8(operatorSetIds[i]));
-        }
-
+        bytes memory quorumNumbers = _getQuorumNumbers(operatorSetIds);
         _deregisterOperator(operator, quorumNumbers);
     }
 
@@ -408,7 +434,7 @@ contract SlashingRegistryCoordinator is
      * @notice Register the operator for one or more quorums. This method updates the
      * operator's quorum bitmap, socket, and status, then registers them with each registry.
      */
-    function _registerOperatorToOperatorSet(
+    function _registerOperator(
         address operator,
         bytes32 operatorId,
         bytes memory quorumNumbers,
@@ -451,12 +477,6 @@ contract SlashingRegistryCoordinator is
 
         emit OperatorSocketUpdate(operatorId, socket);
 
-        // If the operator wasn't registered for any quorums, update their status
-        // and register them with this AVS in EigenLayer core (DelegationManager)
-        if (_operatorInfo[operator].status != OperatorStatus.REGISTERED) {
-            _operatorInfo[operator] = OperatorInfo(operatorId, OperatorStatus.REGISTERED);
-        }
-
         // Register the operator with the BLSApkRegistry, StakeRegistry, and IndexRegistry
         blsApkRegistry.registerOperator(operator, quorumNumbers);
         (results.operatorStakes, results.totalStakes) =
@@ -471,23 +491,14 @@ contract SlashingRegistryCoordinator is
 
     function _registerOperatorWithChurn(
         address operator,
+        bytes32 operatorId,
         bytes memory quorumNumbers,
         string memory socket,
-        IBLSApkRegistry.PubkeyRegistrationParams memory params,
         OperatorKickParam[] memory operatorKickParams,
         SignatureWithSaltAndExpiry memory churnApproverSignature
     ) internal virtual {
         require(operatorKickParams.length == quorumNumbers.length, InputLengthMismatch());
 
-        /**
-         * If the operator has NEVER registered a pubkey before, use `params` to register
-         * their pubkey in blsApkRegistry
-         *
-         * If the operator HAS registered a pubkey, `params` is ignored and the pubkey hash
-         * (operatorId) is fetched instead
-         */
-        bytes32 operatorId = _getOrCreateOperatorId(operator, params);
-
         // Verify the churn approver's signature for the registering operator and kick params
         _verifyChurnApproverSignature({
             registeringOperator: operator,
@@ -498,7 +509,7 @@ contract SlashingRegistryCoordinator is
 
         // Register the operator in each of the registry contracts and update the operator's
         // quorum bitmap and registration status
-        RegisterResults memory results = _registerOperatorToOperatorSet(operator, operatorId, quorumNumbers, socket);
+        RegisterResults memory results = _registerOperator(operator, operatorId, quorumNumbers, socket);
 
         // Check that each quorum's operator count is below the configured maximum. If the max
         // is exceeded, use `operatorKickParams` to deregister an existing operator to make space
@@ -878,6 +889,16 @@ contract SlashingRegistryCoordinator is
         return QuorumBitmapHistoryLib.getQuorumBitmapIndexAtBlockNumber(_operatorBitmapHistory,blockNumber, operatorId);
     }
 
+    /// @notice Returns the quorum numbers for the provided `OperatorSetIds`
+    /// OperatorSetIds are used in the AllocationManager to identify operator sets for a given AVS
+    function _getQuorumNumbers(uint32[] memory operatorSetIds) internal pure returns (bytes memory) {
+        bytes memory quorumNumbers = new bytes(operatorSetIds.length);
+        for (uint256 i = 0; i < operatorSetIds.length; i++) {
+            quorumNumbers[i] = bytes1(uint8(operatorSetIds[i]));
+        }
+        return quorumNumbers;
+    }
+
     function _setOperatorSetParams(
         uint8 quorumNumber,
         OperatorSetParam memory operatorSetParams
@@ -900,10 +921,10 @@ contract SlashingRegistryCoordinator is
         accountIdentifier = _accountIdentifier;
     }
 
-    /// @dev Hook to allow for any pre-register logic in `_registerOperatorToOperatorSet`
+    /// @dev Hook to allow for any pre-register logic in `_registerOperator`
     function _beforeRegisterOperator(address operator, bytes32 operatorId, bytes memory quorumNumbers, uint192 currentBitmap) internal virtual {}
 
-    /// @dev Hook to allow for any post-register logic in `_registerOperatorToOperatorSet`
+    /// @dev Hook to allow for any post-register logic in `_registerOperator`
     function _afterRegisterOperator(address operator, bytes32 operatorId, bytes memory quorumNumbers, uint192 newBitmap) internal virtual {}
 
     /// @dev Hook to allow for any pre-deregister logic in `_deregisterOperator`
diff --git a/test/harnesses/RegistryCoordinatorHarness.t.sol b/test/harnesses/RegistryCoordinatorHarness.t.sol
@@ -45,7 +45,7 @@ contract RegistryCoordinatorHarness is RegistryCoordinator, Test {
         string memory socket,
         SignatureWithSaltAndExpiry memory operatorSignature
     ) external returns (RegisterResults memory results) {
-        return _registerOperator(operator, operatorId, quorumNumbers, socket, operatorSignature);
+        return _registerOperator(operator, operatorId, quorumNumbers, socket);
     }
 
     // @notice exposes the internal `_deregisterOperator` function, overriding all access controls
diff --git a/test/unit/RegistryCoordinatorUnit.t.sol b/test/unit/RegistryCoordinatorUnit.t.sol

Original file line number	Diff line number	Diff line change
`@@ -45,7 +45,7 @@ contract RegistryCoordinatorHarness is RegistryCoordinator, Test {`
`45`	`45`	`string memory socket,`
`46`	`46`	`SignatureWithSaltAndExpiry memory operatorSignature`
`47`	`47`	`) external returns (RegisterResults memory results) {`
`48`		`- return _registerOperator(operator, operatorId, quorumNumbers, socket, operatorSignature);`
	`48`	`+ return _registerOperator(operator, operatorId, quorumNumbers, socket);`
`49`	`49`	`}`
`50`	`50`
`51`	`51`	// @notice exposes the internal `_deregisterOperator` function, overriding all access controls