refactor: KeyRegistry unit testing (#1482)

**Motivation:**

Update `KeyRegistrar` unit tests

**Modifications:**

- Update bindings
- Add .tree file
- Add `_` to internal storage vars

**Result:**

Updated tests. Full coverage (aside from unreachable else statements)

Author: ypatil12

pkg/bindings/BN254TableCalculator/binding.go

@@ -55,10 +55,10 @@ contract KeyRegistrar is KeyRegistrarStorage, PermissionControllerMixin, Signatu
         require(curveType == CurveType.ECDSA || curveType == CurveType.BN254, InvalidCurveType());
 
         // Prevent overwriting existing configurations
-        CurveType _curveType = operatorSetCurveTypes[operatorSet.key()];
+        CurveType _curveType = _operatorSetCurveTypes[operatorSet.key()];
         require(_curveType == CurveType.NONE, ConfigurationAlreadySet());
 
-        operatorSetCurveTypes[operatorSet.key()] = curveType;
+        _operatorSetCurveTypes[operatorSet.key()] = curveType;
 
         emit OperatorSetConfigured(operatorSet, curveType);
     }
@@ -70,11 +70,11 @@ contract KeyRegistrar is KeyRegistrarStorage, PermissionControllerMixin, Signatu
         bytes calldata keyData,
         bytes calldata signature
     ) external checkCanCall(operator) {
-        CurveType curveType = operatorSetCurveTypes[operatorSet.key()];
+        CurveType curveType = _operatorSetCurveTypes[operatorSet.key()];
         require(curveType != CurveType.NONE, OperatorSetNotConfigured());
 
         // Check if the key is already registered
-        require(!operatorKeyInfo[operatorSet.key()][operator].isRegistered, KeyAlreadyRegistered());
+        require(!_operatorKeyInfo[operatorSet.key()][operator].isRegistered, KeyAlreadyRegistered());
 
         // Register key based on curve type - both now require signature verification
         if (curveType == CurveType.ECDSA) {
@@ -95,25 +95,25 @@ contract KeyRegistrar is KeyRegistrarStorage, PermissionControllerMixin, Signatu
             !allocationManager.isOperatorSlashable(operator, operatorSet), OperatorStillSlashable(operatorSet, operator)
         );
 
-        CurveType curveType = operatorSetCurveTypes[operatorSet.key()];
+        CurveType curveType = _operatorSetCurveTypes[operatorSet.key()];
         require(curveType != CurveType.NONE, OperatorSetNotConfigured());
 
-        KeyInfo memory keyInfo = operatorKeyInfo[operatorSet.key()][operator];
+        KeyInfo memory keyInfo = _operatorKeyInfo[operatorSet.key()][operator];
 
         require(keyInfo.isRegistered, KeyNotFound(operatorSet, operator));
 
         // Clear key info
-        delete operatorKeyInfo[operatorSet.key()][operator];
+        delete _operatorKeyInfo[operatorSet.key()][operator];
 
         emit KeyDeregistered(operatorSet, operator, curveType);
     }
 
     /// @inheritdoc IKeyRegistrar
     function checkKey(OperatorSet memory operatorSet, address operator) external view returns (bool) {
-        CurveType curveType = operatorSetCurveTypes[operatorSet.key()];
+        CurveType curveType = _operatorSetCurveTypes[operatorSet.key()];
         require(curveType != CurveType.NONE, OperatorSetNotConfigured());
 
-        KeyInfo memory keyInfo = operatorKeyInfo[operatorSet.key()][operator];
+        KeyInfo memory keyInfo = _operatorKeyInfo[operatorSet.key()][operator];
         return keyInfo.isRegistered;
     }
 
@@ -148,7 +148,7 @@ contract KeyRegistrar is KeyRegistrarStorage, PermissionControllerMixin, Signatu
         bytes32 keyHash = _getKeyHashForKeyData(keyData, CurveType.ECDSA);
 
         // Check global uniqueness
-        require(!globalKeyRegistry[keyHash], KeyAlreadyRegistered());
+        require(!_globalKeyRegistry[keyHash], KeyAlreadyRegistered());
 
         // Get the signable digest for the ECDSA key registration message
         bytes32 signableDigest = getECDSAKeyRegistrationMessageHash(operator, operatorSet, keyAddress);
@@ -203,7 +203,7 @@ contract KeyRegistrar is KeyRegistrarStorage, PermissionControllerMixin, Signatu
 
         // Calculate key hash and check global uniqueness
         bytes32 keyHash = _getKeyHashForKeyData(keyData, CurveType.BN254);
-        require(!globalKeyRegistry[keyHash], KeyAlreadyRegistered());
+        require(!_globalKeyRegistry[keyHash], KeyAlreadyRegistered());
 
         // Store key data
         _storeKeyData(operatorSet, operator, keyData, keyHash);
