fix(I-04): make avs var stateful & remove confusing natspec

src/interfaces/IAVSRegistrarInternal.sol

@@ -17,7 +17,4 @@ interface IAVSRegistrarEvents {
 }
 
 /// @notice Since we have already defined a public interface, we add the events and errors here
-interface IAVSRegistrarInternal is IAVSRegistrarErrors, IAVSRegistrarEvents {
-    /// @notice Returns the address of the AVS in EigenLayer core
-    function getAVS() external view returns (address);
-}
+interface IAVSRegistrarInternal is IAVSRegistrarErrors, IAVSRegistrarEvents {}

src/middlewareV2/registrar/AVSRegistrar.sol

@@ -16,7 +16,7 @@ import {AVSRegistrarStorage} from "./AVSRegistrarStorage.sol";
 import {Initializable} from "@openzeppelin-upgrades/contracts/proxy/utils/Initializable.sol";
 
 /// @notice A minimal AVSRegistrar contract that is used to register/deregister operators for an AVS
-contract AVSRegistrar is Initializable, AVSRegistrarStorage {
+abstract contract AVSRegistrar is Initializable, AVSRegistrarStorage {
     using OperatorSetLib for OperatorSet;
 
     modifier onlyAllocationManager() {
@@ -25,13 +25,19 @@ contract AVSRegistrar is Initializable, AVSRegistrarStorage {
     }
 
     constructor(
-        address _avs,
         IAllocationManager _allocationManager,
         IKeyRegistrar _keyRegistrar
-    ) AVSRegistrarStorage(_avs, _allocationManager, _keyRegistrar) {
+    ) AVSRegistrarStorage(_allocationManager, _keyRegistrar) {
         _disableInitializers();
     }
 
+    /// @dev This initialization function MUST be added to a child's `initialize` function to avoid uninitialized storage.
+    function __AVSRegistrar_init(
+        address _avs
+    ) internal virtual {
+        avs = _avs;
+    }
+
     /// @inheritdoc IAVSRegistrar
     function registerOperator(
         address operator,
@@ -69,11 +75,6 @@ contract AVSRegistrar is Initializable, AVSRegistrarStorage {
         return _avs == avs;
     }
 
-    /// @inheritdoc IAVSRegistrarInternal
-    function getAVS() external view virtual returns (address) {
-        return avs;
-    }
-
     /*
      *
      *                            INTERNAL FUNCTIONS

src/middlewareV2/registrar/AVSRegistrarStorage.sol

@@ -15,18 +15,17 @@ abstract contract AVSRegistrarStorage is IAVSRegistrar, IAVSRegistrarInternal {
      *
      */
 
-    /// @notice The AVS that this registrar is for
-    /// @dev In practice, the AVS address in EigenLayer core is address that initialized the Metadata URI.
-    address internal immutable avs;
-
     /// @notice The allocation manager in EigenLayer core
     IAllocationManager public immutable allocationManager;
 
     /// @notice Pointer to the EigenLayer core Key Registrar
     IKeyRegistrar public immutable keyRegistrar;
 
-    constructor(address _avs, IAllocationManager _allocationManager, IKeyRegistrar _keyRegistrar) {
-        avs = _avs;
+    /// @notice The AVS that this registrar is for
+    /// @dev In practice, the AVS address in EigenLayer core is address that initialized the Metadata URI.
+    address public avs;
+
+    constructor(IAllocationManager _allocationManager, IKeyRegistrar _keyRegistrar) {
         allocationManager = _allocationManager;
         keyRegistrar = _keyRegistrar;
     }
@@ -36,5 +35,5 @@ abstract contract AVSRegistrarStorage is IAVSRegistrar, IAVSRegistrarInternal {
      * variables without shifting down storage in the inheritance chain.
      * See https://docs.openzeppelin.com/contracts/4.x/upgradeable#storage_gaps
      */
-    uint256[50] private __GAP;
+    uint256[49] private __GAP;
 }

src/middlewareV2/registrar/modules/Allowlist.sol

@@ -18,13 +18,7 @@ abstract contract Allowlist is OwnableUpgradeable, AllowlistStorage {
     using OperatorSetLib for OperatorSet;
     using EnumerableSetUpgradeable for EnumerableSetUpgradeable.AddressSet;
 
-    function initialize(
-        address _owner
-    ) public virtual initializer {
-        _initializeAllowlist(_owner);
-    }
-
-    function _initializeAllowlist(
+    function __Allowlist_init(
         address _owner
     ) internal onlyInitializing {
         __Ownable_init();

src/middlewareV2/registrar/presets/AVSRegistrarAsIdentifier.sol

@@ -1,14 +1,16 @@
 // SPDX-License-Identifier: BUSL-1.1
 pragma solidity ^0.8.27;
 
+import {Initializable} from "@openzeppelin-upgrades/contracts/proxy/utils/Initializable.sol";
+
 import {IAVSRegistrar} from "eigenlayer-contracts/src/contracts/interfaces/IAVSRegistrar.sol";
-import {IAVSRegistrarInternal} from "../../../interfaces/IAVSRegistrarInternal.sol";
 import {IAllocationManager} from
     "eigenlayer-contracts/src/contracts/interfaces/IAllocationManager.sol";
 import {IPermissionController} from
     "eigenlayer-contracts/src/contracts/interfaces/IPermissionController.sol";
 import {IKeyRegistrar} from "eigenlayer-contracts/src/contracts/interfaces/IKeyRegistrar.sol";
 
+import {IAVSRegistrarInternal} from "../../../interfaces/IAVSRegistrarInternal.sol";
 import {AVSRegistrar} from "../AVSRegistrar.sol";
 
 /// @notice An AVSRegistrar that is the identifier for the AVS in EigenLayer core.
@@ -17,14 +19,11 @@ contract AVSRegistrarAsIdentifier is AVSRegistrar {
     /// @notice The permission controller for the AVS
     IPermissionController public immutable permissionController;
 
-    /// @dev The immutable avs address `AVSRegistrar` is NOT the address of the AVS in EigenLayer core.
-    /// @dev The address of the AVS in EigenLayer core is the proxy contract, and it is set via the `initialize` function below.
     constructor(
-        address _avs,
         IAllocationManager _allocationManager,
         IPermissionController _permissionController,
         IKeyRegistrar _keyRegistrar
-    ) AVSRegistrar(_avs, _allocationManager, _keyRegistrar) {
+    ) AVSRegistrar(_allocationManager, _keyRegistrar) {
         // Set the permission controller for future interactions
         permissionController = _permissionController;
     }
@@ -36,23 +35,13 @@ contract AVSRegistrarAsIdentifier is AVSRegistrar {
      * @dev This function enables the address of the AVS in the core protocol to be the proxy AVSRegistrarAsIdentifier contract
      */
     function initialize(address admin, string memory metadataURI) public initializer {
+        __AVSRegistrar_init(address(this));
+
         // Set the metadataURI and the registrar for the AVS to this registrar contract
         allocationManager.updateAVSMetadataURI(address(this), metadataURI);
         allocationManager.setAVSRegistrar(address(this), this);
 
         // Set the admin for the AVS
         permissionController.addPendingAdmin(address(this), admin);
     }
-
-    /// @inheritdoc IAVSRegistrar
-    function supportsAVS(
-        address _avs
-    ) public view override returns (bool) {
-        return _avs == address(this);
-    }
-
-    /// @inheritdoc IAVSRegistrarInternal
-    function getAVS() external view override returns (address) {
-        return address(this);
-    }
 }

src/middlewareV2/registrar/presets/AVSRegistrarWithAllowlist.sol

@@ -12,15 +12,16 @@ import {OperatorSet} from "eigenlayer-contracts/src/contracts/libraries/Operator
 
 contract AVSRegistrarWithAllowlist is AVSRegistrar, Allowlist, IAVSRegistrarWithAllowlist {
     constructor(
-        address _avs,
         IAllocationManager _allocationManager,
         IKeyRegistrar _keyRegistrar
-    ) AVSRegistrar(_avs, _allocationManager, _keyRegistrar) {}
+    ) AVSRegistrar(_allocationManager, _keyRegistrar) {}
 
-    function initialize(
-        address admin
-    ) public override initializer {
-        _initializeAllowlist(admin);
+    function initialize(address avs, address admin) external initializer {
+        // Initialize the AVSRegistrar
+        __AVSRegistrar_init(avs);
+
+        // Initialize the allowlist
+        __Allowlist_init(admin);
     }
 
     /// @notice Before registering operator, check if the operator is in the allowlist

src/middlewareV2/registrar/presets/AVSRegistrarWithSocket.sol

@@ -11,10 +11,15 @@ import {SocketRegistry} from "../modules/SocketRegistry.sol";
 
 contract AVSRegistrarWithSocket is AVSRegistrar, SocketRegistry, IAVSRegistrarWithSocket {
     constructor(
-        address _avs,
         IAllocationManager _allocationManager,
         IKeyRegistrar _keyRegistrar
-    ) AVSRegistrar(_avs, _allocationManager, _keyRegistrar) {}
+    ) AVSRegistrar(_allocationManager, _keyRegistrar) {}
+
+    function initialize(
+        address avs
+    ) external initializer {
+        __AVSRegistrar_init(avs);
+    }
 
     /// @notice Set the socket for the operator
     /// @dev This function sets the socket even if the operator is already registered

test/unit/middlewareV2/AVSRegistrarAllowlistUnit.t.sol

@@ -19,7 +19,6 @@ contract AVSRegistrarWithAllowlistUnitTests is
         super.setUp();
 
         avsRegistrarImplementation = new AVSRegistrarWithAllowlist(
-            AVS,
             IAllocationManager(address(allocationManagerMock)),
             IKeyRegistrar(address(keyRegistrarMock))
         );
@@ -30,7 +29,7 @@ contract AVSRegistrarWithAllowlistUnitTests is
                     address(avsRegistrarImplementation),
                     address(proxyAdmin),
                     abi.encodeWithSelector(
-                        AVSRegistrarWithAllowlist.initialize.selector, address(this)
+                        AVSRegistrarWithAllowlist.initialize.selector, AVS, address(this)
                     )
                 )
             )
@@ -57,7 +56,7 @@ contract AVSRegistrarWithAllowlistUnitTests_initialize is AVSRegistrarWithAllowl
 
     function test_revert_alreadyInitialized() public {
         cheats.expectRevert("Initializable: contract is already initialized");
-        avsRegistrarWithAllowlist.initialize(allowlistAdmin);
+        avsRegistrarWithAllowlist.initialize(AVS, allowlistAdmin);
     }
 }
 
@@ -346,11 +345,4 @@ contract AVSRegistrarWithAllowlistUnitTests_ViewFunctions is AVSRegistrarWithAll
             );
         }
     }
-
-    function test_getAVS() public {
-        // Should return the configured AVS address
-        assertEq(
-            avsRegistrarWithAllowlist.getAVS(), AVS, "getAVS: should return configured AVS address"
-        );
-    }
 }

test/unit/middlewareV2/AVSRegistrarAsIdentifierUnit.t.sol

@@ -24,7 +24,6 @@ contract AVSRegistrarAsIdentifierUnitTests is AVSRegistrarBase {
 
         // Deploy the implementation
         avsRegistrarImplementation = new AVSRegistrarAsIdentifier(
-            AVS,
             IAllocationManager(address(allocationManagerMock)),
             permissionController,
             IKeyRegistrar(address(keyRegistrarMock))
@@ -34,7 +33,11 @@ contract AVSRegistrarAsIdentifierUnitTests is AVSRegistrarBase {
         avsRegistrarAsIdentifier = AVSRegistrarAsIdentifier(
             address(
                 new TransparentUpgradeableProxy(
-                    address(avsRegistrarImplementation), address(proxyAdmin), ""
+                    address(avsRegistrarImplementation),
+                    address(proxyAdmin),
+                    abi.encodeWithSelector(
+                        AVSRegistrarAsIdentifier.initialize.selector, AVS, METADATA_URI
+                    )
                 )
             )
         );
@@ -45,7 +48,6 @@ contract AVSRegistrarAsIdentifierUnitTests_constructor is AVSRegistrarAsIdentifi
     function test_constructor() public {
         // Deploy a new implementation to test constructor
         AVSRegistrarAsIdentifier impl = new AVSRegistrarAsIdentifier(
-            AVS,
             IAllocationManager(address(allocationManagerMock)),
             permissionController,
             IKeyRegistrar(address(keyRegistrarMock))
@@ -72,6 +74,14 @@ contract AVSRegistrarAsIdentifierUnitTests_constructor is AVSRegistrarAsIdentifi
 
 contract AVSRegistrarAsIdentifierUnitTests_initialize is AVSRegistrarAsIdentifierUnitTests {
     function test_initialize() public {
+        avsRegistrarAsIdentifier = AVSRegistrarAsIdentifier(
+            address(
+                new TransparentUpgradeableProxy(
+                    address(avsRegistrarImplementation), address(proxyAdmin), ""
+                )
+            )
+        );
+
         // Mock the allocationManager calls
         vm.mockCall(
             address(allocationManagerMock),
@@ -151,7 +161,7 @@ contract AVSRegistrarAsIdentifierUnitTests_initialize is AVSRegistrarAsIdentifie
             ""
         );
 
-        avsRegistrarAsIdentifier.initialize(admin, METADATA_URI);
+        // avsRegistrarAsIdentifier.initialize(admin, METADATA_URI);
 
         // Try to initialize again
         vm.expectRevert("Initializable: contract is already initialized");
@@ -205,43 +215,9 @@ contract AVSRegistrarAsIdentifierUnitTests_supportsAVS is AVSRegistrarAsIdentifi
     }
 }
 
-contract AVSRegistrarAsIdentifierUnitTests_getAVS is AVSRegistrarAsIdentifierUnitTests {
-    function test_getAVS() public {
-        // Should return the proxy address (self) since AVSRegistrarAsIdentifier overrides getAVS
-        assertEq(
-            avsRegistrarAsIdentifier.getAVS(),
-            address(avsRegistrarAsIdentifier),
-            "getAVS: should return self (proxy) address"
-        );
-    }
-}
-
 contract AVSRegistrarAsIdentifierUnitTests_registerOperator is AVSRegistrarAsIdentifierUnitTests {
     using ArrayLib for *;
 
-    function setUp() public virtual override {
-        super.setUp();
-
-        // Initialize the contract
-        vm.mockCall(
-            address(allocationManagerMock),
-            abi.encodeWithSelector(IAllocationManager.updateAVSMetadataURI.selector),
-            ""
-        );
-        vm.mockCall(
-            address(allocationManagerMock),
-            abi.encodeWithSelector(IAllocationManager.setAVSRegistrar.selector),
-            ""
-        );
-        vm.mockCall(
-            address(permissionController),
-            abi.encodeWithSelector(IPermissionController.addPendingAdmin.selector),
-            ""
-        );
-
-        avsRegistrarAsIdentifier.initialize(admin, METADATA_URI);
-    }
-
     function testFuzz_revert_notAllocationManager(
         address notAllocationManager
     ) public filterFuzzedAddressInputs(notAllocationManager) {
@@ -279,14 +255,16 @@ contract AVSRegistrarAsIdentifierUnitTests_registerOperator is AVSRegistrarAsIde
         // Register keys for the operator - Note: using AVS as the avs address in the OperatorSet
         for (uint32 i; i < operatorSetIds.length; ++i) {
             keyRegistrarMock.setIsRegistered(
-                defaultOperator, OperatorSet({avs: AVS, id: operatorSetIds[i]}), true
+                defaultOperator,
+                OperatorSet({avs: address(avsRegistrarAsIdentifier), id: operatorSetIds[i]}),
+                true
             );
         }
 
         // Register operator
+        vm.prank(address(allocationManagerMock));
         vm.expectEmit(true, true, true, true);
         emit OperatorRegistered(defaultOperator, operatorSetIds);
-        vm.prank(address(allocationManagerMock));
         avsRegistrarAsIdentifier.registerOperator(
             defaultOperator, address(avsRegistrarAsIdentifier), operatorSetIds, "0x"
         );
@@ -298,29 +276,6 @@ contract AVSRegistrarAsIdentifierUnitTests_deregisterOperator is
 {
     using ArrayLib for *;
 
-    function setUp() public virtual override {
-        super.setUp();
-
-        // Initialize the contract
-        vm.mockCall(
-            address(allocationManagerMock),
-            abi.encodeWithSelector(IAllocationManager.updateAVSMetadataURI.selector),
-            ""
-        );
-        vm.mockCall(
-            address(allocationManagerMock),
-            abi.encodeWithSelector(IAllocationManager.setAVSRegistrar.selector),
-            ""
-        );
-        vm.mockCall(
-            address(permissionController),
-            abi.encodeWithSelector(IPermissionController.addPendingAdmin.selector),
-            ""
-        );
-
-        avsRegistrarAsIdentifier.initialize(admin, METADATA_URI);
-    }
-
     function testFuzz_revert_notAllocationManager(
         address notAllocationManager
     ) public filterFuzzedAddressInputs(notAllocationManager) {