chore: style updates (#1416)

**Motivation:**

We want consistent style across the repo

**Modifications:**

1. Use modifier to gate access control
2. Don't use burn solo anywhere
3. Don't use virtual functions if not needed

**Result:**

Cleaner code

Author: ypatil12

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;
     }
 
     /**

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

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 =

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

src/contracts/core/StrategyManager.sol

@@ -171,15 +171,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
@@ -194,9 +197,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
@@ -206,7 +210,7 @@ contract StrategyManager is
             emit BurnOrRedistributableSharesDecreased(operatorSet, slashId, strategy, sharesToRemove);
         }
 
-        return sharesToRemove;
+        return amountOut;
     }
 
     /// @inheritdoc IStrategyManager

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

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,

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