@@ -223,10 +223,10 @@ contract KeyRegistrar is KeyRegistrarStorage, PermissionControllerMixin, Signatu
         bytes32 keyHash
     ) internal {
         // Store key data
-        operatorKeyInfo[operatorSet.key()][operator] = KeyInfo({isRegistered: true, keyData: pubkey});
+        _operatorKeyInfo[operatorSet.key()][operator] = KeyInfo({isRegistered: true, keyData: pubkey});
 
         // Update global key registry
-        globalKeyRegistry[keyHash] = true;
+        _globalKeyRegistry[keyHash] = true;
     }
 
     /**
@@ -254,14 +254,14 @@ contract KeyRegistrar is KeyRegistrarStorage, PermissionControllerMixin, Signatu
 
     /// @inheritdoc IKeyRegistrar
     function isRegistered(OperatorSet memory operatorSet, address operator) external view returns (bool) {
-        return operatorKeyInfo[operatorSet.key()][operator].isRegistered;
+        return _operatorKeyInfo[operatorSet.key()][operator].isRegistered;
     }
 
     /// @inheritdoc IKeyRegistrar
     function getOperatorSetCurveType(
         OperatorSet memory operatorSet
     ) external view returns (CurveType) {
-        return operatorSetCurveTypes[operatorSet.key()];
+        return _operatorSetCurveTypes[operatorSet.key()];
     }
 
     /// @inheritdoc IKeyRegistrar
@@ -270,10 +270,10 @@ contract KeyRegistrar is KeyRegistrarStorage, PermissionControllerMixin, Signatu
         address operator
     ) external view returns (BN254.G1Point memory g1Point, BN254.G2Point memory g2Point) {
         // Validate operator set curve type
-        CurveType curveType = operatorSetCurveTypes[operatorSet.key()];
+        CurveType curveType = _operatorSetCurveTypes[operatorSet.key()];
         require(curveType == CurveType.BN254, InvalidCurveType());
 
-        KeyInfo memory keyInfo = operatorKeyInfo[operatorSet.key()][operator];
+        KeyInfo memory keyInfo = _operatorKeyInfo[operatorSet.key()][operator];
 
         if (!keyInfo.isRegistered) {
             // Create default values for an empty key
@@ -290,10 +290,10 @@ contract KeyRegistrar is KeyRegistrarStorage, PermissionControllerMixin, Signatu
     /// @inheritdoc IKeyRegistrar
     function getECDSAKey(OperatorSet memory operatorSet, address operator) public view returns (bytes memory) {
         // Validate operator set curve type
-        CurveType curveType = operatorSetCurveTypes[operatorSet.key()];
+        CurveType curveType = _operatorSetCurveTypes[operatorSet.key()];
         require(curveType == CurveType.ECDSA, InvalidCurveType());
 
-        KeyInfo memory keyInfo = operatorKeyInfo[operatorSet.key()][operator];
+        KeyInfo memory keyInfo = _operatorKeyInfo[operatorSet.key()][operator];
         return keyInfo.keyData; // Returns the 20-byte address as bytes
     }
 
@@ -306,13 +306,13 @@ contract KeyRegistrar is KeyRegistrarStorage, PermissionControllerMixin, Signatu
     function isKeyGloballyRegistered(
         bytes32 keyHash
     ) external view returns (bool) {
-        return globalKeyRegistry[keyHash];
+        return _globalKeyRegistry[keyHash];
     }
 
     /// @inheritdoc IKeyRegistrar
     function getKeyHash(OperatorSet memory operatorSet, address operator) external view returns (bytes32) {
-        KeyInfo memory keyInfo = operatorKeyInfo[operatorSet.key()][operator];
-        CurveType curveType = operatorSetCurveTypes[operatorSet.key()];
+        KeyInfo memory keyInfo = _operatorKeyInfo[operatorSet.key()][operator];
+        CurveType curveType = _operatorSetCurveTypes[operatorSet.key()];
 
         if (!keyInfo.isRegistered) {
             return bytes32(0);

pkg/bindings/CrossChainRegistry/binding.go

@@ -21,13 +21,13 @@ abstract contract KeyRegistrarStorage is IKeyRegistrar {
     // Mutatables
 
     /// @dev Maps (operatorSetKey, operator) to their key info
-    mapping(bytes32 => mapping(address => KeyInfo)) internal operatorKeyInfo;
+    mapping(bytes32 operatorSetKey => mapping(address operator => KeyInfo keyInfo)) internal _operatorKeyInfo;
 
     /// @dev Maps operatorSetKey to the key type
-    mapping(bytes32 => CurveType) internal operatorSetCurveTypes;
+    mapping(bytes32 operatorSetKey => CurveType curveType) internal _operatorSetCurveTypes;
 
     /// @dev Global mapping of key hash to registration status - enforces global uniqueness
-    mapping(bytes32 => bool) internal globalKeyRegistry;
+    mapping(bytes32 keyHash => bool isRegistered) internal _globalKeyRegistry;
 
     // Construction
 

pkg/bindings/ECDSATableCalculator/binding.go

@@ -0,0 +1,115 @@
+.
+└── KeyRegistrar (**** denotes that integration tests are needed to fully validate path)
+    ├── when configureOperatorSet is called
+    │   ├── given that the caller does not have permission
+    │   │   └── it should revert
+    │   ├── given that the curve type is NONE
+    │   │   └── it should revert
+    │   ├── given that the curve type is invalid (not ECDSA or BN254)
+    │   │   └── it should revert
+    │   ├── given that the operator set configuration already exists
+    │   │   └── it should revert
+    │   └── given that all parameters are valid
+    │       ├── given that the curve type is ECDSA
+    │       │   └── it should configure the operator set for ECDSA and emit OperatorSetConfigured
+    │       └── given that the curve type is BN254
+    │           └── it should configure the operator set for BN254 and emit OperatorSetConfigured
+    ├── when registerKey is called
+    │   ├── given that the caller does not have permission
+    │   │   └── it should revert
+    │   ├── given that the operator set is not configured
+    │   │   └── it should revert
+    │   ├── given that the key is already registered for this operator set
+    │   │   └── it should revert
+    │   ├── given that the operator set is configured for ECDSA
+    │   │   ├── given that the key data length is not 20 bytes
+    │   │   │   └── it should revert
+    │   │   ├── given that the decoded address is zero
+    │   │   │   └── it should revert
+    │   │   ├── given that the key hash is already globally registered
+    │   │   │   └── it should revert
+    │   │   ├── given that the signature is invalid
+    │   │   │   └── it should revert
+    │   │   └── given that all validations pass
+    │   │       └── it should register the ECDSA key, update global registry, and emit KeyRegistered
+    │   └── given that the operator set is configured for BN254
+    │       ├── given that the G1 point is zero (0,0)
+    │       │   └── it should revert
+    │       ├── given that the signature verification fails
+    │       │   └── it should revert
+    │       ├── given that the key hash is already globally registered
+    │       │   └── it should revert
+    │       └── given that all validations pass
+    │           └── it should register the BN254 key, update global registry, and emit KeyRegistered
+    ├── when deregisterKey is called
+    │   ├── given that the caller does not have permission
+    │   │   └── it should revert
+    │   ├── given that the operator is still slashable for this operator set ****
+    │   │   └── it should revert
+    │   ├── given that the operator set is not configured
+    │   │   └── it should revert
+    │   ├── given that the key is not registered
+    │   │   └── it should revert
+    │   └── given that all conditions are met
+    │       ├── given that the operator set uses ECDSA
+    │       │   └── it should delete the key info and emit KeyDeregistered (global registry remains)
+    │       └── given that the operator set uses BN254
+    │           └── it should delete the key info and emit KeyDeregistered (global registry remains)
+    ├── when checkKey is called
+    │   ├── given that the operator set is not configured
+    │   │   └── it should revert
+    │   ├── given that the operator has a registered key
+    │   │   └── it should return true
+    │   └── given that the operator does not have a registered key
+    │       └── it should return false
+    ├── when isRegistered is called
+    │   ├── given that the operator has a registered key for the operator set
+    │   │   └── it should return true
+    │   └── given that the operator does not have a registered key for the operator set
+    │       └── it should return false
+    ├── when getOperatorSetCurveType is called
+    │   ├── given that the operator set is not configured
+    │   │   └── it should return CurveType.NONE
+    │   ├── given that the operator set is configured for ECDSA
+    │   │   └── it should return CurveType.ECDSA
+    │   └── given that the operator set is configured for BN254
+    │       └── it should return CurveType.BN254
+    ├── when getBN254Key is called
+    │   ├── given that the operator set is not configured for BN254
+    │   │   └── it should revert
+    │   ├── given that the operator is not registered
+    │   │   └── it should return zero G1 and G2 points
+    │   └── given that the operator is registered
+    │       └── it should return the decoded G1 and G2 points
+    ├── when getECDSAKey is called
+    │   ├── given that the operator set is not configured for ECDSA
+    │   │   └── it should revert
+    │   ├── given that the operator is not registered
+    │   │   └── it should return empty bytes
+    │   └── given that the operator is registered
+    │       └── it should return the 20-byte address as bytes
+    ├── when getECDSAAddress is called
+    │   ├── given that the operator set is not configured for ECDSA
+    │   │   └── it should revert
+    │   ├── given that the operator is not registered
+    │   │   └── it should return address(0)
+    │   └── given that the operator is registered
+    │       └── it should return the decoded address
+    ├── when isKeyGloballyRegistered is called
+    │   ├── given that the key hash exists in global registry
+    │   │   └── it should return true
+    │   └── given that the key hash does not exist in global registry
+    │       └── it should return false
+    ├── when getKeyHash is called
+    │   ├── given that the operator is not registered
+    │   │   └── it should return bytes32(0)
+    │   ├── given that the operator has an ECDSA key
+    │   │   └── it should return keccak256 of the key data
+    │   └── given that the operator has a BN254 key
+    │       └── it should return the BN254 G1 point hash
+    ├── when getECDSAKeyRegistrationMessageHash is called
+    │   └── it should return the EIP-712 compliant message hash for ECDSA registration
+    ├── when getBN254KeyRegistrationMessageHash is called
+    │   └── it should return the EIP-712 compliant message hash for BN254 registration
+    └── when encodeBN254KeyData is called
+        └── it should return the encoded bytes of G1 and G2 points