From 33c4fc4d7f4554061e9da379568dce1b3b9e377a Mon Sep 17 00:00:00 2001 From: Yash Patil <40046473+ypatil12@users.noreply.github.com> Date: Sat, 24 May 2025 14:13:03 -0400 Subject: [PATCH 1/4] chore: styling + update isOperatorRedistributable --- src/test/unit/AllocationManagerUnit.t.sol | 44 +++++++++++++++++++---- 1 file changed, 38 insertions(+), 6 deletions(-) diff --git a/src/test/unit/AllocationManagerUnit.t.sol b/src/test/unit/AllocationManagerUnit.t.sol index b7134bebba..3a9e4b1ebe 100644 --- a/src/test/unit/AllocationManagerUnit.t.sol +++ b/src/test/unit/AllocationManagerUnit.t.sol @@ -4395,20 +4395,32 @@ contract AllocationManagerUnitTests_isOperatorRedistributable is AllocationManag // Assert defaultOperator is not redistributable assertFalse(allocationManager.isOperatorRedistributable(defaultOperator), "operator should not be redistributable"); } +<<<<<<< HEAD +======= + +>>>>>>> 486caf4e (chore: styling + update isOperatorRedistributable) function testFuzz_registered_notAllocated(Randomness r) public rand(r) { // Create redistributing operatorSet OperatorSet memory redistributingOpSet = OperatorSet(defaultAVS, defaultOperatorSet.id + 1); address redistributionRecipient = r.Address(); _createRedistributingOperatorSet(redistributingOpSet, defaultStrategies, redistributionRecipient); +<<<<<<< HEAD +======= + +>>>>>>> 486caf4e (chore: styling + update isOperatorRedistributable) // Register operator to redistributing operatorSet _registerOperator(defaultOperator); cheats.prank(defaultOperator); uint32[] memory operatorSetIds = new uint32[](1); operatorSetIds[0] = redistributingOpSet.id; allocationManager.registerForOperatorSets(defaultOperator, RegisterParams(defaultAVS, operatorSetIds, "")); +<<<<<<< HEAD +======= + +>>>>>>> 486caf4e (chore: styling + update isOperatorRedistributable) // Operator should not be redistributable since it's not allocated assertTrue(allocationManager.isOperatorRedistributable(defaultOperator), "operator should be redistributable"); } @@ -4418,13 +4430,21 @@ contract AllocationManagerUnitTests_isOperatorRedistributable is AllocationManag OperatorSet memory redistributingOpSet = OperatorSet(defaultAVS, defaultOperatorSet.id + 1); address redistributionRecipient = r.Address(); _createRedistributingOperatorSet(redistributingOpSet, defaultStrategies, redistributionRecipient); +<<<<<<< HEAD +======= + +>>>>>>> 486caf4e (chore: styling + update isOperatorRedistributable) // Allocate to redistributing operatorSet AllocateParams[] memory allocateParams = _newAllocateParams(redistributingOpSet, 5e17); cheats.prank(defaultOperator); allocationManager.modifyAllocations(defaultOperator, allocateParams); cheats.roll(block.number + DEFAULT_OPERATOR_ALLOCATION_DELAY); +<<<<<<< HEAD +======= + +>>>>>>> 486caf4e (chore: styling + update isOperatorRedistributable) // Operator should not be redistributable since it's not slashable assertFalse(allocationManager.isOperatorRedistributable(defaultOperator), "operator should not be redistributable"); } @@ -4434,24 +4454,42 @@ contract AllocationManagerUnitTests_isOperatorRedistributable is AllocationManag OperatorSet memory redistributingOpSet = OperatorSet(defaultAVS, defaultOperatorSet.id + 1); address redistributionRecipient = r.Address(); _createRedistributingOperatorSet(redistributingOpSet, defaultStrategies, redistributionRecipient); +<<<<<<< HEAD +======= + +>>>>>>> 486caf4e (chore: styling + update isOperatorRedistributable) // Allocate to redistributing operatorSet AllocateParams[] memory allocateParams = _newAllocateParams(redistributingOpSet, 5e17); cheats.prank(defaultOperator); allocationManager.modifyAllocations(defaultOperator, allocateParams); cheats.roll(block.number + DEFAULT_OPERATOR_ALLOCATION_DELAY); +<<<<<<< HEAD +======= + +>>>>>>> 486caf4e (chore: styling + update isOperatorRedistributable) // Register operator to redistributing operatorSet _registerOperator(defaultOperator); cheats.prank(defaultOperator); uint32[] memory operatorSetIds = new uint32[](1); operatorSetIds[0] = redistributingOpSet.id; allocationManager.registerForOperatorSets(defaultOperator, RegisterParams(defaultAVS, operatorSetIds, "")); +<<<<<<< HEAD // Deregister operator from redistributing operatorSet cheats.prank(defaultOperator); allocationManager.deregisterFromOperatorSets(DeregisterParams(defaultOperator, defaultAVS, operatorSetIds)); +======= + + // Deregister operator from redistributing operatorSet + cheats.prank(defaultOperator); + allocationManager.deregisterFromOperatorSets( + DeregisterParams(defaultOperator, defaultAVS, operatorSetIds) + ); + +>>>>>>> 486caf4e (chore: styling + update isOperatorRedistributable) // Operator should be redistributable assertTrue(allocationManager.isOperatorRedistributable(defaultOperator), "operator should be redistributable"); @@ -4467,18 +4505,12 @@ contract AllocationManagerUnitTests_isOperatorRedistributable is AllocationManag OperatorSet memory redistributingOpSet = OperatorSet(defaultAVS, defaultOperatorSet.id + 1); address redistributionRecipient = r.Address(); _createRedistributingOperatorSet(redistributingOpSet, defaultStrategies, redistributionRecipient); - // Allocate to redistributing operatorSet AllocateParams[] memory allocateParams = _newAllocateParams(redistributingOpSet, 5e17); cheats.prank(defaultOperator); allocationManager.modifyAllocations(defaultOperator, allocateParams); cheats.roll(block.number + DEFAULT_OPERATOR_ALLOCATION_DELAY); - - // Register operator to redistributing operatorSet _registerOperator(defaultOperator); - cheats.prank(defaultOperator); - uint32[] memory operatorSetIds = new uint32[](1); - operatorSetIds[0] = redistributingOpSet.id; allocationManager.registerForOperatorSets(defaultOperator, RegisterParams(defaultAVS, operatorSetIds, "")); // Deallocate from redistributing operatorSet From 2483340e10fe1c397cc611cc8028af8245137a6c Mon Sep 17 00:00:00 2001 From: Yash Patil <40046473+ypatil12@users.noreply.github.com> Date: Wed, 28 May 2025 17:52:57 -0400 Subject: [PATCH 2/4] chore: update for style --- src/contracts/core/DelegationManager.sol | 10 ++--- src/contracts/core/SlashEscrow.sol | 4 +- src/contracts/core/SlashEscrowFactory.sol | 15 +++++-- .../core/SlashEscrowFactoryStorage.sol | 2 +- .../interfaces/IDelegationManager.sol | 4 +- src/test/unit/AllocationManagerUnit.t.sol | 44 +++---------------- 6 files changed, 27 insertions(+), 52 deletions(-) diff --git a/src/contracts/core/DelegationManager.sol b/src/contracts/core/DelegationManager.sol index effb47b7f3..54e3f5fa7b 100644 --- a/src/contracts/core/DelegationManager.sol +++ b/src/contracts/core/DelegationManager.sol @@ -288,7 +288,7 @@ contract DelegationManager is IStrategy strategy, uint64 prevMaxMagnitude, uint64 newMaxMagnitude - ) external onlyAllocationManager nonReentrant returns (uint256 totalDepositSharesToBurn) { + ) external onlyAllocationManager nonReentrant returns (uint256 totalDepositSharesToSlash) { uint256 operatorSharesSlashed = SlashingLib.calcSlashedAmount({ operatorShares: operatorShares[operator][strategy], prevMaxMagnitude: prevMaxMagnitude, @@ -304,7 +304,7 @@ contract DelegationManager is // Calculate the total deposit shares to burn - slashed operator shares plus still-slashable // shares sitting in the withdrawal queue. - totalDepositSharesToBurn = operatorSharesSlashed + scaledSharesSlashedFromQueue; + totalDepositSharesToSlash = operatorSharesSlashed + scaledSharesSlashedFromQueue; // Remove shares from operator _decreaseDelegation({ @@ -315,13 +315,13 @@ contract DelegationManager is }); // Emit event for operator shares being slashed - emit OperatorSharesSlashed(operator, strategy, totalDepositSharesToBurn); + emit OperatorSharesSlashed(operator, strategy, totalDepositSharesToSlash); _getShareManager(strategy).increaseBurnOrRedistributableShares( - operatorSet, slashId, strategy, totalDepositSharesToBurn + operatorSet, slashId, strategy, totalDepositSharesToSlash ); - return totalDepositSharesToBurn; + return totalDepositSharesToSlash; } /** diff --git a/src/contracts/core/SlashEscrow.sol b/src/contracts/core/SlashEscrow.sol index ca4474bab2..e85877228e 100644 --- a/src/contracts/core/SlashEscrow.sol +++ b/src/contracts/core/SlashEscrow.sol @@ -18,7 +18,7 @@ contract SlashEscrow is ISlashEscrow { uint256 slashId, address recipient, IStrategy strategy - ) external virtual { + ) external { // Assert that the deployment parameters are valid by validating against the address of this proxy. require( verifyDeploymentParameters(slashEscrowFactory, slashEscrowImplementation, operatorSet, slashId), @@ -39,7 +39,7 @@ contract SlashEscrow is ISlashEscrow { ISlashEscrow slashEscrowImplementation, OperatorSet calldata operatorSet, uint256 slashId - ) public view virtual returns (bool) { + ) public view returns (bool) { return ClonesUpgradeable.predictDeterministicAddress( address(slashEscrowImplementation), keccak256(abi.encodePacked(operatorSet.key(), slashId)), diff --git a/src/contracts/core/SlashEscrowFactory.sol b/src/contracts/core/SlashEscrowFactory.sol index 089073c9a3..fc273c89fa 100644 --- a/src/contracts/core/SlashEscrowFactory.sol +++ b/src/contracts/core/SlashEscrowFactory.sol @@ -14,12 +14,17 @@ contract SlashEscrowFactory is Initializable, SlashEscrowFactoryStorage, Ownable using OperatorSetLib for *; using EnumerableSet for *; using ClonesUpgradeable for address; + + modifier onlyStrategyManager() { + require(msg.sender == address(strategyManager), OnlyStrategyManager()); + _; + } + /** * * INITIALIZATION * */ - constructor( IAllocationManager _allocationManager, IStrategyManager _strategyManager, @@ -52,9 +57,11 @@ contract SlashEscrowFactory is Initializable, SlashEscrowFactoryStorage, Ownable */ /// @inheritdoc ISlashEscrowFactory - function initiateSlashEscrow(OperatorSet calldata operatorSet, uint256 slashId, IStrategy strategy) external { - require(msg.sender == address(strategyManager), OnlyStrategyManager()); - + function initiateSlashEscrow( + OperatorSet calldata operatorSet, + uint256 slashId, + IStrategy strategy + ) external onlyStrategyManager { // Create storage pointers for readability. EnumerableSet.UintSet storage pendingSlashIds = _pendingSlashIds[operatorSet.key()]; EnumerableSet.AddressSet storage pendingStrategiesForSlashId = diff --git a/src/contracts/core/SlashEscrowFactoryStorage.sol b/src/contracts/core/SlashEscrowFactoryStorage.sol index c5bfa2b99c..947dff988c 100644 --- a/src/contracts/core/SlashEscrowFactoryStorage.sol +++ b/src/contracts/core/SlashEscrowFactoryStorage.sol @@ -51,7 +51,7 @@ abstract contract SlashEscrowFactoryStorage is ISlashEscrowFactory { /// @dev Returns the global escrow delay for all strategies. uint32 internal _globalEscrowDelayBlocks; - /// @dev Returns the operator set delay for a given strategy. + /// @dev Returns the escrow delay for a given strategy. mapping(address strategy => uint32 delay) internal _strategyEscrowDelayBlocks; // Constructor diff --git a/src/contracts/interfaces/IDelegationManager.sol b/src/contracts/interfaces/IDelegationManager.sol index fc543d4bfa..fcbc9a4afd 100644 --- a/src/contracts/interfaces/IDelegationManager.sol +++ b/src/contracts/interfaces/IDelegationManager.sol @@ -365,7 +365,7 @@ interface IDelegationManager is ISignatureUtilsMixin, IDelegationManagerErrors, * @dev Callable only by the AllocationManager. * @dev Note: Assumes `prevMaxMagnitude <= newMaxMagnitude`. This invariant is maintained in * the AllocationManager. - * @return totalDepositSharesToBurn The total deposit shares to burn. + * @return totalDepositSharesToSlash The total deposit shares to burn or redistribute. */ function slashOperatorShares( address operator, @@ -374,7 +374,7 @@ interface IDelegationManager is ISignatureUtilsMixin, IDelegationManagerErrors, IStrategy strategy, uint64 prevMaxMagnitude, uint64 newMaxMagnitude - ) external returns (uint256 totalDepositSharesToBurn); + ) external returns (uint256 totalDepositSharesToSlash); /** * diff --git a/src/test/unit/AllocationManagerUnit.t.sol b/src/test/unit/AllocationManagerUnit.t.sol index 3a9e4b1ebe..b7134bebba 100644 --- a/src/test/unit/AllocationManagerUnit.t.sol +++ b/src/test/unit/AllocationManagerUnit.t.sol @@ -4395,32 +4395,20 @@ contract AllocationManagerUnitTests_isOperatorRedistributable is AllocationManag // Assert defaultOperator is not redistributable assertFalse(allocationManager.isOperatorRedistributable(defaultOperator), "operator should not be redistributable"); } -<<<<<<< HEAD -======= - ->>>>>>> 486caf4e (chore: styling + update isOperatorRedistributable) function testFuzz_registered_notAllocated(Randomness r) public rand(r) { // Create redistributing operatorSet OperatorSet memory redistributingOpSet = OperatorSet(defaultAVS, defaultOperatorSet.id + 1); address redistributionRecipient = r.Address(); _createRedistributingOperatorSet(redistributingOpSet, defaultStrategies, redistributionRecipient); -<<<<<<< HEAD -======= - ->>>>>>> 486caf4e (chore: styling + update isOperatorRedistributable) // Register operator to redistributing operatorSet _registerOperator(defaultOperator); cheats.prank(defaultOperator); uint32[] memory operatorSetIds = new uint32[](1); operatorSetIds[0] = redistributingOpSet.id; allocationManager.registerForOperatorSets(defaultOperator, RegisterParams(defaultAVS, operatorSetIds, "")); -<<<<<<< HEAD -======= - ->>>>>>> 486caf4e (chore: styling + update isOperatorRedistributable) // Operator should not be redistributable since it's not allocated assertTrue(allocationManager.isOperatorRedistributable(defaultOperator), "operator should be redistributable"); } @@ -4430,21 +4418,13 @@ contract AllocationManagerUnitTests_isOperatorRedistributable is AllocationManag OperatorSet memory redistributingOpSet = OperatorSet(defaultAVS, defaultOperatorSet.id + 1); address redistributionRecipient = r.Address(); _createRedistributingOperatorSet(redistributingOpSet, defaultStrategies, redistributionRecipient); -<<<<<<< HEAD -======= - ->>>>>>> 486caf4e (chore: styling + update isOperatorRedistributable) // Allocate to redistributing operatorSet AllocateParams[] memory allocateParams = _newAllocateParams(redistributingOpSet, 5e17); cheats.prank(defaultOperator); allocationManager.modifyAllocations(defaultOperator, allocateParams); cheats.roll(block.number + DEFAULT_OPERATOR_ALLOCATION_DELAY); -<<<<<<< HEAD -======= - ->>>>>>> 486caf4e (chore: styling + update isOperatorRedistributable) // Operator should not be redistributable since it's not slashable assertFalse(allocationManager.isOperatorRedistributable(defaultOperator), "operator should not be redistributable"); } @@ -4454,42 +4434,24 @@ contract AllocationManagerUnitTests_isOperatorRedistributable is AllocationManag OperatorSet memory redistributingOpSet = OperatorSet(defaultAVS, defaultOperatorSet.id + 1); address redistributionRecipient = r.Address(); _createRedistributingOperatorSet(redistributingOpSet, defaultStrategies, redistributionRecipient); -<<<<<<< HEAD -======= - ->>>>>>> 486caf4e (chore: styling + update isOperatorRedistributable) // Allocate to redistributing operatorSet AllocateParams[] memory allocateParams = _newAllocateParams(redistributingOpSet, 5e17); cheats.prank(defaultOperator); allocationManager.modifyAllocations(defaultOperator, allocateParams); cheats.roll(block.number + DEFAULT_OPERATOR_ALLOCATION_DELAY); -<<<<<<< HEAD -======= - ->>>>>>> 486caf4e (chore: styling + update isOperatorRedistributable) // Register operator to redistributing operatorSet _registerOperator(defaultOperator); cheats.prank(defaultOperator); uint32[] memory operatorSetIds = new uint32[](1); operatorSetIds[0] = redistributingOpSet.id; allocationManager.registerForOperatorSets(defaultOperator, RegisterParams(defaultAVS, operatorSetIds, "")); -<<<<<<< HEAD // Deregister operator from redistributing operatorSet cheats.prank(defaultOperator); allocationManager.deregisterFromOperatorSets(DeregisterParams(defaultOperator, defaultAVS, operatorSetIds)); -======= - - // Deregister operator from redistributing operatorSet - cheats.prank(defaultOperator); - allocationManager.deregisterFromOperatorSets( - DeregisterParams(defaultOperator, defaultAVS, operatorSetIds) - ); - ->>>>>>> 486caf4e (chore: styling + update isOperatorRedistributable) // Operator should be redistributable assertTrue(allocationManager.isOperatorRedistributable(defaultOperator), "operator should be redistributable"); @@ -4505,12 +4467,18 @@ contract AllocationManagerUnitTests_isOperatorRedistributable is AllocationManag OperatorSet memory redistributingOpSet = OperatorSet(defaultAVS, defaultOperatorSet.id + 1); address redistributionRecipient = r.Address(); _createRedistributingOperatorSet(redistributingOpSet, defaultStrategies, redistributionRecipient); + // Allocate to redistributing operatorSet AllocateParams[] memory allocateParams = _newAllocateParams(redistributingOpSet, 5e17); cheats.prank(defaultOperator); allocationManager.modifyAllocations(defaultOperator, allocateParams); cheats.roll(block.number + DEFAULT_OPERATOR_ALLOCATION_DELAY); + + // Register operator to redistributing operatorSet _registerOperator(defaultOperator); + cheats.prank(defaultOperator); + uint32[] memory operatorSetIds = new uint32[](1); + operatorSetIds[0] = redistributingOpSet.id; allocationManager.registerForOperatorSets(defaultOperator, RegisterParams(defaultAVS, operatorSetIds, "")); // Deallocate from redistributing operatorSet From a08d80660b22f87ff35e7ce6924e167fe73b674a Mon Sep 17 00:00:00 2001 From: Yash Patil <40046473+ypatil12@users.noreply.github.com> Date: Wed, 28 May 2025 18:14:48 -0400 Subject: [PATCH 3/4] chore: update sm interface --- src/contracts/core/StrategyManager.sol | 12 ++++++++---- src/contracts/interfaces/IStrategyManager.sol | 8 ++++++-- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/contracts/core/StrategyManager.sol b/src/contracts/core/StrategyManager.sol index a44b8b7ddb..6af08a77e8 100644 --- a/src/contracts/core/StrategyManager.sol +++ b/src/contracts/core/StrategyManager.sol @@ -173,15 +173,18 @@ contract StrategyManager is function clearBurnOrRedistributableShares( OperatorSet calldata operatorSet, uint256 slashId - ) external nonReentrant { + ) external nonReentrant returns (uint256[] memory) { // Get the strategies to clear. address[] memory strategies = _burnOrRedistributableShares[operatorSet.key()][slashId].keys(); uint256 length = strategies.length; + uint256[] memory amounts = new uint256[](length); // Note: We don't need to iterate backwards since we're indexing into the `EnumerableMap` directly. for (uint256 i = 0; i < length; ++i) { - clearBurnOrRedistributableSharesByStrategy(operatorSet, slashId, IStrategy(strategies[i])); + amounts[i] = clearBurnOrRedistributableSharesByStrategy(operatorSet, slashId, IStrategy(strategies[i])); } + + return amounts; } /// @inheritdoc IStrategyManager @@ -196,9 +199,10 @@ contract StrategyManager is (, uint256 sharesToRemove) = burnOrRedistributableShares.tryGet(address(strategy)); burnOrRedistributableShares.remove(address(strategy)); + uint256 amountOut; if (sharesToRemove != 0) { // Withdraw the shares to the slash escrow. - IStrategy(strategy).withdraw({ + amountOut = IStrategy(strategy).withdraw({ recipient: address(slashEscrowFactory.getSlashEscrow(operatorSet, slashId)), token: IStrategy(strategy).underlyingToken(), amountShares: sharesToRemove @@ -208,7 +212,7 @@ contract StrategyManager is emit BurnOrRedistributableSharesDecreased(operatorSet, slashId, strategy, sharesToRemove); } - return sharesToRemove; + return amountOut; } /// @inheritdoc IStrategyManager diff --git a/src/contracts/interfaces/IStrategyManager.sol b/src/contracts/interfaces/IStrategyManager.sol index 15febfac2c..50a801f203 100644 --- a/src/contracts/interfaces/IStrategyManager.sol +++ b/src/contracts/interfaces/IStrategyManager.sol @@ -139,15 +139,19 @@ interface IStrategyManager is IStrategyManagerErrors, IStrategyManagerEvents, IS * @notice Removes burned shares from storage and transfers the underlying tokens for the slashId to the slash escrow. * @param operatorSet The operator set to burn shares in. * @param slashId The slash ID to burn shares in. + * @return The amounts of tokens transferred to the slash escrow for each strategy */ - function clearBurnOrRedistributableShares(OperatorSet calldata operatorSet, uint256 slashId) external; + function clearBurnOrRedistributableShares( + OperatorSet calldata operatorSet, + uint256 slashId + ) external returns (uint256[] memory); /** * @notice Removes a single strategy's shares from storage and transfers the underlying tokens for the slashId to the slash escrow. * @param operatorSet The operator set to burn shares in. * @param slashId The slash ID to burn shares in. * @param strategy The strategy to burn shares in. - * @return The amount of shares that were burned. + * @return The amount of tokens transferred to the slash escrow for the strategy. */ function clearBurnOrRedistributableSharesByStrategy( OperatorSet calldata operatorSet, From 475948a9a527c4a81cfd729fdb81aaa0c019a011 Mon Sep 17 00:00:00 2001 From: Yash Patil <40046473+ypatil12@users.noreply.github.com> Date: Wed, 28 May 2025 18:26:23 -0400 Subject: [PATCH 4/4] chore: fix test --- src/test/mocks/StrategyManagerMock.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/mocks/StrategyManagerMock.sol b/src/test/mocks/StrategyManagerMock.sol index 9684b7f9b2..8545762be9 100644 --- a/src/test/mocks/StrategyManagerMock.sol +++ b/src/test/mocks/StrategyManagerMock.sol @@ -110,7 +110,7 @@ contract StrategyManagerMock is Test { return (existingShares, addedShares); } - function clearBurnOrRedistributableShares(OperatorSet calldata operatorSet, uint slashId) external {} + function clearBurnOrRedistributableShares(OperatorSet calldata operatorSet, uint slashId) external returns (address[] memory) {} function clearBurnOrRedistributableSharesByStrategy(OperatorSet calldata operatorSet, uint slashId, IStrategy strategy) external