chore: KeyRegistrar clarifications (#1551)

**Motivation:**

Docs were out of date in some places. Redundant functions were present

**Modifications:**

- Clarify docs/natspec
- Remove `checkKey` in favor of `isKeyRegistered`
- Remove old test file
- Add sequence diagrams 
- Add documentation on errors/events 

**Result:**

Cleaner code

Author: ypatil12

docs/permissions/KeyRegistrar.md

@@ -14,6 +14,8 @@ Keys are stored in a 2-way mapping:
 1. (operator, operatorSet) to key
 2. keyHash to operator address
 
+See [usage patterns](#usage-patterns) below on user flows. 
+
 ---
 
 ## Operator Set Configuration
@@ -59,10 +61,12 @@ Key registration is segmented by curve type: ECDSA and BN254.
  * @notice Registers a cryptographic key for an operator with a specific operator set
  * @param operator Address of the operator to register key for
  * @param operatorSet The operator set to register the key for
- * @param pubkey Public key bytes
- * @param signature Signature proving ownership (only needed for BN254 keys)
+ * @param pubkey Public key bytes. For ECDSA, this is the address of the key. For BN254, this is the G1 and G2 key combined (see `encodeBN254KeyData`)
+ * @param signature Signature proving ownership. For ECDSA this is a signature of the `getECDSAKeyRegistrationMessageHash`. For BN254 this is a signature of the `getBN254KeyRegistrationMessageHash`.
  * @dev Can be called by operator directly or by addresses they've authorized via PermissionController
  * @dev Reverts if key is already registered
+ * @dev There exist no restriction on the state of the operator with respect to the operatorSet. That is, an operator
+ *      does not have to be registered for the operator in the `AllocationManager` to register a key for it
  */
 function registerKey(
     address operator,
@@ -72,11 +76,11 @@ function registerKey(
 ) external;
 ```
 
-There are restrictions on the state of the operator with respect to the operatorSet. That is, an operator does not have to be registered for the operator in the `AllocationManager` to register a key for it. 
+There ARE NO restrictions on the state of the operator with respect to the operatorSet. That is, an operator does not have to be registered for the operator in the `AllocationManager` to register a key for it. 
 
 For ECDSA keys:
 - `pubkey`: 20 bytes representing the Ethereum address
-- `signature`: EIP-712 signature from the key's private key
+- `signature`: EIP-712 signature from the key's private key. Signature is over [`getECDSAKeyRegistrationMessageHash`](#getecdsakeyregistrationmessagehash)
 
 *Effects*:
 * Registers the key for the operator in the specified operator set
@@ -201,7 +205,7 @@ Returns the message hash that must be signed over for BN254 key registration.
  * @notice Deregisters a cryptographic key for an operator with a specific operator set
  * @param operator Address of the operator to deregister key for
  * @param operatorSet The operator set to deregister the key from
- * @dev Can be called by avs directly or by addresses they've authorized via PermissionController
+ * @dev Can be called by operator directly or by addresses they've authorized via PermissionController
  * @dev Reverts if key was not registered
  * @dev Keys remain in global key registry to prevent reuse
  */
@@ -216,9 +220,46 @@ Removes an operator's key from the specified operator set. Note that the key rem
 * The key remains in the global registry
 
 *Requirements*:
-* Caller MUST be authorized for the AVS (via PermissionController)
+* Caller MUST be authorized for the operator (via PermissionController)
 * The operator MUST not be slashable for the AVS, to prevent off-chain race conditions
 * The operator set MUST be configured
 * The operator MUST have a registered key for this operator set
 
 ---
+
+## Usage patterns
+The `KeyRegistrar` introduces new operator/avs registration patterns. Note that these registration patterns are **only for AVSs who have opted to use the new [middlewareV2](https://github.com/Layr-Labs/eigenlayer-middleware/tree/dev/docs/middlewareV2) contract architecture.**
+
+### Operator/AVS Registration
+
+```mermaid
+sequenceDiagram
+    participant OP as Operator
+    participant KR as KeyRegistrar
+    participant AM as AllocationManager
+    participant AVR as AVSRegistrar
+
+    OP->>KR: Tx1: registerKey
+    OP->>AM: Tx2: registerForOperatorSets
+    AM-->>AVR: registerForOperatorSets
+    AVR-->>KR: isRegistered
+```
+
+
+### Deregistration/Key Rotation
+
+Deregistration takes a dependency on the `AllocationManager`. In particular, operators are only allowed to deregister their key from an operatorSet if they are not slashable by said operatorSet. 
+
+To rotate a key, an operator must deregister from the operatorSet, wait until it is not slashable, deregister its key, and then register a new key. If the operator was not slashable, it can rotate its key without a delay. 
+
+```mermaid
+sequenceDiagram
+    participant OP as Operator
+    participant AM as AllocationManager
+    participant KR as KeyRegistrar
+
+    OP->>AM: Tx1: deregisterFromOperatorSets
+    Note over OP: Wait 14 days<br>(if previously allocated)
+    OP->>KR: Tx2: deregisterKey
+    OP->>KR: Tx3: registerKey
+```

src/contracts/interfaces/IKeyRegistrar.sol

@@ -6,16 +6,25 @@ import {BN254} from "../libraries/BN254.sol";
 import "./ISemVerMixin.sol";
 
 interface IKeyRegistrarErrors {
-    /// Key Management
+    /// @notice Error thrown when a key is already registered
     error KeyAlreadyRegistered();
+    /// @notice Error thrown when the key format is invalid
     error InvalidKeyFormat();
+    /// @notice Error thrown when the address is zero
     error ZeroAddress();
+    /// @notice Error thrown when the public key is zero
     error ZeroPubkey();
+    /// @notice Error thrown when the curve type is invalid
     error InvalidCurveType();
+    /// @notice Error thrown when the keypair is invalid
     error InvalidKeypair();
+    /// @notice Error thrown when the configuration is already set
     error ConfigurationAlreadySet();
+    /// @notice Error thrown when the operator set is not configured
     error OperatorSetNotConfigured();
+    /// @notice Error thrown when the key is not found
     error KeyNotFound(OperatorSet operatorSet, address operator);
+    /// @notice Error thrown when the operator is still slashable when trying to deregister a key
     error OperatorStillSlashable(OperatorSet operatorSet, address operator);
 }
 
@@ -35,9 +44,13 @@ interface IKeyRegistrarTypes {
 }
 
 interface IKeyRegistrarEvents is IKeyRegistrarTypes {
+    /// @notice Emitted when a key is registered
     event KeyRegistered(OperatorSet operatorSet, address indexed operator, CurveType curveType, bytes pubkey);
+    /// @notice Emitted when a key is deregistered
     event KeyDeregistered(OperatorSet operatorSet, address indexed operator, CurveType curveType);
+    /// @notice Emitted when the aggregate BN254 key is updated
     event AggregateBN254KeyUpdated(OperatorSet operatorSet, BN254.G1Point newAggregateKey);
+    /// @notice Emitted when an operator set is configured
     event OperatorSetConfigured(OperatorSet operatorSet, CurveType curveType);
 }
 
@@ -54,8 +67,8 @@ interface IKeyRegistrar is IKeyRegistrarErrors, IKeyRegistrarEvents, ISemVerMixi
      * @notice Registers a cryptographic key for an operator with a specific operator set
      * @param operator Address of the operator to register key for
      * @param operatorSet The operator set to register the key for
-     * @param pubkey Public key bytes
-     * @param signature Signature proving ownership (only needed for BN254 keys)
+     * @param pubkey Public key bytes. For ECDSA, this is the address of the key. For BN254, this is the G1 and G2 key combined (see `encodeBN254KeyData`)
+     * @param signature Signature proving ownership. For ECDSA this is a signature of the `getECDSAKeyRegistrationMessageHash`. For BN254 this is a signature of the `getBN254KeyRegistrationMessageHash`.
      * @dev Can be called by operator directly or by addresses they've authorized via PermissionController
      * @dev Reverts if key is already registered
      * @dev There exist no restriction on the state of the operator with respect to the operatorSet. That is, an operator
@@ -79,29 +92,19 @@ interface IKeyRegistrar is IKeyRegistrarErrors, IKeyRegistrarEvents, ISemVerMixi
      */
     function deregisterKey(address operator, OperatorSet memory operatorSet) external;
 
-    /**
-     * @notice Checks if an operator has a registered key
-     * @param operatorSet The operator set to check and update
-     * @param operator Address of the operator
-     * @return whether the operator has a registered key
-     * @dev This function is called by the AVSRegistrar when an operator registers for an AVS
-     * @dev Only authorized callers for the AVS can call this function
-     * @dev Reverts if operator doesn't have a registered key for this operator set
-     */
-    function checkKey(OperatorSet memory operatorSet, address operator) external view returns (bool);
-
     /**
      * @notice Checks if a key is registered for an operator with a specific operator set
      * @param operatorSet The operator set to check
      * @param operator Address of the operator
-     * @return True if the key is registered
+     * @return True if the key is registered, false otherwise
+     * @dev If the operatorSet is not configured, this function will return false
      */
     function isRegistered(OperatorSet memory operatorSet, address operator) external view returns (bool);
 
     /**
-     * @notice Gets the configuration for an operator set
-     * @param operatorSet The operator set to get configuration for
-     * @return The operator set configuration
+     * @notice Gets the curve type for an operator set
+     * @param operatorSet The operator set to get the curve type for
+     * @return The curve type, either ECDSA, BN254, or NONE
      */
     function getOperatorSetCurveType(
         OperatorSet memory operatorSet
@@ -113,6 +116,7 @@ interface IKeyRegistrar is IKeyRegistrarErrors, IKeyRegistrarEvents, ISemVerMixi
      * @param operator Address of the operator
      * @return g1Point The BN254 G1 public key
      * @return g2Point The BN254 G2 public key
+     * @dev Reverts if the operatorSet is not configured for BN254
      */
     function getBN254Key(
         OperatorSet memory operatorSet,
@@ -123,15 +127,17 @@ interface IKeyRegistrar is IKeyRegistrarErrors, IKeyRegistrarEvents, ISemVerMixi
      * @notice Gets the ECDSA public key for an operator with a specific operator set as bytes
      * @param operatorSet The operator set to get the key for
      * @param operator Address of the operator
-     * @return pubkey The ECDSA public key
+     * @return pubkey The ECDSA public key in bytes format
+     * @dev Reverts if the operatorSet is not configured for ECDSA
      */
     function getECDSAKey(OperatorSet memory operatorSet, address operator) external view returns (bytes memory);
 
     /**
      * @notice Gets the ECDSA public key for an operator with a specific operator set
      * @param operatorSet The operator set to get the key for
      * @param operator Address of the operator
-     * @return pubkey The ECDSA public key
+     * @return pubkey The ECDSA public key in address format
+     * @dev Reverts if the operatorSet is not configured for ECDSA
      */
     function getECDSAAddress(OperatorSet memory operatorSet, address operator) external view returns (address);
 
@@ -149,6 +155,7 @@ interface IKeyRegistrar is IKeyRegistrarErrors, IKeyRegistrarEvents, ISemVerMixi
      * @param operatorSet The operator set to get the key hash for
      * @param operator Address of the operator
      * @return keyHash The key hash
+     * @dev Reverts if the operatorSet is not configured
      */
     function getKeyHash(OperatorSet memory operatorSet, address operator) external view returns (bytes32);
 

src/contracts/permissions/KeyRegistrar.sol

@@ -246,15 +246,6 @@ contract KeyRegistrar is KeyRegistrarStorage, PermissionControllerMixin, Signatu
      *
      */
 
-    /// @inheritdoc IKeyRegistrar
-    function checkKey(OperatorSet memory operatorSet, address operator) external view returns (bool) {
-        CurveType curveType = _operatorSetCurveTypes[operatorSet.key()];
-        require(curveType != CurveType.NONE, OperatorSetNotConfigured());
-
-        KeyInfo memory keyInfo = _operatorKeyInfo[operatorSet.key()][operator];
-        return keyInfo.isRegistered;
-    }
-
     /// @inheritdoc IKeyRegistrar
     function isRegistered(OperatorSet memory operatorSet, address operator) public view returns (bool) {
         return _operatorKeyInfo[operatorSet.key()][operator].isRegistered;

src/test/unit/KeyRegistrarUnit.t.sol

@@ -674,59 +674,6 @@ contract KeyRegistrarUnitTests_deregisterKey is KeyRegistrarUnitTests {
     }
 }
 
-/**
- * @title KeyRegistrarUnitTests_checkKey
- * @notice Unit tests for KeyRegistrar.checkKey
- */
-contract KeyRegistrarUnitTests_checkKey is KeyRegistrarUnitTests {
-    function test_checkKey_ECDSA() public {
-        OperatorSet memory operatorSet = _createOperatorSet(avs1, DEFAULT_OPERATOR_SET_ID);
-
-        vm.prank(avs1);
-        keyRegistrar.configureOperatorSet(operatorSet, CurveType.ECDSA);
-
-        bytes memory signature = _generateECDSASignature(operator1, operatorSet, ecdsaAddress1, ecdsaPrivKey1);
-
-        vm.prank(operator1);
-        keyRegistrar.registerKey(operator1, operatorSet, ecdsaKey1, signature);
-
-        bool hasKey = keyRegistrar.checkKey(operatorSet, operator1);
-        assertTrue(hasKey);
-    }
-
-    function test_checkKey_BN254() public {
-        OperatorSet memory operatorSet = _createOperatorSet(avs1, DEFAULT_OPERATOR_SET_ID);
-
-        vm.prank(avs1);
-        keyRegistrar.configureOperatorSet(operatorSet, CurveType.BN254);
-
-        bytes memory signature = _generateBN254Signature(operator1, operatorSet, bn254Key1, bn254PrivKey1);
-
-        vm.prank(operator1);
-        keyRegistrar.registerKey(operator1, operatorSet, bn254Key1, signature);
-
-        bool hasKey = keyRegistrar.checkKey(operatorSet, operator1);
-        assertTrue(hasKey);
-    }
-
-    function test_checkKey_notRegistered() public {
-        OperatorSet memory operatorSet = _createOperatorSet(avs1, DEFAULT_OPERATOR_SET_ID);
-
-        vm.prank(avs1);
-        keyRegistrar.configureOperatorSet(operatorSet, CurveType.ECDSA);
-
-        bool hasKey = keyRegistrar.checkKey(operatorSet, operator1);
-        assertFalse(hasKey);
-    }
-
-    function test_revert_operatorSetNotConfigured() public {
-        OperatorSet memory operatorSet = _createOperatorSet(avs1, DEFAULT_OPERATOR_SET_ID);
-
-        vm.expectRevert(OperatorSetNotConfigured.selector);
-        keyRegistrar.checkKey(operatorSet, operator1);
-    }
-}
-
 /**
  * @title KeyRegistrarUnitTests_ViewFunctions
  * @notice Unit tests for KeyRegistrar view functions