From 74c86d21ba6dcdc060a01f9933085076f9f6facd Mon Sep 17 00:00:00 2001 From: quaq <56312047+0x0aa0@users.noreply.github.com> Date: Tue, 30 Apr 2024 10:19:21 -0400 Subject: [PATCH] feat: reregistration delay (#246) * feat: rereg delay * test: fix tests * chore: natspec * test: unit * fix: grief * refactor: ejection manager * fix: units * fix: name nit * docs: ejector dev tag --- src/EjectionManager.sol | 33 ++++++--------- src/RegistryCoordinator.sol | 34 +++++++++++++-- src/RegistryCoordinatorStorage.sol | 7 +++- src/interfaces/IEjectionManager.sol | 6 +-- test/integration/User.t.sol | 2 + test/unit/EjectionManagerUnit.t.sol | 53 +++++------------------ test/unit/RegistryCoordinatorUnit.t.sol | 56 +++++++++++++++++++++++++ 7 files changed, 121 insertions(+), 70 deletions(-) diff --git a/src/EjectionManager.sol b/src/EjectionManager.sol index 78181063..4668c4c8 100644 --- a/src/EjectionManager.sol +++ b/src/EjectionManager.sol @@ -60,15 +60,12 @@ contract EjectionManager is IEjectionManager, OwnableUpgradeable{ /** * @notice Ejects operators from the AVSs RegistryCoordinator under a ratelimit * @param _operatorIds The ids of the operators 'j' to eject for each quorum 'i' - * @return ejectedOperatorsForQuorum The total number of operators ejected for each quorum 'i' - * @dev This function will eject as many operators as possible without reverting prioritizing operators at the lower index + * @dev This function will eject as many operators as possible prioritizing operators at the lower index * @dev The owner can eject operators without recording of stake ejection */ - function ejectOperators(bytes32[][] memory _operatorIds) external returns (uint32[] memory){ + function ejectOperators(bytes32[][] memory _operatorIds) external { require(isEjector[msg.sender] || msg.sender == owner(), "Ejector: Only owner or ejector can eject"); - uint32[] memory ejectedOperatorsForQuorum = new uint32[](_operatorIds.length); - for(uint i = 0; i < _operatorIds.length; ++i) { uint8 quorumNumber = uint8(i); @@ -76,7 +73,7 @@ contract EjectionManager is IEjectionManager, OwnableUpgradeable{ uint256 stakeForEjection; uint32 ejectedOperators; - bool broke; + bool ratelimitHit; for(uint8 j = 0; j < _operatorIds[i].length; ++j) { uint256 operatorStake = stakeRegistry.getCurrentStake(_operatorIds[i][j], quorumNumber); @@ -90,35 +87,31 @@ contract EjectionManager is IEjectionManager, OwnableUpgradeable{ timestamp: block.timestamp, stakeEjected: stakeForEjection })); - broke = true; + ratelimitHit = true; break; } - //try-catch used to prevent race condition of operator deregistering before ejection - try registryCoordinator.ejectOperator( + stakeForEjection += operatorStake; + ++ejectedOperators; + + registryCoordinator.ejectOperator( registryCoordinator.getOperatorFromId(_operatorIds[i][j]), abi.encodePacked(quorumNumber) - ) { - stakeForEjection += operatorStake; - ++ejectedOperators; - emit OperatorEjected(_operatorIds[i][j], quorumNumber); - } catch (bytes memory err) { - emit FailedOperatorEjection(_operatorIds[i][j], quorumNumber, err); - } + ); + + emit OperatorEjected(_operatorIds[i][j], quorumNumber); } //record the stake ejected if ejector and ratelimit enforced - if(!broke && isEjector[msg.sender]){ + if(!ratelimitHit && isEjector[msg.sender]){ stakeEjectedForQuorum[quorumNumber].push(StakeEjection({ timestamp: block.timestamp, stakeEjected: stakeForEjection })); } - ejectedOperatorsForQuorum[i] = ejectedOperators; + emit QuorumEjection(ejectedOperators, ratelimitHit); } - - return ejectedOperatorsForQuorum; } /** diff --git a/src/RegistryCoordinator.sol b/src/RegistryCoordinator.sol index 19367892..4006d9f4 100644 --- a/src/RegistryCoordinator.sol +++ b/src/RegistryCoordinator.sol @@ -359,15 +359,28 @@ contract RegistryCoordinator is * @notice Forcibly deregisters an operator from one or more quorums * @param operator the operator to eject * @param quorumNumbers the quorum numbers to eject the operator from + * @dev possible race condition if prior to being ejected for a set of quorums the operator self deregisters from a subset */ function ejectOperator( address operator, bytes calldata quorumNumbers ) external onlyEjector { - _deregisterOperator({ - operator: operator, - quorumNumbers: quorumNumbers - }); + lastEjectionTimestamp[operator] = block.timestamp; + + OperatorInfo storage operatorInfo = _operatorInfo[operator]; + bytes32 operatorId = operatorInfo.operatorId; + uint192 quorumsToRemove = uint192(BitmapUtils.orderedBytesArrayToBitmap(quorumNumbers, quorumCount)); + uint192 currentBitmap = _currentOperatorBitmap(operatorId); + if( + operatorInfo.status == OperatorStatus.REGISTERED && + !quorumsToRemove.isEmpty() && + quorumsToRemove.isSubsetOf(currentBitmap) + ){ + _deregisterOperator({ + operator: operator, + quorumNumbers: quorumNumbers + }); + } } /******************************************************************************* @@ -423,6 +436,16 @@ contract RegistryCoordinator is _setEjector(_ejector); } + /** + * @notice Sets the ejection cooldown, which is the time an operator must wait in + * seconds afer ejection before registering for any quorum + * @param _ejectionCooldown the new ejection cooldown in seconds + * @dev only callable by the owner + */ + function setEjectionCooldown(uint256 _ejectionCooldown) external onlyOwner { + ejectionCooldown = _ejectionCooldown; + } + /******************************************************************************* INTERNAL FUNCTIONS *******************************************************************************/ @@ -457,6 +480,9 @@ contract RegistryCoordinator is require(quorumsToAdd.noBitsInCommon(currentBitmap), "RegistryCoordinator._registerOperator: operator already registered for some quorums being registered for"); uint192 newBitmap = uint192(currentBitmap.plus(quorumsToAdd)); + // Check that the operator can reregister if ejected + require(lastEjectionTimestamp[operator] + ejectionCooldown < block.timestamp, "RegistryCoordinator._registerOperator: operator cannot reregister yet"); + /** * Update operator's bitmap, socket, and status. Only update operatorInfo if needed: * if we're `REGISTERED`, the operatorId and status are already correct. diff --git a/src/RegistryCoordinatorStorage.sol b/src/RegistryCoordinatorStorage.sol index e8c42850..5451efb3 100644 --- a/src/RegistryCoordinatorStorage.sol +++ b/src/RegistryCoordinatorStorage.sol @@ -64,6 +64,11 @@ abstract contract RegistryCoordinatorStorage is IRegistryCoordinator { /// @notice the address of the entity allowed to eject operators from the AVS address public ejector; + /// @notice the last timestamp an operator was ejected + mapping(address => uint256) public lastEjectionTimestamp; + /// @notice the delay in seconds before an operator can reregister after being ejected + uint256 public ejectionCooldown; + constructor( IServiceManager _serviceManager, IStakeRegistry _stakeRegistry, @@ -78,5 +83,5 @@ abstract contract RegistryCoordinatorStorage is IRegistryCoordinator { // storage gap for upgradeability // slither-disable-next-line shadowing-state - uint256[41] private __GAP; + uint256[39] private __GAP; } diff --git a/src/interfaces/IEjectionManager.sol b/src/interfaces/IEjectionManager.sol index 454d0e61..9a02991e 100644 --- a/src/interfaces/IEjectionManager.sol +++ b/src/interfaces/IEjectionManager.sol @@ -25,14 +25,14 @@ interface IEjectionManager { event QuorumEjectionParamsSet(uint8 quorumNumber, uint32 rateLimitWindow, uint16 ejectableStakePercent); ///@notice Emitted when an operator is ejected event OperatorEjected(bytes32 operatorId, uint8 quorumNumber); - ///@notice Emitted when an operator ejection fails - event FailedOperatorEjection(bytes32 operatorId, uint8 quorumNumber, bytes err); + ///@notice Emitted when operators are ejected for a quroum + event QuorumEjection(uint32 ejectedOperators, bool ratelimitHit); /** * @notice Ejects operators from the AVSs registryCoordinator under a ratelimit * @param _operatorIds The ids of the operators to eject for each quorum */ - function ejectOperators(bytes32[][] memory _operatorIds) external returns (uint32[] memory); + function ejectOperators(bytes32[][] memory _operatorIds) external; /** * @notice Sets the ratelimit parameters for a quorum diff --git a/test/integration/User.t.sol b/test/integration/User.t.sol index a94e5f6e..fbb55198 100644 --- a/test/integration/User.t.sol +++ b/test/integration/User.t.sol @@ -117,6 +117,7 @@ contract User is Test { function registerOperator(bytes calldata quorums) public createSnapshot virtual returns (bytes32) { _log("registerOperator", quorums); + vm.warp(block.timestamp + 1); registryCoordinator.registerOperator({ quorumNumbers: quorums, socket: NAME, @@ -208,6 +209,7 @@ contract User is Test { expiry: expiry }); + vm.warp(block.timestamp + 1); registryCoordinator.registerOperatorWithChurn({ quorumNumbers: allQuorums, socket: NAME, diff --git a/test/unit/EjectionManagerUnit.t.sol b/test/unit/EjectionManagerUnit.t.sol index 899874f0..7fcd4905 100644 --- a/test/unit/EjectionManagerUnit.t.sol +++ b/test/unit/EjectionManagerUnit.t.sol @@ -1,4 +1,5 @@ // SPDX-License-Identifier: BUSL-1.1 + pragma solidity ^0.8.12; import {EjectionManager} from "../../src/EjectionManager.sol"; @@ -87,11 +88,7 @@ contract EjectionManagerUnitTests is MockAVSDeployer { } cheats.prank(ejector); - uint32[] memory ejectedOperatorsForQuorum = ejectionManager.ejectOperators(operatorIds); - - for(uint8 i = 0; i < numQuorums; i++) { - assertEq(ejectedOperatorsForQuorum[i], operatorsToEject); - } + ejectionManager.ejectOperators(operatorIds); assertEq(uint8(registryCoordinator.getOperatorStatus(defaultOperator)), uint8(IRegistryCoordinator.OperatorStatus.DEREGISTERED)); } @@ -122,11 +119,7 @@ contract EjectionManagerUnitTests is MockAVSDeployer { } cheats.prank(ejector); - uint32[] memory ejectedOperatorsForQuorum = ejectionManager.ejectOperators(operatorIds); - - for(uint8 i = 0; i < numQuorums; i++) { - assertEq(ejectedOperatorsForQuorum[i], operatorsToEject); - } + ejectionManager.ejectOperators(operatorIds); for(uint8 i = 0; i < operatorsToEject; i++) { assertEq(uint8(registryCoordinator.getOperatorStatus(_incrementAddress(defaultOperator, i))), uint8(IRegistryCoordinator.OperatorStatus.DEREGISTERED)); @@ -160,11 +153,7 @@ contract EjectionManagerUnitTests is MockAVSDeployer { } cheats.prank(ejector); - uint32[] memory ejectedOperatorsForQuorum = ejectionManager.ejectOperators(operatorIds); - - for(uint8 i = 0; i < numQuorums; i++) { - assertEq(ejectedOperatorsForQuorum[i], operatorsCanEject); - } + ejectionManager.ejectOperators(operatorIds); for(uint8 i = 0; i < operatorsCanEject; i++) { assertEq(uint8(registryCoordinator.getOperatorStatus(_incrementAddress(defaultOperator, i))), uint8(IRegistryCoordinator.OperatorStatus.DEREGISTERED)); @@ -201,11 +190,7 @@ contract EjectionManagerUnitTests is MockAVSDeployer { } cheats.prank(ejector); - uint32[] memory ejectedOperatorsForQuorum = ejectionManager.ejectOperators(operatorIds); - - for(uint8 i = 0; i < numQuorums; i++) { - assertEq(ejectedOperatorsForQuorum[i], operatorsToEject); - } + ejectionManager.ejectOperators(operatorIds); for(uint8 i = 0; i < operatorsToEject; i++) { assertEq(uint8(registryCoordinator.getOperatorStatus(_incrementAddress(defaultOperator, i))), uint8(IRegistryCoordinator.OperatorStatus.DEREGISTERED)); @@ -233,11 +218,7 @@ contract EjectionManagerUnitTests is MockAVSDeployer { } cheats.prank(ejector); - ejectedOperatorsForQuorum = ejectionManager.ejectOperators(operatorIds); - - for(uint8 i = 0; i < numQuorums; i++) { - assertEq(ejectedOperatorsForQuorum[i], operatorsToEject); - } + ejectionManager.ejectOperators(operatorIds); for(uint8 i = 0; i < operatorsToEject; i++) { assertEq(uint8(registryCoordinator.getOperatorStatus(_incrementAddress(defaultOperator, operatorsToEject + i))), uint8(IRegistryCoordinator.OperatorStatus.DEREGISTERED)); @@ -251,6 +232,8 @@ contract EjectionManagerUnitTests is MockAVSDeployer { testEjectOperators_MultipleOperatorInsideRatelimit(); + vm.warp(block.timestamp + 1); + _registerOperaters(operatorsToEject, stake); vm.warp(block.timestamp + ratelimitWindow); @@ -275,11 +258,7 @@ contract EjectionManagerUnitTests is MockAVSDeployer { } cheats.prank(ejector); - uint32[] memory ejectedOperatorsForQuorum = ejectionManager.ejectOperators(operatorIds); - - for(uint8 i = 0; i < numQuorums; i++) { - assertEq(ejectedOperatorsForQuorum[i], operatorsToEject); - } + ejectionManager.ejectOperators(operatorIds); for(uint8 i = 0; i < operatorsToEject; i++) { assertEq(uint8(registryCoordinator.getOperatorStatus(_incrementAddress(defaultOperator, i))), uint8(IRegistryCoordinator.OperatorStatus.DEREGISTERED)); @@ -312,11 +291,7 @@ contract EjectionManagerUnitTests is MockAVSDeployer { } cheats.prank(registryCoordinatorOwner); - uint32[] memory ejectedOperatorsForQuorum = ejectionManager.ejectOperators(operatorIds); - - for(uint8 i = 0; i < numQuorums; i++) { - assertEq(ejectedOperatorsForQuorum[i], operatorsToEject); - } + ejectionManager.ejectOperators(operatorIds); for(uint8 i = 0; i < operatorsToEject; i++) { assertEq(uint8(registryCoordinator.getOperatorStatus(_incrementAddress(defaultOperator, i))), uint8(IRegistryCoordinator.OperatorStatus.DEREGISTERED)); @@ -345,8 +320,6 @@ contract EjectionManagerUnitTests is MockAVSDeployer { } for(uint8 i = 0; i < numQuorums; i++) { - //cheats.expectEmit(true, true, true, true, address(ejectionManager)); - //emit FailedOperatorEjection(operatorIds[i][0], i, abi.encodePacked("revert: RegistryCoordinator._deregisterOperator: operator is not registered")); for(uint8 j = 1; j < operatorsToEject; j++) { cheats.expectEmit(true, true, true, true, address(ejectionManager)); emit OperatorEjected(operatorIds[i][j], i); @@ -354,11 +327,7 @@ contract EjectionManagerUnitTests is MockAVSDeployer { } cheats.prank(ejector); - uint32[] memory ejectedOperatorsForQuorum = ejectionManager.ejectOperators(operatorIds); - - for(uint8 i = 0; i < numQuorums; i++) { - assertEq(ejectedOperatorsForQuorum[i], operatorsToEject - 1); - } + ejectionManager.ejectOperators(operatorIds); for(uint8 i = 0; i < operatorsToEject; i++) { assertEq(uint8(registryCoordinator.getOperatorStatus(_incrementAddress(defaultOperator, i))), uint8(IRegistryCoordinator.OperatorStatus.DEREGISTERED)); diff --git a/test/unit/RegistryCoordinatorUnit.t.sol b/test/unit/RegistryCoordinatorUnit.t.sol index b906b8be..c262b0da 100644 --- a/test/unit/RegistryCoordinatorUnit.t.sol +++ b/test/unit/RegistryCoordinatorUnit.t.sol @@ -988,6 +988,62 @@ contract RegistryCoordinatorUnitTests_DeregisterOperator_EjectOperator is Regist registryCoordinator._deregisterOperatorExternal(defaultOperator, incorrectQuorum); } + function test_reregisterOperator_revert_reregistrationDelay() public { + uint256 reregistrationDelay = 1 days; + cheats.warp(block.timestamp + reregistrationDelay); + cheats.prank(registryCoordinatorOwner); + registryCoordinator.setEjectionCooldown(reregistrationDelay); + + ISignatureUtils.SignatureWithSaltAndExpiry memory emptySig; + uint32 registrationBlockNumber = 100; + uint32 reregistrationBlockNumber = 200; + + bytes memory quorumNumbers = new bytes(1); + quorumNumbers[0] = bytes1(defaultQuorumNumber); + + _setOperatorWeight(defaultOperator, uint8(quorumNumbers[0]), defaultStake); + + cheats.prank(defaultOperator); + cheats.roll(registrationBlockNumber); + registryCoordinator.registerOperator(quorumNumbers, defaultSocket, pubkeyRegistrationParams, emptySig); + + cheats.prank(ejector); + registryCoordinator.ejectOperator(defaultOperator, quorumNumbers); + + cheats.prank(defaultOperator); + cheats.roll(reregistrationBlockNumber); + cheats.expectRevert("RegistryCoordinator._registerOperator: operator cannot reregister yet"); + registryCoordinator.registerOperator(quorumNumbers, defaultSocket, pubkeyRegistrationParams, emptySig); + } + + function test_reregisterOperator_reregistrationDelay() public { + uint256 reregistrationDelay = 1 days; + cheats.warp(block.timestamp + reregistrationDelay); + cheats.prank(registryCoordinatorOwner); + registryCoordinator.setEjectionCooldown(reregistrationDelay); + + ISignatureUtils.SignatureWithSaltAndExpiry memory emptySig; + uint32 registrationBlockNumber = 100; + uint32 reregistrationBlockNumber = 200; + + bytes memory quorumNumbers = new bytes(1); + quorumNumbers[0] = bytes1(defaultQuorumNumber); + + _setOperatorWeight(defaultOperator, uint8(quorumNumbers[0]), defaultStake); + + cheats.prank(defaultOperator); + cheats.roll(registrationBlockNumber); + registryCoordinator.registerOperator(quorumNumbers, defaultSocket, pubkeyRegistrationParams, emptySig); + + cheats.prank(ejector); + registryCoordinator.ejectOperator(defaultOperator, quorumNumbers); + + cheats.prank(defaultOperator); + cheats.roll(reregistrationBlockNumber); + cheats.warp(block.timestamp + reregistrationDelay + 1); + registryCoordinator.registerOperator(quorumNumbers, defaultSocket, pubkeyRegistrationParams, emptySig); + } + // note: this is not possible to test, because there is no route to getting the operator registered for nonexistent quorums // function test_deregisterOperatorExternal_revert_nonexistentQuorums() public {