Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: reregistration delay #246

Merged
merged 9 commits into from
Apr 30, 2024
Merged

feat: reregistration delay #246

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
f5ba67a
feat: rereg delay
0x0aa0 Apr 26, 2024
a71d3b2
test: fix tests
0x0aa0 Apr 26, 2024
1dc136b
chore: natspec
0x0aa0 Apr 26, 2024
03d40e1
test: unit
0x0aa0 Apr 26, 2024
2f6d7a5
fix: grief
0x0aa0 Apr 27, 2024
b962174
refactor: ejection manager
0x0aa0 Apr 27, 2024
271aad4
fix: units
0x0aa0 Apr 27, 2024
0e69039
fix: name nit
0x0aa0 Apr 27, 2024
0bad4f3
docs: ejector dev tag
0x0aa0 Apr 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 13 additions & 20 deletions src/EjectionManager.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,23 +60,20 @@ 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 {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wdyt about refactoring the ejection logic

  1. That doesn't enforce the rate limit that is callable by the owner
  2. That is callable by the ejector and enforces the rate limit

This should clean up some of the branchy conditionals in this function

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);

uint256 amountEjectable = amountEjectableForQuorum(quorumNumber);
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);

Expand All @@ -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;
stevennevins marked this conversation as resolved.
Show resolved Hide resolved
++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;
}

/**
Expand Down
34 changes: 30 additions & 4 deletions src/RegistryCoordinator.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
){
Comment on lines +374 to +378
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so there's still some weird race condition where an operator can deregister from a subset of quorums to avoid being ejected for more, right?
I guess I can accept that, but it would certainly be nice to resolve if there's an easy solution.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps we can just take the submitted 'quorumNumbers' and actually pass on a combination ('and'ing) of them with the operator's current quorums in the subsequent call?

_deregisterOperator({
operator: operator,
quorumNumbers: quorumNumbers
});
}
}

/*******************************************************************************
Expand Down Expand Up @@ -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
*******************************************************************************/
Expand Down Expand Up @@ -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.
Expand Down
7 changes: 6 additions & 1 deletion src/RegistryCoordinatorStorage.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;
}
6 changes: 3 additions & 3 deletions src/interfaces/IEjectionManager.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions test/integration/User.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -208,6 +209,7 @@ contract User is Test {
expiry: expiry
});

vm.warp(block.timestamp + 1);
registryCoordinator.registerOperatorWithChurn({
quorumNumbers: allQuorums,
socket: NAME,
Expand Down
53 changes: 11 additions & 42 deletions test/unit/EjectionManagerUnit.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// SPDX-License-Identifier: BUSL-1.1

pragma solidity ^0.8.12;

import {EjectionManager} from "../../src/EjectionManager.sol";
Expand Down Expand Up @@ -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));
}
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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));
Expand All @@ -251,6 +232,8 @@ contract EjectionManagerUnitTests is MockAVSDeployer {

testEjectOperators_MultipleOperatorInsideRatelimit();

vm.warp(block.timestamp + 1);

_registerOperaters(operatorsToEject, stake);

vm.warp(block.timestamp + ratelimitWindow);
Expand All @@ -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));
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -345,20 +320,14 @@ 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);
}
}

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));
Expand Down
56 changes: 56 additions & 0 deletions test/unit/RegistryCoordinatorUnit.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down
Loading