Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 10 additions & 4 deletions docs/core/AllocationManager.md
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,9 @@ This pattern is especially useful for complex contracts that require a comprehen
* `DEALLOCATION_DELAY`: The delay in blocks before deallocations take effect.
* Mainnet: `100800 blocks` (14 days).
* Testnet: `50 blocks` (10 minutes).
* `SLASHER_CONFIGURATION_DELAY`: The delay in blocks before slasher changes take effect.
* Currently set to the same value as `ALLOCATION_CONFIGURATION_DELAY`.
* Can be changed independently in future upgrades.

---

Expand Down Expand Up @@ -1073,13 +1076,14 @@ Note that when an operator registers, registration will FAIL if the call to `IAV
function updateSlasher(OperatorSet memory operatorSet, address slasher) external;
```

Updates the slasher that can call [`slashOperator`](#slashoperator) on behalf of an operatorSet. The slasher will become active after `DEALLOCATION_DELAY` blocks. Only 1 address can slash an operatorSet.
Updates the slasher that can call [`slashOperator`](#slashoperator) on behalf of an operatorSet. The slasher will become active after `SLASHER_CONFIGURATION_DELAY` blocks. Only 1 address can slash an operatorSet.

Prior to `v1.9.0`, the address that could set the slasher was settable via the `PermissionController`, which allowed any number of admins/appointees to slash an operatorSet on behalf of an AVS. OperatorSets created prior to `v1.9.0` will have their slasher migrated to the `AllocationManager` - see [`migrateSlashers`](#migrateslashers) for more information.
Prior to `v1.9.0`, the address that could set the slasher was settable via the `PermissionController`, which allowed any number of admins/appointees to slash an operatorSet on behalf of an AVS. OperatorSets created prior to `v1.9.0` will have their slasher migrated to the `AllocationManager` - see [`migrateSlashers`](#migrateslashers) for more information.

*Effects*:
* If the proposed slasher is the same as the currently pending slasher and the pending change hasn't taken effect yet, this is a no-op (delay countdown is not restarted).
* Sets the operatorSet's `pendingSlasher` to the proposed `slasher`, and save the `effectBlock` at which the `pendingSlasher` can be activated
* `effectBlock = uint32(block.number) + ALLOCATION_CONFIGURATION_DELAY + 1`
* `effectBlock = uint32(block.number) + SLASHER_CONFIGURATION_DELAY + 1`
* If the operatorSet has a `pendingDelay`, and if the `effectBlock` has passed, sets the operatorSet's slasher to the pendingSlasher
* Emits an `SlasherUpdated` event

Expand Down Expand Up @@ -1110,7 +1114,9 @@ function migrateSlashers(
) external;
```

Migrates a slasher from the `PermissionController` to the `AllocationManager`. **This function is useful for operatorSets that have been created prior to `v1.9.0`**. Customers *will not* have to migrate a slasher themselves; migration will be done on behalf of all operatorSets upon completion of the `v1.9.0` upgrade.
Migrates a slasher from the `PermissionController` to the `AllocationManager`. **This function is useful for operatorSets that have been created prior to `v1.9.0`**. Customers *will not* have to migrate a slasher themselves; migration will be done on behalf of all operatorSets upon completion of the `v1.9.0` upgrade.

**Gas Warning**: This function can be expensive for AVSs with many slashing appointees. The `PermissionController.getAppointees()` call enumerates the full appointee set (using `EnumerableSet.values()`), which has O(n) gas cost. Large appointee sets may cause this function to exceed block gas limits. Consider batching operator sets if needed.

Only 1 slasher can be slash an operatorSet on behalf of an AVS; however, multiple addresses may have had the ability to slash an operatorSet via the previous `PermissionController`-based access control. Because of this mismatch, slashers are migrated based on the following criteria:
1. If there are no slashers set in the `PermissionController` OR the slasher set is the 0 address, set the slasher to the AVS
Expand Down
35 changes: 33 additions & 2 deletions pkg/bindings/AllocationManager/binding.go

Large diffs are not rendered by default.

33 changes: 32 additions & 1 deletion pkg/bindings/AllocationManagerStorage/binding.go

Large diffs are not rendered by default.

35 changes: 33 additions & 2 deletions pkg/bindings/AllocationManagerView/binding.go

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion pkg/bindings/CrossChainRegistry/binding.go

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion pkg/bindings/DelegationManager/binding.go

Large diffs are not rendered by default.

33 changes: 32 additions & 1 deletion pkg/bindings/IAllocationManager/binding.go

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion pkg/bindings/KeyRegistrar/binding.go

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion pkg/bindings/RewardsCoordinator/binding.go

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion pkg/bindings/StrategyManager/binding.go

Large diffs are not rendered by default.

8 changes: 8 additions & 0 deletions script/releases/TestUtils.sol
Original file line number Diff line number Diff line change
Expand Up @@ -606,6 +606,10 @@ library TestUtils {
allocationManagerView.ALLOCATION_CONFIGURATION_DELAY() == Env.ALLOCATION_CONFIGURATION_DELAY(),
"allocationManagerView ALLOCATION_CONFIGURATION_DELAY incorrect"
);
assertTrue(
allocationManagerView.SLASHER_CONFIGURATION_DELAY() == Env.ALLOCATION_CONFIGURATION_DELAY(),
"allocationManagerView SLASHER_CONFIGURATION_DELAY incorrect"
);
}

function validateAllocationManagerImmutables(
Expand Down Expand Up @@ -637,6 +641,10 @@ library TestUtils {
allocationManager.ALLOCATION_CONFIGURATION_DELAY() == Env.ALLOCATION_CONFIGURATION_DELAY(),
"allocationManager ALLOCATION_CONFIGURATION_DELAY incorrect"
);
assertTrue(
allocationManager.SLASHER_CONFIGURATION_DELAY() == Env.ALLOCATION_CONFIGURATION_DELAY(),
"allocationManager SLASHER_CONFIGURATION_DELAY incorrect"
);
}

function validateAVSDirectoryImmutables(
Expand Down
9 changes: 8 additions & 1 deletion src/contracts/core/AllocationManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,8 @@ contract AllocationManager is
}

/// @inheritdoc IAllocationManagerActions
/// @dev WARNING: Gas cost is O(appointees) per operator set due to `PermissionController.getAppointees()` call.
/// May exceed block gas limit for AVSs with large appointee sets. Consider batching operator sets if needed.
function migrateSlashers(
OperatorSet[] memory operatorSets
) external {
Expand Down Expand Up @@ -753,14 +755,19 @@ contract AllocationManager is
params.slasher = params.pendingSlasher;
}

// No-op if proposing the same pending slasher that hasn't taken effect yet
if (slasher == params.pendingSlasher && block.number < params.effectBlock) {
return;
}

// Set the pending parameters
params.pendingSlasher = slasher;
if (instantEffectBlock) {
// Update slasher field immediately for storage consistency
params.slasher = slasher;
params.effectBlock = uint32(block.number);
} else {
params.effectBlock = uint32(block.number) + ALLOCATION_CONFIGURATION_DELAY + 1;
params.effectBlock = uint32(block.number) + SLASHER_CONFIGURATION_DELAY + 1;
}

_slashers[operatorSet.key()] = params;
Expand Down
2 changes: 2 additions & 0 deletions src/contracts/core/AllocationManagerView.sol
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,7 @@ contract AllocationManagerView layout at 151 is IAllocationManagerView, Allocati
}

/// @inheritdoc IAllocationManagerView
/// @dev Returns (address(0), 0) for non-existent operator sets or when no pending slasher change exists.
function getPendingSlasher(
OperatorSet memory operatorSet
) external view returns (address, uint32) {
Expand All @@ -521,6 +522,7 @@ contract AllocationManagerView layout at 151 is IAllocationManagerView, Allocati
}

/// @inheritdoc IAllocationManagerView
/// @dev Returns address(0) for non-existent operator sets.
function getSlasher(
OperatorSet memory operatorSet
) public view returns (address) {
Expand Down
7 changes: 6 additions & 1 deletion src/contracts/core/storage/AllocationManagerStorage.sol
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,13 @@ abstract contract AllocationManagerStorage is IAllocationManagerStorage {
/// In this window, deallocations still remain slashable by the operatorSet they were allocated to.
uint32 public immutable DEALLOCATION_DELAY;

/// @notice Delay before alloaction delay modifications take effect.
/// @notice Delay before allocation delay modifications take effect.
uint32 public immutable ALLOCATION_CONFIGURATION_DELAY;

/// @notice Delay before slasher changes take effect.
/// @dev Currently set to the same value as ALLOCATION_CONFIGURATION_DELAY.
uint32 public immutable SLASHER_CONFIGURATION_DELAY;
Comment on lines +43 to +48
Copy link
Member

Choose a reason for hiding this comment

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

Note sure I agree with their solution, strange to have to two constants with the same value but different names, will lead to confusion down the road.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Mainly just due to separation of concerns - it's not clear that the ALLOCATION_CONFIGURATION_DELAY var should also be used for the slasher delay


// Mutatables

/// AVS => OPERATOR SET
Expand Down Expand Up @@ -131,6 +135,7 @@ abstract contract AllocationManagerStorage is IAllocationManagerStorage {
eigenStrategy = _eigenStrategy;
DEALLOCATION_DELAY = _DEALLOCATION_DELAY;
ALLOCATION_CONFIGURATION_DELAY = _ALLOCATION_CONFIGURATION_DELAY;
SLASHER_CONFIGURATION_DELAY = _ALLOCATION_CONFIGURATION_DELAY;
}

/// @dev This empty reserved space is put in place to allow future versions to add new
Expand Down
15 changes: 11 additions & 4 deletions src/contracts/interfaces/IAllocationManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,10 @@ interface IAllocationManagerStorage is IAllocationManagerEvents {

/// @notice Delay before alloaction delay modifications take effect.
function ALLOCATION_CONFIGURATION_DELAY() external view returns (uint32);

/// @notice Delay before slasher changes take effect.
/// @dev Currently set to the same value as ALLOCATION_CONFIGURATION_DELAY.
function SLASHER_CONFIGURATION_DELAY() external view returns (uint32);
}

interface IAllocationManagerActions is IAllocationManagerErrors, IAllocationManagerEvents, IAllocationManagerStorage {
Expand Down Expand Up @@ -469,7 +473,8 @@ interface IAllocationManagerActions is IAllocationManagerErrors, IAllocationMana
/// @notice Allows an AVS to update the slasher for an operator set
/// @param operatorSet the operator set to update the slasher for
/// @param slasher the new slasher
/// @dev The new slasher will take effect in DEALLOCATION_DELAY blocks
/// @dev The new slasher will take effect after `SLASHER_CONFIGURATION_DELAY` blocks.
/// @dev No-op if the proposed slasher is already pending and hasn't taken effect yet (delay countdown is not restarted).
/// @dev The slasher can only be updated if it has already been set. The slasher is set either on operatorSet creation or,
/// for operatorSets created prior to v1.9.0, via `migrateSlashers`
/// @dev Reverts for:
Expand All @@ -493,6 +498,8 @@ interface IAllocationManagerActions is IAllocationManagerErrors, IAllocationMana
/// @dev This function does not revert to allow for simpler offchain calling. It will no-op if:
/// - The operator set does not exist
/// - The slasher has already been set, either via migration or creation of the operatorSet
/// @dev WARNING: Gas cost is O(appointees) per operator set due to `PermissionController.getAppointees()` call.
/// May exceed block gas limit for AVSs with large appointee sets. Consider batching operator sets if needed.
function migrateSlashers(
OperatorSet[] memory operatorSets
) external;
Expand Down Expand Up @@ -722,16 +729,16 @@ interface IAllocationManagerView is IAllocationManagerErrors, IAllocationManager

/// @notice Returns the address that can slash a given operator set.
/// @param operatorSet The operator set to query.
/// @return The address that can slash the operator set.
/// @return The address that can slash the operator set. Returns `address(0)` if the operator set doesn't exist.
/// @dev If there is a pending slasher that can be applied after the `effectBlock`, the pending slasher will be returned.
function getSlasher(
OperatorSet memory operatorSet
) external view returns (address);

/// @notice Returns pending slasher for a given operator set.
/// @param operatorSet The operator set to query.
/// @return pendingSlasher The pending slasher for the operator set. This value is 0 if there is no pending slasher.
/// @return effectBlock The block at which the pending slasher will take effect. This value is 0 if there is no pending slasher.
/// @return pendingSlasher The pending slasher for the operator set. Returns `address(0)` if there is no pending slasher or the operator set doesn't exist.
/// @return effectBlock The block at which the pending slasher will take effect. Returns `0` if there is no pending slasher or the operator set doesn't exist.
function getPendingSlasher(
OperatorSet memory operatorSet
) external view returns (address pendingSlasher, uint32 effectBlock);
Expand Down
92 changes: 92 additions & 0 deletions src/test/unit/AllocationManagerUnit.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4357,6 +4357,98 @@ contract AllocationManagerUnitTests_updateSlasher is AllocationManagerUnitTests,
assertEq(params.pendingSlasher, slasher, "pendingSlasher should also be set");
assertEq(params.effectBlock, uint32(block.number), "effectBlock should be current block");
}

/// -----------------------------------------------------------------------
/// Fix 3: Re-proposing same slasher is a no-op
/// -----------------------------------------------------------------------

/// @notice Test that re-proposing the same pending slasher is a no-op (effectBlock doesn't restart)
function test_updateSlasher_reProposeSamePendingSlasher_isNoOp(Randomness r) public rand(r) {
address slasher = r.Address();

// First proposal
cheats.prank(defaultAVS);
allocationManager.updateSlasher(defaultOperatorSet, slasher);

(, uint32 originalEffectBlock) = allocationManager.getPendingSlasher(defaultOperatorSet);

// Roll forward but stay before effectBlock
cheats.roll(block.number + 10);

// Re-propose the same slasher - should be a no-op
cheats.prank(defaultAVS);
allocationManager.updateSlasher(defaultOperatorSet, slasher);

// effectBlock should NOT have changed
(, uint32 newEffectBlock) = allocationManager.getPendingSlasher(defaultOperatorSet);
assertEq(newEffectBlock, originalEffectBlock, "effectBlock should not restart when re-proposing same slasher");
}

/// @notice Test that proposing a different slasher does restart the delay
function test_updateSlasher_proposeDifferentSlasher_restartsDelay(Randomness r) public rand(r) {
address firstSlasher = r.Address();
address secondSlasher = r.Address();
cheats.assume(firstSlasher != secondSlasher);

// First proposal
cheats.prank(defaultAVS);
allocationManager.updateSlasher(defaultOperatorSet, firstSlasher);

(, uint32 originalEffectBlock) = allocationManager.getPendingSlasher(defaultOperatorSet);

// Roll forward but stay before effectBlock
cheats.roll(block.number + 10);

// Propose a different slasher - should restart the delay
cheats.prank(defaultAVS);
allocationManager.updateSlasher(defaultOperatorSet, secondSlasher);

// effectBlock should have changed
(, uint32 newEffectBlock) = allocationManager.getPendingSlasher(defaultOperatorSet);
assertTrue(newEffectBlock > originalEffectBlock, "effectBlock should restart when proposing different slasher");
}

/// -----------------------------------------------------------------------
/// Fix 4: getSlasher/getPendingSlasher return zero for non-existent operator sets
/// -----------------------------------------------------------------------

/// @notice Test that getSlasher returns address(0) for non-existent operator set
function test_getSlasher_nonExistentOperatorSet_returnsZero() public {
OperatorSet memory nonExistent = OperatorSet({avs: address(0x999), id: 999});
address slasher = allocationManager.getSlasher(nonExistent);
assertEq(slasher, address(0), "should return address(0) for non-existent operator set");
}

/// @notice Test that getPendingSlasher returns (address(0), 0) for non-existent operator set
function test_getPendingSlasher_nonExistentOperatorSet_returnsZero() public {
OperatorSet memory nonExistent = OperatorSet({avs: address(0x999), id: 999});
(address pendingSlasher, uint32 effectBlock) = allocationManager.getPendingSlasher(nonExistent);
assertEq(pendingSlasher, address(0), "should return address(0) for non-existent operator set");
assertEq(effectBlock, 0, "should return 0 for non-existent operator set");
}

/// -----------------------------------------------------------------------
/// Fix 5: SLASHER_CONFIGURATION_DELAY constant
/// -----------------------------------------------------------------------

/// @notice Test that SLASHER_CONFIGURATION_DELAY constant exists and equals ALLOCATION_CONFIGURATION_DELAY
function test_SLASHER_CONFIGURATION_DELAY_exists() public view {
uint32 slasherDelay = allocationManager.SLASHER_CONFIGURATION_DELAY();
uint32 allocationDelay = allocationManager.ALLOCATION_CONFIGURATION_DELAY();
assertEq(slasherDelay, allocationDelay, "SLASHER_CONFIGURATION_DELAY should equal ALLOCATION_CONFIGURATION_DELAY initially");
}

/// @notice Test that updateSlasher uses SLASHER_CONFIGURATION_DELAY for effectBlock calculation
function test_updateSlasher_usesSlasherConfigurationDelay(Randomness r) public rand(r) {
address slasher = r.Address();

cheats.prank(defaultAVS);
allocationManager.updateSlasher(defaultOperatorSet, slasher);

(, uint32 effectBlock) = allocationManager.getPendingSlasher(defaultOperatorSet);
uint32 expected = uint32(block.number) + allocationManager.SLASHER_CONFIGURATION_DELAY() + 1;
assertEq(effectBlock, expected, "should use SLASHER_CONFIGURATION_DELAY for effectBlock calculation");
}
}

contract AllocationManagerUnitTests_migrateSlashers is AllocationManagerUnitTests {
Expand Down