diff --git a/src/interfaces/IAVSRegistrarInternal.sol b/src/interfaces/IAVSRegistrarInternal.sol index d5c6740b..f3e2cf6f 100644 --- a/src/interfaces/IAVSRegistrarInternal.sol +++ b/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 {} diff --git a/src/middlewareV2/registrar/AVSRegistrar.sol b/src/middlewareV2/registrar/AVSRegistrar.sol index db1014ac..f650c21a 100644 --- a/src/middlewareV2/registrar/AVSRegistrar.sol +++ b/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 diff --git a/src/middlewareV2/registrar/AVSRegistrarStorage.sol b/src/middlewareV2/registrar/AVSRegistrarStorage.sol index 18512161..f893d1fe 100644 --- a/src/middlewareV2/registrar/AVSRegistrarStorage.sol +++ b/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; } diff --git a/src/middlewareV2/registrar/modules/Allowlist.sol b/src/middlewareV2/registrar/modules/Allowlist.sol index 4d11db52..7d44df7f 100644 --- a/src/middlewareV2/registrar/modules/Allowlist.sol +++ b/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(); diff --git a/src/middlewareV2/registrar/presets/AVSRegistrarAsIdentifier.sol b/src/middlewareV2/registrar/presets/AVSRegistrarAsIdentifier.sol index aa72353d..cc926b4b 100644 --- a/src/middlewareV2/registrar/presets/AVSRegistrarAsIdentifier.sol +++ b/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,6 +35,8 @@ 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); @@ -43,16 +44,4 @@ contract AVSRegistrarAsIdentifier is AVSRegistrar { // 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); - } } diff --git a/src/middlewareV2/registrar/presets/AVSRegistrarWithAllowlist.sol b/src/middlewareV2/registrar/presets/AVSRegistrarWithAllowlist.sol index 470b7b34..8e85f9dd 100644 --- a/src/middlewareV2/registrar/presets/AVSRegistrarWithAllowlist.sol +++ b/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 diff --git a/src/middlewareV2/registrar/presets/AVSRegistrarWithSocket.sol b/src/middlewareV2/registrar/presets/AVSRegistrarWithSocket.sol index 14315a46..ebac2a2d 100644 --- a/src/middlewareV2/registrar/presets/AVSRegistrarWithSocket.sol +++ b/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 diff --git a/test/unit/middlewareV2/AVSRegistrarAllowlistUnit.t.sol b/test/unit/middlewareV2/AVSRegistrarAllowlistUnit.t.sol index c8c94998..0670d89c 100644 --- a/test/unit/middlewareV2/AVSRegistrarAllowlistUnit.t.sol +++ b/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" - ); - } } diff --git a/test/unit/middlewareV2/AVSRegistrarAsIdentifierUnit.t.sol b/test/unit/middlewareV2/AVSRegistrarAsIdentifierUnit.t.sol index 046a052f..879a1a3f 100644 --- a/test/unit/middlewareV2/AVSRegistrarAsIdentifierUnit.t.sol +++ b/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) { diff --git a/test/unit/middlewareV2/AVSRegistrarBase.t.sol b/test/unit/middlewareV2/AVSRegistrarBase.t.sol index 36c5a8e0..beb4f73b 100644 --- a/test/unit/middlewareV2/AVSRegistrarBase.t.sol +++ b/test/unit/middlewareV2/AVSRegistrarBase.t.sol @@ -16,6 +16,21 @@ import { import {ArrayLib} from "eigenlayer-contracts/src/test/utils/ArrayLib.sol"; import "test/utils/Random.sol"; +contract AVSRegistrarImplementation is AVSRegistrar { + constructor( + IAllocationManager _allocationManager, + IKeyRegistrar _keyRegistrar + ) AVSRegistrar(_allocationManager, _keyRegistrar) { + _disableInitializers(); + } + + function initialize( + address _owner + ) external initializer { + __AVSRegistrar_init(_owner); + } +} + abstract contract AVSRegistrarBase is MockEigenLayerDeployer, IAVSRegistrarErrors, diff --git a/test/unit/middlewareV2/AVSRegistrarSocketUnit.t.sol b/test/unit/middlewareV2/AVSRegistrarSocketUnit.t.sol index fe1f8027..295ec163 100644 --- a/test/unit/middlewareV2/AVSRegistrarSocketUnit.t.sol +++ b/test/unit/middlewareV2/AVSRegistrarSocketUnit.t.sol @@ -20,7 +20,6 @@ contract AVSRegistrarSocketUnitTests is super.setUp(); avsRegistrarImplementation = new AVSRegistrarWithSocket( - AVS, IAllocationManager(address(allocationManagerMock)), IKeyRegistrar(address(keyRegistrarMock)) ); @@ -28,7 +27,9 @@ contract AVSRegistrarSocketUnitTests is avsRegistrarWithSocket = AVSRegistrarWithSocket( address( new TransparentUpgradeableProxy( - address(avsRegistrarImplementation), address(proxyAdmin), "" + address(avsRegistrarImplementation), + address(proxyAdmin), + abi.encodeWithSelector(AVSRegistrarWithSocket.initialize.selector, AVS) ) ) ); @@ -203,11 +204,4 @@ contract AVSRegistrarSocketUnitTests_ViewFunctions is AVSRegistrarSocketUnitTest ); } } - - function test_getAVS() public { - // Should return the configured AVS address - assertEq( - avsRegistrarWithSocket.getAVS(), AVS, "getAVS: should return configured AVS address" - ); - } } diff --git a/test/unit/middlewareV2/AVSRegistrarUnit.t.sol b/test/unit/middlewareV2/AVSRegistrarUnit.t.sol index 388e1630..e7562231 100644 --- a/test/unit/middlewareV2/AVSRegistrarUnit.t.sol +++ b/test/unit/middlewareV2/AVSRegistrarUnit.t.sol @@ -8,16 +8,19 @@ contract AVSRegistrarUnitTests is AVSRegistrarBase { function setUp() public override { super.setUp(); - avsRegistrarImplementation = new AVSRegistrar( - AVS, - IAllocationManager(address(allocationManagerMock)), - IKeyRegistrar(address(keyRegistrarMock)) + avsRegistrarImplementation = AVSRegistrar( + new AVSRegistrarImplementation( + IAllocationManager(address(allocationManagerMock)), + IKeyRegistrar(address(keyRegistrarMock)) + ) ); avsRegistrar = AVSRegistrar( address( new TransparentUpgradeableProxy( - address(avsRegistrarImplementation), address(proxyAdmin), "" + address(avsRegistrarImplementation), + address(proxyAdmin), + abi.encodeWithSelector(AVSRegistrarImplementation.initialize.selector, AVS) ) ) ); @@ -130,9 +133,4 @@ contract AVSRegistrarUnitTests_ViewFunctions is AVSRegistrarUnitTests { ); } } - - function test_getAVS() public { - // Should return the configured AVS address - assertEq(avsRegistrar.getAVS(), AVS, "getAVS: should return configured AVS address"); - } } diff --git a/test/unit/middlewareV2/AllowlistUnit.t.sol b/test/unit/middlewareV2/AllowlistUnit.t.sol index 7cc33f7c..8964eeba 100644 --- a/test/unit/middlewareV2/AllowlistUnit.t.sol +++ b/test/unit/middlewareV2/AllowlistUnit.t.sol @@ -16,6 +16,12 @@ import {Random, Randomness} from "test/utils/Random.sol"; // Concrete implementation for testing contract AllowlistImplementation is Allowlist { + function initialize( + address _owner + ) external initializer { + __Allowlist_init(_owner); + } + function version() external pure returns (string memory) { return "1.0.0"; } @@ -70,7 +76,9 @@ contract AllowlistUnitTests is Test, IAllowlistErrors, IAllowlistEvents { new TransparentUpgradeableProxy( address(allowlistImplementation), address(proxyAdmin), - abi.encodeWithSelector(Allowlist.initialize.selector, allowlistOwner) + abi.encodeWithSelector( + AllowlistImplementation.initialize.selector, allowlistOwner + ) ) ) ); @@ -115,7 +123,7 @@ contract AllowlistUnitTests_initialize is AllowlistUnitTests { new TransparentUpgradeableProxy( address(allowlistImplementation), address(proxyAdmin), - abi.encodeWithSelector(Allowlist.initialize.selector, randomOwner) + abi.encodeWithSelector(AllowlistImplementation.initialize.selector, randomOwner) ) ) );