Skip to content

Commit 2b51e9b

Browse files
0xClandestineypatil12
0xClandestine
authored and
ypatil12
committed
fix(I-04): make avs var stateful & remove confusing natspec (#512)
**Motivation:** The following natspec is present but isn't true and suggest improper use. ```solidity /// @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. ``` **Modifications:** - Removed confusing comments - Made AVS var stateful and `AVSRegistrar` abstract **Result:** Improved clarity.
1 parent a37c5c6 commit 2b51e9b

13 files changed

+93
-145
lines changed

‎src/interfaces/IAVSRegistrarInternal.sol‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,4 @@ interface IAVSRegistrarEvents {
1717
}
1818

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

‎src/middlewareV2/registrar/AVSRegistrar.sol‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import {AVSRegistrarStorage} from "./AVSRegistrarStorage.sol";
1616
import {Initializable} from "@openzeppelin-upgrades/contracts/proxy/utils/Initializable.sol";
1717

1818
/// @notice A minimal AVSRegistrar contract that is used to register/deregister operators for an AVS
19-
contract AVSRegistrar is Initializable, AVSRegistrarStorage {
19+
abstract contract AVSRegistrar is Initializable, AVSRegistrarStorage {
2020
using OperatorSetLib for OperatorSet;
2121

2222
modifier onlyAllocationManager() {
@@ -25,13 +25,19 @@ contract AVSRegistrar is Initializable, AVSRegistrarStorage {
2525
}
2626

2727
constructor(
28-
address _avs,
2928
IAllocationManager _allocationManager,
3029
IKeyRegistrar _keyRegistrar
31-
) AVSRegistrarStorage(_avs, _allocationManager, _keyRegistrar) {
30+
) AVSRegistrarStorage(_allocationManager, _keyRegistrar) {
3231
_disableInitializers();
3332
}
3433

34+
/// @dev This initialization function MUST be added to a child's `initialize` function to avoid uninitialized storage.
35+
function __AVSRegistrar_init(
36+
address _avs
37+
) internal virtual {
38+
avs = _avs;
39+
}
40+
3541
/// @inheritdoc IAVSRegistrar
3642
function registerOperator(
3743
address operator,
@@ -69,11 +75,6 @@ contract AVSRegistrar is Initializable, AVSRegistrarStorage {
6975
return _avs == avs;
7076
}
7177

72-
/// @inheritdoc IAVSRegistrarInternal
73-
function getAVS() external view virtual returns (address) {
74-
return avs;
75-
}
76-
7778
/*
7879
*
7980
* INTERNAL FUNCTIONS

‎src/middlewareV2/registrar/AVSRegistrarStorage.sol‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,18 +15,17 @@ abstract contract AVSRegistrarStorage is IAVSRegistrar, IAVSRegistrarInternal {
1515
*
1616
*/
1717

18-
/// @notice The AVS that this registrar is for
19-
/// @dev In practice, the AVS address in EigenLayer core is address that initialized the Metadata URI.
20-
address internal immutable avs;
21-
2218
/// @notice The allocation manager in EigenLayer core
2319
IAllocationManager public immutable allocationManager;
2420

2521
/// @notice Pointer to the EigenLayer core Key Registrar
2622
IKeyRegistrar public immutable keyRegistrar;
2723

28-
constructor(address _avs, IAllocationManager _allocationManager, IKeyRegistrar _keyRegistrar) {
29-
avs = _avs;
24+
/// @notice The AVS that this registrar is for
25+
/// @dev In practice, the AVS address in EigenLayer core is address that initialized the Metadata URI.
26+
address public avs;
27+
28+
constructor(IAllocationManager _allocationManager, IKeyRegistrar _keyRegistrar) {
3029
allocationManager = _allocationManager;
3130
keyRegistrar = _keyRegistrar;
3231
}
@@ -36,5 +35,5 @@ abstract contract AVSRegistrarStorage is IAVSRegistrar, IAVSRegistrarInternal {
3635
* variables without shifting down storage in the inheritance chain.
3736
* See https://docs.openzeppelin.com/contracts/4.x/upgradeable#storage_gaps
3837
*/
39-
uint256[50] private __GAP;
38+
uint256[49] private __GAP;
4039
}

‎src/middlewareV2/registrar/modules/Allowlist.sol‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,7 @@ abstract contract Allowlist is OwnableUpgradeable, AllowlistStorage {
1818
using OperatorSetLib for OperatorSet;
1919
using EnumerableSetUpgradeable for EnumerableSetUpgradeable.AddressSet;
2020

21-
function initialize(
22-
address _owner
23-
) public virtual initializer {
24-
_initializeAllowlist(_owner);
25-
}
26-
27-
function _initializeAllowlist(
21+
function __Allowlist_init(
2822
address _owner
2923
) internal onlyInitializing {
3024
__Ownable_init();

‎src/middlewareV2/registrar/presets/AVSRegistrarAsIdentifier.sol‎

Lines changed: 6 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,16 @@
11
// SPDX-License-Identifier: BUSL-1.1
22
pragma solidity ^0.8.27;
33

4+
import {Initializable} from "@openzeppelin-upgrades/contracts/proxy/utils/Initializable.sol";
5+
46
import {IAVSRegistrar} from "eigenlayer-contracts/src/contracts/interfaces/IAVSRegistrar.sol";
5-
import {IAVSRegistrarInternal} from "../../../interfaces/IAVSRegistrarInternal.sol";
67
import {IAllocationManager} from
78
"eigenlayer-contracts/src/contracts/interfaces/IAllocationManager.sol";
89
import {IPermissionController} from
910
"eigenlayer-contracts/src/contracts/interfaces/IPermissionController.sol";
1011
import {IKeyRegistrar} from "eigenlayer-contracts/src/contracts/interfaces/IKeyRegistrar.sol";
1112

13+
import {IAVSRegistrarInternal} from "../../../interfaces/IAVSRegistrarInternal.sol";
1214
import {AVSRegistrar} from "../AVSRegistrar.sol";
1315

1416
/// @notice An AVSRegistrar that is the identifier for the AVS in EigenLayer core.
@@ -17,14 +19,11 @@ contract AVSRegistrarAsIdentifier is AVSRegistrar {
1719
/// @notice The permission controller for the AVS
1820
IPermissionController public immutable permissionController;
1921

20-
/// @dev The immutable avs address `AVSRegistrar` is NOT the address of the AVS in EigenLayer core.
21-
/// @dev The address of the AVS in EigenLayer core is the proxy contract, and it is set via the `initialize` function below.
2222
constructor(
23-
address _avs,
2423
IAllocationManager _allocationManager,
2524
IPermissionController _permissionController,
2625
IKeyRegistrar _keyRegistrar
27-
) AVSRegistrar(_avs, _allocationManager, _keyRegistrar) {
26+
) AVSRegistrar(_allocationManager, _keyRegistrar) {
2827
// Set the permission controller for future interactions
2928
permissionController = _permissionController;
3029
}
@@ -36,23 +35,13 @@ contract AVSRegistrarAsIdentifier is AVSRegistrar {
3635
* @dev This function enables the address of the AVS in the core protocol to be the proxy AVSRegistrarAsIdentifier contract
3736
*/
3837
function initialize(address admin, string memory metadataURI) public initializer {
38+
__AVSRegistrar_init(address(this));
39+
3940
// Set the metadataURI and the registrar for the AVS to this registrar contract
4041
allocationManager.updateAVSMetadataURI(address(this), metadataURI);
4142
allocationManager.setAVSRegistrar(address(this), this);
4243

4344
// Set the admin for the AVS
4445
permissionController.addPendingAdmin(address(this), admin);
4546
}
46-
47-
/// @inheritdoc IAVSRegistrar
48-
function supportsAVS(
49-
address _avs
50-
) public view override returns (bool) {
51-
return _avs == address(this);
52-
}
53-
54-
/// @inheritdoc IAVSRegistrarInternal
55-
function getAVS() external view override returns (address) {
56-
return address(this);
57-
}
5847
}

‎src/middlewareV2/registrar/presets/AVSRegistrarWithAllowlist.sol‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,16 @@ import {OperatorSet} from "eigenlayer-contracts/src/contracts/libraries/Operator
1212

1313
contract AVSRegistrarWithAllowlist is AVSRegistrar, Allowlist, IAVSRegistrarWithAllowlist {
1414
constructor(
15-
address _avs,
1615
IAllocationManager _allocationManager,
1716
IKeyRegistrar _keyRegistrar
18-
) AVSRegistrar(_avs, _allocationManager, _keyRegistrar) {}
17+
) AVSRegistrar(_allocationManager, _keyRegistrar) {}
1918

20-
function initialize(
21-
address admin
22-
) public override initializer {
23-
_initializeAllowlist(admin);
19+
function initialize(address avs, address admin) external initializer {
20+
// Initialize the AVSRegistrar
21+
__AVSRegistrar_init(avs);
22+
23+
// Initialize the allowlist
24+
__Allowlist_init(admin);
2425
}
2526

2627
/// @notice Before registering operator, check if the operator is in the allowlist

‎src/middlewareV2/registrar/presets/AVSRegistrarWithSocket.sol‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,15 @@ import {SocketRegistry} from "../modules/SocketRegistry.sol";
1111

1212
contract AVSRegistrarWithSocket is AVSRegistrar, SocketRegistry, IAVSRegistrarWithSocket {
1313
constructor(
14-
address _avs,
1514
IAllocationManager _allocationManager,
1615
IKeyRegistrar _keyRegistrar
17-
) AVSRegistrar(_avs, _allocationManager, _keyRegistrar) {}
16+
) AVSRegistrar(_allocationManager, _keyRegistrar) {}
17+
18+
function initialize(
19+
address avs
20+
) external initializer {
21+
__AVSRegistrar_init(avs);
22+
}
1823

1924
/// @notice Set the socket for the operator
2025
/// @dev This function sets the socket even if the operator is already registered

‎test/unit/middlewareV2/AVSRegistrarAllowlistUnit.t.sol‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ contract AVSRegistrarWithAllowlistUnitTests is
1919
super.setUp();
2020

2121
avsRegistrarImplementation = new AVSRegistrarWithAllowlist(
22-
AVS,
2322
IAllocationManager(address(allocationManagerMock)),
2423
IKeyRegistrar(address(keyRegistrarMock))
2524
);
@@ -30,7 +29,7 @@ contract AVSRegistrarWithAllowlistUnitTests is
3029
address(avsRegistrarImplementation),
3130
address(proxyAdmin),
3231
abi.encodeWithSelector(
33-
AVSRegistrarWithAllowlist.initialize.selector, address(this)
32+
AVSRegistrarWithAllowlist.initialize.selector, AVS, address(this)
3433
)
3534
)
3635
)
@@ -57,7 +56,7 @@ contract AVSRegistrarWithAllowlistUnitTests_initialize is AVSRegistrarWithAllowl
5756

5857
function test_revert_alreadyInitialized() public {
5958
cheats.expectRevert("Initializable: contract is already initialized");
60-
avsRegistrarWithAllowlist.initialize(allowlistAdmin);
59+
avsRegistrarWithAllowlist.initialize(AVS, allowlistAdmin);
6160
}
6261
}
6362

@@ -346,11 +345,4 @@ contract AVSRegistrarWithAllowlistUnitTests_ViewFunctions is AVSRegistrarWithAll
346345
);
347346
}
348347
}
349-
350-
function test_getAVS() public {
351-
// Should return the configured AVS address
352-
assertEq(
353-
avsRegistrarWithAllowlist.getAVS(), AVS, "getAVS: should return configured AVS address"
354-
);
355-
}
356348
}

‎test/unit/middlewareV2/AVSRegistrarAsIdentifierUnit.t.sol‎

Lines changed: 18 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ contract AVSRegistrarAsIdentifierUnitTests is AVSRegistrarBase {
2424

2525
// Deploy the implementation
2626
avsRegistrarImplementation = new AVSRegistrarAsIdentifier(
27-
AVS,
2827
IAllocationManager(address(allocationManagerMock)),
2928
permissionController,
3029
IKeyRegistrar(address(keyRegistrarMock))
@@ -34,7 +33,11 @@ contract AVSRegistrarAsIdentifierUnitTests is AVSRegistrarBase {
3433
avsRegistrarAsIdentifier = AVSRegistrarAsIdentifier(
3534
address(
3635
new TransparentUpgradeableProxy(
37-
address(avsRegistrarImplementation), address(proxyAdmin), ""
36+
address(avsRegistrarImplementation),
37+
address(proxyAdmin),
38+
abi.encodeWithSelector(
39+
AVSRegistrarAsIdentifier.initialize.selector, AVS, METADATA_URI
40+
)
3841
)
3942
)
4043
);
@@ -45,7 +48,6 @@ contract AVSRegistrarAsIdentifierUnitTests_constructor is AVSRegistrarAsIdentifi
4548
function test_constructor() public {
4649
// Deploy a new implementation to test constructor
4750
AVSRegistrarAsIdentifier impl = new AVSRegistrarAsIdentifier(
48-
AVS,
4951
IAllocationManager(address(allocationManagerMock)),
5052
permissionController,
5153
IKeyRegistrar(address(keyRegistrarMock))
@@ -72,6 +74,14 @@ contract AVSRegistrarAsIdentifierUnitTests_constructor is AVSRegistrarAsIdentifi
7274

7375
contract AVSRegistrarAsIdentifierUnitTests_initialize is AVSRegistrarAsIdentifierUnitTests {
7476
function test_initialize() public {
77+
avsRegistrarAsIdentifier = AVSRegistrarAsIdentifier(
78+
address(
79+
new TransparentUpgradeableProxy(
80+
address(avsRegistrarImplementation), address(proxyAdmin), ""
81+
)
82+
)
83+
);
84+
7585
// Mock the allocationManager calls
7686
vm.mockCall(
7787
address(allocationManagerMock),
@@ -151,7 +161,7 @@ contract AVSRegistrarAsIdentifierUnitTests_initialize is AVSRegistrarAsIdentifie
151161
""
152162
);
153163

154-
avsRegistrarAsIdentifier.initialize(admin, METADATA_URI);
164+
// avsRegistrarAsIdentifier.initialize(admin, METADATA_URI);
155165

156166
// Try to initialize again
157167
vm.expectRevert("Initializable: contract is already initialized");
@@ -205,43 +215,9 @@ contract AVSRegistrarAsIdentifierUnitTests_supportsAVS is AVSRegistrarAsIdentifi
205215
}
206216
}
207217

208-
contract AVSRegistrarAsIdentifierUnitTests_getAVS is AVSRegistrarAsIdentifierUnitTests {
209-
function test_getAVS() public {
210-
// Should return the proxy address (self) since AVSRegistrarAsIdentifier overrides getAVS
211-
assertEq(
212-
avsRegistrarAsIdentifier.getAVS(),
213-
address(avsRegistrarAsIdentifier),
214-
"getAVS: should return self (proxy) address"
215-
);
216-
}
217-
}
218-
219218
contract AVSRegistrarAsIdentifierUnitTests_registerOperator is AVSRegistrarAsIdentifierUnitTests {
220219
using ArrayLib for *;
221220

222-
function setUp() public virtual override {
223-
super.setUp();
224-
225-
// Initialize the contract
226-
vm.mockCall(
227-
address(allocationManagerMock),
228-
abi.encodeWithSelector(IAllocationManager.updateAVSMetadataURI.selector),
229-
""
230-
);
231-
vm.mockCall(
232-
address(allocationManagerMock),
233-
abi.encodeWithSelector(IAllocationManager.setAVSRegistrar.selector),
234-
""
235-
);
236-
vm.mockCall(
237-
address(permissionController),
238-
abi.encodeWithSelector(IPermissionController.addPendingAdmin.selector),
239-
""
240-
);
241-
242-
avsRegistrarAsIdentifier.initialize(admin, METADATA_URI);
243-
}
244-
245221
function testFuzz_revert_notAllocationManager(
246222
address notAllocationManager
247223
) public filterFuzzedAddressInputs(notAllocationManager) {
@@ -279,14 +255,16 @@ contract AVSRegistrarAsIdentifierUnitTests_registerOperator is AVSRegistrarAsIde
279255
// Register keys for the operator - Note: using AVS as the avs address in the OperatorSet
280256
for (uint32 i; i < operatorSetIds.length; ++i) {
281257
keyRegistrarMock.setIsRegistered(
282-
defaultOperator, OperatorSet({avs: AVS, id: operatorSetIds[i]}), true
258+
defaultOperator,
259+
OperatorSet({avs: address(avsRegistrarAsIdentifier), id: operatorSetIds[i]}),
260+
true
283261
);
284262
}
285263

286264
// Register operator
265+
vm.prank(address(allocationManagerMock));
287266
vm.expectEmit(true, true, true, true);
288267
emit OperatorRegistered(defaultOperator, operatorSetIds);
289-
vm.prank(address(allocationManagerMock));
290268
avsRegistrarAsIdentifier.registerOperator(
291269
defaultOperator, address(avsRegistrarAsIdentifier), operatorSetIds, "0x"
292270
);
@@ -298,29 +276,6 @@ contract AVSRegistrarAsIdentifierUnitTests_deregisterOperator is
298276
{
299277
using ArrayLib for *;
300278

301-
function setUp() public virtual override {
302-
super.setUp();
303-
304-
// Initialize the contract
305-
vm.mockCall(
306-
address(allocationManagerMock),
307-
abi.encodeWithSelector(IAllocationManager.updateAVSMetadataURI.selector),
308-
""
309-
);
310-
vm.mockCall(
311-
address(allocationManagerMock),
312-
abi.encodeWithSelector(IAllocationManager.setAVSRegistrar.selector),
313-
""
314-
);
315-
vm.mockCall(
316-
address(permissionController),
317-
abi.encodeWithSelector(IPermissionController.addPendingAdmin.selector),
318-
""
319-
);
320-
321-
avsRegistrarAsIdentifier.initialize(admin, METADATA_URI);
322-
}
323-
324279
function testFuzz_revert_notAllocationManager(
325280
address notAllocationManager
326281
) public filterFuzzedAddressInputs(notAllocationManager) {

0 commit comments

Comments
 (0)