From 090c5b1d8d352f282879c267ead9e87d6efe7498 Mon Sep 17 00:00:00 2001 From: gpsanant Date: Sat, 1 Feb 2025 02:12:20 -0800 Subject: [PATCH 01/16] chore: start refactor --- src/BLSSignatureChecker.sol | 4 +- src/RegistryCoordinator.sol | 143 ++++++++-------- src/RegistryCoordinatorStorage.sol | 66 ++++++++ src/SlashingRegistryCoordinator.sol | 158 +++++++----------- src/SlashingRegistryCoordinatorStorage.sol | 14 +- src/SocketRegistry.sol | 9 +- src/StakeRegistry.sol | 54 +++--- src/StakeRegistryStorage.sol | 4 +- src/interfaces/IRegistryCoordinator.sol | 17 +- .../ISlashingRegistryCoordinator.sol | 24 --- src/interfaces/ISocketRegistry.sol | 9 +- src/interfaces/IStakeRegistry.sol | 13 +- .../RegistryCoordinatorHarness.t.sol | 12 +- test/utils/MockAVSDeployer.sol | 2 +- 14 files changed, 264 insertions(+), 265 deletions(-) create mode 100644 src/RegistryCoordinatorStorage.sol diff --git a/src/BLSSignatureChecker.sol b/src/BLSSignatureChecker.sol index 5743be64..ff3e12a4 100644 --- a/src/BLSSignatureChecker.sol +++ b/src/BLSSignatureChecker.sol @@ -1,6 +1,8 @@ // SPDX-License-Identifier: BUSL-1.1 pragma solidity ^0.8.27; +import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol"; + import {BitmapUtils} from "./libraries/BitmapUtils.sol"; import {BN254} from "./libraries/BN254.sol"; @@ -18,7 +20,7 @@ contract BLSSignatureChecker is BLSSignatureCheckerStorage { /// MODIFIERS modifier onlyCoordinatorOwner() { - require(msg.sender == registryCoordinator.owner(), OnlyRegistryCoordinatorOwner()); + require(msg.sender == Ownable(address(registryCoordinator)).owner(), OnlyRegistryCoordinatorOwner()); _; } diff --git a/src/RegistryCoordinator.sol b/src/RegistryCoordinator.sol index c1970c08..1523acd6 100644 --- a/src/RegistryCoordinator.sol +++ b/src/RegistryCoordinator.sol @@ -15,6 +15,7 @@ import {BitmapUtils} from "./libraries/BitmapUtils.sol"; import {SlashingRegistryCoordinator} from "./SlashingRegistryCoordinator.sol"; import {ISlashingRegistryCoordinator} from "./interfaces/ISlashingRegistryCoordinator.sol"; import {OwnableUpgradeable} from "@openzeppelin-upgrades/contracts/access/OwnableUpgradeable.sol"; +import {RegistryCoordinatorStorage} from "./RegistryCoordinatorStorage.sol"; /** * @title A `RegistryCoordinator` that has three registries: @@ -24,12 +25,9 @@ import {OwnableUpgradeable} from "@openzeppelin-upgrades/contracts/access/Ownabl * * @author Layr Labs, Inc. */ -contract RegistryCoordinator is IRegistryCoordinator, SlashingRegistryCoordinator { +contract RegistryCoordinator is RegistryCoordinatorStorage { using BitmapUtils for *; - /// @notice the ServiceManager for this AVS, which forwards calls onto EigenLayer's core contracts - IServiceManager public immutable serviceManager; - constructor( IServiceManager _serviceManager, IStakeRegistry _stakeRegistry, @@ -39,17 +37,16 @@ contract RegistryCoordinator is IRegistryCoordinator, SlashingRegistryCoordinato IAllocationManager _allocationManager, IPauserRegistry _pauserRegistry ) - SlashingRegistryCoordinator( + RegistryCoordinatorStorage( + _serviceManager, _stakeRegistry, _blsApkRegistry, _indexRegistry, _socketRegistry, _allocationManager, _pauserRegistry - ) - { - serviceManager = _serviceManager; - } + ) + {} /** * @@ -64,44 +61,23 @@ contract RegistryCoordinator is IRegistryCoordinator, SlashingRegistryCoordinato IBLSApkRegistryTypes.PubkeyRegistrationParams memory params, SignatureWithSaltAndExpiry memory operatorSignature ) external onlyWhenNotPaused(PAUSED_REGISTER_OPERATOR) { - require(!m2QuorumsDisabled, M2QuorumsAlreadyDisabled()); - /** - * If the operator has NEVER registered a pubkey before, use `params` to register - * their pubkey in blsApkRegistry - * - * If the operator HAS registered a pubkey, `params` is ignored and the pubkey hash - * (operatorId) is fetched instead - */ - bytes32 operatorId = _getOrCreateOperatorId(msg.sender, params); - - // Register the operator in each of the registry contracts and update the operator's - // quorum bitmap and registration status - uint32[] memory numOperatorsPerQuorum = _registerOperator({ + require(!isM2QuorumRegistrationDisabled, M2QuorumRegistrationIsDisabled()); + + // Check if the operator has registered before + bool operatorRegisteredBefore = _operatorInfo[msg.sender].status == OperatorStatus.REGISTERED; + + // register the operator with the registry coordinator + _registerOperator({ operator: msg.sender, - operatorId: operatorId, + operatorId: _getOrCreateOperatorId(msg.sender, params), quorumNumbers: quorumNumbers, - socket: socket - }).numOperatorsPerQuorum; - - // For each quorum, validate that the new operator count does not exceed the maximum - // (If it does, an operator needs to be replaced -- see `registerOperatorWithChurn`) - for (uint256 i = 0; i < quorumNumbers.length; i++) { - uint8 quorumNumber = uint8(quorumNumbers[i]); - - require( - numOperatorsPerQuorum[i] <= _quorumParams[quorumNumber].maxOperatorCount, - MaxQuorumsReached() - ); - } - - // If the operator wasn't registered for any quorums, update their status - // and register them with this AVS in EigenLayer core (DelegationManager) - if (_operatorInfo[msg.sender].status != OperatorStatus.REGISTERED) { - _operatorInfo[msg.sender] = - OperatorInfo({operatorId: operatorId, status: OperatorStatus.REGISTERED}); + socket: socket, + checkMaxOperatorCount: true + }); + // If the operator has never registered before, register them with the AVSDirectory + if (!operatorRegisteredBefore) { serviceManager.registerOperatorToAVS(msg.sender, operatorSignature); - emit OperatorRegistered(msg.sender, operatorId); } } @@ -114,34 +90,24 @@ contract RegistryCoordinator is IRegistryCoordinator, SlashingRegistryCoordinato SignatureWithSaltAndExpiry memory churnApproverSignature, SignatureWithSaltAndExpiry memory operatorSignature ) external onlyWhenNotPaused(PAUSED_REGISTER_OPERATOR) { - require(!m2QuorumsDisabled, M2QuorumsAlreadyDisabled()); + require(!isM2QuorumRegistrationDisabled, M2QuorumRegistrationIsDisabled()); - /** - * If the operator has NEVER registered a pubkey before, use `params` to register - * their pubkey in blsApkRegistry - * - * If the operator HAS registered a pubkey, `params` is ignored and the pubkey hash - * (operatorId) is fetched instead - */ - bytes32 operatorId = _getOrCreateOperatorId(msg.sender, params); + // Check if the operator has registered before + bool operatorRegisteredBefore = _operatorInfo[msg.sender].status == OperatorStatus.REGISTERED; + // register the operator with the registry coordinator with churn _registerOperatorWithChurn({ operator: msg.sender, - operatorId: operatorId, + operatorId: _getOrCreateOperatorId(msg.sender, params), quorumNumbers: quorumNumbers, socket: socket, operatorKickParams: operatorKickParams, churnApproverSignature: churnApproverSignature }); - // If the operator wasn't registered for any quorums, update their status - // and register them with this AVS in EigenLayer core (DelegationManager) - if (_operatorInfo[msg.sender].status != OperatorStatus.REGISTERED) { - _operatorInfo[msg.sender] = - OperatorInfo({operatorId: operatorId, status: OperatorStatus.REGISTERED}); - + // If the operator has never registered before, register them with the AVSDirectory + if (!operatorRegisteredBefore) { serviceManager.registerOperatorToAVS(msg.sender, operatorSignature); - emit OperatorRegistered(msg.sender, operatorId); } } @@ -163,7 +129,7 @@ contract RegistryCoordinator is IRegistryCoordinator, SlashingRegistryCoordinato require(!operatorSetsEnabled, OperatorSetsAlreadyEnabled()); // Set the bitmap for M2 quorums - M2quorumBitmap = _getQuorumBitmap(quorumCount); + m2QuorumBitmap = _getQuorumBitmap(quorumCount); // Enable operator sets mode operatorSetsEnabled = true; @@ -175,9 +141,27 @@ contract RegistryCoordinator is IRegistryCoordinator, SlashingRegistryCoordinato function disableM2QuorumRegistration() external onlyOwner { require(operatorSetsEnabled, OperatorSetsNotEnabled()); - m2QuorumsDisabled = true; + isM2QuorumRegistrationDisabled = true; + + emit M2QuorumRegistrationDisabled(); + } + + /** + * + * INTERNAL FUNCTIONS + * + */ - emit M2QuorumsDisabled(); + /// @dev override the _forceDeregisterOperator function to handle M2 quorum deregistration + function _forceDeregisterOperator(address operator, bytes memory quorumNumbers) internal virtual override { + if (operatorSetsEnabled) { + // filter out M2 quorums from the quorum numbers + uint256 operatorSetBitmap = quorumNumbers.orderedBytesArrayToBitmap().minus(m2QuorumBitmap); + if (!operatorSetBitmap.isEmpty()) { + // call the parent _forceDeregisterOperator function for operator sets quorums + super._forceDeregisterOperator(operator, operatorSetBitmap.bitmapToBytesArray()); + } + } } /// @dev Hook to allow for any post-deregister logic @@ -187,7 +171,7 @@ contract RegistryCoordinator is IRegistryCoordinator, SlashingRegistryCoordinato bytes memory quorumNumbers, uint192 newBitmap ) internal virtual override { - uint256 operatorM2QuorumBitmap = newBitmap.minus(M2quorumBitmap); + uint256 operatorM2QuorumBitmap = newBitmap.minus(m2QuorumBitmap); // If the operator is no longer registered for any M2 quorums, update their status and deregister // them from the AVS via the EigenLayer core contracts if (operatorM2QuorumBitmap.isEmpty()) { @@ -208,6 +192,27 @@ contract RegistryCoordinator is IRegistryCoordinator, SlashingRegistryCoordinato return (1 << quorumCount) - 1; } + /// @notice Returns true if the quorum number is an M2 quorum + /// @dev We use bitwise and to check if the quorum number is an M2 quorum + function _isM2Quorum( + uint8 quorumNumber + ) internal view returns (bool) { + return m2QuorumBitmap.isSet(quorumNumber); + } + + /** + * + * VIEW FUNCTIONS + * + */ + + /// @notice Returns true if the quorum number is an M2 quorum + function isM2Quorum( + uint8 quorumNumber + ) external view returns (bool) { + return _isM2Quorum(quorumNumber); + } + /** * @notice Returns the message hash that an operator must sign to register their BLS public key. * @param operator is the address of the operator registering their BLS public key @@ -217,14 +222,4 @@ contract RegistryCoordinator is IRegistryCoordinator, SlashingRegistryCoordinato ) public view returns (bytes32) { return _hashTypedDataV4(keccak256(abi.encode(PUBKEY_REGISTRATION_TYPEHASH, operator))); } - - /// @dev need to override function here since its defined in both these contracts - function owner() - public - view - override(SlashingRegistryCoordinator, ISlashingRegistryCoordinator) - returns (address) - { - return OwnableUpgradeable.owner(); - } } diff --git a/src/RegistryCoordinatorStorage.sol b/src/RegistryCoordinatorStorage.sol new file mode 100644 index 00000000..908fa800 --- /dev/null +++ b/src/RegistryCoordinatorStorage.sol @@ -0,0 +1,66 @@ +// SPDX-License-Identifier: BUSL-1.1 +pragma solidity ^0.8.27; + +import {IPauserRegistry} from "eigenlayer-contracts/src/contracts/interfaces/IPauserRegistry.sol"; +import {IAllocationManager} from + "eigenlayer-contracts/src/contracts/interfaces/IAllocationManager.sol"; +import {IBLSApkRegistry, IBLSApkRegistryTypes} from "./interfaces/IBLSApkRegistry.sol"; +import {IStakeRegistry} from "./interfaces/IStakeRegistry.sol"; +import {IIndexRegistry} from "./interfaces/IIndexRegistry.sol"; +import {IServiceManager} from "./interfaces/IServiceManager.sol"; +import {IRegistryCoordinator} from "./interfaces/IRegistryCoordinator.sol"; +import {ISocketRegistry} from "./interfaces/ISocketRegistry.sol"; + +import {SlashingRegistryCoordinator} from "./SlashingRegistryCoordinator.sol"; + +abstract contract RegistryCoordinatorStorage is IRegistryCoordinator, SlashingRegistryCoordinator { + /** + * + * CONSTANTS AND IMMUTABLES + * + */ + + /// @notice the ServiceManager for this AVS, which forwards calls onto EigenLayer's core contracts + IServiceManager public immutable serviceManager; + + /** + * + * STATE + * + */ + + /// @notice Whether this AVS allows operator sets for registration + /// @dev If true, operators may register to operator sets via the AllocationManager + bool public operatorSetsEnabled; + + /// @notice Whether this AVS allows M2 quorums for registration + /// @dev If true, operators may **not** register to M2 quorums. Deregistration is still allowed. + bool public isM2QuorumRegistrationDisabled; + + /// @notice The bitmap containing all M2 quorums. This is only used for existing AVS middlewares that have M2 quorums + /// and need to call `enableOperatorSets()` to enable operator sets mode. + uint256 internal m2QuorumBitmap; + + constructor( + IServiceManager _serviceManager, + IStakeRegistry _stakeRegistry, + IBLSApkRegistry _blsApkRegistry, + IIndexRegistry _indexRegistry, + ISocketRegistry _socketRegistry, + IAllocationManager _allocationManager, + IPauserRegistry _pauserRegistry + ) + SlashingRegistryCoordinator( + _stakeRegistry, + _blsApkRegistry, + _indexRegistry, + _socketRegistry, + _allocationManager, + _pauserRegistry + ) + { + serviceManager = _serviceManager; + } + + uint256[47] private __GAP; +} \ No newline at end of file diff --git a/src/SlashingRegistryCoordinator.sol b/src/SlashingRegistryCoordinator.sol index 482be871..0d08e62e 100644 --- a/src/SlashingRegistryCoordinator.sol +++ b/src/SlashingRegistryCoordinator.sol @@ -108,12 +108,6 @@ contract SlashingRegistryCoordinator is registries.push(address(stakeRegistry)); registries.push(address(blsApkRegistry)); registries.push(address(indexRegistry)); - - // Set the AVS to be OperatorSets compatible - operatorSetsEnabled = true; - - // Set the AVS to not accept M2 quorums - m2QuorumsDisabled = true; } /// @inheritdoc ISlashingRegistryCoordinator @@ -138,7 +132,6 @@ contract SlashingRegistryCoordinator is IStakeRegistryTypes.StrategyParams[] memory strategyParams, uint32 lookAheadPeriod ) external virtual onlyOwner { - require(operatorSetsEnabled, OperatorSetsNotEnabled()); _createQuorum( operatorSetParams, minimumStake, @@ -154,7 +147,6 @@ contract SlashingRegistryCoordinator is uint32[] memory operatorSetIds, bytes calldata data ) external override onlyAllocationManager onlyWhenNotPaused(PAUSED_REGISTER_OPERATOR) { - require(operatorSetsEnabled, OperatorSetsNotEnabled()); bytes memory quorumNumbers = _getQuorumNumbers(operatorSetIds); ( @@ -179,7 +171,8 @@ contract SlashingRegistryCoordinator is operator: operator, operatorId: operatorId, quorumNumbers: quorumNumbers, - socket: socket + socket: socket, + checkMaxOperatorCount: true }).numOperatorsPerQuorum; // For each quorum, validate that the new operator count does not exceed the maximum @@ -221,13 +214,6 @@ contract SlashingRegistryCoordinator is } else { revert InvalidRegistrationType(); } - - // If the operator wasn't registered for any quorums, update their status - // and register them with this AVS in EigenLayer core (DelegationManager) - if (_operatorInfo[operator].status != OperatorStatus.REGISTERED) { - _operatorInfo[operator] = OperatorInfo(operatorId, OperatorStatus.REGISTERED); - emit OperatorRegistered(operator, operatorId); - } } /// @inheritdoc ISlashingRegistryCoordinator @@ -235,7 +221,6 @@ contract SlashingRegistryCoordinator is address operator, uint32[] memory operatorSetIds ) external override onlyAllocationManager onlyWhenNotPaused(PAUSED_DEREGISTER_OPERATOR) { - require(operatorSetsEnabled, OperatorSetsNotEnabled()); bytes memory quorumNumbers = _getQuorumNumbers(operatorSetIds); _deregisterOperator(operator, quorumNumbers); } @@ -339,10 +324,6 @@ contract SlashingRegistryCoordinator is && quorumsToRemove.isSubsetOf(currentBitmap) ) { _deregisterOperator({operator: operator, quorumNumbers: quorumNumbers}); - - if (operatorSetsEnabled) { - _forceDeregisterOperator(operator, quorumNumbers); - } } } @@ -407,7 +388,8 @@ contract SlashingRegistryCoordinator is address operator, bytes32 operatorId, bytes memory quorumNumbers, - string memory socket + string memory socket, + bool checkMaxOperatorCount ) internal virtual returns (RegisterResults memory results) { /** * Get bitmap of quorums to register for and operator's current bitmap. Validate that: @@ -441,12 +423,26 @@ contract SlashingRegistryCoordinator is emit OperatorSocketUpdate(operatorId, socket); + // If the operator wasn't registered for any quorums, update their status + // and register them with this AVS in EigenLayer core (DelegationManager) + if (_operatorInfo[operator].status != OperatorStatus.REGISTERED) { + _operatorInfo[operator] = OperatorInfo(operatorId, OperatorStatus.REGISTERED); + emit OperatorRegistered(operator, operatorId); + } + // Register the operator with the BLSApkRegistry, StakeRegistry, and IndexRegistry blsApkRegistry.registerOperator(operator, quorumNumbers); (results.operatorStakes, results.totalStakes) = stakeRegistry.registerOperator(operator, operatorId, quorumNumbers); results.numOperatorsPerQuorum = indexRegistry.registerOperator(operatorId, quorumNumbers); + if (checkMaxOperatorCount) { + for (uint256 i = 0; i < quorumNumbers.length; i++) { + OperatorSetParam memory operatorSetParams = _quorumParams[uint8(quorumNumbers[i])]; + require(results.numOperatorsPerQuorum[i] <= operatorSetParams.maxOperatorCount, MaxQuorumsReached()); + } + } + // call hook to allow for any post-register logic _afterRegisterOperator(operator, operatorId, quorumNumbers, newBitmap); @@ -474,7 +470,13 @@ contract SlashingRegistryCoordinator is // Register the operator in each of the registry contracts and update the operator's // quorum bitmap and registration status RegisterResults memory results = - _registerOperator(operator, operatorId, quorumNumbers, socket); + _registerOperator({ + operator: operator, + operatorId: operatorId, + quorumNumbers: quorumNumbers, + socket: socket, + checkMaxOperatorCount: false + }); // Check that each quorum's operator count is below the configured maximum. If the max // is exceeded, use `operatorKickParams` to deregister an existing operator to make space @@ -498,10 +500,6 @@ contract SlashingRegistryCoordinator is bytes memory singleQuorumNumber = new bytes(1); singleQuorumNumber[0] = quorumNumbers[i]; _deregisterOperator(operatorKickParams[i].operator, singleQuorumNumber); - - if (operatorSetsEnabled) { - _forceDeregisterOperator(operatorKickParams[i].operator, singleQuorumNumber); - } } } } @@ -550,43 +548,29 @@ contract SlashingRegistryCoordinator is stakeRegistry.deregisterOperator(operatorId, quorumNumbers); indexRegistry.deregisterOperator(operatorId, quorumNumbers); + // If the caller is not the allocationManager, then this is a force deregistration not consented by the operator + if (msg.sender != address(allocationManager)) { + _forceDeregisterOperator(operator, quorumNumbers); + } + // call hook to allow for any post-deregister logic _afterDeregisterOperator(operator, operatorId, quorumNumbers, newBitmap); } /** * @notice Helper function to handle operator set deregistration for OperatorSets quorums. This is used - * when an operator is force-deregistered from a set of quorums. For any of the quorums that are - * OperatorSets quorums, we need to deregister the operator in the AllocationManager. + * when an operator is force-deregistered from a set of quorums. * @param operator The operator to deregister * @param quorumNumbers The quorum numbers the operator is force-deregistered from */ - function _forceDeregisterOperator(address operator, bytes memory quorumNumbers) internal { - uint32[] memory operatorSetIds = new uint32[](quorumNumbers.length); - uint256 numOperatorSetQuorums; - - // Check each quorum's stake type - for (uint256 i = 0; i < quorumNumbers.length; i++) { - uint8 quorumNumber = uint8(quorumNumbers[i]); - if (_isM2Quorum(quorumNumber)) { - operatorSetIds[numOperatorSetQuorums++] = quorumNumber; - } - } - - // If any OperatorSet quorums found, deregister from AVS in the AllocationManager - if (numOperatorSetQuorums > 0) { - // Resize array to exact size needed - assembly { - mstore(operatorSetIds, numOperatorSetQuorums) - } - allocationManager.deregisterFromOperatorSets( - IAllocationManagerTypes.DeregisterParams({ - operator: operator, - avs: accountIdentifier, - operatorSetIds: operatorSetIds - }) - ); - } + function _forceDeregisterOperator(address operator, bytes memory quorumNumbers) internal virtual { + allocationManager.deregisterFromOperatorSets( + IAllocationManagerTypes.DeregisterParams({ + operator: operator, + avs: accountIdentifier, + operatorSetIds: _getOperatorSetIds(quorumNumbers) + }) + ); } /** @@ -789,25 +773,23 @@ contract SlashingRegistryCoordinator is // Initialize the quorum here and in each registry _setOperatorSetParams(quorumNumber, operatorSetParams); - /// Update the AllocationManager if operatorSetQuorum - if (operatorSetsEnabled && !_isM2Quorum(quorumNumber)) { - // Create array of CreateSetParams for the new quorum - IAllocationManagerTypes.CreateSetParams[] memory createSetParams = - new IAllocationManagerTypes.CreateSetParams[](1); - - // Extract strategies from strategyParams - IStrategy[] memory strategies = new IStrategy[](strategyParams.length); - for (uint256 i = 0; i < strategyParams.length; i++) { - strategies[i] = strategyParams[i].strategy; - } + // Create array of CreateSetParams for the new quorum + IAllocationManagerTypes.CreateSetParams[] memory createSetParams = + new IAllocationManagerTypes.CreateSetParams[](1); - // Initialize CreateSetParams with quorumNumber as operatorSetId - createSetParams[0] = IAllocationManagerTypes.CreateSetParams({ - operatorSetId: quorumNumber, - strategies: strategies - }); - allocationManager.createOperatorSets({avs: accountIdentifier, params: createSetParams}); + // Extract strategies from strategyParams + IStrategy[] memory strategies = new IStrategy[](strategyParams.length); + for (uint256 i = 0; i < strategyParams.length; i++) { + strategies[i] = strategyParams[i].strategy; } + + // Initialize CreateSetParams with quorumNumber as operatorSetId + createSetParams[0] = IAllocationManagerTypes.CreateSetParams({ + operatorSetId: quorumNumber, + strategies: strategies + }); + allocationManager.createOperatorSets({avs: accountIdentifier, params: createSetParams}); + // Initialize stake registry based on stake type if (stakeType == IStakeRegistryTypes.StakeType.TOTAL_DELEGATED) { stakeRegistry.initializeDelegatedStakeQuorum(quorumNumber, minimumStake, strategyParams); @@ -863,12 +845,14 @@ contract SlashingRegistryCoordinator is return quorumNumbers; } - /// @notice Returns true if the quorum number is an M2 quorum - /// @dev We use bitwise and to check if the quorum number is an M2 quorum - function _isM2Quorum( - uint8 quorumNumber - ) internal view returns (bool) { - return M2quorumBitmap.isSet(quorumNumber); + function _getOperatorSetIds( + bytes memory quorumNumbers + ) internal pure returns (uint32[] memory) { + uint32[] memory operatorSetIds = new uint32[](quorumNumbers.length); + for (uint256 i = 0; i < quorumNumbers.length; i++) { + operatorSetIds[i] = uint32(uint8(quorumNumbers[i])); + } + return operatorSetIds; } function _setOperatorSetParams( @@ -972,13 +956,6 @@ contract SlashingRegistryCoordinator is return _operatorInfo[operator].status; } - /// @notice Returns true if the quorum number is an M2 quorum - function isM2Quorum( - uint8 quorumNumber - ) external view returns (bool) { - return _isM2Quorum(quorumNumber); - } - /** * @notice Returns the indices of the quorumBitmaps for the provided `operatorIds` at the given `blockNumber` * @dev Reverts if any of the `operatorIds` was not (yet) registered at `blockNumber` @@ -1076,15 +1053,4 @@ contract SlashingRegistryCoordinator is _hashTypedDataV4(keccak256(abi.encode(PUBKEY_REGISTRATION_TYPEHASH, operator))) ); } - - /// @dev need to override function here since its defined in both these contracts - function owner() - public - view - virtual - override(OwnableUpgradeable, ISlashingRegistryCoordinator) - returns (address) - { - return OwnableUpgradeable.owner(); - } } diff --git a/src/SlashingRegistryCoordinatorStorage.sol b/src/SlashingRegistryCoordinatorStorage.sol index 14946dd9..ed39c0b7 100644 --- a/src/SlashingRegistryCoordinatorStorage.sol +++ b/src/SlashingRegistryCoordinatorStorage.sol @@ -85,23 +85,11 @@ abstract contract SlashingRegistryCoordinatorStorage is ISlashingRegistryCoordin /// @notice the delay in seconds before an operator can reregister after being ejected uint256 public ejectionCooldown; - /// @notice Whether this AVS allows operator sets for registration - /// @dev If true, operators may register to operator sets via the AllocationManager - bool public operatorSetsEnabled; - - /// @notice Whether this AVS allows M2 quorums for registration - /// @dev If true, operators may **not** register to M2 quorums. Deregistration is still allowed. - bool public m2QuorumsDisabled; - /// @notice The account identifier for this AVS (used for UAM integration in EigenLayer) /// @dev NOTE: Updating this value will break existing OperatorSets and UAM integration. /// This value should only be set once. address public accountIdentifier; - /// @notice The bitmap containing all M2 quorums. This is only used for existing AVS middlewares that have M2 quorums - /// and need to call `enableOperatorSets()` to enable operator sets mode. - uint256 internal M2quorumBitmap; - constructor( IStakeRegistry _stakeRegistry, IBLSApkRegistry _blsApkRegistry, @@ -118,5 +106,5 @@ abstract contract SlashingRegistryCoordinatorStorage is ISlashingRegistryCoordin // storage gap for upgradeability // slither-disable-next-line shadowing-state - uint256[37] private __GAP; + uint256[40] private __GAP; } diff --git a/src/SocketRegistry.sol b/src/SocketRegistry.sol index 9e09cb2b..e79d1fa3 100644 --- a/src/SocketRegistry.sol +++ b/src/SocketRegistry.sol @@ -4,6 +4,7 @@ pragma solidity ^0.8.12; import {ISlashingRegistryCoordinator} from "./interfaces/ISlashingRegistryCoordinator.sol"; import {ISocketRegistry} from "./interfaces/ISocketRegistry.sol"; import {SocketRegistryStorage} from "./SocketRegistryStorage.sol"; +import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol"; /** * @title A `Registry` that keeps track of operator sockets. @@ -13,8 +14,8 @@ contract SocketRegistry is ISocketRegistry, SocketRegistryStorage { /// @notice A modifier that only allows the RegistryCoordinator to call a function modifier onlySlashingRegistryCoordinator() { require( - msg.sender == address(slashingRegistryCoordinator), - "SocketRegistry.onlySlashingRegistryCoordinator: caller is not the SlashingRegistryCoordinator" + msg.sender == slashingRegistryCoordinator, + OnlySlashingRegistryCoordinator() ); _; } @@ -22,8 +23,8 @@ contract SocketRegistry is ISocketRegistry, SocketRegistryStorage { /// @notice A modifier that only allows the owner of the SlashingRegistryCoordinator to call a function modifier onlyCoordinatorOwner() { require( - msg.sender == ISlashingRegistryCoordinator(slashingRegistryCoordinator).owner(), - "SocketRegistry.onlyCoordinatorOwner: caller is not the owner of the slashingRegistryCoordinator" + msg.sender == Ownable(slashingRegistryCoordinator).owner(), + OnlySlashingRegistryCoordinatorOwner() ); _; } diff --git a/src/StakeRegistry.sol b/src/StakeRegistry.sol index 4cf3a286..c485cc0e 100644 --- a/src/StakeRegistry.sol +++ b/src/StakeRegistry.sol @@ -1,6 +1,8 @@ // SPDX-License-Identifier: BUSL-1.1 pragma solidity ^0.8.27; +import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol"; + import {IDelegationManager} from "eigenlayer-contracts/src/contracts/interfaces/IDelegationManager.sol"; import {IAVSDirectory} from "eigenlayer-contracts/src/contracts/interfaces/IAVSDirectory.sol"; @@ -235,13 +237,14 @@ contract StakeRegistry is StakeRegistryStorage { uint256 numStratsToAdd = _strategyParams.length; - if (isOperatorSetQuorum(quorumNumber)) { + address avs = registryCoordinator.accountIdentifier(); + if (allocationManager.isOperatorSet(OperatorSet(avs, quorumNumber))) { IStrategy[] memory strategiesToAdd = new IStrategy[](numStratsToAdd); for (uint256 i = 0; i < numStratsToAdd; i++) { strategiesToAdd[i] = _strategyParams[i].strategy; } allocationManager.addStrategiesToOperatorSet({ - avs: ISlashingRegistryCoordinator(registryCoordinator).accountIdentifier(), + avs: avs, operatorSetId: quorumNumber, strategies: strategiesToAdd }); @@ -277,9 +280,10 @@ contract StakeRegistry is StakeRegistryStorage { _strategiesPerQuorum.pop(); } - if (isOperatorSetQuorum(quorumNumber)) { + address avs = registryCoordinator.accountIdentifier(); + if (allocationManager.isOperatorSet(OperatorSet(avs, quorumNumber))) { allocationManager.removeStrategiesFromOperatorSet({ - avs: ISlashingRegistryCoordinator(registryCoordinator).accountIdentifier(), + avs: avs, operatorSetId: quorumNumber, strategies: _strategiesToRemove }); @@ -507,16 +511,16 @@ contract StakeRegistry is StakeRegistryStorage { ) internal view returns (uint256[] memory) { address[] memory operators = new address[](1); operators[0] = operator; - uint32 beforeTimestamp = + uint32 beforeBlock = uint32(block.number + slashableStakeLookAheadPerQuorum[quorumNumber]); uint256[][] memory slashableShares = allocationManager.getMinimumSlashableStake( OperatorSet( - ISlashingRegistryCoordinator(registryCoordinator).accountIdentifier(), quorumNumber + registryCoordinator.accountIdentifier(), quorumNumber ), operators, strategiesPerQuorum[quorumNumber], - beforeTimestamp + beforeBlock ); return slashableShares[0]; @@ -538,21 +542,14 @@ contract StakeRegistry is StakeRegistryStorage { uint256[] memory strategyShares; if (stakeTypePerQuorum[quorumNumber] == IStakeRegistryTypes.StakeType.TOTAL_SLASHABLE) { + // get slashable stake for the operator from AllocationManager strategyShares = _getSlashableStakePerStrategy(quorumNumber, operator); - for (uint256 i = 0; i < stratsLength; i++) { - strategyAndMultiplier = strategyParams[quorumNumber][i]; - if (strategyShares[i] > 0) { - weight += uint96( - strategyShares[i] * strategyAndMultiplier.multiplier / WEIGHTING_DIVISOR - ); - } - } } else { - /// M2 Concept of delegated stake - strategyShares = - delegation.getOperatorShares(operator, strategiesPerQuorum[quorumNumber]); - for (uint256 i = 0; i < stratsLength; i++) { - // accessing i^th StrategyParams struct for the quorumNumber + // get delegated stake for the operator from DelegationManager + strategyShares = delegation.getOperatorShares(operator, strategiesPerQuorum[quorumNumber]); + } + for (uint256 i = 0; i < stratsLength; i++) { + // accessing i'th StrategyParams struct for the quorumNumber strategyAndMultiplier = strategyParams[quorumNumber][i]; // add the weight from the shares for this strategy to the total weight @@ -560,11 +557,10 @@ contract StakeRegistry is StakeRegistryStorage { weight += uint96( strategyShares[i] * strategyAndMultiplier.multiplier / WEIGHTING_DIVISOR ); - } } } - // Return the weight, and `true` if the operator meets the quorum's minimum stake + // return the weight, and `true` if the operator meets the quorum's minimum stake bool hasMinimumStake = weight >= minimumStakeForQuorum[quorumNumber]; return (weight, hasMinimumStake); } @@ -582,15 +578,6 @@ contract StakeRegistry is StakeRegistryStorage { * */ - /// @inheritdoc IStakeRegistry - function isOperatorSetQuorum( - uint8 quorumNumber - ) public view returns (bool) { - bool isM2 = ISlashingRegistryCoordinator(registryCoordinator).isM2Quorum(quorumNumber); - bool isOperatorSet = ISlashingRegistryCoordinator(registryCoordinator).operatorSetsEnabled(); - return isOperatorSet && !isM2; - } - /// @inheritdoc IStakeRegistry function weightOfOperatorForQuorum( uint8 quorumNumber, @@ -785,18 +772,19 @@ contract StakeRegistry is StakeRegistryStorage { * @param _lookAheadBlocks The number of blocks to look ahead when checking shares */ function _setLookAheadPeriod(uint8 quorumNumber, uint32 _lookAheadBlocks) internal { + require(stakeTypePerQuorum[quorumNumber] == IStakeRegistryTypes.StakeType.TOTAL_SLASHABLE, QuorumNotSlashable()); uint32 oldLookAheadDays = slashableStakeLookAheadPerQuorum[quorumNumber]; slashableStakeLookAheadPerQuorum[quorumNumber] = _lookAheadBlocks; emit LookAheadPeriodChanged(oldLookAheadDays, _lookAheadBlocks); } function _checkSlashingRegistryCoordinator() internal view { - require(msg.sender == registryCoordinator, OnlySlashingRegistryCoordinator()); + require(msg.sender == address(registryCoordinator), OnlySlashingRegistryCoordinator()); } function _checkSlashingRegistryCoordinatorOwner() internal view { require( - msg.sender == ISlashingRegistryCoordinator(registryCoordinator).owner(), + msg.sender == Ownable(address(registryCoordinator)).owner(), OnlySlashingRegistryCoordinatorOwner() ); } diff --git a/src/StakeRegistryStorage.sol b/src/StakeRegistryStorage.sol index 2c00aec2..24700aee 100644 --- a/src/StakeRegistryStorage.sol +++ b/src/StakeRegistryStorage.sol @@ -37,7 +37,7 @@ abstract contract StakeRegistryStorage is IStakeRegistry { IAllocationManager public immutable allocationManager; /// @notice the coordinator contract that this registry is associated with - address public immutable registryCoordinator; + ISlashingRegistryCoordinator public immutable registryCoordinator; /// @notice In order to register for a quorum i, an operator must have at least `minimumStakeForQuorum[i]` /// evaluated by this contract's 'VoteWeigher' logic. @@ -70,7 +70,7 @@ abstract contract StakeRegistryStorage is IStakeRegistry { IAVSDirectory _avsDirectory, IAllocationManager _allocationManager ) { - registryCoordinator = address(_slashingRegistryCoordinator); + registryCoordinator = _slashingRegistryCoordinator; delegation = _delegationManager; avsDirectory = _avsDirectory; allocationManager = _allocationManager; diff --git a/src/interfaces/IRegistryCoordinator.sol b/src/interfaces/IRegistryCoordinator.sol index 4c9397dc..10d3c1ef 100644 --- a/src/interfaces/IRegistryCoordinator.sol +++ b/src/interfaces/IRegistryCoordinator.sol @@ -17,7 +17,9 @@ interface IRegistryCoordinatorErrors is ISlashingRegistryCoordinatorErrors { /// @notice Thrown when a quorum is an operator set quorum. error OperatorSetQuorum(); /// @notice Thrown when M2 quorums are already disabled. - error M2QuorumsAlreadyDisabled(); + error M2QuorumRegistrationIsDisabled(); + /// @notice Thrown when operator set operations are attempted while not enabled. + error OperatorSetsNotEnabled(); } interface IRegistryCoordinatorTypes is ISlashingRegistryCoordinatorTypes {} @@ -33,10 +35,10 @@ interface IRegistryCoordinatorEvents is event OperatorSetsEnabled(); /** - * @notice Emitted when M2 quorums are disabled. + * @notice Emitted when M2 quorum registration is disabled. * @dev Emitted in disableM2QuorumRegistration(). */ - event M2QuorumsDisabled(); + event M2QuorumRegistrationDisabled(); } interface IRegistryCoordinator is @@ -113,6 +115,15 @@ interface IRegistryCoordinator is */ function enableOperatorSets() external; + /** + * @notice Checks if a quorum is an M2 quorum. + * @param quorumNumber The quorum identifier. + * @return True if the quorum is M2, false otherwise. + */ + function isM2Quorum( + uint8 quorumNumber + ) external view returns (bool); + /** * @notice Disables M2 quorum registration for the AVS. Once disabled, this cannot be enabled. * @dev When disabled, all registrations to M2 quorums will revert. Deregistrations are still possible. diff --git a/src/interfaces/ISlashingRegistryCoordinator.sol b/src/interfaces/ISlashingRegistryCoordinator.sol index 4984535a..8aaac488 100644 --- a/src/interfaces/ISlashingRegistryCoordinator.sol +++ b/src/interfaces/ISlashingRegistryCoordinator.sol @@ -50,8 +50,6 @@ interface ISlashingRegistryCoordinatorErrors { error NotSorted(); /// @notice Thrown when maximum quorum count is reached. error MaxQuorumsReached(); - /// @notice Thrown when operator set operations are attempted while not enabled. - error OperatorSetsNotEnabled(); } interface ISlashingRegistryCoordinatorTypes { @@ -296,21 +294,6 @@ interface ISlashingRegistryCoordinator is */ function ejectionCooldown() external view returns (uint256); - /** - * @notice Checks if a quorum is an M2 quorum. - * @param quorumNumber The quorum identifier. - * @return True if the quorum is M2, false otherwise. - */ - function isM2Quorum( - uint8 quorumNumber - ) external view returns (bool); - - /** - * @notice Whether operator sets mode is enabled. - * @return True if operator sets mode is enabled, false otherwise. - */ - function operatorSetsEnabled() external view returns (bool); - /// ACTIONS /** @@ -603,13 +586,6 @@ interface ISlashingRegistryCoordinator is address operator ) external view returns (BN254.G1Point memory); - /** - * @notice Returns the address of the contract owner. - * @return The owner's address. - * @dev The owner can update contract configuration and create new quorums. - */ - function owner() external view returns (address); - /** * @notice Returns the account identifier for this AVS (used for UAM integration in EigenLayer) * @dev NOTE: Updating this value will break existing OperatorSets and UAM integration. This value should only be set once. diff --git a/src/interfaces/ISocketRegistry.sol b/src/interfaces/ISocketRegistry.sol index c456a4f9..3235711a 100644 --- a/src/interfaces/ISocketRegistry.sol +++ b/src/interfaces/ISocketRegistry.sol @@ -1,7 +1,14 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.0; -interface ISocketRegistry { +interface ISocketRegistryErrors { + /// @notice Thrown when the caller is not the owner of the SlashingRegistryCoordinator + error OnlySlashingRegistryCoordinatorOwner(); + /// @notice Thrown when the caller is not the SlashingRegistryCoordinator + error OnlySlashingRegistryCoordinator(); +} + +interface ISocketRegistry is ISocketRegistryErrors { /// @notice sets the socket for an operator only callable by the RegistryCoordinator function setOperatorSocket(bytes32 _operatorId, string memory _socket) external; diff --git a/src/interfaces/IStakeRegistry.sol b/src/interfaces/IStakeRegistry.sol index b7e74644..aa8fc3a2 100644 --- a/src/interfaces/IStakeRegistry.sol +++ b/src/interfaces/IStakeRegistry.sol @@ -29,6 +29,8 @@ interface IStakeRegistryErrors { error InvalidBlockNumber(); /// @notice Thrown when attempting to access stake history that doesn't exist for a quorum. error EmptyStakeHistory(); + /// @notice Thrown when the quorum is not slashable and the caller attempts to set the look ahead period. + error QuorumNotSlashable(); } interface IStakeRegistryTypes { @@ -248,16 +250,7 @@ interface IStakeRegistry is IStakeRegistryErrors, IStakeRegistryEvents { ) external; /// VIEW - - /** - * @notice Returns whether a quorum is an operator set quorum. - * @param quorumNumber The quorum number to query. - * @return Whether the quorum is an operator set quorum. - */ - function isOperatorSetQuorum( - uint8 quorumNumber - ) external view returns (bool); - + /** * @notice Returns the minimum stake requirement for a quorum `quorumNumber`. * @dev In order to register for a quorum i, an operator must have at least `minimumStakeForQuorum[i]`. diff --git a/test/harnesses/RegistryCoordinatorHarness.t.sol b/test/harnesses/RegistryCoordinatorHarness.t.sol index 050fae43..8dfa6bf5 100644 --- a/test/harnesses/RegistryCoordinatorHarness.t.sol +++ b/test/harnesses/RegistryCoordinatorHarness.t.sol @@ -49,7 +49,13 @@ contract RegistryCoordinatorHarness is RegistryCoordinator, Test { string memory socket, SignatureWithSaltAndExpiry memory operatorSignature ) external returns (RegisterResults memory results) { - return _registerOperator(operator, operatorId, quorumNumbers, socket); + return _registerOperator({ + operator: operator, + operatorId: operatorId, + quorumNumbers: quorumNumbers, + socket: socket, + checkMaxOperatorCount: true + }); } // @notice exposes the internal `_deregisterOperator` function, overriding all access controls @@ -77,9 +83,9 @@ contract RegistryCoordinatorHarness is RegistryCoordinator, Test { operatorSetsEnabled = enabled; } - function setM2QuorumsDisabled( + function setM2QuorumRegistrationDisabled( bool disabled ) external { - m2QuorumsDisabled = disabled; + isM2QuorumRegistrationDisabled = disabled; } } diff --git a/test/utils/MockAVSDeployer.sol b/test/utils/MockAVSDeployer.sol index 1de4029b..bc9e91db 100644 --- a/test/utils/MockAVSDeployer.sol +++ b/test/utils/MockAVSDeployer.sol @@ -358,7 +358,7 @@ contract MockAVSDeployer is Test { operatorStateRetriever = new OperatorStateRetriever(); registryCoordinator.setOperatorSetsEnabled(false); - registryCoordinator.setM2QuorumsDisabled(false); + registryCoordinator.setM2QuorumRegistrationDisabled(false); } function _labelContracts() internal { From 6f7603b97ecc51e9abc84a6564b74de3dcf6d870 Mon Sep 17 00:00:00 2001 From: gpsanant Date: Sat, 1 Feb 2025 02:32:52 -0800 Subject: [PATCH 02/16] fix: quorum creation --- src/RegistryCoordinator.sol | 21 +++++++++++++-------- src/RegistryCoordinatorStorage.sol | 4 ++-- src/SlashingRegistryCoordinator.sol | 16 ++++++++++++++++ 3 files changed, 31 insertions(+), 10 deletions(-) diff --git a/src/RegistryCoordinator.sol b/src/RegistryCoordinator.sol index 1523acd6..bb68d4f5 100644 --- a/src/RegistryCoordinator.sol +++ b/src/RegistryCoordinator.sol @@ -139,7 +139,7 @@ contract RegistryCoordinator is RegistryCoordinatorStorage { /// @inheritdoc IRegistryCoordinator function disableM2QuorumRegistration() external onlyOwner { - require(operatorSetsEnabled, OperatorSetsNotEnabled()); + require(!isM2QuorumRegistrationDisabled, M2QuorumRegistrationIsDisabled()); isM2QuorumRegistrationDisabled = true; @@ -154,16 +154,21 @@ contract RegistryCoordinator is RegistryCoordinatorStorage { /// @dev override the _forceDeregisterOperator function to handle M2 quorum deregistration function _forceDeregisterOperator(address operator, bytes memory quorumNumbers) internal virtual override { - if (operatorSetsEnabled) { - // filter out M2 quorums from the quorum numbers - uint256 operatorSetBitmap = quorumNumbers.orderedBytesArrayToBitmap().minus(m2QuorumBitmap); - if (!operatorSetBitmap.isEmpty()) { - // call the parent _forceDeregisterOperator function for operator sets quorums - super._forceDeregisterOperator(operator, operatorSetBitmap.bitmapToBytesArray()); - } + // filter out M2 quorums from the quorum numbers + uint256 operatorSetBitmap = quorumNumbers.orderedBytesArrayToBitmap().minus(m2QuorumBitmap); + if (!operatorSetBitmap.isEmpty()) { + // call the parent _forceDeregisterOperator function for operator sets quorums + super._forceDeregisterOperator(operator, operatorSetBitmap.bitmapToBytesArray()); } } + /// @dev Hook to prevent any new quorums from being created if operator sets are not enabled + function _beforeCreateQuorum( + uint8 quorumNumber + ) internal virtual override { + require(operatorSetsEnabled, OperatorSetsNotEnabled()); + } + /// @dev Hook to allow for any post-deregister logic function _afterDeregisterOperator( address operator, diff --git a/src/RegistryCoordinatorStorage.sol b/src/RegistryCoordinatorStorage.sol index 908fa800..b5b37c7a 100644 --- a/src/RegistryCoordinatorStorage.sol +++ b/src/RegistryCoordinatorStorage.sol @@ -29,8 +29,8 @@ abstract contract RegistryCoordinatorStorage is IRegistryCoordinator, SlashingRe * */ - /// @notice Whether this AVS allows operator sets for registration - /// @dev If true, operators may register to operator sets via the AllocationManager + /// @notice Whether this AVS allows operator sets for creation/registration + /// @dev If true, then operator sets may be created and operators may register to operator sets via the AllocationManager bool public operatorSetsEnabled; /// @notice Whether this AVS allows M2 quorums for registration diff --git a/src/SlashingRegistryCoordinator.sol b/src/SlashingRegistryCoordinator.sol index 0d08e62e..e603c7a6 100644 --- a/src/SlashingRegistryCoordinator.sol +++ b/src/SlashingRegistryCoordinator.sol @@ -770,6 +770,9 @@ contract SlashingRegistryCoordinator is // The previous count is the new quorum's number uint8 quorumNumber = prevQuorumCount; + // Hook to allow for any pre-create quorum logic + _beforeCreateQuorum(quorumNumber); + // Initialize the quorum here and in each registry _setOperatorSetParams(quorumNumber, operatorSetParams); @@ -801,6 +804,9 @@ contract SlashingRegistryCoordinator is indexRegistry.initializeQuorum(quorumNumber); blsApkRegistry.initializeQuorum(quorumNumber); + + // Hook to allow for any post-create quorum logic + _afterCreateQuorum(quorumNumber); } /** @@ -883,6 +889,16 @@ contract SlashingRegistryCoordinator is accountIdentifier = _accountIdentifier; } + /// @dev Hook to allow for any pre-create quorum logic + function _beforeCreateQuorum( + uint8 quorumNumber + ) internal virtual {} + + /// @dev Hook to allow for any post-create quorum logic + function _afterCreateQuorum( + uint8 quorumNumber + ) internal virtual {} + /// @dev Hook to allow for any pre-register logic in `_registerOperator` function _beforeRegisterOperator( address operator, From ce20d8cb5a88505f8b76f3bf6fa4bf0e5afeb0d9 Mon Sep 17 00:00:00 2001 From: gpsanant Date: Sat, 1 Feb 2025 02:37:31 -0800 Subject: [PATCH 03/16] fix: bug in registration --- src/RegistryCoordinator.sol | 4 +++- src/interfaces/IRegistryCoordinator.sol | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/RegistryCoordinator.sol b/src/RegistryCoordinator.sol index bb68d4f5..a032343b 100644 --- a/src/RegistryCoordinator.sol +++ b/src/RegistryCoordinator.sol @@ -62,6 +62,7 @@ contract RegistryCoordinator is RegistryCoordinatorStorage { SignatureWithSaltAndExpiry memory operatorSignature ) external onlyWhenNotPaused(PAUSED_REGISTER_OPERATOR) { require(!isM2QuorumRegistrationDisabled, M2QuorumRegistrationIsDisabled()); + require(quorumNumbers.orderedBytesArrayToBitmap().isSubsetOf(m2QuorumBitmap), OnlyM2QuorumsAllowed()); // Check if the operator has registered before bool operatorRegisteredBefore = _operatorInfo[msg.sender].status == OperatorStatus.REGISTERED; @@ -91,7 +92,8 @@ contract RegistryCoordinator is RegistryCoordinatorStorage { SignatureWithSaltAndExpiry memory operatorSignature ) external onlyWhenNotPaused(PAUSED_REGISTER_OPERATOR) { require(!isM2QuorumRegistrationDisabled, M2QuorumRegistrationIsDisabled()); - + require(quorumNumbers.orderedBytesArrayToBitmap().isSubsetOf(m2QuorumBitmap), OnlyM2QuorumsAllowed()); + // Check if the operator has registered before bool operatorRegisteredBefore = _operatorInfo[msg.sender].status == OperatorStatus.REGISTERED; diff --git a/src/interfaces/IRegistryCoordinator.sol b/src/interfaces/IRegistryCoordinator.sol index 10d3c1ef..a8b3939b 100644 --- a/src/interfaces/IRegistryCoordinator.sol +++ b/src/interfaces/IRegistryCoordinator.sol @@ -20,6 +20,8 @@ interface IRegistryCoordinatorErrors is ISlashingRegistryCoordinatorErrors { error M2QuorumRegistrationIsDisabled(); /// @notice Thrown when operator set operations are attempted while not enabled. error OperatorSetsNotEnabled(); + /// @notice Thrown when only M2 quorums are allowed. + error OnlyM2QuorumsAllowed(); } interface IRegistryCoordinatorTypes is ISlashingRegistryCoordinatorTypes {} From 27668d5073a727db7a0cf3c760456ac7712bc05d Mon Sep 17 00:00:00 2001 From: gpsanant Date: Sun, 2 Feb 2025 01:35:43 -0800 Subject: [PATCH 04/16] feat: refactor avs sync --- src/BLSSignatureChecker.sol | 5 +- src/RegistryCoordinator.sol | 27 ++- src/RegistryCoordinatorStorage.sol | 7 +- src/SlashingRegistryCoordinator.sol | 87 +++------ src/StakeRegistry.sol | 133 ++++++++------ .../ISlashingRegistryCoordinator.sol | 10 -- src/interfaces/IStakeRegistry.sol | 24 +-- .../RegistryCoordinatorHarness.t.sol | 9 - test/integration/User.t.sol | 20 ++- test/mocks/StakeRegistryMock.sol | 12 +- test/unit/RegistryCoordinatorUnit.t.sol | 170 +++++++++--------- test/unit/StakeRegistryUnit.t.sol | 69 +++++-- 12 files changed, 302 insertions(+), 271 deletions(-) diff --git a/src/BLSSignatureChecker.sol b/src/BLSSignatureChecker.sol index ff3e12a4..3f5b2444 100644 --- a/src/BLSSignatureChecker.sol +++ b/src/BLSSignatureChecker.sol @@ -20,7 +20,10 @@ contract BLSSignatureChecker is BLSSignatureCheckerStorage { /// MODIFIERS modifier onlyCoordinatorOwner() { - require(msg.sender == Ownable(address(registryCoordinator)).owner(), OnlyRegistryCoordinatorOwner()); + require( + msg.sender == Ownable(address(registryCoordinator)).owner(), + OnlyRegistryCoordinatorOwner() + ); _; } diff --git a/src/RegistryCoordinator.sol b/src/RegistryCoordinator.sol index a032343b..c9719281 100644 --- a/src/RegistryCoordinator.sol +++ b/src/RegistryCoordinator.sol @@ -45,7 +45,7 @@ contract RegistryCoordinator is RegistryCoordinatorStorage { _socketRegistry, _allocationManager, _pauserRegistry - ) + ) {} /** @@ -62,10 +62,14 @@ contract RegistryCoordinator is RegistryCoordinatorStorage { SignatureWithSaltAndExpiry memory operatorSignature ) external onlyWhenNotPaused(PAUSED_REGISTER_OPERATOR) { require(!isM2QuorumRegistrationDisabled, M2QuorumRegistrationIsDisabled()); - require(quorumNumbers.orderedBytesArrayToBitmap().isSubsetOf(m2QuorumBitmap), OnlyM2QuorumsAllowed()); - + require( + quorumNumbers.orderedBytesArrayToBitmap().isSubsetOf(m2QuorumBitmap), + OnlyM2QuorumsAllowed() + ); + // Check if the operator has registered before - bool operatorRegisteredBefore = _operatorInfo[msg.sender].status == OperatorStatus.REGISTERED; + bool operatorRegisteredBefore = + _operatorInfo[msg.sender].status == OperatorStatus.REGISTERED; // register the operator with the registry coordinator _registerOperator({ @@ -92,10 +96,14 @@ contract RegistryCoordinator is RegistryCoordinatorStorage { SignatureWithSaltAndExpiry memory operatorSignature ) external onlyWhenNotPaused(PAUSED_REGISTER_OPERATOR) { require(!isM2QuorumRegistrationDisabled, M2QuorumRegistrationIsDisabled()); - require(quorumNumbers.orderedBytesArrayToBitmap().isSubsetOf(m2QuorumBitmap), OnlyM2QuorumsAllowed()); - + require( + quorumNumbers.orderedBytesArrayToBitmap().isSubsetOf(m2QuorumBitmap), + OnlyM2QuorumsAllowed() + ); + // Check if the operator has registered before - bool operatorRegisteredBefore = _operatorInfo[msg.sender].status == OperatorStatus.REGISTERED; + bool operatorRegisteredBefore = + _operatorInfo[msg.sender].status == OperatorStatus.REGISTERED; // register the operator with the registry coordinator with churn _registerOperatorWithChurn({ @@ -155,7 +163,10 @@ contract RegistryCoordinator is RegistryCoordinatorStorage { */ /// @dev override the _forceDeregisterOperator function to handle M2 quorum deregistration - function _forceDeregisterOperator(address operator, bytes memory quorumNumbers) internal virtual override { + function _forceDeregisterOperator( + address operator, + bytes memory quorumNumbers + ) internal virtual override { // filter out M2 quorums from the quorum numbers uint256 operatorSetBitmap = quorumNumbers.orderedBytesArrayToBitmap().minus(m2QuorumBitmap); if (!operatorSetBitmap.isEmpty()) { diff --git a/src/RegistryCoordinatorStorage.sol b/src/RegistryCoordinatorStorage.sol index b5b37c7a..5d687959 100644 --- a/src/RegistryCoordinatorStorage.sol +++ b/src/RegistryCoordinatorStorage.sol @@ -13,7 +13,10 @@ import {ISocketRegistry} from "./interfaces/ISocketRegistry.sol"; import {SlashingRegistryCoordinator} from "./SlashingRegistryCoordinator.sol"; -abstract contract RegistryCoordinatorStorage is IRegistryCoordinator, SlashingRegistryCoordinator { +abstract contract RegistryCoordinatorStorage is + IRegistryCoordinator, + SlashingRegistryCoordinator +{ /** * * CONSTANTS AND IMMUTABLES @@ -63,4 +66,4 @@ abstract contract RegistryCoordinatorStorage is IRegistryCoordinator, SlashingRe } uint256[47] private __GAP; -} \ No newline at end of file +} diff --git a/src/SlashingRegistryCoordinator.sol b/src/SlashingRegistryCoordinator.sol index e603c7a6..6f021a96 100644 --- a/src/SlashingRegistryCoordinator.sol +++ b/src/SlashingRegistryCoordinator.sol @@ -225,22 +225,6 @@ contract SlashingRegistryCoordinator is _deregisterOperator(operator, quorumNumbers); } - /// @inheritdoc ISlashingRegistryCoordinator - function updateOperators( - address[] memory operators - ) external onlyWhenNotPaused(PAUSED_UPDATE_OPERATOR) { - for (uint256 i = 0; i < operators.length; i++) { - address operator = operators[i]; - OperatorInfo memory operatorInfo = _operatorInfo[operator]; - bytes32 operatorId = operatorInfo.operatorId; - - // Update the operator's stake for their active quorums - uint192 currentBitmap = _currentOperatorBitmap(operatorId); - bytes memory quorumsToUpdate = BitmapUtils.bitmapToBytesArray(currentBitmap); - _updateOperator(operator, operatorInfo, quorumsToUpdate); - } - } - /// @inheritdoc ISlashingRegistryCoordinator function updateOperatorsForQuorum( address[][] memory operatorsPerQuorum, @@ -264,6 +248,7 @@ contract SlashingRegistryCoordinator is QuorumOperatorCountMismatch() ); + bytes32[] memory operatorIds = new bytes32[](currQuorumOperators.length); address prevOperatorAddress = address(0); // For each operator: // - check that they are registered for this quorum @@ -272,11 +257,9 @@ contract SlashingRegistryCoordinator is for (uint256 j = 0; j < currQuorumOperators.length; ++j) { address operator = currQuorumOperators[j]; - OperatorInfo memory operatorInfo = _operatorInfo[operator]; - bytes32 operatorId = operatorInfo.operatorId; - + operatorIds[j] = _operatorInfo[operator].operatorId; { - uint192 currentBitmap = _currentOperatorBitmap(operatorId); + uint192 currentBitmap = _currentOperatorBitmap(operatorIds[j]); // Check that the operator is registered require( BitmapUtils.isSet(currentBitmap, quorumNumber), NotRegisteredForQuorum() @@ -285,10 +268,17 @@ contract SlashingRegistryCoordinator is require(operator > prevOperatorAddress, NotSorted()); } - // Update the operator - _updateOperator(operator, operatorInfo, quorumNumbers[i:i + 1]); prevOperatorAddress = operator; } + bytes memory quorumNumberBytes = new bytes(1); + quorumNumberBytes[0] = bytes1(quorumNumber); + bool[] memory shouldBeDeregistered = + stakeRegistry.updateOperatorsStake(currQuorumOperators, operatorIds, quorumNumber); + for (uint256 j = 0; j < currQuorumOperators.length; ++j) { + if (shouldBeDeregistered[j]) { + _deregisterOperator(currQuorumOperators[j], quorumNumberBytes); + } + } // Update timestamp that all operators in quorum have been updated all at once quorumUpdateBlockNumber[quorumNumber] = block.number; @@ -439,7 +429,10 @@ contract SlashingRegistryCoordinator is if (checkMaxOperatorCount) { for (uint256 i = 0; i < quorumNumbers.length; i++) { OperatorSetParam memory operatorSetParams = _quorumParams[uint8(quorumNumbers[i])]; - require(results.numOperatorsPerQuorum[i] <= operatorSetParams.maxOperatorCount, MaxQuorumsReached()); + require( + results.numOperatorsPerQuorum[i] <= operatorSetParams.maxOperatorCount, + MaxQuorumsReached() + ); } } @@ -469,14 +462,13 @@ contract SlashingRegistryCoordinator is // Register the operator in each of the registry contracts and update the operator's // quorum bitmap and registration status - RegisterResults memory results = - _registerOperator({ - operator: operator, - operatorId: operatorId, - quorumNumbers: quorumNumbers, - socket: socket, - checkMaxOperatorCount: false - }); + RegisterResults memory results = _registerOperator({ + operator: operator, + operatorId: operatorId, + quorumNumbers: quorumNumbers, + socket: socket, + checkMaxOperatorCount: false + }); // Check that each quorum's operator count is below the configured maximum. If the max // is exceeded, use `operatorKickParams` to deregister an existing operator to make space @@ -563,7 +555,10 @@ contract SlashingRegistryCoordinator is * @param operator The operator to deregister * @param quorumNumbers The quorum numbers the operator is force-deregistered from */ - function _forceDeregisterOperator(address operator, bytes memory quorumNumbers) internal virtual { + function _forceDeregisterOperator( + address operator, + bytes memory quorumNumbers + ) internal virtual { allocationManager.deregisterFromOperatorSets( IAllocationManagerTypes.DeregisterParams({ operator: operator, @@ -660,32 +655,6 @@ contract SlashingRegistryCoordinator is ); } - /** - * @notice Updates the StakeRegistry's view of the operator's stake in one or more quorums. - * For any quorums where the StakeRegistry finds the operator is under the configured minimum - * stake, `quorumsToRemove` is returned and used to deregister the operator from those quorums - * @dev does nothing if operator is not registered for any quorums. - */ - function _updateOperator( - address operator, - OperatorInfo memory operatorInfo, - bytes memory quorumsToUpdate - ) internal { - if (operatorInfo.status != OperatorStatus.REGISTERED) { - return; - } - bytes32 operatorId = operatorInfo.operatorId; - uint192 quorumsToRemove = - stakeRegistry.updateOperatorStake(operator, operatorId, quorumsToUpdate); - - if (!quorumsToRemove.isEmpty()) { - _deregisterOperator({ - operator: operator, - quorumNumbers: BitmapUtils.bitmapToBytesArray(quorumsToRemove) - }); - } - } - /** * @notice Returns the stake threshold required for an incoming operator to replace an existing operator * The incoming operator must have more stake than the return value. @@ -898,7 +867,7 @@ contract SlashingRegistryCoordinator is function _afterCreateQuorum( uint8 quorumNumber ) internal virtual {} - + /// @dev Hook to allow for any pre-register logic in `_registerOperator` function _beforeRegisterOperator( address operator, diff --git a/src/StakeRegistry.sol b/src/StakeRegistry.sol index c485cc0e..318fa1fc 100644 --- a/src/StakeRegistry.sol +++ b/src/StakeRegistry.sol @@ -125,12 +125,12 @@ contract StakeRegistry is StakeRegistryStorage { } /// @inheritdoc IStakeRegistry - function updateOperatorStake( - address operator, - bytes32 operatorId, - bytes calldata quorumNumbers - ) external onlySlashingRegistryCoordinator returns (uint192) { - uint192 quorumsToRemove; + function updateOperatorsStake( + address[] memory operators, + bytes32[] memory operatorIds, + uint8 quorumNumber + ) external onlySlashingRegistryCoordinator returns (bool[] memory) { + bool[] memory shouldBeDeregistered = new bool[](operators.length); /** * For each quorum, update the operator's stake and record the delta @@ -140,34 +140,37 @@ contract StakeRegistry is StakeRegistryStorage { * in the quorum, the quorum number is added to `quorumsToRemove`, which * is returned to the registry coordinator. */ - for (uint256 i = 0; i < quorumNumbers.length; i++) { - uint8 quorumNumber = uint8(quorumNumbers[i]); - _checkQuorumExists(quorumNumber); + _checkQuorumExists(quorumNumber); - // Fetch the operator's current stake, applying weighting parameters and checking - // against the minimum stake requirements for the quorum. - (uint96 stakeWeight, bool hasMinimumStake) = - _weightOfOperatorForQuorum(quorumNumber, operator); - // If the operator no longer meets the minimum stake, set their stake to zero and mark them for removal - /// also handle setting the operator's stake to 0 and remove them from the quorum - if (!hasMinimumStake) { - stakeWeight = 0; - quorumsToRemove = uint192(quorumsToRemove.setBit(quorumNumber)); + // Fetch the operators' current stake, applying weighting parameters and checking + // against the minimum stake requirements for the quorum. + (uint96[] memory stakeWeights, bool[] memory hasMinimumStakes) = + _weightOfOperatorsForQuorum(quorumNumber, operators); + + int256 totalStakeDelta = 0; + // If the operator no longer meets the minimum stake, set their stake to zero and mark them for removal + /// also handle setting the operator's stake to 0 and remove them from the quorum + for (uint256 i = 0; i < operators.length; i++) { + if (!hasMinimumStakes[i]) { + stakeWeights[i] = 0; + shouldBeDeregistered[i] = true; } // Update the operator's stake and retrieve the delta // If we're deregistering them, their weight is set to 0 int256 stakeDelta = _recordOperatorStakeUpdate({ - operatorId: operatorId, + operatorId: operatorIds[i], quorumNumber: quorumNumber, - newStake: stakeWeight + newStake: stakeWeights[i] }); - // Apply the delta to the quorum's total stake - _recordTotalStakeUpdate(quorumNumber, stakeDelta); + totalStakeDelta += stakeDelta; } - return quorumsToRemove; + // Apply the delta to the quorum's total stake + _recordTotalStakeUpdate(quorumNumber, totalStakeDelta); + + return shouldBeDeregistered; } /// @inheritdoc IStakeRegistry @@ -504,65 +507,80 @@ contract StakeRegistry is StakeRegistryStorage { ); } - /// Returns total Slashable stake for an operator per strategy that can have the weights applied based on strategy multipliers + /// Returns total Slashable stake for a list of operators per strategy that can have the weights applied based on strategy multipliers function _getSlashableStakePerStrategy( uint8 quorumNumber, - address operator - ) internal view returns (uint256[] memory) { - address[] memory operators = new address[](1); - operators[0] = operator; - uint32 beforeBlock = - uint32(block.number + slashableStakeLookAheadPerQuorum[quorumNumber]); + address[] memory operators + ) internal view returns (uint256[][] memory) { + uint32 beforeBlock = uint32(block.number + slashableStakeLookAheadPerQuorum[quorumNumber]); uint256[][] memory slashableShares = allocationManager.getMinimumSlashableStake( - OperatorSet( - registryCoordinator.accountIdentifier(), quorumNumber - ), + OperatorSet(registryCoordinator.accountIdentifier(), quorumNumber), operators, strategiesPerQuorum[quorumNumber], beforeBlock ); - return slashableShares[0]; + return slashableShares; } /** - * @notice This function computes the total weight of the @param operator in the quorum @param quorumNumber. + * @notice This function computes the total weight of the @param operators in the quorum @param quorumNumber. * @dev this method DOES NOT check that the quorum exists - * @return `uint96` The weighted sum of the operator's shares across each strategy considered by the quorum - * @return `bool` True if the operator meets the quorum's minimum stake + * @return `uint96[] memory` The weighted sum of the operators' shares across each strategy considered by the quorum + * @return `bool[] memory` True if the respective operator meets the quorum's minimum stake */ - function _weightOfOperatorForQuorum( + function _weightOfOperatorsForQuorum( uint8 quorumNumber, - address operator - ) internal view virtual returns (uint96, bool) { - uint96 weight; + address[] memory operators + ) internal view virtual returns (uint96[] memory, bool[] memory) { + uint96[] memory weights; + bool[] memory hasMinimumStakes; + uint256 stratsLength = strategyParamsLength(quorumNumber); StrategyParams memory strategyAndMultiplier; - uint256[] memory strategyShares; + uint256[][] memory strategyShares; if (stakeTypePerQuorum[quorumNumber] == IStakeRegistryTypes.StakeType.TOTAL_SLASHABLE) { - // get slashable stake for the operator from AllocationManager - strategyShares = _getSlashableStakePerStrategy(quorumNumber, operator); + // get slashable stake for the operators from AllocationManager + strategyShares = _getSlashableStakePerStrategy(quorumNumber, operators); } else { - // get delegated stake for the operator from DelegationManager - strategyShares = delegation.getOperatorShares(operator, strategiesPerQuorum[quorumNumber]); + // get delegated stake for the operators from DelegationManager + strategyShares = + delegation.getOperatorsShares(operators, strategiesPerQuorum[quorumNumber]); } - for (uint256 i = 0; i < stratsLength; i++) { - // accessing i'th StrategyParams struct for the quorumNumber - strategyAndMultiplier = strategyParams[quorumNumber][i]; + for (uint256 stratIndex = 0; stratIndex < stratsLength; stratIndex++) { + // accessing i'th StrategyParams struct for the quorumNumber + strategyAndMultiplier = strategyParams[quorumNumber][stratIndex]; + + for (uint256 opIndex = 0; opIndex < operators.length; opIndex++) { // add the weight from the shares for this strategy to the total weight - if (strategyShares[i] > 0) { - weight += uint96( - strategyShares[i] * strategyAndMultiplier.multiplier / WEIGHTING_DIVISOR + if (strategyShares[opIndex][stratIndex] > 0) { + weights[opIndex] += uint96( + strategyShares[opIndex][stratIndex] * strategyAndMultiplier.multiplier + / WEIGHTING_DIVISOR ); + } } + + // check if the operator meets the quorum's minimum stake + hasMinimumStakes[stratIndex] = + weights[stratIndex] >= minimumStakeForQuorum[quorumNumber]; } - // return the weight, and `true` if the operator meets the quorum's minimum stake - bool hasMinimumStake = weight >= minimumStakeForQuorum[quorumNumber]; - return (weight, hasMinimumStake); + return (weights, hasMinimumStakes); + } + + function _weightOfOperatorForQuorum( + uint8 quorumNumber, + address operator + ) internal view virtual returns (uint96, bool) { + address[] memory operators = new address[](1); + operators[0] = operator; + (uint96[] memory weights, bool[] memory hasMinimumStakes) = + _weightOfOperatorsForQuorum(quorumNumber, operators); + return (weights[0], hasMinimumStakes[0]); } /// @notice Returns `true` if the quorum has been initialized @@ -772,7 +790,10 @@ contract StakeRegistry is StakeRegistryStorage { * @param _lookAheadBlocks The number of blocks to look ahead when checking shares */ function _setLookAheadPeriod(uint8 quorumNumber, uint32 _lookAheadBlocks) internal { - require(stakeTypePerQuorum[quorumNumber] == IStakeRegistryTypes.StakeType.TOTAL_SLASHABLE, QuorumNotSlashable()); + require( + stakeTypePerQuorum[quorumNumber] == IStakeRegistryTypes.StakeType.TOTAL_SLASHABLE, + QuorumNotSlashable() + ); uint32 oldLookAheadDays = slashableStakeLookAheadPerQuorum[quorumNumber]; slashableStakeLookAheadPerQuorum[quorumNumber] = _lookAheadBlocks; emit LookAheadPeriodChanged(oldLookAheadDays, _lookAheadBlocks); diff --git a/src/interfaces/ISlashingRegistryCoordinator.sol b/src/interfaces/ISlashingRegistryCoordinator.sol index 8aaac488..ac9a0b53 100644 --- a/src/interfaces/ISlashingRegistryCoordinator.sol +++ b/src/interfaces/ISlashingRegistryCoordinator.sol @@ -321,16 +321,6 @@ interface ISlashingRegistryCoordinator is */ function deregisterOperator(address operator, uint32[] memory operatorSetIds) external; - /** - * @notice Updates stake weights for specified operators. If any operator is found to be below - * the minimum stake for their registered quorums, they are deregistered from those quorums. - * @param operators The operators whose stakes should be updated. - * @dev Stakes are queried from the Eigenlayer core DelegationManager contract. - */ - function updateOperators( - address[] memory operators - ) external; - /** * @notice For each quorum in `quorumNumbers`, updates the StakeRegistry's view of ALL its registered operators' stakes. * Each quorum's `quorumUpdateBlockNumber` is also updated, which tracks the most recent block number when ALL registered diff --git a/src/interfaces/IStakeRegistry.sol b/src/interfaces/IStakeRegistry.sol index aa8fc3a2..5e47deb5 100644 --- a/src/interfaces/IStakeRegistry.sol +++ b/src/interfaces/IStakeRegistry.sol @@ -166,17 +166,17 @@ interface IStakeRegistry is IStakeRegistryErrors, IStakeRegistryEvents { function deregisterOperator(bytes32 operatorId, bytes memory quorumNumbers) external; /** - * @notice Called by the registry coordinator to update an operator's stake for one or more quorums. - * @param operator The address of the operator to update. - * @param operatorId The id of the operator to update. - * @param quorumNumbers The quorum numbers to update the stake for. - * @return A bitmap of quorums where the operator no longer meets the minimum stake and should be deregistered. - */ - function updateOperatorStake( - address operator, - bytes32 operatorId, - bytes calldata quorumNumbers - ) external returns (uint192); + * @notice Called by the registry coordinator to update the stake of a list of operators for a specific quorum. + * @param operators The addresses of the operators to update. + * @param operatorIds The ids of the operators to update. + * @param quorumNumber The quorum number to update the stake for. + * @return A list of bools, true if the corresponding operator should be deregistered since they no longer meet the minimum stake requirement. + */ + function updateOperatorsStake( + address[] memory operators, + bytes32[] memory operatorIds, + uint8 quorumNumber + ) external returns (bool[] memory); /** * @notice Initialize a new quorum created by the registry coordinator by setting strategies, weights, and minimum stake. @@ -250,7 +250,7 @@ interface IStakeRegistry is IStakeRegistryErrors, IStakeRegistryEvents { ) external; /// VIEW - + /** * @notice Returns the minimum stake requirement for a quorum `quorumNumber`. * @dev In order to register for a quorum i, an operator must have at least `minimumStakeForQuorum[i]`. diff --git a/test/harnesses/RegistryCoordinatorHarness.t.sol b/test/harnesses/RegistryCoordinatorHarness.t.sol index 8dfa6bf5..82211204 100644 --- a/test/harnesses/RegistryCoordinatorHarness.t.sol +++ b/test/harnesses/RegistryCoordinatorHarness.t.sol @@ -63,15 +63,6 @@ contract RegistryCoordinatorHarness is RegistryCoordinator, Test { _deregisterOperator(operator, quorumNumbers); } - // @notice exposes the internal `_updateOperator` function, overriding all access controls - function _updateOperatorExternal( - address operator, - OperatorInfo memory operatorInfo, - bytes memory quorumsToUpdate - ) external { - _updateOperator(operator, operatorInfo, quorumsToUpdate); - } - // @notice exposes the internal `_updateOperatorBitmap` function, overriding all access controls function _updateOperatorBitmapExternal(bytes32 operatorId, uint192 quorumBitmap) external { _updateOperatorBitmap(operatorId, quorumBitmap); diff --git a/test/integration/User.t.sol b/test/integration/User.t.sol index 8a1a9b61..f69eb84e 100644 --- a/test/integration/User.t.sol +++ b/test/integration/User.t.sol @@ -240,10 +240,22 @@ contract User is Test { function updateStakes() public virtual createSnapshot { _log("updateStakes (updateOperators)"); - address[] memory addrs = new address[](1); - addrs[0] = address(this); - - registryCoordinator.updateOperators(addrs); + // get all quorums this operator is registered for + uint192 currentBitmap = registryCoordinator.getCurrentQuorumBitmap(operatorId); + bytes memory quorumNumbers = currentBitmap.bitmapToBytesArray(); + + // get all operators in those quorums + address[][] memory operatorsPerQuorum = new address[][](quorumNumbers.length); + for (uint256 i = 0; i < quorumNumbers.length; i++) { + bytes32[] memory operatorIds = indexRegistry.getOperatorListAtBlockNumber( + uint8(quorumNumbers[i]), uint32(block.number) + ); + operatorsPerQuorum[i] = new address[](operatorIds.length); + for (uint256 j = 0; j < operatorIds.length; j++) { + operatorsPerQuorum[i][j] = blsApkRegistry.pubkeyHashToOperator(operatorIds[j]); + } + } + registryCoordinator.updateOperatorsForQuorum(operatorsPerQuorum, quorumNumbers); } /** diff --git a/test/mocks/StakeRegistryMock.sol b/test/mocks/StakeRegistryMock.sol index f428928e..9ddfa002 100644 --- a/test/mocks/StakeRegistryMock.sol +++ b/test/mocks/StakeRegistryMock.sol @@ -264,12 +264,12 @@ contract StakeRegistryMock is IStakeRegistry { * If the operator no longer has the minimum stake required for a quorum, they are * added to the */ - function updateOperatorStake( - address, /*operator*/ - bytes32, /*operatorId*/ - bytes calldata /*quorumNumbers*/ - ) external returns (uint192) { - return updateOperatorStakeReturnBitmap; + function updateOperatorsStake( + address[] memory operators, + bytes32[] memory, + uint8 + ) external pure returns (bool[] memory) { + return new bool[](operators.length); } function getMockOperatorId( diff --git a/test/unit/RegistryCoordinatorUnit.t.sol b/test/unit/RegistryCoordinatorUnit.t.sol index 3e0208bc..bd303cff 100644 --- a/test/unit/RegistryCoordinatorUnit.t.sol +++ b/test/unit/RegistryCoordinatorUnit.t.sol @@ -1953,91 +1953,91 @@ contract RegistryCoordinatorUnitTests_RegisterOperatorWithChurn is RegistryCoord } contract RegistryCoordinatorUnitTests_UpdateOperators is RegistryCoordinatorUnitTests { - function test_updateOperators_revert_paused() public { - cheats.prank(pauser); - registryCoordinator.pause(2 ** PAUSED_UPDATE_OPERATOR); - - address[] memory operatorsToUpdate = new address[](1); - operatorsToUpdate[0] = defaultOperator; - - cheats.expectRevert(bytes4(keccak256("CurrentlyPaused()"))); - registryCoordinator.updateOperators(operatorsToUpdate); - } - - // @notice tests the `updateOperators` function with a single registered operator as input - function test_updateOperators_singleOperator() public { - // register the default operator - ISignatureUtils.SignatureWithSaltAndExpiry memory emptySig; - uint32 registrationBlockNumber = 100; - bytes memory quorumNumbers = new bytes(1); - quorumNumbers[0] = bytes1(defaultQuorumNumber); - _setOperatorWeight(defaultOperator, uint8(quorumNumbers[0]), defaultStake); - cheats.startPrank(defaultOperator); - cheats.roll(registrationBlockNumber); - registryCoordinator.registerOperator( - quorumNumbers, defaultSocket, pubkeyRegistrationParams, emptySig - ); - - address[] memory operatorsToUpdate = new address[](1); - operatorsToUpdate[0] = defaultOperator; - - registryCoordinator.updateOperators(operatorsToUpdate); - } - - // @notice tests the `updateOperators` function with a single registered operator as input - // @dev also sets up return data from the StakeRegistry - function testFuzz_updateOperators_singleOperator( - uint192 registrationBitmap, - uint192 mockReturnData - ) public { - // filter fuzzed inputs to only valid inputs - cheats.assume(registrationBitmap != 0); - mockReturnData = (mockReturnData & registrationBitmap); - emit log_named_uint("mockReturnData", mockReturnData); - - // register the default operator - ISignatureUtils.SignatureWithSaltAndExpiry memory emptySig; - uint32 registrationBlockNumber = 100; - bytes memory quorumNumbers = BitmapUtils.bitmapToBytesArray(registrationBitmap); - for (uint256 i = 0; i < quorumNumbers.length; ++i) { - _setOperatorWeight(defaultOperator, uint8(quorumNumbers[i]), defaultStake); - } - cheats.startPrank(defaultOperator); - cheats.roll(registrationBlockNumber); - registryCoordinator.registerOperator( - quorumNumbers, defaultSocket, pubkeyRegistrationParams, emptySig - ); - - address[] memory operatorsToUpdate = new address[](1); - operatorsToUpdate[0] = defaultOperator; - - uint192 quorumBitmapBefore = registryCoordinator.getCurrentQuorumBitmap(defaultOperatorId); - assertEq(quorumBitmapBefore, registrationBitmap, "operator bitmap somehow incorrect"); - - // make the stake registry return info that the operator should be removed from quorums - uint192 quorumBitmapToRemove = mockReturnData; - bytes memory quorumNumbersToRemove = BitmapUtils.bitmapToBytesArray(quorumBitmapToRemove); - for (uint256 i = 0; i < quorumNumbersToRemove.length; ++i) { - _setOperatorWeight(defaultOperator, uint8(quorumNumbersToRemove[i]), 0); - } - uint256 expectedQuorumBitmap = BitmapUtils.minus(quorumBitmapBefore, quorumBitmapToRemove); - - registryCoordinator.updateOperators(operatorsToUpdate); - uint192 quorumBitmapAfter = registryCoordinator.getCurrentQuorumBitmap(defaultOperatorId); - assertEq(expectedQuorumBitmap, quorumBitmapAfter, "quorum bitmap did not update correctly"); - } - - // @notice tests the `updateOperators` function with a single *un*registered operator as input - function test_updateOperators_unregisteredOperator() public view { - address[] memory operatorsToUpdate = new address[](1); - operatorsToUpdate[0] = defaultOperator; - - // force a staticcall to the `updateOperators` function -- this should *pass* because the call should be a strict no-op! - (bool success,) = address(registryCoordinator).staticcall( - abi.encodeWithSignature("updateOperators(address[])", operatorsToUpdate) - ); - require(success, "staticcall failed!"); - } + // function test_updateOperators_revert_paused() public { + // cheats.prank(pauser); + // registryCoordinator.pause(2 ** PAUSED_UPDATE_OPERATOR); + + // address[] memory operatorsToUpdate = new address[](1); + // operatorsToUpdate[0] = defaultOperator; + + // cheats.expectRevert(bytes4(keccak256("CurrentlyPaused()"))); + // registryCoordinator.updateOperators(operatorsToUpdate); + // } + + // // @notice tests the `updateOperators` function with a single registered operator as input + // function test_updateOperators_singleOperator() public { + // // register the default operator + // ISignatureUtils.SignatureWithSaltAndExpiry memory emptySig; + // uint32 registrationBlockNumber = 100; + // bytes memory quorumNumbers = new bytes(1); + // quorumNumbers[0] = bytes1(defaultQuorumNumber); + // _setOperatorWeight(defaultOperator, uint8(quorumNumbers[0]), defaultStake); + // cheats.startPrank(defaultOperator); + // cheats.roll(registrationBlockNumber); + // registryCoordinator.registerOperator( + // quorumNumbers, defaultSocket, pubkeyRegistrationParams, emptySig + // ); + + // address[] memory operatorsToUpdate = new address[](1); + // operatorsToUpdate[0] = defaultOperator; + + // registryCoordinator.updateOperators(operatorsToUpdate); + // } + + // // @notice tests the `updateOperators` function with a single registered operator as input + // // @dev also sets up return data from the StakeRegistry + // function testFuzz_updateOperators_singleOperator( + // uint192 registrationBitmap, + // uint192 mockReturnData + // ) public { + // // filter fuzzed inputs to only valid inputs + // cheats.assume(registrationBitmap != 0); + // mockReturnData = (mockReturnData & registrationBitmap); + // emit log_named_uint("mockReturnData", mockReturnData); + + // // register the default operator + // ISignatureUtils.SignatureWithSaltAndExpiry memory emptySig; + // uint32 registrationBlockNumber = 100; + // bytes memory quorumNumbers = BitmapUtils.bitmapToBytesArray(registrationBitmap); + // for (uint256 i = 0; i < quorumNumbers.length; ++i) { + // _setOperatorWeight(defaultOperator, uint8(quorumNumbers[i]), defaultStake); + // } + // cheats.startPrank(defaultOperator); + // cheats.roll(registrationBlockNumber); + // registryCoordinator.registerOperator( + // quorumNumbers, defaultSocket, pubkeyRegistrationParams, emptySig + // ); + + // address[] memory operatorsToUpdate = new address[](1); + // operatorsToUpdate[0] = defaultOperator; + + // uint192 quorumBitmapBefore = registryCoordinator.getCurrentQuorumBitmap(defaultOperatorId); + // assertEq(quorumBitmapBefore, registrationBitmap, "operator bitmap somehow incorrect"); + + // // make the stake registry return info that the operator should be removed from quorums + // uint192 quorumBitmapToRemove = mockReturnData; + // bytes memory quorumNumbersToRemove = BitmapUtils.bitmapToBytesArray(quorumBitmapToRemove); + // for (uint256 i = 0; i < quorumNumbersToRemove.length; ++i) { + // _setOperatorWeight(defaultOperator, uint8(quorumNumbersToRemove[i]), 0); + // } + // uint256 expectedQuorumBitmap = BitmapUtils.minus(quorumBitmapBefore, quorumBitmapToRemove); + + // registryCoordinator.updateOperators(operatorsToUpdate); + // uint192 quorumBitmapAfter = registryCoordinator.getCurrentQuorumBitmap(defaultOperatorId); + // assertEq(expectedQuorumBitmap, quorumBitmapAfter, "quorum bitmap did not update correctly"); + // } + + // // @notice tests the `updateOperators` function with a single *un*registered operator as input + // function test_updateOperators_unregisteredOperator() public view { + // address[] memory operatorsToUpdate = new address[](1); + // operatorsToUpdate[0] = defaultOperator; + + // // force a staticcall to the `updateOperators` function -- this should *pass* because the call should be a strict no-op! + // (bool success,) = address(registryCoordinator).staticcall( + // abi.encodeWithSignature("updateOperators(address[])", operatorsToUpdate) + // ); + // require(success, "staticcall failed!"); + // } function test_updateOperatorsForQuorum_revert_paused() public { cheats.prank(pauser); diff --git a/test/unit/StakeRegistryUnit.t.sol b/test/unit/StakeRegistryUnit.t.sol index 94e1d9fe..aab4e08f 100644 --- a/test/unit/StakeRegistryUnit.t.sol +++ b/test/unit/StakeRegistryUnit.t.sol @@ -1782,12 +1782,30 @@ contract StakeRegistryUnitTests_Deregister is StakeRegistryUnitTests { contract StakeRegistryUnitTests_StakeUpdates is StakeRegistryUnitTests { using BitmapUtils for *; + function _wrap( + address operator + ) internal pure returns (address[] memory) { + address[] memory operators = new address[](1); + operators[0] = operator; + return operators; + } + + function _wrap( + bytes32 operatorId + ) internal pure returns (bytes32[] memory) { + bytes32[] memory operatorIds = new bytes32[](1); + operatorIds[0] = operatorId; + return operatorIds; + } + function test_updateOperatorStake_Revert_WhenNotRegistryCoordinator() public { UpdateSetup memory setup = _fuzz_setupUpdateOperatorStake({registeredFor: initializedQuorumBitmap, fuzzy_Delta: 0}); cheats.expectRevert(IStakeRegistryErrors.OnlySlashingRegistryCoordinator.selector); - stakeRegistry.updateOperatorStake(setup.operator, setup.operatorId, setup.quorumNumbers); + stakeRegistry.updateOperatorsStake( + _wrap(setup.operator), _wrap(setup.operatorId), uint8(setup.quorumNumbers[0]) + ); } function testFuzz_updateOperatorStake_Revert_WhenQuorumDoesNotExist( @@ -1802,7 +1820,9 @@ contract StakeRegistryUnitTests_StakeUpdates is StakeRegistryUnitTests { cheats.expectRevert(IStakeRegistryErrors.QuorumDoesNotExist.selector); cheats.prank(address(registryCoordinator)); - stakeRegistry.updateOperatorStake(setup.operator, setup.operatorId, invalidQuorums); + stakeRegistry.updateOperatorsStake( + _wrap(setup.operator), _wrap(setup.operatorId), uint8(invalidQuorums[0]) + ); } /** @@ -1829,8 +1849,13 @@ contract StakeRegistryUnitTests_StakeUpdates is StakeRegistryUnitTests { // updateOperatorStake cheats.prank(address(registryCoordinator)); - uint192 quorumsToRemove = - stakeRegistry.updateOperatorStake(setup.operator, setup.operatorId, setup.quorumNumbers); + bool[] memory shouldBeDeregistered = new bool[](setup.quorumNumbers.length); + for (uint256 i = 0; i < setup.quorumNumbers.length; i++) { + bool[] memory shouldBeDeregisteredForQuorum = stakeRegistry.updateOperatorsStake( + _wrap(setup.operator), _wrap(setup.operatorId), uint8(setup.quorumNumbers[i]) + ); + shouldBeDeregistered[i] = shouldBeDeregisteredForQuorum[i]; + } // Get ending state IStakeRegistry.StakeUpdate[] memory newOperatorStakes = @@ -1873,7 +1898,8 @@ contract StakeRegistryUnitTests_StakeUpdates is StakeRegistryUnitTests { ); // Return value should be empty since we're still above the minimum assertTrue( - quorumsToRemove.isEmpty(), "positive stake delta should not remove any quorums" + !shouldBeDeregistered[i], + "positive stake delta should not lead to deregistration" ); } else if (endingWeight < minimumStake) { // Check updating an operator who is now below the minimum: @@ -1892,9 +1918,7 @@ contract StakeRegistryUnitTests_StakeUpdates is StakeRegistryUnitTests { ); assertEq(newOperatorStake.stake, 0, "operator stake should now be zero"); // IECDSAStakeRegistryTypes.Quorum should be added to return bitmap - assertTrue( - quorumsToRemove.isSet(quorumNumber), "quorum should be in removal bitmap" - ); + assertTrue(shouldBeDeregistered[i], "operator should be deregistered"); } else { // Check that no update occurs if weight remains the same assertTrue( @@ -1907,7 +1931,8 @@ contract StakeRegistryUnitTests_StakeUpdates is StakeRegistryUnitTests { ); // Check that return value is empty - we're still at the minimum, so no quorums should be removed assertTrue( - quorumsToRemove.isEmpty(), "neutral stake delta should not remove any quorums" + !shouldBeDeregistered[i], + "neutral stake delta should not lead to deregistration" ); } } @@ -1947,7 +1972,11 @@ contract StakeRegistryUnitTests_StakeUpdates is StakeRegistryUnitTests { // updateOperatorStake cheats.prank(address(registryCoordinator)); - stakeRegistry.updateOperatorStake(setup.operator, setup.operatorId, setup.quorumNumbers); + for (uint256 j = 0; j < setup.quorumNumbers.length; j++) { + stakeRegistry.updateOperatorsStake( + _wrap(setup.operator), _wrap(setup.operatorId), uint8(setup.quorumNumbers[j]) + ); + } } // Check final results for each quorum @@ -2038,9 +2067,13 @@ contract StakeRegistryUnitTests_StakeUpdates is StakeRegistryUnitTests { // updateOperatorStake cheats.prank(address(registryCoordinator)); - uint192 quorumsToRemove = stakeRegistry.updateOperatorStake( - setup.operator, setup.operatorId, setup.quorumNumbers - ); + bool[] memory shouldBeDeregistered = new bool[](setup.quorumNumbers.length); + for (uint256 i = 0; i < setup.quorumNumbers.length; i++) { + bool[] memory shouldBeDeregisteredForQuorum = stakeRegistry.updateOperatorsStake( + _wrap(setup.operator), _wrap(setup.operatorId), uint8(setup.quorumNumbers[i]) + ); + shouldBeDeregistered[i] = shouldBeDeregisteredForQuorum[i]; + } // Get ending state IStakeRegistry.StakeUpdate[] memory newOperatorStakes = @@ -2084,8 +2117,8 @@ contract StakeRegistryUnitTests_StakeUpdates is StakeRegistryUnitTests { ); // Return value should be empty since we're still above the minimum assertTrue( - quorumsToRemove.isEmpty(), - "positive stake delta should not remove any quorums" + !shouldBeDeregistered[i], + "positive stake delta should not lead to deregistration" ); assertEq( prevOperatorHistoryLengths[i] + 1, @@ -2105,9 +2138,7 @@ contract StakeRegistryUnitTests_StakeUpdates is StakeRegistryUnitTests { // assertEq(prevTotalStake.stake - stakeRemoved, newTotalStake.stake, "failed to remove delta from total stake"); assertEq(newOperatorStake.stake, 0, "operator stake should now be zero"); // IECDSAStakeRegistryTypes.Quorum should be added to return bitmap - assertTrue( - quorumsToRemove.isSet(quorumNumber), "quorum should be in removal bitmap" - ); + assertTrue(shouldBeDeregistered[i], "operator should be deregistered"); if (prevOperatorStake.stake >= minimumStake) { // Total stakes and operator history should be updated assertEq( @@ -2145,7 +2176,7 @@ contract StakeRegistryUnitTests_StakeUpdates is StakeRegistryUnitTests { ); // Check that return value is empty - we're still at the minimum, so no quorums should be removed assertTrue( - quorumsToRemove.isEmpty(), + !shouldBeDeregistered[i], "neutral stake delta should not remove any quorums" ); assertEq( From 85c6748294ad7cf5c14bc1f190c0ddd40d76273c Mon Sep 17 00:00:00 2001 From: gpsanant Date: Mon, 3 Feb 2025 10:03:18 -0800 Subject: [PATCH 05/16] chore: respond to review --- src/RegistryCoordinator.sol | 7 ++++--- src/RegistryCoordinatorStorage.sol | 2 +- src/SlashingRegistryCoordinatorStorage.sol | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/RegistryCoordinator.sol b/src/RegistryCoordinator.sol index c9719281..e9617027 100644 --- a/src/RegistryCoordinator.sol +++ b/src/RegistryCoordinator.sol @@ -149,6 +149,7 @@ contract RegistryCoordinator is RegistryCoordinatorStorage { /// @inheritdoc IRegistryCoordinator function disableM2QuorumRegistration() external onlyOwner { + require(operatorSetsEnabled, OperatorSetsNotEnabled()); require(!isM2QuorumRegistrationDisabled, M2QuorumRegistrationIsDisabled()); isM2QuorumRegistrationDisabled = true; @@ -177,7 +178,7 @@ contract RegistryCoordinator is RegistryCoordinatorStorage { /// @dev Hook to prevent any new quorums from being created if operator sets are not enabled function _beforeCreateQuorum( - uint8 quorumNumber + uint8 ) internal virtual override { require(operatorSetsEnabled, OperatorSetsNotEnabled()); } @@ -185,8 +186,8 @@ contract RegistryCoordinator is RegistryCoordinatorStorage { /// @dev Hook to allow for any post-deregister logic function _afterDeregisterOperator( address operator, - bytes32 operatorId, - bytes memory quorumNumbers, + bytes32, + bytes memory, uint192 newBitmap ) internal virtual override { uint256 operatorM2QuorumBitmap = newBitmap.minus(m2QuorumBitmap); diff --git a/src/RegistryCoordinatorStorage.sol b/src/RegistryCoordinatorStorage.sol index 5d687959..0b0186e2 100644 --- a/src/RegistryCoordinatorStorage.sol +++ b/src/RegistryCoordinatorStorage.sol @@ -65,5 +65,5 @@ abstract contract RegistryCoordinatorStorage is serviceManager = _serviceManager; } - uint256[47] private __GAP; + uint256[48] private __GAP; } diff --git a/src/SlashingRegistryCoordinatorStorage.sol b/src/SlashingRegistryCoordinatorStorage.sol index ed39c0b7..463dd00a 100644 --- a/src/SlashingRegistryCoordinatorStorage.sol +++ b/src/SlashingRegistryCoordinatorStorage.sol @@ -106,5 +106,5 @@ abstract contract SlashingRegistryCoordinatorStorage is ISlashingRegistryCoordin // storage gap for upgradeability // slither-disable-next-line shadowing-state - uint256[40] private __GAP; + uint256[38] private __GAP; } From f882e24c5ca8fd9753c08b48924df9ae72355409 Mon Sep 17 00:00:00 2001 From: Gautham Anant <32277907+gpsanant@users.noreply.github.com> Date: Mon, 3 Feb 2025 14:52:08 -0800 Subject: [PATCH 06/16] Update src/SlashingRegistryCoordinator.sol Co-authored-by: Michael Sun <35479365+8sunyuan@users.noreply.github.com> --- src/SlashingRegistryCoordinator.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SlashingRegistryCoordinator.sol b/src/SlashingRegistryCoordinator.sol index 6f021a96..b687afc6 100644 --- a/src/SlashingRegistryCoordinator.sol +++ b/src/SlashingRegistryCoordinator.sol @@ -411,7 +411,7 @@ contract SlashingRegistryCoordinator is */ _updateOperatorBitmap({operatorId: operatorId, newBitmap: newBitmap}); - emit OperatorSocketUpdate(operatorId, socket); + _setOperatorSocket(operatorId, socket); // If the operator wasn't registered for any quorums, update their status // and register them with this AVS in EigenLayer core (DelegationManager) From 06634ae2f2a213a22209c93126a8f3b7d9e673cd Mon Sep 17 00:00:00 2001 From: gpsanant Date: Mon, 3 Feb 2025 14:54:41 -0800 Subject: [PATCH 07/16] fix: m2 registration validity --- src/RegistryCoordinator.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/RegistryCoordinator.sol b/src/RegistryCoordinator.sol index e9617027..707f0924 100644 --- a/src/RegistryCoordinator.sol +++ b/src/RegistryCoordinator.sol @@ -63,7 +63,7 @@ contract RegistryCoordinator is RegistryCoordinatorStorage { ) external onlyWhenNotPaused(PAUSED_REGISTER_OPERATOR) { require(!isM2QuorumRegistrationDisabled, M2QuorumRegistrationIsDisabled()); require( - quorumNumbers.orderedBytesArrayToBitmap().isSubsetOf(m2QuorumBitmap), + !operatorSetsEnabled || quorumNumbers.orderedBytesArrayToBitmap().isSubsetOf(m2QuorumBitmap), OnlyM2QuorumsAllowed() ); @@ -97,7 +97,7 @@ contract RegistryCoordinator is RegistryCoordinatorStorage { ) external onlyWhenNotPaused(PAUSED_REGISTER_OPERATOR) { require(!isM2QuorumRegistrationDisabled, M2QuorumRegistrationIsDisabled()); require( - quorumNumbers.orderedBytesArrayToBitmap().isSubsetOf(m2QuorumBitmap), + !operatorSetsEnabled || quorumNumbers.orderedBytesArrayToBitmap().isSubsetOf(m2QuorumBitmap), OnlyM2QuorumsAllowed() ); From d7ca39f524fc570aa113331338761e35eee6d400 Mon Sep 17 00:00:00 2001 From: gpsanant Date: Mon, 3 Feb 2025 18:28:05 -0800 Subject: [PATCH 08/16] feat: lazy migration --- src/RegistryCoordinator.sol | 43 ++++++++----------- src/RegistryCoordinatorStorage.sol | 2 +- src/interfaces/IRegistryCoordinator.sol | 7 ---- test/unit/RegistryCoordinatorUnit.t.sol | 55 +------------------------ 4 files changed, 20 insertions(+), 87 deletions(-) diff --git a/src/RegistryCoordinator.sol b/src/RegistryCoordinator.sol index 707f0924..e5dad520 100644 --- a/src/RegistryCoordinator.sol +++ b/src/RegistryCoordinator.sol @@ -63,7 +63,7 @@ contract RegistryCoordinator is RegistryCoordinatorStorage { ) external onlyWhenNotPaused(PAUSED_REGISTER_OPERATOR) { require(!isM2QuorumRegistrationDisabled, M2QuorumRegistrationIsDisabled()); require( - !operatorSetsEnabled || quorumNumbers.orderedBytesArrayToBitmap().isSubsetOf(m2QuorumBitmap), + quorumNumbers.orderedBytesArrayToBitmap().isSubsetOf(m2QuorumBitmap()), OnlyM2QuorumsAllowed() ); @@ -97,7 +97,7 @@ contract RegistryCoordinator is RegistryCoordinatorStorage { ) external onlyWhenNotPaused(PAUSED_REGISTER_OPERATOR) { require(!isM2QuorumRegistrationDisabled, M2QuorumRegistrationIsDisabled()); require( - !operatorSetsEnabled || quorumNumbers.orderedBytesArrayToBitmap().isSubsetOf(m2QuorumBitmap), + quorumNumbers.orderedBytesArrayToBitmap().isSubsetOf(m2QuorumBitmap()), OnlyM2QuorumsAllowed() ); @@ -127,26 +127,11 @@ contract RegistryCoordinator is RegistryCoordinatorStorage { ) external override onlyWhenNotPaused(PAUSED_DEREGISTER_OPERATOR) { // Check that the quorum numbers are M2 quorums for (uint256 i = 0; i < quorumNumbers.length; i++) { - require( - !operatorSetsEnabled || _isM2Quorum(uint8(quorumNumbers[i])), OperatorSetQuorum() - ); + require(_isM2Quorum(uint8(quorumNumbers[i])), OperatorSetQuorum()); } _deregisterOperator({operator: msg.sender, quorumNumbers: quorumNumbers}); } - /// @inheritdoc IRegistryCoordinator - function enableOperatorSets() external onlyOwner { - require(!operatorSetsEnabled, OperatorSetsAlreadyEnabled()); - - // Set the bitmap for M2 quorums - m2QuorumBitmap = _getQuorumBitmap(quorumCount); - - // Enable operator sets mode - operatorSetsEnabled = true; - - emit OperatorSetsEnabled(); - } - /// @inheritdoc IRegistryCoordinator function disableM2QuorumRegistration() external onlyOwner { require(operatorSetsEnabled, OperatorSetsNotEnabled()); @@ -169,7 +154,7 @@ contract RegistryCoordinator is RegistryCoordinatorStorage { bytes memory quorumNumbers ) internal virtual override { // filter out M2 quorums from the quorum numbers - uint256 operatorSetBitmap = quorumNumbers.orderedBytesArrayToBitmap().minus(m2QuorumBitmap); + uint256 operatorSetBitmap = quorumNumbers.orderedBytesArrayToBitmap().minus(m2QuorumBitmap()); if (!operatorSetBitmap.isEmpty()) { // call the parent _forceDeregisterOperator function for operator sets quorums super._forceDeregisterOperator(operator, operatorSetBitmap.bitmapToBytesArray()); @@ -180,7 +165,12 @@ contract RegistryCoordinator is RegistryCoordinatorStorage { function _beforeCreateQuorum( uint8 ) internal virtual override { - require(operatorSetsEnabled, OperatorSetsNotEnabled()); + // If operator sets are not enabled, set the m2 quorum bitmap to the current m2 quorum bitmap + // and enable operator sets + if (!operatorSetsEnabled) { + _m2QuorumBitmap = m2QuorumBitmap(); + operatorSetsEnabled = true; + } } /// @dev Hook to allow for any post-deregister logic @@ -190,7 +180,7 @@ contract RegistryCoordinator is RegistryCoordinatorStorage { bytes memory, uint192 newBitmap ) internal virtual override { - uint256 operatorM2QuorumBitmap = newBitmap.minus(m2QuorumBitmap); + uint256 operatorM2QuorumBitmap = newBitmap.minus(m2QuorumBitmap()); // If the operator is no longer registered for any M2 quorums, update their status and deregister // them from the AVS via the EigenLayer core contracts if (operatorM2QuorumBitmap.isEmpty()) { @@ -200,9 +190,12 @@ contract RegistryCoordinator is RegistryCoordinatorStorage { /// @dev Returns a bitmap with all bits set up to `quorumCount`. Used for bit-masking quorum numbers /// and differentiating between operator sets and M2 quorums - function _getQuorumBitmap( - uint256 quorumCount - ) internal pure returns (uint256) { + function m2QuorumBitmap() public view returns (uint256) { + // If operator sets are enabled, return the current m2 quorum bitmap + if (operatorSetsEnabled) { + return _m2QuorumBitmap; + } + // This creates a number where all bits up to quorumCount are set to 1 // For example: // quorumCount = 3 -> 0111 (7 in decimal) @@ -216,7 +209,7 @@ contract RegistryCoordinator is RegistryCoordinatorStorage { function _isM2Quorum( uint8 quorumNumber ) internal view returns (bool) { - return m2QuorumBitmap.isSet(quorumNumber); + return m2QuorumBitmap().isSet(quorumNumber); } /** diff --git a/src/RegistryCoordinatorStorage.sol b/src/RegistryCoordinatorStorage.sol index 0b0186e2..8124cd95 100644 --- a/src/RegistryCoordinatorStorage.sol +++ b/src/RegistryCoordinatorStorage.sol @@ -42,7 +42,7 @@ abstract contract RegistryCoordinatorStorage is /// @notice The bitmap containing all M2 quorums. This is only used for existing AVS middlewares that have M2 quorums /// and need to call `enableOperatorSets()` to enable operator sets mode. - uint256 internal m2QuorumBitmap; + uint256 internal _m2QuorumBitmap; constructor( IServiceManager _serviceManager, diff --git a/src/interfaces/IRegistryCoordinator.sol b/src/interfaces/IRegistryCoordinator.sol index a8b3939b..e25f1477 100644 --- a/src/interfaces/IRegistryCoordinator.sol +++ b/src/interfaces/IRegistryCoordinator.sol @@ -110,13 +110,6 @@ interface IRegistryCoordinator is bytes memory quorumNumbers ) external; - /** - * @notice Enables operator sets mode for the AVS. Once enabled, this cannot be disabled. - * @dev When enabled, all existing quorums are marked as M2 quorums and future quorums must be explicitly - * created as either M2 or operator set quorums. - */ - function enableOperatorSets() external; - /** * @notice Checks if a quorum is an M2 quorum. * @param quorumNumber The quorum identifier. diff --git a/test/unit/RegistryCoordinatorUnit.t.sol b/test/unit/RegistryCoordinatorUnit.t.sol index bd303cff..9b2d6bf8 100644 --- a/test/unit/RegistryCoordinatorUnit.t.sol +++ b/test/unit/RegistryCoordinatorUnit.t.sol @@ -688,10 +688,6 @@ contract RegistryCoordinatorUnitTests_DeregisterOperator_EjectOperator is } function test_deregisterOperator_revert_notRegistered() public { - // enable operatorSets - cheats.prank(registryCoordinator.owner()); - registryCoordinator.enableOperatorSets(); - bytes memory quorumNumbers = new bytes(1); quorumNumbers[0] = bytes1(defaultQuorumNumber); @@ -701,9 +697,6 @@ contract RegistryCoordinatorUnitTests_DeregisterOperator_EjectOperator is } function test_deregisterOperator_revert_incorrectQuorums() public { - // enable operatorSets - cheats.prank(registryCoordinator.owner()); - registryCoordinator.enableOperatorSets(); console.log("quorumCount", registryCoordinator.quorumCount()); @@ -2408,23 +2401,9 @@ contract RegistryCoordinatorUnitTests_BeforeMigration is RegistryCoordinatorUnit operatorSetParams, minimumStake, strategyParams, lookAheadPeriod ); } - - function test_MigrateToOperatorSets() public { - _deployMockEigenLayerAndAVS(0); - cheats.prank(registryCoordinatorOwner); - registryCoordinator.enableOperatorSets(); - assertTrue(registryCoordinator.operatorSetsEnabled()); - } } contract RegistryCoordinatorUnitTests_AfterMigration is RegistryCoordinatorUnitTests { - function test_MigrateToOperatorSets() public { - cheats.prank(registryCoordinatorOwner); - registryCoordinator.enableOperatorSets(); - assertTrue(registryCoordinator.operatorSetsEnabled()); - } - - /// function test_M2_Deregister() public { _deployMockEigenLayerAndAVS(2); @@ -2467,10 +2446,6 @@ contract RegistryCoordinatorUnitTests_AfterMigration is RegistryCoordinatorUnitT operatorSignature ); - /// migrate to operator sets - vm.prank(registryCoordinatorOwner); - registryCoordinator.enableOperatorSets(); - /// Deregistration for m2 should for the first two operator sets vm.prank(operatorToRegister.key.addr); registryCoordinator.deregisterOperator(new bytes(1)); @@ -2491,9 +2466,6 @@ contract RegistryCoordinatorUnitTests_AfterMigration is RegistryCoordinatorUnitT } function test_M2_Register_Reverts() public { - cheats.prank(registryCoordinatorOwner); - registryCoordinator.enableOperatorSets(); - bytes memory quorumNumbers = new bytes(1); quorumNumbers[0] = bytes1(uint8(0)); IBLSApkRegistryTypes.PubkeyRegistrationParams memory params; @@ -2509,10 +2481,6 @@ contract RegistryCoordinatorUnitTests_AfterMigration is RegistryCoordinatorUnitT // Deploy with 0 quorums _deployMockEigenLayerAndAVS(0); - // Enable operator sets first - cheats.prank(registryCoordinatorOwner); - registryCoordinator.enableOperatorSets(); - // Create quorum params ISlashingRegistryCoordinatorTypes.OperatorSetParam memory operatorSetParams = ISlashingRegistryCoordinatorTypes.OperatorSetParam({ @@ -2538,10 +2506,6 @@ contract RegistryCoordinatorUnitTests_AfterMigration is RegistryCoordinatorUnitT // Deploy with 0 quorums _deployMockEigenLayerAndAVS(0); - // Enable operator sets first - cheats.prank(registryCoordinatorOwner); - registryCoordinator.enableOperatorSets(); - // Create quorum params ISlashingRegistryCoordinatorTypes.OperatorSetParam memory operatorSetParams = ISlashingRegistryCoordinatorTypes.OperatorSetParam({ @@ -2567,10 +2531,6 @@ contract RegistryCoordinatorUnitTests_AfterMigration is RegistryCoordinatorUnitT _deployMockEigenLayerAndAVS(0); - // Enable operator sets first - cheats.prank(registryCoordinatorOwner); - registryCoordinator.enableOperatorSets(); - // Create quorum params ISlashingRegistryCoordinatorTypes.OperatorSetParam memory operatorSetParams = ISlashingRegistryCoordinatorTypes.OperatorSetParam({ @@ -2612,9 +2572,7 @@ contract RegistryCoordinatorUnitTests_AfterMigration is RegistryCoordinatorUnitT function test_registerHook_WithChurn() public { vm.skip(true); _deployMockEigenLayerAndAVS(0); - // Enable operator sets first - cheats.prank(registryCoordinatorOwner); - registryCoordinator.enableOperatorSets(); + // Create quorum params ISlashingRegistryCoordinatorTypes.OperatorSetParam memory operatorSetParams = @@ -2698,9 +2656,6 @@ contract RegistryCoordinatorUnitTests_AfterMigration is RegistryCoordinatorUnitT function test_deregisterHook() public { _deployMockEigenLayerAndAVS(0); - // Enable operator sets first - cheats.prank(registryCoordinatorOwner); - registryCoordinator.enableOperatorSets(); // Create quorum params ISlashingRegistryCoordinatorTypes.OperatorSetParam memory operatorSetParams = @@ -2748,10 +2703,6 @@ contract RegistryCoordinatorUnitTests_AfterMigration is RegistryCoordinatorUnitT function test_registerHook_Reverts_WhenNotALM() public { _deployMockEigenLayerAndAVS(0); - // Enable operator sets first - cheats.prank(registryCoordinatorOwner); - registryCoordinator.enableOperatorSets(); - // Create quorum params ISlashingRegistryCoordinatorTypes.OperatorSetParam memory operatorSetParams = ISlashingRegistryCoordinatorTypes.OperatorSetParam({ @@ -2793,10 +2744,6 @@ contract RegistryCoordinatorUnitTests_AfterMigration is RegistryCoordinatorUnitT function test_deregisterHook_Reverts_WhenNotALM() public { _deployMockEigenLayerAndAVS(0); - // Enable operator sets first - cheats.prank(registryCoordinatorOwner); - registryCoordinator.enableOperatorSets(); - assertTrue(registryCoordinator.operatorSetsEnabled(), "operatorSetsEnabled should be true"); // Create quorum params From 8f52f2459924d744e72bcfd6d0fc43b5410443ea Mon Sep 17 00:00:00 2001 From: Michael Sun Date: Tue, 4 Feb 2025 13:49:36 -0800 Subject: [PATCH 09/16] test: fixes and refactor of weightOfOperators --- src/SlashingRegistryCoordinator.sol | 14 +++---- src/StakeRegistry.sol | 24 ++++++----- .../RegistryCoordinatorHarness.t.sol | 4 ++ test/mocks/DelegationMock.sol | 14 +++++++ test/unit/RegistryCoordinatorUnit.t.sol | 42 +++++++++++++++---- test/utils/MockAVSDeployer.sol | 2 + 6 files changed, 73 insertions(+), 27 deletions(-) diff --git a/src/SlashingRegistryCoordinator.sol b/src/SlashingRegistryCoordinator.sol index b687afc6..028a14e6 100644 --- a/src/SlashingRegistryCoordinator.sol +++ b/src/SlashingRegistryCoordinator.sol @@ -731,17 +731,17 @@ contract SlashingRegistryCoordinator is IStakeRegistryTypes.StakeType stakeType, uint32 lookAheadPeriod ) internal { - // Increment the total quorum count. Fails if we're already at the max - uint8 prevQuorumCount = quorumCount; - require(prevQuorumCount < MAX_QUORUM_COUNT, MaxQuorumsReached()); - quorumCount = prevQuorumCount + 1; - - // The previous count is the new quorum's number - uint8 quorumNumber = prevQuorumCount; + // The previous quorum count is the new quorum's number, + // this is because quorum numbers begin from index 0. + uint8 quorumNumber = quorumCount; // Hook to allow for any pre-create quorum logic _beforeCreateQuorum(quorumNumber); + // Increment the total quorum count. Fails if we're already at the max + require(quorumNumber < MAX_QUORUM_COUNT, MaxQuorumsReached()); + quorumCount += 1; + // Initialize the quorum here and in each registry _setOperatorSetParams(quorumNumber, operatorSetParams); diff --git a/src/StakeRegistry.sol b/src/StakeRegistry.sol index 318fa1fc..d01343a2 100644 --- a/src/StakeRegistry.sol +++ b/src/StakeRegistry.sol @@ -534,11 +534,11 @@ contract StakeRegistry is StakeRegistryStorage { uint8 quorumNumber, address[] memory operators ) internal view virtual returns (uint96[] memory, bool[] memory) { - uint96[] memory weights; - bool[] memory hasMinimumStakes; + uint96[] memory weights = new uint96[](operators.length); + bool[] memory hasMinimumStakes = new bool[](operators.length); uint256 stratsLength = strategyParamsLength(quorumNumber); - StrategyParams memory strategyAndMultiplier; + StrategyParams[] memory stratsAndMultipliers = strategyParams[quorumNumber]; uint256[][] memory strategyShares; if (stakeTypePerQuorum[quorumNumber] == IStakeRegistryTypes.StakeType.TOTAL_SLASHABLE) { @@ -550,12 +550,15 @@ contract StakeRegistry is StakeRegistryStorage { delegation.getOperatorsShares(operators, strategiesPerQuorum[quorumNumber]); } - for (uint256 stratIndex = 0; stratIndex < stratsLength; stratIndex++) { - // accessing i'th StrategyParams struct for the quorumNumber - strategyAndMultiplier = strategyParams[quorumNumber][stratIndex]; + // Calculate weight of each operator and whether they contain minimum stake for the quorum + for (uint256 opIndex = 0; opIndex < operators.length; opIndex++) { + // 1. For the given operator, loop through the strategies and calculate the operator's + // weight for the quorum + for (uint256 stratIndex = 0; stratIndex < stratsLength; stratIndex++) { + // get multiplier for strategy + StrategyParams memory strategyAndMultiplier = stratsAndMultipliers[stratIndex]; - for (uint256 opIndex = 0; opIndex < operators.length; opIndex++) { - // add the weight from the shares for this strategy to the total weight + // calculate added weight for strategy and multiplier if (strategyShares[opIndex][stratIndex] > 0) { weights[opIndex] += uint96( strategyShares[opIndex][stratIndex] * strategyAndMultiplier.multiplier @@ -564,9 +567,8 @@ contract StakeRegistry is StakeRegistryStorage { } } - // check if the operator meets the quorum's minimum stake - hasMinimumStakes[stratIndex] = - weights[stratIndex] >= minimumStakeForQuorum[quorumNumber]; + // 2. Check whether operator is above minimum stake threshold + hasMinimumStakes[opIndex] = weights[opIndex] >= minimumStakeForQuorum[quorumNumber]; } return (weights, hasMinimumStakes); diff --git a/test/harnesses/RegistryCoordinatorHarness.t.sol b/test/harnesses/RegistryCoordinatorHarness.t.sol index 82211204..780ee1f4 100644 --- a/test/harnesses/RegistryCoordinatorHarness.t.sol +++ b/test/harnesses/RegistryCoordinatorHarness.t.sol @@ -79,4 +79,8 @@ contract RegistryCoordinatorHarness is RegistryCoordinator, Test { ) external { isM2QuorumRegistrationDisabled = disabled; } + + function setM2QuorumBitmap(uint256 bitmap) external { + _m2QuorumBitmap = bitmap; + } } diff --git a/test/mocks/DelegationMock.sol b/test/mocks/DelegationMock.sol index 11bce967..ef8648da 100644 --- a/test/mocks/DelegationMock.sol +++ b/test/mocks/DelegationMock.sol @@ -282,6 +282,20 @@ contract DelegationMock is DelegationIntermediate { return shares; } + function getOperatorsShares( + address[] memory operators, + IStrategy[] memory strategies + ) external view override returns (uint256[][] memory) { + uint256[][] memory operatorSharesArray = new uint256[][](operators.length); + for (uint256 i = 0; i < operators.length; i++) { + operatorSharesArray[i] = new uint256[](strategies.length); + for (uint256 j = 0; j < strategies.length; j++) { + operatorSharesArray[i][j] = _weightOf[operators[i]][strategies[j]]; + } + } + return operatorSharesArray; + } + function minWithdrawalDelayBlocks() external view override returns (uint32) { return 10000; } diff --git a/test/unit/RegistryCoordinatorUnit.t.sol b/test/unit/RegistryCoordinatorUnit.t.sol index 9b2d6bf8..771fd2ba 100644 --- a/test/unit/RegistryCoordinatorUnit.t.sol +++ b/test/unit/RegistryCoordinatorUnit.t.sol @@ -8,6 +8,12 @@ import { ISlashingRegistryCoordinatorErrors } from "../../src/interfaces/ISlashingRegistryCoordinator.sol"; +import { + IRegistryCoordinator, + IRegistryCoordinatorTypes, + IRegistryCoordinatorErrors +} from "../../src/interfaces/IRegistryCoordinator.sol"; + import {IBLSApkRegistryTypes} from "../../src/interfaces/IBLSApkRegistry.sol"; import {QuorumBitmapHistoryLib} from "../../src/libraries/QuorumBitmapHistoryLib.sol"; import {BitmapUtils} from "../../src/libraries/BitmapUtils.sol"; @@ -302,7 +308,7 @@ contract RegistryCoordinatorUnitTests_RegisterOperator is RegistryCoordinatorUni quorumNumbersTooLarge[0] = 0xC0; - cheats.expectRevert(BitmapUtils.BitmapValueTooLarge.selector); + cheats.expectRevert(IRegistryCoordinatorErrors.OnlyM2QuorumsAllowed.selector); cheats.prank(defaultOperator); registryCoordinator.registerOperator( quorumNumbersTooLarge, defaultSocket, pubkeyRegistrationParams, emptySig @@ -317,7 +323,7 @@ contract RegistryCoordinatorUnitTests_RegisterOperator is RegistryCoordinatorUni quorumNumbersNotCreated[0] = 0x0B; cheats.prank(defaultOperator); - cheats.expectRevert(BitmapUtils.BitmapValueTooLarge.selector); + cheats.expectRevert(IRegistryCoordinatorErrors.OnlyM2QuorumsAllowed.selector); registryCoordinator.registerOperator( quorumNumbersNotCreated, defaultSocket, pubkeyRegistrationParams, emptySig ); @@ -395,6 +401,8 @@ contract RegistryCoordinatorUnitTests_RegisterOperator is RegistryCoordinatorUni cheats.expectEmit(true, true, true, true, address(registryCoordinator)); emit OperatorSocketUpdate(defaultOperatorId, defaultSocket); + cheats.expectEmit(true, true, true, true, address(registryCoordinator)); + emit OperatorRegistered(defaultOperator, defaultOperatorId); cheats.expectEmit(true, true, true, true, address(blsApkRegistry)); emit OperatorAddedToQuorums(defaultOperator, defaultOperatorId, quorumNumbers); for (uint256 i = 0; i < quorumNumbers.length; i++) { @@ -405,8 +413,6 @@ contract RegistryCoordinatorUnitTests_RegisterOperator is RegistryCoordinatorUni cheats.expectEmit(true, true, true, true, address(indexRegistry)); emit QuorumIndexUpdate(defaultOperatorId, uint8(quorumNumbers[i]), 0); } - cheats.expectEmit(true, true, true, true, address(registryCoordinator)); - emit OperatorRegistered(defaultOperator, defaultOperatorId); uint256 gasBefore = gasleft(); cheats.prank(defaultOperator); @@ -636,6 +642,8 @@ contract RegistryCoordinatorUnitTests_RegisterOperator is RegistryCoordinatorUni cheats.expectEmit(true, true, true, true, address(registryCoordinator)); emit OperatorSocketUpdate(defaultOperatorId, defaultSocket); + cheats.expectEmit(true, true, true, true, address(registryCoordinator)); + emit OperatorRegistered(defaultOperator, defaultOperatorId); cheats.expectEmit(true, true, true, true, address(blsApkRegistry)); emit OperatorAddedToQuorums(defaultOperator, defaultOperatorId, quorumNumbers); cheats.expectEmit(true, true, true, true, address(stakeRegistry)); @@ -1707,6 +1715,9 @@ contract RegistryCoordinatorUnitTests_RegisterOperatorWithChurn is RegistryCoord cheats.expectEmit(true, true, true, true, address(registryCoordinator)); emit OperatorSocketUpdate(operatorToRegisterId, defaultSocket); + cheats.expectEmit(true, true, true, true, address(registryCoordinator)); + emit OperatorRegistered(operatorToRegister, operatorToRegisterId); + cheats.expectEmit(true, true, true, true, address(blsApkRegistry)); emit OperatorAddedToQuorums(operatorToRegister, operatorToRegisterId, quorumNumbers); cheats.expectEmit(true, true, true, false, address(stakeRegistry)); @@ -1725,8 +1736,6 @@ contract RegistryCoordinatorUnitTests_RegisterOperatorWithChurn is RegistryCoord emit OperatorStakeUpdate(operatorToKickId, defaultQuorumNumber, 0); cheats.expectEmit(true, true, true, true, address(indexRegistry)); emit QuorumIndexUpdate(operatorToRegisterId, defaultQuorumNumber, numOperators - 1); - cheats.expectEmit(true, true, true, true, address(registryCoordinator)); - emit OperatorRegistered(operatorToRegister, operatorToRegisterId); { ISignatureUtils.SignatureWithSaltAndExpiry memory emptyAVSRegSig; @@ -2394,12 +2403,26 @@ contract RegistryCoordinatorUnitTests_BeforeMigration is RegistryCoordinatorUnit }); uint32 lookAheadPeriod = 100; + assertEq( + registryCoordinator.quorumCount(), + 0, + "No quorums should exist before" + ); + // Attempt to create quorum with slashable stake type before enabling operator sets cheats.prank(registryCoordinatorOwner); - cheats.expectRevert(); registryCoordinator.createSlashableStakeQuorum( operatorSetParams, minimumStake, strategyParams, lookAheadPeriod ); + assertEq( + registryCoordinator.quorumCount(), + 1, + "New quorum 0 should be created" + ); + assertFalse( + registryCoordinator.isM2Quorum(0), + "Quorum created should not be an M2 quorum" + ); } } @@ -2744,8 +2767,6 @@ contract RegistryCoordinatorUnitTests_AfterMigration is RegistryCoordinatorUnitT function test_deregisterHook_Reverts_WhenNotALM() public { _deployMockEigenLayerAndAVS(0); - assertTrue(registryCoordinator.operatorSetsEnabled(), "operatorSetsEnabled should be true"); - // Create quorum params ISlashingRegistryCoordinatorTypes.OperatorSetParam memory operatorSetParams = ISlashingRegistryCoordinatorTypes.OperatorSetParam({ @@ -2764,6 +2785,9 @@ contract RegistryCoordinatorUnitTests_AfterMigration is RegistryCoordinatorUnitT cheats.prank(registryCoordinatorOwner); registryCoordinator.createTotalDelegatedStakeQuorum(operatorSetParams, 0, strategyParams); + // operator sets should be enabled after creating a new quorum + assertTrue(registryCoordinator.operatorSetsEnabled(), "operatorSetsEnabled should be true"); + // Prank as allocation manager and call register hook uint32[] memory operatorSetIds = new uint32[](1); operatorSetIds[0] = 0; diff --git a/test/utils/MockAVSDeployer.sol b/test/utils/MockAVSDeployer.sol index bc9e91db..0b049bf3 100644 --- a/test/utils/MockAVSDeployer.sol +++ b/test/utils/MockAVSDeployer.sol @@ -357,6 +357,8 @@ contract MockAVSDeployer is Test { operatorStateRetriever = new OperatorStateRetriever(); + // Set RegistryCoordinator as M2 state with existing quorums + registryCoordinator.setM2QuorumBitmap(0); registryCoordinator.setOperatorSetsEnabled(false); registryCoordinator.setM2QuorumRegistrationDisabled(false); } From 1e40a4a4c39f5a7d87037a679bdd228a1abd8dbd Mon Sep 17 00:00:00 2001 From: gpsanant Date: Tue, 4 Feb 2025 20:33:33 -0800 Subject: [PATCH 10/16] feat: add updateOperators --- src/SlashingRegistryCoordinator.sol | 33 +++- .../ISlashingRegistryCoordinator.sol | 11 ++ test/unit/RegistryCoordinatorUnit.t.sol | 170 +++++++++--------- 3 files changed, 126 insertions(+), 88 deletions(-) diff --git a/src/SlashingRegistryCoordinator.sol b/src/SlashingRegistryCoordinator.sol index 028a14e6..cfab3d85 100644 --- a/src/SlashingRegistryCoordinator.sol +++ b/src/SlashingRegistryCoordinator.sol @@ -225,6 +225,34 @@ contract SlashingRegistryCoordinator is _deregisterOperator(operator, quorumNumbers); } + /// @inheritdoc ISlashingRegistryCoordinator + function updateOperators( + address[] memory operators + ) external override onlyWhenNotPaused(PAUSED_UPDATE_OPERATOR) { + for (uint256 i = 0; i < operators.length; i++) { + // create single-element arrays for the operator and operatorId + address[] memory singleOperator = new address[](1); + singleOperator[0] = operators[i]; + bytes32[] memory singleOperatorId = new bytes32[](1); + singleOperatorId[0] = _operatorInfo[operators[i]].operatorId; + + uint192 currentBitmap = _currentOperatorBitmap(singleOperatorId[0]); + bytes memory quorumNumbers = currentBitmap.bitmapToBytesArray(); + for (uint256 j = 0; j < quorumNumbers.length; j++) { + // update the operator's stake for each quorum + uint8 quorumNumber = uint8(quorumNumbers[j]); + bool[] memory shouldBeDeregistered = + stakeRegistry.updateOperatorsStake(singleOperator, singleOperatorId, quorumNumber); + + if (shouldBeDeregistered[0]) { + bytes memory singleQuorumNumber = new bytes(1); + singleQuorumNumber[0] = quorumNumbers[j]; + _deregisterOperator(operators[i], singleQuorumNumber); + } + } + } + } + /// @inheritdoc ISlashingRegistryCoordinator function updateOperatorsForQuorum( address[][] memory operatorsPerQuorum, @@ -270,13 +298,12 @@ contract SlashingRegistryCoordinator is prevOperatorAddress = operator; } - bytes memory quorumNumberBytes = new bytes(1); - quorumNumberBytes[0] = bytes1(quorumNumber); + bool[] memory shouldBeDeregistered = stakeRegistry.updateOperatorsStake(currQuorumOperators, operatorIds, quorumNumber); for (uint256 j = 0; j < currQuorumOperators.length; ++j) { if (shouldBeDeregistered[j]) { - _deregisterOperator(currQuorumOperators[j], quorumNumberBytes); + _deregisterOperator(currQuorumOperators[j], quorumNumbers[i:i+1]); } } diff --git a/src/interfaces/ISlashingRegistryCoordinator.sol b/src/interfaces/ISlashingRegistryCoordinator.sol index ac9a0b53..2201439a 100644 --- a/src/interfaces/ISlashingRegistryCoordinator.sol +++ b/src/interfaces/ISlashingRegistryCoordinator.sol @@ -321,6 +321,17 @@ interface ISlashingRegistryCoordinator is */ function deregisterOperator(address operator, uint32[] memory operatorSetIds) external; + /** + * @notice Updates stake weights for specified operators. If any operator is found to be below + * the minimum stake for their registered quorums, they are deregistered from those quorums. + * @param operators The operators whose stakes should be updated. + * @dev Stakes are queried from the Eigenlayer core DelegationManager contract. + * @dev WILL BE DEPRECATED IN FAVOR OF updateOperatorsForQuorum + */ + function updateOperators( + address[] memory operators + ) external; + /** * @notice For each quorum in `quorumNumbers`, updates the StakeRegistry's view of ALL its registered operators' stakes. * Each quorum's `quorumUpdateBlockNumber` is also updated, which tracks the most recent block number when ALL registered diff --git a/test/unit/RegistryCoordinatorUnit.t.sol b/test/unit/RegistryCoordinatorUnit.t.sol index 771fd2ba..388525a1 100644 --- a/test/unit/RegistryCoordinatorUnit.t.sol +++ b/test/unit/RegistryCoordinatorUnit.t.sol @@ -1955,91 +1955,91 @@ contract RegistryCoordinatorUnitTests_RegisterOperatorWithChurn is RegistryCoord } contract RegistryCoordinatorUnitTests_UpdateOperators is RegistryCoordinatorUnitTests { - // function test_updateOperators_revert_paused() public { - // cheats.prank(pauser); - // registryCoordinator.pause(2 ** PAUSED_UPDATE_OPERATOR); - - // address[] memory operatorsToUpdate = new address[](1); - // operatorsToUpdate[0] = defaultOperator; - - // cheats.expectRevert(bytes4(keccak256("CurrentlyPaused()"))); - // registryCoordinator.updateOperators(operatorsToUpdate); - // } - - // // @notice tests the `updateOperators` function with a single registered operator as input - // function test_updateOperators_singleOperator() public { - // // register the default operator - // ISignatureUtils.SignatureWithSaltAndExpiry memory emptySig; - // uint32 registrationBlockNumber = 100; - // bytes memory quorumNumbers = new bytes(1); - // quorumNumbers[0] = bytes1(defaultQuorumNumber); - // _setOperatorWeight(defaultOperator, uint8(quorumNumbers[0]), defaultStake); - // cheats.startPrank(defaultOperator); - // cheats.roll(registrationBlockNumber); - // registryCoordinator.registerOperator( - // quorumNumbers, defaultSocket, pubkeyRegistrationParams, emptySig - // ); - - // address[] memory operatorsToUpdate = new address[](1); - // operatorsToUpdate[0] = defaultOperator; - - // registryCoordinator.updateOperators(operatorsToUpdate); - // } - - // // @notice tests the `updateOperators` function with a single registered operator as input - // // @dev also sets up return data from the StakeRegistry - // function testFuzz_updateOperators_singleOperator( - // uint192 registrationBitmap, - // uint192 mockReturnData - // ) public { - // // filter fuzzed inputs to only valid inputs - // cheats.assume(registrationBitmap != 0); - // mockReturnData = (mockReturnData & registrationBitmap); - // emit log_named_uint("mockReturnData", mockReturnData); - - // // register the default operator - // ISignatureUtils.SignatureWithSaltAndExpiry memory emptySig; - // uint32 registrationBlockNumber = 100; - // bytes memory quorumNumbers = BitmapUtils.bitmapToBytesArray(registrationBitmap); - // for (uint256 i = 0; i < quorumNumbers.length; ++i) { - // _setOperatorWeight(defaultOperator, uint8(quorumNumbers[i]), defaultStake); - // } - // cheats.startPrank(defaultOperator); - // cheats.roll(registrationBlockNumber); - // registryCoordinator.registerOperator( - // quorumNumbers, defaultSocket, pubkeyRegistrationParams, emptySig - // ); - - // address[] memory operatorsToUpdate = new address[](1); - // operatorsToUpdate[0] = defaultOperator; - - // uint192 quorumBitmapBefore = registryCoordinator.getCurrentQuorumBitmap(defaultOperatorId); - // assertEq(quorumBitmapBefore, registrationBitmap, "operator bitmap somehow incorrect"); - - // // make the stake registry return info that the operator should be removed from quorums - // uint192 quorumBitmapToRemove = mockReturnData; - // bytes memory quorumNumbersToRemove = BitmapUtils.bitmapToBytesArray(quorumBitmapToRemove); - // for (uint256 i = 0; i < quorumNumbersToRemove.length; ++i) { - // _setOperatorWeight(defaultOperator, uint8(quorumNumbersToRemove[i]), 0); - // } - // uint256 expectedQuorumBitmap = BitmapUtils.minus(quorumBitmapBefore, quorumBitmapToRemove); - - // registryCoordinator.updateOperators(operatorsToUpdate); - // uint192 quorumBitmapAfter = registryCoordinator.getCurrentQuorumBitmap(defaultOperatorId); - // assertEq(expectedQuorumBitmap, quorumBitmapAfter, "quorum bitmap did not update correctly"); - // } - - // // @notice tests the `updateOperators` function with a single *un*registered operator as input - // function test_updateOperators_unregisteredOperator() public view { - // address[] memory operatorsToUpdate = new address[](1); - // operatorsToUpdate[0] = defaultOperator; - - // // force a staticcall to the `updateOperators` function -- this should *pass* because the call should be a strict no-op! - // (bool success,) = address(registryCoordinator).staticcall( - // abi.encodeWithSignature("updateOperators(address[])", operatorsToUpdate) - // ); - // require(success, "staticcall failed!"); - // } + function test_updateOperators_revert_paused() public { + cheats.prank(pauser); + registryCoordinator.pause(2 ** PAUSED_UPDATE_OPERATOR); + + address[] memory operatorsToUpdate = new address[](1); + operatorsToUpdate[0] = defaultOperator; + + cheats.expectRevert(bytes4(keccak256("CurrentlyPaused()"))); + registryCoordinator.updateOperators(operatorsToUpdate); + } + + // @notice tests the `updateOperators` function with a single registered operator as input + function test_updateOperators_singleOperator() public { + // register the default operator + ISignatureUtils.SignatureWithSaltAndExpiry memory emptySig; + uint32 registrationBlockNumber = 100; + bytes memory quorumNumbers = new bytes(1); + quorumNumbers[0] = bytes1(defaultQuorumNumber); + _setOperatorWeight(defaultOperator, uint8(quorumNumbers[0]), defaultStake); + cheats.startPrank(defaultOperator); + cheats.roll(registrationBlockNumber); + registryCoordinator.registerOperator( + quorumNumbers, defaultSocket, pubkeyRegistrationParams, emptySig + ); + + address[] memory operatorsToUpdate = new address[](1); + operatorsToUpdate[0] = defaultOperator; + + registryCoordinator.updateOperators(operatorsToUpdate); + } + + // @notice tests the `updateOperators` function with a single registered operator as input + // @dev also sets up return data from the StakeRegistry + function testFuzz_updateOperators_singleOperator( + uint192 registrationBitmap, + uint192 mockReturnData + ) public { + // filter fuzzed inputs to only valid inputs + cheats.assume(registrationBitmap != 0); + mockReturnData = (mockReturnData & registrationBitmap); + emit log_named_uint("mockReturnData", mockReturnData); + + // register the default operator + ISignatureUtils.SignatureWithSaltAndExpiry memory emptySig; + uint32 registrationBlockNumber = 100; + bytes memory quorumNumbers = BitmapUtils.bitmapToBytesArray(registrationBitmap); + for (uint256 i = 0; i < quorumNumbers.length; ++i) { + _setOperatorWeight(defaultOperator, uint8(quorumNumbers[i]), defaultStake); + } + cheats.startPrank(defaultOperator); + cheats.roll(registrationBlockNumber); + registryCoordinator.registerOperator( + quorumNumbers, defaultSocket, pubkeyRegistrationParams, emptySig + ); + + address[] memory operatorsToUpdate = new address[](1); + operatorsToUpdate[0] = defaultOperator; + + uint192 quorumBitmapBefore = registryCoordinator.getCurrentQuorumBitmap(defaultOperatorId); + assertEq(quorumBitmapBefore, registrationBitmap, "operator bitmap somehow incorrect"); + + // make the stake registry return info that the operator should be removed from quorums + uint192 quorumBitmapToRemove = mockReturnData; + bytes memory quorumNumbersToRemove = BitmapUtils.bitmapToBytesArray(quorumBitmapToRemove); + for (uint256 i = 0; i < quorumNumbersToRemove.length; ++i) { + _setOperatorWeight(defaultOperator, uint8(quorumNumbersToRemove[i]), 0); + } + uint256 expectedQuorumBitmap = BitmapUtils.minus(quorumBitmapBefore, quorumBitmapToRemove); + + registryCoordinator.updateOperators(operatorsToUpdate); + uint192 quorumBitmapAfter = registryCoordinator.getCurrentQuorumBitmap(defaultOperatorId); + assertEq(expectedQuorumBitmap, quorumBitmapAfter, "quorum bitmap did not update correctly"); + } + + // @notice tests the `updateOperators` function with a single *un*registered operator as input + function test_updateOperators_unregisteredOperator() public view { + address[] memory operatorsToUpdate = new address[](1); + operatorsToUpdate[0] = defaultOperator; + + // force a staticcall to the `updateOperators` function -- this should *pass* because the call should be a strict no-op! + (bool success,) = address(registryCoordinator).staticcall( + abi.encodeWithSignature("updateOperators(address[])", operatorsToUpdate) + ); + require(success, "staticcall failed!"); + } function test_updateOperatorsForQuorum_revert_paused() public { cheats.prank(pauser); From 11817b7c759acbdaf39b400c839a899f12355c98 Mon Sep 17 00:00:00 2001 From: Michael Sun Date: Wed, 5 Feb 2025 13:52:14 -0500 Subject: [PATCH 11/16] fix: tests and `_afterDeregisterOperator` hook --- src/RegistryCoordinator.sol | 40 +++++++++++++++------- src/SlashingRegistryCoordinator.sol | 9 ++--- test/integration/CoreRegistration.t.sol | 10 ++++-- test/integration/IntegrationConfig.t.sol | 7 ++++ test/integration/IntegrationDeployer.t.sol | 22 +++++++----- test/integration/User.t.sol | 2 ++ test/unit/StakeRegistryUnit.t.sol | 15 ++++---- 7 files changed, 70 insertions(+), 35 deletions(-) diff --git a/src/RegistryCoordinator.sol b/src/RegistryCoordinator.sol index e5dad520..c5807ea5 100644 --- a/src/RegistryCoordinator.sol +++ b/src/RegistryCoordinator.sol @@ -126,9 +126,11 @@ contract RegistryCoordinator is RegistryCoordinatorStorage { bytes memory quorumNumbers ) external override onlyWhenNotPaused(PAUSED_DEREGISTER_OPERATOR) { // Check that the quorum numbers are M2 quorums - for (uint256 i = 0; i < quorumNumbers.length; i++) { - require(_isM2Quorum(uint8(quorumNumbers[i])), OperatorSetQuorum()); - } + require( + quorumNumbers.orderedBytesArrayToBitmap().isSubsetOf(m2QuorumBitmap()), + OnlyM2QuorumsAllowed() + ); + _deregisterOperator({operator: msg.sender, quorumNumbers: quorumNumbers}); } @@ -154,7 +156,8 @@ contract RegistryCoordinator is RegistryCoordinatorStorage { bytes memory quorumNumbers ) internal virtual override { // filter out M2 quorums from the quorum numbers - uint256 operatorSetBitmap = quorumNumbers.orderedBytesArrayToBitmap().minus(m2QuorumBitmap()); + uint256 operatorSetBitmap = + quorumNumbers.orderedBytesArrayToBitmap().minus(m2QuorumBitmap()); if (!operatorSetBitmap.isEmpty()) { // call the parent _forceDeregisterOperator function for operator sets quorums super._forceDeregisterOperator(operator, operatorSetBitmap.bitmapToBytesArray()); @@ -180,7 +183,13 @@ contract RegistryCoordinator is RegistryCoordinatorStorage { bytes memory, uint192 newBitmap ) internal virtual override { - uint256 operatorM2QuorumBitmap = newBitmap.minus(m2QuorumBitmap()); + // Bitmap representing all quorums including M2 and OperatorSet quorums + uint256 totalQuorumBitmap = _getTotalQuorumBitmap(); + // Bitmap representing only OperatorSet quorums. Equal to 0 if operatorSets not enabled + uint256 operatorSetQuorumBitmap = totalQuorumBitmap.minus(m2QuorumBitmap()); + // Operators updated M2 quorum bitmap, clear all the bits of operatorSetQuorumBitmap which gives the + // operator's M2 quorum bitmap. + uint256 operatorM2QuorumBitmap = newBitmap.minus(operatorSetQuorumBitmap); // If the operator is no longer registered for any M2 quorums, update their status and deregister // them from the AVS via the EigenLayer core contracts if (operatorM2QuorumBitmap.isEmpty()) { @@ -188,14 +197,8 @@ contract RegistryCoordinator is RegistryCoordinatorStorage { } } - /// @dev Returns a bitmap with all bits set up to `quorumCount`. Used for bit-masking quorum numbers - /// and differentiating between operator sets and M2 quorums - function m2QuorumBitmap() public view returns (uint256) { - // If operator sets are enabled, return the current m2 quorum bitmap - if (operatorSetsEnabled) { - return _m2QuorumBitmap; - } - + /// @notice Return bitmap representing all quorums(Legacy M2 and OperatorSet) quorums + function _getTotalQuorumBitmap() internal view returns (uint256) { // This creates a number where all bits up to quorumCount are set to 1 // For example: // quorumCount = 3 -> 0111 (7 in decimal) @@ -218,6 +221,17 @@ contract RegistryCoordinator is RegistryCoordinatorStorage { * */ + /// @dev Returns a bitmap with all bits set up to `quorumCount`. Used for bit-masking quorum numbers + /// and differentiating between operator sets and M2 quorums + function m2QuorumBitmap() public view returns (uint256) { + // If operator sets are enabled, return the current m2 quorum bitmap + if (operatorSetsEnabled) { + return _m2QuorumBitmap; + } + + return _getTotalQuorumBitmap(); + } + /// @notice Returns true if the quorum number is an M2 quorum function isM2Quorum( uint8 quorumNumber diff --git a/src/SlashingRegistryCoordinator.sol b/src/SlashingRegistryCoordinator.sol index cfab3d85..1b5b4cdf 100644 --- a/src/SlashingRegistryCoordinator.sol +++ b/src/SlashingRegistryCoordinator.sol @@ -241,9 +241,10 @@ contract SlashingRegistryCoordinator is for (uint256 j = 0; j < quorumNumbers.length; j++) { // update the operator's stake for each quorum uint8 quorumNumber = uint8(quorumNumbers[j]); - bool[] memory shouldBeDeregistered = - stakeRegistry.updateOperatorsStake(singleOperator, singleOperatorId, quorumNumber); - + bool[] memory shouldBeDeregistered = stakeRegistry.updateOperatorsStake( + singleOperator, singleOperatorId, quorumNumber + ); + if (shouldBeDeregistered[0]) { bytes memory singleQuorumNumber = new bytes(1); singleQuorumNumber[0] = quorumNumbers[j]; @@ -303,7 +304,7 @@ contract SlashingRegistryCoordinator is stakeRegistry.updateOperatorsStake(currQuorumOperators, operatorIds, quorumNumber); for (uint256 j = 0; j < currQuorumOperators.length; ++j) { if (shouldBeDeregistered[j]) { - _deregisterOperator(currQuorumOperators[j], quorumNumbers[i:i+1]); + _deregisterOperator(currQuorumOperators[j], quorumNumbers[i:i + 1]); } } diff --git a/test/integration/CoreRegistration.t.sol b/test/integration/CoreRegistration.t.sol index 42359be0..80719427 100644 --- a/test/integration/CoreRegistration.t.sol +++ b/test/integration/CoreRegistration.t.sol @@ -188,14 +188,20 @@ contract Test_CoreRegistration is MockAVSDeployer { emit log_named_bytes("quorumNumbers", quorumNumbers); _registerOperator(quorumNumbers); + IAVSDirectoryTypes.OperatorAVSRegistrationStatus operatorStatus = + avsDirectory.avsOperatorStatus(address(serviceManager), operator); + assertEq( + uint8(operatorStatus), + uint8(IAVSDirectoryTypes.OperatorAVSRegistrationStatus.REGISTERED) + ); + // Deregister Operator with single quorum quorumNumbers = new bytes(1); cheats.prank(operator); registryCoordinator.deregisterOperator(quorumNumbers); // Check operator is still registered - IAVSDirectoryTypes.OperatorAVSRegistrationStatus operatorStatus = - avsDirectory.avsOperatorStatus(address(serviceManager), operator); + operatorStatus = avsDirectory.avsOperatorStatus(address(serviceManager), operator); assertEq( uint8(operatorStatus), uint8(IAVSDirectoryTypes.OperatorAVSRegistrationStatus.REGISTERED) diff --git a/test/integration/IntegrationConfig.t.sol b/test/integration/IntegrationConfig.t.sol index 92c394bb..c3ae4f67 100644 --- a/test/integration/IntegrationConfig.t.sol +++ b/test/integration/IntegrationConfig.t.sol @@ -185,6 +185,13 @@ contract IntegrationConfig is IntegrationDeployer, G2Operations, Constants { }); } + /// Setup the RegistryCoordinator as M2 RegistryCoordinator with M2 quorums + /// TODO: refactor Integration framework to test both M2 upgrade path and new + /// registration/deregistration flow of operatorSets + _setOperatorSetsEnabled(false); + _setM2QuorumsDisabled(false); + _setM2QuorumBitmap(0); + // Decide how many operators to register for each quorum initially uint256 initialOperators = _randInitialOperators(operatorSet); emit log( diff --git a/test/integration/IntegrationDeployer.t.sol b/test/integration/IntegrationDeployer.t.sol index 3e38d697..877c5910 100644 --- a/test/integration/IntegrationDeployer.t.sol +++ b/test/integration/IntegrationDeployer.t.sol @@ -419,11 +419,7 @@ abstract contract IntegrationDeployer is Test, IUserDeployer { churnApprover, ejector, 0, /*initialPausedStatus*/ - new IRegistryCoordinator.OperatorSetParam[](0), - new uint96[](0), - new IStakeRegistryTypes.StrategyParams[][](0), - quorumStakeTypes, - slashableStakeQuorumLookAheadPeriods + address(serviceManager) /* accountIdentifier */ ) ); @@ -465,6 +461,7 @@ abstract contract IntegrationDeployer is Test, IUserDeployer { _setOperatorSetsEnabled(false); _setM2QuorumsDisabled(false); + _setM2QuorumBitmap(0); } /// @notice Overwrite RegistryCoordinator.operatorSetsEnabled to the specified value. @@ -474,14 +471,14 @@ abstract contract IntegrationDeployer is Test, IUserDeployer { ) internal { // 1. First read the current value of the entire slot // which holds operatorSetsEnabled, m2QuorumsDisabled, and accountIdentifier - bytes32 currentSlot = cheats.load(address(registryCoordinator), bytes32(uint256(161))); + bytes32 currentSlot = cheats.load(address(registryCoordinator), bytes32(uint256(200))); // 2. Clear only the first byte (operatorSetsEnabled) while keeping the rest bytes32 newSlot = (currentSlot & ~bytes32(uint256(0xff))) | bytes32(uint256(operatorSetsEnabled ? 0x01 : 0x00)); // 3. Store the modified slot - cheats.store(address(registryCoordinator), bytes32(uint256(161)), newSlot); + cheats.store(address(registryCoordinator), bytes32(uint256(200)), newSlot); } /// @notice Overwrite RegistryCoordinator.m2QuorumsDisabled to the specified value. @@ -490,14 +487,21 @@ abstract contract IntegrationDeployer is Test, IUserDeployer { ) internal { // 1. First read the current value of the entire slot // which holds operatorSetsEnabled, m2QuorumsDisabled, and accountIdentifier - bytes32 currentSlot = cheats.load(address(registryCoordinator), bytes32(uint256(161))); + bytes32 currentSlot = cheats.load(address(registryCoordinator), bytes32(uint256(200))); // 2. Clear only the second byte (m2QuorumsDisabled) while keeping the rest bytes32 newSlot = (currentSlot & ~bytes32(uint256(0xff) << 8)) | bytes32(uint256(m2QuorumsDisabled ? 0x01 : 0x00) << 8); // 3. Store the modified slot - cheats.store(address(registryCoordinator), bytes32(uint256(161)), newSlot); + cheats.store(address(registryCoordinator), bytes32(uint256(200)), newSlot); + } + + /// @notice Overwrite RegistryCoordinator._m2QuorumBitmap to the specified value + function _setM2QuorumBitmap(uint256 m2QuorumBitmap) internal { + bytes32 currentSlot = cheats.load(address(registryCoordinator), bytes32(uint256(200))); + + cheats.store(address(registryCoordinator), bytes32(uint256(200)), bytes32(m2QuorumBitmap)); } /// @dev Deploy a strategy and its underlying token, push to global lists of tokens/strategies, and whitelist diff --git a/test/integration/User.t.sol b/test/integration/User.t.sol index f69eb84e..0ea3d320 100644 --- a/test/integration/User.t.sol +++ b/test/integration/User.t.sol @@ -254,6 +254,8 @@ contract User is Test { for (uint256 j = 0; j < operatorIds.length; j++) { operatorsPerQuorum[i][j] = blsApkRegistry.pubkeyHashToOperator(operatorIds[j]); } + + operatorsPerQuorum[i] = Sort.sortAddresses(operatorsPerQuorum[i]); } registryCoordinator.updateOperatorsForQuorum(operatorsPerQuorum, quorumNumbers); } diff --git a/test/unit/StakeRegistryUnit.t.sol b/test/unit/StakeRegistryUnit.t.sol index aab4e08f..ba98c4d7 100644 --- a/test/unit/StakeRegistryUnit.t.sol +++ b/test/unit/StakeRegistryUnit.t.sol @@ -1817,11 +1817,12 @@ contract StakeRegistryUnitTests_StakeUpdates is StakeRegistryUnitTests { // Get a list of valid quorums ending in an invalid quorum number bytes memory invalidQuorums = _fuzz_getInvalidQuorums(rand); + uint256 length = invalidQuorums.length; cheats.expectRevert(IStakeRegistryErrors.QuorumDoesNotExist.selector); cheats.prank(address(registryCoordinator)); stakeRegistry.updateOperatorsStake( - _wrap(setup.operator), _wrap(setup.operatorId), uint8(invalidQuorums[0]) + _wrap(setup.operator), _wrap(setup.operatorId), uint8(invalidQuorums[length - 1]) ); } @@ -1848,13 +1849,13 @@ contract StakeRegistryUnitTests_StakeUpdates is StakeRegistryUnitTests { _getLatestTotalStakeUpdates(setup.quorumNumbers); // updateOperatorStake - cheats.prank(address(registryCoordinator)); bool[] memory shouldBeDeregistered = new bool[](setup.quorumNumbers.length); for (uint256 i = 0; i < setup.quorumNumbers.length; i++) { + cheats.prank(address(registryCoordinator)); bool[] memory shouldBeDeregisteredForQuorum = stakeRegistry.updateOperatorsStake( _wrap(setup.operator), _wrap(setup.operatorId), uint8(setup.quorumNumbers[i]) ); - shouldBeDeregistered[i] = shouldBeDeregisteredForQuorum[i]; + shouldBeDeregistered[i] = shouldBeDeregisteredForQuorum[0]; } // Get ending state @@ -1971,8 +1972,8 @@ contract StakeRegistryUnitTests_StakeUpdates is StakeRegistryUnitTests { UpdateSetup memory setup = setups[i]; // updateOperatorStake - cheats.prank(address(registryCoordinator)); for (uint256 j = 0; j < setup.quorumNumbers.length; j++) { + cheats.prank(address(registryCoordinator)); stakeRegistry.updateOperatorsStake( _wrap(setup.operator), _wrap(setup.operatorId), uint8(setup.quorumNumbers[j]) ); @@ -2065,14 +2066,14 @@ contract StakeRegistryUnitTests_StakeUpdates is StakeRegistryUnitTests { uint256 currBlock = startBlock + j; cheats.roll(currBlock); - // updateOperatorStake - cheats.prank(address(registryCoordinator)); + // updateOperatorsStake bool[] memory shouldBeDeregistered = new bool[](setup.quorumNumbers.length); for (uint256 i = 0; i < setup.quorumNumbers.length; i++) { + cheats.prank(address(registryCoordinator)); bool[] memory shouldBeDeregisteredForQuorum = stakeRegistry.updateOperatorsStake( _wrap(setup.operator), _wrap(setup.operatorId), uint8(setup.quorumNumbers[i]) ); - shouldBeDeregistered[i] = shouldBeDeregisteredForQuorum[i]; + shouldBeDeregistered[i] = shouldBeDeregisteredForQuorum[0]; } // Get ending state From 441cff47edf114a10edd13081a79a703f4f56d6c Mon Sep 17 00:00:00 2001 From: Michael Sun Date: Wed, 5 Feb 2025 15:11:15 -0500 Subject: [PATCH 12/16] refactor: remove operatorSetEnabled as require --- src/RegistryCoordinator.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/src/RegistryCoordinator.sol b/src/RegistryCoordinator.sol index c5807ea5..5a7340b9 100644 --- a/src/RegistryCoordinator.sol +++ b/src/RegistryCoordinator.sol @@ -136,7 +136,6 @@ contract RegistryCoordinator is RegistryCoordinatorStorage { /// @inheritdoc IRegistryCoordinator function disableM2QuorumRegistration() external onlyOwner { - require(operatorSetsEnabled, OperatorSetsNotEnabled()); require(!isM2QuorumRegistrationDisabled, M2QuorumRegistrationIsDisabled()); isM2QuorumRegistrationDisabled = true; From b3df455b3bc669f51582dccf59682a83185050c7 Mon Sep 17 00:00:00 2001 From: Michael Sun Date: Wed, 5 Feb 2025 16:36:23 -0500 Subject: [PATCH 13/16] chore: fmt --- src/SocketRegistry.sol | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/SocketRegistry.sol b/src/SocketRegistry.sol index e79d1fa3..f8938152 100644 --- a/src/SocketRegistry.sol +++ b/src/SocketRegistry.sol @@ -13,10 +13,7 @@ import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol"; contract SocketRegistry is ISocketRegistry, SocketRegistryStorage { /// @notice A modifier that only allows the RegistryCoordinator to call a function modifier onlySlashingRegistryCoordinator() { - require( - msg.sender == slashingRegistryCoordinator, - OnlySlashingRegistryCoordinator() - ); + require(msg.sender == slashingRegistryCoordinator, OnlySlashingRegistryCoordinator()); _; } From d73f48386f131f0bd8d62e3072b2971ca83836dc Mon Sep 17 00:00:00 2001 From: Michael Sun Date: Wed, 5 Feb 2025 16:44:28 -0500 Subject: [PATCH 14/16] chore: fmt test files --- .../RegistryCoordinatorHarness.t.sol | 4 +++- test/integration/IntegrationDeployer.t.sol | 4 +++- test/unit/RegistryCoordinatorUnit.t.sol | 19 +++---------------- 3 files changed, 9 insertions(+), 18 deletions(-) diff --git a/test/harnesses/RegistryCoordinatorHarness.t.sol b/test/harnesses/RegistryCoordinatorHarness.t.sol index 780ee1f4..faaf785a 100644 --- a/test/harnesses/RegistryCoordinatorHarness.t.sol +++ b/test/harnesses/RegistryCoordinatorHarness.t.sol @@ -80,7 +80,9 @@ contract RegistryCoordinatorHarness is RegistryCoordinator, Test { isM2QuorumRegistrationDisabled = disabled; } - function setM2QuorumBitmap(uint256 bitmap) external { + function setM2QuorumBitmap( + uint256 bitmap + ) external { _m2QuorumBitmap = bitmap; } } diff --git a/test/integration/IntegrationDeployer.t.sol b/test/integration/IntegrationDeployer.t.sol index 877c5910..6aea8d7f 100644 --- a/test/integration/IntegrationDeployer.t.sol +++ b/test/integration/IntegrationDeployer.t.sol @@ -498,7 +498,9 @@ abstract contract IntegrationDeployer is Test, IUserDeployer { } /// @notice Overwrite RegistryCoordinator._m2QuorumBitmap to the specified value - function _setM2QuorumBitmap(uint256 m2QuorumBitmap) internal { + function _setM2QuorumBitmap( + uint256 m2QuorumBitmap + ) internal { bytes32 currentSlot = cheats.load(address(registryCoordinator), bytes32(uint256(200))); cheats.store(address(registryCoordinator), bytes32(uint256(200)), bytes32(m2QuorumBitmap)); diff --git a/test/unit/RegistryCoordinatorUnit.t.sol b/test/unit/RegistryCoordinatorUnit.t.sol index 388525a1..6fd226e0 100644 --- a/test/unit/RegistryCoordinatorUnit.t.sol +++ b/test/unit/RegistryCoordinatorUnit.t.sol @@ -705,7 +705,6 @@ contract RegistryCoordinatorUnitTests_DeregisterOperator_EjectOperator is } function test_deregisterOperator_revert_incorrectQuorums() public { - console.log("quorumCount", registryCoordinator.quorumCount()); assertTrue( @@ -2403,26 +2402,15 @@ contract RegistryCoordinatorUnitTests_BeforeMigration is RegistryCoordinatorUnit }); uint32 lookAheadPeriod = 100; - assertEq( - registryCoordinator.quorumCount(), - 0, - "No quorums should exist before" - ); + assertEq(registryCoordinator.quorumCount(), 0, "No quorums should exist before"); // Attempt to create quorum with slashable stake type before enabling operator sets cheats.prank(registryCoordinatorOwner); registryCoordinator.createSlashableStakeQuorum( operatorSetParams, minimumStake, strategyParams, lookAheadPeriod ); - assertEq( - registryCoordinator.quorumCount(), - 1, - "New quorum 0 should be created" - ); - assertFalse( - registryCoordinator.isM2Quorum(0), - "Quorum created should not be an M2 quorum" - ); + assertEq(registryCoordinator.quorumCount(), 1, "New quorum 0 should be created"); + assertFalse(registryCoordinator.isM2Quorum(0), "Quorum created should not be an M2 quorum"); } } @@ -2596,7 +2584,6 @@ contract RegistryCoordinatorUnitTests_AfterMigration is RegistryCoordinatorUnitT vm.skip(true); _deployMockEigenLayerAndAVS(0); - // Create quorum params ISlashingRegistryCoordinatorTypes.OperatorSetParam memory operatorSetParams = ISlashingRegistryCoordinatorTypes.OperatorSetParam({ From 6261e2ffaa84ed1cca418ee4994e7e979ecf30c9 Mon Sep 17 00:00:00 2001 From: Michael Sun Date: Wed, 5 Feb 2025 17:14:46 -0500 Subject: [PATCH 15/16] fix: failing ci test --- test/unit/SocketRegistryUnit.t.sol | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/unit/SocketRegistryUnit.t.sol b/test/unit/SocketRegistryUnit.t.sol index a9d0aa7d..c5409ff1 100644 --- a/test/unit/SocketRegistryUnit.t.sol +++ b/test/unit/SocketRegistryUnit.t.sol @@ -3,6 +3,7 @@ pragma solidity ^0.8.12; import {SocketRegistry} from "../../src/SocketRegistry.sol"; +import {ISocketRegistry, ISocketRegistryErrors} from "../../src/interfaces/ISocketRegistry.sol"; import {IRegistryCoordinator} from "../../src/interfaces/IRegistryCoordinator.sol"; import "../utils/MockAVSDeployer.sol"; @@ -20,7 +21,7 @@ contract SocketRegistryUnitTests is MockAVSDeployer { function test_setOperatorSocket_revert_notSlashingRegistryCoordinator() public { vm.startPrank(address(0)); vm.expectRevert( - "SocketRegistry.onlySlashingRegistryCoordinator: caller is not the SlashingRegistryCoordinator" + ISocketRegistryErrors.OnlySlashingRegistryCoordinator.selector ); socketRegistry.setOperatorSocket(defaultOperatorId, "testSocket"); } From e47a0fb0f5a39652a2f5bb714a71f98725aa95db Mon Sep 17 00:00:00 2001 From: Michael Sun Date: Wed, 5 Feb 2025 17:17:22 -0500 Subject: [PATCH 16/16] chore: test fmt --- test/unit/SocketRegistryUnit.t.sol | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/unit/SocketRegistryUnit.t.sol b/test/unit/SocketRegistryUnit.t.sol index c5409ff1..8eb01715 100644 --- a/test/unit/SocketRegistryUnit.t.sol +++ b/test/unit/SocketRegistryUnit.t.sol @@ -20,9 +20,7 @@ contract SocketRegistryUnitTests is MockAVSDeployer { function test_setOperatorSocket_revert_notSlashingRegistryCoordinator() public { vm.startPrank(address(0)); - vm.expectRevert( - ISocketRegistryErrors.OnlySlashingRegistryCoordinator.selector - ); + vm.expectRevert(ISocketRegistryErrors.OnlySlashingRegistryCoordinator.selector); socketRegistry.setOperatorSocket(defaultOperatorId, "testSocket"); } }